Merge pull request #109 from cfstacks/validate_template

Implement client-side template validation for null values

stacks/cf.py

@@ -13,6 +13,7 @@
 from fnmatch import fnmatch
 from operator import attrgetter
 from os import path
+from collections import Mapping, Set, Sequence
 
 import boto
 import jinja2
@@ -48,9 +49,12 @@ def gen_template(tpl_file, config):
         sys.exit(1)
 
     if len(docs) == 2:
-        return json.dumps(docs[1], indent=2, sort_keys=True), docs[0]
+        tpl, metadata = docs[1], docs[0]
     else:
-        return json.dumps(docs[0], indent=2, sort_keys=True), None
+        tpl, metadata = docs[0], None
+
+    errors = validate_template(tpl)
+    return json.dumps(tpl, indent=2, sort_keys=True), metadata, errors
 
 
 def _check_missing_vars(env, tpl_file, config):
@@ -165,7 +169,7 @@ def list_stacks(conn, name_filter='*', verbose=False):
 
 def create_stack(conn, stack_name, tpl_file, config, update=False, dry=False, create_on_update=False):
     """Create or update CloudFormation stack from a jinja2 template"""
-    tpl, metadata = gen_template(tpl_file, config)
+    tpl, metadata, errors = gen_template(tpl_file, config)
 
     # Set default tags which cannot be overwritten
     default_tags = {
@@ -188,6 +192,11 @@ def create_stack(conn, stack_name, tpl_file, config, update=False, dry=False, cr
     if not stack_name:
         print('Stack name must be specified via command line argument or stack metadata.')
         sys.exit(1)
+    if errors:
+        for err in errors:
+            print('ERROR: ' + err)
+        if not dry:
+            sys.exit(1)
 
     tpl_size = len(tpl)
 
@@ -356,3 +365,35 @@ def stack_exists(conn, stack_name):
 def normalize_events_timestamps(events):
     for ev in events:
         ev.timestamp = ev.timestamp.replace(tzinfo=pytz.UTC)
+
+
+def traverse_template(obj, obj_path=(), memo=None):
+    def iteritems(mapping):
+        return getattr(mapping, 'iteritems', mapping.items)()
+
+    if memo is None:
+        memo = set()
+    iterator = None
+    if isinstance(obj, Mapping):
+        iterator = iteritems
+    elif isinstance(obj, (Sequence, Set)) and not isinstance(obj, (str, bytes)):
+        iterator = enumerate
+    if iterator:
+        if id(obj) not in memo:
+            memo.add(id(obj))
+            for path_component, value in iterator(obj):
+                for result in traverse_template(value, obj_path + (path_component,), memo):
+                    yield result
+            memo.remove(id(obj))
+    else:
+        yield obj_path, obj
+
+
+def validate_template(tpl):
+    errors = []
+    for k, v in traverse_template(tpl):
+        if v is None:
+            errors.append(
+                "/{} 'null' values are not allowed in templates".format('/'.join(map(str, k)))
+            )
+    return errors

tests/fixtures/invalid_template_with_null_value.yaml

@@ -0,0 +1,37 @@
+---
+metadata:
+  name: {{ env }}-test-stack
+  tags:
+  - key: Test
+    value: {{ test_tag }}
+---
+AWSTemplateFormatVersion: '2010-09-09'
+Description: Test Stack
+Parameters:
+  Foo:
+    Type: String
+    Default: Bar
+Resources:
+  VPC:
+    Type: AWS::EC2::VPC
+    Properties:
+      CidrBlock: 10.50.0.0/16
+      EnableDnsSupport: true
+      EnableDnsHostnames: true
+      Tags:
+        - Key: Name
+          Value: {{ env }}-test-vpc
+        - Key: Env
+          Value: null
+Outputs:
+  Foo:
+    Description: 'Just to test reference and substitution'
+    Value: !Ref Foo
+    Export:
+      Name: !Sub ${AWS::StackName}-Foo
+  VPCCidrBlock:
+    Description: 'Just to test attribute selection'
+    Value: !GetAtt VPC.CidrBlock
+    Export:
+      Name: !Sub ${AWS::StackName}-VPCCidrBlock
+

tests/test_cf.py

@@ -1,4 +1,5 @@
 import unittest
+
 import boto
 from moto import mock_cloudformation_deprecated
 
@@ -10,26 +11,35 @@ class TestTemplate(unittest.TestCase):
     def test_gen_valid_template(self):
         config = {'env': 'dev', 'test_tag': 'testing'}
         tpl_file = open('tests/fixtures/valid_template.yaml')
-        tpl, options = cf.gen_template(tpl_file, config)
+        tpl, metadata, errors = cf.gen_template(tpl_file, config)
         self.assertIsInstance(tpl, str)
-        self.assertIsInstance(options, dict)
+        self.assertIsInstance(metadata, dict)
+        self.assertEqual(len(errors), 0)
 
     def test_gen_invalid_template(self):
         config = {'env': 'dev', 'test_tag': 'testing'}
         tpl_file = open('tests/fixtures/invalid_template.yaml')
 
         with self.assertRaises(SystemExit) as err:
-            tpl, options = cf.gen_template(tpl_file, config)
+            cf.gen_template(tpl_file, config)
         self.assertEqual(err.exception.code, 1)
 
     def test_gen_template_missing_properties(self):
         config = {'env': 'unittest'}
         tpl_file = open('tests/fixtures/valid_template.yaml')
 
         with self.assertRaises(SystemExit) as err:
-            tpl, options = cf.gen_template(tpl_file, config)
+            cf.gen_template(tpl_file, config)
         self.assertEqual(err.exception.code, 1)
 
+    def test_gen_invalid_template_with_null_value(self):
+        config = {'env': 'dev', 'test_tag': 'testing'}
+        tpl_file = open('tests/fixtures/invalid_template_with_null_value.yaml')
+        tpl, metadata, errors = cf.gen_template(tpl_file, config)
+        self.assertIsInstance(tpl, str)
+        self.assertIsInstance(metadata, dict)
+        self.assertEqual(len(errors), 1)
+
 
 @mock_cloudformation_deprecated
 class TestStackActions(unittest.TestCase):
@@ -86,5 +96,6 @@ def test_create_stack_no_metadata(self):
         self.assertEqual(self.config['env'], stack.tags['Env'])
         self.assertEqual('b08c2e9d7003f62ba8ffe5c985c50a63', stack.tags['MD5Sum'])
 
+
 if __name__ == '__main__':
     unittest.main()