fix(synthetics): default execution role breaks in non aws partitions #12096
Conversation
|
|
github-actions
bot
added
the
@aws-cdk/aws-synthetics
Canary default execution role should be partition aware instead of hardcoding aws. Fixes #12094
|
Is there anything else that needs to be done to get this merged? |
|
@NetaNir could we get this fix merged? |
| @@ -309,6 +309,7 @@ export class Canary extends cdk.Resource { | |||
| * Returns a default role for the canary | |||
| */ | |||
| private createDefaultRole(prefix?: string): iam.IRole { | |||
| const { partition } = cdk.Stack.of(this); | |||
There was a problem hiding this comment.
stack.of returns the encompassing stack, not the partition. This should be:
cdk.Stack.of(this).partition
Was this change successfully deployed?
There was a problem hiding this comment.
This is using object destructuring to only select the partition from the returned stack object similar to this code. I've run the tests successfully but I've not deployed this exact CR to AWS Cloudformation. Could we merge without doing a deployment?
There was a problem hiding this comment.
How were you able to update the integ.canary.expected.json file without deploying it?
There was a problem hiding this comment.
I made the update based on similar changes I made to my Cloudformation project and deployed to AWS. I'm new to this Github repository so I'm not sure how to reference an unpublished version of AWS CDK from Github and deploy to my AWS account.
There was a problem hiding this comment.
This file needs to be updated by executing the integration test - not manually. The stack defined integ.canary.ts will be deployed and if successful the expectation file will be updated.
From the @aws-cdk/aws-synthetics folder, executing:
yarn integ integ.canary.js
Will deploy the stack and afterward update the file.
There was a problem hiding this comment.
Thanks for walking me through the process. I successfully deployed the changes to my AWS account and the integration test passed succesfully
gitpod /workspace/aws-cdk/packages/@aws-cdk/aws-synthetics $ yarn integ integ.canary.js
yarn run v1.22.5
$ cdk-integ integ.canary.js
Synthesizing integ.canary.js.
Selected stack: canary-one
Deploying integ.canary.js...
Deployment succeeded, updating snapshot.
Cleaning up.
Done in 129.24s.
There were no additional changes after the snapshot was updated.
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Canary default execution role should be partition aware instead of hardcoding aws.
Fixes #12094
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license