diff --git a/packages/aws-cdk/lib/util/content-hash.ts b/packages/aws-cdk/lib/util/content-hash.ts index aa78d55dd688d..fefe2dfb2ee1c 100644 --- a/packages/aws-cdk/lib/util/content-hash.ts +++ b/packages/aws-cdk/lib/util/content-hash.ts @@ -2,4 +2,43 @@ import * as crypto from 'crypto'; export function contentHash(data: string | Buffer | DataView) { return crypto.createHash('sha256').update(data).digest('hex'); +} + +/** + * A stably sorted hash of an arbitrary JS object + */ +export function contentHashAny(value: unknown) { + const ret = crypto.createHash('sha256'); + recurse(value); + return ret.digest('hex'); + + function recurse(x: unknown) { + if (typeof x === 'string') { + ret.update(x); + return; + } + + if (Array.isArray(x)) { + ret.update('['); + for (const e of x) { + recurse(e); + ret.update('||'); + } + ret.update(']'); + return; + } + + if (x && typeof x === 'object') { + ret.update('{'); + for (const key of Object.keys(x).sort()) { + ret.update(key); + ret.update(':'); + recurse((x as any)[key]); + } + ret.update('}'); + return; + } + + ret.update(`${x}${typeof x}`); // typeof to make sure hash('123') !== hash(123) + } } \ No newline at end of file diff --git a/packages/aws-cdk/lib/util/work-graph-builder.ts b/packages/aws-cdk/lib/util/work-graph-builder.ts index 4da85242d4c20..1c61d307014a8 100644 --- a/packages/aws-cdk/lib/util/work-graph-builder.ts +++ b/packages/aws-cdk/lib/util/work-graph-builder.ts @@ -1,5 +1,6 @@ import * as cxapi from '@aws-cdk/cx-api'; import { AssetManifest, IManifestEntry } from 'cdk-assets'; +import { contentHashAny } from './content-hash'; import { WorkGraph } from './work-graph'; import { DeploymentState, AssetBuildNode, WorkNode } from './work-graph-types'; @@ -26,7 +27,7 @@ export class WorkGraphBuilder { this.graph.addNodes({ type: 'stack', id: `${this.idPrefix}${artifact.id}`, - dependencies: new Set(this.getDepIds(onlyStacks(artifact.dependencies))), + dependencies: new Set(this.stackArtifactIds(onlyStacks(artifact.dependencies))), stack: artifact, deploymentState: DeploymentState.PENDING, priority: WorkGraphBuilder.PRIORITIES.stack, @@ -37,27 +38,26 @@ export class WorkGraphBuilder { * Oof, see this parameter list */ // eslint-disable-next-line max-len - private addAsset(parentStack: cxapi.CloudFormationStackArtifact, assetArtifact: cxapi.AssetManifestArtifact, assetManifest: AssetManifest, asset: IManifestEntry) { + private addAsset(parentStack: cxapi.CloudFormationStackArtifact, assetManifestArtifact: cxapi.AssetManifestArtifact, assetManifest: AssetManifest, asset: IManifestEntry) { // Just the artifact identifier const assetId = asset.id.assetId; - // Unique per destination where the artifact needs to go - const assetDestinationId = `${asset.id}`; - const buildId = `${this.idPrefix}${assetId}-build`; - const publishNodeId = `${this.idPrefix}${assetDestinationId}-publish`; + const buildId = `build-${assetId}-${contentHashAny([assetId, asset.genericSource]).substring(0, 10)}`; + const publishId = `publish-${assetId}-${contentHashAny([assetId, asset.genericDestination]).substring(0, 10)}`; // Build node only gets added once because they are all the same if (!this.graph.tryGetNode(buildId)) { const node: AssetBuildNode = { type: 'asset-build', id: buildId, + note: assetId, dependencies: new Set([ - ...this.getDepIds(assetArtifact.dependencies), + ...this.stackArtifactIds(assetManifestArtifact.dependencies), // If we disable prebuild, then assets inherit (stack) dependencies from their parent stack - ...!this.prebuildAssets ? this.getDepIds(onlyStacks(parentStack.dependencies)) : [], + ...!this.prebuildAssets ? this.stackArtifactIds(onlyStacks(parentStack.dependencies)) : [], ]), - parentStack, - assetManifestArtifact: assetArtifact, + parentStack: parentStack, + assetManifestArtifact, assetManifest, asset, deploymentState: DeploymentState.PENDING, @@ -66,16 +66,17 @@ export class WorkGraphBuilder { this.graph.addNodes(node); } - const publishNode = this.graph.tryGetNode(publishNodeId); + const publishNode = this.graph.tryGetNode(publishId); if (!publishNode) { this.graph.addNodes({ type: 'asset-publish', - id: publishNodeId, + id: publishId, + note: `${asset.id}`, dependencies: new Set([ buildId, ]), parentStack, - assetManifestArtifact: assetArtifact, + assetManifestArtifact, assetManifest, asset, deploymentState: DeploymentState.PENDING, @@ -83,7 +84,7 @@ export class WorkGraphBuilder { }); } - for (const inheritedDep of this.getDepIds(onlyStacks(parentStack.dependencies))) { + for (const inheritedDep of this.stackArtifactIds(onlyStacks(parentStack.dependencies))) { // The asset publish step also depends on the stacks that the parent depends on. // This is purely cosmetic: if we don't do this, the progress printing of asset publishing // is going to interfere with the progress bar of the stack deployment. We could remove this @@ -91,11 +92,11 @@ export class WorkGraphBuilder { // Note: this may introduce a cycle if one of the parent's dependencies is another stack that // depends on this asset. To workaround this we remove these cycles once all nodes have // been added to the graph. - this.graph.addDependency(publishNodeId, inheritedDep); + this.graph.addDependency(publishId, inheritedDep); } // This will work whether the stack node has been added yet or not - this.graph.addDependency(`${this.idPrefix}${parentStack.id}`, publishNodeId); + this.graph.addDependency(`${this.idPrefix}${parentStack.id}`, publishId); } public build(artifacts: cxapi.CloudArtifact[]): WorkGraph { @@ -131,17 +132,15 @@ export class WorkGraphBuilder { return this.graph; } - private getDepIds(deps: cxapi.CloudArtifact[]): string[] { - const ids = []; - for (const artifact of deps) { - if (cxapi.AssetManifestArtifact.isAssetManifestArtifact(artifact)) { - // Depend on only the publish step. The publish step will depend on the build step on its own. - ids.push(`${this.idPrefix}${artifact.id}-publish`); - } else { - ids.push(`${this.idPrefix}${artifact.id}`); - } + private stackArtifactIds(deps: cxapi.CloudArtifact[]): string[] { + return deps.flatMap((d) => cxapi.CloudFormationStackArtifact.isCloudFormationStackArtifact(d) ? [this.stackArtifactId(d)] : []); + } + + private stackArtifactId(artifact: cxapi.CloudArtifact): string { + if (!cxapi.CloudFormationStackArtifact.isCloudFormationStackArtifact(artifact)) { + throw new Error(`Can only call this on CloudFormationStackArtifact, got: ${artifact.constructor.name}`); } - return ids; + return `${this.idPrefix}${artifact.id}`; } /** @@ -174,4 +173,4 @@ function stacksFromAssets(artifacts: cxapi.CloudArtifact[]) { function onlyStacks(artifacts: cxapi.CloudArtifact[]) { return artifacts.filter(cxapi.CloudFormationStackArtifact.isCloudFormationStackArtifact); -} \ No newline at end of file +} diff --git a/packages/aws-cdk/lib/util/work-graph-types.ts b/packages/aws-cdk/lib/util/work-graph-types.ts index 9da5c59d85ec0..4a20295bf5cc1 100644 --- a/packages/aws-cdk/lib/util/work-graph-types.ts +++ b/packages/aws-cdk/lib/util/work-graph-types.ts @@ -16,6 +16,8 @@ export interface WorkNodeCommon { readonly id: string; readonly dependencies: Set; deploymentState: DeploymentState; + /** Some readable information to attach to the id, which may be unreadable */ + readonly note?: string; } export interface StackNode extends WorkNodeCommon { diff --git a/packages/aws-cdk/lib/util/work-graph.ts b/packages/aws-cdk/lib/util/work-graph.ts index a06a969d5f0dd..e0e3336f34d25 100644 --- a/packages/aws-cdk/lib/util/work-graph.ts +++ b/packages/aws-cdk/lib/util/work-graph.ts @@ -217,19 +217,16 @@ export class WorkGraph { function renderNode(id: string, node: WorkNode): string[] { const ret = []; if (node.deploymentState === DeploymentState.COMPLETED) { - ret.push(` "${simplifyId(id)}" [style=filled,fillcolor=yellow];`); + ret.push(` ${gv(id, { style: 'filled', fillcolor: 'yellow', comment: node.note })};`); } else { - ret.push(` "${simplifyId(id)}";`); + ret.push(` ${gv(id, { comment: node.note })};`); } for (const dep of node.dependencies) { - ret.push(` "${simplifyId(id)}" -> "${simplifyId(dep)}";`); + ret.push(` ${gv(id)} -> ${gv(dep)};`); } return ret; } - function simplifyId(id: string) { - return id.replace(/([0-9a-f]{6})[0-9a-f]{6,}/g, '$1'); - } } /** @@ -392,3 +389,13 @@ function sum(xs: number[]) { function retainOnly(xs: A[], pred: (x: A) => boolean) { xs.splice(0, xs.length, ...xs.filter(pred)); } + +function gv(id: string, attrs?: Record) { + const attrString = Object.entries(attrs ?? {}).flatMap(([k, v]) => v !== undefined ? [`${k}="${v}"`] : []).join(','); + + return attrString ? `"${simplifyId(id)}" [${attrString}]` : `"${simplifyId(id)}"`; +} + +function simplifyId(id: string) { + return id.replace(/([0-9a-f]{6})[0-9a-f]{6,}/g, '$1'); +} \ No newline at end of file diff --git a/packages/aws-cdk/test/work-graph-builder.test.ts b/packages/aws-cdk/test/work-graph-builder.test.ts index 686affae6bfb6..a8457bc48f639 100644 --- a/packages/aws-cdk/test/work-graph-builder.test.ts +++ b/packages/aws-cdk/test/work-graph-builder.test.ts @@ -3,6 +3,8 @@ import * as path from 'path'; import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import { CloudAssemblyBuilder } from '@aws-cdk/cx-api'; +// eslint-disable-next-line import/no-extraneous-dependencies +import { expect } from '@jest/globals'; import { WorkGraph } from '../lib/util/work-graph'; import { WorkGraphBuilder } from '../lib/util/work-graph-builder'; import { AssetBuildNode, AssetPublishNode, StackNode, WorkNode } from '../lib/util/work-graph-types'; @@ -16,6 +18,25 @@ afterEach(() => { rootBuilder.delete(); }); +function superset(xs: A[]): Set { + const ret = new Set(xs); + (ret as any).isSuperset = true; + return ret; +} + +expect.addEqualityTesters([ + function(exp: unknown, act: unknown): boolean | undefined { + if (exp instanceof Set && isIterable(act)) { + if ((exp as any).isSuperset) { + const actSet = new Set(act); + return Array.from(exp as any).every((x) => actSet.has(x)); + } + return this.equals(Array.from(exp).sort(), Array.from(act).sort()); + } + return undefined; + }, +]); + describe('with some stacks and assets', () => { let assembly: cxapi.CloudAssembly; beforeEach(() => { @@ -28,34 +49,34 @@ describe('with some stacks and assets', () => { expect(assertableNode(graph.node('stack2'))).toEqual(expect.objectContaining({ type: 'stack', - dependencies: expect.arrayContaining(['F1:D1-publish']), - } as StackNode)); + dependencies: superset(['publish-F1-add54bdbcb']), + } as Partial)); }); test('asset publishing step depends on asset building step', () => { const graph = new WorkGraphBuilder(true).build(assembly.artifacts); - expect(graph.node('F1:D1-publish')).toEqual(expect.objectContaining({ + expect(graph.node('publish-F1-add54bdbcb')).toEqual(expect.objectContaining({ type: 'asset-publish', - dependencies: new Set(['F1-build']), - } as Partial)); + dependencies: superset(['build-F1-a533139934']), + } satisfies Partial)); }); test('with prebuild off, asset building inherits dependencies from their parent stack', () => { const graph = new WorkGraphBuilder(false).build(assembly.artifacts); - expect(graph.node('F1-build')).toEqual(expect.objectContaining({ + expect(graph.node('build-F1-a533139934')).toEqual(expect.objectContaining({ type: 'asset-build', - dependencies: new Set(['stack0', 'stack1']), + dependencies: superset(['stack0', 'stack1']), } as Partial)); }); test('with prebuild on, assets only have their own dependencies', () => { const graph = new WorkGraphBuilder(true).build(assembly.artifacts); - expect(graph.node('F1-build')).toEqual(expect.objectContaining({ + expect(graph.node('build-F1-a533139934')).toEqual(expect.objectContaining({ type: 'asset-build', - dependencies: new Set(['stack0']), + dependencies: superset(['stack0']), } as Partial)); }); }); @@ -84,13 +105,16 @@ test('can handle nested assemblies', async () => { let workDone = 0; const graph = new WorkGraphBuilder(true).build(assembly.artifacts); + await graph.doParallel(10, { deployStack: async () => { workDone += 1; }, buildAsset: async () => { }, publishAsset: async () => { workDone += 1; }, }); - expect(workDone).toEqual(8); + // The asset is shared between parent assembly and nested assembly, but the stacks will be deployed + // 3 stacks + 1 asset + 3 stacks (1 reused asset) + expect(workDone).toEqual(7); }); test('dependencies on unselected artifacts are silently ignored', async () => { @@ -143,8 +167,8 @@ describe('tests that use assets', () => { const traversal = await traverseAndRecord(graph); expect(traversal).toEqual([ - 'work-graph-builder.test.js-build', - 'work-graph-builder.test.js:D1-publish', + expect.stringMatching(/^build-work-graph-builder.test.js-.*$/), + expect.stringMatching(/^publish-work-graph-builder.test.js-.*$/), 'StackA', 'StackB', ]); @@ -205,11 +229,56 @@ describe('tests that use assets', () => { const traversal = await traverseAndRecord(graph); expect(traversal).toEqual([ - 'abcdef-build', - 'abcdef:D1-publish', - 'abcdef:D2-publish', + expect.stringMatching(/^build-abcdef-.*$/), + expect.stringMatching(/^publish-abcdef-.*$/), + expect.stringMatching(/^publish-abcdef-.*$/), + 'StackA', + expect.stringMatching(/^publish-abcdef-.*$/), + 'StackB', + ]); + }); + + test('different parameters for the same named definition are both published', async () => { + addStack(rootBuilder, 'StackA', { + environment: 'aws://11111/us-east-1', + dependencies: ['StackA.assets'], + }); + addAssets(rootBuilder, 'StackA.assets', { + files: { + abcdef: { + source: { path: __dirname }, + destinations: { + D: { bucketName: 'bucket1', objectKey: 'key' }, + }, + }, + }, + }); + + addStack(rootBuilder, 'StackB', { + environment: 'aws://11111/us-east-1', + dependencies: ['StackB.assets', 'StackA'], + }); + addAssets(rootBuilder, 'StackB.assets', { + files: { + abcdef: { + source: { path: __dirname }, + destinations: { + D: { bucketName: 'bucket2', objectKey: 'key' }, + }, + }, + }, + }); + + const assembly = rootBuilder.buildAssembly(); + + const graph = new WorkGraphBuilder(true).build(assembly.artifacts); + const traversal = await traverseAndRecord(graph); + + expect(traversal).toEqual([ + expect.stringMatching(/^build-abcdef-.*$/), + expect.stringMatching(/^publish-abcdef-.*$/), 'StackA', - 'abcdef:D3-publish', + expect.stringMatching(/^publish-abcdef-.*$/), 'StackB', ]); }); @@ -302,4 +371,8 @@ async function traverseAndRecord(graph: WorkGraph) { publishAsset: async (node) => { ret.push(node.id); }, }); return ret; +} + +function isIterable(x: unknown): x is Iterable { + return x && typeof x === 'object' && (x as any)[Symbol.iterator]; } \ No newline at end of file