From ac4fc210ee02f12020efe313c9f33dab04e8d51a Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 10 Jul 2017 21:34:35 +0100 Subject: [PATCH] Add jsonschema validation to Deckhand Previously Deckhand was using manual validation of YAML files. However, schema validation is much cleaner, robust and thorough. So, this commit adds jsonschema as a dependence in Deckhand. Tempest already uses it as a dependence as well -- so a precedence already exists. This commit also updates unit tests as needed. Documentation changes will be made in a follow-up patch. --- deckhand/engine/schema/__init__.py | 0 deckhand/engine/schema/v1_0/__init__.py | 0 deckhand/engine/schema/v1_0/default_schema.py | 69 ++++++++++ deckhand/engine/secret_substitution.py | 30 ++--- .../unit/engine/test_secret_substitution.py | 125 +++++++++--------- requirements.txt | 1 + test-requirements.txt | 1 - 7 files changed, 139 insertions(+), 87 deletions(-) create mode 100644 deckhand/engine/schema/__init__.py create mode 100644 deckhand/engine/schema/v1_0/__init__.py create mode 100644 deckhand/engine/schema/v1_0/default_schema.py diff --git a/deckhand/engine/schema/__init__.py b/deckhand/engine/schema/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/deckhand/engine/schema/v1_0/__init__.py b/deckhand/engine/schema/v1_0/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/deckhand/engine/schema/v1_0/default_schema.py b/deckhand/engine/schema/v1_0/default_schema.py new file mode 100644 index 00000000..f2402e6b --- /dev/null +++ b/deckhand/engine/schema/v1_0/default_schema.py @@ -0,0 +1,69 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +substitution_schema = { + 'type': 'object', + 'properties': { + 'dest': {'type': 'string'}, + 'src': { + 'type': 'object', + 'properties': { + 'apiVersion': { + 'type': 'string', + 'choices': ['deckhand/v1'] + }, + 'kind': {'type': 'string'}, + 'name': {'type': 'string'} + }, + 'additionalProperties': False, + 'required': ['apiVersion', 'kind', 'name'] + } + }, + 'additionalProperties': False, + 'required': ['dest', 'src'] +} + +schema = { + 'type': 'object', + 'properties': { + 'apiVersion': { + 'type': 'string', + # TODO(fm577c): These should be enumerated. + 'choices': ['service/v1'] + }, + 'kind': { + 'type': 'string', + # TODO(fm577c): These should be enumerated. + 'choices': ['ConsumerOfCertificateData'] + }, + 'metadata': { + 'type': 'object', + 'properties': { + 'name': {'type': 'string'}, + 'storage': {'type': 'string'}, + 'substitutions': { + 'type': 'array', + 'items': substitution_schema + } + }, + 'additionalProperties': False, + 'required': ['name', 'storage', 'substitutions'] + }, + 'data': { + 'type': 'object' + } + }, + 'additionalProperties': False, + 'required': ['apiVersion', 'kind', 'metadata', 'data'] +} diff --git a/deckhand/engine/secret_substitution.py b/deckhand/engine/secret_substitution.py index 80ac27c8..8a16c99f 100644 --- a/deckhand/engine/secret_substitution.py +++ b/deckhand/engine/secret_substitution.py @@ -14,7 +14,10 @@ import yaml +import jsonschema + from deckhand import errors +from deckhand.engine.schema.v1_0 import default_schema class SecretSubstitution(object): @@ -64,32 +67,17 @@ class SecretSubstitution(object): certificate: null # Data to be substituted. certificateKey: null # Data to be substituted. """ - # Validate that data section exists. try: - self.data['data'] - except (KeyError, TypeError) as e: - raise errors.InvalidFormat( - 'The provided YAML file has no data section: %s' % e) - # Validate that substitutions section exists. - try: - substitutions = self.data['metadata']['substitutions'] - except (KeyError, TypeError) as e: - raise errors.InvalidFormat( - 'The provided YAML file has no metadata/substitutions ' - 'section: %s' % e) - - # Validate that "src" and "dest" fields exist per substitution entry. - error_message = ('The provided YAML file is missing the "%s" field ' - 'for the %s substition.') - for s in substitutions: - if 'src' not in s: - raise errors.InvalidFormat(error_message % ('src', s)) - elif 'dest' not in s: - raise errors.InvalidFormat(error_message % ('dest', s)) + jsonschema.validate(self.data, default_schema.schema) + except jsonschema.exceptions.ValidationError as e: + raise errors.InvalidFormat('The provided YAML file is invalid. ' + 'Exception: %s.' % e.message) # Validate that each "dest" field exists in the YAML data. + substitutions = self.data['metadata']['substitutions'] destinations = [s['dest'] for s in substitutions] sub_data = self.data['data'] + for dest in destinations: result, missing_attr = self._multi_getattr(dest, sub_data) if not result: diff --git a/deckhand/tests/unit/engine/test_secret_substitution.py b/deckhand/tests/unit/engine/test_secret_substitution.py index b2b9019a..cf470fe3 100644 --- a/deckhand/tests/unit/engine/test_secret_substitution.py +++ b/deckhand/tests/unit/engine/test_secret_substitution.py @@ -12,8 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import os import testtools +import yaml from oslo_serialization import jsonutils as json import six @@ -30,83 +32,76 @@ class TestSecretSubtitution(testtools.TestCase): test_yaml_path = os.path.abspath(os.path.join( dir_path, os.pardir, 'resources', 'sample.yaml')) - with open(test_yaml_path, 'r') as yaml_data: - self.yaml_data = yaml_data.read() + with open(test_yaml_path, 'r') as yaml_file: + yaml_data = yaml_file.read() + self.data = yaml.safe_load(yaml_data) - def test_initialization_missing_substitutions_section(self): - expected_err = ( - "The provided YAML file has no metadata/substitutions section") + def _corrupt_data(self, key): + corrupted_data = copy.deepcopy(self.data) + + if '.' in key: + _corrupted_data = corrupted_data + nested_keys = key.split('.') + for nested_key in nested_keys: + if nested_key == nested_keys[-1]: + break + if nested_key.isdigit(): + _corrupted_data = _corrupted_data[int(nested_key)] + else: + _corrupted_data = _corrupted_data[nested_key] + _corrupted_data.pop(nested_keys[-1]) + else: + corrupted_data.pop(key) + + return yaml.safe_dump(corrupted_data) + + def _format_data(self, data=None): + if data is None: + data = self.data + return yaml.safe_dump(data) + + def test_initialization(self): + sub = secret_substitution.SecretSubstitution(self._format_data()) + self.assertIsInstance(sub, secret_substitution.SecretSubstitution) + + def test_initialization_missing_sections(self): + expected_err = ("The provided YAML file is invalid. Exception: '%s' " + "is a required property.") invalid_data = [ - {"data": []}, - {"data": [], "metadata": None}, - {"data": [], "metadata": {"missing_substitutions": None}} + (self._corrupt_data('data'), 'data'), + (self._corrupt_data('metadata'), 'metadata'), + (self._corrupt_data('metadata.name'), 'name'), + (self._corrupt_data('metadata.storage'), 'storage'), + (self._corrupt_data('metadata.substitutions'), 'substitutions'), + (self._corrupt_data('metadata.substitutions.0.dest'), 'dest'), + (self._corrupt_data('metadata.substitutions.0.src'), 'src') ] - for invalid_entry in invalid_data: - invalid_entry = json.dumps(invalid_entry) + for invalid_entry, missing_key in invalid_data: with six.assertRaisesRegex(self, errors.InvalidFormat, - expected_err): + expected_err % missing_key): secret_substitution.SecretSubstitution(invalid_entry) - expected_err = ( - "The provided YAML file has no metadata/substitutions section") - invalid_data = [ - {"data": [], "metadata": None}, - ] - - def test_initialization_missing_data_section(self): - expected_err = ( - "The provided YAML file has no data section") - invalid_data = '{"metadata": {"substitutions": []}}' - - with six.assertRaisesRegex(self, errors.InvalidFormat, expected_err): - secret_substitution.SecretSubstitution(invalid_data) - - def test_initialization_missing_src_dest_sections(self): - expected_err = ('The provided YAML file is missing the "%s" field for ' - 'the %s substition.') - invalid_data = [ - {"data": [], "metadata": {"substitutions": [{"dest": "foo"}]}}, - {"data": [], "metadata": {"substitutions": [{"src": "bar"}]}}, - ] - - def _test(invalid_entry, field, substitution): - invalid_entry = json.dumps(invalid_entry) - _expected_err = expected_err % (field, substitution) - - with six.assertRaisesRegex(self, errors.InvalidFormat, - _expected_err): - secret_substitution.SecretSubstitution(invalid_entry) - - _test(invalid_data[0], "src", {"dest": "foo"}) - _test(invalid_data[1], "dest", {"src": "bar"}) - def test_initialization_bad_substitutions(self): expected_err = ('The attribute "%s" included in the "dest" field "%s" ' - 'is missing from the YAML data: "%s".') - invalid_data = [ - # Missing attribute. - {"data": {}, "metadata": {"substitutions": [ - {"src": "", "dest": "foo"} - ]}}, - # Missing attribute. - {"data": {"foo": None}, "metadata": {"substitutions": [ - {"src": "", "dest": "bar"} - ]}}, - # Missing nested attribute. - {"data": {"foo": {"baz": None}}, "metadata": {"substitutions": [ - {"src": "", "dest": "foo.bar"} - ]}}, - ] + 'is missing from the YAML data') + invalid_data = [] - def _test(invalid_entry, field, dest, substitution): - invalid_entry = json.dumps(invalid_entry) - _expected_err = expected_err % (field, dest, substitution) + data = copy.deepcopy(self.data) + data['metadata']['substitutions'][0]['dest'] = 'foo' + invalid_data.append(self._format_data(data)) + data = copy.deepcopy(self.data) + data['metadata']['substitutions'][0]['dest'] = 'tls_endpoint.bar' + invalid_data.append(self._format_data(data)) + + def _test(invalid_entry, field, dest): + _expected_err = expected_err % (field, dest) with six.assertRaisesRegex(self, errors.InvalidFormat, _expected_err): secret_substitution.SecretSubstitution(invalid_entry) - _test(invalid_data[0], "foo", "foo", {}) - _test(invalid_data[1], "bar", "bar", {"foo": None}) - _test(invalid_data[2], "bar", "foo.bar", {'foo': {'baz': None}}) + # Verify that invalid body dest reference is invalid. + _test(invalid_data[0], "foo", "foo") + # Verify that nested invalid body dest reference is invalid. + _test(invalid_data[1], "bar", "tls_endpoint.bar") diff --git a/requirements.txt b/requirements.txt index 97850f1a..a50fe325 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,7 @@ falcon==1.1.0 pbr!=2.1.0,>=2.0.0 # Apache-2.0 six>=1.9.0 # MIT stevedore>=1.20.0 # Apache-2.0 +jsonschema!=2.5.0,<3.0.0,>=2.0.0 # MIT keystoneauth1>=2.21.0 # Apache-2.0 oslo.config>=3.22.0 # Apache-2.0 oslo.context>=2.14.0 # Apache-2.0 diff --git a/test-requirements.txt b/test-requirements.txt index 646fffc9..da76afd5 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,6 @@ falcon==1.1.0 mock>=2.0 fixtures>=3.0.0 # Apache-2.0/BSD -mock>=2.0 # BSD mox3!=0.19.0,>=0.7.0 # Apache-2.0 python-subunit>=0.0.18 # Apache-2.0/BSD oslotest>=1.10.0 # Apache-2.0