fix: allow setting InvokeRole to NONE or null #986
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## develop #986 +/- ##
===========================================
+ Coverage 94.71% 94.77% +0.05%
===========================================
Files 69 69
Lines 3160 3252 +92
Branches 606 635 +29
===========================================
+ Hits 2993 3082 +89
- Misses 85 88 +3
Partials 82 82
Continue to review full report at Codecov.
|
keetonian
commented
Jun 20, 2019
| method_invoke_role = method_auth_config.get('InvokeRole') | ||
| if not method_invoke_role and 'InvokeRole' in method_auth_config: | ||
| method_invoke_role = 'NONE' | ||
| api_invoke_role = api_auth_config.get('InvokeRole') |
There was a problem hiding this comment.
This section looks for two things:
- If there is an
InvokeRoleset, it gets the value - If
InvokeRoleis set to null, it sets it to 'NONE' - If
InvokeRoleis not set, it is passed as usual and is set toCallerCredentials.
praneetap
reviewed
Jun 21, 2019
praneetap
approved these changes
Jul 2, 2019
brettstack
suggested changes
Jul 16, 2019
| @@ -8,7 +8,7 @@ Resources: | |||
| StageName: Prod | |||
| Auth: | |||
| DefaultAuthorizer: AWS_IAM | |||
| InvokeRole: CALLER_CREDENTIALS | |||
| InvokeRole: CALLER_CREDENTIALS # default, set to NONE or null to not require credentials | |||
There was a problem hiding this comment.
This isn't accurate. IAM credentials are still required, they just aren't used as the invocation role for the Lambda Function. Setting to NONE will use the Lambda Function's configured role; setting to a different IAM role will invoke using that role.
brettstack
approved these changes
Jul 16, 2019
jlhood
approved these changes
Jul 26, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#923
Description of changes:
Allows invoke role override (removes the "credentials" field as discussed in #923)
Description of how you validated changes:
Verified that the transformed template removes the "credentials" section when InvokeRole is set to 'None' or null. Also deployed template to CFN to verify that "Invoke with caller credentials" and "execution role" were not set on the api method.
I also attempted to make it more explicit that
CALLER_CREDENTIALSis the default value, due to a conversation in the above issue.Checklist:
make prpassesexamples/2016-10-31By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.