diff --git a/CHANGELOG.md b/CHANGELOG.md index e04d3a7..a18439b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Versioning](http://semver.org/spec/v2.0.0.html). ### Fixed - Merge PR #135, fixing parts.join exception when !Join was called on non-array - Merge PR #141, allowing lists of values to be specified on the command line +- Merge PR #139, allowing Fn::GetAtt to return an array of values ### Changed - Merge PR #136, updated usage prompt for a better CLI experience diff --git a/src/awsData.ts b/src/awsData.ts index 5bbed04..80d6d82 100644 --- a/src/awsData.ts +++ b/src/awsData.ts @@ -74,7 +74,7 @@ export interface ResourceType { } export interface PrimitiveAttribute { - ItemType: AWSPrimitiveType + PrimitiveType: AWSPrimitiveType } export interface ListAttribute { diff --git a/src/resourcesSpec.ts b/src/resourcesSpec.ts index ae5742f..3ddc399 100644 --- a/src/resourcesSpec.ts +++ b/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) { diff --git a/src/test/resourcesTest.ts b/src/test/resourcesTest.ts index 53aac13..bb5d408 100644 --- a/src/test/resourcesTest.ts +++ b/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); + }); + + + }); + }); diff --git a/src/test/validatorTest.ts b/src/test/validatorTest.ts index e83e4de..da5acf0 100644 --- a/src/test/validatorTest.ts +++ b/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); }); diff --git a/src/validator.ts b/src/validator.ts index d032a44..0ebefc8 100644 --- a/src/validator.ts +++ b/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; diff --git a/testData/valid/yaml/issue-134-invalid-fngetatt-att-does-not-exist.yaml b/testData/valid/yaml/issue-134-invalid-fngetatt-att-does-not-exist.yaml new file mode 100644 index 0000000..69612a6 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/testData/valid/yaml/issue-134-invalid-fngetatt-returns-array.yaml b/testData/valid/yaml/issue-134-invalid-fngetatt-returns-array.yaml new file mode 100644 index 0000000..3145c8a --- /dev/null +++ b/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 \ No newline at end of file diff --git a/testData/valid/yaml/issue-134-valid-fngetatt-returns-array.yaml b/testData/valid/yaml/issue-134-valid-fngetatt-returns-array.yaml new file mode 100644 index 0000000..8042864 --- /dev/null +++ b/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 \ No newline at end of file