Skip to content

Add EKS container host in ContainerCredentialsProvider , also validate the resolved endpoint for RelativeURI#4695

Merged
joviegas merged 2 commits into
masterfrom
joviegas/http_credential_provider
Nov 16, 2023
Merged

Add EKS container host in ContainerCredentialsProvider , also validate the resolved endpoint for RelativeURI#4695
joviegas merged 2 commits into
masterfrom
joviegas/http_credential_provider

Conversation

@joviegas

@joviegas joviegas commented Nov 13, 2023

Copy link
Copy Markdown
Contributor

Motivation and Context

  • Add support for EKS host and auth token file, EKS: IRSAv2 auth
  • Also validation is updated to validate the loopBacks even for Relative URI

Modifications

  • EKS hosts IPV4 and IPV6 added in the allow hosts
  • Validation of Endpoint is done as soon as final endpoint is resolved

Testing

  • Added new Test cases from specifications

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

License

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas joviegas requested a review from a team as a code owner November 13, 2023 23:54
@joviegas joviegas force-pushed the joviegas/http_credential_provider branch from b7ea4dd to ad6d6cb Compare November 14, 2023 02:42
Comment on lines +240 to +243
if (!Files.exists(path)) {
throw SdkClientException.create(
String.format("Failed to read authorization token from '%s': no such file or directory", filePath));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this check is necessary; it's also not atomic since the file could exist during this check but not exist when we try to run it. Can we just to Files.readAllBytes below and catch the exception for file not exists?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Specs mentioned

      "reason": "failed to read authorization token from '/full/path/to/token/file': no such file or directory"

Thus I had added this specific case.

But , I think we can let user use the sdkException.getCause() which will give NoSuchFileException. Updated the code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. I think without this change, we will still meet that requirement

@sonarqubecloud

sonarqubecloud Bot commented Nov 16, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

96.0% 96.0% Coverage
0.0% 0.0% Duplication

@joviegas joviegas merged commit dec9133 into master Nov 16, 2023
@joviegas joviegas deleted the joviegas/http_credential_provider branch May 15, 2024 20:31
aws-sdk-java-automation added a commit that referenced this pull request Jan 27, 2026
…d93dc17b1

Pull request: release <- staging/064f0d85-e0a4-4c4b-b942-fdfd93dc17b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants