Fix mirror loading failure when a mirror has an invalid credential

[minwoox]

Motivation:

• The SSH host key verification change made SshGitMirror's constructor throw a MirrorException whenever the credential is not an SshKeyCredential (e.g. a missing credential that resolves to Credential.NONE).

Modifications:

• Wrap the per-mirror MirrorConverter.convertToMirror call in handleAllMirrors with a try/catch that logs a warning and skips the bad mirror (returns null) instead of failing the whole list.

Result:

• A mirror with an invalid or missing credential no longer prevents the rest of the project's mirrors from being loaded and scheduled; only the bad mirror is skipped with a warning.

[coderabbitai]

📝 Walkthrough

Walkthrough

DefaultMetaRepository.handleAllMirrors gains per-mirror try/catch logic that logs malformed configurations at debug level and skips them via null-filtering instead of propagating exceptions. The ZoneAwareMirrorTest.shouldWarnUnknownZoneForScheduledJob test is fixed to create a valid SSH credential for the bar-unknown-zone repository and corrects the credential name reference in the MirrorConfig constructor.

Changes

Mirror Config Resilience and Test Fix

Layer / File(s)	Summary
Per-mirror conversion error handling server/src/main/java/.../DefaultMetaRepository.java	Adds a static SLF4J logger and wraps each mirror config conversion in a per-entry try/catch; failures are logged at debug level and the entry is skipped by returning null, which the existing nonNull filter removes.
Integration test credential setup it/mirror/src/test/java/.../ZoneAwareMirrorTest.java	shouldWarnUnknownZoneForScheduledJob now sends an authenticated credential creation request for bar-unknown-zone and asserts 201 CREATED before constructing the mirror; the MirrorConfig credential-name argument is corrected to use credentialName(FOO_PROJ, "bar-unknown-zone", PRIVATE_KEY_FILE).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A mirror once cracked at a bad config's sight,
Now logs a debug note and hops out of the fight.
The unknown-zone test needed a key of its own,
So credentials were planted before seeds were sown.
Each mirror now glides past the brambles with grace! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main fix: handling mirror loading failures caused by invalid credentials.
Description check	✅ Passed	The description is clearly related to the changeset, providing motivation, modifications, and expected results that align with the code changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/DefaultMetaRepository.java`:
- Around line 191-193: In the DefaultMetaRepository class, change the
logger.debug call to logger.warn in the exception handler that logs when a
mirror configuration fails to convert to a mirror. This ensures that when a
mirror is intentionally skipped due to a conversion error, operators are
notified at the warn level in production logs without requiring debug logging to
be enabled, making production incidents visible.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 925ea8e7-8ad5-4463-8b94-4921fdec9e22

📥 Commits

Reviewing files that changed from the base of the PR and between c5371ab and 06b1c01.

📒 Files selected for processing (2)

• it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ZoneAwareMirrorTest.java
• server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/DefaultMetaRepository.java

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use warning-level logging when skipping malformed mirrors.

This path intentionally drops a mirror config; logging it at debug can make production incidents invisible unless debug is enabled. Promote this to warn so operators are notified when a mirror is skipped.

Proposed change

-                                        logger.debug("Failed to convert a mirror configuration to a mirror. " +
-                                                     "project: {}, mirror: {}", parent().name(), mirrorConfig,
-                                                     e);
+                                        logger.warn("Skipping malformed mirror configuration. " +
+                                                    "project: {}, mirror: {}", parent().name(), mirrorConfig,
+                                                    e);

📝 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

	logger.debug("Failed to convert a mirror configuration to a mirror. " +
	"project: {}, mirror: {}", parent().name(), mirrorConfig,
	e);
	logger.warn("Skipping malformed mirror configuration. " +
	"project: {}, mirror: {}", parent().name(), mirrorConfig,
	e);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/DefaultMetaRepository.java`
around lines 191 - 193, In the DefaultMetaRepository class, change the
logger.debug call to logger.warn in the exception handler that logs when a
mirror configuration fails to convert to a mirror. This ensures that when a
mirror is intentionally skipped due to a conversion error, operators are
notified at the warn level in production logs without requiring debug logging to
be enabled, making production incidents visible.

[jrhee17]

👍 👍

[ikhoon]

👍👍