Add validation logic to Test endpoints

This PS adds validation logic recently implemented in
armada.utils.validate [0] for validating documents and
Armada-generated Manifests to the Test and Tests controller classes.

Also refactors some exception handling for both controller classes to
better bubble up the appropriate exception.

Finally unit tests have been added for the Armada Test controller
to verify above changes work.

[0] https://review.gerrithub.io/#/c/378700/

Change-Id: I01f73c1778bf7c2e38032d5fddabd327c013edbb

armada/api/__init__.py

@@ -17,6 +17,8 @@ import logging as log
 import uuid
 import yaml
 
+from oslo_utils import excutils
+
 import falcon
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -41,23 +43,22 @@ class BaseResource(object):
         resp.headers['Allow'] = ','.join(allowed_methods)
         resp.status = falcon.HTTP_200
 
-    def req_yaml(self, req):
+    def req_yaml(self, req, default=None):
         if req.content_length is None or req.content_length == 0:
-            return None
+            return default
 
         raw_body = req.stream.read(req.content_length or 0)
 
         if raw_body is None:
-            return None
+            return default
 
         try:
-            return yaml.safe_load_all(raw_body.decode('utf-8'))
+            return list(yaml.safe_load_all(raw_body.decode('utf-8')))
         except yaml.YAMLError as jex:
+            with excutils.save_and_reraise_exception():
                 self.error(
                     req.context,
                     "Invalid YAML in request: \n%s" % raw_body.decode('utf-8'))
-            raise Exception(
-                "%s: Invalid YAML in body: %s" % (req.path, jex))
 
     def req_json(self, req):
         if req.content_length is None or req.content_length == 0:

armada/api/controller/test.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import json
+import yaml
 
 import falcon
 from oslo_config import cfg
@@ -23,19 +24,20 @@ from armada import const
 from armada.handlers.tiller import Tiller
 from armada.handlers.manifest import Manifest
 from armada.utils.release import release_prefix
+from armada.utils import validate
 
 CONF = cfg.CONF
 
 
 class Test(api.BaseResource):
     '''
-    Test helm releases via release name
+    Test Helm releases via release name.
     '''
 
     @policy.enforce('armada:test_release')
     def on_get(self, req, resp, release):
-        try:
         self.logger.info('RUNNING: %s', release)
