Skip to content

Commit c121180

Browse files
authored
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*
1 parent 9c8f549 commit c121180

File tree

2 files changed

+83
-14
lines changed

2 files changed

+83
-14
lines changed

packages/aws-cdk-lib/aws-ecr/lib/repository.ts

+23-14
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424

2525
const AUTO_DELETE_IMAGES_RESOURCE_TYPE = 'Custom::ECRAutoDeleteImages';
2626
const AUTO_DELETE_IMAGES_TAG = 'aws-cdk:auto-delete-images';
27+
const REPO_ARN_SYMBOL = Symbol.for('@aws-cdk/aws-ecr.RepoArns');
2728

2829
/**
2930
* Represents an ECR repository.
@@ -857,26 +858,34 @@ export class Repository extends RepositoryBase {
857858
}
858859

859860
private enableAutoDeleteImages() {
860-
// Use a iam policy to allow the custom resource to list & delete
861-
// images in the repository and the ability to get all repositories to find the arn needed on delete.
861+
const firstTime = Stack.of(this).node.tryFindChild(`${AUTO_DELETE_IMAGES_RESOURCE_TYPE}CustomResourceProvider`) === undefined;
862862
const provider = CustomResourceProvider.getOrCreateProvider(this, AUTO_DELETE_IMAGES_RESOURCE_TYPE, {
863863
codeDirectory: path.join(__dirname, 'auto-delete-images-handler'),
864864
runtime: builtInCustomResourceProviderNodeRuntime(this),
865865
description: `Lambda function for auto-deleting images in ${this.repositoryName} repository.`,
866-
policyStatements: [
867-
{
868-
Effect: 'Allow',
869-
Action: [
870-
'ecr:BatchDeleteImage',
871-
'ecr:DescribeRepositories',
872-
'ecr:ListImages',
873-
'ecr:ListTagsForResource',
874-
],
875-
Resource: [this._resource.attrArn],
876-
},
877-
],
878866
});
879867

868+
if (firstTime) {
869+
const repoArns = [this._resource.attrArn];
870+
(provider as any)[REPO_ARN_SYMBOL] = repoArns;
871+
872+
// Use a iam policy to allow the custom resource to list & delete
873+
// images in the repository and the ability to get all repositories to find the arn needed on delete.
874+
// We lazily produce a list of repositories associated with this custom resource provider.
875+
provider.addToRolePolicy({
876+
Effect: 'Allow',
877+
Action: [
878+
'ecr:BatchDeleteImage',
879+
'ecr:DescribeRepositories',
880+
'ecr:ListImages',
881+
'ecr:ListTagsForResource',
882+
],
883+
Resource: Lazy.list({ produce: () => repoArns }),
884+
});
885+
} else {
886+
(provider as any)[REPO_ARN_SYMBOL].push(this._resource.attrArn);
887+
}
888+
880889
const customResource = new CustomResource(this, 'AutoDeleteImagesCustomResource', {
881890
resourceType: AUTO_DELETE_IMAGES_RESOURCE_TYPE,
882891
serviceToken: provider.serviceToken,

packages/aws-cdk-lib/aws-ecr/test/repository.test.ts

+60
Original file line numberDiff line numberDiff line change
@@ -976,4 +976,64 @@ describe('repository', () => {
976976
});
977977
});
978978
});
979+
980+
describe('when auto delete images is set to true', () => {
981+
test('permissions are correctly for multiple ecr repos', () => {
982+
const stack = new cdk.Stack();
983+
new ecr.Repository(stack, 'Repo1', {
984+
autoDeleteImages: true,
985+
removalPolicy: cdk.RemovalPolicy.DESTROY,
986+
});
987+
new ecr.Repository(stack, 'Repo2', {
988+
autoDeleteImages: true,
989+
removalPolicy: cdk.RemovalPolicy.DESTROY,
990+
});
991+
992+
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
993+
Policies: [
994+
{
995+
PolicyName: 'Inline',
996+
PolicyDocument: {
997+
Version: '2012-10-17',
998+
Statement: [
999+
{
1000+
Effect: 'Allow',
1001+
Action: [
1002+
'ecr:BatchDeleteImage',
1003+
'ecr:DescribeRepositories',
1004+
'ecr:ListImages',
1005+
'ecr:ListTagsForResource',
1006+
],
1007+
Resource: [
1008+
{
1009+
'Fn::GetAtt': [
1010+
'Repo1DBD717D9',
1011+
'Arn',
1012+
],
1013+
},
1014+
{
1015+
'Fn::GetAtt': [
1016+
'Repo2730A8200',
1017+
'Arn',
1018+
],
1019+
},
1020+
],
1021+
},
1022+
],
1023+
},
1024+
},
1025+
],
1026+
});
1027+
});
1028+
1029+
test('synth fails when removal policy is not DESTROY', () => {
1030+
const stack = new cdk.Stack();
1031+
expect(() => {
1032+
new ecr.Repository(stack, 'Repo', {
1033+
autoDeleteImages: true,
1034+
removalPolicy: cdk.RemovalPolicy.RETAIN,
1035+
});
1036+
}).toThrowError('Cannot use \'autoDeleteImages\' property on a repository without setting removal policy to \'DESTROY\'.');
1037+
});
1038+
});
9791039
});

0 commit comments

Comments
 (0)