Merge "Add validation for conflicting merge strategies"
This commit is contained in:
@@ -88,8 +88,8 @@ def parse_param(p_val, p_schema):
|
||||
return p_val
|
||||
|
||||
|
||||
def merge_parameters(old, new, param_schemata,
|
||||
merge_strategies):
|
||||
def merge_parameters(old, new, param_schemata, strategies_in_file,
|
||||
available_strategies, env_file):
|
||||
|
||||
def param_merge(p_key, p_value, p_schema, deep_merge=False):
|
||||
p_type = p_schema.type
|
||||
@@ -106,14 +106,24 @@ def merge_parameters(old, new, param_schemata,
|
||||
raise exception.InvalidMergeStrategyForParam(strategy=MERGE,
|
||||
param=p_key)
|
||||
|
||||
new_strategies = {}
|
||||
|
||||
if not old:
|
||||
return new
|
||||
return new, new_strategies
|
||||
|
||||
for key, value in new.items():
|
||||
# if key not in param_schemata ignore it
|
||||
if key in param_schemata and value:
|
||||
param_merge_strategy = get_param_merge_strategy(merge_strategies,
|
||||
key)
|
||||
param_merge_strategy = get_param_merge_strategy(
|
||||
strategies_in_file, key)
|
||||
if key not in available_strategies:
|
||||
new_strategies[key] = param_merge_strategy
|
||||
|
||||
elif param_merge_strategy != available_strategies[key]:
|
||||
raise exception.ConflictingMergeStrategyForParam(
|
||||
strategy=param_merge_strategy,
|
||||
param=key, env_file=env_file)
|
||||
|
||||
if param_merge_strategy == DEEP_MERGE:
|
||||
param_merge(key, value,
|
||||
param_schemata[key],
|
||||
@@ -123,7 +133,7 @@ def merge_parameters(old, new, param_schemata,
|
||||
else:
|
||||
old[key] = value
|
||||
|
||||
return old
|
||||
return old, new_strategies
|
||||
|
||||
|
||||
def merge_environments(environment_files, files,
|
||||
@@ -149,20 +159,23 @@ def merge_environments(environment_files, files,
|
||||
if not environment_files:
|
||||
return
|
||||
|
||||
available_strategies = {}
|
||||
|
||||
for filename in environment_files:
|
||||
raw_env = files[filename]
|
||||
parsed_env = env_fmt.parse(raw_env)
|
||||
merge_strategies = parsed_env.pop(
|
||||
strategies_in_file = parsed_env.pop(
|
||||
env_fmt.PARAMETER_MERGE_STRATEGIES, {})
|
||||
|
||||
for section_key, section_value in parsed_env.items():
|
||||
if section_value:
|
||||
if section_key in (env_fmt.PARAMETERS,
|
||||
env_fmt.PARAMETER_DEFAULTS):
|
||||
params[section_key] = merge_parameters(params[section_key],
|
||||
section_value,
|
||||
param_schemata,
|
||||
merge_strategies)
|
||||
params[section_key], new_strategies = merge_parameters(
|
||||
params[section_key], section_value,
|
||||
param_schemata, strategies_in_file,
|
||||
available_strategies, filename)
|
||||
available_strategies.update(new_strategies)
|
||||
else:
|
||||
params[section_key] = merge_map(params[section_key],
|
||||
section_value)
|
||||
|
||||
@@ -139,8 +139,13 @@ class ImmutableParameterModified(HeatException):
|
||||
|
||||
|
||||
class InvalidMergeStrategyForParam(HeatException):
|
||||
msg_fmt = _("Invalid merge strategy %(strategy)s for "
|
||||
"parameter %(param)s.")
|
||||
msg_fmt = _("Invalid merge strategy '%(strategy)s' for "
|
||||
"parameter '%(param)s'.")
|
||||
|
||||
|
||||
class ConflictingMergeStrategyForParam(HeatException):
|
||||
msg_fmt = _("Conflicting merge strategy '%(strategy)s' for "
|
||||
"parameter '%(param)s' in file '%(env_file)s'.")
|
||||
|
||||
|
||||
class InvalidTemplateAttribute(HeatException):
|
||||
|
||||
@@ -14,6 +14,8 @@
|
||||
import json
|
||||
|
||||
from heat.common import environment_util as env_util
|
||||
from heat.common import exception
|
||||
from heat.engine import parameters
|
||||
from heat.tests import common
|
||||
from heat.tests import utils
|
||||
|
||||
@@ -58,17 +60,9 @@ class TestMergeEnvironments(common.HeatTestCase):
|
||||
super(TestMergeEnvironments, self).setUp()
|
||||
self.ctx = utils.dummy_context(tenant_id='stack_service_test_tenant')
|
||||
# Setup
|
||||
self.params = {'parameters': {
|
||||
'str_value1': "test1",
|
||||
'str_value2': "test2",
|
||||
'del_lst_value1': '',
|
||||
'del_lst_value2': '',
|
||||
'lst_value1': [],
|
||||
'lst_value2': [],
|
||||
'json_value1': {},
|
||||
'json_value2': {}},
|
||||
'resource_registry': {}
|
||||
}
|
||||
self.params = {'parameters': {},
|
||||
'resource_registry': {},
|
||||
'parameter_defaults': {}}
|
||||
|
||||
self.env_1 = {'parameters': {
|
||||
'str_value1': "string1",
|
||||
@@ -76,51 +70,43 @@ class TestMergeEnvironments(common.HeatTestCase):
|
||||
'del_lst_value1': '1,2',
|
||||
'del_lst_value2': '3,4',
|
||||
'lst_value1': [1, 2],
|
||||
'lst_value2': [3, 4],
|
||||
'json_value1': {"1": ["str1", "str2"]},
|
||||
'json_value2': {"2": ["test1", "test2"]}},
|
||||
'resource_registry': {
|
||||
'test::R1': "OS::Heat::RandomString",
|
||||
'test::R2': "BROKEN"}
|
||||
}
|
||||
'test::R2': "BROKEN"},
|
||||
'parameter_defaults': {
|
||||
'lst_value2': [3, 4]}}
|
||||
self.env_2 = {'parameters': {
|
||||
'str_value1': "string3",
|
||||
'str_value2': "string4",
|
||||
'del_lst_value1': '5,6',
|
||||
'del_lst_value2': '7,8',
|
||||
'lst_value1': [5, 6],
|
||||
'lst_value2': [7, 8],
|
||||
'json_value1': {"3": ["str3", "str4"]},
|
||||
'json_value2': {"4": ["test3", "test4"]}},
|
||||
'resource_registry': {
|
||||
'test::R2': "OS::Heat::None"}
|
||||
}
|
||||
'test::R2': "OS::Heat::None"},
|
||||
'parameter_defaults': {
|
||||
'lst_value2': [7, 8]}}
|
||||
|
||||
class mock_schema(object):
|
||||
types = (MAP, LIST, STRING, NUMBER) = (
|
||||
'json', 'comma_delimited_list', 'string', 'number')
|
||||
self.env_3 = {'parameters': {
|
||||
'lst_value1': [9, 10],
|
||||
'json_value1': {"5": ["str5"]}}}
|
||||
|
||||
def __init__(self, d):
|
||||
self.__dict__ = d
|
||||
self.env_4 = {'parameter_defaults': {
|
||||
'lst_value2': [9, 10]}}
|
||||
|
||||
self.param_schemata = {
|
||||
'str_value1': mock_schema({
|
||||
'type': "string"}),
|
||||
'str_value2': mock_schema({
|
||||
'type': "string"}),
|
||||
'del_lst_value1': mock_schema({
|
||||
'type': "comma_delimited_list"}),
|
||||
'del_lst_value2': mock_schema({
|
||||
'type': "comma_delimited_list"}),
|
||||
'lst_value1': mock_schema({
|
||||
'type': "comma_delimited_list"}),
|
||||
'lst_value2': mock_schema({
|
||||
'type': "comma_delimited_list"}),
|
||||
'json_value1': mock_schema({
|
||||
'type': "json"}),
|
||||
'json_value2': mock_schema({
|
||||
'type': "json"})
|
||||
}
|
||||
'str_value1': parameters.Schema(parameters.Schema.STRING),
|
||||
'str_value2': parameters.Schema(parameters.Schema.STRING),
|
||||
'del_lst_value1': parameters.Schema(parameters.Schema.LIST),
|
||||
'del_lst_value2': parameters.Schema(parameters.Schema.LIST),
|
||||
'lst_value1': parameters.Schema(parameters.Schema.LIST,
|
||||
default=[7]),
|
||||
'lst_value2': parameters.Schema(parameters.Schema.LIST),
|
||||
'json_value1': parameters.Schema(parameters.Schema.MAP),
|
||||
'json_value2': parameters.Schema(parameters.Schema.MAP)}
|
||||
|
||||
def test_merge_envs_with_param_default_merge_strategy(self):
|
||||
files = {'env_1': json.dumps(self.env_1),
|
||||
@@ -138,12 +124,13 @@ class TestMergeEnvironments(common.HeatTestCase):
|
||||
'del_lst_value1': '5,6',
|
||||
'del_lst_value2': '7,8',
|
||||
'lst_value1': [5, 6],
|
||||
'lst_value2': [7, 8],
|
||||
'str_value1': u'string3',
|
||||
'str_value2': u'string4'},
|
||||
'resource_registry': {
|
||||
'test::R1': "OS::Heat::RandomString",
|
||||
'test::R2': "OS::Heat::None"}}
|
||||
'test::R2': "OS::Heat::None"},
|
||||
'parameter_defaults': {
|
||||
'lst_value2': [7, 8]}}
|
||||
self.assertEqual(expected, self.params)
|
||||
|
||||
def test_merge_envs_with_specified_default(self):
|
||||
@@ -166,19 +153,21 @@ class TestMergeEnvironments(common.HeatTestCase):
|
||||
'del_lst_value1': '1,2,5,6',
|
||||
'del_lst_value2': '3,4,7,8',
|
||||
'lst_value1': [1, 2, 5, 6], # added
|
||||
'lst_value2': [3, 4, 7, 8],
|
||||
'str_value1': u'string1string3',
|
||||
'str_value2': u'string2string4'},
|
||||
'resource_registry': {
|
||||
'test::R1': "OS::Heat::RandomString",
|
||||
'test::R2': "OS::Heat::None"}}
|
||||
'test::R2': "OS::Heat::None"},
|
||||
'parameter_defaults': {
|
||||
'lst_value2': [3, 4, 7, 8]}}
|
||||
self.assertEqual(expected, self.params)
|
||||
|
||||
def test_merge_envs_with_param_specific_merge_strategy(self):
|
||||
merge_strategies = {
|
||||
'default': "overwrite",
|
||||
'lst_value1': "merge",
|
||||
'json_value1': "deep_merge"}
|
||||
'default': 'overwrite',
|
||||
'lst_value1': 'merge',
|
||||
'lst_value2': 'merge',
|
||||
'json_value1': 'deep_merge'}
|
||||
|
||||
self.env_2['parameter_merge_strategies'] = merge_strategies
|
||||
|
||||
@@ -198,14 +187,54 @@ class TestMergeEnvironments(common.HeatTestCase):
|
||||
'del_lst_value1': '5,6',
|
||||
'del_lst_value2': '7,8',
|
||||
'lst_value1': [1, 2, 5, 6], # added
|
||||
'lst_value2': [7, 8],
|
||||
'str_value1': u'string3',
|
||||
'str_value2': u'string4'},
|
||||
'resource_registry': {
|
||||
'test::R1': 'OS::Heat::RandomString',
|
||||
'test::R2': 'OS::Heat::None'}}
|
||||
'test::R2': 'OS::Heat::None'},
|
||||
'parameter_defaults': {
|
||||
'lst_value2': [3, 4, 7, 8]}}
|
||||
self.assertEqual(expected, self.params)
|
||||
|
||||
def test_merge_envs_with_param_conflicting_merge_strategy(self):
|
||||
merge_strategies = {
|
||||
'default': "overwrite",
|
||||
'lst_value1': "merge",
|
||||
'json_value1': "deep_merge"}
|
||||
|
||||
self.env_2['parameter_merge_strategies'] = merge_strategies
|
||||
|
||||
files = {'env_1': json.dumps(self.env_1),
|
||||
'env_2': json.dumps(self.env_2),
|
||||
'env_3': json.dumps(self.env_3)}
|
||||
|
||||
environment_files = ['env_1', 'env_2', 'env_3']
|
||||
|
||||
# Test
|
||||
self.assertRaises(exception.ConflictingMergeStrategyForParam,
|
||||
env_util.merge_environments,
|
||||
environment_files, files,
|
||||
self.params, self.param_schemata)
|
||||
|
||||
def test_merge_envs_with_param_defaults_conflicting_merge_strategy(self):
|
||||
merge_strategies = {
|
||||
'default': "overwrite",
|
||||
'lst_value2': "merge"}
|
||||
|
||||
self.env_2['parameter_merge_strategies'] = merge_strategies
|
||||
|
||||
files = {'env_1': json.dumps(self.env_1),
|
||||
'env_2': json.dumps(self.env_2),
|
||||
'env_4': json.dumps(self.env_4)}
|
||||
|
||||
environment_files = ['env_1', 'env_2', 'env_4']
|
||||
|
||||
# Test
|
||||
self.assertRaises(exception.ConflictingMergeStrategyForParam,
|
||||
env_util.merge_environments,
|
||||
environment_files, files,
|
||||
self.params, self.param_schemata)
|
||||
|
||||
def test_merge_environments_no_env_files(self):
|
||||
files = {'env_1': json.dumps(self.env_1)}
|
||||
|
||||
@@ -214,15 +243,8 @@ class TestMergeEnvironments(common.HeatTestCase):
|
||||
self.param_schemata)
|
||||
|
||||
# Verify
|
||||
expected = {'parameters': {
|
||||
'str_value1': "test1",
|
||||
'str_value2': "test2",
|
||||
'del_lst_value1': '',
|
||||
'del_lst_value2': '',
|
||||
'lst_value1': [],
|
||||
'lst_value2': [],
|
||||
'json_value1': {},
|
||||
'json_value2': {}},
|
||||
'resource_registry': {}}
|
||||
expected = {'parameters': {},
|
||||
'resource_registry': {},
|
||||
'parameter_defaults': {}}
|
||||
|
||||
self.assertEqual(expected, self.params)
|
||||
|
||||
Reference in New Issue
Block a user