Skip to content

Commit

Permalink
feat(cfn-include): add support for the DependsOn attribute
Browse files Browse the repository at this point in the history
DependsOn is modeled in the CDK L1s as references to the actual object instances.
To make it possible to get those references from inside the fromCloudFormation
static methods of the L1 classes that are used in the CfnInclude,
add a new interface to @aws-cdk/core, ICfnFinder,
as a 4th parameter to the fromCloudFormation method
(wrapped in an Options interface for future-proofing)
that serves as a callback
to find the L1 object instance with the given logical ID.

----

*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 committed May 19, 2020
1 parent 9eb8db2 commit 613df1b
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 11 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cloudformation-include/README.md
Expand Up @@ -105,7 +105,7 @@ All items unchecked below are currently not supported.

- [x] Properties
- [ ] Condition
- [ ] DependsOn
- [x] DependsOn
- [ ] CreationPolicy
- [ ] UpdatePolicy
- [x] UpdateReplacePolicy
Expand Down
16 changes: 14 additions & 2 deletions packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts
Expand Up @@ -92,7 +92,7 @@ export class CfnInclude extends core.CfnElement {
throw new Error(`Unrecognized CloudFormation resource type: '${resourceAttributes.Type}'`);
}
// fail early for resource attributes we don't support yet
const knownAttributes = ['Type', 'Properties', 'DeletionPolicy', 'UpdateReplacePolicy', 'Metadata'];
const knownAttributes = ['Type', 'Properties', 'DependsOn', 'DeletionPolicy', 'UpdateReplacePolicy', 'Metadata'];
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. ` +
Expand All @@ -103,7 +103,19 @@ export class CfnInclude extends core.CfnElement {
const [moduleName, ...className] = l1ClassFqn.split('.');
const module = require(moduleName); // eslint-disable-line @typescript-eslint/no-require-imports
const jsClassFromModule = module[className.join('.')];
const l1Instance = jsClassFromModule.fromCloudFormation(this, logicalId, resourceAttributes);
const self = this;
const finder: core.ICfnFinder = {
findResource(lId: string): core.CfnResource | undefined {
if (!(lId in (self.template.Resources || {}))) {
return undefined;
}
return self.getOrCreateResource(lId);
},
};
const options: core.FromCloudFormationOptions = {
finder,
};
const l1Instance = jsClassFromModule.fromCloudFormation(this, logicalId, resourceAttributes, options);

if (this.preserveLogicalIds) {
// override the logical ID to match the original template
Expand Down
Expand Up @@ -40,6 +40,12 @@ describe('CDK Include', () => {
SynthUtils.synthesize(stack);
}).toThrow(/allowedOrigins: required but missing/);
});

test("throws a validation exception for a template with a DependsOn that doesn't exist", () => {
expect(() => {
includeTestTemplate(stack, 'non-existent-depends-on.json');
}).toThrow(/Resource 'Bucket2' depends on 'Bucket1' that doesn't exist/);
});
});

function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {
Expand Down
@@ -0,0 +1,11 @@
{
"Resources": {
"Bucket2": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "bucket2"
},
"DependsOn": "Bucket1"
}
}
}
@@ -0,0 +1,17 @@
{
"Resources": {
"Bucket0": {
"Type": "AWS::S3::Bucket"
},
"Bucket1": {
"Type": "AWS::S3::Bucket"
},
"Bucket2": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "bucket2"
},
"DependsOn": ["Bucket0", "Bucket1"]
}
}
}
Expand Up @@ -5,6 +5,9 @@
},
"Bucket2": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "bucket2"
},
"DependsOn": "Bucket1"
}
}
Expand Down
@@ -1,3 +1,4 @@
import { ResourcePart } from '@aws-cdk/assert';
import '@aws-cdk/assert/jest';
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
Expand Down Expand Up @@ -199,6 +200,38 @@ describe('CDK Include', () => {
);
});

test('resolves DependsOn with a single String value to the actual L1 class instance', () => {
const cfnTemplate = includeTestTemplate(stack, 'resource-attribute-depends-on.json');
const cfnBucket2 = cfnTemplate.getResource('Bucket2');

expect(cfnBucket2.node.dependencies).toHaveLength(1);
// we always render dependsOn as an array, even if it's a single string
expect(stack).toHaveResourceLike('AWS::S3::Bucket', {
"Properties": {
"BucketName": "bucket2",
},
"DependsOn": [
"Bucket1",
],
}, ResourcePart.CompleteDefinition);
});

test('resolves DependsOn with an array of String values to the actual L1 class instances', () => {
const cfnTemplate = includeTestTemplate(stack, 'resource-attribute-depends-on-array.json');
const cfnBucket2 = cfnTemplate.getResource('Bucket2');

expect(cfnBucket2.node.dependencies).toHaveLength(2);
expect(stack).toHaveResourceLike('AWS::S3::Bucket', {
"Properties": {
"BucketName": "bucket2",
},
"DependsOn": [
"Bucket0",
"Bucket1",
],
}, ResourcePart.CompleteDefinition);
});

test("throws an exception when encountering a Resource type it doesn't recognize", () => {
expect(() => {
includeTestTemplate(stack, 'non-existent-resource-type.json');
Expand All @@ -217,12 +250,6 @@ describe('CDK Include', () => {
}).toThrow(/The Condition resource attribute is not supported by cloudformation-include yet/);
});

test('throws an exception when encountering the DependsOn attribute in a resource', () => {
expect(() => {
includeTestTemplate(stack, 'resource-attribute-depends-on.json');
}).toThrow(/The DependsOn resource attribute is not supported by cloudformation-include yet/);
});

test('throws an exception when encountering the CreationPolicy attribute in a resource', () => {
expect(() => {
includeTestTemplate(stack, 'resource-attribute-creation-policy.json');
Expand Down
29 changes: 29 additions & 0 deletions packages/@aws-cdk/core/lib/from-cfn.ts
@@ -0,0 +1,29 @@
import { CfnResource } from './cfn-resource';

/**
* An interface that represents callbacks into a CloudFormation template.
* Used by the fromCloudFormation methods in the generated L1 classes.
*
* @experimental
*/
export interface ICfnFinder {
/**
* Returns the resource with the given logical ID in the template.
* If a resource with that logical ID was not found in the template,
* returns undefined.
*/
findResource(logicalId: string): CfnResource | undefined;
}

/**
* The interface used as the last argument to the fromCloudFormation
* static method of the generated L1 classes.
*
* @experimental
*/
export interface FromCloudFormationOptions {
/**
* The finder interface used to resolve references across the template.
*/
readonly finder: ICfnFinder;
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/core/lib/index.ts
Expand Up @@ -28,6 +28,7 @@ export * from './cfn-tag';
export * from './removal-policy';
export * from './arn';
export * from './duration';
export * from './from-cfn';
export * from './size';
export * from './stack-trace';

Expand Down
17 changes: 15 additions & 2 deletions tools/cfn2ts/lib/codegen.ts
Expand Up @@ -232,7 +232,7 @@ export default class CodeGenerator {
this.code.line(' *');
this.code.line(' * @experimental');
this.code.line(' */');
this.code.openBlock(`public static fromCloudFormation(scope: ${CONSTRUCT_CLASS}, id: string, resourceAttributes: any): ` +
this.code.openBlock(`public static fromCloudFormation(scope: ${CONSTRUCT_CLASS}, id: string, resourceAttributes: any, options: ${CORE}.FromCloudFormationOptions): ` +
`${resourceName.className}`);
this.code.line('resourceAttributes = resourceAttributes || {};');
if (propsType) {
Expand All @@ -252,11 +252,24 @@ export default class CodeGenerator {
this.code.line(`cfnOptions.deletionPolicy = ${CFN_PARSE}.FromCloudFormation.parseDeletionPolicy(resourceAttributes.DeletionPolicy);`);
this.code.line(`cfnOptions.updateReplacePolicy = ${CFN_PARSE}.FromCloudFormation.parseDeletionPolicy(resourceAttributes.UpdateReplacePolicy);`);
this.code.line(`cfnOptions.metadata = ${CFN_PARSE}.FromCloudFormation.parseValue(resourceAttributes.Metadata);`);

// handle DependsOn
this.code.line('// handle DependsOn');
// DependsOn can be either a single string, or an array of strings
this.code.line('resourceAttributes.DependsOn = resourceAttributes.DependsOn ?? [];');
this.code.line('const dependencies: string[] = Array.isArray(resourceAttributes.DependsOn) ? resourceAttributes.DependsOn : [resourceAttributes.DependsOn];');
this.code.openBlock('for (const dep of dependencies)');
this.code.line('const depResource = options.finder.findResource(dep);');
this.code.openBlock('if (!depResource)');
this.code.line("throw new Error(`Resource '${id}' depends on '${dep}' that doesn't exist`);");
this.code.closeBlock();
this.code.line('ret.node.addDependency(depResource);');
this.code.closeBlock();

// ToDo handle:
// 1. Condition
// 2. CreationPolicy
// 3. UpdatePolicy
// 4. DependsOn

this.code.line('return ret;');
this.code.closeBlock();
Expand Down

0 comments on commit 613df1b

Please sign in to comment.