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): Base Service tokenized Service Arn doesn't contain region or account. #18246

Open
tobytipton opened this issue Jan 3, 2022 · 0 comments
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@tobytipton
Copy link
Contributor

What is the problem?

The tokenized ARN](

this.serviceArn = this.getResourceArnAttribute(this.resource.ref, {
service: 'ecs',
resource: 'service',
resourceName: `${props.cluster.clusterName}/${this.physicalName}`,
});
) is using the clusterName and physicalName to generate the service ARN token.

  1. The ARN should be using the serviceName vs the physicalName because the serviceName aligns with what the actual ARN would be.
  2. The tokenized ARN should be fully qualified as a multiple parts so the ARN can be leveraged in Ec2Service.fromEc2ServiceAttributes or FargateService.fromFargateServiceAttributes to determine the region and account when deploying across regions.

Reproduction Steps

This generates the token for the serviceArn.

const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'MyVpc', {});
const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef');

taskDefinition.addContainer('web', {
  image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
  memoryLimitMiB: 512,
});

const service = new ecs.Ec2Service(stack, 'Ec2Service', {
  cluster,
  taskDefinition,
});

service.serviceArn generates a token, if this is a different account or region the in the cause of using a code pipeline which is deploying across multiple accounts or regions leveraging Ec2Service.fromEc2ServiceAttributes with the service ARN would be used in the pipeline to get the service, as you directly pass the service across boundaries.

This means to get the serviceArn after the service definition you need to do something like below to get the serviceArn to not be a full token. Also this is related to issue #16634.

const serviceArn = this.formatArn({
  service: 'ecs',
  resource: 'service',
  resourceName: `${cluster.clusterName}/${service.serviceName},
});

What did you expect to happen?

I would expect the serviceArn look like this arn:${Token[AWS.Partition.5]}:ecs:${Token[AWS.Region.6]}:${Token[AWS.AccountId.2]}:service/${Token[TOKEN.425]}/${Token[TOKEN.481]}

Which will allow the region, account as well as the clusterName and serviceName to be populated correctly when deploying the token in.

What actually happened?

The current serviceArn looks likes ${Token[TOKEN.489]} which means using the serviceArn for Ec2Service.fromEc2ServiceAttributes doesn't set the region or account correctly.

CDK CLI Version

N/A

Framework Version

No response

Node.js Version

v16.13.1

OS

Mac

Language

Typescript

Language Version

No response

Other information

This issue is seen in CDK tests so not related to specific CDK version.

@tobytipton tobytipton added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 3, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Jan 3, 2022
@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort labels Jan 3, 2022
@madeline-k madeline-k removed the needs-triage This issue or PR still needs to be triaged. label Mar 22, 2022
@madeline-k madeline-k removed their assignment Mar 22, 2022
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. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants