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

refactor(aws-ecs): DRY up propagateTaskTagsFrom #15105

Merged
merged 3 commits into from
Jun 14, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
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();
},
},
});