Skip to content

Commit

Permalink
feat(CLI): improved nested stack diff (#29172)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)


### Reason for this change

The existing nested stack diff places a fake property, `NestedTemplate`, in the templates to be diffed. This prevents displaying resource replacement information in the diff, like we do for top level stacks. This PR does *not* add changeset replacement information from changesets, but it does add replacement information from the spec. 

### Description of changes

Reworked nested stack diff to treat nested stacks as top level stacks. This improves the visual UX and sets us up for using changesets with nested stacks. 

#### Before

<img width="957" alt="Screenshot 2024-02-19 at 1 47 59 PM" src="https://github.com/aws/aws-cdk/assets/66279577/a94275c4-e7c3-4d2c-a924-ee61c36bea4d">


#### After
<img width="957" alt="Screenshot 2024-02-19 at 1 48 48 PM" src="https://github.com/aws/aws-cdk/assets/66279577/5263aaf9-ef2f-4228-b413-81e780c4b8f8">



### Description of how you validated changes

Unit tests + manual tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*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 committed Mar 13, 2024
1 parent 4582ac5 commit 135b520
Show file tree
Hide file tree
Showing 14 changed files with 757 additions and 463 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
Expand Up @@ -54,7 +54,7 @@ export function fullDiff(
normalize(newTemplate);
const theDiff = diffTemplate(currentTemplate, newTemplate);
if (changeSet) {
filterFalsePositivies(theDiff, changeSet);
filterFalsePositives(theDiff, changeSet);
addImportInformation(theDiff, changeSet);
}
if (isImport) {
Expand All @@ -64,7 +64,7 @@ export function fullDiff(
return theDiff;
}

function diffTemplate(
export function diffTemplate(
currentTemplate: { [key: string]: any },
newTemplate: { [key: string]: any },
): types.TemplateDiff {
Expand Down Expand Up @@ -235,7 +235,7 @@ function addImportInformation(diff: types.TemplateDiff, changeSet?: CloudFormati
}
}

function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
const replacements = findResourceReplacements(changeSet);
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
if (change.resourceType.includes('AWS::Serverless')) {
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/cloudformation-diff/lib/format.ts
Expand Up @@ -30,7 +30,7 @@ export interface FormatStream extends NodeJS.WritableStream {
export function formatDifferences(
stream: FormatStream,
templateDiff: TemplateDiff,
logicalToPathMap: { [logicalId: string]: string } = { },
logicalToPathMap: { [logicalId: string]: string } = {},
context: number = 3) {
const formatter = new Formatter(stream, logicalToPathMap, templateDiff, context);

Expand Down Expand Up @@ -59,7 +59,7 @@ export function formatDifferences(
export function formatSecurityChanges(
stream: NodeJS.WritableStream,
templateDiff: TemplateDiff,
logicalToPathMap: {[logicalId: string]: string} = {},
logicalToPathMap: { [logicalId: string]: string } = {},
context?: number) {
const formatter = new Formatter(stream, logicalToPathMap, templateDiff, context);

Expand Down Expand Up @@ -254,7 +254,7 @@ class Formatter {
const oldStr = JSON.stringify(oldObject, null, 2);
const newStr = JSON.stringify(newObject, null, 2);
const diff = _diffStrings(oldStr, newStr, this.context);
for (let i = 0 ; i < diff.length ; i++) {
for (let i = 0; i < diff.length; i++) {
this.print('%s %s %s', linePrefix, i === 0 ? '└─' : ' ', diff[i]);
}
} else {
Expand Down Expand Up @@ -466,7 +466,7 @@ function _diffStrings(oldStr: string, newStr: string, context: number): string[]
function _findIndent(lines: string[]): number {
let indent = Number.MAX_SAFE_INTEGER;
for (const line of lines) {
for (let i = 1 ; i < line.length ; i++) {
for (let i = 1; i < line.length; i++) {
if (line.charAt(i) !== ' ') {
indent = indent > i - 1 ? i - 1 : indent;
break;
Expand Down
10 changes: 3 additions & 7 deletions packages/aws-cdk/lib/api/deployments.ts
Expand Up @@ -7,7 +7,7 @@ import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/s
import { deployStack, DeployStackResult, destroyStack, DeploymentMethod } from './deploy-stack';
import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources';
import { HotswapMode } from './hotswap/common';
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, flattenNestedStackNames, TemplateWithNestedStackCount } from './nested-stack-helpers';
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, RootTemplateWithNestedStacks } from './nested-stack-helpers';
import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries } from './util/cloudformation';
import { StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
import { replaceEnvPlaceholders } from './util/placeholders';
Expand Down Expand Up @@ -327,13 +327,9 @@ export class Deployments {
public async readCurrentTemplateWithNestedStacks(
rootStackArtifact: cxapi.CloudFormationStackArtifact,
retrieveProcessedTemplate: boolean = false,
): Promise<TemplateWithNestedStackCount> {
): Promise<RootTemplateWithNestedStacks> {
const sdk = (await this.prepareSdkWithLookupOrDeployRole(rootStackArtifact)).stackSdk;
const templateWithNestedStacks = await loadCurrentTemplateWithNestedStacks(rootStackArtifact, sdk, retrieveProcessedTemplate);
return {
deployedTemplate: templateWithNestedStacks.deployedTemplate,
nestedStackCount: flattenNestedStackNames(templateWithNestedStacks.nestedStackNames).length,
};
return loadCurrentTemplateWithNestedStacks(rootStackArtifact, sdk, retrieveProcessedTemplate);
}

public async readCurrentTemplate(stackArtifact: cxapi.CloudFormationStackArtifact): Promise<Template> {
Expand Down
34 changes: 16 additions & 18 deletions packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts
@@ -1,7 +1,7 @@
import * as AWS from 'aws-sdk';
import { PromiseResult } from 'aws-sdk/lib/request';
import { ISDK } from './aws-auth';
import { NestedStackNames } from './nested-stack-helpers';
import { NestedStackTemplates } from './nested-stack-helpers';

export interface ListStackResources {
listStackResources(): Promise<AWS.CloudFormation.StackResourceSummary[]>;
Expand Down Expand Up @@ -102,7 +102,7 @@ export interface EvaluateCloudFormationTemplateProps {
readonly partition: string;
readonly urlSuffix: (region: string) => string;
readonly sdk: ISDK;
readonly nestedStackNames?: { [nestedStackLogicalId: string]: NestedStackNames };
readonly nestedStacks?: { [nestedStackLogicalId: string]: NestedStackTemplates };
}

export class EvaluateCloudFormationTemplate {
Expand All @@ -114,7 +114,7 @@ export class EvaluateCloudFormationTemplate {
private readonly partition: string;
private readonly urlSuffix: (region: string) => string;
private readonly sdk: ISDK;
private readonly nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames };
private readonly nestedStacks: { [nestedStackLogicalId: string]: NestedStackTemplates };
private readonly stackResources: ListStackResources;
private readonly lookupExport: LookupExport;

Expand All @@ -136,7 +136,7 @@ export class EvaluateCloudFormationTemplate {
this.sdk = props.sdk;

// We need names of nested stack so we can evaluate cross stack references
this.nestedStackNames = props.nestedStackNames ?? {};
this.nestedStacks = props.nestedStacks ?? {};

// The current resources of the Stack.
// We need them to figure out the physical name of a resource in case it wasn't specified by the user.
Expand All @@ -163,7 +163,7 @@ export class EvaluateCloudFormationTemplate {
partition: this.partition,
urlSuffix: this.urlSuffix,
sdk: this.sdk,
nestedStackNames: this.nestedStackNames,
nestedStacks: this.nestedStacks,
});
}

Expand Down Expand Up @@ -386,17 +386,15 @@ export class EvaluateCloudFormationTemplate {
}

if (foundResource.ResourceType == 'AWS::CloudFormation::Stack' && attribute?.startsWith('Outputs.')) {
// need to resolve attributes from another stack's Output section
const dependantStackName = this.findNestedStack(logicalId, this.nestedStackNames);
if (!dependantStackName) {
const dependantStack = this.findNestedStack(logicalId, this.nestedStacks);
if (!dependantStack || !dependantStack.physicalName) {
//this is a newly created nested stack and cannot be hotswapped
return undefined;
}
const dependantStackTemplate = this.template.Resources[logicalId];
const evaluateCfnTemplate = await this.createNestedEvaluateCloudFormationTemplate(
dependantStackName,
dependantStackTemplate?.Properties?.NestedTemplate,
dependantStackTemplate.newValue?.Properties?.Parameters);
dependantStack.physicalName,
dependantStack.generatedTemplate,
dependantStack.generatedTemplate.Parameters!);

// Split Outputs.<refName> into 'Outputs' and '<refName>' and recursively call evaluate
return evaluateCfnTemplate.evaluateCfnExpression({ 'Fn::GetAtt': attribute.split(/\.(.*)/s) });
Expand All @@ -406,14 +404,14 @@ export class EvaluateCloudFormationTemplate {
return this.formatResourceAttribute(foundResource, attribute);
}

private findNestedStack(logicalId: string, nestedStackNames: {
[nestedStackLogicalId: string]: NestedStackNames;
}): string | undefined {
for (const [nestedStackLogicalId, { nestedChildStackNames, nestedStackPhysicalName }] of Object.entries(nestedStackNames)) {
private findNestedStack(logicalId: string, nestedStacks: {
[nestedStackLogicalId: string]: NestedStackTemplates;
}): NestedStackTemplates | undefined {
for (const nestedStackLogicalId of Object.keys(nestedStacks)) {
if (nestedStackLogicalId === logicalId) {
return nestedStackPhysicalName;
return nestedStacks[nestedStackLogicalId];
}
const checkInNestedChildStacks = this.findNestedStack(logicalId, nestedChildStackNames);
const checkInNestedChildStacks = this.findNestedStack(logicalId, nestedStacks[nestedStackLogicalId].nestedStackTemplates);
if (checkInNestedChildStacks) return checkInNestedChildStacks;
}
return undefined;
Expand Down
22 changes: 11 additions & 11 deletions packages/aws-cdk/lib/api/hotswap-deployments.ts
Expand Up @@ -11,7 +11,7 @@ import { isHotswappableEcsServiceChange } from './hotswap/ecs-services';
import { isHotswappableLambdaFunctionChange } from './hotswap/lambda-functions';
import { skipChangeForS3DeployCustomResourcePolicy, isHotswappableS3BucketDeploymentChange } from './hotswap/s3-bucket-deployments';
import { isHotswappableStateMachineChange } from './hotswap/stepfunctions-state-machines';
import { loadCurrentTemplateWithNestedStacks, NestedStackNames } from './nested-stack-helpers';
import { NestedStackTemplates, loadCurrentTemplateWithNestedStacks } from './nested-stack-helpers';
import { CloudFormationStack } from './util/cloudformation';
import { print } from '../logging';

Expand Down Expand Up @@ -78,12 +78,12 @@ export async function tryHotswapDeployment(
partition: (await sdk.currentAccount()).partition,
urlSuffix: (region) => sdk.getEndpointSuffix(region),
sdk,
nestedStackNames: currentTemplate.nestedStackNames,
nestedStacks: currentTemplate.nestedStacks,
});

const stackChanges = cfn_diff.fullDiff(currentTemplate.deployedTemplate, stackArtifact.template);
const stackChanges = cfn_diff.fullDiff(currentTemplate.deployedRootTemplate, stackArtifact.template);
const { hotswappableChanges, nonHotswappableChanges } = await classifyResourceChanges(
stackChanges, evaluateCfnTemplate, sdk, currentTemplate.nestedStackNames,
stackChanges, evaluateCfnTemplate, sdk, currentTemplate.nestedStacks,
);

logNonHotswappableChanges(nonHotswappableChanges, hotswapMode);
Expand All @@ -109,7 +109,7 @@ async function classifyResourceChanges(
stackChanges: cfn_diff.TemplateDiff,
evaluateCfnTemplate: EvaluateCloudFormationTemplate,
sdk: ISDK,
nestedStackNames: { [nestedStackName: string]: NestedStackNames },
nestedStackNames: { [nestedStackName: string]: NestedStackTemplates },
): Promise<ClassifiedResourceChanges> {
const resourceDifferences = getStackResourceDifferences(stackChanges);

Expand Down Expand Up @@ -225,12 +225,12 @@ function filterDict<T>(dict: { [key: string]: T }, func: (t: T) => boolean): { [
async function findNestedHotswappableChanges(
logicalId: string,
change: cfn_diff.ResourceDifference,
nestedStackNames: { [nestedStackName: string]: NestedStackNames },
nestedStackTemplates: { [nestedStackName: string]: NestedStackTemplates },
evaluateCfnTemplate: EvaluateCloudFormationTemplate,
sdk: ISDK,
): Promise<ClassifiedResourceChanges> {
const nestedStackName = nestedStackNames[logicalId].nestedStackPhysicalName;
if (!nestedStackName) {
const nestedStack = nestedStackTemplates[logicalId];
if (!nestedStack.physicalName) {
return {
hotswappableChanges: [],
nonHotswappableChanges: [{
Expand All @@ -244,14 +244,14 @@ async function findNestedHotswappableChanges(
}

const evaluateNestedCfnTemplate = await evaluateCfnTemplate.createNestedEvaluateCloudFormationTemplate(
nestedStackName, change.newValue?.Properties?.NestedTemplate, change.newValue?.Properties?.Parameters,
nestedStack.physicalName, nestedStack.generatedTemplate, change.newValue?.Properties?.Parameters,
);

const nestedDiff = cfn_diff.fullDiff(
change.oldValue?.Properties?.NestedTemplate, change.newValue?.Properties?.NestedTemplate,
nestedStackTemplates[logicalId].deployedTemplate, nestedStackTemplates[logicalId].generatedTemplate,
);

return classifyResourceChanges(nestedDiff, evaluateNestedCfnTemplate, sdk, nestedStackNames[logicalId].nestedChildStackNames);
return classifyResourceChanges(nestedDiff, evaluateNestedCfnTemplate, sdk, nestedStackTemplates[logicalId].nestedStackTemplates);
}

/** Returns 'true' if a pair of changes is for the same resource. */
Expand Down
85 changes: 23 additions & 62 deletions packages/aws-cdk/lib/api/nested-stack-helpers.ts
Expand Up @@ -5,59 +5,38 @@ import { ISDK } from './aws-auth';
import { LazyListStackResources, ListStackResources } from './evaluate-cloudformation-template';
import { CloudFormationStack, Template } from './util/cloudformation';

export interface TemplateWithNestedStackNames {
export interface NestedStackTemplates {
readonly physicalName: string | undefined;
readonly deployedTemplate: Template;
readonly nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames };
readonly generatedTemplate: Template;
readonly nestedStackTemplates: { [nestedStackLogicalId: string]: NestedStackTemplates};
}

export interface NestedStackNames {
readonly nestedStackPhysicalName: string | undefined;
readonly nestedChildStackNames: { [logicalId: string]: NestedStackNames };
}

export interface TemplateWithNestedStackCount {
readonly deployedTemplate: Template;
readonly nestedStackCount: number;
export interface RootTemplateWithNestedStacks {
readonly deployedRootTemplate: Template;
readonly nestedStacks: { [nestedStackLogicalId: string]: NestedStackTemplates };
}

/**
* Reads the currently deployed template from CloudFormation and adds a
* property, `NestedTemplate`, to any nested stacks that appear in either
* the deployed template or the newly synthesized template. `NestedTemplate`
* is populated with contents of the nested template by mutating the
* `template` property of `rootStackArtifact`. This is done for all
* nested stack resources to arbitrary depths.
* Reads the currently deployed template and all of its nested stack templates from CloudFormation.
*/
export async function loadCurrentTemplateWithNestedStacks(
rootStackArtifact: cxapi.CloudFormationStackArtifact, sdk: ISDK,
retrieveProcessedTemplate: boolean = false,
): Promise<TemplateWithNestedStackNames> {
const deployedTemplate = await loadCurrentTemplate(rootStackArtifact, sdk, retrieveProcessedTemplate);
const nestedStackNames = await addNestedTemplatesToGeneratedAndDeployedStacks(rootStackArtifact, sdk, {
): Promise<RootTemplateWithNestedStacks> {
const deployedRootTemplate = await loadCurrentTemplate(rootStackArtifact, sdk, retrieveProcessedTemplate);
const nestedStacks = await loadNestedStacks(rootStackArtifact, sdk, {
generatedTemplate: rootStackArtifact.template,
deployedTemplate: deployedTemplate,
deployedTemplate: deployedRootTemplate,
deployedStackName: rootStackArtifact.stackName,
});

return {
deployedTemplate,
nestedStackNames,
deployedRootTemplate,
nestedStacks,
};
}

export function flattenNestedStackNames(nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames }): string[] {
const nameList = [];
for (const key of Object.keys(nestedStackNames)) {
nameList.push(key);

if (Object.keys(nestedStackNames[key].nestedChildStackNames).length !== 0) {
flattenNestedStacksHelper(nestedStackNames[key].nestedChildStackNames, nameList);
}
}

return nameList;
}

/**
* Returns the currently deployed template from CloudFormation that corresponds to `stackArtifact`.
*/
Expand All @@ -76,13 +55,13 @@ async function loadCurrentStackTemplate(
return stack.template();
}

async function addNestedTemplatesToGeneratedAndDeployedStacks(
async function loadNestedStacks(
rootStackArtifact: cxapi.CloudFormationStackArtifact,
sdk: ISDK,
parentTemplates: StackTemplates,
): Promise<{ [nestedStackLogicalId: string]: NestedStackNames }> {
): Promise<{ [nestedStackLogicalId: string]: NestedStackTemplates }> {
const listStackResources = parentTemplates.deployedStackName ? new LazyListStackResources(sdk, parentTemplates.deployedStackName) : undefined;
const nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames } = {};
const nestedStacks: { [nestedStackLogicalId: string]: NestedStackTemplates } = {};
for (const [nestedStackLogicalId, generatedNestedStackResource] of Object.entries(parentTemplates.generatedTemplate.Resources ?? {})) {
if (!isCdkManagedNestedStack(generatedNestedStackResource)) {
continue;
Expand All @@ -91,27 +70,19 @@ async function addNestedTemplatesToGeneratedAndDeployedStacks(
const assetPath = generatedNestedStackResource.Metadata['aws:asset:path'];
const nestedStackTemplates = await getNestedStackTemplates(rootStackArtifact, assetPath, nestedStackLogicalId, listStackResources, sdk);

generatedNestedStackResource.Properties.NestedTemplate = nestedStackTemplates.generatedTemplate;

const deployedParentTemplate = parentTemplates.deployedTemplate;
deployedParentTemplate.Resources = deployedParentTemplate.Resources ?? {};
const deployedNestedStackResource = deployedParentTemplate.Resources[nestedStackLogicalId] ?? {};
deployedParentTemplate.Resources[nestedStackLogicalId] = deployedNestedStackResource;
deployedNestedStackResource.Type = deployedNestedStackResource.Type ?? 'AWS::CloudFormation::Stack';
deployedNestedStackResource.Properties = deployedNestedStackResource.Properties ?? {};
deployedNestedStackResource.Properties.NestedTemplate = nestedStackTemplates.deployedTemplate;

nestedStackNames[nestedStackLogicalId] = {
nestedStackPhysicalName: nestedStackTemplates.deployedStackName,
nestedChildStackNames: await addNestedTemplatesToGeneratedAndDeployedStacks(
nestedStacks[nestedStackLogicalId] = {
deployedTemplate: nestedStackTemplates.deployedTemplate,
generatedTemplate: nestedStackTemplates.generatedTemplate,
physicalName: nestedStackTemplates.deployedStackName,
nestedStackTemplates: await loadNestedStacks(
rootStackArtifact,
sdk,
nestedStackTemplates,
),
};
}

return nestedStackNames;
return nestedStacks;
}

async function getNestedStackTemplates(
Expand Down Expand Up @@ -153,16 +124,6 @@ function isCdkManagedNestedStack(stackResource: any): stackResource is NestedSta
return stackResource.Type === 'AWS::CloudFormation::Stack' && stackResource.Metadata && stackResource.Metadata['aws:asset:path'];
}

function flattenNestedStacksHelper(nestedStackNames: { [logicalId: string]: NestedStackNames }, nameList: string[]) {
for (const key of Object.keys(nestedStackNames)) {
nameList.push(key);

if (Object.keys(nestedStackNames[key].nestedChildStackNames).length !== 0) {
flattenNestedStacksHelper(nestedStackNames[key].nestedChildStackNames, nameList);
}
}
}

interface StackTemplates {
readonly generatedTemplate: any;
readonly deployedTemplate: any;
Expand Down

0 comments on commit 135b520

Please sign in to comment.