Skip to content

Commit

Permalink
refactor(aws-ecs): DRY up propagateTaskTagsFrom (#15105)
Browse files Browse the repository at this point in the history
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
SoManyHs authored Jun 14, 2021
1 parent cbd7552 commit 7ea9e48
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 46 deletions.
17 changes: 16 additions & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,15 @@ export interface BaseServiceOptions {
*/
readonly propagateTags?: PropagatedTagSource;

/**
* Specifies whether to propagate the tags from the task definition or the service to the tasks in the service.
* Tags can only be propagated to the tasks within the service during service creation.
*
* @deprecated Use `propagateTags` instead.
* @default PropagatedTagSource.NONE
*/
readonly propagateTaskTagsFrom?: PropagatedTagSource;

/**
* Specifies whether to enable Amazon ECS managed tags for the tasks within the service. For more information, see
* [Tagging Your Amazon ECS Resources](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-using-tags.html)
Expand Down Expand Up @@ -373,13 +382,19 @@ export abstract class BaseService extends Resource
physicalName: props.serviceName,
});

if (props.propagateTags && props.propagateTaskTagsFrom) {
throw new Error('You can only specify either propagateTags or propagateTaskTagsFrom. Alternatively, you can leave both blank');
}

this.taskDefinition = taskDefinition;

// launchType will set to undefined if using external DeploymentController or capacityProviderStrategies
const launchType = props.deploymentController?.type === DeploymentControllerType.EXTERNAL ||
props.capacityProviderStrategies !== undefined ?
undefined : props.launchType;

const propagateTagsFromSource = props.propagateTaskTagsFrom ?? props.propagateTags ?? PropagatedTagSource.NONE;

this.resource = new CfnService(this, 'Service', {
desiredCount: props.desiredCount,
serviceName: this.physicalName,
Expand All @@ -392,7 +407,7 @@ export abstract class BaseService extends Resource
rollback: props.circuitBreaker.rollback ?? false,
} : undefined,
},
propagateTags: props.propagateTags === PropagatedTagSource.NONE ? undefined : props.propagateTags,
propagateTags: propagateTagsFromSource === PropagatedTagSource.NONE ? undefined : props.propagateTags,
enableEcsManagedTags: props.enableECSManagedTags ?? false,
deploymentController: props.circuitBreaker ? {
type: DeploymentControllerType.ECS,
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ export class TaskDefinition extends TaskDefinitionBase {
}

/**
* Adds the specified extention to the task definition.
* Adds the specified extension to the task definition.
*
* Extension can be used to apply a packaged modification to
* a task definition.
Expand Down
18 changes: 1 addition & 17 deletions packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as ec2 from '@aws-cdk/aws-ec2';
import { Lazy, Resource, Stack } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { BaseService, BaseServiceOptions, DeploymentControllerType, IBaseService, IService, LaunchType, PropagatedTagSource } from '../base/base-service';
import { BaseService, BaseServiceOptions, DeploymentControllerType, IBaseService, IService, LaunchType } from '../base/base-service';
import { fromServiceAtrributes } from '../base/from-service-attributes';
import { NetworkMode, TaskDefinition } from '../base/task-definition';
import { ICluster } from '../cluster';
Expand Down Expand Up @@ -82,15 +82,6 @@ export interface Ec2ServiceProps extends BaseServiceOptions {
* @default false
*/
readonly daemon?: boolean;

/**
* Specifies whether to propagate the tags from the task definition or the service to the tasks in the service.
* Tags can only be propagated to the tasks within the service during service creation.
*
* @deprecated Use `propagateTags` instead.
* @default PropagatedTagSource.NONE
*/
readonly propagateTaskTagsFrom?: PropagatedTagSource;
}

/**
Expand Down Expand Up @@ -173,23 +164,16 @@ export class Ec2Service extends BaseService implements IEc2Service {
throw new Error('Supplied TaskDefinition is not configured for compatibility with EC2');
}

if (props.propagateTags && props.propagateTaskTagsFrom) {
throw new Error('You can only specify either propagateTags or propagateTaskTagsFrom. Alternatively, you can leave both blank');
}

if (props.securityGroup !== undefined && props.securityGroups !== undefined) {
throw new Error('Only one of SecurityGroup or SecurityGroups can be populated.');
}

const propagateTagsFromSource = props.propagateTaskTagsFrom ?? props.propagateTags ?? PropagatedTagSource.NONE;

super(scope, id, {
...props,
desiredCount: props.desiredCount,
maxHealthyPercent: props.daemon && props.maxHealthyPercent === undefined ? 100 : props.maxHealthyPercent,
minHealthyPercent: props.daemon && props.minHealthyPercent === undefined ? 0 : props.minHealthyPercent,
launchType: LaunchType.EC2,
propagateTags: propagateTagsFromSource,
enableECSManagedTags: props.enableECSManagedTags,
},
{
Expand Down
20 changes: 1 addition & 19 deletions packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as ec2 from '@aws-cdk/aws-ec2';
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { BaseService, BaseServiceOptions, DeploymentControllerType, IBaseService, IService, LaunchType, PropagatedTagSource } from '../base/base-service';
import { BaseService, BaseServiceOptions, DeploymentControllerType, IBaseService, IService, LaunchType } from '../base/base-service';
import { fromServiceAtrributes } from '../base/from-service-attributes';
import { TaskDefinition } from '../base/task-definition';
import { ICluster } from '../cluster';
Expand Down Expand Up @@ -58,16 +58,6 @@ export interface FargateServiceProps extends BaseServiceOptions {
* @default Latest
*/
readonly platformVersion?: FargatePlatformVersion;

/**
* Specifies whether to propagate the tags from the task definition or the service to the tasks in the service.
* Tags can only be propagated to the tasks within the service during service creation.
*
* @deprecated Use `propagateTags` instead.
* @default PropagatedTagSource.NONE
*/
readonly propagateTaskTagsFrom?: PropagatedTagSource;

}

