fix(cli): asset not uploaded with different synthesizer configs

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)
+  }
 }

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,36 +66,37 @@ 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,
         priority: WorkGraphBuilder.PRIORITIES['asset-publish'],
       });
     }
 
-    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
       // for overall faster deployments if we ever have a better method of progress displaying.
       // 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);
-}
+}

packages/aws-cdk/lib/util/work-graph-types.ts

@@ -16,6 +16,8 @@ export interface WorkNodeCommon {
   readonly id: string;
   readonly dependencies: Set<string>;
   deploymentState: DeploymentState;
+  /** Some readable information to attach to the id, which may be unreadable */
+  readonly note?: string;
 }
 
 export interface StackNode extends WorkNodeCommon {

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<A>(xs: A[], pred: (x: A) => boolean) {
   xs.splice(0, xs.length, ...xs.filter(pred));
 }
+
+function gv(id: string, attrs?: Record<string, string | undefined>) {
+  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');
+}

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<A>(xs: A[]): Set<A> {
+  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<StackNode>));
   });
 
   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<AssetPublishNode>));
+      dependencies: superset(['build-F1-a533139934']),
+    } satisfies Partial<AssetPublishNode>));
   });
 
   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<AssetBuildNode>));
   });
 
   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<AssetBuildNode>));
   });
 });
@@ -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<any> {
+  return x && typeof x === 'object' && (x as any)[Symbol.iterator];
 }