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
Govcloud custom resource fix #6996
Govcloud custom resource fix #6996
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6996 +/- ##
==========================================
+ Coverage 88.43% 88.48% +0.05%
==========================================
Files 229 229
Lines 8420 8414 -6
==========================================
- Hits 7446 7445 -1
+ Misses 974 969 -5
Continue to review full report at Codecov.
|
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.
@jmb12686 great thanks for proposal! We definitely should sort this out, still please see my comments
…and china regions. Add utility function to obtain partition from region
I made the modifications requested. I also found 3 additional locations under the |
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.
Thank you @jmb12686 it looks great!
I've just proposed one improvement, which should allow us to also resolve partition in all cases via getEnvironment
and not try to assume it from region (which doesn't seem 100% bulletproof)
@medikoo I've updated the PR with the latest changes you requested. |
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.
Looks great! Thank you @jmb12686
I've run it against our integration tests, and all works as expected
What did you implement
Closes #6995 . Fix GovCloud Region ARN and Environment parsing utilities in the AWS Plugin for Custom Resources.
How can we verify it
Deploy a serverless function to
us-gov-west-1
orus-gov-east-1
that uses S3 Bucket events on an existing S3 bucket, for example:Todos
Useful Scripts
npm run test-ci
--> Run all validation checks on proposed changesnpm run lint-updated
--> Lint all the updated filesnpm run lint:fix
--> Automatically fix lint problems (if possible)npm run prettier-check-updated
--> Check if updated files adhere to Prettier confignpm run prettify-updated
--> Prettify all the updated filesIs this ready for review?: YES
Is it a breaking change?: NO