diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 67311721..971c8cb6 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -490,6 +490,8 @@ class DocumentLayering(object): self.secrets_substitution.substitute_all(doc)) if substituted_data: rendered_data_by_layer[layer_idx] = substituted_data[0] + self.secrets_substitution.update_substitution_sources( + doc.schema, doc.name, substituted_data[0].data) else: rendered_data_by_layer[layer_idx] = doc @@ -525,6 +527,8 @@ class DocumentLayering(object): # children in deeper layers can reference the most up-to-date # changes. rendered_data_by_layer[child_layer_idx] = rendered_data + self.secrets_substitution.update_substitution_sources( + child.schema, child.name, rendered_data.data) # Handle edge case for parentless documents that require substitution. # If a document has no parent, then the for loop above doesn't iterate @@ -536,6 +540,8 @@ class DocumentLayering(object): self.secrets_substitution.substitute_all(doc)) if substituted_data: doc = substituted_data[0] + self.secrets_substitution.update_substitution_sources( + doc.schema, doc.name, substituted_data[0].data) # Return only concrete documents. return [d for d in self._documents_to_layer if d.is_abstract is False] diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 7e68248b..9b6df28d 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -105,6 +105,26 @@ class SecretsManager(object): class SecretsSubstitution(object): """Class for document substitution logic for YAML files.""" + @staticmethod + def sanitize_potential_secrets(document): + """Sanitize all secret data that may have been substituted into the + document. Uses references in ``document.substitutions`` to determine + which values to sanitize. Only meaningful to call this on post-rendered + documents. + + :param DocumentDict document: Document to sanitize. + """ + to_sanitize = copy.deepcopy(document) + safe_message = 'Sanitized to avoid exposing secret.' + + for sub in document.substitutions: + replaced_data = utils.jsonpath_replace( + to_sanitize['data'], safe_message, sub['dest']['path']) + if replaced_data: + to_sanitize['data'] = replaced_data + + return to_sanitize + def __init__(self, substitution_sources=None, fail_on_missing_sub_src=True): """SecretSubstitution constructor. @@ -207,17 +227,11 @@ class SecretsSubstitution(object): try: substituted_data = utils.jsonpath_replace( document['data'], src_secret, dest_path, dest_pattern) - sub_source = self._substitution_sources.get( - (document.schema, document.name)) - if (isinstance(document['data'], dict) and - isinstance(substituted_data, dict)): + if (isinstance(document['data'], dict) + and isinstance(substituted_data, dict)): document['data'].update(substituted_data) - if sub_source: - sub_source['data'].update(substituted_data) elif substituted_data: document['data'] = substituted_data - if sub_source: - sub_source['data'] = substituted_data else: message = ( 'Failed to create JSON path "%s" in the ' @@ -234,22 +248,12 @@ class SecretsSubstitution(object): yield document - @staticmethod - def sanitize_potential_secrets(document): - """Sanitize all secret data that may have been substituted into the - document. Uses references in ``document.substitutions`` to determine - which values to sanitize. Only meaningful to call this on post-rendered - documents. + def update_substitution_sources(self, schema, name, data): + if (schema, name) not in self._substitution_sources: + return - :param DocumentDict document: Document to sanitize. - """ - to_sanitize = copy.deepcopy(document) - safe_message = 'Sanitized to avoid exposing secret.' - - for sub in document.substitutions: - replaced_data = utils.jsonpath_replace( - to_sanitize['data'], safe_message, sub['dest']['path']) - if replaced_data: - to_sanitize['data'] = replaced_data - - return to_sanitize + substitution_src = self._substitution_sources[(schema, name)] + if isinstance(data, dict) and isinstance(substitution_src.data, dict): + substitution_src.data.update(data) + else: + substitution_src.data = data diff --git a/deckhand/factories.py b/deckhand/factories.py index 0c28137c..51606806 100644 --- a/deckhand/factories.py +++ b/deckhand/factories.py @@ -172,7 +172,7 @@ class DocumentFactory(DeckhandFactory): "be equal to the value of 'num_layers'.") for doc_count in docs_per_layer: - if doc_count < 1: + if doc_count < 0: raise ValueError( "Each entry in 'docs_per_layer' must be >= 1.") diff --git a/deckhand/tests/unit/engine/test_document_layering.py b/deckhand/tests/unit/engine/test_document_layering.py index 89090b3d..7608e2e3 100644 --- a/deckhand/tests/unit/engine/test_document_layering.py +++ b/deckhand/tests/unit/engine/test_document_layering.py @@ -118,6 +118,88 @@ class TestDocumentLayeringScenarios(TestDocumentLayering): self.assertRegex(m_log.warning.mock_calls[0][1][0][0], r'Could not find substitution source document .*') + def test_layering_substitution_source_skips_layering(self): + """This scenario consists of a layerOrder with global, region, site, + with 1 global documents and 2 sites documents. 1 site document ('e') + layers with the parent (the global document) and the other site + document substitutes from the 1st site document. + """ + + payload = """ +--- +schema: aic/Versions/v1 +metadata: + name: d + schema: metadata/Document/v1 + labels: + selector: foo1 + layeringDefinition: + abstract: True + layer: global +data: + conf: + foo: default +--- +schema: aic/Versions/v1 +metadata: + name: e + schema: metadata/Document/v1 + labels: + selector: baz1 + layeringDefinition: + abstract: False + layer: site + parentSelector: + selector: foo1 + actions: + - method: merge + path: . +data: + conf: + bar: override +--- +schema: armada/Chart/v1 +metadata: + name: f + schema: metadata/Document/v1 + layeringDefinition: + abstract: False + layer: site + substitutions: + - src: + schema: aic/Versions/v1 + name: e + path: .conf + dest: + path: .application.conf +data: + application: + conf: {} +--- +schema: deckhand/LayeringPolicy/v1 +metadata: + schema: metadata/Control/v1 + name: layering-policy +data: + layerOrder: + - global + - region + - site +... +""" + + documents = list(yaml.safe_load_all(payload)) + + site_expected = [ + {'conf': {'foo': 'default', 'bar': 'override'}}, + {'application': {'conf': {'bar': 'override', 'foo': 'default'}}} + ] + region_expected = None + global_expected = None + self._test_layering( + documents, site_expected, region_expected, global_expected, + substitution_sources=[documents[1]]) + class TestDocumentLayering2Layers(TestDocumentLayering):