airship
/
deckhand
Code Issues Proposed changes

Engine implementation for document replacement 

This adds support for document replacement to the
Deckhand engine _only_ for the following scenarios
_only_:

  * generic case (a replaces b, returns a only)
  * substitution case (a replaces b, c substitutes from a instead)

TODO:

  * layering case (a replaces b, c layers with a instead)
  * Modify Document unique constraint to work with
    (schema, name, layer) throughout all of Deckhand
    (including controllers, database models, and anywhere
     else as needed)

Change-Id: Ie2cea2a49ba3b9ebc42706fbe1060d94db2e5daa
This commit is contained in:
Felipe Monteiro 2018-03-14 19:41:38 +00:00 committed by Scott Hussey
parent 1264e5af6c
commit 4799acdbcc
7 changed files with 324 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
deckhand/common/document.py
View File

@ -32,6 +32,10 @@ class DocumentDict(dict):
"""
def __init__(self, *args, **kwargs):
super(DocumentDict, self).__init__(*args, **kwargs)
self._replaced_by = None
@classmethod
def from_dict(self, documents):
"""Convert a list of documents or single document into an instance of
@ -123,6 +127,22 @@ class DocumentDict(dict):
def is_encrypted(self):
return self.storage_policy == 'encrypted'
@property
def is_replacement(self):
return utils.jsonpath_parse(self, 'metadata.replacement') is True
@property
def has_replacement(self):
return isinstance(self._replaced_by, DocumentDict)
@property
def replaced_by(self):
return self._replaced_by
@replaced_by.setter
def replaced_by(self, other):
self._replaced_by = other
def __hash__(self):
return hash(json.dumps(self, sort_keys=True))

1
deckhand/control/revision_documents.py
View File

@ -116,6 +116,7 @@ class RenderedDocumentsResource(api_base.BaseResource):
rendered_documents = document_layering.render()
except (errors.InvalidDocumentLayer,
errors.InvalidDocumentParent,
errors.InvalidDocumentReplacement,
errors.IndeterminateDocumentParent,
errors.MissingDocumentKey,
errors.UnsupportedActionMethod) as e:

138
deckhand/engine/layering.py
View File

