Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion backend/infrahub/core/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from infrahub.core.diff.model.path import BranchTrackingId
from infrahub.core.manager import NodeManager
from infrahub.core.models import SchemaUpdateValidationResult
from infrahub.core.protocols import CoreRepository
from infrahub.core.protocols import CoreReadOnlyRepository, CoreRepository
from infrahub.core.registry import registry
from infrahub.core.timestamp import Timestamp
from infrahub.exceptions import MergeFailedError, ValidationError
Expand Down Expand Up @@ -223,6 +223,30 @@ async def rollback(self) -> None:
await self.diff_merger.rollback(at=self._merge_at)

async def merge_repositories(self) -> None:
await self.merge_core_read_only_repositories()
await self.merge_core_repositories()

async def merge_core_read_only_repositories(self) -> None:
repos_in_main_list = await NodeManager.query(schema=CoreReadOnlyRepository, db=self.db)
repos_in_main = {repo.id: repo for repo in repos_in_main_list}

repos_in_branch_list = await NodeManager.query(
schema=CoreReadOnlyRepository, db=self.db, branch=self.source_branch
)
for repo in repos_in_branch_list:
if repo.id not in repos_in_main:
continue

model = GitRepositoryMerge(
repository_id=repo.id,
repository_name=repo.name.value,
source_branch=self.source_branch.name,
destination_branch=self.destination_branch.name,
destination_branch_id=str(self.destination_branch.get_uuid()),
)
await self.workflow.submit_workflow(workflow=GIT_REPOSITORIES_MERGE, parameters={"model": model})
Comment on lines +229 to +247
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify whether read-only repositories should have status checks.

The merge_core_read_only_repositories method merges all read-only repositories found in both branches without checking internal_status (like the INACTIVE check at line 260 in merge_core_repositories) or conditional merge criteria (like sync_with_git at line 263). This could potentially merge repositories that shouldn't be merged.

Run this script to check if read-only repositories have an internal_status field:

If read-only repositories do have an internal_status field, consider adding similar checks as in merge_core_repositories.


🏁 Script executed:

#!/bin/bash
# Check the CoreReadOnlyRepository protocol definition

ast-grep --pattern $'class CoreReadOnlyRepository($$$) {
  $$$
}'

Length of output: 0



---

🏁 Script executed:

```shell
#!/bin/bash
# Search for CoreReadOnlyRepository definition/import
rg -n "CoreReadOnlyRepository" --type=py -B2 -A5 | head -50

Length of output: 4349


🏁 Script executed:

#!/bin/bash
# Look at the merge_core_repositories method to understand the pattern at lines 260 and 263
rg -n "def merge_core_repositories" --type=py -A35 backend/infrahub/core/merge.py

Length of output: 1551


🏁 Script executed:

#!/bin/bash
# Search for internal_status field usage in the codebase
rg -n "internal_status" --type=py -B2 -A2 | head -60

Length of output: 4149


🏁 Script executed:

#!/bin/bash
# Search for sync_with_git field usage
rg -n "sync_with_git" --type=py -B2 -A2 | head -60

Length of output: 4110



---

🏁 Script executed:

```shell
#!/bin/bash
# Find CoreReadOnlyRepository protocol definition
fd "protocols" --type=d backend/infrahub/core/

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for the protocol definition more broadly
rg -n "CoreReadOnlyRepository" --type=py -B3 -A20 backend/infrahub/core/protocols.py

Length of output: 668


🏁 Script executed:

#!/bin/bash
# Check the entire merge_core_read_only_repositories context in the merge.py file
rg -n "def merge_core_read_only_repositories" --type=py -B5 -A30 backend/infrahub/core/merge.py

Length of output: 1846


🏁 Script executed:

#!/bin/bash
# Check if there are any comments or context about read_only repository merging
rg -n "read.only|read_only" --type=py -B3 -A3 backend/infrahub/core/merge.py

Length of output: 479



---

🏁 Script executed:

