Issue #134 - FN::GetAtt can return an array

src/awsData.ts

@@ -74,7 +74,7 @@ export interface ResourceType {
 }
 
 export interface PrimitiveAttribute {
-    ItemType: AWSPrimitiveType
+    PrimitiveType: AWSPrimitiveType
 }
 
 export interface ListAttribute {

src/resourcesSpec.ts

@@ -4,7 +4,8 @@ import {
     ResourceType,
     ResourcePropertyType,
     Property,
-    AWSPrimitiveType
+    AWSPrimitiveType,
+    Attribute
 } from './awsData';
 
 import CustomError = require('./util/CustomError');
@@ -37,6 +38,19 @@ export class NoSuchPropertyType extends CustomError {
     }
 }
 
+export class NoSuchResourceTypeAttribute extends CustomError {
+    resourceType: string;
+    attributeName: string;
+    constructor(type: string, attributeName: string) {
+        super(`No such attribute ${attributeName} on ${type}`);
+
+        CustomError.fixErrorInheritance(this, NoSuchResourceTypeAttribute)
+
+        this.resourceType = type;
+        this.attributeName = attributeName;
+    }
+}
+
 export function getResourceType(type: string){
     // If the type starts with Custom::, it's a custom resource.
     if(type.indexOf('Custom::') === 0){
@@ -50,6 +64,18 @@ export function getResourceType(type: string){
     return resourceType;
 }
 
+export function getResourceTypeAttribute(type: string, attributeName: string): Attribute {
+    const resourceAttributes = getResourceType(type).Attributes
+    if (!resourceAttributes) {
+        throw new NoSuchResourceTypeAttribute(type, attributeName);
+    }
+    const resourceAttribute = resourceAttributes[attributeName]
+    if (!resourceAttribute) {
+        throw new NoSuchResourceTypeAttribute(type, attributeName);
+    }
+    return resourceAttribute
+}
+
 function getPropertyType(type: string){
     const propertyType =  specification.PropertyTypes[type];
     if (!propertyType) {

src/test/resourcesTest.ts

@@ -2,6 +2,10 @@ import chai = require('chai');
 const expect = chai.expect;
 const assert = chai.assert;
 import resourceSpec = require('../resourcesSpec');
+import {
+    PrimitiveAttribute,
+    ListAttribute
+} from "../awsData"
 
 describe('resourceSpec', () =>{
 
@@ -124,4 +128,34 @@ describe('resourceSpec', () =>{
 
     });
 
+    describe('getResourceTypeAttribute', () => {
+
+        it('should return "String" for AWS::ECS::Service attribute Name', () => {
+            let result = resourceSpec.getResourceTypeAttribute("AWS::ECS::Service","Name");
+            let res = result as PrimitiveAttribute;
+            expect(res.PrimitiveType).to.equal("String");
+        });
+
+        it('should return "List of String" for AWS::Route53::HostedZone attribute NameServers', () => {
+            let result = resourceSpec.getResourceTypeAttribute("AWS::Route53::HostedZone","NameServers");
+            let res = result as ListAttribute;
+            expect(res.Type).to.equal("List");
+            expect(res.PrimitiveItemType).to.equal("String");
+        });
+
+        it('should throw NoSuchResourceTypeAttribute for any attrbute on a type with no attributes', () => {
+            expect(
+                () => resourceSpec.getResourceTypeAttribute("AWS::CloudFormation::WaitConditionHandle", "Anything")
+            ).to.throw(resourceSpec.NoSuchResourceTypeAttribute);
+        });
+
+        it('should throw NoSuchResourceTypeAttribute for an attrbute that does not exist on a type', () => {
+            expect(
+                () => resourceSpec.getResourceTypeAttribute("AWS::ECS::Service", "AttributeThatDoesNotExist")
+            ).to.throw(resourceSpec.NoSuchResourceTypeAttribute);
+        });
+
+
+    });
+
 });

src/test/validatorTest.ts

@@ -452,6 +452,34 @@ describe('validator', () => {
             expect(validator.fnGetAtt('Custom', 'SomeAttribute')).to.equal('mockAttr_Custom_SomeAttribute');
             expect(validator.fnGetAtt('Custom2', 'SomeAttribute')).to.equal('mockAttr_Custom2_SomeAttribute');
         })
+
+        it("should pass validation where !GetAtt returns a list", () => {
+          const input = 'testData/valid/yaml/issue-134-valid-fngetatt-returns-array.yaml';
+          let result = validator.validateFile(input);
+          expect(result).to.have.deep.property('templateValid', true);
+          expect(result['errors']['crit']).to.have.lengthOf(0);
+          expect(result['errors']['warn']).to.have.lengthOf(0);
+        });
+
+        it("should not pass validation where !GetAtt returns a list", () => {
+          const input = 'testData/valid/yaml/issue-134-invalid-fngetatt-returns-array.yaml';
+          let result = validator.validateFile(input);
+          expect(result).to.have.deep.property('templateValid', false);
+          expect(result['errors']['crit']).to.have.lengthOf(1);
+          expect(result['errors']['warn']).to.have.lengthOf(0);
+        });
+
+        it("should not pass validation with !GetAtt where attribute does not exist", () => {
+          const input = 'testData/valid/yaml/issue-134-invalid-fngetatt-att-does-not-exist.yaml';
+          let result = validator.validateFile(input);
+          expect(result).to.have.deep.property('templateValid', false);
+          expect(result['errors']['crit']).to.have.lengthOf(2);
+          expect(result['errors']['crit'][0]).to.have.property('message', 'No such attribute VeryLostNameServers on AWS::Route53::HostedZone');
+          expect(result['errors']['crit'][0]).to.have.property('resource', 'Resources > DNSVPCDelegation > Properties > ResourceRecords');
+          expect(result['errors']['crit'][0]).to.have.property('documentation', 'http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-route53-hostedzone.html');
+          expect(result['errors']['crit'][1]).to.have.property('message', "Expecting a list, got 'INVALID_REFERENCE_OR_ATTR_ON_GET_ATT'");
+          expect(result['errors']['warn']).to.have.lengthOf(0);
+        });
     })
 
     describe('conditions', () => {
@@ -647,7 +675,6 @@ describe('validator', () => {
         it('a valid template with APIG string results in validTemplate = true, 0 crit error', () => {
             const input = 'testData/valid/yaml/issue-81-api-gateway.yaml';
             let result = validator.validateFile(input);
-            console.log(result['errors']['crit']);
             expect(result).to.have.deep.property('templateValid', true);
             expect(result['errors']['crit']).to.have.lengthOf(0);
         });

src/validator.ts

@@ -7,7 +7,9 @@ const mockArnPrefix = "arn:aws:mock:region:123456789012:";
 import {
     awsParameterTypes as parameterTypesSpec,
     awsRefOverrides,
-    awsIntrinsicFunctions
+    awsIntrinsicFunctions,
+    PrimitiveAttribute,
+    ListAttribute
 } from './awsData';
 import docs = require('./docs');
 
@@ -1076,14 +1078,37 @@ function fnJoin(join: any, parts: any){
     return parts.join(join);
 }
 
-export function fnGetAtt(reference: string, attribute: string){
+export function fnGetAtt(reference: string, attributeName: string){
     if(workingInput['Resources'].hasOwnProperty(reference)){
         const resource = workingInput['Resources'][reference];
-        if (resource['Attributes'].hasOwnProperty(attribute)){
-            return resource['Attributes'][attribute];
-        } else if (resource['Type'].indexOf('Custom::') === 0 || resource['Type'] === 'AWS::CloudFormation::CustomResource') {
-            return `mockAttr_${reference}_${attribute}`;
-        } 
+        if (resource['Type'].indexOf('Custom::') === 0) {
+            return `mockAttr_${reference}_${attributeName}`;
+        } else if (resource['Type'] === 'AWS::CloudFormation::CustomResource') {
+            return `mockAttr_${reference}_${attributeName}`;
+        } else {
+            try {
+                // Lookup attribute
+                const attribute = resourcesSpec.getResourceTypeAttribute(resource['Type'], attributeName)
+                const primitiveAttribute = attribute as PrimitiveAttribute
+                if(!!primitiveAttribute['PrimitiveType']) {
+                    return resource['Attributes'][attributeName];
+                }
+                const listAttribute = attribute as ListAttribute
+                if(listAttribute['Type'] == 'List') {
+                    return [ resource['Attributes'][attributeName], resource['Attributes'][attributeName] ]
+                }
+            } catch (e) {
+                if (e instanceof resourcesSpec.NoSuchResourceTypeAttribute) {
+                    addError('crit',
+                        e.message,
+                        placeInTemplate,
+                        resource['Type']
+                    );
+                } else {
+                    throw e;
+                }
+            }
+        }
     }
     // Return null if not found
     return null;

testData/valid/yaml/issue-134-invalid-fngetatt-att-does-not-exist.yaml

@@ -0,0 +1,23 @@
+AWSTemplateFormatVersion: "2010-09-09"
+Description: "AWS CloudFormation Template to demonstrate Fn::GetAtt returning an array"
+Resources: 
+  DNSVPCDomain: 
+    Type: "AWS::Route53::HostedZone"
+    Properties: 
+      Name: "staging.DOMAIN.COM"
+      HostedZoneConfig: 
+        Comment: "ENVIRONMENT staging Public DNS"
+      HostedZoneTags: 
+      - Key: Name
+        Value: "ENVIRONMENT-staging-hosted-zone"
+  DNSVPCDelegation: 
+    Type: "AWS::Route53::RecordSet"
+    DependsOn: 
+    - DNSVPCDomain
+    Properties: 
+      Name: "staging.DOMAIN.COM"
+      Comment: "ENVIRONMENT staging Subdomain Delegation"
+      HostedZoneId: "Z3**************"
+      ResourceRecords: !GetAtt DNSVPCDomain.VeryLostNameServers
+      TTL: 300
+      Type: NS

testData/valid/yaml/issue-134-invalid-fngetatt-returns-array.yaml

@@ -0,0 +1,25 @@
+AWSTemplateFormatVersion: "2010-09-09"
+Description: "AWS CloudFormation Template to demonstrate Fn::GetAtt returning an array"
+Resources: 
+  DNSVPCDomain: 
+    Type: "AWS::Route53::HostedZone"
+    Properties: 
+      Name: "staging.DOMAIN.COM"
+      HostedZoneConfig: 
+        Comment: "ENVIRONMENT staging Public DNS"
+      HostedZoneTags: 
+      - Key: Name
+        Value: "ENVIRONMENT-staging-hosted-zone"
+  DNSVPCDelegation: 
+    Type: "AWS::Route53::RecordSet"
+    DependsOn: 
+    - DNSVPCDomain
+    Properties: 
+      Name: !GetAtt DNSVPCDomain.NameServers
+      Comment: "ENVIRONMENT staging Subdomain Delegation"
+      HostedZoneId: "Z3**************"
+      ResourceRecords: 
+      - "ns1.domain.com"
+      - "ns2.domain.com"
+      TTL: 300
+      Type: NS

testData/valid/yaml/issue-134-valid-fngetatt-returns-array.yaml

@@ -0,0 +1,23 @@
+AWSTemplateFormatVersion: "2010-09-09"
+Description: "AWS CloudFormation Template to demonstrate Fn::GetAtt returning an array"
+Resources: 
+  DNSVPCDomain: 
+    Type: "AWS::Route53::HostedZone"
+    Properties: 
+      Name: "staging.DOMAIN.COM"
+      HostedZoneConfig: 
+        Comment: "ENVIRONMENT staging Public DNS"
+      HostedZoneTags: 
+      - Key: Name
+        Value: "ENVIRONMENT-staging-hosted-zone"
+  DNSVPCDelegation: 
+    Type: "AWS::Route53::RecordSet"
+    DependsOn: 
+    - DNSVPCDomain
+    Properties: 
+      Name: "staging.DOMAIN.COM"
+      Comment: "ENVIRONMENT staging Subdomain Delegation"
+      HostedZoneId: "Z3**************"
+      ResourceRecords: !GetAtt DNSVPCDomain.NameServers
+      TTL: 300
+      Type: NS