feat(validation) Validation messaging

- Validation messaging to match UCP convention
- Adding some missing fields to Chart validation schema
- Minor update: Adding debug logging to each CLI call
- Fixing some typos and exception messages

Change-Id: I7dc1165432c8b3d138cabe6fd5f3a6e1878810ae

armada/cli/apply.py

@@ -130,9 +130,7 @@ def apply_create(ctx, locations, api, disable_update_post, disable_update_pre,
                  dry_run, enable_chart_cleanup, set, tiller_host, tiller_port,
                  tiller_namespace, timeout, values, wait, target_manifest,
                  debug):
-    if debug:
     CONF.debug = debug
-
     ApplyManifest(ctx, locations, api, disable_update_post, disable_update_pre,
                   dry_run, enable_chart_cleanup, set, tiller_host, tiller_port,
                   tiller_namespace, timeout, values, wait,

armada/cli/delete.py

@@ -15,6 +15,7 @@
 import yaml
 
 import click
+from oslo_config import cfg
 
 from armada.cli import CliAction
 from armada import const
@@ -22,6 +23,8 @@ from armada.handlers.manifest import Manifest
 from armada.handlers.tiller import Tiller
 from armada.utils.release import release_prefix
 
+CONF = cfg.CONF
+
 
 @click.group()
 def delete():
@@ -71,8 +74,13 @@ SHORT_DESC = "Command deletes releases."
               help="Tiller host port.",
               type=int,
               default=44134)
+@click.option('--debug',
+              help="Enable debug logging.",
+              is_flag=True)
 @click.pass_context
-def delete_charts(ctx, manifest, releases, no_purge, tiller_host, tiller_port):
+def delete_charts(ctx, manifest, releases, no_purge, tiller_host, tiller_port,
+                  debug):
+    CONF.debug = debug
     DeleteChartManifest(ctx, manifest, releases, no_purge, tiller_host,
                         tiller_port).safe_invoke()
 

armada/cli/test.py

@@ -77,9 +77,13 @@ SHORT_DESC = "Command tests releases."
               help=("The target manifest to run. Required for specifying "
                     "which manifest to run when multiple are available."),
               default=None)
+@click.option('--debug',
+              help="Enable debug logging.",
+              is_flag=True)
 @click.pass_context
 def test_charts(ctx, file, release, tiller_host, tiller_port, tiller_namespace,
-                target_manifest):
+                target_manifest, debug):
+    CONF.debug = debug
     TestChartManifest(
         ctx, file, release, tiller_host, tiller_port, tiller_namespace,
         target_manifest).safe_invoke()

armada/cli/tiller.py

@@ -67,9 +67,13 @@ SHORT_DESC = "Command gets Tiller information."
 @click.option('--status',
               help="Status of Armada services.",
               is_flag=True)
+@click.option('--debug',
+              help="Enable debug logging.",
+              is_flag=True)
 @click.pass_context
 def tiller_service(ctx, tiller_host, tiller_port, tiller_namespace, releases,
-                   status):
+                   status, debug):
+    CONF.debug = debug
     TillerServices(ctx, tiller_host, tiller_port, tiller_namespace, releases,
                    status).safe_invoke()
 

armada/cli/validate.py

@@ -14,11 +14,14 @@
 
 import click
 import yaml
+from oslo_config import cfg
 
 from armada.cli import CliAction
 from armada.utils.validate import validate_armada_documents
 from armada.handlers.document import ReferenceResolver
 
+CONF = cfg.CONF
+
 
 @click.group()
 def validate():
@@ -44,8 +47,12 @@ SHORT_DESC = "Command validates Armada Manifest."
                   short_help=SHORT_DESC)
 @click.argument('locations',
                 nargs=-1)
+@click.option('--debug',
+              help="Enable debug logging.",
+              is_flag=True)
 @click.pass_context
-def validate_manifest(ctx, locations):
+def validate_manifest(ctx, locations, debug):
+    CONF.debug = debug
     ValidateManifest(ctx, locations).safe_invoke()
 
 
