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
This commit is contained in:
Felipe Monteiro
Stacey Fletcher
parent
18316ff6c9
commit
e4a4d2ea68
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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'])
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
@ -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:
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue