diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index 2c8b1911..8f852dfc 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -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, {}) diff --git a/armada/handlers/override.py b/armada/handlers/override.py index c4280603..7a9d9646 100644 --- a/armada/handlers/override.py +++ b/armada/handlers/override.py @@ -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 diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index c9f6b359..acd06463 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -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) diff --git a/armada/tests/unit/handlers/test_override.py b/armada/tests/unit/handlers/test_override.py index f45fa45d..62edaafa 100644 --- a/armada/tests/unit/handlers/test_override.py +++ b/armada/tests/unit/handlers/test_override.py @@ -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) diff --git a/armada/utils/validate.py b/armada/utils/validate.py index 44ab2c6d..7dffb853 100644 --- a/armada/utils/validate.py +++ b/armada/utils/validate.py @@ -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