Prevents multiple Git mirroring jobs from running in the same directory

[ikhoon]

Motivation:

A Git mirroring task creates a local directory derived from the remote URL and fetches upstream updates.

centraldogma/server-mirror-git/src/main/java/com/linecorp/centraldogma/server/internal/mirror/AbstractGitMirror.java

Lines 143 to 146 in 4c617bb

	final File repoDir = new File(
	workDir,
	CONSECUTIVE_UNDERSCORES.matcher(DISALLOWED_CHARS.matcher(
	remoteRepoUri().toASCIIString()).replaceAll("_")).replaceAll("_"));

Because the tasks runs asynchronously, multiple fetch operations can end up running in the same directory if the sample remote URL is used across multiple mirror configurations.

centraldogma/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/MirrorSchedulingService.java

Lines 313 to 314 in 4c617bb

	runAsync(new MirrorTask(m, User.SYSTEM, Instant.now(),
	currentZone, true));

In an internal use case, a single Git repository was used to configure 36 mirroring jobs, each targeting a different branch.

As a result, the Git internal structure or state could be corrupted with the following exception:

Caused by: org.eclipse.jgit.api.errors.TransportException: Expected ACK/NAK, found EOF
	at org.eclipse.jgit.api.FetchCommand.call(FetchCommand.java:249)
	at com.linecorp.centraldogma.server.internal.mirror.AbstractGitMirror.fetchRemoteHeadAndGetCommitId(AbstractGitMirror.java:657)
	at com.linecorp.centraldogma.server.internal.mirror.AbstractGitMirror.mirrorRemoteToLocal(AbstractGitMirror.java:361)
	... 15 common frames omitted
Caused by: org.eclipse.jgit.errors.TransportException: Expected ACK/NAK, found EOF
	at org.eclipse.jgit.transport.BasePackFetchConnection.doFetch(BasePackFetchConnection.java:458)
	at org.eclipse.jgit.transport.BasePackFetchConnection.fetch(BasePackFetchConnection.java:351)
	at org.eclipse.jgit.transport.BasePackFetchConnection.fetch(BasePackFetchConnection.java:343)

Modifications:

• Use a unique mirroring directory per Git mirroring job so that each job gets its own Git directory.

Result:

Fixed a race condition that could occur when multiple Git mirroring jobs shared the same directory for a remote URL

[coderabbitai]

📝 Walkthrough

Walkthrough

Updates repository directory naming in AbstractGitMirror to prevent collisions by using localRepo parent name, localRepo name, and id instead of deriving from remote URL. Adds validation in integration tests to verify mirror directories are created with valid HEAD references.

Changes

Cohort / File(s)	Summary
Mirror Implementation server-mirror-git/src/main/java/com/linecorp/centraldogma/server/internal/mirror/AbstractGitMirror.java	Removed DISALLOWED_CHARS and CONSECUTIVE_UNDERSCORES regex patterns. Modified openGit() to generate unique repository directory names using localRepo parent/name and id, replacing URL-derived naming to prevent potential collisions.
Mirror Tests it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorIntegrationTest.java	Added validation to verify that after mirroring an empty remote repository, the internal mirrors directory is created and contains a valid HEAD reference file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

• trustin
• jrhee17

Poem

🐰 A mirror's naming, once derived from URLs afar,
Now finds its unique home with parent, name, and id star—
No more collisions shall occur in the mirrors' sacred place,
A test now guards the HEAD, ensuring all is in grace! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: preventing multiple Git mirroring jobs from using the same directory by implementing unique directory naming per job.
Description check	✅ Passed	The pull request description clearly explains the motivation (race condition from shared directories), modifications (unique directory per job), and expected result (fixing corruption issues).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@server-mirror-git/src/main/java/com/linecorp/centraldogma/server/internal/mirror/AbstractGitMirror.java`:
- Around line 139-142: The directory naming using dirName =
localRepo().parent().name() + '-' + localRepo().name() + '-' + id() in
AbstractGitMirror can collide when hyphens appear in project/repo names; change
the scheme to produce a collision-safe key (e.g., compute a stable digest/hash
of the tuple [localRepo().parent().name(), localRepo().name(), id()] or encode
each component with an unambiguous separator or percent-encoding) and use that
digest/encoded string when constructing repoDir = new File(workDir, ...),
ensuring uniqueness across different (project, repo, mirrorId) combinations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 23f6685f-449b-4554-bde8-4866dd7d1b60

📥 Commits

Reviewing files that changed from the base of the PR and between e392c43 and e47f53e.

📒 Files selected for processing (2)

• it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorIntegrationTest.java
• server-mirror-git/src/main/java/com/linecorp/centraldogma/server/internal/mirror/AbstractGitMirror.java

[jrhee17]

👍 👍

[minwoox]

👍 👍