Skip to content

Commit 2c2bc0b

Browse files
authored
fix(cfn-include): detect a resource cycle in the included template (#19871)
Add code that detects when the CloudFormation template being included contains a cycle between any of its resources. While that's not allowed in pure CloudFormation, Serverless templates can unfortunately contain cycles before they are processed. Fixes #16654 ---- ### All Submissions: * [ ] 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*
1 parent 5472b11 commit 2c2bc0b

File tree

3 files changed

+29
-2
lines changed

3 files changed

+29
-2
lines changed

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,12 @@ export class CfnInclude extends core.CfnElement {
584584
return cfnCondition;
585585
}
586586

587-
private getOrCreateResource(logicalId: string): core.CfnResource {
587+
private getOrCreateResource(logicalId: string, cycleChain: string[] = []): core.CfnResource {
588+
cycleChain = cycleChain.concat([logicalId]);
589+
if (cycleChain.length !== new Set(cycleChain).size) {
590+
throw new Error(`Found a cycle between resources in the template: ${cycleChain.join(' depends on ')}`);
591+
}
592+
588593
const ret = this.resources[logicalId];
589594
if (ret) {
590595
return ret;
@@ -618,7 +623,7 @@ export class CfnInclude extends core.CfnElement {
618623
if (!(lId in (self.template.Resources || {}))) {
619624
return undefined;
620625
}
621-
return self.getOrCreateResource(lId);
626+
return self.getOrCreateResource(lId, cycleChain);
622627
},
623628

624629
findRefTarget(elementName: string): core.CfnElement | undefined {

packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts

+6
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,12 @@ describe('CDK Include', () => {
139139
includeTestTemplate(stack, 'short-form-get-att-no-dot.yaml');
140140
}).toThrow(/Short-form Fn::GetAtt must contain a '.' in its string argument, got: 'Bucket1Arn'/);
141141
});
142+
143+
test('detects a cycle between resources in the template', () => {
144+
expect(() => {
145+
includeTestTemplate(stack, 'cycle-in-resources.json');
146+
}).toThrow(/Found a cycle between resources in the template: Bucket1 depends on Bucket2 depends on Bucket1/);
147+
});
142148
});
143149

144150
function includeTestTemplate(scope: constructs.Construct, testTemplate: string): inc.CfnInclude {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"Resources": {
3+
"Bucket1": {
4+
"Type": "AWS::S3::Bucket",
5+
"Properties": {
6+
"BucketName": {
7+
"Ref": "Bucket2"
8+
}
9+
}
10+
},
11+
"Bucket2": {
12+
"Type": "AWS::S3::Bucket",
13+
"DependsOn": "Bucket1"
14+
}
15+
}
16+
}

0 commit comments

Comments
 (0)