diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 9fc8976a..5cb6d55f 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -124,11 +124,11 @@ class RenderedDocumentsResource(api_base.BaseResource): # Filters to be applied post-rendering, because many documents are # involved in rendering. User filters can only be applied once all - # documents have been rendered. + # documents have been rendered. Note that `layering` module only + # returns concrete documents, so no filtering for that is needed here. order_by = sanitized_params.pop('order', None) sort_by = sanitized_params.pop('sort', None) user_filters = sanitized_params.copy() - user_filters['metadata.layeringDefinition.abstract'] = False rendered_documents = [ d for d in rendered_documents if utils.deepfilter( diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index cbfc1c4b..759bbf24 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -383,8 +383,7 @@ class DocumentLayering(object): the child, and its ``metadata.labels`` must much the child's ``metadata.layeringDefinition.parentSelector``. - :returns: The list of rendered documents (does not include layering - policy document). + :returns: The list of concrete rendered documents. :rtype: List[dict] :raises UnsupportedActionMethod: If the layering action isn't found @@ -458,4 +457,5 @@ class DocumentLayering(object): if substituted_data: doc = substituted_data[0] - return self._documents_to_layer + # Return only concrete documents. + return [d for d in self._documents_to_layer if d.is_abstract is False] diff --git a/deckhand/tests/unit/engine/test_document_layering.py b/deckhand/tests/unit/engine/test_document_layering.py index 528b3b63..7186c73a 100644 --- a/deckhand/tests/unit/engine/test_document_layering.py +++ b/deckhand/tests/unit/engine/test_document_layering.py @@ -48,27 +48,33 @@ class TestDocumentLayering(test_base.DeckhandTestCase): if layer == 'global': global_docs.append(doc) - if site_expected: + 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'), 'Actual site data does not match expected.') - if region_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'), 'Actual region data does not match expected.') - if global_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'), 'Actual global data does not match expected.') + else: + self.assertEmpty(global_docs) class TestDocumentLayering2Layers(TestDocumentLayering): @@ -122,7 +128,7 @@ class TestDocumentLayering2Layers(TestDocumentLayering): def test_layering_default_scenario_multi_parentselector_no_match(self): mapping = { "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, - "_SITE_DATA_1_": {"data": {"b": 4}}, + "_SITE_DATA_1_": {}, "_SITE_ACTIONS_1_": { "actions": [{"method": "merge", "path": "."}]} } @@ -241,8 +247,8 @@ class TestDocumentLayering2LayersAbstractConcrete(TestDocumentLayering): documents = doc_factory.gen_test(mapping, site_abstract=True, global_abstract=True) - site_expected = {"a": {"x": 7, "z": 3}, "b": 4} - global_expected = {'a': {'x': 1, 'y': 2}, 'c': 9} + site_expected = None + global_expected = None self._test_layering(documents, site_expected, global_expected=global_expected) @@ -375,7 +381,7 @@ class TestDocumentLayering3Layers(TestDocumentLayering): documents = doc_factory.gen_test(mapping, site_abstract=False) site_expected = {'a': {'z': 3}, 'b': 4} - region_expected = {"a": {"z": 3}} # Region is abstract. + region_expected = None # Region is abstract. self._test_layering(documents, site_expected, region_expected) def test_layering_delete_everything(self): @@ -538,11 +544,11 @@ class TestDocumentLayering3LayersAbstractConcrete(TestDocumentLayering): "actions": [{"method": "replace", "path": ".b"}]} } doc_factory = factories.DocumentFactory(3, [1, 1, 1]) - documents = doc_factory.gen_test(mapping, site_abstract=False, - region_abstract=True) + documents = doc_factory.gen_test( + mapping, site_abstract=False, region_abstract=True) site_expected = {"a": {"x": 1, "y": 2, "z": 3}, "b": 4} - region_expected = {"a": {"z": 3}, "b": 5, "c": 11} + region_expected = None self._test_layering(documents, site_expected, region_expected) def test_layering_site_region_and_global_concrete(self): @@ -739,13 +745,13 @@ class TestDocumentLayering3LayersScenario(TestDocumentLayering): --- metadata: labels: {name: kubernetes-etcd-global} - layeringDefinition: {abstract: true, layer: global} + layeringDefinition: {abstract: false, layer: global} name: kubernetes-etcd-global schema: metadata/Document/v1 storagePolicy: cleartext schema: armada/Chart/v1 data: - chart_name: etcd + chart_name: global-etcd --- # This document is included so that this middle layer isn't stripped away. metadata: @@ -785,7 +791,9 @@ data: ... """ documents = list(yaml.safe_load_all(test_yaml)) - self._test_layering(documents, site_expected={'chart_name': 'etcd'}) + self._test_layering( + documents, site_expected={'chart_name': 'global-etcd'}, + region_expected={}, global_expected={'chart_name': 'global-etcd'}) def test_layering_using_first_parent_as_actual_parent(self): """Test that layering works when a child document has layer N and has @@ -847,7 +855,8 @@ data: """ documents = list(yaml.safe_load_all(test_yaml)) self._test_layering( - documents, site_expected={'chart_name': 'region-etcd'}) + documents, site_expected={'chart_name': 'region-etcd'}, + region_expected={'chart_name': 'region-etcd'}) class TestDocumentLayering3Layers2Regions2Sites(TestDocumentLayering): @@ -884,8 +893,8 @@ class TestDocumentLayering3Layers2Regions2Sites(TestDocumentLayering): site_expected = [{"a": 1, "b": 2, "c": 3, "d": 4, "g": 7, "h": 8}, {"a": 1, "b": 2, "e": 5, "f": 6, "i": 9, "j": 10}] - region_expected = [{"c": 3, "d": 4}, {"e": 5, "f": 6}] - global_expected = {"a": 1, "b": 2} + region_expected = None + global_expected = None self._test_layering(documents, site_expected, region_expected, global_expected) @@ -923,6 +932,6 @@ class TestDocumentLayering3Layers2Regions2Sites(TestDocumentLayering): {"a": 1, "b": 2, "e": 5, "f": 6, "i": 9, "j": 10}] region_expected = [{"a": 1, "b": 2, "c": 3, "d": 4}, {"a": 1, "b": 2, "e": 5, "f": 6}] - global_expected = {"a": 1, "b": 2} + global_expected = None self._test_layering(documents, site_expected, region_expected, global_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 9de16451..dce8233b 100644 --- a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py @@ -41,7 +41,8 @@ class TestDocumentLayeringWithSubstitution( "actions": [{"method": "merge", "path": "."}]} } doc_factory = factories.DocumentFactory(2, [1, 1]) - documents = doc_factory.gen_test(mapping, site_abstract=False) + documents = doc_factory.gen_test(mapping, site_abstract=False, + global_abstract=False) secrets_factory = factories.DocumentSecretFactory() certificate = secrets_factory.gen_test( @@ -79,7 +80,8 @@ class TestDocumentLayeringWithSubstitution( "actions": [{"method": "merge", "path": "."}]} } doc_factory = factories.DocumentFactory(2, [1, 1]) - documents = doc_factory.gen_test(mapping, site_abstract=False) + documents = doc_factory.gen_test(mapping, site_abstract=False, + global_abstract=False) # Remove the labels from the global document so that the site document # (the child) has no parent. @@ -121,7 +123,8 @@ class TestDocumentLayeringWithSubstitution( "actions": [{"method": "merge", "path": "."}]} } doc_factory = factories.DocumentFactory(2, [1, 1]) - documents = doc_factory.gen_test(mapping, site_abstract=False) + documents = doc_factory.gen_test(mapping, site_abstract=False, + global_abstract=False) # Remove the labels from the global document so that the site document # (the child) has no parent. @@ -176,7 +179,8 @@ class TestDocumentLayeringWithSubstitution( }], } doc_factory = factories.DocumentFactory(2, [1, 1]) - documents = doc_factory.gen_test(mapping, site_abstract=False) + 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'} @@ -245,7 +249,8 @@ class TestDocumentLayeringWithSubstitution( }], } doc_factory = factories.DocumentFactory(2, [1, 1]) - documents = doc_factory.gen_test(mapping, site_abstract=False) + documents = doc_factory.gen_test(mapping, site_abstract=False, + global_abstract=False) documents[0]['data']['layerOrder'] = [ 'empty_1', 'empty_2', 'global', 'empty_3', 'site'] secrets_factory = factories.DocumentSecretFactory() diff --git a/deckhand/tests/unit/engine/test_document_layering_negative.py b/deckhand/tests/unit/engine/test_document_layering_negative.py index b3dd49a1..07d18b56 100644 --- a/deckhand/tests/unit/engine/test_document_layering_negative.py +++ b/deckhand/tests/unit/engine/test_document_layering_negative.py @@ -68,9 +68,10 @@ class TestDocumentLayeringNegative( @mock.patch.object(layering, 'LOG', autospec=True) def test_layering_with_empty_layer(self, mock_log): doc_factory = factories.DocumentFactory(1, [1]) - documents = doc_factory.gen_test({}, site_abstract=False) + documents = doc_factory.gen_test({}, global_abstract=False) - self._test_layering([documents[0]], global_expected={}) + # Only pass in the LayeringPolicy. + self._test_layering([documents[0]], global_expected=None) mock_log.info.assert_has_calls([ mock.call( '%s is an empty layer with no documents. It will be discarded ' @@ -161,11 +162,11 @@ class TestDocumentLayeringNegative( @mock.patch.object(layering, 'LOG', autospec=True) def test_multiple_layering_policy_logs_warning(self, mock_log): doc_factory = factories.DocumentFactory(1, [1]) - documents = doc_factory.gen_test({}, site_abstract=False) + documents = doc_factory.gen_test({}, global_abstract=False) # Copy the same layering policy so that 2 are passed in, causing a # warning to be raised. documents.append(documents[0]) - self._test_layering(documents, site_expected={}) + self._test_layering(documents, global_expected={}) mock_log.warning.assert_called_once_with( 'More than one layering policy document was passed in. Using the ' 'first one found: [%s] %s.', documents[0]['schema'],