Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,11 @@ export enum ArtifactMetadataEntryType {
* Represents tags of a stack.
*/
STACK_TAGS = 'aws:cdk:stack-tags',

/**
* Whether the resource should be skipped during refactoring.
*/
SKIP_REFACTOR = 'aws:cdk:skip-refactor',
Copy link
Contributor

@mrgrain mrgrain May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be called exclude now? Or

Suggested change
SKIP_REFACTOR = 'aws:cdk:skip-refactor',
NO_REFACTOR = 'aws:cdk:no-refactor',

or

Suggested change
SKIP_REFACTOR = 'aws:cdk:skip-refactor',
DO_NOT_REFACTOR = 'aws:cdk:do-not-refactor',

}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { TypedMapping } from '@aws-cdk/cloudformation-diff';
import type * as cxapi from '@aws-cdk/cx-api';

export interface CloudFormationTemplate {
Expand All @@ -15,3 +16,54 @@ export interface CloudFormationStack {
readonly stackName: string;
readonly template: CloudFormationTemplate;
}

/**
* This class mirrors the `ResourceLocation` interface from CloudFormation,
* but is richer, since it has a reference to the stack object, rather than
* merely the stack name.
*/
export class ResourceLocation {
constructor(public readonly stack: CloudFormationStack, public readonly logicalResourceId: string) {
}

public toPath(): string {
const stack = this.stack;
const resource = stack.template.Resources?.[this.logicalResourceId];
const result = resource?.Metadata?.['aws:cdk:path'];

if (result != null) {
return result;
}

// If the path is not available, we can use stack name and logical ID
return `${stack.stackName}.${this.logicalResourceId}`;
}

public getType(): string {
const resource = this.stack.template.Resources?.[this.logicalResourceId ?? ''];
return resource?.Type ?? 'Unknown';
}

public equalTo(other: ResourceLocation): boolean {
return this.logicalResourceId === other.logicalResourceId && this.stack.stackName === other.stack.stackName;
}
}

/**
* A mapping between a source and a destination location.
*/
export class ResourceMapping {
constructor(public readonly source: ResourceLocation, public readonly destination: ResourceLocation) {
}

public toTypedMapping(): TypedMapping {
return {
// the type is the same in both source and destination,
// so we can use either one
type: this.source.getType(),
sourcePath: this.source.toPath(),
destinationPath: this.destination.toPath(),
};
}
}

89 changes: 25 additions & 64 deletions packages/@aws-cdk/tmp-toolkit-helpers/src/api/refactoring/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import type { SdkProvider } from '../aws-auth';
import { Mode } from '../plugin';
import { StringWriteStream } from '../streams';
import type { CloudFormationStack } from './cloudformation';
import { ResourceMapping, ResourceLocation } from './cloudformation';
import { computeResourceDigests, hashObject } from './digest';
import { NeverSkipList, type SkipList } from './skip';

export * from './skip';

/**
* Represents a set of possible movements of a resource from one location
Expand All @@ -33,56 +37,6 @@ export class AmbiguityError extends Error {
}
}

/**
* This class mirrors the `ResourceLocation` interface from CloudFormation,
* but is richer, since it has a reference to the stack object, rather than
* merely the stack name.
*/
export class ResourceLocation {
constructor(public readonly stack: CloudFormationStack, public readonly logicalResourceId: string) {
}

public toPath(): string {
const stack = this.stack;
const resource = stack.template.Resources?.[this.logicalResourceId];
const result = resource?.Metadata?.['aws:cdk:path'];

if (result != null) {
return result;
}

// If the path is not available, we can use stack name and logical ID
return `${stack.stackName}.${this.logicalResourceId}`;
}

public getType(): string {
const resource = this.stack.template.Resources?.[this.logicalResourceId ?? ''];
return resource?.Type ?? 'Unknown';
}

public equalTo(other: ResourceLocation): boolean {
return this.logicalResourceId === other.logicalResourceId && this.stack.stackName === other.stack.stackName;
}
}

/**
* A mapping between a source and a destination location.
*/
export class ResourceMapping {
constructor(public readonly source: ResourceLocation, public readonly destination: ResourceLocation) {
}

public toTypedMapping(): TypedMapping {
return {
// the type is the same in both source and destination,
// so we can use either one
type: this.source.getType(),
sourcePath: this.source.toPath(),
destinationPath: this.destination.toPath(),
};
}
}

function groupByKey<A>(entries: [string, A][]): Record<string, A[]> {
const result: Record<string, A[]> = {};

Expand Down Expand Up @@ -118,25 +72,27 @@ export function ambiguousMovements(movements: ResourceMovement[]) {
* Converts a list of unambiguous resource movements into a list of resource mappings.
*
*/
export function resourceMappings(movements: ResourceMovement[], stacks?: CloudFormationStack[]): ResourceMapping[] {
const predicate = 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));
};
export function resourceMappings(
movements: ResourceMovement[],
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));
};

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(predicate);
.filter(stacksPredicate);
}

function removeUnmovedResources(
m: Record<string, ResourceMovement>,
): Record<string, ResourceMovement> {
function removeUnmovedResources(m: Record<string, ResourceMovement>): Record<string, ResourceMovement> {
const result: Record<string, ResourceMovement> = {};
for (const [hash, [before, after]] of Object.entries(m)) {
const common = before.filter((b) => after.some((a) => a.equalTo(b)));
Expand Down Expand Up @@ -196,6 +152,7 @@ function resourceDigests(stack: CloudFormationStack): [string, ResourceLocation]
export async function findResourceMovements(
stacks: CloudFormationStack[],
sdkProvider: SdkProvider,
skipList: SkipList = new NeverSkipList(),
): Promise<ResourceMovement[]> {
const stackGroups: Map<string, [CloudFormationStack[], CloudFormationStack[]]> = new Map();

Expand All @@ -216,7 +173,11 @@ export async function findResourceMovements(
for (const [_, [before, after]] of stackGroups) {
result.push(...resourceMovements(before, after));
}
return result;

return result.filter(mov => {
const after = mov[1];
return after.every(l => !skipList.isSkipped(l));
});
}

async function getDeployedStacks(
Expand Down
127 changes: 127 additions & 0 deletions packages/@aws-cdk/tmp-toolkit-helpers/src/api/refactoring/skip.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import * as fs from 'node:fs';
import type { AssemblyManifest } from '@aws-cdk/cloud-assembly-schema';
import { ArtifactMetadataEntryType, ArtifactType } from '@aws-cdk/cloud-assembly-schema';
import type { ResourceLocation as CfnResourceLocation } from '@aws-sdk/client-cloudformation';
import { ToolkitError } from '../toolkit-error';
import type { ResourceLocation } from './cloudformation';

export interface SkipList {
isSkipped(location: ResourceLocation): boolean;
}

export class ManifestSkipList implements SkipList {
private readonly skippedLocations: CfnResourceLocation[];

constructor(manifest: AssemblyManifest) {
this.skippedLocations = this.getSkippedLocations(manifest);
}

private getSkippedLocations(asmManifest: AssemblyManifest): CfnResourceLocation[] {
// First, we need to filter the artifacts to only include CloudFormation stacks
const stackManifests = Object.entries(asmManifest.artifacts ?? {}).filter(
([_, manifest]) => manifest.type === ArtifactType.AWS_CLOUDFORMATION_STACK,
);

const result: CfnResourceLocation[] = [];
for (let [stackName, manifest] of stackManifests) {
const locations = Object.values(manifest.metadata ?? {})
// Then pick only the resources in each stack marked with SKIP_REFACTOR
.filter((entries) =>
entries.some((entry) => entry.type === ArtifactMetadataEntryType.SKIP_REFACTOR && entry.data === true),
)
// Finally, get the logical ID of each resource
.map((entries) => {
const logicalIdEntry = entries.find((entry) => entry.type === ArtifactMetadataEntryType.LOGICAL_ID);
const location: CfnResourceLocation = {
StackName: stackName,
LogicalResourceId: logicalIdEntry!.data! as string,
};
return location;
});
result.push(...locations);
}
return result;
}

isSkipped(location: ResourceLocation): boolean {
return this.skippedLocations.some(
(loc) => loc.StackName === location.stack.stackName && loc.LogicalResourceId === location.logicalResourceId,
);
}
}

export class SkipFile implements SkipList {
private readonly skippedLocations: CfnResourceLocation[];
private readonly skippedPaths: string[];

constructor(private readonly filePath?: string) {
this.skippedLocations = [];
this.skippedPaths = [];

if (!this.filePath) {
return;
}

const parsedData = JSON.parse(fs.readFileSync(this.filePath, 'utf-8'));
if (!isValidSkipFileContent(parsedData)) {
throw new ToolkitError('The content of a skip file must be a JSON array of strings');
}

const locationRegex = /^[A-Za-z0-9]+\.[A-Za-z0-9]+$/;
const pathRegex = /^\w*(\/.*)*$/;

parsedData.forEach((item: string) => {
if (locationRegex.test(item)) {
const [stackName, logicalId] = item.split('.');
this.skippedLocations.push({
StackName: stackName,
LogicalResourceId: logicalId,
});
} else if (pathRegex.test(item)) {
this.skippedPaths.push(item);
} else {
throw new ToolkitError(
`Invalid resource location format: ${item}. Expected formats: stackName.logicalId or a construct path`,
);
}
});
}

isSkipped(location: ResourceLocation): boolean {
const containsLocation = this.skippedLocations.some((loc) => {
return loc.StackName === location.stack.stackName && loc.LogicalResourceId === location.logicalResourceId;
});

const containsPath = this.skippedPaths.some((path) => location.toPath() === path);
return containsLocation || containsPath;
}
}

function isValidSkipFileContent(data: any): data is string[] {
return Array.isArray(data) && data.every((item: any) => typeof item === 'string');
}

export class UnionSkipList implements SkipList {
constructor(private readonly skipLists: SkipList[]) {
}

isSkipped(location: ResourceLocation): boolean {
return this.skipLists.some((skipList) => skipList.isSkipped(location));
}
}

export class NeverSkipList implements SkipList {
isSkipped(_location: ResourceLocation): boolean {
return false;
}
}

export class AlwaysSkipList implements SkipList {
isSkipped(_location: ResourceLocation): boolean {
return true;
}
}

export function fromManifestAndSkipFile(manifest: AssemblyManifest, skipFile?: string): SkipList {
return new UnionSkipList([new ManifestSkipList(manifest), new SkipFile(skipFile)]);
}
14 changes: 14 additions & 0 deletions packages/@aws-cdk/toolkit-lib/lib/actions/refactor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,18 @@ export interface RefactorOptions {
* @default - all stacks
*/
stacks?: StackSelector;

/**
* The absolute path to a file that contains a list of resources to
* skip during the refactor. The file should be in JSON format and
* contain an array of _destination_ locations that should be skipped,
* i.e., the location to which a resource would be moved if the
* refactor were to happen.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is destination the better choice over source? My gut reaction was: "huh, but how do I know what the destination is?" Is this a concern?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear: I genuinely don't know the answer here. But if there's two choices we always run the risk of picking the less natural one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to be consistent with the manifest use case. When you declare in the cloud assembly that a certain construct should be excluded, it can only refer to the destination. I understand that it may be confusing, but I think it would be even more confusing to have different sources (no pun intended) of truth for different use cases. Happy to be convinced otherwise, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the thinking here is: The code base (and therefore the output assembly) have already been changed by the user. Refactoring is matching the new target state with the previous state from CFN. We are addressing resources by what they are currently called in the users local code base?

I can dig that. We might want to tweak the wording and explanation here a bit (maybe AI can help?), but otherwise all good.

*
* The format of the locations in the file can be either:
*
* - Stack name and logical ID (e.g. `Stack1.MyQueue`)
* - A construct path (e.g. `Stack1/Foo/Bar/Resource`).
*/
skipFile?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking for a better name. If anyone has any suggestions, let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For toolkit-lib, this should be an array. We can make the read from file a CLI feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking for a better name. If anyone has any suggestions, let me know.

exclude?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm going with exclude.

}
Loading