Skip to content

Commit ca15ac0

Browse files
committed
fix(ecs): using secret JSON field with fargate task does not fail
For tasks that use the Fargate launch type, it is only supported to inject the full contents of a secret as an environment variable. Specifying a specific JSON key or version is not supported at this time. Also clean up/refactor a bit. See https://docs.aws.amazon.com/AmazonECS/latest/userguide/specifying-sensitive-data-secrets.html#secrets-considerations Closes #7272
1 parent b14172d commit ca15ac0

File tree

6 files changed

+68
-25
lines changed

6 files changed

+68
-25
lines changed

packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ export class TaskDefinition extends TaskDefinitionBase {
477477
}
478478
}
479479

480-
return this.containers.map(x => x.renderContainerDefinition(this));
480+
return this.containers.map(x => x.renderContainerDefinition());
481481
}
482482
}
483483

packages/@aws-cdk/aws-ecs/lib/container-definition.ts

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,24 @@ export abstract class Secret {
3636
public static fromSecretsManager(secret: secretsmanager.ISecret, field?: string): Secret {
3737
return {
3838
arn: field ? `${secret.secretArn}:${field}::` : secret.secretArn,
39+
hasField: true,
3940
grantRead: grantee => secret.grantRead(grantee),
4041
};
4142
}
4243

44+
/**
45+
* The ARN of the secret
46+
*/
4347
public abstract readonly arn: string;
48+
49+
/**
50+
* Whether this secret uses a specific JSON field
51+
*/
52+
public abstract readonly hasField?: boolean;
53+
54+
/**
55+
* Grants reading the secret to a principal
56+
*/
4457
public abstract grantRead(grantee: iam.IGrantable): iam.Grant;
4558
}
4659

