Implement global lint and lint by site logic

With the implementation of revisioned repository to the CLI in
https://review.openstack.org/#/c/577886 there was a change to
the lint command [0], which changed it from being a global lint
to a site-level (targetted lint)... kind of: Only the CLI logic
was modified to support targetted single-site linting. Thus,
the first issue this patch set addresses is implementing the
back-end logic to realize targetted, single-site linting.

The second issue this patch set addresses is re-supporting global
linting (linting all sites within a repository) which means that
this partially reverts [0] which had (kind of) replaced global
linting with per-site linting.

So, this patch set:

1) Implements targetted, single-site linting back-end logic
2) Re-implements global linting for all sites in a repo
3) Adds unit tests for both
4) Adds some helper functions to util.engine.definition to
   help with 1) and 2)

[0] https://review.openstack.org/#/c/577886/4/src/bin/pegleg/pegleg/cli.py@191

Change-Id: I5147282556763d93dfaf06912d2c4c876e1bd69f

Makefile

@@ -99,8 +99,8 @@ clean:
 
 .PHONY: py_lint
 py_lint:
-	cd pegleg;tox -e pep8
+	tox -e pep8
 
 .PHONY: py_format
 py_format:
-	cd pegleg;tox -e fmt
+	tox -e fmt

doc/source/cli/cli.rst

@@ -83,17 +83,13 @@ Enable debug logging.
 
 .. _site:
 
-Site
+Repo
-----
+====
 
-This allows you to set the primary and auxiliary repositories.
+Allows you to perform repository-level operations.
-
-::
-
-  ./pegleg.sh site -r <site_repo> -e <extra_repo> <command> <options>
 
 Options
-^^^^^^^
+-------
 
 **-r / --site-repository** (Required).
 
@@ -101,6 +97,46 @@ Path to the root of the site repository (containing site_definition.yaml) repo.
 
 For example: /opt/aic-site-clcp-manifests.
 
+The revision can also be specified via (for example):
+
+::
+
+  -r /opt/aic-site-clcp-manifests@revision
+
+.. _cli-repo-lint:
+
+Lint
+----
+
+Sanity checks for repository content (all sites in the repository). To lint
+a specific site, see :ref:`site-level linting <cli-site-lint>`.
+
+See :ref:`linting` for more information.
+
+Site
+====
+
+Allows you to perform site-level operations.
+
+::
+
+  ./pegleg.sh site -r <site_repo> -e <extra_repo> <command> <options>
+
+Options
+-------
+
+**-r / --site-repository** (Required).
+
+Path to the root of the site repository (containing site_definition.yaml) repo.
+
+For example: /opt/aic-site-clcp-manifests.
+
+The revision can also be specified via (for example):
+
+::
+
+  -r /opt/aic-site-clcp-manifests@revision
+
 **-e / --extra-repository** (Optional).
 
 Path to the root of extra repositories used for overriding those specified
@@ -112,6 +148,9 @@ These should be named per the site-definition file, e.g.:
 
   -e global=/opt/global -e secrets=/opt/secrets
 
+Repository Overrides
+^^^^^^^^^^^^^^^^^^^^
+
 By default, the revision specified in the :file:`site-definition.yaml` for the
 site will be leveraged but can be
 :ref:`overridden <command-line-repository-overrides>` using:
@@ -172,8 +211,8 @@ Will warn of lint failures from the specified lint options.
 
 **--validate** (Optional, validation only). False by default.
 
-Perform validation of documents prior to collection. See :ref:`linting` for
+Perform validation of documents prior to collection. See :ref:`cli-site-lint`
-additional information on document linting. It is recommended that document
+for additional information on document linting. It is recommended that document
 linting be executed prior to document collection. However, ``--validate``
 is False by default for backwards compatibility concerns.
 
@@ -283,43 +322,18 @@ Example:
 
   ./pegleg site -r /opt/aic-clcp-site-manifests render site_name -o output
 
-.. _linting:
+.. _cli-site-lint:
 
 Lint
 ----
 
-Sanity checks for repository content. Validations for linting are done
+Sanity checks for repository content (for a specific site in the repository).
-utilizing `Deckhand Validations`_.
+Validations for linting are done utilizing `Deckhand Validations`_.
 
-**-f / --fail-on-missing-sub-src** (Optional).
+To lint all sites in the repository, see
+:ref:`repository-level linting <cli-repo-lint>`.
 
-Raise Deckhand exception on missing substitution sources. Defaults to True.
+See :ref:`linting` for more information.
-
-**-x <code>** (Optional).
-
-Will exclude the specified lint option. -w takes priority over -x.
-
-**-w <code>** (Optional).
-
-Will warn of lint failures from the specified lint options.
-
-::
-
-  If you expect certain lint failures, then those lint options can be
-  excluded or you can choose to be warned about those failures using the
-  codes below.
-
-  P001 - Document has storagePolicy cleartext (expected is encrypted) yet
-  its schema is a mandatory encrypted type.
-
-  Where mandatory encrypted schema type is one of:
-  * deckhand/CertificateAuthorityKey/v1
-  * deckhand/CertificateKey/v1
-  * deckhand/Passphrase/v1
-  * deckhand/PrivateKey/v1
-
-  P002 - Deckhand rendering is expected to complete without errors.
-  P003 - All repos contain expected directories.
 
 Examples
 ^^^^^^^^
