From e4abca1cd725d2a5345023b3d56ca78958a2741e Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 3 Feb 2018 19:28:59 -0500 Subject: [PATCH] Use DAG to resolve substitution dependency chain Currently, Deckhand fails to perform substitution given a substitution dependency chain for a group of documents. That is, if A depends on B and C depends on B for substitution then substitution will fail. Deckhand, at present, can only perform substitution if A depends on B and C depends on B and D depends on B, for example: In this case, the dependency chain is no bigger than 2. However, for cases where the substitution dependency chain is larger than 2, then the dependency is no longer trivial. This is because the substitution dependencies form a DAG that must be topologically sorted in order to derive the correct order of substitution. Once the documents are correctly sorted according to this scheme, then substitution can be carried out as usual. This PS makes the aforementioned changes to Deckhand's layering module to make substitution work for non-cyclical dependencies: A DAG is used to topologically sort the documents according to their substitution dependency chain. A unit and functional test has been added to verify the code. If a cycle is detected, a critical error is thrown. Unit tests have been added to validate this case as well. Change-Id: Iaca3963f44aec6c897ad9fd690ce314a3a4d97a2 --- deckhand/engine/layering.py | 57 ++++++- deckhand/engine/secrets_manager.py | 9 +- deckhand/errors.py | 13 +- deckhand/factories.py | 16 +- ...bucket-with-layering-and-substitution.yaml | 86 +++++++++- .../layering-and-substitution-dag-sample.yaml | 147 ++++++++++++++++++ ... => layering-and-substitution-sample.yaml} | 1 + .../unit/engine/test_document_layering.py | 41 +++-- ...test_document_layering_and_substitution.py | 96 +++++++++++- ...ment_layering_and_substitution_negative.py | 115 ++++++++++++++ .../engine/test_document_layering_negative.py | 10 +- requirements.txt | 1 + 12 files changed, 558 insertions(+), 34 deletions(-) create mode 100644 deckhand/tests/functional/gabbits/resources/layering-and-substitution-dag-sample.yaml rename deckhand/tests/functional/gabbits/resources/{layering-and-substitution-example.yaml => layering-and-substitution-sample.yaml} (98%) create mode 100644 deckhand/tests/unit/engine/test_document_layering_and_substitution_negative.py 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