Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@comcalvi
comcalvi committed Feb 1, 2022
1 parent 38b5723 commit 9b78354
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 83 deletions.
65 changes: 24 additions & 41 deletions packages/aws-cdk/lib/api/cloudformation-deployments.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,12 @@ export class CloudFormationDeployments {
this.sdkProvider = props.sdkProvider;
}

public async readCurrentTemplateWithNestedStacks(rootStackArtifact: cxapi.CloudFormationStackArtifact): Promise<Template> {
public async readCurrentTemplateWithNestedStacks(rootStackArtifact: cxapi.CloudFormationStackArtifact): Promise<Template> { // TODO: rename
const sdk = await this.prepareSdkWithLookupOrDeployRole(rootStackArtifact);
const deployedTemplate = await this.readCurrentTemplate(rootStackArtifact, sdk);
await this.replaceNestedStacksInRootStack(rootStackArtifact, deployedTemplate, sdk);
await this.addNestedTemplatesToGeneratedAndDeployedStacks(
rootStackArtifact, rootStackArtifact.template, deployedTemplate, rootStackArtifact.stackName, sdk,
);
return deployedTemplate;
}

Expand Down Expand Up @@ -382,59 +384,42 @@ export class CloudFormationDeployments {
}

private async readCurrentStackTemplate(stackName: string, stackSdk: ISDK) : Promise<Template> {
// if `stackName !== stackArtifact.stackName`, then `stackArtifact` is an ancestor to a nested stack with name `stackName`.
const cfn = stackSdk.cloudFormation();
const stack = await CloudFormationStack.lookup(cfn, stackName);
return stack.template();
}

private async replaceNestedStacksInRootStack(
rootStackArtifact: cxapi.CloudFormationStackArtifact, deployedTemplate: any, sdk: ISDK,
): Promise<void> {
await this.replaceNestedStacksInParentTemplate(
rootStackArtifact, rootStackArtifact.template, deployedTemplate, rootStackArtifact.stackName, sdk,
);
}

private async replaceNestedStacksInParentTemplate(
private async addNestedTemplatesToGeneratedAndDeployedStacks(
rootStackArtifact: cxapi.CloudFormationStackArtifact,
generatedParentTemplate: any,
deployedParentTemplate: any,
parentStackName: string | undefined,
sdk: ISDK,
): Promise<void> {
const listStackResources = parentStackName ? new LazyListStackResources(sdk, parentStackName) : undefined;
for (const [nestedStackLogicalId, nestedStackResource] of Object.entries(generatedParentTemplate.Resources ?? {})) {
if (!this.isCdkManagedNestedStack(nestedStackResource)) {
for (const [nestedStackLogicalId, generatedNestedStackResource] of Object.entries(generatedParentTemplate.Resources ?? {})) {
if (!this.isCdkManagedNestedStack(generatedNestedStackResource)) {
continue;
}

const assetPath = nestedStackResource.Metadata['aws:asset:path'];
const assetPath = generatedNestedStackResource.Metadata['aws:asset:path'];
const nestedStackTemplates = await this.getNestedStackTemplates(
rootStackArtifact, assetPath, nestedStackLogicalId, parentStackName, listStackResources, sdk,
);

//if (!generatedParentTemplate.Resources[nestedStackLogicalId].Properties) {
// generatedParentTemplate.Resources[nestedStackLogicalId].Properties = {};
//}
generatedParentTemplate.Resources[nestedStackLogicalId].Properties.NestedTemplate = nestedStackTemplates.generatedNestedTemplate;
generatedNestedStackResource.Properties.NestedTemplate = nestedStackTemplates.generatedNestedTemplate;

if (!deployedParentTemplate.Resources) {
deployedParentTemplate.Resources = {};
}
if (!deployedParentTemplate.Resources[nestedStackLogicalId]) {
deployedParentTemplate.Resources[nestedStackLogicalId] = {};
deployedParentTemplate.Resources[nestedStackLogicalId].Type = 'AWS::CloudFormation::Stack';
}
if (!deployedParentTemplate.Resources[nestedStackLogicalId].Properties) {
deployedParentTemplate.Resources[nestedStackLogicalId].Properties = {};
}
deployedParentTemplate.Resources[nestedStackLogicalId].Properties.NestedTemplate = nestedStackTemplates.deployedNestedTemplate;
deployedParentTemplate.Resources = deployedParentTemplate.Resources ?? {};
const deployedNestedStackResource = deployedParentTemplate.Resources[nestedStackLogicalId] ?? {};
deployedParentTemplate.Resources[nestedStackLogicalId] = deployedNestedStackResource;
deployedNestedStackResource.Type = 'AWS::CloudFormation::Stack';
deployedNestedStackResource.Properties = deployedNestedStackResource.Properties ?? {};
deployedNestedStackResource.Properties.NestedTemplate = nestedStackTemplates.deployedNestedTemplate;

await this.replaceNestedStacksInParentTemplate(
await this.addNestedTemplatesToGeneratedAndDeployedStacks(
rootStackArtifact,
generatedParentTemplate.Resources[nestedStackLogicalId].Properties.NestedTemplate,
deployedParentTemplate.Resources[nestedStackLogicalId].Properties.NestedTemplate ?? {},
nestedStackTemplates.generatedNestedTemplate,
nestedStackTemplates.deployedNestedTemplate,
nestedStackTemplates.nestedStackName,
sdk,
);
Expand All @@ -446,11 +431,11 @@ export class CloudFormationDeployments {
parentStackName: string | undefined, listStackResources: ListStackResources | undefined, sdk: ISDK,
): Promise<NestedStackTemplates> {
const nestedTemplatePath = path.join(rootStackArtifact.assembly.directory, nestedTemplateAssetPath);
const nestedStackArn = await this.getNestedStackArn(nestedStackLogicalId, parentStackName, listStackResources);

// CFN generates the nested stack name in the form `ParentStackName-NestedStackLogicalID-SomeHashWeCan'tCompute,
// the arn is of the form: arn:aws:cloudformation:region:123456789012:stack/NestedStackName/AnotherHashWeDon'tNeed
// so we get the ARN and manually extract the name.
const nestedStackArn = await this.getNestedStackArn(nestedStackLogicalId, parentStackName, listStackResources);
const nestedStackName = nestedStackArn?.slice(nestedStackArn.indexOf('/') + 1, nestedStackArn.lastIndexOf('/'));

return {
Expand All @@ -467,10 +452,7 @@ export class CloudFormationDeployments {
): Promise<string | undefined> {
try {
const stackResources = await listStackResources?.listStackResources();

if (stackResources) {
return stackResources.find(sr => sr.LogicalResourceId === nestedStackLogicalId)?.PhysicalResourceId;
}
return stackResources?.find(sr => sr.LogicalResourceId === nestedStackLogicalId)?.PhysicalResourceId;
} catch (e) {
if (e.message !== `Stack with id ${parentStackName} does not exist`) {
throw e;
Expand Down Expand Up @@ -567,11 +549,12 @@ function isAssetManifestArtifact(art: cxapi.CloudArtifact): art is cxapi.AssetMa
}

interface NestedStackTemplates {
readonly generatedNestedTemplate: any,
readonly deployedNestedTemplate: any,
readonly nestedStackName: string | undefined,
readonly generatedNestedTemplate: any;
readonly deployedNestedTemplate: any;
readonly nestedStackName: string | undefined;
}

interface NestedStackResource {
readonly Metadata: { 'aws:asset:path': string };
readonly Properties: any;
}
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cdk-toolkit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class CdkToolkit {
// Compare N stacks against deployed templates
for (const stack of stacks.stackArtifacts) {
stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
const currentTemplate = await this.props.cloudFormation.readCurrentTemplateWithNestedStacks(stack);
const currentTemplate = await this.props.cloudFormation.readCurrentTemplateWithNestedStacks(stack); // TODO; renmae
diffs += options.securityOnly
? numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening))
: printStackDiff(currentTemplate, stack, strict, contextLines, stream);
Expand Down
60 changes: 38 additions & 22 deletions packages/aws-cdk/test/api/cloudformation-deployments.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import { CloudFormationDeployments } from '../../lib/api/cloudformation-deployme
import { deployStack } from '../../lib/api/deploy-stack';
import { ToolkitInfo } from '../../lib/api/toolkit-info';
import { CloudFormationStack } from '../../lib/api/util/cloudformation';
import { instanceMockFrom, testStack } from '../util';
import { testStack } from '../util';
import { mockBootstrapStack, MockSdkProvider } from '../util/mock-sdk';
import { FakeCloudformationStack } from './fake-cloudformation-stack';

let sdkProvider: MockSdkProvider;
let deployments: CloudFormationDeployments;
Expand Down Expand Up @@ -163,11 +164,14 @@ test('if toolkit stack cannot be found but SSM parameter name is present deploym
});

test('readCurrentTemplateWithNestedStacks() can handle non-Resources in the template', async () => {
const cfnStack = instanceMockFrom((CloudFormationStack as any));
CloudFormationStack.lookup = jest.fn().mockImplementation((_, stackName: string) => {
const cfnStack = new FakeCloudformationStack({
stackName: 'OutputParamRoot',
stackId: 'StackId',
});
CloudFormationStack.lookup = (async (_, stackName: string) => {
switch (stackName) {
case 'OutputParamRoot':
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
NestedStack: {
Type: 'AWS::CloudFormation::Stack',
Expand All @@ -183,7 +187,7 @@ test('readCurrentTemplateWithNestedStacks() can handle non-Resources in the temp
break;

case 'NestedStack':
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
NestedResource: {
Type: 'AWS::Something',
Expand All @@ -208,7 +212,7 @@ test('readCurrentTemplateWithNestedStacks() can handle non-Resources in the temp
break;

default:
throw new Error('unknown stack name ' + stackName + ' found in cloudformation-deployments.test.ts');
throw new Error('unknown stack name ' + stackName + ' found');
}

return cfnStack;
Expand Down Expand Up @@ -320,11 +324,14 @@ test('readCurrentTemplateWithNestedStacks() can handle non-Resources in the temp
});

test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling structure works', async () => {
const cfnStack = instanceMockFrom((CloudFormationStack as any));
CloudFormationStack.lookup = jest.fn().mockImplementation((_, stackName: string) => {
const cfnStack = new FakeCloudformationStack({
stackName: 'MultiLevelRoot',
stackId: 'StackId',
});
CloudFormationStack.lookup = (async (_, stackName: string) => {
switch (stackName) {
case 'MultiLevelRoot':
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
NestedStack: {
Type: 'AWS::CloudFormation::Stack',
Expand All @@ -340,7 +347,7 @@ test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling stru
break;

case 'NestedStack':
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
SomeResource: {
Type: 'AWS::Something',
Expand Down Expand Up @@ -371,7 +378,7 @@ test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling stru
break;

case 'NestedStackA':
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
SomeResource: {
Type: 'AWS::Something',
Expand All @@ -384,7 +391,7 @@ test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling stru
break;

case 'NestedStackB':
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
SomeResource: {
Type: 'AWS::Something',
Expand Down Expand Up @@ -588,9 +595,12 @@ test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling stru

test('readCurrentTemplateWithNestedStacks() on an undeployed parent stack with an (also undeployed) nested stack works', async () => {
// GIVEN
const cfnStack = instanceMockFrom((CloudFormationStack as any));
CloudFormationStack.lookup = jest.fn().mockImplementation((_cfn: CloudFormation, _stackName: string) => {
(cfnStack as any).template = jest.fn().mockReturnValue({ });
const cfnStack = new FakeCloudformationStack({
stackName: 'UndeployedParent',
stackId: 'StackId',
});
CloudFormationStack.lookup = (async (_cfn, _stackName: string) => {
cfnStack.template = async () => ({});

return cfnStack;
});
Expand Down Expand Up @@ -638,9 +648,12 @@ test('readCurrentTemplateWithNestedStacks() on an undeployed parent stack with a

test('readCurrentTemplateWithNestedStacks() caches calls to listStackResources()', async () => {
// GIVEN
const cfnStack = instanceMockFrom((CloudFormationStack as any));
CloudFormationStack.lookup = jest.fn().mockImplementation((_cfn: CloudFormation, _stackName: string) => {
(cfnStack as any).template = jest.fn().mockReturnValue({
const cfnStack = new FakeCloudformationStack({
stackName: 'CachingRoot',
stackId: 'StackId',
});
CloudFormationStack.lookup = (async (_cfn, _stackName: string) => {
cfnStack.template = async () => ({
Resources:
{
NestedStackA: {
Expand Down Expand Up @@ -711,10 +724,13 @@ test('readCurrentTemplateWithNestedStacks() caches calls to listStackResources()

test('readCurrentTemplateWithNestedStacks() succesfully ignores stacks without metadata', async () => {
// GIVEN
const cfnStack = instanceMockFrom((CloudFormationStack as any));
CloudFormationStack.lookup = jest.fn().mockImplementation((_cfn: CloudFormation, stackName: string) => {
const cfnStack = new FakeCloudformationStack({
stackName: 'MetadataRoot',
stackId: 'StackId',
});
CloudFormationStack.lookup = (async (_, stackName: string) => {
if (stackName === 'MetadataRoot') {
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
WithMetadata: {
Type: 'AWS::CloudFormation::Stack',
Expand All @@ -729,7 +745,7 @@ test('readCurrentTemplateWithNestedStacks() succesfully ignores stacks without m
});

} else {
(cfnStack as any).template = jest.fn().mockReturnValue({
cfnStack.template = async () => ({
Resources: {
SomeResource: {
Type: 'AWS::Something',
Expand Down
38 changes: 19 additions & 19 deletions packages/aws-cdk/test/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,25 +49,6 @@ function clone(obj: any) {
return JSON.parse(JSON.stringify(obj));
}

function addNestedStacks(templatePath: string, outdir: string, rootStackTemplate?: any) {
let template = rootStackTemplate;

if (!template) {
const templatePathWithDir = path.join('diff-nested-stacks-templates', templatePath);
template = JSON.parse(fs.readFileSync(path.join(__dirname, templatePathWithDir)).toString());
fs.writeFileSync(path.join(outdir, templatePath), JSON.stringify(template, undefined, 2));
}

for (const logicalId in template.Resources) {
if (template.Resources[logicalId].Type === 'AWS::CloudFormation::Stack') {
if (template.Resources[logicalId].Metadata && template.Resources[logicalId].Metadata['aws:asset:path']) {
const nestedTemplatePath = template.Resources[logicalId].Metadata['aws:asset:path'];
addNestedStacks(nestedTemplatePath, outdir);
}
}
}
}

function addAttributes(assembly: TestAssembly, builder: cxapi.CloudAssemblyBuilder) {
for (const stack of assembly.stacks) {
const templateFile = `${stack.stackName}.template.json`;
Expand Down Expand Up @@ -104,6 +85,25 @@ function addAttributes(assembly: TestAssembly, builder: cxapi.CloudAssemblyBuild
}
}

function addNestedStacks(templatePath: string, outdir: string, rootStackTemplate?: any) {
let template = rootStackTemplate;

if (!template) {
const templatePathWithDir = path.join('diff-nested-stacks-templates', templatePath);
template = JSON.parse(fs.readFileSync(path.join(__dirname, templatePathWithDir)).toString());
fs.writeFileSync(path.join(outdir, templatePath), JSON.stringify(template, undefined, 2));
}

for (const logicalId in template.Resources) {
if (template.Resources[logicalId].Type === 'AWS::CloudFormation::Stack') {
if (template.Resources[logicalId].Metadata && template.Resources[logicalId].Metadata['aws:asset:path']) {
const nestedTemplatePath = template.Resources[logicalId].Metadata['aws:asset:path'];
addNestedStacks(nestedTemplatePath, outdir);
}
}
}
}

export function testAssembly(assembly: TestAssembly): cxapi.CloudAssembly {
const builder = new cxapi.CloudAssemblyBuilder();
addAttributes(assembly, builder);
Expand Down

0 comments on commit 9b78354

Please sign in to comment.