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): correctly parse YAML strings in short-form GetAtt #10197

Merged
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
38 changes: 22 additions & 16 deletions packages/@aws-cdk/cloudformation-include/lib/file-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,29 @@ const shortForms: yaml_types.Schema.CustomTag[] = [
makeTagForCfnIntrinsic('Ref', false),
makeTagForCfnIntrinsic('Condition', false),
makeTagForCfnIntrinsic('GetAtt', true, (_doc: yaml.Document, cstNode: yaml_cst.CST.Node): any => {
// The position of the leftmost period and opening bracket tell us what syntax is being used
// If no brackets are found, then the dot notation is being used; the leftmost dot separates the
// logical ID from the attribute.
//
// If a bracket is found, then the list notation is being used; if present, the leftmost dot separates the
// logical ID from the attribute.
const firstDot = cstNode.toString().indexOf('.');
const firstBracket = cstNode.toString().indexOf('[');
const parsedArguments = parseYamlStrWithCfnTags(cstNode.toString().substring('!GetAtt'.length));

return {
'Fn::GetAtt': firstDot !== -1 && firstBracket === -1
? [
cstNode.toString().substring('!GetAtt '.length, firstDot),
parseYamlStrWithCfnTags((cstNode.toString().substring(firstDot + 1))),
]
: 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 };
}),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,19 @@ describe('CDK Include', () => {
}).toThrow(/Element referenced in Fn::Sub expression with logical ID: '' was not found in the template/);
});

test('throws an error when a template supplies an invalid string to a number parameter', () => {
test("throws an exception for a template with a non-number string passed to a property with type 'number'", () => {
includeTestTemplate(stack, 'alphabetical-string-passed-to-number.json');

expect(() => {
SynthUtils.synthesize(stack);
}).toThrow(/"abc" should be a number/);
});

test('throws an exception for a template with a short-form Fn::GetAtt whose string argument does not contain a dot', () => {
expect(() => {
includeTestTemplate(stack, 'short-form-get-att-no-dot.yaml');
}).toThrow(/Short-form Fn::GetAtt must contain a '.' in its string argument, got: 'Bucket1Arn'/);
});
});

function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Resources:
Bucket1:
Type: AWS::S3::Bucket
Bucket2:
Type: AWS::S3::Bucket
Metadata:
Bucket1Name: !GetAtt Bucket1Arn
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ Resources:
Type: AWS::S3::Bucket
Properties:
BucketName: !GetAtt Bucket0.Arn
AccessControl: !GetAtt [ ELB, SourceSecurityGroup.GroupName ]
AccessControl: !GetAtt [ELB, SourceSecurityGroup.GroupName]
Bucket2:
Type: AWS::S3::Bucket
Properties:
BucketName: !GetAtt [ Bucket1, Arn ]
AccessControl: !GetAtt ELB.SourceSecurityGroup.GroupName
AccessControl: !GetAtt 'ELB.SourceSecurityGroup.GroupName'
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ describe('CDK Include', () => {
});
});

// Note that this yaml template fails validation. It is unclear how to invoke !Transform.
test('can ingest a template with the short form !Transform function', () => {
// Note that this yaml template fails validation. It is unclear how to invoke !Transform.
includeTestTemplate(stack, 'invalid/short-form-transform.yaml');

expect(stack).toMatchTemplate({
Expand Down