Prevent replication plugin start with invalid remotes
Allow the Gerrit admin to detect early a configuration
issue with the remotes. Implement the credentials validation
as first step; more validations to follow.
Change-Id: I128422677f9fa9e70aae539ca7a591bddc07008e
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
index 8e21a81..132fa4f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
@@ -79,4 +79,9 @@
private boolean needsReload() {
return config.getConfig().getBoolean("gerrit", "autoReload", false) && secureStore.isOutdated();
}
+
+ @Override
+ public boolean validate(String remoteConfigName) {
+ return secureCredentialsFactory.get().validate(remoteConfigName);
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java
index 3bb64ab..141fb2d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java
@@ -18,4 +18,8 @@
public interface CredentialsFactory {
CredentialsProvider create(String remoteName);
+
+ default boolean validate(String remoteConfigName) {
+ return true;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
index 6054a4a..08ed7cb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -97,6 +97,8 @@
private static final String PROJECT_NOT_AVAILABLE = "source project %s not available";
+ private final CredentialsFactory credentialsFactory;
+
public interface Factory {
Destination create(DestinationConfiguration config);
}
@@ -149,6 +151,7 @@
GroupIncludeCache groupIncludeCache,
DynamicItem<EventDispatcher> eventDispatcher,
Provider<ReplicationTasksStorage> rts,
+ CredentialsFactory credentialsFactory,
@Assisted DestinationConfiguration cfg) {
this.eventDispatcher = eventDispatcher;
gitManager = gitRepositoryManager;
@@ -157,6 +160,7 @@
this.projectCache = projectCache;
this.stateLog = stateLog;
this.replicationTasksStorage = rts;
+ this.credentialsFactory = credentialsFactory;
config = cfg;
CurrentUser remoteUser;
if (!cfg.getAuthGroupNames().isEmpty()) {
@@ -216,6 +220,10 @@
threadScoper = child.getInstance(PerThreadRequestScope.Scoper.class);
}
+ public boolean validate() {
+ return (credentialsFactory == null || credentialsFactory.validate(getRemoteConfigName()));
+ }
+
private void addRecursiveParents(
AccountGroup.UUID g,
ImmutableSet.Builder<AccountGroup.UUID> builder,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
index 8a41945..dc3b05e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
@@ -180,6 +180,12 @@
public synchronized void startup(WorkQueue workQueue) {
shuttingDown = false;
for (Destination cfg : destinations) {
+ if (!cfg.validate()) {
+ throw new IllegalStateException(
+ "Unable to start replication plugin because remote "
+ + cfg.getRemoteConfigName()
+ + " is not valid");
+ }
cfg.start(workQueue);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java
index 210c4d8..957744e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.replication;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.securestore.SecureStore;
import com.google.inject.Inject;
import java.util.Objects;
@@ -22,6 +23,7 @@
/** Looks up a remote's password in SecureStore */
class SecureCredentialsFactory implements CredentialsFactory {
+ private static final FluentLogger log = FluentLogger.forEnclosingClass();
private final SecureStore secureStore;
@Inject
@@ -35,4 +37,17 @@
String pass = Objects.toString(secureStore.get("remote", remoteName, "password"), "");
return new UsernamePasswordCredentialsProvider(user, pass);
}
+
+ @Override
+ public boolean validate(String remoteName) {
+ try {
+ String unused = secureStore.get("remote", remoteName, "username");
+ unused = secureStore.get("remote", remoteName, "password");
+ return true;
+ } catch (Throwable t) {
+ log.atSevere().withCause(t).log(
+ "Credentials for replication remote %s are invalid", remoteName);
+ return false;
+ }
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java
index 9244f87..935fdb4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java
@@ -57,7 +57,8 @@
public final DestinationConfiguration config;
protected FakeDestination(DestinationConfiguration config) {
- super(injectorMock(), null, null, null, null, null, null, null, null, null, null, config);
+ super(
+ injectorMock(), null, null, null, null, null, null, null, null, null, null, null, config);
this.config = config;
}