@@ -519,9 +532,9 @@ export class ContainerDefinition extends cdk.Construct {
519532
/**
520533
* Render this container definition to a CloudFormation object
521534
*
522-
* @param taskDefinition [disable-awslint:ref-via-interface] (made optional to avoid breaking change)
535+
* @param _taskDefinition [disable-awslint:ref-via-interface] (unused but kept to avoid breaking change)
523536
*/
524-
public renderContainerDefinition(taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
537+
public renderContainerDefinition(_taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
525538
return {
526539
command: this.props.command,
527540
cpu: this.props.cpu,
@@ -551,23 +564,35 @@ export class ContainerDefinition extends cdk.Construct {
551564
workingDirectory: this.props.workingDirectory,
552565
logConfiguration: this.logDriverConfig,
553566
environment: this.props.environment && renderKV(this.props.environment, 'name', 'value'),
554-
secrets: this.props.secrets && Object.entries(this.props.secrets)
555-
.map(([k, v]) => {
556-
if (taskDefinition) {
557-
v.grantRead(taskDefinition.obtainExecutionRole());
558-
}
559-
return {
560-
name: k,
561-
valueFrom: v.arn
562-
};
563-
}),
567+
secrets: this.renderSecrets(),
564568
extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
565569
healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),
566570
links: cdk.Lazy.listValue({ produce: () => this.links }, { omitEmpty: true }),
567571
linuxParameters: this.linuxParameters && this.linuxParameters.renderLinuxParameters(),
568572
resourceRequirements: (this.props.gpuCount !== undefined) ? renderResourceRequirements(this.props.gpuCount) : undefined,
569573
};
570574
}
575+
576+
private renderSecrets(): CfnTaskDefinition.SecretProperty[] | undefined {
577+
if (!this.props.secrets) {
578+
return undefined;
579+
}
580+
581+
const secrets: CfnTaskDefinition.SecretProperty[] = [];
582+
583+
for (const [k, v] of Object.entries(this.props.secrets)) {
584+
if (this.taskDefinition.isFargateCompatible && v.hasField) {
585+
throw new Error(`Cannot specify secret JSON field for a task using the FARGATE launch type: '${k}' in container '${this.node.id}'`);
586+
}
587+
v.grantRead(this.taskDefinition.obtainExecutionRole());
588+
secrets.push({
589+
name: k,
590+
valueFrom: v.arn
591+
});
592+
}
593+
594+
return secrets;
595+
}
571596
}
572597

573598
/**

packages/@aws-cdk/aws-ecs/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,6 @@
137137
"docs-public-apis:@aws-cdk/aws-ecs.GelfCompressionType.GZIP",
138138
"docs-public-apis:@aws-cdk/aws-ecs.WindowsOptimizedVersion.SERVER_2016",
139139
"docs-public-apis:@aws-cdk/aws-ecs.WindowsOptimizedVersion.SERVER_2019",
140-
"docs-public-apis:@aws-cdk/aws-ecs.Secret.arn",
141-
"docs-public-apis:@aws-cdk/aws-ecs.Secret.grantRead",
142140
"props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.egressIgnoredIPs",
143141
"props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.egressIgnoredPorts",
144142
"props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.ignoredGID",

packages/@aws-cdk/aws-ecs/test/fargate/integ.secret-json-key.expected.json renamed to packages/@aws-cdk/aws-ecs/test/ec2/integ.secret-json-field.expected.json

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
{
3434
"Essential": true,
3535
"Image": "amazon/amazon-ecs-sample",
36+
"Memory": 256,
3637
"Name": "web",
3738
"Secrets": [
3839
{
@@ -52,18 +53,16 @@
5253
]
5354
}
5455
],
55-
"Cpu": "512",
5656
"ExecutionRoleArn": {
5757
"Fn::GetAtt": [
5858
"TaskDefExecutionRoleB4775C97",
5959
"Arn"
6060
]
6161
},
62-
"Family": "awsecsintegsecretjsonkeyTaskDefC01C0E99",
63-
"Memory": "1024",
64-
"NetworkMode": "awsvpc",
62+
"Family": "awsecsintegsecretjsonfieldTaskDef1C2EE990",
63+
"NetworkMode": "bridge",
6564
"RequiresCompatibilities": [
66-
"FARGATE"
65+
"EC2"
6766
],
6867
"TaskRoleArn": {
6968
"Fn::GetAtt": [

packages/@aws-cdk/aws-ecs/test/fargate/integ.secret-json-key.ts renamed to packages/@aws-cdk/aws-ecs/test/ec2/integ.secret-json-field.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as cdk from '@aws-cdk/core';
33
import * as ecs from '../../lib';
44

55
const app = new cdk.App();
6-
const stack = new cdk.Stack(app, 'aws-ecs-integ-secret-json-key');
6+
const stack = new cdk.Stack(app, 'aws-ecs-integ-secret-json-field');
77

88
const secret = new secretsmanager.Secret(stack, 'Secret', {
99
generateSecretString: {
@@ -12,13 +12,11 @@ const secret = new secretsmanager.Secret(stack, 'Secret', {
1212
}
1313
});
1414

15-
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef', {
16-
memoryLimitMiB: 1024,
17-
cpu: 512
18-
});
15+
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
1916

2017
taskDefinition.addContainer('web', {
2118
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
19+
memoryLimitMiB: 256,
2220
secrets: {
2321
PASSWORD: ecs.Secret.fromSecretsManager(secret, 'password')
2422
}

packages/@aws-cdk/aws-ecs/test/test.container-definition.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,29 @@ export = {
843843

844844
},
845845

846+
'throws when using a specific secret JSON field as environment variable for a Fargate task'(test: Test) {
847+
// GIVEN
848+
const app = new cdk.App();
849+
const stack = new cdk.Stack(app, 'Stack');
850+
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef');
851+
852+
const secret = new secretsmanager.Secret(stack, 'Secret');
853+
854+
// WHEN
855+
taskDefinition.addContainer('cont', {
856+
image: ecs.ContainerImage.fromRegistry('test'),
857+
memoryLimitMiB: 1024,
858+
secrets: {
859+
SECRET_KEY: ecs.Secret.fromSecretsManager(secret, 'specificKey'),
860+
}
861+
});
862+
863+
// THEN
864+
test.throws(() => app.synth(), /Cannot specify secret JSON field for a task using the FARGATE launch type/);
865+
866+
test.done();
867+
},
868+
846869
'can add AWS logging to container definition'(test: Test) {
847870
// GIVEN
848871
const stack = new cdk.Stack();

0 commit comments

Comments
 (0)