-
Notifications
You must be signed in to change notification settings - Fork 90
fix:Removing env var credentials provider as default. #1161
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
Conversation
SnapStart uses a different credentials provider so when it is hardcoded like this, the default will fail.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1161 +/- ##
============================================
- Coverage 71.32% 71.20% -0.13%
Complexity 535 535
============================================
Files 70 71 +1
Lines 2239 2240 +1
Branches 237 240 +3
============================================
- Hits 1597 1595 -2
- Misses 529 532 +3
Partials 113 113
☔ View full report in Codecov by Sentry. |
SnapStart uses a different credentials provider so when it is hardcoded like this, the default will fail.
// AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start | ||
// when using snap-start mode, the env var creds provider isn't used and causes a fatal error | ||
// fall back to the default provider chain if the mode is anything other than on-demand. | ||
if (System.getenv().get(Constants.AWS_LAMBDA_INITIALIZATION_TYPE) == "on-demand") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem: An equality operator (== or !=) is used to compare strings, which matches strings based on address.
Fix: Do content comparison using the equals() method to compare the values of the strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
...tools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SSMProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go!
Issue #, if available:
#1160
Description of changes:
SnapStart uses a different credentials provider to on-demand, so when it is hardcoded like this, the default will fail.
Lambda sets the env var
AWS_LAMBDA_INITIALIZATION_TYPE
to eithersnap-start
oron-demand
depending on the mode. We can use this to configure the DynamoDB correctly.Testing this is difficult, the configuration isn't an open attribute, so I don't think I can write a unit test easily (please let me know if I'm wrong). An integration test would be best.
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.