From 80d49a878caf219cb2d826ea48f8964e33831bc3 Mon Sep 17 00:00:00 2001 From: Piradeep Kandasamy Date: Tue, 25 Jun 2019 09:23:32 -0700 Subject: [PATCH 1/3] Remove temporary workaround for long arn support --- .../@aws-cdk/aws-ecs/lib/base/base-service.ts | 26 ++----------------- .../test/fargate/test.fargate-service.ts | 16 +++--------- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts index 19664c4a4fd2d..52079f93944d9 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts @@ -4,7 +4,7 @@ import ec2 = require('@aws-cdk/aws-ec2'); import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2'); import iam = require('@aws-cdk/aws-iam'); import cloudmap = require('@aws-cdk/aws-servicediscovery'); -import { Construct, Duration, Fn, IResolvable, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; +import { Construct, Duration, IResolvable, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { NetworkMode, TaskDefinition } from '../base/task-definition'; import { ICluster } from '../cluster'; import { CfnService } from '../ecs.generated'; @@ -73,21 +73,6 @@ export interface BaseServiceProps { * @default - AWS Cloud Map service discovery is not enabled. */ readonly cloudMapOptions?: CloudMapOptions; - - /** - * Whether the new long ARN format has been enabled on ECS services. - * NOTE: This assumes customer has opted into the new format for the IAM role used for the service, and is a - * workaround for a current bug in Cloudformation in which the service name is not correctly returned when long ARN is - * enabled. - * - * Old ARN format: arn:aws:ecs:region:aws_account_id:service/service-name - * New ARN format: arn:aws:ecs:region:aws_account_id:service/cluster-name/service-name - * - * See: https://docs.aws.amazon.com/AmazonECS/latest/userguide/ecs-resource-ids.html - * - * @default false - */ - readonly longArnEnabled?: boolean; } /** @@ -157,19 +142,12 @@ export abstract class BaseService extends Resource ...additionalProps }); - // This is a workaround for CFN bug that returns the cluster name instead of the service name when long ARN formats - // are enabled for the principal in a given region. - const longArnEnabled = props.longArnEnabled !== undefined ? props.longArnEnabled : false; - const serviceName = longArnEnabled - ? Fn.select(2, Fn.split('/', this.resource.ref)) - : this.resource.attrName; - this.serviceArn = this.getResourceArnAttribute(this.resource.ref, { service: 'ecs', resource: 'service', resourceName: `${props.cluster.clusterName}/${this.physicalName}`, }); - this.serviceName = this.getResourceNameAttribute(serviceName); + this.serviceName = this.getResourceNameAttribute(this.resource.attrName); this.cluster = props.cluster; diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/test.fargate-service.ts b/packages/@aws-cdk/aws-ecs/test/fargate/test.fargate-service.ts index 71a2c21ce78c1..329f210a5189b 100644 --- a/packages/@aws-cdk/aws-ecs/test/fargate/test.fargate-service.ts +++ b/packages/@aws-cdk/aws-ecs/test/fargate/test.fargate-service.ts @@ -273,8 +273,7 @@ export = { const service = new ecs.FargateService(stack, 'Service', { cluster, - taskDefinition, - longArnEnabled: true + taskDefinition }); const lb = new elbv2.ApplicationLoadBalancer(stack, "lb", { vpc }); @@ -305,16 +304,9 @@ export = { }, "/", { - "Fn::Select": [ - 2, - { - "Fn::Split": [ - "/", - { - Ref: "ServiceD69D759B" - } - ] - } + "Fn::GetAtt": [ + "ServiceD69D759B", + "Name" ] } ] From 1f8e097533a0da8e525468b62d81554993d909d5 Mon Sep 17 00:00:00 2001 From: Piradeep Kandasamy Date: Thu, 27 Jun 2019 17:31:05 -0700 Subject: [PATCH 2/3] Add deprecated annotation --- .../@aws-cdk/aws-ecs/lib/base/base-service.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts index 52079f93944d9..f34191870c517 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts @@ -73,6 +73,22 @@ export interface BaseServiceProps { * @default - AWS Cloud Map service discovery is not enabled. */ readonly cloudMapOptions?: CloudMapOptions; + + /** + * Whether the new long ARN format has been enabled on ECS services. + * NOTE: This assumes customer has opted into the new format for the IAM role used for the service, and is a + * workaround for a current bug in Cloudformation in which the service name is not correctly returned when long ARN is + * enabled. + * + * Old ARN format: arn:aws:ecs:region:aws_account_id:service/service-name + * New ARN format: arn:aws:ecs:region:aws_account_id:service/cluster-name/service-name + * + * See: https://docs.aws.amazon.com/AmazonECS/latest/userguide/ecs-resource-ids.html + * + * @default none + * @deprecated this property is no longer used. Long arns are automatically handled by cloud formation. + */ + readonly longArnEnabled?: boolean; } /** From 87527ca0d4299dbbfc0a29bcef437338e6efa0d9 Mon Sep 17 00:00:00 2001 From: Piradeep Kandasamy Date: Mon, 1 Jul 2019 14:02:05 -0700 Subject: [PATCH 3/3] Revert "Add deprecated annotation" This reverts commit 1f8e097533a0da8e525468b62d81554993d909d5. --- .../@aws-cdk/aws-ecs/lib/base/base-service.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts index f34191870c517..52079f93944d9 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts @@ -73,22 +73,6 @@ export interface BaseServiceProps { * @default - AWS Cloud Map service discovery is not enabled. */ readonly cloudMapOptions?: CloudMapOptions; - - /** - * Whether the new long ARN format has been enabled on ECS services. - * NOTE: This assumes customer has opted into the new format for the IAM role used for the service, and is a - * workaround for a current bug in Cloudformation in which the service name is not correctly returned when long ARN is - * enabled. - * - * Old ARN format: arn:aws:ecs:region:aws_account_id:service/service-name - * New ARN format: arn:aws:ecs:region:aws_account_id:service/cluster-name/service-name - * - * See: https://docs.aws.amazon.com/AmazonECS/latest/userguide/ecs-resource-ids.html - * - * @default none - * @deprecated this property is no longer used. Long arns are automatically handled by cloud formation. - */ - readonly longArnEnabled?: boolean; } /**