Skip to content

Commit b96f6a9

Browse files
authored
Fix mirror loading failure when a mirror has an invalid credential (#1316)
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.
1 parent c5371ab commit b96f6a9

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ZoneAwareMirrorTest.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,25 @@ void shouldWarnUnknownZoneForScheduledJob() throws Exception {
184184
final CentralDogmaRepository repo = client.forRepo(FOO_PROJ, Project.REPO_DOGMA);
185185
final String mirrorId = TEST_MIRROR_ID + "-unknown-zone";
186186
final String unknownZone = "unknown-zone";
187+
188+
// Create a valid SSH credential so that the mirror can be loaded and reach the zone validation.
189+
// Otherwise, the git+ssh mirror fails to be constructed and is skipped before the zone is checked.
190+
final BlockingWebClient webClient = WebClient.builder("http://127.0.0.1:" + serverPort)
191+
.auth(AuthToken.ofOAuth2(accessToken))
192+
.build()
193+
.blocking();
194+
final CreateCredentialRequest credential =
195+
getCreateCredentialRequest(FOO_PROJ, "bar-unknown-zone");
196+
final ResponseEntity<PushResultDto> credentialResponse =
197+
webClient.prepare()
198+
.post("/api/v1/projects/{proj}/repos/{repo}/credentials")
199+
.pathParam("proj", FOO_PROJ)
200+
.pathParam("repo", "bar-unknown-zone")
201+
.contentJson(credential)
202+
.asJson(PushResultDto.class)
203+
.execute();
204+
assertThat(credentialResponse.status()).isEqualTo(HttpStatus.CREATED);
205+
187206
final MirrorConfig mirrorConfig =
188207
new MirrorConfig(mirrorId,
189208
true,
@@ -194,7 +213,7 @@ void shouldWarnUnknownZoneForScheduledJob() throws Exception {
194213
URI.create("git+ssh://github.com/line/centraldogma-authtest.git/#main"),
195214
null,
196215
null,
197-
credentialName("foo", "bar-unknown-zone", "credential-id"),
216+
credentialName(FOO_PROJ, "bar-unknown-zone", PRIVATE_KEY_FILE),
198217
unknownZone);
199218
final Change<JsonNode> change = Change.ofJsonUpsert(
200219
"/repos/bar-unknown-zone/mirrors/" + mirrorId + ".json",

server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/DefaultMetaRepository.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.util.regex.Pattern;
3030

3131
import org.jspecify.annotations.Nullable;
32+
import org.slf4j.Logger;
33+
import org.slf4j.LoggerFactory;
3234

3335
import com.cronutils.model.Cron;
3436
import com.cronutils.model.field.CronField;
@@ -61,6 +63,8 @@
6163

6264
public final class DefaultMetaRepository extends RepositoryWrapper implements MetaRepository {
6365

66+
private static final Logger logger = LoggerFactory.getLogger(DefaultMetaRepository.class);
67+
6468
private static final Pattern MIRROR_PATH_PATTERN = Pattern.compile("/repos/[^/]+/mirrors/[^/]+\\.json");
6569

6670
private static final Pattern REPO_CREDENTIAL_PATH_PATTERN =
@@ -177,8 +181,19 @@ private CompletableFuture<List<Mirror>> handleAllMirrors(Map<String, Entry<?>> e
177181
return future.thenApply(credentials -> {
178182
final List<MirrorConfig> mirrorConfigs = toMirrorConfigs(entries);
179183
return mirrorConfigs.stream()
180-
.map(mirrorConfig -> MirrorConverter.convertToMirror(
181-
mirrorConfig, parent(), credentials, trustedHostKeys))
184+
.map(mirrorConfig -> {
185+
try {
186+
return MirrorConverter.convertToMirror(
187+
mirrorConfig, parent(), credentials, trustedHostKeys);
188+
} catch (Exception e) {
189+
// Skip a malformed mirror so that a single bad mirror does not
190+
// prevent the rest of the mirrors from being loaded.
191+
logger.debug("Failed to convert a mirror configuration to a mirror. " +
192+
"project: {}, mirror: {}", parent().name(), mirrorConfig,
193+
e);
194+
return null;
195+
}
196+
})
182197
.filter(Objects::nonNull)
183198
.collect(toImmutableList());
184199
});

0 commit comments

Comments
 (0)