@@ -65,7 +72,9 @@ class ValidateManifest(CliAction):
             try:
                 valid, details = validate_armada_documents(documents)
 
-                if valid:
+                if not documents:
+                    self.logger.warn('No documents to validate.')
+                elif valid:
                     self.logger.info('Successfully validated: %s',
                                      self.locations)
                 else:

armada/handlers/manifest.py

@@ -71,7 +71,7 @@ class Manifest(object):
 
     def _find_documents(self, target_manifest=None):
         """Returns the chart documents, chart group documents,
-        and armada manifest
+        and Armada manifest
 
         If multiple documents with schema "armada/Manifest/v1" are provided,
         specify ``target_manifest`` to select the target one.
@@ -203,10 +203,10 @@ class Manifest(object):
             return chart_group
 
     def build_armada_manifest(self):
-        """Builds the armmada manifest while pulling out data
+        """Builds the Armada manifest while pulling out data
         from the chart_group.
 
-        :returns: The armada manifest with the data of the chart groups.
+        :returns: The Armada manifest with the data of the chart groups.
         :rtype: dict
         :raises ManifestException: If a chart group's data listed
             under ``chart_group['data']`` could not be found.
@@ -234,9 +234,9 @@ class Manifest(object):
     def get_manifest(self):
         """Builds all of the documents including the dependencies of the
         chart documents, the charts in the chart_groups, and the
-        armada manifest
+        Armada manifest
 
-        :returns: The armada manifest.
+        :returns: The Armada manifest.
         :rtype: dict
         """
         self.build_charts_deps()

armada/schemas/armada-chart-schema.yaml

@@ -74,9 +74,11 @@ data:
         subpath:
           type: string
         reference:
-          type:
+          type: string
-            - string
+        proxy_server:
-            - "null"
+          type: string
+        auth_method:
+          type: string
       required:
         - location
         - subpath

armada/tests/unit/api/test_test_controller.py

@@ -138,7 +138,11 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest):
             {'message': (
                 'An error occurred while generating the manifest: Could not '
                 'find dependency chart helm-toolkit in armada/Chart/v1.'),
-             'error': True},
+             'error': True,
+             'kind': 'ValidationMessage',
+             'level': 'Error',
+             'name': 'ARM001',
+             'documents': []},
             resp_body['details']['messageList'])
         self.assertEqual(('Failed to validate documents or generate Armada '
                           'Manifest from documents.'),
@@ -168,7 +172,11 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest):
         self.assertEqual(
             [{'message': (
                 'An error occurred while generating the manifest: foo.'),
-              'error': True}],
+              'error': True,
+              'kind': 'ValidationMessage',
+              'level': 'Error',
+              'name': 'ARM001',
+              'documents': []}],
             resp_body['details']['messageList'])
         self.assertEqual(('Failed to validate documents or generate Armada '
                           'Manifest from documents.'),

armada/tests/unit/handlers/test_armada.py

@@ -54,7 +54,6 @@ data:
       type: local
       location: /tmp/dummy/armada
       subpath: chart_2
-      reference: null
     dependencies: []
     timeout: 5
 ---
@@ -117,7 +116,6 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
                                     'release': 'test_chart_2',
                                     'source': {
                                         'location': '/tmp/dummy/armada',
-                                        'reference': None,
                                         'subpath': 'chart_2',
                                         'type': 'local'
                                     },

armada/tests/unit/handlers/test_manifest.py

@@ -191,7 +191,7 @@ class ManifestTestCase(testtools.TestCase):
         self.assertIsNotNone(built_armada_manifest)
         self.assertIsInstance(built_armada_manifest, dict)
 
-        # the first chart group in the armada manifest
+        # the first chart group in the Armada manifest
         keystone_infra_services_chart_group = armada_manifest. \
             find_chart_group_document('keystone-infra-services')
         keystone_infra_services_chart_group_data = \
@@ -200,7 +200,7 @@ class ManifestTestCase(testtools.TestCase):
         self.assertEqual(keystone_infra_services_chart_group_data,
                          built_armada_manifest['data']['chart_groups'][0])
 
-        # the first chart group in the armada manifest
+        # the first chart group in the Armada manifest
         openstack_keystone_chart_group = armada_manifest. \
             find_chart_group_document('openstack-keystone')
         openstack_keystone_chart_group_data = \

armada/utils/source.py

@@ -163,7 +163,11 @@ def source_cleanup(git_path):
             LOG.warning('%s is not a valid git repository. Details: %s',
                         git_path, e)
         else:
+            try:
                 shutil.rmtree(git_path)
+            except OSError as e:
+                LOG.warning('Could not delete the path %s. Details: %s',
+                            git_path, e)
     else:
         LOG.warning('Could not delete the path %s. Is it a git repository?',
                     git_path)

armada/utils/validate.py

@@ -23,6 +23,7 @@ from oslo_log import log as logging
 from armada.const import KEYWORD_GROUPS, KEYWORD_CHARTS, KEYWORD_RELEASE
 from armada.handlers.manifest import Manifest
 from armada.exceptions.manifest_exceptions import ManifestException
+from armada.utils.validation_message import ValidationMessage
 
 LOG = logging.getLogger(__name__)
 # Creates a mapping between ``metadata.name``: ``data`` where the
@@ -65,14 +66,18 @@ def _validate_armada_manifest(manifest):
         the second value is the validation details with a minimum
         keyset of (message(str), error(bool))
     :rtype: tuple.
-
     """
     details = []
 
     try:
         armada_object = manifest.get_manifest().get('armada')
     except ManifestException as me:
-        details.append(dict(message=str(me), error=True))
+        vmsg = ValidationMessage(message=str(me),
+                                 error=True,
+                                 name='ARM001',
+                                 level='Error')
+        LOG.error('ValidationMessage: %s', vmsg.get_output_json())
+        details.append(vmsg.get_output())
         return False, details
 
     groups = armada_object.get(KEYWORD_GROUPS)
@@ -80,7 +85,12 @@ def _validate_armada_manifest(manifest):
     if not isinstance(groups, list):
         message = '{} entry is of wrong type: {} (expected: {})'.format(
             KEYWORD_GROUPS, type(groups), 'list')
-        details.append(dict(message=message, error=True))
+        vmsg = ValidationMessage(message=message,
+                                 error=True,
+                                 name='ARM101',
+                                 level='Error')
+        LOG.info('ValidationMessage: %s', vmsg.get_output_json())
+        details.append(vmsg.get_output())
 
     for group in groups:
         for chart in group.get(KEYWORD_CHARTS):
@@ -88,7 +98,12 @@ def _validate_armada_manifest(manifest):
             if KEYWORD_RELEASE not in chart_obj:
                 message = 'Could not find {} keyword in {}'.format(
                     KEYWORD_RELEASE, chart_obj.get('release'))
-                details.append(dict(message=message, error=True))
+                vmsg = ValidationMessage(message=message,
+                                         error=True,
+                                         name='ARM102',
+                                         level='Error')
+                LOG.info('ValidationMessage: %s', vmsg.get_output_json())
+                details.append(vmsg.get_output())
 
     if len([x for x in details if x.get('error', False)]) > 0:
         return False, details
@@ -108,8 +123,8 @@ def validate_armada_manifests(documents):
     for document in documents:
         if document.get('schema', '') == 'armada/Manifest/v1':
             target = document.get('metadata').get('name')
-            manifest = Manifest(documents,
+            # TODO(MarshM) explore: why does this pass 'documents'?
-                                target_manifest=target)
+            manifest = Manifest(documents, target_manifest=target)
             is_valid, details = _validate_armada_manifest(manifest)
             all_valid = all_valid and is_valid
             messages.extend(details)
@@ -138,28 +153,44 @@ def validate_armada_document(document):
     schema = document.get('schema', '<missing>')
     document_name = document.get('metadata', {}).get('name', None)
     details = []
+    LOG.debug('Validating document [%s] %s', schema, document_name)
 
     if schema in SCHEMAS:
         try:
             validator = jsonschema.Draft4Validator(SCHEMAS[schema])
             for error in validator.iter_errors(document.get('data')):
