diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 759bbf24..61eb5421 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -15,6 +15,9 @@ import collections import copy +import networkx +from networkx.algorithms.cycles import find_cycle +from networkx.algorithms.dag import topological_sort from oslo_log import log as logging from deckhand.engine import document_wrapper @@ -103,13 +106,20 @@ class DocumentLayering(object): return is_actual_child def _calc_document_children(self, document): - potential_children = set() + potential_children = [] for label_key, label_val in document.labels.items(): _potential_children = self._documents_by_labels.get( (label_key, label_val), []) - potential_children |= set(_potential_children) + potential_children.extend(_potential_children) + # NOTE(fmontei): The intention here is to preserve the order of all + # the documents that were sorted by `_topologically_sort_documents` + # in order to substitute documents in the right order. But at the same + # time, only unique children should be found. So, this trick below + # maintains the order (unlike set) and guarantees uniqueness. + unique_potential_children = collections.OrderedDict.fromkeys( + potential_children) - for potential_child in potential_children: + for potential_child in unique_potential_children: if self._is_actual_child_document(document, potential_child): yield potential_child @@ -199,6 +209,41 @@ class DocumentLayering(object): 'will be performed.') return layer_order + def _topologically_sort_documents(self, documents): + """Topologically sorts the DAG formed from the documents' substitution + dependency chain. + """ + documents_by_name = {} + result = [] + + g = networkx.DiGraph() + for document in documents: + document = document_wrapper.DocumentDict(document) + documents_by_name.setdefault((document.schema, document.name), + document) + for sub in document.substitutions: + g.add_edge((document.schema, document.name), + (sub['src']['schema'], sub['src']['name'])) + + try: + cycle = find_cycle(g) + except networkx.exception.NetworkXNoCycle: + pass + else: + LOG.error('Cannot determine substitution order as a dependency ' + 'cycle exists for the following documents: %s.', cycle) + raise errors.SubstitutionDependencyCycle(cycle=cycle) + + sorted_documents = reversed(list(topological_sort(g))) + + for document in sorted_documents: + if document in documents_by_name: + result.append(documents_by_name.pop(document)) + for document in documents_by_name.values(): + result.append(document) + + return result + def __init__(self, documents, substitution_sources=None): """Contructor for ``DocumentLayering``. @@ -245,7 +290,9 @@ class DocumentLayering(object): LOG.error(error_msg) raise errors.LayeringPolicyNotFound() - for document in documents: + sorted_documents = self._topologically_sort_documents(documents) + + for document in sorted_documents: document = document_wrapper.DocumentDict(document) if document.layering_definition: self._documents_to_layer.append(document) @@ -432,12 +479,12 @@ class DocumentLayering(object): # Update the actual document data if concrete. if not child.is_abstract: + child_index = self._documents_to_layer.index(child) child.data = rendered_data.data substituted_data = list( self.secrets_substitution.substitute_all(child)) if substituted_data: rendered_data = substituted_data[0] - child_index = self._documents_to_layer.index(child) self._documents_to_layer[child_index].data = ( rendered_data.data) diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 18c800af..050e4ab8 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -210,10 +210,17 @@ class SecretsSubstitution(object): try: substituted_data = utils.jsonpath_replace( document['data'], src_secret, dest_path, dest_pattern) - if isinstance(substituted_data, dict): + sub_source = self._substitution_sources.get( + (document.schema, document.name)) + 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: LOG.warning( 'Failed to create JSON path "%s" in the ' diff --git a/deckhand/errors.py b/deckhand/errors.py index c5bc4528..325459de 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -211,7 +211,18 @@ class IndeterminateDocumentParent(DeckhandException): **Troubleshoot:** """ - msg_fmt = ("Too many parent documents found for document %(document)s.") + msg_fmt = "Too many parent documents found for document %(document)s." + code = 400 + + +class SubstitutionDependencyCycle(DeckhandException): + """An illegal substitution depdencency cycle was detected. + + **Troubleshoot:** + * Check that there is no two-way substitution dependency between documents. + """ + msg_fmt = ('Cannot determine substitution order as a dependency ' + 'cycle exists for the following documents: %(cycle)s.') code = 400 diff --git a/deckhand/factories.py b/deckhand/factories.py index d3eb1aba..0c28137c 100644 --- a/deckhand/factories.py +++ b/deckhand/factories.py @@ -236,10 +236,20 @@ class DocumentFactory(DeckhandFactory): layer_template = copy.deepcopy(self.LAYER_TEMPLATE) layer_name = layer_order[layer_idx] - # Set name. layer_template = copy.deepcopy(layer_template) - layer_template['metadata']['name'] = "%s%d" % ( - test_utils.rand_name(layer_name), count + 1) + + # Set name. + name_key = "_%s_NAME_%d_" % (layer_name.upper(), count + 1) + if name_key in mapping: + layer_template['metadata']['name'] = mapping[name_key] + else: + layer_template['metadata']['name'] = "%s%d" % ( + test_utils.rand_name(layer_name), count + 1) + + # Set schema. + schema_key = "_%s_SCHEMA_%d_" % (layer_name.upper(), count + 1) + if schema_key in mapping: + layer_template['schema'] = mapping[schema_key] # Set layer. layer_template['metadata']['layeringDefinition'][ diff --git a/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-layering-and-substitution.yaml b/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-layering-and-substitution.yaml index 5025edb3..6e7614a6 100644 --- a/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-layering-and-substitution.yaml +++ b/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-layering-and-substitution.yaml @@ -3,6 +3,7 @@ # * Substitution/layering works with top layer empty # * Substitution/layering works with multiple top layers empty # * Substitution/layering works with intermediate layer empty +# * Substitution/layering works with substitution dependency chain # # Base case: # 1. Purges existing data to ensure test isolation. @@ -19,6 +20,11 @@ # 11. Creates LayeringPolicy with 5 layers, with empty interspersed layers. # 12. Adds initial documents for layering/substitution base case. # 13. Verifies fully substituted/layered document data. +# DAG case: +# 14. Re-creates the layering policy with 2 layers: region and site. +# 15. Adds documents with a substitution dependency chain that +# requires sorting in order to resolve. +# 16. Verifies fully substituted/layered document data. defaults: request_headers: @@ -59,7 +65,7 @@ tests: substitutions from secret documents. PUT: /api/v1.0/buckets/mop/documents status: 200 - data: <@resources/layering-and-substitution-example.yaml + data: <@resources/layering-and-substitution-sample.yaml - name: verify_base_case desc: Check for expected substitutions @@ -106,7 +112,7 @@ tests: Same case as before, except with a top empty layer. PUT: /api/v1.0/buckets/mop/documents status: 200 - data: <@resources/layering-and-substitution-example.yaml + data: <@resources/layering-and-substitution-sample.yaml - name: verify_empty_top_layer desc: Check for expected substitutions @@ -155,7 +161,7 @@ tests: Same case as before, except with multiple empty top layers. PUT: /api/v1.0/buckets/mop/documents status: 200 - data: <@resources/layering-and-substitution-example.yaml + data: <@resources/layering-and-substitution-sample.yaml - name: verify_multiple_empty_top_layers desc: Check for expected substitutions @@ -205,7 +211,7 @@ tests: Same case as before, except with multiple empty interspersed layers. PUT: /api/v1.0/buckets/mop/documents status: 200 - data: <@resources/layering-and-substitution-example.yaml + data: <@resources/layering-and-substitution-sample.yaml - name: verify_multiple_empty_top_layers desc: Check for expected substitutions @@ -228,3 +234,75 @@ tests: KEY DATA some_url: http://admin:my-secret-password@service-name:8080/v1 from-parent: parent-val + + - name: initialize_layering_policy_base_case_again + desc: | + Initailize the layering policy with 2 layers. + PUT: /api/v1.0/buckets/mop/documents + status: 200 + data: |- + --- + schema: deckhand/LayeringPolicy/v1 + metadata: + schema: metadata/Control/v1 + name: layering-policy + data: + layerOrder: + - region + - site + ... + + - name: initialize_substitution_dependency_chain + desc: | + Base case for testing layering alongside substitution in which a DAG + must be used to topologically sort the substitutions in order to derive + the correct substitution order. + PUT: /api/v1.0/buckets/mop/documents + status: 200 + data: <@resources/layering-and-substitution-dag-sample.yaml + + - name: verify_substitution_dependency_chain + desc: | + The dependency chain is: armada-chart-03 -> armada-chart-02 -> armada-chart-01 + but the documents are passed to the server in reverse order. This verifies + that the server re-orders the documents according to the topologically sorted + order of the substitution dependency DAG. Also, armada-chart-02 and + armada-chart-03 are layered with armada-chart-01 except that armada-chart-02 + deletes everything and armada-chart-03 merges everything. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents + query_parameters: + schema: armada/Chart/v1 + sort: metadata.name + status: 200 + response_multidoc_jsonpaths: + $.`len`: 3 + $.[0].metadata.name: armada-chart-01 + $.[0].data: + region: + certificate: | + CERTIFICATE DATA + certificatekey: | + KEY DATA + passphrase: http://admin:my-secret-password@service-name:8080/v1 + $.[1].metadata.name: armada-chart-02 + $.[1].data: + site: + certificate: | + CERTIFICATE DATA + certificatekey: | + KEY DATA + passphrase: http://admin:my-secret-password@service-name:8080/v1 + $.[2].metadata.name: armada-chart-03 + $.[2].data: + region: + certificate: | + CERTIFICATE DATA + certificatekey: | + KEY DATA + passphrase: http://admin:my-secret-password@service-name:8080/v1 + site-alt: + certificate: | + CERTIFICATE DATA + certificatekey: | + KEY DATA + passphrase: http://admin:my-secret-password@service-name:8080/v1 diff --git a/deckhand/tests/functional/gabbits/resources/layering-and-substitution-dag-sample.yaml b/deckhand/tests/functional/gabbits/resources/layering-and-substitution-dag-sample.yaml new file mode 100644 index 00000000..d3ce220b --- /dev/null +++ b/deckhand/tests/functional/gabbits/resources/layering-and-substitution-dag-sample.yaml @@ -0,0 +1,147 @@ +--- +schema: deckhand/Certificate/v1 +metadata: + name: example-cert + schema: metadata/Document/v1 + layeringDefinition: + layer: site + storagePolicy: cleartext +data: | + CERTIFICATE DATA +--- +schema: deckhand/CertificateKey/v1 +metadata: + name: example-key + schema: metadata/Document/v1 + layeringDefinition: + layer: site + storagePolicy: cleartext +data: | + KEY DATA +--- +schema: deckhand/Passphrase/v1 +metadata: + name: example-password + schema: metadata/Document/v1 + layeringDefinition: + layer: site + storagePolicy: cleartext +data: my-secret-password +--- +# NOTE(fmontei): The documents below are included in reverse order with +# respect to their substitution dependency hierarchy in order to verify +# that the dependency chain is correctly resolved in the code. +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: armada-chart-03 + layeringDefinition: + abstract: false + layer: site + parentSelector: + key1: value1 + actions: + - method: merge + path: . + substitutions: + - dest: + path: .site-alt.certificate + src: + schema: armada/Chart/v1 + name: armada-chart-02 + path: .site.certificate + - dest: + path: .site-alt.certificatekey + src: + schema: armada/Chart/v1 + name: armada-chart-02 + path: .site.certificatekey + - dest: + path: .site-alt.passphrase + src: + schema: armada/Chart/v1 + name: armada-chart-02 + path: .site.passphrase +data: + site-alt: + certificate: placeholder + certificatekey: placeholder + passphrase: placeholder +--- +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: armada-chart-02 + layeringDefinition: + abstract: false + layer: site + parentSelector: + key1: value1 + actions: + - method: delete + path: . + substitutions: + - dest: + path: .site.certificate + src: + schema: armada/Chart/v1 + name: armada-chart-01 + path: .region.certificate + - dest: + path: .site.certificatekey + src: + schema: armada/Chart/v1 + name: armada-chart-01 + path: .region.certificatekey + - dest: + path: .site.passphrase + src: + schema: armada/Chart/v1 + name: armada-chart-01 + path: .region.passphrase +data: + site: + certificate: placeholder + certificatekey: placeholder + passphrase: placeholder +--- +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: armada-chart-01 + labels: + key1: value1 + layeringDefinition: + abstract: false + layer: region + parentSelector: + key1: value1 + actions: + - method: merge + path: . + substitutions: + - dest: + path: .region.certificate + src: + schema: deckhand/Certificate/v1 + name: example-cert + path: . + - dest: + path: .region.certificatekey + src: + schema: deckhand/CertificateKey/v1 + name: example-key + path: . + - dest: + path: .region.passphrase + pattern: INSERT_[A-Z]+_HERE + src: + schema: deckhand/Passphrase/v1 + name: example-password + path: . +data: + region: + certificate: placeholder + certificatekey: placeholder + passphrase: http://admin:INSERT_PASSWORD_HERE@service-name:8080/v1 +... diff --git a/deckhand/tests/functional/gabbits/resources/layering-and-substitution-example.yaml b/deckhand/tests/functional/gabbits/resources/layering-and-substitution-sample.yaml similarity index 98% rename from deckhand/tests/functional/gabbits/resources/layering-and-substitution-example.yaml rename to deckhand/tests/functional/gabbits/resources/layering-and-substitution-sample.yaml index 25a9d66c..7bd6ad00 100644 --- a/deckhand/tests/functional/gabbits/resources/layering-and-substitution-example.yaml +++ b/deckhand/tests/functional/gabbits/resources/layering-and-substitution-sample.yaml @@ -24,6 +24,7 @@ metadata: name: example-password schema: metadata/Document/v1 layeringDefinition: + abstract: false layer: site storagePolicy: cleartext data: my-secret-password diff --git a/deckhand/tests/unit/engine/test_document_layering.py b/deckhand/tests/unit/engine/test_document_layering.py index 7186c73a..08fbb873 100644 --- a/deckhand/tests/unit/engine/test_document_layering.py +++ b/deckhand/tests/unit/engine/test_document_layering.py @@ -42,37 +42,48 @@ class TestDocumentLayering(test_base.DeckhandTestCase): continue layer = doc['metadata']['layeringDefinition']['layer'] if layer == 'site': - site_docs.append(doc) + site_docs.append(doc.get('data')) if layer == 'region': - region_docs.append(doc) + region_docs.append(doc.get('data')) if layer == 'global': - global_docs.append(doc) + global_docs.append(doc.get('data')) if site_expected is not None: if not isinstance(site_expected, list): site_expected = [site_expected] - for idx, expected in enumerate(site_expected): - self.assertEqual(expected, site_docs[idx].get('data'), + for expected in site_expected: + self.assertIn(expected, site_docs) + idx = site_docs.index(expected) + self.assertEqual(expected, site_docs[idx], 'Actual site data does not match expected.') + site_docs.remove(expected) else: self.assertEmpty(site_docs) + if region_expected is not None: if not isinstance(region_expected, list): region_expected = [region_expected] - for idx, expected in enumerate(region_expected): - self.assertEqual(expected, region_docs[idx].get('data'), + for expected in region_expected: + self.assertIn(expected, region_docs) + idx = region_docs.index(expected) + self.assertEqual(expected, region_docs[idx], 'Actual region data does not match expected.') + region_docs.remove(expected) else: self.assertEmpty(region_docs) + if global_expected is not None: if not isinstance(global_expected, list): global_expected = [global_expected] - for idx, expected in enumerate(global_expected): - self.assertEqual(expected, global_docs[idx].get('data'), + for expected in global_expected: + self.assertIn(expected, global_docs) + idx = global_docs.index(expected) + self.assertEqual(expected, global_docs[idx], 'Actual global data does not match expected.') + global_docs.remove(expected) else: self.assertEmpty(global_docs) @@ -310,8 +321,8 @@ class TestDocumentLayering2Layers2Sites2Globals(TestDocumentLayering): mapping, site_abstract=False, site_parent_selectors=[ {'global': 'global1'}, {'global': 'global2'}]) - site_expected = [{'a': {'x': 1, 'y': 2}, 'b': 4}, - {'a': {'x': 1, 'y': 2}, 'b': 3}] + site_expected = [{'a': {'x': 1, 'y': 2}, 'b': 3}, + {'a': {'x': 1, 'y': 2}, 'b': 4}] self._test_layering(documents, site_expected) def test_layering_two_parents_one_child_each_1(self): @@ -330,8 +341,8 @@ class TestDocumentLayering2Layers2Sites2Globals(TestDocumentLayering): mapping, site_abstract=False, site_parent_selectors=[ {'global': 'global1'}, {'global': 'global2'}]) - site_expected = [{'a': {'x': 1, 'y': 2}, 'b': 4}, - {'a': {'x': 1, 'y': 2}, 'b': 3}] + site_expected = [{'a': {'x': 1, 'y': 2}, 'b': 3}, + {'a': {'x': 1, 'y': 2}, 'b': 4}] self._test_layering(documents, site_expected) def test_layering_two_parents_one_child_each_2(self): @@ -359,8 +370,8 @@ class TestDocumentLayering2Layers2Sites2Globals(TestDocumentLayering): mapping, site_abstract=False, site_parent_selectors=[ {'global': 'global1'}, {'global': 'global2'}]) - site_expected = [{'a': {'x': 1, 'y': 2}, 'b': 4}, - {"b": {"f": -9, "g": 71}}] + site_expected = [{"b": {"f": -9, "g": 71}}, + {'a': {'x': 1, 'y': 2}, 'b': 4}] self._test_layering(documents, site_expected) diff --git a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py index dce8233b..bffdc464 100644 --- a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import itertools + import mock from deckhand import factories @@ -232,7 +234,6 @@ class TestDocumentLayeringWithSubstitution( "name": "global-cert", "path": "." } - }], "_SITE_DATA_1_": {"data": {"c": "need-site-secret"}}, "_SITE_ACTIONS_1_": { @@ -277,3 +278,96 @@ class TestDocumentLayeringWithSubstitution( expected_log_calls = [mock.call(expected_message, layer) for layer in ('empty_1', 'empty_2', 'empty_3')] mock_log.info.assert_has_calls(expected_log_calls) + + def test_layering_with_substitution_dependency_chain(self): + """Validate that parent with multiple children that substitute from + each other works no matter the order of the documents. + """ + mapping = { + "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, + "_GLOBAL_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".b" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "global-cert", + "path": "." + } + }], + "_SITE_NAME_1_": "site-1", + "_SITE_DATA_1_": {"data": {"c": "placeholder"}}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".c" + }, + "src": { + "schema": "deckhand/CertificateKey/v1", + "name": "site-cert", + "path": "." + } + }], + "_SITE_NAME_2_": "site-2", + "_SITE_DATA_2_": {"data": {"d": "placeholder"}}, + "_SITE_ACTIONS_2_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_2_": [{ + "dest": { + "path": ".d" + }, + "src": { + "schema": "example/Kind/v1", + "name": "site-1", + "path": ".c" + } + }], + "_SITE_NAME_3_": "site-3", + "_SITE_DATA_3_": {"data": {"e": "placeholder"}}, + "_SITE_ACTIONS_3_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_3_": [{ + "dest": { + "path": ".e" + }, + "src": { + "schema": "example/Kind/v1", + "name": "site-2", + "path": ".d" + } + }] + } + doc_factory = factories.DocumentFactory(2, [1, 3]) + documents = doc_factory.gen_test(mapping, site_abstract=False, + global_abstract=False) + secrets_factory = factories.DocumentSecretFactory() + + global_expected = {'a': {'x': 1, 'y': 2}, 'b': 'global-secret'} + site_expected = [ + {'a': {'x': 1, 'y': 2}, 'b': 'global-secret', 'c': 'site-secret'}, + {'a': {'x': 1, 'y': 2}, 'b': 'global-secret', 'd': 'site-secret'}, + {'a': {'x': 1, 'y': 2}, 'b': 'global-secret', 'e': 'site-secret'} + ] + + certificate = secrets_factory.gen_test( + 'Certificate', 'cleartext', data='global-secret', + name='global-cert') + certificate_key = secrets_factory.gen_test( + 'CertificateKey', 'cleartext', data='site-secret', + name='site-cert') + + # Pass in the documents in reverse order to ensure that the dependency + # chain by default is not linear and thus requires sorting. + self._test_layering( + list(reversed(documents)), site_expected=site_expected, + global_expected=global_expected, + substitution_sources=[certificate, certificate_key] + documents) + + # Try different permutations of document orders for good measure. + for document_order in list(itertools.permutations(documents))[:10]: + self._test_layering( + document_order, site_expected=site_expected, + global_expected=global_expected, + substitution_sources=[ + certificate, certificate_key] + documents) 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 new file mode 100644 index 00000000..089c25a6 --- /dev/null +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution_negative.py @@ -0,0 +1,115 @@ +# Copyright 2018 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. + +from deckhand.engine import layering +from deckhand import errors +from deckhand import factories +from deckhand.tests.unit.engine import test_document_layering + + +class TestDocumentLayeringWithSubstitutionNegative( + test_document_layering.TestDocumentLayering): + + def test_layering_with_substitution_cycle_fails(self): + """Validate that a substitution dependency cycle raises a critical + failure. + + In the case below, the cycle exists between + site-1 -> site-2 -> site-3 -> site-1 + """ + mapping = { + "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, + "_SITE_NAME_1_": "site-1", + "_SITE_DATA_1_": {"data": {"c": "placeholder"}}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".c" + }, + "src": { + "schema": "example/Kind/v1", + "name": "site-3", + "path": "." + } + }], + "_SITE_NAME_2_": "site-2", + "_SITE_DATA_2_": {"data": {"d": "placeholder"}}, + "_SITE_ACTIONS_2_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_2_": [{ + "dest": { + "path": ".d" + }, + "src": { + "schema": "example/Kind/v1", + "name": "site-1", + "path": ".c" + } + }], + "_SITE_NAME_3_": "site-3", + "_SITE_DATA_3_": {"data": {"e": "placeholder"}}, + "_SITE_ACTIONS_3_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_3_": [{ + "dest": { + "path": ".e" + }, + "src": { + "schema": "example/Kind/v1", + "name": "site-2", + "path": ".d" + } + }] + } + doc_factory = factories.DocumentFactory(2, [1, 3]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + + # Pass in the documents in reverse order to ensure that the dependency + # chain by default is not linear and thus requires sorting. + self.assertRaises( + errors.SubstitutionDependencyCycle, layering.DocumentLayering, + documents, substitution_sources=documents) + + def test_layering_with_substitution_self_reference_fails(self): + """Validate that a substitution self-reference fails. + + In the case below, a self-reference or cycle exists for site-1 with + itself. + """ + mapping = { + "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, + "_SITE_NAME_1_": "site-1", + "_SITE_DATA_1_": {"data": {"c": "placeholder"}}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]}, + "_SITE_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".c" + }, + "src": { + "schema": "example/Kind/v1", + "name": "site-1", + "path": "." + } + }] + } + doc_factory = factories.DocumentFactory(2, [1, 1]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + + # Pass in the documents in reverse order to ensure that the dependency + # chain by default is not linear and thus requires sorting. + self.assertRaises( + errors.SubstitutionDependencyCycle, self._test_layering, documents, + substitution_sources=documents) diff --git a/deckhand/tests/unit/engine/test_document_layering_negative.py b/deckhand/tests/unit/engine/test_document_layering_negative.py index 07d18b56..65d7d811 100644 --- a/deckhand/tests/unit/engine/test_document_layering_negative.py +++ b/deckhand/tests/unit/engine/test_document_layering_negative.py @@ -121,9 +121,10 @@ class TestDocumentLayeringNegative( def test_layering_duplicate_parent_selector_2_layer(self): # Validate that documents belonging to the same layer cannot have the # same unique parent identifier referenced by `parentSelector`. - doc_factory = factories.DocumentFactory(2, [1, 1]) + doc_factory = factories.DocumentFactory(2, [2, 1]) documents = doc_factory.gen_test({}, site_abstract=False) - documents.append(documents[1]) # Copy global layer. + # Make both global documents have the same exact labels. + documents[2]['metadata']['labels'] = documents[1]['metadata']['labels'] self.assertRaises(errors.IndeterminateDocumentParent, layering.DocumentLayering, documents) @@ -131,9 +132,10 @@ class TestDocumentLayeringNegative( def test_layering_duplicate_parent_selector_3_layer(self): # Validate that documents belonging to the same layer cannot have the # same unique parent identifier referenced by `parentSelector`. - doc_factory = factories.DocumentFactory(3, [1, 1, 1]) + doc_factory = factories.DocumentFactory(3, [1, 2, 1]) documents = doc_factory.gen_test({}, site_abstract=False) - documents.append(documents[2]) # Copy region layer. + # Make both region documents have the same exact labels. + documents[3]['metadata']['labels'] = documents[2]['metadata']['labels'] self.assertRaises(errors.IndeterminateDocumentParent, layering.DocumentLayering, documents) diff --git a/requirements.txt b/requirements.txt index ad9ab2cd..d2e7d792 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ PasteDeploy>=1.5.0 # MIT Paste # MIT Routes>=2.3.1 # MIT keystoneauth1>=3.2.0 # Apache-2.0 +networkx==2.1 six>=1.9.0 # MIT stevedore>=1.20.0 # Apache-2.0