From c338cf25f9c03a997857325bfdf13a9c84edb947 Mon Sep 17 00:00:00 2001 From: Adrien Mannocci Date: Wed, 21 Oct 2026 10:39:19 +0200 Subject: [PATCH] feat: add --include-dependents to auto-scope Add an opt-in `--include-dependents` flag to `plan` and `apply` that expands the `--auto-scope` selection with every group that imports, directly or transitively, from a selected group. Without it, a change to a producer group left its consumers out of the run, so stale inputs could go unnoticed. The flag requires `-A`, and is not offered on `destroy`, `docs` or `migrate`. --- README.md | 5 ++++ src/terranova/commands/apply.py | 8 ++++++ src/terranova/commands/helpers.py | 35 +++++++++++++++++++++++++ src/terranova/commands/plan.py | 8 ++++++ src/terranova/graph.py | 27 +++++++++++++++++++ tests/e2e/test_apply_command.py | 16 ++++++++++++ tests/e2e/test_plan_command.py | 38 +++++++++++++++++++++++++++ tests/it/test_graph.py | 24 ++++++++++++++++- tests/it/test_helpers.py | 43 +++++++++++++++++++++++++++++++ 9 files changed, 203 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9c2aa0e..27d44f2 100644 --- a/README.md +++ b/README.md @@ -355,6 +355,10 @@ terranova apply --strategy parallel --auto-approve --fail-at-end `HEAD` instead of `HEAD`. Commits already made on the branch are included, along with staged, unstaged and untracked changes. This is what you want in CI, where the working tree is clean. `--base` requires `--auto-scope`, and the history must be available (e.g. `fetch-depth: 0` on a CI checkout). +- With `--include-dependents` (`plan` and `apply` only), resource groups that import from a selected one, + directly or through a chain of imports, are added to the run and ordered after their sources. Without it, a + change in a group that others import from doesn't select them, so their plan can go stale. It is not + available for `destroy`, which should never widen its scope implicitly. - It can't be combined with an explicit `path` (or, for `apply`, a `.tnplan` file), and it must be run from inside a git repository. - With `docs`, the docs directory isn't wiped first, so documentation of groups outside the scope is kept. @@ -363,6 +367,7 @@ terranova apply --strategy parallel --auto-approve --fail-at-end terranova plan --auto-scope terranova apply -A --auto-approve terranova plan -A --base origin/main +terranova plan -A --base origin/main --include-dependents ``` - To apply exactly what was planned, combine it with `plan --out`: the saved `.tnplan` file only contains diff --git a/src/terranova/commands/apply.py b/src/terranova/commands/apply.py index a4d056a..09deca6 100644 --- a/src/terranova/commands/apply.py +++ b/src/terranova/commands/apply.py @@ -27,8 +27,11 @@ auto_scope_resource_dirs, base_option, check_base_requires_auto_scope, + check_dependents_require_auto_scope, complete_resource_path, execute_tasks, + expand_with_dependents, + include_dependents_option, parse_execution_plan, read_manifests_and_waves, resource_dirs, @@ -100,6 +103,7 @@ def run(self) -> None: ) @auto_scope_option @base_option +@include_dependents_option @click.option( "--auto-approve", help="Skip interactive approval of plan before applying.", @@ -135,6 +139,7 @@ def apply( path_or_plan: str | None, auto_scope: bool, base: str | None, + include_dependents: bool, auto_approve: bool, target: str, fail_at_end: bool, @@ -146,6 +151,7 @@ def apply( execution_plan: dict[str, str] | None paths: list[tuple[Path, str]] check_base_requires_auto_scope(auto_scope, base) + check_dependents_require_auto_scope(auto_scope, include_dependents) if auto_scope and path_or_plan: raise click.UsageError( "`--auto-scope`/`-A` can't be combined with `path_or_plan`." @@ -153,6 +159,8 @@ def apply( if auto_scope: execution_plan = None paths = auto_scope_resource_dirs(ctx.conf_dir, ctx.resources_dir, base) + if include_dependents: + paths = expand_with_dependents(ctx.resources_dir, paths) elif path_or_plan and path_or_plan.endswith("tnplan"): execution_plan = parse_execution_plan( Path(path_or_plan).read_text(Constants.ENCODING_UTF_8) diff --git a/src/terranova/commands/helpers.py b/src/terranova/commands/helpers.py index 50455aa..95c2f61 100644 --- a/src/terranova/commands/helpers.py +++ b/src/terranova/commands/helpers.py @@ -43,6 +43,7 @@ build_dependency_graph, compute_waves, normalize_rel_path, + with_dependents, ) from terranova.process import ErrorReturnCode from terranova.resources import Resource, ResourcesFinder, ResourcesManifest, Selector @@ -69,6 +70,14 @@ ) """Shared `--base` option, only valid together with `--auto-scope`.""" +include_dependents_option = click.option( + "--include-dependents", + help="With `--auto-scope`, also include the resource groups that import " + + "(directly or not) from a selected one, ordered after their sources.", + is_flag=True, +) +"""Shared `--include-dependents` option, only valid together with `--auto-scope`.""" + flat_strategy_option = click.option( "--strategy", help="Execution strategy across resource groups: `sequential` (default) runs " @@ -317,6 +326,24 @@ def _match_shared_dependents( return matched +def expand_with_dependents( + resources_dir: Path, selected: list[tuple[Path, str]] +) -> list[tuple[Path, str]]: + """Add to `selected` every group that transitively imports from one of them.""" + if not selected: + return selected + all_dirs = find_all_resource_dirs(resources_dir) + manifests = {rel_path: read_manifest(full_path) for full_path, rel_path in all_dirs} + graph = build_dependency_graph(all_dirs, manifests) + keep = with_dependents( + graph, {normalize_rel_path(rel_path) for _, rel_path in selected} + ) + return sorted( + (entry for entry in all_dirs if normalize_rel_path(entry[1]) in keep), + key=lambda entry: entry[1], + ) + + def auto_scope_resource_dirs( conf_dir: Path, resources_dir: Path, base: str | None = None ) -> list[tuple[Path, str]]: @@ -363,6 +390,14 @@ def check_base_requires_auto_scope(auto_scope: bool, base: str | None) -> None: raise click.UsageError("`--base` requires `--auto-scope`/`-A`.") +def check_dependents_require_auto_scope( + auto_scope: bool, include_dependents: bool +) -> None: + """Reject `--include-dependents` given without `--auto-scope`.""" + if include_dependents and not auto_scope: + raise click.UsageError("`--include-dependents` requires `--auto-scope`/`-A`.") + + def resolve_resource_dirs( conf_dir: Path, resources_dir: Path, diff --git a/src/terranova/commands/plan.py b/src/terranova/commands/plan.py index 50375d8..26f68df 100644 --- a/src/terranova/commands/plan.py +++ b/src/terranova/commands/plan.py @@ -26,8 +26,11 @@ TerraformTask, auto_scope_option, base_option, + check_dependents_require_auto_scope, complete_resource_path, execute_tasks, + expand_with_dependents, + include_dependents_option, read_manifests_and_waves, resolve_resource_dirs, write_execution_plan, @@ -114,6 +117,7 @@ def run(self) -> None: @click.argument("path", type=str, required=False, shell_complete=complete_resource_path) @auto_scope_option @base_option +@include_dependents_option @click.option( "--input/--no-input", help="Ask for input for variables if not directly set.", @@ -175,6 +179,7 @@ def plan( path: str | None, auto_scope: bool, base: str | None, + include_dependents: bool, input: bool, no_color: bool, parallelism: int, @@ -186,9 +191,12 @@ def plan( ) -> None: """Show changes required by the current configuration.""" # Find all resources manifests + check_dependents_require_auto_scope(auto_scope, include_dependents) paths = resolve_resource_dirs( ctx.conf_dir, ctx.resources_dir, path, auto_scope, base ) + if include_dependents: + paths = expand_with_dependents(ctx.resources_dir, paths) # Execution plan execution_plan: dict[str, str] = {} diff --git a/src/terranova/graph.py b/src/terranova/graph.py index 8aa135b..e89c3af 100644 --- a/src/terranova/graph.py +++ b/src/terranova/graph.py @@ -15,6 +15,7 @@ # limitations under the License. # import os +from collections import defaultdict from dataclasses import dataclass, field from graphlib import CycleError, TopologicalSorter from itertools import pairwise @@ -97,6 +98,32 @@ def build_dependency_graph( ) +def with_dependents(graph: DependencyGraph, roots: set[str]) -> set[str]: + """ + Expand `roots` with every node that transitively imports from one of them. + + Args: + graph: the dependency graph. + roots: nodes to start from, all of which are part of the result. + + Returns: + `roots` plus all of their direct and indirect dependents. + """ + importers: defaultdict[str, set[str]] = defaultdict(set) + for node, sources in graph.depends_on.items(): + for source in sources: + importers[source].add(node) + + found = set(roots) + pending = list(roots) + while pending: + for importer in importers[pending.pop()]: + if importer not in found: + found.add(importer) + pending.append(importer) + return found + + def compute_waves(graph: DependencyGraph) -> list[Wave]: """ Compute topological execution waves from a dependency graph. diff --git a/tests/e2e/test_apply_command.py b/tests/e2e/test_apply_command.py index c144fe6..de44af2 100644 --- a/tests/e2e/test_apply_command.py +++ b/tests/e2e/test_apply_command.py @@ -282,6 +282,22 @@ def test_apply_base_without_auto_scope_is_usage_error( assert "--base" in result.output +def test_apply_include_dependents_without_auto_scope_is_usage_error( + runner: CliRunner, fake_terraform_bin: FakeTerraform, tmp_path: Path +) -> None: + _ = fake_terraform_bin + conf_dir = tmp_path / "conf" + copy_as_git_repo(PROJECT_TESTS_FIXTURES_DIR / "import_chain", conf_dir) + + result = runner.invoke( + main, + args=["--conf-dir", str(conf_dir), "apply", "--include-dependents"], + ) + + assert result.exit_code != 0 + assert "--include-dependents" in result.output + + def test_apply_with_non_string_tnplan_value_raises_type_error( runner: CliRunner, tmp_path: Path ) -> None: diff --git a/tests/e2e/test_plan_command.py b/tests/e2e/test_plan_command.py index 9b43630..dfb20a7 100644 --- a/tests/e2e/test_plan_command.py +++ b/tests/e2e/test_plan_command.py @@ -278,6 +278,44 @@ def test_plan_base_without_auto_scope_is_usage_error( assert "--base" in result.output +def test_plan_auto_scope_include_dependents_adds_importing_group( + runner: CliRunner, fake_terraform_bin: FakeTerraform, tmp_path: Path +) -> None: + fake_terraform_bin.set_stdout("chained-value") + conf_dir = tmp_path / "conf" + copy_as_git_repo(PROJECT_TESTS_FIXTURES_DIR / "import_chain", conf_dir) + manifest = conf_dir / "resources" / "producer" / "manifest.yml" + manifest.write_text(manifest.read_text() + "\n# changed\n") + + scoped = runner.invoke(main, args=["--conf-dir", str(conf_dir), "plan", "-A"]) + expanded = runner.invoke( + main, + args=["--conf-dir", str(conf_dir), "plan", "-A", "--include-dependents"], + ) + + assert scoped.exit_code == 0 + assert "consumer" not in scoped.stdout + assert expanded.exit_code == 0 + assert "producer" in expanded.stdout + assert "consumer" in expanded.stdout + assert expanded.stdout.index("producer") < expanded.stdout.index("consumer") + + +def test_plan_include_dependents_without_auto_scope_is_usage_error( + runner: CliRunner, fake_terraform_bin: FakeTerraform, tmp_path: Path +) -> None: + _ = fake_terraform_bin + conf_dir = tmp_path / "conf" + copy_as_git_repo(PROJECT_TESTS_FIXTURES_DIR / "import_chain", conf_dir) + + result = runner.invoke( + main, args=["--conf-dir", str(conf_dir), "plan", "--include-dependents"] + ) + + assert result.exit_code != 0 + assert "--include-dependents" in result.output + + def test_plan_strategy_parallel_fail_at_end_stops_next_wave( runner: CliRunner, fake_terraform_bin: FakeTerraform ) -> None: diff --git a/tests/it/test_graph.py b/tests/it/test_graph.py index 377a727..261776a 100644 --- a/tests/it/test_graph.py +++ b/tests/it/test_graph.py @@ -7,7 +7,12 @@ import pytest from terranova.exceptions import CyclicImportError -from terranova.graph import DependencyGraph, build_dependency_graph, compute_waves +from terranova.graph import ( + DependencyGraph, + build_dependency_graph, + compute_waves, + with_dependents, +) from terranova.resources import ResourcesManifest _MANIFEST_NO_IMPORTS: Final[str] = """ @@ -107,6 +112,23 @@ def test_empty_graph_no_waves(self) -> None: assert compute_waves(graph) == [] +class TestWithDependents: + def test_includes_roots_and_transitive_importers(self) -> None: + graph = DependencyGraph( + nodes={"a", "b", "c", "d"}, + depends_on={"a": set(), "b": {"a"}, "c": {"b"}, "d": set()}, + ) + assert with_dependents(graph, {"a"}) == {"a", "b", "c"} + + def test_does_not_include_sources_of_a_root(self) -> None: + graph = DependencyGraph(nodes={"a", "b"}, depends_on={"a": set(), "b": {"a"}}) + assert with_dependents(graph, {"b"}) == {"b"} + + def test_no_roots_returns_empty(self) -> None: + graph = DependencyGraph(nodes={"a"}, depends_on={"a": set()}) + assert with_dependents(graph, set()) == set() + + class TestBuildDependencyGraph: def test_no_imports_all_independent(self, resources_dir: Path) -> None: dir_a, rel_a = _write_manifest_dir( diff --git a/tests/it/test_helpers.py b/tests/it/test_helpers.py index a2755e8..44320fb 100644 --- a/tests/it/test_helpers.py +++ b/tests/it/test_helpers.py @@ -15,6 +15,7 @@ auto_scope_resource_dirs, discover_resources, engine_versions_in_use, + expand_with_dependents, extract_import_vars, extract_output_var, find_all_resource_dirs, @@ -722,6 +723,48 @@ def test_empty_base_without_auto_scope_raises_usage_error( resolve_resource_dirs(tmp_path, resources_dir, None, False, "") +class TestExpandWithDependents: + def _repo(self, tmp_path: Path) -> dict[str, Path]: + resources = tmp_path / "resources" + groups = { + "producer": _write_manifest_dir(tmp_path, "resources", "producer"), + "consumer": _write_manifest_dir_importing( + resources, "consumer", "producer" + ), + "chained": _write_manifest_dir_importing(resources, "chained", "consumer"), + "unrelated": _write_manifest_dir(tmp_path, "resources", "unrelated"), + } + _init_git_repo(tmp_path) + return groups + + @pytest.mark.parametrize( + ("changed", "expected"), + [ + ("producer", ["chained", "consumer", "producer"]), + ("consumer", ["chained", "consumer"]), + ("chained", ["chained"]), + ("unrelated", ["unrelated"]), + ], + ) + def test_adds_direct_and_transitive_importers_only( + self, tmp_path: Path, resources_dir: Path, changed: str, expected: list[str] + ) -> None: + groups = self._repo(tmp_path) + (groups[changed] / "main.tf").write_text('resource "aws_x" "y" {}\n') + selected = auto_scope_resource_dirs(tmp_path, resources_dir) + + result = expand_with_dependents(resources_dir, selected) + + assert result == [(groups[name], name) for name in expected] + + def test_no_selected_group_returns_empty_list( + self, tmp_path: Path, resources_dir: Path + ) -> None: + self._repo(tmp_path) + + assert expand_with_dependents(resources_dir, []) == [] + + class TestResolveResourceDirs: def test_path_and_auto_scope_together_raises_usage_error( self, tmp_path: Path, resources_dir: Path