Skip to content

Commit

Permalink
feat(CLI): Diff Supports Import Change Sets (#28787)
Browse files Browse the repository at this point in the history 
#28336 made diff create and parse change sets to determine accurate resource replacement information. This PR expands the change set parsing to support import type change sets. 

This shows the output of diff with a single resource import: 

<img width="1609" alt="Screenshot 2024-01-19 at 2 44 09 PM" src="https://github.com/aws/aws-cdk/assets/66279577/a67761fa-f0aa-4cb1-b8ec-049e116400b6">


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@comcalvi
comcalvi committed Jan 31, 2024
1 parent 492ef12 commit d973615
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 12 deletions.
21 changes: 21 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function fullDiff(
const theDiff = diffTemplate(currentTemplate, newTemplate);
if (changeSet) {
filterFalsePositivies(theDiff, changeSet);
addImportInformation(theDiff, changeSet);
}

return theDiff;
Expand Down Expand Up @@ -208,6 +209,15 @@ function deepCopy(x: any): any {
return x;
}

function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
const imports = findResourceImports(changeSet);
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
if (imports.includes(logicalId)) {
change.isImport = true;
}
});
}

function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
const replacements = findResourceReplacements(changeSet);
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
Expand Down Expand Up @@ -245,6 +255,17 @@ function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormati
});
}

function findResourceImports(changeSet: CloudFormation.DescribeChangeSetOutput): string[] {
const importedResourceLogicalIds = [];
for (const resourceChange of changeSet.Changes ?? []) {
if (resourceChange.ResourceChange?.Action === 'Import') {
importedResourceLogicalIds.push(resourceChange.ResourceChange.LogicalResourceId!);
}
}

return importedResourceLogicalIds;
}

