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): fromServiceAtrributes service ARN formatting #18137

Closed
tobytipton opened this issue Dec 22, 2021 · 3 comments · Fixed by #18140
Closed

(ecs): fromServiceAtrributes service ARN formatting #18137

tobytipton opened this issue Dec 22, 2021 · 3 comments · Fixed by #18140
Assignees
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. duplicate This issue is a duplicate. in-progress This issue is being actively worked on. p1

Comments

@tobytipton
Copy link
Contributor

What is the problem?

Based on https://docs.aws.amazon.com/AmazonECS/latest/userguide/ecs-account-settings.html the ECS service ARN for new services has changed.

Old: arn:aws:ecs:region:aws_account_id:service/service-name

New: arn:aws:ecs:region:aws_account_id:service/cluster-name/service-name

The fromServiceAttributes does not take that into effect.

Reproduction Steps

This can be see when reviewing the EcsDeployAction when passing in a serviceArn with the old format it generates the codepipeline action configuration with the correct service name, when passing in the new format the service name becomes the clusterName/serviceName

Old Arn Format

const cluserName = 'cluster-name';
const serviceArn = 'arn:aws:ecs:region:aws_account_id:service/service-name';
const stack = new cdk.Stack();
const cluster = new ecs.Cluster(stack, 'EcsCluster', {
    clusterName=clusterName,
});

const service = ecs.Ec2Service.fromEc2ServiceAttributes(stack, 'EcsService', {
  serviceArn: serviceArn,
  cluster,
});
...
const action = new cpactions.EcsDeployAction({
  actionName: 'ECS',
  service,
  input: artifact,
});
pipeline.addStage({
  stageName: 'Deploy',
  actions: [action]
});

Results in the following service name and cluster in the ecs deploy action step.

Configuration: {
  ClusterName: 'cluster-name',
  ServiceName: 'service-name',
}

New Arn format

const cluserName = 'cluster-name';
const serviceArn = `arn:aws:ecs:region:aws_account_id:service/${clusterName}/service-name`;
const stack = new cdk.Stack();
const cluster = new ecs.Cluster(stack, 'EcsCluster', {
    clusterName=clusterName,
});

const service = ecs.Ec2Service.fromEc2ServiceAttributes(stack, 'EcsService', {
  serviceArn: serviceArn,
  cluster,
});
...
const action = new cpactions.EcsDeployAction({
  actionName: 'ECS',
  service,
  input: artifact,
});
pipeline.addStage({
  stageName: 'Deploy',
  actions: [action]
});

Results in the following service name and cluster in the ecs deploy action step.

Configuration: {
  ClusterName: 'cluster-name',
  ServiceName: 'cluster-name/service-name',
}

What did you expect to happen?

I would expect when passing either the old ARN format for services which existed previously or the new ARN format for the services would result in the correct serviceName in the configuration.

What actually happened?

The old format ARN generated the correct serviceName, the new ARN format generated the incorrect serviceName.

CDK CLI Version

N/A

Framework Version

No response

Node.js Version

N/A

OS

N/A

Language

Typescript

Language Version

No response

Other information

This can be tested by creating tests against EcsDeployAction so CDK version, nodejs, os, etc are N/A.

@tobytipton tobytipton added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Dec 22, 2021
@jumic
Copy link
Contributor

jumic commented Dec 22, 2021

Hi @tobytipton,

Thanks for reporting this. The problem was already reported in issue #16634.
I've started working on it and will submit a fix in the next days.

@NGL321 NGL321 added duplicate This issue is a duplicate. in-progress This issue is being actively worked on. p1 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 23, 2021
@NGL321
Copy link
Contributor

NGL321 commented Dec 23, 2021

Thank you for being so on top of this @jumic, the assist is very much appreciated. I am going to close this as a duplicate, and poke on getting that PR merged.

🎉 🚀 :shipit:

@NGL321 NGL321 closed this as completed Dec 23, 2021
@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.

mergify bot pushed a commit that referenced this issue Jul 28, 2022
AWS has changed the [ARN format for ECS](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-account-settings.html#ecs-resource-ids). Currently, CDK doesn't return the correct values/ARNs if the new ARN format is used in ECS.

Changed methods:
- `Ec2Service.fromEc2ServiceAttributes()`
- `Ec2Service.fromEc2ServiceArn()`
- `FargateService.fromFargateServiceAttributes()`
- `FargateService.fromFargateServiceArn()`

The logic automatically detects whether the old or new format is used. The format cannot be recognized automatically for tokenized values. Therefore the feature flag `ECS_ARN_FORMAT_INCLUDES_CLUSTER_NAME` is introduced, which controls whether the old or the new format should be used.

In `Ec2Service.fromEc2ServiceAttributes()` and `FargateService.fromFargateServiceAttributes()` an ARN is created. In these methods the feature flag be considered to construct the ARN in the correct format.

Closes #16634.
Closes #18137.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
imanolympic pushed a commit to imanolympic/aws-cdk that referenced this issue Aug 8, 2022
AWS has changed the [ARN format for ECS](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-account-settings.html#ecs-resource-ids). Currently, CDK doesn't return the correct values/ARNs if the new ARN format is used in ECS.

Changed methods:
- `Ec2Service.fromEc2ServiceAttributes()`
- `Ec2Service.fromEc2ServiceArn()`
- `FargateService.fromFargateServiceAttributes()`
- `FargateService.fromFargateServiceArn()`

The logic automatically detects whether the old or new format is used. The format cannot be recognized automatically for tokenized values. Therefore the feature flag `ECS_ARN_FORMAT_INCLUDES_CLUSTER_NAME` is introduced, which controls whether the old or the new format should be used.

In `Ec2Service.fromEc2ServiceAttributes()` and `FargateService.fromFargateServiceAttributes()` an ARN is created. In these methods the feature flag be considered to construct the ARN in the correct format.

Closes aws#16634.
Closes aws#18137.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

cr: https://code.amazon.com/reviews/CR-73367009
josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
AWS has changed the [ARN format for ECS](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-account-settings.html#ecs-resource-ids). Currently, CDK doesn't return the correct values/ARNs if the new ARN format is used in ECS.

Changed methods:
- `Ec2Service.fromEc2ServiceAttributes()`
- `Ec2Service.fromEc2ServiceArn()`
- `FargateService.fromFargateServiceAttributes()`
- `FargateService.fromFargateServiceArn()`

The logic automatically detects whether the old or new format is used. The format cannot be recognized automatically for tokenized values. Therefore the feature flag `ECS_ARN_FORMAT_INCLUDES_CLUSTER_NAME` is introduced, which controls whether the old or the new format should be used.

In `Ec2Service.fromEc2ServiceAttributes()` and `FargateService.fromFargateServiceAttributes()` an ARN is created. In these methods the feature flag be considered to construct the ARN in the correct format.

Closes aws#16634.
Closes aws#18137.

----

*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. duplicate This issue is a duplicate. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants