Fix rendered documents not returning all concrete documents

Currently, the rendered-documents endpoint returns only documents
that require substitution, rather than all concrete documents, as
specified in the requirements (DECKHAND-65).

This PS adds a filter to the endpoint so that only concrete documents
are returned. Also, all concrete documents are returned, not just
the ones that require substitution.

Included in this PS:
  - logic changes described above
  - unit test to verify the above logic

Change-Id: Ib552b084bb00b6e180bba973be420449a292fb05
This commit is contained in:
Felipe Monteiro 2017-11-01 16:06:10 +00:00
parent 1ef4a78f02
commit 9d7604a949
4 changed files with 62 additions and 11 deletions

View File

@ -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')

View File

@ -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

View File

@ -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': '@',

View File

@ -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(