Fix Sub not throwing errors, raised in issue #39

src/validator.es6

@@ -598,7 +598,7 @@ function doIntrinsicSub(ref, key){
     }
 
     // Extract the replacement parts
-    let regex = /\${([A-Za-z:.!]+)/gm
+    let regex = /\${([A-Za-z0-9:.!]+)/gm;
     let matches = [];
     let match;
     while (match = regex.exec(replacementStr)) {
@@ -621,14 +621,23 @@ function doIntrinsicSub(ref, key){
                 // Use Fn::GetAtt
                 let parts = m.split('.');
                 replacementVal = fnGetAtt(parts[0], parts[1]);
+                if(replacementVal === null){
+                    addError('crit', `Intrinsic Sub does not reference valid resource attribute '${m}'`, placeInTemplate, 'Fn::Sub');
+                }
             }
         }else{
-            if(definedParams !== null && definedParams.hasOwnProperty(m) && typeof definedParams[m] !== 'string'){
-                definedParams[m] = resolveIntrinsicFunction(definedParams[m], Object.keys(m)[0]);
-                replacementVal = definedParams[m];
+            if(definedParams !== null && definedParams.hasOwnProperty(m)){
+                if(typeof definedParams[m] !== 'string') {
+                    replacementVal = resolveIntrinsicFunction(definedParams[m], Object.keys(m)[0]);
+                }else{
+                    replacementVal = definedParams[m];
+                }
             }else {
                 // Use Ref
                 replacementVal = getRef(m);
+                if(replacementVal === null){
+                    addError('crit', `Intrinsic Sub does not reference valid resource or mapping '${m}'`, placeInTemplate, 'Fn::Sub');
+                }
             }
         }
 

test/data/invalid/yaml/invalid_sub_getatt.yaml

@@ -0,0 +1,8 @@
+Resources:
+  MyMisspeltBucket:
+    Type: AWS::S3::Bucket
+
+  MyLogBucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: !Sub "${MyBucket.DomainName}-Logs"

test/data/invalid/yaml/invalid_sub_reference.yaml

@@ -0,0 +1,8 @@
+Resources:
+  MyMisspeltBucket:
+    Type: AWS::S3::Bucket
+
+  MyLogBucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: !Sub "${MyBucket}-Logs"

test/data/invalid/yaml/issue-39-output-reference-invalid.yaml

@@ -0,0 +1,13 @@
+Resources:
+  MyBucket:
+    Type: AWS::S3::Bucket
+
+Outputs:
+  SomeOutput:
+    Value: !Ref SomethingMissing
+
+  SomeOtherOutput:
+    Value: !Sub "Something-${SomethingMissing}"
+
+  YetAnotherOutput:
+    Value: !Join [" ", ["A", !Ref SomethingMissing]]

test/data/valid/yaml/issue-39-output-reference-check.yaml

@@ -0,0 +1,22 @@
+Resources:
+  MyBucket:
+    Type: AWS::S3::Bucket
+
+Outputs:
+  SomeOtherOutput:
+    Value: !Ref MyBucket
+
+  Output2:
+    Value: !Sub "Output-${MyBucket}"
+
+  Output3:
+    Value: !GetAtt MyBucket.DomainName
+
+  Output4:
+    Value: !Join ["", [!Ref MyBucket, "Something"]]
+
+  Output5:
+    Value:
+      Fn::Sub:
+        - "Some ${Param1} or {$Param2} string"
+        - {Param1: "a", Param2: !Ref "MyBucket"}

test/data/valid/yaml/valid_sub_with_var_map.yaml → test/data/valid/yaml/valid_sub_operations.yaml

@@ -1,24 +1,27 @@
-AWSTemplateFormatVersion: '2010-09-09'
-
-Resources:
-  WebServerSecurityGroup:
-    Type: AWS::EC2::SecurityGroup
-    Properties:
-      GroupDescription: "Enable HTTP access via port 80 locked down to the load balancer + SSH access"
-      SecurityGroupIngress:
-      - CidrIp: 0.0.0.0/0
-        FromPort: '80'
-        IpProtocol: tcp
-        ToPort: '80'
-
-Outputs:
-  Output2:
-    Description: Output2
-    Value: !Sub "${WebServerSecurityGroup}"
-
-  Output3:
-    Description: Output1
-    Value:
-      Fn::Sub:
-        - "Some ${Param1} or {$Param2} string"
-        - {Param1: "a", Param2: !Ref "WebServerSecurityGroup"}
+AWSTemplateFormatVersion: '2010-09-09'
+
+Resources:
+  WebServerSecurityGroup:
+    Type: AWS::EC2::SecurityGroup
+    Properties:
+      GroupDescription: "Enable HTTP access via port 80 locked down to the load balancer + SSH access"
+      SecurityGroupIngress:
+      - CidrIp: 0.0.0.0/0
+        FromPort: '80'
+        IpProtocol: tcp
+        ToPort: '80'
+
+Outputs:
+  Output2:
+    Description: Output2
+    Value: !Sub "${WebServerSecurityGroup}"
+
+  Output3:
+    Description: Output1
+    Value:
+      Fn::Sub:
+        - "Some ${Param1} or {$Param2} string"
+        - {Param1: "a", Param2: !Ref "WebServerSecurityGroup"}
+
+  Output1:
+    Value: !Sub "${WebServerSecurityGroup.GroupId}"

test/validatorTest.js

@@ -146,13 +146,29 @@ describe('validator', () => {
 
     describe('Fn::Sub', () => {
 
-        it('1 valid Fn::Sub with var map should return an object with validTemplate = true, 0 crit errors', () => {
-            const input = './test/data/valid/yaml/valid_sub_with_var_map.yaml';
+        it('3 valid Fn::Sub should return an object with validTemplate = true, 0 crit errors', () => {
+            const input = './test/data/valid/yaml/valid_sub_operations.yaml';
             let result = validator.validateFile(input);
             expect(result).to.have.deep.property('templateValid', true);
             expect(result['errors']['crit']).to.have.lengthOf(0);
         });
 
+        it('a sub referencing an invalid resource should result in validTemplate = false, 1 crit errors, no warnings', () => {
+            const input = 'test/data/invalid/yaml/invalid_sub_reference.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('a sub getatt an invalid resource should result in validTemplate = false, 1 crit errors, no warnings', () => {
+            const input = 'test/data/invalid/yaml/invalid_sub_getatt.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);
+        });
+
     });
 
     describe('Fn::ImportValue', () => {
@@ -406,4 +422,25 @@ describe('validator', () => {
             expect(result['errors']['warn']).to.have.lengthOf(0);
         });
     });
+
+    describe('output-references', () => {
+
+
+        it('an output referencing an invalid resource should result in validTemplate = false, 1 crit errors, no warnings', () => {
+            const input = 'test/data/invalid/yaml/issue-39-output-reference-invalid.yaml';
+            let result = validator.validateFile(input);
+            expect(result).to.have.deep.property('templateValid', false);
+            expect(result['errors']['crit']).to.have.lengthOf(3);
+            expect(result['errors']['warn']).to.have.lengthOf(0);
+        });
+
+        it('an output referencing an valid resource should result in validTemplate = true, 0 crit errors, no warnings', () => {
+            const input = 'test/data/valid/yaml/issue-39-output-reference-check.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);
+        });
+
+    });
 });