Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
12 changes: 10 additions & 2 deletions packages/@aws-cdk/cloud-assembly-schema/schema/assets.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cloud-assembly-schema/schema/version.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"schemaHash": "4244f1ed6fcece9abcfb319c637fd2eb863a5deef9cc36f05f7d52377ce60012",
"revision": 40
"schemaHash": "ba7d47a7a023c39293e99a374af293384eaf1ccd207e515dbdc59dfb5cae4ed6",
"revision": 41
}
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/api/deployments/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`);
}
}

Expand All @@ -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)}`);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/api/work-graph/work-graph-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
]),
Expand Down
36 changes: 34 additions & 2 deletions packages/aws-cdk/test/cli/cdk-toolkit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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: {},
},
},
},
Expand Down
49 changes: 44 additions & 5 deletions packages/cdk-assets/lib/asset-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ export class AssetManifest {
}

function makeEntries<A, B, C>(
assets: Record<string, { source: A; destinations: Record<string, B> }>,
ctor: new (id: DestinationIdentifier, source: A, destination: B) => C,
assets: Record<string, { source: A; displayName?: string; destinations: Record<string, B> }>,
ctor: new (id: DestinationIdentifier, displayName: string | undefined, source: A, destination: B) => C,
): C[] {
const ret = new Array<C>();
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;
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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 */
Expand All @@ -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;
}
}
}

/**
Expand All @@ -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 */
Expand All @@ -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 {
/**
Expand Down
12 changes: 6 additions & 6 deletions packages/cdk-assets/lib/publishing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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) {
Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -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) {
Expand Down
18 changes: 18 additions & 0 deletions packages/cdk-assets/test/manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
),
Expand Down Expand Up @@ -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) {
Expand Down