@@ -421,5 +435,39 @@ Valid Git references for checking out repositories include:
   * refs/changes/79/47079/12 (ref)
   * master (branch name)
 
+.. _linting:
+
+Linting
+=======
+
+**-f / --fail-on-missing-sub-src** (Optional).
+
+Raise Deckhand exception on missing substitution sources. Defaults to True.
+
+**-x <code>** (Optional).
+
+Will exclude the specified lint option. -w takes priority over -x.
+
+**-w <code>** (Optional).
+
+Will warn of lint failures from the specified lint options.
+
+If you expect certain lint failures, then those lint options can be
+excluded or you can choose to be warned about those failures using the
+codes below.
+
+P001 - Document has storagePolicy cleartext (expected is encrypted) yet
+its schema is a mandatory encrypted type.
+
+Where mandatory encrypted schema type is one of:
+
+* deckhand/CertificateAuthorityKey/v1
+* deckhand/CertificateKey/v1
+* deckhand/Passphrase/v1
+* deckhand/PrivateKey/v1
+
+P002 - Deckhand rendering is expected to complete without errors.
+P003 - All repos contain expected directories.
+
 .. _Deckhand: https://airship-deckhand.readthedocs.io/en/latest/rendering.html
 .. _Deckhand Validations: https://airship-deckhand.readthedocs.io/en/latest/validation.html

doc/source/index.rst

@@ -49,5 +49,5 @@ Operator's Guide
 .. toctree::
    :maxdepth: 2
 
-   cli
+   cli/cli
    exceptions

pegleg/cli.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
 import logging
 import sys
 
@@ -28,24 +29,7 @@ CONTEXT_SETTINGS = {
     'help_option_names': ['-h', '--help'],
 }
 
