From 37b2a31b6f0532efc47db6565c533942a1f5b10a Mon Sep 17 00:00:00 2001 From: Krysta Date: Mon, 5 Mar 2018 08:10:38 -0600 Subject: [PATCH] Allow linting policies to be selectable Adds option -x to exclude certain linting policies and -w to warn of failure for certain linting policies if failures are expected. Updates gitignore to exclude files created when running tests. Adds requirements for testing. Adds unit test for new cli options and test site-definition.yaml Documentation for cli options can be found here [0]. [0]-https://review.gerrithub.io/#/c/403216/ Change-Id: I6e905c1ba7a23d0b2fdbf9552bec8a6620ff9731 --- .gitignore | 3 + docs/source/index.rst | 2 +- site_yamls/site-definition.yaml | 11 +++ src/bin/pegleg/pegleg/cli.py | 18 +++- src/bin/pegleg/pegleg/engine/lint.py | 74 +++++++++++---- src/bin/pegleg/test-requirements.txt | 7 ++ src/bin/pegleg/tests/__init__.py | 0 src/bin/pegleg/tests/unit/__init__.py | 0 .../tests/unit/test_selectable_linitng.py | 92 +++++++++++++++++++ src/bin/pegleg/tox.ini | 5 + 10 files changed, 190 insertions(+), 22 deletions(-) create mode 100644 site_yamls/site-definition.yaml create mode 100644 src/bin/pegleg/test-requirements.txt create mode 100644 src/bin/pegleg/tests/__init__.py create mode 100644 src/bin/pegleg/tests/unit/__init__.py create mode 100644 src/bin/pegleg/tests/unit/test_selectable_linitng.py diff --git a/.gitignore b/.gitignore index 53395021..701c5226 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,6 @@ pegleg.egg-info /ChangeLog /AUTHORS *.swp +build +dist +.cache diff --git a/docs/source/index.rst b/docs/source/index.rst index a7bcce6c..a9abdf14 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -16,7 +16,7 @@ .. tip:: - The Undercloud Platform is part of the AIC CP (AT&T Integrated Cloud + The Undercloud Platform is part of the AIC CP (AT&T Integrated Cloud Containerized Platform). More details may be found by using the `Treasuremap`_ Building this Documentation diff --git a/site_yamls/site-definition.yaml b/site_yamls/site-definition.yaml new file mode 100644 index 00000000..3005e268 --- /dev/null +++ b/site_yamls/site-definition.yaml @@ -0,0 +1,11 @@ +--- +data: + revision: v1.0 + site_type: cicd +metadata: + layeringDefinition: {abstract: false, layer: site} + name: test-file + schema: metadata/Document/v1 + storagePolicy: cleartext +schema: pegleg/SiteDefinition/v1 +... diff --git a/src/bin/pegleg/pegleg/cli.py b/src/bin/pegleg/pegleg/cli.py index 9590d262..edb7a4e5 100644 --- a/src/bin/pegleg/pegleg/cli.py +++ b/src/bin/pegleg/pegleg/cli.py @@ -190,10 +190,24 @@ def site_type(*, revision, site_type): 'aux_repo', multiple=True, help='Path to the root of a auxiliary repo.') -def lint(*, fail_on_missing_sub_src, primary_repo, aux_repo): +@click.option( + '-x', + '--exclude', + 'exclude_lint', + multiple=True, + help='Excludes specified linting checks. Warnings will still be issued. ' + '-w takes priority over -x.') +@click.option( + '-w', + '--warn', + 'warn_lint', + multiple=True, + help='Warn if linting check fails. -w takes priority over -x.') +def lint(*, fail_on_missing_sub_src, primary_repo, aux_repo, exclude_lint, + warn_lint): config.set_primary_repo(primary_repo) config.set_auxiliary_repo_list(aux_repo or []) - warns = engine.lint.full(fail_on_missing_sub_src) + warns = engine.lint.full(fail_on_missing_sub_src, exclude_lint, warn_lint) if warns: click.echo("Linting passed, but produced some warnings.") for w in warns: diff --git a/src/bin/pegleg/pegleg/engine/lint.py b/src/bin/pegleg/pegleg/engine/lint.py index 39b9f9f4..caaa0bb9 100644 --- a/src/bin/pegleg/pegleg/engine/lint.py +++ b/src/bin/pegleg/pegleg/engine/lint.py @@ -4,8 +4,12 @@ import os import pkg_resources import yaml -from pegleg.engine import util from pegleg import config +from pegleg.engine import util + +SCHEMA_STORAGE_POLICY_MISMATCH_FLAG = 'P001' +DECKHAND_RENDERING_INCOMPLETE_FLAG = 'P002' +REPOS_MISSING_DIRECTORIES_FLAG = 'P003' __all__ = ['full'] @@ -18,12 +22,39 @@ DECKHAND_SCHEMAS = { } -def full(fail_on_missing_sub_src=False): +def full(fail_on_missing_sub_src=False, exclude_lint=None, warn_lint=None): errors = [] warns = [] - warns.extend(_verify_no_unexpected_files()) - errors.extend(_verify_file_contents()) - errors.extend(_verify_deckhand_render(fail_on_missing_sub_src)) + + messages = _verify_file_contents() + # If policy is cleartext and error is added this will put + # that particular message into the warns list and all other will + # be added to the error list if SCHMEA_STORAGE_POLICY_MITCHMATCH_FLAG + for msg in messages: + if (SCHEMA_STORAGE_POLICY_MISMATCH_FLAG in warn_lint + and SCHEMA_STORAGE_POLICY_MISMATCH_FLAG == msg[0]): + warns.append(msg) + else: + errors.append(msg) + + # Deckhand Rendering completes without error + if DECKHAND_RENDERING_INCOMPLETE_FLAG in warn_lint: + warns.extend( + [(DECKHAND_RENDERING_INCOMPLETE_FLAG, x) + for x in _verify_deckhand_render(fail_on_missing_sub_src)]) + elif DECKHAND_RENDERING_INCOMPLETE_FLAG not in exclude_lint: + errors.extend( + [(DECKHAND_RENDERING_INCOMPLETE_FLAG, x) + for x in _verify_deckhand_render(fail_on_missing_sub_src)]) + + # All repos contain expected directories + if REPOS_MISSING_DIRECTORIES_FLAG in warn_lint: + warns.extend([(REPOS_MISSING_DIRECTORIES_FLAG, x) + for x in _verify_no_unexpected_files()]) + elif REPOS_MISSING_DIRECTORIES_FLAG not in exclude_lint: + errors.extend([(REPOS_MISSING_DIRECTORIES_FLAG, x) + for x in _verify_no_unexpected_files()]) + if errors: raise click.ClickException('\n'.join( ['Linting failed:'] + errors + ['Linting warnings:'] + warns)) @@ -40,16 +71,18 @@ def _verify_no_unexpected_files(): found_directories = util.files.existing_directories() LOG.debug('found_directories: %s', found_directories) - msgs = [] + errors = [] for unused_dir in sorted(found_directories - expected_directories): - msgs.append('%s exists, but is unused' % unused_dir) + errors.append((REPOS_MISSING_DIRECTORIES_FLAG, + '%s exists, but is unused' % unused_dir)) for missing_dir in sorted(expected_directories - found_directories): if not missing_dir.endswith('common'): - msgs.append( - '%s was not found, but expected by manifest' % missing_dir) + errors.append( + (REPOS_MISSING_DIRECTORIES_FLAG, + '%s was not found, but expected by manifest' % missing_dir)) - return msgs + return errors def _verify_file_contents(): @@ -97,22 +130,25 @@ def _verify_document(document, schemas, filename): layer = _layer(document) if layer is not None and layer != _expected_layer(filename): errors.append( - '%s (document %s) had unexpected layer "%s", expected "%s"' % - (filename, name, layer, _expected_layer(filename))) + ('N/A', + '%s (document %s) had unexpected layer "%s", expected "%s"' % + (filename, name, layer, _expected_layer(filename)))) # secrets must live in the appropriate directory, and must be # "storagePolicy: encrypted". if document.get('schema') in MANDATORY_ENCRYPTED_TYPES: storage_policy = document.get('metadata', {}).get('storagePolicy') - if storage_policy != 'encrypted': - errors.append('%s (document %s) is a secret, but has unexpected ' - 'storagePolicy: "%s"' % (filename, name, - storage_policy)) + + if (storage_policy != 'encrypted'): + errors.append((SCHEMA_STORAGE_POLICY_MISMATCH_FLAG, + '%s (document %s) is a secret, but has unexpected ' + 'storagePolicy: "%s"' % (filename, name, + storage_policy))) if not _filename_in_section(filename, 'secrets/'): - errors.append( - '%s (document %s) is a secret, is not stored in a secrets path' - % (filename, name)) + errors.append(('N/A', + '%s (document %s) is a secret, is not stored in a ' + 'secrets path' % (filename, name))) return errors diff --git a/src/bin/pegleg/test-requirements.txt b/src/bin/pegleg/test-requirements.txt new file mode 100644 index 00000000..4403325c --- /dev/null +++ b/src/bin/pegleg/test-requirements.txt @@ -0,0 +1,7 @@ +# Testing +pytest==3.2.1 +pytest-cov==2.5.1 +mock==2.0.0 + +# Linting +flake8==3.3.0 diff --git a/src/bin/pegleg/tests/__init__.py b/src/bin/pegleg/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/bin/pegleg/tests/unit/__init__.py b/src/bin/pegleg/tests/unit/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/bin/pegleg/tests/unit/test_selectable_linitng.py b/src/bin/pegleg/tests/unit/test_selectable_linitng.py new file mode 100644 index 00000000..d18d734e --- /dev/null +++ b/src/bin/pegleg/tests/unit/test_selectable_linitng.py @@ -0,0 +1,92 @@ +# Copyright 2018 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 click +import mock +import pytest + +from pegleg import config +from pegleg.engine import lint + + +@mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) +@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) +def test_lint_excludes_P001(*args): + exclude_lint = ['P001'] + config.set_primary_repo('../pegleg/site_yamls/') + + msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"' + msg_2 = 'test msg' + msgs = [msg_1, msg_2] + + with mock.patch.object(lint, '_verify_file_contents', return_value=msgs) as mock_methed: + with pytest.raises(click.ClickException) as expected_exc: + results = lint.full(False, exclude_lint, []) + e = str(expected_exc) + assert msg_1 in expected_exc + assert msg_2 in expected_exc + + +@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) +def test_lint_excludes_P002(*args): + exclude_lint = ['P002'] + config.set_primary_repo('../pegleg/site_yamls/') + with mock.patch.object(lint, '_verify_deckhand_render') as mock_method: + lint.full(False, exclude_lint, []) + mock_method.assert_not_called() + + +@mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) +def test_lint_excludes_P003(*args): + exclude_lint = ['P003'] + with mock.patch.object(lint, '_verify_no_unexpected_files') as mock_method: + lint.full(False, exclude_lint, []) + mock_method.assert_not_called() + + +@mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) +@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) +def test_lint_warns_P001(*args): + warn_lint = ['P001'] + config.set_primary_repo('../pegleg/site_yamls/') + + msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"' + msg_2 = 'test msg' + msgs = [msg_1, msg_2] + + with mock.patch.object(lint, '_verify_file_contents', return_value=msgs) as mock_methed: + with pytest.raises(click.ClickException) as expected_exc: + lint.full(False, [], warn_lint) + e = str(expected_exc) + assert msg_1 not in expected_exc + assert msg_2 in expected_exc + + +@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) +def test_lint_warns_P002(*args): + warn_lint = ['P002'] + config.set_primary_repo('../pegleg/site_yamls/') + + with mock.patch.object(lint, '_verify_deckhand_render') as mock_method: + lint.full(False, [], warn_lint) + mock_method.assert_called() + + +@mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) +def test_lint_warns_P003(*args): + warn_lint = ['P003'] + config.set_primary_repo('../pegleg/site_yamls/') + + with mock.patch.object(lint, '_verify_no_unexpected_files') as mock_method: + lint.full(False, [], warn_lint) + mock_method.assert_called() diff --git a/src/bin/pegleg/tox.ini b/src/bin/pegleg/tox.ini index 3509e01b..9ecfb9e9 100644 --- a/src/bin/pegleg/tox.ini +++ b/src/bin/pegleg/tox.ini @@ -2,7 +2,12 @@ envlist = lint [testenv] +deps = -r{toxinidir}/test-requirements.txt + basepython=python3 +commands= + pytest \ + {posargs} [testenv:fmt] deps = yapf==0.20.0