From f7f23a7f418aa2e4c694c008f3d8895a8f74101b Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Mon, 18 Jul 2022 14:11:06 -0700 Subject: [PATCH] fix(cfn-include): preserve unrecognized resource attributes (#19920) When including, currently, we error out if we encounter any resource attribute that we don't recognize. This PR instead handles the unknown attributes using "escape hatch" overrides. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cloudformation-include/lib/cfn-include.ts | 27 +++++++++---------- .../test/invalid-templates.test.ts | 4 +-- .../non-existent-resource-attribute.json | 8 ++++-- .../non-existent-resource-attribute.json | 8 ++++++ .../test/valid-templates.test.ts | 8 ++++++ 5 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/non-existent-resource-attribute.json diff --git a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts index 238118f20ab51..778bd9a124152 100644 --- a/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts +++ b/packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts @@ -595,20 +595,6 @@ export class CfnInclude extends core.CfnElement { return ret; } - const resourceAttributes: any = this.template.Resources[logicalId]; - - // fail early for resource attributes we don't support yet - const knownAttributes = [ - 'Condition', 'DependsOn', 'Description', 'Metadata', 'Properties', 'Type', 'Version', - 'CreationPolicy', 'DeletionPolicy', 'UpdatePolicy', 'UpdateReplacePolicy', - ]; - for (const attribute of Object.keys(resourceAttributes)) { - if (!knownAttributes.includes(attribute)) { - throw new Error(`The '${attribute}' resource attribute is not supported by cloudformation-include yet. ` + - 'Either remove it from the template, or use the CdkInclude class from the core package instead.'); - } - } - const self = this; const finder: cfn_parse.ICfnFinder = { findCondition(conditionName: string): core.CfnCondition | undefined { @@ -639,6 +625,7 @@ export class CfnInclude extends core.CfnElement { parameters: this.parametersToReplace, }); + const resourceAttributes: any = this.template.Resources[logicalId]; let l1Instance: core.CfnResource; if (this.nestedStacksToInclude[logicalId]) { l1Instance = this.createNestedStack(logicalId, cfnParser); @@ -667,6 +654,18 @@ export class CfnInclude extends core.CfnElement { this.overrideLogicalIdIfNeeded(l1Instance, logicalId); this.resources[logicalId] = l1Instance; + + // handle any unknown attributes using overrides + const knownAttributes = [ + 'Condition', 'DependsOn', 'Description', 'Metadata', 'Properties', 'Type', 'Version', + 'CreationPolicy', 'DeletionPolicy', 'UpdatePolicy', 'UpdateReplacePolicy', + ]; + for (const [attrName, attrValue] of Object.entries(resourceAttributes)) { + if (!knownAttributes.includes(attrName)) { + l1Instance.addOverride(attrName, cfnParser.parseValue(attrValue)); + } + } + return l1Instance; } 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 eeaf03c77dcd2..2fa4a3f4d1f6a 100644 --- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts @@ -79,10 +79,10 @@ describe('CDK Include', () => { }).toThrow(/Unsupported CloudFormation function 'Fn::ValueOfAll'/); }); - test('throws a validation exception when encountering an unrecognized resource attribute', () => { + test('parses even the unrecognized attributes of resources', () => { expect(() => { includeTestTemplate(stack, 'non-existent-resource-attribute.json'); - }).toThrow(/The 'NonExistentResourceAttribute' resource attribute is not supported by cloudformation-include yet/); + }).toThrow(/Element used in Ref expression with logical ID: 'NonExistentResource' not found/); }); test("throws a validation exception when encountering a Ref-erence to a template element that doesn't exist", () => { diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/non-existent-resource-attribute.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/non-existent-resource-attribute.json index fc8d8f5f83454..075766e90e7dd 100644 --- a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/non-existent-resource-attribute.json +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/non-existent-resource-attribute.json @@ -1,8 +1,12 @@ { "Resources": { - "Bucket2": { + "Bucket": { "Type": "AWS::S3::Bucket", - "NonExistentResourceAttribute": "Bucket1" + "NonExistentResourceAttribute": { + "SomeProp": { + "Ref": "NonExistentResource" + } + } } } } diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/non-existent-resource-attribute.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/non-existent-resource-attribute.json new file mode 100644 index 0000000000000..fc8d8f5f83454 --- /dev/null +++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/non-existent-resource-attribute.json @@ -0,0 +1,8 @@ +{ + "Resources": { + "Bucket2": { + "Type": "AWS::S3::Bucket", + "NonExistentResourceAttribute": "Bucket1" + } + } +} 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 f5d4504cfc00e..dd7ce7e875a23 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -616,6 +616,14 @@ describe('CDK Include', () => { ); }); + test('preserves unknown resource attributes', () => { + includeTestTemplate(stack, 'non-existent-resource-attribute.json'); + + Template.fromStack(stack).templateMatches( + loadTestFileToJsObject('non-existent-resource-attribute.json'), + ); + }); + test("correctly handles referencing the ingested template's resources across Stacks", () => { // for cross-stack sharing to work, we need an App const app = new core.App();