Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(stepfunctions): athena-start-query-execution tasks generate invalid S3 ARNs #22650

Closed
aaronatbissell opened this issue Oct 26, 2022 · 5 comments
Closed

(stepfunctions): athena-start-query-execution tasks generate invalid S3 ARNs #22650

aaronatbissell opened this issue Oct 26, 2022 · 5 comments
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@aaronatbissell
Copy link
Contributor

aaronatbissell commented Oct 26, 2022 •
edited
Loading

Describe the bug

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 as described here. When trying to deploy, you end up with an error that looks like this:

Error: The stack failed to deploy: UPDATE_ROLLBACK_COMPLETE: Resource arn:aws:s3:us-east-1:accountId:bucket/export can not contain region information.

I believe this bug was introduced in PR #22314

Expected Behavior

Auto-generated policy includes s3 bucket without region or account ID

Current Behavior

Deployment failure

Reproduction Steps

packages/@aws-cdk/aws-stepfunctions-tasks/test/athena/start-query-execution.test.ts
Test Name: "bucket arn is formatted as expected in generated policy"

This test is currently incorrect. It includes the region and account ID on the S3 bucket ARN

Possible Solution

Revisit PR #22314

Additional Information/Context

No response

CDK CLI Version

2.43.0

Framework Version

No response

Node.js Version

16.15.1

OS

macOS 12.6

Language

Typescript

Language Version

4.7.3

Other information

No response
@aaronatbissell aaronatbissell added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 26, 2022
@github-actions github-actions bot added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label Oct 26, 2022
@kaizencc kaizencc added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 28, 2022
@kaizencc kaizencc removed their assignment Oct 28, 2022
@kaizencc
Copy link
Contributor

kaizencc commented Oct 28, 2022 •
edited
Loading

Thanks for the report, @aaronatbissell. Do you have time to contribute a fix for this?

Edit: actually, i think i can grab this real quick

@aaronatbissell
Copy link
Contributor Author

OK - I can take a stab at it if you'd like. Just let me know!

@aaronatbissell aaronatbissell mentioned this issue Oct 28, 2022
Closed
4 tasks
@aaronatbissell
Copy link
Contributor Author

@kaizencc - I created the PR, let me know what you think!

@TheRealAmazonKendra TheRealAmazonKendra mentioned this issue Oct 28, 2022
Merged
4 tasks
@TheRealAmazonKendra
Copy link
Contributor

Looks like the fix for this was already merged.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@aaronatbissell aaronatbissell mentioned this issue Oct 29, 2022
Merged
4 tasks
mergify bot pushed a commit that referenced this issue Oct 31, 2022
@aaronatbissell
chore(stepfunctions-tasks): athenaStartQueryExecution addition to int…
98eb981 
…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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(stepfunctions): proper s3 arn naming aaronatbissell/aws-cdk
3 participants
@kaizencc @TheRealAmazonKendra @aaronatbissell