Skip to content

Commit

Permalink
fix(cfn-include): NestedStack's Parameters are not converted to strin…
Browse files Browse the repository at this point in the history
…gs (aws#15098)

When we include a NestedStack when creating a CfnInclude instance,
the conversion of the Parameters property of the AWS::CloudFormation::Stack resource is performed manually
(because of the eventuality that one of those Parameters was also requested to be removed when including the nested stack).
In that manual conversion, we did not correctly convert the values to strings,
which is what the underlying CfnStack class expects.
And so, if the parent stack passed a non-string primitive value to a nested stack Parameter,
like a number or boolean, including the nested stack would fail with a validation exception.

Fixes aws#15092

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 authored and hollanddd committed Aug 26, 2021
1 parent 6c2e331 commit 7e18494
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 13 deletions.
16 changes: 12 additions & 4 deletions packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,8 @@ export class CfnInclude extends core.CfnElement {
const nestedStackProps = cfnParser.parseValue(nestedStackAttributes.Properties);
const nestedStack = new core.NestedStack(this, nestedStackId, {
parameters: this.parametersForNestedStack(nestedStackProps.Parameters, nestedStackId),
notificationArns: nestedStackProps.NotificationArns,
timeout: nestedStackProps.Timeout,
notificationArns: cfn_parse.FromCloudFormation.getStringArray(nestedStackProps.NotificationARNs).value,
timeout: this.timeoutForNestedStack(nestedStackProps.TimeoutInMinutes),
});
const template = new CfnInclude(nestedStack, nestedStackId, this.nestedStacksToInclude[nestedStackId]);
this.nestedStacks[nestedStackId] = { stack: nestedStack, includedTemplate: template };
Expand All @@ -694,7 +694,7 @@ export class CfnInclude extends core.CfnElement {
return nestedStackResource;
}

private parametersForNestedStack(parameters: any, nestedStackId: string): { [key: string]: any } | undefined {
private parametersForNestedStack(parameters: any, nestedStackId: string): { [key: string]: string } | undefined {
if (parameters == null) {
return undefined;
}
Expand All @@ -703,12 +703,20 @@ export class CfnInclude extends core.CfnElement {
const ret: { [key: string]: string } = {};
for (const paramName of Object.keys(parameters)) {
if (!(paramName in parametersToReplace)) {
ret[paramName] = parameters[paramName];
ret[paramName] = cfn_parse.FromCloudFormation.getString(parameters[paramName]).value;
}
}
return ret;
}

private timeoutForNestedStack(value: any): core.Duration | undefined {
if (value == null) {
return undefined;
}

return core.Duration.minutes(cfn_parse.FromCloudFormation.getNumber(value).value);
}

private overrideLogicalIdIfNeeded(element: core.CfnElement, id: string): void {
if (this.preserveLogicalIds) {
element.overrideLogicalId(id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,28 @@ describe('CDK Include for nested stacks', () => {
}).toThrow(/Nested Stack 'AnotherChildStack' was not included in the parent template/);
});

test('correctly handles references in nested stacks Parameters', () => {
new inc.CfnInclude(stack, 'ParentStack', {
templateFile: testTemplateFilePath('cross-stack-refs.json'),
loadNestedStacks: {
'ChildStack': {
templateFile: testTemplateFilePath('child-import-stack.json'),
},
},
});

expect(stack).toHaveResourceLike('AWS::CloudFormation::Stack', {
"Parameters": {
"Param1": {
"Ref": "Param",
},
"Param2": {
"Fn::GetAtt": ["Bucket", "Arn"],
},
},
});
});

test('correctly handles renaming of references across nested stacks', () => {
const parentTemplate = new inc.CfnInclude(stack, 'ParentStack', {
templateFile: testTemplateFilePath('cross-stack-refs.json'),
Expand Down Expand Up @@ -386,15 +408,12 @@ describe('CDK Include for nested stacks', () => {
});

test("handles Metadata, DeletionPolicy, and UpdateReplacePolicy attributes of the nested stack's resource", () => {
const cfnTemplate = new inc.CfnInclude(stack, 'ParentStack', {
new inc.CfnInclude(stack, 'ParentStack', {
templateFile: testTemplateFilePath('parent-with-attributes.json'),
loadNestedStacks: {
'ChildStack': {
templateFile: testTemplateFilePath('child-import-stack.json'),
},
'AnotherChildStack': {
templateFile: testTemplateFilePath('child-import-stack.json'),
},
},
});

Expand All @@ -408,20 +427,50 @@ describe('CDK Include for nested stacks', () => {
],
"UpdateReplacePolicy": "Retain",
}, ResourcePart.CompleteDefinition);

cfnTemplate.getNestedStack('AnotherChildStack');
});

test('correctly parses NotificationsARNs, Timeout', () => {
new inc.CfnInclude(stack, 'ParentStack', {
templateFile: testTemplateFilePath('parent-with-attributes.json'),
loadNestedStacks: {
'ChildStack': {
templateFile: testTemplateFilePath('custom-resource.json'),
},
'AnotherChildStack': {
templateFile: testTemplateFilePath('custom-resource.json'),
},
},
});

expect(stack).toHaveResourceLike('AWS::CloudFormation::Stack', {
"TemplateURL": "https://cfn-templates-set.s3.amazonaws.com/child-import-stack.json",
"NotificationARNs": ["arn1"],
"TimeoutInMinutes": 5,
});
expect(stack).toHaveResourceLike('AWS::CloudFormation::Stack', {
"NotificationARNs": { "Ref": "ArrayParam" },
"TimeoutInMinutes": {
"Fn::Select": [0, {
"Ref": "ArrayParam",
}],
},
});
});

test('can ingest a NestedStack with a Number CFN Parameter passed as a number', () => {
new inc.CfnInclude(stack, 'MyScope', {
templateFile: testTemplateFilePath('parent-number-in-child-params.yaml'),
loadNestedStacks: {
'NestedStack': {
templateFile: testTemplateFilePath('child-with-number-parameter.yaml'),
},
},
});

expect(stack).toHaveResourceLike('AWS::CloudFormation::Stack', {
"Parameters": {
"Number": "60",
},
});
});

test('can lazily include a single child nested stack', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
AWSTemplateFormatVersion: '2010-09-09'
Parameters:
Number:
Type: Number
Resources:
S3Bucket:
Type: AWS::S3::Bucket
Properties:
BucketName: 'testbucket1234unique'
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
AWSTemplateFormatVersion: '2010-09-09'
Resources:
NestedStack:
Type: AWS::CloudFormation::Stack
Properties:
TemplateURL: 'https://s3.amazonaws.com/masonme-cdk-test/templates/nested-bucket.yaml'
Parameters:
Number: 60
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
{
"Parameters": {
"ArrayParam": {
"Type": "CommaDelimitedList"
}
},
"Resources": {
"ChildStack": {
"Type": "AWS::CloudFormation::Stack",
"Properties": {
"TemplateURL": "https://cfn-templates-set.s3.amazonaws.com/child-import-stack.json"
"TemplateURL": "https://cfn-templates-set.s3.amazonaws.com/child-import-stack.json",
"NotificationARNs": { "Ref": "ArrayParam" },
"TimeoutInMinutes": {
"Fn::Select": [0, {
"Ref": "ArrayParam"
}]
}
},
"DependsOn": [
"AnotherChildStack"
Expand All @@ -23,4 +34,4 @@
}
}
}
}
}

0 comments on commit 7e18494

Please sign in to comment.