Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(CLI): diff reports wrong count of changed stacks #26796

Merged
merged 7 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 7 additions & 3 deletions packages/aws-cdk/lib/api/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ISDK } from './aws-auth/sdk';
import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/sdk-provider';
import { deployStack, DeployStackResult, destroyStack, makeBodyParameterAndUpload, DeploymentMethod } from './deploy-stack';
import { HotswapMode } from './hotswap/common';
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate } from './nested-stack-helpers';
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, flattenNestedStackNames, TemplateWithNestedStackCount } from './nested-stack-helpers';
import { ToolkitInfo } from './toolkit-info';
import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries } from './util/cloudformation';
import { StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
Expand Down Expand Up @@ -300,9 +300,13 @@ export class Deployments {
public async readCurrentTemplateWithNestedStacks(
rootStackArtifact: cxapi.CloudFormationStackArtifact,
retrieveProcessedTemplate: boolean = false,
): Promise<Template> {
): Promise<TemplateWithNestedStackCount> {
const sdk = (await this.prepareSdkWithLookupOrDeployRole(rootStackArtifact)).stackSdk;
return (await loadCurrentTemplateWithNestedStacks(rootStackArtifact, sdk, retrieveProcessedTemplate)).deployedTemplate;
const templateWithNestedStacks = await loadCurrentTemplateWithNestedStacks(rootStackArtifact, sdk, retrieveProcessedTemplate);
return {
deployedTemplate: templateWithNestedStacks.deployedTemplate,
nestedStackCount: flattenNestedStackNames(templateWithNestedStacks.nestedStackNames).length,
};
}

public async readCurrentTemplate(stackArtifact: cxapi.CloudFormationStackArtifact): Promise<Template> {
Expand Down
28 changes: 28 additions & 0 deletions packages/aws-cdk/lib/api/nested-stack-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ export interface NestedStackNames {
readonly nestedChildStackNames: { [logicalId: string]: NestedStackNames };
}

export interface TemplateWithNestedStackCount {
readonly deployedTemplate: Template;
readonly nestedStackCount: number;
}

/**
* Reads the currently deployed template from CloudFormation and adds a
* property, `NestedTemplate`, to any nested stacks that appear in either
Expand All @@ -40,6 +45,19 @@ export async function loadCurrentTemplateWithNestedStacks(
};
}

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 Down Expand Up @@ -135,6 +153,16 @@ 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
16 changes: 12 additions & 4 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,18 @@ export class CdkToolkit {
stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
}

const currentTemplate = await this.props.deployments.readCurrentTemplateWithNestedStacks(stack, options.compareAgainstProcessedTemplate);
diffs += options.securityOnly
? numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening))
: printStackDiff(currentTemplate, stack, strict, contextLines, quiet, stream);
const templateWithNames = await this.props.deployments.readCurrentTemplateWithNestedStacks(
stack, options.compareAgainstProcessedTemplate,
);
const currentTemplate = templateWithNames.deployedTemplate;
const nestedStackCount = templateWithNames.nestedStackCount;

const stackCount =
options.securityOnly
? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening)) > 0 ? 1 : 0)
: (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, stream) > 0 ? 1 : 0);

diffs += stackCount + nestedStackCount;
}
}

Expand Down
16 changes: 12 additions & 4 deletions packages/aws-cdk/test/api/cloudformation-deployments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,11 @@ test('readCurrentTemplateWithNestedStacks() can handle non-Resources in the temp
);

// WHEN
const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;

// THEN
expect(nestedStackCount).toEqual(1);
expect(deployedTemplate).toEqual({
Resources: {
NestedStack: {
Expand Down Expand Up @@ -451,9 +453,11 @@ test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling stru
);

// WHEN
const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;

// THEN
expect(nestedStackCount).toEqual(3);
expect(deployedTemplate).toEqual({
Resources: {
NestedStack: {
Expand Down Expand Up @@ -608,9 +612,11 @@ test('readCurrentTemplateWithNestedStacks() on an undeployed parent stack with a
});

// WHEN
const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;

// THEN
expect(nestedStackCount).toEqual(2);
expect(deployedTemplate).toEqual({
Resources: {
NestedStack: {
Expand Down Expand Up @@ -781,9 +787,11 @@ test('readCurrentTemplateWithNestedStacks() succesfully ignores stacks without m
));

// WHEN
const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;

// THEN
expect(nestedStackCount).toEqual(1);
expect(deployedTemplate).toEqual({
Resources: {
WithMetadata: {
Expand Down
103 changes: 76 additions & 27 deletions packages/aws-cdk/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,15 @@ describe('non-nested stacks', () => {
// Default implementations
cloudFormation.readCurrentTemplateWithNestedStacks.mockImplementation((stackArtifact: CloudFormationStackArtifact) => {
if (stackArtifact.stackName === 'D') {
return Promise.resolve({ resource: 'D' });
return Promise.resolve({
deployedTemplate: { resource: 'D' },
nestedStackCount: 0,
});
}
return Promise.resolve({});
return Promise.resolve({
deployedTemplate: {},
nestedStackCount: 0,
});
});
cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({
noOp: true,
Expand Down Expand Up @@ -85,6 +91,43 @@ describe('non-nested stacks', () => {
expect(exitCode).toBe(0);
});

test('diff number of stack diffs, not resource diffs', async () => {
// GIVEN
cloudExecutable = new MockCloudExecutable({
stacks: [{
stackName: 'A',
template: { resourceA: 'A', resourceB: 'B' },
},
{
stackName: 'B',
template: { resourceC: 'C' },
}],
});

toolkit = new CdkToolkit({
cloudExecutable,
deployments: cloudFormation,
configuration: cloudExecutable.configuration,
sdkProvider: cloudExecutable.sdkProvider,
});

const buffer = new StringWritable();

// WHEN
const exitCode = await toolkit.diff({
stackNames: ['A', 'B'],
stream: buffer,
});

// THEN
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
expect(plainTextOutput).toContain('Stack A');
expect(plainTextOutput).toContain('Stack B');

expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 2');
expect(exitCode).toBe(0);
});

test('exits with 1 with diffs and fail set to true', async () => {
// GIVEN
const buffer = new StringWritable();
Expand Down Expand Up @@ -161,7 +204,7 @@ describe('nested stacks', () => {
cloudExecutable = new MockCloudExecutable({
stacks: [{
stackName: 'Parent',
template: { },
template: {},
}],
});

Expand Down Expand Up @@ -209,41 +252,47 @@ describe('nested stacks', () => {
},
};
return Promise.resolve({
Resources: {
AdditionChild: {
Type: 'AWS::CloudFormation::Stack',
Resources: {
SomeResource: {
Type: 'AWS::Something',
deployedTemplate: {
Resources: {
AdditionChild: {
Type: 'AWS::CloudFormation::Stack',
Resources: {
SomeResource: {
Type: 'AWS::Something',
},
},
},
},
DeletionChild: {
Type: 'AWS::CloudFormation::Stack',
Resources: {
SomeResource: {
Type: 'AWS::Something',
Properties: {
Prop: 'value-to-be-removed',
DeletionChild: {
Type: 'AWS::CloudFormation::Stack',
Resources: {
SomeResource: {
Type: 'AWS::Something',
Properties: {
Prop: 'value-to-be-removed',
},
},
},
},
},
ChangedChild: {
Type: 'AWS::CloudFormation::Stack',
Resources: {
SomeResource: {
Type: 'AWS::Something',
Properties: {
Prop: 'old-value',
ChangedChild: {
Type: 'AWS::CloudFormation::Stack',
Resources: {
SomeResource: {
Type: 'AWS::Something',
Properties: {
Prop: 'old-value',
},
},
},
},
},
},
nestedStackCount: 3,
});
}
return Promise.resolve({});
return Promise.resolve({
deployedTemplate: {},
nestedStackCount: 0,
});
});
});

Expand Down Expand Up @@ -279,7 +328,7 @@ Resources
└─ [+] new-value


✨ Number of stacks with differences: 3`);
✨ Number of stacks with differences: 4`);

expect(exitCode).toBe(0);
});
Expand Down
Loading