[fix] Substitution source documents accidentally modified
This resolves a regression introduced in [0] which was causing
substitution source documents to become modified while performing
substitution into destination documents. A unit test has been
included to prevent the regression.
[0] 1583b78902
Change-Id: I737b8688cfea54dc6b14d8ca243fb43f8363dcaf
This commit is contained in:
parent
a0f00013b5
commit
59836a0e64
|
@ -13,6 +13,7 @@
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
import ast
|
import ast
|
||||||
|
import copy
|
||||||
import re
|
import re
|
||||||
import string
|
import string
|
||||||
|
|
||||||
|
@ -170,6 +171,11 @@ def jsonpath_replace(data, value, jsonpath, pattern=None):
|
||||||
doc['data'].update(replaced_data)
|
doc['data'].update(replaced_data)
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
# These are O(1) reference copies to avoid accidentally modifying source
|
||||||
|
# data. We only want to update destination data.
|
||||||
|
data_copy = copy.copy(data)
|
||||||
|
value_copy = copy.copy(value)
|
||||||
|
|
||||||
jsonpath = _normalize_jsonpath(jsonpath)
|
jsonpath = _normalize_jsonpath(jsonpath)
|
||||||
|
|
||||||
if not jsonpath == '$' and not jsonpath.startswith('$.'):
|
if not jsonpath == '$' and not jsonpath.startswith('$.'):
|
||||||
|
@ -180,7 +186,7 @@ def jsonpath_replace(data, value, jsonpath, pattern=None):
|
||||||
|
|
||||||
def _execute_replace(path, path_to_change):
|
def _execute_replace(path, path_to_change):
|
||||||
if path_to_change:
|
if path_to_change:
|
||||||
new_value = value
|
new_value = value_copy
|
||||||
if pattern:
|
if pattern:
|
||||||
to_replace = path_to_change[0].value
|
to_replace = path_to_change[0].value
|
||||||
# `new_value` represents the value to inject into `to_replace`
|
# `new_value` represents the value to inject into `to_replace`
|
||||||
|
@ -192,23 +198,23 @@ def jsonpath_replace(data, value, jsonpath, pattern=None):
|
||||||
# Raise an exception in case the path isn't present in the
|
# Raise an exception in case the path isn't present in the
|
||||||
# data and a pattern has been provided since it is
|
# data and a pattern has been provided since it is
|
||||||
# otherwise impossible to do the look-up.
|
# otherwise impossible to do the look-up.
|
||||||
new_value = re.sub(pattern, str(value), to_replace)
|
new_value = re.sub(pattern, str(value_copy), to_replace)
|
||||||
except TypeError as e:
|
except TypeError as e:
|
||||||
LOG.error('Failed to substitute the value %s into %s '
|
LOG.error('Failed to substitute the value %s into %s '
|
||||||
'using pattern %s. Details: %s', str(value),
|
'using pattern %s. Details: %s', str(value_copy),
|
||||||
to_replace, pattern, six.text_type(e))
|
to_replace, pattern, six.text_type(e))
|
||||||
raise errors.MissingDocumentPattern(jsonpath=jsonpath,
|
raise errors.MissingDocumentPattern(jsonpath=jsonpath,
|
||||||
pattern=pattern)
|
pattern=pattern)
|
||||||
|
|
||||||
return path.update(data, new_value)
|
return path.update(data_copy, new_value)
|
||||||
|
|
||||||
# Deckhand should be smart enough to create the nested keys in the
|
# Deckhand should be smart enough to create the nested keys in the
|
||||||
# data if they don't exist and a pattern isn't required.
|
# data if they don't exist and a pattern isn't required.
|
||||||
path = _jsonpath_parse(jsonpath)
|
path = _jsonpath_parse(jsonpath)
|
||||||
path_to_change = path.find(data)
|
path_to_change = path.find(data_copy)
|
||||||
if not path_to_change:
|
if not path_to_change:
|
||||||
_execute_data_expansion(jsonpath, data)
|
_execute_data_expansion(jsonpath, data_copy)
|
||||||
path_to_change = path.find(data)
|
path_to_change = path.find(data_copy)
|
||||||
return _execute_replace(path, path_to_change)
|
return _execute_replace(path, path_to_change)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
import itertools
|
import itertools
|
||||||
|
|
||||||
import mock
|
import mock
|
||||||
|
import yaml
|
||||||
|
|
||||||
from deckhand import factories
|
from deckhand import factories
|
||||||
from deckhand.tests.unit.engine import test_document_layering
|
from deckhand.tests.unit.engine import test_document_layering
|
||||||
|
@ -711,3 +712,253 @@ class TestDocumentLayeringWithSubstitution(
|
||||||
global_expected = None
|
global_expected = None
|
||||||
self._test_layering(documents, site_expected,
|
self._test_layering(documents, site_expected,
|
||||||
global_expected=global_expected, strict=False)
|
global_expected=global_expected, strict=False)
|
||||||
|
|
||||||
|
def test_substitution_does_not_alter_source_document(self):
|
||||||
|
"""Regression test to ensure that substitution source documents aren't
|
||||||
|
accidentally modified during a substitution into the destination
|
||||||
|
document.
|
||||||
|
|
||||||
|
:note: This data below is quite long because it's supposed to be
|
||||||
|
mirror production-level data, which is where the issue was
|
||||||
|
caught.
|
||||||
|
|
||||||
|
"""
|
||||||
|
|
||||||
|
documents = """
|
||||||
|
---
|
||||||
|
schema: deckhand/LayeringPolicy/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Control/v1
|
||||||
|
name: layering-policy
|
||||||
|
data:
|
||||||
|
layerOrder:
|
||||||
|
- global
|
||||||
|
- site
|
||||||
|
---
|
||||||
|
data: {}
|
||||||
|
id: 91
|
||||||
|
metadata:
|
||||||
|
layeringDefinition: {abstract: false, layer: global}
|
||||||
|
name: ucp-rabbitmq
|
||||||
|
replacement: false
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
storagePolicy: cleartext
|
||||||
|
substitutions:
|
||||||
|
- dest: {path: .source}
|
||||||
|
src:
|
||||||
|
name: software-versions
|
||||||
|
path: .charts.ucp.rabbitmq
|
||||||
|
schema: pegleg/SoftwareVersions/v1
|
||||||
|
- dest: {path: .values.images.tags}
|
||||||
|
src:
|
||||||
|
name: software-versions
|
||||||
|
path: .images.ucp.rabbitmq
|
||||||
|
schema: pegleg/SoftwareVersions/v1
|
||||||
|
- dest: {path: .values.endpoints.oslo_messaging}
|
||||||
|
src:
|
||||||
|
name: ucp_endpoints
|
||||||
|
path: .ucp.oslo_messaging
|
||||||
|
schema: pegleg/EndpointCatalogue/v1
|
||||||
|
- dest: {path: .values.endpoints.oslo_messaging.auth.user}
|
||||||
|
src:
|
||||||
|
name: ucp_service_accounts
|
||||||
|
path: .ucp.oslo_messaging.admin
|
||||||
|
schema: pegleg/AccountCatalogue/v1
|
||||||
|
- dest: {path: .values.endpoints.oslo_messaging.auth.erlang_cookie}
|
||||||
|
src:
|
||||||
|
name: ucp_rabbitmq_erlang_cookie
|
||||||
|
path: .
|
||||||
|
schema: deckhand/Passphrase/v1
|
||||||
|
- dest: {path: .values.endpoints.oslo_messaging.auth.user.password}
|
||||||
|
src:
|
||||||
|
name: ucp_oslo_messaging_password
|
||||||
|
path: .
|
||||||
|
schema: deckhand/Passphrase/v1
|
||||||
|
schema: armada/Chart/v1
|
||||||
|
status: {bucket: design, revision: 1}
|
||||||
|
---
|
||||||
|
schema: pegleg/SoftwareVersions/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
name: software-versions
|
||||||
|
labels:
|
||||||
|
name: software-versions
|
||||||
|
layeringDefinition:
|
||||||
|
abstract: false
|
||||||
|
layer: global
|
||||||
|
storagePolicy: cleartext
|
||||||
|
data:
|
||||||
|
charts:
|
||||||
|
ucp:
|
||||||
|
rabbitmq:
|
||||||
|
type: git
|
||||||
|
location: https://git.openstack.org/openstack/openstack-helm
|
||||||
|
subpath: rabbitmq
|
||||||
|
reference: f902cd14fac7de4c4c9f7d019191268a6b4e9601
|
||||||
|
images:
|
||||||
|
ucp:
|
||||||
|
rabbitmq:
|
||||||
|
rabbitmq: docker.io/rabbitmq:3.7
|
||||||
|
dep_check: quay.io/stackanetes/kubernetes-entrypoint:v0.3.1
|
||||||
|
---
|
||||||
|
schema: pegleg/EndpointCatalogue/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
name: ucp_endpoints
|
||||||
|
layeringDefinition:
|
||||||
|
abstract: false
|
||||||
|
layer: site
|
||||||
|
storagePolicy: cleartext
|
||||||
|
data:
|
||||||
|
ucp:
|
||||||
|
oslo_messaging:
|
||||||
|
namespace: null
|
||||||
|
hosts:
|
||||||
|
default: rabbitmq
|
||||||
|
host_fqdn_override:
|
||||||
|
default: null
|
||||||
|
path: /openstack
|
||||||
|
scheme: rabbit
|
||||||
|
port:
|
||||||
|
amqp:
|
||||||
|
default: 5672
|
||||||
|
---
|
||||||
|
schema: pegleg/AccountCatalogue/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
name: ucp_service_accounts
|
||||||
|
layeringDefinition:
|
||||||
|
abstract: false
|
||||||
|
layer: site
|
||||||
|
storagePolicy: cleartext
|
||||||
|
data:
|
||||||
|
ucp:
|
||||||
|
oslo_messaging:
|
||||||
|
admin:
|
||||||
|
username: rabbitmq
|
||||||
|
---
|
||||||
|
schema: deckhand/Passphrase/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
name: ucp_rabbitmq_erlang_cookie
|
||||||
|
layeringDefinition:
|
||||||
|
abstract: false
|
||||||
|
layer: site
|
||||||
|
storagePolicy: cleartext
|
||||||
|
data: 111df8c05b0f041d4764
|
||||||
|
---
|
||||||
|
schema: deckhand/Passphrase/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
name: ucp_oslo_messaging_password
|
||||||
|
layeringDefinition:
|
||||||
|
abstract: false
|
||||||
|
layer: site
|
||||||
|
storagePolicy: cleartext
|
||||||
|
data: password15
|
||||||
|
"""
|
||||||
|
|
||||||
|
global_expected = [
|
||||||
|
{
|
||||||
|
'charts': {
|
||||||
|
'ucp': {
|
||||||
|
'rabbitmq': {
|
||||||
|
'subpath': 'rabbitmq',
|
||||||
|
'type': 'git',
|
||||||
|
'location': ('https://git.openstack.org/openstack/'
|
||||||
|
'openstack-helm'),
|
||||||
|
'reference': (
|
||||||
|
'f902cd14fac7de4c4c9f7d019191268a6b4e9601')
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
'images': {
|
||||||
|
'ucp': {
|
||||||
|
'rabbitmq': {
|
||||||
|
'rabbitmq': 'docker.io/rabbitmq:3.7',
|
||||||
|
'dep_check': (
|
||||||
|
'quay.io/stackanetes/kubernetes-entrypoint:'
|
||||||
|
'v0.3.1')
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
'source': {
|
||||||
|
'subpath': 'rabbitmq',
|
||||||
|
'type': 'git',
|
||||||
|
'location': (
|
||||||
|
'https://git.openstack.org/openstack/openstack-helm'),
|
||||||
|
'reference': 'f902cd14fac7de4c4c9f7d019191268a6b4e9601'
|
||||||
|
},
|
||||||
|
'values': {
|
||||||
|
'images': {
|
||||||
|
'tags': {
|
||||||
|
'rabbitmq': 'docker.io/rabbitmq:3.7',
|
||||||
|
'dep_check': (
|
||||||
|
'quay.io/stackanetes/kubernetes-entrypoint:'
|
||||||
|
'v0.3.1')
|
||||||
|
}
|
||||||
|
},
|
||||||
|
'endpoints': {
|
||||||
|
'oslo_messaging': {
|
||||||
|
'namespace': None,
|
||||||
|
'scheme': 'rabbit',
|
||||||
|
'port': {'amqp': {'default': 5672}},
|
||||||
|
'hosts': {'default': 'rabbitmq'},
|
||||||
|
'path': '/openstack',
|
||||||
|
'auth': {
|
||||||
|
'user': {
|
||||||
|
'username': 'rabbitmq',
|
||||||
|
'password': 'password15'
|
||||||
|
},
|
||||||
|
'erlang_cookie': '111df8c05b0f041d4764'
|
||||||
|
},
|
||||||
|
'host_fqdn_override': {'default': None}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
]
|
||||||
|
site_expected = [
|
||||||
|
{
|
||||||
|
'ucp': {
|
||||||
|
'oslo_messaging': {
|
||||||
|
'admin': {'username': 'rabbitmq'}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
'111df8c05b0f041d4764',
|
||||||
|
{
|
||||||
|
'ucp': {
|
||||||
|
# If a copy of the source document isn't made, then these
|
||||||
|
# values below may be incorrectly modified. The data
|
||||||
|
# should be unmodified:
|
||||||
|
#
|
||||||
|
# oslo_messaging:
|
||||||
|
# namespace: null
|
||||||
|
# hosts:
|
||||||
|
# default: rabbitmq
|
||||||
|
# host_fqdn_override:
|
||||||
|
# default: null
|
||||||
|
# path: /openstack
|
||||||
|
# scheme: rabbit
|
||||||
|
# port:
|
||||||
|
# amqp:
|
||||||
|
# default: 5672
|
||||||
|
'oslo_messaging': {
|
||||||
|
'namespace': None,
|
||||||
|
'hosts': {'default': 'rabbitmq'},
|
||||||
|
'host_fqdn_override': {'default': None},
|
||||||
|
'path': '/openstack',
|
||||||
|
'scheme': 'rabbit',
|
||||||
|
'port': {'amqp': {'default': 5672}},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
'password15'
|
||||||
|
]
|
||||||
|
|
||||||
|
docs = list(yaml.safe_load_all(documents))
|
||||||
|
self._test_layering(docs, site_expected=site_expected,
|
||||||
|
global_expected=global_expected)
|
||||||
|
|
Loading…
Reference in New Issue