-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(apigateway): Token resolution should occur within the prepare context #3906
Conversation
…text stack.resolve() works only after the CDK app has been fully prepared. During the 'prepare' phase, it should instead resolve the token partially and within the local context. fixes #3705
Pull Request Checklist
|
Pull Request Checklist
|
Pull Request Checklist
|
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.
Missing unit test
preparing: false | ||
scope: options.scope, | ||
resolver: options.resolver, | ||
preparing: options.preparing || false |
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.
||
is problematic for boolean fields (albeit here it will work). Prefer options.preparing !== undefined ? options.preparing : false
.
Closed in favour of #4010 |
stack.resolve()
works only after the CDK app has been fully prepared.During the 'prepare' phase, it should instead resolve the token
partially and within the local context.
fixes #3705
DO NOT MERGE:
While this change fixes the error reported in #3705, I'm unable to test it since it now creates the cyclic reference error reported in #3000. For the same reason, tests are missing!
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license