From bd36c6c595784997ac4545ec9c5cdee1019d4f05 Mon Sep 17 00:00:00 2001
From: Penghao He <penghaoh@amazon.com>
Date: Wed, 2 Oct 2019 11:58:31 -0700
Subject: [PATCH] feat(ecs): Add warning message when pulling ECR image (#4334)

* Add warning message when pulling ECR image using fromRegistry()

* Refactor the regex to bind method, add token validation, and add more test case
---
 .../@aws-cdk/aws-ecs/lib/images/repository.ts | 16 +++++++-
 .../test/ec2/test.ec2-task-definition.ts      | 37 +++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/aws-ecs/lib/images/repository.ts b/packages/@aws-cdk/aws-ecs/lib/images/repository.ts
index cfccb6b4461bd..fff998aa59413 100644
--- a/packages/@aws-cdk/aws-ecs/lib/images/repository.ts
+++ b/packages/@aws-cdk/aws-ecs/lib/images/repository.ts
@@ -1,8 +1,15 @@
 import secretsmanager = require('@aws-cdk/aws-secretsmanager');
-import { Construct } from '@aws-cdk/core';
+import { Construct, Token } from '@aws-cdk/core';
 import { ContainerDefinition } from "../container-definition";
 import { ContainerImage, ContainerImageConfig } from "../container-image";
 
+/**
+ * Regex pattern to check if it is an ECR image URL.
+ *
+ * @experimental
+ */
+const ECR_IMAGE_REGEX = /(^[a-zA-Z0-9][a-zA-Z0-9-_]*).dkr.ecr.([a-zA-Z0-9][a-zA-Z0-9-_]*).amazonaws.com(.cn)?\/.*/;
+
 /**
  * The properties for an image hosted in a public or private repository.
  */
@@ -27,7 +34,12 @@ export class RepositoryImage extends ContainerImage {
     super();
   }
 
-  public bind(_scope: Construct, containerDefinition: ContainerDefinition): ContainerImageConfig {
+  public bind(scope: Construct, containerDefinition: ContainerDefinition): ContainerImageConfig {
+    // name could be a Token - in that case, skip validation altogether
+    if (!Token.isUnresolved(this.imageName) && ECR_IMAGE_REGEX.test(this.imageName)) {
+      scope.node.addWarning("Proper policies need to be attached before pulling from ECR repository, or use 'fromEcrRepository'.");
+    }
+
     if (this.props.credentials) {
       this.props.credentials.grantRead(containerDefinition.taskDefinition.obtainExecutionRole());
     }
diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-task-definition.ts b/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-task-definition.ts
index 3af32a234cc33..0b75d1e8dd6e5 100644
--- a/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-task-definition.ts
+++ b/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-task-definition.ts
@@ -472,6 +472,43 @@ export = {
       test.done();
     },
 
+    "warns when setting containers from ECR repository using fromRegistry method"(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+
+      const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef');
+
+      // WHEN
+      const container = taskDefinition.addContainer("web", {
+        image: ecs.ContainerImage.fromRegistry("ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY"),
+        memoryLimitMiB: 512
+      });
+
+      // THEN
+      test.deepEqual(container.node.metadata[0].data, "Proper policies need to be attached before pulling from ECR repository, or use 'fromEcrRepository'.");
+      test.done();
+    },
+
+    "warns when setting containers from ECR repository by creating a RepositoryImage class"(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+
+      const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef');
+
+      const repo = new ecs.RepositoryImage("ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY");
+
+      // WHEN
+      const container = taskDefinition.addContainer("web", {
+        image: repo,
+        memoryLimitMiB: 512
+      });
+
+      // THEN
+      test.deepEqual(container.node.metadata[0].data, "Proper policies need to be attached before pulling from ECR repository, or use 'fromEcrRepository'.");
+
+      test.done();
+    },
+
     "correctly sets containers from asset using default props"(test: Test) {
       // GIVEN
       const stack = new cdk.Stack();