From fbfb9e79af8788f05ddda6e870687049ac45c5aa Mon Sep 17 00:00:00 2001
From: Felipe Monteiro <felipe.monteiro@att.com>
Date: Tue, 6 Mar 2018 18:03:43 -0500
Subject: [PATCH] Fix abstract parent documents substitutions not propagating

This is to fix abstract parent documents that have registered
substitutions but are not propagating those substitutions
down to concrete children that rely on that substituted data
(whereas it is irrelevant to the end user whether the abstract
parent receives those substitutions or not).

After this change abstract documents will always undergo
substitution such that their concrete children documents should
inherit those changes.

A unit test has been added for regression.

Change-Id: I73ed8d4caa6923492cc5ff042ecc57fbfeeb7c1c
---
 deckhand/engine/layering.py                   | 12 ++++--
 ...test_document_layering_and_substitution.py | 37 +++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py
index 62f399dd..585ede84 100644
--- a/deckhand/engine/layering.py
+++ b/deckhand/engine/layering.py
@@ -483,16 +483,20 @@ class DocumentLayering(object):
                     self._documents_by_index[(doc.schema, doc.name)] = (
                         rendered_data)
 
-            # Update the actual document data if concrete.
-            if not doc.is_abstract:
+            # Perform substitutions on abstract data for child documents that
+            # inherit from it, but only update the document's data if concrete.
+            if doc.substitutions:
                 substituted_data = list(
                     self.secrets_substitution.substitute_all(doc))
                 if substituted_data:
                     rendered_data = substituted_data[0]
-                    doc.data = rendered_data.data
+                    # Update the actual document data if concrete.
+                    if not doc.is_abstract:
+                        doc.data = rendered_data.data
                     self.secrets_substitution.update_substitution_sources(
                         doc.schema, doc.name, rendered_data.data)
-                    self._documents_by_index[(doc.schema, doc.name)] = doc
+                    self._documents_by_index[(doc.schema, doc.name)] = (
+                        rendered_data)
 
         # Return only concrete documents.
         return [d for d in self._sorted_documents if d.is_abstract is False]
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 63dca721..030ce0c3 100644
--- a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py
+++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py
@@ -676,3 +676,40 @@ class TestDocumentLayeringWithSubstitution(
                 global_expected=global_expected,
                 substitution_sources=[
                     certificate, certificate_key] + documents)
+
+    def test_layering_and_substitution_site_abstract_and_global_concrete(self):
+        """Verifies that if a global document is abstract, yet has
+        substitutions, those substitutions are performed and carry down to
+        concrete children that inherit from the abstract parent.
+        """
+        secrets_factory = factories.DocumentSecretFactory()
+        certificate = secrets_factory.gen_test(
+            'Certificate', 'cleartext', data='global-secret',
+            name='global-cert')
+
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"global": "random"}},
+            "_GLOBAL_SUBSTITUTIONS_1_": [{
+                "dest": {
+                    "path": ".cert"
+                },
+                "src": {
+                    "schema": certificate['schema'],
+                    "name": certificate['metadata']['name'],
+                    "path": "."
+                }
+            }],
+            "_SITE_DATA_1_": {"data": {"site": "stuff"}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "merge", "path": "."}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False,
+                                         global_abstract=True)
+
+        site_expected = {"global": "random", "cert": "global-secret",
+                         "site": "stuff"}
+        global_expected = None
+        self._test_layering(documents, site_expected,
+                            global_expected=global_expected,
+                            substitution_sources=[certificate])