Skip to content

Commit

Permalink
fix(diff): handle YAML short-forms like '!GetAtt' in diff (aws#10381)
Browse files Browse the repository at this point in the history
CloudFormation allows using short-form versions of intrinsic functions like `!GetAtt`.
We handled them correctly in the `@aws-cdk/cloudformation-include` module,
so extract that logic to a common package,
and use it from the CLI in the `diff` command as well.

Fixes aws#6537

----

*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 Sep 21, 2020
1 parent 7b409ae commit 457e109
Show file tree
Hide file tree
Showing 22 changed files with 478 additions and 104 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@
"@aws-cdk/cloud-assembly-schema/jsonschema/**",
"@aws-cdk/cloud-assembly-schema/semver",
"@aws-cdk/cloud-assembly-schema/semver/**",
"@aws-cdk/cloudformation-include/yaml",
"@aws-cdk/cloudformation-include/yaml/**",
"@aws-cdk/core/fs-extra",
"@aws-cdk/core/fs-extra/**",
"@aws-cdk/core/minimatch",
"@aws-cdk/core/minimatch/**",
"@aws-cdk/cx-api/semver",
"@aws-cdk/cx-api/semver/**",
"@aws-cdk/yaml-cfn/yaml",
"@aws-cdk/yaml-cfn/yaml/**",
"aws-cdk-lib/case",
"aws-cdk-lib/case/**",
"aws-cdk-lib/fs-extra",
Expand Down
63 changes: 2 additions & 61 deletions packages/@aws-cdk/cloudformation-include/lib/file-utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import * as fs from 'fs';
import * as yaml from 'yaml';
import * as yaml_cst from 'yaml/parse-cst';
import * as yaml_types from 'yaml/types';
import * as yaml_cfn from '@aws-cdk/yaml-cfn';

export function readJsonSync(filePath: string): any {
const fileContents = fs.readFileSync(filePath);
Expand All @@ -10,62 +8,5 @@ export function readJsonSync(filePath: string): any {

export function readYamlSync(filePath: string): any {
const fileContents = fs.readFileSync(filePath);
return parseYamlStrWithCfnTags(fileContents.toString());
}

function makeTagForCfnIntrinsic(
intrinsicName: string, addFnPrefix: boolean = true,
resolveFun?: (_doc: yaml.Document, cstNode: yaml_cst.CST.Node) => any): yaml_types.Schema.CustomTag {

return {
identify(value: any) { return typeof value === 'string'; },
tag: `!${intrinsicName}`,
resolve: resolveFun || ((_doc: yaml.Document, cstNode: yaml_cst.CST.Node) => {
const ret: any = {};
ret[addFnPrefix ? `Fn::${intrinsicName}` : intrinsicName] =
// the +1 is to account for the ! the short form begins with
parseYamlStrWithCfnTags(cstNode.toString().substring(intrinsicName.length + 1));
return ret;
}),
};
}

const shortForms: yaml_types.Schema.CustomTag[] = [
'Base64', 'Cidr', 'FindInMap', 'GetAZs', 'ImportValue', 'Join', 'Sub',
'Select', 'Split', 'Transform', 'And', 'Equals', 'If', 'Not', 'Or',
].map(name => makeTagForCfnIntrinsic(name)).concat(
makeTagForCfnIntrinsic('Ref', false),
makeTagForCfnIntrinsic('Condition', false),
makeTagForCfnIntrinsic('GetAtt', true, (_doc: yaml.Document, cstNode: yaml_cst.CST.Node): any => {
const parsedArguments = parseYamlStrWithCfnTags(cstNode.toString().substring('!GetAtt'.length));

let value: any;
if (typeof parsedArguments === 'string') {
// if the arguments to !GetAtt are a string,
// the part before the first '.' is the logical ID,
// and the rest is the attribute name
// (which can contain '.')
const firstDot = parsedArguments.indexOf('.');
if (firstDot === -1) {
throw new Error(`Short-form Fn::GetAtt must contain a '.' in its string argument, got: '${parsedArguments}'`);
}
value = [
parsedArguments.substring(0, firstDot),
parsedArguments.substring(firstDot + 1), // the + 1 is to skip the actual '.'
];
} else {
// this is the form where the arguments to Fn::GetAtt are already an array -
// in this case, nothing more to do
value = parsedArguments;
}

return { 'Fn::GetAtt': value };
}),
);

function parseYamlStrWithCfnTags(text: string): any {
return yaml.parse(text, {
customTags: shortForms,
schema: 'yaml-1.1',
});
return yaml_cfn.deserialize(fileContents.toString());
}
21 changes: 9 additions & 12 deletions packages/@aws-cdk/cloudformation-include/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"@aws-cdk/aws-apigatewayv2": "0.0.0",
"@aws-cdk/aws-appconfig": "0.0.0",
"@aws-cdk/aws-applicationautoscaling": "0.0.0",
"@aws-cdk/aws-applicationinsights": "0.0.0",
"@aws-cdk/aws-appmesh": "0.0.0",
"@aws-cdk/aws-appstream": "0.0.0",
"@aws-cdk/aws-appsync": "0.0.0",
Expand All @@ -89,6 +90,7 @@
"@aws-cdk/aws-codecommit": "0.0.0",
"@aws-cdk/aws-codedeploy": "0.0.0",
"@aws-cdk/aws-codeguruprofiler": "0.0.0",
"@aws-cdk/aws-codegurureviewer": "0.0.0",
"@aws-cdk/aws-codepipeline": "0.0.0",
"@aws-cdk/aws-codestar": "0.0.0",
"@aws-cdk/aws-codestarconnections": "0.0.0",
Expand Down Expand Up @@ -180,9 +182,7 @@
"@aws-cdk/aws-wafv2": "0.0.0",
"@aws-cdk/aws-workspaces": "0.0.0",
"@aws-cdk/core": "0.0.0",
"yaml": "1.10.0",
"@aws-cdk/aws-applicationinsights": "0.0.0",
"@aws-cdk/aws-codegurureviewer": "0.0.0"
"@aws-cdk/yaml-cfn": "0.0.0"
},
"peerDependencies": {
"@aws-cdk/alexa-ask": "0.0.0",
Expand All @@ -194,6 +194,7 @@
"@aws-cdk/aws-apigatewayv2": "0.0.0",
"@aws-cdk/aws-appconfig": "0.0.0",
"@aws-cdk/aws-applicationautoscaling": "0.0.0",
"@aws-cdk/aws-applicationinsights": "0.0.0",
"@aws-cdk/aws-appmesh": "0.0.0",
"@aws-cdk/aws-appstream": "0.0.0",
"@aws-cdk/aws-appsync": "0.0.0",
Expand All @@ -215,6 +216,7 @@
"@aws-cdk/aws-codecommit": "0.0.0",
"@aws-cdk/aws-codedeploy": "0.0.0",
"@aws-cdk/aws-codeguruprofiler": "0.0.0",
"@aws-cdk/aws-codegurureviewer": "0.0.0",
"@aws-cdk/aws-codepipeline": "0.0.0",
"@aws-cdk/aws-codestar": "0.0.0",
"@aws-cdk/aws-codestarconnections": "0.0.0",
Expand Down Expand Up @@ -257,6 +259,7 @@
"@aws-cdk/aws-iotanalytics": "0.0.0",
"@aws-cdk/aws-iotevents": "0.0.0",
"@aws-cdk/aws-iotthingsgraph": "0.0.0",
"@aws-cdk/aws-kendra": "0.0.0",
"@aws-cdk/aws-kinesis": "0.0.0",
"@aws-cdk/aws-kinesisanalytics": "0.0.0",
"@aws-cdk/aws-kinesisfirehose": "0.0.0",
Expand Down Expand Up @@ -296,6 +299,7 @@
"@aws-cdk/aws-sns": "0.0.0",
"@aws-cdk/aws-sqs": "0.0.0",
"@aws-cdk/aws-ssm": "0.0.0",
"@aws-cdk/aws-sso": "0.0.0",
"@aws-cdk/aws-stepfunctions": "0.0.0",
"@aws-cdk/aws-synthetics": "0.0.0",
"@aws-cdk/aws-transfer": "0.0.0",
Expand All @@ -304,25 +308,18 @@
"@aws-cdk/aws-wafv2": "0.0.0",
"@aws-cdk/aws-workspaces": "0.0.0",
"@aws-cdk/core": "0.0.0",
"constructs": "^3.0.4",
"@aws-cdk/aws-applicationinsights": "0.0.0",
"@aws-cdk/aws-codegurureviewer": "0.0.0",
"@aws-cdk/aws-kendra": "0.0.0",
"@aws-cdk/aws-sso": "0.0.0"
"@aws-cdk/yaml-cfn": "0.0.0",
"constructs": "^3.0.4"
},
"devDependencies": {
"@aws-cdk/assert": "0.0.0",
"@types/jest": "^26.0.14",
"@types/yaml": "1.9.6",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"jest": "^26.4.2",
"pkglint": "0.0.0",
"ts-jest": "^26.3.0"
},
"bundledDependencies": [
"yaml"
],
"keywords": [
"aws",
"cdk",
Expand Down
3 changes: 3 additions & 0 deletions packages/@aws-cdk/yaml-cfn/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const baseConfig = require('cdk-build-tools/config/eslintrc');
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
module.exports = baseConfig;
18 changes: 18 additions & 0 deletions packages/@aws-cdk/yaml-cfn/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
*.js
*.js.map
*.d.ts
node_modules
dist
tsconfig.json
.jsii

.LAST_BUILD
.LAST_PACKAGE
*.snk
.nyc_output
coverage
nyc.config.js
!.eslintrc.js
!jest.config.js

junit.xml
26 changes: 26 additions & 0 deletions packages/@aws-cdk/yaml-cfn/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Don't include original .ts files when doing `npm pack`
*.ts
!*.d.ts
coverage
.nyc_output
*.tgz

dist
.LAST_PACKAGE
.LAST_BUILD
!*.js

# Include .jsii
!.jsii

*.snk

*.tsbuildinfo

tsconfig.json
.eslintrc.js
jest.config.js

# exclude cdk artifacts
**/cdk.out
junit.xml
Loading

0 comments on commit 457e109

Please sign in to comment.