feat(aws-ecs): addSecret() method on ContainerDefinition

closes aws#18959
FlorinAsavoaie · Aug 30, 2022 · 75557f5 · 75557f5
1 parent 82ce4a1
commit 75557f5
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 40 deletions.
diff --git a/packages/@aws-cdk/aws-ecs/README.md b/packages/@aws-cdk/aws-ecs/README.md
@@ -479,6 +479,8 @@ const newContainer = taskDefinition.addContainer('container', {
   },
 });
 newContainer.addEnvironment('QUEUE_NAME', 'MyQueue');
+newContainer.addSecret('API_KEY', ecs.Secret.fromSecretsManager(secret));
+newContainer.addSecret('DB_PASSWORD', ecs.Secret.fromSecretsManager(secret, 'password'));
 ```
 
 The task execution role is automatically granted read permissions on the secrets/parameters. Support for environment

diff --git a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
@@ -379,8 +379,6 @@ export class TaskDefinition extends TaskDefinitionBase {
 
   private _passRoleStatement?: iam.PolicyStatement;
 
-  private _referencesSecretJsonField?: boolean;
-
   private runtimePlatform?: RuntimePlatform;
 
   /**
@@ -614,9 +612,6 @@ export class TaskDefinition extends TaskDefinitionBase {
     if (this.defaultContainer === undefined && container.essential) {
       this.defaultContainer = container;
     }
-    if (container.referencesSecretJsonField) {
-      this._referencesSecretJsonField = true;
-    }
   }
 
   /**
@@ -695,7 +690,12 @@ export class TaskDefinition extends TaskDefinitionBase {
    * specific JSON field of a secret stored in Secrets Manager.
    */
   public get referencesSecretJsonField(): boolean | undefined {
-    return this._referencesSecretJsonField;
+    for (const container of this.containers) {
+      if (container.referencesSecretJsonField) {
+        return true;
+      }
+    }
+    return false;
   }
 
   /**

diff --git a/packages/@aws-cdk/aws-ecs/lib/container-definition.ts b/packages/@aws-cdk/aws-ecs/lib/container-definition.ts
@@ -435,12 +435,6 @@ export class ContainerDefinition extends Construct {
    */
   public readonly logDriverConfig?: LogDriverConfig;
 
-  /**
-   * Whether this container definition references a specific JSON field of a secret
-   * stored in Secrets Manager.
-   */
-  public readonly referencesSecretJsonField?: boolean;
-
   /**
    * The name of the image referenced by this container.
    */
@@ -458,7 +452,7 @@ export class ContainerDefinition extends Construct {
 
   private readonly imageConfig: ContainerImageConfig;
 
-  private readonly secrets?: CfnTaskDefinition.SecretProperty[];
+  private readonly secrets: CfnTaskDefinition.SecretProperty[] = [];
 
   private readonly environment: { [key: string]: string };
 
@@ -486,16 +480,8 @@ export class ContainerDefinition extends Construct {
     }
 
     if (props.secrets) {
-      this.secrets = [];
       for (const [name, secret] of Object.entries(props.secrets)) {
-        if (secret.hasField) {
-          this.referencesSecretJsonField = true;
-        }
-        secret.grantRead(this.taskDefinition.obtainExecutionRole());
-        this.secrets.push({
-          name,
-          valueFrom: secret.arn,
-        });
+        this.addSecret(name, secret);
       }
     }
 
@@ -602,6 +588,18 @@ export class ContainerDefinition extends Construct {
     this.environment[name] = value;
   }
 
+  /**
+   * This method adds a secret as environment variable to the container.
+   */
+  public addSecret(name: string, secret: Secret) {
+    secret.grantRead(this.taskDefinition.obtainExecutionRole());
+
+    this.secrets.push({
+      name,
+      valueFrom: secret.arn,
+    });
+  }
+
   /**
    * This method adds one or more resources to the container.
    */
@@ -658,6 +656,19 @@ export class ContainerDefinition extends Construct {
     return undefined;
   }
 
+  /**
+   * Whether this container definition references a specific JSON field of a secret
+   * stored in Secrets Manager.
+   */
+  public get referencesSecretJsonField(): boolean | undefined {
+    for (const secret of this.secrets) {
+      if (secret.valueFrom.endsWith('::')) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * The inbound rules associated with the security group the task or service will use.
    *
@@ -726,7 +737,7 @@ export class ContainerDefinition extends Construct {
       logConfiguration: this.logDriverConfig,
       environment: this.environment && Object.keys(this.environment).length ? renderKV(this.environment, 'name', 'value') : undefined,
       environmentFiles: this.environmentFiles && renderEnvironmentFiles(cdk.Stack.of(this).partition, this.environmentFiles),
-      secrets: this.secrets,
+      secrets: this.secrets.length ? this.secrets : undefined,
       extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
       healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),
       links: cdk.Lazy.list({ produce: () => this.links }, { omitEmpty: true }),

diff --git a/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts b/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts
@@ -128,11 +128,6 @@ export class FargateService extends BaseService implements IFargateService {
       throw new Error('Only one of SecurityGroup or SecurityGroups can be populated.');
     }
 
-    if (props.taskDefinition.referencesSecretJsonField
-        && props.platformVersion
-        && 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.`);
-    }
     super(scope, id, {
       ...props,
       desiredCount: props.desiredCount,
@@ -154,6 +149,14 @@ export class FargateService extends BaseService implements IFargateService {
 
     this.configureAwsVpcNetworkingWithSecurityGroups(props.cluster.vpc, props.assignPublicIp, props.vpcSubnets, securityGroups);
 
+    this.node.addValidation({
+      validate: () => this.taskDefinition.referencesSecretJsonField
+      && props.platformVersion
+      && SECRET_JSON_FIELD_UNSUPPORTED_PLATFORM_VERSIONS.includes(props.platformVersion)
+        ? [`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.`]
+        : [],
+    });
+
     this.node.addValidation({
       validate: () => !this.taskDefinition.defaultContainer ? ['A TaskDefinition must have at least one essential container'] : [],
     });

diff --git a/packages/@aws-cdk/aws-ecs/package.json b/packages/@aws-cdk/aws-ecs/package.json
@@ -85,6 +85,7 @@
     "@aws-cdk/aws-efs": "0.0.0",
     "@aws-cdk/cdk-build-tools": "0.0.0",
     "@aws-cdk/integ-runner": "0.0.0",
+    "@aws-cdk/integ-tests": "0.0.0",
     "@aws-cdk/cfn2ts": "0.0.0",
     "@aws-cdk/cx-api": "0.0.0",
     "@aws-cdk/pkglint": "0.0.0",

diff --git a/packages/@aws-cdk/aws-ecs/test/container-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/container-definition.test.ts
@@ -548,7 +548,7 @@ describe('container definition', () => {
         const actual = container.containerPort;
         const expected = 8080;
         expect(actual).toEqual(expected);
-      }).toThrow(/Container MyContainer hasn't defined any ports. Call addPortMappings()./);
+      }).toThrow(/Container MyContainer hasn't defined any ports. Call addPortMappings\(\)./);
 
 
     });
@@ -597,7 +597,7 @@ describe('container definition', () => {
           const actual = container.ingressPort;
           const expected = 8080;
           expect(actual).toEqual(expected);
-        }).toThrow(/Container MyContainer hasn't defined any ports. Call addPortMappings()./);
+        }).toThrow(/Container MyContainer hasn't defined any ports. Call addPortMappings\(\)./);
 
 
       });
