fix(stepfunctions): proper s3 arn naming#22694
fix(stepfunctions): proper s3 arn naming#22694aaronatbissell wants to merge 1 commit intoaws:mainfrom
Conversation
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
TheRealAmazonKendra
left a comment
There was a problem hiding this comment.
This is a good catch, thanks for submitting this! I'm concerned, however, that the update wasn't caught by any integration tests. That means that either the fix didn't make a change to the template as we'd expect or that we are missing integration test coverage. I think this is a coverage issue. Please add an integration test that covers this fix.
Additionally, please make sure that your PR title confirms to the conventional commit standard (fix, feat, chore) and that it is written in a style that will reflect correctly in the change log (See Contributing Guide, Pull Requests).
|
Looks like this fix has actually already been merged. Closing this one. |
…egration tests for better coverage (#22699) Based on the recommendation from @TheRealAmazonKendra in #22694, here is a modification to the integration test for the fix to #22650 . This is in addition to PR #22692 which fixed the original problem, thanks to @kaizencc. I know this bug has already been closed, but I already had this written, so I just figured I would submit it anyways. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
When using an athena start-query-execution task, the CDK generates a default policy including some permissions for the S3 buckets used as an output location. This S3 bucket policy includes the S3 bucket ARN as a resource, but the auto-generated ARN includes region and account ID, and shouldn't. When trying to deploy, you end up with an error that looks like this:
I believe this bug was introduced in PR #22314
fixes #22650
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license