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

fix(aws-ecs): handle long ARN formats for services #2176

Merged
merged 5 commits into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,21 @@ export interface BaseServiceProps {
* Options for enabling AWS Cloud Map service discovery for the service
*/
readonly serviceDiscoveryOptions?: ServiceDiscoveryOptions;

/**
* 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
SoManyHs marked this conversation as resolved.
Show resolved Hide resolved
*
* @default false
*/
readonly longArnEnabled?: boolean;
}

/**
Expand Down Expand Up @@ -128,15 +143,22 @@ export abstract class BaseService extends cdk.Construct
serviceRegistries: new cdk.Token(() => this.serviceRegistries),
...additionalProps
});

this.serviceArn = this.resource.serviceArn;
this.serviceName = this.resource.serviceName;

// 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;
this.serviceName = longArnEnabled
? cdk.Fn.select(2, cdk.Fn.split('/', this.serviceArn))
: this.resource.serviceName;

this.clusterName = clusterName;
this.cluster = props.cluster;

if (props.serviceDiscoveryOptions) {
this.enableServiceDiscovery(props.serviceDiscoveryOptions);
}

}

/**
Expand Down Expand Up @@ -177,7 +199,7 @@ export abstract class BaseService extends cdk.Construct

return this.scalableTaskCount = new ScalableTaskCount(this, 'TaskCount', {
serviceNamespace: appscaling.ServiceNamespace.Ecs,
resourceId: `service/${this.clusterName}/${this.resource.serviceName}`,
resourceId: `service/${this.clusterName}/${this.serviceName}`,
dimension: 'ecs:service:DesiredCount',
role: this.makeAutoScalingRole(),
...props
Expand Down
85 changes: 84 additions & 1 deletion packages/@aws-cdk/aws-ecs/test/fargate/test.fargate-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,25 @@ export = {
// THEN
expect(stack).to(haveResource('AWS::ApplicationAutoScaling::ScalableTarget', {
MaxCapacity: 10,
MinCapacity: 1
MinCapacity: 1,
ResourceId: {
"Fn::Join": [
"",
[
"service/",
{
Ref: "EcsCluster97242B84"
},
"/",
{
"Fn::GetAtt": [
"ServiceD69D759B",
"Name"
]
}
]
]
},
}));

expect(stack).to(haveResource('AWS::ApplicationAutoScaling::ScalingPolicy', {
Expand All @@ -236,6 +254,71 @@ export = {
}
}));

test.done();
},

'allows auto scaling by ALB with new service arn format'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.VpcNetwork(stack, 'MyVpc', {});
const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'FargateTaskDef');
const container = taskDefinition.addContainer('MainContainer', {
image: ContainerImage.fromRegistry('hello'),
});
container.addPortMappings({ containerPort: 8000 });

const service = new ecs.FargateService(stack, 'Service', {
cluster,
taskDefinition,
longArnEnabled: true
});

const lb = new elbv2.ApplicationLoadBalancer(stack, "lb", { vpc });
const listener = lb.addListener("listener", { port: 80 });
const targetGroup = listener.addTargets("target", {
port: 80,
targets: [service]
});

// WHEN
const capacity = service.autoScaleTaskCount({ maxCapacity: 10, minCapacity: 1 });
capacity.scaleOnRequestCount("ScaleOnRequests", {
requestsPerTarget: 1000,
targetGroup
});

// THEN
expect(stack).to(haveResource('AWS::ApplicationAutoScaling::ScalableTarget', {
MaxCapacity: 10,
MinCapacity: 1,
ResourceId: {
"Fn::Join": [
"",
[
"service/",
{
Ref: "EcsCluster97242B84"
},
"/",
{
"Fn::Select": [
2,
{
"Fn::Split": [
"/",
{
Ref: "ServiceD69D759B"
}
]
}
]
}
]
]
},
}));

test.done();
}
},
Expand Down