Skip to content

Commit

Permalink
[RESOLVE #946] Fixing bug preventing StackGroup dependencies (#1116)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jfalkenstein authored Oct 6, 2021
1 parent ab9afba commit d45ffaf
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
5 changes: 4 additions & 1 deletion sceptre/config/reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. "
Expand Down
33 changes: 33 additions & 0 deletions tests/test_config_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit d45ffaf

Please sign in to comment.