From d45ffafcf9105376a81d4f75b538130f29890e33 Mon Sep 17 00:00:00 2001 From: Jon Falkenstein <77296393+jfalkenstein@users.noreply.github.com> Date: Wed, 6 Oct 2021 13:39:42 -0500 Subject: [PATCH] [RESOLVE #946] Fixing bug preventing StackGroup dependencies (#1116) This resolves issue #946 by solving the issue of inherited dependencies. Turns out, the problem was just a small bug with inherited objects in memory. There was a small bug in the ConfigReader that has to do with the way list-joining works on individual stacks and how dependencies are resolved. The basic situation was this: * Sceptre uses the "list_join" strategy to combine StackGroup dependencies with Stack dependencies. * The list_join strategy creates a NEW list by combining A + B... **but only if both A and B exist**. * In the case where the StackGroup dependencies exist _but there are no Stack dependencies_, Sceptre just selects A and passes each stack in that group the exact same dependencies list (i.e. **same object in memory**) from the StackGroup * The process of resolving stacks replaces the stack string with a Stack object _in place_. * If two stacks have inherited the exact same dependencies list because neither of them defined their own dependencies lists, the first stack to be resolved will replace those objects in memory with Stack objects and the second stack to be resolved will receive those Stack objects rather than strings. * In the attempt to resolve the Stack object (believing it's a string), Sceptre will blow up because you cannot call `.replace()` on a Stack instance. But we don't need to do that at all if it's already a Stack. This PR fixes that bug by skipping resolution for dependencies that are already Stack objects. --- sceptre/config/reader.py | 5 ++++- tests/test_config_reader.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/sceptre/config/reader.py b/sceptre/config/reader.py index bc2d8f489..fc760c06f 100644 --- a/sceptre/config/reader.py +++ b/sceptre/config/reader.py @@ -272,7 +272,10 @@ def resolve_stacks(self, stack_map): if not self.context.ignore_dependencies: for i, dep in enumerate(stack.dependencies): try: - stack.dependencies[i] = stack_map[sceptreise_path(dep)] + if not isinstance(dep, Stack): + # If the dependency was inherited from a stack group, it might already + # have been mapped and so doesn't need to be mapped again. + stack.dependencies[i] = stack_map[sceptreise_path(dep)] except KeyError: raise DependencyDoesNotExistError( "{stackname}: Dependency {dep} not found. " diff --git a/tests/test_config_reader.py b/tests/test_config_reader.py index 6e5111718..67afd082f 100644 --- a/tests/test_config_reader.py +++ b/tests/test_config_reader.py @@ -525,6 +525,39 @@ def test_missing_dependency( else: assert False + @pytest.mark.parametrize( + "filepaths, dependency, parent_config_path", [ + (["A/1.yaml", "A/2.yaml", "B/1.yaml"], "B/1.yaml", 'A/config.yaml'), + (["A/1.yaml", "A/2.yaml"], "A/1.yaml", 'A/config.yaml'), + ] + ) + def test_inherited_dependency_already_resolved(self, filepaths, dependency, parent_config_path): + project_path, config_dir = self.create_project() + parent_config = { + 'dependencies': [dependency] + } + abs_path = os.path.join(config_dir, parent_config_path) + self.write_config(abs_path, parent_config) + + for rel_path in filepaths: + # Set up config with reference to an existing stack + config = { + "project_code": "project_code", + "region": "region", + "template_path": rel_path, + } + + abs_path = os.path.join(config_dir, rel_path) + self.write_config(abs_path, config) + self.context.project_path = project_path + try: + config_reader = ConfigReader(self.context) + all_stacks, command_stacks = config_reader.construct_stacks() + except Exception: + raise + else: + assert True + def test_resolve_node_tag(self): mock_loader = MagicMock(yaml.Loader) mock_loader.resolve.return_value = "new_tag"