diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts index 3b61d7310..1d172cfab 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts @@ -4,7 +4,7 @@ import { EnvironmentPlaceholders } from '@aws-cdk/cloud-assembly-api'; import type { StackDefinition } from '@aws-sdk/client-cloudformation'; import type { CloudFormationStack } from './cloudformation'; import { ResourceLocation, ResourceMapping } from './cloudformation'; -import type { GraphDirection } from './digest'; +import type { GraphDirection, PropertyHashCache } from './digest'; import { computeResourceDigests } from './digest'; import { ToolkitError } from '../../toolkit/toolkit-error'; import { equalSets, setDiff } from '../../util/sets'; @@ -44,8 +44,10 @@ export class RefactoringContext { constructor(props: RefactoringContextOptions) { this.environment = props.environment; - const moves = resourceMoves(props.deployedStacks, props.localStacks, 'direct', props.ignoreModifications); - const additionalOverrides = structuralOverrides(props.deployedStacks, props.localStacks); + // Both passes below hash the same resources; share the property hashes. + const propertyHashes: PropertyHashCache = new Map(); + const moves = resourceMoves(props.deployedStacks, props.localStacks, 'direct', props.ignoreModifications, propertyHashes); + const additionalOverrides = structuralOverrides(props.deployedStacks, props.localStacks, propertyHashes); const overrides = (props.overrides ?? []).concat(additionalOverrides); const [nonAmbiguousMoves, ambiguousMoves] = partitionByAmbiguity(overrides, moves); this.ambiguousMoves = ambiguousMoves; @@ -186,8 +188,11 @@ export class RefactoringContext { * 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', true); +function structuralOverrides( + deployedStacks: CloudFormationStack[], + localStacks: CloudFormationStack[], + propertyHashes?: PropertyHashCache): ResourceMapping[] { + const moves = resourceMoves(deployedStacks, localStacks, 'opposite', true, propertyHashes); const [nonAmbiguousMoves] = partitionByAmbiguity([], moves); return resourceMappings(nonAmbiguousMoves); } @@ -196,9 +201,10 @@ function resourceMoves( before: CloudFormationStack[], after: CloudFormationStack[], direction: GraphDirection = 'direct', - ignoreModifications: boolean = false): ResourceMove[] { - const digestsBefore = resourceDigests(before, direction); - const digestsAfter = resourceDigests(after, direction); + ignoreModifications: boolean = false, + propertyHashes?: PropertyHashCache): ResourceMove[] { + const digestsBefore = resourceDigests(before, direction, propertyHashes); + const digestsAfter = resourceDigests(after, direction, propertyHashes); if (!(ignoreModifications || isomorphic(digestsBefore, digestsAfter))) { const message = ['A refactor operation cannot add, remove or update resources. Only resource moves and renames are allowed.']; @@ -307,14 +313,17 @@ function zip( /** * Computes a list of pairs [digest, location] for each resource in the stack. */ -function resourceDigests(stacks: CloudFormationStack[], direction: GraphDirection): Record { +function resourceDigests( + stacks: CloudFormationStack[], + direction: GraphDirection, + propertyHashes?: PropertyHashCache): Record { // index stacks by name const stacksByName = new Map(); for (const stack of stacks) { stacksByName.set(stack.stackName, stack); } - const digests = computeResourceDigests(stacks, direction); + const digests = computeResourceDigests(stacks, direction, propertyHashes); return groupByKey( Object.entries(digests).map(([loc, digest]) => { diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts index 9a008576d..9e268620f 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts @@ -23,7 +23,23 @@ export type GraphDirection = * CloudFormation template form a directed acyclic graph, this function is * well-defined. */ -export function computeResourceDigests(stacks: CloudFormationStack[], direction: GraphDirection = 'direct'): Record { +/** + * Caches the part of a resource's digest that does not depend on the direction + * of the resource graph: the hash of its own (reference-stripped) properties. + * + * A single refactor operation computes digests four times — for the deployed + * and the local stacks, in both graph directions — and that property hash is + * the same in all of them. Pass the same cache to each call to compute it once + * per resource. Keys are the resource objects themselves, so a cache must not + * outlive the templates it was built from. + */ +export type PropertyHashCache = Map; + +export function computeResourceDigests( + stacks: CloudFormationStack[], + direction: GraphDirection = 'direct', + propertyHashes: PropertyHashCache = new Map(), +): Record { const exports: { [p: string]: { stackName: string; value: any } } = Object.fromEntries( stacks.flatMap((s) => Object.values(s.template.Outputs ?? {}) @@ -47,19 +63,24 @@ export function computeResourceDigests(stacks: CloudFormationStack[], direction: ? ResourceGraph.fromStacks(stacks) : ResourceGraph.fromStacks(stacks).opposite(); - return computeDigestsInTopologicalOrder(graph, resources, exports); + return computeDigestsInTopologicalOrder(graph, resources, exports, propertyHashes); } function computeDigestsInTopologicalOrder( graph: ResourceGraph, resources: Record, - exports: Record): Record { + exports: Record, + propertyHashes: PropertyHashCache): Record { const nodes = graph.sortedNodes.filter(n => resources[n] != null); const result: Record = {}; for (const id of nodes) { const resource = resources[id]; const depDigests = Array.from(graph.outNeighbors(id)).map((d) => result[d]); - const propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports)); + let propertiesHash = propertyHashes.get(resource); + if (propertiesHash == null) { + propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports)); + propertyHashes.set(resource, propertiesHash); + } const toHash = resource.Type + propertiesHash + depDigests.join(''); result[id] = crypto.createHash('sha256').update(toHash).digest('hex'); } diff --git a/packages/@aws-cdk/toolkit-lib/test/api/refactoring/refactoring.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/refactoring/refactoring.test.ts index 25c1cf70f..5c7f06f10 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/refactoring/refactoring.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/refactoring/refactoring.test.ts @@ -392,6 +392,35 @@ describe(computeResourceDigests, () => { expect(result['Stack1.Q1']).toBe(result['Stack1.Q2']); }); + test('a shared property hash cache does not change the digests it produces', () => { + const template = { + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { Prop: 'my-bucket' }, + Metadata: { 'aws:cdk:path': 'Stack/Bucket/Resource' }, + }, + Topic: { + Type: 'AWS::SNS::Topic', + DependsOn: 'Bucket', + Properties: { DisplayName: 'my-topic', Sub: { Ref: 'Bucket' } }, + }, + }, + }; + const stacks = makeStacks([template]); + + for (const direction of ['direct', 'opposite'] as const) { + const uncached = computeResourceDigests(stacks, direction); + + // The same cache is reused across both directions, as RefactoringContext does + const cache = new Map(); + computeResourceDigests(stacks, direction === 'direct' ? 'opposite' : 'direct', cache); + const cached = computeResourceDigests(stacks, direction, cache); + + expect(cached).toEqual(uncached); + } + }); + test('different physical IDs lead to different digests', () => { mockLoadResourceModel.mockReturnValue({ primaryIdentifier: ['FooName'],