Manifest undefined data validation

This change verifies that manifests generated by Jinja2 do not contain
undefined data. If Spyglass attempts to generate a manifest file that
references undefined data, all previously created manifests will be
deleted and an error will be thrown. Users may bypass this function by
using the "--force" CLI option which will change all undefined data
errors to warnings instead.

Adds undefined data validation to Jinja2 manifest generation.

Adds "--force" option to bypass undefined data validation.

Adds tests for site_processor.py and enables tox testing/coverage.

Change-Id: Iff000eb173995156fbc6b44e621c59ba4dffae35

spyglass/cli.py

@@ -49,6 +49,13 @@ MANIFEST_DIR_OPTION = click.option(
     required=False,
     help='Path to place created manifest files.')
 
+FORCE_OPTION = click.option(
+    '--force',
+    'force',
+    is_flag=True,
+    default=False,
+    help="Forces manifests to be written, regardless of undefined data.")
+
 
 @click.option(
     '-v',
@@ -124,15 +131,16 @@ def intermediary_processor(plugin_type, **kwargs):
     type=click.Path(exists=True, readable=True, dir_okay=False))
 @TEMPLATE_DIR_OPTION
 @MANIFEST_DIR_OPTION
+@FORCE_OPTION
 def generate_manifests_using_intermediary(
-        *, intermediary_file, template_dir, manifest_dir):
+        *, intermediary_file, template_dir, manifest_dir, force):
     LOG.info("Loading intermediary from user provided input")
     with open(intermediary_file, 'r') as f:
         raw_data = f.read()
         intermediary_yaml = yaml.safe_load(raw_data)
 
     LOG.info("Generating site Manifests")
-    processor_engine = SiteProcessor(intermediary_yaml, manifest_dir)
+    processor_engine = SiteProcessor(intermediary_yaml, manifest_dir, force)
     processor_engine.render_template(template_dir)
 
 

spyglass/site_processors/site_processor.py

@@ -1,4 +1,4 @@
-# Copyright 2018 AT&T Intellectual Property.  All other rights reserved.
+# Copyright 2019 AT&T Intellectual Property.  All other rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -14,9 +14,9 @@
 
 import logging
 import os
+import shutil
 
-from jinja2 import Environment
+import jinja2
-from jinja2 import FileSystemLoader
 
 from spyglass.site_processors.base import BaseProcessor
 
@@ -25,53 +25,73 @@ LOG = logging.getLogger(__name__)
 
 class SiteProcessor(BaseProcessor):
 
-    def __init__(self, intermediary_yaml, manifest_dir):
+    def __init__(self, intermediary_yaml, manifest_dir, force_write):
         super().__init__()
         self.yaml_data = intermediary_yaml
         self.manifest_dir = manifest_dir
+        self.force_write = force_write
 
     def render_template(self, template_dir):
         """The method  renders network config yaml from j2 templates.
 
-
         Network configs common to all racks (i.e oam, overlay, storage,
         calico) are generated in a single file. Rack specific
         configs( pxe and oob) are generated per rack.
         """
         # Check of manifest_dir exists
         if self.manifest_dir is not None:
-            site_manifest_dir = self.manifest_dir + "/pegleg_manifests/site/"
+            site_manifest_dir = os.path.join(
+                self.manifest_dir, 'pegleg_manifests', 'site')
         else:
-            site_manifest_dir = "pegleg_manifests/site/"
+            site_manifest_dir = os.path.join('pegleg_manifests', 'site')
         LOG.info("Site manifest output dir:{}".format(site_manifest_dir))
 
-        template_software_dir = template_dir + "/"
+        LOG.debug("Template Path: %s", template_dir)
-        template_dir_abspath = os.path.dirname(template_software_dir)
-        LOG.debug("Template Path:%s", template_dir_abspath)
 
