diff --git a/armada/api/__init__.py b/armada/api/__init__.py index db6df8cb..895fb2bf 100644 --- a/armada/api/__init__.py +++ b/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: - 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)) + with excutils.save_and_reraise_exception(): + self.error( + req.context, + "Invalid YAML in request: \n%s" % raw_body.decode('utf-8')) def req_json(self, req): if req.content_length is None or req.content_length == 0: diff --git a/armada/api/controller/test.py b/armada/api/controller/test.py index c76b678c..91cc2c96 100644 --- a/armada/api/controller/test.py +++ b/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): + self.logger.info('RUNNING: %s', release) try: - self.logger.info('RUNNING: %s', release) tiller = Tiller( tiller_host=req.get_param('tiller_host'), tiller_port=req.get_param_as_int( @@ -43,45 +45,87 @@ class Test(api.BaseResource): tiller_namespace=req.get_param( 'tiller_namespace', default=CONF.tiller_namespace)) tiller_resp = tiller.testing_release(release) - msg = { - 'result': '', - 'message': '' - } - - if tiller_resp: - test_status = getattr( - tiller_resp.info.status, 'last_test_suite_run', 'FAILED') - - if test_status.result[0].status: - msg['result'] = 'PASSED: {}'.format(release) - msg['message'] = 'MESSAGE: Test Pass' - self.logger.info(msg) - else: - msg['result'] = 'FAILED: {}'.format(release) - msg['message'] = 'MESSAGE: Test Fail' - self.logger.info(msg) - else: - msg['result'] = 'FAILED: {}'.format(release) - msg['message'] = 'MESSAGE: No test found' - - resp.body = json.dumps(msg) - resp.status = falcon.HTTP_200 - resp.content_type = 'application/json' - + # 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) - self.return_error( + return self.return_error( resp, falcon.HTTP_500, message=err_message) + msg = { + 'result': '', + 'message': '' + } + + if tiller_resp: + test_status = getattr( + tiller_resp.info.status, 'last_test_suite_run', 'FAILED') + + if test_status.result[0].status: + msg['result'] = 'PASSED: {}'.format(release) + msg['message'] = 'MESSAGE: Test Pass' + self.logger.info(msg) + else: + msg['result'] = 'FAILED: {}'.format(release) + msg['message'] = 'MESSAGE: Test Fail' + self.logger.info(msg) + else: + msg['result'] = 'FAILED: {}'.format(release) + msg['message'] = 'MESSAGE: No test found' + + resp.body = json.dumps(msg) + resp.status = falcon.HTTP_200 + resp.content_type = 'application/json' + 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,57 +133,66 @@ class Tests(api.BaseResource): 'tiller_port') or CONF.tiller_port, tiller_namespace=req.get_param( 'tiller_namespace', default=CONF.tiller_namespace)) - - 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()] - - message = { - 'tests': { - 'passed': [], - 'skipped': [], - 'failed': [] - } - } - - for group in armada_obj.get(const.KEYWORD_ARMADA).get( - const.KEYWORD_GROUPS): - for ch in group.get(const.KEYWORD_CHARTS): - release_name = release_prefix( - prefix, ch.get('chart').get('chart_name')) - - if release_name in known_releases: - self.logger.info('RUNNING: %s tests', release_name) - resp = tiller.testing_release(release_name) - - if not resp: - continue - - test_status = getattr( - resp.info.status, 'last_test_suite_run', - 'FAILED') - if test_status.results[0].status: - self.logger.info("PASSED: %s", release_name) - message['test']['passed'].append(release_name) - else: - self.logger.info("FAILED: %s", release_name) - message['test']['failed'].append(release_name) - else: - self.logger.info( - 'Release %s not found - SKIPPING', release_name) - message['test']['skipped'].append(release_name) - - resp.status = falcon.HTTP_200 - - resp.body = json.dumps(message) - resp.content_type = 'application/json' - + # TODO(fmontei): Provide more sensible exception(s) here. except Exception as e: - err_message = 'Failed to test manifest: {}'.format(e) + err_message = 'Failed to initialize Tiller handler.' self.error(req.context, err_message) - self.return_error( + 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 + + 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()] + + message = { + 'tests': { + 'passed': [], + 'skipped': [], + 'failed': [] + } + } + + for group in armada_obj.get(const.KEYWORD_ARMADA).get( + const.KEYWORD_GROUPS): + for ch in group.get(const.KEYWORD_CHARTS): + release_name = release_prefix( + prefix, ch.get('chart').get('chart_name')) + + if release_name in known_releases: + self.logger.info('RUNNING: %s tests', release_name) + resp = tiller.testing_release(release_name) + + if not resp: + continue + + test_status = getattr( + resp.info.status, 'last_test_suite_run', + 'FAILED') + if test_status.results[0].status: + self.logger.info("PASSED: %s", release_name) + message['test']['passed'].append(release_name) + else: + self.logger.info("FAILED: %s", release_name) + message['test']['failed'].append(release_name) + else: + self.logger.info( + 'Release %s not found - SKIPPING', release_name) + message['test']['skipped'].append(release_name) + + resp.status = falcon.HTTP_200 + resp.body = json.dumps(message) + resp.content_type = 'application/json' diff --git a/armada/exceptions/base_exception.py b/armada/exceptions/base_exception.py index 67ba2310..d345dd32 100644 --- a/armada/exceptions/base_exception.py +++ b/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) diff --git a/armada/tests/unit/api/test_test_controller.py b/armada/tests/unit/api/test_test_controller.py index c28bcf60..38db8f30 100644 --- a/armada/tests/unit/api/test_test_controller.py +++ b/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']) diff --git a/docs/source/operations/exceptions/guide-exceptions.rst b/docs/source/operations/exceptions/guide-exceptions.rst index 5b58e631..a7083474 100644 --- a/docs/source/operations/exceptions/guide-exceptions.rst +++ b/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 diff --git a/docs/source/operations/exceptions/k8s-exceptions.rst b/docs/source/operations/exceptions/k8s-exceptions.inc similarity index 92% rename from docs/source/operations/exceptions/k8s-exceptions.rst rename to docs/source/operations/exceptions/k8s-exceptions.inc index d5a4ef8e..187c0428 100644 --- a/docs/source/operations/exceptions/k8s-exceptions.rst +++ b/docs/source/operations/exceptions/k8s-exceptions.inc @@ -26,8 +26,8 @@ :members: :show-inheritance: :undoc-members: - * - KubernetesUnknownStreamngEventTypeException - - .. autoexception:: armada.exceptions.k8s_exceptions.KubernetesUnknownStreamngEventTypeException + * - KubernetesUnknownStreamingEventTypeException + - .. autoexception:: armada.exceptions.k8s_exceptions.KubernetesUnknownStreamingEventTypeException :members: :show-inheritance: :undoc-members: diff --git a/docs/source/operations/exceptions/lint-exceptions.inc b/docs/source/operations/exceptions/validate-exceptions.inc similarity index 61% rename from docs/source/operations/exceptions/lint-exceptions.inc rename to docs/source/operations/exceptions/validate-exceptions.inc index 095ab254..f312b72e 100644 --- a/docs/source/operations/exceptions/lint-exceptions.inc +++ b/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: diff --git a/entrypoint.sh b/entrypoint.sh index c483bfa5..099085b6 100755 --- a/entrypoint.sh +++ b/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