airship
/
deckhand
Code Issues Proposed changes

Fix: Allow generic documents to be used as substitution sources. 

This PS fixes a bug related to Deckhand only using "secret"
document types to be used as substitution sources; the substitution
logic should be made generic, because it shouldn't just apply to
secrets.

This entailed removing the "is_secret" database column from the
Document table as it's no longer needed and dropping it from a DB
query made to find the source document for substitution in the
secrets_manager module.

This PS also increased resiliency via exception handling and some
edge cases surrounding substitution.

Finally, unit tests and functional tests were added to validate
substitition using a generic document as the source.

Change-Id: I2c4b49b2eb55473c56b8253a456803e793b0b0b0
This commit is contained in:
Felipe Monteiro 2018-01-03 19:44:20 +00:00
parent 69db7f81fa
commit 4b70927bb2
13 changed files with 212 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
deckhand/control/revision_documents.py
View File

@ -115,7 +115,8 @@ class RenderedDocumentsResource(api_base.BaseResource):
errors.UnsupportedActionMethod,
errors.MissingDocumentKey) as e:
raise falcon.HTTPBadRequest(description=e.format_message())
except errors.LayeringPolicyNotFound as e:
except (errors.LayeringPolicyNotFound,
errors.SubstitutionDependencyNotFound) as e:
raise falcon.HTTPConflict(description=e.format_message())
# Filters to be applied post-rendering, because many documents are

2
deckhand/db/sqlalchemy/api.py
View File

@ -246,8 +246,6 @@ def _documents_create(bucket_name, values_list, session=None):
for values in values_list:
values.setdefault('data', {})
values = _fill_in_metadata_defaults(values)
values['is_secret'] = values['schema'].startswith(
types.DOCUMENT_SECRET_TYPES)
# Hash the document's metadata and data to later efficiently check
# whether those data have changed.

1
deckhand/db/sqlalchemy/models.py
View File

@ -135,7 +135,6 @@ class Document(BASE, DeckhandBase):
data = Column(JSONB, nullable=True)
data_hash = Column(String, nullable=False)
metadata_hash = Column(String, nullable=False)
is_secret = Column(Boolean, nullable=False, default=False)
bucket_id = Column(Integer, ForeignKey('buckets.id', ondelete='CASCADE'),
nullable=False)
revision_id = Column(

11
deckhand/engine/document.py
View File

@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import six
class Document(object):
"""Object wrapper for documents.
@ -40,9 +38,7 @@ class Document(object):
concrete.
"""
try:
abstract = self._inner['metadata']['layeringDefinition'][
'abstract']
return six.text_type(abstract) == 'True'
return self._inner['metadata']['layeringDefinition']['abstract']
except KeyError:
return False
@ -53,7 +49,10 @@ class Document(object):
return self._inner['metadata']['name']
def get_layer(self):
return self._inner['metadata']['layeringDefinition']['layer']
try:
return self._inner['metadata']['layeringDefinition']['layer']
except Exception:
return None
def get_parent_selector(self):
"""Return the `parentSelector` for the document.

19
deckhand/engine/secrets_manager.py
View File

@ -13,10 +13,12 @@
# limitations under the License.
from oslo_log import log as logging
import six
from deckhand.barbican import driver
from deckhand.db.sqlalchemy import api as db_api
from deckhand.engine import document as document_wrapper
from deckhand import errors
from deckhand import utils
LOG = logging.getLogger(__name__)
@ -141,7 +143,7 @@ class SecretsSubstitution(object):
# TODO(fmontei): Use SecretsManager for this logic. Need to
# check Barbican for the secret if it has been encrypted.
src_doc = db_api.document_get(
schema=src_schema, name=src_name, is_secret=True,
schema=src_schema, name=src_name,
**{'metadata.layeringDefinition.abstract': False})
# If the data is a dictionary, retrieve the nested secret
@ -159,9 +161,18 @@ class SecretsSubstitution(object):
LOG.debug('Substituting from schema=%s name=%s src_path=%s '
'into dest_path=%s, dest_pattern=%s', src_schema,
src_name, src_path, dest_path, dest_pattern)
substituted_data = utils.jsonpath_replace(
doc['data'], src_secret, dest_path, dest_pattern)
doc['data'].update(substituted_data)
try:
substituted_data = utils.jsonpath_replace(
doc['data'], src_secret, dest_path, dest_pattern)
if isinstance(substituted_data, dict):
doc['data'].update(substituted_data)
else:
doc['data'] = substituted_data
except Exception as e:
LOG.error('Unexpected exception occurred while attempting '
'secret substitution. %s', six.text_type(e))
raise errors.SubstitutionDependencyNotFound(
details=six.text_type(e))
substituted_docs.append(doc.to_dict())
return substituted_docs

53
deckhand/errors.py
View File

@ -182,20 +182,6 @@ class InvalidDocumentSchema(DeckhandException):
code = 400
class DocumentExists(DeckhandException):
msg_fmt = ("Document with schema %(schema)s and metadata.name "
"%(name)s already exists in bucket %(bucket)s.")
code = 409
class SingletonDocumentConflict(DeckhandException):
msg_fmt = ("A singleton document by the name %(document)s already "
"exists in the system. The new document %(conflict)s cannot be "
"created. To create a document with a new name, delete the "
"current one first.")
code = 409
class IndeterminateDocumentParent(DeckhandException):
msg_fmt = ("Too many parent documents found for document %(document)s.")
code = 400
@ -204,6 +190,7 @@ class IndeterminateDocumentParent(DeckhandException):
class MissingDocumentKey(DeckhandException):
msg_fmt = ("Missing document key %(key)s from either parent or child. "
"Parent: %(parent)s. Child: %(child)s.")
code = 400
class UnsupportedActionMethod(DeckhandException):
@ -211,6 +198,12 @@ class UnsupportedActionMethod(DeckhandException):
code = 400
class RevisionTagBadFormat(DeckhandException):
msg_fmt = ("The requested tag data %(data)s must either be null or "
"dictionary.")
code = 400
class DocumentNotFound(DeckhandException):
msg_fmt = ("The requested document %(document)s was not found.")
code = 404
@ -227,11 +220,6 @@ class RevisionTagNotFound(DeckhandException):
code = 404
class LayeringPolicyNotFound(DeckhandException):
msg_fmt = ("Required LayeringPolicy was not found for layering.")
code = 409
class ValidationNotFound(DeckhandException):
msg_fmt = ("The requested validation entry %(entry_id)s was not found "
"for validation name %(validation_name)s and revision ID "
@ -239,10 +227,29 @@ class ValidationNotFound(DeckhandException):
code = 404
class RevisionTagBadFormat(DeckhandException):
msg_fmt = ("The requested tag data %(data)s must either be null or "
"dictionary.")
code = 400
class DocumentExists(DeckhandException):
msg_fmt = ("Document with schema %(schema)s and metadata.name "
"%(name)s already exists in bucket %(bucket)s.")
code = 409
class SingletonDocumentConflict(DeckhandException):
msg_fmt = ("A singleton document by the name %(document)s already "
"exists in the system. The new document %(conflict)s cannot be "
"created. To create a document with a new name, delete the "
"current one first.")
code = 409
class LayeringPolicyNotFound(DeckhandException):
msg_fmt = ("Required LayeringPolicy was not found for layering.")
code = 409
class SubstitutionDependencyNotFound(DeckhandException):
msg_fmt = ('Failed to find a dependent source document required for '
'substitution. Details: %(details)s')
code = 409
class BarbicanException(DeckhandException):

37
deckhand/tests/functional/gabbits/document-render-success-single-bucket-with-substitution-generic.yaml Normal file
View File

@ -0,0 +1,37 @@
# Tests success path for using a generic document as a substitution source.
# In this case, the DataSchema-registered document unusual/DictWithSecret/v1
# is used as the source.
#
# 1. Purges existing data to ensure test isolation
# 2. Adds initial documents from substitution sample of design doc
# 3. Verifies fully substituted document data
defaults:
request_headers:
content-type: application/x-yaml
response_headers:
content-type: application/x-yaml
tests:
- name: purge
desc: Begin testing from known state.
DELETE: /api/v1.0/revisions
status: 204
response_headers: null
- name: initialize
desc: Create initial documents
PUT: /api/v1.0/buckets/mop/documents
status: 200
data: <@resources/design-doc-substitution-generic-sample.yaml
- name: verify_substitutions
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
$.[0].metadata.name: example-chart-01
$.[0].data: secret-from-generic-document

54
deckhand/tests/functional/gabbits/resources/design-doc-substitution-generic-sample.yaml Normal file
View File

@ -0,0 +1,54 @@
---
schema: deckhand/LayeringPolicy/v1
metadata:
schema: metadata/Control/v1
name: layering-policy
data:
layerOrder:
- region
- site
---
schema: deckhand/DataSchema/v1
metadata:
schema: metadata/Control/v1
name: unusual/DictWithSecret/v1
data:
$schema: http://json-schema.org/schema#
type: object
properties:
secret:
type: string
public:
type: string
additionalProperties: false
required:
- secret
- public
---
schema: unusual/DictWithSecret/v1
metadata:
schema: metadata/Document/v1
name: dict-with-secret
storagePolicy: cleartext
layeringDefinition:
abstract: false
layer: site
data:
secret: secret-from-generic-document
public: random
---
schema: armada/Chart/v1
metadata:
name: example-chart-01
schema: metadata/Document/v1
layeringDefinition:
layer: region
substitutions:
- dest:
path: .
src:
schema: unusual/DictWithSecret/v1
name: dict-with-secret
path: .secret
data: need secret from unusual/DictWithSecret/v1
...

3
deckhand/tests/unit/db/test_documents.py
View File

@ -158,7 +158,6 @@ class TestDocuments(base.TestDbBase):
self.assertIn('Certificate', created_documents[-1]['schema'])
self.assertEqual(storage_policy, created_documents[-1][
'metadata']['storagePolicy'])
self.assertTrue(created_documents[-1]['is_secret'])
self.assertEqual(rand_secret, created_documents[-1]['data'])
def test_create_certificate_key(self):
@ -176,7 +175,6 @@ class TestDocuments(base.TestDbBase):
self.assertIn('CertificateKey', created_documents[-1]['schema'])
self.assertEqual(storage_policy, created_documents[-1][
'metadata']['storagePolicy'])
self.assertTrue(created_documents[-1]['is_secret'])
self.assertEqual(rand_secret, created_documents[-1]['data'])
def test_create_passphrase(self):
@ -194,7 +192,6 @@ class TestDocuments(base.TestDbBase):
self.assertIn('Passphrase', created_documents[-1]['schema'])
self.assertEqual(storage_policy, created_documents[-1][
'metadata']['storagePolicy'])
self.assertTrue(created_documents[-1]['is_secret'])
self.assertEqual(rand_secret, created_documents[-1]['data'])
def test_delete_document(self):

6
deckhand/tests/unit/engine/test_document_layering_and_substitution.py
View File

@ -58,7 +58,7 @@ class TestDocumentLayeringWithSubstitution(
global_expected=global_expected)
mock_document_get.assert_called_once_with(
schema=certificate['schema'], name=certificate['metadata']['name'],
is_secret=True, **{'metadata.layeringDefinition.abstract': False})
**{'metadata.layeringDefinition.abstract': False})
def test_layering_and_substitution_no_children(self):
mapping = {
@ -97,7 +97,7 @@ class TestDocumentLayeringWithSubstitution(
global_expected=global_expected)
mock_document_get.assert_called_once_with(
schema=certificate['schema'], name=certificate['metadata']['name'],
is_secret=True, **{'metadata.layeringDefinition.abstract': False})
**{'metadata.layeringDefinition.abstract': False})
def test_layering_parent_and_child_undergo_substitution(self):
mapping = {
@ -152,10 +152,8 @@ class TestDocumentLayeringWithSubstitution(
mock_document_get.assert_has_calls([
mock.call(
schema="deckhand/Certificate/v1", name='global-cert',
is_secret=True,
**{'metadata.layeringDefinition.abstract': False}),
mock.call(
schema="deckhand/CertificateKey/v1", name='site-cert',
is_secret=True,
**{'metadata.layeringDefinition.abstract': False})
])

41
deckhand/tests/unit/engine/test_secrets_manager.py
View File

@ -282,3 +282,44 @@ class TestSecretsSubstitution(test_base.TestDbBase):
self._test_secret_substitution(
document_mapping, [certificate, certificate_key, passphrase],
expected_data)
def test_substitution_with_generic_document_as_source(self):
src_data = 'data-from-generic-document'
# Create DataSchema document to register generic source document.
dataschema_factory = factories.DataSchemaFactory()
dataschema = dataschema_factory.gen_test(
'unusual/DictWithSecret/v1', {})
# Create the generic source document from which data will be extracted.
generic_document_mapping = {
"_GLOBAL_DATA_1_": {
'data': {'public': 'random', 'money': src_data}
}
}
payload = self.document_factory.gen_test(generic_document_mapping,
global_abstract=False)
payload[-1]['schema'] = "unusual/DictWithSecret/v1"
payload[-1]['metadata']['name'] = 'dict-with-secret'
# Store both documents to be created by helper.
dependent_documents = [payload[-1], dataschema]
# Mapping for destination document.
document_mapping = {
"_GLOBAL_DATA_1_": {
'data': {}
},
"_GLOBAL_SUBSTITUTIONS_1_": [{
"dest": {
"path": "."
},
"src": {
"schema": "unusual/DictWithSecret/v1",
"name": "dict-with-secret",
"path": ".money"
}
}]
}
self._test_secret_substitution(
document_mapping, dependent_documents, expected_data=src_data)

12
deckhand/utils.py
View File

@ -13,6 +13,7 @@
# limitations under the License.
import ast
import copy
import re
import string
@ -62,7 +63,9 @@ def jsonpath_parse(data, jsonpath, match_all=False):
src_secret = utils.jsonpath_parse(src_doc['data'], src_path)
# Do something with the extracted secret from the source document.
"""
if jsonpath.startswith('.'):
if jsonpath == '.':
jsonpath = '$'
elif jsonpath.startswith('.'):
jsonpath = '$' + jsonpath
p = jsonpath_ng.parse(jsonpath)
@ -104,8 +107,11 @@ def jsonpath_replace(data, value, jsonpath, pattern=None):
# http://admin:super-duper-secret@svc-name:8080/v1
doc['data'].update(replaced_data)
"""
data = data.copy()
if jsonpath.startswith('.'):
data = copy.copy(data)
if jsonpath == '.':
jsonpath = '$'
elif jsonpath.startswith('.'):
jsonpath = '$' + jsonpath
def _do_replace():

21
doc/source/substitution.rst
View File

@ -16,13 +16,23 @@
.. _substitution:
Secret Substitution
===================
Document Substitution
=====================
Document substitution, simply put, allows one document to overwrite *parts* of
its own data with that of another document. Substitution involves a source
document sharing data with a destination document, which replaces its own data
with the shared data.
Substitution is primarily designed as a mechanism for inserting secrets into
configuration documents, but works for unencrypted source documents as well.
Substitution is applied at each layer after all merge actions occur. Further,
substitution is only applied to the ``data`` section of a document.
Substitution is applied at each layer after all merge actions occur.
.. note::
Substitution is only applied to the ``data`` section of a document. This is
because a document's ``metadata`` and ``schema`` sections should be
immutable within the scope of a revision, for obvious reasons.
Concrete (non-abstract) documents can be used as a source of substitution
into other documents. This substitution is layer-independent, so given the 3
@ -138,3 +148,6 @@ The rendered document will look like:
key: |
KEY DATA
...
This substitution is also ``schema`` agnostic, meaning that source and
destination documents can have a different ``schema``.