-        for dirpath, dirs, files in os.walk(template_dir_abspath):
+        if self.force_write:
+            logging_undefined = \
+                jinja2.make_logging_undefined(LOG, base=jinja2.Undefined)
+        else:
+            logging_undefined = \
+                jinja2.make_logging_undefined(LOG, base=jinja2.StrictUndefined)
+
+        template_folder_name = os.path.split(template_dir)[1]
+        created_file_list = []
+        created_dir_list = []
+
+        for dirpath, dirs, files in os.walk(template_dir):
+            loader = jinja2.FileSystemLoader(dirpath)
             for filename in files:
-                j2_env = Environment(
+                j2_env = jinja2.Environment(
                     autoescape=True,
-                    loader=FileSystemLoader(dirpath),
+                    loader=loader,
-                    trim_blocks=True)
+                    trim_blocks=True,
+                    undefined=logging_undefined)
                 j2_env.filters["get_role_wise_nodes"] = \
                     self.get_role_wise_nodes
                 templatefile = os.path.join(dirpath, filename)
-                outdirs = dirpath.split("templates")[1]
+                LOG.debug("Template file: %s", templatefile)
+                outdirs = dirpath.split(template_folder_name)[1].lstrip(os.sep)
+                LOG.debug("outdirs: %s", outdirs)
 