@ -55,23 +55,73 @@ class DocumentLayering(object):
_SUPPORTED_METHODS = (_MERGE_ACTION, _REPLACE_ACTION, _DELETE_ACTION) = (
'merge', 'replace', 'delete')
def _calc_replacements_and_substitutions(
self, substitution_sources):
for document in self._documents_by_index.values():
if document.is_replacement:
parent_meta = self._parents.get(document.meta)
parent = self._documents_by_index.get(parent_meta)
if not parent_meta or not parent:
error_message = (
'Document replacement requires that the document with '
'`replacement: true` have a parent.')
raise errors.InvalidDocumentReplacement(
schema=document.schema, name=document.name,
layer=document.layer, reason=error_message)
# This checks that a document can only be a replacement for
# another document with the same `metadata.name` and `schema`.
if (document.schema == parent.schema and
document.name == parent.name):
parent.replaced_by = document
else:
error_message = (
'Document replacement requires that both documents '
'have the same `schema` and `metadata.name`.')
raise errors.InvalidDocumentReplacement(
schema=document.schema, name=document.name,
layer=document.layer, reason=error_message)
# Since a substitution source only provides the document's
# `metadata.name` and `schema`, their tuple acts as the dictionary key.
# If a substitution source has a replacement, the replacement is used
# instead.
substitution_source_map = {}
for src in substitution_sources:
src_ref = document_wrapper.DocumentDict(src)
if src_ref.meta in self._documents_by_index:
src_ref = self._documents_by_index[src_ref.meta]
# If the document has a replacement, use the replacement as the
# substitution source instead.
if src_ref.has_replacement:
if src_ref.is_replacement:
error_message = ('A replacement document cannot itself'
' be replaced by another document.')
raise errors.InvalidDocumentReplacement(
schema=src_ref.schema, name=src_ref.name,
layer=src_ref.layer, reason=error_message)
src_ref = src_ref.replaced_by
substitution_source_map[(src_ref.schema, src_ref.name)] = src_ref
return substitution_source_map
def _replace_older_parent_with_younger_parent(self, child, parent,
all_children):
# If child has layer N, parent N+1, and current_parent N+2, then swap
# parent with current_parent. In other words, if parent's layer is
# closer to child's layer than current_parent's layer, then use parent.
current_parent_index = self._parents.get((child.schema, child.name))
current_parent = self._documents_by_index.get(
current_parent_index, None)
parent_meta = self._parents.get(child.meta)
current_parent = self._documents_by_index.get(parent_meta, None)
if current_parent:
if (self._layer_order.index(parent.layer) >
self._layer_order.index(current_parent.layer)):
self._parents[(child.schema, child.name)] = \
(parent.schema, parent.name)
self._parents[child.meta] = parent.meta
all_children[child] -= 1
else:
self._parents.setdefault((child.schema, child.name),
(parent.schema, parent.name))
self._parents.setdefault(child.meta, parent.meta)
def _is_actual_child_document(self, document, potential_child):
if document == potential_child:
@ -213,26 +263,29 @@ class DocumentLayering(object):
'will be performed.')
return layer_order
def _topologically_sort_documents(self, documents):
def _topologically_sort_documents(self, substitution_sources):
"""Topologically sorts the DAG formed from the documents' layering
and substitution dependency chain.
"""
documents_by_name = {}
result = []
g = networkx.DiGraph()
for document in documents:
document = document_wrapper.DocumentDict(document)
documents_by_name.setdefault((document.schema, document.name),
document)
for document in self._documents_by_index.values():
if document.parent_selector:
parent = self._parents.get((document.schema, document.name))
parent = self._parents.get(document.meta)
if parent:
g.add_edge((document.schema, document.name), parent)
g.add_edge(document.meta, parent)
for sub in document.substitutions:
g.add_edge((document.schema, document.name),
(sub['src']['schema'], sub['src']['name']))
# Retrieve the correct substitution source using
# ``substitution_sources``. Necessary for 2 reasons:
# 1) It accounts for document replacements.
# 2) It effectively maps a 2-tuple key to a 3-tuple document
# unique identifier (meta).
src = substitution_sources.get(
(sub['src']['schema'], sub['src']['name']))
if src:
g.add_edge(document.meta, src.meta)
try:
cycle = find_cycle(g)
@ -245,11 +298,12 @@ class DocumentLayering(object):
sorted_documents = reversed(list(topological_sort(g)))
for document in sorted_documents:
if document in documents_by_name:
result.append(documents_by_name.pop(document))
for document in documents_by_name.values():
result.append(document)
for document_meta in sorted_documents:
if document_meta in self._documents_by_index:
result.append(self._documents_by_index[document_meta])
for document in self._documents_by_index.values():
if document not in result:
result.append(document)
return result
@ -311,6 +365,8 @@ class DocumentLayering(object):
self._sorted_documents = {}
self._documents_by_index = {}
substitution_sources = substitution_sources or []
# TODO(fmontei): Add a hook for post-validation too.
if validate:
self._pre_validate_documents(documents)
@ -336,8 +392,9 @@ class DocumentLayering(object):
for document in documents:
document = document_wrapper.DocumentDict(document)
self._documents_by_index.setdefault(
(document.schema, document.name), document)
self._documents_by_index.setdefault(document.meta, document)
if document.layer:
if document.layer not in self._layering_policy.layer_order:
LOG.error('Document layer %s for document [%s] %s not '
@ -375,11 +432,15 @@ class DocumentLayering(object):
if not d.is_abstract
]
substitution_sources = self._calc_replacements_and_substitutions(
substitution_sources)
self.secrets_substitution = secrets_manager.SecretsSubstitution(
substitution_sources,
fail_on_missing_sub_src=fail_on_missing_sub_src)
self._sorted_documents = self._topologically_sort_documents(documents)
self._sorted_documents = self._topologically_sort_documents(
substitution_sources)
del self._documents_by_layer
del self._documents_by_labels
@ -494,30 +555,31 @@ class DocumentLayering(object):
continue
if doc.parent_selector:
parent_meta = self._parents.get((doc.schema, doc.name))
parent_meta = self._parents.get(doc.meta)
if parent_meta:
parent = self._documents_by_index[parent_meta]
if doc.actions:
rendered_data = parent
# Apply each action to the current document.
for action in doc.actions:
LOG.debug('Applying action %s to document with '
'name=%s, schema=%s, layer=%s.', action,
doc.name, doc.schema, doc.layer)
'schema=%s, name=%s, layer=%s.', action,
*doc.meta)
rendered_data = self._apply_action(
action, doc, rendered_data)
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)] = (
rendered_data)
self._documents_by_index[doc.meta] = rendered_data
else:
LOG.info('Skipped layering for document [%s] %s which '
'has a parent [%s] %s, but no associated '
'layering actions.', doc.schema, doc.name,
parent.schema, parent.name)
LOG.debug(
'Skipped layering for document [%s, %s] %s which '
'has a parent [%s, %s] %s, but no associated '
'layering actions.', doc.schema, doc.layer,
doc.name, parent.schema, parent.layer, parent.name)
# Perform substitutions on abstract data for child documents that
# inherit from it, but only update the document's data if concrete.
@ -531,11 +593,11 @@ class DocumentLayering(object):
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)] = (
rendered_data)
self._documents_by_index[doc.meta] = rendered_data
# Return only concrete documents.
return [d for d in self._sorted_documents if d.is_abstract is False]
# Return only concrete documents and non-replacements.
return [d for d in self._sorted_documents
if d.is_abstract is False and d.has_replacement is False]
@property
def documents(self):

21
deckhand/engine/secrets_manager.py
View File

@ -214,8 +214,9 @@ class SecretsSubstitution(object):
the constructor.
:param substitution_sources: List of documents that are potential
sources for substitution. Should only include concrete documents.
:type substitution_sources: List[dict]
sources for substitution. Or dict of documents keyed on tuple of
(schema, metadata.name). Should only include concrete documents.
:type substitution_sources: List[dict] or dict
:param bool fail_on_missing_sub_src: Whether to fail on a missing
substitution source. Default is True.
"""
@ -227,12 +228,16 @@ class SecretsSubstitution(object):
self._substitution_sources = {}
self._fail_on_missing_sub_src = fail_on_missing_sub_src
for document in substitution_sources:
if not isinstance(document, document_wrapper.DocumentDict):
document = document_wrapper.DocumentDict(document)
if document.schema and document.name:
self._substitution_sources.setdefault(
(document.schema, document.name), document)
if isinstance(substitution_sources, dict):
self._substitution_sources = substitution_sources
else:
self._substitution_sources = dict()
for document in substitution_sources:
if not isinstance(document, document_wrapper.DocumentDict):
document = document_wrapper.DocumentDict(document)
if document.schema and document.name:
self._substitution_sources.setdefault(
(document.schema, document.name), document)
def substitute_all(self, documents):
"""Substitute all documents that have a `metadata.substitutions` field.

16
deckhand/errors.py
View File

