Merge "Fix rendered documents not returning all concrete documents"

deckhand/control/revision_documents.py

@@ -90,6 +90,7 @@ class RenderedDocumentsResource(api_base.BaseResource):
             'deckhand:list_encrypted_documents', req.context, do_raise=False)
 
         filters = sanitized_params.copy()
+        filters['metadata.layeringDefinition.abstract'] = False
         filters['metadata.storagePolicy'] = ['cleartext']
         if include_encrypted:
             filters['metadata.storagePolicy'].append('encrypted')

deckhand/engine/secrets_manager.py

@@ -106,9 +106,16 @@ class SecretsSubstitution(object):
         """
         if not isinstance(documents, (list, tuple)):
             documents = [documents]
-        substitute_docs = [document_wrapper.Document(d) for d in documents if
-                           'substitutions' in d['metadata']]
-        self.documents = substitute_docs
+
+        self.docs_to_sub = []
+        self.other_docs = []
+
+        for document in documents:
+            doc = document_wrapper.Document(document)
+            if doc.get_substitutions():
+                self.docs_to_sub.append(doc)
+            else:
+                self.other_docs.append(document)
 
     def substitute_all(self):
         """Substitute all documents that have a `metadata.substitutions` field.
@@ -120,10 +127,11 @@ class SecretsSubstitution(object):
 
         :returns: List of fully substituted documents.
         """
-        LOG.debug('Substituting secrets for documents: %s', self.documents)
+        LOG.debug('Substituting secrets for documents: %s',
+                  self.docs_to_sub)
         substituted_docs = []
 
-        for doc in self.documents:
+        for doc in self.docs_to_sub:
             LOG.debug(
                 'Checking for substitutions in schema=%s, metadata.name=%s',
                 doc.get_name(), doc.get_schema())
@@ -152,4 +160,4 @@ class SecretsSubstitution(object):
                 doc['data'].update(substituted_data)
 
             substituted_docs.append(doc.to_dict())
-        return substituted_docs
+        return substituted_docs + self.other_docs

deckhand/tests/unit/control/test_rendered_documents_controller.py

@@ -21,13 +21,55 @@ from deckhand import factories
 from deckhand.tests.unit.control import base as test_base
 
 
+class TestRenderedDocumentsController(test_base.BaseControllerTest):
+
+    def test_list_rendered_documents_exclude_abstract(self):
+        rules = {'deckhand:list_cleartext_documents': '@',
+                 'deckhand:list_encrypted_documents': '@',
+                 'deckhand:create_cleartext_documents': '@'}
+        self.policy.set_rules(rules)
+
+        # Create 2 docs: one concrete, one abstract.
+        documents_factory = factories.DocumentFactory(2, [1, 1])
+        payload = documents_factory.gen_test(
+            {}, global_abstract=False, region_abstract=True)[1:]
+        concrete_doc = payload[0]
+
+        resp = self.app.simulate_put(
+            '/api/v1.0/buckets/mop/documents',
+            headers={'Content-Type': 'application/x-yaml'},
+            body=yaml.safe_dump_all(payload))
+        self.assertEqual(200, resp.status_code)
+        revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][
+            'revision']
+
+        # Verify that the concrete document is returned, but not the abstract
+        # one.
+        resp = self.app.simulate_get(
+            '/api/v1.0/revisions/%s/rendered-documents' % revision_id,
+            headers={'Content-Type': 'application/x-yaml'})
+        self.assertEqual(200, resp.status_code)
+
+        rendered_documents = list(yaml.safe_load_all(resp.text))
+        self.assertEqual(1, len(rendered_documents))
+        is_abstract = rendered_documents[0]['metadata']['layeringDefinition'][
+            'abstract']
+        self.assertFalse(is_abstract)
+        for key, value in concrete_doc.items():
+            if isinstance(value, dict):
+                self.assertDictContainsSubset(value,
+                                              rendered_documents[0][key])
+            else:
+                self.assertEqual(value, rendered_documents[0][key])
+
+
 class TestRenderedDocumentsControllerNegativeRBAC(
         test_base.BaseControllerTest):
     """Test suite for validating negative RBAC scenarios for rendered documents
     controller.
     """
 
-    def test_list_cleartext_revision_documents_insufficient_permissions(self):
+    def test_list_cleartext_rendered_documents_insufficient_permissions(self):
         rules = {'deckhand:list_cleartext_documents': 'rule:admin_api',
                  'deckhand:create_cleartext_documents': '@'}
         self.policy.set_rules(rules)
@@ -49,7 +91,7 @@ class TestRenderedDocumentsControllerNegativeRBAC(
             headers={'Content-Type': 'application/x-yaml'})
         self.assertEqual(403, resp.status_code)
 
-    def test_list_encrypted_revision_documents_insufficient_permissions(self):
+    def test_list_encrypted_rendered_documents_insufficient_permissions(self):
         rules = {'deckhand:list_cleartext_documents': '@',
                  'deckhand:list_encrypted_documents': 'rule:admin_api',
                  'deckhand:create_cleartext_documents': '@',

deckhand/tests/unit/engine/test_secrets_manager.py

@@ -90,13 +90,13 @@ class TestSecretsSubstitution(test_base.TestDbBase):
         documents = self.create_documents(
             bucket_name, secret_documents + [payload[-1]])
 
-        expected_documents = copy.deepcopy([documents[-1]])
-        expected_documents[0]['data'] = expected_data
+        expected_document = copy.deepcopy(documents[-1])
+        expected_document['data'] = expected_data
 
         secret_substitution = secrets_manager.SecretsSubstitution(documents)
         substituted_docs = secret_substitution.substitute_all()
 
-        self.assertEqual(expected_documents, substituted_docs)
+        self.assertIn(expected_document, substituted_docs)
 
     def test_secret_substitution_single_cleartext(self):
         certificate = self.secrets_factory.gen_test(