@@ -1258,7 +1258,7 @@ describe('container definition', () => {
     });
 
     // WHEN
-    taskDefinition.addContainer('cont', {
+    const container = taskDefinition.addContainer('cont', {
       image: ecs.ContainerImage.fromRegistry('test'),
       memoryLimitMiB: 1024,
       secrets: {
@@ -1268,6 +1268,7 @@ describe('container definition', () => {
         SECRET_STAGE: ecs.Secret.fromSecretsManagerVersion(secret, { versionStage: 'version-stage' }),
       },
     });
+    container.addSecret('LATER_SECRET', ecs.Secret.fromSecretsManager(secret, 'field'));
 
     // THEN
     Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
@@ -1331,6 +1332,20 @@ describe('container definition', () => {
                 ],
               },
             },
+            {
+              Name: 'LATER_SECRET',
+              ValueFrom: {
+                'Fn::Join': [
+                  '',
+                  [
+                    {
+                      Ref: 'SecretA720EF05',
+                    },
+                    ':field::',
+                  ],
+                ],
+              },
+            },
           ],
         }),
       ],

diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.secret-json-field.ts b/packages/@aws-cdk/aws-ecs/test/ec2/integ.secret-json-field.ts
@@ -1,5 +1,6 @@
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
 import * as cdk from '@aws-cdk/core';
