Skip to content

Commit

Permalink
fix(core): CFN version and description template sections were merged …
Browse files Browse the repository at this point in the history
…incorrectly (#8251)

In the merge logic in Stack when rendering the template,
it was mistakenly assumed that all CFN sections are objects.
However, there are some sections, like Description and AWSTemplateFormatVersion,
that are in fact strings.
Add special logic for those cases in the merge functionality
(multiple provided CFN versions are checked for being identical,
and mutliple descriptions are merged together, with a newline in between).

Fixes #8151
  • Loading branch information
skinny85 authored Jun 2, 2020
1 parent 103c144 commit b7e328d
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 11 deletions.
58 changes: 47 additions & 11 deletions packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -839,24 +839,60 @@ export class Stack extends Construct implements ITaggable {
}
}

function merge(template: any, part: any) {
for (const section of Object.keys(part)) {
const src = part[section];
function merge(template: any, fragment: any): void {
for (const section of Object.keys(fragment)) {
const src = fragment[section];

// create top-level section if it doesn't exist
let dest = template[section];
const dest = template[section];
if (!dest) {
template[section] = dest = src;
template[section] = src;
} else {
// add all entities from source section to destination section
for (const id of Object.keys(src)) {
if (id in dest) {
throw new Error(`section '${section}' already contains '${id}'`);
}
dest[id] = src[id];
template[section] = mergeSection(section, dest, src);
}
}
}

function mergeSection(section: string, val1: any, val2: any): any {
switch (section) {
case 'Description':
return `${val1}\n${val2}`;
case 'AWSTemplateFormatVersion':
if (val1 != null && val2 != null && val1 !== val2) {
throw new Error(`Conflicting CloudFormation template versions provided: '${val1}' and '${val2}`);
}
return val1 ?? val2;
case 'Resources':
case 'Conditions':
case 'Parameters':
case 'Outputs':
case 'Mappings':
case 'Metadata':
case 'Transform':
return mergeObjectsWithoutDuplicates(section, val1, val2);
default:
throw new Error(`CDK doesn't know how to merge two instances of the CFN template section '${section}' - ` +
'please remove one of them from your code');
}
}

function mergeObjectsWithoutDuplicates(section: string, dest: any, src: any): any {
if (typeof dest !== 'object') {
throw new Error(`Expecting ${JSON.stringify(dest)} to be an object`);
}
if (typeof src !== 'object') {
throw new Error(`Expecting ${JSON.stringify(src)} to be an object`);
}

// add all entities from source section to destination section
for (const id of Object.keys(src)) {
if (id in dest) {
throw new Error(`section '${section}' already contains '${id}'`);
}
dest[id] = src[id];
}

return dest;
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/core/test/test.include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,30 @@ export = {
test.throws(() => toCloudFormation(stack));
test.done();
},

'correctly merges template sections that contain strings'(test: Test) {
const stack = new Stack();

new CfnInclude(stack, 'T1', {
template: {
AWSTemplateFormatVersion: '2010-09-09',
Description: 'Test 1',
},
});
new CfnInclude(stack, 'T2', {
template: {
AWSTemplateFormatVersion: '2010-09-09',
Description: 'Test 2',
},
});

test.deepEqual(toCloudFormation(stack), {
AWSTemplateFormatVersion: '2010-09-09',
Description: 'Test 1\nTest 2',
});

test.done();
},
};

const template = {
Expand Down

0 comments on commit b7e328d

Please sign in to comment.