Validation refactor
This PS refines the logic in override.update_manifests when validating documents to properly deduce the correct exception that needs to be thrown. Added appropriate logic in armada.py to handle the exceptions thrwon by override.update_manifests. Also validate_armada_documents now logs appropriate error/debug messages. Introduced ArmadaNegativeHandlerTestCase class in test_armada.py, along with updating/adding unit tests in test_override.py. Change-Id: I84051ae4901011093f987479861df5f89561bb2c
This commit is contained in:
parent
e81f1a03f6
commit
fbbfdef4ed
|
@ -22,6 +22,7 @@ from oslo_log import log as logging
|
|||
|
||||
from armada import const
|
||||
from armada.exceptions import armada_exceptions
|
||||
from armada.exceptions import override_exceptions
|
||||
from armada.exceptions import source_exceptions
|
||||
from armada.exceptions import tiller_exceptions
|
||||
from armada.exceptions import validate_exceptions
|
||||
|
@ -32,7 +33,6 @@ from armada.handlers.test import test_release_for_success
|
|||
from armada.handlers.tiller import Tiller
|
||||
from armada.utils.release import release_prefixer
|
||||
from armada.utils import source
|
||||
from armada.utils import validate
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
CONF = cfg.CONF
|
||||
|
@ -101,8 +101,13 @@ class Armada(object):
|
|||
tiller_port=tiller_port,
|
||||
tiller_namespace=tiller_namespace,
|
||||
dry_run=dry_run)
|
||||
self.documents = Override(
|
||||
documents, overrides=set_ovr, values=values).update_manifests()
|
||||
try:
|
||||
self.documents = Override(
|
||||
documents, overrides=set_ovr,
|
||||
values=values).update_manifests()
|
||||
except (validate_exceptions.InvalidManifestException,
|
||||
override_exceptions.InvalidOverrideValueException):
|
||||
raise
|
||||
self.k8s_wait_attempts = k8s_wait_attempts
|
||||
self.k8s_wait_attempt_sleep = k8s_wait_attempt_sleep
|
||||
self.manifest = Manifest(
|
||||
|
@ -127,18 +132,6 @@ class Armada(object):
|
|||
if not self.tiller.tiller_status():
|
||||
raise tiller_exceptions.TillerServicesUnavailableException()
|
||||
|
||||
valid, details = validate.validate_armada_documents(self.documents)
|
||||
|
||||
if details:
|
||||
for msg in details:
|
||||
if msg.get('error', False):
|
||||
LOG.error(msg.get('message', 'Unknown validation error.'))
|
||||
else:
|
||||
LOG.debug(msg.get('message', 'Validation succeeded.'))
|
||||
if not valid:
|
||||
raise validate_exceptions.InvalidManifestException(
|
||||
error_messages=details)
|
||||
|
||||
# Clone the chart sources
|
||||
repos = {}
|
||||
manifest_data = self.manifest.get(const.KEYWORD_ARMADA, {})
|
||||
|
|
|
@ -18,6 +18,7 @@ import yaml
|
|||
|
||||
from armada import const
|
||||
from armada.exceptions import override_exceptions
|
||||
from armada.exceptions import validate_exceptions
|
||||
from armada.utils import validate
|
||||
|
||||
|
||||
|
@ -38,6 +39,19 @@ class Override(object):
|
|||
except IOError:
|
||||
raise override_exceptions.InvalidOverrideFileException(doc)
|
||||
|
||||
def _document_checker(self, doc, ovr=None):
|
||||
# Validate document or raise the appropriate exception
|
||||
try:
|
||||
valid, details = validate.validate_armada_documents(doc)
|
||||
except (RuntimeError, TypeError):
|
||||
raise override_exceptions.InvalidOverrideValueException(ovr)
|
||||
if not valid:
|
||||
if ovr:
|
||||
raise override_exceptions.InvalidOverrideValueException(ovr)
|
||||
else:
|
||||
raise validate_exceptions.InvalidManifestException(
|
||||
error_messages=details)
|
||||
|
||||
def update(self, d, u):
|
||||
for k, v in u.items():
|
||||
if isinstance(v, collections.Mapping):
|
||||
|
@ -146,6 +160,9 @@ class Override(object):
|
|||
for value in self.values:
|
||||
merging_values = self._load_yaml_file(value)
|
||||
self.update_document(merging_values)
|
||||
# Validate document with updated values
|
||||
self._document_checker(self.documents, self.values)
|
||||
|
||||
if self.overrides:
|
||||
for override in self.overrides:
|
||||
new_value = override.split('=')[1]
|
||||
|
@ -153,11 +170,11 @@ class Override(object):
|
|||
data_path = doc_path.pop().split('.')
|
||||
|
||||
self.override_manifest_value(doc_path, data_path, new_value)
|
||||
# Validate document with overrides
|
||||
self._document_checker(self.documents, self.overrides)
|
||||
|
||||
try:
|
||||
validate.validate_armada_documents(self.documents)
|
||||
except Exception:
|
||||
raise override_exceptions.InvalidOverrideValueException(
|
||||
self.overrides)
|
||||
if not (self.values and self.overrides):
|
||||
# Valiate document
|
||||
self._document_checker(self.documents)
|
||||
|
||||
return self.documents
|
||||
|
|
|
@ -20,6 +20,9 @@ from armada.handlers import armada
|
|||
from armada.tests.unit import base
|
||||
from armada.tests.test_utils import AttrDict
|
||||
from armada.utils.release import release_prefixer
|
||||
from armada.exceptions import ManifestException
|
||||
from armada.exceptions.override_exceptions import InvalidOverrideValueException
|
||||
from armada.exceptions.validate_exceptions import InvalidManifestException
|
||||
from armada.exceptions import tiller_exceptions
|
||||
from armada.exceptions.armada_exceptions import ProtectedReleaseException
|
||||
|
||||
|
@ -615,3 +618,43 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
|||
wait=True)
|
||||
]
|
||||
mock_tiller.return_value.install_release.assert_has_calls(method_calls)
|
||||
|
||||
|
||||
class ArmadaNegativeHandlerTestCase(base.ArmadaTestCase):
|
||||
|
||||
@mock.patch.object(armada, 'source')
|
||||
@mock.patch('armada.handlers.armada.Tiller')
|
||||
def test_armada_get_manifest_exception(self, mock_tiller, mock_source):
|
||||
"""Test armada handling with invalid manifest."""
|
||||
yaml_documents = list(yaml.safe_load_all(TEST_YAML))
|
||||
error_re = ('Documents must be a list of documents with at least one '
|
||||
'of each of the following schemas: .*')
|
||||
self.assertRaisesRegexp(ManifestException, error_re, armada.Armada,
|
||||
yaml_documents[:1])
|
||||
|
||||
@mock.patch.object(armada, 'source')
|
||||
@mock.patch('armada.handlers.armada.Tiller')
|
||||
def test_armada_override_exception(self, mock_tiller, mock_source):
|
||||
"""Test Armada checks with invalid chart override."""
|
||||
yaml_documents = list(yaml.safe_load_all(TEST_YAML))
|
||||
override = ('chart:example-chart-2:name=' 'overridden', )
|
||||
|
||||
error_re = ('is not a valid override statement')
|
||||
with self.assertRaisesRegexp(InvalidOverrideValueException, error_re):
|
||||
armada.Armada(yaml_documents, set_ovr=override)
|
||||
|
||||
@mock.patch.object(armada, 'source')
|
||||
@mock.patch('armada.handlers.armada.Tiller')
|
||||
def test_armada_manifest_exception_override_none(self, mock_tiller,
|
||||
mock_source):
|
||||
"""Test Armada checks with invalid manifest."""
|
||||
yaml_documents = list(yaml.safe_load_all(TEST_YAML))
|
||||
example_document = [
|
||||
d for d in yaml_documents
|
||||
if d['metadata']['name'] == 'example-chart-4'
|
||||
][0]
|
||||
del example_document['data']['release']
|
||||
|
||||
error_re = ('Invalid document .*')
|
||||
with self.assertRaisesRegexp(InvalidManifestException, error_re):
|
||||
armada.Armada(yaml_documents, set_ovr=None)
|
||||
|
|
|
@ -66,7 +66,6 @@ class OverrideTestCase(testtools.TestCase):
|
|||
values_documents = list(yaml.safe_load_all(g.read()))
|
||||
|
||||
override = ('manifest:simple-armada:release_prefix=' 'overridden', )
|
||||
|
||||
# Case 1: Checking if primitive gets updated.
|
||||
ovr = Override(original_documents, override, [values_yaml])
|
||||
ovr.update_manifests()
|
||||
|
@ -103,7 +102,6 @@ class OverrideTestCase(testtools.TestCase):
|
|||
|
||||
original_documents[-1]['data']['test'] = {'foo': 'bar'}
|
||||
override = ('manifest:simple-armada:test=' '{"foo": "bar"}', )
|
||||
|
||||
ovr = Override(original_documents, override, [])
|
||||
self.assertRaises(json.decoder.JSONDecodeError, ovr.update_manifests)
|
||||
|
||||
|
@ -321,6 +319,14 @@ class OverrideNegativeTestCase(testtools.TestCase):
|
|||
self.base_manifest = '{}/templates/base.yaml'.format(self.basepath)
|
||||
|
||||
def test_update_manifests_invalid(self):
|
||||
with open(self.base_manifest) as f:
|
||||
original_documents = list(yaml.safe_load_all(f.read()))
|
||||
|
||||
override = ('manifest:simple-armada:release_prefix=' '\overridden', )
|
||||
ovr = Override(original_documents, override)
|
||||
self.assertRaises(json.decoder.JSONDecodeError, ovr.update_manifests)
|
||||
|
||||
def test_update_manifests_invalid_dic_override(self):
|
||||
missing_yaml = "{}/templates/non_existing_yaml.yaml". \
|
||||
format(self.basepath)
|
||||
with open(self.base_manifest):
|
||||
|
@ -329,6 +335,15 @@ class OverrideNegativeTestCase(testtools.TestCase):
|
|||
override_exceptions.InvalidOverrideValueException,
|
||||
ovr.update_manifests)
|
||||
|
||||
def test_update_manifests_invalid_override(self):
|
||||
with open(self.base_manifest) as f:
|
||||
original_documents = list(yaml.safe_load_all(f.read()))
|
||||
|
||||
override = ('manifest:simple-armada:name=' 'overridden', )
|
||||
ovr = Override(original_documents, override)
|
||||
self.assertRaises(override_exceptions.InvalidOverrideValueException,
|
||||
ovr.update_manifests)
|
||||
|
||||
def test_load_yaml_file_invalid(self):
|
||||
missing_yaml = "{}/templates/non_existing_yaml.yaml". \
|
||||
format(self.basepath)
|
||||
|
|
|
@ -219,6 +219,11 @@ def validate_armada_documents(documents):
|
|||
valid, details = validate_armada_manifests(documents)
|
||||
all_valid = all_valid and valid
|
||||
messages.extend(details)
|
||||
for msg in messages:
|
||||
if msg.get('error', False):
|
||||
LOG.error(msg.get('message', 'Unknown validation error.'))
|
||||
else:
|
||||
LOG.debug(msg.get('message', 'Validation succeeded.'))
|
||||
|
||||
return all_valid, messages
|
||||
|
||||
|
|
Loading…
Reference in New Issue