+        try:
             tiller = Tiller(
                 tiller_host=req.get_param('tiller_host'),
                 tiller_port=req.get_param_as_int(
@@ -43,6 +45,13 @@ class Test(api.BaseResource):
                 tiller_namespace=req.get_param(
                     'tiller_namespace', default=CONF.tiller_namespace))
             tiller_resp = tiller.testing_release(release)
+        # TODO(fmontei): Provide more sensible exception(s) here.
+        except Exception as e:
+            err_message = 'Failed to test {}: {}'.format(release, e)
+            self.error(req.context, err_message)
+            return self.return_error(
+                resp, falcon.HTTP_500, message=err_message)
+
         msg = {
             'result': '',
             'message': ''
@@ -68,20 +77,55 @@ class Test(api.BaseResource):
         resp.status = falcon.HTTP_200
         resp.content_type = 'application/json'
 
-        except Exception as e:
-            err_message = 'Failed to test {}: {}'.format(release, e)
-            self.error(req.context, err_message)
-            self.return_error(
-                resp, falcon.HTTP_500, message=err_message)
-
 
 class Tests(api.BaseResource):
     '''
-    Test helm releases via a manifest
+    Test Helm releases via a Manifest.
     '''
 
+    def _format_validation_response(self, req, resp, result, details):
+        resp.content_type = 'application/json'
+        resp_body = {
+            'kind': 'Status',
+            'apiVersion': 'v1.0',
+            'metadata': {},
+            'reason': 'Validation',
+            'details': {},
+        }
+
+        error_details = [m for m in details if m.get('error', False)]
+
+        resp_body['details']['errorCount'] = len(error_details)
+        resp_body['details']['messageList'] = details
+
+        if result:
+            resp.status = falcon.HTTP_200
+            resp_body['status'] = 'Success'
+            resp_body['message'] = 'Armada validations succeeded.'
+            resp_body['code'] = 200
+        else:
+            resp.status = falcon.HTTP_400
+            resp_body['status'] = 'Failure'
+            resp_body['message'] = (
+                'Failed to validate documents or generate Armada Manifest '
+                'from documents.')
+            resp_body['code'] = 400
+            self.error(req.context, resp_body['message'])
+
+        resp.body = json.dumps(resp_body)
+        return result
+
+    def _validate_documents(self, req, resp, documents):
+        result, details = validate.validate_armada_documents(documents)
+        return self._format_validation_response(req, resp, result,
+                                                details)
+
     @policy.enforce('armada:tests_manifest')
     def on_post(self, req, resp):
+        # TODO(fmontei): Validation Content-Type is application/x-yaml.
+
+        target_manifest = req.get_param('target_manifest', None)
+
         try:
             tiller = Tiller(
                 tiller_host=req.get_param('tiller_host'),
@@ -89,11 +133,27 @@ class Tests(api.BaseResource):
                     'tiller_port') or CONF.tiller_port,
                 tiller_namespace=req.get_param(
                     'tiller_namespace', default=CONF.tiller_namespace))
+        # TODO(fmontei): Provide more sensible exception(s) here.
+        except Exception as e:
+            err_message = 'Failed to initialize Tiller handler.'
+            self.error(req.context, err_message)
+            return self.return_error(
+                resp, falcon.HTTP_500, message=err_message)
+
+        try:
+            documents = self.req_yaml(req, default=[])
+        except yaml.YAMLError:
+            err_message = 'Documents must be valid YAML.'
+            return self.return_error(
+                resp, falcon.HTTP_400, message=err_message)
+
+        is_valid = self._validate_documents(req, resp, documents)
+        if not is_valid:
+            return resp
 
-            documents = self.req_yaml(req)
-            target_manifest = req.get_param('target_manifest', None)
         armada_obj = Manifest(
             documents, target_manifest=target_manifest).get_manifest()
+
         prefix = armada_obj.get(const.KEYWORD_ARMADA).get(
             const.KEYWORD_PREFIX)
         known_releases = [release[0] for release in tiller.list_charts()]
@@ -134,12 +194,5 @@ class Tests(api.BaseResource):
                     message['test']['skipped'].append(release_name)
 
         resp.status = falcon.HTTP_200
-
         resp.body = json.dumps(message)
         resp.content_type = 'application/json'
-
-        except Exception as e:
-            err_message = 'Failed to test manifest: {}'.format(e)
-            self.error(req.context, err_message)
-            self.return_error(
-                resp, falcon.HTTP_500, message=err_message)

armada/exceptions/base_exception.py

@@ -29,9 +29,9 @@ class ArmadaBaseException(Exception):
 
     def __init__(self, message=None, **kwargs):
         self.message = message or self.message
-        try:
+        try:  # nosec
             self.message = self.message % kwargs
-        except TypeError:
+        except Exception:
             pass
         super(ArmadaBaseException, self).__init__(self.message)
 

armada/tests/unit/api/test_test_controller.py

@@ -12,11 +12,124 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import json
+import os
+import yaml
+
+import mock
+
+from armada.api.controller import test
 from armada.common.policies import base as policy_base
+from armada.exceptions import manifest_exceptions
 from armada.tests import test_utils
 from armada.tests.unit.api import base
 
 
+class TestControllerTest(base.BaseControllerTest):
+
+    @mock.patch.object(test, 'Manifest')
+    @mock.patch.object(test, 'Tiller')
+    def test_test_controller_with_manifest(self, mock_tiller, mock_manifest):
+        rules = {'armada:tests_manifest': '@'}
+        self.policy.set_rules(rules)
+
+        manifest_path = os.path.join(os.getcwd(), 'examples',
+                                     'keystone-manifest.yaml')
+        with open(manifest_path, 'r') as f:
+            payload = f.read()
+        documents = list(yaml.safe_load_all(payload))
+
+        resp = self.app.simulate_post('/api/v1.0/tests', body=payload)
+        self.assertEqual(200, resp.status_code)
+
+        result = json.loads(resp.text)
+        expected = {
+            "tests": {"passed": [], "skipped": [], "failed": []}
+        }
+        self.assertEqual(expected, result)
+
+        mock_manifest.assert_called_once_with(
+            documents, target_manifest=None)
+        self.assertTrue(mock_tiller.called)
+
+
+@test_utils.attr(type=['negative'])
+class TestControllerNegativeTest(base.BaseControllerTest):
+
+    @mock.patch.object(test, 'Manifest')
+    @mock.patch.object(test, 'Tiller')
+    def test_test_controller_tiller_exc_returns_500(self, mock_tiller, _):
+        rules = {'armada:tests_manifest': '@'}
+        self.policy.set_rules(rules)
+
+        mock_tiller.side_effect = Exception
+
+        resp = self.app.simulate_post('/api/v1.0/tests')
+        self.assertEqual(500, resp.status_code)
+
+    @mock.patch.object(test, 'Manifest')
+    @mock.patch.object(test, 'Tiller')
+    def test_test_controller_validation_failure_returns_400(
+            self, *_):
+        rules = {'armada:tests_manifest': '@'}
+        self.policy.set_rules(rules)
+
+        manifest_path = os.path.join(os.getcwd(), 'examples',
+                                     'keystone-manifest.yaml')
+        with open(manifest_path, 'r') as f:
+            payload = f.read()
+
+        documents = list(yaml.safe_load_all(payload))
+        documents[0]['schema'] = 'totally-invalid'
+        invalid_payload = yaml.safe_dump_all(documents)
+
+        resp = self.app.simulate_post('/api/v1.0/tests', body=invalid_payload)
+        self.assertEqual(400, resp.status_code)
+
+        resp_body = json.loads(resp.text)
+        self.assertEqual(400, resp_body['code'])
+        self.assertEqual(1, resp_body['details']['errorCount'])
+        self.assertIn(
+            {'message': (
+                'An error occurred while generating the manifest: Could not '
+                'find dependency chart helm-toolkit in armada/Chart/v1.'),
+             'error': True},
+            resp_body['details']['messageList'])
+        self.assertEqual(('Failed to validate documents or generate Armada '
+                          'Manifest from documents.'),
+                         resp_body['message'])
+
+    @mock.patch('armada.utils.validate.Manifest')
+    @mock.patch.object(test, 'Tiller')
+    def test_test_controller_manifest_failure_returns_400(
+            self, _, mock_manifest):
+        rules = {'armada:tests_manifest': '@'}
+        self.policy.set_rules(rules)
+
+        mock_manifest.return_value.get_manifest.side_effect = (
+            manifest_exceptions.ManifestException(details='foo'))
+
+        manifest_path = os.path.join(os.getcwd(), 'examples',
+                                     'keystone-manifest.yaml')
+        with open(manifest_path, 'r') as f:
+            payload = f.read()
+
+        resp = self.app.simulate_post('/api/v1.0/tests', body=payload)
+        self.assertEqual(400, resp.status_code)
+
+        resp_body = json.loads(resp.text)
+        self.assertEqual(400, resp_body['code'])
+        self.assertEqual(1, resp_body['details']['errorCount'])
+        self.assertEqual(
+            [{'message': (
+                'An error occurred while generating the manifest: foo.'),
+              'error': True}],
+            resp_body['details']['messageList'])
+        self.assertEqual(('Failed to validate documents or generate Armada '
+                          'Manifest from documents.'),
+                         resp_body['message'])
+
+
 class TestControllerNegativeRbacTest(base.BaseControllerTest):
 
     @test_utils.attr(type=['negative'])

docs/source/operations/exceptions/guide-exceptions.rst

@@ -22,8 +22,8 @@ Armada Exceptions
 .. include:: base-exceptions.inc
 .. include:: chartbuilder-exceptions.inc
 .. include:: k8s-exceptions.inc
-.. include:: lint-exceptions.inc
 .. include:: manifest-exceptions.inc
 .. include:: override-exceptions.inc
 .. include:: source-exceptions.inc
 .. include:: tiller-exceptions.inc
+.. include:: validate-exceptions.inc

docs/source/operations/exceptions/k8s-exceptions.inc

@@ -26,8 +26,8 @@
          :members:
          :show-inheritance:
          :undoc-members:
-  * - KubernetesUnknownStreamngEventTypeException
+  * - KubernetesUnknownStreamingEventTypeException
-    - .. autoexception:: armada.exceptions.k8s_exceptions.KubernetesUnknownStreamngEventTypeException
+    - .. autoexception:: armada.exceptions.k8s_exceptions.KubernetesUnknownStreamingEventTypeException
          :members:
          :show-inheritance:
          :undoc-members:

docs/source/operations/exceptions/validate-exceptions.inc

@@ -20,13 +20,23 @@
 
   * - Exception Name
     - Description
-  * - InvalidArmadaObjectException
-    - .. autoexception:: armada.exceptions.lint_exceptions.InvalidArmadaObjectException
-         :members:
-         :show-inheritance:
-         :undoc-members:
   * - InvalidManifestException
-    - .. autoexception:: armada.exceptions.lint_exceptions.InvalidManifestException
+    - .. autoexception:: armada.exceptions.validate_exceptions.InvalidManifestException
+         :members:
+         :show-inheritance:
+         :undoc-members:
+  * - InvalidChartDefinitionException
+    - .. autoexception:: armada.exceptions.validate_exceptions.InvalidChartDefinitionException
+         :members:
+         :show-inheritance:
+         :undoc-members:
+  * - InvalidReleaseException
+    - .. autoexception:: armada.exceptions.validate_exceptions.InvalidReleaseException
+         :members:
+         :show-inheritance:
+         :undoc-members:
+  * - InvalidArmadaObjectException
+    - .. autoexception:: armada.exceptions.validate_exceptions.InvalidArmadaObjectException
          :members:
          :show-inheritance:
          :undoc-members:

entrypoint.sh

@@ -19,7 +19,7 @@ set -ex
 CMD="armada"
 
 # Define port
-PORT=${ARMADA_API_PORT:-9000}
+PORT=${ARMADA_API_PORT:-8000}
 # How long uWSGI should wait for each Armada response
 ARMADA_API_TIMEOUT=${ARMADA_API_TIMEOUT:-"3600"}
 # Number of uWSGI workers to handle API requests