Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cfn-include): fails to load SAM resources #9442

Merged
merged 3 commits into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import * as path from 'path';
import '@aws-cdk/assert/jest';
import * as core from '@aws-cdk/core';
import * as inc from '../lib';
import * as futils from '../lib/file-utils';

/* eslint-disable quote-props */
/* eslint-disable quotes */

describe('CDK Include for templates with SAM transform', () => {
let stack: core.Stack;

beforeEach(() => {
stack = new core.Stack();
});

test('can ingest a template with only a minimal SAM function using S3Location for CodeUri, and output it unchanged', () => {
includeTestTemplate(stack, 'only-minimal-sam-function-codeuri-as-s3location.yaml');

expect(stack).toMatchTemplate(
loadTestFileToJsObject('only-minimal-sam-function-codeuri-as-s3location.yaml'),
);
});

test('can ingest a template with only a SAM function using an array with DDB CRUD for Policies, and output it unchanged', () => {
includeTestTemplate(stack, 'only-sam-function-policies-array-ddb-crud.yaml');

expect(stack).toMatchTemplate(
loadTestFileToJsObject('only-sam-function-policies-array-ddb-crud.yaml'),
);
});

test('can ingest a template with only a minimal SAM function using a parameter for CodeUri, and output it unchanged', () => {
includeTestTemplate(stack, 'only-minimal-sam-function-codeuri-as-param.yaml');

expect(stack).toMatchTemplate(
loadTestFileToJsObject('only-minimal-sam-function-codeuri-as-param.yaml'),
);
});

test('can ingest a template with only a minimal SAM function using a parameter for CodeUri Bucket property, and output it unchanged', () => {
includeTestTemplate(stack, 'only-minimal-sam-function-codeuri-bucket-as-param.yaml');

expect(stack).toMatchTemplate(
loadTestFileToJsObject('only-minimal-sam-function-codeuri-bucket-as-param.yaml'),
);
});

test('can ingest a template with only a SAM function using an array with DDB CRUD for Policies with an Fn::If expression, and output it unchanged', () => {
includeTestTemplate(stack, 'only-sam-function-policies-array-ddb-crud-if.yaml');

expect(stack).toMatchTemplate(
loadTestFileToJsObject('only-sam-function-policies-array-ddb-crud-if.yaml'),
);
});
});

function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {
return new inc.CfnInclude(scope, 'MyScope', {
templateFile: _testTemplateFilePath(testTemplate),
});
}

function loadTestFileToJsObject(testTemplate: string): any {
return futils.readYamlSync(_testTemplateFilePath(testTemplate));
}

function _testTemplateFilePath(testTemplate: string) {
return path.join(__dirname, 'test-templates', 'sam', testTemplate);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Parameters:
CodeLocation:
Type: String
Resources:
MicroserviceHttpEndpoint:
Type: AWS::Serverless::Function
Properties:
Handler: index.handler
Runtime: nodejs12.x
CodeUri:
Ref: CodeLocation
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Resources:
MicroserviceHttpEndpoint:
Type: AWS::Serverless::Function
Properties:
Handler: index.handler
Runtime: nodejs12.x
CodeUri:
Bucket: awsserverlessrepo-changesets-1f9ifp952i9h0
Key: 123456789012/arn:aws:serverlessrepo:us-east-1:077246666028:applications-microservice-http-endpoint-versions-1.0.4/dc38a8c1-d27f-44f3-b545-4cfff4f8b865
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Parameters:
CodeLocation:
Type: String
Resources:
MicroserviceHttpEndpoint:
Type: AWS::Serverless::Function
Properties:
Handler: index.handler
Runtime: nodejs12.x
CodeUri:
Bucket:
Ref: CodeLocation
Key: my-key
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Conditions:
SomeCondition:
Fn::Equals: [1, 2]
Parameters:
TableNameParameter:
Type: String
Resources:
MicroserviceHttpEndpoint:
Type: AWS::Serverless::Function
Properties:
Handler: index.handler
Runtime: nodejs12.x
CodeUri:
Bucket: awsserverlessrepo-changesets-1f9ifp952i9h0
Key: 828671620168/arn:aws:serverlessrepo:us-east-1:077246666028:applications-microservice-http-endpoint-versions-1.0.4/dc38a8c1-d27f-44f3-b545-4cfff4f8b865
Policies:
- Fn::If:
- SomeCondition
- DynamoDBCrudPolicy:
TableName:
Ref: TableNameParameter
- DynamoDBCrudPolicy:
TableName:
Ref: TableNameParameter
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Parameters:
TableNameParameter:
Type: String
Resources:
MicroserviceHttpEndpoint:
Type: AWS::Serverless::Function
Properties:
Handler: index.handler
Runtime: nodejs12.x
CodeUri: my-code-uri
Policies:
- DynamoDBCrudPolicy:
TableName:
Ref: TableNameParameter
70 changes: 46 additions & 24 deletions packages/@aws-cdk/core/lib/cfn-parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ICfnFinder } from './from-cfn';
import { Lazy } from './lazy';
import { CfnReference } from './private/cfn-reference';
import { IResolvable } from './resolvable';
import { Mapper, Validator } from './runtime';
import { isResolvableObject, Token } from './token';

