fix(ecr): autoDeleteImages fails on multiple repositories (#25964)

When setting `autoDeleteImages: true` for multiple repositories in the same stack, permissions to do the actual deleting only get added to the first one. This is because the policy statement is added inside of the `getOrCreateProvider` method, and that method ensures that the provider is only created once. Instead, this adds the policy statement on the provider itself, regardless of whether it was created or referenced. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Jun 15, 2023 · c121180 · c121180
1 parent 9c8f549
commit c121180
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 14 deletions.
diff --git a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts
@@ -24,6 +24,7 @@ import {
 
 const AUTO_DELETE_IMAGES_RESOURCE_TYPE = 'Custom::ECRAutoDeleteImages';
 const AUTO_DELETE_IMAGES_TAG = 'aws-cdk:auto-delete-images';
+const REPO_ARN_SYMBOL = Symbol.for('@aws-cdk/aws-ecr.RepoArns');
 
 /**
  * Represents an ECR repository.
@@ -857,26 +858,34 @@ export class Repository extends RepositoryBase {
   }
 
   private enableAutoDeleteImages() {
-    // Use a iam policy to allow the custom resource to list & delete
-    // images in the repository and the ability to get all repositories to find the arn needed on delete.
+    const firstTime = Stack.of(this).node.tryFindChild(`${AUTO_DELETE_IMAGES_RESOURCE_TYPE}CustomResourceProvider`) === undefined;
     const provider = CustomResourceProvider.getOrCreateProvider(this, AUTO_DELETE_IMAGES_RESOURCE_TYPE, {
       codeDirectory: path.join(__dirname, 'auto-delete-images-handler'),
       runtime: builtInCustomResourceProviderNodeRuntime(this),
       description: `Lambda function for auto-deleting images in ${this.repositoryName} repository.`,
-      policyStatements: [
-        {
-          Effect: 'Allow',
-          Action: [
-            'ecr:BatchDeleteImage',
-            'ecr:DescribeRepositories',
-            'ecr:ListImages',
-            'ecr:ListTagsForResource',
-          ],
-          Resource: [this._resource.attrArn],
-        },
-      ],
     });
 
+    if (firstTime) {
+      const repoArns = [this._resource.attrArn];
+      (provider as any)[REPO_ARN_SYMBOL] = repoArns;
+
+      // Use a iam policy to allow the custom resource to list & delete
+      // images in the repository and the ability to get all repositories to find the arn needed on delete.
+      // We lazily produce a list of repositories associated with this custom resource provider.
+      provider.addToRolePolicy({
+        Effect: 'Allow',
+        Action: [
+          'ecr:BatchDeleteImage',
+          'ecr:DescribeRepositories',
+          'ecr:ListImages',
+          'ecr:ListTagsForResource',
+        ],
+        Resource: Lazy.list({ produce: () => repoArns }),
+      });
+    } else {
+      (provider as any)[REPO_ARN_SYMBOL].push(this._resource.attrArn);
+    }
+
     const customResource = new CustomResource(this, 'AutoDeleteImagesCustomResource', {
       resourceType: AUTO_DELETE_IMAGES_RESOURCE_TYPE,
       serviceToken: provider.serviceToken,

diff --git a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts
@@ -976,4 +976,64 @@ describe('repository', () => {
       });
     });
   });
+
+  describe('when auto delete images is set to true', () => {
+    test('permissions are correctly for multiple ecr repos', () => {
+      const stack = new cdk.Stack();
+      new ecr.Repository(stack, 'Repo1', {
+        autoDeleteImages: true,
+        removalPolicy: cdk.RemovalPolicy.DESTROY,
+      });
+      new ecr.Repository(stack, 'Repo2', {
+        autoDeleteImages: true,
+        removalPolicy: cdk.RemovalPolicy.DESTROY,
+      });
+
+      Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
+        Policies: [
+          {
+            PolicyName: 'Inline',
+            PolicyDocument: {
+              Version: '2012-10-17',
+              Statement: [
+                {
+                  Effect: 'Allow',
+                  Action: [
+                    'ecr:BatchDeleteImage',
+                    'ecr:DescribeRepositories',
+                    'ecr:ListImages',
+                    'ecr:ListTagsForResource',
+                  ],
+                  Resource: [
+                    {
+                      'Fn::GetAtt': [
+                        'Repo1DBD717D9',
+                        'Arn',
+                      ],
+                    },
+                    {
+                      'Fn::GetAtt': [
+                        'Repo2730A8200',
+                        'Arn',
+                      ],
+                    },
+                  ],
+                },
+              ],
+            },
+          },
+        ],
+      });
+    });
+
+    test('synth fails when removal policy is not DESTROY', () => {
+      const stack = new cdk.Stack();
+      expect(() => {
+        new ecr.Repository(stack, 'Repo', {
+          autoDeleteImages: true,
+          removalPolicy: cdk.RemovalPolicy.RETAIN,
+        });
+      }).toThrowError('Cannot use \'autoDeleteImages\' property on a repository without setting removal policy to \'DESTROY\'.');
+    });
+  });
 });