fix(ecs): using secret JSON field with fargate task does not fail (aws#7317)

jogold · karupanerura · commit de97f4e3d255 · 2020-05-07T23:39:35.000+09:00
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 aws#7272
diff --git a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
@@ -497,7 +497,7 @@ export class TaskDefinition extends TaskDefinitionBase {
       }
     }
 
-    return this.containers.map(x => x.renderContainerDefinition(this));
+    return this.containers.map(x => x.renderContainerDefinition());
   }
 }
 
diff --git a/packages/@aws-cdk/aws-ecs/lib/container-definition.ts b/packages/@aws-cdk/aws-ecs/lib/container-definition.ts
@@ -36,11 +36,24 @@ export abstract class Secret {
   public static fromSecretsManager(secret: secretsmanager.ISecret, field?: string): Secret {
     return {
       arn: field ? `${secret.secretArn}:${field}::` : secret.secretArn,
+      hasField: !!field,
       grantRead: grantee => secret.grantRead(grantee),
     };
   }
 
+  /**
+   * The ARN of the secret
+   */
   public abstract readonly arn: string;
+
+  /**
+   * Whether this secret uses a specific JSON field
+   */
+  public abstract readonly hasField?: boolean;
+
+  /**
+   * Grants reading the secret to a principal
+   */
   public abstract grantRead(grantee: iam.IGrantable): iam.Grant;
 }
 
@@ -348,6 +361,8 @@ export class ContainerDefinition extends cdk.Construct {
 
   private readonly imageConfig: ContainerImageConfig;
 
+  private readonly secrets?: CfnTaskDefinition.SecretProperty[];
+
   /**
    * Constructs a new instance of the ContainerDefinition class.
    */
@@ -369,6 +384,20 @@ export class ContainerDefinition extends cdk.Construct {
       this.logDriverConfig = props.logging.bind(this, this);
     }
     props.taskDefinition._linkContainer(this);
+
+    if (props.secrets) {
+      this.secrets = [];
+      for (const [name, secret] of Object.entries(props.secrets)) {
+        if (this.taskDefinition.isFargateCompatible && secret.hasField) {
+          throw new Error(`Cannot specify secret JSON field for a task using the FARGATE launch type: '${name}' in container '${this.node.id}'`);
+        }
+        secret.grantRead(this.taskDefinition.obtainExecutionRole());
+        this.secrets.push({
+          name,
+          valueFrom: secret.arn,
+        });
+      }
+    }
   }
 
   /**
@@ -519,9 +548,9 @@ export class ContainerDefinition extends cdk.Construct {
   /**
    * Render this container definition to a CloudFormation object
    *
-   * @param taskDefinition [disable-awslint:ref-via-interface] (made optional to avoid breaking change)
+   * @param _taskDefinition [disable-awslint:ref-via-interface] (unused but kept to avoid breaking change)
    */
