diff --git a/backend/infrahub/git/base.py b/backend/infrahub/git/base.py index 32da2c63a50..22c7c4fed51 100644 --- a/backend/infrahub/git/base.py +++ b/backend/infrahub/git/base.py @@ -666,14 +666,24 @@ async def create_branch_in_git(self, branch_name: str, branch_id: str | None = N if branch_name in local_branches: return False - # TODO Catch potential exceptions coming from repo.git.branch & repo.git.worktree - repo.git.branch(branch_name) + remote_branch = None + if self.has_origin: + remote_branch = next( + (br for br in repo.remotes.origin.refs if br.name == f"origin/{branch_name}"), + None, + ) + + # When a matching remote branch exists, point the local branch directly at the + # remote tip. Branching from HEAD and then pulling would fail whenever the remote + # branch's history diverges from the default branch. + if remote_branch is not None: + repo.git.branch(branch_name, remote_branch.name) + else: + repo.git.branch(branch_name) + self.create_branch_worktree(branch_name=branch_name, branch_id=branch_id or branch_name) - # If there is not remote configured, we are done - # Since the branch is a match for the main branch we don't need to create a commit worktree - # If there is a remote, Check if there is an existing remote branch with the same name and if so track it. - if not self.has_origin: + if remote_branch is None: log.debug( f"Branch {branch_name} created in Git without tracking a remote branch.", repository=self.name, @@ -681,24 +691,14 @@ async def create_branch_in_git(self, branch_name: str, branch_id: str | None = N ) return True - remote_branch = [br for br in repo.remotes.origin.refs if br.name == f"origin/{branch_name}"] - - if remote_branch: - br_repo = self.get_git_repo_worktree(identifier=branch_name) - br_repo.head.reference.set_tracking_branch(remote_branch[0]) - try: - br_repo.remotes.origin.pull(branch_name) - except GitCommandError as exc: - await self._raise_enriched_error(error=exc, branch_name=branch_name) - self.create_commit_worktree(str(br_repo.head.reference.commit)) - log.debug( - f"Branch {branch_name} created in Git, tracking remote branch {remote_branch[0]}.", - repository=self.name, - branch=branch_name, - ) - else: - log.debug(f"Branch {branch_name} created in Git without tracking a remote branch.", repository=self.name) - + br_repo = self.get_git_repo_worktree(identifier=branch_name) + br_repo.head.reference.set_tracking_branch(remote_branch) + self.create_commit_worktree(str(br_repo.head.reference.commit)) + log.debug( + f"Branch {branch_name} created in Git, tracking remote branch {remote_branch.name}.", + repository=self.name, + branch=branch_name, + ) return True def create_commit_worktree(self, commit: str) -> bool | Worktree: @@ -881,8 +881,8 @@ def validate_remote_branch(self, branch_name: str) -> bool: """Process a remote branch to validate that we can use it safely. - Make sure that the branch name won't conflict with infrahub's default branch - - Make sure that a representation if the branch can be created in the database - - Make sure that there are no conflicts that would prevent it from being merged + - Make sure that a representation of the branch can be created in the database + - Warn (but do not block) when the branch would conflict with the default branch on merge """ if branch_name == registry.default_branch and branch_name != self.default_branch: # If the default branch of Infrahub and the git repository differs we map the repository @@ -900,7 +900,8 @@ def validate_remote_branch(self, branch_name: str) -> bool: ) return False - # Make sure the branch won't conflict on merge + # Surface a warning when the branch conflicts with the default branch so users + # know a future merge will be rejected, but still allow the import to proceed. try: has_conflicts = self.has_conflicting_changes(target_branch=self.default_branch, source_branch=branch_name) except GitCommandError as exc: @@ -910,17 +911,13 @@ def validate_remote_branch(self, branch_name: str) -> bool: repository=self.name, error=str(exc), ) - return False + return True if has_conflicts: get_run_logger().warning( - f"Remote branch {branch_name} will cause conflicts, they need to be resolved before importing the branch into Infrahub" + f"Remote branch {branch_name} conflicts with {self.default_branch}; " + "the merge will be rejected until the conflict is resolved upstream" ) - return False - - # Find the commit on the remote branch - # Check out the commit in a worktree - # Validate return True diff --git a/backend/tests/unit/git/test_git_repository.py b/backend/tests/unit/git/test_git_repository.py index 739cdf7af72..1ba483ef611 100644 --- a/backend/tests/unit/git/test_git_repository.py +++ b/backend/tests/unit/git/test_git_repository.py @@ -1,4 +1,7 @@ +import logging +from collections.abc import Iterator from pathlib import Path +from unittest.mock import patch import pytest from git import Repo @@ -11,16 +14,110 @@ from tests.helpers.file_repo import MultipleStagesFileRepo from tests.helpers.test_client import dummy_async_request +PREFECT_LOGGER_NAME = "infrahub.git.base" -async def test_has_conflicting_changes_no_false_positive( + +@pytest.fixture +def patch_prefect_logger() -> Iterator[None]: + """Replace Prefect's `get_run_logger` with a stdlib logger so calls outside a flow context succeed.""" + with patch( + "infrahub.git.base.get_run_logger", + return_value=logging.getLogger(PREFECT_LOGGER_NAME), + ): + yield + + +def _build_source_with_conflicting_branches(source_dir: Path) -> None: + """Initialize a git source repo with `main` and `change1` whose tips edit the same lines.""" + source = Repo.init(source_dir, initial_branch="main") + with source.config_writer() as cfg: + cfg.set_value("user", "name", "Test") + cfg.set_value("user", "email", "test@test.local") + target = source_dir / "data.txt" + target.write_text("line 1\nline 2\nline 3\n", encoding="utf-8") + source.index.add(["data.txt"]) + base_commit = source.index.commit("base") + + source.git.checkout("-b", "change1") + target.write_text("change1 version\nline 2\nline 3\n", encoding="utf-8") + source.index.add(["data.txt"]) + source.index.commit("change on change1") + + source.git.checkout("main") + source.git.reset("--hard", base_commit.hexsha) + target.write_text("main version\nline 2\nline 3\n", encoding="utf-8") + source.index.add(["data.txt"]) + source.index.commit("change on main") + + +async def _build_repository_with_conflict( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, name: str = "conflicting-repo" +) -> InfrahubRepository: + """Build an `InfrahubRepository` whose remote has `main` and a divergent `change1`. + + Asserts the pre-condition that the two branches actually conflict so a helper drift + fails loudly instead of silently making downstream assertions meaningless. + """ + repos_dir = tmp_path / "repositories" + repos_dir.mkdir() + monkeypatch.setattr(registry, "_default_branch", "main") + monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) + + source_dir = tmp_path / "source-repo" + source_dir.mkdir() + _build_source_with_conflicting_branches(source_dir) + + repository = await InfrahubRepository.new( + id=UUIDT.new(), + name=name, + location=str(source_dir), + default_branch_name="main", + client=InfrahubClient(config=Config(requester=dummy_async_request)), + ) + assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") + return repository + + +async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """has_conflicting_changes() should not report false positives when. + """A remote branch that conflicts with the default branch must still be imported locally. + + The local branch should land at the remote tip so that downstream merge attempts can + surface the conflict at merge time, rather than aborting the entire import. + """ + repository = await _build_repository_with_conflict(tmp_path, monkeypatch) + + remote_branches = repository.get_branches_from_remote() + expected_commit = remote_branches["change1"].commit + + await repository.create_branch_in_git(branch_name="change1", branch_id=str(UUIDT.new())) + + local_branches = repository.get_branches_from_local(include_worktree=False) + assert "change1" in local_branches + assert local_branches["change1"].commit == expected_commit - file content contains conflict marker characters like '======='. +async def test_validate_remote_branch_allows_conflicting_branch( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + patch_prefect_logger: None, +) -> None: + """validate_remote_branch must accept a branch that conflicts with the default branch. + + Skipping the branch would prevent it from being imported. The conflict is surfaced at + merge time instead. """ + repository = await _build_repository_with_conflict(tmp_path, monkeypatch) + assert repository.validate_remote_branch(branch_name="change1") is True + + +async def test_has_conflicting_changes_no_false_positive( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """has_conflicting_changes() must not flag a diff that only adds lines containing '======='.""" repos_dir = tmp_path / "repositories" repos_dir.mkdir() monkeypatch.setattr(registry, "_default_branch", "main") diff --git a/changelog/2293.fixed.md b/changelog/2293.fixed.md new file mode 100644 index 00000000000..d808860ca1a --- /dev/null +++ b/changelog/2293.fixed.md @@ -0,0 +1 @@ +Importing a Git repository now succeeds even when one of its branches has a merge conflict against the default branch. The conflicting branch is imported as-is and a warning is logged; the conflict is reported when a user later attempts to merge the branch. diff --git a/dev/guidelines/backend/testing.md b/dev/guidelines/backend/testing.md index 9bfd018b59c..101dd02d0dc 100644 --- a/dev/guidelines/backend/testing.md +++ b/dev/guidelines/backend/testing.md @@ -240,6 +240,7 @@ If you find yourself wanting to mock: - External HTTP APIs with no test mode (use `responses` or `httpx_mock` sparingly) - Time-dependent behavior (`freezegun`) +- Prefect's `get_run_logger` when calling a `.fn` outside a flow context — patch it to return a stdlib `logging.getLogger(...)` so `caplog` can capture output. See [Backend Testing — Logging](../../knowledge/backend/testing.md#logging-use-caplog-instead-of-mocking-get_run_logger) for the pattern. Even in these cases, prefer adapter patterns when the dependency is used widely. diff --git a/dev/rules/testing-python.md b/dev/rules/testing-python.md index ec1ee893c59..39af0e456b6 100644 --- a/dev/rules/testing-python.md +++ b/dev/rules/testing-python.md @@ -30,6 +30,7 @@ Acceptable exceptions only: - External HTTP APIs with no test mode: use `httpx_mock` or `responses` - Time-dependent behavior: `freezegun` +- Prefect's `get_run_logger`: when calling a Prefect-decorated function via `.fn` outside a flow context, patch `get_run_logger` to return a stdlib `logging.getLogger(...)` so `caplog` can capture output. See `dev/knowledge/backend/testing.md` for the full pattern. ## Parametrized tests