fix(cfn-include): correctly parse YAML strings in short-form GetAtt (#10197)

We weren't recursively parsing the argument of the short-form `Fn::GetAtt`
if the arguments to it where given in the string form;
which meant, if they were quoted (which is legal in YAML),
we would add the quote to the logical ID of the resource,
which is obviously incorrect.

Fixes #10177

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: skinny85

packages/@aws-cdk/cloudformation-include/lib/file-utils.ts

@@ -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 };
   }),
 );
 

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

@@ -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 {

packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/short-form-get-att-no-dot.yaml

@@ -0,0 +1,7 @@
+Resources:
+  Bucket1:
+    Type: AWS::S3::Bucket
+  Bucket2:
+    Type: AWS::S3::Bucket
+    Metadata:
+      Bucket1Name: !GetAtt Bucket1Arn

packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-get-att.yaml

@@ -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'

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

@@ -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({