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.

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

ecs s3 environment's arn path is incorrect if partition is aws-gov #18429

Closed
ivyxjc opened this issue Jan 14, 2022 · 1 comment · Fixed by #18496
Closed

ecs s3 environment's arn path is incorrect if partition is aws-gov #18429

ivyxjc opened this issue Jan 14, 2022 · 1 comment · Fixed by #18496
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. p1

Comments

@ivyxjc
Copy link

ivyxjc commented Jan 14, 2022

What is the problem?

We use aws-gov cloud. And I try to bind s3 environment to ecs. The generated cloud formation of the config is still
arn:aws:s3:::.....

following is the sample code

const bucket = s3.Bucket.fromBucketArn(this, `${props.resourcePrefix}-Bucket`, `arn:aws-us-gov:s3:::${props.configBucket}`)
....

const container = taskDefinition.addContainer(`${props.resourcePrefix}-Web`, {
      containerName: `${props.resourcePrefix}-Web`,
      image: ecs.ContainerImage.fromEcrRepository(repo, props.tag),
      memoryReservationMiB: props.containerMemoryReservation,
      logging,
      environmentFiles: [EnvironmentFile.fromBucket(bucket, `${props.configPath}`)]
    });

I review the source code of aws-cdk, I think the following code is the root cause. The arn may should use bucket's arn otherwise hard code arn:aws:s3:::....

value: `arn:aws:s3:::${s3Location.bucketName}/${s3Location.objectKey}`,

Reproduction Steps


What did you expect to happen?

cloud formation of the container environment should start with arn:aws-gov:s3:::.....

What actually happened?

cloud formation of the container environment starts with arn:aws:s3:::.....

CDK CLI Version

2.8.0 (build 8a5eb49)

Framework Version

No response

Node.js Version

v16.13.1

OS

macOS 12.1 21C52 arm64

Language

Typescript

Language Version

No response

Other information

No response

@ivyxjc ivyxjc added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 14, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Jan 14, 2022
@ryparker ryparker added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jan 14, 2022
@madeline-k madeline-k removed their assignment Jan 20, 2022
@mergify mergify bot closed this as completed in #18496 Jan 21, 2022
mergify bot pushed a commit that referenced this issue Jan 21, 2022
instead of assuming `aws` partition, use the stack to determine partition (which will result in a reference to `AWS::Partition`)

fixes #18429 
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
instead of assuming `aws` partition, use the stack to determine partition (which will result in a reference to `AWS::Partition`)

fixes aws#18429 
----

*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
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants