From 42b75e17913d65ab9fe735fb5435ce7048deea88 Mon Sep 17 00:00:00 2001 From: Marty Sweet Date: Wed, 18 Jul 2018 06:35:39 +0100 Subject: [PATCH] Fix nested resources within an if condition not being resolved and AWS::NoValue (#177) * Add support for recursive resolving, which allows for Fn::If parameters to be resolved for whole blocks which depend on a Condition, see #106 * Update CHANGELOG.md --- CHANGELOG.md | 1 + src/test/validatorTest.ts | 7 + src/validator.ts | 19 +- .../valid/yaml/issue_106_conditional_if.yaml | 231 ++++++++++++++++++ 4 files changed, 256 insertions(+), 2 deletions(-) create mode 100644 testData/valid/yaml/issue_106_conditional_if.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 8feef5b..b85fe8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +- Merge PR #177, fix nested resources within an if condition not being resolved. Fix resolution of AWS::NoValue. - Merge PR #179, fixing support for AWS::Partition pseudo parameter ## [1.7.3] - 2018-06-30 diff --git a/src/test/validatorTest.ts b/src/test/validatorTest.ts index aaba71f..3b2c310 100644 --- a/src/test/validatorTest.ts +++ b/src/test/validatorTest.ts @@ -534,6 +534,13 @@ describe('validator', () => { expect(result['errors']['crit'][0]['message']).to.contain('Condition should consist of an intrinsic function of type'); }); + it('1 valid condition value type should return an object with validTemplate = true, 0 crit errors', () => { + const input = './testData/valid/yaml/issue_106_conditional_if.yaml'; + let result = validator.validateFile(input); + expect(result).to.have.deep.property('templateValid', true); + expect(result['errors']['crit']).to.have.lengthOf(0); + }); + }); describe('Fn::Split', () => { diff --git a/src/validator.ts b/src/validator.ts index 3978cee..5b89f8c 100644 --- a/src/validator.ts +++ b/src/validator.ts @@ -568,6 +568,12 @@ let lastPositionInTemplate: any = null; let lastPositionInTemplateKey: string | null = null; function recursiveDecent(ref: any){ + + // Save the current variables to allow for nested decents + let placeInTemplateX = placeInTemplate; + let lastPositionInTemplateX = lastPositionInTemplate; + let lastPositionInTemplateKeyX = lastPositionInTemplateKey; + // Step into next attribute for(let i=0; i < Object.keys(ref).length; i++){ let key = Object.keys(ref)[i]; @@ -602,6 +608,11 @@ function recursiveDecent(ref: any){ } placeInTemplate.pop(); + + // Restore the variables to support nested resolving + placeInTemplate = placeInTemplateX; + lastPositionInTemplate = lastPositionInTemplateX; + lastPositionInTemplateKey = lastPositionInTemplateKeyX; } function resolveCondition(ref: any, key: string){ @@ -1018,7 +1029,9 @@ function doIntrinsicIf(ref: any, key: string){ if (awsIntrinsicFunctions['Fn::If']['supportedFunctions'].indexOf(keys[0]) !== -1) { return resolveIntrinsicFunction(value, keys[0]); }else{ - addError('crit', `Fn::If does not allow ${keys[0]} as a nested function`, placeInTemplate, 'Fn::If'); + // TODO: Do we need to make sure this isn't an attempt at an invalid supportedFunction? + recursiveDecent(value); + return value } }else { return value; @@ -1783,7 +1796,9 @@ function checkList(objectType: NamedProperty, listToCheck: any[]) { const itemType = getItemType(objectType); for (const [index, item] of listToCheck.entries()) { placeInTemplate.push(index); - check(itemType, item); + if(!util.isUndefined(item)){ + check(itemType, item); + } placeInTemplate.pop(); } } diff --git a/testData/valid/yaml/issue_106_conditional_if.yaml b/testData/valid/yaml/issue_106_conditional_if.yaml new file mode 100644 index 0000000..6bc1dcd --- /dev/null +++ b/testData/valid/yaml/issue_106_conditional_if.yaml @@ -0,0 +1,231 @@ +--- +AWSTemplateFormatVersion: '2010-09-09' +Description: This is a test template. DO NOT USE! +Parameters: + AmiId: + Description: ID of the AMI to launch + Type: String + AllowedPattern: "^ami-[0-9a-z]{8}$|^ami-[0-9a-z]{17}$" + Default: ami-1c8ee466 + AmiDistro: + Description: Linux distro of the AMI + Type: String + Default: CentOS + AllowedValues: + - AmazonLinux + - CentOS + - RedHat + AppVolumeDevice: + Description: Decision on whether to mount an extra EBS volume. Leave as default + ("false") to launch without an extra application volume + Type: String + Default: 'false' + AllowedValues: + - 'true' + - 'false' + AppVolumeType: + Description: Type of EBS volume to create. Ignored if "AppVolumeDevice" is false + Type: String + Default: gp2 + AllowedValues: + - gp2 + - io1 + - sc1 + - st1 + - standard + AppVolumeSize: + Description: Size in GB of the EBS volume to create. Ignored if "AppVolumeDevice" + is false + Type: Number + Default: '1' + MinValue: '1' + MaxValue: '16384' + ConstraintDescription: Must be between 1GB and 16384GB. + InstanceType: + Description: Amazon EC2 instance type + Type: String + Default: t2.micro + AllowedValues: + - t2.micro + - t2.small + - t2.medium + - t2.large + - t2.xlarge + - c4.large + - c4.xlarge + - m4.large + - m4.xlarge + - c5.large + - c5.xlarge + - m5.large + - m5.xlarge +Conditions: + CreateAppVolume: + Fn::Equals: + - Ref: AppVolumeDevice + - 'true' + CreateAppVolume2: + Fn::Equals: + - Ref: AppVolumeDevice + - 'false' +Mappings: + Distro2RootDevice: + AmazonLinux: + DeviceName: xvda + RedHat: + DeviceName: sda1 + CentOS: + DeviceName: sda1 + InstanceTypeCapabilities: + t2.micro: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + t2.small: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + t2.medium: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + t2.large: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + t2.xlarge: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + c4.large: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + c4.xlarge: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + m4.large: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + m4.xlarge: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/xvdf" + c5.large: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/nvme1n1" + c5.xlarge: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/nvme1n1" + m5.large: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/nvme1n1" + m5.xlarge: + ExternDeviceName: "/dev/xvdf" + InternDeviceName: "/dev/nvme1n1" +Resources: + WatchmakerInstance: + Type: AWS::EC2::Instance + CreationPolicy: + ResourceSignal: + Count: '1' + Timeout: PT30M + Properties: + ImageId: + Ref: AmiId + InstanceType: + Ref: InstanceType + BlockDeviceMappings: + - DeviceName: + Fn::Join: + - '' + - - "/dev/" + - Fn::FindInMap: + - Distro2RootDevice + - Ref: AmiDistro + - DeviceName + Ebs: + VolumeType: gp2 + DeleteOnTermination: 'true' + - Fn::If: + - CreateAppVolume + - DeviceName: + Fn::FindInMap: + - InstanceTypeCapabilities + - Ref: InstanceType + - ExternDeviceName + Ebs: + VolumeSize: + Ref: AppVolumeSize + VolumeType: + Ref: AppVolumeType + DeleteOnTermination: 'true' + - Ref: AWS::NoValue + UserData: + Ref: AWS::NoValue + WatchmakerInstanceWithVolume: + Type: AWS::EC2::Instance + CreationPolicy: + ResourceSignal: + Count: '1' + Timeout: PT30M + Properties: + ImageId: + Ref: AmiId + InstanceType: + Ref: InstanceType + BlockDeviceMappings: + - DeviceName: + Fn::Join: + - '' + - - "/dev/" + - Fn::FindInMap: + - Distro2RootDevice + - Ref: AmiDistro + - DeviceName + Ebs: + VolumeType: gp2 + DeleteOnTermination: 'true' + - Fn::If: + - CreateAppVolume2 + - DeviceName: + Fn::FindInMap: + - InstanceTypeCapabilities + - Ref: InstanceType + - ExternDeviceName + Ebs: + VolumeSize: + Ref: AppVolumeSize + VolumeType: + Ref: AppVolumeType + DeleteOnTermination: 'true' + - Ref: AWS::NoValue + UserData: + Ref: AWS::NoValue +Outputs: + WatchmakerInstanceId: + Value: + Ref: WatchmakerInstance + Description: Instance ID + + +# +#Parameters: +# HTTPSListener: +# Description: HTTPS Listener +# Default: False +# Type: String +# AllowedValues: +# - True +# - False +# +#Conditions: +# ArnCondition: !Equals [ True, True ] +# CreateHTTPSListener: !Equals [ !Ref HTTPSListener, False ] +# +#Resources: +# ListenerHTTPS: +# Type: AWS::ElasticLoadBalancingV2::Listener +# Condition: CreateHTTPSListener +# Properties: +# DefaultActions: +# - Type: forward +# TargetGroupArn: "arn:aws:abc" +# LoadBalancerArn: "arn:aws:load-balancer" +# Port: '443' +# Protocol: HTTPS +# Certificates: +# - CertificateArn: !If [ArnCondition, "arn:aws:cert:a", "arn:aws:ssl:a"] \ No newline at end of file