-
+REPOSITORY_OPTION = click.option(
-@click.group(context_settings=CONTEXT_SETTINGS)
-@click.option(
-    '-v',
-    '--verbose',
-    is_flag=bool,
-    default=False,
-    help='Enable debug logging')
-def main(*, verbose):
-    if verbose:
-        log_level = logging.DEBUG
-    else:
-        log_level = logging.INFO
-    logging.basicConfig(format=LOG_FORMAT, level=log_level)
-
-
-@main.group(help='Commands related to sites')
-@click.option(
     '-r',
     '--site-repository',
     'site_repository',
@@ -53,7 +37,8 @@ def main(*, verbose):
     help=
     'Path or URL to the primary repository (containing site_definition.yaml) '
     'repo.')
-@click.option(
+
+EXTRA_REPOSITORY_OPTION = click.option(
     '-e',
     '--extra-repository',
     'extra_repositories',
@@ -63,6 +48,105 @@ def main(*, verbose):
     'secrets=/opt/secrets. By default, the revision specified in the '
     'site-definition for the site will be leveraged but can be overridden '
     'using -e global=/opt/global@revision.')
+
+ALLOW_MISSING_SUBSTITUTIONS_OPTION = click.option(
+    '-f',
+    '--fail-on-missing-sub-src',
+    required=False,
+    type=click.BOOL,
+    default=True,
+    help=
+    "Raise Deckhand exception on missing substition sources. Defaults to True."
+)
+
+EXCLUDE_LINT_OPTION = click.option(
+    '-x',
+    '--exclude',
+    'exclude_lint',
+    multiple=True,
+    help='Excludes specified linting checks. Warnings will still be issued. '
+    '-w takes priority over -x.')
+
+WARN_LINT_OPTION = click.option(
+    '-w',
+    '--warn',
+    'warn_lint',
+    multiple=True,
+    help='Warn if linting check fails. -w takes priority over -x.')
+
+
+@click.group(context_settings=CONTEXT_SETTINGS)
+@click.option(
+    '-v',
+    '--verbose',
+    is_flag=bool,
+    default=False,
+    help='Enable debug logging')
+def main(*, verbose):
+    """Main CLI meta-group, which includes the following groups:
+
+    * site: site-level actions
+    * repo: repository-level actions
+    * stub (DEPRECATED)
+
+    """
+
+    if verbose:
+        log_level = logging.DEBUG
+    else:
+        log_level = logging.INFO
+    logging.basicConfig(format=LOG_FORMAT, level=log_level)
+
+
+@main.group(help='Commands related to repositories')
+@REPOSITORY_OPTION
+def repo(*, site_repository):
+    """Group for repo-level actions, which include:
+
+    * lint: lint all sites across the repository
+
+    """
+
+    config.set_site_repo(site_repository)
+
+
+def _lint_helper(*,
+                 fail_on_missing_sub_src,
+                 exclude_lint,
+                 warn_lint,
+                 site_name=None):
+    """Helper for executing lint on specific site or all sites in repo."""
+    if site_name:
+        func = functools.partial(engine.lint.site, site_name=site_name)
+    else:
+        func = engine.lint.full
+    warns = func(
+        fail_on_missing_sub_src=fail_on_missing_sub_src,
+        exclude_lint=exclude_lint,
+        warn_lint=warn_lint)
+    if warns:
+        click.echo("Linting passed, but produced some warnings.")
+        for w in warns:
+            click.echo(w)
+
+
+@repo.command('lint', help='Lint all sites in a repository')
+@ALLOW_MISSING_SUBSTITUTIONS_OPTION
+@EXCLUDE_LINT_OPTION
+@WARN_LINT_OPTION
+def lint_repo(*, fail_on_missing_sub_src, exclude_lint, warn_lint):
+    """Lint all sites using checks defined in :mod:`pegleg.engine.errorcodes`.
+    """
+    engine.repository.process_site_repository(update_config=True)
+    _lint_helper(
+        fail_on_missing_sub_src=fail_on_missing_sub_src,
+        exclude_lint=exclude_lint,
+        warn_lint=warn_lint)
+
+
+@main.group(help='Commands related to sites')
+@REPOSITORY_OPTION
+@EXTRA_REPOSITORY_OPTION
 @click.option(
     '-k',
     '--repo-key',
@@ -78,6 +162,15 @@ def main(*, verbose):
     'specified in the site-definition file. Any occurrences of REPO_USERNAME '
     'will be replaced with this value.')
 def site(*, site_repository, extra_repositories, repo_key, repo_username):
+    """Group for site-level actions, which include:
+
+    * list: list available sites in a manifests repo
+    * lint: lint a site along with all its dependencies
+    * render: render a site using Deckhand
+    * show: show a sites' files
+
+    """
+
     config.set_site_repo(site_repository)
     config.set_extra_repo_store(extra_repositories or [])
     config.set_repo_key(repo_key)
@@ -130,7 +223,8 @@ def collect(*, save_location, validate, exclude_lint, warn_lint, site_name):
 
     if validate:
         # Lint the primary repo prior to document collection.
-        _lint(
+        _lint_helper(
+            site_name=site_name,
             fail_on_missing_sub_src=True,
             exclude_lint=exclude_lint,
             warn_lint=warn_lint)
@@ -178,40 +272,18 @@ def render(*, output_stream, site_name):
     engine.site.render(site_name, output_stream)
 
 
-@site.command('lint', help='Lint a site')
+@site.command('lint', help='Lint a given site in a repository')
-@click.option(
+@ALLOW_MISSING_SUBSTITUTIONS_OPTION
-    '-f',
+@EXCLUDE_LINT_OPTION
-    '--fail-on-missing-sub-src',
+@WARN_LINT_OPTION
-    'fail_on_missing_sub_src',
-    required=False,
-    type=click.BOOL,
-    default=True,
-    help='Fail when there is missing substitution source.')
-@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.')
 @click.argument('site_name')
-def lint(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name):
+def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name):
+    """Lint a given site using checks defined in
+    :mod:`pegleg.engine.errorcodes`.
+    """
     engine.repository.process_repositories(site_name)
-    _lint(
+    _lint_helper(
+        site_name=site_name,
         fail_on_missing_sub_src=fail_on_missing_sub_src,
         exclude_lint=exclude_lint,
         warn_lint=warn_lint)
-
-
-def _lint(*, fail_on_missing_sub_src, exclude_lint, warn_lint):
-    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:
-            click.echo(w)

pegleg/engine/lint.py

@@ -19,13 +19,13 @@ import pkg_resources
 import yaml
 
 from pegleg import config
-from pegleg.engine import util
 from pegleg.engine.errorcodes import DOCUMENT_LAYER_MISMATCH
 from pegleg.engine.errorcodes import FILE_CONTAINS_INVALID_YAML
 from pegleg.engine.errorcodes import FILE_MISSING_YAML_DOCUMENT_HEADER
 from pegleg.engine.errorcodes import REPOS_MISSING_DIRECTORIES_FLAG
 from pegleg.engine.errorcodes import SCHEMA_STORAGE_POLICY_MISMATCH_FLAG
 from pegleg.engine.errorcodes import SECRET_NOT_ENCRYPTED_POLICY
+from pegleg.engine import util
 
 __all__ = ['full']
 
@@ -39,6 +39,21 @@ DECKHAND_SCHEMAS = {
 
 
 def full(fail_on_missing_sub_src=False, exclude_lint=None, warn_lint=None):
+    """Lint all sites in a repository.
+
+    :param bool fail_on_missing_sub_src: Whether to allow Deckhand rendering
+        to fail in the absence of a missing substitution source document which
+        might be the case in "offline mode".
+    :param list exclude_lint: List of lint rules to exclude. See those
+        defined in :mod:`pegleg.engine.errorcodes`.
+    :param list warn_lint: List of lint rules to warn about. See those
+        defined in :mod:`pegleg.engine.errorcodes`.
+    :raises ClickException: If a lint check was caught and it isn't contained
+        in ``exclude_lint`` or ``warn_lint``.
+    :returns: List of warnings produced, if any.
+
+    """
+
     exclude_lint = exclude_lint or []
     warn_lint = warn_lint or []
     messages = []
@@ -55,11 +70,83 @@ def full(fail_on_missing_sub_src=False, exclude_lint=None, warn_lint=None):
     # after site definitions have been refactored to move the manifests
     # under fake repository folders into the common/ folders.
     #
-    # All repos contain expected directories
     # messages.extend(_verify_no_unexpected_files())
 
-    # Deckhand Rendering completes without error
+    # Deckhand rendering completes without error
-    messages.extend(_verify_deckhand_render(fail_on_missing_sub_src))
+    messages.extend(
+        _verify_deckhand_render(
+            fail_on_missing_sub_src=fail_on_missing_sub_src))
+
+    return _filter_messages_by_warn_and_error_lint(
+        messages=messages, exclude_lint=exclude_lint, warn_lint=warn_lint)
+
+
+def site(site_name,
+         fail_on_missing_sub_src=False,
+         exclude_lint=None,
+         warn_lint=None):
+    """Lint ``site_name``.
+
+    :param str site_name: Name of site to lint.
+    :param bool fail_on_missing_sub_src: Whether to allow Deckhand rendering
+        to fail in the absence of a missing substitution source document which
+        might be the case in "offline mode".
+    :param list exclude_lint: List of lint rules to exclude. See those
+        defined in :mod:`pegleg.engine.errorcodes`.
+    :param list warn_lint: List of lint rules to warn about. See those
+        defined in :mod:`pegleg.engine.errorcodes`.
+    :raises ClickException: If a lint check was caught and it isn't contained
+        in ``exclude_lint`` or ``warn_lint``.
+    :returns: List of warnings produced, if any.
+
+    """
+
+    exclude_lint = exclude_lint or []
+    warn_lint = warn_lint or []
+    messages = []
+
+    # FIXME(felipemonteiro): Now that we are using revisioned repositories
+    # instead of flat directories with subfolders mirroring "revisions",
+    # this lint check analyzes ALL the directories (including these
+    # no-longer-valid "revision directories") against the new subset of
+    # relevant directories. We need to rewrite this check so that it works
+    # after site definitions have been refactored to move the manifests
+    # under fake repository folders into the common/ folders.
+    #
+    # messages.extend(_verify_no_unexpected_files(sitenames=[site_name]))
+
+    # If policy is cleartext and error is added this will put
+    # that particular message into the warns list and all others will
+    # be added to the error list if SCHEMA_STORAGE_POLICY_MISMATCH_FLAG
+    messages.extend(_verify_file_contents(sitename=site_name))
+
+    # Deckhand rendering completes without error
+    messages.extend(
+        _verify_deckhand_render(
+            sitename=site_name,
+            fail_on_missing_sub_src=fail_on_missing_sub_src))
+
+    return _filter_messages_by_warn_and_error_lint(
+        messages=messages, exclude_lint=exclude_lint, warn_lint=warn_lint)
+
+
+def _filter_messages_by_warn_and_error_lint(*,
+                                            messages=None,
+                                            exclude_lint=None,
+                                            warn_lint=None):
+    """Helper that only filters messages depending on whether or not they
+    are present in ``exclude_lint`` or ``warn_lint``.
+
+    Bubbles up errors only if the corresponding code for each is **not** found
+    in either ``exclude_lint`` or ``warn_lint``. If the code is found in
+    ``exclude_lint``, the lint code is ignored; if the code is found in
+    ``warn_lint``, the lint is warned about.
+
+    """
+
+    messages = messages or []
+    exclude_lint = exclude_lint or []
+    warn_lint = warn_lint or []
 
     errors = []
     warns = []
@@ -75,9 +162,11 @@ def full(fail_on_missing_sub_src=False, exclude_lint=None, warn_lint=None):
     return warns
 
 
-def _verify_no_unexpected_files():
+def _verify_no_unexpected_files(*, sitenames=None):
+    sitenames = sitenames or util.files.list_sites()
+
     expected_directories = set()
-    for site_name in util.files.list_sites():
+    for site_name in sitenames:
         params = util.definition.load_as_params(site_name)
         expected_directories.update(
             util.files.directories_for(
@@ -100,11 +189,15 @@ def _verify_no_unexpected_files():
     return errors
 
 
-def _verify_file_contents():
+def _verify_file_contents(*, sitename=None):
+    if sitename:
+        files = util.definition.site_files(sitename)
+    else:
+        files = util.files.all()
     schemas = _load_schemas()
 
     errors = []
-    for filename in util.files.all():
+    for filename in files:
         errors.extend(_verify_single_file(filename, schemas))
     return errors
 
@@ -171,47 +264,36 @@ def _verify_document(document, schemas, filename):
     return errors
 
 
-def _gather_relevant_documents_per_site():
+def _verify_deckhand_render(*, sitename=None, fail_on_missing_sub_src=False):
-    """Gathers all relevant documents per site, which includes all type and
-    global documents that are needed to render each site document.
-
-    :returns: Dictionary of documents, keyed by each site name.
-    :rtype: dict
-    """
-    sitenames = list(util.files.list_sites())
-    documents_to_render = {s: [] for s in sitenames}
-
-    for sitename in sitenames:
-        params = util.definition.load_as_params(sitename)
-        paths = util.files.directories_for(
-            site_name=params['site_name'], site_type=params['site_type'])
-        filenames = set(util.files.search(paths))
-        for filename in filenames:
-            with open(filename) as f:
-                documents_to_render[sitename].extend(
-                    list(yaml.safe_load_all(f)))
-
-    return documents_to_render
-
-
-def _verify_deckhand_render(fail_on_missing_sub_src=False):
     """Verify Deckhand render works by using all relevant deployment files.
 
     :returns: List of errors generated during rendering.
     """
     all_errors = []
-    documents_to_render = _gather_relevant_documents_per_site()
 
-    for sitename, documents in documents_to_render.items():
+    if sitename:
+        documents_to_render = util.definition.documents_for_site(sitename)
         LOG.debug('Rendering documents for site: %s.', sitename)
         _, errors = util.deckhand.deckhand_render(
-            documents=documents,
+            documents=documents_to_render,
             fail_on_missing_sub_src=fail_on_missing_sub_src,
             validate=True,
         )
         LOG.debug('Generated %d rendering errors for site: %s.', len(errors),
                   sitename)
         all_errors.extend(errors)
+    else:
+        documents_to_render = util.definition.documents_for_each_site()
+        for site_name, documents in documents_to_render.items():
+            LOG.debug('Rendering documents for site: %s.', site_name)
+            _, errors = util.deckhand.deckhand_render(
+                documents=documents,
+                fail_on_missing_sub_src=fail_on_missing_sub_src,
+                validate=True,
+            )
+            LOG.debug('Generated %d rendering errors for site: %s.',
+                      len(errors), site_name)
+            all_errors.extend(errors)
 
     return list(set(all_errors))
 

pegleg/engine/util/definition.py

@@ -15,17 +15,14 @@
 import os
 
 import click
+import yaml
 
 from pegleg import config
-from . import files
+from pegleg.engine.util import files
 
 __all__ = [
-    'load',
+    'load', 'load_as_params', 'path', 'pluck', 'site_files',
-    'load_as_params',
+    'site_files_by_repo', 'documents_for_each_site', 'documents_for_site'
-    'path',
-    'pluck',
-    'site_files',
-    'site_files_by_repo',
 ]
 
 
@@ -80,3 +77,50 @@ def site_files_by_repo(site_name):
     for repo, dl in dir_map.items():
         for filename in files.search(dl):
             yield (repo, filename)
+
+
+def documents_for_each_site():
+    """Gathers all relevant documents per site, which includes all type and
+    global documents that are needed to render each site document.
+
+    :returns: Dictionary of documents, keyed by each site name.
+    :rtype: dict
+
+    """
+
+    sitenames = list(files.list_sites())
+    documents = {s: [] for s in sitenames}
+
+    for sitename in sitenames:
+        params = load_as_params(sitename)
+        paths = files.directories_for(
+            site_name=params['site_name'], site_type=params['site_type'])
+        filenames = set(files.search(paths))
+        for filename in filenames:
+            with open(filename) as f:
+                documents[sitename].extend(list(yaml.safe_load_all(f)))
+
+    return documents
+
+
+def documents_for_site(sitename):
+    """Gathers all relevant documents for a site, which includes all type and
+    global documents that are needed to render each site document.
+
+    :param str sitename: Site name for which to gather documents.
+    :returns: List of relevant documents.
+    :rtype: list
+
+    """
+
+    documents = []
+
+    params = load_as_params(sitename)
+    paths = files.directories_for(
+        site_name=params['site_name'], site_type=params['site_type'])
+    filenames = set(files.search(paths))
+    for filename in filenames:
+        with open(filename) as f:
+            documents.extend(list(yaml.safe_load_all(f)))
+
+    return documents

tests/unit/engine/test_selectable_linting.py

@@ -18,94 +18,115 @@ import pytest
 
 from pegleg import config
 from pegleg.engine import lint
+from tests.unit.fixtures import create_tmp_deployment_files
 
 _SKIP_P003_REASON = """Currently broken with revisioned repositories
-directory layout changes. The old pseudo-revision folders like 'v4.0' is
+directory layout changes. The old pseudo-revision folders like 'v4.0' are
 no longer relevant and so the lint logic for this rule needs to be updated.
 For more information, see: https://storyboard.openstack.org/#!/story/2003762
 """
 
 
-@mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
+class TestSelectableLinting(object):
-@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
+    @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
-def test_lint_excludes_P001(*args):
+    @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
-    exclude_lint = ['P001']
+    def test_lint_excludes_P001(*args):
-    config.set_site_repo('../pegleg/site_yamls/')
+        exclude_lint = ['P001']
+        config.set_site_repo('../pegleg/site_yamls/')
 
-    code_1 = 'X001'
+        code_1 = 'X001'
-    msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"'
+        msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"'
-    code_2 = 'X002'
+        code_2 = 'X002'
-    msg_2 = 'test msg'
+        msg_2 = 'test msg'
-    msgs = [(code_1, msg_1), (code_2, msg_2)]
+        msgs = [(code_1, msg_1), (code_2, msg_2)]
 
-    with mock.patch.object(
+        with mock.patch.object(
-            lint, '_verify_file_contents', return_value=msgs) as mock_methed:
+                lint, '_verify_file_contents',
-        with pytest.raises(click.ClickException) as expected_exc:
+                return_value=msgs) as mock_methed:
-            results = lint.full(False, exclude_lint, [])
+            with pytest.raises(click.ClickException) as expected_exc:
-            assert msg_1 in expected_exc
+                results = lint.full(False, exclude_lint, [])
-            assert msg_2 in 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_site_repo('../pegleg/site_yamls/')
+        with mock.patch.object(
+                lint,
+                '_verify_deckhand_render',
+                return_value=[('P002', 'test message')]) as mock_method:
+            lint.full(False, exclude_lint, [])
+        mock_method.assert_called()
 
-@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
+    @pytest.mark.skip(reason=_SKIP_P003_REASON)
-def test_lint_excludes_P002(*args):
+    @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
-    exclude_lint = ['P002']
+    def test_lint_excludes_P003(*args):
-    config.set_site_repo('../pegleg/site_yamls/')
+        exclude_lint = ['P003']
-    with mock.patch.object(
+        with mock.patch.object(
-            lint,
+                lint,
-            '_verify_deckhand_render',
+                '_verify_no_unexpected_files',
-            return_value=[('P002', 'test message')]) as mock_method:
+                return_value=[('P003', 'test message')]) as mock_method:
-        lint.full(False, exclude_lint, [])
+            lint.full(False, exclude_lint, [])
-    mock_method.assert_called()
+        mock_method.assert_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_site_repo('../pegleg/site_yamls/')
 
-@pytest.mark.skip(reason=_SKIP_P003_REASON)
+        code_1 = 'X001'
-@mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
+        msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"'
-def test_lint_excludes_P003(*args):
+        code_2 = 'X002'
-    exclude_lint = ['P003']
+        msg_2 = 'test msg'
-    with mock.patch.object(
+        msgs = [(code_1, msg_1), (code_2, msg_2)]
-            lint,
-            '_verify_no_unexpected_files',
-            return_value=[('P003', 'test message')]) as mock_method:
-        lint.full(False, exclude_lint, [])
-    mock_method.assert_called()
 
+        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)
+                assert msg_1 not in expected_exc
+                assert msg_2 in expected_exc
 
-@mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
+    @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
-@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
+    def test_lint_warns_P002(*args):
-def test_lint_warns_P001(*args):
+        warn_lint = ['P002']
-    warn_lint = ['P001']
+        config.set_site_repo('../pegleg/site_yamls/')
-    config.set_site_repo('../pegleg/site_yamls/')
 
-    code_1 = 'X001'
+        with mock.patch.object(lint, '_verify_deckhand_render') as mock_method:
-    msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"'
-    code_2 = 'X002'
-    msg_2 = 'test msg'
-    msgs = [(code_1, msg_1), (code_2, 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)
-            assert msg_1 not in expected_exc
+        mock_method.assert_called()
-            assert msg_2 in expected_exc
+
+    @pytest.mark.skip(reason=_SKIP_P003_REASON)
+    @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
+    def test_lint_warns_P003(*args):
+        warn_lint = ['P003']
+        config.set_site_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()
 
 
-@mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
+class TestSelectableLintingHelperFunctions(object):
-def test_lint_warns_P002(*args):
+    """The fixture ``create_tmp_deployment_files`` produces many linting errors
-    warn_lint = ['P002']
+    by default.
-    config.set_site_repo('../pegleg/site_yamls/')
 
-    with mock.patch.object(lint, '_verify_deckhand_render') as mock_method:
+    """
-        lint.full(False, [], warn_lint)
-    mock_method.assert_called()
 
+    def test_verify_file_contents(self, create_tmp_deployment_files):
+        """Validate that linting by a specific site ("cicd") produces a subset
+        of all the linting errors produced for all sites.
 
-@pytest.mark.skip(reason=_SKIP_P003_REASON)
+        """
-@mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
-def test_lint_warns_P003(*args):
-    warn_lint = ['P003']
-    config.set_site_repo('../pegleg/site_yamls/')
 
-    with mock.patch.object(lint, '_verify_no_unexpected_files') as mock_method:
+        all_lint_errors = lint._verify_file_contents()
-        lint.full(False, [], warn_lint)
+        assert all_lint_errors
-    mock_method.assert_called()
+
+        cicd_lint_errors = lint._verify_file_contents(sitename="cicd")
+        assert cicd_lint_errors
+
+        assert len(cicd_lint_errors) < len(all_lint_errors)

tests/unit/engine/util/test_definition.py

@@ -0,0 +1,66 @@
+# 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.
+
+from pegleg.engine.util import definition
+from tests.unit.fixtures import create_tmp_deployment_files
+
+
+class TestSiteDefinitionHelpers(object):
+    def _test_documents_for_site(self, sitename):
+        documents = definition.documents_for_site(sitename)
+        global_documents = []
+        site_documents = []
+
+        for document in documents:
+            name = document["metadata"]["name"]
+            # Assert that the document is either global level or a relevant
+            # site document.
+            assert name.startswith("global") or name.startswith(sitename)
+
+            if name.startswith("global"):
+                global_documents.append(document)
+            elif name.startswith(sitename):
+                site_documents.append(document)
+            else:
+                raise AssertionError("Unexpected document retrieved by "
+                                     "`documents_for_site`: %s" % document)
+
+        # Assert that documents from both levels appear.
+        assert global_documents
+        assert site_documents
+
+        return global_documents + site_documents
+
+    def test_documents_for_site(self, create_tmp_deployment_files):
+        self._test_documents_for_site("cicd")
+        self._test_documents_for_site("lab")
+
+    def test_documents_for_each_site(self, create_tmp_deployment_files):
+        documents_by_site = definition.documents_for_each_site()
+        sort_func = lambda x: x['metadata']['name']
+
+        # Validate that both expected site documents are found.
+        assert 2 == len(documents_by_site)
+        assert "cicd" in documents_by_site
+        assert "lab" in documents_by_site
+
+        cicd_documents = self._test_documents_for_site("cicd")
+        lab_documents = self._test_documents_for_site("lab")
+
+        # Validate that each set of site documents matches the same set of
+        # documents returned by ``documents_for_site`` for that site.
+        assert (sorted(cicd_documents, key=sort_func) == sorted(
+            documents_by_site["cicd"], key=sort_func))
+        assert (sorted(lab_documents, key=sort_func) == sorted(
+            documents_by_site["lab"], key=sort_func))

tests/unit/test_cli.py

@@ -18,9 +18,11 @@ import shutil
 import tempfile
 
 from click.testing import CliRunner
+import mock
 import pytest
 
 from pegleg import cli
+from pegleg.engine import errorcodes
 from pegleg.engine.util import git
 
 
@@ -38,7 +40,7 @@ def is_connected():
 
 @pytest.mark.skipif(
     not is_connected(), reason='git clone requires network connectivity.')
-class TestSiteCliActions(object):
+class BaseCLIActionTest(object):
     @classmethod
     def setup_class(cls):
         cls.runner = CliRunner()
@@ -59,6 +61,12 @@ class TestSiteCliActions(object):
             if git.is_repository(path):
                 shutil.rmtree(path, ignore_errors=True)
 
+
+class TestSiteCliActions(BaseCLIActionTest):
+    """Tests site-level CLI actions."""
+
+    ### Collect tests ###
+
     def test_collect_using_remote_repo_url(self):
         """Validates collect action using a remote URL."""
         # Scenario:
@@ -104,3 +112,135 @@ class TestSiteCliActions(object):
         # Validates that site manifests collected from cloned repositories
         # are written out to sensibly named files like airship-treasuremap.yaml
         assert collected_files[0] == ("%s.yaml" % self.repo_name)
+
+    ### Lint tests ###
+
+    def test_lint_site_using_remote_repo_url_with_exclude(self):
+        """Validates site lint action using remote repo URL."""
+        # Scenario:
+        #
+        # 1) Mock out Deckhand render (so we can ignore P005 issues)
+        # 2) Lint site with exclude flags (should clone repo automatically)
+
+        repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name,
+                                                           self.repo_rev)
+
+        lint_command = ['-r', repo_url, 'lint', self.site_name]
+        exclude_lint_command = [
+            '-x', errorcodes.SCHEMA_STORAGE_POLICY_MISMATCH_FLAG, '-x',
+            errorcodes.SECRET_NOT_ENCRYPTED_POLICY
+        ]
+
+        with mock.patch('pegleg.engine.site.util.deckhand') as mock_deckhand:
+            mock_deckhand.deckhand_render.return_value = ([], [])
+            result = self.runner.invoke(cli.site,
+                                        lint_command + exclude_lint_command)
+
+        assert result.exit_code == 0
+        # A successful result (while setting lint checks to exclude) should
+        # output nothing.
+        assert not result.output
+
+    def test_lint_site_using_local_path_with_exclude(self):
+        """Validates site lint action using local repo path."""
+        # Scenario:
+        #
+        # 1) Mock out Deckhand render (so we can ignore P005 issues)
+        # 2) Lint site with exclude flags (should skip clone repo)
+
+        repo_path = self.treasuremap_path
+        lint_command = ['-r', repo_path, 'lint', self.site_name]
+        exclude_lint_command = [
+            '-x', errorcodes.SCHEMA_STORAGE_POLICY_MISMATCH_FLAG, '-x',
+            errorcodes.SECRET_NOT_ENCRYPTED_POLICY
+        ]
+
+        with mock.patch('pegleg.engine.site.util.deckhand') as mock_deckhand:
+            mock_deckhand.deckhand_render.return_value = ([], [])
+            result = self.runner.invoke(cli.site,
+                                        lint_command + exclude_lint_command)
+
+        assert result.exit_code == 0
+        # A successful result (while setting lint checks to exclude) should
+        # output nothing.
+        assert not result.output
+
+    def test_lint_site_using_local_path_with_warn(self):
+        """Validates site lint action using local repo path."""
+        # Scenario:
+        #
+        # 1) Mock out Deckhand render (so we can ignore P005 issues)
+        # 2) Lint site with warn flags (should skip clone repo)
+
+        repo_path = self.treasuremap_path
+        lint_command = ['-r', repo_path, 'lint', self.site_name]
+        warn_lint_command = [
+            '-w', errorcodes.SCHEMA_STORAGE_POLICY_MISMATCH_FLAG, '-w',
+            errorcodes.SECRET_NOT_ENCRYPTED_POLICY
+        ]
+
+        with mock.patch('pegleg.engine.site.util.deckhand') as mock_deckhand:
+            mock_deckhand.deckhand_render.return_value = ([], [])
+            result = self.runner.invoke(cli.site,
+                                        lint_command + warn_lint_command)
+
+        assert result.exit_code == 0
+        # A successful result (while setting lint checks to warns) should
+        # output warnings.
+        assert result.output
+
+
+class TestRepoCliActions(BaseCLIActionTest):
+    """Tests repo-level CLI actions."""
+
+    ### Lint tests ###
+
+    def test_lint_repo_using_remote_repo_url_with_exclude(self):
+        """Validates repo lint action using remote repo URL."""
+        # Scenario:
+        #
+        # 1) Mock out Deckhand render (so we can ignore P005 issues)
+        # 2) Lint repo with exclude flags (should clone repo automatically)
+
+        repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name,
+                                                           self.repo_rev)
+
+        lint_command = ['-r', repo_url, 'lint']
+        exclude_lint_command = [
+            '-x', errorcodes.SCHEMA_STORAGE_POLICY_MISMATCH_FLAG, '-x',
+            errorcodes.SECRET_NOT_ENCRYPTED_POLICY
+        ]
+
+        with mock.patch('pegleg.engine.site.util.deckhand') as mock_deckhand:
+            mock_deckhand.deckhand_render.return_value = ([], [])
+            result = self.runner.invoke(cli.repo,
+                                        lint_command + exclude_lint_command)
+
+        assert result.exit_code == 0
+        # A successful result (while setting lint checks to exclude) should
+        # output nothing.
+        assert not result.output
+
+    def test_lint_repo_using_local_path_with_exclude(self):
+        """Validates repo lint action using local repo path."""
+        # Scenario:
+        #
+        # 1) Mock out Deckhand render (so we can ignore P005 issues)
+        # 2) Lint repo with exclude flags (should skip clone repo)
+
+        repo_path = self.treasuremap_path
+        lint_command = ['-r', repo_path, 'lint']
+        exclude_lint_command = [
+            '-x', errorcodes.SCHEMA_STORAGE_POLICY_MISMATCH_FLAG, '-x',
+            errorcodes.SECRET_NOT_ENCRYPTED_POLICY
+        ]
+
+        with mock.patch('pegleg.engine.site.util.deckhand') as mock_deckhand:
+            mock_deckhand.deckhand_render.return_value = ([], [])
+            result = self.runner.invoke(cli.repo,
+                                        lint_command + exclude_lint_command)
+
+        assert result.exit_code == 0
+        # A successful result (while setting lint checks to exclude) should
+        # output nothing.
+        assert not result.output

tools/gate/build-docs.sh

@@ -4,7 +4,11 @@
 # files. Must be run from root project directory.
 
 set -ex
-rm -rf doc/build
+
-sphinx-build -b html doc/source doc/build/html -W -n -v
+# Generate architectural images and move them into place.
 python -m plantuml doc/source/diagrams/*.uml
 mv doc/source/diagrams/*.png doc/source/images
+
+# Build documentation.
+rm -rf doc/build
+sphinx-build -b html doc/source doc/build/html -W -n -v