/**
Expand Down Expand Up @@ -78,36 +79,40 @@ export class FromCloudFormation {
}

// in all other cases, delegate to the standard mapping logic
return this.getArray(value, this.getString);
return this.getArray(this.getString)(value);
}

public static getArray<T>(value: any, mapper: (arg: any) => T): T[] {
if (!Array.isArray(value)) {
// break the type system, and just return the given value,
// which hopefully will be reported as invalid by the validator
// of the property we're transforming
// (unless it's a deploy-time value,
// which we can't map over at build time anyway)
return value;
}
public static getArray<T>(mapper: (arg: any) => T): (x: any) => T[] {
return (value: any) => {
if (!Array.isArray(value)) {
// break the type system, and just return the given value,
// which hopefully will be reported as invalid by the validator
// of the property we're transforming
// (unless it's a deploy-time value,
// which we can't map over at build time anyway)
return value;
}

return value.map(mapper);
return value.map(mapper);
};
}

public static getMap<T>(value: any, mapper: (arg: any) => T): { [key: string]: T } {
if (typeof value !== 'object') {
// if the input is not a map (= object in JS land),
// just return it, and let the validator of this property handle it
// (unless it's a deploy-time value,
// which we can't map over at build time anyway)
return value;
}
public static getMap<T>(mapper: (arg: any) => T): (x: any) => { [key: string]: T } {
return (value: any) => {
if (typeof value !== 'object') {
// if the input is not a map (= object in JS land),
// just return it, and let the validator of this property handle it
// (unless it's a deploy-time value,
// which we can't map over at build time anyway)
return value;
}

const ret: { [key: string]: T } = {};
for (const [key, val] of Object.entries(value)) {
ret[key] = mapper(val);
}
return ret;
const ret: { [key: string]: T } = {};
for (const [key, val] of Object.entries(value)) {
ret[key] = mapper(val);
}
return ret;
};
}

public static getCfnTag(tag: any): CfnTag {
Expand All @@ -118,6 +123,23 @@ export class FromCloudFormation {
value: tag.Value,
};
}

/**
* Return a function that, when applied to a value, will return the first validly deserialized one
*/
public static getTypeUnion(validators: Validator[], mappers: Mapper[]): (x: any) => any {
return (value: any): any => {
for (let i = 0; i < validators.length; i++) {
const candidate = mappers[i](value);
if (validators[i](candidate).isSuccess) {
return candidate;
}
}

// if nothing matches, just return the input unchanged, and let validators catch it
return value;
};
}
}

/**
Expand Down
14 changes: 13 additions & 1 deletion packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -964,20 +964,32 @@ function mergeSection(section: string, val1: any, val2: any): any {
throw new Error(`Conflicting CloudFormation template versions provided: '${val1}' and '${val2}`);
}
return val1 ?? val2;
case 'Transform':
return mergeSets(val1, val2);
case 'Resources':
case 'Conditions':
case 'Parameters':
case 'Outputs':
case 'Mappings':
case 'Metadata':
case 'Transform':
return mergeObjectsWithoutDuplicates(section, val1, val2);
default:
throw new Error(`CDK doesn't know how to merge two instances of the CFN template section '${section}' - ` +
'please remove one of them from your code');
}
}

function mergeSets(val1: any, val2: any): any {
const array1 = val1 == null ? [] : (Array.isArray(val1) ? val1 : [val1]);
const array2 = val2 == null ? [] : (Array.isArray(val2) ? val2 : [val2]);
for (const value of array2) {
if (!array1.includes(value)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally quadratic here. Not sure the set ever grows big enough for it to be an issue, but hope you thought about this and weighed the risk intentionally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I don't think this will ever realistically contain more than 2 elements.

array1.push(value);
}
}
return array1.length === 1 ? array1[0] : array1;
}

function mergeObjectsWithoutDuplicates(section: string, dest: any, src: any): any {
if (typeof dest !== 'object') {
throw new Error(`Expecting ${JSON.stringify(dest)} to be an object`);
Expand Down
Loading