+import * as integ from '@aws-cdk/integ-tests';
 import * as ecs from '../../lib';
 
 const app = new cdk.App();
@@ -14,12 +15,18 @@ const secret = new secretsmanager.Secret(stack, 'Secret', {
 
 const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
 
-taskDefinition.addContainer('web', {
+const container = taskDefinition.addContainer('web', {
   image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
   memoryLimitMiB: 256,
   secrets: {
     PASSWORD: ecs.Secret.fromSecretsManager(secret, 'password'),
   },
 });
 
+container.addSecret('APIKEY', ecs.Secret.fromSecretsManager(secret, 'apikey'));
+
+new integ.IntegTest(app, 'aws-ecs-ec2-integ-secret-json-field', {
+  testCases: [stack],
+});
+
 app.synth();
diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/fargate-service.test.ts b/packages/@aws-cdk/aws-ecs/test/fargate/fargate-service.test.ts
@@ -587,13 +587,16 @@ describe('fargate service', () => {
         },
       });
 
+      // Errors on validation, not on construction.
+      new ecs.FargateService(stack, 'FargateService', {
+        cluster,
+        taskDefinition,
+        platformVersion: ecs.FargatePlatformVersion.VERSION1_3,
+      });
+
       // THEN
       expect(() => {
-        new ecs.FargateService(stack, 'FargateService', {
-          cluster,
-          taskDefinition,
-          platformVersion: ecs.FargatePlatformVersion.VERSION1_3,
-        });
+        Template.fromStack(stack);
       }).toThrow(new RegExp(`uses at least one container that references a secret JSON field.+platform version ${ecs.FargatePlatformVersion.VERSION1_4} or later`));
     });
 
@@ -1262,7 +1265,7 @@ describe('fargate service', () => {
               containerPort: 8001,
             })],
           });
-        }).toThrow(/No container named 'SideContainer'. Did you call "addContainer()"?/);
+        }).toThrow(/No container named 'SideContainer'. Did you call "addContainer\(\)"?/);
       });
     });
 
@@ -2178,7 +2181,7 @@ describe('fargate service', () => {
           ],
         });
       });
-    }),
+    });
 
     test('with serviceArn old format', () => {
       // GIVEN

diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.ts b/packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.ts
@@ -1,5 +1,6 @@
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
 import * as cdk from '@aws-cdk/core';
+import * as integ from '@aws-cdk/integ-tests';
 import * as ecs from '../../lib';
 
 const app = new cdk.App();
@@ -14,12 +15,18 @@ const secret = new secretsmanager.Secret(stack, 'Secret', {
 
 const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef');
 
-taskDefinition.addContainer('web', {
+const container = taskDefinition.addContainer('web', {
   image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
   secrets: {
     SECRET: ecs.Secret.fromSecretsManager(secret),
     PASSWORD: ecs.Secret.fromSecretsManager(secret, 'password'),
   },
 });
 
+container.addSecret('APIKEY', ecs.Secret.fromSecretsManager(secret, 'apikey'));
+
+new integ.IntegTest(app, 'aws-ecs-fargate-integ-secret', {
+  testCases: [stack],
+});
+
 app.synth();