feat: take all neighbors into account when computing resource digests

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts

@@ -1,6 +1,7 @@
 import type { Environment } from '@aws-cdk/cx-api';
 import type { CloudFormationStack } from './cloudformation';
 import { ResourceLocation, ResourceMapping } from './cloudformation';
+import type { GraphDirection } from './digest';
 import { computeResourceDigests } from './digest';
 import { ToolkitError } from '../../toolkit/toolkit-error';
 import { equalSets } from '../../util/sets';
@@ -29,9 +30,12 @@ export class RefactoringContext {
 
   constructor(props: RefactorManagerOptions) {
     this.environment = props.environment;
-    const moves = resourceMoves(props.deployedStacks, props.localStacks);
-    const [nonAmbiguousMoves, ambiguousMoves] = partitionByAmbiguity(props.overrides ?? [], moves);
+    const moves = resourceMoves(props.deployedStacks, props.localStacks, 'direct');
+    const additionalOverrides = structuralOverrides(props.deployedStacks, props.localStacks);
+    const overrides = (props.overrides ?? []).concat(additionalOverrides);
+    const [nonAmbiguousMoves, ambiguousMoves] = partitionByAmbiguity(overrides, moves);
     this.ambiguousMoves = ambiguousMoves;
+
     this._mappings = resourceMappings(nonAmbiguousMoves);
   }
 
@@ -48,9 +52,32 @@ export class RefactoringContext {
   }
 }
 
