From 12c844dd3dfca8a8b6af9d8051eac29f747b2640 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Tue, 13 Jun 2023 18:13:30 -0400 Subject: [PATCH 1/3] Update repository.ts --- .../aws-cdk-lib/aws-ecr/lib/repository.ts | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts index 9338a9c73e781..11c6daa320179 100644 --- a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts +++ b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts @@ -863,18 +863,17 @@ export class Repository extends RepositoryBase { 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], - }, + }); + + provider.addToRolePolicy({ + Effect: 'Allow', + Action: [ + 'ecr:BatchDeleteImage', + 'ecr:DescribeRepositories', + 'ecr:ListImages', + 'ecr:ListTagsForResource', ], + Resource: [this._resource.attrArn], }); const customResource = new CustomResource(this, 'AutoDeleteImagesCustomResource', { From 63fbce5d9985454475312f1a203bf7cdad2f087b Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 13 Jun 2023 18:20:16 -0400 Subject: [PATCH 2/3] add test --- .../aws-ecr/test/repository.test.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts index f192a76c06ef1..db7777e5ddca2 100644 --- 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,76 @@ 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', + ], + }, + ], + }, + { + Effect: 'Allow', + Action: [ + 'ecr:BatchDeleteImage', + 'ecr:DescribeRepositories', + 'ecr:ListImages', + 'ecr:ListTagsForResource', + ], + Resource: [ + { + '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\'.'); + }); + }); }); From 97f824d049f77ab141ae087992450a3bbb675696 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Wed, 14 Jun 2023 15:24:56 -0400 Subject: [PATCH 3/3] lazily evaluate repository arns --- .../aws-cdk-lib/aws-ecr/lib/repository.ts | 36 ++++++++++++------- .../aws-ecr/test/repository.test.ts | 12 ------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts index 11c6daa320179..1cd39b8fe84bd 100644 --- 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,24 +858,33 @@ 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.`, }); - - provider.addToRolePolicy({ - 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, diff --git a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts index db7777e5ddca2..6b58bbb3e2d88 100644 --- a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts +++ b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts @@ -977,7 +977,6 @@ describe('repository', () => { }); }); - describe('when auto delete images is set to true', () => { test('permissions are correctly for multiple ecr repos', () => { const stack = new cdk.Stack(); @@ -1012,17 +1011,6 @@ describe('repository', () => { 'Arn', ], }, - ], - }, - { - Effect: 'Allow', - Action: [ - 'ecr:BatchDeleteImage', - 'ecr:DescribeRepositories', - 'ecr:ListImages', - 'ecr:ListTagsForResource', - ], - Resource: [ { 'Fn::GetAtt': [ 'Repo2730A8200',