/**
Expand Down Expand Up @@ -134,10 +124,6 @@ export class FargateService extends BaseService implements IFargateService {
throw new Error('Supplied TaskDefinition is not configured for compatibility with Fargate');
}

if (props.propagateTags && props.propagateTaskTagsFrom) {
throw new Error('You can only specify either propagateTags or propagateTaskTagsFrom. Alternatively, you can leave both blank');
}

if (props.securityGroup !== undefined && props.securityGroups !== undefined) {
throw new Error('Only one of SecurityGroup or SecurityGroups can be populated.');
}
Expand All @@ -147,15 +133,11 @@ export class FargateService extends BaseService implements IFargateService {
&& SECRET_JSON_FIELD_UNSUPPORTED_PLATFORM_VERSIONS.includes(props.platformVersion)) {
throw new Error(`The task definition of this service uses at least one container that references a secret JSON field. This feature requires platform version ${FargatePlatformVersion.VERSION1_4} or later.`);
}

const propagateTagsFromSource = props.propagateTaskTagsFrom ?? props.propagateTags ?? PropagatedTagSource.NONE;

super(scope, id, {
...props,
desiredCount: props.desiredCount,
launchType: LaunchType.FARGATE,
capacityProviderStrategies: props.capacityProviderStrategies,
propagateTags: propagateTagsFromSource,
enableECSManagedTags: props.enableECSManagedTags,
}, {
cluster: props.cluster.clusterName,
Expand Down
15 changes: 8 additions & 7 deletions packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1859,13 +1859,14 @@ nodeunitShim({
memoryLimitMiB: 512,
});

test.throws(() => new ecs.Ec2Service(stack, 'Ec2Service', {
cluster,
taskDefinition,
propagateTags: PropagatedTagSource.SERVICE,
propagateTaskTagsFrom: PropagatedTagSource.SERVICE,
}));

test.throws(() => {
new ecs.Ec2Service(stack, 'Ec2Service', {
cluster,
taskDefinition,
propagateTags: PropagatedTagSource.SERVICE,
propagateTaskTagsFrom: PropagatedTagSource.SERVICE,
});
}, /You can only specify either propagateTags or propagateTaskTagsFrom. Alternatively, you can leave both blank/);
test.done();
},

Expand Down
26 changes: 25 additions & 1 deletion packages/@aws-cdk/aws-ecs/test/fargate/fargate-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as cloudmap from '@aws-cdk/aws-servicediscovery';
import * as cdk from '@aws-cdk/core';
import { nodeunitShim, Test } from 'nodeunit-shim';
import * as ecs from '../../lib';
import { DeploymentControllerType, LaunchType } from '../../lib/base/base-service';
import { DeploymentControllerType, LaunchType, PropagatedTagSource } from '../../lib/base/base-service';

nodeunitShim({
'When creating a Fargate Service': {
Expand Down Expand Up @@ -2948,5 +2948,29 @@ nodeunitShim({

test.done();
},

'with both propagateTags and propagateTaskTagsFrom defined'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'MyVpc', {});
const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'FargateTaskDef');

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

// THEN
test.throws(() => {
new ecs.FargateService(stack, 'FargateService', {
cluster,
taskDefinition,
propagateTags: PropagatedTagSource.SERVICE,
propagateTaskTagsFrom: PropagatedTagSource.SERVICE,
});
}, /You can only specify either propagateTags or propagateTaskTagsFrom. Alternatively, you can leave both blank/);
test.done();
},
},
});

0 comments on commit 7ea9e48

Please sign in to comment.