Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contains Ignorecase Check for ProfileCredentials property keys read from ~/.aws/credentials file. #4164

Closed
wants to merge 3 commits into from

Conversation

lovababu
Copy link

@lovababu lovababu commented Jul 6, 2023

Motivation and Context

There is no guarantee that every developer machine having profile configured with lower case property keys.
For example in our case, we are using some python reusable script to setup ~/.aws/credentials file, it actually creates file as below

[default]
AWS_ACCESS_KEY_ID=   
AWS_SECRET_ACCESS_KEY=   
AWS_SESSION_TOKEN=

Due property keys are in upper case, ProfileCredentialsUtil.java @line 143 and 147 is failing due to case sensitive check with containsKey of properties map object.

Modifications

Storing profile credential data as TreeMap with String.CASE_INSENSITIVE_ORDER, it actually doesn't alter any property key-values, but while checking contains key, it considers case insensitivity.

Changes to file: software.amazon.awssdk.profiles.Profile.java#BuilderImpl.java @line 194.

Testing

Maven build passed locally.
Validated through sample java application with updated SDK changes.
Added UT to validate actual change.

Types of changes

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

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

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

@lovababu lovababu requested a review from a team as a code owner July 6, 2023 17:00
@lovababu lovababu changed the title Read and Store Profile property keys in lower case. Contains Ignorecase Check for ProfileCredentials property keys read from ~/.aws/credentials file. Jul 7, 2023
@lovababu lovababu force-pushed the master branch 2 times, most recently from d894aae to c3e97ed Compare July 18, 2023 07:04
@debora-ito debora-ito added the needs-review This issue or PR needs review from the team. label Aug 3, 2023
@debora-ito
Copy link
Member

For example in our case, we are using some python reusable script to setup ~/.aws/credentials file, it actually creates file as below

Which SDK do you use that works only with uppercase properties?

@debora-ito debora-ito removed the needs-review This issue or PR needs review from the team. label Aug 8, 2023
@debora-ito
Copy link
Member

@lovababu we did some research and found out that most AWS SDKs don't support uppercase properties.

Because of this, we won't be able to merge the PR in the Java SDK v2. If this gets implemented, we need to support it consistently across all the SDKs.

We still want to know if more people are interested in this feature, so I created a feature request in our AWS cross-SDK github repo here: #5689. Feel free to add a 👍 to show you are interested.

@debora-ito debora-ito closed this Aug 8, 2023
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