address comments

aws · Jun 26, 2019 · fb6c5fb · fb6c5fb
1 parent 68c8e7b
commit fb6c5fb
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 9 deletions.
diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -805,7 +805,8 @@ export class Project extends ProjectBase {
 
     const hasEnvironmentVars = Object.keys(vars).length > 0;
 
-    if (isECRImage(this.buildImage.imageId)) {
+    // An image id is a token if and only if it's an ECR image
+    if (Token.isUnresolved(this.buildImage.imageId)) {
       this.attachEcrPermission();
     }
 
@@ -947,6 +948,12 @@ export enum ComputeType {
   LARGE = 'BUILD_GENERAL1_LARGE'
 }
 
+/**
+ *  The type of credentials AWS CodeBuild uses to pull images in your build. There are two valid values:
+ *  - CODEBUILD specifies that AWS CodeBuild uses its own credentials.
+ *  This requires that you modify your ECR repository policy to trust AWS CodeBuild's service principal.
+ *  - SERVICE_ROLE specifies that AWS CodeBuild uses your build project's service role.
+ */
 export enum ImagePullCredentialsType {
   CODEBUILD = 'CODEBUILD',
   SERVICE_ROLE = 'SERVICE_ROLE'
@@ -1266,10 +1273,3 @@ export enum BuildEnvironmentVariableType {
    */
   PARAMETER_STORE = 'PARAMETER_STORE'
 }
-
-function isECRImage(imageUri: string) {
-  if (!Token.isUnresolved(imageUri)) {
-    return /^(.+).dkr.ecr.(.+).amazonaws.com[.]{0,1}[a-z]{0,3}\/([^:]+):?.*$/.test(imageUri);
-  }
-  return false;
-}
diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.docker-asset.lit.expected.json b/packages/@aws-cdk/aws-codebuild/test/integ.docker-asset.lit.expected.json
@@ -235,6 +235,16 @@
       "Properties": {
         "PolicyDocument": {
           "Statement": [
+            {
+              "Action": [
+                "ecr:GetAutheticationToken",
+                "ecr:GetDownloadUrlForLayer",
+                "ecr:BatchGetImage",
+                "ecr:BatchCheckLayerAvailability"
+              ],
+              "Effect": "Allow",
+              "Resource": "*"
+            },
             {
               "Action": [
                 "logs:CreateLogGroup",

diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.docker-registry.lit.ts b/packages/@aws-cdk/aws-codebuild/test/integ.docker-registry.lit.ts
@@ -7,7 +7,7 @@ class TestStack extends cdk.Stack {
     super(scope, id);
 
     const secrets = secretsmanager.Secret.fromSecretArn(this, "MySecrets",
-    `arn:aws:secretsmanager:${this.region}:${this.account}:secret:my-secrets-123456`);
+      `arn:aws:secretsmanager:${this.region}:${this.account}:secret:my-secrets-123456`);
 
     new codebuild.Project(this, 'MyProject', {
       buildSpec: codebuild.BuildSpec.fromObject({

diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.ecr.lit.expected.json b/packages/@aws-cdk/aws-codebuild/test/integ.ecr.lit.expected.json
@@ -36,6 +36,16 @@
       "Properties": {
         "PolicyDocument": {
           "Statement": [
+            {
+              "Action": [
+                "ecr:GetAutheticationToken",
+                "ecr:GetDownloadUrlForLayer",
+                "ecr:BatchGetImage",
+                "ecr:BatchCheckLayerAvailability"
+              ],
+              "Effect": "Allow",
+              "Resource": "*"
+            },
             {
               "Action": [
                 "logs:CreateLogGroup",