From 7c65899a0826d9f62d37fa5b299aa547350a8b62 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Mon, 3 Mar 2025 17:00:58 +0100 Subject: [PATCH] feat: assets can have a display name (#175) Add a new field to the asset schema: `displayName`. If supplied, it will be used instead of the asset ID for display purposes in `cdk-assets` and in the CLI. This adds support for display names to the contract and to the CLI. A future PR in `aws-cdk-lib` will make it possible to configure display names on Asset objects, and add the same support to CDK Pipelines. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions Co-authored-by: github-actions --- .../lib/assets/docker-image-asset.ts | 11 ++++- .../lib/assets/file-asset.ts | 7 +++ .../schema/assets.schema.json | 12 ++++- .../cloud-assembly-schema/schema/version.json | 4 +- .../lib/api/deployments/deployments.ts | 4 +- .../lib/api/work-graph/work-graph-builder.ts | 4 +- packages/aws-cdk/test/cli/cdk-toolkit.test.ts | 36 +++++++++++++- packages/cdk-assets/lib/asset-manifest.ts | 49 +++++++++++++++++-- packages/cdk-assets/lib/publishing.ts | 12 ++--- packages/cdk-assets/test/manifest.test.ts | 18 +++++++ 10 files changed, 134 insertions(+), 23 deletions(-) diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/assets/docker-image-asset.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/assets/docker-image-asset.ts index 70c9761f..bbcbe65e 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/assets/docker-image-asset.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/assets/docker-image-asset.ts @@ -5,12 +5,19 @@ import { AwsDestination } from './aws-destination'; */ export interface DockerImageAsset { /** - * Source description for file assets + * A display name for this asset + * + * @default - The identifier will be used as the display name + */ + readonly displayName?: string; + + /** + * Source description for container assets */ readonly source: DockerImageSource; /** - * Destinations for this file asset + * Destinations for this container asset */ readonly destinations: { [id: string]: DockerImageDestination }; } diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/assets/file-asset.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/assets/file-asset.ts index 58c7e0cc..3d4c2284 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/assets/file-asset.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/assets/file-asset.ts @@ -4,6 +4,13 @@ import { AwsDestination } from './aws-destination'; * A file asset */ export interface FileAsset { + /** + * A display name for this asset + * + * @default - The identifier will be used as the display name + */ + readonly displayName?: string; + /** * Source description for file assets */ diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/assets.schema.json b/packages/@aws-cdk/cloud-assembly-schema/schema/assets.schema.json index f6cabb20..c1f88aea 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/assets.schema.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/assets.schema.json @@ -32,6 +32,10 @@ "description": "A file asset", "type": "object", "properties": { + "displayName": { + "description": "A display name for this asset (Default - The identifier will be used as the display name)", + "type": "string" + }, "source": { "$ref": "#/definitions/FileSource", "description": "Source description for file assets" @@ -113,12 +117,16 @@ "description": "A file asset", "type": "object", "properties": { + "displayName": { + "description": "A display name for this asset (Default - The identifier will be used as the display name)", + "type": "string" + }, "source": { "$ref": "#/definitions/DockerImageSource", - "description": "Source description for file assets" + "description": "Source description for container assets" }, "destinations": { - "description": "Destinations for this file asset", + "description": "Destinations for this container asset", "type": "object", "additionalProperties": { "$ref": "#/definitions/DockerImageDestination" diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/version.json b/packages/@aws-cdk/cloud-assembly-schema/schema/version.json index eff5ca19..29669cfc 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/version.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/version.json @@ -1,4 +1,4 @@ { - "schemaHash": "4244f1ed6fcece9abcfb319c637fd2eb863a5deef9cc36f05f7d52377ce60012", - "revision": 40 + "schemaHash": "ba7d47a7a023c39293e99a374af293384eaf1ccd207e515dbdc59dfb5cae4ed6", + "revision": 41 } \ No newline at end of file diff --git a/packages/aws-cdk/lib/api/deployments/deployments.ts b/packages/aws-cdk/lib/api/deployments/deployments.ts index 0184f07b..e6b366db 100644 --- a/packages/aws-cdk/lib/api/deployments/deployments.ts +++ b/packages/aws-cdk/lib/api/deployments/deployments.ts @@ -634,7 +634,7 @@ export class Deployments { const publisher = this.cachedPublisher(assetManifest, resolvedEnvironment, options.stackName); await publisher.buildEntry(asset); if (publisher.hasFailures) { - throw new ToolkitError(`Failed to build asset ${asset.id}`); + throw new ToolkitError(`Failed to build asset ${asset.displayName(false)}`); } } @@ -652,7 +652,7 @@ export class Deployments { const publisher = this.cachedPublisher(assetManifest, stackEnv, options.stackName); await publisher.publishEntry(asset, { allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(options.stack) }); if (publisher.hasFailures) { - throw new ToolkitError(`Failed to publish asset ${asset.id}`); + throw new ToolkitError(`Failed to publish asset ${asset.displayName(true)}`); } } diff --git a/packages/aws-cdk/lib/api/work-graph/work-graph-builder.ts b/packages/aws-cdk/lib/api/work-graph/work-graph-builder.ts index 7711a150..b013b5bb 100644 --- a/packages/aws-cdk/lib/api/work-graph/work-graph-builder.ts +++ b/packages/aws-cdk/lib/api/work-graph/work-graph-builder.ts @@ -62,7 +62,7 @@ export class WorkGraphBuilder { const node: AssetBuildNode = { type: 'asset-build', id: buildId, - note: assetId, + note: asset.displayName(false), dependencies: new Set([ ...this.stackArtifactIds(assetManifestArtifact.dependencies), // If we disable prebuild, then assets inherit (stack) dependencies from their parent stack @@ -83,7 +83,7 @@ export class WorkGraphBuilder { this.graph.addNodes({ type: 'asset-publish', id: publishId, - note: `${asset.id}`, + note: asset.displayName(true), dependencies: new Set([ buildId, ]), diff --git a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts index 2c3e01bd..82c8d6ca 100644 --- a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts @@ -656,6 +656,32 @@ describe('deploy', () => { }); }); + test('uses display names to reference assets', async () => { + // GIVEN + cloudExecutable = new MockCloudExecutable({ + stacks: [MockStack.MOCK_STACK_WITH_ASSET], + }); + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({}), + }); + stderrMock.mockImplementation((...x) => { + console.error(...x); + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: [MockStack.MOCK_STACK_WITH_ASSET.stackName] }, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }); + + // THEN + expect(stderrMock).toHaveBeenCalledWith(expect.stringContaining('Building Asset Display Name')); + expect(stderrMock).toHaveBeenCalledWith(expect.stringContaining('Publishing Asset Display Name (desto)')); + }); + test('with stacks all stacks specified as wildcard', async () => { // GIVEN const toolkit = defaultToolkitSetup(); @@ -1640,10 +1666,16 @@ class MockStack { version: Manifest.version(), files: { xyz: { + displayName: 'Asset Display Name', source: { - path: path.resolve(__dirname, '..', 'LICENSE'), + path: path.resolve(__dirname, '..', '..', 'LICENSE'), + }, + destinations: { + desto: { + bucketName: 'some-bucket', + objectKey: 'some-key', + }, }, - destinations: {}, }, }, }, diff --git a/packages/cdk-assets/lib/asset-manifest.ts b/packages/cdk-assets/lib/asset-manifest.ts index 241bd404..141f296f 100644 --- a/packages/cdk-assets/lib/asset-manifest.ts +++ b/packages/cdk-assets/lib/asset-manifest.ts @@ -144,13 +144,13 @@ export class AssetManifest { } function makeEntries( - assets: Record }>, - ctor: new (id: DestinationIdentifier, source: A, destination: B) => C, + assets: Record }>, + ctor: new (id: DestinationIdentifier, displayName: string | undefined, source: A, destination: B) => C, ): C[] { const ret = new Array(); for (const [assetId, asset] of Object.entries(assets)) { for (const [destId, destination] of Object.entries(asset.destinations)) { - ret.push(new ctor(new DestinationIdentifier(assetId, destId), asset.source, destination)); + ret.push(new ctor(new DestinationIdentifier(assetId, destId), asset.displayName, asset.source, destination)); } } return ret; @@ -183,6 +183,20 @@ export interface IManifestEntry { * Type-dependent destination data */ readonly genericDestination: unknown; + + /** + * Return a display name for this asset + * + * The `includeDestination` parameter controls whether or not to include the + * destination ID in the display name. + * + * - Pass `false` if you are displaying notifications about building the + * asset, or if you are describing the work of building the asset and publishing + * to all destinations at the same time. + * - Pass `true` if you are displaying notifications about publishing to a + * specific destination. + */ + displayName(includeDestination: boolean): string; } /** @@ -196,6 +210,7 @@ export class FileManifestEntry implements IManifestEntry { constructor( /** Identifier for this asset */ public readonly id: DestinationIdentifier, + private readonly _displayName: string | undefined, /** Source of the file asset */ public readonly source: FileSource, /** Destination for the file asset */ @@ -204,6 +219,14 @@ export class FileManifestEntry implements IManifestEntry { this.genericSource = source; this.genericDestination = destination; } + + public displayName(includeDestination: boolean): string { + if (includeDestination) { + return this._displayName ? `${this._displayName} (${this.id.destinationId})` : `${this.id}`; + } else { + return this._displayName ? this._displayName : this.id.assetId; + } + } } /** @@ -217,6 +240,7 @@ export class DockerImageManifestEntry implements IManifestEntry { constructor( /** Identifier for this asset */ public readonly id: DestinationIdentifier, + private readonly _displayName: string | undefined, /** Source of the file asset */ public readonly source: DockerImageSource, /** Destination for the file asset */ @@ -225,13 +249,28 @@ export class DockerImageManifestEntry implements IManifestEntry { this.genericSource = source; this.genericDestination = destination; } + + public displayName(includeDestination: boolean): string { + if (includeDestination) { + return this._displayName ? `${this._displayName} (${this.id.destinationId})` : `${this.id}`; + } else { + return this._displayName ? this._displayName : this.id.assetId; + } + } } /** * Identify an asset destination in an asset manifest * - * When stringified, this will be a combination of the source - * and destination IDs. + * This class is used to identify both an asset to be built as well as a + * destination where an asset will be published. However, when reasoning about + * building assets the destination part can be ignored, because the same asset + * being sent to multiple destinations will only need to be built once and their + * assetIds are all the same. + * + * When stringified, this will be a combination of the source and destination + * IDs; if a string representation of the source is necessary, use `id.assetId` + * instead. */ export class DestinationIdentifier { /** diff --git a/packages/cdk-assets/lib/publishing.ts b/packages/cdk-assets/lib/publishing.ts index d13642fa..0f4fda5e 100644 --- a/packages/cdk-assets/lib/publishing.ts +++ b/packages/cdk-assets/lib/publishing.ts @@ -153,7 +153,7 @@ export class AssetPublishing implements IPublishProgress { */ public async buildEntry(asset: IManifestEntry) { try { - if (this.progressEvent(EventType.START, `Building ${asset.id}`)) { + if (this.progressEvent(EventType.START, `Building ${asset.displayName(false)}`)) { return false; } @@ -165,7 +165,7 @@ export class AssetPublishing implements IPublishProgress { } this.completedOperations++; - if (this.progressEvent(EventType.SUCCESS, `Built ${asset.id}`)) { + if (this.progressEvent(EventType.SUCCESS, `Built ${asset.displayName(false)}`)) { return false; } } catch (e: any) { @@ -184,7 +184,7 @@ export class AssetPublishing implements IPublishProgress { */ public async publishEntry(asset: IManifestEntry, options: PublishOptions = {}) { try { - if (this.progressEvent(EventType.START, `Publishing ${asset.id}`)) { + if (this.progressEvent(EventType.START, `Publishing ${asset.displayName(true)}`)) { return false; } @@ -196,7 +196,7 @@ export class AssetPublishing implements IPublishProgress { } this.completedOperations++; - if (this.progressEvent(EventType.SUCCESS, `Published ${asset.id}`)) { + if (this.progressEvent(EventType.SUCCESS, `Published ${asset.displayName(true)}`)) { return false; } } catch (e: any) { @@ -225,7 +225,7 @@ export class AssetPublishing implements IPublishProgress { */ private async publishAsset(asset: IManifestEntry, options: PublishOptions = {}) { try { - if (this.progressEvent(EventType.START, `Publishing ${asset.id}`)) { + if (this.progressEvent(EventType.START, `Publishing ${asset.displayName(true)}`)) { return false; } @@ -244,7 +244,7 @@ export class AssetPublishing implements IPublishProgress { } this.completedOperations++; - if (this.progressEvent(EventType.SUCCESS, `Published ${asset.id}`)) { + if (this.progressEvent(EventType.SUCCESS, `Published ${asset.displayName(true)}`)) { return false; } } catch (e: any) { diff --git a/packages/cdk-assets/test/manifest.test.ts b/packages/cdk-assets/test/manifest.test.ts index 0f61f2f3..57d662fa 100644 --- a/packages/cdk-assets/test/manifest.test.ts +++ b/packages/cdk-assets/test/manifest.test.ts @@ -60,21 +60,25 @@ test('.entries() iterates over all destinations', () => { expect(manifest.entries).toEqual([ new FileManifestEntry( new DestinationIdentifier('asset1', 'dest1'), + undefined, { path: 'S1' }, { bucketName: 'D1', objectKey: 'X' }, ), new FileManifestEntry( new DestinationIdentifier('asset1', 'dest2'), + undefined, { path: 'S1' }, { bucketName: 'D2', objectKey: 'X' }, ), new DockerImageManifestEntry( new DestinationIdentifier('asset2', 'dest1'), + undefined, { directory: 'S2' }, { repositoryName: 'D3', imageTag: 'X' }, ), new DockerImageManifestEntry( new DestinationIdentifier('asset2', 'dest2'), + undefined, { directory: 'S2' }, { repositoryName: 'D4', imageTag: 'X' }, ), @@ -136,6 +140,20 @@ test('parse *:DEST the same as :DEST', () => { expect(DestinationPattern.parse('*:a')).toEqual(DestinationPattern.parse(':a')); }); +test.each([ + ['Display Name', false, 'Display Name'], + ['Display Name', true, 'Display Name (dest2)'], + [undefined, false, 'asset1'], + [undefined, true, 'asset1:dest2'], +])('with displayName %p and including destination %p => %p', (displayName, includeDestination, expected) => { + expect(new FileManifestEntry( + new DestinationIdentifier('asset1', 'dest2'), + displayName, + { path: 'S1' }, + { bucketName: 'D2', objectKey: 'X' }, + ).displayName(includeDestination)).toEqual(expected); +}); + function f(obj: unknown, ...keys: string[]): any { for (const k of keys) { if (typeof obj === 'object' && obj !== null && k in obj) {