diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 7d3abde2..3e19dc24 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -213,9 +213,10 @@ class SecretsSubstitution(object): substitution; documents need not be filtered prior to being passed to the constructor. - :param substitution_sources: List of documents that are potential - sources for substitution. Or dict of documents keyed on tuple of - (schema, metadata.name). Should only include concrete documents. + :param substitution_sources: (DEPRECATED) List of documents that are + potential sources for substitution. Or dict of documents keyed on + tuple of (schema, metadata.name). Should only include concrete + documents. :type substitution_sources: List[dict] or dict :param bool fail_on_missing_sub_src: Whether to fail on a missing substitution source. Default is True. @@ -239,6 +240,51 @@ class SecretsSubstitution(object): self._substitution_sources.setdefault( (document.schema, document.name), document) + def _handle_unknown_substitution_exc(self, exc_message, src_doc, dest_doc): + if self._fail_on_missing_sub_src: + LOG.error(exc_message) + raise errors.UnknownSubstitutionError( + src_schema=src_doc.schema, src_layer=src_doc.layer, + src_name=src_doc.name, schema=dest_doc.schema, + layer=dest_doc.layer, name=dest_doc.name, details=exc_message) + else: + LOG.warning(exc_message) + + def _get_encrypted_secret(self, src_secret, src_doc, dest_doc): + try: + src_secret = SecretsManager.get(src_secret) + except errors.BarbicanException as e: + LOG.error( + 'Failed to resolve a Barbican reference for substitution ' + 'source document [%s, %s] %s referenced in document [%s, %s] ' + '%s. Details: %s', src_doc.schema, src_doc.layer, src_doc.name, + dest_doc.schema, dest_doc.layer, dest_doc.name, + e.format_message()) + raise errors.UnknownSubstitutionError( + src_schema=src_doc.schema, src_layer=src_doc.layer, + src_name=src_doc.name, schema=dest_doc.schema, + layer=dest_doc.layer, name=dest_doc.name, + details=e.format_message()) + else: + return src_secret + + def _check_src_secret_is_not_none(self, src_secret, src_path, src_doc, + dest_doc): + if src_secret is None: + if self._fail_on_missing_sub_src: + raise errors.SubstitutionSourceDataNotFound( + src_path=src_path, src_schema=src_doc.schema, + src_layer=src_doc.layer, src_name=src_doc.name, + dest_schema=dest_doc.schema, dest_layer=dest_doc.layer, + dest_name=dest_doc.name) + else: + LOG.warning('Could not find source path %s in source document ' + 'or the secret extracted is None. Source document:' + ' [%s, %s] %s. Destination document: [%s, %s] %s.', + src_path, src_doc.schema, src_doc.layer, + src_doc.name, dest_doc.schema, dest_doc.layer, + dest_doc.name) + def substitute_all(self, documents): """Substitute all documents that have a `metadata.substitutions` field. @@ -307,24 +353,14 @@ class SecretsSubstitution(object): else: src_secret = src_doc.get('data') + self._check_src_secret_is_not_none(src_secret, src_path, + src_doc, document) + # If the document has storagePolicy == encrypted then resolve # the Barbican reference into the actual secret. if src_doc.is_encrypted: - try: - src_secret = SecretsManager.get(src_secret) - except errors.BarbicanException as e: - LOG.error( - 'Failed to resolve a Barbican reference for ' - 'substitution source document [%s, %s] %s ' - 'referenced in document [%s, %s] %s. Details: %s', - src_schema, src_doc.layer, src_name, - document.schema, document.layer, document.name, - e.format_message()) - raise errors.UnknownSubstitutionError( - src_schema=src_schema, src_layer=src_doc.layer, - src_name=src_name, schema=document.schema, - layer=document.layer, name=document.name, - details=e.format_message()) + src_secret = self._get_encrypted_secret( + src_secret, src_doc, document) dest_path = sub['dest']['path'] dest_pattern = sub['dest'].get('pattern', None) @@ -357,12 +393,8 @@ class SecretsSubstitution(object): exc_message = six.text_type(e) finally: if exc_message: - LOG.error(exc_message) - raise errors.UnknownSubstitutionError( - src_schema=src_schema, src_layer=src_doc.layer, - src_name=src_name, schema=document.schema, - layer=document.layer, name=document.name, - details=exc_message) + self._handle_unknown_substitution_exc( + exc_message, src_doc, document) yield document diff --git a/deckhand/errors.py b/deckhand/errors.py index bf502f9a..f6fb234d 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -302,6 +302,26 @@ class RevisionTagBadFormat(DeckhandException): code = 400 +class SubstitutionSourceDataNotFound(DeckhandException): + """Required substitution source secret was not found in the substitution + source document at the path ``metadata.substitutions.[*].src.path`` in the + destination document. + + **Troubleshoot:** + + * Ensure that the missing source secret exists at the ``src.path`` + specified under the given substitution in the destination document and + that the ``src.path`` itself exists in the source document. + """ + msg_fmt = ( + "Required substitution source secret was not found at path " + "%(src_path)s in source document [%(src_schema)s, %(src_layer)s] " + "%(src_name)s which is referenced by destination document " + "[%(dest_schema)s, %(dest_layer)s] %(dest_name)s under its " + "`metadata.substitutions`.") + code = 400 + + class DocumentNotFound(DeckhandException): """The requested document could not be found. @@ -376,7 +396,7 @@ class LayeringPolicyNotFound(DeckhandException): class SubstitutionSourceNotFound(DeckhandException): - """Required substitution source document was not found for layering. + """Required substitution source document was not found. **Troubleshoot:** diff --git a/deckhand/tests/unit/engine/test_document_layering_and_substitution_negative.py b/deckhand/tests/unit/engine/test_document_layering_and_substitution_negative.py index cf336c77..b1a2c7e3 100644 --- a/deckhand/tests/unit/engine/test_document_layering_and_substitution_negative.py +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution_negative.py @@ -90,6 +90,8 @@ class TestDocumentLayeringWithSubstitutionNegative( In the case below, a self-reference or cycle exists for site-1 with itself. """ + + # TODO(fmontei): Move to test_secrets_manager (negative) mapping = { "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, "_SITE_NAME_1_": "site-1", @@ -119,6 +121,8 @@ class TestDocumentLayeringWithSubstitutionNegative( def test_layering_with_missing_substitution_source_raises_exc( self, mock_log): """Validate that a missing substitution source document fails.""" + + # TODO(fmontei): Move to test_secrets_manager (negative) mapping = { "_GLOBAL_SUBSTITUTIONS_1_": [{ "dest": { diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index 34b0ae9e..779f9b19 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -737,8 +737,7 @@ class TestSecretsSubstitutionNegative(test_base.TestDbBase): documents = self.create_documents( bucket_name, [certificate] + [payload[-1]]) - secrets_substitution = secrets_manager.SecretsSubstitution( - [certificate]) + secrets_substitution = secrets_manager.SecretsSubstitution(documents) with testtools.ExpectedException(expected_exception): next(secrets_substitution.substitute_all(documents)) @@ -755,3 +754,35 @@ class TestSecretsSubstitutionNegative(test_base.TestDbBase): mock_utils.jsonpath_replace.side_effect = Exception('test') self._test_secrets_substitution( 'cleartext', errors.UnknownSubstitutionError) + + def test_secret_substititon_missing_src_path_in_src_doc_raises_exc(self): + """Validates that if a secret can't be found in a substitution + source document then an exception is raised. + """ + certificate = self.secrets_factory.gen_test( + 'Certificate', 'cleartext', data={}) + certificate['metadata']['name'] = 'example-cert' + + document_mapping = { + "_GLOBAL_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".chart.values.tls.certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": ".path-to-nowhere" + } + + }] + } + payload = self.document_factory.gen_test(document_mapping, + global_abstract=False) + bucket_name = test_utils.rand_name('bucket') + documents = self.create_documents( + bucket_name, [certificate] + [payload[-1]]) + + secrets_substitution = secrets_manager.SecretsSubstitution(documents) + with testtools.ExpectedException( + errors.SubstitutionSourceDataNotFound): + next(secrets_substitution.substitute_all(documents)) diff --git a/docs/source/exceptions.rst b/docs/source/exceptions.rst index f4e95e4f..693512ae 100644 --- a/docs/source/exceptions.rst +++ b/docs/source/exceptions.rst @@ -94,6 +94,11 @@ Deckhand Exceptions :members: :show-inheritance: :undoc-members: + * - SubstitutionSourceDataNotFound + - .. autoexception:: deckhand.errors.SubstitutionSourceDataNotFound + :members: + :show-inheritance: + :undoc-members: * - SubstitutionSourceNotFound - .. autoexception:: deckhand.errors.SubstitutionSourceNotFound :members: diff --git a/docs/source/replacement.rst b/docs/source/replacement.rst index 23572afc..aabba8f5 100644 --- a/docs/source/replacement.rst +++ b/docs/source/replacement.rst @@ -190,7 +190,7 @@ For example: - method: replace path: .debug data: - debug: True + debug: true ... And: