From 7e1849453af0b04836fb24682363bcfedac4f9d1 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Mon, 14 Jun 2021 14:12:55 -0700 Subject: [PATCH] fix(cfn-include): NestedStack's Parameters are not converted to strings (#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 #15092 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cloudformation-include/lib/cfn-include.ts | 16 +++-- .../test/nested-stacks.test.ts | 63 ++++++++++++++++--- .../nested/child-with-number-parameter.yaml | 9 +++ .../nested/parent-number-in-child-params.yaml | 8 +++ .../nested/parent-with-attributes.json | 15 ++++- 5 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/nested/child-with-number-parameter.yaml create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-number-in-child-params.yaml diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 5a54d36aefcc8..957cb68594128 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -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 }; @@ -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; } @@ -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); diff --git a/packages/@aws-cdk/cloudformation-include/test/nested-stacks.test.ts b/packages/@aws-cdk/cloudformation-include/test/nested-stacks.test.ts index 1a0d9763280ca..2a323657f089e 100644 --- a/packages/@aws-cdk/cloudformation-include/test/nested-stacks.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/nested-stacks.test.ts @@ -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'), @@ -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'), - }, }, }); @@ -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', () => { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/child-with-number-parameter.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/child-with-number-parameter.yaml new file mode 100644 index 0000000000000..0101bc0c91baa --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/child-with-number-parameter.yaml @@ -0,0 +1,9 @@ +AWSTemplateFormatVersion: '2010-09-09' +Parameters: + Number: + Type: Number +Resources: + S3Bucket: + Type: AWS::S3::Bucket + Properties: + BucketName: 'testbucket1234unique' diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-number-in-child-params.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-number-in-child-params.yaml new file mode 100644 index 0000000000000..10ad404e4dc44 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-number-in-child-params.yaml @@ -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 diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-with-attributes.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-with-attributes.json index c12bd223063c1..8307976b35ba1 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-with-attributes.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/nested/parent-with-attributes.json @@ -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" @@ -23,4 +34,4 @@ } } } -} \ No newline at end of file +}