diff --git a/docs/_source/docs/stack_config.rst b/docs/_source/docs/stack_config.rst index 52ff8db03..d8e88075a 100644 --- a/docs/_source/docs/stack_config.rst +++ b/docs/_source/docs/stack_config.rst @@ -15,13 +15,16 @@ particular Stack. The available keys are listed below. - `template_path`_ or `template`_ *(required)* - `dependencies`_ *(optional)* +- `dependencies_inheritance`_ *(optional)* - `hooks`_ *(optional)* +- `hooks_inheritance`_ *(optional)* - `ignore`_ *(optional)* - `notifications`_ *(optional)* - `obsolete`_ *(optional)* - `on_failure`_ *(optional)* - `disable_rollback`_ *(optional)* - `parameters`_ *(optional)* +- `parameters_inheritance`_ *(optional)* - `protected`_ *(optional)* - `role_arn`_ *(optional)* - `cloudformation_service_role`_ *(optional)* @@ -30,8 +33,10 @@ particular Stack. The available keys are listed below. - `iam_role_session_duration`_ *(optional)* - `sceptre_role_session_duration`_ *(optional)* - `sceptre_user_data`_ *(optional)* +- `sceptre_user_data_inheritance`_ *(optional)* - `stack_name`_ *(optional)* - `stack_tags`_ *(optional)* +- `stack_tags_inheritance`_ *(optional)* - `stack_timeout`_ *(optional)* It is not possible to define both `template_path`_ and `template`_. If you do so, @@ -80,7 +85,7 @@ dependencies ~~~~~~~~~~~~ * Resolvable: No * Can be inherited from StackGroup: Yes -* Inheritance strategy: Appended to parent's dependencies +* Inheritance strategy: Appended to parent's dependencies. Configurable with ``dependencies_inheritance`` parameter. A list of other Stacks in the environment that this Stack depends on. Note that if a Stack fetches an output value from another Stack using the @@ -97,15 +102,39 @@ and that Stack need not be added as an explicit dependency. situation by either (a) setting those ``dependencies`` on individual Stack Configs rather than the the StackGroup Config, or (b) moving those dependency stacks outside of the StackGroup. +dependencies_inheritance +~~~~~~~~~~~~~~~~~~~~~~~~ +* Resolvable: No +* Can be inherited from StackGroup: Yes +* Inheritance strategy: Overrides parent if set + +This configuration will override the default inheritance strategy of `dependencies`. + +The default value for this is ``merge``. + +Valid values for this config are: ``merge``, or ``override``. + hooks ~~~~~ * Resolvable: No (but you can use resolvers _in_ hook arguments!) * Can be inherited from StackGroup: Yes -* Inheritance strategy: Overrides parent if set +* Inheritance strategy: Overrides parent if set. Configurable with ``hooks_inheritance`` parameter. A list of arbitrary shell or Python commands or scripts to run. Find out more in the :doc:`hooks` section. +hooks_inheritance +~~~~~~~~~~~~~~~~~~~~~~~~ +* Resolvable: No +* Can be inherited from StackGroup: Yes +* Inheritance strategy: Overrides parent if set + +This configuration will override the default inheritance strategy of `hooks`. + +The default value for this is ``override``. + +Valid values for this config are: ``merge``, or ``override``. + ignore ~~~~~~ * Resolvable: No @@ -155,7 +184,7 @@ notifications List of SNS topic ARNs to publish Stack related events to. A maximum of 5 ARNs can be specified per Stack. This configuration will be used by the ``create``, -``update``, and ``delete`` commands. More information about Stack notifications +``update``, or ``delete`` commands. More information about Stack notifications can found under the relevant section in the `AWS CloudFormation API documentation`_. @@ -231,7 +260,7 @@ parameters ~~~~~~~~~~ * Resolvable: Yes * Can be inherited from StackGroup: Yes -* Inheritance strategy: Overrides parent if set +* Inheritance strategy: Overrides parent if set. Configurable with ``parameters_inheritance`` parameter. .. warning:: @@ -241,7 +270,7 @@ parameters environment variable resolver. A dictionary of key-value pairs to be supplied to a template as parameters. The -keys must match up with the name of the parameter, and the value must be of the +keys must match up with the name of the parameter, or the value must be of the type as defined in the template. .. note:: @@ -292,6 +321,18 @@ Example: - !stack_output security-groups.yaml::BaseSecurityGroupId - !file_contents /file/with/security_group_id.txt +parameters_inheritance +~~~~~~~~~~~~~~~~~~~~~~~~ +* Resolvable: No +* Can be inherited from StackGroup: Yes +* Inheritance strategy: Overrides parent if set + +This configuration will override the default inheritance strategy of `parameters`. + +The default value for this is ``override``. + +Valid values for this config are: ``merge``, or ``override``. + protected ~~~~~~~~~ * Resolvable: No @@ -391,12 +432,24 @@ sceptre_user_data ~~~~~~~~~~~~~~~~~ * Resolvable: Yes * Can be inherited from StackGroup: Yes -* Inheritance strategy: Overrides parent if set +* Inheritance strategy: Overrides parent if set. Configurable with ``sceptre_user_data_inheritance`` parameter. Represents data to be passed to the ``sceptre_handler(sceptre_user_data)`` function in Python templates or accessible under ``sceptre_user_data`` variable key within Jinja2 templates. +sceptre_user_data_inheritance +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Resolvable: No +* Can be inherited from StackGroup: Yes +* Inheritance strategy: Overrides parent if set + +This configuration will override the default inheritance strategy of `sceptre_user_data`. + +The default value for this is ``override``. + +Valid values for this config are: ``merge``, or ``override``. + stack_name ~~~~~~~~~~ * Resolvable: No @@ -436,10 +489,22 @@ stack_tags ~~~~~~~~~~ * Resolvable: Yes * Can be inherited from StackGroup: Yes -* Inheritance strategy: Overrides parent if set +* Inheritance strategy: Overrides parent if set. Configurable with ``stack_tags_inheritance`` parameter. A dictionary of `CloudFormation Tags`_ to be applied to the Stack. +stack_tags_inheritance +~~~~~~~~~~~~~~~~~~~~~~~~ +* Resolvable: No +* Can be inherited from StackGroup: Yes +* Inheritance strategy: Overrides parent if set + +This configuration will override the default inheritance strategy of `stack_tags`. + +The default value for this is ``override``. + +Valid values for this config are: ``merge``, or ``override``. + stack_timeout ~~~~~~~~~~~~~ * Resolvable: No diff --git a/docs/_source/docs/stack_group_config.rst b/docs/_source/docs/stack_group_config.rst index 3da527e4b..2638dc231 100644 --- a/docs/_source/docs/stack_group_config.rst +++ b/docs/_source/docs/stack_group_config.rst @@ -175,7 +175,7 @@ configurations should be defined at a lower directory level. YAML files that define configuration settings with conflicting keys, the child configuration file will usually take precedence (see the specific config keys as documented -for the inheritance strategy employed). +for the inheritance strategy employed and `Inheritance Strategy Override`_). In the above directory structure, ``config/config.yaml`` will be read in first, followed by ``config/account-1/config.yaml``, followed by @@ -185,6 +185,16 @@ For example, if you wanted the ``dev`` StackGroup to build to a different region, this setting could be specified in the ``config/dev/config.yaml`` file, and would only be applied to builds in the ``dev`` StackGroup. +Inheritance Strategy Override +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The inheritance strategy of some properties may be overridden by the stack group config. + +Strategy options: + +* ``merge``: Child config is merged with parent configs, with child taking precedence for conflicting dictionary keys. +* ``override``: Overrides the parent config, if set. + .. _setting_dependencies_for_stack_groups: Setting Dependencies for StackGroups diff --git a/sceptre/config/reader.py b/sceptre/config/reader.py index c1a10188e..fc7421772 100644 --- a/sceptre/config/reader.py +++ b/sceptre/config/reader.py @@ -39,16 +39,30 @@ ConfigAttributes = collections.namedtuple("Attributes", "required optional") + +CONFIG_MERGE_STRATEGY_OVERRIDES = { + "dependencies": strategies.LIST_STRATEGIES, + "hooks": strategies.LIST_STRATEGIES, + "notifications": strategies.LIST_STRATEGIES, + "parameters": strategies.DICT_STRATEGIES, + "sceptre_user_data": strategies.DICT_STRATEGIES, + "stack_tags": strategies.DICT_STRATEGIES, +} + CONFIG_MERGE_STRATEGIES = { "dependencies": strategies.list_join, + "dependencies_inheritance": strategies.child_or_parent, "hooks": strategies.child_wins, + "hooks_inheritance": strategies.child_or_parent, "iam_role": strategies.child_wins, "sceptre_role": strategies.child_wins, "iam_role_session_duration": strategies.child_wins, "sceptre_role_session_duration": strategies.child_wins, "notifications": strategies.child_wins, + "notifications_inheritance": strategies.child_or_parent, "on_failure": strategies.child_wins, "parameters": strategies.child_wins, + "parameters_inheritance": strategies.child_or_parent, "profile": strategies.child_wins, "project_code": strategies.child_wins, "protect": strategies.child_wins, @@ -57,8 +71,10 @@ "role_arn": strategies.child_wins, "cloudformation_service_role": strategies.child_wins, "sceptre_user_data": strategies.child_wins, + "sceptre_user_data_inheritance": strategies.child_or_parent, "stack_name": strategies.child_wins, "stack_tags": strategies.child_wins, + "stack_tags_inheritance": strategies.child_or_parent, "stack_timeout": strategies.child_wins, "template_bucket_name": strategies.child_wins, "template_key_value": strategies.child_wins, @@ -68,6 +84,7 @@ "obsolete": strategies.child_wins, } + STACK_GROUP_CONFIG_ATTRIBUTES = ConfigAttributes( {"project_code", "region"}, { @@ -84,7 +101,9 @@ "template_path", "template", "dependencies", + "dependencies_inheritance", "hooks", + "hooks_inheritance", "iam_role", "sceptre_role", "iam_role_session_duration", @@ -92,13 +111,16 @@ "notifications", "on_failure", "parameters", + "parameters_inheritance", "profile", "protect", "role_arn", "cloudformation_service_role", "sceptre_user_data", + "sceptre_user_data_inheritance", "stack_name", "stack_tags", + "stack_tags_inheritance", "stack_timeout", }, ) @@ -352,11 +374,8 @@ def _read(self, rel_path, base_config=None): # Parse and read in the config files. this_config = self._recursive_read(directory_path, filename, config) - - if "dependencies" in config or "dependencies" in this_config: - this_config["dependencies"] = CONFIG_MERGE_STRATEGIES["dependencies"]( - this_config.get("dependencies"), config.get("dependencies") - ) + # Apply merge strategies with the config that includes base_config values. + this_config.update(self._get_merge_with_stratgies(config, this_config)) config.update(this_config) self._check_version(config) @@ -395,16 +414,39 @@ def _recursive_read( # Read config file and overwrite inherited properties child_config = self._render(directory_path, filename, config_group) or {} + child_config.update(self._get_merge_with_stratgies(config, child_config)) + config.update(child_config) + return config - for config_key, strategy in CONFIG_MERGE_STRATEGIES.items(): - value = strategy(config.get(config_key), child_config.get(config_key)) + def _get_merge_with_stratgies(self, left: dict, right: dict) -> dict: + """ + Returns a new dict with only the merge values of the two inputs, using the + merge strategies defined for each key. + """ + merge = {} + + # Then apply the merge strategies to each item + for config_key, default_strategy in CONFIG_MERGE_STRATEGIES.items(): + strategy = default_strategy + override_key = f"{config_key}_inheritance" + if override_key in CONFIG_MERGE_STRATEGIES: + name = CONFIG_MERGE_STRATEGIES[override_key]( + left.get(override_key), right.get(override_key) + ) + if not name: + pass + elif name not in CONFIG_MERGE_STRATEGY_OVERRIDES[config_key]: + raise SceptreException( + f"{name} is not a valid inheritance strategy for {config_key}" + ) + else: + strategy = CONFIG_MERGE_STRATEGY_OVERRIDES[config_key][name] + value = strategy(left.get(config_key), right.get(config_key)) if value: - child_config[config_key] = value + merge[config_key] = value - config.update(child_config) - - return config + return merge def _render(self, directory_path, basename, stack_group_config): """ diff --git a/sceptre/config/strategies.py b/sceptre/config/strategies.py index 29b37d9ea..b0dc9891b 100644 --- a/sceptre/config/strategies.py +++ b/sceptre/config/strategies.py @@ -71,3 +71,26 @@ def child_wins(a, b): :returns: b """ return b + + +def child_or_parent(a, b): + """ + Returns the second arg if it is not empty, else the first. + + :param a: An object. + :type a: object + :param b: An object. + :type b: object + :returns: b + """ + return b or a + + +LIST_STRATEGIES = { + "merge": list_join, + "override": child_wins, +} +DICT_STRATEGIES = { + "merge": dict_merge, + "override": child_wins, +} diff --git a/tests/test_config_reader.py b/tests/test_config_reader.py index c86095281..1bd9f7eaf 100644 --- a/tests/test_config_reader.py +++ b/tests/test_config_reader.py @@ -2,6 +2,7 @@ import errno import os +import string from unittest.mock import patch, sentinel, MagicMock, ANY import pytest @@ -13,6 +14,7 @@ from sceptre.config.reader import ConfigReader from sceptre.context import SceptreContext +from sceptre.resolvers.stack_attr import StackAttr from sceptre.exceptions import ( DependencyDoesNotExistError, @@ -295,7 +297,7 @@ def test_construct_stacks_constructs_stack( sceptre_user_data={}, hooks={}, s3_details=sentinel.s3_details, - dependencies=["child/level", "top/level"], + dependencies=["top/level", "child/level"], iam_role=None, sceptre_role=None, iam_role_session_duration=None, @@ -667,3 +669,167 @@ def test_render_invalid_yaml__raises_and_creates_debug_file(self): with pytest.raises(ValueError, match="Error parsing .*"): config_reader._render("configs", basename, stack_group_config) assert len(glob("/tmp/rendered_*")) == 1 + + @pytest.mark.parametrize( + "config_key", ("parameters", "sceptre_user_data", "stack_tags") + ) + @pytest.mark.parametrize( + "inheritance_at,values,expected_value", + [ + ( + 0, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {}], + {"a": "a", "b": "b", "c": "c"}, + ), + ( + 1, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {}], + {"a": "a", "b": "b", "c": "c"}, + ), + ( + 0, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {"d": "d"}], + {"a": "a", "b": "b", "c": "c", "d": "d"}, + ), + ( + 2, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {}], + {"b": "b", "c": "c"}, + ), + ( + 3, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {}], + {"c": "c"}, + ), + ( + 99, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {}], + {}, + ), + ( + 99, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {"d": "d"}], + {"d": "d"}, + ), + ( + 3, + [{"a": "a"}, {"b": "b"}, {"c": "c"}, {"c": "x", "d": "d"}], + {"c": "x", "d": "d"}, + ), + ], + ) + def test_inheritance_strategy_override_dict_merge( + self, + config_key, + inheritance_at, + values, + expected_value, + ): + project_path, config_dir = self.create_project() + filepaths = [ + "/".join(string.ascii_uppercase[:i]) + "/config.yaml" + for i in range(1, len(values)) + ] + filepaths.append( + "/".join(string.ascii_uppercase[: len(values) - 1]) + "/1.yaml" + ) + + for i, (stack_path, stack_values) in enumerate(zip(filepaths, values)): + params = {config_key: stack_values} + if i == inheritance_at: + params[f"{config_key}_inheritance"] = "merge" + config = { + "region": "region", + "project_code": "project_code", + "template": { + "path": stack_path, + }, + **params, + } + abs_path = os.path.join(config_dir, stack_path) + self.write_config(abs_path, config) + + self.context.project_path = project_path + config_reader = ConfigReader(self.context) + stack = list(config_reader.construct_stacks()[0])[-1] + stack_key = StackAttr.STACK_ATTR_MAP.get(config_key, config_key) + assert getattr(stack, stack_key) == expected_value + + # "dependencies" is not included here as it has other tests and testing + # it requires configs to be created which makes setup harder. + @pytest.mark.parametrize("config_key", ("hooks", "notifications")) + @pytest.mark.parametrize( + "inheritance_at,values,expected_value", + [ + (0, [["a"], ["b"], ["c"], []], ["a", "b", "c"]), + (1, [["a"], ["b"], ["c"], []], ["a", "b", "c"]), + (2, [["a"], ["b"], ["c"], []], ["b", "c"]), + (3, [["a"], ["b"], ["c"], ["d"]], ["c", "d"]), + (99, [["a"], ["b"], ["c"], ["d"]], ["d"]), + ], + ) + def test_inheritance_strategy_override_list_join( + self, + config_key, + inheritance_at, + values, + expected_value, + ): + project_path, config_dir = self.create_project() + filepaths = [ + "/".join(string.ascii_uppercase[:i]) + "/config.yaml" + for i in range(1, len(values)) + ] + filepaths.append( + "/".join(string.ascii_uppercase[: len(values) - 1]) + "/1.yaml" + ) + + for i, (stack_path, stack_values) in enumerate(zip(filepaths, values)): + params = {config_key: stack_values} + if i == inheritance_at: + params[f"{config_key}_inheritance"] = "merge" + config = { + "region": "region", + "project_code": "project_code", + "template": { + "path": stack_path, + }, + **params, + } + abs_path = os.path.join(config_dir, stack_path) + self.write_config(abs_path, config) + + self.context.project_path = project_path + config_reader = ConfigReader(self.context) + stack = list(config_reader.construct_stacks()[0])[-1] + stack_key = StackAttr.STACK_ATTR_MAP.get(config_key, config_key) + assert getattr(stack, stack_key) == expected_value + + @pytest.mark.parametrize( + "config_key,strategy", + ( + ("hooks", "foo"), + ("hooks", "deepcopy"), + ("hooks", "child_or_parent"), + ("stack_tags", "foo"), + ), + ) + def test_inheritance_strategy_override_errors_on_invalid_strategy( + self, config_key, strategy + ): + project_path, config_dir = self.create_project() + stack_path = "A/1.yaml" + config = { + "region": "region", + "project_code": "project_code", + "template": { + "path": stack_path, + }, + f"{config_key}_inheritance": strategy, + } + abs_path = os.path.join(config_dir, stack_path) + self.write_config(abs_path, config) + self.context.project_path = project_path + config_reader = ConfigReader(self.context) + with pytest.raises(SceptreException): + config_reader.construct_stacks()