Skip to content

Commit d9346bc

Browse files
authored
fix(cli): unhandled nextToken returned by listImagesCommand in garbage collector for ECR (#32679)
### Issue # (if applicable) Closes #32498 ### Reason for this change When `listImagesCommand` returns nextToken in the `readRepoInBatches` function, nextToken is not passed as an argument for the subsequent `listImagesCommand` execution, causing `listImagesCommand` to continue executing. https://github.com/aws/aws-cdk/blob/v2.173.4/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts#L621 According to the `listImagesCommand` documentation, if maxResults is not specified, a maximum of 100 images will be returned, so this bug requires at least 100 images in the asset repository. https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-ecr/Interface/ListImagesCommandInput/ #### Reproduction Steps The following bash script and Dockerfile saved locally and executed, will push 120 container images to the asset repository. ```bash #!/usr/bin/env bash set -eu ACCOUNT_ID="your account id" REGION="your region" REPO_NAME="cdk-hnb659fds-container-assets-${ACCOUNT_ID}-${REGION}" IMAGE_NAME="test-image" AWS_PROFILE="your AWS profile" echo "Logging in to ECR..." aws ecr get-login-password --region "${REGION}" --profile "${AWS_PROFILE}" \ | docker login --username AWS --password-stdin "${ACCOUNT_ID}.dkr.ecr.${REGION}.amazonaws.com" for i in $(seq 1 120); do hash=$(head -c 32 /dev/urandom | xxd -p -c 64) echo "Building and pushing image with tag: ${hash}" touch "${i}.txt" docker build \ --build-arg BUILD_NO="${i}" \ -t "${IMAGE_NAME}:${i}" \ . docker tag "${IMAGE_NAME}:${i}" \ "${ACCOUNT_ID}.dkr.ecr.${REGION}.amazonaws.com/${REPO_NAME}:${hash}" docker push \ "${ACCOUNT_ID}.dkr.ecr.${REGION}.amazonaws.com/${REPO_NAME}:${hash}" rm "${i}.txt" sleep 0.01 done echo "Done!" ``` ```dockerfile FROM scratch ARG BUILD_NO ENV BUILD_NO=${BUILD_NO} COPY ${BUILD_NO}.txt / ``` You can reproduce this bug by running the following command after the images have been pushed. ```bash $ cdk gc aws://{account id}/{region} --type ecr --unstable=gc --created-buffer-days 0 --action full --confirm=true ``` ### Description of changes Fix the problem of correctly handling nextToken when executing `listImagesCommand` in the `readRepoInBatches` function. ### Describe any new or updated permissions being added Nothing. ### Description of how you validated changes Verifying that this bug has been fixed using the CLI integration tests is difficult, so only unit tests are added. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 318e725 commit d9346bc

File tree

2 files changed

+86
-0
lines changed

2 files changed

+86
-0
lines changed

packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts

+1
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,7 @@ export class GarbageCollector {
621621
while (batch.length < batchSize) {
622622
const response = await ecr.listImages({
623623
repositoryName: repo,
624+
nextToken: continuationToken,
624625
});
625626

626627
// No images in the repository

packages/aws-cdk/test/api/garbage-collection.test.ts

+85
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,91 @@ describe('ECR Garbage Collection', () => {
571571
imageTag: expect.stringContaining(`1-${ECR_ISOLATED_TAG}`),
572572
});
573573
});
574+
575+
test('listImagesCommand returns nextToken', async () => {
576+
// This test is to ensure that the garbage collector can handle paginated responses from the ECR API
577+
// If not handled correctly, the garbage collector will continue to make requests to the ECR API
578+
mockTheToolkitInfo({
579+
Outputs: [
580+
{
581+
OutputKey: 'BootstrapVersion',
582+
OutputValue: '999',
583+
},
584+
],
585+
});
586+
587+
prepareDefaultEcrMock();
588+
ecrClient.on(ListImagesCommand).resolves({ // default response
589+
imageIds: [
590+
{
591+
imageDigest: 'digest1',
592+
imageTag: 'abcde',
593+
},
594+
{
595+
imageDigest: 'digest2',
596+
imageTag: 'fghij',
597+
},
598+
],
599+
nextToken: 'nextToken',
600+
}).on(ListImagesCommand, { // response when nextToken is provided
601+
repositoryName: 'REPO_NAME',
602+
nextToken: 'nextToken',
603+
}).resolves({
604+
imageIds: [
605+
{
606+
imageDigest: 'digest3',
607+
imageTag: 'klmno',
608+
},
609+
],
610+
});
611+
ecrClient.on(BatchGetImageCommand).resolvesOnce({
612+
images: [
613+
{ imageId: { imageDigest: 'digest1' } },
614+
{ imageId: { imageDigest: 'digest2' } },
615+
],
616+
}).resolvesOnce({
617+
images: [
618+
{ imageId: { imageDigest: 'digest3' } },
619+
],
620+
});
621+
ecrClient.on(DescribeImagesCommand).resolvesOnce({
622+
imageDetails: [
623+
{
624+
imageDigest: 'digest1',
625+
imageTags: ['abcde'],
626+
imagePushedAt: daysInThePast(100),
627+
imageSizeInBytes: 1_000_000_000,
628+
},
629+
{ imageDigest: 'digest2', imageTags: ['fghij'], imagePushedAt: daysInThePast(10), imageSizeInBytes: 300_000_000 },
630+
],
631+
}).resolvesOnce({
632+
imageDetails: [
633+
{ imageDigest: 'digest3', imageTags: ['klmno'], imagePushedAt: daysInThePast(2), imageSizeInBytes: 100 },
634+
],
635+
});
636+
prepareDefaultCfnMock();
637+
638+
garbageCollector = gc({
639+
type: 'ecr',
640+
rollbackBufferDays: 0,
641+
action: 'full',
642+
});
643+
await garbageCollector.garbageCollect();
644+
645+
expect(ecrClient).toHaveReceivedCommandTimes(DescribeImagesCommand, 2);
646+
expect(ecrClient).toHaveReceivedCommandTimes(ListImagesCommand, 4);
647+
648+
// no tagging
649+
expect(ecrClient).toHaveReceivedCommandTimes(PutImageCommand, 0);
650+
651+
expect(ecrClient).toHaveReceivedCommandWith(BatchDeleteImageCommand, {
652+
repositoryName: 'REPO_NAME',
653+
imageIds: [
654+
{ imageDigest: 'digest2' },
655+
{ imageDigest: 'digest3' },
656+
],
657+
});
658+
});
574659
});
575660

576661
describe('CloudFormation API calls', () => {

0 commit comments

Comments
 (0)