From a17c6829d893707ee2f8bd0c3ab3c2c2aa26ffa4 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Mon, 21 Oct 2024 12:27:56 -0700 Subject: [PATCH 01/17] refactor s3 gc into its own function --- .../api/garbage-collection/garbage-collector.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index 7fe512805e0fc..d98f06f717a2f 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -162,7 +162,6 @@ export class GarbageCollector { // SDKs const sdk = (await this.props.sdkProvider.forEnvironment(this.props.resolvedEnvironment, Mode.ForWriting)).sdk; const cfn = sdk.cloudFormation(); - const s3 = sdk.s3(); const qualifier = await this.bootstrapQualifier(sdk, this.bootstrapStackName); const activeAssets = new ActiveAssetCache(); @@ -178,6 +177,19 @@ export class GarbageCollector { }); backgroundStackRefresh.start(); + try { + if (this.garbageCollectS3Assets) { + await this.garbageCollectS3(sdk, activeAssets, backgroundStackRefresh); + } + } catch (err: any) { + throw new Error(err); + } finally { + backgroundStackRefresh.stop(); + } + } + + public async garbageCollectS3(sdk: ISDK, activeAssets: ActiveAssetCache, backgroundStackRefresh: BackgroundStackRefresh) { + const s3 = sdk.s3(); const bucket = await this.bootstrapBucketName(sdk, this.bootstrapStackName); const numObjects = await this.numObjectsInBucket(s3, bucket); const printer = new ProgressPrinter(numObjects, 1000); @@ -267,7 +279,6 @@ export class GarbageCollector { } catch (err: any) { throw new Error(err); } finally { - backgroundStackRefresh.stop(); printer.stop(); } } From 97a0fc01262597b1de2d997180250882fcb27a42 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Mon, 21 Oct 2024 13:13:36 -0700 Subject: [PATCH 02/17] grab repository name and get total images --- .../lib/api/bootstrap/bootstrap-props.ts | 2 +- .../garbage-collection/garbage-collector.ts | 48 ++++++++++++++++--- packages/aws-cdk/lib/api/toolkit-info.ts | 11 ++++- .../aws-cdk/test/util/mock-toolkitinfo.ts | 3 ++ 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts index b575acb50fc78..47ce8e2a78532 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts @@ -1,7 +1,7 @@ import { Tag } from '../../cdk-toolkit'; export const BUCKET_NAME_OUTPUT = 'BucketName'; -export const REPOSITORY_NAME_OUTPUT = 'RepositoryName'; +export const REPOSITORY_NAME_OUTPUT = 'ImageRepositoryName'; export const BUCKET_DOMAIN_NAME_OUTPUT = 'BucketDomainName'; export const BOOTSTRAP_VERSION_OUTPUT = 'BootstrapVersion'; export const BOOTSTRAP_VERSION_RESOURCE = 'CdkBootstrapVersion'; diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index d98f06f717a2f..ec97db9eb0f0a 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -1,5 +1,5 @@ import * as cxapi from '@aws-cdk/cx-api'; -import { S3 } from 'aws-sdk'; +import { ECR, S3 } from 'aws-sdk'; import * as chalk from 'chalk'; import * as promptly from 'promptly'; import { debug, print } from '../../logging'; @@ -148,11 +148,6 @@ export class GarbageCollector { this.confirm = props.confirm ?? true; this.bootstrapStackName = props.bootstrapStackName ?? DEFAULT_TOOLKIT_STACK_NAME; - - // TODO: ECR garbage collection - if (this.garbageCollectEcrAssets) { - throw new Error('ECR garbage collection is not yet supported'); - } } /** @@ -181,6 +176,10 @@ export class GarbageCollector { if (this.garbageCollectS3Assets) { await this.garbageCollectS3(sdk, activeAssets, backgroundStackRefresh); } + + if (this.garbageCollectEcrAssets) { + await this.garbageCollectEcr(sdk, activeAssets, backgroundStackRefresh); + } } catch (err: any) { throw new Error(err); } finally { @@ -188,6 +187,21 @@ export class GarbageCollector { } } + /** + * Perform garbage collection on ECR assets + */ + public async garbageCollectEcr(sdk: ISDK, _activeAssets: ActiveAssetCache, _backgroundStackRefresh: BackgroundStackRefresh) { + const ecr = sdk.ecr(); + const repo = await this.bootstrapRepositoryName(sdk, this.bootstrapStackName); + const numImages = await this.numImagesInRepo(ecr, repo); + // const printer = new ProgressPrinter(numImages, 1000); + + debug(`Found bootstrap repo ${repo} ${numImages}`); + } + + /** + * Perform garbage collection on S3 assets + */ public async garbageCollectS3(sdk: ISDK, activeAssets: ActiveAssetCache, backgroundStackRefresh: BackgroundStackRefresh) { const s3 = sdk.s3(); const bucket = await this.bootstrapBucketName(sdk, this.bootstrapStackName); @@ -388,6 +402,11 @@ export class GarbageCollector { return info.bucketName; } + private async bootstrapRepositoryName(sdk: ISDK, bootstrapStackName: string): Promise { + const info = await ToolkitInfo.lookup(this.props.resolvedEnvironment, sdk, bootstrapStackName); + return info.repositoryName; + } + private async bootstrapQualifier(sdk: ISDK, bootstrapStackName: string): Promise { const info = await ToolkitInfo.lookup(this.props.resolvedEnvironment, sdk, bootstrapStackName); return info.bootstrapStack.parameters.Qualifier; @@ -410,6 +429,23 @@ export class GarbageCollector { return totalCount; } + private async numImagesInRepo(ecr: ECR, repo: string): Promise { + let totalCount = 0; + let nextToken: string | undefined; + + do { + const response = await ecr.listImages({ + repositoryName: repo, + nextToken: nextToken, + }).promise(); + + totalCount += response.imageIds?.length ?? 0; + nextToken = response.nextToken; + } while (nextToken); + + return totalCount; + } + /** * Generator function that reads objects from the S3 Bucket in batches. */ diff --git a/packages/aws-cdk/lib/api/toolkit-info.ts b/packages/aws-cdk/lib/api/toolkit-info.ts index 795a6d694420f..d16b0d7116aa4 100644 --- a/packages/aws-cdk/lib/api/toolkit-info.ts +++ b/packages/aws-cdk/lib/api/toolkit-info.ts @@ -2,7 +2,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import { ISDK } from './aws-auth'; import { debug } from '../logging'; -import { BOOTSTRAP_VERSION_OUTPUT, BUCKET_DOMAIN_NAME_OUTPUT, BUCKET_NAME_OUTPUT, BOOTSTRAP_VARIANT_PARAMETER, DEFAULT_BOOTSTRAP_VARIANT } from './bootstrap/bootstrap-props'; +import { BOOTSTRAP_VERSION_OUTPUT, BUCKET_DOMAIN_NAME_OUTPUT, BUCKET_NAME_OUTPUT, BOOTSTRAP_VARIANT_PARAMETER, DEFAULT_BOOTSTRAP_VARIANT, REPOSITORY_NAME_OUTPUT } from './bootstrap/bootstrap-props'; import { stabilizeStack, CloudFormationStack } from './util/cloudformation'; export const DEFAULT_TOOLKIT_STACK_NAME = 'CDKToolkit'; @@ -73,6 +73,7 @@ export abstract class ToolkitInfo { public abstract readonly found: boolean; public abstract readonly bucketUrl: string; public abstract readonly bucketName: string; + public abstract readonly repositoryName: string; public abstract readonly version: number; public abstract readonly variant: string; public abstract readonly bootstrapStack: CloudFormationStack; @@ -100,6 +101,10 @@ class ExistingToolkitInfo extends ToolkitInfo { return this.requireOutput(BUCKET_NAME_OUTPUT); } + public get repositoryName() { + return this.requireOutput(REPOSITORY_NAME_OUTPUT); + } + public get version() { return parseInt(this.bootstrapStack.outputs[BOOTSTRAP_VERSION_OUTPUT] ?? '0', 10); } @@ -161,6 +166,10 @@ class BootstrapStackNotFoundInfo extends ToolkitInfo { throw new Error(this.errorMessage); } + public get repositoryName(): string { + throw new Error(this.errorMessage); + } + public get version(): number { throw new Error(this.errorMessage); } diff --git a/packages/aws-cdk/test/util/mock-toolkitinfo.ts b/packages/aws-cdk/test/util/mock-toolkitinfo.ts index 0f4de75fcddbe..f45a4aa6ab901 100644 --- a/packages/aws-cdk/test/util/mock-toolkitinfo.ts +++ b/packages/aws-cdk/test/util/mock-toolkitinfo.ts @@ -5,6 +5,7 @@ import { CloudFormationStack } from '../../lib/api/util/cloudformation'; export interface MockToolkitInfoProps { readonly bucketName?: string; readonly bucketUrl?: string; + readonly repositoryName?: string; readonly version?: number; readonly bootstrapStack?: CloudFormationStack; } @@ -26,6 +27,7 @@ export class MockToolkitInfo extends ToolkitInfo { public readonly found = true; public readonly bucketUrl: string; public readonly bucketName: string; + public readonly repositoryName: string; public readonly version: number; public readonly variant: string; public readonly stackName = 'MockBootstrapStack'; @@ -37,6 +39,7 @@ export class MockToolkitInfo extends ToolkitInfo { this.bucketName = props.bucketName ?? 'MockToolkitBucketName'; this.bucketUrl = props.bucketUrl ?? `https://${this.bucketName}.s3.amazonaws.com/`; + this.repositoryName = props.repositoryName ?? 'MockToolkitRepositoryName'; this.version = props.version ?? 1; this.variant = DEFAULT_BOOTSTRAP_VARIANT; this._bootstrapStack = props.bootstrapStack; From 7fdd0dd3651c9db31ee19b3947a37a62cfd0aeed Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Mon, 21 Oct 2024 15:02:34 -0700 Subject: [PATCH 03/17] get deletabes/taggables/untaggables --- .../garbage-collection/garbage-collector.ts | 131 +++++++++++++++++- 1 file changed, 127 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index ec97db9eb0f0a..fbf461acba582 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -16,6 +16,30 @@ const ISOLATED_TAG = 'aws-cdk:isolated'; const P_LIMIT = 50; const DAY = 24 * 60 * 60 * 1000; // Number of milliseconds in a day +export class ImageAsset { + public constructor(public readonly digest: string, public readonly size: number, public readonly tags: string[]) {} + + private getTag(tagPrefix: string) { + return this.tags.find(t => t.startsWith(tagPrefix)); + } + + private hasTag(tagPrefix: string) { + return this.tags.some(t => t.startsWith(tagPrefix)); + } + + public hasIsolatedTag() { + return this.hasTag(ISOLATED_TAG); + } + + public isolatedTagBefore(date: Date) { + const tagValue = this.getTag(ISOLATED_TAG); + if (!tagValue || tagValue == '') { + return false; + } + return new Date(tagValue) < date; + } +} + export class S3Asset { private cached_tags: S3.TagSet | undefined = undefined; @@ -190,13 +214,59 @@ export class GarbageCollector { /** * Perform garbage collection on ECR assets */ - public async garbageCollectEcr(sdk: ISDK, _activeAssets: ActiveAssetCache, _backgroundStackRefresh: BackgroundStackRefresh) { + public async garbageCollectEcr(sdk: ISDK, activeAssets: ActiveAssetCache, backgroundStackRefresh: BackgroundStackRefresh) { const ecr = sdk.ecr(); const repo = await this.bootstrapRepositoryName(sdk, this.bootstrapStackName); const numImages = await this.numImagesInRepo(ecr, repo); - // const printer = new ProgressPrinter(numImages, 1000); + const printer = new ProgressPrinter(numImages, 1000); debug(`Found bootstrap repo ${repo} ${numImages}`); + + try { + // const batches = 1; + const batchSize = 1000; + const currentTime = Date.now(); + const graceDays = this.props.rollbackBufferDays; + + debug(`Parsing through ${numImages} images in batches`); + + for await (const batch of this.readRepoInBatches(ecr, repo, batchSize, currentTime)) { + await backgroundStackRefresh.noOlderThan(600_000); // 10 mins + printer.start(); + + const { included: isolated, excluded: notIsolated } = partition(batch, asset => !asset.tags.some(t => activeAssets.contains(t))); + + console.log(isolated, notIsolated); + + debug(`${isolated.length} isolated images`); + debug(`${notIsolated.length} not isolated images`); + debug(`${batch.length} images total`); + + let deletables: ImageAsset[] = isolated; + let taggables: ImageAsset[] = []; + let untaggables: ImageAsset[] = []; + + if (graceDays > 0) { + debug('Filtering out images that are not old enough to delete'); + + // We delete images that are not referenced in ActiveAssets and have the Isolated Tag with a date + // earlier than the current time - grace period. + deletables = isolated.filter(img => img.isolatedTagBefore(new Date(currentTime - (graceDays * DAY)))); + + // We tag images that are not referenced in ActiveAssets and do not have the Isolated Tag. + taggables = isolated.filter(img => !img.hasIsolatedTag()); + + // We untag images that are referenced in ActiveAssets and currently have the Isolated Tag. + untaggables = notIsolated.filter(img => img.hasIsolatedTag()); + } + + console.log(deletables, taggables, untaggables); + } + } catch (err: any) { + throw new Error(err); + } finally { + printer.stop(); + } } /** @@ -211,7 +281,6 @@ export class GarbageCollector { debug(`Found bootstrap bucket ${bucket}`); try { - const batches = 1; const batchSize = 1000; const currentTime = Date.now(); const graceDays = this.props.rollbackBufferDays; @@ -223,7 +292,6 @@ export class GarbageCollector { // where gc is run for the first time on a long-standing bucket where ~100% of objects are isolated. for await (const batch of this.readBucketInBatches(s3, bucket, batchSize, currentTime)) { await backgroundStackRefresh.noOlderThan(600_000); // 10 mins - print(chalk.green(`Processing batch ${batches} of ${Math.floor(numObjects / batchSize) + 1}`)); printer.start(); const { included: isolated, excluded: notIsolated } = partition(batch, asset => !activeAssets.contains(asset.fileName())); @@ -446,6 +514,49 @@ export class GarbageCollector { return totalCount; } + private async *readRepoInBatches(ecr: ECR, repo: string, batchSize: number = 1000, currentTime: number): AsyncGenerator { + let continuationToken: string | undefined; + + do { + const batch: ImageAsset[] = []; + + while (batch.length < batchSize) { + const response = await ecr.listImages({ + repositoryName: repo, + }).promise(); + + // map unique image digest to (possibly multiple) tags + const images = imageMap(response.imageIds ?? []); + const imageIds = Object.keys(images).map(key => ({ + imageDigest: key, + })); + + const imagesInfo = await ecr.describeImages({ + imageIds, + repositoryName: repo, + }).promise(); + + for (const image of imagesInfo.imageDetails ?? []) { + const lastModified = image.imagePushedAt ?? new Date(currentTime); + // Store the image if it was pushed earlier than today - createdBufferDays + if (image.imageDigest && lastModified < new Date(currentTime - (this.props.createdBufferDays * DAY))) { + batch.push(new ImageAsset(image.imageDigest, image.imageSizeInBytes ?? 0, image.imageTags ?? [])); + } + + console.log(image.imageDigest, image.imagePushedAt); + } + + continuationToken = response.nextToken; + + if (!continuationToken) break; // No more images to fetch + } + + if (batch.length > 0) { + yield batch; + } + } while (continuationToken); + } + /** * Generator function that reads objects from the S3 Bucket in batches. */ @@ -499,4 +610,16 @@ function partition(xs: Iterable, pred: (x: A) => boolean): { included: A[] } return result; +} + +function imageMap(imageIds: ECR.ImageIdentifierList) { + const images: Record = {}; + for (const image of imageIds ?? []) { + if (!image.imageDigest || !image.imageTag) { continue; } + if (!images[image.imageDigest]) { + images[image.imageDigest] = []; + } + images[image.imageDigest].push(image.imageTag); + } + return images; } \ No newline at end of file From 1865e9bcfe9bd81a101e52c044ddad6a27bfbfaa Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Mon, 21 Oct 2024 16:12:02 -0700 Subject: [PATCH 04/17] delete works too --- .../garbage-collection/garbage-collector.ts | 220 ++++++++++++++---- .../garbage-collection/progress-printer.ts | 56 ++--- 2 files changed, 198 insertions(+), 78 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index fbf461acba582..112421a379581 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -12,12 +12,19 @@ import { ActiveAssetCache, BackgroundStackRefresh, refreshStacks } from './stack // eslint-disable-next-line @typescript-eslint/no-require-imports const pLimit: typeof import('p-limit') = require('p-limit'); -const ISOLATED_TAG = 'aws-cdk:isolated'; +const ISOLATED_TAG = 'aws-cdk.isolated'; const P_LIMIT = 50; const DAY = 24 * 60 * 60 * 1000; // Number of milliseconds in a day +export type GcAsset = ImageAsset | ObjectAsset; + export class ImageAsset { - public constructor(public readonly digest: string, public readonly size: number, public readonly tags: string[]) {} + public constructor( + public readonly digest: string, + public readonly size: number, + public readonly tags: string[], + public readonly manifest: string, + ) {} private getTag(tagPrefix: string) { return this.tags.find(t => t.startsWith(tagPrefix)); @@ -31,8 +38,13 @@ export class ImageAsset { return this.hasTag(ISOLATED_TAG); } + public getIsolatedTag() { + return this.getTag(ISOLATED_TAG); + } + public isolatedTagBefore(date: Date) { - const tagValue = this.getTag(ISOLATED_TAG); + const tagValue = this.getIsolatedTag()?.split('-')[2]; + console.log(tagValue); if (!tagValue || tagValue == '') { return false; } @@ -40,7 +52,7 @@ export class ImageAsset { } } -export class S3Asset { +export class ObjectAsset { private cached_tags: S3.TagSet | undefined = undefined; public constructor(private readonly bucket: string, public readonly key: string, public readonly size: number) {} @@ -220,7 +232,7 @@ export class GarbageCollector { const numImages = await this.numImagesInRepo(ecr, repo); const printer = new ProgressPrinter(numImages, 1000); - debug(`Found bootstrap repo ${repo} ${numImages}`); + debug(`Found bootstrap repo ${repo} with ${numImages} images`); try { // const batches = 1; @@ -236,8 +248,6 @@ export class GarbageCollector { const { included: isolated, excluded: notIsolated } = partition(batch, asset => !asset.tags.some(t => activeAssets.contains(t))); - console.log(isolated, notIsolated); - debug(`${isolated.length} isolated images`); debug(`${notIsolated.length} not isolated images`); debug(`${batch.length} images total`); @@ -260,7 +270,24 @@ export class GarbageCollector { untaggables = notIsolated.filter(img => img.hasIsolatedTag()); } - console.log(deletables, taggables, untaggables); + debug(`${deletables.length} deletable assets`); + debug(`${taggables.length} taggable assets`); + debug(`${untaggables.length} assets to untag`); + + if (this.permissionToDelete && deletables.length > 0) { + await this.confirmationPrompt(printer, deletables); + await this.parallelDeleteEcr(ecr, repo, deletables, printer); + } + + if (this.permissionToTag && taggables.length > 0) { + await this.parallelTagEcr(ecr, repo, taggables, currentTime, printer); + } + + if (this.permissionToTag && untaggables.length > 0) { + await this.parallelUntagEcr(ecr, repo, untaggables); + } + + printer.reportScannedAsset(batch.length); } } catch (err: any) { throw new Error(err); @@ -278,7 +305,7 @@ export class GarbageCollector { const numObjects = await this.numObjectsInBucket(s3, bucket); const printer = new ProgressPrinter(numObjects, 1000); - debug(`Found bootstrap bucket ${bucket}`); + debug(`Found bootstrap bucket ${bucket} with ${numObjects} objects`); try { const batchSize = 1000; @@ -300,9 +327,9 @@ export class GarbageCollector { debug(`${notIsolated.length} not isolated assets`); debug(`${batch.length} objects total`); - let deletables: S3Asset[] = isolated; - let taggables: S3Asset[] = []; - let untaggables: S3Asset[] = []; + let deletables: ObjectAsset[] = isolated; + let taggables: ObjectAsset[] = []; + let untaggables: ObjectAsset[] = []; if (graceDays > 0) { debug('Filtering out assets that are not old enough to delete'); @@ -324,39 +351,19 @@ export class GarbageCollector { debug(`${untaggables.length} assets to untag`); if (this.permissionToDelete && deletables.length > 0) { - if (this.confirm) { - const message = [ - `Found ${deletables.length} objects to delete based off of the following criteria:`, - `- objects have been isolated for > ${this.props.rollbackBufferDays} days`, - `- objects were created > ${this.props.createdBufferDays} days ago`, - '', - 'Delete this batch (yes/no/delete-all)?', - ].join('\n'); - printer.pause(); - const response = await promptly.prompt(message, - { trim: true }, - ); - - // Anything other than yes/y/delete-all is treated as no - if (!response || !['yes', 'y', 'delete-all'].includes(response.toLowerCase())) { - throw new Error('Deletion aborted by user'); - } else if (response.toLowerCase() == 'delete-all') { - this.confirm = false; - } - } - printer.resume(); - await this.parallelDelete(s3, bucket, deletables, printer); + await this.confirmationPrompt(printer, deletables); + await this.parallelDeleteS3(s3, bucket, deletables, printer); } if (this.permissionToTag && taggables.length > 0) { - await this.parallelTag(s3, bucket, taggables, currentTime, printer); + await this.parallelTagS3(s3, bucket, taggables, currentTime, printer); } if (this.permissionToTag && untaggables.length > 0) { - await this.parallelUntag(s3, bucket, untaggables); + await this.parallelUntagS3(s3, bucket, untaggables); } - printer.reportScannedObjects(batch.length); + printer.reportScannedAsset(batch.length); } } catch (err: any) { throw new Error(err); @@ -365,7 +372,7 @@ export class GarbageCollector { } } - private async parallelReadAllTags(s3: S3, objects: S3Asset[]) { + private async parallelReadAllTags(s3: S3, objects: ObjectAsset[]) { const limit = pLimit(P_LIMIT); for (const obj of objects) { @@ -373,11 +380,33 @@ export class GarbageCollector { } } + /** + * Untag assets that were previously tagged, but now currently referenced. + * Since this is treated as an implementation detail, we do not print the results in the printer. + */ + private async parallelUntagEcr(ecr: ECR, repo: string, untaggables: ImageAsset[]) { + const limit = pLimit(P_LIMIT); + + for (const img of untaggables) { + const tag = img.getIsolatedTag(); + await limit(() => + ecr.batchDeleteImage({ + repositoryName: repo, + imageIds: [{ + imageTag: tag, + }] + }).promise(), + ); + } + + debug(`Untagged ${untaggables.length} assets`); + } + /** * Untag assets that were previously tagged, but now currently referenced. * Since this is treated as an implementation detail, we do not print the results in the printer. */ - private async parallelUntag(s3: S3, bucket: string, untaggables: S3Asset[]) { + private async parallelUntagS3(s3: S3, bucket: string, untaggables: ObjectAsset[]) { const limit = pLimit(P_LIMIT); for (const obj of untaggables) { @@ -404,11 +433,32 @@ export class GarbageCollector { debug(`Untagged ${untaggables.length} assets`); } + /** + * Tag images in parallel using p-limit + */ + private async parallelTagEcr(ecr: ECR, repo: string, taggables: ImageAsset[], date: number, printer: ProgressPrinter) { + const limit = pLimit(P_LIMIT); + + for (const img of taggables) { + await limit(() => + ecr.putImage({ + repositoryName: repo, + imageDigest: img.digest, + imageManifest: img.manifest, + imageTag: `${ISOLATED_TAG}-${String(date)}`, + }).promise(), + ); + } + + printer.reportTaggedAsset(taggables); + debug(`Tagged ${taggables.length} assets`); + } + /** * Tag objects in parallel using p-limit. The putObjectTagging API does not * support batch tagging so we must handle the parallelism client-side. */ - private async parallelTag(s3: S3, bucket: string, taggables: S3Asset[], date: number, printer: ProgressPrinter) { + private async parallelTagS3(s3: S3, bucket: string, taggables: ObjectAsset[], date: number, printer: ProgressPrinter) { const limit = pLimit(P_LIMIT); for (const obj of taggables) { @@ -428,14 +478,44 @@ export class GarbageCollector { ); } - printer.reportTaggedObjects(taggables); + printer.reportTaggedAsset(taggables); debug(`Tagged ${taggables.length} assets`); } + /** + * Delete images in parallel. The deleteImage API supports batches of 100. + */ + private async parallelDeleteEcr(ecr: ECR, repo: string, deletables: ImageAsset[], printer: ProgressPrinter) { + const batchSize = 100; + const imagesToDelete: ECR.ImageIdentifierList = deletables.map(img => ({ + imageDigest: img.digest, + })); + + try { + const batches = []; + for (let i = 0; i < imagesToDelete.length; i += batchSize) { + batches.push(imagesToDelete.slice(i, i + batchSize)); + } + // Delete images in batches + for (const batch of batches) { + await ecr.batchDeleteImage({ + imageIds: batch, + repositoryName: repo, + }).promise(); + + const deletedCount = batch.length; + debug(`Deleted ${deletedCount} assets`); + printer.reportDeletedAsset(deletables.slice(0, deletedCount)); + } + } catch (err) { + print(chalk.red(`Error deleting images: ${err}`)); + } + } + /** * Delete objects in parallel. The deleteObjects API supports batches of 1000. */ - private async parallelDelete(s3: S3, bucket: string, deletables: S3Asset[], printer: ProgressPrinter) { + private async parallelDeleteS3(s3: S3, bucket: string, deletables: ObjectAsset[], printer: ProgressPrinter) { const batchSize = 1000; const objectsToDelete: S3.ObjectIdentifierList = deletables.map(asset => ({ Key: asset.key, @@ -458,7 +538,7 @@ export class GarbageCollector { const deletedCount = batch.length; debug(`Deleted ${deletedCount} assets`); - printer.reportDeletedObjects(deletables.slice(0, deletedCount)); + printer.reportDeletedAsset(deletables.slice(0, deletedCount)); } } catch (err) { print(chalk.red(`Error deleting objects: ${err}`)); @@ -531,16 +611,32 @@ export class GarbageCollector { imageDigest: key, })); - const imagesInfo = await ecr.describeImages({ - imageIds, + const describeImageInfo = await ecr.describeImages({ + repositoryName: repo, + imageIds: imageIds, + }).promise(); + + const getImageInfo = await ecr.batchGetImage({ repositoryName: repo, + imageIds: imageIds, }).promise(); + + const combinedImageInfo = describeImageInfo.imageDetails?.map(imageDetail => { + const matchingImage = getImageInfo.images?.find( + img => img.imageId?.imageDigest === imageDetail.imageDigest + ); + + return { + ...imageDetail, + manifest: matchingImage?.imageManifest, + }; + }); - for (const image of imagesInfo.imageDetails ?? []) { + for (const image of combinedImageInfo ?? []) { const lastModified = image.imagePushedAt ?? new Date(currentTime); // Store the image if it was pushed earlier than today - createdBufferDays if (image.imageDigest && lastModified < new Date(currentTime - (this.props.createdBufferDays * DAY))) { - batch.push(new ImageAsset(image.imageDigest, image.imageSizeInBytes ?? 0, image.imageTags ?? [])); + batch.push(new ImageAsset(image.imageDigest, image.imageSizeInBytes ?? 0, image.imageTags ?? [], image.manifest ?? '')); } console.log(image.imageDigest, image.imagePushedAt); @@ -560,11 +656,11 @@ export class GarbageCollector { /** * Generator function that reads objects from the S3 Bucket in batches. */ - private async *readBucketInBatches(s3: S3, bucket: string, batchSize: number = 1000, currentTime: number): AsyncGenerator { + private async *readBucketInBatches(s3: S3, bucket: string, batchSize: number = 1000, currentTime: number): AsyncGenerator { let continuationToken: string | undefined; do { - const batch: S3Asset[] = []; + const batch: ObjectAsset[] = []; while (batch.length < batchSize) { const response = await s3.listObjectsV2({ @@ -579,7 +675,7 @@ export class GarbageCollector { // Store the object if it has a Key and // if it has not been modified since today - createdBufferDays if (key && lastModified < new Date(currentTime - (this.props.createdBufferDays * DAY))) { - batch.push(new S3Asset(bucket, key, size)); + batch.push(new ObjectAsset(bucket, key, size)); } }); @@ -593,6 +689,30 @@ export class GarbageCollector { } } while (continuationToken); } + + private async confirmationPrompt(printer: ProgressPrinter, deletables: GcAsset[]) { + if (this.confirm) { + const message = [ + `Found ${deletables.length} assets to delete based off of the following criteria:`, + `- assets have been isolated for > ${this.props.rollbackBufferDays} days`, + `- assets were created > ${this.props.createdBufferDays} days ago`, + '', + 'Delete this batch (yes/no/delete-all)?', + ].join('\n'); + printer.pause(); + const response = await promptly.prompt(message, + { trim: true }, + ); + + // Anything other than yes/y/delete-all is treated as no + if (!response || !['yes', 'y', 'delete-all'].includes(response.toLowerCase())) { + throw new Error('Deletion aborted by user'); + } else if (response.toLowerCase() == 'delete-all') { + this.confirm = false; + } + } + printer.resume(); + } } function partition(xs: Iterable, pred: (x: A) => boolean): { included: A[]; excluded: A[] } { diff --git a/packages/aws-cdk/lib/api/garbage-collection/progress-printer.ts b/packages/aws-cdk/lib/api/garbage-collection/progress-printer.ts index 441bb5fb3977c..c57609ea564fe 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/progress-printer.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/progress-printer.ts @@ -1,43 +1,43 @@ import * as chalk from 'chalk'; -import { S3Asset } from './garbage-collector'; +import { GcAsset as GCAsset } from './garbage-collector'; import { print } from '../../logging'; export class ProgressPrinter { - private totalObjects: number; - private objectsScanned: number; - private taggedObjects: number; - private taggedObjectsSizeMb: number; - private deletedObjects: number; - private deletedObjectsSizeMb: number; + private totalAssets: number; + private assetsScanned: number; + private taggedAsset: number; + private taggedAssetsSizeMb: number; + private deletedAssets: number; + private deletedAssetsSizeMb: number; private interval: number; private setInterval?: NodeJS.Timer; private isPaused: boolean; - constructor(totalObjects: number, interval?: number) { - this.totalObjects = totalObjects; - this.objectsScanned = 0; - this.taggedObjects = 0; - this.taggedObjectsSizeMb = 0; - this.deletedObjects = 0; - this.deletedObjectsSizeMb = 0; + constructor(totalAssets: number, interval?: number) { + this.totalAssets = totalAssets; + this.assetsScanned = 0; + this.taggedAsset = 0; + this.taggedAssetsSizeMb = 0; + this.deletedAssets = 0; + this.deletedAssetsSizeMb = 0; this.interval = interval ?? 10_000; this.isPaused = false; } - public reportScannedObjects(amt: number) { - this.objectsScanned += amt; + public reportScannedAsset(amt: number) { + this.assetsScanned += amt; } - public reportTaggedObjects(objects: S3Asset[]) { - this.taggedObjects += objects.length; - const sizeInBytes = objects.reduce((total, asset) => total + asset.size, 0); - this.taggedObjectsSizeMb += sizeInBytes / 1_048_576; + public reportTaggedAsset(assets: GCAsset[]) { + this.taggedAsset += assets.length; + const sizeInBytes = assets.reduce((total, asset) => total + asset.size, 0); + this.taggedAssetsSizeMb += sizeInBytes / 1_048_576; } - public reportDeletedObjects(objects: S3Asset[]) { - this.deletedObjects += objects.length; - const sizeInBytes = objects.reduce((total, asset) => total + asset.size, 0); - this.deletedObjectsSizeMb += sizeInBytes / 1_048_576; + public reportDeletedAsset(assets: GCAsset[]) { + this.deletedAssets += assets.length; + const sizeInBytes = assets.reduce((total, asset) => total + asset.size, 0); + this.deletedAssetsSizeMb += sizeInBytes / 1_048_576; } public start() { @@ -65,12 +65,12 @@ export class ProgressPrinter { } private print() { - const percentage = ((this.objectsScanned / this.totalObjects) * 100).toFixed(2); + const percentage = ((this.assetsScanned / this.totalAssets) * 100).toFixed(2); // print in MiB until we hit at least 1 GiB of data tagged/deleted - if (Math.max(this.taggedObjectsSizeMb, this.deletedObjectsSizeMb) >= 1000) { - print(chalk.green(`[${percentage}%] ${this.objectsScanned} files scanned: ${this.taggedObjects} objects (${(this.taggedObjectsSizeMb / 1000).toFixed(2)} GiB) tagged, ${this.deletedObjects} objects (${(this.deletedObjectsSizeMb / 1000).toFixed(2)} GiB) deleted.`)); + if (Math.max(this.taggedAssetsSizeMb, this.deletedAssetsSizeMb) >= 1000) { + print(chalk.green(`[${percentage}%] ${this.assetsScanned} files scanned: ${this.taggedAsset} assets (${(this.taggedAssetsSizeMb / 1000).toFixed(2)} GiB) tagged, ${this.deletedAssets} assets (${(this.deletedAssetsSizeMb / 1000).toFixed(2)} GiB) deleted.`)); } else { - print(chalk.green(`[${percentage}%] ${this.objectsScanned} files scanned: ${this.taggedObjects} objects (${this.taggedObjectsSizeMb.toFixed(2)} MiB) tagged, ${this.deletedObjects} objects (${this.deletedObjectsSizeMb.toFixed(2)} MiB) deleted.`)); + print(chalk.green(`[${percentage}%] ${this.assetsScanned} files scanned: ${this.taggedAsset} assets (${this.taggedAssetsSizeMb.toFixed(2)} MiB) tagged, ${this.deletedAssets} assets (${this.deletedAssetsSizeMb.toFixed(2)} MiB) deleted.`)); } } } \ No newline at end of file From 34b1db098576f91bfcc0113f103fa0d50dd46289 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 22 Oct 2024 11:45:51 -0700 Subject: [PATCH 05/17] refactor test suite --- .../test/api/garbage-collection.test.ts | 127 +++++++++--------- 1 file changed, 60 insertions(+), 67 deletions(-) diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index d373816bf51d9..39295e71443ad 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -20,7 +20,7 @@ let mockPutObjectTagging: (params: AWS.S3.Types.PutObjectTaggingRequest) => AWS. let stderrMock: jest.SpyInstance; let sdk: MockSdkProvider; -const ISOLATED_TAG = 'aws-cdk:isolated'; +const ISOLATED_TAG = 'aws-cdk.isolated'; const DAY = 24 * 60 * 60 * 1000; // Number of milliseconds in a day function mockTheToolkitInfo(stackProps: Partial) { @@ -52,6 +52,57 @@ function gc(props: { }); } +function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { + mockListStacks = jest.fn().mockResolvedValue({ + StackSummaries: [ + { StackName: 'Stack1', StackStatus: 'CREATE_COMPLETE' }, + { StackName: 'Stack2', StackStatus: 'UPDATE_COMPLETE' }, + ], + }); + mockGetTemplateSummary = jest.fn().mockReturnValue({ + Parameters: [{ + ParameterKey: 'BootstrapVersion', + DefaultValue: '/cdk-bootstrap/abcde/version', + }], + }); + mockGetTemplate = jest.fn().mockReturnValue({ + TemplateBody: 'abcde', + }); + mockListObjectsV2 = jest.fn().mockImplementation(() => { + return Promise.resolve({ + Contents: [ + { Key: 'asset1', LastModified: new Date(Date.now() - (2 * DAY)) }, + { Key: 'asset2', LastModified: new Date(Date.now() - (10 * DAY)) }, + { Key: 'asset3', LastModified: new Date(Date.now() - (100 * DAY)) }, + ], + KeyCount: 3, + }); + }); + mockGetObjectTagging = jest.fn().mockImplementation((params) => { + return Promise.resolve({ + TagSet: params.Key === 'asset2' ? [{ Key: ISOLATED_TAG, Value: new Date().toISOString() }] : [], + }); + }); + mockPutObjectTagging = jest.fn(); + mockDeleteObjects = jest.fn(); + mockDeleteObjectTagging = jest.fn(); + mockDescribeStacks = jest.fn(); + + mockSdk.stubCloudFormation({ + listStacks: mockListStacks, + getTemplateSummary: mockGetTemplateSummary, + getTemplate: mockGetTemplate, + describeStacks: mockDescribeStacks, + }); + mockSdk.stubS3({ + listObjectsV2: mockListObjectsV2, + getObjectTagging: mockGetObjectTagging, + deleteObjects: mockDeleteObjects, + deleteObjectTagging: mockDeleteObjectTagging, + putObjectTagging: mockPutObjectTagging, + }); +} + beforeEach(() => { sdk = new MockSdkProvider({ realSdk: false }); // By default, we'll return a non-found toolkit info @@ -63,56 +114,9 @@ afterEach(() => { stderrMock.mockRestore(); }); -describe('Garbage Collection', () => { +describe('S3 Garbage Collection', () => { beforeEach(() => { - mockListStacks = jest.fn().mockResolvedValue({ - StackSummaries: [ - { StackName: 'Stack1', StackStatus: 'CREATE_COMPLETE' }, - { StackName: 'Stack2', StackStatus: 'UPDATE_COMPLETE' }, - ], - }); - mockGetTemplateSummary = jest.fn().mockReturnValue({ - Parameters: [{ - ParameterKey: 'BootstrapVersion', - DefaultValue: '/cdk-bootstrap/abcde/version', - }], - }); - mockGetTemplate = jest.fn().mockReturnValue({ - TemplateBody: 'abcde', - }); - mockListObjectsV2 = jest.fn().mockImplementation(() => { - return Promise.resolve({ - Contents: [ - { Key: 'asset1', LastModified: new Date(Date.now() - (2 * DAY)) }, - { Key: 'asset2', LastModified: new Date(Date.now() - (10 * DAY)) }, - { Key: 'asset3', LastModified: new Date(Date.now() - (100 * DAY)) }, - ], - KeyCount: 3, - }); - }); - mockGetObjectTagging = jest.fn().mockImplementation((params) => { - return Promise.resolve({ - TagSet: params.Key === 'asset2' ? [{ Key: ISOLATED_TAG, Value: new Date().toISOString() }] : [], - }); - }); - mockPutObjectTagging = jest.fn(); - mockDeleteObjects = jest.fn(); - mockDeleteObjectTagging = jest.fn(); - mockDescribeStacks = jest.fn(); - - sdk.stubCloudFormation({ - listStacks: mockListStacks, - getTemplateSummary: mockGetTemplateSummary, - getTemplate: mockGetTemplate, - describeStacks: mockDescribeStacks, - }); - sdk.stubS3({ - listObjectsV2: mockListObjectsV2, - getObjectTagging: mockGetObjectTagging, - deleteObjects: mockDeleteObjects, - deleteObjectTagging: mockDeleteObjectTagging, - putObjectTagging: mockPutObjectTagging, - }); + setupS3GarbageCollectionMocks(sdk); }); afterEach(() => { @@ -184,23 +188,6 @@ describe('Garbage Collection', () => { expect(mockDeleteObjects).toHaveBeenCalledTimes(0); }); - test('type = ecr -- throws error', async () => { - mockTheToolkitInfo({ - Outputs: [ - { - OutputKey: 'BootstrapVersion', - OutputValue: '999', - }, - ], - }); - - expect(() => garbageCollector = gc({ - type: 'ecr', - rollbackBufferDays: 3, - action: 'full', - })).toThrow(/ECR garbage collection is not yet supported/); - }); - test('createdAtBufferDays > 0 -- assets to be tagged', async () => { mockTheToolkitInfo({ Outputs: [ @@ -360,6 +347,12 @@ describe('Garbage Collection', () => { }, }); }); +}); + +describe('CloudFormation API calls', () => { + beforeEach(() => { + setupS3GarbageCollectionMocks(sdk); + }); test('bootstrap filters out other bootstrap versions', async () => { mockTheToolkitInfo({ From 84329f0a94f088c8db0731bc2bb87039e3c8e3f8 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 22 Oct 2024 12:59:34 -0700 Subject: [PATCH 06/17] unit tests and refactoring --- .../garbage-collection/garbage-collector.ts | 3 - .../test/api/garbage-collection.test.ts | 245 +++++++++++++++--- packages/aws-cdk/test/util/mock-sdk.ts | 1 + 3 files changed, 206 insertions(+), 43 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index 112421a379581..b062f69d766f2 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -44,7 +44,6 @@ export class ImageAsset { public isolatedTagBefore(date: Date) { const tagValue = this.getIsolatedTag()?.split('-')[2]; - console.log(tagValue); if (!tagValue || tagValue == '') { return false; } @@ -638,8 +637,6 @@ export class GarbageCollector { if (image.imageDigest && lastModified < new Date(currentTime - (this.props.createdBufferDays * DAY))) { batch.push(new ImageAsset(image.imageDigest, image.imageSizeInBytes ?? 0, image.imageTags ?? [], image.manifest ?? '')); } - - console.log(image.imageDigest, image.imagePushedAt); } continuationToken = response.nextToken; diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index 39295e71443ad..afe730f680c3f 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -52,7 +52,7 @@ function gc(props: { }); } -function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { +function setupCFNGarbageCollectionMocks(mockSdk: MockSdkProvider) { mockListStacks = jest.fn().mockResolvedValue({ StackSummaries: [ { StackName: 'Stack1', StackStatus: 'CREATE_COMPLETE' }, @@ -68,6 +68,26 @@ function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { mockGetTemplate = jest.fn().mockReturnValue({ TemplateBody: 'abcde', }); + mockDescribeStacks = jest.fn(); + + mockSdk.stubCloudFormation({ + listStacks: mockListStacks, + getTemplateSummary: mockGetTemplateSummary, + getTemplate: mockGetTemplate, + describeStacks: mockDescribeStacks, + }); + + return { + mockListStacks, + mockGetTemplateSummary, + mockGetTemplate, + mockDescribeStacks, + } +} + +function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { + const mocks = setupCFNGarbageCollectionMocks(mockSdk); + mockListObjectsV2 = jest.fn().mockImplementation(() => { return Promise.resolve({ Contents: [ @@ -86,14 +106,7 @@ function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { mockPutObjectTagging = jest.fn(); mockDeleteObjects = jest.fn(); mockDeleteObjectTagging = jest.fn(); - mockDescribeStacks = jest.fn(); - mockSdk.stubCloudFormation({ - listStacks: mockListStacks, - getTemplateSummary: mockGetTemplateSummary, - getTemplate: mockGetTemplate, - describeStacks: mockDescribeStacks, - }); mockSdk.stubS3({ listObjectsV2: mockListObjectsV2, getObjectTagging: mockGetObjectTagging, @@ -101,6 +114,64 @@ function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { deleteObjectTagging: mockDeleteObjectTagging, putObjectTagging: mockPutObjectTagging, }); + + return { + ...mocks, + mockListObjectsV2, + mockGetObjectTagging, + mockDeleteObjects, + mockDeleteObjectTagging, + mockPutObjectTagging, + } +} + +function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { + const mocks = setupCFNGarbageCollectionMocks(mockSdk); + const mockBatchGetImage = jest.fn().mockImplementation(() => { + return Promise.resolve({ + images: [ + { imageId: { imageDigest: 'digest1', }, imageManifest: {}}, + { imageId: { imageDigest: 'digest2', }, imageManifest: {}}, + { imageId: { imageDigest: 'digest3', }, imageManifest: {}}, + ] + }); + }); + const mockDescribeImages = jest.fn().mockImplementation(() => { + return Promise.resolve({ + imageDetails: [ + { imageDigest: 'digest3', imageTags: ['klmno'], imagePushedAt: Date.now() - (2 * DAY), imageSizeInBytes: 100 }, + { imageDigest: 'digest2', imageTags: ['fghij'], imagePushedAt: Date.now() - (10 * DAY), imageSizeInBytes: 300_000_000 }, + { imageDigest: 'digest1', imageTags: ['abcde'], imagePushedAt: Date.now() - (100 * DAY), imageSizeInBytes: 1_000_000_000 }, + ], + }); + }); + const mockBatchDeleteImage = jest.fn(); + const mockPutImage = jest.fn(); + const mockListImages = jest.fn().mockImplementation(() => { + return Promise.resolve({ + imageIds: [ + { imageDigest: 'digest1', imageTag: 'abcde' }, // inuse + { imageDigest: 'digest2', imageTag: 'fghij' }, + { imageDigest: 'digest3', imageTag: 'klmno' }, + ], + }); + }); + + mockSdk.stubEcr({ + batchGetImage: mockBatchGetImage, + describeImages: mockDescribeImages, + batchDeleteImage: mockBatchDeleteImage, + putImage: mockPutImage, + listImages: mockListImages, + }); + + return { + ...mocks, + mockDescribeImages, + mockBatchDeleteImage, + mockPutImage, + mockListImages, + } } beforeEach(() => { @@ -115,8 +186,9 @@ afterEach(() => { }); describe('S3 Garbage Collection', () => { + let mocks: any; beforeEach(() => { - setupS3GarbageCollectionMocks(sdk); + mocks = setupS3GarbageCollectionMocks(sdk); }); afterEach(() => { @@ -140,14 +212,14 @@ describe('S3 Garbage Collection', () => { }); await garbageCollector.garbageCollect(); - expect(mockListStacks).toHaveBeenCalledTimes(1); - expect(mockListObjectsV2).toHaveBeenCalledTimes(2); + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + expect(mocks.mockListObjectsV2).toHaveBeenCalledTimes(2); // no tagging - expect(mockGetObjectTagging).toHaveBeenCalledTimes(0); - expect(mockPutObjectTagging).toHaveBeenCalledTimes(0); + expect(mocks.mockGetObjectTagging).toHaveBeenCalledTimes(0); + expect(mocks.mockPutObjectTagging).toHaveBeenCalledTimes(0); // assets are to be deleted - expect(mockDeleteObjects).toHaveBeenCalledWith({ + expect(mocks.mockDeleteObjects).toHaveBeenCalledWith({ Bucket: 'BUCKET_NAME', Delete: { Objects: [ @@ -177,18 +249,18 @@ describe('S3 Garbage Collection', () => { }); await garbageCollector.garbageCollect(); - expect(mockListStacks).toHaveBeenCalledTimes(1); - expect(mockListObjectsV2).toHaveBeenCalledTimes(2); + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + expect(mocks.mockListObjectsV2).toHaveBeenCalledTimes(2); // assets tagged - expect(mockGetObjectTagging).toHaveBeenCalledTimes(3); - expect(mockPutObjectTagging).toHaveBeenCalledTimes(2); // one asset already has the tag + expect(mocks.mockGetObjectTagging).toHaveBeenCalledTimes(3); + expect(mocks.mockPutObjectTagging).toHaveBeenCalledTimes(2); // one asset already has the tag // no deleting - expect(mockDeleteObjects).toHaveBeenCalledTimes(0); + expect(mocks.mockDeleteObjects).toHaveBeenCalledTimes(0); }); - test('createdAtBufferDays > 0 -- assets to be tagged', async () => { + test('createdAtBufferDays > 0', async () => { mockTheToolkitInfo({ Outputs: [ { @@ -206,10 +278,11 @@ describe('S3 Garbage Collection', () => { }); await garbageCollector.garbageCollect(); - expect(mockDeleteObjects).toHaveBeenCalledWith({ + expect(mocks.mockDeleteObjects).toHaveBeenCalledWith({ Bucket: 'BUCKET_NAME', Delete: { Objects: [ + // asset1 not deleted because it is too young { Key: 'asset2' }, { Key: 'asset3' }, ], @@ -235,15 +308,15 @@ describe('S3 Garbage Collection', () => { }); await garbageCollector.garbageCollect(); - expect(mockListStacks).toHaveBeenCalledTimes(1); - expect(mockListObjectsV2).toHaveBeenCalledTimes(2); + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + expect(mocks.mockListObjectsV2).toHaveBeenCalledTimes(2); // get tags, but dont put tags - expect(mockGetObjectTagging).toHaveBeenCalledTimes(3); - expect(mockPutObjectTagging).toHaveBeenCalledTimes(0); + expect(mocks.mockGetObjectTagging).toHaveBeenCalledTimes(3); + expect(mocks.mockPutObjectTagging).toHaveBeenCalledTimes(0); // no deleting - expect(mockDeleteObjects).toHaveBeenCalledTimes(0); + expect(mocks.mockDeleteObjects).toHaveBeenCalledTimes(0); }); test('action = tag -- does not delete', async () => { @@ -263,15 +336,15 @@ describe('S3 Garbage Collection', () => { }); await garbageCollector.garbageCollect(); - expect(mockListStacks).toHaveBeenCalledTimes(1); - expect(mockListObjectsV2).toHaveBeenCalledTimes(2); + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + expect(mocks.mockListObjectsV2).toHaveBeenCalledTimes(2); // tags objects - expect(mockGetObjectTagging).toHaveBeenCalledTimes(3); - expect(mockPutObjectTagging).toHaveBeenCalledTimes(2); // one object already has the tag + expect(mocks.mockGetObjectTagging).toHaveBeenCalledTimes(3); + expect(mocks.mockPutObjectTagging).toHaveBeenCalledTimes(2); // one object already has the tag // no deleting - expect(mockDeleteObjects).toHaveBeenCalledTimes(0); + expect(mocks.mockDeleteObjects).toHaveBeenCalledTimes(0); }); test('action = delete-tagged -- does not tag', async () => { @@ -291,12 +364,12 @@ describe('S3 Garbage Collection', () => { }); await garbageCollector.garbageCollect(); - expect(mockListStacks).toHaveBeenCalledTimes(1); - expect(mockListObjectsV2).toHaveBeenCalledTimes(2); + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + expect(mocks.mockListObjectsV2).toHaveBeenCalledTimes(2); // get tags, but dont put tags - expect(mockGetObjectTagging).toHaveBeenCalledTimes(3); - expect(mockPutObjectTagging).toHaveBeenCalledTimes(0); + expect(mocks.mockGetObjectTagging).toHaveBeenCalledTimes(3); + expect(mocks.mockPutObjectTagging).toHaveBeenCalledTimes(0); }); test('ignore objects that are modified after gc start', async () => { @@ -335,7 +408,7 @@ describe('S3 Garbage Collection', () => { await garbageCollector.garbageCollect(); // assets are to be deleted - expect(mockDeleteObjects).toHaveBeenCalledWith({ + expect(mocks.mockDeleteObjects).toHaveBeenCalledWith({ Bucket: 'BUCKET_NAME', Delete: { Objects: [ @@ -349,9 +422,101 @@ describe('S3 Garbage Collection', () => { }); }); +describe('ECR Garbage Collection', () => { + let mocks: any; + beforeEach(() => { + mocks = setupEcrGarbageCollectionMocks(sdk); + }); + + test('rollbackBufferDays = 0 -- assets to be deleted', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 0, + action: 'full', + }); + await garbageCollector.garbageCollect(); + + expect(mocks.mockDescribeImages).toHaveBeenCalledTimes(1); + expect(mocks.mockListImages).toHaveBeenCalledTimes(2); + + // no tagging + expect(mocks.mockPutImage).toHaveBeenCalledTimes(0); + + // assets are to be deleted + expect(mocks.mockBatchDeleteImage).toHaveBeenCalledWith({ + repositoryName: 'REPO_NAME', + imageIds: [ + { imageDigest: 'digest3' }, + { imageDigest: 'digest2' }, + ], + }); + }); + + test('rollbackBufferDays > 0 -- assets to be tagged', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 3, + action: 'full', + }); + await garbageCollector.garbageCollect(); + + // assets tagged + expect(mocks.mockPutImage).toHaveBeenCalledTimes(2); + + // no deleting + expect(mocks.mockBatchDeleteImage).toHaveBeenCalledTimes(0); + }); + + test('createdAtBufferDays > 0', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 0, + createdAtBufferDays: 5, + action: 'full', + }); + await garbageCollector.garbageCollect(); + + expect(mocks.mockBatchDeleteImage).toHaveBeenCalledWith({ + repositoryName: 'REPO_NAME', + imageIds: [ + // digest3 is too young to be deleted + { imageDigest: 'digest2' }, + ], + }); + }); +}); + describe('CloudFormation API calls', () => { + let mocks: any; beforeEach(() => { - setupS3GarbageCollectionMocks(sdk); + mocks = setupS3GarbageCollectionMocks(sdk); }); test('bootstrap filters out other bootstrap versions', async () => { @@ -375,8 +540,8 @@ describe('CloudFormation API calls', () => { }); await garbageCollector.garbageCollect(); - expect(mockGetTemplateSummary).toHaveBeenCalledTimes(2); - expect(mockGetTemplate).toHaveBeenCalledTimes(0); + expect(mocks.mockGetTemplateSummary).toHaveBeenCalledTimes(2); + expect(mocks.mockGetTemplate).toHaveBeenCalledTimes(0); }); test('parameter hashes are included', async () => { @@ -409,7 +574,7 @@ describe('CloudFormation API calls', () => { }); await garbageCollector.garbageCollect(); - expect(mockListStacks).toHaveBeenCalledTimes(1); + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); expect(mockListObjectsV2).toHaveBeenCalledTimes(2); // no tagging expect(mockGetObjectTagging).toHaveBeenCalledTimes(0); diff --git a/packages/aws-cdk/test/util/mock-sdk.ts b/packages/aws-cdk/test/util/mock-sdk.ts index d280ef9f02942..8f53b089fae3a 100644 --- a/packages/aws-cdk/test/util/mock-sdk.ts +++ b/packages/aws-cdk/test/util/mock-sdk.ts @@ -308,6 +308,7 @@ export function mockBootstrapStack(sdk: ISDK | undefined, stack?: Partial Date: Tue, 22 Oct 2024 13:29:23 -0700 Subject: [PATCH 07/17] finish unit test --- .../test/api/garbage-collection.test.ts | 134 +++++++++++++++++- 1 file changed, 129 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index afe730f680c3f..714426b461ae0 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -16,6 +16,11 @@ let mockGetObjectTagging: (params: AWS.S3.Types.GetObjectTaggingRequest) => AWS. let mockDeleteObjects: (params: AWS.S3.Types.DeleteObjectsRequest) => AWS.S3.Types.DeleteObjectsOutput; let mockDeleteObjectTagging: (params: AWS.S3.Types.DeleteObjectTaggingRequest) => AWS.S3.Types.DeleteObjectTaggingOutput; let mockPutObjectTagging: (params: AWS.S3.Types.PutObjectTaggingRequest) => AWS.S3.Types.PutObjectTaggingOutput; +let mockBatchDeleteImage: (params: AWS.ECR.Types.BatchDeleteImageRequest) => AWS.ECR.Types.BatchDeleteImageResponse; +let mockBatchGetImage: (params: AWS.ECR.Types.BatchGetImageRequest) => AWS.ECR.Types.BatchGetImageResponse; +let mockPutImage: (params: AWS.ECR.Types.PutImageRequest) => AWS.ECR.Types.PutImageResponse; +let mockListImages: (params: AWS.ECR.Types.ListImagesRequest) => AWS.ECR.Types.ListImagesResponse; +let mockDescribeImages: (params: AWS.ECR.Types.DescribeImagesRequest) => AWS.ECR.Types.DescribeImagesResponse; let stderrMock: jest.SpyInstance; let sdk: MockSdkProvider; @@ -127,7 +132,7 @@ function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { const mocks = setupCFNGarbageCollectionMocks(mockSdk); - const mockBatchGetImage = jest.fn().mockImplementation(() => { + mockBatchGetImage = jest.fn().mockImplementation(() => { return Promise.resolve({ images: [ { imageId: { imageDigest: 'digest1', }, imageManifest: {}}, @@ -136,7 +141,7 @@ function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { ] }); }); - const mockDescribeImages = jest.fn().mockImplementation(() => { + mockDescribeImages = jest.fn().mockImplementation(() => { return Promise.resolve({ imageDetails: [ { imageDigest: 'digest3', imageTags: ['klmno'], imagePushedAt: Date.now() - (2 * DAY), imageSizeInBytes: 100 }, @@ -145,9 +150,9 @@ function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { ], }); }); - const mockBatchDeleteImage = jest.fn(); - const mockPutImage = jest.fn(); - const mockListImages = jest.fn().mockImplementation(() => { + mockBatchDeleteImage = jest.fn(); + mockPutImage = jest.fn(); + mockListImages = jest.fn().mockImplementation(() => { return Promise.resolve({ imageIds: [ { imageDigest: 'digest1', imageTag: 'abcde' }, // inuse @@ -511,6 +516,125 @@ describe('ECR Garbage Collection', () => { ], }); }); + + test('action = print -- does not tag or delete', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 3, + action: 'print', + }); + await garbageCollector.garbageCollect(); + + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + + // dont put tags + expect(mocks.mockPutImage).toHaveBeenCalledTimes(0); + + // no deleting + expect(mocks.mockBatchDeleteImage).toHaveBeenCalledTimes(0); + }); + + test('action = tag -- does not delete', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 3, + action: 'tag', + }); + await garbageCollector.garbageCollect(); + + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + + // tags objects + expect(mocks.mockPutImage).toHaveBeenCalledTimes(2); + + // no deleting + expect(mocks.mockBatchDeleteImage).toHaveBeenCalledTimes(0); + }); + + test('action = delete-tagged -- does not tag', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 3, + action: 'delete-tagged', + }); + await garbageCollector.garbageCollect(); + + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + + // dont put tags + expect(mocks.mockPutImage).toHaveBeenCalledTimes(0); + }); + + test('ignore images that are modified after gc start', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + const mockDescribeImagesFuture = jest.fn().mockImplementation(() => { + return Promise.resolve({ + imageDetails: [ + { imageDigest: 'digest3', imageTags: ['klmno'], imagePushedAt: Date.now() - (2 * DAY), imageSizeInBytes: 100 }, + { imageDigest: 'digest2', imageTags: ['fghij'], imagePushedAt: Number(new Date().setFullYear(new Date().getFullYear() + 1).toString()), imageSizeInBytes: 300_000_000 }, + { imageDigest: 'digest1', imageTags: ['abcde'], imagePushedAt: Date.now() - (100 * DAY), imageSizeInBytes: 1_000_000_000 }, + ], + }); + }); + + sdk.stubEcr({ + batchGetImage: mockBatchGetImage, + describeImages: mockDescribeImagesFuture, + batchDeleteImage: mockBatchDeleteImage, + putImage: mockPutImage, + listImages: mockListImages, + }); + + garbageCollector = garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 0, + action: 'full', + }); + await garbageCollector.garbageCollect(); + + // assets are to be deleted + expect(mocks.mockBatchDeleteImage).toHaveBeenCalledWith({ + repositoryName: 'REPO_NAME', + imageIds: [ + { imageDigest: 'digest3' }, + ] + }); + }); }); describe('CloudFormation API calls', () => { From d82d3494ae11c26375f189d4c82a5a26f4ffa728 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 22 Oct 2024 15:48:26 -0700 Subject: [PATCH 08/17] 3 integ tests working --- .../cli-integ/lib/with-cdk-app.ts | 14 +- .../cli-integ/resources/cdk-apps/app/app.js | 14 ++ .../garbage-collection.integtest.ts | 192 +++++++++++++++++- .../garbage-collection/garbage-collector.ts | 46 +++-- .../test/api/garbage-collection.test.ts | 16 +- 5 files changed, 249 insertions(+), 33 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts index 33fea7ed959cd..7cd070c38e9d1 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts @@ -1,4 +1,5 @@ /* eslint-disable no-console */ +import * as assert from 'assert'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -544,6 +545,17 @@ export class TestFixture extends ShellHelper { return JSON.parse(fs.readFileSync(templatePath, { encoding: 'utf-8' }).toString()); } + public async bootstrapRepoName(): Promise { + await ensureBootstrapped(this); + + const response = await this.aws.cloudFormation.send(new DescribeStacksCommand({})); + + const stack = (response.Stacks ?? []) + .filter((s) => s.StackName && s.StackName == this.bootstrapStackName); + assert(stack.length == 1); + return outputFromStack('ImageRepositoryName', stack[0]) ?? ''; + } + public get bootstrapStackName() { return this.fullStackName('bootstrap-stack'); } @@ -569,7 +581,7 @@ export class TestFixture extends ShellHelper { } /** - * Cleanup leftover stacks and buckets + * Cleanup leftover stacks and bootstrapped resources */ public async dispose(success: boolean) { const stacksToDelete = await this.deleteableStacks(this.stackNamePrefix); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index 1458df41723bc..0f88ddf3bd467 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -606,6 +606,19 @@ class DockerStack extends cdk.Stack { } } +class DockerInUseStack extends cdk.Stack { + constructor(parent, id, props) { + super(parent, id, props); + + // Use the docker file in a lambda otherwise it will not be referenced in the template + const fn = new lambda.Function(this, 'my-function', { + code: lambda.Code.fromAssetImage(path.join(__dirname, 'docker')), + runtime: lambda.Runtime.FROM_IMAGE, + handler: lambda.Handler.FROM_IMAGE, + }); + } +} + class DockerStackWithCustomFile extends cdk.Stack { constructor(parent, id, props) { super(parent, id, props); @@ -814,6 +827,7 @@ switch (stackSet) { new EcsHotswapStack(app, `${stackPrefix}-ecs-hotswap`); new AppSyncHotswapStack(app, `${stackPrefix}-appsync-hotswap`); new DockerStack(app, `${stackPrefix}-docker`); + new DockerInUseStack(app, `${stackPrefix}-docker-in-use`); new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`); new NotificationArnPropStack(app, `${stackPrefix}-notification-arn-prop`, { diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts index 1ded54aa7f05f..87d67ac38a530 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts @@ -1,10 +1,11 @@ +import { BatchGetImageCommand, DescribeImagesCommand, ListImagesCommand } from '@aws-sdk/client-ecr'; import { GetObjectTaggingCommand, ListObjectsV2Command, PutObjectTaggingCommand } from '@aws-sdk/client-s3'; import { integTest, randomString, withoutBootstrap } from '../../lib'; jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime integTest( - 'Garbage Collection deletes unused assets', + 'Garbage Collection deletes unused s3 objects', withoutBootstrap(async (fixture) => { const toolkitStackName = fixture.bootstrapStackName; const bootstrapBucketName = `aws-cdk-garbage-collect-integ-test-bckt-${randomString()}`; @@ -50,7 +51,50 @@ integTest( ); integTest( - 'Garbage Collection keeps in use assets', + 'Garbage Collection deletes unused ecr images', + withoutBootstrap(async (fixture) => { + const toolkitStackName = fixture.bootstrapStackName; + + await fixture.cdkBootstrapModern({ + toolkitStackName, + }); + + const repoName = await fixture.bootstrapRepoName(); + + await fixture.cdkDeploy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + fixture.log('Setup complete!'); + + await fixture.cdkDestroy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + + await fixture.cdkGarbageCollect({ + rollbackBufferDays: 0, + type: 'ecr', + bootstrapStackName: toolkitStackName, + }); + fixture.log('Garbage collection complete!'); + + // assert that the bootstrap repository is empty + await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })) + .then((result) => { + expect(result.imageIds).toEqual([]); + }); + }), +); + +integTest( + 'Garbage Collection keeps in use s3 objects', withoutBootstrap(async (fixture) => { const toolkitStackName = fixture.bootstrapStackName; const bootstrapBucketName = `aws-cdk-garbage-collect-integ-test-bckt-${randomString()}`; @@ -97,7 +141,50 @@ integTest( ); integTest( - 'Garbage Collection tags unused assets', + 'Garbage Collection keeps in use ecr images', + withoutBootstrap(async (fixture) => { + const toolkitStackName = fixture.bootstrapStackName; + + await fixture.cdkBootstrapModern({ + toolkitStackName, + }); + + const repoName = await fixture.bootstrapRepoName(); + + await fixture.cdkDeploy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + fixture.log('Setup complete!'); + + await fixture.cdkGarbageCollect({ + rollbackBufferDays: 0, + type: 'ecr', + bootstrapStackName: toolkitStackName, + }); + fixture.log('Garbage collection complete!'); + + // assert that the bootstrap repository is empty + await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })) + .then((result) => { + expect(result.imageIds).toHaveLength(1); + }); + + await fixture.cdkDestroy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + }), +); + +integTest( + 'Garbage Collection tags unused s3 objects', withoutBootstrap(async (fixture) => { const toolkitStackName = fixture.bootstrapStackName; const bootstrapBucketName = `aws-cdk-garbage-collect-integ-test-bckt-${randomString()}`; @@ -142,11 +229,63 @@ integTest( const tags = await fixture.aws.s3.send(new GetObjectTaggingCommand({ Bucket: bootstrapBucketName, Key: key })); expect(tags.TagSet).toHaveLength(1); }); + + await fixture.cdkDestroy('lambda', { + options: [ + '--context', `bootstrapBucket=${bootstrapBucketName}`, + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + }), +); + +integTest( + 'Garbage Collection tags unused ecr images', + withoutBootstrap(async (fixture) => { + const toolkitStackName = fixture.bootstrapStackName; + + await fixture.cdkBootstrapModern({ + toolkitStackName, + }); + + const repoName = await fixture.bootstrapRepoName(); + + await fixture.cdkDeploy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + fixture.log('Setup complete!'); + + await fixture.cdkDestroy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + + await fixture.cdkGarbageCollect({ + rollbackBufferDays: 100, // this will ensure that we do not delete assets immediately (and just tag them) + type: 'ecr', + bootstrapStackName: toolkitStackName, + }); + fixture.log('Garbage collection complete!'); + + // assert that the bootstrap repository is empty + await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })) + .then((result) => { + expect(result.imageIds).toHaveLength(2); // the second tag comes in as a second 'id' + }); }), ); integTest( - 'Garbage Collection untags in-use assets', + 'Garbage Collection untags in-use s3 objects', withoutBootstrap(async (fixture) => { const toolkitStackName = fixture.bootstrapStackName; const bootstrapBucketName = `aws-cdk-garbage-collect-integ-test-bckt-${randomString()}`; @@ -200,3 +339,48 @@ integTest( }]); }), ); + +integTest( + 'Garbage Collection untags in-use ecr images', + withoutBootstrap(async (fixture) => { + const toolkitStackName = fixture.bootstrapStackName; + + await fixture.cdkBootstrapModern({ + toolkitStackName, + }); + + const repoName = await fixture.bootstrapRepoName(); + + await fixture.cdkDeploy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + fixture.log('Setup complete!'); + + // Artificially add tagging to the asset in the bootstrap bucket + + await fixture.cdkGarbageCollect({ + rollbackBufferDays: 100, // this will ensure that we do not delete assets immediately (and just tag them) + type: 'ecr', + bootstrapStackName: toolkitStackName, + }); + fixture.log('Garbage collection complete!'); + + // assert that the bootstrap repository is empty + const imageIds = await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })); + const digest = imageIds.imageIds![0].imageDigest; + const imageManifests = await fixture.aws.ecr.send(new BatchGetImageCommand({ repositoryName: repoName, imageIds: [{ imageDigest: digest }])); + const manifest = imageManifests.images![0].imageManifest; + + await fixture.cdkDestroy('docker-in-use', { + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, + '--toolkit-stack-name', toolkitStackName, + '--force', + ], + }); + }), +); diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index b062f69d766f2..f921ee8563f74 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -379,28 +379,28 @@ export class GarbageCollector { } } - /** + /** * Untag assets that were previously tagged, but now currently referenced. * Since this is treated as an implementation detail, we do not print the results in the printer. */ - private async parallelUntagEcr(ecr: ECR, repo: string, untaggables: ImageAsset[]) { - const limit = pLimit(P_LIMIT); - - for (const img of untaggables) { - const tag = img.getIsolatedTag(); - await limit(() => - ecr.batchDeleteImage({ - repositoryName: repo, - imageIds: [{ - imageTag: tag, - }] - }).promise(), - ); - } - - debug(`Untagged ${untaggables.length} assets`); + private async parallelUntagEcr(ecr: ECR, repo: string, untaggables: ImageAsset[]) { + const limit = pLimit(P_LIMIT); + + for (const img of untaggables) { + const tag = img.getIsolatedTag(); + await limit(() => + ecr.batchDeleteImage({ + repositoryName: repo, + imageIds: [{ + imageTag: tag, + }], + }).promise(), + ); } + debug(`Untagged ${untaggables.length} assets`); + } + /** * Untag assets that were previously tagged, but now currently referenced. * Since this is treated as an implementation detail, we do not print the results in the printer. @@ -604,8 +604,14 @@ export class GarbageCollector { repositoryName: repo, }).promise(); + // No images in the repository + if (!response.imageIds || response.imageIds.length === 0) { + continue; + } + // map unique image digest to (possibly multiple) tags const images = imageMap(response.imageIds ?? []); + const imageIds = Object.keys(images).map(key => ({ imageDigest: key, })); @@ -622,15 +628,15 @@ export class GarbageCollector { const combinedImageInfo = describeImageInfo.imageDetails?.map(imageDetail => { const matchingImage = getImageInfo.images?.find( - img => img.imageId?.imageDigest === imageDetail.imageDigest + img => img.imageId?.imageDigest === imageDetail.imageDigest, ); - + return { ...imageDetail, manifest: matchingImage?.imageManifest, }; }); - + for (const image of combinedImageInfo ?? []) { const lastModified = image.imagePushedAt ?? new Date(currentTime); // Store the image if it was pushed earlier than today - createdBufferDays diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index 714426b461ae0..e99b4833c90ba 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -87,7 +87,7 @@ function setupCFNGarbageCollectionMocks(mockSdk: MockSdkProvider) { mockGetTemplateSummary, mockGetTemplate, mockDescribeStacks, - } + }; } function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { @@ -127,7 +127,7 @@ function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { mockDeleteObjects, mockDeleteObjectTagging, mockPutObjectTagging, - } + }; } function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { @@ -135,10 +135,10 @@ function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { mockBatchGetImage = jest.fn().mockImplementation(() => { return Promise.resolve({ images: [ - { imageId: { imageDigest: 'digest1', }, imageManifest: {}}, - { imageId: { imageDigest: 'digest2', }, imageManifest: {}}, - { imageId: { imageDigest: 'digest3', }, imageManifest: {}}, - ] + { imageId: { imageDigest: 'digest1' }, imageManifest: {} }, + { imageId: { imageDigest: 'digest2' }, imageManifest: {} }, + { imageId: { imageDigest: 'digest3' }, imageManifest: {} }, + ], }); }); mockDescribeImages = jest.fn().mockImplementation(() => { @@ -176,7 +176,7 @@ function setupEcrGarbageCollectionMocks(mockSdk: MockSdkProvider) { mockBatchDeleteImage, mockPutImage, mockListImages, - } + }; } beforeEach(() => { @@ -632,7 +632,7 @@ describe('ECR Garbage Collection', () => { repositoryName: 'REPO_NAME', imageIds: [ { imageDigest: 'digest3' }, - ] + ], }); }); }); From dc86f76ed1eaef2cea09e7c5422193324e08b607 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 22 Oct 2024 16:21:35 -0700 Subject: [PATCH 09/17] another test --- .../garbage-collection.integtest.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts index 87d67ac38a530..75142daaa41ca 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts @@ -1,4 +1,4 @@ -import { BatchGetImageCommand, DescribeImagesCommand, ListImagesCommand } from '@aws-sdk/client-ecr'; +import { BatchGetImageCommand, ListImagesCommand, PutImageCommand } from '@aws-sdk/client-ecr'; import { GetObjectTaggingCommand, ListObjectsV2Command, PutObjectTaggingCommand } from '@aws-sdk/client-s3'; import { integTest, randomString, withoutBootstrap } from '../../lib'; @@ -276,7 +276,6 @@ integTest( }); fixture.log('Garbage collection complete!'); - // assert that the bootstrap repository is empty await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })) .then((result) => { expect(result.imageIds).toHaveLength(2); // the second tag comes in as a second 'id' @@ -314,7 +313,7 @@ integTest( Key: key, Tagging: { TagSet: [{ - Key: 'aws-cdk:isolated', + Key: 'aws-cdk.isolated', Value: '12345', }, { Key: 'bogus', @@ -361,6 +360,11 @@ integTest( fixture.log('Setup complete!'); // Artificially add tagging to the asset in the bootstrap bucket + const imageIds = await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })); + const digest = imageIds.imageIds![0].imageDigest; + const imageManifests = await fixture.aws.ecr.send(new BatchGetImageCommand({ repositoryName: repoName, imageIds: [{ imageDigest: digest }] })); + const manifest = imageManifests.images![0].imageManifest; + await fixture.aws.ecr.send(new PutImageCommand({ repositoryName: repoName, imageManifest: manifest, imageDigest: digest, imageTag: 'aws-cdk.isolated-12345' })); await fixture.cdkGarbageCollect({ rollbackBufferDays: 100, // this will ensure that we do not delete assets immediately (and just tag them) @@ -369,12 +373,11 @@ integTest( }); fixture.log('Garbage collection complete!'); - // assert that the bootstrap repository is empty - const imageIds = await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })); - const digest = imageIds.imageIds![0].imageDigest; - const imageManifests = await fixture.aws.ecr.send(new BatchGetImageCommand({ repositoryName: repoName, imageIds: [{ imageDigest: digest }])); - const manifest = imageManifests.images![0].imageManifest; - + await fixture.aws.ecr.send(new ListImagesCommand({ repositoryName: repoName })) + .then((result) => { + expect(result.imageIds).toHaveLength(1); // the second tag has been removed + }); + await fixture.cdkDestroy('docker-in-use', { options: [ '--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`, From e90aa1aa6aa511efeeb483af1f53246565385e3a Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 22 Oct 2024 16:34:56 -0700 Subject: [PATCH 10/17] readme updates --- packages/aws-cdk/README.md | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index deec121451f77..b29cde6054757 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -883,28 +883,35 @@ CDK Garbage Collection. > [!CAUTION] > CDK Garbage Collection is under development and therefore must be opted in via the `--unstable` flag: `cdk gc --unstable=gc`. -> -> [!WARNING] -> `cdk gc` currently only supports garbage collecting S3 Assets. You must specify `cdk gc --unstable=gc --type=s3` as ECR asset garbage collection has not yet been implemented. -`cdk gc` garbage collects unused S3 assets from your bootstrap bucket via the following mechanism: +`cdk gc` garbage collects unused assets from your bootstrap bucket via the following mechanism: - for each object in the bootstrap S3 Bucket, check to see if it is referenced in any existing CloudFormation templates - if not, it is treated as unused and gc will either tag it or delete it, depending on your configuration. +The high-level mechanism works identically for unused assets in bootstrapped ECR Repositories. + The most basic usage looks like this: +```console +cdk gc --unstable=gc +``` + +To specify one type of asset, use the `type` option: + ```console cdk gc --unstable=gc --type=s3 ``` -This will garbage collect S3 assets from the current bootstrapped environment(s) and immediately delete them. Note that, since the default bootstrap S3 Bucket is versioned, object deletion will be handled by the lifecycle +Otherwise `cdk gc` defaults to collecting assets in both the bootstrapped S3 Bucket and ECR Repository. + +`cdk gc` will garbage collect S3 and ECR assets from the current bootstrapped environment(s) and immediately delete them. Note that, since the default bootstrap S3 Bucket is versioned, object deletion will be handled by the lifecycle policy on the bucket. Before we begin to delete your assets, you will be prompted: ```console -cdk gc --unstable=gc --type=s3 +cdk gc --unstable=gc Found X objects to delete based off of the following criteria: - objects have been isolated for > 0 days @@ -913,11 +920,11 @@ Found X objects to delete based off of the following criteria: Delete this batch (yes/no/delete-all)? ``` -Since it's quite possible that the bootstrap bucket has many objects, we work in batches of 1000 objects. To skip the -prompt either reply with `delete-all`, or use the `--confirm=false` option. +Since it's quite possible that the bootstrap bucket has many objects, we work in batches of 1000 objects or 100 images. +To skip the prompt either reply with `delete-all`, or use the `--confirm=false` option. ```console -cdk gc --unstable=gc --type=s3 --confirm=false +cdk gc --unstable=gc --confirm=false ``` If you are concerned about deleting assets too aggressively, there are multiple levers you can configure: @@ -933,7 +940,7 @@ When using `created-buffer-days`, we simply filter out any assets that have not of days. ```console -cdk gc --unstable=gc --type=s3 --rollback-buffer-days=30 --created-buffer-days=1 +cdk gc --unstable=gc --rollback-buffer-days=30 --created-buffer-days=1 ``` You can also configure the scope that `cdk gc` performs via the `--action` option. By default, all actions @@ -944,7 +951,7 @@ are performed, but you can specify `print`, `tag`, or `delete-tagged`. - `delete-tagged` deletes assets that have been tagged for longer than the buffer days, but does not tag newly unused assets. ```console -cdk gc --unstable=gc --type=s3 --action=delete-tagged --rollback-buffer-days=30 +cdk gc --unstable=gc --action=delete-tagged --rollback-buffer-days=30 ``` This will delete assets that have been unused for >30 days, but will not tag additional assets. From 75c1d797123739c34b1549c937f70448d3e77160 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 22 Oct 2024 16:36:03 -0700 Subject: [PATCH 11/17] minor --- packages/aws-cdk/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index b29cde6054757..427d3e8d9064d 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -897,7 +897,7 @@ The most basic usage looks like this: cdk gc --unstable=gc ``` -To specify one type of asset, use the `type` option: +To specify one type of asset, use the `type` option (options are `all`, `s3`, `ecr`): ```console cdk gc --unstable=gc --type=s3 From e45acdf18f026019fff57f0f60a233de9aca00fc Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Wed, 23 Oct 2024 13:54:41 -0400 Subject: [PATCH 12/17] readme update --- packages/aws-cdk/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 427d3e8d9064d..dd35e71b0d787 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -897,6 +897,8 @@ The most basic usage looks like this: cdk gc --unstable=gc ``` +This will garbage collect all unused assets in all environments of the existing CDK App. + To specify one type of asset, use the `type` option (options are `all`, `s3`, `ecr`): ```console From 5163f4c3d612e881dd4a8d881adbb0717da414d4 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Wed, 23 Oct 2024 15:51:29 -0400 Subject: [PATCH 13/17] unique tags and ignore failures --- .../garbage-collection/garbage-collector.ts | 42 +++++++++++++------ .../test/api/garbage-collection.test.ts | 35 ++++++++++++++++ 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index f921ee8563f74..8e0fb760900e1 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -18,6 +18,9 @@ const DAY = 24 * 60 * 60 * 1000; // Number of milliseconds in a day export type GcAsset = ImageAsset | ObjectAsset; +/** + * An image asset that lives in the bootstrapped ECR Repository + */ export class ImageAsset { public constructor( public readonly digest: string, @@ -43,7 +46,8 @@ export class ImageAsset { } public isolatedTagBefore(date: Date) { - const tagValue = this.getIsolatedTag()?.split('-')[2]; + // isolatedTag will look like "aws-cdk.isolated-X-YYYYY" + const tagValue = this.getIsolatedTag()?.split('-')[3]; if (!tagValue || tagValue == '') { return false; } @@ -51,6 +55,9 @@ export class ImageAsset { } } +/** + * An object asset that lives in the bootstrapped S3 Bucket + */ export class ObjectAsset { private cached_tags: S3.TagSet | undefined = undefined; @@ -279,7 +286,7 @@ export class GarbageCollector { } if (this.permissionToTag && taggables.length > 0) { - await this.parallelTagEcr(ecr, repo, taggables, currentTime, printer); + await this.parallelTagEcr(ecr, repo, taggables, printer); } if (this.permissionToTag && untaggables.length > 0) { @@ -435,18 +442,29 @@ export class GarbageCollector { /** * Tag images in parallel using p-limit */ - private async parallelTagEcr(ecr: ECR, repo: string, taggables: ImageAsset[], date: number, printer: ProgressPrinter) { + private async parallelTagEcr(ecr: ECR, repo: string, taggables: ImageAsset[], printer: ProgressPrinter) { const limit = pLimit(P_LIMIT); - for (const img of taggables) { - await limit(() => - ecr.putImage({ - repositoryName: repo, - imageDigest: img.digest, - imageManifest: img.manifest, - imageTag: `${ISOLATED_TAG}-${String(date)}`, - }).promise(), - ); + for (let i = 0; i < taggables.length; i++) { + const img = taggables[i]; + await limit(() => { + const date = Date.now(); + let imageTag; + try { + imageTag = `${ISOLATED_TAG}-${i}-${String(date)}`; + ecr.putImage({ + repositoryName: repo, + imageDigest: img.digest, + imageManifest: img.manifest, + imageTag, + }).promise(); + } catch (error) { + // This is a false negative -- an isolated asset is untagged + // likely due to an imageTag collision. We can safely ignore, + // and the isolated asset will be tagged next time. + debug(`Warning: unable to tag image ${JSON.stringify(img.tags)} with ${imageTag} due to the following error: ${error}`); + } + }); } printer.reportTaggedAsset(taggables); diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index e99b4833c90ba..facedda99c8bf 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -635,6 +635,41 @@ describe('ECR Garbage Collection', () => { ], }); }); + + test('tags are unique', async () => { + mockTheToolkitInfo({ + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '999', + }, + ], + }); + + garbageCollector = garbageCollector = gc({ + type: 'ecr', + rollbackBufferDays: 3, + action: 'tag', + }); + await garbageCollector.garbageCollect(); + + expect(mocks.mockListStacks).toHaveBeenCalledTimes(1); + + // tags objects + expect(mocks.mockPutImage).toHaveBeenCalledTimes(2); + expect(mocks.mockPutImage).toHaveBeenCalledWith({ + repositoryName: 'REPO_NAME', + imageDigest: 'digest3', + imageManifest: expect.any(Object), + imageTag: expect.stringContaining(`${ISOLATED_TAG}-0-`), + }); + expect(mocks.mockPutImage).toHaveBeenCalledWith({ + repositoryName: 'REPO_NAME', + imageDigest: 'digest2', + imageManifest: expect.any(Object), + imageTag: expect.stringContaining(`${ISOLATED_TAG}-1-`), + }); + }); }); describe('CloudFormation API calls', () => { From 6f61cc6f1b8626c866e58d286c26bd43df65bc63 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Wed, 23 Oct 2024 16:47:37 -0400 Subject: [PATCH 14/17] differet tags --- .../api/garbage-collection/garbage-collector.ts | 17 +++++++++-------- .../aws-cdk/test/api/garbage-collection.test.ts | 11 +++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index 8e0fb760900e1..f40ac213ce3ea 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -12,7 +12,8 @@ import { ActiveAssetCache, BackgroundStackRefresh, refreshStacks } from './stack // eslint-disable-next-line @typescript-eslint/no-require-imports const pLimit: typeof import('p-limit') = require('p-limit'); -const ISOLATED_TAG = 'aws-cdk.isolated'; +export const S3_ISOLATED_TAG = 'aws-cdk:isolated'; +export const ECR_ISOLATED_TAG = 'aws-cdk.isolated'; // ':' is not valid in ECR tags const P_LIMIT = 50; const DAY = 24 * 60 * 60 * 1000; // Number of milliseconds in a day @@ -38,11 +39,11 @@ export class ImageAsset { } public hasIsolatedTag() { - return this.hasTag(ISOLATED_TAG); + return this.hasTag(ECR_ISOLATED_TAG); } public getIsolatedTag() { - return this.getTag(ISOLATED_TAG); + return this.getTag(ECR_ISOLATED_TAG); } public isolatedTagBefore(date: Date) { @@ -92,11 +93,11 @@ export class ObjectAsset { } public hasIsolatedTag() { - return this.hasTag(ISOLATED_TAG); + return this.hasTag(S3_ISOLATED_TAG); } public isolatedTagBefore(date: Date) { - const tagValue = this.getTag(ISOLATED_TAG); + const tagValue = this.getTag(S3_ISOLATED_TAG); if (!tagValue || tagValue == '') { return false; } @@ -417,7 +418,7 @@ export class GarbageCollector { for (const obj of untaggables) { const tags = await obj.allTags(s3); - const updatedTags = tags.filter(tag => tag.Key !== ISOLATED_TAG); + const updatedTags = tags.filter(tag => tag.Key !== S3_ISOLATED_TAG); await limit(() => s3.deleteObjectTagging({ Bucket: bucket, @@ -451,7 +452,7 @@ export class GarbageCollector { const date = Date.now(); let imageTag; try { - imageTag = `${ISOLATED_TAG}-${i}-${String(date)}`; + imageTag = `${ECR_ISOLATED_TAG}-${i}-${String(date)}`; ecr.putImage({ repositoryName: repo, imageDigest: img.digest, @@ -486,7 +487,7 @@ export class GarbageCollector { Tagging: { TagSet: [ { - Key: ISOLATED_TAG, + Key: S3_ISOLATED_TAG, Value: String(date), }, ], diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index facedda99c8bf..9708a01da4560 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -2,7 +2,7 @@ const mockGarbageCollect = jest.fn(); -import { GarbageCollector, ToolkitInfo } from '../../lib/api'; +import { GarbageCollector, ToolkitInfo, ECR_ISOLATED_TAG, S3_ISOLATED_TAG } from '../../lib/api'; import { ActiveAssetCache, BackgroundStackRefresh, BackgroundStackRefreshProps } from '../../lib/api/garbage-collection/stack-refresh'; import { mockBootstrapStack, MockSdk, MockSdkProvider } from '../util/mock-sdk'; @@ -25,7 +25,6 @@ let mockDescribeImages: (params: AWS.ECR.Types.DescribeImagesRequest) => AWS.ECR let stderrMock: jest.SpyInstance; let sdk: MockSdkProvider; -const ISOLATED_TAG = 'aws-cdk.isolated'; const DAY = 24 * 60 * 60 * 1000; // Number of milliseconds in a day function mockTheToolkitInfo(stackProps: Partial) { @@ -105,7 +104,7 @@ function setupS3GarbageCollectionMocks(mockSdk: MockSdkProvider) { }); mockGetObjectTagging = jest.fn().mockImplementation((params) => { return Promise.resolve({ - TagSet: params.Key === 'asset2' ? [{ Key: ISOLATED_TAG, Value: new Date().toISOString() }] : [], + TagSet: params.Key === 'asset2' ? [{ Key: S3_ISOLATED_TAG, Value: new Date().toISOString() }] : [], }); }); mockPutObjectTagging = jest.fn(); @@ -661,13 +660,13 @@ describe('ECR Garbage Collection', () => { repositoryName: 'REPO_NAME', imageDigest: 'digest3', imageManifest: expect.any(Object), - imageTag: expect.stringContaining(`${ISOLATED_TAG}-0-`), + imageTag: expect.stringContaining(`${ECR_ISOLATED_TAG}-0-`), }); expect(mocks.mockPutImage).toHaveBeenCalledWith({ repositoryName: 'REPO_NAME', imageDigest: 'digest2', imageManifest: expect.any(Object), - imageTag: expect.stringContaining(`${ISOLATED_TAG}-1-`), + imageTag: expect.stringContaining(`${ECR_ISOLATED_TAG}-1-`), }); }); }); @@ -883,7 +882,7 @@ describe('Garbage Collection with large # of objects', () => { // of the 2000 in use assets, 1000 are tagged. mockGetObjectTaggingLarge = jest.fn().mockImplementation((params) => { return Promise.resolve({ - TagSet: Number(params.Key[params.Key.length - 5]) % 2 === 0 ? [{ Key: ISOLATED_TAG, Value: new Date(2000, 1, 1).toISOString() }] : [], + TagSet: Number(params.Key[params.Key.length - 5]) % 2 === 0 ? [{ Key: S3_ISOLATED_TAG, Value: new Date(2000, 1, 1).toISOString() }] : [], }); }); mockPutObjectTagging = jest.fn(); From 7fe93360773a97420b389e28205b2055f36d59c8 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Wed, 23 Oct 2024 17:25:19 -0400 Subject: [PATCH 15/17] proper tagging ecr --- .../tests/cli-integ-tests/garbage-collection.integtest.ts | 7 +++++-- .../lib/api/garbage-collection/garbage-collector.ts | 7 ++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts index 75142daaa41ca..55acec43d5659 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts @@ -2,6 +2,9 @@ import { BatchGetImageCommand, ListImagesCommand, PutImageCommand } from '@aws-s import { GetObjectTaggingCommand, ListObjectsV2Command, PutObjectTaggingCommand } from '@aws-sdk/client-s3'; import { integTest, randomString, withoutBootstrap } from '../../lib'; +const S3_ISOLATED_TAG = 'aws-cdk:isolated'; +const ECR_ISOLATED_TAG = 'aws-cdk.isolated'; + jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime integTest( @@ -313,7 +316,7 @@ integTest( Key: key, Tagging: { TagSet: [{ - Key: 'aws-cdk.isolated', + Key: S3_ISOLATED_TAG, Value: '12345', }, { Key: 'bogus', @@ -364,7 +367,7 @@ integTest( const digest = imageIds.imageIds![0].imageDigest; const imageManifests = await fixture.aws.ecr.send(new BatchGetImageCommand({ repositoryName: repoName, imageIds: [{ imageDigest: digest }] })); const manifest = imageManifests.images![0].imageManifest; - await fixture.aws.ecr.send(new PutImageCommand({ repositoryName: repoName, imageManifest: manifest, imageDigest: digest, imageTag: 'aws-cdk.isolated-12345' })); + await fixture.aws.ecr.send(new PutImageCommand({ repositoryName: repoName, imageManifest: manifest, imageDigest: digest, imageTag: `${ECR_ISOLATED_TAG}-12345` })); await fixture.cdkGarbageCollect({ rollbackBufferDays: 100, // this will ensure that we do not delete assets immediately (and just tag them) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index f40ac213ce3ea..1e16ce4df7267 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -448,12 +448,12 @@ export class GarbageCollector { for (let i = 0; i < taggables.length; i++) { const img = taggables[i]; - await limit(() => { + const tagEcr = async () => { const date = Date.now(); let imageTag; try { imageTag = `${ECR_ISOLATED_TAG}-${i}-${String(date)}`; - ecr.putImage({ + await ecr.putImage({ repositoryName: repo, imageDigest: img.digest, imageManifest: img.manifest, @@ -465,7 +465,8 @@ export class GarbageCollector { // and the isolated asset will be tagged next time. debug(`Warning: unable to tag image ${JSON.stringify(img.tags)} with ${imageTag} due to the following error: ${error}`); } - }); + }; + await limit(() => tagEcr()); } printer.reportTaggedAsset(taggables); From 5bb09be5616df92403c039182278e463ae4763a6 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Thu, 24 Oct 2024 12:32:52 -0400 Subject: [PATCH 16/17] change up how image tags are created --- .../garbage-collection.integtest.ts | 2 +- .../garbage-collection/garbage-collector.ts | 32 +++++++++++-------- .../test/api/garbage-collection.test.ts | 4 +-- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts index 55acec43d5659..c96426ecb21ed 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/garbage-collection.integtest.ts @@ -367,7 +367,7 @@ integTest( const digest = imageIds.imageIds![0].imageDigest; const imageManifests = await fixture.aws.ecr.send(new BatchGetImageCommand({ repositoryName: repoName, imageIds: [{ imageDigest: digest }] })); const manifest = imageManifests.images![0].imageManifest; - await fixture.aws.ecr.send(new PutImageCommand({ repositoryName: repoName, imageManifest: manifest, imageDigest: digest, imageTag: `${ECR_ISOLATED_TAG}-12345` })); + await fixture.aws.ecr.send(new PutImageCommand({ repositoryName: repoName, imageManifest: manifest, imageDigest: digest, imageTag: `0-${ECR_ISOLATED_TAG}-12345` })); await fixture.cdkGarbageCollect({ rollbackBufferDays: 100, // this will ensure that we do not delete assets immediately (and just tag them) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index 1e16ce4df7267..75e290bc96258 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -30,12 +30,12 @@ export class ImageAsset { public readonly manifest: string, ) {} - private getTag(tagPrefix: string) { - return this.tags.find(t => t.startsWith(tagPrefix)); + private getTag(tag: string) { + return this.tags.find(t => t.includes(tag)); } - private hasTag(tagPrefix: string) { - return this.tags.some(t => t.startsWith(tagPrefix)); + private hasTag(tag: string) { + return this.tags.some(t => t.includes(tag)); } public hasIsolatedTag() { @@ -47,12 +47,21 @@ export class ImageAsset { } public isolatedTagBefore(date: Date) { - // isolatedTag will look like "aws-cdk.isolated-X-YYYYY" - const tagValue = this.getIsolatedTag()?.split('-')[3]; - if (!tagValue || tagValue == '') { + const dateIsolated = this.dateIsolated(); + if (!dateIsolated || dateIsolated == '') { return false; } - return new Date(tagValue) < date; + return new Date(dateIsolated) < date; + } + + public buildImageTag(inc: number) { + // isolatedTag will look like "X-aws-cdk.isolated-YYYYY" + return `${inc}-${ECR_ISOLATED_TAG}-${String(Date.now())}`; + } + + public dateIsolated() { + // isolatedTag will look like "X-aws-cdk.isolated-YYYYY" + return this.getIsolatedTag()?.split('-')[3]; } } @@ -449,21 +458,18 @@ export class GarbageCollector { for (let i = 0; i < taggables.length; i++) { const img = taggables[i]; const tagEcr = async () => { - const date = Date.now(); - let imageTag; try { - imageTag = `${ECR_ISOLATED_TAG}-${i}-${String(date)}`; await ecr.putImage({ repositoryName: repo, imageDigest: img.digest, imageManifest: img.manifest, - imageTag, + imageTag: img.buildImageTag(i), }).promise(); } catch (error) { // This is a false negative -- an isolated asset is untagged // likely due to an imageTag collision. We can safely ignore, // and the isolated asset will be tagged next time. - debug(`Warning: unable to tag image ${JSON.stringify(img.tags)} with ${imageTag} due to the following error: ${error}`); + debug(`Warning: unable to tag image ${JSON.stringify(img.tags)} with ${img.buildImageTag(i)} due to the following error: ${error}`); } }; await limit(() => tagEcr()); diff --git a/packages/aws-cdk/test/api/garbage-collection.test.ts b/packages/aws-cdk/test/api/garbage-collection.test.ts index 9708a01da4560..405c5a561619d 100644 --- a/packages/aws-cdk/test/api/garbage-collection.test.ts +++ b/packages/aws-cdk/test/api/garbage-collection.test.ts @@ -660,13 +660,13 @@ describe('ECR Garbage Collection', () => { repositoryName: 'REPO_NAME', imageDigest: 'digest3', imageManifest: expect.any(Object), - imageTag: expect.stringContaining(`${ECR_ISOLATED_TAG}-0-`), + imageTag: expect.stringContaining(`0-${ECR_ISOLATED_TAG}`), }); expect(mocks.mockPutImage).toHaveBeenCalledWith({ repositoryName: 'REPO_NAME', imageDigest: 'digest2', imageManifest: expect.any(Object), - imageTag: expect.stringContaining(`${ECR_ISOLATED_TAG}-1-`), + imageTag: expect.stringContaining(`1-${ECR_ISOLATED_TAG}`), }); }); }); From 8e2d92d3b1ec6498e2bf404a74c57ca1ba18a53c Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Fri, 25 Oct 2024 19:22:43 -0400 Subject: [PATCH 17/17] merge issues --- .../lib/api/garbage-collection/garbage-collector.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts index 2ad177d825c05..67f7f98cd4ec0 100644 --- a/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts +++ b/packages/aws-cdk/lib/api/garbage-collection/garbage-collector.ts @@ -211,9 +211,6 @@ export class GarbageCollector { if (crypto.getFips() === 1) { throw new Error('Garbage Collection is currently not supported in FIPS environments'); } - const s3 = sdk.s3({ - needsMd5Checksums: true, - }); const qualifier = await this.bootstrapQualifier(sdk, this.bootstrapStackName); const activeAssets = new ActiveAssetCache(); @@ -320,7 +317,9 @@ export class GarbageCollector { * Perform garbage collection on S3 assets */ public async garbageCollectS3(sdk: ISDK, activeAssets: ActiveAssetCache, backgroundStackRefresh: BackgroundStackRefresh) { - const s3 = sdk.s3(); + const s3 = sdk.s3({ + needsMd5Checksums: true, + }); const bucket = await this.bootstrapBucketName(sdk, this.bootstrapStackName); const numObjects = await this.numObjectsInBucket(s3, bucket); const printer = new ProgressPrinter(numObjects, 1000);