replacement: Fix update substitution source for replacement
This patchset fixes an edge case with respect to updating substitution sources after a document has been rendered vis-a-vis replacement. Substitution sources only use schema/name which doesn't uniquely identify replacement documents. Thus, an additional check is required in `update_substitution_sources` to ensure that the parent-replacement document doesn't overwrite data for the child-replacement document. Note that Deckhand prioritizes the child-replacement to be rendered immediately after the parent-replacement document, meaning that the child-replacement document will be the one who correctly updates the substitution sources (which don't include parent-replacement documents). Afterward, all other documents that reference the parent-replacement should get the correct data. Unit and functional tests have been added for a multi-layer replacement scenario which regression-test the bug. Change-Id: Ie6d921d98b7aa87e35a7aa5256cc7da2c0a256b0
This commit is contained in:
parent
bb36db23ce
commit
6f86088a9a
deckhand
engine
tests
functional/gabbits/replacement
unit/engine
tools
@ -659,7 +659,7 @@ class DocumentLayering(object):
|
||||
pass
|
||||
doc.data = rendered_data.data
|
||||
self.secrets_substitution.update_substitution_sources(
|
||||
doc.schema, doc.name, rendered_data.data)
|
||||
doc.meta, rendered_data.data)
|
||||
self._documents_by_index[doc.meta] = rendered_data
|
||||
else:
|
||||
LOG.debug(
|
||||
@ -686,7 +686,7 @@ class DocumentLayering(object):
|
||||
doc.data = rendered_data.data
|
||||
if not doc.has_replacement:
|
||||
self.secrets_substitution.update_substitution_sources(
|
||||
doc.schema, doc.name, rendered_data.data)
|
||||
doc.meta, rendered_data.data)
|
||||
self._documents_by_index[doc.meta] = rendered_data
|
||||
# Otherwise, retrieve the encrypted data for the document if its
|
||||
# data has been encrypted so that future references use the actual
|
||||
@ -697,7 +697,7 @@ class DocumentLayering(object):
|
||||
if not doc.is_abstract:
|
||||
doc.data = encrypted_data
|
||||
self.secrets_substitution.update_substitution_sources(
|
||||
doc.schema, doc.name, encrypted_data)
|
||||
doc.meta, encrypted_data)
|
||||
self._documents_by_index[doc.meta] = encrypted_data
|
||||
|
||||
# NOTE: Since the child-replacement is always prioritized, before
|
||||
|
@ -344,11 +344,36 @@ class SecretsSubstitution(object):
|
||||
|
||||
yield document
|
||||
|
||||
def update_substitution_sources(self, schema, name, data):
|
||||
def update_substitution_sources(self, meta, data):
|
||||
"""Update substitution sources with rendered data so that future
|
||||
layering and substitution sources reference the latest rendered data
|
||||
rather than stale data.
|
||||
|
||||
:param meta: Tuple of (schema, layer, name).
|
||||
:type meta: tuple
|
||||
:param data: Dictionary of just-rendered document data that belongs
|
||||
to the document uniquely identified by ``meta``.
|
||||
:type data: dict
|
||||
:returns None
|
||||
"""
|
||||
schema, layer, name = meta
|
||||
|
||||
if (schema, name) not in self._substitution_sources:
|
||||
return
|
||||
|
||||
# Substitution sources only use schema/name which doesn't uniquely
|
||||
# identify replacement documents. The check below ensures that the
|
||||
# exact document is selected. Note that Deckhand prioritizes the
|
||||
# child-replacement to be rendered immediately after the
|
||||
# parent-replacement document, meaning that the child-replacement
|
||||
# document will be the one who correctly updates the substitution
|
||||
# sources below (which don't include parent-replacement documents).
|
||||
# Afterward, all other documents that reference the parent-replacement
|
||||
# should get the correct data.
|
||||
substitution_src = self._substitution_sources[(schema, name)]
|
||||
if substitution_src.meta != meta:
|
||||
return
|
||||
|
||||
if isinstance(data, dict) and isinstance(substitution_src.data, dict):
|
||||
substitution_src.data.update(data)
|
||||
else:
|
||||
|
@ -0,0 +1,129 @@
|
||||
# Tests success path for advanced replacement scenario, where
|
||||
# parent-replacement (type layer) layers with global document, after which
|
||||
# the parent-replacement is replaced by the child-replacement (site layer).
|
||||
#
|
||||
# 1. Purges existing data to ensure test isolation.
|
||||
# 2. Adds initial documents with replacement scenario described above.
|
||||
# 3. Verifies correctly layered, substituted and replaced data.
|
||||
|
||||
defaults:
|
||||
request_headers:
|
||||
content-type: application/x-yaml
|
||||
response_headers:
|
||||
content-type: application/x-yaml
|
||||
verbose: true
|
||||
|
||||
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 to validate following scenario:
|
||||
|
||||
* Global document called nova-global
|
||||
* Region document called nova (layers with nova-global)
|
||||
* Site document (replaces nova)
|
||||
|
||||
PUT: /api/v1.0/buckets/mop/documents
|
||||
status: 200
|
||||
data: |-
|
||||
---
|
||||
schema: deckhand/LayeringPolicy/v1
|
||||
metadata:
|
||||
schema: metadata/Control/v1
|
||||
name: layering-policy
|
||||
data:
|
||||
layerOrder:
|
||||
- global
|
||||
- type
|
||||
- site
|
||||
---
|
||||
schema: armada/Chart/v1
|
||||
metadata:
|
||||
schema: metadata/Document/v1
|
||||
name: nova-global
|
||||
labels:
|
||||
name: nova-global
|
||||
component: nova
|
||||
layeringDefinition:
|
||||
abstract: false
|
||||
layer: global
|
||||
data:
|
||||
values:
|
||||
pod:
|
||||
replicas:
|
||||
server: 16
|
||||
---
|
||||
schema: armada/Chart/v1
|
||||
metadata:
|
||||
schema: metadata/Document/v1
|
||||
name: nova
|
||||
labels:
|
||||
name: nova-5ec
|
||||
component: nova
|
||||
layeringDefinition:
|
||||
abstract: false
|
||||
layer: type
|
||||
parentSelector:
|
||||
name: nova-global
|
||||
actions:
|
||||
- method: merge
|
||||
path: .
|
||||
data: {}
|
||||
---
|
||||
schema: armada/Chart/v1
|
||||
metadata:
|
||||
schema: metadata/Document/v1
|
||||
replacement: true
|
||||
name: nova
|
||||
layeringDefinition:
|
||||
abstract: false
|
||||
layer: site
|
||||
parentSelector:
|
||||
name: nova-5ec
|
||||
actions:
|
||||
- method: merge
|
||||
path: .
|
||||
data:
|
||||
values:
|
||||
pod:
|
||||
replicas:
|
||||
api_metadata: 16
|
||||
placement: 2
|
||||
osapi: 16
|
||||
conductor: 16
|
||||
consoleauth: 2
|
||||
scheduler: 2
|
||||
novncproxy: 2
|
||||
|
||||
- name: verify_multi_layer_replacement_document
|
||||
desc: |
|
||||
Tests success path for advanced replacement scenario, where
|
||||
parent-replacement document (type layer) layers with document from
|
||||
global layer, after which it is replaced by the child-replacement
|
||||
document (site layer).
|
||||
GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents
|
||||
query_parameters:
|
||||
schema: armada/Chart/v1
|
||||
metadata.name: nova
|
||||
status: 200
|
||||
response_multidoc_jsonpaths:
|
||||
$.`len`: 1
|
||||
$.[*].metadata.name: nova
|
||||
$.[*].metadata.layeringDefinition.layer: site
|
||||
$.[*].data:
|
||||
values:
|
||||
pod:
|
||||
replicas:
|
||||
api_metadata: 16
|
||||
placement: 2
|
||||
osapi: 16
|
||||
conductor: 16
|
||||
consoleauth: 2
|
||||
scheduler: 2
|
||||
novncproxy: 2
|
||||
server: 16
|
@ -251,3 +251,115 @@ data:
|
||||
self._test_layering(
|
||||
documents, site_expected=site_expected,
|
||||
region_expected=None)
|
||||
|
||||
def test_multi_layer_replacement(self):
|
||||
"""Validate the following scenario:
|
||||
|
||||
* Global document called nova-global
|
||||
* Region document called nova (layers with nova-global)
|
||||
* Site document (replaces nova)
|
||||
"""
|
||||
self.documents = list(yaml.safe_load_all("""
|
||||
---
|
||||
schema: deckhand/LayeringPolicy/v1
|
||||
metadata:
|
||||
schema: metadata/Control/v1
|
||||
name: layering-policy
|
||||
data:
|
||||
layerOrder:
|
||||
- global
|
||||
- region
|
||||
- site
|
||||
---
|
||||
schema: armada/Chart/v1
|
||||
metadata:
|
||||
schema: metadata/Document/v1
|
||||
name: nova-global
|
||||
labels:
|
||||
name: nova-global
|
||||
component: nova
|
||||
layeringDefinition:
|
||||
abstract: false
|
||||
layer: global
|
||||
data:
|
||||
values:
|
||||
pod:
|
||||
replicas:
|
||||
server: 16
|
||||
---
|
||||
schema: armada/Chart/v1
|
||||
metadata:
|
||||
schema: metadata/Document/v1
|
||||
name: nova
|
||||
labels:
|
||||
name: nova-5ec
|
||||
component: nova
|
||||
layeringDefinition:
|
||||
abstract: false
|
||||
layer: region
|
||||
parentSelector:
|
||||
name: nova-global
|
||||
actions:
|
||||
- method: merge
|
||||
path: .
|
||||
data: {}
|
||||
---
|
||||
schema: armada/Chart/v1
|
||||
metadata:
|
||||
schema: metadata/Document/v1
|
||||
replacement: true
|
||||
name: nova
|
||||
layeringDefinition:
|
||||
abstract: false
|
||||
layer: site
|
||||
parentSelector:
|
||||
name: nova-5ec
|
||||
actions:
|
||||
- method: merge
|
||||
path: .
|
||||
data:
|
||||
values:
|
||||
pod:
|
||||
replicas:
|
||||
api_metadata: 16
|
||||
placement: 2
|
||||
osapi: 16
|
||||
conductor: 16
|
||||
consoleauth: 2
|
||||
scheduler: 2
|
||||
novncproxy: 2
|
||||
"""))
|
||||
|
||||
site_expected = [
|
||||
{
|
||||
"values": {
|
||||
"pod": {
|
||||
"replicas": {
|
||||
"api_metadata": 16,
|
||||
"placement": 2,
|
||||
"osapi": 16,
|
||||
"conductor": 16,
|
||||
"consoleauth": 2,
|
||||
"scheduler": 2,
|
||||
"novncproxy": 2,
|
||||
"server": 16
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
global_expected = [
|
||||
{
|
||||
"values": {
|
||||
"pod": {
|
||||
"replicas": {
|
||||
"server": 16
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
self._test_layering(self.documents,
|
||||
site_expected=site_expected,
|
||||
region_expected=None,
|
||||
global_expected=global_expected)
|
||||
|
@ -7,6 +7,8 @@ RES=$(find . \
|
||||
-not -path "*/releasenotes/build/*" \
|
||||
-not -path "*/doc/build/*" \
|
||||
-not -name "*.tgz" \
|
||||
-not -name "*.html" \
|
||||
-not -name "*.pyc" \
|
||||
-type f -exec egrep -l " +$" {} \;)
|
||||
|
||||
if [[ -n $RES ]]; then
|
||||
|
Loading…
x
Reference in New Issue
Block a user