@ -261,6 +261,22 @@ class MissingDocumentPattern(DeckhandException):
code = 400
class InvalidDocumentReplacement(DeckhandException):
"""The document replacement is invalid.
**Troubleshoot:**
* Check that the replacement document has the same ``schema`` and
``metadata.name`` as the document it replaces.
* Check that the document with ``replacement: true`` has a parent.
* Check that the document replacement isn't being replaced by another
document. Only one level of replacement is permitted.
"""
msg_fmt = ("Replacement document [%(schema)s, %(layer)s] %(name)s is "
"invalid. Reason: %(reason)s")
code = 400
class UnsupportedActionMethod(DeckhandException):
"""The action is not in the list of supported methods.

112
deckhand/tests/unit/engine/test_document_layering.py
View File

@ -304,11 +304,8 @@ data:
site_expected = 'should not change'
self._test_layering(documents, site_expected, global_expected=None)
mock_log.info.assert_called_once_with(
'Skipped layering for document [%s] %s which has a parent [%s] '
'%s, but no associated layering actions.', documents[2]['schema'],
documents[2]['metadata']['name'], documents[1]['schema'],
documents[1]['metadata']['name'])
error_re = r'^Skipped layering for document.*'
self.assertRegex(mock_log.debug.mock_calls[0][1][0], error_re)
class TestDocumentLayering2Layers(TestDocumentLayering):
@ -1316,3 +1313,108 @@ class TestDocumentLayering3Layers2Regions2Sites(TestDocumentLayering):
global_expected = None
self._test_layering(documents, site_expected, region_expected,
global_expected)
class TestDocumentLayeringWithReplacement(TestDocumentLayering):
def setUp(self):
super(TestDocumentLayeringWithReplacement, self).setUp()
self.documents = list(yaml.safe_load_all("""
---
schema: deckhand/LayeringPolicy/v1
metadata:
schema: metadata/Control/v1
name: layering-policy
data:
layerOrder:
- global
- site
---
schema: aic/Versions/v1
metadata:
name: a
labels:
selector: foo
layeringDefinition:
abstract: False
layer: global
data:
conf:
foo: default
---
schema: aic/Versions/v1
metadata:
name: a
labels:
selector: baz
replacement: true
layeringDefinition:
abstract: False
layer: site
parentSelector:
selector: foo
actions:
- method: merge
path: .
data:
conf:
bar: override
---
schema: armada/Chart/v1
metadata:
name: c
layeringDefinition:
abstract: False
layer: global
substitutions:
- src:
schema: aic/Versions/v1
name: a
path: .conf
dest:
path: .application.conf
data:
application:
conf: {}
...
"""))
def test_basic_replacement(self):
"""Verify that the replacement document is the only one returned."""
site_expected = [{"conf": {"foo": "default", "bar": "override"}}]
global_expected = None
self.documents = self.documents[:-1]
self._test_layering(self.documents, site_expected,
global_expected=global_expected)
def test_replacement_with_substitution_from_replacer(self):
"""Verify that using a replacement document as a substitution source
works.
"""
site_expected = [{"conf": {"foo": "default", "bar": "override"}}]
global_expected = [
{"application": {"conf": {"foo": "default", "bar": "override"}}}]
# Pass in the replacee and replacer as substitution sources. The
# replacer should be used as the source.
self._test_layering(self.documents, site_expected,
global_expected=global_expected,
substitution_sources=self.documents[1:3])
# Attempt the same scenario but reverse the order of the substitution
# sources, which verifies that the replacer always takes priority.
self._test_layering(
self.documents, site_expected, global_expected=global_expected,
substitution_sources=list(reversed(self.documents[1:3])))
# Pass in the replacee as the only substitution source. The replacer
# should replace it and be used as the source.
self._test_layering(self.documents, site_expected,
global_expected=global_expected,
substitution_sources=[self.documents[1]])
# Pass in the replacer as the only substitution source, which should be
# used as the source.
self._test_layering(self.documents, site_expected,
global_expected=global_expected,
substitution_sources=[self.documents[2]])

67
deckhand/tests/unit/engine/test_document_layering_negative.py
View File

@ -276,3 +276,70 @@ class TestDocumentLayeringValidationNegative(
self.assertRaisesRegexp(
errors.InvalidDocumentFormat, error_re, self._test_layering,
[layering_policy, document], validate=True)
class TestDocumentLayeringReplacementNegative(
test_document_layering.TestDocumentLayering):
def test_replacement_with_incompatible_name_or_schema_raises_exc(self):
"""Validate that attempting to replace a child with its parent when
they don't have the same ``metadata.name`` and ``schema`` results in
exception.
"""
doc_factory = factories.DocumentFactory(2, [1, 1])
documents = doc_factory.gen_test({})
# Validate case where names mismatch.
documents[1]['metadata']['name'] = 'foo'
documents[2]['metadata']['replacement'] = True
documents[2]['metadata']['name'] = 'bar'
error_re = (r'.*Document replacement requires that both documents '
'have the same `schema` and `metadata.name`.')
self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re,
self._test_layering, documents)
# Validate case where schemas mismatch.
documents[1]['metadata']['schema'] = 'example/Kind/v1'
documents[2]['metadata']['replacement'] = True
documents[2]['metadata']['schema'] = 'example/Other/v1'
error_re = (r'Document replacement requires that both documents '
'have the same `schema` and `metadata.name`.')
self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re,
self._test_layering, documents)
def test_replacement_without_parent_raises_exc(self):
"""Validate that attempting to do replacement without a parent document
raises an exception.
"""
doc_factory = factories.DocumentFactory(2, [1, 1])
documents = doc_factory.gen_test({})
documents[2]['metadata']['replacement'] = True
documents[2]['metadata']['layeringDefinition'].pop('parentSelector')
error_re = (r'Document replacement requires that the document with '
'`replacement: true` have a parent.')
self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re,
self._test_layering, documents)
def test_replacement_that_is_replaced_raises_exc(self):
"""Validate that attempting replace a replacement document raises an
exception.
"""
doc_factory = factories.DocumentFactory(3, [1, 1, 1])
documents = doc_factory.gen_test({})
for document in documents[1:]:
document['metadata']['name'] = 'foo'
document['schema'] = 'example/Kind/v1'
documents[2]['metadata']['replacement'] = True
documents[3]['metadata']['replacement'] = True
error_re = (r'A replacement document cannot itself be replaced by '
'another document.')
self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re,
self._test_layering, documents,
substitution_sources=documents[1:])