-                msg = "Invalid document [%s] %s: %s." % \
+                error_message = "Invalid document [%s] %s: %s." % \
                     (schema, document_name, error.message)
-                details.append(dict(message=msg,
+                vmsg = ValidationMessage(message=error_message,
                                          error=True,
-                                    doc_schema=schema,
+                                         name='ARM100',
-                                    doc_name=document_name))
+                                         level='Error',
+                                         schema=schema,
+                                         doc_name=document_name)
+                LOG.info('ValidationMessage: %s', vmsg.get_output_json())
+                details.append(vmsg.get_output())
         except jsonschema.SchemaError as e:
             error_message = ('The built-in Armada JSON schema %s is invalid. '
                              'Details: %s.' % (e.schema, e.message))
-            LOG.error(error_message)
+            vmsg = ValidationMessage(message=error_message,
-            details.append(dict(message=error_message, error=True))
+                                     error=True,
+                                     name='ARM000',
+                                     level='Error',
+                                     diagnostic='Armada is misconfigured.')
+            LOG.error('ValidationMessage: %s', vmsg.get_output_json())
+            details.append(vmsg.get_output())
     else:
-        error_message = (
+        vmsg = ValidationMessage(message='Unsupported document type.',
-            'Document [%s] %s is not supported.' %
+                                 error=False,
-            (schema, document_name))
+                                 name='ARM002',
-        LOG.info(error_message)
+                                 level='Warning',
-        details.append(dict(message=error_message, error=False))
+                                 schema=schema,
+                                 doc_name=document_name,
+                                 diagnostic='Please ensure document is one of '
+                                            'the following schema types: %s' %
+                                            list(SCHEMAS.keys()))
+        LOG.info('ValidationMessage: %s', vmsg.get_output_json())
+        # Validation API doesn't care about this type of message, don't send
 
     if len([x for x in details if x.get('error', False)]) > 0:
         return False, details
@@ -202,6 +233,7 @@ def validate_manifest_url(value):
         return False
 
 
+# TODO(MarshM) unused except in unit tests, is this useful?
 def validate_manifest_filepath(value):
     return os.path.isfile(value)
 

armada/utils/validation_message.py

@@ -0,0 +1,73 @@
+# 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 json
+
+
+class ValidationMessage(object):
+    """ ValidationMessage per UCP convention:
+    https://github.com/att-comdev/ucp-integration/blob/master/docs/source/api-conventions.rst#output-structure  # noqa
+
+    Construction of ValidationMessage message:
+
+    :param string message: Validation failure message.
+    :param boolean error: True or False, if this is an error message.
+    :param string name: Identifying name of the validation.
+    :param string level: The severity of validation result, as "Error",
+        "Warning", or "Info"
+    :param string schema: The schema of the document being validated.
+    :param string doc_name: The name of the document being validated.
+    :param string diagnostic: Information about what lead to the message,
+        or details for resolution.
+    """
+
+    def __init__(self,
+                 message='Document validation error.',
+                 error=True,
+                 name='Armada error',
+                 level='Error',
+                 schema=None,
+                 doc_name=None,
+                 diagnostic=None):
+
+        # TODO(MarshM) should validate error and level inputs
+
+        self.output = {
+            'message': message,
+            'error': error,
+            'name': name,
+            'documents': [],
+            'level': level,
+            'kind': 'ValidationMessage'
+        }
+        if schema and doc_name:
+            self.output['documents'].append(dict(schema=schema, name=doc_name))
+        if diagnostic:
+            self.output.update(diagnostic=diagnostic)
+
+    def get_output(self):
+        """ Return ValidationMessage message.
+
+        :returns: The ValidationMessage for the Validation API response.
+        :rtype: dict
+        """
+        return self.output
+
+    def get_output_json(self):
+        """ Return ValidationMessage message as JSON.
+
+        :returns: The ValidationMessage formatted in JSON, for logging.
+        :rtype: json
+        """
+        return json.dumps(self.output, indent=2)