prevent attempting to update a diff for a deleted branch (#7668)

* prevent attempting to update a diff for a deleted branch

* add test for log message

* prevent attempting a diff update on a deleted branch

Author: ajtmccarty

backend/infrahub/core/branch/models.py

@@ -141,8 +141,9 @@ async def get_by_name(cls, name: str, db: InfrahubDatabase, ignore_deleting: boo
         """
 
         params: dict[str, Any] = {"name": name}
+        params["ignore_statuses"] = []
         if ignore_deleting:
-            params["ignore_statuses"] = [BranchStatus.DELETING.value]
+            params["ignore_statuses"].append(BranchStatus.DELETING.value)
 
         results = await db.execute_query(query=query, params=params, name="branch_get_by_name", type=QueryType.READ)
 

backend/infrahub/core/branch/tasks.py

@@ -502,9 +502,13 @@ async def post_process_branch_merge(source_branch: str, target_branch: str, cont
             parameters={"branch": target_branch, "source": GeneratorDefinitionRunSource.MERGE},
         )
 
+        active_branches = await Branch.get_list(db=db)
+        active_branch_names = {branch.name for branch in active_branches}
+
         for diff_root in branch_diff_roots:
             if (
                 diff_root.base_branch_name != diff_root.diff_branch_name
+                and diff_root.diff_branch_name in active_branch_names
                 and diff_root.tracking_id
                 and isinstance(diff_root.tracking_id, BranchTrackingId)
             ):

backend/infrahub/core/diff/tasks.py

@@ -8,6 +8,7 @@
 from infrahub.core.diff.models import RequestDiffUpdate  # noqa: TC001  needed for prefect flow
 from infrahub.core.diff.repository.repository import DiffRepository
 from infrahub.dependencies.registry import get_component_registry
+from infrahub.exceptions import BranchNotFoundError
 from infrahub.log import get_logger
 from infrahub.workers.dependencies import get_database, get_workflow
 from infrahub.workflows.catalogue import DIFF_REFRESH
@@ -24,7 +25,11 @@ async def update_diff(model: RequestDiffUpdate) -> None:
     async with database.start_session(read_only=False) as db:
         component_registry = get_component_registry()
         base_branch = await registry.get_branch(db=db, branch=registry.default_branch)
-        diff_branch = await registry.get_branch(db=db, branch=model.branch_name)
+        try:
+            diff_branch = await registry.get_branch(db=db, branch=model.branch_name)
+        except BranchNotFoundError:
+            log.warn(f"Branch {model.branch_name} not found, skipping diff update")
+            return
 
         diff_coordinator = await component_registry.get_component(DiffCoordinator, db=db, branch=diff_branch)
 

backend/tests/integration/diff/test_diff_update.py

@@ -29,6 +29,7 @@
     from tests.adapters.message_bus import BusSimulator
 
 BRANCH_NAME = "branch1"
+DELETED_BRANCH_NAME = "deleted_branch"
 PROPOSED_CHANGE_NAME = "branch1-pc"
 DIFF_UPDATE_QUERY = """
 mutation DiffUpdate($branch_name: String!, $wait_for_completion: Boolean = true) {
@@ -195,15 +196,36 @@ async def create_diff(self, db: InfrahubDatabase, initial_dataset, client: Infra
         john.age.value = 26  # type: ignore[attr-defined]
         await john.save(db=db)
 
+    @pytest.fixture(scope="class")
+    async def deleted_branch(self, db: InfrahubDatabase) -> Branch:
+        return await create_branch(db=db, branch_name=DELETED_BRANCH_NAME)
+
+    @pytest.fixture(scope="class")
+    async def diff_on_deleted_branch(
+        self, db: InfrahubDatabase, initial_dataset, client: InfrahubClient, deleted_branch: Branch
+    ) -> EnrichedDiffRoot:
+        kara = await NodeManager.get_one(db=db, branch=deleted_branch, id=initial_dataset["kara"].id)
+        kara.description.value = "I think she's an angel now"
+        await kara.save(db=db)
+
+        result = await client.execute_graphql(query=DIFF_UPDATE_QUERY, variables={"branch_name": DELETED_BRANCH_NAME})
+        assert result["DiffUpdate"]["ok"]
+
+        diff = await self.get_branch_diff(db=db, branch=deleted_branch)
+        assert len(diff.nodes) == 1
+
+        await deleted_branch.delete(db=db)
+        return diff
+
     @staticmethod
     async def get_branch_diff(db: InfrahubDatabase, branch: Branch) -> EnrichedDiffRoot:
         # Validate if the diff has been updated properly
         component_registry = get_component_registry()
         diff_repo = await component_registry.get_component(DiffRepository, db=db, branch=branch)
 
         return await diff_repo.get_one(
-            tracking_id=BranchTrackingId(name=BRANCH_NAME),
-            diff_branch_name=BRANCH_NAME,
+            tracking_id=BranchTrackingId(name=branch.name),
+            diff_branch_name=branch.name,
         )
 
     async def _get_proposed_change_and_data_validator(self, db) -> tuple[Node, Node]:
@@ -222,7 +244,7 @@ async def _get_proposed_change_and_data_validator(self, db) -> tuple[Node, Node]
         return (pc, data_validator)
 
     async def test_diff_first_update(
-        self, db: InfrahubDatabase, initial_dataset, create_diff, client: InfrahubClient
+        self, db: InfrahubDatabase, initial_dataset, create_diff, diff_on_deleted_branch, client: InfrahubClient
     ) -> None:
         """Validate if the diff is properly created the first time"""
 
@@ -1147,7 +1169,13 @@ async def test_create_another_proposed_change_data_checks_created(
         assert len(core_data_checks) == len(data_checks_by_conflict_id)
 
     async def test_merge_proposed_change(
-        self, db: InfrahubDatabase, initial_dataset, default_branch, client: InfrahubClient
+        self,
+        db: InfrahubDatabase,
+        initial_dataset,
+        default_branch,
+        diff_on_deleted_branch: EnrichedDiffRoot,
+        deleted_branch: Branch,
+        client: InfrahubClient,
     ) -> None:
         pc, _ = await self._get_proposed_change_and_data_validator(db=db)
         result = await client.execute_graphql(
@@ -1224,3 +1252,7 @@ async def test_merge_proposed_change(
         murphy_customer_rel = omnicorp_customers_rels_by_peer_id[murphy_id]
         owner_of_property = await murphy_customer_rel.get_owner(db=db)
         assert owner_of_property.get_id() == cardinality_many_property_conflict.expected_value
+
+        # validate diff not updated for deleted branch
+        fresh_deleted_branch_diff = await self.get_branch_diff(db=db, branch=deleted_branch)
+        assert fresh_deleted_branch_diff.to_time == diff_on_deleted_branch.to_time

backend/tests/unit/core/diff/test_diff_task.py

@@ -0,0 +1,41 @@
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+from infrahub.core.branch import Branch
+from infrahub.core.diff.models import RequestDiffUpdate
+from infrahub.core.diff.tasks import update_diff
+from infrahub.database import InfrahubDatabase
+from infrahub.dependencies.component.registry import ComponentDependencyRegistry
+from infrahub.dependencies.registry import get_component_registry
+
+
+class DummyComponentRegistry:
+    async def get_component(self, *args, **kwargs):
+        mock_component = AsyncMock()
+        # Needed so .run_update can be awaited and does nothing
+        mock_component.run_update = AsyncMock()
+        return mock_component
+
+
+@pytest.fixture
+def wrapped_component_registry() -> ComponentDependencyRegistry:
+    component_registry = get_component_registry()
+    wrapped_component_registry = AsyncMock(wraps=component_registry)
+
+    with patch("infrahub.dependencies.registry.get_component_registry", return_value=wrapped_component_registry):
+        yield wrapped_component_registry
+
+
+async def test_diff_update_for_deleted_branch(
+    db: InfrahubDatabase,
+    default_branch: Branch,
+    wrapped_component_registry: ComponentDependencyRegistry,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    diff_update = RequestDiffUpdate(branch_name="pretend_branch", name="diff")
+
+    await update_diff(diff_update)
+
+    wrapped_component_registry.get_component.assert_not_awaited()
+    assert "Branch pretend_branch not found, skipping diff update" in caplog.text

backend/tests/unit/message_bus/operations/event/test_branch.py

@@ -10,7 +10,9 @@
 from infrahub.core.diff.model.path import BranchTrackingId, EnrichedDiffRoot, NameTrackingId
 from infrahub.core.diff.models import RequestDiffUpdate
 from infrahub.core.diff.repository.repository import DiffRepository
+from infrahub.core.initialization import create_branch
 from infrahub.core.timestamp import Timestamp
+from infrahub.database import InfrahubDatabase
 from infrahub.dependencies.component.registry import ComponentDependencyRegistry
 from infrahub.generators.constants import GeneratorDefinitionRunSource
 from infrahub.services import InfrahubServices
@@ -40,7 +42,9 @@ def context():
     )
 
 
-async def test_merged(default_branch: Branch, prefect_test_fixture, context: InfrahubContext, init_service):
+async def test_merged(
+    db: InfrahubDatabase, default_branch: Branch, prefect_test_fixture, context: InfrahubContext, init_service
+):
     """
     Test that merge flow triggers corrects events/workflows. It does not actually test these events/workflows behaviors
     as they are mocked.
@@ -49,8 +53,10 @@ async def test_merged(default_branch: Branch, prefect_test_fixture, context: Inf
     source_branch_name = "cr1234"
     target_branch_name = "main"
     right_now = Timestamp()
-    tracked_diff_roots = [
-        EnrichedDiffRoot(
+    tracked_diff_roots = []
+
+    for _ in range(2):
+        tracked_diff_root = EnrichedDiffRoot(
             base_branch_name=target_branch_name,
             diff_branch_name=str(uuid4()),
             from_time=right_now,
@@ -59,10 +65,23 @@ async def test_merged(default_branch: Branch, prefect_test_fixture, context: Inf
             partner_uuid=str(uuid4()),
             tracking_id=BranchTrackingId(name=str(uuid4())),
         )
-        for _ in range(2)
-    ]
-    untracked_diff_roots = [
-        EnrichedDiffRoot(
+        tracked_diff_roots.append(tracked_diff_root)
+        await create_branch(db=db, branch_name=tracked_diff_root.diff_branch_name)
+
+    # diff root for a deleted branch
+    tracked_deleted_root = EnrichedDiffRoot(
+        base_branch_name=target_branch_name,
+        diff_branch_name=str(uuid4()),
+        from_time=right_now,
+        to_time=right_now,
+        uuid=str(uuid4()),
+        partner_uuid=str(uuid4()),
+        tracking_id=BranchTrackingId(name=str(uuid4())),
+    )
+
+    untracked_diff_roots = []
+    for _ in range(2):
+        untracked_diff_root = EnrichedDiffRoot(
             base_branch_name=target_branch_name,
             diff_branch_name=str(uuid4()),
             from_time=right_now,
@@ -71,10 +90,11 @@ async def test_merged(default_branch: Branch, prefect_test_fixture, context: Inf
             partner_uuid=str(uuid4()),
             tracking_id=NameTrackingId(name=str(uuid4())),
         )
-        for _ in range(2)
-    ]
+        await create_branch(db=db, branch_name=untracked_diff_root.diff_branch_name)
+        untracked_diff_roots.append(untracked_diff_root)
+
     diff_repo = AsyncMock(spec=DiffRepository)
-    diff_repo.get_roots_metadata.return_value = untracked_diff_roots + tracked_diff_roots
+    diff_repo.get_roots_metadata.return_value = untracked_diff_roots + tracked_diff_roots + [tracked_deleted_root]
     mock_component_registry = Mock(spec=ComponentDependencyRegistry)
     mock_get_component_registry = MagicMock(return_value=mock_component_registry)
     mock_component_registry.get_component.return_value = diff_repo

changelog/7666.fixed.md

@@ -0,0 +1 @@
+Prevent attempting diff update on a deleted branch. Log a warning instead.