-  public renderContainerDefinition(taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
+  public renderContainerDefinition(_taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
     return {
       command: this.props.command,
       cpu: this.props.cpu,
@@ -551,16 +580,7 @@ export class ContainerDefinition extends cdk.Construct {
       workingDirectory: this.props.workingDirectory,
       logConfiguration: this.logDriverConfig,
       environment: this.props.environment && renderKV(this.props.environment, 'name', 'value'),
-      secrets: this.props.secrets && Object.entries(this.props.secrets)
-        .map(([k, v]) => {
-          if (taskDefinition) {
-            v.grantRead(taskDefinition.obtainExecutionRole());
-          }
-          return {
-            name: k,
-            valueFrom: v.arn,
-          };
-        }),
+      secrets: this.secrets,
       extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
       healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),
       links: cdk.Lazy.listValue({ produce: () => this.links }, { omitEmpty: true }),
diff --git a/packages/@aws-cdk/aws-ecs/lib/firelens-log-router.ts b/packages/@aws-cdk/aws-ecs/lib/firelens-log-router.ts
@@ -236,9 +236,9 @@ export class FirelensLogRouter extends ContainerDefinition {
   /**
    * Render this container definition to a CloudFormation object
    */
-  public renderContainerDefinition(taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
+  public renderContainerDefinition(_taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
     return {
-      ...(super.renderContainerDefinition(taskDefinition)),
+      ...(super.renderContainerDefinition()),
       firelensConfiguration: this.firelensConfig && renderFirelensConfig(this.firelensConfig),
     };
   }
diff --git a/packages/@aws-cdk/aws-ecs/package.json b/packages/@aws-cdk/aws-ecs/package.json
@@ -135,8 +135,6 @@
       "docs-public-apis:@aws-cdk/aws-ecs.GelfCompressionType.GZIP",
       "docs-public-apis:@aws-cdk/aws-ecs.WindowsOptimizedVersion.SERVER_2016",
       "docs-public-apis:@aws-cdk/aws-ecs.WindowsOptimizedVersion.SERVER_2019",
-      "docs-public-apis:@aws-cdk/aws-ecs.Secret.arn",
-      "docs-public-apis:@aws-cdk/aws-ecs.Secret.grantRead",
       "props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.egressIgnoredIPs",
       "props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.egressIgnoredPorts",
       "props-default-doc:@aws-cdk/aws-ecs.AppMeshProxyConfigurationProps.ignoredGID",
diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.secret-json-field.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.secret-json-field.expected.json
@@ -33,6 +33,7 @@
           {
             "Essential": true,
             "Image": "amazon/amazon-ecs-sample",
+            "Memory": 256,
             "Name": "web",
             "Secrets": [
               {
@@ -52,18 +53,16 @@
             ]
           }
         ],
-        "Cpu": "512",
         "ExecutionRoleArn": {
           "Fn::GetAtt": [
             "TaskDefExecutionRoleB4775C97",
             "Arn"
           ]
         },
-        "Family": "awsecsintegsecretjsonkeyTaskDefC01C0E99",
-        "Memory": "1024",
-        "NetworkMode": "awsvpc",
+        "Family": "awsecsintegsecretjsonfieldTaskDef1C2EE990",
+        "NetworkMode": "bridge",
         "RequiresCompatibilities": [
-          "FARGATE"
+          "EC2"
         ],
         "TaskRoleArn": {
           "Fn::GetAtt": [
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
@@ -3,7 +3,7 @@ import * as cdk from '@aws-cdk/core';
 import * as ecs from '../../lib';
 
 const app = new cdk.App();
-const stack = new cdk.Stack(app, 'aws-ecs-integ-secret-json-key');
+const stack = new cdk.Stack(app, 'aws-ecs-integ-secret-json-field');
 
 const secret = new secretsmanager.Secret(stack, 'Secret', {
   generateSecretString: {
@@ -12,13 +12,11 @@ const secret = new secretsmanager.Secret(stack, 'Secret', {
   },
 });
 
-const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef', {
-  memoryLimitMiB: 1024,
-  cpu: 512,
-});
+const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
 
 taskDefinition.addContainer('web', {
   image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
+  memoryLimitMiB: 256,
   secrets: {
     PASSWORD: ecs.Secret.fromSecretsManager(secret, 'password'),
   },
diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.expected.json b/packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.expected.json
@@ -0,0 +1,109 @@
+{
+  "Resources": {
+    "SecretA720EF05": {
+      "Type": "AWS::SecretsManager::Secret",
+      "Properties": {
+        "GenerateSecretString": {
+          "GenerateStringKey": "password",
+          "SecretStringTemplate": "{\"username\":\"user\"}"
+        }
+      }
+    },
+    "TaskDefTaskRole1EDB4A67": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "AssumeRolePolicyDocument": {
+          "Statement": [
+            {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": {
+                "Service": "ecs-tasks.amazonaws.com"
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "TaskDef54694570": {
+      "Type": "AWS::ECS::TaskDefinition",
+      "Properties": {
+        "ContainerDefinitions": [
+          {
+            "Essential": true,
+            "Image": "amazon/amazon-ecs-sample",
+            "Name": "web",
+            "Secrets": [
+              {
+                "Name": "SECRET",
+                "ValueFrom": {
+                  "Ref": "SecretA720EF05"
+                }
+              }
+            ]
+          }
+        ],
+        "Cpu": "256",
+        "ExecutionRoleArn": {
+          "Fn::GetAtt": [
+            "TaskDefExecutionRoleB4775C97",
+            "Arn"
+          ]
+        },
+        "Family": "awsecsintegsecretTaskDef58AA207D",
+        "Memory": "512",
+        "NetworkMode": "awsvpc",
+        "RequiresCompatibilities": [
+          "FARGATE"
+        ],
+        "TaskRoleArn": {
+          "Fn::GetAtt": [
+            "TaskDefTaskRole1EDB4A67",
+            "Arn"
+          ]
+        }
+      }
+    },
+    "TaskDefExecutionRoleB4775C97": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "AssumeRolePolicyDocument": {
+          "Statement": [
+            {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": {
+                "Service": "ecs-tasks.amazonaws.com"
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "TaskDefExecutionRoleDefaultPolicy0DBB737A": {
+      "Type": "AWS::IAM::Policy",
+      "Properties": {
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "secretsmanager:GetSecretValue",
+              "Effect": "Allow",
+              "Resource": {
+                "Ref": "SecretA720EF05"
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        },
+        "PolicyName": "TaskDefExecutionRoleDefaultPolicy0DBB737A",
+        "Roles": [
+          {
+            "Ref": "TaskDefExecutionRoleB4775C97"
+          }
+        ]
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.ts b/packages/@aws-cdk/aws-ecs/test/fargate/integ.secret.ts
@@ -0,0 +1,24 @@
+import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
+import * as cdk from '@aws-cdk/core';
+import * as ecs from '../../lib';
+
+const app = new cdk.App();
+const stack = new cdk.Stack(app, 'aws-ecs-integ-secret');
+
+const secret = new secretsmanager.Secret(stack, 'Secret', {
+  generateSecretString: {
+    generateStringKey: 'password',
+    secretStringTemplate: JSON.stringify({ username: 'user' }),
+  },
+});
+
+const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef');
+
+taskDefinition.addContainer('web', {
+  image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
+  secrets: {
+    SECRET: ecs.Secret.fromSecretsManager(secret),
+  },
+});
+
+app.synth();
diff --git a/packages/@aws-cdk/aws-ecs/test/test.container-definition.ts b/packages/@aws-cdk/aws-ecs/test/test.container-definition.ts
@@ -843,6 +843,25 @@ export = {
 
   },
 
+  'throws when using a specific secret JSON field as environment variable for a Fargate task'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const taskDefinition = new ecs.FargateTaskDefinition(stack, 'TaskDef');
+
+    const secret = new secretsmanager.Secret(stack, 'Secret');
+
+    // THEN
+    test.throws(() => taskDefinition.addContainer('cont', {
+      image: ecs.ContainerImage.fromRegistry('test'),
+      memoryLimitMiB: 1024,
+      secrets: {
+        SECRET_KEY: ecs.Secret.fromSecretsManager(secret, 'specificKey'),
+      },
+    }), /Cannot specify secret JSON field for a task using the FARGATE launch type/);
+
+    test.done();
+  },
+
   'can add AWS logging to container definition'(test: Test) {
     // GIVEN
     const stack = new cdk.Stack();

Original file line number	Diff line number	Diff line change
`@@ -497,7 +497,7 @@ export class TaskDefinition extends TaskDefinitionBase {`
`497`	`497`	`}`
`498`	`498`	`}`
`499`	`499`
`500`		`- return this.containers.map(x => x.renderContainerDefinition(this));`
	`500`	`+ return this.containers.map(x => x.renderContainerDefinition());`
`501`	`501`	`}`
`502`	`502`	`}`
`503`	`503`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@`
`33`	`33`	`{`
`34`	`34`	`"Essential": true,`
`35`	`35`	`"Image": "amazon/amazon-ecs-sample",`
	`36`	`+ "Memory": 256,`
`36`	`37`	`"Name": "web",`
`37`	`38`	`"Secrets": [`
`38`	`39`	`{`
`@@ -52,18 +53,16 @@`
`52`	`53`	`]`
`53`	`54`	`}`
`54`	`55`	`],`
`55`		`- "Cpu": "512",`
`56`	`56`	`"ExecutionRoleArn": {`
`57`	`57`	`"Fn::GetAtt": [`
`58`	`58`	`"TaskDefExecutionRoleB4775C97",`
`59`	`59`	`"Arn"`
`60`	`60`	`]`
`61`	`61`	`},`
`62`		`- "Family": "awsecsintegsecretjsonkeyTaskDefC01C0E99",`
`63`		`- "Memory": "1024",`
`64`		`- "NetworkMode": "awsvpc",`
	`62`	`+ "Family": "awsecsintegsecretjsonfieldTaskDef1C2EE990",`
	`63`	`+ "NetworkMode": "bridge",`
`65`	`64`	`"RequiresCompatibilities": [`
`66`		`- "FARGATE"`
	`65`	`+ "EC2"`
`67`	`66`	`],`
`68`	`67`	`"TaskRoleArn": {`
`69`	`68`	`"Fn::GetAtt": [`