Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ecr): auto delete images on ECR repository containing manifest list #25391

Closed
wants to merge 4 commits into from
Closed

fix(ecr): auto delete images on ECR repository containing manifest list #25391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8dcf1f2
fix(aws-ecr): auto delete images on ECR repository containing manifes…
hkford May 2, 2023
ea57af2
fix two test cases: deletes all objects when the name changes on upda…
hkford May 2, 2023
9ead2af
fix test case: delete event where repo has many images does recurse a…
hkford May 2, 2023
dfa043c
Merge branch 'main' into fix-ecr-delete
hkford May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/aws-cdk-lib/aws-ecr/lib/auto-delete-images-handler/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,23 @@ async function emptyRepository(params: ECR.ListImagesRequest) {
const listedImages = await ecr.listImages(params).promise();

const imageIds = listedImages?.imageIds ?? [];
const imageIdsTagged = imageIds.filter((imageId) => 'imageTag' in imageId) ?? [];

const nextToken = listedImages.nextToken ?? null;
if (imageIds.length === 0) {
return;
}

if (imageIdsTagged.length !== 0) {
await ecr.batchDeleteImage({
repositoryName: params.repositoryName,
imageIds: imageIdsTagged,
}).promise();
}

await ecr.batchDeleteImage({
repositoryName: params.repositoryName,
imageIds,
imageIds: imageIds,
}).promise();

if (nextToken) {
Expand Down
59 changes: 53 additions & 6 deletions packages/aws-cdk-lib/aws-ecr/test/auto-delete-images-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,15 @@ test('deletes all objects when the name changes on update event', async () => {
// THEN
expect(mockECRClient.listImages).toHaveBeenCalledTimes(1);
expect(mockECRClient.listImages).toHaveBeenCalledWith({ repositoryName: 'MyRepo' });
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledTimes(1);
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledWith({
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledTimes(2);
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(1, {
repositoryName: 'MyRepo',
imageIds: [
{ imageDigest: 'ImageDigest1', imageTag: 'ImageTag1' },
{ imageDigest: 'ImageDigest2', imageTag: 'ImageTag2' },
],
});
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(2, {
repositoryName: 'MyRepo',
imageIds: [
{ imageDigest: 'ImageDigest1', imageTag: 'ImageTag1' },
Expand Down Expand Up @@ -218,8 +225,21 @@ test('deletes all images on delete event', async () => {
// THEN
expect(mockECRClient.listImages).toHaveBeenCalledTimes(1);
expect(mockECRClient.listImages).toHaveBeenCalledWith({ repositoryName: 'MyRepo' });
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledTimes(1);
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledWith({
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledTimes(2);
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(1, {
repositoryName: 'MyRepo',
imageIds: [
{
imageTag: 'tag1',
imageDigest: 'sha256-1',
},
{
imageTag: 'tag2',
imageDigest: 'sha256-2',
},
],
});
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(2, {
repositoryName: 'MyRepo',
imageIds: [
{
Expand Down Expand Up @@ -286,7 +306,8 @@ test('delete event where repo has many images does recurse appropriately', async
],
nextToken: 'token1',
})
.mockResolvedValueOnce(undefined) // batchDeleteImage() call
.mockResolvedValueOnce(undefined) // batchDeleteImage() call for tagged images
.mockResolvedValueOnce(undefined) // batchDeleteImage() call for all images
.mockResolvedValueOnce({ // listedImages() call
imageIds: [
{
Expand Down Expand Up @@ -314,7 +335,7 @@ test('delete event where repo has many images does recurse appropriately', async
expect(mockECRClient.describeRepositories).toHaveBeenCalledTimes(1);
expect(mockECRClient.listImages).toHaveBeenCalledTimes(2);
expect(mockECRClient.listImages).toHaveBeenCalledWith({ repositoryName: 'MyRepo' });
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledTimes(2);
expect(mockECRClient.batchDeleteImage).toHaveBeenCalledTimes(4);
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(1, {
repositoryName: 'MyRepo',
imageIds: [
Expand All @@ -329,6 +350,32 @@ test('delete event where repo has many images does recurse appropriately', async
],
});
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(2, {
repositoryName: 'MyRepo',
imageIds: [
{
imageTag: 'tag1',
imageDigest: 'sha256-1',
},
{
imageTag: 'tag2',
imageDigest: 'sha256-2',
},
],
});
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(3, {
repositoryName: 'MyRepo',
imageIds: [
{
imageTag: 'tag3',
imageDigest: 'sha256-3',
},
{
imageTag: 'tag4',
imageDigest: 'sha256-4',
},
],
});
expect(mockECRClient.batchDeleteImage).toHaveBeenNthCalledWith(4, {
repositoryName: 'MyRepo',
imageIds: [
{
Expand Down