From db60c27a772d485cc1a9e2099b201700c983da3a Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 4 Jun 2020 17:44:41 -0400 Subject: [PATCH 01/46] doc: added slack link to readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index ba7a422bb528f..286f7a917ec73 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,7 @@ for tracking bugs and feature requests. * Ask a question on [Stack Overflow](https://stackoverflow.com/questions/tagged/aws-cdk) and tag it with `aws-cdk` * Come join the AWS CDK community on [Gitter](https://gitter.im/awslabs/aws-cdk) +* You'll need an invite to join the [Slack](https://awsdevelopers.slack.com) community * Open a support ticket with [AWS Support](https://console.aws.amazon.com/support/home#/) * If it turns out that you may have found a bug, please open an [issue](https://github.com/aws/aws-cdk/issues/new) From f0c85a36146bdf813638f926f61184661b3683f4 Mon Sep 17 00:00:00 2001 From: comcalvi <66279577+comcalvi@users.noreply.github.com> Date: Fri, 5 Jun 2020 12:49:05 -0400 Subject: [PATCH 02/46] Update README.md Co-authored-by: Adam Ruka --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 286f7a917ec73..9b51cbaba4118 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ for tracking bugs and feature requests. * Ask a question on [Stack Overflow](https://stackoverflow.com/questions/tagged/aws-cdk) and tag it with `aws-cdk` * Come join the AWS CDK community on [Gitter](https://gitter.im/awslabs/aws-cdk) -* You'll need an invite to join the [Slack](https://awsdevelopers.slack.com) community +* Talk in the CDK channel of the [AWS Developers Slack workspace](https://awsdevelopers.slack.com) (invite required) * Open a support ticket with [AWS Support](https://console.aws.amazon.com/support/home#/) * If it turns out that you may have found a bug, please open an [issue](https://github.com/aws/aws-cdk/issues/new) From c6664af7ca1ab35c62782527c5d2a7b8137de5ec Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 15 Jun 2020 19:08:00 -0400 Subject: [PATCH 03/46] added support for Fn::Select, Fn::FindInMap, Fn::Cidr, Fn::GetAZs, Fn::And, Fn::Not, and Fn::Or, including unit tests. --- .../test/test-templates/conditions.json | 48 ++++++++++++ .../test/test-templates/functions.json | 75 +++++++++++++++++++ .../test/valid-templates.test.ts | 16 ++++ packages/@aws-cdk/core/lib/cfn-parse.ts | 31 ++++++++ 4 files changed, 170 insertions(+) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json new file mode 100644 index 0000000000000..5b23794ea3a14 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json @@ -0,0 +1,48 @@ +{ + "Conditions": { + "AndCond": { + "Fn::And": [ + {"Condition": "AlwaysTrueCond"}, + {"Condition": "AlwaysFalseCond"} + ] + }, + "OrCond": { + "Fn::Or": [ + {"Condition": "AlwaysTrueCond"}, + {"Condition": "AlwaysFalseCond"} + ] + }, + "AlwaysTrueCond": { + "Fn::Not": [{ + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + }] + }, + "AlwaysFalseCond": { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + } + }, + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::If": [ + "AlwaysTrueCond", + "Name1", + "Name2" + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json new file mode 100644 index 0000000000000..6360df54104fd --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json @@ -0,0 +1,75 @@ +{ + "Mappings" : { + "RegionMap" : { + "region-1" : { + "HVM64" : "name1", "HVMG2" : "name2" + } + } + }, + "Resources": { + + "Vpc": { + "Type" : "AWS::EC2::VPC", + "Properties" : { + "CidrBlock" : { + "Fn::Cidr": [ "10.0.0.0/24", "6", "5"] + } + } + }, + "2ArgVpc": { + "Type" : "AWS::EC2::VPC", + "Properties" : { + "CidrBlock" : { + "Fn::Cidr": [ "10.0.0.0/24", "6"] + } + } + }, + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::FindInMap": [ + "RegionMap", + "region-1", + "HVM64" + ] + } + } + }, + "subnet1" : { + "Type" : "AWS::EC2::Subnet", + "Properties" : { + "VpcId" : { + "Ref" : "VPC" + }, + "CidrBlock" : "10.0.0.0/24", + "AvailabilityZone" : { + "Fn::Select" : [ + "0", + { + "Fn::GetAZs" : "" + } + ] + } + } + }, + + "subnet2" : { + "Type" : "AWS::EC2::Subnet", + "Properties" : { + "VpcId" : { + "Fn::Select" : ["3", ["foo", "aValidVPCID", "foo2"]] + }, + "CidrBlock" : "10.0.0.0/24", + "AvailabilityZone" : { + "Fn::Select" : [ + "0", + { + "Fn::GetAZs" : "eu-west-2" + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 0291de98eba95..0b9042c77ae36 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -184,6 +184,22 @@ describe('CDK Include', () => { ); }); + test('can ingest a template with many conditions, and output it unchanged', () => { + includeTestTemplate(stack, 'conditions.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('conditions.json'), + ); + }); + + test('can ingest a template with multiple cloudformation intrinsic functions, and output it unchanged', () => { + includeTestTemplate(stack, 'functions.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('functions.json'), + ); + }); + test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 67e2d2390ef62..f8fcfd15c49ed 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -179,6 +179,25 @@ function parseIfCfnIntrinsic(object: any): any { const value = parseCfnValueToCdkValue(object[key]); return Fn.join(value[0], value[1]); } + case 'Fn::Cidr': { + const value = parseCfnValueToCdkValue(object[key]); + + if (value.length == 2) + return Fn.cidr(value[0], value[1]); + return Fn.cidr(value[0], value[1], value[2]); + } + case 'Fn::FindInMap': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.findInMap(value[0], value[1], value[2]); + } + case 'Fn::Select': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.select(value[0], value[1]); + } + case 'Fn::GetAZs': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.getAzs(value); + } case 'Fn::If': { // Fn::If takes a 3-element list as its argument // ToDo the first argument is the name of the condition, @@ -191,6 +210,18 @@ function parseIfCfnIntrinsic(object: any): any { const value = parseCfnValueToCdkValue(object[key]); return Fn.conditionEquals(value[0], value[1]); } + case 'Fn::And': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.conditionAnd(...value); + } + case 'Fn::Not': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.conditionNot(value[0]); + } + case 'Fn::Or': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.conditionOr(...value); + } default: throw new Error(`Unsupported CloudFormation function '${key}'`); } From d0f64d2cdedf75e440c31948164fee1257b3302f Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 16 Jun 2020 14:11:24 -0400 Subject: [PATCH 04/46] added support for the 'Fn::Transform' cloudformation intrinsic function. --- .../test/test-templates/functions.json | 18 ++++++++++++++- packages/@aws-cdk/core/lib/cfn-fn.ts | 22 +++++++++++++++++++ packages/@aws-cdk/core/lib/cfn-parse.ts | 12 ++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json index 6360df54104fd..9ee7fed30f3e1 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json @@ -40,7 +40,10 @@ "Type" : "AWS::EC2::Subnet", "Properties" : { "VpcId" : { - "Ref" : "VPC" + "Fn::Split": [ + ",", + { "Fn::ImportValue" : "ImportedVpcId" } + ] }, "CidrBlock" : "10.0.0.0/24", "AvailabilityZone" : { @@ -70,6 +73,19 @@ ] } } + }, + "transformBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::Transform": { + "Name": "AWS::Include", + "Parameters": { + "Location": "location" + } + } + } + } } } } \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/cfn-fn.ts b/packages/@aws-cdk/core/lib/cfn-fn.ts index 30fb6cf435bec..f6e5d02923fa3 100644 --- a/packages/@aws-cdk/core/lib/cfn-fn.ts +++ b/packages/@aws-cdk/core/lib/cfn-fn.ts @@ -172,6 +172,15 @@ export class Fn { return new FnFindInMap(mapName, topLevelKey, secondLevelKey).toString(); } + /** + * + * @param macroNameParameters The name of the macro to perform the processing, and the Parameters to pass to it + * @returns a token represented as a string + */ + public static transform(macroNameParameters: string): string { + return new FnTransform(macroNameParameters).toString(); + } + /** * Returns true if all the specified conditions evaluate to true, or returns * false if any one of the conditions evaluates to false. ``Fn::And`` acts as @@ -356,6 +365,19 @@ class FnFindInMap extends FnBase { } } +/** + * The intrinsic function ``Fn::Transform`` specifies a macro to perform custom processing on part of a stack template. + */ +class FnTransform extends FnBase { + /** + * + * @param macroNameParameters The name of the macro to be invoked, and the parameters to pass to it + */ + constructor(macroNameParameters: string) { + super('Fn::Transform', macroNameParameters); + } +} + /** * The ``Fn::GetAtt`` intrinsic function returns the value of an attribute from a resource in the template. */ diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index f8fcfd15c49ed..78249ec1d5f8a 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -198,6 +198,18 @@ function parseIfCfnIntrinsic(object: any): any { const value = parseCfnValueToCdkValue(object[key]); return Fn.getAzs(value); } + case 'Fn::ImportValue': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.importValue(value); + } + case 'Fn::Split': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.split(value[0], value[1]); + } + case 'Fn::Transform': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.transform(value); + } case 'Fn::If': { // Fn::If takes a 3-element list as its argument // ToDo the first argument is the name of the condition, From d69e46dce96360149a7f06be64894912e63a8d10 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 16 Jun 2020 14:24:45 -0400 Subject: [PATCH 05/46] added support for the Fn::Base64 Intrinsic Function --- .../test/test-templates/functions.json | 5 ++++- .../only-codecommit-repo-using-cfn-functions.json | 2 +- .../cloudformation-include/test/valid-templates.test.ts | 2 +- packages/@aws-cdk/core/lib/cfn-parse.ts | 4 ++++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json index 9ee7fed30f3e1..fa57eab81ed29 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json @@ -81,7 +81,10 @@ "Fn::Transform": { "Name": "AWS::Include", "Parameters": { - "Location": "location" + "Location": "location", + "AnotherParameter": { + "Fn:Base64": "AnotherValue" + } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/only-codecommit-repo-using-cfn-functions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/only-codecommit-repo-using-cfn-functions.json index a1037dbf2402a..10e907c0ea753 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/only-codecommit-repo-using-cfn-functions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/only-codecommit-repo-using-cfn-functions.json @@ -5,7 +5,7 @@ "Properties": { "RepositoryName": "my-repository", "RepositoryDescription": { - "Fn::Base64": "my description, in base-64!" + "Fn::DoesNotExist": "my description, in base-64!" } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 0b9042c77ae36..16b45ec30f03e 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -289,7 +289,7 @@ describe('CDK Include', () => { test("throws an exception when encountering a CFN function it doesn't support", () => { expect(() => { includeTestTemplate(stack, 'only-codecommit-repo-using-cfn-functions.json'); - }).toThrow(/Unsupported CloudFormation function 'Fn::Base64'/); + }).toThrow(/Unsupported CloudFormation function 'Fn::DoesNotExist'/); }); test('throws an exception when encountering the CreationPolicy attribute in a resource', () => { diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 78249ec1d5f8a..816902b97b3f4 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -210,6 +210,10 @@ function parseIfCfnIntrinsic(object: any): any { const value = parseCfnValueToCdkValue(object[key]); return Fn.transform(value); } + case 'Fn::Base64': { + const value = parseCfnValueToCdkValue(object[key]); + return Fn.base64(value); + } case 'Fn::If': { // Fn::If takes a 3-element list as its argument // ToDo the first argument is the name of the condition, From df1446a8db9e404341ddd8b584b2d68bbe299a43 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 16 Jun 2020 16:11:19 -0400 Subject: [PATCH 06/46] tested more complex combinations of conditional and non-conditional intrinsic functions --- .../functions-and-conditions.json | 104 ++++++++++++++++++ .../test/valid-templates.test.ts | 9 ++ 2 files changed, 113 insertions(+) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json new file mode 100644 index 0000000000000..17ccd663564ca --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json @@ -0,0 +1,104 @@ +{ + "Mappings" : { + "RegionMap" : { + "region-1" : { + "HVM64" : "name1", "HVMG2" : "name2" + } + } + }, + "Conditions": { + "AlwaysTrueCond": { + "Fn::Not": [{ + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + }] + }, + "AlwaysFalseCond": { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + } + }, + "Resources": { + "Vpc": { + "Type" : "AWS::EC2::VPC", + "Properties" : { + "CidrBlock" : { + "Fn::If": [ + "AlwaysTrueCond", + { "Fn::Cidr": [ "192.168.1.1/24", "2"] }, + { "Fn::Cidr": [ "10.0.0.0/24", "6", "5"] } + ] + } + } + }, + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::If": [ + {"Fn::Or": ["AlwaysFalseCond", "AlwaysFalseCond"]}, + "Unreachable", + {"Fn::FindInMap": [ + "RegionMap", + "region-1", + "HVM64" + ]} + ] + } + } + }, + "subnet1" : { + "Type" : "AWS::EC2::Subnet", + "Properties" : { + "VpcId" : { + "Fn::If": [ + {"Fn::And": ["AlwaysTrueCond", "AlwaysTrueCond"]}, + { + "Fn::Split": [ + ",", + { "Fn::ImportValue" : "ImportedVpcId" } + ]}, + "Unreachable" + ] + }, + "CidrBlock" : "10.0.0.0/24", + "AvailabilityZone" : { + "Fn::Select" : [ + "0", + { + "Fn::GetAZs" : "" + } + ] + } + } + }, + "transformBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::If": [ + { "Fn::Or": ["AlwaysFalseCond", "AlwaysTrueCond"]}, { + "Fn::Transform": { + "Name": "AWS::Include", + "Parameters": { + "Location": "location", + "AnotherParameter": { + "Fn:Base64": "AnotherValue" + } + } + } + } + ] + } + } + } + } + } \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 16b45ec30f03e..738d3c1187d2f 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -200,6 +200,15 @@ describe('CDK Include', () => { ); }); + test('can ingest a template with intrinsic functions and conditions, and output it unchanged', () => { + includeTestTemplate(stack, 'functions-and-conditions.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('functions-and-conditions.json'), + ); + }); + + test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); From 132cb4d87a39ab64b9438107b8ffabef2f8156fe Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 16 Jun 2020 18:03:01 -0400 Subject: [PATCH 07/46] fixed linter issues --- packages/@aws-cdk/core/lib/cfn-fn.ts | 7 ++++--- packages/@aws-cdk/core/lib/cfn-parse.ts | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/core/lib/cfn-fn.ts b/packages/@aws-cdk/core/lib/cfn-fn.ts index f6e5d02923fa3..88c27bb538e29 100644 --- a/packages/@aws-cdk/core/lib/cfn-fn.ts +++ b/packages/@aws-cdk/core/lib/cfn-fn.ts @@ -173,8 +173,10 @@ export class Fn { } /** - * - * @param macroNameParameters The name of the macro to perform the processing, and the Parameters to pass to it + * The intrinsic function Fn::Transform specifies a macro to perform custom processing on part of a stack template. + * Macros enable you to perform custom processing on templates, from simple actions like find-and-replace operations + * to extensive transformations of entire templates. + * @param macroNameParameters The name of the macro to perform the processing, and the Parameters to pass to it. * @returns a token represented as a string */ public static transform(macroNameParameters: string): string { @@ -370,7 +372,6 @@ class FnFindInMap extends FnBase { */ class FnTransform extends FnBase { /** - * * @param macroNameParameters The name of the macro to be invoked, and the parameters to pass to it */ constructor(macroNameParameters: string) { diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 816902b97b3f4..d386c5f512e55 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -181,9 +181,10 @@ function parseIfCfnIntrinsic(object: any): any { } case 'Fn::Cidr': { const value = parseCfnValueToCdkValue(object[key]); - - if (value.length == 2) + + if (value.length === 2) { return Fn.cidr(value[0], value[1]); + } return Fn.cidr(value[0], value[1], value[2]); } case 'Fn::FindInMap': { From ae7c5707c74526cb7f67b71f479ed34f029446ca Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 16 Jun 2020 20:36:53 -0400 Subject: [PATCH 08/46] implmented Adam's requests and fixed additional linter issues --- .../test/invalid-templates.test.ts | 6 ++ .../test/test-templates/conditions.json | 48 ---------- .../functions-and-conditions.json | 52 ++++++---- .../test/test-templates/functions.json | 94 ------------------- ...y-codecommit-repo-using-cfn-functions.json | 0 .../test/valid-templates.test.ts | 23 ----- packages/@aws-cdk/core/lib/cfn-fn.ts | 15 +-- packages/@aws-cdk/core/lib/cfn-parse.ts | 2 +- 8 files changed, 51 insertions(+), 189 deletions(-) delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json rename packages/@aws-cdk/cloudformation-include/test/test-templates/{ => invalid}/only-codecommit-repo-using-cfn-functions.json (100%) diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index 038ea1e9e6dde..9fb6f91131329 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -52,6 +52,12 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'non-existent-condition.json'); }).toThrow(/Resource 'Bucket' uses Condition 'AlwaysFalseCond' that doesn't exist/); }); + + test("throws an exception when encountering a CFN function it doesn't support", () => { + expect(() => { + includeTestTemplate(stack, 'only-codecommit-repo-using-cfn-functions.json'); + }).toThrow(/Unsupported CloudFormation function 'Fn::DoesNotExist'/); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json deleted file mode 100644 index 5b23794ea3a14..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/conditions.json +++ /dev/null @@ -1,48 +0,0 @@ -{ - "Conditions": { - "AndCond": { - "Fn::And": [ - {"Condition": "AlwaysTrueCond"}, - {"Condition": "AlwaysFalseCond"} - ] - }, - "OrCond": { - "Fn::Or": [ - {"Condition": "AlwaysTrueCond"}, - {"Condition": "AlwaysFalseCond"} - ] - }, - "AlwaysTrueCond": { - "Fn::Not": [{ - "Fn::Equals": [ - { - "Ref": "AWS::Region" - }, - "completely-made-up-region" - ] - }] - }, - "AlwaysFalseCond": { - "Fn::Equals": [ - { - "Ref": "AWS::Region" - }, - "completely-made-up-region" - ] - } - }, - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::If": [ - "AlwaysTrueCond", - "Name1", - "Name2" - ] - } - } - } - } -} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json index 17ccd663564ca..a9031ab3ae58c 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json @@ -9,21 +9,22 @@ "Conditions": { "AlwaysTrueCond": { "Fn::Not": [{ - "Fn::Equals": [ - { - "Ref": "AWS::Region" - }, - "completely-made-up-region" - ] - }] + "Fn::Equals": [{"Ref": "AWS::Region"}, "completely-made-up-region"]}] }, "AlwaysFalseCond": { - "Fn::Equals": [ - { - "Ref": "AWS::Region" - }, - "completely-made-up-region" - ] + "Fn::Equals": [{ "Ref": "AWS::Region" }, "completely-made-up-region"] + }, + "AndCond": { + "Fn::And": [ + {"Condition": "AlwaysTrueCond"}, + {"Condition": "AlwaysFalseCond"} + ] + }, + "OrCond": { + "Fn::Or": [ + {"Condition": "AlwaysTrueCond"}, + {"Condition": "AlwaysFalseCond"} + ] } }, "Resources": { @@ -33,8 +34,8 @@ "CidrBlock" : { "Fn::If": [ "AlwaysTrueCond", - { "Fn::Cidr": [ "192.168.1.1/24", "2"] }, - { "Fn::Cidr": [ "10.0.0.0/24", "6", "5"] } + { "Fn::Cidr": [ "192.168.1.1/24", 2] }, + { "Fn::Cidr": [ "10.0.0.0/24", 6, 5] } ] } } @@ -55,7 +56,7 @@ } } }, - "subnet1" : { + "Subnet1" : { "Type" : "AWS::EC2::Subnet", "Properties" : { "VpcId" : { @@ -80,7 +81,24 @@ } } }, - "transformBucket": { + "Subnet2" : { + "Type" : "AWS::EC2::Subnet", + "Properties" : { + "VpcId" : { + "Fn::Select" : ["3", ["foo", "aValidVPCID", "foo2"]] + }, + "CidrBlock" : "10.0.0.0/24", + "AvailabilityZone" : { + "Fn::Select" : [ + "0", + { + "Fn::GetAZs" : "eu-west-2" + } + ] + } + } + }, + "TransformBucket": { "Type": "AWS::S3::Bucket", "Properties": { "BucketName": { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json deleted file mode 100644 index fa57eab81ed29..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions.json +++ /dev/null @@ -1,94 +0,0 @@ -{ - "Mappings" : { - "RegionMap" : { - "region-1" : { - "HVM64" : "name1", "HVMG2" : "name2" - } - } - }, - "Resources": { - - "Vpc": { - "Type" : "AWS::EC2::VPC", - "Properties" : { - "CidrBlock" : { - "Fn::Cidr": [ "10.0.0.0/24", "6", "5"] - } - } - }, - "2ArgVpc": { - "Type" : "AWS::EC2::VPC", - "Properties" : { - "CidrBlock" : { - "Fn::Cidr": [ "10.0.0.0/24", "6"] - } - } - }, - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::FindInMap": [ - "RegionMap", - "region-1", - "HVM64" - ] - } - } - }, - "subnet1" : { - "Type" : "AWS::EC2::Subnet", - "Properties" : { - "VpcId" : { - "Fn::Split": [ - ",", - { "Fn::ImportValue" : "ImportedVpcId" } - ] - }, - "CidrBlock" : "10.0.0.0/24", - "AvailabilityZone" : { - "Fn::Select" : [ - "0", - { - "Fn::GetAZs" : "" - } - ] - } - } - }, - - "subnet2" : { - "Type" : "AWS::EC2::Subnet", - "Properties" : { - "VpcId" : { - "Fn::Select" : ["3", ["foo", "aValidVPCID", "foo2"]] - }, - "CidrBlock" : "10.0.0.0/24", - "AvailabilityZone" : { - "Fn::Select" : [ - "0", - { - "Fn::GetAZs" : "eu-west-2" - } - ] - } - } - }, - "transformBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::Transform": { - "Name": "AWS::Include", - "Parameters": { - "Location": "location", - "AnotherParameter": { - "Fn:Base64": "AnotherValue" - } - } - } - } - } - } - } -} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/only-codecommit-repo-using-cfn-functions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/only-codecommit-repo-using-cfn-functions.json similarity index 100% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/only-codecommit-repo-using-cfn-functions.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/only-codecommit-repo-using-cfn-functions.json diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 738d3c1187d2f..70372ad7f3c96 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -184,22 +184,6 @@ describe('CDK Include', () => { ); }); - test('can ingest a template with many conditions, and output it unchanged', () => { - includeTestTemplate(stack, 'conditions.json'); - - expect(stack).toMatchTemplate( - loadTestFileToJsObject('conditions.json'), - ); - }); - - test('can ingest a template with multiple cloudformation intrinsic functions, and output it unchanged', () => { - includeTestTemplate(stack, 'functions.json'); - - expect(stack).toMatchTemplate( - loadTestFileToJsObject('functions.json'), - ); - }); - test('can ingest a template with intrinsic functions and conditions, and output it unchanged', () => { includeTestTemplate(stack, 'functions-and-conditions.json'); @@ -208,7 +192,6 @@ describe('CDK Include', () => { ); }); - test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); @@ -295,12 +278,6 @@ describe('CDK Include', () => { }).toThrow(/Unrecognized CloudFormation resource type: 'AWS::FakeService::DoesNotExist'/); }); - test("throws an exception when encountering a CFN function it doesn't support", () => { - expect(() => { - includeTestTemplate(stack, 'only-codecommit-repo-using-cfn-functions.json'); - }).toThrow(/Unsupported CloudFormation function 'Fn::DoesNotExist'/); - }); - test('throws an exception when encountering the CreationPolicy attribute in a resource', () => { expect(() => { includeTestTemplate(stack, 'resource-attribute-creation-policy.json'); diff --git a/packages/@aws-cdk/core/lib/cfn-fn.ts b/packages/@aws-cdk/core/lib/cfn-fn.ts index 88c27bb538e29..98a368b9fdc38 100644 --- a/packages/@aws-cdk/core/lib/cfn-fn.ts +++ b/packages/@aws-cdk/core/lib/cfn-fn.ts @@ -176,11 +176,12 @@ export class Fn { * The intrinsic function Fn::Transform specifies a macro to perform custom processing on part of a stack template. * Macros enable you to perform custom processing on templates, from simple actions like find-and-replace operations * to extensive transformations of entire templates. - * @param macroNameParameters The name of the macro to perform the processing, and the Parameters to pass to it. + * @param macroName The name of the macro to perform the processing + * @param parameters The parameters to be passed to the macro * @returns a token represented as a string */ - public static transform(macroNameParameters: string): string { - return new FnTransform(macroNameParameters).toString(); + public static transform(macroName: string, parameters: '{[name: string]: any}'): string { + return new FnTransform(macroName, parameters).toString(); } /** @@ -372,10 +373,12 @@ class FnFindInMap extends FnBase { */ class FnTransform extends FnBase { /** - * @param macroNameParameters The name of the macro to be invoked, and the parameters to pass to it + * creates an ``Fn::Transform`` function. + * @param macroName The name of the macro to be invoked + * @param parameters the parameters to pass to it */ - constructor(macroNameParameters: string) { - super('Fn::Transform', macroNameParameters); + constructor(macroName: string, parameters: '{[name: string]: any}') { + super('Fn::Transform', {Name: macroName, Parameters: parameters}); } } diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index d386c5f512e55..876635e9b57f1 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -209,7 +209,7 @@ function parseIfCfnIntrinsic(object: any): any { } case 'Fn::Transform': { const value = parseCfnValueToCdkValue(object[key]); - return Fn.transform(value); + return Fn.transform(value.Name, value.Parameters); } case 'Fn::Base64': { const value = parseCfnValueToCdkValue(object[key]); From 7ca707e91f463af8e035312acea1138634eb8e46 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 16 Jun 2020 20:41:43 -0400 Subject: [PATCH 09/46] updated README to reflect the newly supported cloudformation functions --- .../@aws-cdk/cloudformation-include/README.md | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index a64d7b988e9bb..0a9b21dbcc173 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -136,16 +136,16 @@ All items unchecked below are currently not supported. - [x] Fn::GetAtt - [x] Fn::Join - [x] Fn::If -- [ ] Fn::And +- [x] Fn::And - [x] Fn::Equals -- [ ] Fn::Not -- [ ] Fn::Or -- [ ] Fn::Base64 -- [ ] Fn::Cidr -- [ ] Fn::FindInMap -- [ ] Fn::GetAZs -- [ ] Fn::ImportValue -- [ ] Fn::Select -- [ ] Fn::Split +- [x] Fn::Not +- [x] Fn::Or +- [x] Fn::Base64 +- [x] Fn::Cidr +- [x] Fn::FindInMap +- [x] Fn::GetAZs +- [x] Fn::ImportValue +- [x] Fn::Select +- [x] Fn::Split - [ ] Fn::Sub -- [ ] Fn::Transform +- [x] Fn::Transform From 63c27a775196a33b04fde0bcd0d822d87aa132f9 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 17 Jun 2020 17:35:10 -0400 Subject: [PATCH 10/46] removed quotes from the type of Transform's parameter argument, modified the return type of transform to IResolvable, and updated transform docs --- .../functions-and-conditions.json | 246 +++++++++++------- packages/@aws-cdk/core/lib/cfn-fn.ts | 12 +- 2 files changed, 151 insertions(+), 107 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json index a9031ab3ae58c..873bbe2254d48 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json @@ -1,122 +1,168 @@ { - "Mappings" : { - "RegionMap" : { - "region-1" : { - "HVM64" : "name1", "HVMG2" : "name2" - } + "Mappings": { + "RegionMap": { + "region-1": { + "HVM64": "name1", + "HVMG2": "name2" } + } + }, + "Conditions": { + "AlwaysTrueCond": { + "Fn::Not": [ + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + } + ] }, - "Conditions": { - "AlwaysTrueCond": { - "Fn::Not": [{ - "Fn::Equals": [{"Ref": "AWS::Region"}, "completely-made-up-region"]}] + "AlwaysFalseCond": { + "Fn::Equals": [ + { + "Ref": "AWS::Region" }, - "AlwaysFalseCond": { - "Fn::Equals": [{ "Ref": "AWS::Region" }, "completely-made-up-region"] + "completely-made-up-region" + ] + }, + "AndCond": { + "Fn::And": [ + { + "Condition": "AlwaysTrueCond" }, - "AndCond": { - "Fn::And": [ - {"Condition": "AlwaysTrueCond"}, - {"Condition": "AlwaysFalseCond"} - ] + { + "Condition": "AlwaysFalseCond" + } + ] + }, + "OrCond": { + "Fn::Or": [ + { + "Condition": "AlwaysTrueCond" }, - "OrCond": { - "Fn::Or": [ - {"Condition": "AlwaysTrueCond"}, - {"Condition": "AlwaysFalseCond"} - ] + { + "Condition": "AlwaysFalseCond" } - }, - "Resources": { - "Vpc": { - "Type" : "AWS::EC2::VPC", - "Properties" : { - "CidrBlock" : { - "Fn::If": [ - "AlwaysTrueCond", - { "Fn::Cidr": [ "192.168.1.1/24", 2] }, - { "Fn::Cidr": [ "10.0.0.0/24", 6, 5] } + ] + } + }, + "Resources": { + "Vpc": { + "Type": "AWS::EC2::VPC", + "Properties": { + "CidrBlock": { + "Fn::If": [ + "AlwaysTrueCond", + { + "Fn::Cidr": [ + "192.168.1.1/24", + 2 ] - } + }, + { + "Fn::Cidr": [ + "10.0.0.0/24", + 6, + 5 + ] + } + ] } - }, - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::If": [ - {"Fn::Or": ["AlwaysFalseCond", "AlwaysFalseCond"]}, - "Unreachable", - {"Fn::FindInMap": [ + } + }, + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::If": [ + "OrCond", + { + "Fn::FindInMap": [ "RegionMap", "region-1", "HVM64" - ]} - ] - } + ] + }, + "Unreachable" + ] } - }, - "Subnet1" : { - "Type" : "AWS::EC2::Subnet", - "Properties" : { - "VpcId" : { - "Fn::If": [ - {"Fn::And": ["AlwaysTrueCond", "AlwaysTrueCond"]}, - { - "Fn::Split": [ - ",", - { "Fn::ImportValue" : "ImportedVpcId" } - ]}, - "Unreachable" - ] - }, - "CidrBlock" : "10.0.0.0/24", - "AvailabilityZone" : { - "Fn::Select" : [ - "0", - { - "Fn::GetAZs" : "" - } - ] - } + } + }, + "Subnet1": { + "Type": "AWS::EC2::Subnet", + "Properties": { + "VpcId": { + "Fn::If": [ + "AlwaysTrueCond", + { + "Fn::Split": [ + ",", + { + "Fn::ImportValue": "ImportedVpcId" + } + ] + }, + "Unreachable" + ] + }, + "CidrBlock": "10.0.0.0/24", + "AvailabilityZone": { + "Fn::Select": [ + "0", + { + "Fn::GetAZs": "" + } + ] } - }, - "Subnet2" : { - "Type" : "AWS::EC2::Subnet", - "Properties" : { - "VpcId" : { - "Fn::Select" : ["3", ["foo", "aValidVPCID", "foo2"]] - }, - "CidrBlock" : "10.0.0.0/24", - "AvailabilityZone" : { - "Fn::Select" : [ - "0", - { - "Fn::GetAZs" : "eu-west-2" - } + } + }, + "Subnet2": { + "Type": "AWS::EC2::Subnet", + "Properties": { + "VpcId": { + "Fn::Select": [ + "3", + [ + "foo", + "aValidVPCID", + "foo2" ] - } + ] + }, + "CidrBlock": "10.0.0.0/24", + "AvailabilityZone": { + "Fn::Select": [ + "0", + { + "Fn::GetAZs": "eu-west-2" + } + ] } - }, - "TransformBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::If": [ - { "Fn::Or": ["AlwaysFalseCond", "AlwaysTrueCond"]}, { - "Fn::Transform": { - "Name": "AWS::Include", - "Parameters": { - "Location": "location", - "AnotherParameter": { - "Fn:Base64": "AnotherValue" - } + } + }, + "TransformBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::If": [ + "AndCond", + { + "Fn::Transform": { + "Name": "AWS::Include", + "Parameters": { + "Location": "location", + "AnotherParameter": { + "Fn:Base64": "AnotherValue" } } } - ] - } + } + ] } } } - } \ No newline at end of file + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/cfn-fn.ts b/packages/@aws-cdk/core/lib/cfn-fn.ts index 98a368b9fdc38..31269f531a792 100644 --- a/packages/@aws-cdk/core/lib/cfn-fn.ts +++ b/packages/@aws-cdk/core/lib/cfn-fn.ts @@ -173,15 +173,13 @@ export class Fn { } /** - * The intrinsic function Fn::Transform specifies a macro to perform custom processing on part of a stack template. - * Macros enable you to perform custom processing on templates, from simple actions like find-and-replace operations - * to extensive transformations of entire templates. + * Creates a token representing the ``Fn::Transform`` expression * @param macroName The name of the macro to perform the processing * @param parameters The parameters to be passed to the macro - * @returns a token represented as a string + * @returns a token representing the transform expression */ - public static transform(macroName: string, parameters: '{[name: string]: any}'): string { - return new FnTransform(macroName, parameters).toString(); + public static transform(macroName: string, parameters: { [name: string]: any }): IResolvable { + return new FnTransform(macroName, parameters); } /** @@ -377,7 +375,7 @@ class FnTransform extends FnBase { * @param macroName The name of the macro to be invoked * @param parameters the parameters to pass to it */ - constructor(macroName: string, parameters: '{[name: string]: any}') { + constructor(macroName: string, parameters: { [name: string]: any }) { super('Fn::Transform', {Name: macroName, Parameters: parameters}); } } From 41792710439cd994d2e582fbd69de0a696c5142e Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 17 Jun 2020 20:09:23 -0400 Subject: [PATCH 11/46] fixed teseting issue related to Fn::Select --- .../functions-and-conditions.json | 48 ++++++++----------- packages/@aws-cdk/core/lib/cfn-fn.ts | 3 +- packages/@aws-cdk/core/lib/cfn-parse.ts | 4 -- 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json index 873bbe2254d48..c0ac971e316a6 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json @@ -20,31 +20,20 @@ } ] }, - "AlwaysFalseCond": { - "Fn::Equals": [ - { - "Ref": "AWS::Region" - }, - "completely-made-up-region" - ] - }, "AndCond": { "Fn::And": [ { "Condition": "AlwaysTrueCond" }, { - "Condition": "AlwaysFalseCond" - } - ] - }, - "OrCond": { - "Fn::Or": [ - { - "Condition": "AlwaysTrueCond" - }, - { - "Condition": "AlwaysFalseCond" + "Fn::Or": [ + { + "Condition": "AlwaysTrueCond" + }, + { + "Condition": "AlwaysTrueCond" + } + ] } ] } @@ -59,14 +48,15 @@ { "Fn::Cidr": [ "192.168.1.1/24", - 2 + 2, + 5 ] }, { "Fn::Cidr": [ "10.0.0.0/24", - 6, - 5 + "6", + "5" ] } ] @@ -124,12 +114,14 @@ "Properties": { "VpcId": { "Fn::Select": [ - "3", - [ - "foo", - "aValidVPCID", - "foo2" - ] + 0, + { + "Fn::Cidr": [ + "10.0.0.0/24", + 5, + 2 + ] + } ] }, "CidrBlock": "10.0.0.0/24", diff --git a/packages/@aws-cdk/core/lib/cfn-fn.ts b/packages/@aws-cdk/core/lib/cfn-fn.ts index 31269f531a792..73d0f61c03236 100644 --- a/packages/@aws-cdk/core/lib/cfn-fn.ts +++ b/packages/@aws-cdk/core/lib/cfn-fn.ts @@ -174,6 +174,7 @@ export class Fn { /** * Creates a token representing the ``Fn::Transform`` expression + * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-transform.html * @param macroName The name of the macro to perform the processing * @param parameters The parameters to be passed to the macro * @returns a token representing the transform expression @@ -376,7 +377,7 @@ class FnTransform extends FnBase { * @param parameters the parameters to pass to it */ constructor(macroName: string, parameters: { [name: string]: any }) { - super('Fn::Transform', {Name: macroName, Parameters: parameters}); + super('Fn::Transform', { Name: macroName, Parameters: parameters }); } } diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 876635e9b57f1..f8c2a84a98fd4 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -181,10 +181,6 @@ function parseIfCfnIntrinsic(object: any): any { } case 'Fn::Cidr': { const value = parseCfnValueToCdkValue(object[key]); - - if (value.length === 2) { - return Fn.cidr(value[0], value[1]); - } return Fn.cidr(value[0], value[1], value[2]); } case 'Fn::FindInMap': { From 14a0d647bdf042f6707dd5457900985c1d3fdbbf Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 18 Jun 2020 11:03:33 -0400 Subject: [PATCH 12/46] merge conflict resolution --- .../test/valid-templates.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 987b96cf54ec2..1855c46acab5a 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -299,18 +299,6 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'non-existent-resource-type.json'); }).toThrow(/Unrecognized CloudFormation resource type: 'AWS::FakeService::DoesNotExist'/); }); - - test('throws an exception when encountering the CreationPolicy attribute in a resource', () => { - expect(() => { - includeTestTemplate(stack, 'resource-attribute-creation-policy.json'); - }).toThrow(/The CreationPolicy resource attribute is not supported by cloudformation-include yet/); - }); - - test('throws an exception when encountering the UpdatePolicy attribute in a resource', () => { - expect(() => { - includeTestTemplate(stack, 'resource-attribute-update-policy.json'); - }).toThrow(/The UpdatePolicy resource attribute is not supported by cloudformation-include yet/); - }); }); interface IncludeTestTemplateProps { From 4c9d1eca5bf1ec9e6492932aec13404c46dcc398 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 18 Jun 2020 12:57:08 -0400 Subject: [PATCH 13/46] fixed typo in condition name --- .../test/test-templates/functions-and-conditions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json index c0ac971e316a6..ce57b3e1a8b9f 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/functions-and-conditions.json @@ -68,7 +68,7 @@ "Properties": { "BucketName": { "Fn::If": [ - "OrCond", + "AndCond", { "Fn::FindInMap": [ "RegionMap", From 42c0228f7702216f7ccde494d9edaeb761134bf8 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 18 Jun 2020 18:26:05 -0400 Subject: [PATCH 14/46] removed parameters from _toCloudFormation() --- .../cloudformation-include/lib/cfn-include.ts | 40 +++++++++++++++++-- .../bucket-with-parameters.json | 15 +++++++ .../test/valid-templates.test.ts | 14 +++++++ 3 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 07c6dee3bfcbf..0c351bedb6781 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -23,6 +23,7 @@ export interface CfnIncludeProps { export class CfnInclude extends core.CfnElement { private readonly conditions: { [conditionName: string]: core.CfnCondition } = {}; private readonly resources: { [logicalId: string]: core.CfnResource } = {}; + private readonly parameters: { [parameterName: string]: core.CfnParameter } = {}; private readonly template: any; private readonly preserveLogicalIds: boolean; @@ -44,6 +45,11 @@ export class CfnInclude extends core.CfnElement { for (const logicalId of Object.keys(this.template.Resources || {})) { this.getOrCreateResource(logicalId); } + + // instantiate all parameters + for (const parameterName of Object.keys(this.template.Parameters || {})) { + this.createParameter(parameterName); + } } /** @@ -72,7 +78,7 @@ export class CfnInclude extends core.CfnElement { /** * Returns the CfnCondition object from the 'Conditions' - * section of the CloudFormation template with the give name. + * section of the CloudFormation template with the given name. * Any modifications performed on that object will be reflected in the resulting CDK template. * * If a Condition with the given name is not present in the template, @@ -88,14 +94,34 @@ export class CfnInclude extends core.CfnElement { return ret; } + /** + * Returns the CfnParameter object from the 'Parameters' + * section of the CloudFormation template with the given name. + * Any modifications performed on that object will be reflected in the resulting CDK template. + * + * If a Parameter with the given name is not present in the template, + * throws an exception. + * + * @param parameterName the name of the Parameter in the CloudFormation template file + */ + /* public getParameter(parameterName: string): core.CfnParameter { + const ret = this.parameters[parameterName]; + if (!ret) { + throw new Error(`Parameter with name '${parameterName}' was not found in the template`); + } + return ret; + } + */ + /** @internal */ public _toCloudFormation(): object { const ret: { [section: string]: any } = {}; for (const section of Object.keys(this.template)) { // render all sections of the template unchanged, - // except Conditions and Resources, which will be taken care of by the created L1s - if (section !== 'Conditions' && section !== 'Resources') { + // except Conditions, Resources, and Parameters, which will be taken care of by the created L1s + // ToDo + if (section !== 'Conditions' && section !== 'Resources' && section !== 'Parameters') { ret[section] = this.template[section]; } } @@ -103,6 +129,14 @@ export class CfnInclude extends core.CfnElement { return ret; } + private createParameter(parameterName: string): void { + const expression = cfn_parse.FromCloudFormation.parseValue(this.template.Parameters[parameterName]); + const CfnParameter = new core.CfnParameter(this, parameterName, expression); + + CfnParameter.overrideLogicalId(parameterName); + this.parameters[parameterName] = CfnParameter; + } + private createCondition(conditionName: string): void { // ToDo condition expressions can refer to other conditions - // will be important when implementing preserveLogicalIds=false diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json new file mode 100644 index 0000000000000..0104ef6541831 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json @@ -0,0 +1,15 @@ +{ + "Parameters": { + "BucketName": "MyS3Bucket" + }, + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Ref": "BucketName" + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 1855c46acab5a..52cf132eaab46 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -251,6 +251,19 @@ describe('CDK Include', () => { ); }); + test("correctly parses templates with parameters", () => { + const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); + //const bucketNameParam = cfnTemplate.getParameter('BucketName'); + + expect(stack).toHaveResourceLike('AWS::S3::Bucket', { + "Properties": { + "BucketName": "MyS3Bucket" + } + }); + + //expect(bucketNameParam).toBe("MyS3Bucket"); + }); + test('reflects changes to a retrieved CfnCondition object in the resulting template', () => { const cfnTemplate = includeTestTemplate(stack, 'resource-attribute-condition.json'); const alwaysFalseCondition = cfnTemplate.getCondition('AlwaysFalseCond'); @@ -299,6 +312,7 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'non-existent-resource-type.json'); }).toThrow(/Unrecognized CloudFormation resource type: 'AWS::FakeService::DoesNotExist'/); }); + }); interface IncludeTestTemplateProps { From 6a2df1e910e4302c8348827bc02f6b2d8e112ada Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Jun 2020 13:23:02 -0400 Subject: [PATCH 15/46] added support for parameters in templates --- .../cloudformation-include/lib/cfn-include.ts | 56 ++++++++++--------- .../bucket-with-parameters.json | 6 +- .../test/valid-templates.test.ts | 45 +++++++++++++-- 3 files changed, 74 insertions(+), 33 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 0c351bedb6781..df26cc198d5b2 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -23,7 +23,7 @@ export interface CfnIncludeProps { export class CfnInclude extends core.CfnElement { private readonly conditions: { [conditionName: string]: core.CfnCondition } = {}; private readonly resources: { [logicalId: string]: core.CfnResource } = {}; - private readonly parameters: { [parameterName: string]: core.CfnParameter } = {}; + private readonly parameters: { [logicalId: string]: core.CfnParameter } = {}; private readonly template: any; private readonly preserveLogicalIds: boolean; @@ -47,27 +47,11 @@ export class CfnInclude extends core.CfnElement { } // instantiate all parameters - for (const parameterName of Object.keys(this.template.Parameters || {})) { - this.createParameter(parameterName); + for (const logicalId of Object.keys(this.template.Parameters || {})) { + this.createParameter(logicalId); } } - /** - * Returns the low-level CfnResource from the template with the given logical ID. - * Any modifications performed on that resource will be reflected in the resulting CDK template. - * - * The returned object will be of the proper underlying class; - * you can always cast it to the correct type in your code: - * - * // assume the template contains an AWS::S3::Bucket with logical ID 'Bucket' - * const cfnBucket = cfnTemplate.getResource('Bucket') as s3.CfnBucket; - * // cfnBucket is of type s3.CfnBucket - * - * If the template does not contain a resource with the given logical ID, - * an exception will be thrown. - * - * @param logicalId the logical ID of the resource in the CloudFormation template file - */ public getResource(logicalId: string): core.CfnResource { const ret = this.resources[logicalId]; if (!ret) { @@ -94,6 +78,14 @@ export class CfnInclude extends core.CfnElement { return ret; } + public getParameter(parameterName: string): core.CfnParameter { + const ret = this.parameters[parameterName]; + if (!ret) { + throw new Error(`Parameter with name '${parameterName}' was not found in the template`); + } + return ret; + } + /** * Returns the CfnParameter object from the 'Parameters' * section of the CloudFormation template with the given name. @@ -102,7 +94,7 @@ export class CfnInclude extends core.CfnElement { * If a Parameter with the given name is not present in the template, * throws an exception. * - * @param parameterName the name of the Parameter in the CloudFormation template file + * @param logicalId the name of the Parameter in the CloudFormation template file */ /* public getParameter(parameterName: string): core.CfnParameter { const ret = this.parameters[parameterName]; @@ -129,12 +121,24 @@ export class CfnInclude extends core.CfnElement { return ret; } - private createParameter(parameterName: string): void { - const expression = cfn_parse.FromCloudFormation.parseValue(this.template.Parameters[parameterName]); - const CfnParameter = new core.CfnParameter(this, parameterName, expression); - - CfnParameter.overrideLogicalId(parameterName); - this.parameters[parameterName] = CfnParameter; + private createParameter(logicalId: string): void { + const expression = cfn_parse.FromCloudFormation.parseValue(this.template.Parameters[logicalId]); + const expressionCaseCorrected = { + type: expression.Type, + default: expression.Default, + allowedPattern: expression.AllowedPattern, + constraintDescription: expression.ConstraintDescription, + description: expression.Description, + maxLength: expression.MaxLength, + maxValue: expression.MaxValue, + minLength: expression.MinLength, + minValue: expression.MinValue, + noEcho: expression.NoEcho, + }; + const CfnParameter = new core.CfnParameter(this, logicalId, expressionCaseCorrected); + + CfnParameter.overrideLogicalId(logicalId); + this.parameters[logicalId] = CfnParameter; } private createCondition(conditionName: string): void { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json index 0104ef6541831..531db51c9f10d 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json @@ -1,6 +1,10 @@ { "Parameters": { - "BucketName": "MyS3Bucket" + "BucketName": { + "Default": "MyS3Bucket", + "Description": "The name of your bucket", + "Type": "String" + } }, "Resources": { "Bucket": { diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 52cf132eaab46..a7e80df958d54 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -253,15 +253,48 @@ describe('CDK Include', () => { test("correctly parses templates with parameters", () => { const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); - //const bucketNameParam = cfnTemplate.getParameter('BucketName'); + const param = cfnTemplate.getParameter('BucketName'); + const bucket = new s3.CfnBucket(stack, "newBucket", {"bucketName": param.valueAsString}); - expect(stack).toHaveResourceLike('AWS::S3::Bucket', { - "Properties": { - "BucketName": "MyS3Bucket" + expect(stack).toMatchTemplate( + { + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Ref": "BucketName" + } + } + }, + "newBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Ref": "BucketName" + } + } + } + }, + "Parameters": { + "BucketName": { + "Type": "String", + "Default": "MyS3Bucket", + "Description": "The name of your bucket" + } + } } - }); + ); + + expect(bucket.bucketName).toBeDefined(); + }); - //expect(bucketNameParam).toBe("MyS3Bucket"); + test('getParameter() throws an exception when a parameter is not found', () => { + const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); + + expect(() => { + cfnTemplate.getParameter("FakeBucketNameThatDoesNotExist"); + }).toThrow(/Parameter with name 'FakeBucketNameThatDoesNotExist' was not found in the template/); }); test('reflects changes to a retrieved CfnCondition object in the resulting template', () => { From cd27ff6053f12d310d823c6f0e88dc3e0afb5d25 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Jun 2020 14:59:14 -0400 Subject: [PATCH 16/46] updated documentation --- .../cloudformation-include/lib/cfn-include.ts | 13 ++++++++++- .../test/valid-templates.test.ts | 22 +++++++++---------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index df26cc198d5b2..b9a0ff9f4cc72 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -78,6 +78,17 @@ export class CfnInclude extends core.CfnElement { return ret; } + /** + * Returns the CfnParameter object from the 'Parameters' + * section of the included template + * Any modifications performed on that object will be reflected in the resulting CDK template. + * + * If a Parameter with the given name is not present in the template, + * throws an exception. + * + * @param parameterName the name of the parameter to retrieve + */ + public getParameter(parameterName: string): core.CfnParameter { const ret = this.parameters[parameterName]; if (!ret) { @@ -136,7 +147,7 @@ export class CfnInclude extends core.CfnElement { noEcho: expression.NoEcho, }; const CfnParameter = new core.CfnParameter(this, logicalId, expressionCaseCorrected); - + CfnParameter.overrideLogicalId(logicalId); this.parameters[logicalId] = CfnParameter; } diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 47d7d70899fec..fe9c50341d6db 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -263,27 +263,27 @@ describe('CDK Include', () => { "Type": "AWS::S3::Bucket", "Properties": { "BucketName": { - "Ref": "BucketName" - } - } + "Ref": "BucketName", + }, + }, }, "newBucket": { "Type": "AWS::S3::Bucket", "Properties": { "BucketName": { - "Ref": "BucketName" - } - } - } + "Ref": "BucketName", + }, + }, + }, }, "Parameters": { "BucketName": { "Type": "String", "Default": "MyS3Bucket", - "Description": "The name of your bucket" - } - } - } + "Description": "The name of your bucket", + }, + }, + }, ); expect(bucket.bucketName).toBeDefined(); From 1e72465697888396fb7b3ba0594ab3681f80cad2 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Jun 2020 15:06:01 -0400 Subject: [PATCH 17/46] updated readme --- packages/@aws-cdk/cloudformation-include/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 4023f2b214069..188f9989075ea 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -115,7 +115,7 @@ All items unchecked below are currently not supported. ### Ability to retrieve CloudFormation objects from the template: - [x] Resources -- [ ] Parameters +- [x] Parameters - [x] Conditions - [ ] Outputs From 070f902759c5f8528cf3a509dada354d115972ce Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Jun 2020 18:22:52 -0400 Subject: [PATCH 18/46] incorporated adam's comments --- .../cloudformation-include/lib/cfn-include.ts | 55 ++++++++----------- .../bucket-with-parameters.json | 30 +++++++++- .../test/valid-templates.test.ts | 46 ++++++---------- 3 files changed, 70 insertions(+), 61 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index b9a0ff9f4cc72..7906bd92c5cb5 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -36,6 +36,10 @@ export class CfnInclude extends core.CfnElement { // ToDo implement preserveLogicalIds=false this.preserveLogicalIds = true; + // instantiate all parameters + for (const logicalId of Object.keys(this.template.Parameters || {})) { + this.createParameter(logicalId); + } // first, instantiate the conditions for (const conditionName of Object.keys(this.template.Conditions || {})) { this.createCondition(conditionName); @@ -45,13 +49,24 @@ export class CfnInclude extends core.CfnElement { for (const logicalId of Object.keys(this.template.Resources || {})) { this.getOrCreateResource(logicalId); } - - // instantiate all parameters - for (const logicalId of Object.keys(this.template.Parameters || {})) { - this.createParameter(logicalId); - } } + /** + * Returns the low-level CfnResource from the template with the given logical ID. + * Any modifications performed on that resource will be reflected in the resulting CDK template. + * + * The returned object will be of the proper underlying class; + * you can always cast it to the correct type in your code: + * + * // assume the template contains an AWS::S3::Bucket with logical ID 'Bucket' + * const cfnBucket = cfnTemplate.getResource('Bucket') as s3.CfnBucket; + * // cfnBucket is of type s3.CfnBucket + * + * If the template does not contain a resource with the given logical ID, + * an exception will be thrown. + * + * @param logicalId the logical ID of the resource in the CloudFormation template file + */ public getResource(logicalId: string): core.CfnResource { const ret = this.resources[logicalId]; if (!ret) { @@ -88,7 +103,6 @@ export class CfnInclude extends core.CfnElement { * * @param parameterName the name of the parameter to retrieve */ - public getParameter(parameterName: string): core.CfnParameter { const ret = this.parameters[parameterName]; if (!ret) { @@ -97,25 +111,6 @@ export class CfnInclude extends core.CfnElement { return ret; } - /** - * Returns the CfnParameter object from the 'Parameters' - * section of the CloudFormation template with the given name. - * Any modifications performed on that object will be reflected in the resulting CDK template. - * - * If a Parameter with the given name is not present in the template, - * throws an exception. - * - * @param logicalId the name of the Parameter in the CloudFormation template file - */ - /* public getParameter(parameterName: string): core.CfnParameter { - const ret = this.parameters[parameterName]; - if (!ret) { - throw new Error(`Parameter with name '${parameterName}' was not found in the template`); - } - return ret; - } - */ - /** @internal */ public _toCloudFormation(): object { const ret: { [section: string]: any } = {}; @@ -123,7 +118,6 @@ export class CfnInclude extends core.CfnElement { for (const section of Object.keys(this.template)) { // render all sections of the template unchanged, // except Conditions, Resources, and Parameters, which will be taken care of by the created L1s - // ToDo if (section !== 'Conditions' && section !== 'Resources' && section !== 'Parameters') { ret[section] = this.template[section]; } @@ -134,7 +128,7 @@ export class CfnInclude extends core.CfnElement { private createParameter(logicalId: string): void { const expression = cfn_parse.FromCloudFormation.parseValue(this.template.Parameters[logicalId]); - const expressionCaseCorrected = { + const cfnParameter = new core.CfnParameter(this, logicalId, { type: expression.Type, default: expression.Default, allowedPattern: expression.AllowedPattern, @@ -145,11 +139,10 @@ export class CfnInclude extends core.CfnElement { minLength: expression.MinLength, minValue: expression.MinValue, noEcho: expression.NoEcho, - }; - const CfnParameter = new core.CfnParameter(this, logicalId, expressionCaseCorrected); + }); - CfnParameter.overrideLogicalId(logicalId); - this.parameters[logicalId] = CfnParameter; + cfnParameter.overrideLogicalId(logicalId); + this.parameters[logicalId] = cfnParameter; } private createCondition(conditionName: string): void { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json index 531db51c9f10d..deec4ffa24e81 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/bucket-with-parameters.json @@ -2,8 +2,21 @@ "Parameters": { "BucketName": { "Default": "MyS3Bucket", + "AllowedPattern": "^[a-zA-Z0-9]*$", + "ConstraintDescription": "a string consisting only of alphanumeric characters", "Description": "The name of your bucket", - "Type": "String" + "MaxLength": "10", + "MinLength": "1", + "Type": "String", + "NoEcho": "true" + }, + "CorsMaxAge": { + "Default": "3", + "Description": "the time in seconds that a browser will cache the preflight response", + "MaxValue": "300", + "MinValue": "0", + "Type": "Number", + "NoEcho": "true" } }, "Resources": { @@ -12,6 +25,21 @@ "Properties": { "BucketName": { "Ref": "BucketName" + }, + "CorsConfiguration": { + "CorsRules": [{ + "AllowedMethods": [ + "GET", + "POST" + ], + "AllowedOrigins": [ + "origin1", + "origin2" + ], + "MaxAge": { + "Ref": "CorsMaxAge" + } + }] } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index fe9c50341d6db..91cc14be619b2 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -253,43 +253,31 @@ describe('CDK Include', () => { test("correctly parses templates with parameters", () => { const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); + const originalTemplate = loadTestFileToJsObject('bucket-with-parameters.json'); const param = cfnTemplate.getParameter('BucketName'); - const bucket = new s3.CfnBucket(stack, "newBucket", {"bucketName": param.valueAsString}); + new s3.CfnBucket(stack, "NewBucket", { + "bucketName": param.valueAsString, + }); - expect(stack).toMatchTemplate( - { - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Ref": "BucketName", - }, - }, - }, - "newBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Ref": "BucketName", - }, + expect(stack).toMatchTemplate({ + "Resources": { + ...originalTemplate.Resources, + "NewBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Ref": "BucketName", }, }, }, - "Parameters": { - "BucketName": { - "Type": "String", - "Default": "MyS3Bucket", - "Description": "The name of your bucket", - }, - }, }, - ); - - expect(bucket.bucketName).toBeDefined(); + "Parameters": { + ...originalTemplate.Parameters, + }, + }); }); - test('getParameter() throws an exception when a parameter is not found', () => { + test('getParameter() throws an exception if asked for a Parameter with a name that is not present in the template', () => { const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); expect(() => { From b10a1ed08ba7171ea5394ed8f5baa36f4d32f319 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Jun 2020 19:08:41 -0400 Subject: [PATCH 19/46] fixed spacing --- packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 7906bd92c5cb5..600a97d65f991 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -40,6 +40,7 @@ export class CfnInclude extends core.CfnElement { for (const logicalId of Object.keys(this.template.Parameters || {})) { this.createParameter(logicalId); } + // first, instantiate the conditions for (const conditionName of Object.keys(this.template.Conditions || {})) { this.createCondition(conditionName); From 685d57dea6100127077ac7157fc7851203aba18d Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 29 Jun 2020 14:03:00 -0400 Subject: [PATCH 20/46] added outputs array --- packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 49c5a769de73e..fed550419fd68 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -24,6 +24,7 @@ export class CfnInclude extends core.CfnElement { private readonly conditions: { [conditionName: string]: core.CfnCondition } = {}; private readonly resources: { [logicalId: string]: core.CfnResource } = {}; private readonly parameters: { [logicalId: string]: core.CfnParameter } = {}; + private readonly outputs: { [outputName: string]: core.CfnOutput } = {}; private readonly template: any; private readonly preserveLogicalIds: boolean; From e334814a2bef242e07e827b107dadc7b27791da0 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 17:47:50 -0400 Subject: [PATCH 21/46] added support for retrieving and modifying outputs --- .../cloudformation-include/lib/cfn-include.ts | 62 +++++++++++++++++- .../outputs-with-references.json | 42 +++++++++++++ .../test/test-templates/outputs.json | 28 +++++++++ .../test/valid-templates.test.ts | 63 +++++++++++++++++++ packages/@aws-cdk/core/lib/cfn-output.ts | 40 ++++++++++-- 5 files changed, 228 insertions(+), 7 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index fed550419fd68..879c5b5082402 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -2,6 +2,7 @@ import * as core from '@aws-cdk/core'; import * as cfn_parse from '@aws-cdk/core/lib/cfn-parse'; import * as cfn_type_to_l1_mapping from './cfn-type-to-l1-mapping'; import * as futils from './file-utils'; +import { CfnCondition } from '@aws-cdk/core'; /** * Construction properties of {@link CfnInclude}. @@ -24,7 +25,7 @@ export class CfnInclude extends core.CfnElement { private readonly conditions: { [conditionName: string]: core.CfnCondition } = {}; private readonly resources: { [logicalId: string]: core.CfnResource } = {}; private readonly parameters: { [logicalId: string]: core.CfnParameter } = {}; - private readonly outputs: { [outputName: string]: core.CfnOutput } = {}; + private readonly outputs: { [logicalId: string]: core.CfnOutput } = {}; private readonly template: any; private readonly preserveLogicalIds: boolean; @@ -47,6 +48,10 @@ export class CfnInclude extends core.CfnElement { this.createCondition(conditionName); } + for (const logicalId of Object.keys(this.template.Outputs || {})) { + this.createOutput(logicalId); + } + // instantiate all resources as CDK L1 objects for (const logicalId of Object.keys(this.template.Resources || {})) { this.getOrCreateResource(logicalId); @@ -113,14 +118,32 @@ export class CfnInclude extends core.CfnElement { return ret; } + /** + * Returns the CfnOutput object from the 'Outputs' + * section of the included template + * Any modifications performed on that object will be reflected in the resulting CDK template. + * + * If a Parameter with the given name is not present in the template, + * throws an exception. + * + * @param outputName the name of the output to retrieve + */ + public getOutput(outputName: string): core.CfnOutput { + const ret = this.outputs[outputName]; + if (!ret) { + throw new Error(`Output with logical ID '${outputName}' was not found in the template`); + } + return ret; + } + /** @internal */ public _toCloudFormation(): object { const ret: { [section: string]: any } = {}; for (const section of Object.keys(this.template)) { // render all sections of the template unchanged, - // except Conditions, Resources, and Parameters, which will be taken care of by the created L1s - if (section !== 'Conditions' && section !== 'Resources' && section !== 'Parameters') { + // except Conditions, Resources, Parameters, and Outputs which will be taken care of by the created L1s + if (section !== 'Conditions' && section !== 'Resources' && section !== 'Parameters' && section !== 'Outputs') { ret[section] = this.template[section]; } } @@ -153,6 +176,39 @@ export class CfnInclude extends core.CfnElement { this.parameters[logicalId] = cfnParameter; } + private createOutput(logicalId: string): void { + const self = this; + const expression = new cfn_parse.CfnParser({ + finder: { + findResource(lId: string): core.CfnResource | undefined { + if (!(lId in (self.template.Resources || {}))) { + return undefined; + } + return self.getOrCreateResource(lId); + }, + findRefTarget(elementName: string): core.CfnElement | undefined { + if (elementName in self.parameters) { + return self.parameters[elementName]; + } + + return this.findResource(elementName); + }, + findCondition(): undefined { + return undefined; + }, + }, + }).parseValue(this.template.Outputs[logicalId]); + const cfnOutput = new core.CfnOutput(this, logicalId, { + value: expression.Value, + description: expression.Description, + exportName: expression.Export, + condition: expression.Condition ? self.getCondition(expression.Condition) as CfnCondition : undefined + }); + + cfnOutput.overrideLogicalId(logicalId); + this.outputs[logicalId] = cfnOutput; + } + private createCondition(conditionName: string): void { // ToDo condition expressions can refer to other conditions - // will be important when implementing preserveLogicalIds=false diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json new file mode 100644 index 0000000000000..80518492651e1 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json @@ -0,0 +1,42 @@ +{ + "Conditions": { + "AlwaysFalseCond": { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + } + }, + "Parameters": { + "MyParam": { + "Type": "String" + } + }, + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": "my-referenced-bucket-name" + }, + "Condition": "AlwaysFalseCond" + } + }, + "Outputs": { + "Output1": { + "Value": { "Ref": "Bucket" }, + "Description": { "Ref": "MyParam" }, + "Condition": "AlwaysFalseCond" + }, + "Output2": { + "Value": { + "Fn::GetAtt": [ + "Bucket", + "Arn" + ] + }, + "Condition": "AlwaysFalseCond" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json new file mode 100644 index 0000000000000..b2343a4ce3145 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json @@ -0,0 +1,28 @@ +{ + "Conditions": { + "AlwaysFalseCond": { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "completely-made-up-region" + ] + } + }, + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": "my-referenced-bucket-name" + } + } + }, + "Outputs": { + "Output1": { + "Description": "the description of the output value", + "Value": "the output value", + "Export": "the export", + "Condition": "AlwaysFalseCond" + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 6994a77f6bff1..2032fe6ec473f 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -6,6 +6,7 @@ import * as core from '@aws-cdk/core'; import * as path from 'path'; import * as inc from '../lib'; import * as futils from '../lib/file-utils'; +import { CfnCondition } from '@aws-cdk/core'; // tslint:disable:object-literal-key-quotes /* eslint-disable quotes */ @@ -414,6 +415,68 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'non-existent-resource-type.json'); }).toThrow(/Unrecognized CloudFormation resource type: 'AWS::FakeService::DoesNotExist'/); }); + + test("can ingest a template that contains outputs and modify them", () => { + const cfnTemplate = includeTestTemplate(stack, 'outputs.json'); + const output = cfnTemplate.getOutput('Output1'); + const bucket = cfnTemplate.getResource('Bucket'); + + output.getValue(); + output.setValue('a mutated value'); + output.setDescription(undefined); + output.setExport("an export"); + output.setCondition(new core.CfnCondition(stack, 'MyCondition', {})); + + new core.CfnOutput(stack, 'Output2', { + value: bucket.ref, + description: "a description", + }); + + const originalTemplate = loadTestFileToJsObject('outputs.json'); + + expect(stack).toMatchTemplate({ + "Conditions": { + ...originalTemplate.Conditions, + }, + "Outputs": { + "Output1": { + "Value": "a mutated value", + "Export": { + "Name": "an export", + }, + "Condition": "MyCondition", + }, + "Output2": { + "Value": { + "Ref": "Bucket", + }, + "Description": "a description", + }, + }, + "Resources": { + ...originalTemplate.Resources, + }, + }); + }); + + test("can ingest a template that contains outputs and get those outputs", () => { + const cfnTemplate = includeTestTemplate(stack, 'outputs.json'); + const output = cfnTemplate.getOutput('Output1'); + + expect(output.getDescription()).toBe("the description of the output value"); + expect(output.getValue()).toBe("the output value"); + expect(output.getExport()).toBe("the export"); + expect(output.getCondition()).toBe(cfnTemplate.getCondition('AlwaysFalseCond')); + }); + + test('can ingest a template with outputs that reference resources', () => { + const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); + + expect(stack).toMatchTemplate(loadTestFileToJsObject('outputs-with-references.json')); + expect(() => { + cfnTemplate.getOutput('FakeOutput'); + }).toThrow(/Output with logical ID 'FakeOutput' was not found in the template/); + }); }); interface IncludeTestTemplateProps { diff --git a/packages/@aws-cdk/core/lib/cfn-output.ts b/packages/@aws-cdk/core/lib/cfn-output.ts index d99622badb1ef..6806c456ed01e 100644 --- a/packages/@aws-cdk/core/lib/cfn-output.ts +++ b/packages/@aws-cdk/core/lib/cfn-output.ts @@ -36,10 +36,10 @@ export interface CfnOutputProps { } export class CfnOutput extends CfnElement { - private readonly _description?: string; - private readonly _condition?: CfnCondition; - private readonly _value?: any; - private readonly _export?: string; + private _description?: string; + private _condition?: CfnCondition; + private _value?: any; + private _export?: string; /** * Creates an CfnOutput value for this stack. @@ -59,6 +59,38 @@ export class CfnOutput extends CfnElement { this._export = props.exportName; } + public getDescription(): string | undefined { + return this._description; + } + + public getValue(): any { + return this._value; + } + + public getCondition(): CfnCondition | undefined { + return this._condition; + } + + public getExport(): string | undefined { + return this._export; + } + + public setDescription(newDescription: string | undefined): void { + this._description = newDescription; + } + + setValue(newValue: any): void { + this._value = newValue; + } + + public setCondition(newCondition: CfnCondition | undefined): void { + this._condition = newCondition; + } + + public setExport(newExport: string): void { + this._export = newExport; + } + /** * @internal */ From 34d5c34a90e7ada88d20ae6e7c8b34ab6e2b2452 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 17:53:10 -0400 Subject: [PATCH 22/46] fixed linter issues --- .../cloudformation-include/lib/cfn-include.ts | 13 ++++++------- .../test/valid-templates.test.ts | 3 +-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 879c5b5082402..e186a36d55ccc 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -2,7 +2,6 @@ import * as core from '@aws-cdk/core'; import * as cfn_parse from '@aws-cdk/core/lib/cfn-parse'; import * as cfn_type_to_l1_mapping from './cfn-type-to-l1-mapping'; import * as futils from './file-utils'; -import { CfnCondition } from '@aws-cdk/core'; /** * Construction properties of {@link CfnInclude}. @@ -123,15 +122,15 @@ export class CfnInclude extends core.CfnElement { * section of the included template * Any modifications performed on that object will be reflected in the resulting CDK template. * - * If a Parameter with the given name is not present in the template, + * If an Output with the given name is not present in the template, * throws an exception. * - * @param outputName the name of the output to retrieve + * @param logicalId the name of the output to retrieve */ - public getOutput(outputName: string): core.CfnOutput { - const ret = this.outputs[outputName]; + public getOutput(logicalId: string): core.CfnOutput { + const ret = this.outputs[logicalId]; if (!ret) { - throw new Error(`Output with logical ID '${outputName}' was not found in the template`); + throw new Error(`Output with logical ID '${logicalId}' was not found in the template`); } return ret; } @@ -202,7 +201,7 @@ export class CfnInclude extends core.CfnElement { value: expression.Value, description: expression.Description, exportName: expression.Export, - condition: expression.Condition ? self.getCondition(expression.Condition) as CfnCondition : undefined + condition: expression.Condition ? self.getCondition(expression.Condition) : undefined, }); cfnOutput.overrideLogicalId(logicalId); diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 2032fe6ec473f..5895d1ba68b09 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -6,7 +6,6 @@ import * as core from '@aws-cdk/core'; import * as path from 'path'; import * as inc from '../lib'; import * as futils from '../lib/file-utils'; -import { CfnCondition } from '@aws-cdk/core'; // tslint:disable:object-literal-key-quotes /* eslint-disable quotes */ @@ -473,7 +472,7 @@ describe('CDK Include', () => { const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); expect(stack).toMatchTemplate(loadTestFileToJsObject('outputs-with-references.json')); - expect(() => { + expect(() => { cfnTemplate.getOutput('FakeOutput'); }).toThrow(/Output with logical ID 'FakeOutput' was not found in the template/); }); From 95badbeb3304c25660c1a42fd0943c497eb219c0 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 17:56:16 -0400 Subject: [PATCH 23/46] updated README --- packages/@aws-cdk/cloudformation-include/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 68f7b0e2a8b12..17d009e001f11 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -135,7 +135,7 @@ All items unchecked below are currently not supported. - [x] Resources - [x] Parameters - [x] Conditions -- [ ] Outputs +- [x] Outputs ### [Resource attributes](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-product-attribute-reference.html): From fa39d72177a3d50c41c8c191e1b4edcb931a58a9 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 18:06:23 -0400 Subject: [PATCH 24/46] updated documentation --- .../cloudformation-include/lib/cfn-include.ts | 1 - packages/@aws-cdk/core/lib/cfn-output.ts | 30 ++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index e186a36d55ccc..ebc6d7360960a 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -189,7 +189,6 @@ export class CfnInclude extends core.CfnElement { if (elementName in self.parameters) { return self.parameters[elementName]; } - return this.findResource(elementName); }, findCondition(): undefined { diff --git a/packages/@aws-cdk/core/lib/cfn-output.ts b/packages/@aws-cdk/core/lib/cfn-output.ts index 6806c456ed01e..a66308592ef48 100644 --- a/packages/@aws-cdk/core/lib/cfn-output.ts +++ b/packages/@aws-cdk/core/lib/cfn-output.ts @@ -59,34 +59,62 @@ export class CfnOutput extends CfnElement { this._export = props.exportName; } + /** + * Returns the description of this Output + */ public getDescription(): string | undefined { return this._description; } + /** + * Returns the value of this Output + */ public getValue(): any { return this._value; } + /** + * Returns the condition of this Output + */ public getCondition(): CfnCondition | undefined { return this._condition; } + /** + * Returns the export of this Output + */ public getExport(): string | undefined { return this._export; } + /** + * Sets this output's description to the parameter + * @param newDescription the description to update this Output's description to + */ public setDescription(newDescription: string | undefined): void { this._description = newDescription; } - setValue(newValue: any): void { + /** + * Sets this output's value to the parameter + * @param newValue the value to update this Output's value to + */ + public setValue(newValue: any): void { this._value = newValue; } + /** + * Sets this output's condition to the parameter + * @param newCondition the condition to update this Output's condition to + */ public setCondition(newCondition: CfnCondition | undefined): void { this._condition = newCondition; } + /** + * Sets this output's export to the parameter + * @param newRxport the export to update this Output's export to + */ public setExport(newExport: string): void { this._export = newExport; } From a3940e340ee33e982e75fed1feccb6f571f1787a Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 18:11:44 -0400 Subject: [PATCH 25/46] removed unneeded line in tests --- .../@aws-cdk/cloudformation-include/test/valid-templates.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 5895d1ba68b09..ce44d8ceb3d73 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -420,7 +420,6 @@ describe('CDK Include', () => { const output = cfnTemplate.getOutput('Output1'); const bucket = cfnTemplate.getResource('Bucket'); - output.getValue(); output.setValue('a mutated value'); output.setDescription(undefined); output.setExport("an export"); From fbf17f2f2df09d80f04ea5a7a483d2c2b49ae0d3 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 18:13:36 -0400 Subject: [PATCH 26/46] added newline --- .../test/test-templates/outputs-with-references.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json index 80518492651e1..8742eb36a819f 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json @@ -39,4 +39,4 @@ "Condition": "AlwaysFalseCond" } } -} \ No newline at end of file +} From c719e85908198fc43febd0cb2203a024334d819b Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 30 Jun 2020 21:41:42 -0400 Subject: [PATCH 27/46] incorporated PR requests --- .../@aws-cdk/cloudformation-include/README.md | 18 +++++++ .../cloudformation-include/lib/cfn-include.ts | 30 +++++------ .../outputs-with-references.json | 23 ++++---- .../test/test-templates/outputs.json | 28 ---------- .../test/valid-templates.test.ts | 54 ++++++++++--------- packages/@aws-cdk/core/lib/cfn-output.ts | 48 ++++++++--------- 6 files changed, 91 insertions(+), 110 deletions(-) delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 17d009e001f11..1547228dafcfc 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -124,6 +124,24 @@ and any changes you make to it will be reflected in the resulting template: condition.expression = core.Fn.conditionEquals(1, 2); ``` +## Outputs + +If your template uses [CloudFormation Outputs](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/outputs-section-structure.html), +you can retrieve them from your template: + +```typescript +import * as core from '@aws-cdk/core'; + +const output: core.CfnOutput = cfnTemplate.getOutput('MyOutput'); +``` + +The `CfnOutput` object is mutable, +and any changes you make to it will be reflected in the resulting template: + +```typescript +output.value = "some new value"); +``` + ## Known limitations This module is still in its early, experimental stage, diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index ebc6d7360960a..9f37f7b90ffb1 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -47,14 +47,14 @@ export class CfnInclude extends core.CfnElement { this.createCondition(conditionName); } - for (const logicalId of Object.keys(this.template.Outputs || {})) { - this.createOutput(logicalId); - } - // instantiate all resources as CDK L1 objects for (const logicalId of Object.keys(this.template.Resources || {})) { this.getOrCreateResource(logicalId); } + + for (const logicalId of Object.keys(this.template.Outputs || {})) { + this.createOutput(logicalId); + } } /** @@ -177,19 +177,13 @@ export class CfnInclude extends core.CfnElement { private createOutput(logicalId: string): void { const self = this; - const expression = new cfn_parse.CfnParser({ + const outputAttributes = new cfn_parse.CfnParser({ finder: { - findResource(lId: string): core.CfnResource | undefined { - if (!(lId in (self.template.Resources || {}))) { - return undefined; - } - return self.getOrCreateResource(lId); + findResource(lId): core.CfnResource | undefined { + return self.resources[lId]; }, findRefTarget(elementName: string): core.CfnElement | undefined { - if (elementName in self.parameters) { - return self.parameters[elementName]; - } - return this.findResource(elementName); + return self.resources[elementName] ?? self.parameters[elementName]; }, findCondition(): undefined { return undefined; @@ -197,10 +191,10 @@ export class CfnInclude extends core.CfnElement { }, }).parseValue(this.template.Outputs[logicalId]); const cfnOutput = new core.CfnOutput(this, logicalId, { - value: expression.Value, - description: expression.Description, - exportName: expression.Export, - condition: expression.Condition ? self.getCondition(expression.Condition) : undefined, + value: outputAttributes.Value, + description: outputAttributes.Description, + exportName: outputAttributes.Export, + condition: outputAttributes.Condition ? self.getCondition(outputAttributes.Condition) : undefined, }); cfnOutput.overrideLogicalId(logicalId); diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json index 8742eb36a819f..25666c8354155 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json @@ -16,27 +16,22 @@ }, "Resources": { "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": "my-referenced-bucket-name" - }, - "Condition": "AlwaysFalseCond" + "Type": "AWS::S3::Bucket" } }, "Outputs": { "Output1": { - "Value": { "Ref": "Bucket" }, - "Description": { "Ref": "MyParam" }, - "Condition": "AlwaysFalseCond" - }, - "Output2": { "Value": { - "Fn::GetAtt": [ - "Bucket", - "Arn" + "Fn::Join": [ "", [ + { "Ref": "MyParam" }, + { "Fn::GetAtt": [ "Bucket", "Arn" ] } + ] ] }, + "Description": { + "Ref": "Bucket" + }, "Condition": "AlwaysFalseCond" } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json deleted file mode 100644 index b2343a4ce3145..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "Conditions": { - "AlwaysFalseCond": { - "Fn::Equals": [ - { - "Ref": "AWS::Region" - }, - "completely-made-up-region" - ] - } - }, - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": "my-referenced-bucket-name" - } - } - }, - "Outputs": { - "Output1": { - "Description": "the description of the output value", - "Value": "the output value", - "Export": "the export", - "Condition": "AlwaysFalseCond" - } - } -} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index ce44d8ceb3d73..161360734d279 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -396,8 +396,8 @@ describe('CDK Include', () => { ], }, }, - "Metadata" : { - "Object1" : "Location1", + "Metadata": { + "Object1": "Location1", "KeyRef": { "Ref": "TotallyDifferentKey" }, "KeyArn": { "Fn::GetAtt": ["TotallyDifferentKey", "Arn"] }, }, @@ -416,25 +416,36 @@ describe('CDK Include', () => { }); test("can ingest a template that contains outputs and modify them", () => { - const cfnTemplate = includeTestTemplate(stack, 'outputs.json'); + const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); const output = cfnTemplate.getOutput('Output1'); - const bucket = cfnTemplate.getResource('Bucket'); - output.setValue('a mutated value'); - output.setDescription(undefined); - output.setExport("an export"); - output.setCondition(new core.CfnCondition(stack, 'MyCondition', {})); + const ifCond = core.Fn.conditionIf("AlwaysFalseCond", 'AWS::NoValue', 'AWS::NoValue'); - new core.CfnOutput(stack, 'Output2', { - value: bucket.ref, - description: "a description", + output.value = 'a mutated value'; + output.description = undefined; + output.exportName = "an export"; + output.condition = new core.CfnCondition(stack, 'MyCondition', { + expression: ifCond, }); - const originalTemplate = loadTestFileToJsObject('outputs.json'); + const originalTemplate = loadTestFileToJsObject('outputs-with-references.json'); expect(stack).toMatchTemplate({ "Conditions": { ...originalTemplate.Conditions, + "MyCondition": { + "Fn::If": [ + "AlwaysFalseCond", + "AWS::NoValue", + "AWS::NoValue", + ], + }, + }, + "Parameters": { + ...originalTemplate.Parameters, + }, + "Resources": { + ...originalTemplate.Resources, }, "Outputs": { "Output1": { @@ -444,27 +455,18 @@ describe('CDK Include', () => { }, "Condition": "MyCondition", }, - "Output2": { - "Value": { - "Ref": "Bucket", - }, - "Description": "a description", - }, - }, - "Resources": { - ...originalTemplate.Resources, }, }); }); test("can ingest a template that contains outputs and get those outputs", () => { - const cfnTemplate = includeTestTemplate(stack, 'outputs.json'); + const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); const output = cfnTemplate.getOutput('Output1'); - expect(output.getDescription()).toBe("the description of the output value"); - expect(output.getValue()).toBe("the output value"); - expect(output.getExport()).toBe("the export"); - expect(output.getCondition()).toBe(cfnTemplate.getCondition('AlwaysFalseCond')); + expect(output.condition).toBe(cfnTemplate.getCondition('AlwaysFalseCond')); + expect(output.description).toBeDefined(); + expect(output.value).toBeDefined(); + expect(output.exportName).toBeUndefined(); }); test('can ingest a template with outputs that reference resources', () => { diff --git a/packages/@aws-cdk/core/lib/cfn-output.ts b/packages/@aws-cdk/core/lib/cfn-output.ts index a66308592ef48..9d988ec9a6d92 100644 --- a/packages/@aws-cdk/core/lib/cfn-output.ts +++ b/packages/@aws-cdk/core/lib/cfn-output.ts @@ -62,61 +62,61 @@ export class CfnOutput extends CfnElement { /** * Returns the description of this Output */ - public getDescription(): string | undefined { + public get description() { return this._description; } /** - * Returns the value of this Output + * Sets this output's description to the parameter + * @param newDescription the description to update this Output's description to */ - public getValue(): any { - return this._value; + public set description(description: string | undefined) { + this._description = description; } /** - * Returns the condition of this Output + * Returns the value of this Output */ - public getCondition(): CfnCondition | undefined { - return this._condition; + public get value() { + return this._value; } /** - * Returns the export of this Output + * Sets this output's value to the parameter + * @param newValue the value to update this Output's value to */ - public getExport(): string | undefined { - return this._export; + public set value(value: any) { + this._value = value; } /** - * Sets this output's description to the parameter - * @param newDescription the description to update this Output's description to + * Returns the condition of this Output */ - public setDescription(newDescription: string | undefined): void { - this._description = newDescription; + public get condition() { + return this._condition; } /** - * Sets this output's value to the parameter - * @param newValue the value to update this Output's value to + * Sets this output's condition to the parameter + * @param newCondition the condition to update this Output's condition to */ - public setValue(newValue: any): void { - this._value = newValue; + public set condition(condition: CfnCondition | undefined) { + this._condition = condition; } /** - * Sets this output's condition to the parameter - * @param newCondition the condition to update this Output's condition to + * Returns the export of this Output */ - public setCondition(newCondition: CfnCondition | undefined): void { - this._condition = newCondition; + public get exportName() { + return this._export; } /** * Sets this output's export to the parameter * @param newRxport the export to update this Output's export to */ - public setExport(newExport: string): void { - this._export = newExport; + public set exportName(exportName: string | undefined) { + this._export = exportName; } /** From f05aae1b3883c3a7d9f19ecad8f4bcb01bc55574 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 1 Jul 2020 12:01:39 -0400 Subject: [PATCH 28/46] updated the example in the readme --- packages/@aws-cdk/cloudformation-include/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 1547228dafcfc..01be06067dde4 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -139,7 +139,7 @@ The `CfnOutput` object is mutable, and any changes you make to it will be reflected in the resulting template: ```typescript -output.value = "some new value"); +output.value = cfnBucket.attrArn; ``` ## Known limitations From 69223d2acf488eb02095ae0aad465c86e0265179 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 1 Jul 2020 15:19:46 -0400 Subject: [PATCH 29/46] added support for common-named outputs. Fixed a bug in the export name output --- .../cloudformation-include/lib/cfn-include.ts | 10 ++++++---- .../outputs-with-references.json | 8 +++++++- .../test/valid-templates.test.ts | 2 +- packages/@aws-cdk/core/lib/cfn-output.ts | 18 +++++++++--------- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 9f37f7b90ffb1..1805818863f5f 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -52,8 +52,10 @@ export class CfnInclude extends core.CfnElement { this.getOrCreateResource(logicalId); } + const outputScope = new core.Construct(this, '$Ouputs'); + for (const logicalId of Object.keys(this.template.Outputs || {})) { - this.createOutput(logicalId); + this.createOutput(logicalId, outputScope); } } @@ -175,7 +177,7 @@ export class CfnInclude extends core.CfnElement { this.parameters[logicalId] = cfnParameter; } - private createOutput(logicalId: string): void { + private createOutput(logicalId: string, scope: core.Construct): void { const self = this; const outputAttributes = new cfn_parse.CfnParser({ finder: { @@ -190,10 +192,10 @@ export class CfnInclude extends core.CfnElement { }, }, }).parseValue(this.template.Outputs[logicalId]); - const cfnOutput = new core.CfnOutput(this, logicalId, { + const cfnOutput = new core.CfnOutput(scope, logicalId, { value: outputAttributes.Value, description: outputAttributes.Description, - exportName: outputAttributes.Export, + exportName: outputAttributes.Export ? outputAttributes.Export.Name: undefined, condition: outputAttributes.Condition ? self.getCondition(outputAttributes.Condition) : undefined, }); diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json index 25666c8354155..48197c1938c8e 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json @@ -17,6 +17,9 @@ "Resources": { "Bucket": { "Type": "AWS::S3::Bucket" + }, + "Output1": { + "Type": "AWS::S3::Bucket" } }, "Outputs": { @@ -31,7 +34,10 @@ "Description": { "Ref": "Bucket" }, - "Condition": "AlwaysFalseCond" + "Condition": "AlwaysFalseCond", + "Export": { + "Name": "Bucket" + } } } } \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 161360734d279..f28029d9cd396 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -466,7 +466,7 @@ describe('CDK Include', () => { expect(output.condition).toBe(cfnTemplate.getCondition('AlwaysFalseCond')); expect(output.description).toBeDefined(); expect(output.value).toBeDefined(); - expect(output.exportName).toBeUndefined(); + expect(output.exportName).toBeDefined(); }); test('can ingest a template with outputs that reference resources', () => { diff --git a/packages/@aws-cdk/core/lib/cfn-output.ts b/packages/@aws-cdk/core/lib/cfn-output.ts index 9d988ec9a6d92..387c5216ead91 100644 --- a/packages/@aws-cdk/core/lib/cfn-output.ts +++ b/packages/@aws-cdk/core/lib/cfn-output.ts @@ -39,7 +39,7 @@ export class CfnOutput extends CfnElement { private _description?: string; private _condition?: CfnCondition; private _value?: any; - private _export?: string; + private _exportName?: string; /** * Creates an CfnOutput value for this stack. @@ -56,7 +56,7 @@ export class CfnOutput extends CfnElement { this._description = props.description; this._value = props.value; this._condition = props.condition; - this._export = props.exportName; + this._exportName = props.exportName; } /** @@ -68,7 +68,7 @@ export class CfnOutput extends CfnElement { /** * Sets this output's description to the parameter - * @param newDescription the description to update this Output's description to + * @param description the description to update this Output's description to */ public set description(description: string | undefined) { this._description = description; @@ -83,7 +83,7 @@ export class CfnOutput extends CfnElement { /** * Sets this output's value to the parameter - * @param newValue the value to update this Output's value to + * @param value the value to update this Output's value to */ public set value(value: any) { this._value = value; @@ -98,7 +98,7 @@ export class CfnOutput extends CfnElement { /** * Sets this output's condition to the parameter - * @param newCondition the condition to update this Output's condition to + * @param condition the condition to update this Output's condition to */ public set condition(condition: CfnCondition | undefined) { this._condition = condition; @@ -108,15 +108,15 @@ export class CfnOutput extends CfnElement { * Returns the export of this Output */ public get exportName() { - return this._export; + return this._exportName; } /** * Sets this output's export to the parameter - * @param newRxport the export to update this Output's export to + * @param exportName the export to update this Output's export to */ public set exportName(exportName: string | undefined) { - this._export = exportName; + this._exportName = exportName; } /** @@ -128,7 +128,7 @@ export class CfnOutput extends CfnElement { [this.logicalId]: { Description: this._description, Value: this._value, - Export: this._export != null ? { Name: this._export } : undefined, + Export: this._exportName != null ? { Name: this._exportName } : undefined, Condition: this._condition ? this._condition.logicalId : undefined, }, }, From 0c551800738d13aa5ef94928dac376a1ec6c1439 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 1 Jul 2020 17:41:47 -0400 Subject: [PATCH 30/46] added a negative test case and a new error message if an output references a nonexistant condition --- .../cloudformation-include/lib/cfn-include.ts | 13 +++++++++-- .../test/invalid-templates.test.ts | 6 +++++ ...put-referencing-nonexistant-condition.json | 7 ++++++ .../outputs-with-references.json | 8 ++++--- .../test/valid-templates.test.ts | 22 +++++++++++-------- 5 files changed, 42 insertions(+), 14 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/output-referencing-nonexistant-condition.json diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 1805818863f5f..035631fd282d5 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -195,8 +195,17 @@ export class CfnInclude extends core.CfnElement { const cfnOutput = new core.CfnOutput(scope, logicalId, { value: outputAttributes.Value, description: outputAttributes.Description, - exportName: outputAttributes.Export ? outputAttributes.Export.Name: undefined, - condition: outputAttributes.Condition ? self.getCondition(outputAttributes.Condition) : undefined, + exportName: outputAttributes.Export ? outputAttributes.Export.Name : undefined, + condition: (() => { + if (!outputAttributes.Condition) { + return undefined; + } else if (this.conditions[outputAttributes.Condition]) { + return self.getCondition(outputAttributes.Condition); + } + + throw new Error(`Output with name '${logicalId}' refers to a Condition with name\ + '${outputAttributes.Condition}' which was not found in this template`); + })(), }); cfnOutput.overrideLogicalId(logicalId); diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index eb04ef059e15e..0d1ba70795e5d 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -76,6 +76,12 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'getting-attribute-of-a-non-existent-resource.json'); }).toThrow(/Resource used in GetAtt expression with logical ID: 'DoesNotExist' not found/); }); + + test("throws a validation exception when an output references a condition that doesn't exist", () => { + expect(() => { + includeTestTemplate(stack, 'output-referencing-nonexistant-condition.json'); + }).toThrow(/Output with name 'SomeOutput' refers to a Condition with name 'NonexistantCondition' which was not found in this template/); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/output-referencing-nonexistant-condition.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/output-referencing-nonexistant-condition.json new file mode 100644 index 0000000000000..fbb06694b8ae6 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/output-referencing-nonexistant-condition.json @@ -0,0 +1,7 @@ +{ + "Outputs": { + "SomeOutput": { + "Condition": "NonexistantCondition" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json index 48197c1938c8e..206b1d1d15009 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/outputs-with-references.json @@ -26,9 +26,8 @@ "Output1": { "Value": { "Fn::Join": [ "", [ - { "Ref": "MyParam" }, - { "Fn::GetAtt": [ "Bucket", "Arn" ] } - ] + { "Ref": "MyParam" }, + { "Fn::GetAtt": [ "Bucket", "Arn" ] } ] ] }, "Description": { @@ -38,6 +37,9 @@ "Export": { "Name": "Bucket" } + }, + "OutputWithNoCondition": { + "Value": "some-value" } } } \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index f28029d9cd396..13b7d050970a0 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -251,7 +251,7 @@ describe('CDK Include', () => { ); }); - test("correctly parses templates with parameters", () => { + test('correctly parses templates with parameters', () => { const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); const param = cfnTemplate.getParameter('BucketName'); new s3.CfnBucket(stack, 'NewBucket', { @@ -415,17 +415,15 @@ describe('CDK Include', () => { }).toThrow(/Unrecognized CloudFormation resource type: 'AWS::FakeService::DoesNotExist'/); }); - test("can ingest a template that contains outputs and modify them", () => { + test('can ingest a template that contains outputs and modify them', () => { const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); - const output = cfnTemplate.getOutput('Output1'); - - const ifCond = core.Fn.conditionIf("AlwaysFalseCond", 'AWS::NoValue', 'AWS::NoValue'); + const output = cfnTemplate.getOutput('Output1'); output.value = 'a mutated value'; output.description = undefined; - output.exportName = "an export"; + output.exportName = 'an export'; output.condition = new core.CfnCondition(stack, 'MyCondition', { - expression: ifCond, + expression: core.Fn.conditionIf('AlwaysFalseCond', 'AWS::NoValue', 'AWS::NoValue'), }); const originalTemplate = loadTestFileToJsObject('outputs-with-references.json'); @@ -455,11 +453,14 @@ describe('CDK Include', () => { }, "Condition": "MyCondition", }, + "OutputWithNoCondition": { + "Value": "some-value", + }, }, }); }); - test("can ingest a template that contains outputs and get those outputs", () => { + test('can ingest a template that contains outputs and get those outputs', () => { const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); const output = cfnTemplate.getOutput('Output1'); @@ -467,12 +468,15 @@ describe('CDK Include', () => { expect(output.description).toBeDefined(); expect(output.value).toBeDefined(); expect(output.exportName).toBeDefined(); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('outputs-with-references.json'), + ); }); test('can ingest a template with outputs that reference resources', () => { const cfnTemplate = includeTestTemplate(stack, 'outputs-with-references.json'); - expect(stack).toMatchTemplate(loadTestFileToJsObject('outputs-with-references.json')); expect(() => { cfnTemplate.getOutput('FakeOutput'); }).toThrow(/Output with logical ID 'FakeOutput' was not found in the template/); From 0541ded240f67b643fd01e4836ccc5b87ba73137 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 23 Jul 2020 17:35:16 -0400 Subject: [PATCH 31/46] added fn::sub now turns into fn::join in cfnInclude --- .../test/integ.fn-sub.expected.json | 27 +++++++++++++ .../test/integ.fn-sub.ts | 12 ++++++ .../test/invalid-templates.test.ts | 18 +++++++++ .../test/test-templates/fn-sub-shadow.json | 16 ++++++++ .../test-templates/fn-sub-valid-string.json | 16 ++++++++ .../test/test-templates/fn-sub.json | 16 ++++++++ .../invalid/fn-sub-invalid-key-string.json | 10 +++++ .../invalid/fn-sub-invalid-key.json | 10 +++++ .../invalid/fn-sub-invalid-syntax.json | 10 +++++ .../test/valid-templates.test.ts | 16 ++++++++ packages/@aws-cdk/core/lib/cfn-parse.ts | 38 +++++++++++++++++-- 11 files changed, 186 insertions(+), 3 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json diff --git a/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json b/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json new file mode 100644 index 0000000000000..f49839cfa5cdc --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json @@ -0,0 +1,27 @@ +{ + "Resources": { + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}" + } + } + }, + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::Sub": [ + "unusued-different-sub-${Name}", + { + "Name": { + "Ref": "AnotherBucket" + } + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts b/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts new file mode 100644 index 0000000000000..5337c3df82705 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts @@ -0,0 +1,12 @@ +import * as core from '@aws-cdk/core'; +import * as inc from '../lib'; + +const app = new core.App(); + +const stack = new core.Stack(app, 'SubStack'); + +new inc.CfnInclude(stack, 'ParentStack', { + templateFile: 'test-templates/fn-sub.json', +}); + +app.synth(); diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index ef93cd9809d82..eb9075cb11cc7 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -100,6 +100,24 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'output-referencing-nonexistant-condition.json'); }).toThrow(/Output with name 'SomeOutput' refers to a Condition with name 'NonexistantCondition' which was not found in this template/); }); + + /*test("throws a validation exception when Fn::Sub uses a key that isn't in the template", () => { + expect(() => { + includeTestTemplate(stack, 'fn-sub-invalid-key.json'); + }).toThrow(/Element used in Ref expression with logical ID: 'FakeResource' not found/); + });*/ + + /*test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => { + expect(() => { + includeTestTemplate(stack, 'fn-sub-invalid-key-string.json'); + }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); + }); + + test("throws a validation exception when Fn::Sub in string form has invalid syntax", () => { + expect(() => { + includeTestTemplate(stack, 'fn-sub-invalid-syntax.json'); + }).toThrow(/variable names in Fn::Sub syntax must contain only alphanumeric characters, underscores, periods, and colons/); + });*/ }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json new file mode 100644 index 0000000000000..c501ca07e7270 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json @@ -0,0 +1,16 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": [ "unusued-different-sub-${Name}", { "AnotherBucket": {"Ref" : "AnotherBucket" }} ]} + } + }, + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${!Literal}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json new file mode 100644 index 0000000000000..8bde3d18a9d00 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json @@ -0,0 +1,16 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": "some-bucket1231232343453452234" + } + }, + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${Bucket}" } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json new file mode 100644 index 0000000000000..51e266b452489 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json @@ -0,0 +1,16 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": [ "unusued-different-sub-${Name}", { "Name": {"Ref" : "AnotherBucket" }} ]} + } + }, + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json new file mode 100644 index 0000000000000..5e148cc9780c9 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json @@ -0,0 +1,10 @@ +{ + "Resources": { + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "AccessControl": { "Fn::Sub": "unusued-bucket-fn-sub-${ AFakeResource}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json new file mode 100644 index 0000000000000..5b0fc58be3a45 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json @@ -0,0 +1,10 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": [ "unusued-different-sub-${Name}", { "Name": {"Ref" : "AFakeResource" }} ]} + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json new file mode 100644 index 0000000000000..aa79385fbef47 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json @@ -0,0 +1,10 @@ +{ + "Resources": { + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "AccessControl": { "Fn::Sub": "unusued-bucket-fn-sub-${ x ! AFakeResource}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 912525b7c369f..766df32c9daa3 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -192,6 +192,22 @@ describe('CDK Include', () => { ); }); + test('can ingest a template with Fn::Sub in string form and output it unchanged', () => { + includeTestTemplate(stack, 'fn-sub-valid-string.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('fn-sub-shadow.json'), + ); + }); + + /*test('can ingest a template with Fn::Sub with a key that is a logicalID and a literal and output it unchanged', () => { + includeTestTemplate(stack, 'fn-sub-shadow.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('fn-sub-shadow.json'), + ); + });*/ + test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 6c880b80cd420..13cb9ded59fee 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -10,6 +10,7 @@ import { ICfnFinder } from './from-cfn'; import { CfnReference } from './private/cfn-reference'; import { IResolvable } from './resolvable'; import { isResolvableObject, Token } from './token'; +import { CfnRefElement } from './cfn-element'; /** * This class contains static methods called when going from @@ -111,7 +112,7 @@ export class FromCloudFormation { public static getCfnTag(tag: any): CfnTag { return tag == null - ? { } as any // break the type system - this should be detected at runtime by a tag validator + ? {} as any // break the type system - this should be detected at runtime by a tag validator : { key: tag.Key, value: tag.Value, @@ -419,6 +420,37 @@ export class CfnParser { const value = this.parseValue(object[key]); return Fn.conditionOr(...value); } + case 'Fn::Sub': { + const value = this.parseValue(object[key]); + if (typeof value === 'object') { + console.log(value); + } + if (typeof value === 'string') { + if (value.indexOf('!') !== -1) { + // loop from the '{' to the '!', eg "${ ! ..." + for (let i = value.indexOf('${') + 2; i < value.indexOf('!'); i++) { + if (value[i] !== ' ') { + throw new Error('variable names in Fn::Sub syntax must contain only alphanumeric characters, underscores, periods, and colons'); + } + } + break; + } + const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); + const specialRef = specialCaseRefs(refTarget); + if (specialRef) { + return specialRef; + } else { + const refElement = this.options.finder.findRefTarget(refTarget); + if (!refElement) { + throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + } + //this.parseIfCfnIntrinsic(refTarget); + console.log('refTarget === ' + refTarget); + return "my-string" + CfnReference.for(refElement, 'Ref') + } + } + return "my-other-string"; + } case 'Condition': { // a reference to a Condition from another Condition const condition = this.options.finder.findCondition(object[key]); @@ -441,8 +473,8 @@ export class CfnParser { const key = objectKeys[0]; return key === 'Ref' || key.startsWith('Fn::') || - // special intrinsic only available in the 'Conditions' section - (this.options.context === CfnParsingContext.CONDITIONS && key === 'Condition') + // special intrinsic only available in the 'Conditions' section + (this.options.context === CfnParsingContext.CONDITIONS && key === 'Condition') ? key : undefined; } From f7f6a258d9d103a81f68347e326428cae8fc7d08 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 23 Jul 2020 19:02:59 -0400 Subject: [PATCH 32/46] added a new way to resolve tokens --- .../test/invalid-templates.test.ts | 4 ++-- packages/@aws-cdk/core/lib/cfn-parse.ts | 4 +--- packages/@aws-cdk/core/lib/private/cfn-reference.ts | 9 ++++++++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index eb9075cb11cc7..d56ec49313472 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -107,7 +107,7 @@ describe('CDK Include', () => { }).toThrow(/Element used in Ref expression with logical ID: 'FakeResource' not found/); });*/ - /*test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => { + test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => { expect(() => { includeTestTemplate(stack, 'fn-sub-invalid-key-string.json'); }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); @@ -117,7 +117,7 @@ describe('CDK Include', () => { expect(() => { includeTestTemplate(stack, 'fn-sub-invalid-syntax.json'); }).toThrow(/variable names in Fn::Sub syntax must contain only alphanumeric characters, underscores, periods, and colons/); - });*/ + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 13cb9ded59fee..178e61ae87a43 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -444,9 +444,7 @@ export class CfnParser { if (!refElement) { throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } - //this.parseIfCfnIntrinsic(refTarget); - console.log('refTarget === ' + refTarget); - return "my-string" + CfnReference.for(refElement, 'Ref') + return Fn.sub( value.substring(0, value.indexOf('$')) + CfnReference.for(refElement, 'Sub')); } } return "my-other-string"; diff --git a/packages/@aws-cdk/core/lib/private/cfn-reference.ts b/packages/@aws-cdk/core/lib/private/cfn-reference.ts index cd28a221958c5..b6a60324bc9f6 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-reference.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-reference.ts @@ -36,7 +36,14 @@ export class CfnReference extends Reference { */ public static for(target: CfnElement, attribute: string) { return CfnReference.singletonReference(target, attribute, () => { - const cfnIntrinsic = attribute === 'Ref' ? { Ref: target.logicalId } : { 'Fn::GetAtt': [ target.logicalId, attribute ]}; + let cfnIntrinsic; + if (attribute === 'Ref') { + cfnIntrinsic = { Ref: target.logicalId }; + } else if (attribute === 'Sub') { + cfnIntrinsic = `\$\{${target.logicalId}\}`; + } else { + cfnIntrinsic = { 'Fn::GetAtt': [ target.logicalId, attribute ]}; + } return new CfnReference(cfnIntrinsic, attribute, target); }); } From 344d3e9f21c036b5865c18a6b837f521542f51ab Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 24 Jul 2020 11:09:23 -0400 Subject: [PATCH 33/46] we now support pseudo parameters --- .../test-templates/fn-sub-valid-string.json | 2 +- packages/@aws-cdk/core/lib/cfn-parse.ts | 52 +++++++++++++++++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json index 8bde3d18a9d00..0660fa0771a96 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json @@ -9,7 +9,7 @@ "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${Bucket}" } + "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}-${AWS::Region}" } } } } diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 178e61ae87a43..aabb9ac5dba7f 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -10,7 +10,6 @@ import { ICfnFinder } from './from-cfn'; import { CfnReference } from './private/cfn-reference'; import { IResolvable } from './resolvable'; import { isResolvableObject, Token } from './token'; -import { CfnRefElement } from './cfn-element'; /** * This class contains static methods called when going from @@ -435,16 +434,29 @@ export class CfnParser { } break; } + + /*let openIndices = [], closedIndicies = []; + for (let i = 0; i < value.length; i++) { + if (value[i] === "$" && value[i+1] === '{') { + openIndices.push(i); + } else if (value[i] === '}') { + closedIndicies.push(i); + } + } + for (let index of openIndices) { + + }*/ const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); - const specialRef = specialCaseRefs(refTarget); + const specialRef = specialCaseSubRefs(refTarget); if (specialRef) { - return specialRef; + console.log(specialRef) + return Fn.sub(value.substring(0, value.indexOf('$')) + specialRef); } else { const refElement = this.options.finder.findRefTarget(refTarget); if (!refElement) { throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } - return Fn.sub( value.substring(0, value.indexOf('$')) + CfnReference.for(refElement, 'Sub')); + return Fn.sub(value.substring(0, value.indexOf('$')) + CfnReference.for(refElement, 'Sub')); } } return "my-other-string"; @@ -476,8 +488,25 @@ export class CfnParser { ? key : undefined; } + + private parseSubRef(value: string): string { + //TODO need base case + const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); + const specialRef = specialCaseSubRefs(refTarget); + if (specialRef) { + console.log(specialRef) + return Fn.sub(value.substring(0, value.indexOf('$')) + specialRef); + } else { + const refElement = this.options.finder.findRefTarget(refTarget); + if (!refElement) { + throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + } + return CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}'))); + } + } } + function specialCaseRefs(value: any): any { switch (value) { case 'AWS::AccountId': return Aws.ACCOUNT_ID; @@ -492,6 +521,21 @@ function specialCaseRefs(value: any): any { } } +function specialCaseSubRefs(value: any): any { + switch (value) { + case 'AWS::AccountId': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::Region': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::Partition': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::URLSuffix': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::NotificationARNs': return Token.asList(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::StackId': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::StackName': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::NoValue': return Token.asString(`\$\{${value}\}`, { displayHint: value }); + case 'AWS::NoValue': return Aws.NO_VALUE; + default: return undefined; + } +} + function undefinedIfAllValuesAreEmpty(object: object): object | undefined { return Object.values(object).some(v => v !== undefined) ? object : undefined; } From 2d95d9d3e793e857f9df4caff9ff8033fc75175f Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 24 Jul 2020 12:52:30 -0400 Subject: [PATCH 34/46] added full support for the string form of Fn::Sub, including literals --- .../test/invalid-templates.test.ts | 6 --- .../test-templates/fn-sub-valid-string.json | 6 +-- .../test/valid-templates.test.ts | 1 + packages/@aws-cdk/core/lib/cfn-parse.ts | 51 +++++-------------- 4 files changed, 17 insertions(+), 47 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index d56ec49313472..317728e941992 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -112,12 +112,6 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'fn-sub-invalid-key-string.json'); }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); }); - - test("throws a validation exception when Fn::Sub in string form has invalid syntax", () => { - expect(() => { - includeTestTemplate(stack, 'fn-sub-invalid-syntax.json'); - }).toThrow(/variable names in Fn::Sub syntax must contain only alphanumeric characters, underscores, periods, and colons/); - }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json index 0660fa0771a96..569e4223fd96a 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json @@ -3,14 +3,14 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": "some-bucket1231232343453452234" + "BucketName": "some-bucket1231${!AWS::AccountId}232343453${ ! AWS::Region}452${!Immediate}234" } }, "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}-${AWS::Region}" } + "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}-gibberish-${Bucket}-${!Literal}-${AWS::Region}" } } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 766df32c9daa3..4b7bc40e8162e 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -266,6 +266,7 @@ describe('CDK Include', () => { loadTestFileToJsObject('resource-attribute-condition.json'), ); }); + //TODO: need Element used in Ref expression with logical ID: 'A WS::Region' in Fn::Sub not found test('correctly change references to Conditions when renaming them', () => { const cfnTemplate = includeTestTemplate(stack, 'condition-same-name-as-resource.json'); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index aabb9ac5dba7f..95d0e2dda9c36 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -425,41 +425,10 @@ export class CfnParser { console.log(value); } if (typeof value === 'string') { - if (value.indexOf('!') !== -1) { - // loop from the '{' to the '!', eg "${ ! ..." - for (let i = value.indexOf('${') + 2; i < value.indexOf('!'); i++) { - if (value[i] !== ' ') { - throw new Error('variable names in Fn::Sub syntax must contain only alphanumeric characters, underscores, periods, and colons'); - } - } - break; - } - - /*let openIndices = [], closedIndicies = []; - for (let i = 0; i < value.length; i++) { - if (value[i] === "$" && value[i+1] === '{') { - openIndices.push(i); - } else if (value[i] === '}') { - closedIndicies.push(i); - } - } - for (let index of openIndices) { - - }*/ - const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); - const specialRef = specialCaseSubRefs(refTarget); - if (specialRef) { - console.log(specialRef) - return Fn.sub(value.substring(0, value.indexOf('$')) + specialRef); - } else { - const refElement = this.options.finder.findRefTarget(refTarget); - if (!refElement) { - throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); - } - return Fn.sub(value.substring(0, value.indexOf('$')) + CfnReference.for(refElement, 'Sub')); - } + // + return Fn.sub(value.substring(0, value.indexOf('${')) + this.parseSubRef(value.substring(value.indexOf('${')))); } - return "my-other-string"; + return 'we dont support non string form yet' } case 'Condition': { // a reference to a Condition from another Condition @@ -490,18 +459,24 @@ export class CfnParser { } private parseSubRef(value: string): string { - //TODO need base case const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); + console.log('-------------------------------') + console.log(refTarget) + if (refTarget === '') { + return ''; + } else if (refTarget[0] === '!') { + return value.substring(0, value.indexOf('}') + 1) + this.parseSubRef(value.substring(value.indexOf('}')+1)); + } + const specialRef = specialCaseSubRefs(refTarget); if (specialRef) { - console.log(specialRef) - return Fn.sub(value.substring(0, value.indexOf('$')) + specialRef); + return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubRef(value.substring(value.indexOf('}')+1)); } else { const refElement = this.options.finder.findRefTarget(refTarget); if (!refElement) { throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } - return CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}'))); + return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}')+1)); } } } From d6406fa438cc2214b851c1ddc9aa8504e767a6f8 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 24 Jul 2020 17:03:00 -0400 Subject: [PATCH 35/46] added support for basic usage of both Fn::Sub forms. --- .../{fn-sub.json => fn-sub-valid-map.json} | 2 +- .../test/valid-templates.test.ts | 10 +++++- packages/@aws-cdk/core/lib/cfn-parse.ts | 35 ++++++++++++++----- 3 files changed, 37 insertions(+), 10 deletions(-) rename packages/@aws-cdk/cloudformation-include/test/test-templates/{fn-sub.json => fn-sub-valid-map.json} (63%) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json similarity index 63% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json index 51e266b452489..5488199d4cd17 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json @@ -3,7 +3,7 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "unusued-different-sub-${Name}", { "Name": {"Ref" : "AnotherBucket" }} ]} + "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} } }, "AnotherBucket": { diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 4b7bc40e8162e..3caf0aeaf550d 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -196,7 +196,15 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'fn-sub-valid-string.json'); expect(stack).toMatchTemplate( - loadTestFileToJsObject('fn-sub-shadow.json'), + loadTestFileToJsObject('fn-sub-valid-string.json'), + ); + }); + + test('can ingest a template with Fn::Sub in string form and output it unchanged', () => { + includeTestTemplate(stack, 'fn-sub-valid-map.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('fn-sub-valid-string.json'), ); }); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 95d0e2dda9c36..59316bf08cb6a 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -420,15 +420,14 @@ export class CfnParser { return Fn.conditionOr(...value); } case 'Fn::Sub': { + console.log(object); const value = this.parseValue(object[key]); - if (typeof value === 'object') { - console.log(value); - } if (typeof value === 'string') { - // + // let parseSubRef() handle the references return Fn.sub(value.substring(0, value.indexOf('${')) + this.parseSubRef(value.substring(value.indexOf('${')))); + } else { + return Fn.sub(value[0].substring(0, value[0].indexOf('${')) + this.parseSubMapRef(value[0].substring(value[0].indexOf('${')), value[1])); } - return 'we dont support non string form yet' } case 'Condition': { // a reference to a Condition from another Condition @@ -460,8 +459,6 @@ export class CfnParser { private parseSubRef(value: string): string { const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); - console.log('-------------------------------') - console.log(refTarget) if (refTarget === '') { return ''; } else if (refTarget[0] === '!') { @@ -479,8 +476,30 @@ export class CfnParser { return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}')+1)); } } -} + private parseSubMapRef(value: string, map: any): string { + const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); + if (refTarget === '') { + return ''; + } else if (refTarget[0] === '!') { + return value.substring(0, value.indexOf('}') + 1) + this.parseSubMapRef(value.substring(value.indexOf('}')+1), map); + } + + //TODO: remove special ref foo + const specialRef = specialCaseSubRefs(refTarget); + if (specialRef) { + return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubMapRef(value.substring(value.indexOf('}')+1), map); + } else { + const refElement = map[refTarget]; + //TODO: can be a regular ID not in the map + //const refElement = this.options.finder.findRefTarget(refTarget); + if (!refElement) { + throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + } + return value.substring(0, value.indexOf('${')) + '${' + refTarget + '}' + this.parseSubMapRef(value.substring(value.indexOf('}')+1), map); + } + } +} function specialCaseRefs(value: any): any { switch (value) { From 2ac07ef991684e675aa336045f7f8771cc24d224 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 27 Jul 2020 12:28:02 -0400 Subject: [PATCH 36/46] refactored code and improved testing --- .../test/invalid-templates.test.ts | 18 +++++--- .../test/test-templates/fn-sub-valid-map.json | 10 ++++- .../test/test-templates/fn-sub.json | 16 +++++++ .../invalid/fn-sub-invalid-key-map.json | 10 +++++ .../invalid/fn-sub-invalid-key-string.json | 2 +- .../invalid/fn-sub-invalid-key.json | 10 ----- .../invalid/fn-sub-key-not-in-map.json | 16 +++++++ .../test/valid-templates.test.ts | 4 +- packages/@aws-cdk/core/lib/cfn-parse.ts | 43 ++++++++++++------- 9 files changed, 93 insertions(+), 36 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-map.json delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index 317728e941992..cddc730eac1a9 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -101,17 +101,23 @@ describe('CDK Include', () => { }).toThrow(/Output with name 'SomeOutput' refers to a Condition with name 'NonexistantCondition' which was not found in this template/); }); - /*test("throws a validation exception when Fn::Sub uses a key that isn't in the template", () => { - expect(() => { - includeTestTemplate(stack, 'fn-sub-invalid-key.json'); - }).toThrow(/Element used in Ref expression with logical ID: 'FakeResource' not found/); - });*/ - test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => { expect(() => { includeTestTemplate(stack, 'fn-sub-invalid-key-string.json'); }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); }); + + test("throws a validation exception when Fn::Sub in map form uses a key that isn't in the template", () => { + expect(() => { + includeTestTemplate(stack, 'fn-sub-invalid-key-map.json'); + }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' not found/); + }); + + test("throws a validation exception when Fn::Sub in map form uses a key that isn't in the template and isn't in the map", () => { + expect(() => { + includeTestTemplate(stack, 'fn-sub-key-not-in-map.json'); + }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json index 5488199d4cd17..d8034c022ae9e 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json @@ -3,13 +3,19 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} + "BucketName": { "Fn::Sub": [ "${Name}-${Region}-${!Immediate}-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} } }, "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}" } + "BucketName": { "Fn::Sub": "${!Literal}-${AWS::Region}-${LastResource}" } + } + }, + "LastResource": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": "my-bucket" } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json new file mode 100644 index 0000000000000..5488199d4cd17 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json @@ -0,0 +1,16 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} + } + }, + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-map.json new file mode 100644 index 0000000000000..7f3be30f56a96 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-map.json @@ -0,0 +1,10 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AFakeResource}", { "Name": {"Ref" : "AFakeResource" }, "Region": {"Fn::Base64": "a-string" }} ]} + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json index 5e148cc9780c9..c1ad988c12046 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json @@ -3,7 +3,7 @@ "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "AccessControl": { "Fn::Sub": "unusued-bucket-fn-sub-${ AFakeResource}" } + "AccessControl": { "Fn::Sub": "unusued-bucket-fn-sub-${AFakeResource}" } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json deleted file mode 100644 index 5b0fc58be3a45..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { "Fn::Sub": [ "unusued-different-sub-${Name}", { "Name": {"Ref" : "AFakeResource" }} ]} - } - } - } -} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json new file mode 100644 index 0000000000000..10bfb69a085dc --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json @@ -0,0 +1,16 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AFakeResource}", { "Name": {"Ref" : "AnotherResource" }, "Region": {"Fn::Base64": "a-string" }} ]} + } + }, + "AnotherResource": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": "my-bucket" + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 3caf0aeaf550d..ec28dd9e7b65f 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -200,11 +200,11 @@ describe('CDK Include', () => { ); }); - test('can ingest a template with Fn::Sub in string form and output it unchanged', () => { + test('can ingest a template with Fn::Sub in map form and output it unchanged', () => { includeTestTemplate(stack, 'fn-sub-valid-map.json'); expect(stack).toMatchTemplate( - loadTestFileToJsObject('fn-sub-valid-string.json'), + loadTestFileToJsObject('fn-sub-valid-map.json'), ); }); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 59316bf08cb6a..bfd972700bbe5 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -420,13 +420,12 @@ export class CfnParser { return Fn.conditionOr(...value); } case 'Fn::Sub': { - console.log(object); const value = this.parseValue(object[key]); if (typeof value === 'string') { // let parseSubRef() handle the references return Fn.sub(value.substring(0, value.indexOf('${')) + this.parseSubRef(value.substring(value.indexOf('${')))); - } else { - return Fn.sub(value[0].substring(0, value[0].indexOf('${')) + this.parseSubMapRef(value[0].substring(value[0].indexOf('${')), value[1])); + } else { //TODO: this value, value is terrible + return Fn.sub(value[0].substring(0, value[0].indexOf('${')) + this.parseSubMapRef(value[0].substring(value[0].indexOf('${')), value[1]), value[1]); } } case 'Condition': { @@ -462,41 +461,55 @@ export class CfnParser { if (refTarget === '') { return ''; } else if (refTarget[0] === '!') { - return value.substring(0, value.indexOf('}') + 1) + this.parseSubRef(value.substring(value.indexOf('}')+1)); + return value.substring(0, value.indexOf('}') + 1) + this.parseSubRef(value.substring(value.indexOf('}') + 1)); } const specialRef = specialCaseSubRefs(refTarget); if (specialRef) { - return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubRef(value.substring(value.indexOf('}')+1)); + return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubRef(value.substring(value.indexOf('}') + 1)); } else { const refElement = this.options.finder.findRefTarget(refTarget); if (!refElement) { throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } - return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}')+1)); + return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}') + 1)); } } + // TODO: need to combine this method and parseSubRef() into one method, likely the one below. private parseSubMapRef(value: string, map: any): string { - const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); + const leftBrace = value.indexOf('${'); + const rightBrace = value.indexOf('}') + 1; + const leftHalf = value.substring(0, leftBrace); + const rightHalf = value.substring(rightBrace); + //const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); + // don't include left and right braces in refTarget + const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim(); if (refTarget === '') { return ''; } else if (refTarget[0] === '!') { - return value.substring(0, value.indexOf('}') + 1) + this.parseSubMapRef(value.substring(value.indexOf('}')+1), map); + //return value.substring(0, value.indexOf('}') + 1) + this.parseSubMapRef(value.substring(value.indexOf('}') + 1), map); + return value.substring(0, rightBrace) + this.parseSubMapRef(rightHalf, map); } - //TODO: remove special ref foo const specialRef = specialCaseSubRefs(refTarget); if (specialRef) { - return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubMapRef(value.substring(value.indexOf('}')+1), map); + //return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubMapRef(value.substring(value.indexOf('}') + 1), map); + return leftHalf + specialRef + this.parseSubMapRef(rightHalf, map); } else { - const refElement = map[refTarget]; - //TODO: can be a regular ID not in the map - //const refElement = this.options.finder.findRefTarget(refTarget); + let refElement = map[refTarget]; + //TODO: can be a regular ID not in the map; GetAtt (ie ${A.B}) if (!refElement) { - throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + refElement = this.options.finder.findRefTarget(refTarget); + if (!refElement) { + throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + } + // it's in the template + //return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}') + 1)); + return leftHalf + CfnReference.for(refElement, 'Sub') + this.parseSubRef(rightHalf); } - return value.substring(0, value.indexOf('${')) + '${' + refTarget + '}' + this.parseSubMapRef(value.substring(value.indexOf('}')+1), map); + //return value.substring(0, value.indexOf('${')) + '${' + refTarget + '}' + this.parseSubMapRef(value.substring(value.indexOf('}') + 1), map); + return leftHalf + '${' + refTarget + '}' + this.parseSubMapRef(rightHalf, map); } } } From 40520d06b9ae638e36420510917fb8ab03006e08 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 27 Jul 2020 13:20:42 -0400 Subject: [PATCH 37/46] substantially refactored the parsing of the string argument to Fn::Sub --- .../test/test-templates/fn-sub-valid-map.json | 2 +- packages/@aws-cdk/core/lib/cfn-parse.ts | 65 +++++++------------ 2 files changed, 23 insertions(+), 44 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json index d8034c022ae9e..4383566e02002 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json @@ -3,7 +3,7 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "${Name}-${Region}-${!Immediate}-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} + "BucketName": { "Fn::Sub": [ "${Name}-${Region}-foo-${!Immediate}-foo-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} } }, "AnotherBucket": { diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index bfd972700bbe5..536db1d198ad2 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -423,9 +423,9 @@ export class CfnParser { const value = this.parseValue(object[key]); if (typeof value === 'string') { // let parseSubRef() handle the references - return Fn.sub(value.substring(0, value.indexOf('${')) + this.parseSubRef(value.substring(value.indexOf('${')))); + return Fn.sub(value.substring(0, value.indexOf('${')) + this.parseFnSubString(value.substring(value.indexOf('${')))); } else { //TODO: this value, value is terrible - return Fn.sub(value[0].substring(0, value[0].indexOf('${')) + this.parseSubMapRef(value[0].substring(value[0].indexOf('${')), value[1]), value[1]); + return Fn.sub(value[0].substring(0, value[0].indexOf('${')) + this.parseFnSubString(value[0].substring(value[0].indexOf('${')), value[1]), value[1]); } } case 'Condition': { @@ -456,61 +456,40 @@ export class CfnParser { : undefined; } - private parseSubRef(value: string): string { - const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); - if (refTarget === '') { - return ''; - } else if (refTarget[0] === '!') { - return value.substring(0, value.indexOf('}') + 1) + this.parseSubRef(value.substring(value.indexOf('}') + 1)); - } - - const specialRef = specialCaseSubRefs(refTarget); - if (specialRef) { - return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubRef(value.substring(value.indexOf('}') + 1)); - } else { - const refElement = this.options.finder.findRefTarget(refTarget); - if (!refElement) { - throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); - } - return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}') + 1)); - } - } - - // TODO: need to combine this method and parseSubRef() into one method, likely the one below. - private parseSubMapRef(value: string, map: any): string { + private parseFnSubString(value: string, map?: any): string { const leftBrace = value.indexOf('${'); const rightBrace = value.indexOf('}') + 1; const leftHalf = value.substring(0, leftBrace); const rightHalf = value.substring(rightBrace); - //const refTarget = value.substring(value.indexOf('${') + 2, value.indexOf('}')).trim(); // don't include left and right braces in refTarget const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim(); if (refTarget === '') { return ''; } else if (refTarget[0] === '!') { - //return value.substring(0, value.indexOf('}') + 1) + this.parseSubMapRef(value.substring(value.indexOf('}') + 1), map); - return value.substring(0, rightBrace) + this.parseSubMapRef(rightHalf, map); + return value.substring(0, rightBrace) + this.parseFnSubString(rightHalf, map); + } + // TODO: need to add getatt support and better comments + + let refElement; + if (map) { + refElement = map[refTarget]; + if (refElement) { + return leftHalf + '${' + refTarget + '}' + this.parseFnSubString(rightHalf, map); + } } const specialRef = specialCaseSubRefs(refTarget); if (specialRef) { - //return value.substring(0, value.indexOf('${')) + specialRef + this.parseSubMapRef(value.substring(value.indexOf('}') + 1), map); - return leftHalf + specialRef + this.parseSubMapRef(rightHalf, map); - } else { - let refElement = map[refTarget]; - //TODO: can be a regular ID not in the map; GetAtt (ie ${A.B}) - if (!refElement) { - refElement = this.options.finder.findRefTarget(refTarget); - if (!refElement) { - throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); - } - // it's in the template - //return value.substring(0, value.indexOf('${')) + CfnReference.for(refElement, 'Sub') + this.parseSubRef(value.substring(value.indexOf('}') + 1)); - return leftHalf + CfnReference.for(refElement, 'Sub') + this.parseSubRef(rightHalf); - } - //return value.substring(0, value.indexOf('${')) + '${' + refTarget + '}' + this.parseSubMapRef(value.substring(value.indexOf('}') + 1), map); - return leftHalf + '${' + refTarget + '}' + this.parseSubMapRef(rightHalf, map); + return leftHalf + specialRef + this.parseFnSubString(rightHalf, map); + } + + refElement = this.options.finder.findRefTarget(refTarget); + if (refElement) { + + return leftHalf + CfnReference.for(refElement, 'Sub') + this.parseFnSubString(rightHalf); } + + throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } } From ae3a04b1ab84cf74907dcca394d6511c5dbedd8a Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 27 Jul 2020 16:12:10 -0400 Subject: [PATCH 38/46] improved tests and refactored implementation --- .../test/integ.fn-sub.expected.json | 27 ---------------- .../test/integ.fn-sub.ts | 12 ------- .../test/test-templates/fn-sub-shadow.json | 4 +-- .../test/test-templates/fn-sub-valid-map.json | 11 ++----- .../test-templates/fn-sub-valid-string.json | 4 +-- .../test/test-templates/fn-sub.json | 16 ---------- .../test/valid-templates.test.ts | 4 +-- packages/@aws-cdk/core/lib/cfn-parse.ts | 32 +++++++++++++------ packages/@aws-cdk/core/lib/cfn-resource.ts | 5 +-- .../core/lib/private/cfn-reference.ts | 21 ++++++++---- 10 files changed, 50 insertions(+), 86 deletions(-) delete mode 100644 packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json delete mode 100644 packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json diff --git a/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json b/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json deleted file mode 100644 index f49839cfa5cdc..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.expected.json +++ /dev/null @@ -1,27 +0,0 @@ -{ - "Resources": { - "AnotherBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}" - } - } - }, - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { - "Fn::Sub": [ - "unusued-different-sub-${Name}", - { - "Name": { - "Ref": "AnotherBucket" - } - } - ] - } - } - } - } -} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts b/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts deleted file mode 100644 index 5337c3df82705..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/integ.fn-sub.ts +++ /dev/null @@ -1,12 +0,0 @@ -import * as core from '@aws-cdk/core'; -import * as inc from '../lib'; - -const app = new core.App(); - -const stack = new core.Stack(app, 'SubStack'); - -new inc.CfnInclude(stack, 'ParentStack', { - templateFile: 'test-templates/fn-sub.json', -}); - -app.synth(); diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json index c501ca07e7270..2ccc03c033d67 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json @@ -3,13 +3,13 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "unusued-different-sub-${Name}", { "AnotherBucket": {"Ref" : "AnotherBucket" }} ]} + "BucketName": { "Fn::Sub": [ "${AnotherBucket}", { "AnotherBucket": { "Ref" : "AnotherBucket" }} ]} } }, "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${!Literal}" } + "BucketName": "another-bucket" } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json index 4383566e02002..8bdcbbf14c995 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json @@ -3,19 +3,14 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "${Name}-${Region}-foo-${!Immediate}-foo-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} + "BucketName": { "Fn::Sub": [ "${Region}-foo-${!Immediate}-foo-${AnotherBucket}-${AnotherBucket.DomainName}-${Name}", + { "Name": { "Ref" : "AnotherBucket" }, "Region": { "Fn::Base64": "AWS::Region" }} ]} } }, "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "${!Literal}-${AWS::Region}-${LastResource}" } - } - }, - "LastResource": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": "my-bucket" + "BucketName": "another-bucket" } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json index 569e4223fd96a..b62f9c1d569d2 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json @@ -3,13 +3,13 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": "some-bucket1231${!AWS::AccountId}232343453${ ! AWS::Region}452${!Immediate}234" + "BucketName": "some-bucket${!AWS::AccountId}2453${ ! AWS::Region}1-1${!Immediate}234" } }, "AnotherBucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}-gibberish-${Bucket}-${!Literal}-${AWS::Region}" } + "BucketName": { "Fn::Sub": "1-${AWS::Region}-foo-${Bucket}-${!Literal}-${Bucket.DomainName}-${AWS::Region}" } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json deleted file mode 100644 index 5488199d4cd17..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AnotherBucket}", { "Name": {"Ref" : "AnotherBucket" }, "Region": {"Fn::Base64": "AWS::Region" }} ]} - } - }, - "AnotherBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { "Fn::Sub": "unusued-bucket-fn-sub-${AWS::Region}" } - } - } - } -} diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index ec28dd9e7b65f..695882ee5b88d 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -208,13 +208,13 @@ describe('CDK Include', () => { ); }); - /*test('can ingest a template with Fn::Sub with a key that is a logicalID and a literal and output it unchanged', () => { + test('can ingest a template with Fn::Sub with a key that is a logicalID and a literal and output it unchanged', () => { includeTestTemplate(stack, 'fn-sub-shadow.json'); expect(stack).toMatchTemplate( loadTestFileToJsObject('fn-sub-shadow.json'), ); - });*/ + }); test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 536db1d198ad2..2299e44f0d298 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -421,12 +421,19 @@ export class CfnParser { } case 'Fn::Sub': { const value = this.parseValue(object[key]); + let valString: string; + let map: any | undefined; if (typeof value === 'string') { - // let parseSubRef() handle the references - return Fn.sub(value.substring(0, value.indexOf('${')) + this.parseFnSubString(value.substring(value.indexOf('${')))); - } else { //TODO: this value, value is terrible - return Fn.sub(value[0].substring(0, value[0].indexOf('${')) + this.parseFnSubString(value[0].substring(value[0].indexOf('${')), value[1]), value[1]); + valString = value; + map = undefined; + } else { + valString = value[0]; + map = value[1]; } + + const leftBrace = valString.indexOf('${'); + const leftHalf = valString.substring(0, leftBrace); + return Fn.sub(leftHalf + this.parseFnSubString(valString.substring(leftBrace), map), map); } case 'Condition': { // a reference to a Condition from another Condition @@ -456,20 +463,20 @@ export class CfnParser { : undefined; } - private parseFnSubString(value: string, map?: any): string { + private parseFnSubString(value: string, map: any): string { const leftBrace = value.indexOf('${'); const rightBrace = value.indexOf('}') + 1; const leftHalf = value.substring(0, leftBrace); const rightHalf = value.substring(rightBrace); - // don't include left and right braces in refTarget + // don't include left and right braces when searching for the target of the reference const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim(); if (refTarget === '') { return ''; } else if (refTarget[0] === '!') { return value.substring(0, rightBrace) + this.parseFnSubString(rightHalf, map); } - // TODO: need to add getatt support and better comments + // lookup in map let refElement; if (map) { refElement = map[refTarget]; @@ -478,15 +485,22 @@ export class CfnParser { } } + // since it's not in the map, check if it's a pseudo parameter const specialRef = specialCaseSubRefs(refTarget); if (specialRef) { return leftHalf + specialRef + this.parseFnSubString(rightHalf, map); } + // handle Ref refElement = this.options.finder.findRefTarget(refTarget); if (refElement) { - - return leftHalf + CfnReference.for(refElement, 'Sub') + this.parseFnSubString(rightHalf); + return leftHalf + CfnReference.for(refElement, 'Sub', 'Ref') + this.parseFnSubString(rightHalf, map); + } + + // handle GetAtt + refElement = this.options.finder.findResource(refTarget.substring(0, refTarget.indexOf('.'))); + if (refElement) { + return leftHalf + refElement.getAtt(refTarget.substring(refTarget.indexOf('.') + 1), 'GetAtt') + this.parseFnSubString(rightHalf, map); } throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); diff --git a/packages/@aws-cdk/core/lib/cfn-resource.ts b/packages/@aws-cdk/core/lib/cfn-resource.ts index 39b60a3d248b7..9afd883488b8a 100644 --- a/packages/@aws-cdk/core/lib/cfn-resource.ts +++ b/packages/@aws-cdk/core/lib/cfn-resource.ts @@ -133,9 +133,10 @@ export class CfnResource extends CfnRefElement { * Ideally, use generated attribute accessors (e.g. `resource.arn`), but this can be used for future compatibility * in case there is no generated attribute. * @param attributeName The name of the attribute. + * @param sub optional, indicates if this is in a Fn::Sub call from cloudformation-include */ - public getAtt(attributeName: string): Reference { - return CfnReference.for(this, attributeName); + public getAtt(attributeName: string, sub?: string): Reference { + return CfnReference.for(this, attributeName, sub); } /** diff --git a/packages/@aws-cdk/core/lib/private/cfn-reference.ts b/packages/@aws-cdk/core/lib/private/cfn-reference.ts index b6a60324bc9f6..f0240982e8a73 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-reference.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-reference.ts @@ -33,19 +33,27 @@ export class CfnReference extends Reference { * important that the state isn't lost if it's lazily created, like so: * * Lazy.stringValue({ produce: () => new CfnReference(...) }) + * + * If sub is defined, this reference will resolve as + * + * ${logicalID} + * + * This allows cloudformation-include to correctly handle Fn::Sub */ - public static for(target: CfnElement, attribute: string) { + public static for(target: CfnElement, attribute: string, sub?: string) { return CfnReference.singletonReference(target, attribute, () => { let cfnIntrinsic; if (attribute === 'Ref') { - cfnIntrinsic = { Ref: target.logicalId }; - } else if (attribute === 'Sub') { + cfnIntrinsic = { Ref: target.logicalId }; + } else if (sub === 'Ref') { cfnIntrinsic = `\$\{${target.logicalId}\}`; + } else if (sub === 'GetAtt') { + cfnIntrinsic = `\$\{${target.logicalId}.${attribute}\}`; } else { cfnIntrinsic = { 'Fn::GetAtt': [ target.logicalId, attribute ]}; } return new CfnReference(cfnIntrinsic, attribute, target); - }); + }, sub); } /** @@ -65,15 +73,16 @@ export class CfnReference extends Reference { /** * Get or create the table + * Defining sub allows cloudformation-include to correctly handle Fn::Sub */ - private static singletonReference(target: Construct, attribKey: string, fresh: () => CfnReference) { + private static singletonReference(target: Construct, attribKey: string, fresh: () => CfnReference, sub?: string) { let attribs = CfnReference.referenceTable.get(target); if (!attribs) { attribs = new Map(); CfnReference.referenceTable.set(target, attribs); } let ref = attribs.get(attribKey); - if (!ref) { + if (!ref || sub) { ref = fresh(); attribs.set(attribKey, ref); } From ea0372d1d6c41da2ece51d434420f8e5e49e77ef Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 27 Jul 2020 16:14:24 -0400 Subject: [PATCH 39/46] improved documentation --- packages/@aws-cdk/core/lib/private/cfn-reference.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/@aws-cdk/core/lib/private/cfn-reference.ts b/packages/@aws-cdk/core/lib/private/cfn-reference.ts index f0240982e8a73..a1b5cdb9f9293 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-reference.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-reference.ts @@ -34,10 +34,7 @@ export class CfnReference extends Reference { * * Lazy.stringValue({ produce: () => new CfnReference(...) }) * - * If sub is defined, this reference will resolve as - * - * ${logicalID} - * + * If sub is defined, this reference will resolve as ${logicalID} * This allows cloudformation-include to correctly handle Fn::Sub */ public static for(target: CfnElement, attribute: string, sub?: string) { From 6ecf47978568e5885de9d86b3f68c53129568361 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 27 Jul 2020 16:21:57 -0400 Subject: [PATCH 40/46] updated README --- packages/@aws-cdk/cloudformation-include/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 160bddd94cf6c..75f72960c8af1 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -260,5 +260,5 @@ All items unchecked below are currently not supported. - [x] Fn::ImportValue - [x] Fn::Select - [x] Fn::Split -- [ ] Fn::Sub +- [x] Fn::Sub - [x] Fn::Transform From bb032561bb540a11db22830b0f522ae761291401 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Mon, 27 Jul 2020 17:19:04 -0400 Subject: [PATCH 41/46] added support for yaml templates that use Fn::Sub --- .../cloudformation-include/lib/file-utils.ts | 4 +--- .../test/test-templates/fn-sub-valid-string.json | 2 +- .../test/test-templates/yaml/long-form-vpc.yaml | 7 +++++++ .../test-templates/yaml/short-form-sub-map.yaml | 15 +++++++++++++++ .../test/test-templates/yaml/sub-string.yaml | 11 +++++++++++ .../test/yaml-templates.test.ts | 16 ++++++++++++++++ packages/@aws-cdk/core/lib/cfn-parse.ts | 5 +++-- 7 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/sub-string.yaml diff --git a/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts b/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts index dd58b76777d5d..65a05101bcb90 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts @@ -31,11 +31,9 @@ function makeTagForCfnIntrinsic( } const shortForms: yaml_types.Schema.CustomTag[] = [ - 'Base64', 'Cidr', 'FindInMap', 'GetAZs', 'ImportValue', 'Join', + 'Base64', 'Cidr', 'FindInMap', 'GetAZs', 'ImportValue', 'Join', 'Sub', 'Select', 'Split', 'Transform', 'And', 'Equals', 'If', 'Not', 'Or', ].map(name => makeTagForCfnIntrinsic(name)).concat( - // ToDo: special logic for ImportValue will be needed when support for Fn::Sub is added. See - // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-importvalue.html makeTagForCfnIntrinsic('Ref', false), makeTagForCfnIntrinsic('GetAtt', true, (_doc: yaml.Document, cstNode: yaml_cst.CST.Node): any => { // The position of the leftmost period and opening bracket tell us what syntax is being used diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json index b62f9c1d569d2..3c510422b5fc7 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json @@ -3,7 +3,7 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": "some-bucket${!AWS::AccountId}2453${ ! AWS::Region}1-1${!Immediate}234" + "BucketName": { "Fn::Sub": "some-bucket${!AWS::AccountId}7896${ ! AWS::Region}1-1${!Immediate}234" } } }, "AnotherBucket": { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml index de8b072887d23..2e1e3d4714487 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml @@ -42,3 +42,10 @@ Resources: Location: location, AnotherParameter: Fn::Base64: AnotherValue + AccessControl: + Fn::Sub: + - "${Region}-foo-${!Immediate}-foo-${Vpc}-${Vpc.Id}-${Name}" + - Name: + Ref: Vpc + Region: + Fn::Base64: AWS::Region diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml new file mode 100644 index 0000000000000..80450b205abba --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml @@ -0,0 +1,15 @@ +Resources: + Bucket: + Type: AWS::S3::Bucket + Properties: + BucketName: + !Sub + - "${Region}-foo-${!Immediate}-foo-${AnotherBucket}-${AnotherBucket.DomainName}-${Name}" + - Name: + Ref: AnotherBucket + Region: + Fn::Base64: AWS::Region + AnotherBucket: + Type: AWS::S3::Bucket + Properties: + BucketName: another-bucket diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/sub-string.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/sub-string.yaml new file mode 100644 index 0000000000000..72ccedb2c61c9 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/sub-string.yaml @@ -0,0 +1,11 @@ +Resources: + Bucket: + Type: AWS::S3::Bucket + Properties: + BucketName: + Fn::Sub: some-bucket${!AWS::AccountId}7896${ ! AWS::Region}1-1${!Immediate}234 + AnotherBucket: + Type: AWS::S3::Bucket + Properties: + BucketName: + !Sub 1-${AWS::Region}-foo-${Bucket}-${!Literal}-${Bucket.DomainName}-${AWS::Region} diff --git a/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts index 01bba1610a9b5..1c4e4064a504c 100644 --- a/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts @@ -323,6 +323,22 @@ describe('CDK Include', () => { loadTestFileToJsObject('long-form-subnet.yaml'), ); }); + + test('can ingest a yaml tempalte with Fn::Sub in string form and output it unchanged', () => { + includeTestTemplate(stack, 'sub-string.yaml'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('sub-string.yaml'), + ); + }); + + test('can ingest a yaml tempalte with Fn::Sub in map form and output it unchanged', () => { + includeTestTemplate(stack, 'short-form-sub-map.yaml'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('short-form-sub-map.yaml'), + ); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 2299e44f0d298..a65fb6d66bcbf 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -470,8 +470,8 @@ export class CfnParser { const rightHalf = value.substring(rightBrace); // don't include left and right braces when searching for the target of the reference const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim(); - if (refTarget === '') { - return ''; + if (leftBrace === -1) { + return value; } else if (refTarget[0] === '!') { return value.substring(0, rightBrace) + this.parseFnSubString(rightHalf, map); } @@ -503,6 +503,7 @@ export class CfnParser { return leftHalf + refElement.getAtt(refTarget.substring(refTarget.indexOf('.') + 1), 'GetAtt') + this.parseFnSubString(rightHalf, map); } + if (refTarget === '2') { console.log(leftBrace)}; throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } } From 970fa59e31101811abd5a6d935376beb39518bcd Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 28 Jul 2020 14:44:10 -0400 Subject: [PATCH 42/46] incorporated review comments --- .../@aws-cdk/cloudformation-include/README.md | 44 ----------- .../test/invalid-templates.test.ts | 10 ++- .../fn-sub-map-dotted-attributes.json | 32 ++++++++ ...-map.json => fn-sub-shadow-attribute.json} | 10 ++- .../test/test-templates/fn-sub-shadow.json | 9 ++- .../test-templates/fn-sub-valid-escaping.json | 10 +++ .../test-templates/fn-sub-valid-string.json | 2 +- .../invalid/fn-sub-bad-get-att.json | 10 +++ ...=> fn-sub-key-not-in-template-string.json} | 0 ...p.json => fn-sub-key-not-in-template.json} | 0 .../test/valid-templates.test.ts | 42 ++++++++-- packages/@aws-cdk/core/lib/cfn-parse.ts | 77 +++++++++---------- packages/@aws-cdk/core/lib/cfn-resource.ts | 5 +- .../core/lib/private/cfn-reference.ts | 38 ++++----- 14 files changed, 170 insertions(+), 119 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json rename packages/@aws-cdk/cloudformation-include/test/test-templates/{fn-sub-valid-map.json => fn-sub-shadow-attribute.json} (52%) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json rename packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/{fn-sub-invalid-key-string.json => fn-sub-key-not-in-template-string.json} (100%) rename packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/{fn-sub-invalid-key-map.json => fn-sub-key-not-in-template.json} (100%) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 75f72960c8af1..0e08b7f5fb745 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -218,47 +218,3 @@ bucketReadRole.addToPolicy(new iam.PolicyStatement({ resources: [bucket.attrArn], })); ``` - -## Known limitations - -This module is still in its early, experimental stage, -and so does not implement all features of CloudFormation templates. -All items unchecked below are currently not supported. - -### Ability to retrieve CloudFormation objects from the template: - -- [x] Resources -- [x] Parameters -- [x] Conditions -- [x] Outputs - -### [Resource attributes](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-product-attribute-reference.html): - -- [x] Properties -- [x] Condition -- [x] DependsOn -- [x] CreationPolicy -- [x] UpdatePolicy -- [x] UpdateReplacePolicy -- [x] DeletionPolicy -- [x] Metadata - -### [CloudFormation functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html): - -- [x] Ref -- [x] Fn::GetAtt -- [x] Fn::Join -- [x] Fn::If -- [x] Fn::And -- [x] Fn::Equals -- [x] Fn::Not -- [x] Fn::Or -- [x] Fn::Base64 -- [x] Fn::Cidr -- [x] Fn::FindInMap -- [x] Fn::GetAZs -- [x] Fn::ImportValue -- [x] Fn::Select -- [x] Fn::Split -- [x] Fn::Sub -- [x] Fn::Transform diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index cddc730eac1a9..c1b57f3318c1a 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -103,13 +103,13 @@ describe('CDK Include', () => { test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => { expect(() => { - includeTestTemplate(stack, 'fn-sub-invalid-key-string.json'); + includeTestTemplate(stack, 'fn-sub-key-not-in-template-string.json'); }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); }); test("throws a validation exception when Fn::Sub in map form uses a key that isn't in the template", () => { expect(() => { - includeTestTemplate(stack, 'fn-sub-invalid-key-map.json'); + includeTestTemplate(stack, 'fn-sub-key-not-in-template.json'); }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' not found/); }); @@ -118,6 +118,12 @@ describe('CDK Include', () => { includeTestTemplate(stack, 'fn-sub-key-not-in-map.json'); }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); }); + + test("throws a validation exception when Fn::Sub uses a resource attribute that doesn't exist", () => { + expect(() => { + includeTestTemplate(stack, 'fn-sub-bad-get-att.json'); + }).toThrow(/Resource referenced in Fn::Sub expression with logical ID: 'FakeBucket' not found/); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json new file mode 100644 index 0000000000000..d83248dec0e74 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json @@ -0,0 +1,32 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { + "Fn::Sub": [ + "${Region}${ELB}-${ELB.SourceSecurityGroup.GroupName}-${Name}", + { + "Name": { "Ref" : "ELB" }, + "Region": { "Fn::Base64": "AWS::Region" } + } + ] + } + } + }, + "ELB": { + "Type": "AWS::ElasticLoadBalancing::LoadBalancer", + "Properties": { + "AvailabilityZones": [ + "us-east-1a" + ], + "Listeners": [ + { + "LoadBalancerPort": "80", + "InstancePort": "80", + "Protocol": "HTTP" + }] + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json similarity index 52% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json index 8bdcbbf14c995..81b035d04d520 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-map.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json @@ -3,8 +3,14 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "${Region}-foo-${!Immediate}-foo-${AnotherBucket}-${AnotherBucket.DomainName}-${Name}", - { "Name": { "Ref" : "AnotherBucket" }, "Region": { "Fn::Base64": "AWS::Region" }} ]} + "BucketName": { + "Fn::Sub": [ + "${AnotherBucket.DomainName}", + { + "AnotherBucket": "whatever" + } + ] + } } }, "AnotherBucket": { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json index 2ccc03c033d67..14e0c075fa501 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json @@ -3,7 +3,14 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": [ "${AnotherBucket}", { "AnotherBucket": { "Ref" : "AnotherBucket" }} ]} + "BucketName": { + "Fn::Sub": [ + "${AnotherBucket}", + { + "AnotherBucket": { "Ref" : "AnotherBucket" } + } + ] + } } }, "AnotherBucket": { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json new file mode 100644 index 0000000000000..47ce8fd65e0ba --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json @@ -0,0 +1,10 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "some-bucket${!AWS::AccountId}7896${ ! AWS::Region}${!Immediate}234" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json index 3c510422b5fc7..2936a59bb55fc 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json @@ -3,7 +3,7 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "some-bucket${!AWS::AccountId}7896${ ! AWS::Region}1-1${!Immediate}234" } + "BucketName": "bucket" } }, "AnotherBucket": { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json new file mode 100644 index 0000000000000..53421fd679f4f --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json @@ -0,0 +1,10 @@ +{ + "Resources": { + "Resource": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "${FakeBucket.BucketName}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json similarity index 100% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-string.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template.json similarity index 100% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-key-map.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template.json diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index 695882ee5b88d..ae0bc8303addf 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -192,7 +192,7 @@ describe('CDK Include', () => { ); }); - test('can ingest a template with Fn::Sub in string form and output it unchanged', () => { + test('can ingest a template with Fn::Sub in string form with escaped and unescaped references and output it unchanged', () => { includeTestTemplate(stack, 'fn-sub-valid-string.json'); expect(stack).toMatchTemplate( @@ -200,15 +200,23 @@ describe('CDK Include', () => { ); }); + test('can ingest a template with Fn::Sub in string form with escaped references and output it unchanged', () => { + includeTestTemplate(stack, 'fn-sub-valid-escaping.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('fn-sub-valid-escaping.json'), + ); + }); + test('can ingest a template with Fn::Sub in map form and output it unchanged', () => { - includeTestTemplate(stack, 'fn-sub-valid-map.json'); + includeTestTemplate(stack, 'fn-sub-map-dotted-attributes.json'); expect(stack).toMatchTemplate( - loadTestFileToJsObject('fn-sub-valid-map.json'), + loadTestFileToJsObject('fn-sub-map-dotted-attributes.json'), ); }); - test('can ingest a template with Fn::Sub with a key that is a logicalID and a literal and output it unchanged', () => { + test('can ingest a template with Fn::Sub with a key that is a logicalID and output it unchanged', () => { includeTestTemplate(stack, 'fn-sub-shadow.json'); expect(stack).toMatchTemplate( @@ -216,6 +224,31 @@ describe('CDK Include', () => { ); }); + test('can ingest a template with Fn::Sub with a key that is a logicalID with an attribue and output it unchanged', () => { + includeTestTemplate(stack, 'fn-sub-shadow-attribute.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('fn-sub-shadow-attribute.json'), + ); + }); + + test('can modify resources used in Fn::Sub references and see the changes in the template', () => { + const cfnTemplate = includeTestTemplate(stack, 'fn-sub-shadow.json'); + + cfnTemplate.getResource('AnotherBucket').overrideLogicalId('NewBucket'); + + expect(stack).toHaveResourceLike('AWS::S3::Bucket', { + "BucketName": { + "Fn::Sub": [ + "${AnotherBucket}", + { + "AnotherBucket": { "Ref": "NewBucket" }, + }, + ], + }, + }); + }); + test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); @@ -274,7 +307,6 @@ describe('CDK Include', () => { loadTestFileToJsObject('resource-attribute-condition.json'), ); }); - //TODO: need Element used in Ref expression with logical ID: 'A WS::Region' in Fn::Sub not found test('correctly change references to Conditions when renaming them', () => { const cfnTemplate = includeTestTemplate(stack, 'condition-same-name-as-resource.json'); diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index a65fb6d66bcbf..bbc91d22fd644 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -111,7 +111,7 @@ export class FromCloudFormation { public static getCfnTag(tag: any): CfnTag { return tag == null - ? {} as any // break the type system - this should be detected at runtime by a tag validator + ? { } as any // break the type system - this should be detected at runtime by a tag validator : { key: tag.Key, value: tag.Value, @@ -421,19 +421,17 @@ export class CfnParser { } case 'Fn::Sub': { const value = this.parseValue(object[key]); - let valString: string; - let map: any | undefined; + let fnSubString: string; + let map: { [key: string]: any } | undefined; if (typeof value === 'string') { - valString = value; + fnSubString = value; map = undefined; } else { - valString = value[0]; + fnSubString = value[0]; map = value[1]; } - const leftBrace = valString.indexOf('${'); - const leftHalf = valString.substring(0, leftBrace); - return Fn.sub(leftHalf + this.parseFnSubString(valString.substring(leftBrace), map), map); + return Fn.sub(this.parseFnSubString(fnSubString, map), map); } case 'Condition': { // a reference to a Condition from another Condition @@ -457,13 +455,13 @@ export class CfnParser { const key = objectKeys[0]; return key === 'Ref' || key.startsWith('Fn::') || - // special intrinsic only available in the 'Conditions' section - (this.options.context === CfnParsingContext.CONDITIONS && key === 'Condition') + // special intrinsic only available in the 'Conditions' section + (this.options.context === CfnParsingContext.CONDITIONS && key === 'Condition') ? key : undefined; } - private parseFnSubString(value: string, map: any): string { + private parseFnSubString(value: string, map: { [key: string]: any } | undefined): string { const leftBrace = value.indexOf('${'); const rightBrace = value.indexOf('}') + 1; const leftHalf = value.substring(0, leftBrace); @@ -477,12 +475,8 @@ export class CfnParser { } // lookup in map - let refElement; - if (map) { - refElement = map[refTarget]; - if (refElement) { - return leftHalf + '${' + refTarget + '}' + this.parseFnSubString(rightHalf, map); - } + if (refTarget in (map || {})) { + return leftHalf + '${' + refTarget + '}' + this.parseFnSubString(rightHalf, map); } // since it's not in the map, check if it's a pseudo parameter @@ -491,20 +485,21 @@ export class CfnParser { return leftHalf + specialRef + this.parseFnSubString(rightHalf, map); } - // handle Ref - refElement = this.options.finder.findRefTarget(refTarget); - if (refElement) { - return leftHalf + CfnReference.for(refElement, 'Sub', 'Ref') + this.parseFnSubString(rightHalf, map); - } - - // handle GetAtt - refElement = this.options.finder.findResource(refTarget.substring(0, refTarget.indexOf('.'))); - if (refElement) { - return leftHalf + refElement.getAtt(refTarget.substring(refTarget.indexOf('.') + 1), 'GetAtt') + this.parseFnSubString(rightHalf, map); + const isRef = refTarget.indexOf('.') === -1; + if (isRef) { + const refElement = this.options.finder.findRefTarget(refTarget); + if (!refElement) { + throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + } + return leftHalf + CfnReference.for(refElement, 'Ref', true).toString() + this.parseFnSubString(rightHalf, map); + } else { + const targetId = refTarget.substring(0, refTarget.indexOf('.')); + const refResource = this.options.finder.findResource(targetId); + if (!refResource) { + throw new Error(`Resource referenced in Fn::Sub expression with logical ID: '${targetId}' not found`); + } + return leftHalf + CfnReference.for(refResource, refTarget.substring(refTarget.indexOf('.') + 1), true).toString() + this.parseFnSubString(rightHalf, map); } - - if (refTarget === '2') { console.log(leftBrace)}; - throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); } } @@ -524,19 +519,23 @@ function specialCaseRefs(value: any): any { function specialCaseSubRefs(value: any): any { switch (value) { - case 'AWS::AccountId': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::Region': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::Partition': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::URLSuffix': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::NotificationARNs': return Token.asList(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::StackId': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::StackName': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::NoValue': return Token.asString(`\$\{${value}\}`, { displayHint: value }); - case 'AWS::NoValue': return Aws.NO_VALUE; + case 'AWS::AccountId': return pseudoString(value); + case 'AWS::Region': return pseudoString(value); + case 'AWS::Partition': return pseudoString(value); + case 'AWS::URLSuffix': return pseudoString(value); + case 'AWS::NotificationARNs': return pseudoString(value); + case 'AWS::StackId': return pseudoString(value); + case 'AWS::StackName': return pseudoString(value); + case 'AWS::NoValue': return pseudoString(value); + case 'AWS::NoValue': return pseudoString(value); default: return undefined; } } +function pseudoString(value: any): any { + return Token.asString('${' + value + '}', {displayHint: value }); +} + function undefinedIfAllValuesAreEmpty(object: object): object | undefined { return Object.values(object).some(v => v !== undefined) ? object : undefined; } diff --git a/packages/@aws-cdk/core/lib/cfn-resource.ts b/packages/@aws-cdk/core/lib/cfn-resource.ts index 9afd883488b8a..5146764d32d14 100644 --- a/packages/@aws-cdk/core/lib/cfn-resource.ts +++ b/packages/@aws-cdk/core/lib/cfn-resource.ts @@ -133,10 +133,9 @@ export class CfnResource extends CfnRefElement { * Ideally, use generated attribute accessors (e.g. `resource.arn`), but this can be used for future compatibility * in case there is no generated attribute. * @param attributeName The name of the attribute. - * @param sub optional, indicates if this is in a Fn::Sub call from cloudformation-include */ - public getAtt(attributeName: string, sub?: string): Reference { - return CfnReference.for(this, attributeName, sub); + public getAtt(attributeName: string ): Reference { + return CfnReference.for(this, attributeName); } /** diff --git a/packages/@aws-cdk/core/lib/private/cfn-reference.ts b/packages/@aws-cdk/core/lib/private/cfn-reference.ts index a1b5cdb9f9293..530e0297a4d7d 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-reference.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-reference.ts @@ -34,30 +34,23 @@ export class CfnReference extends Reference { * * Lazy.stringValue({ produce: () => new CfnReference(...) }) * - * If sub is defined, this reference will resolve as ${logicalID} - * This allows cloudformation-include to correctly handle Fn::Sub + * If fnSub is true, then this reference will resolve as ${logicalID}. + * This allows cloudformation-include to correctly handle Fn::Sub. */ - public static for(target: CfnElement, attribute: string, sub?: string) { - return CfnReference.singletonReference(target, attribute, () => { - let cfnIntrinsic; - if (attribute === 'Ref') { - cfnIntrinsic = { Ref: target.logicalId }; - } else if (sub === 'Ref') { - cfnIntrinsic = `\$\{${target.logicalId}\}`; - } else if (sub === 'GetAtt') { - cfnIntrinsic = `\$\{${target.logicalId}.${attribute}\}`; - } else { - cfnIntrinsic = { 'Fn::GetAtt': [ target.logicalId, attribute ]}; - } + public static for(target: CfnElement, attribute: string, fnSub: boolean = false) { + return CfnReference.singletonReference(target, attribute, fnSub, () => { + const cfnIntrinsic = fnSub + ? (attribute === 'Ref' ? '${' + target.logicalId + '}' : '${' + target.logicalId + '.' + attribute + '}') + : (attribute === 'Ref' ? { Ref: target.logicalId } : { 'Fn::GetAtt': [ target.logicalId, attribute ] }); return new CfnReference(cfnIntrinsic, attribute, target); - }, sub); + }); } /** * Return a CfnReference that references a pseudo referencd */ public static forPseudo(pseudoName: string, scope: Construct) { - return CfnReference.singletonReference(scope, `Pseudo:${pseudoName}`, () => { + return CfnReference.singletonReference(scope, `Pseudo:${pseudoName}`, false, () => { const cfnIntrinsic = { Ref: pseudoName }; return new CfnReference(cfnIntrinsic, pseudoName, scope); }); @@ -69,19 +62,20 @@ export class CfnReference extends Reference { private static referenceTable = new Map>(); /** - * Get or create the table - * Defining sub allows cloudformation-include to correctly handle Fn::Sub + * Get or create the table. + * Defining sub allows cloudformation-include to correctly handle Fn::Sub. */ - private static singletonReference(target: Construct, attribKey: string, fresh: () => CfnReference, sub?: string) { + private static singletonReference(target: Construct, attribKey: string, fnSub: boolean, fresh: () => CfnReference) { let attribs = CfnReference.referenceTable.get(target); if (!attribs) { attribs = new Map(); CfnReference.referenceTable.set(target, attribs); } - let ref = attribs.get(attribKey); - if (!ref || sub) { + const cacheKey = attribKey + (fnSub ? 'Fn::Sub' : ''); + let ref = attribs.get(cacheKey); + if (!ref) { ref = fresh(); - attribs.set(attribKey, ref); + attribs.set(cacheKey, ref); } return ref; } From 1d91d61c77cb85a10669285206a708a3207518af Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 29 Jul 2020 15:57:28 -0400 Subject: [PATCH 43/46] incorporated review requests, improved testing --- .../test/invalid-templates.test.ts | 20 ++----- .../test-templates/fn-sub-brace-edges.json | 55 +++++++++++++++++++ ...lid-escaping.json => fn-sub-escaping.json} | 2 +- .../fn-sub-map-dotted-attributes.json | 11 +--- .../test/test-templates/fn-sub-override.json | 16 ++++++ .../test-templates/fn-sub-parsing-edges.json | 0 .../fn-sub-shadow-attribute.json | 5 +- .../test/test-templates/fn-sub-shadow.json | 5 +- ...b-valid-string.json => fn-sub-string.json} | 0 ...-bad-get-att.json => fn-sub-${}-only.json} | 6 +- .../invalid/fn-sub-invalid-syntax.json | 10 ---- .../invalid/fn-sub-key-not-in-map.json | 16 ------ .../fn-sub-key-not-in-template-string.json | 4 +- .../invalid/fn-sub-key-not-in-template.json | 10 ---- .../yaml/invalid/short-form-import-sub.yaml | 7 +++ .../test-templates/yaml/long-form-vpc.yaml | 13 +++-- ...ring.yaml => short-form-fnsub-string.yaml} | 0 .../test/valid-templates.test.ts | 36 +++++++++--- .../test/yaml-templates.test.ts | 14 +++-- packages/@aws-cdk/core/lib/cfn-parse.ts | 41 +++++--------- packages/@aws-cdk/core/lib/cfn-resource.ts | 2 +- .../core/lib/private/cfn-reference.ts | 6 +- 22 files changed, 157 insertions(+), 122 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json rename packages/@aws-cdk/cloudformation-include/test/test-templates/{fn-sub-valid-escaping.json => fn-sub-escaping.json} (80%) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parsing-edges.json rename packages/@aws-cdk/cloudformation-include/test/test-templates/{fn-sub-valid-string.json => fn-sub-string.json} (100%) rename packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/{fn-sub-bad-get-att.json => fn-sub-${}-only.json} (53%) delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json delete mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template.json create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml rename packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/{sub-string.yaml => short-form-fnsub-string.yaml} (100%) diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts index c1b57f3318c1a..113e58773a242 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -104,25 +104,13 @@ describe('CDK Include', () => { test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => { expect(() => { includeTestTemplate(stack, 'fn-sub-key-not-in-template-string.json'); - }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); + }).toThrow(/Element referenced in Fn::Sub expression with logical ID: 'AFakeResource' was not found in the template/); }); - test("throws a validation exception when Fn::Sub in map form uses a key that isn't in the template", () => { + test('throws a validation exception when Fn::Sub has an empty ${} reference', () => { expect(() => { - includeTestTemplate(stack, 'fn-sub-key-not-in-template.json'); - }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' not found/); - }); - - test("throws a validation exception when Fn::Sub in map form uses a key that isn't in the template and isn't in the map", () => { - expect(() => { - includeTestTemplate(stack, 'fn-sub-key-not-in-map.json'); - }).toThrow(/Element used in Ref expression with logical ID: 'AFakeResource' in Fn::Sub not found/); - }); - - test("throws a validation exception when Fn::Sub uses a resource attribute that doesn't exist", () => { - expect(() => { - includeTestTemplate(stack, 'fn-sub-bad-get-att.json'); - }).toThrow(/Resource referenced in Fn::Sub expression with logical ID: 'FakeBucket' not found/); + includeTestTemplate(stack, 'fn-sub-${}-only.json'); + }).toThrow(/Element referenced in Fn::Sub expression with logical ID: '' was not found in the template/); }); }); diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json new file mode 100644 index 0000000000000..6cb7e0832a8d4 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json @@ -0,0 +1,55 @@ +{ + "Resources": { + "Bucket": { + "Type": "Custom::ManyStrings", + "Properties": { + "SymbolsOnly": { + "DollarSign": { + "Fn::Sub": "$" + }, + "OpeningBrace": { + "Fn::Sub": "{" + }, + "ClosingBrace": { + "Fn::Sub": "}" + }, + "DollarOpeningBrace": { + "Fn::Sub": "${" + }, + "DollarClosingBrace": { + "Fn::Sub": "$}" + }, + "OpeningBraceDollar": { + "Fn::Sub": "{$" + }, + "ClosingBraceDollar": { + "Fn::Sub": "}$" + } + }, + "SymbolsAndResources": { + "DollarSign": { + "Fn::Sub": "DoesNotExist$DoesNotExist" + }, + "OpeningBrace": { + "Fn::Sub": "DoesNotExist{DoesNotExist" + }, + "ClosingBrace": { + "Fn::Sub": "DoesNotExist}DoesNotExist" + }, + "DollarOpeningBrace": { + "Fn::Sub": "DoesNotExist${DoesNotExist" + }, + "DollarClosingBrace": { + "Fn::Sub": "DoesNotExist$}DoesNotExist" + }, + "OpeningBraceDollar": { + "Fn::Sub": "DoesNotExist{$DoesNotExist" + }, + "ClosingBraceDollar": { + "Fn::Sub": "DoesNotExist}$DoesNotExist" + } + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json similarity index 80% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json index 47ce8fd65e0ba..915a65819aec7 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-escaping.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json @@ -3,7 +3,7 @@ "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "some-bucket${!AWS::AccountId}7896${ ! AWS::Region}${!Immediate}234" } + "BucketName": { "Fn::Sub": "some-bucket${!AWS::AccountId}7896${ ! DoesNotExist}${!Immediate}234" } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json index d83248dec0e74..5bc772b8f5860 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json @@ -4,13 +4,7 @@ "Type": "AWS::S3::Bucket", "Properties": { "BucketName": { - "Fn::Sub": [ - "${Region}${ELB}-${ELB.SourceSecurityGroup.GroupName}-${Name}", - { - "Name": { "Ref" : "ELB" }, - "Region": { "Fn::Base64": "AWS::Region" } - } - ] + "Fn::Sub": "${ELB.SourceSecurityGroup.GroupName}" } } }, @@ -20,8 +14,7 @@ "AvailabilityZones": [ "us-east-1a" ], - "Listeners": [ - { + "Listeners": [{ "LoadBalancerPort": "80", "InstancePort": "80", "Protocol": "HTTP" diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json new file mode 100644 index 0000000000000..cdaa623bfd7fa --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json @@ -0,0 +1,16 @@ +{ + "Resources": { + "Bucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": "bucket" + } + }, + "AnotherBucket": { + "Type": "AWS::S3::Bucket", + "Properties": { + "BucketName": { "Fn::Sub": "${Bucket}-${!Bucket}-${Bucket.DomainName}" } + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parsing-edges.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parsing-edges.json new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json index 81b035d04d520..8401ee9a79ccb 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json @@ -14,10 +14,7 @@ } }, "AnotherBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": "another-bucket" - } + "Type": "AWS::S3::Bucket" } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json index 14e0c075fa501..6e5cdbee0be2c 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json @@ -14,10 +14,7 @@ } }, "AnotherBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": "another-bucket" - } + "Type": "AWS::S3::Bucket" } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-string.json similarity index 100% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-valid-string.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-string.json diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json similarity index 53% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json rename to packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json index 53421fd679f4f..87f9556e5a6b0 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-bad-get-att.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json @@ -1,9 +1,11 @@ { "Resources": { - "Resource": { + "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "BucketName": { "Fn::Sub": "${FakeBucket.BucketName}" } + "BucketName": { + "Fn::Sub": "${}" + } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json deleted file mode 100644 index aa79385fbef47..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-invalid-syntax.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "Resources": { - "AnotherBucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "AccessControl": { "Fn::Sub": "unusued-bucket-fn-sub-${ x ! AFakeResource}" } - } - } - } -} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json deleted file mode 100644 index 10bfb69a085dc..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-map.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AFakeResource}", { "Name": {"Ref" : "AnotherResource" }, "Region": {"Fn::Base64": "a-string" }} ]} - } - }, - "AnotherResource": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": "my-bucket" - } - } - } -} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json index c1ad988c12046..c5e9ff5b13b8d 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json @@ -1,9 +1,9 @@ { "Resources": { - "AnotherBucket": { + "Bucket": { "Type": "AWS::S3::Bucket", "Properties": { - "AccessControl": { "Fn::Sub": "unusued-bucket-fn-sub-${AFakeResource}" } + "AccessControl": { "Fn::Sub": "${AFakeResource}" } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template.json deleted file mode 100644 index 7f3be30f56a96..0000000000000 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "Resources": { - "Bucket": { - "Type": "AWS::S3::Bucket", - "Properties": { - "BucketName": { "Fn::Sub": [ "${Name}-${Region}-here-${AFakeResource}", { "Name": {"Ref" : "AFakeResource" }, "Region": {"Fn::Base64": "a-string" }} ]} - } - } - } -} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml new file mode 100644 index 0000000000000..899904f61a8cf --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml @@ -0,0 +1,7 @@ +Resources: + Bucket: + Type: AWS::S3::Bucket + Properties: + BucketName: + !ImportValue + !Sub ${AWS::Region} diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml index 2e1e3d4714487..f32def7fd072a 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml @@ -43,9 +43,10 @@ Resources: AnotherParameter: Fn::Base64: AnotherValue AccessControl: - Fn::Sub: - - "${Region}-foo-${!Immediate}-foo-${Vpc}-${Vpc.Id}-${Name}" - - Name: - Ref: Vpc - Region: - Fn::Base64: AWS::Region + Fn::ImportValue: + Fn::Sub: + - "${Region}-foo-${!Immediate}-foo-${Vpc}-${Vpc.Id}-${Name}" + - Name: + Ref: Vpc + Region: + Fn::Base64: AWS::Region diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/sub-string.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-fnsub-string.yaml similarity index 100% rename from packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/sub-string.yaml rename to packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-fnsub-string.yaml diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts index ae0bc8303addf..07529248be53b 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -193,18 +193,18 @@ describe('CDK Include', () => { }); test('can ingest a template with Fn::Sub in string form with escaped and unescaped references and output it unchanged', () => { - includeTestTemplate(stack, 'fn-sub-valid-string.json'); + includeTestTemplate(stack, 'fn-sub-string.json'); expect(stack).toMatchTemplate( - loadTestFileToJsObject('fn-sub-valid-string.json'), + loadTestFileToJsObject('fn-sub-string.json'), ); }); - test('can ingest a template with Fn::Sub in string form with escaped references and output it unchanged', () => { - includeTestTemplate(stack, 'fn-sub-valid-escaping.json'); + test('can parse the string argument Fn::Sub with escaped references that contain whitespace', () => { + includeTestTemplate(stack, 'fn-sub-escaping.json'); expect(stack).toMatchTemplate( - loadTestFileToJsObject('fn-sub-valid-escaping.json'), + loadTestFileToJsObject('fn-sub-escaping.json'), ); }); @@ -216,7 +216,7 @@ describe('CDK Include', () => { ); }); - test('can ingest a template with Fn::Sub with a key that is a logicalID and output it unchanged', () => { + test('can ingest a template with Fn::Sub shadowing a logical ID from the template and output it unchanged', () => { includeTestTemplate(stack, 'fn-sub-shadow.json'); expect(stack).toMatchTemplate( @@ -224,7 +224,7 @@ describe('CDK Include', () => { ); }); - test('can ingest a template with Fn::Sub with a key that is a logicalID with an attribue and output it unchanged', () => { + test('can ingest a template with Fn::Sub attribute expression shadowing a logical ID from the template, and output it unchanged', () => { includeTestTemplate(stack, 'fn-sub-shadow-attribute.json'); expect(stack).toMatchTemplate( @@ -232,7 +232,7 @@ describe('CDK Include', () => { ); }); - test('can modify resources used in Fn::Sub references and see the changes in the template', () => { + test('can modify resources used in Fn::Sub in map form references and see the changes in the template', () => { const cfnTemplate = includeTestTemplate(stack, 'fn-sub-shadow.json'); cfnTemplate.getResource('AnotherBucket').overrideLogicalId('NewBucket'); @@ -249,6 +249,26 @@ describe('CDK Include', () => { }); }); + test('can modify resources used in Fn::Sub in string form and see the changes in the template', () => { + const cfnTemplate = includeTestTemplate(stack, 'fn-sub-override.json'); + + cfnTemplate.getResource('Bucket').overrideLogicalId('NewBucket'); + + expect(stack).toHaveResourceLike('AWS::S3::Bucket', { + "BucketName": { + "Fn::Sub": "${NewBucket}-${!Bucket}-${NewBucket.DomainName}", + }, + }); + }); + + test('can ingest a template with Fn::Sub with brace edge cases and output it unchanged', () => { + includeTestTemplate(stack, 'fn-sub-brace-edges.json'); + + expect(stack).toMatchTemplate( + loadTestFileToJsObject('fn-sub-brace-edges.json'), + ); + }); + test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => { includeTestTemplate(stack, 'ref-array-property.json'); diff --git a/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts index 1c4e4064a504c..11180842bdb34 100644 --- a/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts @@ -324,21 +324,27 @@ describe('CDK Include', () => { ); }); - test('can ingest a yaml tempalte with Fn::Sub in string form and output it unchanged', () => { - includeTestTemplate(stack, 'sub-string.yaml'); + test('can ingest a YAML tempalte with Fn::Sub in string form and output it unchanged', () => { + includeTestTemplate(stack, 'short-form-fnsub-string.yaml'); expect(stack).toMatchTemplate( - loadTestFileToJsObject('sub-string.yaml'), + loadTestFileToJsObject('short-form-fnsub-string.yaml'), ); }); - test('can ingest a yaml tempalte with Fn::Sub in map form and output it unchanged', () => { + test('can ingest a YAML tmeplate with Fn::Sub in map form and output it unchanged', () => { includeTestTemplate(stack, 'short-form-sub-map.yaml'); expect(stack).toMatchTemplate( loadTestFileToJsObject('short-form-sub-map.yaml'), ); }); + + test('the parser throws an error on a YAML tmeplate with short form import value that uses short form sub', () => { + expect(() => { + includeTestTemplate(stack, 'invalid/short-form-import-sub.yaml'); + }).toThrow(/A node can have at most one tag/); + }); }); function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude { diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index bbc91d22fd644..b38d9a96e553c 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -461,16 +461,18 @@ export class CfnParser { : undefined; } - private parseFnSubString(value: string, map: { [key: string]: any } | undefined): string { + private parseFnSubString(value: string, map: { [key: string]: any } = {}): string { const leftBrace = value.indexOf('${'); const rightBrace = value.indexOf('}') + 1; + // don't include left and right braces when searching for the target of the reference + if (leftBrace === -1 || leftBrace >= rightBrace) { + return value; + } + const leftHalf = value.substring(0, leftBrace); const rightHalf = value.substring(rightBrace); - // don't include left and right braces when searching for the target of the reference const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim(); - if (leftBrace === -1) { - return value; - } else if (refTarget[0] === '!') { + if (refTarget[0] === '!') { return value.substring(0, rightBrace) + this.parseFnSubString(rightHalf, map); } @@ -485,20 +487,22 @@ export class CfnParser { return leftHalf + specialRef + this.parseFnSubString(rightHalf, map); } - const isRef = refTarget.indexOf('.') === -1; + const dotIndex = refTarget.indexOf('.'); + const isRef = dotIndex === -1; if (isRef) { const refElement = this.options.finder.findRefTarget(refTarget); if (!refElement) { - throw new Error(`Element used in Ref expression with logical ID: '${refTarget}' in Fn::Sub not found`); + throw new Error(`Element referenced in Fn::Sub expression with logical ID: '${refTarget}' was not found in the template`); } return leftHalf + CfnReference.for(refElement, 'Ref', true).toString() + this.parseFnSubString(rightHalf, map); } else { - const targetId = refTarget.substring(0, refTarget.indexOf('.')); + const targetId = refTarget.substring(0, dotIndex); const refResource = this.options.finder.findResource(targetId); if (!refResource) { - throw new Error(`Resource referenced in Fn::Sub expression with logical ID: '${targetId}' not found`); + throw new Error(`Resource referenced in Fn::Sub expression with logical ID: '${targetId}' was not found in the template`); } - return leftHalf + CfnReference.for(refResource, refTarget.substring(refTarget.indexOf('.') + 1), true).toString() + this.parseFnSubString(rightHalf, map); + const attribute = refTarget.substring(dotIndex + 1); + return leftHalf + CfnReference.for(refResource, attribute, true).toString() + this.parseFnSubString(rightHalf, map); } } } @@ -518,22 +522,7 @@ function specialCaseRefs(value: any): any { } function specialCaseSubRefs(value: any): any { - switch (value) { - case 'AWS::AccountId': return pseudoString(value); - case 'AWS::Region': return pseudoString(value); - case 'AWS::Partition': return pseudoString(value); - case 'AWS::URLSuffix': return pseudoString(value); - case 'AWS::NotificationARNs': return pseudoString(value); - case 'AWS::StackId': return pseudoString(value); - case 'AWS::StackName': return pseudoString(value); - case 'AWS::NoValue': return pseudoString(value); - case 'AWS::NoValue': return pseudoString(value); - default: return undefined; - } -} - -function pseudoString(value: any): any { - return Token.asString('${' + value + '}', {displayHint: value }); + return value.indexOf('::') === -1 ? undefined: '${' + value + '}'; } function undefinedIfAllValuesAreEmpty(object: object): object | undefined { diff --git a/packages/@aws-cdk/core/lib/cfn-resource.ts b/packages/@aws-cdk/core/lib/cfn-resource.ts index 5146764d32d14..39b60a3d248b7 100644 --- a/packages/@aws-cdk/core/lib/cfn-resource.ts +++ b/packages/@aws-cdk/core/lib/cfn-resource.ts @@ -134,7 +134,7 @@ export class CfnResource extends CfnRefElement { * in case there is no generated attribute. * @param attributeName The name of the attribute. */ - public getAtt(attributeName: string ): Reference { + public getAtt(attributeName: string): Reference { return CfnReference.for(this, attributeName); } diff --git a/packages/@aws-cdk/core/lib/private/cfn-reference.ts b/packages/@aws-cdk/core/lib/private/cfn-reference.ts index 530e0297a4d7d..491232e344840 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-reference.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-reference.ts @@ -40,8 +40,8 @@ export class CfnReference extends Reference { public static for(target: CfnElement, attribute: string, fnSub: boolean = false) { return CfnReference.singletonReference(target, attribute, fnSub, () => { const cfnIntrinsic = fnSub - ? (attribute === 'Ref' ? '${' + target.logicalId + '}' : '${' + target.logicalId + '.' + attribute + '}') - : (attribute === 'Ref' ? { Ref: target.logicalId } : { 'Fn::GetAtt': [ target.logicalId, attribute ] }); + ? ('${' + target.logicalId + (attribute === 'Ref' ? '' : `.${attribute}`) + '}') + : (attribute === 'Ref' ? { Ref: target.logicalId } : { 'Fn::GetAtt': [target.logicalId, attribute] }); return new CfnReference(cfnIntrinsic, attribute, target); }); } @@ -63,7 +63,7 @@ export class CfnReference extends Reference { /** * Get or create the table. - * Defining sub allows cloudformation-include to correctly handle Fn::Sub. + * Passing fnSub = true allows cloudformation-include to correctly handle Fn::Sub. */ private static singletonReference(target: Construct, attribKey: string, fnSub: boolean, fresh: () => CfnReference) { let attribs = CfnReference.referenceTable.get(target); From 58a34960a889e400e302623e3bbc91025b80e6c0 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 29 Jul 2020 15:59:51 -0400 Subject: [PATCH 44/46] added newline --- .../test/test-templates/fn-sub-brace-edges.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json index 6cb7e0832a8d4..b8ef634ac6cd0 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json @@ -52,4 +52,4 @@ } } } -} \ No newline at end of file +} From 3067019950e383e2cc7f38b35ccb13fba580c26f Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Wed, 29 Jul 2020 14:06:51 -0700 Subject: [PATCH 45/46] Update the type of specialCaseSubRefs like in the comment --- packages/@aws-cdk/core/lib/cfn-parse.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index b38d9a96e553c..64b898ddca63b 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -521,7 +521,7 @@ function specialCaseRefs(value: any): any { } } -function specialCaseSubRefs(value: any): any { +function specialCaseSubRefs(value: string): string | undefined { return value.indexOf('::') === -1 ? undefined: '${' + value + '}'; } From 4932e78ad8f1135db254b714cf87d2d9f290d2c2 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Wed, 29 Jul 2020 14:14:17 -0700 Subject: [PATCH 46/46] Remove the redundant map || {} expression --- packages/@aws-cdk/core/lib/cfn-parse.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts index 64b898ddca63b..c821b66971073 100644 --- a/packages/@aws-cdk/core/lib/cfn-parse.ts +++ b/packages/@aws-cdk/core/lib/cfn-parse.ts @@ -477,7 +477,7 @@ export class CfnParser { } // lookup in map - if (refTarget in (map || {})) { + if (refTarget in map) { return leftHalf + '${' + refTarget + '}' + this.parseFnSubString(rightHalf, map); }