From 225591b1b57f38738aa07eb263733618153e9ab2 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Fri, 27 Jun 2025 14:02:59 +0100 Subject: [PATCH 01/10] feat!: modification is forbidden during a refactor --- .../cloudformation-diff/lib/mappings.ts | 30 +- .../toolkit-lib/docs/message-registry.md | 1 + .../toolkit-lib/lib/actions/refactor/index.ts | 22 +- .../lib/api/io/private/messages.ts | 6 + .../lib/api/refactoring/context.ts | 125 ++++---- .../toolkit-lib/lib/api/refactoring/digest.ts | 60 +--- .../toolkit-lib/lib/api/refactoring/index.ts | 104 ++++-- .../toolkit-lib/lib/toolkit/toolkit.ts | 109 +++---- .../_fixtures/stack-with-two-buckets/index.ts | 10 + .../toolkit-lib/test/actions/refactor.test.ts | 298 ++++++++++++++++-- .../test/api/refactoring/context.test.ts | 208 ++---------- .../test/api/refactoring/refactoring.test.ts | 130 -------- packages/aws-cdk/README.md | 18 +- packages/aws-cdk/lib/cli/cdk-toolkit.ts | 27 +- packages/aws-cdk/lib/cli/cli-config.ts | 12 +- packages/aws-cdk/lib/cli/cli.ts | 3 +- .../aws-cdk/lib/cli/convert-to-user-input.ts | 5 +- .../lib/cli/parse-command-line-arguments.ts | 14 +- packages/aws-cdk/lib/cli/user-input.ts | 19 +- 19 files changed, 615 insertions(+), 586 deletions(-) create mode 100644 packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-two-buckets/index.ts diff --git a/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts b/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts index feda7f371..498590fc3 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts @@ -8,41 +8,33 @@ export interface TypedMapping { readonly destinationPath: string; } -export function formatMappingsHeader(stream: NodeJS.WritableStream) { +export function formatEnvironmentSectionHeader(stream: NodeJS.WritableStream, env: string) { const formatter = new Formatter(stream, {}); - formatter.printSectionHeader('The following resources were moved or renamed:\n'); + formatter.printSectionHeader(`${env}\n`); } -export function formatTypedMappings(stream: NodeJS.WritableStream, mappings: TypedMapping[], env: string) { - const header = [['Resource Type', 'Old Construct Path', 'New Construct Path']]; - const rows = mappings.map((m) => [m.type, m.sourcePath, m.destinationPath]); - - const formatter = new Formatter(stream, {}); - formatter.print(`${env}:`); +export function formatTypedMappings(stream: NodeJS.WritableStream, mappings: TypedMapping[]) { if (mappings.length > 0) { + const header = [['Resource Type', 'Old Construct Path', 'New Construct Path']]; + const rows = mappings.map((m) => [m.type, m.sourcePath, m.destinationPath]); + const formatter = new Formatter(stream, {}); + + formatter.print('The following resources were moved or renamed:'); formatter.print(chalk.green(formatTable(header.concat(rows), undefined))); - } else { - formatter.print('Nothing to refactor.'); + formatter.print(' '); } - formatter.print(' '); -} - -export function formatAmbiguitySectionHeader(stream: NodeJS.WritableStream) { - const formatter = new Formatter(stream, {}); - formatter.printSectionHeader('Ambiguous Resource Name Changes:\n'); } export function formatAmbiguousMappings( stream: NodeJS.WritableStream, pairs: [string[], string[]][], - env: string, ) { const tables = pairs.map(renderTable); const formatter = new Formatter(stream, {}); - formatter.print(`${env}:`); + formatter.print('Detected ambiguities:'); formatter.print(tables.join('\n\n')); - formatter.printSectionFooter(); + formatter.print(' '); function renderTable([removed, added]: [string[], string[]]) { return formatTable([['', 'Resource'], renderRemoval(removed), renderAddition(added)], undefined); diff --git a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md index 66f342305..faf35634d 100644 --- a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md +++ b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md @@ -125,6 +125,7 @@ Please let us know by [opening an issue](https://github.com/aws/aws-cdk-cli/issu | `CDK_TOOLKIT_I7900` | Stack deletion succeeded | `result` | [cxapi.CloudFormationStackArtifact](https://docs.aws.amazon.com/cdk/api/v2/docs/@aws-cdk_cx-api.CloudFormationStackArtifact.html) | | `CDK_TOOLKIT_E7010` | Action was aborted due to negative confirmation of request | `error` | n/a | | `CDK_TOOLKIT_E7900` | Stack deletion failed | `error` | {@link ErrorPayload} | +| `CDK_TOOLKIT_E8900` | Stack refactor failed | `error` | {@link ErrorPayload} | | `CDK_TOOLKIT_I8900` | Refactor result | `result` | {@link RefactorResult} | | `CDK_TOOLKIT_W8010` | Refactor execution not yet supported | `warn` | n/a | | `CDK_TOOLKIT_I9000` | Provides bootstrap times | `info` | {@link Duration} | diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts index 743ed0618..4080c3959 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts @@ -1,4 +1,3 @@ -import type { StackSelector } from '../../api/cloud-assembly'; import type { ExcludeList } from '../../api/refactoring'; import { InMemoryExcludeList, NeverExclude } from '../../api/refactoring'; @@ -71,16 +70,25 @@ export interface RefactorOptions { readonly dryRun?: boolean; /** - * Criteria for selecting stacks to deploy - * - * @default - All stacks + * How the toolkit should obtain the mappings */ - stacks?: StackSelector; + mappingSource?: MappingSource; /** - * How the toolkit should obtain the mappings + * List of patterns for filtering local stacks. If no patterns are passed, + * then all stacks, except the bootstrap stacks are considered. If you want + * to consider all stacks (including bootstrap stacks), pass the wildcard + * '*'. */ - mappingSource?: MappingSource; + localStacks?: string[]; + + /** + * List of patterns for filtering deployed stacks. If no patterns are passed, + * then all stacks, except the bootstrap stacks are considered. If you want + * to consider all stacks (including bootstrap stacks), pass the wildcard + * '*'. + */ + deployedStacks?: string[]; } export interface MappingGroup { diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts index 105620e1f..9314e8530 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts @@ -369,6 +369,12 @@ export const IO = { }), // 8. Refactor (8xxx) + CDK_TOOLKIT_E8900: make.error({ + code: 'CDK_TOOLKIT_E8900', + description: 'Stack refactor failed', + interface: 'ErrorPayload', + }), + CDK_TOOLKIT_I8900: make.result({ code: 'CDK_TOOLKIT_I8900', description: 'Refactor result', 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 963df434b..0de295785 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts @@ -16,7 +16,6 @@ export interface RefactorManagerOptions { localStacks: CloudFormationStack[]; deployedStacks: CloudFormationStack[]; mappings?: ResourceMapping[]; - filteredStacks?: CloudFormationStack[]; } /** @@ -33,18 +32,12 @@ export class RefactoringContext { this._mappings = props.mappings; } else { const moves = resourceMoves(props.deployedStacks, props.localStacks); - this.ambiguousMoves = ambiguousMoves(moves); - - if (!this.isAmbiguous) { - this._mappings = resourceMappings(resourceMoves(props.deployedStacks, props.localStacks), props.filteredStacks); - } + this.ambiguousMoves = moves.filter(isAmbiguousMove); + const nonAmbiguousMoves = moves.filter((move) => !isAmbiguousMove(move)); + this._mappings = resourceMappings(nonAmbiguousMoves); } } - public get isAmbiguous(): boolean { - return this.ambiguousMoves.length > 0; - } - public get ambiguousPaths(): [string[], string[]][] { return this.ambiguousMoves.map(([a, b]) => [convert(a), convert(b)]); @@ -54,24 +47,36 @@ export class RefactoringContext { } public get mappings(): ResourceMapping[] { - if (this.isAmbiguous) { - throw new ToolkitError( - 'Cannot access mappings when there are ambiguous resource moves. Please resolve the ambiguity first.', - ); - } return this._mappings; } } function resourceMoves(before: CloudFormationStack[], after: CloudFormationStack[]): ResourceMove[] { - return Object.values( - removeUnmovedResources(zip(groupByKey(resourceDigests(before)), groupByKey(resourceDigests(after)))), - ); + const digestsBefore = resourceDigests(before); + const digestsAfter = resourceDigests(after); + + if (!isomorphic(digestsBefore, digestsAfter)) { + const message = 'A refactor operation cannot add, remove or update resources. ' + + 'Only resource moves and renames are allowed. ' + + "Run 'cdk diff' to compare the local templates to the deployed stacks."; + throw new ToolkitError(message); + } + + return Object.values(removeUnmovedResources(zip(digestsBefore, digestsAfter))); } -function removeUnmovedResources(m: Record): Record { +/** + * Whether two sets of resources have the same elements (uniquely identified by the digest), and + * each element is in the same number of locations. The locations themselves may be different. + */ +function isomorphic(a: Record, b: Record): boolean { + const sameKeys = equalSets(new Set(Object.keys(a)), new Set(Object.keys(b))); + return sameKeys && Object.entries(a).every(([digest, locations]) => locations.length === b[digest].length); +} + +function removeUnmovedResources(moves: Record): Record { const result: Record = {}; - for (const [hash, [before, after]] of Object.entries(m)) { + for (const [hash, [before, after]] of Object.entries(moves)) { const common = before.filter((b) => after.some((a) => a.equalTo(b))); result[hash] = [ before.filter((b) => !common.some((c) => b.equalTo(c))), @@ -109,24 +114,10 @@ function zip( return result; } -function groupByKey(entries: [string, A][]): Record { - const result: Record = {}; - - for (const [hash, location] of entries) { - if (hash in result) { - result[hash].push(location); - } else { - result[hash] = [location]; - } - } - - return result; -} - /** * Computes a list of pairs [digest, location] for each resource in the stack. */ -function resourceDigests(stacks: CloudFormationStack[]): [string, ResourceLocation][] { +function resourceDigests(stacks: CloudFormationStack[]): Record { // index stacks by name const stacksByName = new Map(); for (const stack of stacks) { @@ -135,35 +126,55 @@ function resourceDigests(stacks: CloudFormationStack[]): [string, ResourceLocati const digests = computeResourceDigests(stacks); - return Object.entries(digests).map(([loc, digest]) => { - const [stackName, logicalId] = loc.split('.'); - const location: ResourceLocation = new ResourceLocation(stacksByName.get(stackName)!, logicalId); - return [digest, location]; - }); + return groupByKey( + Object.entries(digests).map(([loc, digest]) => { + const [stackName, logicalId] = loc.split('.'); + const location: ResourceLocation = new ResourceLocation(stacksByName.get(stackName)!, logicalId); + return [digest, location]; + }), + ); + + function groupByKey(entries: [string, A][]): Record { + const result: Record = {}; + + for (const [key, value] of entries) { + if (key in result) { + result[key].push(value); + } else { + result[key] = [value]; + } + } + + return result; + } } -function ambiguousMoves(movements: ResourceMove[]) { +function isAmbiguousMove(move: ResourceMove): boolean { + const [pre, post] = move; + // A move is considered ambiguous if two conditions are met: // 1. Both sides have at least one element (otherwise, it's just addition or deletion) // 2. At least one side has more than one element - return movements - .filter(([pre, post]) => pre.length > 0 && post.length > 0) - .filter(([pre, post]) => pre.length > 1 || post.length > 1); + return pre.length > 0 && post.length > 0 && (pre.length > 1 || post.length > 1); } -function resourceMappings(movements: ResourceMove[], stacks?: CloudFormationStack[]): ResourceMapping[] { - const stacksPredicate = - stacks == null - ? () => true - : (m: ResourceMapping) => { - // Any movement that involves one of the selected stacks (either moving from or to) - // is considered a candidate for refactoring. - const stackNames = [m.source.stack.stackName, m.destination.stack.stackName]; - return stacks.some((stack) => stackNames.includes(stack.stackName)); - }; - +function resourceMappings(movements: ResourceMove[]): ResourceMapping[] { return movements .filter(([pre, post]) => pre.length === 1 && post.length === 1 && !pre[0].equalTo(post[0])) - .map(([pre, post]) => new ResourceMapping(pre[0], post[0])) - .filter(stacksPredicate); + .map(([pre, post]) => new ResourceMapping(pre[0], post[0])); +} + +/** + * Are two sets equal to each other + */ +function equalSets(a: Set, b: Set) { + if (a.size !== b.size) { + return false; + } + for (const x of a) { + if (!b.has(x)) { + return false; + } + } + return true; } 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 fe8072561..966596b18 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts @@ -1,5 +1,4 @@ import * as crypto from 'node:crypto'; -import { loadResourceModel } from '@aws-cdk/cloudformation-diff/lib/diff/util'; import type { CloudFormationResource, CloudFormationStack } from './cloudformation'; import { ResourceGraph } from './graph'; @@ -8,18 +7,12 @@ import { ResourceGraph } from './graph'; * * Conceptually, the digest is computed as: * - * d(resource) = hash(type + physicalId) , if physicalId is defined - * = hash(type + properties + dependencies.map(d)) , otherwise + * digest(resource) = hash(type + properties + dependencies.map(d)) * - * where `hash` is a cryptographic hash function. In other words, if a resource has - * a physical ID, we use the physical ID plus its type to uniquely identify - * that resource. In this case, the digest can be computed from these two fields - * alone. A corollary is that such resources can be renamed and have their - * properties updated at the same time, and still be considered equivalent. - * - * Otherwise, the digest is computed from its type, its own properties (that is, - * excluding properties that refer to other resources), and the digests of each of - * its dependencies. + * where `hash` is a cryptographic hash function. In other words, the digest of a + * resource is computed from its type, its own properties (that is, excluding + * properties that refer to other resources), and the digests of each of its + * dependencies. * * The digest of a resource, defined recursively this way, remains stable even if * one or more of its dependencies gets renamed. Since the resources in a @@ -36,41 +29,24 @@ export function computeResourceDigests(stacks: CloudFormationStack[]): Record - Object.entries(s.template.Resources ?? {}).map( - ([id, res]) => [`${s.stackName}.${id}`, res] as [string, CloudFormationResource], - ), - ), + 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], + ); + }), ); const graph = new ResourceGraph(stacks); - const nodes = graph.sortedNodes; + const nodes = graph.sortedNodes.filter(n => resources[n] != null); // 4. Compute digests in sorted order const result: Record = {}; for (const id of nodes) { const resource = resources[id]; - const resourceProperties = resource.Properties ?? {}; - const model = loadResourceModel(resource.Type); - const identifier = intersection(Object.keys(resourceProperties), model?.primaryIdentifier ?? []); - let toHash: string; - - if (identifier.length === model?.primaryIdentifier?.length) { - // The resource has a physical ID defined, so we can - // use the ID and the type as the identity of the resource. - toHash = - resource.Type + - identifier - .sort() - .map((attr) => JSON.stringify(resourceProperties[attr])) - .join(''); - } else { - // The resource does not have a physical ID defined, so we need to - // compute the digest based on its properties and dependencies. - const depDigests = Array.from(graph.outNeighbors(id)).map((d) => result[d]); - const propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports)); - toHash = resource.Type + propertiesHash + depDigests.join(''); - } - + const depDigests = Array.from(graph.outNeighbors(id)).map((d) => result[d]); + const propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports)); + const toHash = resource.Type + propertiesHash + depDigests.join(''); result[id] = crypto.createHash('sha256').update(toHash).digest('hex'); } @@ -146,7 +122,3 @@ function stripConstructPath(resource: any): any { delete copy.Metadata['aws:cdk:path']; return copy; } - -function intersection(a: T[], b: T[]): T[] { - return a.filter((value) => b.includes(value)); -} diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts index 179eb8aed..792e22096 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts @@ -1,32 +1,40 @@ import type { TypedMapping } from '@aws-cdk/cloudformation-diff'; import { formatAmbiguousMappings as fmtAmbiguousMappings, - formatMappingsHeader as fmtMappingsHeader, + formatEnvironmentSectionHeader as fmtEnvironmentSectionHeader, formatTypedMappings as fmtTypedMappings, - formatAmbiguitySectionHeader as fmtAmbiguitySectionHeader, } from '@aws-cdk/cloudformation-diff'; import type * as cxapi from '@aws-cdk/cx-api'; import type { StackSummary } from '@aws-sdk/client-cloudformation'; +import { minimatch } from 'minimatch'; +import { major } from 'semver'; import { deserializeStructure } from '../../util'; import type { SdkProvider } from '../aws-auth/private'; import { Mode } from '../plugin'; import { StringWriteStream } from '../streams'; import type { CloudFormationStack } from './cloudformation'; import { ResourceLocation, ResourceMapping } from './cloudformation'; +import { hashObject } from './digest'; import type { MappingGroup } from '../../actions'; import { ToolkitError } from '../../toolkit/toolkit-error'; export * from './exclude'; +interface StackGroup { + environment: cxapi.Environment; + localStacks: CloudFormationStack[]; + deployedStacks: CloudFormationStack[]; +} + export async function usePrescribedMappings( mappingGroups: MappingGroup[], sdkProvider: SdkProvider, ): Promise { - interface StackGroup extends MappingGroup { + interface MappingGroupWithStacks extends MappingGroup { stacks: CloudFormationStack[]; } - const stackGroups: StackGroup[] = []; + const stackGroups: MappingGroupWithStacks[] = []; for (const group of mappingGroups) { stackGroups.push({ ...group, @@ -135,26 +143,17 @@ export async function getDeployedStacks( return Promise.all(summaries.map(normalize)); } -export function formatMappingsHeader(): string { - return formatToStream(fmtMappingsHeader); -} - -export function formatTypedMappings(environment: cxapi.Environment, mappings: TypedMapping[]): string { - return formatToStream((stream) => { - const env = `aws://${environment.account}/${environment.region}`; - fmtTypedMappings(stream, mappings, env); - }); +export function formatEnvironmentSectionHeader(environment: cxapi.Environment) { + const env = `aws://${environment.account}/${environment.region}`; + return formatToStream(stream => fmtEnvironmentSectionHeader(stream, env)); } -export function formatAmbiguitySectionHeader(): string { - return formatToStream(fmtAmbiguitySectionHeader); +export function formatTypedMappings(mappings: TypedMapping[]): string { + return formatToStream((stream) => fmtTypedMappings(stream, mappings)); } -export function formatAmbiguousMappings(environment: cxapi.Environment, paths: [string[], string[]][]): string { - return formatToStream((stream) => { - const env = `aws://${environment.account}/${environment.region}`; - fmtAmbiguousMappings(stream, paths, env); - }); +export function formatAmbiguousMappings(paths: [string[], string[]][]): string { + return formatToStream((stream) => fmtAmbiguousMappings(stream, paths)); } function formatToStream(cb: (stream: NodeJS.WritableStream) => void): string { @@ -162,3 +161,68 @@ function formatToStream(cb: (stream: NodeJS.WritableStream) => void): string { cb(stream); return stream.toString(); } + +/** + * Returns a list of stack groups, each containing the local stacks and the deployed stacks that match the given patterns. + */ +export async function groupStacks(sdkProvider: SdkProvider, cloudAssembly: cxapi.CloudAssembly, localPatterns: string[], deployedPatterns: string[]) { + const localStacks = getLocalStacks(cloudAssembly, localPatterns); + + const environments: Map = new Map(); + + for (const stack of localStacks) { + const environment = await sdkProvider.resolveEnvironment(stack.environment); + const key = hashObject(environment); + environments.set(key, environment); + } + + const localByEnvironment = await indexBy(localStacks, + async (s) => hashObject(await sdkProvider.resolveEnvironment(s.environment)), + ); + + const groups: StackGroup[] = []; + const deployedFilter = matchingPatterns(deployedPatterns, (s) => s.stackName); + for (let key of localByEnvironment.keys()) { + const environment = environments.get(key)!; + groups.push({ + environment, + deployedStacks: (await getDeployedStacks(sdkProvider, environment)).filter(deployedFilter), + localStacks: localByEnvironment.get(key)!, + }); + } + + return groups; +} + +function getLocalStacks(cloudAssembly: cxapi.CloudAssembly, patterns: string[]) { + const all = major(cloudAssembly.version) < 10 ? cloudAssembly.stacks : cloudAssembly.stacksRecursively; + return all.filter(matchingPatterns(patterns ?? [], (s) => s.hierarchicalId)); +} + +function matchingPatterns(patterns: string[], id: (a: S) => string): (s: S) => boolean { + if (patterns.length === 0) { + return (s) => !Object.entries(s.template?.Resources ?? {}) + .some(([logicalId, resource]: [string, any]) => logicalId === 'CdkBootstrapVersion' && resource.Type === 'AWS::SSM::Parameter'); + } + + if (patterns.includes('*')) { + return () => true; + } + + return (s: S) => patterns.some(pattern => minimatch(id(s), pattern)); +} + +async function indexBy(xs: A[], fn: (a: A) => Promise): Promise> { + const ret = new Map(); + for (const x of xs) { + const key = await fn(x); + const group = ret.get(key); + if (group) { + group.push(x); + } else { + ret.set(key, [x]); + } + } + return ret; +} + diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 7ec6e770c..3d5438546 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -59,17 +59,15 @@ import { asIoHelper, IO, SPAN, withoutColor, withoutEmojis, withTrimmedWhitespac import { CloudWatchLogEventMonitor, findCloudWatchLogGroups } from '../api/logs-monitor'; import { Mode, PluginHost } from '../api/plugin'; import { - formatAmbiguitySectionHeader, formatAmbiguousMappings, - formatMappingsHeader, + formatEnvironmentSectionHeader, formatTypedMappings, - getDeployedStacks, + groupStacks, ManifestExcludeList, usePrescribedMappings, } from '../api/refactoring'; -import type { CloudFormationStack, ResourceMapping } from '../api/refactoring/cloudformation'; +import type { ResourceMapping } from '../api/refactoring/cloudformation'; import { RefactoringContext } from '../api/refactoring/context'; -import { hashObject } from '../api/refactoring/digest'; import { ResourceMigrator } from '../api/resource-import'; import { tagsForStack } from '../api/tags/private'; import { DEFAULT_TOOLKIT_STACK_NAME } from '../api/toolkit-info'; @@ -142,12 +140,6 @@ export interface ToolkitOptions { readonly unstableFeatures?: Array; } -interface StackGroup { - environment: cxapi.Environment; - localStacks: CloudFormationStack[]; - deployedStacks: CloudFormationStack[]; -} - /** * Names of toolkit features that are still under development, and may change in * the future. @@ -1072,73 +1064,52 @@ export class Toolkit extends CloudAssemblySourceBuilder { } const sdkProvider = await this.sdkProvider('refactor'); - const stacks = await assembly.selectStacksV2(ALL_STACKS); + + const groups = await groupStacks(sdkProvider, assembly.cloudAssembly, options.localStacks ?? [], options.deployedStacks ?? []); + const mappingSource = options.mappingSource ?? MappingSource.auto(); const exclude = mappingSource.exclude.union(new ManifestExcludeList(assembly.cloudAssembly.manifest)); - const filteredStacks = await assembly.selectStacksV2(options.stacks ?? ALL_STACKS); - - const refactoringContexts: RefactoringContext[] = []; - for (let { environment, localStacks, deployedStacks } of await groupStacksByEnvironment()) { - refactoringContexts.push(new RefactoringContext({ - environment, - deployedStacks, - localStacks, - filteredStacks: filteredStacks.stackArtifacts, - mappings: await getUserProvidedMappings(environment), - })); - } - const nonAmbiguousContexts = refactoringContexts.filter(c => !c.isAmbiguous); - if (nonAmbiguousContexts.length > 0) { - await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(formatMappingsHeader(), {})); - } - for (const context of nonAmbiguousContexts) { - const mappings = context.mappings.filter((m) => !exclude.isExcluded(m.destination)); - const typedMappings = mappings.map(m => m.toTypedMapping()); - const environment = context.environment; - await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(formatTypedMappings(environment, typedMappings), { - typedMappings, - })); - } + for (let { environment, localStacks, deployedStacks } of groups) { + await notifyInfo(formatEnvironmentSectionHeader(environment)); - const ambiguousContexts = refactoringContexts.filter(c => c.isAmbiguous); - if (ambiguousContexts.length > 0) { - await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(formatAmbiguitySectionHeader(), {})); - } - for (const context of ambiguousContexts) { - const paths = context.ambiguousPaths; - const environment = context.environment; - await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(formatAmbiguousMappings(environment, paths), { - ambiguousPaths: paths, - })); - } + try { + const context = new RefactoringContext({ + environment, + deployedStacks, + localStacks, + mappings: await getUserProvidedMappings(environment), + }); - async function groupStacksByEnvironment(): Promise { - const stackGroups: Map = new Map(); - const environments: Map = new Map(); + const mappings = context.mappings.filter((m) => !exclude.isExcluded(m.destination)); - for (const stack of stacks.stackArtifacts) { - const environment = await sdkProvider.resolveEnvironment(stack.environment); - const key = hashObject(environment); - environments.set(key, environment); - if (stackGroups.has(key)) { - stackGroups.get(key)![1].push(stack); - } else { - // The first time we see an environment, we need to fetch all stacks deployed to it. - const before = await getDeployedStacks(sdkProvider, environment); - stackGroups.set(key, [before, [stack]]); + if (context.ambiguousPaths.length > 0) { + const paths = context.ambiguousPaths; + await notifyInfo(formatAmbiguousMappings(paths), { ambiguousPaths: paths }); + continue; } - } - const result: StackGroup[] = []; - for (const [hash, [deployedStacks, localStacks]] of stackGroups) { - result.push({ - environment: environments.get(hash)!, - localStacks, - deployedStacks, - }); + if (mappings.length === 0) { + await notifyInfo('Nothing to refactor.'); + continue; + } + + const typedMappings = mappings + .map(m => m.toTypedMapping()) + .filter(m => m.type !== 'AWS::CDK::Metadata'); + + await notifyInfo(formatTypedMappings(typedMappings), { typedMappings }); + } catch (e: any) { + await notifyError(e.message, e); } - return result; + } + + async function notifyInfo(message: string, data: any = {}) { + await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(message, data)); + } + + async function notifyError(message: string, error: Error) { + await ioHelper.notify(IO.CDK_TOOLKIT_E8900.msg(message, { error })); } async function getUserProvidedMappings(environment: cxapi.Environment): Promise { diff --git a/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-two-buckets/index.ts b/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-two-buckets/index.ts new file mode 100644 index 000000000..dbd2a4c51 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-two-buckets/index.ts @@ -0,0 +1,10 @@ +import * as s3 from 'aws-cdk-lib/aws-s3'; +import * as core from 'aws-cdk-lib/core'; + +export default async () => { + const app = new core.App({ autoSynth: false }); + const stack = new core.Stack(app, 'Stack1'); + new s3.Bucket(stack, 'MyBucket1'); + new s3.Bucket(stack, 'MyBucket2'); + return app.synth(); +}; diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index 50a0b54cd..2e6e15764 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -1,5 +1,5 @@ import { GetTemplateCommand, ListStacksCommand } from '@aws-sdk/client-cloudformation'; -import { MappingSource, StackSelectionStrategy, Toolkit } from '../../lib'; +import { MappingSource, type RefactorOptions, Toolkit } from '../../lib'; import { SdkProvider } from '../../lib/api/aws-auth/private'; import { builderFixture, TestIoHost } from '../_helpers'; import { mockCloudFormationClient, MockSdk } from '../_helpers/mock-sdk'; @@ -86,6 +86,166 @@ test('detects the same resource in different locations', async () => { ); }); +test('only considers deployed stacks that match the given filter', async () => { + // GIVEN + mockCloudFormationClient.on(ListStacksCommand).resolves({ + StackSummaries: [ + { + StackName: 'Stack1', + StackId: 'arn:aws:cloudformation:us-east-1:999999999999:stack/Stack1', + StackStatus: 'CREATE_COMPLETE', + CreationTime: new Date(), + }, + { + StackName: 'Stack2', + StackId: 'arn:aws:cloudformation:us-east-1:999999999999:stack/Stack2', + StackStatus: 'CREATE_COMPLETE', + CreationTime: new Date(), + }, + { + StackName: 'CDKToolkit', + StackId: 'arn:aws:cloudformation:us-east-1:999999999999:stack/CDKToolkit', + StackStatus: 'CREATE_COMPLETE', + CreationTime: new Date(), + }, + ], + }); + + mockCloudFormationClient + .on(GetTemplateCommand, { + StackName: 'Stack1', + }) + .resolves({ + TemplateBody: JSON.stringify({ + Resources: { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }, + }), + }); + + mockCloudFormationClient + .on(GetTemplateCommand, { + StackName: 'Stack2', + }) + .resolves({ + TemplateBody: JSON.stringify({ + Resources: { + Queue: { + Type: 'AWS::SQS::Queue', + UpdateReplacePolicy: 'Delete', + DeletionPolicy: 'Delete', + Metadata: { + 'aws:cdk:path': 'Stack2/Queue/Resource', + }, + }, + }, + }), + }); + + mockCloudFormationClient + .on(GetTemplateCommand, { + StackName: 'CDKToolkit', + }) + .resolves({ + TemplateBody: JSON.stringify({ + Resources: { + CdkBootstrapVersion: { + Type: 'AWS::SSM::Parameter', + Properties: { + Type: 'String', + Name: { + 'Fn::Sub': '/cdk-bootstrap/${Qualifier}/version', + }, + Value: '1', + }, + }, + }, + }), + }); + + await expectRefactorBehavior('stack-with-bucket', + { + dryRun: true, + // We are not passing any filter, which means that Stack2 will also be included in the comparison. + // This results in the set of deployed resources being different from the local resources, which + // results in an error. + }, + { + action: 'refactor', + level: 'error', + code: 'CDK_TOOLKIT_E8900', + message: expect.stringMatching(/A refactor operation cannot add, remove or update resources/), + }, + ); + + await expectRefactorBehavior('stack-with-bucket', + { + dryRun: true, + // To avoid the error, we tell the toolkit to only consider Stack1 in the deployed stacks. + deployedStacks: ['Stack1'], + }, + { + action: 'refactor', + level: 'result', + code: 'CDK_TOOLKIT_I8900', + message: expect.stringMatching(/AWS::S3::Bucket.*Stack1\/OldLogicalID\/Resource.*Stack1\/MyBucket\/Resource/), + data: expect.objectContaining({ + typedMappings: [ + { + sourcePath: 'Stack1/OldLogicalID/Resource', + destinationPath: 'Stack1/MyBucket/Resource', + type: 'AWS::S3::Bucket', + }, + ], + }), + }, + ); + + await expectRefactorBehavior('two-different-stacks', + { + dryRun: true, + // In this case, we are not passing any filter, either, but local and deployed are + // the same, except for the bootstrap stack. But that is ignored by default. + }, { + action: 'refactor', + level: 'result', + code: 'CDK_TOOLKIT_I8900', + data: expect.objectContaining({ + typedMappings: [ + { + sourcePath: 'Stack1/OldLogicalID/Resource', + destinationPath: 'Stack1/MyBucket/Resource', + type: 'AWS::S3::Bucket', + }, + { + sourcePath: 'Stack2/Queue/Resource', + destinationPath: 'Stack2/MyQueue/Resource', + type: 'AWS::SQS::Queue', + }, + ], + }), + }); + + await expectRefactorBehavior('two-different-stacks', + { + dryRun: true, + // But if we pass a wildcard, even the bootstrap stack will be included in the comparison. + deployedStacks: ['*'], + }, { + action: 'refactor', + level: 'error', + code: 'CDK_TOOLKIT_E8900', + message: expect.stringMatching(/A refactor operation cannot add, remove or update resources/), + }); +}); + test('detects ambiguous mappings', async () => { // GIVEN mockCloudFormationClient.on(ListStacksCommand).resolves({ @@ -106,6 +266,7 @@ test('detects ambiguous mappings', async () => { .resolves({ TemplateBody: JSON.stringify({ Resources: { + // These two buckets were replaced with two other buckets CatPhotos: { Type: 'AWS::S3::Bucket', UpdateReplacePolicy: 'Retain', @@ -127,7 +288,7 @@ test('detects ambiguous mappings', async () => { }); // WHEN - const cx = await builderFixture(toolkit, 'stack-with-bucket'); + const cx = await builderFixture(toolkit, 'stack-with-two-buckets'); await toolkit.refactor(cx, { dryRun: true, }); @@ -145,19 +306,84 @@ test('detects ambiguous mappings', async () => { │ - │ Stack1/CatPhotos/Resource │ │ │ Stack1/DogPhotos/Resource │ ├───┼───────────────────────────┤ - │ + │ Stack1/Bucket/Resource │ + │ + │ Stack1/MyBucket1/Resource │ + │ │ Stack1/MyBucket2/Resource │ └───┴───────────────────────────┘ */ message: expect.stringMatching( - /-.*Stack1\/CatPhotos\/Resource.*\s+.*Stack1\/DogPhotos\/Resource.*\s+.*\s+.*\+.*Stack1\/MyBucket\/Resource/gm, + /-.*Stack1\/CatPhotos\/Resource.*\s+.*Stack1\/DogPhotos\/Resource.*\s+.*\s+.*\+.*Stack1\/MyBucket1\/Resource.*\s+.*Stack1\/MyBucket2\/Resource/gm, ), data: { - ambiguousPaths: [[['Stack1/CatPhotos/Resource', 'Stack1/DogPhotos/Resource'], ['Stack1/MyBucket/Resource']]], + ambiguousPaths: [ + [ + ['Stack1/CatPhotos/Resource', 'Stack1/DogPhotos/Resource'], + ['Stack1/MyBucket1/Resource', 'Stack1/MyBucket2/Resource'], + ], + ], }, }), ); }); +test('detects modifications to the infrastructure', async () => { + // GIVEN + mockCloudFormationClient.on(ListStacksCommand).resolves({ + StackSummaries: [ + { + StackName: 'Stack1', + StackId: 'arn:aws:cloudformation:us-east-1:999999999999:stack/Stack1', + StackStatus: 'CREATE_COMPLETE', + CreationTime: new Date(), + }, + ], + }); + + mockCloudFormationClient + .on(GetTemplateCommand, { + StackName: 'Stack1', + }) + .resolves({ + TemplateBody: JSON.stringify({ + Resources: { + // This resource would be refactored + OldName: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldName/Resource', + }, + }, + // But there is an additional resource that will prevent it + Queue: { + Type: 'AWS::S3::Queue', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/Queue/Resource', + }, + }, + }, + }), + }); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: true, + }); + + // THEN + expect(ioHost.notifySpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'refactor', + level: 'error', + code: 'CDK_TOOLKIT_E8900', + message: expect.stringMatching(/A refactor operation cannot add, remove or update resources/), + }), + ); +}); + test('fails when dry-run is false', async () => { const cx = await builderFixture(toolkit, 'stack-with-bucket'); await expect( @@ -226,10 +452,8 @@ test('filters stacks when stack selector is passed', async () => { const cx = await builderFixture(toolkit, 'two-different-stacks'); await toolkit.refactor(cx, { dryRun: true, - stacks: { - strategy: StackSelectionStrategy.PATTERN_MATCH, - patterns: ['Stack1'], - }, + localStacks: ['Stack1'], + deployedStacks: ['Stack1'], }); // Resources were renamed in both stacks, but we are only including Stack1. @@ -416,13 +640,15 @@ test('uses the reverse of an explicit mapping when provided', async () => { await toolkit.refactor(cx, { dryRun: true, // ... this is the mapping we used, and now we want to revert it - mappingSource: MappingSource.reverse([{ - account: '123456789012', - region: 'us-east-1', - resources: { - 'Stack1.OldLogicalID': 'Stack1.NewLogicalID', + mappingSource: MappingSource.reverse([ + { + account: '123456789012', + region: 'us-east-1', + resources: { + 'Stack1.OldLogicalID': 'Stack1.NewLogicalID', + }, }, - }]), + ]), }); // THEN @@ -519,18 +745,24 @@ test('computes one set of mappings per environment', async () => { }); // THEN - expect(ioHost.notifySpy).toHaveBeenCalledTimes(3); + expect(ioHost.notifySpy).toHaveBeenCalledTimes(4); - expect(ioHost.notifySpy).toHaveBeenNthCalledWith(2, expect.objectContaining({ - message: expect.stringMatching('aws://123456789012/us-east-1'), - })); + expect(ioHost.notifySpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + message: expect.stringMatching('aws://123456789012/us-east-1'), + }), + ); - expect(ioHost.notifySpy).toHaveBeenNthCalledWith(2, + expect(ioHost.notifySpy).toHaveBeenNthCalledWith( + 2, expect.objectContaining({ action: 'refactor', level: 'result', code: 'CDK_TOOLKIT_I8900', - message: expect.stringMatching(/AWS::S3::Bucket.*Stack1\/OldBucketName\/Resource.*Stack1\/NewBucketNameInStack1\/Resource/), + message: expect.stringMatching( + /AWS::S3::Bucket.*Stack1\/OldBucketName\/Resource.*Stack1\/NewBucketNameInStack1\/Resource/, + ), data: expect.objectContaining({ typedMappings: [ { @@ -543,16 +775,22 @@ test('computes one set of mappings per environment', async () => { }), ); - expect(ioHost.notifySpy).toHaveBeenNthCalledWith(3, expect.objectContaining({ - message: expect.stringMatching('aws://123456789012/us-east-2'), - })); + expect(ioHost.notifySpy).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + message: expect.stringMatching('aws://123456789012/us-east-2'), + }), + ); - expect(ioHost.notifySpy).toHaveBeenNthCalledWith(3, + expect(ioHost.notifySpy).toHaveBeenNthCalledWith( + 4, expect.objectContaining({ action: 'refactor', level: 'result', code: 'CDK_TOOLKIT_I8900', - message: expect.stringMatching(/AWS::S3::Bucket.*Stack2\/OldBucketName\/Resource.*Stack2\/NewBucketNameInStack2\/Resource/), + message: expect.stringMatching( + /AWS::S3::Bucket.*Stack2\/OldBucketName\/Resource.*Stack2\/NewBucketNameInStack2\/Resource/, + ), data: expect.objectContaining({ typedMappings: [ { @@ -565,3 +803,11 @@ test('computes one set of mappings per environment', async () => { }), ); }); + +async function expectRefactorBehavior(fixtureName: string, input: RefactorOptions, output: E) { + const host = new TestIoHost(); + const tk = new Toolkit({ ioHost: host, unstableFeatures: ['refactor'] }); + await tk.refactor(await builderFixture(tk, fixtureName), input); + expect(host.notifySpy).toHaveBeenCalledWith(expect.objectContaining(output)); +} + diff --git a/packages/@aws-cdk/toolkit-lib/test/api/refactoring/context.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/refactoring/context.test.ts index e7c895a5e..17051183f 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/refactoring/context.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/refactoring/context.test.ts @@ -50,7 +50,7 @@ describe('typed mappings', () => { expect(context.mappings).toEqual([]); }); - test('returns empty mappings when there are only removals', () => { + test('resource removal is not allowed', () => { const stack1 = { environment, stackName: 'Foo', @@ -73,15 +73,14 @@ describe('typed mappings', () => { }, }; - const context = new RefactoringContext({ + expect(() => new RefactoringContext({ environment, deployedStacks: [stack1], localStacks: [stack2], - }); - expect(context.mappings).toEqual([]); + })).toThrow(/A refactor operation cannot add, remove or update resources/); }); - test('returns empty mappings when there are only additions', () => { + test('resource addition is not allowed', () => { const stack1 = { environment, stackName: 'Foo', @@ -104,15 +103,14 @@ describe('typed mappings', () => { }, }; - const context = new RefactoringContext({ + expect(() => new RefactoringContext({ environment, deployedStacks: [stack1], localStacks: [stack2], - }); - expect(context.mappings).toEqual([]); + })).toThrow(/A refactor operation cannot add, remove or update resources/); }); - test('normal updates are not mappings', () => { + test('resource update is not allowed', () => { const stack1 = { environment, stackName: 'Foo', @@ -136,17 +134,17 @@ describe('typed mappings', () => { Bucket1: { Type: 'AWS::S3::Bucket', // Updated property - Properties: { Prop: 'old value' }, + Properties: { Prop: 'new value' }, }, }, }, }; - const context = new RefactoringContext({ + + expect(() => new RefactoringContext({ environment, deployedStacks: [stack1], localStacks: [stack2], - }); - expect(context.mappings).toEqual([]); + })).toThrow(/A refactor operation cannot add, remove or update resources/); }); test('moving resources across stacks', () => { @@ -299,16 +297,14 @@ describe('typed mappings', () => { }, }; - const context = new RefactoringContext({ - environment, - deployedStacks: [stack1], - localStacks: [stack2], - }); - // We don't consider that a resource was moved from Foo.OldName to Bar.NewName, // even though they have the same properties. Since they have different types, // they are considered different resources. - expect(context.mappings).toEqual([]); + expect(() => new RefactoringContext({ + environment, + deployedStacks: [stack1], + localStacks: [stack2], + })).toThrow(/A refactor operation cannot add, remove or update resources/); }); test('ambiguous resources from multiple stacks', () => { @@ -351,12 +347,11 @@ describe('typed mappings', () => { }, }; - const context = new RefactoringContext({ + expect(() => new RefactoringContext({ environment, deployedStacks: [stack1, stack2], localStacks: [stack3], - }); - expect(context.ambiguousPaths).toEqual([[['Stack1.Bucket1', 'Stack2.Bucket2'], ['Stack3.Bucket3']]]); + })).toThrow(/A refactor operation cannot add, remove or update resources/); }); test('ambiguous pairs', () => { @@ -450,174 +445,11 @@ describe('typed mappings', () => { }, }; - const context = new RefactoringContext({ + expect(() => new RefactoringContext({ environment, deployedStacks: [stack1], localStacks: [stack2], - }); - expect(context.mappings.map(toCfnMapping)).toEqual([ - { - Source: { LogicalResourceId: 'OldName', StackName: 'Foo' }, - Destination: { LogicalResourceId: 'NewName', StackName: 'Foo' }, - }, - ]); - }); - - test('stack filtering', () => { - // eslint-disable-next-line @typescript-eslint/no-shadow - const environment = { - name: 'prod', - account: '123456789012', - region: 'us-east-1', - }; - - // Scenario: - // Foo.Bucket1 -> Bar.Bucket1 - // Zee.OldName -> Zee.NewName - - const stack1 = { - environment, - stackName: 'Foo', - template: { - Resources: { - Bucket1: { - Type: 'AWS::S3::Bucket', - Properties: { Prop: 'XXXXXXXXX' }, - }, - }, - }, - }; - - const stack2 = { - environment, - stackName: 'Bar', - template: { - Resources: { - Bucket1: { - Type: 'AWS::S3::Bucket', - Properties: { Prop: 'XXXXXXXXX' }, - }, - }, - }, - }; - - const stack3 = { - environment, - stackName: 'Zee', - template: { - Resources: { - OldName: { - Type: 'AWS::SQS::Queue', - Properties: { Prop: 'YYYYYYYYY' }, - }, - }, - }, - }; - - const stack4 = { - environment, - stackName: 'Zee', - template: { - Resources: { - NewName: { - Type: 'AWS::SQS::Queue', - Properties: { Prop: 'YYYYYYYYY' }, - }, - }, - }, - }; - - // Testing different filters: - - // Only Foo. Should include Foo and Bar - let context = new RefactoringContext({ - environment, - deployedStacks: [stack1, stack3], - localStacks: [stack2, stack4], - filteredStacks: [stack1], - }); - expect(context.mappings.map(toCfnMapping)).toEqual([ - { - Destination: { - LogicalResourceId: 'Bucket1', - StackName: 'Bar', - }, - Source: { - LogicalResourceId: 'Bucket1', - StackName: 'Foo', - }, - }, - ]); - - // Only Bar. Should include Foo and Bar - context = new RefactoringContext({ - environment, - deployedStacks: [stack1, stack3], - localStacks: [stack2, stack4], - filteredStacks: [stack2], - }); - expect(context.mappings.map(toCfnMapping)).toEqual([ - { - Destination: { - LogicalResourceId: 'Bucket1', - StackName: 'Bar', - }, - Source: { - LogicalResourceId: 'Bucket1', - StackName: 'Foo', - }, - }, - ]); - - // Only Zee. Should include Zee - context = new RefactoringContext({ - environment, - deployedStacks: [stack1, stack3], - localStacks: [stack2, stack4], - filteredStacks: [stack3], - }); - expect(context.mappings.map(toCfnMapping)).toEqual([ - { - Destination: { - LogicalResourceId: 'NewName', - StackName: 'Zee', - }, - Source: { - LogicalResourceId: 'OldName', - StackName: 'Zee', - }, - }, - ]); - - // Foo and Zee. Should include all - context = new RefactoringContext({ - environment, - deployedStacks: [stack1, stack3], - localStacks: [stack2, stack4], - filteredStacks: [stack1, stack3], - }); - expect(context.mappings.map(toCfnMapping)).toEqual([ - { - Destination: { - LogicalResourceId: 'Bucket1', - StackName: 'Bar', - }, - Source: { - LogicalResourceId: 'Bucket1', - StackName: 'Foo', - }, - }, - { - Destination: { - LogicalResourceId: 'NewName', - StackName: 'Zee', - }, - Source: { - LogicalResourceId: 'OldName', - StackName: 'Zee', - }, - }, - ]); + })).toThrow(/A refactor operation cannot add, remove or update resources/); }); }); 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 113f1a2c8..367f2f593 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 @@ -334,74 +334,6 @@ describe(computeResourceDigests, () => { expect(result['Stack1.Q1']).toBe(result['Stack1.Q2']); }); - test('uses physical ID if present', () => { - mockLoadResourceModel.mockReturnValue({ - primaryIdentifier: ['FooName'], - }); - - const template = { - Resources: { - Foo1: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - ShouldBeIgnored: true, - }, - }, - Foo2: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - ShouldAlsoBeIgnored: true, - }, - }, - }, - }; - - const result = computeResourceDigests(makeStacks([template])); - expect(result['Stack1.Foo1']).toBeDefined(); - expect(result['Stack1.Foo2']).toBeDefined(); - expect(result['Stack1.Foo1']).toEqual(result['Stack1.Foo2']); - }); - - test('uses physical ID if present - with dependencies', () => { - mockLoadResourceModel.mockReturnValue({ - primaryIdentifier: ['FooName'], - }); - - const template = { - Resources: { - Foo1: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - ShouldBeIgnored: true, - }, - }, - Foo2: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - ShouldAlsoBeIgnored: true, - }, - }, - Bucket1: { - Type: 'AWS::S3::Bucket', - Properties: { Dep: { Ref: 'Foo1' } }, - }, - Bucket2: { - Type: 'AWS::S3::Bucket', - Properties: { Dep: { Ref: 'Foo2' } }, - }, - }, - }; - - const result = computeResourceDigests(makeStacks([template])); - expect(result['Stack1.Bucket1']).toBeDefined(); - expect(result['Stack1.Bucket2']).toBeDefined(); - expect(result['Stack1.Bucket1']).toEqual(result['Stack1.Bucket2']); - }); - test('different physical IDs lead to different digests', () => { mockLoadResourceModel.mockReturnValue({ primaryIdentifier: ['FooName'], @@ -464,68 +396,6 @@ describe(computeResourceDigests, () => { expect(result['Stack1.Foo1']).not.toEqual(result['Stack1.Foo2']); }); - test('primaryIdentifier is a composite field - same value', () => { - mockLoadResourceModel.mockReturnValue({ - primaryIdentifier: ['FooName', 'BarName'], - }); - - const template = { - Resources: { - Foo1: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - BarName: 'YYYYYYYYY', - ShouldBeIgnored: true, - }, - }, - Foo2: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - BarName: 'YYYYYYYYY', - ShouldAlsoBeIgnored: true, - }, - }, - }, - }; - - const result = computeResourceDigests(makeStacks([template])); - expect(result['Stack1.Foo1']).toBeDefined(); - expect(result['Stack1.Foo2']).toBeDefined(); - expect(result['Stack1.Foo1']).toEqual(result['Stack1.Foo2']); - }); - - test('primaryIdentifier is a composite field but not all of them are set in the resource', () => { - mockLoadResourceModel.mockReturnValue({ - primaryIdentifier: ['FooName', 'BarName'], - }); - - const template = { - Resources: { - Foo1: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - ShouldBeIgnored: true, - }, - }, - Foo2: { - Type: 'AWS::S3::Foo', - Properties: { - FooName: 'XXXXXXXXX', - ShouldAlsoBeIgnored: true, - }, - }, - }, - }; - - const result = computeResourceDigests(makeStacks([template])); - expect(result['Stack1.Foo1']).toBeDefined(); - expect(result['Stack1.Foo2']).toBeDefined(); - expect(result['Stack1.Foo1']).not.toEqual(result['Stack1.Foo2']); - }); - test('resource properties does not contain primaryIdentifier - different values', () => { mockLoadResourceModel.mockReturnValue({ primaryIdentifier: ['FooName'], diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index b1dcf95b3..d966a250d 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -1101,8 +1101,12 @@ when using this command. Compares the infrastructure specified in the current state of the CDK app with the currently deployed application, to determine if any resource was moved -(to a different stack or to a different logical ID, or both). The CLI will -show the correspondence between the old and new locations in a table: +(to a different stack or to a different logical ID, or both). In keeping with +the CloudFormation API, you are not allowed to modify the set of resources +as part of a refactor. In other words, adding, deleting or updating resources +is considered an error. + +The CLI will show the correspondence between the old and new locations in a table: ``` $ cdk refactor --unstable=refactor --dry-run @@ -1145,13 +1149,17 @@ $ cdk refactor --exclude-file exclude.txt --unstable=refactor --dry-run ``` If your application has more than one stack, and you want the refactor -command to consider only a subset of them, you can pass a list of stack -patterns as a parameter: +command to consider only a subset of them, you can specify the stacks you +want, both local and deployed: ```shell -$ cdk refactor Web* --unstable=refactor --dry-run +$ cdk refactor --local-stack Foo --local-stack Bar --deployed-stack Foo --unstable=refactor --dry-run ``` +This is useful if, for example, you have more than one CDK application deployed +to a given environment, and you want to only include the deployed stacks that +belong to the application that you are refactoring. + The pattern language is the same as the one used in the `cdk deploy` command. However, unlike `cdk deploy`, in the absence of this parameter, all stacks are considered. diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 2ac0c21c6..3e48e8c7d 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -1227,10 +1227,8 @@ export class CdkToolkit { try { await this.toolkit.refactor(this.props.cloudExecutable, { dryRun: options.dryRun, - stacks: { - patterns: options.selector.patterns, - strategy: options.selector.patterns.length > 0 ? StackSelectionStrategy.PATTERN_MATCH : StackSelectionStrategy.ALL_STACKS, - }, + localStacks: options.localStacks, + deployedStacks: options.deployedStacks, mappingSource: await mappingSource(), }); } catch (e) { @@ -1960,11 +1958,6 @@ export interface RefactorOptions { */ readonly dryRun: boolean; - /** - * Criteria for selecting stacks to deploy - */ - selector: StackSelector; - /** * The absolute path to a file that contains a list of resources that * should be excluded during the refactor. This file should contain a @@ -2012,6 +2005,22 @@ export interface RefactorOptions { * that was previously applied. */ revert?: boolean; + + /** + * List of patterns for filtering local stacks. If no patterns are passed, + * then all stacks, except the bootstrap stacks are considered. If you want + * to consider all stacks (including bootstrap stacks), pass the wildcard + * '*'. + */ + localStacks?: string[]; + + /** + * List of patterns for filtering deployed stacks. If no patterns are passed, + * then all stacks, except the bootstrap stacks are considered. If you want + * to consider all stacks (including bootstrap stacks), pass the wildcard + * '*'. + */ + deployedStacks?: string[]; } /** diff --git a/packages/aws-cdk/lib/cli/cli-config.ts b/packages/aws-cdk/lib/cli/cli-config.ts index 3ea32e677..0975f86a7 100644 --- a/packages/aws-cdk/lib/cli/cli-config.ts +++ b/packages/aws-cdk/lib/cli/cli-config.ts @@ -436,11 +436,15 @@ export async function makeConfig(): Promise { }, refactor: { description: 'Moves resources between stacks or within the same stack', - arg: { - name: 'STACKS', - variadic: true, - }, options: { + 'local-stack': { + type: 'array', + desc: 'Filter to apply for stacks in the cloud assembly', + }, + 'deployed-stack': { + type: 'array', + desc: 'Filter to apply for stacks deployed to the AWS account', + }, 'dry-run': { type: 'boolean', desc: 'Do not perform any changes, just show what would be done', diff --git a/packages/aws-cdk/lib/cli/cli.ts b/packages/aws-cdk/lib/cli/cli.ts index 32dfdbea6..a72d9bd33 100644 --- a/packages/aws-cdk/lib/cli/cli.ts +++ b/packages/aws-cdk/lib/cli/cli.ts @@ -276,10 +276,11 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise): any { }), ) .command('doctor', 'Check your set-up for potential problems') - .command('refactor [STACKS..]', 'Moves resources between stacks or within the same stack', (yargs: Argv) => + .command('refactor', 'Moves resources between stacks or within the same stack', (yargs: Argv) => yargs + .option('local-stack', { + type: 'array', + desc: 'Filter to apply for stacks in the cloud assembly', + nargs: 1, + requiresArg: true, + }) + .option('deployed-stack', { + type: 'array', + desc: 'Filter to apply for stacks deployed to the AWS account', + nargs: 1, + requiresArg: true, + }) .option('dry-run', { default: false, type: 'boolean', diff --git a/packages/aws-cdk/lib/cli/user-input.ts b/packages/aws-cdk/lib/cli/user-input.ts index 0aa41a901..344e438d2 100644 --- a/packages/aws-cdk/lib/cli/user-input.ts +++ b/packages/aws-cdk/lib/cli/user-input.ts @@ -1419,6 +1419,20 @@ export interface DocsOptions { * @struct */ export interface RefactorOptions { + /** + * Filter to apply for stacks in the cloud assembly + * + * @default - undefined + */ + readonly localStack?: Array; + + /** + * Filter to apply for stacks deployed to the AWS account + * + * @default - undefined + */ + readonly deployedStack?: Array; + /** * Do not perform any changes, just show what would be done * @@ -1446,9 +1460,4 @@ export interface RefactorOptions { * @default - false */ readonly revert?: boolean; - - /** - * Positional argument for refactor - */ - readonly STACKS?: Array; } From b2ad63d430c9900b082397776d94ee0ea9b9ee9e Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Tue, 1 Jul 2025 11:39:41 +0100 Subject: [PATCH 02/10] feat: refactor execution --- .projenrc.ts | 1 + .../@aws-cdk/toolkit-lib/.projen/deps.json | 4 + .../@aws-cdk/toolkit-lib/.projen/tasks.json | 2 +- .../toolkit-lib/docs/message-registry.md | 1 + .../toolkit-lib/lib/actions/refactor/index.ts | 5 + .../toolkit-lib/lib/api/aws-auth/sdk.ts | 40 + .../lib/api/io/private/messages.ts | 17 +- .../lib/api/refactoring/cloudformation.ts | 18 + .../lib/api/refactoring/context.ts | 56 + .../lib/api/refactoring/stack-definitions.ts | 618 +------ .../toolkit-lib/lib/toolkit/toolkit.ts | 38 +- packages/@aws-cdk/toolkit-lib/package.json | 1 + .../toolkit-lib/test/actions/refactor.test.ts | 488 ++++- .../test/api/refactoring/refactoring.test.ts | 1629 ++--------------- packages/aws-cdk/lib/cli/cdk-toolkit.ts | 6 + packages/aws-cdk/lib/cli/cli-config.ts | 5 + packages/aws-cdk/lib/cli/cli.ts | 1 + .../aws-cdk/lib/cli/convert-to-user-input.ts | 2 + .../lib/cli/parse-command-line-arguments.ts | 5 + packages/aws-cdk/lib/cli/user-input.ts | 7 + 20 files changed, 864 insertions(+), 2080 deletions(-) diff --git a/.projenrc.ts b/.projenrc.ts index 78cd20272..a08b4b209 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -802,6 +802,7 @@ const toolkitLib = configureProject( 'cdk-from-cfn', 'chalk@^4', 'chokidar@^3', + 'fast-deep-equal', 'fs-extra@^9', 'glob', 'minimatch', diff --git a/packages/@aws-cdk/toolkit-lib/.projen/deps.json b/packages/@aws-cdk/toolkit-lib/.projen/deps.json index 8525a9819..d47070aa0 100644 --- a/packages/@aws-cdk/toolkit-lib/.projen/deps.json +++ b/packages/@aws-cdk/toolkit-lib/.projen/deps.json @@ -332,6 +332,10 @@ "version": "^3", "type": "runtime" }, + { + "name": "fast-deep-equal", + "type": "runtime" + }, { "name": "fs-extra", "version": "^9", diff --git a/packages/@aws-cdk/toolkit-lib/.projen/tasks.json b/packages/@aws-cdk/toolkit-lib/.projen/tasks.json index 06febfc3d..6ea7e094e 100644 --- a/packages/@aws-cdk/toolkit-lib/.projen/tasks.json +++ b/packages/@aws-cdk/toolkit-lib/.projen/tasks.json @@ -53,7 +53,7 @@ }, "steps": [ { - "exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --no-deprecated --dep=dev,peer,prod,optional --filter=@aws-cdk/aws-service-spec,@cdklabs/eslint-plugin,@jest/environment,@jest/globals,@jest/types,@microsoft/api-extractor,@smithy/util-stream,@types/fs-extra,@types/jest,@types/jest-when,@types/split2,aws-cdk-lib,aws-sdk-client-mock,aws-sdk-client-mock-jest,eslint-config-prettier,eslint-import-resolver-typescript,eslint-plugin-import,eslint-plugin-jest,eslint-plugin-jsdoc,eslint-plugin-prettier,fast-check,jest,jest-environment-node,jest-when,license-checker,ts-jest,xml-js,archiver,cdk-from-cfn,glob,minimatch,semver,split2,uuid" + "exec": "npx npm-check-updates@16 --upgrade --target=minor --peer --no-deprecated --dep=dev,peer,prod,optional --filter=@aws-cdk/aws-service-spec,@cdklabs/eslint-plugin,@jest/environment,@jest/globals,@jest/types,@microsoft/api-extractor,@smithy/util-stream,@types/fs-extra,@types/jest,@types/jest-when,@types/split2,aws-cdk-lib,aws-sdk-client-mock,aws-sdk-client-mock-jest,eslint-config-prettier,eslint-import-resolver-typescript,eslint-plugin-import,eslint-plugin-jest,eslint-plugin-jsdoc,eslint-plugin-prettier,fast-check,jest,jest-environment-node,jest-when,license-checker,ts-jest,xml-js,archiver,cdk-from-cfn,fast-deep-equal,glob,minimatch,semver,split2,uuid" } ] }, diff --git a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md index faf35634d..813713991 100644 --- a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md +++ b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md @@ -127,6 +127,7 @@ Please let us know by [opening an issue](https://github.com/aws/aws-cdk-cli/issu | `CDK_TOOLKIT_E7900` | Stack deletion failed | `error` | {@link ErrorPayload} | | `CDK_TOOLKIT_E8900` | Stack refactor failed | `error` | {@link ErrorPayload} | | `CDK_TOOLKIT_I8900` | Refactor result | `result` | {@link RefactorResult} | +| `CDK_TOOLKIT_I8910` | Confirm refactor | `info` | {@link ConfirmationRequest} | | `CDK_TOOLKIT_W8010` | Refactor execution not yet supported | `warn` | n/a | | `CDK_TOOLKIT_I9000` | Provides bootstrap times | `info` | {@link Duration} | | `CDK_TOOLKIT_I9100` | Bootstrap progress | `info` | {@link BootstrapEnvironmentProgress} | diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts index 4080c3959..52382b627 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts @@ -89,6 +89,11 @@ export interface RefactorOptions { * '*'. */ deployedStacks?: string[]; + + /** + * Whether to do the refactor without prompting the user for confirmation. + */ + force?: boolean; } export interface MappingGroup { diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk.ts b/packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk.ts index 028832e78..caeed6644 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk.ts @@ -50,6 +50,7 @@ import type { CreateGeneratedTemplateCommandOutput, CreateStackCommandInput, CreateStackCommandOutput, + CreateStackRefactorCommandInput, DeleteChangeSetCommandInput, DeleteChangeSetCommandOutput, DeleteGeneratedTemplateCommandInput, @@ -97,6 +98,9 @@ import type { DetectStackResourceDriftCommandInput, DetectStackResourceDriftCommandOutput, DescribeStackResourceDriftsCommandInput, + ExecuteStackRefactorCommandInput, + DescribeStackRefactorCommandInput, + CreateStackRefactorCommandOutput, ExecuteStackRefactorCommandOutput, } from '@aws-sdk/client-cloudformation'; import { paginateListStacks, @@ -105,6 +109,7 @@ import { CreateChangeSetCommand, CreateGeneratedTemplateCommand, CreateStackCommand, + CreateStackRefactorCommand, DeleteChangeSetCommand, DeleteGeneratedTemplateCommand, DeleteStackCommand, @@ -115,6 +120,7 @@ import { DescribeStackResourcesCommand, DescribeStacksCommand, ExecuteChangeSetCommand, + ExecuteStackRefactorCommand, GetGeneratedTemplateCommand, GetTemplateCommand, GetTemplateSummaryCommand, @@ -132,6 +138,8 @@ import { DescribeStackResourceDriftsCommand, DetectStackDriftCommand, DetectStackResourceDriftCommand, + waitUntilStackRefactorCreateComplete, + waitUntilStackRefactorExecuteComplete, } from '@aws-sdk/client-cloudformation'; import type { FilterLogEventsCommandInput, @@ -460,6 +468,10 @@ export interface ICloudFormationClient { describeStackEvents(input: DescribeStackEventsCommandInput): Promise; listStackResources(input: ListStackResourcesCommandInput): Promise; paginatedListStacks(input: ListStacksCommandInput): Promise; + createStackRefactor(input: CreateStackRefactorCommandInput): Promise; + executeStackRefactor(input: ExecuteStackRefactorCommandInput): Promise; + waitUntilStackRefactorCreateComplete(input: DescribeStackRefactorCommandInput): Promise; + waitUntilStackRefactorExecuteComplete(input: DescribeStackRefactorCommandInput): Promise; } export interface ICloudWatchLogsClient { @@ -766,6 +778,34 @@ export class SDK { } return stackResources; }, + createStackRefactor: (input: CreateStackRefactorCommandInput): Promise => { + return client.send(new CreateStackRefactorCommand(input)); + }, + executeStackRefactor: (input: ExecuteStackRefactorCommandInput): Promise => { + return client.send(new ExecuteStackRefactorCommand(input)); + }, + waitUntilStackRefactorCreateComplete: (input: DescribeStackRefactorCommandInput): Promise => { + return waitUntilStackRefactorCreateComplete( + { + client, + maxWaitTime: 600, + minDelay: 6, + maxDelay: 6, + }, + input, + ); + }, + waitUntilStackRefactorExecuteComplete: (input: DescribeStackRefactorCommandInput): Promise => { + return waitUntilStackRefactorExecuteComplete( + { + client, + maxWaitTime: 600, + minDelay: 6, + maxDelay: 6, + }, + input, + ); + }, }; } diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts index 9314e8530..a48fc44bd 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts @@ -17,7 +17,16 @@ import type { StackRollbackProgress } from '../../../payloads/rollback'; import type { MfaTokenRequest, SdkTrace } from '../../../payloads/sdk'; import type { StackActivity, StackMonitoringControlEvent } from '../../../payloads/stack-activity'; import type { StackSelectionDetails } from '../../../payloads/synth'; -import type { AssemblyData, ConfirmationRequest, ContextProviderMessageSource, Duration, ErrorPayload, SingleStack, StackAndAssemblyData } from '../../../payloads/types'; +import type { + AssemblyData, + ConfirmationRequest, + ContextProviderMessageSource, + DataRequest, + Duration, + ErrorPayload, + SingleStack, + StackAndAssemblyData, +} from '../../../payloads/types'; import type { FileWatchEvent, WatchSettings } from '../../../payloads/watch'; /** @@ -381,6 +390,12 @@ export const IO = { interface: 'RefactorResult', }), + CDK_TOOLKIT_I8910: make.question({ + code: 'CDK_TOOLKIT_I8910', + description: 'Confirm refactor', + interface: 'ConfirmationRequest', + }), + CDK_TOOLKIT_W8010: make.warn({ code: 'CDK_TOOLKIT_W8010', description: 'Refactor execution not yet supported', diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts index 5f2c51461..ce1996e30 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts @@ -1,5 +1,6 @@ import type { TypedMapping } from '@aws-cdk/cloudformation-diff'; import type * as cxapi from '@aws-cdk/cx-api'; +import type { ResourceMapping as CfnResourceMapping } from '@aws-sdk/client-cloudformation'; export interface CloudFormationResource { Type: string; @@ -51,6 +52,10 @@ export class ResourceLocation { public equalTo(other: ResourceLocation): boolean { return this.logicalResourceId === other.logicalResourceId && this.stack.stackName === other.stack.stackName; } + + public get stackName(): string { + return this.stack.stackName; + } } /** @@ -69,4 +74,17 @@ export class ResourceMapping { destinationPath: this.destination.toPath(), }; } + + public toCloudFormation(): CfnResourceMapping { + return { + Source: { + StackName: this.source.stack.stackName, + LogicalResourceId: this.source.logicalResourceId, + }, + Destination: { + StackName: this.destination.stack.stackName, + LogicalResourceId: this.destination.logicalResourceId, + }, + }; + } } 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 0de295785..1ba925a96 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts @@ -1,8 +1,14 @@ import type { Environment } from '@aws-cdk/cx-api'; +import type { StackDefinition } from '@aws-sdk/client-cloudformation'; import type { CloudFormationStack } from './cloudformation'; import { ResourceLocation, ResourceMapping } from './cloudformation'; import { computeResourceDigests } from './digest'; import { ToolkitError } from '../../toolkit/toolkit-error'; +import type { SDK } from '../aws-auth/sdk'; +import type { SdkProvider } from '../aws-auth/sdk-provider'; +import { EnvironmentResourcesRegistry } from '../environment'; +import type { IoHelper } from '../io/private'; +import { Mode } from '../plugin'; /** * Represents a set of possible moves of a resource from one location @@ -49,6 +55,56 @@ export class RefactoringContext { public get mappings(): ResourceMapping[] { return this._mappings; } + + public async execute(stackDefinitions: StackDefinition[], sdkProvider: SdkProvider, ioHelper: IoHelper): Promise { + if (this.mappings.length === 0) { + return; + } + + const sdk = (await sdkProvider.forEnvironment(this.environment, Mode.ForWriting)).sdk; + + await this.checkBootstrapVersion(sdk, ioHelper); + + const cfn = sdk.cloudFormation(); + const mappings = this.mappings; + + const input = { + EnableStackCreation: true, + ResourceMappings: mappings.map((m) => m.toCloudFormation()), + StackDefinitions: stackDefinitions, + }; + const refactor = await cfn.createStackRefactor(input); + + await cfn.waitUntilStackRefactorCreateComplete({ + StackRefactorId: refactor.StackRefactorId, + }); + + await cfn.executeStackRefactor({ + StackRefactorId: refactor.StackRefactorId, + }); + + await cfn.waitUntilStackRefactorExecuteComplete({ + StackRefactorId: refactor.StackRefactorId, + }); + } + + private async checkBootstrapVersion(sdk: SDK, ioHelper: IoHelper) { + const environmentResourcesRegistry = new EnvironmentResourcesRegistry(); + const envResources = environmentResourcesRegistry.for(this.environment, sdk, ioHelper); + let bootstrapVersion: number | undefined = undefined; + try { + // Try to get the bootstrap version + bootstrapVersion = (await envResources.lookupToolkit()).version; + } catch (e) { + // But if we can't, keep going. Maybe we can still succeed. + } + if (bootstrapVersion != null && bootstrapVersion < 28) { + const environment = `aws://${this.environment.account}/${this.environment.region}`; + throw new ToolkitError( + `The CDK toolkit stack in environment ${environment} doesn't support refactoring. Please run 'cdk bootstrap ${environment}' to update it.`, + ); + } + } } function resourceMoves(before: CloudFormationStack[], after: CloudFormationStack[]): ResourceMove[] { diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts index 4b449a946..ad41bf0a9 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts @@ -1,593 +1,61 @@ -/* - * The Cloudformation refactoring API needs, in addition to the mappings, the - * resulting templates for each affected stack. The resulting templates are - * basically the synthesis produced, but with some differences: - * - * - Resources that exist in the local stacks, but not in the remote stacks, are - * not included. - * - Resources that exist in the remote stacks, but not in the local stacks, are - * preserved. - * - For resources that exist in both stacks, but have different properties, the - * deployed properties are used, but the references may need to be updated, if - * the resources they reference were moved in the refactoring. - * - * Why does the last difference exist, to begin with? By default, to establish - * whether two given resources are the same, roughly speaking we compute the hash - * of their properties and compare them. But there is a better source of resource - * identity, that we can exploit when it is present: the physical name. In such - * cases, we can track a resource move even if the properties are different, as - * long as the physical name is the same. - * - * The process of computing the resulting templates consists in: - * - * 1. Computing a graph of deployed resources. - * 2. Mapping edges and nodes according to the mappings (that we either - * computed or got directly from the user). - * 3. Computing the resulting templates by traversing the graph and - * collecting the resources that are not mapped out, and updating the - * references to the resources that were moved. - */ - import type { StackDefinition } from '@aws-sdk/client-cloudformation'; -import type { CloudFormationStack, CloudFormationTemplate, ResourceMapping } from './cloudformation'; -import { ResourceLocation } from './cloudformation'; -import { ToolkitError } from '../../toolkit/toolkit-error'; +import type { CloudFormationStack, ResourceMapping } from './cloudformation'; +// namespace object imports won't work in the bundle for function exports +// eslint-disable-next-line @typescript-eslint/no-require-imports +const deepEqual = require('fast-deep-equal'); export function generateStackDefinitions( mappings: ResourceMapping[], deployedStacks: CloudFormationStack[], localStacks: CloudFormationStack[], ): StackDefinition[] { - const localExports: Record = indexExports(localStacks); - const deployedExports: Record = indexExports(deployedStacks); - const edgeMapper = new EdgeMapper(mappings); - - // Build a graph of the deployed stacks - const deployedGraph = graph(deployedStacks, deployedExports); - - // Map all the edges, including their endpoints, to their new locations. - const edges = edgeMapper.mapEdges(deployedGraph.edges); - - // All the edges have been mapped, which means that isolated nodes were left behind. Map them too. - const nodes = mapNodes(deployedGraph.isolatedNodes, mappings); - - // Now we can generate the templates for each stack - const templates = generateTemplates(edges, nodes, edgeMapper.affectedStackNames, localExports, deployedStacks); - - // Finally, generate the stack definitions, to be included in the refactor request. - return Object.entries(templates).map(([stackName, template]) => ({ - StackName: stackName, - TemplateBody: JSON.stringify(template), - })); -} - -function graph(deployedStacks: CloudFormationStack[], deployedExports: Record): -{ edges: ResourceEdge[]; isolatedNodes: ResourceNode[] } { - const deployedNodeMap: Map = buildNodes(deployedStacks); - const deployedNodes = Array.from(deployedNodeMap.values()); - - const edges = buildEdges(deployedNodeMap, deployedExports); - - const isolatedNodes = deployedNodes.filter((node) => { - return !edges.some( - (edge) => - edge.source.location.equalTo(node.location) || - edge.targets.some((target) => target.location.equalTo(node.location)), - ); - }); - - return { edges, isolatedNodes }; -} - -function buildNodes(stacks: CloudFormationStack[]): Map { - const result = new Map(); - - for (const stack of stacks) { - const template = stack.template; - for (const [logicalId, resource] of Object.entries(template.Resources ?? {})) { - const location = new ResourceLocation(stack, logicalId); - result.set(`${stack.stackName}.${logicalId}`, { - location, - rawValue: resource, - }); - } - } - - return result; -} - -function buildEdges( - nodeMap: Map, - exports: Record< - string, - { - stackName: string; - value: any; - } - >, -): ResourceEdge[] { - const nodes = Array.from(nodeMap.values()); - return nodes.flatMap((node) => buildEdgesForResource(node, node.rawValue)); - - function buildEdgesForResource(source: ResourceNode, value: any, path: string[] = []): ResourceEdge[] { - if (!value || typeof value !== 'object') return []; - if (Array.isArray(value)) { - return value.flatMap((x, index) => buildEdgesForResource(source, x, path.concat(String(index)))); - } - - if ('Ref' in value) { - return [makeRef(source.location.stack.stackName, value.Ref)]; - } - - if ('Fn::GetAtt' in value) { - return [makeGetAtt(source.location.stack.stackName, value['Fn::GetAtt'])]; - } - - if ('Fn::ImportValue' in value) { - const exportName = value['Fn::ImportValue']; - const x = exports[exportName]!; - - if ('Ref' in x.value) { - return [ - { - ...makeRef(x.stackName, x.value.Ref), - reference: new ImportValue(Ref.INSTANCE), - }, - ]; + const deployedStackMap: Map = new Map(deployedStacks.map((s) => [s.stackName, s])); + + // For every local stack that is also deployed, update the local template, + // overwriting its CDKMetadata resource with the one from the deployed stack + for (const localStack of localStacks) { + const deployedStack = deployedStackMap.get(localStack.stackName); + if (deployedStack) { + const localTemplate = localStack.template; + const deployedTemplate = deployedStack.template; + localTemplate.Resources = localTemplate.Resources ?? {}; + deployedTemplate.Resources = deployedTemplate.Resources ?? {}; + + // We need to preserve whatever CDKMetadata we had before. + // Otherwise, CloudFormation will consider this a modification. + if (deployedTemplate.Resources?.CDKMetadata != null) { + localTemplate.Resources.CDKMetadata = deployedTemplate.Resources.CDKMetadata; } - if ('Fn::GetAtt' in x.value) { - const getAtt = makeGetAtt(x.stackName, x.value['Fn::GetAtt']); - return [ - { - ...getAtt, - reference: new ImportValue(getAtt.reference), - }, - ]; - } - - return []; - } - - if ('Fn::Sub' in value) { - let inputString: string; - let variables: Record | undefined; - const sub = value['Fn::Sub']; - if (typeof sub === 'string') { - inputString = sub; - } else { - [inputString, variables] = sub; - } - - let varNames = Array.from(inputString.matchAll(/\${([a-zA-Z0-9_.]+)}/g)) - .map((x) => x[1]) - .filter((varName) => (value['Fn::Sub'][1] ?? {})[varName] == null); - - const edges = varNames.map((varName) => { - return varName.includes('.') - ? makeGetAtt(source.location.stack.stackName, varName) - : makeRef(source.location.stack.stackName, varName); - }); - - const edgesFromInputString = [ - { - source, - targets: edges.flatMap((edge) => edge.targets), - reference: new Sub(inputString, varNames), - path: path.concat('Fn::Sub', '0'), - }, - ]; - - const edgesFromVariables = buildEdgesForResource(source, variables, path.concat('Fn::Sub', '1')); - - return [...edgesFromInputString, ...edgesFromVariables]; - } - - const edges: ResourceEdge[] = []; - - // DependsOn is only handled at the top level of the resource - if ('DependsOn' in value && path.length === 0) { - if (typeof value.DependsOn === 'string') { - edges.push({ - ...makeRef(source.location.stack.stackName, value.DependsOn), - reference: DependsOn.INSTANCE, - }); - } else if (Array.isArray(value.DependsOn)) { - edges.push({ - source, - targets: value.DependsOn.flatMap( - (dependsOn: string) => makeRef(source.location.stack.stackName, dependsOn).targets, - ), - path: path.concat('DependsOn'), - reference: DependsOn.INSTANCE, - }); - } - } - - edges.push(...Object.entries(value).flatMap(([k, v]) => buildEdgesForResource(source, v, path.concat(k)))); - - return edges; - - function makeRef(stackName: string, logicalId: string): ResourceEdge { - const key = `${stackName}.${logicalId}`; - const target = nodeMap.get(key)!; - - return { - path, - source, - targets: [target], - reference: Ref.INSTANCE, - }; - } - - function makeGetAtt(stackName: string, att: string | string[]): ResourceEdge { - let logicalId: string = ''; - let attributeName: string = ''; - if (typeof att === 'string') { - [logicalId, attributeName] = att.split(/\.(.*)/s); - } else if (Array.isArray(att) && att.length === 2) { - [logicalId, attributeName] = att; - } - - const key = `${stackName}.${logicalId}`; - const target = nodeMap.get(key)!; - - return { - path, - source, - targets: [target], - reference: new GetAtt(attributeName), - }; - } - } -} - -function mapNodes(nodes: ResourceNode[], mappings: ResourceMapping[]): ResourceNode[] { - return nodes.map((node) => { - const newLocation = mapLocation(node.location, mappings); - return { - location: newLocation, - rawValue: node.rawValue, - } as ResourceNode; - }); -} - -function generateTemplates( - edges: ResourceEdge[], - nodes: ResourceNode[], - stackNames: string[], - exports: Record, - deployedStacks: CloudFormationStack[]): Record { - updateReferences(edges, exports); - const templates: Record = {}; - - // Take the CloudFormation raw value of each the node and put it into the appropriate template. - const allNodes = unique(edges.flatMap((e) => [e.source, ...e.targets]).concat(nodes)); - allNodes.forEach((node) => { - const stackName = node.location.stack.stackName; - const logicalId = node.location.logicalResourceId; - - if (templates[stackName] === undefined) { - templates[stackName] = { - Resources: {}, - }; - } - templates[stackName].Resources![logicalId] = node.rawValue; - }); - - // Add outputs to the templates - edges.forEach((edge) => { - if (edge.reference instanceof ImportValue) { - const stackName = edge.targets[0].location.stack.stackName; - const template = templates[stackName]; - template.Outputs = { - ...(template.Outputs ?? {}), - ...edge.reference.output, - }; - } - }); - - // The freshly generated templates contain only resources and outputs. - // Combine them with the existing templates to preserve metadata and other properties. - return Object.fromEntries( - stackNames.map((stackName) => { - const oldTemplate = deployedStacks.find((s) => s.stackName === stackName)?.template ?? {}; - const newTemplate = templates[stackName] ?? { Resources: {} }; - const combinedTemplate = { ...oldTemplate, ...newTemplate }; - - sanitizeDependencies(combinedTemplate); - return [stackName, combinedTemplate]; - }), - ); -} - -/** - * Update the CloudFormation resources based on information from the edges. - * Each edge corresponds to a path in some resource object. The value at that - * path is updated to the CloudFormation value represented by the edge's annotation. - */ -function updateReferences(edges: ResourceEdge[], exports: Record) { - edges.forEach((edge) => { - const cfnValue = edge.reference.toCfn(edge.targets, exports); - const obj = edge.path.slice(0, edge.path.length - 1).reduce(getPropValue, edge.source.rawValue); - setPropValue(obj, edge.path[edge.path.length - 1], cfnValue); - }); - - function getPropValue(obj: any, prop: string): any { - const index = parseInt(prop); - return obj[Number.isNaN(index) ? prop : index]; - } - - function setPropValue(obj: any, prop: string, value: any) { - const index = parseInt(prop); - obj[Number.isNaN(index) ? prop : index] = value; - } -} - -class EdgeMapper { - public readonly affectedStacks: Set = new Set(); - private readonly nodeMap: Map = new Map(); - - constructor(private readonly mappings: ResourceMapping[]) { - } - - /** - * For each input edge, produce an output edge such that: - * - The source and targets are mapped to their new locations - * - The annotation is converted between in-stack and cross-stack references, as appropriate - */ - mapEdges(edges: ResourceEdge[]): ResourceEdge[] { - return edges - .map((edge) => { - const oldSource = edge.source; - const oldTargets = edge.targets; - const newSource = this.mapNode(oldSource); - const newTargets = oldTargets.map((t) => this.mapNode(t)); - - const oldSourceStackName = oldSource.location.stack.stackName; - const oldTargetStackName = oldTargets[0].location.stack.stackName; - - const newSourceStackName = newSource.location.stack.stackName; - const newTargetStackName = newTargets[0].location.stack.stackName; - - this.affectedStacks.add(newSourceStackName); - this.affectedStacks.add(newTargetStackName); - this.affectedStacks.add(oldSourceStackName); - this.affectedStacks.add(oldTargetStackName); - - let reference: CloudFormationReference = edge.reference; - if (oldSourceStackName === oldTargetStackName && newSourceStackName !== newTargetStackName) { - if (edge.reference instanceof DependsOn) { - return undefined; - } + // For every resource in the local template, take the Metadata['aws:cdk:path'] from the corresponding resource in the deployed template. + // A corresponding resource is one that the local maps to (using the `mappings` parameter). If there is no entry mapping the local + // resource, use the same id + // TODO Remove this logic once CloudFormation starts allowing changes to the construct path. + // But we need it for now, otherwise we won't be able to refactor anything. + for (const [logicalId, localResource] of Object.entries(localTemplate.Resources)) { + const mapping = mappings.find( + (m) => m.destination.stackName === localStack.stackName && m.destination.logicalResourceId === logicalId, + ); - // in-stack reference to cross-stack reference: wrap the old annotation - reference = new ImportValue(edge.reference); - } else if (oldSourceStackName !== oldTargetStackName && newSourceStackName === newTargetStackName) { - // cross-stack reference to in-stack reference: unwrap the old annotation - if (edge.reference instanceof ImportValue) { - reference = edge.reference.reference; + if (mapping != null) { + const deployed = deployedStackMap.get(mapping.source.stackName)!; + const deployedResource = deployed.template?.Resources?.[mapping.source.logicalResourceId]!; + if (deployedResource.Metadata != null || localResource.Metadata != null) { + localResource.Metadata = localResource.Metadata ?? {}; + localResource.Metadata['aws:cdk:path'] = deployedResource?.Metadata?.['aws:cdk:path']; } } - - return { - path: edge.path, - source: newSource, - targets: newTargets, - reference, - }; - }) - .filter((edge) => edge !== undefined); - } - - get affectedStackNames(): string[] { - const fromMappings = this.mappings.flatMap((m) => [m.source.stack.stackName, m.destination.stack.stackName]); - return unique([...this.affectedStacks, ...fromMappings]); - } - - private mapNode(node: ResourceNode): ResourceNode { - const newLocation = mapLocation(node.location, this.mappings); - const key = `${newLocation.stack.stackName}.${newLocation.logicalResourceId}`; - if (!this.nodeMap.has(key)) { - this.nodeMap.set(key, { - location: newLocation, - rawValue: node.rawValue, - }); - } - return this.nodeMap.get(key)!; - } -} - -function mapLocation(location: ResourceLocation, mappings: ResourceMapping[]): ResourceLocation { - const mapping = mappings.find((m) => m.source.equalTo(location)); - if (mapping) { - return mapping.destination; - } - return location; -} - -function indexExports(stacks: CloudFormationStack[]): Record { - return Object.fromEntries( - stacks.flatMap((s) => - Object.entries(s.template.Outputs ?? {}) - .filter( - ([_, o]) => typeof o.Export?.Name === 'string' && (o.Value.Ref != null || o.Value['Fn::GetAtt'] != null), - ) - .map(([name, o]) => [o.Export.Name, { stackName: s.stackName, outputName: name, value: o.Value }]), - ), - ); -} - -function unique(arr: Array) { - return Array.from(new Set(arr)); -} - -/** - * Updates the DependsOn property of all resources, removing references - * to resources that do not exist in the template. Unlike Refs and GetAtts, - * which get transformed to ImportValues when the referenced resource is - * moved to another stack, DependsOn doesn't cross stack boundaries. - */ -function sanitizeDependencies(template: CloudFormationTemplate) { - const resources = template.Resources ?? {}; - for (const resource of Object.values(resources)) { - if (typeof resource.DependsOn === 'string' && resources[resource.DependsOn] == null) { - delete resource.DependsOn; - } - - if (Array.isArray(resource.DependsOn)) { - resource.DependsOn = resource.DependsOn.filter((dep) => resources[dep] != null); - if (resource.DependsOn.length === 0) { - delete resource.DependsOn; } } } -} - -interface ScopedExport { - stackName: string; - outputName: string; - value: any; -} - -interface ResourceNode { - location: ResourceLocation; - rawValue: any; -} - -/** - * An edge in the resource graph, representing a reference from one resource - * to one or more target resources. (Technically, a hyperedge.) - */ -interface ResourceEdge { - /** - * The source resource of the edge. - */ - source: ResourceNode; - - /** - * The target resources of the edge. In case of DependsOn, - * this can be multiple resources. - */ - targets: ResourceNode[]; - - /** - * The path in the source resource where the reference is located. - */ - path: string[]; - - /** - * The CloudFormation reference that this edge represents. - */ - reference: CloudFormationReference; -} - -interface CloudFormationReference { - toCfn(targets: ResourceNode[], exports: Record): any; -} - -class Ref implements CloudFormationReference { - public static INSTANCE = new Ref(); - - private constructor() { - } - - toCfn(targets: ResourceNode[]): any { - return { Ref: targets[0].location.logicalResourceId }; - } -} - -class GetAtt implements CloudFormationReference { - constructor(public readonly attributeName: string) { - } - - toCfn(targets: ResourceNode[]): any { - return { - 'Fn::GetAtt': [targets[0].location.logicalResourceId, this.attributeName], - }; - } -} - -class ImportValue implements CloudFormationReference { - private outputName?: string; - private outputContent?: any; - - constructor(public readonly reference: CloudFormationReference) { - } - - toCfn(targets: ResourceNode[], exports: Record): any { - const exp = this.findExport(targets, exports); - if (exp) { - this.outputName = exp[1].outputName; - this.outputContent = { - Value: exp[1].value, - Export: { - Name: exp[0], - }, - }; - return { 'Fn::ImportValue': exp[0] }; - } - // TODO better message - throw new ToolkitError('Unknown export for ImportValue: ' + JSON.stringify(this.reference)); - } - private findExport(targets: ResourceNode[], exports: Record) { - const target = targets[0]; - if (this.reference instanceof Ref) { - return Object.entries(exports).find(([_, exportValue]) => { - return ( - exportValue.stackName === target.location.stack.stackName && - exportValue.value.Ref === target.location.logicalResourceId - ); - }); - } else { - return Object.entries(exports).find(([_, exportValue]) => { - const getAtt = this.reference as GetAtt; - - return ( - exportValue.stackName === target.location.stack.stackName && - exportValue.value['Fn::GetAtt'] && - ((exportValue.value['Fn::GetAtt'][0] === target.location.logicalResourceId && - exportValue.value['Fn::GetAtt'][1] === getAtt.attributeName) || - exportValue.value['Fn::GetAtt'] === `${target.location.logicalResourceId}.${getAtt.attributeName}`) - ); - }); - } - } - - get output(): Record { - if (this.outputName == null) { - throw new ToolkitError('Cannot access output before calling toCfn'); - } - return { [this.outputName]: this.outputContent }; - } -} - -class Sub implements CloudFormationReference { - constructor(public readonly inputString: string, public readonly varNames: string[]) { - } - - toCfn(targets: ResourceNode[]): any { - let inputString = this.inputString; - - this.varNames.forEach((varName, index) => { - const [_, attr] = varName.split(/\.(.*)/s); - const target = targets[index]; - inputString = inputString.replace(`\${${varName}`, `\${${target.location.logicalResourceId}${attr ? `.${attr}` : ''}`, - ); - }); - - return inputString; - } -} - -class DependsOn implements CloudFormationReference { - public static INSTANCE = new DependsOn(); - - private constructor() { - } + const stacksToProcess = localStacks.filter((localStack) => { + const deployedStack = deployedStackMap.get(localStack.stackName); + return !deployedStack || !deepEqual(localStack.template, deployedStack.template); + }); - toCfn(targets: ResourceNode[]): any { - return targets.map((t) => t.location.logicalResourceId); - } + return stacksToProcess.map((stack) => ({ + StackName: stack.stackName, + TemplateBody: JSON.stringify(stack.template), + })); } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 3d5438546..726757789 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -68,6 +68,7 @@ import { } from '../api/refactoring'; import type { ResourceMapping } from '../api/refactoring/cloudformation'; import { RefactoringContext } from '../api/refactoring/context'; +import { generateStackDefinitions } from '../api/refactoring/stack-definitions'; import { ResourceMigrator } from '../api/resource-import'; import { tagsForStack } from '../api/tags/private'; import { DEFAULT_TOOLKIT_STACK_NAME } from '../api/toolkit-info'; @@ -1059,10 +1060,6 @@ export class Toolkit extends CloudAssemblySourceBuilder { } private async _refactor(assembly: StackAssembly, ioHelper: IoHelper, options: RefactorOptions = {}): Promise { - if (!options.dryRun) { - throw new ToolkitError('Refactor is not available yet. Too see the proposed changes, use the --dry-run flag.'); - } - const sdkProvider = await this.sdkProvider('refactor'); const groups = await groupStacks(sdkProvider, assembly.cloudAssembly, options.localStacks ?? [], options.deployedStacks ?? []); @@ -1096,14 +1093,47 @@ export class Toolkit extends CloudAssemblySourceBuilder { const typedMappings = mappings .map(m => m.toTypedMapping()) + // Exclude CDK Metadata resources because they will never be refactorable. + // Most of the time, their content will be different from the deployed one. + // Even if the content does remain the same, the location won't change anyway. .filter(m => m.type !== 'AWS::CDK::Metadata'); await notifyInfo(formatTypedMappings(typedMappings), { typedMappings }); + + const stackDefinitions = generateStackDefinitions(mappings, deployedStacks, localStacks); + + if (options.dryRun || context.mappings.length === 0) { + // Nothing left to do. + continue; + } + + // In interactive mode (TTY) we need confirmation before proceeding + if (process.stdout.isTTY && !await confirm(options.force ?? false)) { + await notifyInfo(chalk.red(`Refactoring canceled for environment aws://${environment.account}/${environment.region}\n`)); + continue; + } + + await notifyInfo('Refactoring...'); + await context.execute(stackDefinitions, sdkProvider, ioHelper); + await notifyInfo('✅ Stack refactor complete'); } catch (e: any) { await notifyError(e.message, e); } } + async function confirm(force: boolean): Promise { + // 'force' is set to true is the equivalent of having pre-approval for any refactor + if (force) { + return true; + } + + const question = 'Do you wish to refactor these resources?'; + const response = await ioHelper.requestResponse(IO.CDK_TOOLKIT_I8910.req(question, { + responseDescription: '[Y]es/[n]o', + }, 'y')); + return ['y', 'yes'].includes(response.toLowerCase()); + } + async function notifyInfo(message: string, data: any = {}) { await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(message, data)); } diff --git a/packages/@aws-cdk/toolkit-lib/package.json b/packages/@aws-cdk/toolkit-lib/package.json index 225e4c49f..0f78ea200 100644 --- a/packages/@aws-cdk/toolkit-lib/package.json +++ b/packages/@aws-cdk/toolkit-lib/package.json @@ -112,6 +112,7 @@ "cdk-from-cfn": "^0.222.0", "chalk": "^4", "chokidar": "^3", + "fast-deep-equal": "^3.1.3", "fs-extra": "^9", "glob": "^11.0.3", "minimatch": "10.0.3", diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index 2e6e15764..498246a8b 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -1,8 +1,16 @@ -import { GetTemplateCommand, ListStacksCommand } from '@aws-sdk/client-cloudformation'; +import { + CreateStackRefactorCommand, + DescribeStackRefactorCommand, + DescribeStacksCommand, + ExecuteStackRefactorCommand, + GetTemplateCommand, + ListStacksCommand, +} from '@aws-sdk/client-cloudformation'; +import { GetCallerIdentityCommand } from '@aws-sdk/client-sts'; import { MappingSource, type RefactorOptions, Toolkit } from '../../lib'; import { SdkProvider } from '../../lib/api/aws-auth/private'; import { builderFixture, TestIoHost } from '../_helpers'; -import { mockCloudFormationClient, MockSdk } from '../_helpers/mock-sdk'; +import { mockCloudFormationClient, MockSdk, mockSTSClient } from '../_helpers/mock-sdk'; const ioHost = new TestIoHost(); const toolkit = new Toolkit({ ioHost, unstableFeatures: ['refactor'] }); @@ -170,7 +178,8 @@ test('only considers deployed stacks that match the given filter', async () => { }), }); - await expectRefactorBehavior('stack-with-bucket', + await expectRefactorBehavior( + 'stack-with-bucket', { dryRun: true, // We are not passing any filter, which means that Stack2 will also be included in the comparison. @@ -185,7 +194,8 @@ test('only considers deployed stacks that match the given filter', async () => { }, ); - await expectRefactorBehavior('stack-with-bucket', + await expectRefactorBehavior( + 'stack-with-bucket', { dryRun: true, // To avoid the error, we tell the toolkit to only consider Stack1 in the deployed stacks. @@ -208,12 +218,14 @@ test('only considers deployed stacks that match the given filter', async () => { }, ); - await expectRefactorBehavior('two-different-stacks', + await expectRefactorBehavior( + 'two-different-stacks', { dryRun: true, // In this case, we are not passing any filter, either, but local and deployed are // the same, except for the bootstrap stack. But that is ignored by default. - }, { + }, + { action: 'refactor', level: 'result', code: 'CDK_TOOLKIT_I8900', @@ -231,19 +243,23 @@ test('only considers deployed stacks that match the given filter', async () => { }, ], }), - }); + }, + ); - await expectRefactorBehavior('two-different-stacks', + await expectRefactorBehavior( + 'two-different-stacks', { dryRun: true, // But if we pass a wildcard, even the bootstrap stack will be included in the comparison. deployedStacks: ['*'], - }, { + }, + { action: 'refactor', level: 'error', code: 'CDK_TOOLKIT_E8900', message: expect.stringMatching(/A refactor operation cannot add, remove or update resources/), - }); + }, + ); }); test('detects ambiguous mappings', async () => { @@ -384,15 +400,6 @@ test('detects modifications to the infrastructure', async () => { ); }); -test('fails when dry-run is false', async () => { - const cx = await builderFixture(toolkit, 'stack-with-bucket'); - await expect( - toolkit.refactor(cx, { - dryRun: false, - }), - ).rejects.toThrow('Refactor is not available yet. Too see the proposed changes, use the --dry-run flag.'); -}); - test('filters stacks when stack selector is passed', async () => { // GIVEN mockCloudFormationClient.on(ListStacksCommand).resolves({ @@ -804,10 +811,451 @@ test('computes one set of mappings per environment', async () => { ); }); +describe('refactor execution', () => { + beforeEach(() => { + process.stdout.isTTY = false; + }); + + test('happy path', async () => { + // GIVEN + givenDeployedStackWithResources('Stack1', { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }); + + mockSTSClient.on(GetCallerIdentityCommand).resolves({ + Account: '333333333333', + Arn: 'arn:aws:sts::333333333333:assumed-role/role-name/role-session-name', + }); + + mockCloudFormationClient.on(DescribeStacksCommand).resolves({ + Stacks: [ + { + StackName: 'CDKToolkit', + CreationTime: new Date(), + StackStatus: 'UPDATE_COMPLETE', + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '28', + }, + ], + }, + ], + }); + + mockCloudFormationClient.on(CreateStackRefactorCommand).resolves({ + StackRefactorId: 'refactor-id', + }); + + mockCloudFormationClient.on(DescribeStackRefactorCommand).resolves({ + Status: 'CREATE_COMPLETE', + ExecutionStatus: 'EXECUTE_COMPLETE', + }); + + mockCloudFormationClient.on(ExecuteStackRefactorCommand).resolves({}); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: false, + }); + + // THEN + expect(mockCloudFormationClient).toHaveReceivedCommandWith(CreateStackRefactorCommand, { + EnableStackCreation: true, + ResourceMappings: [ + { + Destination: { LogicalResourceId: 'MyBucketF68F3FF0', StackName: 'Stack1' }, + Source: { LogicalResourceId: 'OldLogicalID', StackName: 'Stack1' }, + }, + ], + StackDefinitions: [ + { + StackName: 'Stack1', + TemplateBody: JSON.stringify({ + Resources: { + MyBucketF68F3FF0: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { 'aws:cdk:path': 'Stack1/OldLogicalID/Resource' }, + }, + CDKMetadata: { + Type: 'AWS::CDK::Metadata', + Properties: { + Analytics: + 'v2:deflate64:H4sIAAAAAAAA/zPSMzIw1DNQTCwv1k1OydbNyUzSqw4uSUzO1kksL44vNtardipNzk4t0XFOy4Owamt18vJTUvWyivXLjIz0DM30DBSzijMzdYtK80oyc1P1giA0AJXmpMZbAAAA', + }, + Metadata: { 'aws:cdk:path': 'Stack1/CDKMetadata/Default' }, + Condition: 'CDKMetadataAvailable', + }, + }, + Conditions: { + CDKMetadataAvailable: { + 'Fn::Or': [ + { + 'Fn::Or': [ + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'af-south-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-east-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-northeast-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-northeast-2'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-northeast-3'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-south-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-south-2'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-2'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-3'] }, + ], + }, + { + 'Fn::Or': [ + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-4'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ca-central-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ca-west-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'cn-north-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'cn-northwest-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-central-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-central-2'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-north-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-south-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-south-2'] }, + ], + }, + { + 'Fn::Or': [ + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-west-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-west-2'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-west-3'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'il-central-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'me-central-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'me-south-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'sa-east-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-east-1'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-east-2'] }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-west-1'] }, + ], + }, + { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-west-2'] }, + ], + }, + }, + Parameters: { + BootstrapVersion: { + Type: 'AWS::SSM::Parameter::Value', + Default: '/cdk-bootstrap/hnb659fds/version', + Description: + 'Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]', + }, + }, + Rules: { + CheckBootstrapVersion: { + Assertions: [ + { + Assert: { + 'Fn::Not': [{ 'Fn::Contains': [['1', '2', '3', '4', '5'], { Ref: 'BootstrapVersion' }] }], + }, + AssertDescription: + "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI.", + }, + ], + }, + }, + }), + }, + ], + }); + + expect(ioHost.notifySpy).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringMatching(/Stack refactor complete/), + }), + ); + }); + + test('interactive mode, without force', async () => { + // GIVEN + givenDeployedStackWithResources('Stack1', { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }); + + try { + process.stdout.isTTY = true; + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: false, + }); + + // THEN + expect(ioHost.requestSpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'refactor', + code: 'CDK_TOOLKIT_I8910', + defaultResponse: 'y', + level: 'info', + message: 'Do you wish to refactor these resources?', + }), + ); + } finally { + process.stdout.isTTY = false; + } + }); + + test('interactive mode, with force', async () => { + // GIVEN + givenDeployedStackWithResources('Stack1', { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }); + process.stdout.isTTY = true; + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: false, + force: true, + }); + + // THEN + // No confirmation is requested from the user + expect(ioHost.requestSpy).not.toHaveBeenCalled(); + }); + + test('refactor execution fails', async () => { + // GIVEN + givenDeployedStackWithResources('Stack1', { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }); + + mockSTSClient.on(GetCallerIdentityCommand).resolves({ + Account: '333333333333', + Arn: 'arn:aws:sts::333333333333:assumed-role/role-name/role-session-name', + }); + + mockCloudFormationClient.on(DescribeStacksCommand).resolves({ + Stacks: [ + { + StackName: 'CDKToolkit', + CreationTime: new Date(), + StackStatus: 'UPDATE_COMPLETE', + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '28', + }, + ], + }, + ], + }); + + mockCloudFormationClient.on(CreateStackRefactorCommand).resolves({ + StackRefactorId: 'refactor-id', + }); + + mockCloudFormationClient.on(DescribeStackRefactorCommand).resolves({ + Status: 'CREATE_COMPLETE', + ExecutionStatus: 'EXECUTE_FAILED', + StatusReason: 'Some error occurred during execution', + }); + + mockCloudFormationClient.on(ExecuteStackRefactorCommand).resolves({}); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: false, + }); + + // THEN + expect(ioHost.notifySpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'refactor', + code: 'CDK_TOOLKIT_E8900', + level: 'error', + message: expect.stringMatching('Some error occurred during execution'), + }), + ); + }); + + test('refactor creation fails', async () => { + // GIVEN + givenDeployedStackWithResources('Stack1', { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }); + + mockSTSClient.on(GetCallerIdentityCommand).resolves({ + Account: '333333333333', + Arn: 'arn:aws:sts::333333333333:assumed-role/role-name/role-session-name', + }); + + mockCloudFormationClient.on(DescribeStacksCommand).resolves({ + Stacks: [ + { + StackName: 'CDKToolkit', + CreationTime: new Date(), + StackStatus: 'UPDATE_COMPLETE', + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '28', + }, + ], + }, + ], + }); + + mockCloudFormationClient.on(CreateStackRefactorCommand).resolves({ + StackRefactorId: 'refactor-id', + }); + + mockCloudFormationClient.on(DescribeStackRefactorCommand).resolves({ + Status: 'CREATE_FAILED', + StatusReason: 'Some error occurred during creation', + }); + + mockCloudFormationClient.on(ExecuteStackRefactorCommand).resolves({}); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: false, + }); + + // THEN + expect(ioHost.notifySpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'refactor', + code: 'CDK_TOOLKIT_E8900', + level: 'error', + message: expect.stringMatching('Some error occurred during creation'), + }), + ); + }); + + test('bootstrap version lower than minimum required', async () => { + // GIVEN + givenDeployedStackWithResources('Stack1', { + OldLogicalID: { + Type: 'AWS::S3::Bucket', + UpdateReplacePolicy: 'Retain', + DeletionPolicy: 'Retain', + Metadata: { + 'aws:cdk:path': 'Stack1/OldLogicalID/Resource', + }, + }, + }); + + mockSTSClient.on(GetCallerIdentityCommand).resolves({ + Account: '333333333333', + Arn: 'arn:aws:sts::333333333333:assumed-role/role-name/role-session-name', + }); + + mockCloudFormationClient.on(DescribeStacksCommand).resolves({ + Stacks: [ + { + StackName: 'CDKToolkit', + CreationTime: new Date(), + StackStatus: 'UPDATE_COMPLETE', + Outputs: [ + { + OutputKey: 'BootstrapVersion', + OutputValue: '27', + }, + ], + }, + ], + }); + + mockCloudFormationClient.on(CreateStackRefactorCommand).resolves({ + StackRefactorId: 'refactor-id', + }); + + mockCloudFormationClient.on(DescribeStackRefactorCommand).resolves({ + Status: 'CREATE_FAILED', + }); + + mockCloudFormationClient.on(ExecuteStackRefactorCommand).resolves({}); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.refactor(cx, { + dryRun: false, + }); + + // THEN + expect(ioHost.notifySpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'refactor', + code: 'CDK_TOOLKIT_E8900', + level: 'error', + message: + "The CDK toolkit stack in environment aws://123456789012/us-east-1 doesn't support refactoring. Please run 'cdk bootstrap aws://123456789012/us-east-1' to update it.", + }), + ); + }); +}); + +function givenDeployedStackWithResources(stackName: string, resources: Record) { + mockCloudFormationClient.on(ListStacksCommand).resolves({ + StackSummaries: [ + { + StackName: stackName, + StackId: `arn:aws:cloudformation:us-east-1:333333333333:stack/${stackName}`, + StackStatus: 'CREATE_COMPLETE', + CreationTime: new Date(), + }, + ], + }); + + mockCloudFormationClient + .on(GetTemplateCommand, { + StackName: stackName, + }) + .resolves({ + TemplateBody: JSON.stringify({ + Resources: resources, + }), + }); +} + async function expectRefactorBehavior(fixtureName: string, input: RefactorOptions, output: E) { const host = new TestIoHost(); const tk = new Toolkit({ ioHost: host, unstableFeatures: ['refactor'] }); await tk.refactor(await builderFixture(tk, fixtureName), input); expect(host.notifySpy).toHaveBeenCalledWith(expect.objectContaining(output)); } - 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 367f2f593..4b42fd5e7 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 @@ -846,80 +846,6 @@ describe(generateStackDefinitions, () => { region: 'us-east-1', }; - test('renames a resource within the same stack', () => { - const stack1: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - Bucket1: { - Type: 'AWS::S3::Bucket', - }, - NotInvolved: { - Type: 'AWS::X::Y', - }, - Consumer: { - Type: 'AWS::X::Y', - Properties: { - Bucket: { Ref: 'Bucket1' }, - }, - }, - }, - }, - }; - - const stack2: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - Bucket2: { - Type: 'AWS::S3::Bucket', - }, - NotInvolved: { - Type: 'AWS::X::Y', - }, - Consumer: { - Type: 'AWS::X::Y', - Properties: { - Bucket: { Ref: 'Bucket2' }, - }, - }, - }, - }, - }; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(stack1, 'Bucket1'), new ResourceLocation(stack1, 'Bucket2')), - ]; - - const result = generateStackDefinitions(mappings, [stack1], [stack2]); - expect(result).toEqual([ - { - StackName: 'Foo', - TemplateBody: JSON.stringify({ - Resources: { - Consumer: { - Type: 'AWS::X::Y', - Properties: { - // The reference has also been updated - Bucket: { Ref: 'Bucket2' }, - }, - }, - Bucket2: { - Type: 'AWS::S3::Bucket', - }, - // Not involved in the refactor, but still part of the - // original template. Should be included. - NotInvolved: { - Type: 'AWS::X::Y', - }, - }, - }), - }, - ]); - }); - test('moves a resource to another stack that has already been deployed', () => { const deployedStack1: CloudFormationStack = { environment, @@ -981,11 +907,7 @@ describe(generateStackDefinitions, () => { ), ]; - const result = generateStackDefinitions( - mappings, - [deployedStack1, deployedStack2], - [localStack1, localStack2], - ); + const result = generateStackDefinitions(mappings, [deployedStack1, deployedStack2], [localStack1, localStack2]); expect(result).toEqual([ { StackName: 'Stack1', @@ -1004,16 +926,106 @@ describe(generateStackDefinitions, () => { StackName: 'Stack2', TemplateBody: JSON.stringify({ Resources: { + // Wasn't touched by the refactor + B: { + Type: 'AWS::B::B', + }, + // Old Bucket1 is now Bucket2 here Bucket2: { Type: 'AWS::S3::Bucket', }, + }, + }), + }, + ]); + }); - // Wasn't touched by the refactor - B: { - Type: 'AWS::B::B', + test('with cross-stack references', () => { + const deployedStacks: CloudFormationStack[] = [ + { + environment, + stackName: 'StackX', + template: { + Resources: { + A: { + Type: 'AWS::A::A', + Properties: { + Props: { 'Fn::ImportValue': 'BFromOtherStack' }, + }, + }, + }, + }, + }, + { + environment, + stackName: 'StackY', + template: { + Outputs: { + Bout: { + Value: { Ref: 'B' }, + Export: { + Name: 'BFromOtherStack', + }, + }, + }, + Resources: { + B: { Type: 'AWS::B::B' }, + }, + }, + }, + ]; + + const localStacks: CloudFormationStack[] = [ + { + environment, + stackName: 'StackX', + template: { + Resources: { + A: { + Type: 'AWS::A::A', + Properties: { + Props: { Ref: 'B' }, + }, }, + B: { Type: 'AWS::B::B' }, }, + }, + }, + { + environment, + stackName: 'StackY', + template: { + Resources: {}, + }, + }, + ]; + + const mappings: ResourceMapping[] = [ + new ResourceMapping(new ResourceLocation(deployedStacks[1], 'B'), new ResourceLocation(localStacks[0], 'B')), + ]; + + const result = generateStackDefinitions(mappings, deployedStacks, localStacks); + expect(result).toEqual([ + { + StackName: 'StackX', + TemplateBody: JSON.stringify({ + Resources: { + A: { + Type: 'AWS::A::A', + Properties: { + // The reference has been updated to the moved resource + Props: { Ref: 'B' }, + }, + }, + B: { Type: 'AWS::B::B' }, + }, + }), + }, + { + StackName: 'StackY', + TemplateBody: JSON.stringify({ + Resources: {}, }), }, ]); @@ -1066,26 +1078,26 @@ describe(generateStackDefinitions, () => { const result = generateStackDefinitions(mappings, [deployedStack], [localStack1, localStack2]); expect(result).toEqual([ { - StackName: 'Stack1', + StackName: 'Stack2', TemplateBody: JSON.stringify({ Resources: { - // Wasn't touched by the refactor - A: { - Type: 'AWS::A::A', + // Old Bucket1 is now Bucket2 here + Bucket2: { + Type: 'AWS::S3::Bucket', }, - - // Bucket1 doesn't exist anymore }, }), }, { - StackName: 'Stack2', + StackName: 'Stack1', TemplateBody: JSON.stringify({ Resources: { - // Old Bucket1 is now Bucket2 here - Bucket2: { - Type: 'AWS::S3::Bucket', + // Wasn't touched by the refactor + A: { + Type: 'AWS::A::A', }, + + // Bucket1 doesn't exist anymore }, }), }, @@ -1162,11 +1174,7 @@ describe(generateStackDefinitions, () => { ), ]; - const result = generateStackDefinitions( - mappings, - [deployedStack1, deployedStack2], - [localStack1, localStack2], - ); + const result = generateStackDefinitions(mappings, [deployedStack1, deployedStack2], [localStack1, localStack2]); expect(result).toEqual([ { StackName: 'Stack1', @@ -1250,13 +1258,10 @@ describe(generateStackDefinitions, () => { ), ]; - const result = generateStackDefinitions( - mappings, - [deployedStack1, deployedStack2], - [localStack1, localStack2], - ); + const result = generateStackDefinitions(mappings, [deployedStack1, deployedStack2], [localStack1, localStack2]); expect(result).toEqual([ { + // Stack2 and Stack3 are not involved in the refactoring. Only Stack1 is. StackName: 'Stack1', TemplateBody: JSON.stringify({ Resources: { @@ -1269,7 +1274,7 @@ describe(generateStackDefinitions, () => { ]); }); - test('local stacks have more resources than deployed stacks', async () => { + test('CDK path in Metadata is preserved', () => { const deployedStack: CloudFormationStack = { environment, stackName: 'Stack1', @@ -1277,6 +1282,9 @@ describe(generateStackDefinitions, () => { Resources: { Bucket1: { Type: 'AWS::S3::Bucket', + Metadata: { + 'aws:cdk:path': 'Stack1/Bucket1/Resource', + }, }, }, }, @@ -1289,9 +1297,10 @@ describe(generateStackDefinitions, () => { Resources: { Bucket2: { Type: 'AWS::S3::Bucket', - }, - ExtraStuff: { - Type: 'AWS::X::Y', + Metadata: { + // Here the CDK path is consistent with the new logical ID... + 'aws:cdk:path': 'Stack1/Bucket2/Resource', + }, }, }, }, @@ -1312,15 +1321,18 @@ describe(generateStackDefinitions, () => { Resources: { Bucket2: { Type: 'AWS::S3::Bucket', + Metadata: { + // ...but we keep the original CDK path from the deployed stack, to make CloudFormation happy. + 'aws:cdk:path': 'Stack1/Bucket1/Resource', + }, }, - // ExtraStuff is not involved in the refactor and was not part of the deployed stack, so we keep it out. }, }), }, ]); }); - test('local stacks have fewer resources than deployed stacks', () => { + test('stack definitions come from the local templates', () => { const deployedStack: CloudFormationStack = { environment, stackName: 'Stack1', @@ -1328,62 +1340,17 @@ describe(generateStackDefinitions, () => { Resources: { Bucket1: { Type: 'AWS::S3::Bucket', - }, - ExtraStuff: { - Type: 'AWS::X::Y', - }, - }, - }, - }; - - const localStack: CloudFormationStack = { - environment, - stackName: 'Stack1', - template: { - Resources: { - Bucket2: { - Type: 'AWS::S3::Bucket', - }, - }, - }, - }; - - const mappings: ResourceMapping[] = [ - new ResourceMapping( - new ResourceLocation(deployedStack, 'Bucket1'), - new ResourceLocation(deployedStack, 'Bucket2'), - ), - ]; - - const result = generateStackDefinitions(mappings, [deployedStack], [localStack]); - expect(result).toEqual([ - { - StackName: 'Stack1', - TemplateBody: JSON.stringify({ - Resources: { - Bucket2: { - Type: 'AWS::S3::Bucket', - }, - // ExtraStuff is not involved in the refactor, but it was part of the deployed stack, so we keep it in. - ExtraStuff: { - Type: 'AWS::X::Y', + Properties: { + Foo: 'Bar', }, }, - }), - }, - ]); - }); - - test('CDK path in Metadata is preserved', () => { - const deployedStack: CloudFormationStack = { - environment, - stackName: 'Stack1', - template: { - Resources: { - Bucket1: { - Type: 'AWS::S3::Bucket', + CDKMetadata: { + Type: 'AWS::CDK::Metadata', + Properties: { + Analytics: 'v2:deflate64:deployed', + }, Metadata: { - 'aws:cdk:path': 'Stack1/Bucket1/Resource', + 'aws:cdk:path': 'Stack1/CDKMetadata/Default', }, }, }, @@ -1397,9 +1364,17 @@ describe(generateStackDefinitions, () => { Resources: { Bucket2: { Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Bar', + }, + }, + CDKMetadata: { + Type: 'AWS::CDK::Metadata', + Properties: { + Analytics: 'v2:deflate64:local', + }, Metadata: { - // Here the CDK path is consistent with the new logical ID... - 'aws:cdk:path': 'Stack1/Bucket2/Resource', + 'aws:cdk:path': 'Stack1/CDKMetadata/Default', }, }, }, @@ -1419,11 +1394,21 @@ describe(generateStackDefinitions, () => { StackName: 'Stack1', TemplateBody: JSON.stringify({ Resources: { + // For regular resources, we pick the local one Bucket2: { Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Bar', + }, + }, + CDKMetadata: { + Type: 'AWS::CDK::Metadata', + Properties: { + // But for CDKMetadata, we pick the deployed one + Analytics: 'v2:deflate64:deployed', + }, Metadata: { - // ...but we keep the original CDK path from the deployed stack, to make CloudFormation happy. - 'aws:cdk:path': 'Stack1/Bucket1/Resource', + 'aws:cdk:path': 'Stack1/CDKMetadata/Default', }, }, }, @@ -1431,1318 +1416,4 @@ describe(generateStackDefinitions, () => { }, ]); }); - - describe('With references between resources', () => { - describe('Divergence - reference starts within the same stack and, in some cases, crosses stacks', () => { - test('No stack move', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { - Type: 'AWS::B::B', - }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'Bn' }, - }, - }, - Bn: { - Type: 'AWS::B::B', - }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'B'), new ResourceLocation(localStacks[0], 'Bn')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'Bn' }, // Updated reference - }, - }, - Bn: { - Type: 'AWS::B::B', - }, - }, - }), - }, - ]); - }); - - test('tail of the reference moved', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { - Type: 'AWS::B::B', - }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackY', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackX', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'A'), new ResourceLocation(localStacks[0], 'A')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, // Reference to the moved resource - }, - }, - }, - }), - }, - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - B: { Type: 'AWS::B::B' }, // The moved resource - }, - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - }), - }, - ]); - }); - - test('head of the reference moved', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { - Type: 'AWS::B::B', - }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'B'), new ResourceLocation(localStacks[1], 'B')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - // A was moved - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, // Reference to the resource that stayed behind - }, - }, - }, - }), - }, - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Resources: { - B: { Type: 'AWS::B::B' }, - }, - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - }), - }, - ]); - }); - - test('both moved to the same stack', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { - Type: 'AWS::B::B', - }, - C: { - Type: 'AWS::C::C', - }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - C: { - Type: 'AWS::C::C', - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { - Type: 'AWS::B::B', - }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'B'), new ResourceLocation(localStacks[1], 'B')), - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'A'), new ResourceLocation(localStacks[1], 'A')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { Type: 'AWS::B::B' }, - }, - }), - }, - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - C: { - Type: 'AWS::C::C', - }, - }, - }), - }, - ]); - }); - - test('both moved to different stacks', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { - Type: 'AWS::B::B', - }, - C: { - Type: 'AWS::C::C', - }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - C: { - Type: 'AWS::C::C', - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackZ', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { - Type: 'AWS::B::B', - }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'A'), new ResourceLocation(localStacks[1], 'A')), - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'B'), new ResourceLocation(localStacks[2], 'B')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }), - }, - { - StackName: 'StackZ', - TemplateBody: JSON.stringify({ - Resources: { - B: { - Type: 'AWS::B::B', - }, - }, - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - }), - }, - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - C: { - Type: 'AWS::C::C', - }, - }, - }), - }, - ]); - }); - }); - - describe('Convergence - reference starts cross-stack and, in some cases, moves to within the same stack', () => { - test('No stack move', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BnFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { - Name: 'BnFromOtherStack', - }, - }, - }, - Resources: { - Bn: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping( - new ResourceLocation(deployedStacks[1], 'B'), - new ResourceLocation(deployedStacks[1], 'Bn'), - ), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { Props: { 'Fn::ImportValue': 'BnFromOtherStack' } }, - }, - }, - }), - }, - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { - Name: 'BnFromOtherStack', - }, - }, - }, - Resources: { - Bn: { Type: 'AWS::B::B' }, - }, - }), - }, - ]); - }); - - test('tail of the reference moved', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackY', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'A'), new ResourceLocation(localStacks[0], 'A')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Outputs: { - Bout: { Value: { Ref: 'B' }, Export: { Name: 'BFromOtherStack' } }, - }, - Resources: { - A: { Type: 'AWS::A::A', Properties: { Props: { Ref: 'B' } } }, - B: { Type: 'AWS::B::B' }, - }, - }), - }, - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ Resources: {} }), - }, - ]); - }); - - test('head of the reference moved', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[1], 'B'), new ResourceLocation(localStacks[0], 'B')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - // The reference has been updated to the moved resource - Props: { Ref: 'B' }, - }, - }, - B: { Type: 'AWS::B::B' }, - }, - }), - }, - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { Name: 'BFromOtherStack' }, - }, - }, - Resources: {}, - }), - }, - ]); - }); - - test('both moved', () => { - const deployedStacks: CloudFormationStack[] = [ - { - environment, - stackName: 'StackX', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { 'Fn::ImportValue': 'BFromOtherStack' }, - }, - }, - }, - }, - }, - { - environment, - stackName: 'StackY', - template: { - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const localStacks: CloudFormationStack[] = [ - { - environment, - // This is a third stack that will receive both resources - stackName: 'StackZ', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { Type: 'AWS::B::B' }, - }, - }, - }, - ]; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStacks[0], 'A'), new ResourceLocation(localStacks[0], 'A')), - new ResourceMapping(new ResourceLocation(deployedStacks[1], 'B'), new ResourceLocation(localStacks[0], 'B')), - ]; - - const result = generateStackDefinitions(mappings, deployedStacks, localStacks); - expect(result).toEqual([ - { - StackName: 'StackZ', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Props: { Ref: 'B' }, - }, - }, - B: { Type: 'AWS::B::B' }, - }, - }), - }, - { - StackName: 'StackX', - TemplateBody: JSON.stringify({ Resources: {} }), - }, - { - StackName: 'StackY', - TemplateBody: JSON.stringify({ - Outputs: { - Bout: { - Value: { Ref: 'B' }, - Export: { Name: 'BFromOtherStack' }, - }, - }, - Resources: {}, - }), - }, - ]); - }); - }); - }); - - describe('Local and deployed resources are not identical', () => { - // This may happen if the resource has a physical ID defined, which gives - // the user freedom to change the resource properties, while the matching - // algorithm still recognizes them as being the same. - - test('deployed resource configuration prevails', () => { - const deployedStack: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { Ref: 'B' }, - Prop2: { 'Fn::GetAtt': ['C', 'Banana'] }, - Prop3: { 'Fn::Sub': ['${C} ${SomeVar} ${B.foo}', { SomeVar: { Ref: 'B' } }] }, - Foo: 123, - }, - DependsOn: ['D'], - }, - B: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - C: { - Type: 'AWS::C::C', - Properties: { - Banana: 'BananaValue', - }, - }, - D: { - Type: 'AWS::D::D', - Properties: { - Foo: 123, - }, - }, - }, - }, - }; - - const localStack: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { Ref: 'Bn' }, - Prop2: { 'Fn::GetAtt': ['Cn', 'Banana'] }, - Prop3: { 'Fn::Sub': ['${Cn} ${SomeVar} ${Bn.foo}', { SomeVar: { Ref: 'Bn' } }] }, - Bar: 456, // Different property - }, - DependsOn: ['Dn'], - }, - Bn: { - Type: 'AWS::B::B', - Properties: { - Bar: 456, - }, - }, - Cn: { - Type: 'AWS::C::C', - Properties: { - Banana: 'BananaValue', - }, - }, - Dn: { - Type: 'AWS::D::D', - Properties: { - Bar: 456, - }, - }, - }, - }, - }; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStack, 'B'), new ResourceLocation(deployedStack, 'Bn')), - new ResourceMapping(new ResourceLocation(deployedStack, 'C'), new ResourceLocation(deployedStack, 'Cn')), - new ResourceMapping(new ResourceLocation(deployedStack, 'D'), new ResourceLocation(deployedStack, 'Dn')), - ]; - - const result = generateStackDefinitions(mappings, [deployedStack], [localStack]); - expect(result).toEqual([ - { - StackName: 'Foo', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { Ref: 'Bn' }, - Prop2: { 'Fn::GetAtt': ['Cn', 'Banana'] }, - Prop3: { 'Fn::Sub': ['${Cn} ${SomeVar} ${Bn.foo}', { SomeVar: { Ref: 'Bn' } }] }, - Foo: 123, - }, - DependsOn: ['Dn'], - }, - Dn: { - Type: 'AWS::D::D', - Properties: { - Foo: 123, - }, - }, - Bn: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - Cn: { - Type: 'AWS::C::C', - Properties: { - Banana: 'BananaValue', - }, - }, - }, - }), - }, - ]); - }); - - test('within -> cross', () => { - const deployedStack: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { Ref: 'B' }, // Reference to a resource in the same stack - Foo: 123, - }, - DependsOn: 'B', - }, - B: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - }, - }, - }; - - const localStack1: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { 'Fn::ImportValue': 'BFromOtherStack' }, // Reference to a resource in the same stack - Bar: 456, // Different property - }, - }, - }, - }, - }; - - const localStack2: CloudFormationStack = { - environment: environment, - stackName: 'Bar', - template: { - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - Bn: { - Type: 'AWS::B::B', - Properties: { - Bar: 456, - }, - }, - }, - }, - }; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStack, 'B'), new ResourceLocation(localStack2, 'Bn')), - ]; - - const result = generateStackDefinitions(mappings, [deployedStack], [localStack1, localStack2]); - expect(result).toEqual([ - { - StackName: 'Foo', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { 'Fn::ImportValue': 'BFromOtherStack' }, // Reference to the moved resource - Foo: 123, // But we keep the original property from the deployed stack - }, - // Note the absence of DependsOn - }, - }, - }), - }, - { - StackName: 'Bar', - TemplateBody: JSON.stringify({ - Resources: { - Bn: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - }, - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - }), - }, - ]); - }); - - test('cross -> within', () => { - const deployedStack1: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { 'Fn::ImportValue': 'BFromOtherStack' }, // Reference to a resource in the same stack - Foo: 123, - }, - }, - }, - }, - }; - - const deployedStack2: CloudFormationStack = { - environment: environment, - stackName: 'Bar', - template: { - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - Bn: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - }, - }, - }; - - const localStack: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { Ref: 'B' }, - Bar: 456, - }, - }, - B: { - Type: 'AWS::B::B', - Properties: { - Bar: 456, - }, - }, - }, - }, - }; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStack2, 'Bn'), new ResourceLocation(localStack, 'B')), - ]; - - const result = generateStackDefinitions(mappings, [deployedStack1, deployedStack2], [localStack]); - expect(result).toEqual([ - { - StackName: 'Foo', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { Ref: 'B' }, - Foo: 123, // We keep the original property from the deployed stack - }, - }, - B: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, // We keep the original property from the deployed stack - }, - }, - }, - }), - }, - { - StackName: 'Bar', - TemplateBody: JSON.stringify({ - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { Name: 'BFromOtherStack' }, - }, - }, - Resources: {}, - }), - }, - ]); - }); - - test('cross -> cross', () => { - const deployedStack1: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { 'Fn::ImportValue': 'BFromOtherStack' }, // Reference to a resource in the same stack - Foo: 123, - }, - }, - }, - }, - }; - - const deployedStack2: CloudFormationStack = { - environment: environment, - stackName: 'Bar', - template: { - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - Bn: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - }, - }, - }; - - const localStack1: CloudFormationStack = { - environment: environment, - stackName: 'Foo', - template: { - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { - Prop: { 'Fn::ImportValue': 'BFromOtherStack' }, - Bar: 456, - }, - }, - }, - }, - }; - - const localStack2: CloudFormationStack = { - environment: environment, - stackName: 'Zee', - template: { - Outputs: { - Bout: { - Value: { Ref: 'Bn2' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - Resources: { - Bn2: { - Type: 'AWS::B::B', - Properties: { - Bar2: 456, - }, - }, - }, - }, - }; - - const mappings: ResourceMapping[] = [ - new ResourceMapping(new ResourceLocation(deployedStack2, 'Bn'), new ResourceLocation(localStack2, 'Bn2')), - ]; - - const result = generateStackDefinitions( - mappings, - [deployedStack1, deployedStack2], - [localStack1, localStack2], - ); - expect(result).toEqual([ - { - StackName: 'Foo', - TemplateBody: JSON.stringify({ - Resources: { - A: { - Type: 'AWS::A::A', - Properties: { Prop: { 'Fn::ImportValue': 'BFromOtherStack' }, Foo: 123 }, - }, - }, - }), - }, - { - StackName: 'Zee', - TemplateBody: JSON.stringify({ - Resources: { - Bn2: { - Type: 'AWS::B::B', - Properties: { - Foo: 123, - }, - }, - }, - Outputs: { - Bout: { - Value: { Ref: 'Bn2' }, - Export: { - Name: 'BFromOtherStack', - }, - }, - }, - }), - }, - { - StackName: 'Bar', - TemplateBody: JSON.stringify({ - Outputs: { - Bout: { - Value: { Ref: 'Bn' }, - Export: { Name: 'BFromOtherStack' }, - }, - }, - Resources: {}, - }), - }, - ]); - }); - }); }); - diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 3e48e8c7d..f335eb12e 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -1229,6 +1229,7 @@ export class CdkToolkit { dryRun: options.dryRun, localStacks: options.localStacks, deployedStacks: options.deployedStacks, + force: options.force, mappingSource: await mappingSource(), }); } catch (e) { @@ -2021,6 +2022,11 @@ export interface RefactorOptions { * '*'. */ deployedStacks?: string[]; + + /** + * Whether to do the refactor without prompting the user for confirmation. + */ + force?: boolean; } /** diff --git a/packages/aws-cdk/lib/cli/cli-config.ts b/packages/aws-cdk/lib/cli/cli-config.ts index 0975f86a7..29a4bf8ba 100644 --- a/packages/aws-cdk/lib/cli/cli-config.ts +++ b/packages/aws-cdk/lib/cli/cli-config.ts @@ -465,6 +465,11 @@ export async function makeConfig(): Promise { default: false, desc: 'If specified, the command will revert the refactor operation. This is only valid if a mapping file was provided.', }, + 'force': { + type: 'boolean', + default: false, + desc: 'Whether to do the refactor without asking for confirmation', + }, }, }, }, diff --git a/packages/aws-cdk/lib/cli/cli.ts b/packages/aws-cdk/lib/cli/cli.ts index a72d9bd33..51cad3f72 100644 --- a/packages/aws-cdk/lib/cli/cli.ts +++ b/packages/aws-cdk/lib/cli/cli.ts @@ -281,6 +281,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise): any { default: false, type: 'boolean', desc: 'If specified, the command will revert the refactor operation. This is only valid if a mapping file was provided.', + }) + .option('force', { + default: false, + type: 'boolean', + desc: 'Whether to do the refactor without asking for confirmation', }), ) .version(helpers.cliVersion()) diff --git a/packages/aws-cdk/lib/cli/user-input.ts b/packages/aws-cdk/lib/cli/user-input.ts index 344e438d2..0701f1f7d 100644 --- a/packages/aws-cdk/lib/cli/user-input.ts +++ b/packages/aws-cdk/lib/cli/user-input.ts @@ -1460,4 +1460,11 @@ export interface RefactorOptions { * @default - false */ readonly revert?: boolean; + + /** + * Whether to do the refactor without asking for confirmation + * + * @default - false + */ + readonly force?: boolean; } From 9355e19a22dcfa50418ead63d38c1a88955fbb1d Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Fri, 4 Jul 2025 09:32:00 +0100 Subject: [PATCH 03/10] Better error message --- .../toolkit-lib/lib/toolkit/toolkit.ts | 18 +++++++++++++++++- .../toolkit-lib/test/actions/refactor.test.ts | 3 ++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 726757789..47867b4d6 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -1117,7 +1117,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { await context.execute(stackDefinitions, sdkProvider, ioHelper); await notifyInfo('✅ Stack refactor complete'); } catch (e: any) { - await notifyError(e.message, e); + await notifyError(`❌ Refactor failed: ${formatError(e)}`, e); } } @@ -1151,6 +1151,22 @@ export class Toolkit extends CloudAssemblySourceBuilder { return g.account === environment.account && g.region === environment.region; } } + + function formatError(error: any): string { + try { + const payload = JSON.parse(error.message); + const messages: string[] = []; + if (payload.reason?.StatusReason) { + messages.push(`Refactor creation: [${payload.reason?.Status}] ${payload.reason.StatusReason}`); + } + if (payload.reason?.ExecutionStatusReason) { + messages.push(`Refactor execution: [${payload.reason?.Status}] ${payload.reason.ExecutionStatusReason}`); + } + return messages.length > 0 ? messages.join('\n') : 'Unknown error'; + } catch (e) { + return formatErrorMessage(error); + } + } } /** diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index 498246a8b..c5673d712 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -1223,8 +1223,9 @@ describe('refactor execution', () => { action: 'refactor', code: 'CDK_TOOLKIT_E8900', level: 'error', - message: + message: expect.stringMatching( "The CDK toolkit stack in environment aws://123456789012/us-east-1 doesn't support refactoring. Please run 'cdk bootstrap aws://123456789012/us-east-1' to update it.", + ), }), ); }); From 06c001f8d2344994750cf126b93d436c2978482e Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Fri, 4 Jul 2025 09:54:48 +0100 Subject: [PATCH 04/10] Remove Rules and Parameters for new stacks --- .../lib/api/refactoring/cloudformation.ts | 2 + .../lib/api/refactoring/stack-definitions.ts | 12 +++ .../test/api/refactoring/refactoring.test.ts | 99 +++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts index ce1996e30..8630f5413 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts @@ -14,6 +14,8 @@ export interface CloudFormationTemplate { [logicalId: string]: CloudFormationResource; }; Outputs?: Record; + Rules?: Record; + Parameters?: Record; } export interface CloudFormationStack { diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts index ad41bf0a9..c38006d05 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts @@ -54,6 +54,18 @@ export function generateStackDefinitions( return !deployedStack || !deepEqual(localStack.template, deployedStack.template); }); + // For stacks created by the refactor, CloudFormation does not allow Rules or Parameters + for (const stack of stacksToProcess) { + if (!deployedStacks.some(deployed => deployed.stackName === stack.stackName)) { + if ('Rules' in stack.template) { + delete stack.template.Rules; + } + if ('Parameters' in stack.template) { + delete stack.template.Parameters; + } + } + } + return stacksToProcess.map((stack) => ({ StackName: stack.stackName, TemplateBody: JSON.stringify(stack.template), 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 4b42fd5e7..fdec05aa0 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 @@ -1416,4 +1416,103 @@ describe(generateStackDefinitions, () => { }, ]); }); + + test('Rules and Parameters are removed for new stacks', () => { + const deployedStack: CloudFormationStack = { + environment, + stackName: 'Stack1', + template: { + Resources: { + Bucket1: { + Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Bar', + }, + }, + Bucket2: { + Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Zee', + }, + }, + }, + }, + }; + + const localStack1: CloudFormationStack = { + environment, + stackName: 'Stack1', + template: { + Resources: { + Bucket1: { + Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Bar', + }, + }, + }, + }, + }; + + const localStack2: CloudFormationStack = { + environment, + stackName: 'Stack2', + template: { + Resources: { + // Moved out of the original stack to a new one. + Bucket2: { + Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Zee', + }, + }, + }, + Rules: { + CheckBootstrapVersion: { + Assertions: [], + }, + }, + Parameters: { + BootstrapVersion: { + Type: 'AWS::SSM::Parameter::Value', + Default: '/cdk-bootstrap/hnb659fds/version', + Description: 'Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]', + }, + }, + }, + }; + + const mappings: ResourceMapping[] = [ + new ResourceMapping(new ResourceLocation(deployedStack, 'Bucket2'), new ResourceLocation(localStack2, 'Bucket2')), + ]; + + const result = generateStackDefinitions(mappings, [deployedStack], [localStack1, localStack2]); + expect(result).toEqual([ + { + StackName: 'Stack1', + TemplateBody: JSON.stringify({ + Resources: { + Bucket1: { + Type: 'AWS::S3::Bucket', + Properties: { + Foo: 'Bar', + }, + }, + }, + }), + }, + { + StackName: 'Stack2', + // No Rules or Parameters, even though they are present in the local stack + TemplateBody: JSON.stringify({ + Resources: { + Bucket2: { + Type: 'AWS::S3::Bucket', + Properties: { Foo: 'Zee' }, + }, + }, + }), + }, + ]); + }); }); From 97aae1dc0f2c4442c1dd7909f1d5d731232b198a Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Fri, 4 Jul 2025 14:06:55 +0100 Subject: [PATCH 05/10] Fix test --- .../test/_fixtures/stack-with-bucket/index.ts | 7 ++- .../toolkit-lib/test/actions/refactor.test.ts | 50 ------------------- 2 files changed, 6 insertions(+), 51 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-bucket/index.ts b/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-bucket/index.ts index bae858d71..214c732dd 100644 --- a/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-bucket/index.ts +++ b/packages/@aws-cdk/toolkit-lib/test/_fixtures/stack-with-bucket/index.ts @@ -3,7 +3,12 @@ import * as core from 'aws-cdk-lib/core'; export default async () => { const app = new core.App({ autoSynth: false }); - const stack = new core.Stack(app, 'Stack1'); + const stack = new core.Stack(app, 'Stack1', { + env: { + account: '123456789012', + region: 'us-east-1', + }, + }); new s3.Bucket(stack, 'MyBucket'); return app.synth(); }; diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index c5673d712..44962f359 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -894,56 +894,6 @@ describe('refactor execution', () => { 'v2:deflate64:H4sIAAAAAAAA/zPSMzIw1DNQTCwv1k1OydbNyUzSqw4uSUzO1kksL44vNtardipNzk4t0XFOy4Owamt18vJTUvWyivXLjIz0DM30DBSzijMzdYtK80oyc1P1giA0AJXmpMZbAAAA', }, Metadata: { 'aws:cdk:path': 'Stack1/CDKMetadata/Default' }, - Condition: 'CDKMetadataAvailable', - }, - }, - Conditions: { - CDKMetadataAvailable: { - 'Fn::Or': [ - { - 'Fn::Or': [ - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'af-south-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-east-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-northeast-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-northeast-2'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-northeast-3'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-south-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-south-2'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-2'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-3'] }, - ], - }, - { - 'Fn::Or': [ - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ap-southeast-4'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ca-central-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'ca-west-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'cn-north-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'cn-northwest-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-central-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-central-2'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-north-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-south-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-south-2'] }, - ], - }, - { - 'Fn::Or': [ - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-west-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-west-2'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'eu-west-3'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'il-central-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'me-central-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'me-south-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'sa-east-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-east-1'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-east-2'] }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-west-1'] }, - ], - }, - { 'Fn::Equals': [{ Ref: 'AWS::Region' }, 'us-west-2'] }, - ], }, }, Parameters: { From 71e2cf4f519f13b67ce965d237fb0b1c62548f49 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Fri, 4 Jul 2025 14:23:43 +0100 Subject: [PATCH 06/10] Don't send CDKMetadata if deployed doesn't have it --- .../toolkit-lib/lib/api/refactoring/stack-definitions.ts | 2 ++ .../@aws-cdk/toolkit-lib/test/actions/refactor.test.ts | 8 -------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts index c38006d05..c67049478 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts @@ -25,6 +25,8 @@ export function generateStackDefinitions( // Otherwise, CloudFormation will consider this a modification. if (deployedTemplate.Resources?.CDKMetadata != null) { localTemplate.Resources.CDKMetadata = deployedTemplate.Resources.CDKMetadata; + } else { + delete localTemplate.Resources.CDKMetadata; } // For every resource in the local template, take the Metadata['aws:cdk:path'] from the corresponding resource in the deployed template. diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index 44962f359..2b11800ce 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -887,14 +887,6 @@ describe('refactor execution', () => { DeletionPolicy: 'Retain', Metadata: { 'aws:cdk:path': 'Stack1/OldLogicalID/Resource' }, }, - CDKMetadata: { - Type: 'AWS::CDK::Metadata', - Properties: { - Analytics: - 'v2:deflate64:H4sIAAAAAAAA/zPSMzIw1DNQTCwv1k1OydbNyUzSqw4uSUzO1kksL44vNtardipNzk4t0XFOy4Owamt18vJTUvWyivXLjIz0DM30DBSzijMzdYtK80oyc1P1giA0AJXmpMZbAAAA', - }, - Metadata: { 'aws:cdk:path': 'Stack1/CDKMetadata/Default' }, - }, }, Parameters: { BootstrapVersion: { From e9f45d995e0a6c6f829d6553b81d0e7aad5930c4 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Wed, 16 Jul 2025 13:54:18 +0100 Subject: [PATCH 07/10] fixes after merge --- .../lib/api/refactoring/context.ts | 14 +---- .../toolkit-lib/lib/api/refactoring/index.ts | 2 - .../lib/api/refactoring/stack-definitions.ts | 55 +++++++++---------- .../toolkit-lib/lib/toolkit/toolkit.ts | 35 +++++++++--- .../toolkit-lib/test/actions/refactor.test.ts | 11 ---- .../aws-cdk/lib/cli/cli-type-registry.json | 5 ++ 6 files changed, 60 insertions(+), 62 deletions(-) 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 069d91939..68638b89d 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts @@ -5,12 +5,12 @@ 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'; import type { SDK } from '../aws-auth/sdk'; import type { SdkProvider } from '../aws-auth/sdk-provider'; import { EnvironmentResourcesRegistry } from '../environment'; import type { IoHelper } from '../io/private'; import { Mode } from '../plugin'; -import { equalSets } from '../../util/sets'; /** * Represents a set of possible moves of a resource from one location @@ -159,18 +159,6 @@ function resourceMoves( return Object.values(removeUnmovedResources(zip(digestsBefore, digestsAfter))); } -/** - * Whether two sets of resources have the same elements (uniquely identified by the digest), and - * each element is in the same number of locations. The locations themselves may be different. - */ -function isomorphic(a: Record, b: Record): boolean { - const sameKeys = equalSets(new Set(Object.keys(a)), new Set(Object.keys(b))); - return sameKeys && Object.entries(a).every(([digest, locations]) => locations.length === b[digest].length); -} - - return Object.values(removeUnmovedResources(zip(digestsBefore, digestsAfter))); -} - /** * Whether two sets of resources have the same elements (uniquely identified by the digest), and * each element is in the same number of locations. The locations themselves may be different. diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts index 0dcd8e022..e19033ecf 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts @@ -6,8 +6,6 @@ import { } from '@aws-cdk/cloudformation-diff'; import type * as cxapi from '@aws-cdk/cx-api'; import type { StackSummary } from '@aws-sdk/client-cloudformation'; -import { minimatch } from 'minimatch'; -import { major } from 'semver'; import { deserializeStructure, indexBy } from '../../util'; import type { SdkProvider } from '../aws-auth/private'; import { Mode } from '../plugin'; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts index c67049478..fd459feb4 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts @@ -15,37 +15,36 @@ export function generateStackDefinitions( // overwriting its CDKMetadata resource with the one from the deployed stack for (const localStack of localStacks) { const deployedStack = deployedStackMap.get(localStack.stackName); - if (deployedStack) { - const localTemplate = localStack.template; - const deployedTemplate = deployedStack.template; - localTemplate.Resources = localTemplate.Resources ?? {}; - deployedTemplate.Resources = deployedTemplate.Resources ?? {}; + const localTemplate = localStack.template; + const deployedTemplate = deployedStack?.template; - // We need to preserve whatever CDKMetadata we had before. - // Otherwise, CloudFormation will consider this a modification. - if (deployedTemplate.Resources?.CDKMetadata != null) { - localTemplate.Resources.CDKMetadata = deployedTemplate.Resources.CDKMetadata; - } else { - delete localTemplate.Resources.CDKMetadata; - } + // The CDKMetadata resource is never part of a refactor. So at this point we need + // to adjust the template we will send to the API to make sure it has the same CDKMetadata + // as the deployed template. And if the deployed template doesn't have any, we cannot + // send any either. + if (deployedTemplate?.Resources?.CDKMetadata != null) { + localTemplate.Resources = localTemplate.Resources ?? {}; + localTemplate.Resources.CDKMetadata = deployedTemplate.Resources.CDKMetadata; + } else { + delete localTemplate.Resources?.CDKMetadata; + } - // For every resource in the local template, take the Metadata['aws:cdk:path'] from the corresponding resource in the deployed template. - // A corresponding resource is one that the local maps to (using the `mappings` parameter). If there is no entry mapping the local - // resource, use the same id - // TODO Remove this logic once CloudFormation starts allowing changes to the construct path. - // But we need it for now, otherwise we won't be able to refactor anything. - for (const [logicalId, localResource] of Object.entries(localTemplate.Resources)) { - const mapping = mappings.find( - (m) => m.destination.stackName === localStack.stackName && m.destination.logicalResourceId === logicalId, - ); + // For every resource in the local template, take the Metadata['aws:cdk:path'] from the corresponding resource in the deployed template. + // A corresponding resource is one that the local maps to (using the `mappings` parameter). If there is no entry mapping the local + // resource, use the same id + // TODO Remove this logic once CloudFormation starts allowing changes to the construct path. + // But we need it for now, otherwise we won't be able to refactor anything. + for (const [logicalId, localResource] of Object.entries(localTemplate.Resources ?? {})) { + const mapping = mappings.find( + (m) => m.destination.stackName === localStack.stackName && m.destination.logicalResourceId === logicalId, + ); - if (mapping != null) { - const deployed = deployedStackMap.get(mapping.source.stackName)!; - const deployedResource = deployed.template?.Resources?.[mapping.source.logicalResourceId]!; - if (deployedResource.Metadata != null || localResource.Metadata != null) { - localResource.Metadata = localResource.Metadata ?? {}; - localResource.Metadata['aws:cdk:path'] = deployedResource?.Metadata?.['aws:cdk:path']; - } + if (mapping != null) { + const deployed = deployedStackMap.get(mapping.source.stackName)!; + const deployedResource = deployed.template?.Resources?.[mapping.source.logicalResourceId]!; + if (deployedResource.Metadata != null || localResource.Metadata != null) { + localResource.Metadata = localResource.Metadata ?? {}; + localResource.Metadata['aws:cdk:path'] = deployedResource?.Metadata?.['aws:cdk:path']; } } } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index c2c588c36..49187b29c 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -11,7 +11,7 @@ import { NonInteractiveIoHost } from './non-interactive-io-host'; import type { ToolkitServices } from './private'; import { assemblyFromSource } from './private'; import { ToolkitError } from './toolkit-error'; -import type { FeatureFlag, DeployResult, DestroyResult, RollbackResult } from './types'; +import type { DeployResult, DestroyResult, FeatureFlag, RollbackResult } from './types'; import type { BootstrapEnvironments, BootstrapOptions, @@ -64,11 +64,7 @@ import { formatEnvironmentSectionHeader, formatTypedMappings, groupStacks, - ManifestExcludeList, - usePrescribedMappings, - groupStacks, } from '../api/refactoring'; -import type { ResourceMapping } from '../api/refactoring/cloudformation'; import type { CloudFormationStack } from '../api/refactoring/cloudformation'; import { ResourceMapping, ResourceLocation } from '../api/refactoring/cloudformation'; import { RefactoringContext } from '../api/refactoring/context'; @@ -1101,20 +1097,30 @@ export class Toolkit extends CloudAssemblySourceBuilder { refactorResult.ambiguousPaths = paths; } + await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(refactorMessage, refactorResult)); + + if (options.dryRun || context.mappings.length === 0) { + // Nothing left to do. + continue; + } + // In interactive mode (TTY) we need confirmation before proceeding if (process.stdout.isTTY && !await confirm(options.force ?? false)) { - await notifyInfo(chalk.red(`Refactoring canceled for environment aws://${environment.account}/${environment.region}\n`)); + await ioHelper.defaults.info(chalk.red(`Refactoring canceled for environment aws://${environment.account}/${environment.region}\n`)); continue; } - await notifyInfo('Refactoring...'); + await ioHelper.defaults.info('Refactoring...'); await context.execute(stackDefinitions, sdkProvider, ioHelper); - await notifyInfo('✅ Stack refactor complete'); + await ioHelper.defaults.info('✅ Stack refactor complete'); await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(refactorMessage, refactorResult)); } catch (e: any) { const message = `❌ Refactor failed: ${formatError(e)}`; await ioHelper.notify(IO.CDK_TOOLKIT_E8900.msg(message, { error: e })); + + // Also debugging the error, because the API does not always return a user-friendly message + await ioHelper.defaults.debug(e.message); } } @@ -1157,6 +1163,19 @@ export class Toolkit extends CloudAssemblySourceBuilder { } } + async function confirm(force: boolean): Promise { + // 'force' is set to true is the equivalent of having pre-approval for any refactor + if (force) { + return true; + } + + const question = 'Do you wish to refactor these resources?'; + const response = await ioHelper.requestResponse(IO.CDK_TOOLKIT_I8910.req(question, { + responseDescription: '[Y]es/[n]o', + }, 'y')); + return ['y', 'yes'].includes(response.toLowerCase()); + } + function formatError(error: any): string { try { const payload = JSON.parse(error.message); diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index a82c2cb3d..51756a576 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -7,8 +7,6 @@ import { ListStacksCommand, } from '@aws-sdk/client-cloudformation'; import { GetCallerIdentityCommand } from '@aws-sdk/client-sts'; -import { MappingSource, type RefactorOptions, Toolkit } from '../../lib'; -import { GetTemplateCommand, ListStacksCommand } from '@aws-sdk/client-cloudformation'; import { type RefactorOptions, StackSelectionStrategy, Toolkit } from '../../lib'; import { SdkProvider } from '../../lib/api/aws-auth/private'; import { builderFixture, TestIoHost } from '../_helpers'; @@ -358,15 +356,6 @@ test('detects modifications to the infrastructure', async () => { ); }); -test('fails when dry-run is false', async () => { - const cx = await builderFixture(toolkit, 'stack-with-bucket'); - await expect( - toolkit.refactor(cx, { - dryRun: false, - }), - ).rejects.toThrow('Refactor is not available yet. Too see the proposed changes, use the --dry-run flag.'); -}); - test('overrides can be used to resolve ambiguities', async () => { // GIVEN mockCloudFormationClient.on(ListStacksCommand).resolves({ diff --git a/packages/aws-cdk/lib/cli/cli-type-registry.json b/packages/aws-cdk/lib/cli/cli-type-registry.json index 0065a9f41..fb642023c 100644 --- a/packages/aws-cdk/lib/cli/cli-type-registry.json +++ b/packages/aws-cdk/lib/cli/cli-type-registry.json @@ -924,6 +924,11 @@ "type": "boolean", "default": false, "desc": "If specified, the command will revert the refactor operation. This is only valid if a mapping file was provided." + }, + "force": { + "type": "boolean", + "default": false, + "desc": "Whether to do the refactor without asking for confirmation" } } }, From 95d8daa9734fb2d45fc45d5e4001e9f52e38b1ff Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Wed, 16 Jul 2025 15:37:32 +0100 Subject: [PATCH 08/10] Improve ambiguity message --- packages/@aws-cdk/cloudformation-diff/lib/mappings.ts | 2 ++ packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts b/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts index 498590fc3..296906a07 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/mappings.ts @@ -35,6 +35,8 @@ export function formatAmbiguousMappings( formatter.print('Detected ambiguities:'); formatter.print(tables.join('\n\n')); formatter.print(' '); + formatter.print(chalk.yellow('Please provide an override file to resolve these ambiguous mappings.')); + formatter.print(' '); function renderTable([removed, added]: [string[], string[]]) { return formatTable([['', 'Resource'], renderRemoval(removed), renderAddition(added)], undefined); diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 49187b29c..04d11a78b 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -1099,7 +1099,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(refactorMessage, refactorResult)); - if (options.dryRun || context.mappings.length === 0) { + if (options.dryRun || context.mappings.length === 0 || context.ambiguousPaths.length > 0) { // Nothing left to do. continue; } From 66cc6597fdc9ef16b11f3f9af51a0789a010f779 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Thu, 17 Jul 2025 10:35:01 +0100 Subject: [PATCH 09/10] Overrides can be construct paths --- .../toolkit-lib/lib/toolkit/toolkit.ts | 51 ++++++++----------- .../toolkit-lib/test/actions/refactor.test.ts | 12 +++-- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 04d11a78b..7cc89d8c0 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -1113,8 +1113,6 @@ export class Toolkit extends CloudAssemblySourceBuilder { await ioHelper.defaults.info('Refactoring...'); await context.execute(stackDefinitions, sdkProvider, ioHelper); await ioHelper.defaults.info('✅ Stack refactor complete'); - - await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(refactorMessage, refactorResult)); } catch (e: any) { const message = `❌ Refactor failed: ${formatError(e)}`; await ioHelper.notify(IO.CDK_TOOLKIT_E8900.msg(message, { error: e })); @@ -1128,39 +1126,30 @@ export class Toolkit extends CloudAssemblySourceBuilder { const mappingGroup = options.overrides ?.find(g => g.region === environment.region && g.account === environment.account); - let overrides: ResourceMapping[] = []; - if (mappingGroup != null) { - overrides = Object.entries(mappingGroup.resources ?? {}).map(([source, destination]) => { - const sourceStack = findStack(source, deployedStacks); - const sourceLogicalId = source.split('.')[1]; - - const destinationStack = findStack(destination, localStacks); - const destinationLogicalId = destination.split('.')[1]; - - return new ResourceMapping( - new ResourceLocation(sourceStack, sourceLogicalId), - new ResourceLocation(destinationStack, destinationLogicalId), - ); - }); - } - - return overrides; + return mappingGroup == null + ? [] + : Object.entries(mappingGroup.resources ?? {}) + .map(([source, destination]) => new ResourceMapping( + getResourceLocation(source, deployedStacks), + getResourceLocation(destination, localStacks), + )); + } - function findStack(location: string, stacks: CloudFormationStack[]): CloudFormationStack { - const result = stacks.find(stack => { - const [stackName, logicalId] = location.split('.'); - if (stackName == null || logicalId == null) { - throw new ToolkitError(`Invalid location '${location}'`); + function getResourceLocation(location: string, stacks: CloudFormationStack[]): ResourceLocation { + for (let stack of stacks) { + const [stackName, logicalId] = location.split('.'); + if (stackName != null && logicalId != null && stack.stackName === stackName && stack.template.Resources?.[logicalId] != null) { + return new ResourceLocation(stack, logicalId); + } else { + const resourceEntry = Object + .entries(stack.template.Resources ?? {}) + .find(([_, r]) => r.Metadata?.['aws:cdk:path'] === location); + if (resourceEntry != null) { + return new ResourceLocation(stack, resourceEntry[0]); } - return stack.stackName === stackName && stack.template.Resources?.[logicalId] != null; - }); - - if (result == null) { - throw new ToolkitError(`Cannot find resource in location ${location}`); } - - return result; } + throw new ToolkitError(`Cannot find resource in location ${location}`); } async function confirm(force: boolean): Promise { diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts index 51756a576..a3a9bb11e 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/refactor.test.ts @@ -356,7 +356,12 @@ test('detects modifications to the infrastructure', async () => { ); }); -test('overrides can be used to resolve ambiguities', async () => { +test.each([ + // In each case, only one entry is enough to disambiguate the pair. + // In general, for n elements, we can disambiguate with n - 1 entries. + ['logical ID', { 'Stack1.CatPhotos': 'Stack1.MyBucket1553EAA46' }], + ['construct path', { 'Stack1/CatPhotos/Resource': 'Stack1/MyBucket1/Resource' }], +])('overrides can be used to resolve ambiguities (using %s)', async (_, overrides) => { // GIVEN mockCloudFormationClient.on(ListStacksCommand).resolves({ StackSummaries: [ @@ -405,10 +410,7 @@ test('overrides can be used to resolve ambiguities', async () => { { account: '123456789012', region: 'us-east-1', - resources: { - // Only one override is enough in this case - 'Stack1.CatPhotos': 'Stack1.MyBucket1553EAA46', - }, + resources: overrides, }, ], stacks: { From 44a9b660f160c6e0eaf6108113cca27ed9a36279 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Thu, 17 Jul 2025 11:00:57 +0100 Subject: [PATCH 10/10] update test: new stack creation without CDKMetadata --- .../test/api/refactoring/refactoring.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 fdec05aa0..b4a411844 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 @@ -1055,6 +1055,15 @@ describe(generateStackDefinitions, () => { Bucket2: { Type: 'AWS::S3::Bucket', }, + CDKMetadata: { + Type: 'AWS::CDK::Metadata', + Properties: { + Analytics: 'v2:deflate64:AAA', + }, + Metadata: { + 'aws:cdk:path': 'Data/CDKMetadata/Default', + }, + }, }, }, }; @@ -1085,6 +1094,7 @@ describe(generateStackDefinitions, () => { Bucket2: { Type: 'AWS::S3::Bucket', }, + // CDKMetadata was not included }, }), },