Fix nested resources within an if condition not being resolved and AW…

…S::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

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

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', () => {

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();
     }
 }

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"]