```shell
#!/bin/bash
# Search for CoreGenericRepository definition to see if it has internal_status
rg -n "class CoreGenericRepository" --type=py -A30 backend/infrahub/core/protocols.py

Length of output: 908


🏁 Script executed:

#!/bin/bash
# Search for internal_status definition in protocols
rg -n "internal_status" --type=py -B2 -A2 backend/infrahub/core/protocols.py

Length of output: 161


🏁 Script executed:

#!/bin/bash
# Check git history or comments around merge_core_read_only_repositories
git log --oneline -n 20 --all -- backend/infrahub/core/merge.py | head -20

Length of output: 1082


🏁 Script executed:

#!/bin/bash
# Look for TODO or FIXME comments in merge.py related to read-only repos
rg -n "TODO|FIXME" --type=py -B2 -A2 backend/infrahub/core/merge.py

Length of output: 276


Add status and sync checks to merge_core_read_only_repositories to match regular repository merge logic.

Read-only repositories inherit internal_status from CoreGenericRepository (line 119 in protocols.py). The method currently merges all read-only repos found in both branches without checking:

  1. Status check (like line 260 in merge_core_repositories): Skip INACTIVE repositories
  2. Conditional merge (like line 263): Only merge if sync_with_git is enabled OR internal_status is STAGING

Apply the same checks to merge_core_read_only_repositories and pass internal_status to the GitRepositoryMerge model for consistency.

🤖 Prompt for AI Agents
In backend/infrahub/core/merge.py around lines 229 to 247, the
merge_core_read_only_repositories function currently enqueues merges for every
read-only repo present in both branches; update it to mirror
merge_core_repositories by (1) skipping repositories whose internal_status is
INACTIVE, (2) only enqueuing a merge when repo.sync_with_git is True OR
repo.internal_status is STAGING, and (3) include the repository's
internal_status when constructing the GitRepositoryMerge model so the workflow
receives that status; implement these checks before creating the model and
submitting the workflow.


async def merge_core_repositories(self) -> None:
# Collect all Repositories in Main because we'll need the commit in Main for each one.
repos_in_main_list = await NodeManager.query(schema=CoreRepository, db=self.db)
repos_in_main = {repo.id: repo for repo in repos_in_main_list}
Expand Down
4 changes: 2 additions & 2 deletions backend/infrahub/git/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ class GitRepositoryMerge(BaseModel):

repository_id: str = Field(..., description="The unique ID of the Repository")
repository_name: str = Field(..., description="The name of the repository")
internal_status: str = Field(..., description="Administrative status of the repository")
internal_status: str | None = Field(default=None, description="Administrative status of the repository")
source_branch: str = Field(..., description="The source branch")
destination_branch: str = Field(..., description="The destination branch")
destination_branch_id: str = Field(..., description="The ID of the destination branch")
default_branch: str = Field(..., description="The default branch in Git")
default_branch: str | None = Field(default=None, description="The default branch in Git")


class GitRepositoryImportObjects(BaseModel):
Expand Down
24 changes: 24 additions & 0 deletions backend/infrahub/git/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ async def pull_read_only(model: GitRepositoryPullReadOnly) -> None:
flow_run_name="Merge {model.source_branch} > {model.destination_branch} in git repository",
)
async def merge_git_repository(model: GitRepositoryMerge) -> None:
log = get_run_logger()
await add_tags(branches=[model.source_branch, model.destination_branch], nodes=[model.repository_id])

client = get_client()
Expand All @@ -461,6 +462,7 @@ async def merge_git_repository(model: GitRepositoryMerge) -> None:
)

if model.internal_status == RepositoryInternalStatus.STAGING.value:
log.info(f"Merging {InfrahubKind.GENERICREPOSITORY}")
repo_source = await client.get(
kind=InfrahubKind.GENERICREPOSITORY, id=model.repository_id, branch=model.source_branch
)
Expand All @@ -472,6 +474,28 @@ async def merge_git_repository(model: GitRepositoryMerge) -> None:
repo_main.commit.value = commit

await repo_main.save()
log.info(f"Finished merging {InfrahubKind.GENERICREPOSITORY}")

elif not model.default_branch:
repo_source = await client.get(
kind=InfrahubKind.READONLYREPOSITORY, id=model.repository_id, branch=model.source_branch
)
repo_destination = await client.get(
kind=InfrahubKind.READONLYREPOSITORY, id=model.repository_id, branch=model.destination_branch
)

if (
repo_destination.ref.value != repo_source.ref.value
or repo_destination.commit.value != repo_source.commit.value
):
log.info(f"Merging {InfrahubKind.READONLYREPOSITORY}")

repo_destination.ref.value = repo_source.ref.value
repo_destination.commit.value = repo_source.commit.value
await repo_destination.save()

log.info(f"Finished merging {InfrahubKind.READONLYREPOSITORY}")

else:
async with lock.registry.get(name=model.repository_name, namespace="repository"):
await repo.merge(source_branch=model.source_branch, dest_branch=model.destination_branch)
Expand Down
11 changes: 10 additions & 1 deletion backend/tests/unit/core/diff/test_coordinator_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from infrahub.core.merge import BranchMerger
from infrahub.core.node import Node
from infrahub.core.schema import SchemaRoot
from infrahub.core.schema.definitions.core.repository import core_repository
from infrahub.core.schema.definitions.core.repository import core_read_only_repository, core_repository
from infrahub.core.schema.node_schema import NodeSchema
from infrahub.core.timestamp import Timestamp
from infrahub.database import InfrahubDatabase, get_db
Expand Down Expand Up @@ -55,6 +55,15 @@ async def dummy_repository_schema(self, db: InfrahubDatabase, default_branch: Br
default_branch.update_schema_hash()
await default_branch.save(db=db)

dummy_repository = NodeSchema(
name=core_read_only_repository.name,
namespace=core_read_only_repository.namespace,
)
schema = SchemaRoot(nodes=[dummy_repository])
registry.schema.register_schema(schema=schema, branch=default_branch.name)
default_branch.update_schema_hash()
await default_branch.save(db=db)

async def get_diff_coordinator(self, db: InfrahubDatabase, diff_branch: Branch) -> DiffCoordinator:
config.SETTINGS.database.max_depth_search_hierarchy = 10
component_registry = get_component_registry()
Expand Down
1 change: 1 addition & 0 deletions changelog/5978.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Merge core read only repositories on branch merge
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use hyphenation for compound adjective.

The phrase "read only" should be hyphenated as "read-only" when used as a compound adjective.

Apply this diff:

-Merge core read only repositories on branch merge
+Merge core read-only repositories on branch merge
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Merge core read only repositories on branch merge
Merge core read-only repositories on branch merge
🧰 Tools
🪛 LanguageTool

[grammar] ~1-~1: Use a hyphen to join words.
Context: Merge core read only repositories on branch merge

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents
In changelog/5978.added.md around lines 1 to 1 the phrase "read only" is used as
a compound adjective; update it to "read-only" so the line reads "Merge core
read-only repositories on branch merge".