diff --git a/deckhand/engine/document.py b/deckhand/engine/document.py index e9462475..b1e1a292 100644 --- a/deckhand/engine/document.py +++ b/deckhand/engine/document.py @@ -43,16 +43,22 @@ class Document(object): return False def get_schema(self): - return self._inner['schema'] + try: + return self._inner['schema'] + except Exception: + return '' def get_name(self): - return self._inner['metadata']['name'] + try: + return self._inner['metadata']['name'] + except Exception: + return '' def get_layer(self): try: return self._inner['metadata']['layeringDefinition']['layer'] except Exception: - return None + return '' def get_parent_selector(self): """Return the `parentSelector` for the document. @@ -60,24 +66,27 @@ class Document(object): The topmost document defined by the `layerOrder` in the LayeringPolicy does not have a `parentSelector` as it has no parent. - :returns: `parentSelcetor` for the document if present, else None. + :returns: `parentSelector` for the document if present, else None. """ try: return self._inner['metadata']['layeringDefinition'][ 'parentSelector'] - except KeyError: - return None + except Exception: + return {} def get_labels(self): return self._inner['metadata']['labels'] def get_substitutions(self): - return self._inner['metadata'].get('substitutions', None) + try: + return self._inner['metadata'].get('substitutions', []) + except Exception: + return [] def get_actions(self): try: return self._inner['metadata']['layeringDefinition']['actions'] - except KeyError: + except Exception: return [] def get_children(self, nested=False): diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 20a97883..c77096c8 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -68,7 +68,7 @@ class DocumentLayering(object): """ layered_docs = list( filter(lambda x: 'layeringDefinition' in x['metadata'], - self.documents)) + self._documents)) # ``all_children`` is a counter utility for verifying that each # document has exactly one parent. @@ -78,8 +78,8 @@ class DocumentLayering(object): children = [] doc_layer = doc.get_layer() try: - next_layer_idx = self.layer_order.index(doc_layer) + 1 - children_doc_layer = self.layer_order[next_layer_idx] + next_layer_idx = self._layer_order.index(doc_layer) + 1 + children_doc_layer = self._layer_order[next_layer_idx] except IndexError: # The lowest layer has been reached, so no children. Return # empty list. @@ -106,7 +106,7 @@ class DocumentLayering(object): return children - for layer in self.layer_order: + for layer in self._layer_order: docs_by_layer = list(filter( (lambda x: x.get_layer() == layer), layered_docs)) @@ -119,7 +119,7 @@ class DocumentLayering(object): all_children_elements = list(all_children.elements()) secondary_docs = list( - filter(lambda d: d.get_layer() != self.layer_order[0], + filter(lambda d: d.get_layer() != self._layer_order[0], layered_docs)) for doc in secondary_docs: # Unless the document is the topmost document in the @@ -130,6 +130,7 @@ class DocumentLayering(object): 'schema=%s, layer=%s, parentSelector=%s.', doc.get_name(), doc.get_schema(), doc.get_layer(), doc.get_parent_selector()) + self._parentless_documents.append(doc) # If the document is a child document of more than 1 parent, then # the document has too many parents, which is a validation error. elif all_children[doc] != 1: @@ -167,17 +168,18 @@ class DocumentLayering(object): sources for substitution. Should only include concrete documents. :type substitution_sources: List[dict] """ - self.layering_policy, self.documents = self._extract_layering_policy( + self._layering_policy, self._documents = self._extract_layering_policy( documents) - if self.layering_policy is None: + if self._layering_policy is None: error_msg = ( 'No layering policy found in the system so could not reder ' 'documents.') LOG.error(error_msg) raise errors.LayeringPolicyNotFound() - self.layer_order = list(self.layering_policy['data']['layerOrder']) - self.layered_docs = self._calc_document_children() - self.substitution_sources = substitution_sources or [] + self._layer_order = list(self._layering_policy['data']['layerOrder']) + self._parentless_documents = [] + self._layered_documents = self._calc_document_children() + self._substitution_sources = substitution_sources or [] def _apply_action(self, action, child_data, overall_data): """Apply actions to each layer that is rendered. @@ -257,7 +259,7 @@ class DocumentLayering(object): def _apply_substitutions(self, document): try: secrets_substitution = secrets_manager.SecretsSubstitution( - document, self.substitution_sources) + document, self._substitution_sources) return secrets_substitution.substitute_all() except errors.DocumentNotFound as e: LOG.error('Failed to render the documents because a secret ' @@ -284,11 +286,11 @@ class DocumentLayering(object): # NOTE(fmontei): ``global_docs`` represents the topmost documents in # the system. It should probably be impossible for more than 1 # top-level doc to exist, but handle multiple for now. - global_docs = [doc for doc in self.layered_docs - if doc.get_layer() == self.layer_order[0]] + global_docs = [doc for doc in self._layered_documents + if doc.get_layer() == self._layer_order[0]] for doc in global_docs: - layer_idx = self.layer_order.index(doc.get_layer()) + layer_idx = self._layer_order.index(doc.get_layer()) if doc.get_substitutions(): substituted_data = self._apply_substitutions(doc.to_dict()) rendered_data_by_layer[layer_idx] = substituted_data[0] @@ -299,7 +301,7 @@ class DocumentLayering(object): for child in doc.get_children(nested=True): # Retrieve the most up-to-date rendered_data (by # referencing the child's parent's data). - child_layer_idx = self.layer_order.index(child.get_layer()) + child_layer_idx = self._layer_order.index(child.get_layer()) rendered_data = rendered_data_by_layer[child_layer_idx - 1] # Apply each action to the current document. @@ -316,9 +318,13 @@ class DocumentLayering(object): if child.get_substitutions(): rendered_data['metadata'][ 'substitutions'] = child.get_substitutions() - self._apply_substitutions(rendered_data) - self.layered_docs[self.layered_docs.index(child)][ - 'data'] = rendered_data['data'] + substituted_data = self._apply_substitutions( + rendered_data) + if substituted_data: + rendered_data = substituted_data[0] + child_index = self._layered_documents.index(child) + self._layered_documents[child_index]['data'] = ( + rendered_data['data']) # Update ``rendered_data_by_layer`` for this layer so that # children in deeper layers can reference the most up-to-date @@ -328,7 +334,19 @@ class DocumentLayering(object): if 'children' in doc: del doc['children'] + # Handle edge case for parentless documents that require substitution. + # If a document has no parent, then the for loop above doesn't iterate + # over the parentless document, so substitution must be done here for + # parentless documents. + for doc in self._parentless_documents: + if not doc.is_abstract(): + substituted_data = self._apply_substitutions(doc.to_dict()) + if substituted_data: + # TODO(fmontei): Use property after implementing it in + # document wrapper class. + doc._inner = substituted_data[0] + return ( - [d.to_dict() for d in self.layered_docs] + - [self.layering_policy.to_dict()] + [d.to_dict() for d in self._layered_documents] + + [self._layering_policy.to_dict()] ) diff --git a/deckhand/errors.py b/deckhand/errors.py index aa52462c..8c0cee6c 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -187,6 +187,12 @@ class MissingDocumentKey(DeckhandException): code = 400 +class MissingDocumentPattern(DeckhandException): + msg_fmt = ("Missing document pattern %(pattern)s in %(data)s at path " + "%(path)s.") + code = 400 + + class UnsupportedActionMethod(DeckhandException): msg_fmt = ("Method in %(actions)s is invalid for document %(document)s.") code = 400 diff --git a/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-substitution.yaml b/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-substitution.yaml index 6fc967d0..0cd12468 100644 --- a/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-substitution.yaml +++ b/deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-substitution.yaml @@ -3,6 +3,11 @@ # 1. Purges existing data to ensure test isolation # 2. Adds initial documents from substitution sample of design doc # 3. Verifies fully substituted document data +# 4. Purges existing data to ensure test isolation +# 5. Adds initial documents from substitution sample of design doc using just +# a single layer in order to verify that substitution is carried out for +# this edge case +# 6. Verifies fully substituted document data (again) defaults: request_headers: @@ -43,3 +48,36 @@ tests: key: | KEY DATA some_url: http://admin:my-secret-password@service-name:8080/v1 + + - name: purge_again + desc: Begin testing from known state. + DELETE: /api/v1.0/revisions + status: 204 + response_headers: null + + - name: initialize_single_layer + desc: Create initial documents + PUT: /api/v1.0/buckets/mop/documents + status: 200 + data: <@resources/design-doc-substitution-sample-single-layer.yaml + + - name: verify_substitutions_single_layer + desc: Check for expected substitutions + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents + query_parameters: + schema: armada/Chart/v1 + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[*].metadata.name: example-chart-01 + $.[*].data: + chart: + details: + data: here + values: + tls: + certificate: | + CERTIFICATE DATA + key: | + KEY DATA + some_url: http://admin:my-secret-password@service-name:8080/v1 diff --git a/deckhand/tests/functional/gabbits/resources/design-doc-substitution-sample-single-layer.yaml b/deckhand/tests/functional/gabbits/resources/design-doc-substitution-sample-single-layer.yaml new file mode 100644 index 00000000..323e8c8c --- /dev/null +++ b/deckhand/tests/functional/gabbits/resources/design-doc-substitution-sample-single-layer.yaml @@ -0,0 +1,72 @@ +--- +schema: deckhand/LayeringPolicy/v1 +metadata: + schema: metadata/Control/v1 + name: layering-policy +data: + layerOrder: + - region + - site +--- +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 +--- +schema: armada/Chart/v1 +metadata: + name: example-chart-01 + schema: metadata/Document/v1 + layeringDefinition: + layer: site + substitutions: + - dest: + path: .chart.values.tls.certificate + src: + schema: deckhand/Certificate/v1 + name: example-cert + path: . + - dest: + path: .chart.values.tls.key + src: + schema: deckhand/CertificateKey/v1 + name: example-key + path: . + - dest: + path: .chart.values.some_url + pattern: INSERT_[A-Z]+_HERE + src: + schema: deckhand/Passphrase/v1 + name: example-password + path: . +data: + chart: + details: + data: here + values: + some_url: http://admin:INSERT_PASSWORD_HERE@service-name:8080/v1 +... 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 7608907f..7948c8fd 100644 --- a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py @@ -20,6 +20,7 @@ class TestDocumentLayeringWithSubstitution( test_document_layering.TestDocumentLayering): def test_layering_and_substitution_default_scenario(self): + # Validate that layering and substitution work together. mapping = { "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, "_GLOBAL_SUBSTITUTIONS_1_": [{ @@ -53,6 +54,8 @@ class TestDocumentLayeringWithSubstitution( substitution_sources=[certificate]) def test_layering_and_substitution_no_children(self): + # Validate that a document with no children still undergoes + # substitution. mapping = { "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, "_GLOBAL_SUBSTITUTIONS_1_": [{ @@ -73,6 +76,8 @@ class TestDocumentLayeringWithSubstitution( doc_factory = factories.DocumentFactory(2, [1, 1]) documents = doc_factory.gen_test(mapping, site_abstract=False) + # Remove the labels from the global document so that the site document + # (the child) has no parent. documents[1]['metadata']['labels'] = {} secrets_factory = factories.DocumentSecretFactory() certificate = secrets_factory.gen_test( @@ -86,6 +91,44 @@ class TestDocumentLayeringWithSubstitution( global_expected=global_expected, substitution_sources=[certificate]) + def test_layering_and_substitution_no_parent(self): + # Validate that a document with no parent undergoes substitution. + mapping = { + "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, + "_SITE_DATA_1_": {"data": {"b": 4}}, + "_SITE_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".c" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "site-cert", + "path": "." + } + + }], + # No layering should be applied as the document has no parent. + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]} + } + doc_factory = factories.DocumentFactory(2, [1, 1]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + + # Remove the labels from the global document so that the site document + # (the child) has no parent. + documents[1]['metadata']['labels'] = {} + secrets_factory = factories.DocumentSecretFactory() + certificate = secrets_factory.gen_test( + 'Certificate', 'cleartext', data='site-secret', + name='site-cert') + + global_expected = {'a': {'x': 1, 'y': 2}} + site_expected = {'b': 4, 'c': 'site-secret'} + + self._test_layering(documents, site_expected=site_expected, + global_expected=global_expected, + substitution_sources=[certificate]) + def test_layering_parent_and_child_undergo_substitution(self): mapping = { "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}},