-function resourceMoves(before: CloudFormationStack[], after: CloudFormationStack[]): ResourceMove[] {
-  const digestsBefore = resourceDigests(before);
-  const digestsAfter = resourceDigests(after);
+/**
+ * Generates an automatic list of overrides that can be deduced from the structure of the opposite resource graph.
+ * Suppose we have the following resource graph:
+ *
+ *     A --> B
+ *     C --> D
+ *
+ * such that B and D are identical, but A is different from C. Then digest(B) = digest(D). If both resources are moved,
+ * we have an ambiguity. But if we reverse the arrows:
+ *
+ *     A <-- B
+ *     C <-- D
+ *
+ * then digest(B) ≠ digest(D), because they now have different dependencies. If we compute the mappings from this
+ * opposite graph, we can use them as a set of overrides to disambiguate the original moves.
+ *
+ */
+function structuralOverrides(deployedStacks: CloudFormationStack[], localStacks: CloudFormationStack[]): ResourceMapping[] {
+  const moves = resourceMoves(deployedStacks, localStacks, 'opposite');
+  const [nonAmbiguousMoves] = partitionByAmbiguity([], moves);
+  return resourceMappings(nonAmbiguousMoves);
+}
+
+function resourceMoves(before: CloudFormationStack[], after: CloudFormationStack[], direction: GraphDirection): ResourceMove[] {
+  const digestsBefore = resourceDigests(before, direction);
+  const digestsAfter = resourceDigests(after, direction);
 
   const stackNames = (stacks: CloudFormationStack[]) => stacks.map((s) => s.stackName).sort().join(', ');
   if (!isomorphic(digestsBefore, digestsAfter)) {
@@ -119,14 +146,14 @@ function zip(
 /**
  * Computes a list of pairs [digest, location] for each resource in the stack.
  */
-function resourceDigests(stacks: CloudFormationStack[]): Record<string, ResourceLocation[]> {
+function resourceDigests(stacks: CloudFormationStack[], direction: GraphDirection): Record<string, ResourceLocation[]> {
   // index stacks by name
   const stacksByName = new Map<string, CloudFormationStack>();
   for (const stack of stacks) {
     stacksByName.set(stack.stackName, stack);
   }
 
-  const digests = computeResourceDigests(stacks);
+  const digests = computeResourceDigests(stacks, direction);
 
   return groupByKey(
     Object.entries(digests).map(([loc, digest]) => {

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts

@@ -2,6 +2,10 @@ import * as crypto from 'node:crypto';
 import type { CloudFormationResource, CloudFormationStack } from './cloudformation';
 import { ResourceGraph } from './graph';
 
+export type GraphDirection =
+  'direct' // Edge A -> B mean that A depends on B
+  | 'opposite'; // Edge A -> B mean that B depends on A
+
 /**
  * Computes the digest for each resource in the template.
  *
@@ -19,28 +23,38 @@ import { ResourceGraph } from './graph';
  * CloudFormation template form a directed acyclic graph, this function is
  * well-defined.
  */
-export function computeResourceDigests(stacks: CloudFormationStack[]): Record<string, string> {
+export function computeResourceDigests(stacks: CloudFormationStack[], direction: GraphDirection = 'direct'): Record<string, string> {
   const exports: { [p: string]: { stackName: string; value: any } } = Object.fromEntries(
     stacks.flatMap((s) =>
       Object.values(s.template.Outputs ?? {})
         .filter((o) => o.Export != null && typeof o.Export.Name === 'string')
-        .map((o) => [o.Export.Name, { stackName: s.stackName, value: o.Value }] as [string, { stackName: string; value: any }]),
+        .map(
+          (o) =>
+            [o.Export.Name, { stackName: s.stackName, value: o.Value }] as [string, { stackName: string; value: any }],
+        ),
     ),
   );
 
   const resources = Object.fromEntries(
     stacks.flatMap((s) => {
       return Object.entries(s.template.Resources ?? {})
         .filter(([_, res]) => res.Type !== 'AWS::CDK::Metadata')
-        .map(
-          ([id, res]) => [`${s.stackName}.${id}`, res] as [string, CloudFormationResource],
-        );
+        .map(([id, res]) => [`${s.stackName}.${id}`, res] as [string, CloudFormationResource]);
     }),
   );
 
-  const graph = new ResourceGraph(stacks);
+  const graph = direction == 'direct'
+    ? ResourceGraph.fromStacks(stacks)
+    : ResourceGraph.fromStacks(stacks).opposite();
+
+  return computeDigestsInTopologicalOrder(graph, resources, exports);
+}
+
+function computeDigestsInTopologicalOrder(
+  graph: ResourceGraph,
+  resources: Record<string, CloudFormationResource>,
+  exports: Record<string, { stackName: string; value: any }>): Record<string, string> {
   const nodes = graph.sortedNodes.filter(n => resources[n] != null);
-  // 4. Compute digests in sorted order
   const result: Record<string, string> = {};
   for (const id of nodes) {
     const resource = resources[id];

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/graph.ts

@@ -5,10 +5,7 @@ import { ToolkitError } from '../../toolkit/toolkit-error';
  * An immutable directed graph of resources from multiple CloudFormation stacks.
  */
 export class ResourceGraph {
-  private readonly edges: Record<string, Set<string>> = {};
-  private readonly reverseEdges: Record<string, Set<string>> = {};
-
-  constructor(stacks: Omit<CloudFormationStack, 'environment'>[]) {
+  public static fromStacks(stacks: Omit<CloudFormationStack, 'environment'>[]): ResourceGraph {
     const exports: { [p: string]: { stackName: string; value: any } } = Object.fromEntries(
       stacks.flatMap((s) =>
         Object.values(s.template.Outputs ?? {})
@@ -35,9 +32,11 @@ export class ResourceGraph {
     );
 
     // 1. Build adjacency lists
+    const edges: Record<string, Set<string>> = {};
+    const reverseEdges: Record<string, Set<string>> = {};
     for (const id of Object.keys(resources)) {
-      this.edges[id] = new Set();
-      this.reverseEdges[id] = new Set();
+      edges[id] = new Set();
+      reverseEdges[id] = new Set();
     }
 
     // 2. Detect dependencies by searching for Ref/Fn::GetAtt
@@ -84,11 +83,21 @@ export class ResourceGraph {
       const deps = findDependencies(stackName, res || {});
       for (const dep of deps) {
         if (dep in resources && dep !== id) {
-          this.edges[id].add(dep);
-          this.reverseEdges[dep].add(id);
+          edges[id].add(dep);
+          reverseEdges[dep].add(id);
         }
       }
     }
+
+    return new ResourceGraph(edges, reverseEdges);
+  }
+
+  private readonly edges: Record<string, Set<string>> = {};
+  private readonly reverseEdges: Record<string, Set<string>> = {};
+
+  private constructor(edges: Record<string, Set<string>>, reverseEdges: Record<string, Set<string>>) {
+    this.edges = edges;
+    this.reverseEdges = reverseEdges;
   }
 
   /**
@@ -129,4 +138,11 @@ export class ResourceGraph {
     }
     return Array.from(this.edges[node] || []);
   }
+
+  /**
+   * Returns another graph with the same nodes, but with the edges inverted
+   */
+  public opposite(): ResourceGraph {
+    return new ResourceGraph(this.reverseEdges, this.edges);
+  }
 }

packages/@aws-cdk/toolkit-lib/test/api/refactoring/context.test.ts

@@ -402,6 +402,117 @@ describe('typed mappings', () => {
     ]);
   });
 
+  test('ambiguous pairs that can be disambiguated by the structure', () => {
+    const stack1 = {
+      environment,
+      stackName: 'Foo',
+      template: {
+        Resources: {
+          Bucket1: {
+            Type: 'AWS::S3::Bucket',
+            Properties: {},
+          },
+          Bucket2: {
+            Type: 'AWS::S3::Bucket',
+            Properties: {},
+          },
+          Depender1: {
+            Type: 'AWS::Foo::Foo',
+            Properties: {
+              SomeProp: { Ref: 'Bucket1' },
+            },
+          },
+          Depender2: {
+            Type: 'AWS::Bar::Bar',
+            Properties: {
+              SomeProp: { Ref: 'Bucket2' },
+            },
+          },
+        },
+      },
+    };
+
+    const stack2 = {
+      environment,
+      stackName: 'Bar',
+      template: {
+        Resources: {
+          Bucket3: {
+            Type: 'AWS::S3::Bucket',
+            Properties: {},
+          },
+          Bucket4: {
+            Type: 'AWS::S3::Bucket',
+            Properties: {},
+          },
+          Depender1: {
+            Type: 'AWS::Foo::Foo',
+            Properties: {
+              SomeProp: { Ref: 'Bucket3' },
+            },
+          },
+          Depender2: {
+            Type: 'AWS::Bar::Bar',
+            Properties: {
+              SomeProp: { Ref: 'Bucket4' },
+            },
+          },
+        },
+      },
+    };
+
+    const context = new RefactoringContext({
+      environment,
+      deployedStacks: [stack1],
+      localStacks: [stack2],
+    });
+    expect(context.ambiguousPaths.length).toEqual(0);
+    expect(context.mappings.map(toCfnMapping)).toEqual([
+      // Despite Bucket1 and Bucket2 being identical, we could still disambiguate
+      // them based on the resources that depend on them.
+      {
+        Destination: {
+          LogicalResourceId: 'Bucket3',
+          StackName: 'Bar',
+        },
+        Source: {
+          LogicalResourceId: 'Bucket1',
+          StackName: 'Foo',
+        },
+      },
+      {
+        Destination: {
+          LogicalResourceId: 'Bucket4',
+          StackName: 'Bar',
+        },
+        Source: {
+          LogicalResourceId: 'Bucket2',
+          StackName: 'Foo',
+        },
+      },
+      {
+        Destination: {
+          LogicalResourceId: 'Depender1',
+          StackName: 'Bar',
+        },
+        Source: {
+          LogicalResourceId: 'Depender1',
+          StackName: 'Foo',
+        },
+      },
+      {
+        Destination: {
+          LogicalResourceId: 'Depender2',
+          StackName: 'Bar',
+        },
+        Source: {
+          LogicalResourceId: 'Depender2',
+          StackName: 'Foo',
+        },
+      },
+    ]);
+  });
+
   test('combines addition, deletion, update, and rename', () => {
     const stack1 = {
       environment,