From 1190a3b1131dab3ef8e971012d102133a4b8c622 Mon Sep 17 00:00:00 2001 From: Adrien Mannocci Date: Fri, 16 Oct 2026 19:21:12 +0200 Subject: [PATCH] feat: add --base option to scope auto-scope to a git ref --auto-scope only compared the working tree to HEAD, so a clean CI checkout selected nothing. With --base , changes are taken relative to the merge base of the ref and HEAD, including committed ones. --- README.md | 5 ++ src/terranova/binds.py | 21 +++++-- src/terranova/commands/apply.py | 7 ++- src/terranova/commands/destroy.py | 10 +++- src/terranova/commands/docs.py | 14 ++++- src/terranova/commands/helpers.py | 37 ++++++++++-- src/terranova/commands/migrate.py | 14 ++++- src/terranova/commands/plan.py | 7 ++- src/terranova/exceptions.py | 12 ++++ tests/conftest.py | 8 +++ tests/e2e/conftest.py | 7 +-- tests/e2e/test_apply_command.py | 17 ++++++ tests/e2e/test_plan_command.py | 40 ++++++++++++- tests/it/test_helpers.py | 95 +++++++++++++++++++++++++++++-- 14 files changed, 268 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 0574343..9c2aa0e 100644 --- a/README.md +++ b/README.md @@ -351,6 +351,10 @@ terranova apply --strategy parallel --auto-approve --fail-at-end outside any resource group is ignored, and several changes in one group select it only once. - A change below the `shared` directory selects every resource group whose manifest `dependencies` reference it: a `source` that is a directory matches any file inside it. +- With `--base ` (e.g. `--base origin/main`), changes are compared to the merge base of `` and + `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). - 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. @@ -358,6 +362,7 @@ terranova apply --strategy parallel --auto-approve --fail-at-end ```bash terranova plan --auto-scope terranova apply -A --auto-approve +terranova plan -A --base origin/main ``` - To apply exactly what was planned, combine it with `plan --out`: the saved `.tnplan` file only contains diff --git a/src/terranova/binds.py b/src/terranova/binds.py index 13bf8d6..72c59eb 100644 --- a/src/terranova/binds.py +++ b/src/terranova/binds.py @@ -584,18 +584,31 @@ def repo_root(self) -> str: ).exec() return capture.getvalue().strip() - def changed_files(self) -> list[str]: + def merge_base(self, ref: str) -> str: + """Show the best common ancestor commit between `ref` and HEAD.""" + capture = StringIO() + # `--` keeps a ref that looks like an option from being parsed as one, and + # stderr is kept out of `capture` so a git warning can't end up in the hash. + self._cmd.args("merge-base", "--", ref, "HEAD").stdout(capture).stderr( + StringIO() + ).exec() + return capture.getvalue().strip() + + def changed_files(self, base: str = "HEAD") -> list[str]: """ - List files changed relative to HEAD (staged + unstaged) plus untracked files. + List files changed relative to `base` (staged + unstaged) plus untracked files. + + Args: + base: commit-ish the working tree is compared to. Returns: paths relative to `cwd()` - callers should set `cwd()` to the repo root first (via `repo_root()`) so output is unambiguous. """ capture = StringIO() - self._cmd.args("diff", "--name-only", "HEAD").stdout(capture).stderr( + self._cmd.args("diff", "--name-only", "--no-renames", base).stdout( capture - ).exec() + ).stderr(capture).exec() tracked = [line for line in capture.getvalue().splitlines() if line] capture = StringIO() diff --git a/src/terranova/commands/apply.py b/src/terranova/commands/apply.py index aa694d2..a4d056a 100644 --- a/src/terranova/commands/apply.py +++ b/src/terranova/commands/apply.py @@ -25,6 +25,8 @@ TerraformTask, auto_scope_option, auto_scope_resource_dirs, + base_option, + check_base_requires_auto_scope, complete_resource_path, execute_tasks, parse_execution_plan, @@ -97,6 +99,7 @@ def run(self) -> None: "path_or_plan", type=str, required=False, shell_complete=complete_resource_path ) @auto_scope_option +@base_option @click.option( "--auto-approve", help="Skip interactive approval of plan before applying.", @@ -131,6 +134,7 @@ def apply( ctx: AppContext, path_or_plan: str | None, auto_scope: bool, + base: str | None, auto_approve: bool, target: str, fail_at_end: bool, @@ -141,13 +145,14 @@ def apply( # Check if there is a plan to apply execution_plan: dict[str, str] | None paths: list[tuple[Path, str]] + check_base_requires_auto_scope(auto_scope, base) if auto_scope and path_or_plan: raise click.UsageError( "`--auto-scope`/`-A` can't be combined with `path_or_plan`." ) if auto_scope: execution_plan = None - paths = auto_scope_resource_dirs(ctx.conf_dir, ctx.resources_dir) + paths = auto_scope_resource_dirs(ctx.conf_dir, ctx.resources_dir, base) 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/destroy.py b/src/terranova/commands/destroy.py index 6319ced..5e03ccb 100644 --- a/src/terranova/commands/destroy.py +++ b/src/terranova/commands/destroy.py @@ -19,6 +19,7 @@ from terranova.commands.helpers import ( auto_scope_option, + base_option, complete_resource_path, mount_context, resolve_resource_dirs, @@ -30,11 +31,16 @@ @click.command("destroy") @click.argument("path", type=str, required=False, shell_complete=complete_resource_path) @auto_scope_option +@base_option @click.pass_obj -def destroy(ctx: AppContext, path: str | None, auto_scope: bool) -> None: +def destroy( + ctx: AppContext, path: str | None, auto_scope: bool, base: str | None +) -> None: """Destroy previously-created resources.""" # Find all resources manifests - paths = resolve_resource_dirs(ctx.conf_dir, ctx.resources_dir, path, auto_scope) + paths = resolve_resource_dirs( + ctx.conf_dir, ctx.resources_dir, path, auto_scope, base + ) # Format all paths for full_path, rel_path in paths: diff --git a/src/terranova/commands/docs.py b/src/terranova/commands/docs.py index c24a184..61f950d 100644 --- a/src/terranova/commands/docs.py +++ b/src/terranova/commands/docs.py @@ -25,6 +25,7 @@ from terranova.commands.helpers import ( auto_scope_option, + base_option, complete_resource_path, discover_resources, read_manifest, @@ -42,6 +43,7 @@ def format_markdown(text: str) -> str: @click.command("docs") @click.argument("path", type=str, required=False, shell_complete=complete_resource_path) @auto_scope_option +@base_option @click.option( "--docs-dir", help="Docs directory path.", @@ -50,10 +52,18 @@ def format_markdown(text: str) -> str: default="./docs", ) @click.pass_obj -def docs(ctx: AppContext, path: str | None, auto_scope: bool, docs_dir: Path) -> None: +def docs( + ctx: AppContext, + path: str | None, + auto_scope: bool, + base: str | None, + docs_dir: Path, +) -> None: """Generate documentation for all resources.""" # Find all resources manifests - paths = resolve_resource_dirs(ctx.conf_dir, ctx.resources_dir, path, auto_scope) + paths = resolve_resource_dirs( + ctx.conf_dir, ctx.resources_dir, path, auto_scope, base + ) jobs: list[tuple[Path, Path]] = [ (full_path, docs_dir.joinpath(rel_path)) for full_path, rel_path in paths ] diff --git a/src/terranova/commands/helpers.py b/src/terranova/commands/helpers.py index 0d1f2ad..50455aa 100644 --- a/src/terranova/commands/helpers.py +++ b/src/terranova/commands/helpers.py @@ -29,6 +29,7 @@ from terranova.engines import default_engine_manager from terranova.exceptions import ( EngineError, + GitBaseRefError, GitRepositoryError, GraphError, ImportOutputError, @@ -58,6 +59,16 @@ ) """Shared `--auto-scope`/`-A` option for `plan`, `apply`, `destroy` and `docs`.""" +base_option = click.option( + "--base", + help="With `--auto-scope`, compare against the merge base of this git ref " + + "and HEAD (e.g. `origin/main`) instead of HEAD, so changes already " + + "committed on the branch are included.", + type=str, + default=None, +) +"""Shared `--base` option, only valid together with `--auto-scope`.""" + flat_strategy_option = click.option( "--strategy", help="Execution strategy across resource groups: `sequential` (default) runs " @@ -307,12 +318,13 @@ def _match_shared_dependents( def auto_scope_resource_dirs( - conf_dir: Path, resources_dir: Path + conf_dir: Path, resources_dir: Path, base: str | None = None ) -> list[tuple[Path, str]]: """ Scope resource dirs to those affected by the current git diff (working tree and staged changes vs HEAD, including untracked files). A change below the `shared` dir also selects every group depending on it. + With `base`, changes are taken relative to the merge base of `base` and HEAD. This function handle errors by logging and exiting. """ # `conf_dir` is guaranteed to exist (`--conf-dir` requires it), unlike @@ -327,7 +339,13 @@ def auto_scope_resource_dirs( GitRepositoryError(conf_dir), ) git.cwd(root) - changed_files = [root / rel for rel in git.changed_files()] + base_commit = "HEAD" + if base is not None: + try: + base_commit = git.merge_base(base) + except ErrorReturnCode: + log.fatal(f"resolve the git base ref `{base}`", GitBaseRefError(base)) + changed_files = [root / rel for rel in git.changed_files(base_commit)] all_dirs = find_all_resource_dirs(resources_dir) matched = dict(_match_resource_dirs(resources_dir, all_dirs, changed_files)) @@ -339,16 +357,27 @@ def auto_scope_resource_dirs( return sorted(matched.items(), key=lambda entry: entry[1]) +def check_base_requires_auto_scope(auto_scope: bool, base: str | None) -> None: + """Reject `--base` given without `--auto-scope`.""" + if base is not None and not auto_scope: + raise click.UsageError("`--base` requires `--auto-scope`/`-A`.") + + def resolve_resource_dirs( - conf_dir: Path, resources_dir: Path, path: str | None, auto_scope: bool + conf_dir: Path, + resources_dir: Path, + path: str | None, + auto_scope: bool, + base: str | None = None, ) -> list[tuple[Path, str]]: """Shared `path`/`--auto-scope` resolution for `plan`, `destroy` and `docs`.""" + check_base_requires_auto_scope(auto_scope, base) if auto_scope and path: raise click.UsageError( "`--auto-scope`/`-A` can't be combined with an explicit `path`." ) if auto_scope: - return auto_scope_resource_dirs(conf_dir, resources_dir) + return auto_scope_resource_dirs(conf_dir, resources_dir, base) return resource_dirs(resources_dir, path) diff --git a/src/terranova/commands/migrate.py b/src/terranova/commands/migrate.py index 666e860..d0383cb 100644 --- a/src/terranova/commands/migrate.py +++ b/src/terranova/commands/migrate.py @@ -19,6 +19,7 @@ from terranova.commands.helpers import ( auto_scope_option, + base_option, complete_resource_path, resolve_resource_dirs, ) @@ -31,6 +32,7 @@ @click.command("migrate") @click.argument("path", type=str, required=False, shell_complete=complete_resource_path) @auto_scope_option +@base_option @click.option( "--dry-run", help="Report what would change without writing any manifest.", @@ -38,9 +40,17 @@ default=False, ) @click.pass_obj -def migrate(ctx: AppContext, path: str | None, auto_scope: bool, dry_run: bool) -> None: +def migrate( + ctx: AppContext, + path: str | None, + auto_scope: bool, + base: str | None, + dry_run: bool, +) -> None: """Migrate manifests to the latest manifest version in batch.""" - dirs = resolve_resource_dirs(ctx.conf_dir, ctx.resources_dir, path, auto_scope) + dirs = resolve_resource_dirs( + ctx.conf_dir, ctx.resources_dir, path, auto_scope, base + ) # Recipes prompt on first use only, then the answers apply to every manifest. book = RecipeBook() diff --git a/src/terranova/commands/plan.py b/src/terranova/commands/plan.py index 2c8550c..50375d8 100644 --- a/src/terranova/commands/plan.py +++ b/src/terranova/commands/plan.py @@ -25,6 +25,7 @@ from terranova.commands.helpers import ( TerraformTask, auto_scope_option, + base_option, complete_resource_path, execute_tasks, read_manifests_and_waves, @@ -112,6 +113,7 @@ def run(self) -> None: @click.command("plan") @click.argument("path", type=str, required=False, shell_complete=complete_resource_path) @auto_scope_option +@base_option @click.option( "--input/--no-input", help="Ask for input for variables if not directly set.", @@ -172,6 +174,7 @@ def plan( ctx: AppContext, path: str | None, auto_scope: bool, + base: str | None, input: bool, no_color: bool, parallelism: int, @@ -183,7 +186,9 @@ def plan( ) -> None: """Show changes required by the current configuration.""" # Find all resources manifests - paths = resolve_resource_dirs(ctx.conf_dir, ctx.resources_dir, path, auto_scope) + paths = resolve_resource_dirs( + ctx.conf_dir, ctx.resources_dir, path, auto_scope, base + ) # Execution plan execution_plan: dict[str, str] = {} diff --git a/src/terranova/exceptions.py b/src/terranova/exceptions.py index 14b96f4..325ca72 100644 --- a/src/terranova/exceptions.py +++ b/src/terranova/exceptions.py @@ -127,6 +127,18 @@ def __init__(self, resources_dir: Path) -> None: ) +class GitBaseRefError(ExplainedError): + """Represents a failure to resolve the git base ref given to `--base`.""" + + def __init__(self, ref: str) -> None: + """Init git base ref error.""" + super().__init__( + cause=f"can't compute the merge base between `{ref}` and HEAD", + resolution="Check that the ref exists and that the history is available " + + "(e.g. `fetch-depth: 0` on a CI checkout).", + ) + + class GraphError(ExplainedError): """Represents an error building a dependency graph between resource groups.""" diff --git a/tests/conftest.py b/tests/conftest.py index 26d21e8..736fa57 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ import pytest +from terranova.binds import Git from terranova.utils import IS_WINDOWS, log @@ -44,6 +45,13 @@ def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: item.add_marker(skip) +def commit_all(path: Path, message: str) -> None: + """Stage everything in the git repo at `path` and commit it.""" + git = Git(path) + git.add() + git.commit(message, author=("test", "test@example.com")) + + _FAKE_TERRAFORM_SCRIPT = """#!/usr/bin/env python3 import base64 import json diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index de9382e..d699a0a 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -27,6 +27,7 @@ from terranova.engines import EngineManager from terranova.exceptions import EngineError from terranova.resources import ResourcesEngine +from tests.conftest import commit_all # Needs `terraform_data` (1.4+); pinned so the checksum-verified download is reproducible. REAL_TERRAFORM_VERSION: Final[str] = "1.9.8" @@ -62,10 +63,8 @@ def copy_as_git_repo(fixture_dir: Path, dest: Path) -> None: """ shutil.copytree(fixture_dir, dest) - git = Git(dest) - git.init() - git.add() - git.commit("initial", author=("test", "test@example.com")) + Git(dest).init() + commit_all(dest, "initial") def assert_result(result: Result) -> tuple[str, str | None]: diff --git a/tests/e2e/test_apply_command.py b/tests/e2e/test_apply_command.py index 2a13cf1..c144fe6 100644 --- a/tests/e2e/test_apply_command.py +++ b/tests/e2e/test_apply_command.py @@ -265,6 +265,23 @@ def test_apply_auto_scope_and_path_or_plan_together_is_usage_error( assert "--auto-scope" in result.output +def test_apply_base_without_auto_scope_is_usage_error( + runner: CliRunner, fake_terraform_bin: FakeTerraform, tmp_path: Path +) -> None: + _ = fake_terraform_bin + fixture_dir = PROJECT_TESTS_FIXTURES_DIR / "plan_multi_group" + conf_dir = tmp_path / "conf" + copy_as_git_repo(fixture_dir, conf_dir) + + result = runner.invoke( + main, + args=["--conf-dir", str(conf_dir), "apply", "--base", "HEAD~1"], + ) + + assert result.exit_code != 0 + assert "--base" 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 86d3cde..9b43630 100644 --- a/tests/e2e/test_plan_command.py +++ b/tests/e2e/test_plan_command.py @@ -7,7 +7,7 @@ from terranova.cli import main from tests import PROJECT_TESTS_FIXTURES_DIR -from tests.conftest import FakeTerraform +from tests.conftest import FakeTerraform, commit_all from tests.e2e.conftest import copy_as_git_repo @@ -240,6 +240,44 @@ def test_plan_auto_scope_and_path_together_is_usage_error( assert "--auto-scope" in result.output +def test_plan_auto_scope_base_includes_committed_changes( + runner: CliRunner, fake_terraform_bin: FakeTerraform, tmp_path: Path +) -> None: + _ = fake_terraform_bin + fixture_dir = PROJECT_TESTS_FIXTURES_DIR / "plan_multi_group" + conf_dir = tmp_path / "conf" + copy_as_git_repo(fixture_dir, conf_dir) + manifest = conf_dir / "resources" / "group_a" / "manifest.yml" + manifest.write_text(manifest.read_text() + "\n# changed\n") + commit_all(conf_dir, "second") + + # The working tree is clean (as in CI): only `--base` sees the commit. + result = runner.invoke( + main, + args=["--conf-dir", str(conf_dir), "plan", "-A", "--base", "HEAD~1"], + ) + + assert result.exit_code == 0 + assert "group_a" in result.stdout + assert "group_b" not in result.stdout + + +def test_plan_base_without_auto_scope_is_usage_error( + runner: CliRunner, fake_terraform_bin: FakeTerraform, tmp_path: Path +) -> None: + _ = fake_terraform_bin + fixture_dir = PROJECT_TESTS_FIXTURES_DIR / "plan_multi_group" + conf_dir = tmp_path / "conf" + copy_as_git_repo(fixture_dir, conf_dir) + + result = runner.invoke( + main, args=["--conf-dir", str(conf_dir), "plan", "--base", "HEAD~1"] + ) + + assert result.exit_code != 0 + assert "--base" 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_helpers.py b/tests/it/test_helpers.py index ae130d1..a2755e8 100644 --- a/tests/it/test_helpers.py +++ b/tests/it/test_helpers.py @@ -32,7 +32,7 @@ Selector, ) from terranova.utils import AppContext -from tests.conftest import FakeTerraform +from tests.conftest import FakeTerraform, commit_all _VALID_MANIFEST: Final[str] = """ version: "1.0" @@ -44,10 +44,8 @@ def _init_git_repo(path: Path) -> None: """Init a real git repo at `path` and commit everything currently in it.""" - git = Git(path) - git.init() - git.add() - git.commit("initial", author=("test", "test@example.com")) + Git(path).init() + commit_all(path, "initial") def _write_manifest_dir(base: Path, *parts: str) -> Path: @@ -637,6 +635,93 @@ def test_group_changed_directly_and_through_shared_is_listed_once( assert result == [(group_a, "group_a")] +class TestAutoScopeBaseRef: + def test_base_picks_up_committed_changes_since_the_merge_base( + self, tmp_path: Path, resources_dir: Path + ) -> None: + group_a = _write_manifest_dir(tmp_path, "resources", "group_a") + _write_manifest_dir(tmp_path, "resources", "group_b") + _init_git_repo(tmp_path) + (group_a / "main.tf").write_text('resource "aws_x" "y" {}\n') + commit_all(tmp_path, "second") + + result = auto_scope_resource_dirs(tmp_path, resources_dir, base="HEAD~1") + + assert result == [(group_a, "group_a")] + + def test_base_also_includes_uncommitted_changes( + self, tmp_path: Path, resources_dir: Path + ) -> None: + group_a = _write_manifest_dir(tmp_path, "resources", "group_a") + group_b = _write_manifest_dir(tmp_path, "resources", "group_b") + _init_git_repo(tmp_path) + (group_a / "main.tf").write_text('resource "aws_x" "y" {}\n') + commit_all(tmp_path, "second") + (group_b / "main.tf").write_text('resource "aws_x" "z" {}\n') + + result = auto_scope_resource_dirs(tmp_path, resources_dir, base="HEAD~1") + + assert result == [(group_a, "group_a"), (group_b, "group_b")] + + def test_base_applies_to_shared_dependencies( + self, tmp_path: Path, resources_dir: Path + ) -> None: + shared = tmp_path / "shared" + shared.mkdir() + (shared / "github.tf").write_text("# v1\n") + group_a = _write_manifest_dir_depending_on( + tmp_path / "resources", "group_a", "github.tf" + ) + _init_git_repo(tmp_path) + (shared / "github.tf").write_text("# v2\n") + commit_all(tmp_path, "second") + + result = auto_scope_resource_dirs(tmp_path, resources_dir, base="HEAD~1") + + assert result == [(group_a, "group_a")] + + def test_unknown_base_ref_calls_log_fatal( + self, tmp_path: Path, resources_dir: Path + ) -> None: + _write_manifest_dir(tmp_path, "resources", "group_a") + _init_git_repo(tmp_path) + + with pytest.raises(Exit): + auto_scope_resource_dirs(tmp_path, resources_dir, base="no-such-ref") + + +class TestAutoScopeBaseRefEdgeCases: + def test_renamed_file_selects_both_source_and_destination_groups( + self, tmp_path: Path, resources_dir: Path + ) -> None: + group_a = _write_manifest_dir(tmp_path, "resources", "group_a") + group_b = _write_manifest_dir(tmp_path, "resources", "group_b") + (group_a / "main.tf").write_text('resource "aws_x" "y" {}\n' * 20) + _init_git_repo(tmp_path) + (group_a / "main.tf").rename(group_b / "main.tf") + commit_all(tmp_path, "move") + + result = auto_scope_resource_dirs(tmp_path, resources_dir, base="HEAD~1") + + assert result == [(group_a, "group_a"), (group_b, "group_b")] + + @pytest.mark.parametrize("base", ["", "--no-such-option", "no-such-ref"]) + def test_invalid_base_calls_log_fatal( + self, tmp_path: Path, resources_dir: Path, base: str + ) -> None: + _write_manifest_dir(tmp_path, "resources", "group_a") + _init_git_repo(tmp_path) + + with pytest.raises(Exit): + auto_scope_resource_dirs(tmp_path, resources_dir, base=base) + + def test_empty_base_without_auto_scope_raises_usage_error( + self, tmp_path: Path, resources_dir: Path + ) -> None: + with pytest.raises(click.UsageError): + resolve_resource_dirs(tmp_path, resources_dir, None, False, "") + + class TestResolveResourceDirs: def test_path_and_auto_scope_together_raises_usage_error( self, tmp_path: Path, resources_dir: Path