Skip to content

Commit

Permalink
fix(cfn-include): preserve unrecognized resource attributes (aws#19920)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
skinny85 authored and comcalvi committed Jul 25, 2022
1 parent 74d358f commit 7d3decb
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 18 deletions.
27 changes: 13 additions & 14 deletions packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
{
"Resources": {
"Bucket2": {
"Bucket": {
"Type": "AWS::S3::Bucket",
"NonExistentResourceAttribute": "Bucket1"
"NonExistentResourceAttribute": {
"SomeProp": {
"Ref": "NonExistentResource"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"Resources": {
"Bucket2": {
"Type": "AWS::S3::Bucket",
"NonExistentResourceAttribute": "Bucket1"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 7d3decb

Please sign in to comment.