function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): types.ResourceReplacements {
const replacements: types.ResourceReplacements = {};
for (const resourceChange of changeSet.Changes ?? []) {
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,8 @@ export enum ResourceImpact {
WILL_DESTROY = 'WILL_DESTROY',
/** The existing physical resource will be removed from CloudFormation supervision */
WILL_ORPHAN = 'WILL_ORPHAN',
/** The existing physical resource will be added to CloudFormation supervision */
WILL_IMPORT = 'WILL_IMPORT',
/** There is no change in this resource */
NO_CHANGE = 'NO_CHANGE',
}
Expand All @@ -495,6 +497,7 @@ function worstImpact(one: ResourceImpact, two?: ResourceImpact): ResourceImpact
if (!two) { return one; }
const badness = {
[ResourceImpact.NO_CHANGE]: 0,
[ResourceImpact.WILL_IMPORT]: 0,
[ResourceImpact.WILL_UPDATE]: 1,
[ResourceImpact.WILL_CREATE]: 2,
[ResourceImpact.WILL_ORPHAN]: 3,
Expand Down Expand Up @@ -528,6 +531,11 @@ export class ResourceDifference implements IDifference<Resource> {
*/
public readonly isRemoval: boolean;

/**
* Whether this resource was imported
*/
public isImport?: boolean;

/** Property-level changes on the resource */
private readonly propertyDiffs: { [key: string]: PropertyDifference<any> };

Expand All @@ -552,6 +560,7 @@ export class ResourceDifference implements IDifference<Resource> {

this.isAddition = oldValue === undefined;
this.isRemoval = newValue === undefined;
this.isImport = undefined;
}

public get oldProperties(): PropertyMap | undefined {
Expand Down Expand Up @@ -647,6 +656,9 @@ export class ResourceDifference implements IDifference<Resource> {
}

public get changeImpact(): ResourceImpact {
if (this.isImport) {
return ResourceImpact.WILL_IMPORT;
}
// Check the Type first
if (this.resourceTypes.oldType !== this.resourceTypes.newType) {
if (this.resourceTypes.oldType === undefined) { return ResourceImpact.WILL_CREATE; }
Expand Down
11 changes: 10 additions & 1 deletion packages/@aws-cdk/cloudformation-diff/lib/format.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ const ADDITION = chalk.green('[+]');
const CONTEXT = chalk.grey('[ ]');
const UPDATE = chalk.yellow('[~]');
const REMOVAL = chalk.red('[-]');
const IMPORT = chalk.blue('[←]');

class Formatter {
constructor(
Expand Down Expand Up @@ -159,7 +160,7 @@ class Formatter {
const resourceType = diff.isRemoval ? diff.oldResourceType : diff.newResourceType;

// eslint-disable-next-line max-len
this.print(`${this.formatPrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`);
this.print(`${this.formatResourcePrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`);

if (diff.isUpdate) {
const differenceCount = diff.differenceCount;
Expand All @@ -171,6 +172,12 @@ class Formatter {
}
}

public formatResourcePrefix(diff: ResourceDifference) {
if (diff.isImport) { return IMPORT; }

return this.formatPrefix(diff);
}

public formatPrefix<T>(diff: Difference<T>) {
if (diff.isAddition) { return ADDITION; }
if (diff.isUpdate) { return UPDATE; }
Expand Down Expand Up @@ -204,6 +211,8 @@ class Formatter {
return chalk.italic(chalk.bold(chalk.red('destroy')));
case ResourceImpact.WILL_ORPHAN:
return chalk.italic(chalk.yellow('orphan'));
case ResourceImpact.WILL_IMPORT:
return chalk.italic(chalk.blue('import'));
case ResourceImpact.WILL_UPDATE:
case ResourceImpact.WILL_CREATE:
case ResourceImpact.NO_CHANGE:
Expand Down
65 changes: 65 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1117,4 +1117,69 @@ describe('changeset', () => {
});
expect(differences.resources.differenceCount).toBe(1);
});

test('imports are respected for new stacks', async () => {
// GIVEN
const currentTemplate = {};

// WHEN
const newTemplate = {
Resources: {
BucketResource: {
Type: 'AWS::S3::Bucket',
},
},
};

let differences = fullDiff(currentTemplate, newTemplate, {
Changes: [
{
Type: 'Resource',
ResourceChange: {
Action: 'Import',
LogicalResourceId: 'BucketResource',
},
},
],
});
expect(differences.resources.differenceCount).toBe(1);
expect(differences.resources.get('BucketResource').changeImpact === ResourceImpact.WILL_IMPORT);
});

test('imports are respected for existing stacks', async () => {
// GIVEN
const currentTemplate = {
Resources: {
OldResource: {
Type: 'AWS::Something::Resource',
},
},
};

// WHEN
const newTemplate = {
Resources: {
OldResource: {
Type: 'AWS::Something::Resource',
},
BucketResource: {
Type: 'AWS::S3::Bucket',
},
},
};

let differences = fullDiff(currentTemplate, newTemplate, {
Changes: [
{
Type: 'Resource',
ResourceChange: {
Action: 'Import',
LogicalResourceId: 'BucketResource',
},
},
],
});
expect(differences.resources.differenceCount).toBe(1);
expect(differences.resources.get('BucketResource').changeImpact === ResourceImpact.WILL_IMPORT);
});
});
20 changes: 17 additions & 3 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ export type PrepareChangeSetOptions = {
sdkProvider: SdkProvider;
stream: NodeJS.WritableStream;
parameters: { [name: string]: string | undefined };
resourcesToImport?: ResourcesToImport;
}

export type CreateChangeSetOptions = {
Expand All @@ -303,6 +304,8 @@ export type CreateChangeSetOptions = {
stack: cxapi.CloudFormationStackArtifact;
bodyParameter: TemplateBodyParameter;
parameters: { [name: string]: string | undefined };
resourcesToImport?: ResourcesToImport;
role?: string;
}

/**
Expand Down Expand Up @@ -337,7 +340,9 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
const cfn = preparedSdk.stackSdk.cloudFormation();
const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;

const executionRoleArn = preparedSdk.cloudFormationRoleArn;
options.stream.write('Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)\n');

return await createChangeSet({
cfn,
changeSetName: 'cdk-diff-change-set',
Expand All @@ -347,6 +352,8 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
willExecute: options.willExecute,
bodyParameter,
parameters: options.parameters,
resourcesToImport: options.resourcesToImport,
role: executionRoleArn,
});
} catch (e: any) {
debug(e.message);
Expand All @@ -367,12 +374,14 @@ async function createChangeSet(options: CreateChangeSetOptions): Promise<CloudFo
const changeSet = await options.cfn.createChangeSet({
StackName: options.stack.stackName,
ChangeSetName: options.changeSetName,
ChangeSetType: options.exists ? 'UPDATE' : 'CREATE',
ChangeSetType: options.resourcesToImport ? 'IMPORT' : options.exists ? 'UPDATE' : 'CREATE',
Description: `CDK Changeset for diff ${options.uuid}`,
ClientToken: `diff${options.uuid}`,
TemplateURL: options.bodyParameter.TemplateURL,
TemplateBody: options.bodyParameter.TemplateBody,
Parameters: stackParams.apiParameters,
ResourcesToImport: options.resourcesToImport,
RoleARN: options.role,
Capabilities: ['CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM', 'CAPABILITY_AUTO_EXPAND'],
}).promise();

Expand All @@ -384,12 +393,17 @@ async function createChangeSet(options: CreateChangeSetOptions): Promise<CloudFo
return createdChangeSet;
}

export async function cleanupOldChangeset(exists: boolean, changeSetName: string, stackName: string, cfn: CloudFormation) {
if (exists) {
export async function cleanupOldChangeset(stackExists: boolean, changeSetName: string, stackName: string, cfn: CloudFormation) {
if (stackExists) {
// Delete any existing change sets generated by CDK since change set names must be unique.
// The delete request is successful as long as the stack exists (even if the change set does not exist).
debug(`Removing existing change set with name ${changeSetName} if it exists`);
await cfn.deleteChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise();
} else {
// delete the stack since creating a changeset for a stack that doesn't exist leaves that stack in a REVIEW_IN_PROGRESS state
// that prevents other changesets from being created, even after the changeset has been deleted.
debug(`Removing stack with name ${stackName}`);
await cfn.deleteStack({ StackName: stackName }).promise();
}
}

Expand Down
25 changes: 21 additions & 4 deletions packages/aws-cdk/lib/cdk-toolkit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { createDiffChangeSet, ResourcesToImport } from './api/util/cloudformatio
import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor';
import { generateCdkApp, generateStack, readFromPath, readFromStack, setEnvironment, validateSourceOptions } from './commands/migrate';
import { printSecurityDiff, printStackDiff, RequireApproval } from './diff';
import { ResourceImporter } from './import';
import { ResourceImporter, removeNonImportResources } from './import';
import { data, debug, error, highlight, print, success, warning, withCorkedLogging } from './logging';
import { deserializeStructure, serializeStructure } from './serialize';
import { Configuration, PROJECT_CONFIG } from './settings';
Expand Down Expand Up @@ -162,16 +162,26 @@ export class CdkToolkit {
const currentTemplate = templateWithNames.deployedTemplate;
const nestedStackCount = templateWithNames.nestedStackCount;

const resourcesToImport = await this.tryGetResources(await this.props.deployments.resolveEnvironment(stack));
if (resourcesToImport) {
removeNonImportResources(stack);
}

const changeSet = options.changeSet ? await createDiffChangeSet({
stack,
uuid: uuid.v4(),
deployments: this.props.deployments,
willExecute: false,
sdkProvider: this.props.sdkProvider,
parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]),
resourcesToImport,
stream,
}) : undefined;

if (resourcesToImport) {
stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
}

const stackCount =
options.securityOnly
? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)) > 0 ? 1 : 0)
Expand Down Expand Up @@ -205,6 +215,12 @@ export class CdkToolkit {
const elapsedSynthTime = new Date().getTime() - startSynthTime;
print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime));

if (stackCollection.stackCount === 0) {
// eslint-disable-next-line no-console
console.error('This app contains no stacks');
return;
}

await this.tryMigrateResources(stackCollection, options);

const requireApproval = options.requireApproval ?? RequireApproval.Broadening;
Expand Down Expand Up @@ -884,7 +900,7 @@ export class CdkToolkit {
private async tryMigrateResources(stacks: StackCollection, options: DeployOptions): Promise<void> {
const stack = stacks.stackArtifacts[0];
const migrateDeployment = new ResourceImporter(stack, this.props.deployments);
const resourcesToImport = await this.tryGetResources(migrateDeployment);
const resourcesToImport = await this.tryGetResources(await migrateDeployment.resolveEnvironment());

if (resourcesToImport) {
print('%s: creating stack for resource migration...', chalk.bold(stack.displayName));
Expand Down Expand Up @@ -918,18 +934,19 @@ export class CdkToolkit {
print('\n✨ Resource migration time: %ss\n', formatTime(elapsedDeployTime));
}

private async tryGetResources(migrateDeployment: ResourceImporter) {
private async tryGetResources(environment: cxapi.Environment): Promise<ResourcesToImport | undefined> {
try {
const migrateFile = fs.readJsonSync('migrate.json', { encoding: 'utf-8' });
const sourceEnv = (migrateFile.Source as string).split(':');
const environment = await migrateDeployment.resolveEnvironment();
if (sourceEnv[0] === 'localfile' ||
(sourceEnv[4] === environment.account && sourceEnv[3] === environment.region)) {
return migrateFile.Resources;
}
} catch (e) {
// Nothing to do
}

return undefined;
}
}

Expand Down
16 changes: 12 additions & 4 deletions packages/aws-cdk/lib/import.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,21 @@ export class ResourceImporter {
* @returns template with import resources only
*/
private removeNonImportResources() {
const template = this.stack.template;
delete template.Resources.CDKMetadata;
delete template.Outputs;
return template;
return removeNonImportResources(this.stack);
}
}

/**
* Removes CDKMetadata and Outputs in the template so that only resources for importing are left.
* @returns template with import resources only
*/
export function removeNonImportResources(stack: cxapi.CloudFormationStackArtifact) {
const template = stack.template;
delete template.Resources.CDKMetadata;
delete template.Outputs;
return template;
}

/**
* Information about a resource in the template that is importable
*/
Expand Down
Loading

0 comments on commit d973615

Please sign in to comment.