Skip to content

Commit cb03a60

Browse files
jogoldjogold
authored
fix(ecs): using secret JSON field with fargate task does not fail (#7317)
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 c498f60 commit cb03a60

File tree

9 files changed

+194
-27
lines changed

9 files changed

+194
-27
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
@@ -497,7 +497,7 @@ export class TaskDefinition extends TaskDefinitionBase {
497497
}
498498
}
499499

500-
return this.containers.map(x => x.renderContainerDefinition(this));
500+
return this.containers.map(x => x.renderContainerDefinition());
501501
}
502502
}
503503

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

Lines changed: 32 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: !!field,
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

@@ -348,6 +361,8 @@ export class ContainerDefinition extends cdk.Construct {
348361

349362
private readonly imageConfig: ContainerImageConfig;
350363

364+
private readonly secrets?: CfnTaskDefinition.SecretProperty[];
365+
351366
/**
352367
* Constructs a new instance of the ContainerDefinition class.
353368
*/
@@ -369,6 +384,20 @@ export class ContainerDefinition extends cdk.Construct {
369384
this.logDriverConfig = props.logging.bind(this, this);
370385
}
371386
props.taskDefinition._linkContainer(this);
387+
388+
if (props.secrets) {
389+
this.secrets = [];
390+
for (const [name, secret] of Object.entries(props.secrets)) {
391+
if (this.taskDefinition.isFargateCompatible && secret.hasField) {
392+
throw new Error(`Cannot specify secret JSON field for a task using the FARGATE launch type: '${name}' in container '${this.node.id}'`);
393+
}
394+
secret.grantRead(this.taskDefinition.obtainExecutionRole());
395+
this.secrets.push({
396+
name,
397+
valueFrom: secret.arn,
398+
});
399+
}
400+
}
372401
}
373402

