From 88ac6648839dd9497834acf93604c73b5b763329 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 31 Jan 2018 19:29:19 +0000 Subject: [PATCH] Fix jsonpath_replace failing to create missing array keys This PS resolves two issues, both related to not dynamically creating missing arrays in the destination document, when the substitution destination path indicates that an array should be created. The first issue is that the array itself isn't created. For example, if substitutions.dest.path = '.foo.bar[0].baz' then the data section of the destination document should render like this: data: foo: bar: - baz: The second issue this PS resolves is not nuking the destination document data if JSON path creation failed (for whatever reason). This means that if jsonpath_replace in utils returns None, then a warning is instead logged, with no substitution taking place for a specific substitution in a document. Change-Id: I87d0bb606b74fc1e05669da639ab22ec7bf55b25 --- deckhand/engine/secrets_manager.py | 10 +- .../tests/unit/engine/test_secrets_manager.py | 166 +++++++++++++++++- deckhand/utils.py | 37 +++- 3 files changed, 203 insertions(+), 10 deletions(-) diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 3291cc8e..20ee33ad 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -204,15 +204,21 @@ class SecretsSubstitution(object): document['data'], src_secret, dest_path, dest_pattern) if isinstance(substituted_data, dict): document['data'].update(substituted_data) - else: + elif substituted_data: document['data'] = substituted_data + else: + LOG.warning( + 'Failed to create JSON path "%s" in the ' + 'destination document [%s] %s. No data was ' + 'substituted.', dest_path, document.schema, + document.name) except Exception as e: LOG.error('Unexpected exception occurred while attempting ' 'secret substitution. %s', six.text_type(e)) raise errors.SubstitutionDependencyNotFound( details=six.text_type(e)) - yield document + yield document @staticmethod def sanitize_potential_secrets(document): diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index 51ca0047..2cfcb3f2 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -96,8 +96,7 @@ class TestSecretsSubstitution(test_base.TestDbBase): secret_substitution = secrets_manager.SecretsSubstitution( substitution_sources) - substituted_docs = secret_substitution.substitute_all(documents) - + substituted_docs = list(secret_substitution.substitute_all(documents)) self.assertIn(expected_document, substituted_docs) def test_secret_substitution_single_cleartext(self): @@ -130,6 +129,169 @@ class TestSecretsSubstitution(test_base.TestDbBase): self._test_secret_substitution( document_mapping, [certificate], expected_data) + def test_create_destination_path_with_array(self): + # Validate that the destination data will be populated with an array + # where the data will be contained in array[0]. + certificate = self.secrets_factory.gen_test( + 'Certificate', 'cleartext', data='CERTIFICATE DATA') + certificate['metadata']['name'] = 'example-cert' + + document_mapping = { + "_GLOBAL_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".chart[0].values.tls.certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + + }] + } + expected_data = { + 'chart': [{ + 'values': { + 'tls': { + 'certificate': 'CERTIFICATE DATA' + } + } + }] + } + self._test_secret_substitution( + document_mapping, [certificate], expected_data) + + def test_create_destination_path_with_array_sequential_indices(self): + # Validate that the destination data will be populated with an array + # with multiple sequential indices successfully populated. + certificate = self.secrets_factory.gen_test( + 'Certificate', 'cleartext', data='CERTIFICATE DATA') + certificate['metadata']['name'] = 'example-cert' + + document_mapping = { + "_GLOBAL_SUBSTITUTIONS_1_": [ + { + "dest": { + "path": ".chart[0].values.tls.certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + }, + { + "dest": { + "path": ".chart[1].values.tls.same_certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + } + ] + } + expected_data = { + 'chart': [ + { + 'values': { + 'tls': { + 'certificate': 'CERTIFICATE DATA', + } + } + }, + { + 'values': { + 'tls': { + 'same_certificate': 'CERTIFICATE DATA', + } + } + } + ] + } + self._test_secret_substitution( + document_mapping, [certificate], expected_data) + + def test_create_destination_path_with_array_multiple_subs(self): + # Validate that the destination data will be populated with an array + # with multiple successful substitutions. + certificate = self.secrets_factory.gen_test( + 'Certificate', 'cleartext', data='CERTIFICATE DATA') + certificate['metadata']['name'] = 'example-cert' + + document_mapping = { + "_GLOBAL_SUBSTITUTIONS_1_": [ + { + "dest": { + "path": ".chart[0].values.tls.certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + }, + { + "dest": { + "path": ".chart[0].values.tls.same_certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + } + ] + } + expected_data = { + 'chart': [{ + 'values': { + 'tls': { + 'certificate': 'CERTIFICATE DATA', + 'same_certificate': 'CERTIFICATE DATA', + } + } + }] + } + self._test_secret_substitution( + document_mapping, [certificate], expected_data) + + def test_create_destination_path_with_nested_arrays(self): + # Validate that the destination data will be populated with an array + # that contains yet another array. + certificate = self.secrets_factory.gen_test( + 'Certificate', 'cleartext', data='CERTIFICATE DATA') + certificate['metadata']['name'] = 'example-cert' + document_mapping = { + "_GLOBAL_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".chart[0].values[0].tls.certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + + }] + } + expected_data = { + 'chart': [ + { + 'values': [ + { + 'tls': { + 'certificate': 'CERTIFICATE DATA' + } + } + ] + } + ] + } + self._test_secret_substitution( + document_mapping, [certificate], expected_data) + def test_secret_substitution_single_cleartext_with_pattern(self): passphrase = self.secrets_factory.gen_test( 'Passphrase', 'cleartext', data='my-secret-password') diff --git a/deckhand/utils.py b/deckhand/utils.py index d2b75c02..b1e17ee7 100644 --- a/deckhand/utils.py +++ b/deckhand/utils.py @@ -75,6 +75,36 @@ def jsonpath_parse(data, jsonpath, match_all=False): return result if match_all else result[0] +def _populate_data_with_attributes(jsonpath, data): + # Populates ``data`` with any path specified in ``jsonpath``. For example, + # if jsonpath is ".foo[0].bar.baz" then for each subpath -- foo[0], bar, + # and baz -- that key will be added to ``data`` if missing. + array_re = re.compile(r'.*[\d].*') + + d = data + for path in jsonpath.split('.')[1:]: + # Handle case where an array needs to be created. + if array_re.match(path): + try: + path_pieces = path.split('[') + path_piece = path_pieces[0] + path_index = int(path_pieces[1][:-1]) + + d.setdefault(path_piece, []) + while len(d[path_piece]) < (path_index + 1): + d[path_piece].append({}) + + d = d[path_piece][path_index] + + continue + except (IndexError, ValueError): + pass + # Handle case where an object needs to be created. + elif path not in d: + d.setdefault(path, {}) + d = d.get(path) + + def jsonpath_replace(data, value, jsonpath, pattern=None): """Update value in ``data`` at the path specified by ``jsonpath``. @@ -145,12 +175,7 @@ def jsonpath_replace(data, value, jsonpath, pattern=None): # However, Deckhand should be smart enough to create the nested keys in the # data if they don't exist and a pattern isn't required. - d = data - for path in jsonpath.split('.')[1:]: - if path not in d: - d.setdefault(path, {}) - d = d.get(path) - + _populate_data_with_attributes(jsonpath, data) return _do_replace()