-                outfile_path = "{}{}{}".format(
+                outfile_path = os.path.join(
                     site_manifest_dir, self.yaml_data["region_name"], outdirs)
-                outfile_yaml = templatefile.split(".j2")[0].split("/")[-1]
+                LOG.debug("outfile path: %s", outfile_path)
-                outfile = outfile_path + "/" + outfile_yaml
+                outfile_yaml = os.path.split(templatefile)[1]
+                outfile_yaml = os.path.splitext(outfile_yaml)[0]
+                outfile = os.path.join(outfile_path, outfile_yaml)
+                LOG.debug("outfile: %s", outfile)
                 outfile_dir = os.path.dirname(outfile)
                 if not os.path.exists(outfile_dir):
                     os.makedirs(outfile_dir)
+                    created_dir_list.append(outfile_dir)
                 template_j2 = j2_env.get_template(filename)
                 try:
                     out = open(outfile, "w")
-                    template_j2.stream(data=self.yaml_data).dump(out)
+                    created_file_list.append(outfile)
                     LOG.info("Rendering {}".format(outfile_yaml))
+                    rendered = template_j2.render(data=self.yaml_data)
+                    out.write(rendered)
                     out.close()
                 except IOError as ioe:
                     LOG.error(
@@ -79,3 +99,8 @@ class SiteProcessor(BaseProcessor):
                     raise SystemExit(
                         "Error when generating {:s}:\n{:s}".format(
                             outfile, ioe.strerror))
+                except jinja2.UndefinedError as e:
+                    LOG.info('Undefined data found, rolling back changes...')
+                    out.close()
+                    shutil.rmtree(site_manifest_dir)
+                    raise e

tests/shared/documents/invalid/invalid.yaml

@@ -4,7 +4,7 @@ foo: Not a number
 bar: "Doesn't equal constant"
 baz:
   staticProperty:
-  - This array needs at least one number
+    - This array needs at least one number
   property1: The propertyNames keyword is an alternative to patternProperties
-  pr()perty2: "All property names must match supplied conditions (in this"
+  pr()perty2: "All property names must match supplied conditions (in this gcase, it's a regex)"
-              "case, it's a regex)"
+...

tests/unit/site_processors/test_site_processor.py

@@ -0,0 +1,131 @@
+# Copyright 2019 AT&T Intellectual Property.  All other rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import logging
+import os
+from tempfile import mkdtemp
+
+from jinja2 import UndefinedError
+import pytest
+
+from spyglass.site_processors.site_processor import SiteProcessor
+
+LOG = logging.getLogger(__name__)
+LOG.level = logging.DEBUG
+
+J2_TPL = """---
+schema: pegleg/SiteDefinition/v1
+metadata:
+  schema: metadata/Document/v1
+  layeringDefinition:
+    abstract: false
+    layer: site
+  name: {{ data['region_name'] }}
+  storagePolicy: cleartext
+data:
+  site_type:{{ data['site_info']['sitetype'] }}
+..."""
+
+
+def test_render_template():
+    _tpl_parent_dir = mkdtemp()
+    _tpl_dir = mkdtemp(dir=_tpl_parent_dir)
+    _tpl_file = os.path.join(_tpl_dir, "test.yaml.j2")
+    with open(_tpl_file, 'w') as f:
+        f.write(J2_TPL)
+        LOG.debug("Writing test template to %s", _tpl_file)
+    _input_yaml = {
+        "region_name": "test",
+        "site_info": {
+            "sitetype": "test_type"
+        }
+    }
+    _out_dir = mkdtemp()
+    site_processor = SiteProcessor(_input_yaml, _out_dir, force_write=False)
+    site_processor.render_template(_tpl_parent_dir)
+
+    expected_output = """---
+schema: pegleg/SiteDefinition/v1
+metadata:
+  schema: metadata/Document/v1
+  layeringDefinition:
+    abstract: false
+    layer: site
+  name: test
+  storagePolicy: cleartext
+data:
+  site_type:test_type
+..."""
+
+    output_file = os.path.join(
+        _out_dir, "pegleg_manifests", "site", _input_yaml["region_name"],
+        os.path.split(_tpl_dir)[1], "test.yaml")
+    LOG.debug(output_file)
+    assert (os.path.exists(output_file))
+    with open(output_file, 'r') as f:
+        content = f.read()
+        assert (expected_output == content)
+
+
+def test_render_template_missing_data():
+    _tpl_parent_dir = mkdtemp()
+    _tpl_dir = mkdtemp(dir=_tpl_parent_dir)
+    _tpl_file = os.path.join(_tpl_dir, "test.yaml.j2")
+    with open(_tpl_file, 'w') as f:
+        f.write(J2_TPL)
+        LOG.debug("Writing test template to %s", _tpl_file)
+    _input_yaml = {"region_name": "test", "site_info": {}}
+    _out_dir = mkdtemp()
+    site_processor = SiteProcessor(_input_yaml, _out_dir, force_write=False)
+    with pytest.raises(UndefinedError):
+        site_processor.render_template(_tpl_parent_dir)
+
+    output_file = os.path.join(
+        _out_dir, "pegleg_manifests", "site", _input_yaml["region_name"],
+        os.path.split(_tpl_dir)[1], "test.yaml")
+    assert (not os.path.exists(output_file))
+
+
+def test_render_template_missing_data_force():
+    _tpl_parent_dir = mkdtemp()
+    _tpl_dir = mkdtemp(dir=_tpl_parent_dir)
+    _tpl_file = os.path.join(_tpl_dir, "test.yaml.j2")
+    with open(_tpl_file, 'w') as f:
+        f.write(J2_TPL)
+        LOG.debug("Writing test template to %s", _tpl_file)
+    _input_yaml = {"region_name": "test", "site_info": {}}
+    _out_dir = mkdtemp()
+    site_processor = SiteProcessor(_input_yaml, _out_dir, force_write=True)
+    site_processor.render_template(_tpl_parent_dir)
+
+    expected_output = """---
+schema: pegleg/SiteDefinition/v1
+metadata:
+  schema: metadata/Document/v1
+  layeringDefinition:
+    abstract: false
+    layer: site
+  name: test
+  storagePolicy: cleartext
+data:
+  site_type:
+..."""
+
+    output_file = os.path.join(
+        _out_dir, "pegleg_manifests", "site", _input_yaml["region_name"],
+        os.path.split(_tpl_dir)[1], "test.yaml")
+    assert (os.path.exists(output_file))
+    with open(output_file, 'r') as f:
+        content = f.read()
+        assert (expected_output == content)