From 0d6eca47a1c7fbaff7421a524fccaea324895150 Mon Sep 17 00:00:00 2001 From: "Ian H. Pittwood" Date: Thu, 25 Apr 2019 07:56:18 -0500 Subject: [PATCH] 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 | 12 +- spyglass/site_processors/site_processor.py | 63 ++++++--- tests/shared/documents/invalid/invalid.yaml | 6 +- tests/unit/site_processors/__init__.py | 0 .../site_processors/test_site_processor.py | 131 ++++++++++++++++++ 5 files changed, 188 insertions(+), 24 deletions(-) create mode 100644 tests/unit/site_processors/__init__.py create mode 100644 tests/unit/site_processors/test_site_processor.py diff --git a/spyglass/cli.py b/spyglass/cli.py index b98f057..7ff6eb8 100644 --- a/spyglass/cli.py +++ b/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) diff --git a/spyglass/site_processors/site_processor.py b/spyglass/site_processors/site_processor.py index 421bb9e..e299f39 100644 --- a/spyglass/site_processors/site_processor.py +++ b/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 -from jinja2 import FileSystemLoader +import jinja2 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 + "/" - template_dir_abspath = os.path.dirname(template_software_dir) - LOG.debug("Template Path:%s", template_dir_abspath) + LOG.debug("Template Path: %s", template_dir) - 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), - trim_blocks=True) + loader=loader, + 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] - outfile = outfile_path + "/" + outfile_yaml + LOG.debug("outfile path: %s", outfile_path) + 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 diff --git a/tests/shared/documents/invalid/invalid.yaml b/tests/shared/documents/invalid/invalid.yaml index 8ba3673..28fd819 100644 --- a/tests/shared/documents/invalid/invalid.yaml +++ b/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" - "case, it's a regex)" + pr()perty2: "All property names must match supplied conditions (in this gcase, it's a regex)" +... diff --git a/tests/unit/site_processors/__init__.py b/tests/unit/site_processors/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/site_processors/test_site_processor.py b/tests/unit/site_processors/test_site_processor.py new file mode 100644 index 0000000..474b270 --- /dev/null +++ b/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)