374403
/**
@@ -519,9 +548,9 @@ export class ContainerDefinition extends cdk.Construct {
519548
/**
520549
* Render this container definition to a CloudFormation object
521550
*
522-
* @param taskDefinition [disable-awslint:ref-via-interface] (made optional to avoid breaking change)
551+
* @param _taskDefinition [disable-awslint:ref-via-interface] (unused but kept to avoid breaking change)
523552
*/
524-
public renderContainerDefinition(taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
553+
public renderContainerDefinition(_taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
525554
return {
526555
command: this.props.command,
527556
cpu: this.props.cpu,
@@ -551,16 +580,7 @@ export class ContainerDefinition extends cdk.Construct {
551580
workingDirectory: this.props.workingDirectory,
552581
logConfiguration: this.logDriverConfig,
553582
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-
}),
583+
secrets: this.secrets,
564584
extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
565585
healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),
566586
links: cdk.Lazy.listValue({ produce: () => this.links }, { omitEmpty: true }),

‎packages/@aws-cdk/aws-ecs/lib/firelens-log-router.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,9 @@ export class FirelensLogRouter extends ContainerDefinition {
236236
/**
237237
* Render this container definition to a CloudFormation object
238238
*/
239-
public renderContainerDefinition(taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
239+
public renderContainerDefinition(_taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
240240
return {
241-
...(super.renderContainerDefinition(taskDefinition)),
241+
...(super.renderContainerDefinition()),
242242
firelensConfiguration: this.firelensConfig && renderFirelensConfig(this.firelensConfig),
243243
};
244244
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,6 @@
135135
"docs-public-apis:@aws-cdk/aws-ecs.GelfCompressionType.GZIP",
136136
"docs-public-apis:@aws-cdk/aws-ecs.WindowsOptimizedVersion.SERVER_2016",
137137
"docs-public-apis:@aws-cdk/aws-ecs.WindowsOptimizedVersion.SERVER_2019",
138-
"docs-public-apis:@aws-cdk/aws-ecs.Secret.arn",
139-
"docs-public-apis:@aws-cdk/aws-ecs.Secret.grantRead",
140138
"props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.egressIgnoredIPs",
141139
"props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.egressIgnoredPorts",
142140
"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/fargate/integ.secret.expected.json‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
{
2+
"Resources": {
3+
"SecretA720EF05": {
4+
"Type": "AWS::SecretsManager::Secret",
5+
"Properties": {
6+
"GenerateSecretString": {
7+
"GenerateStringKey": "password",
8+
"SecretStringTemplate": "{\"username\":\"user\"}"
9+
}
10+
}
11+
},
12+
"TaskDefTaskRole1EDB4A67": {
13+
"Type": "AWS::IAM::Role",
14+
"Properties": {
15+
"AssumeRolePolicyDocument": {
16+
"Statement": [
17+
{
18+
"Action": "sts:AssumeRole",
19+
"Effect": "Allow",
20+
"Principal": {
21+
"Service": "ecs-tasks.amazonaws.com"
22+
}
23+
}
24+
],
25+
"Version": "2012-10-17"
26+
}
27+
}
28+
},
29+
"TaskDef54694570": {
30+
"Type": "AWS::ECS::TaskDefinition",
31+
"Properties": {
32+
"ContainerDefinitions": [
33+
{
34+
"Essential": true,
35+
"Image": "amazon/amazon-ecs-sample",
36+
"Name": "web",
37+
"Secrets": [
38+
{
39+
"Name": "SECRET",
40+
"ValueFrom": {
41+
"Ref": "SecretA720EF05"
42+
}
43+
}
44+
]
45+
}
46+
],
47+
"Cpu": "256",
48+
"ExecutionRoleArn": {
49+
"Fn::GetAtt": [
50+
"TaskDefExecutionRoleB4775C97",
51+
"Arn"
52+
]
53+
},
54+
"Family": "awsecsintegsecretTaskDef58AA207D",
55+
"Memory": "512",
56+
"NetworkMode": "awsvpc",
57+
"RequiresCompatibilities": [
58+
"FARGATE"
59+
],
60+
"TaskRoleArn": {
61+
"Fn::GetAtt": [
62+
"TaskDefTaskRole1EDB4A67",
63+
"Arn"
64+
]
65+
}
66+
}
67+
},
68+
"TaskDefExecutionRoleB4775C97": {
69+
"Type": "AWS::IAM::Role",
70+
"Properties": {
71+
"AssumeRolePolicyDocument": {
72+
"Statement": [
73+
{
74+
"Action": "sts:AssumeRole",
75+
"Effect": "Allow",
76+
"Principal": {
77+
"Service": "ecs-tasks.amazonaws.com"
78+
}
79+
}
80+
],
81+
"Version": "2012-10-17"
82+
}
83+
}
84+
},
85+
"TaskDefExecutionRoleDefaultPolicy0DBB737A": {
86+
"Type": "AWS::IAM::Policy",
87+
"Properties": {
88+
"PolicyDocument": {
89+
"Statement": [
90+
{
91+
"Action": "secretsmanager:GetSecretValue",
92+
"Effect": "Allow",
93+
"Resource": {
94+
"Ref": "SecretA720EF05"
95+
}
96+
}
97+
],
98+
"Version": "2012-10-17"
99+
},
100+
"PolicyName": "TaskDefExecutionRoleDefaultPolicy0DBB737A",
101+
"Roles": [
102+
{
103+
"Ref": "TaskDefExecutionRoleB4775C97"
104+
}
105+
]
106+
}
107+
}
108+
}
109+
}

‎packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
2+
import * as cdk from '@aws-cdk/core';
3+
import * as ecs from '../../lib';
4+
5+
const app = new cdk.App();
6+
const stack = new cdk.Stack(app, 'aws-ecs-integ-secret');
7+
8+
const secret = new secretsmanager.Secret(stack, 'Secret', {
9+
generateSecretString: {
10+
generateStringKey: 'password',
11+
secretStringTemplate: JSON.stringify({ username: 'user' }),
12+
},
13+
});
14+
15+
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef');
16+
17+
taskDefinition.addContainer('web', {
18+
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
19+
secrets: {
20+
SECRET: ecs.Secret.fromSecretsManager(secret),
21+
},
22+
});
23+
24+
app.synth();

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,25 @@ 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 stack = new cdk.Stack();
849+
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef');
850+
851+
const secret = new secretsmanager.Secret(stack, 'Secret');
852+
853+
// THEN
854+
test.throws(() => taskDefinition.addContainer('cont', {
855+
image: ecs.ContainerImage.fromRegistry('test'),
856+
memoryLimitMiB: 1024,
857+
secrets: {
858+
SECRET_KEY: ecs.Secret.fromSecretsManager(secret, 'specificKey'),
859+
},
860+
}), /Cannot specify secret JSON field for a task using the FARGATE launch type/);
861+
862+
test.done();
863+
},
864+
846865
'can add AWS logging to container definition'(test: Test) {
847866
// GIVEN
848867
const stack = new cdk.Stack();

0 commit comments

Comments
 (0)