Merge branch 'stable-3.10'
* stable-3.10:
PushOne: Remove unused addRef() method
TasksStorage: Remove synchronized from methods
Place the replaying flag clearing in a finally
Demote delete errors when distributor is enabled
distributor: Reduce log level for no-op consolidations
Change-Id: I3ce1519c44ec6cbae7725527ea626e55401cc488
Release-Notes: skip
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
index 2c049fc..5e2c758 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
@@ -128,4 +128,9 @@
public Config getConfig() {
return currentConfig.getConfig();
}
+
+ @Override
+ public boolean useLegacyCredentials() {
+ return currentConfig.useLegacyCredentials();
+ }
}
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 e9409a6..32c439b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
@@ -14,49 +14,60 @@
package com.googlesource.gerrit.plugins.replication;
-import static com.google.gerrit.common.FileUtil.lastModified;
-
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.common.UsedAt.Project;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig;
import java.io.IOException;
-import java.nio.file.Files;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.transport.CredentialsProvider;
+@UsedAt(Project.PLUGIN_PULL_REPLICATION)
public class AutoReloadSecureCredentialsFactoryDecorator implements CredentialsFactory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final AtomicReference<SecureCredentialsFactory> secureCredentialsFactory;
- private volatile long secureCredentialsFactoryLoadTs;
+ private final AtomicReference<CredentialsFactory> secureCredentialsFactory;
+ private final SecureStore secureStore;
private final SitePaths site;
- private ReplicationConfig config;
+ private final ReplicationConfig config;
@Inject
- public AutoReloadSecureCredentialsFactoryDecorator(SitePaths site, ReplicationConfig config)
+ public AutoReloadSecureCredentialsFactoryDecorator(
+ SitePaths site, SecureStore secureStore, ReplicationConfig config)
throws ConfigInvalidException, IOException {
this.site = site;
+ this.secureStore = secureStore;
this.config = config;
- this.secureCredentialsFactory = new AtomicReference<>(new SecureCredentialsFactory(site));
- this.secureCredentialsFactoryLoadTs = getSecureConfigLastEditTs();
+ this.secureCredentialsFactory =
+ new AtomicReference<>(newSecureCredentialsFactory(site, secureStore, config));
+ if (config.useLegacyCredentials()) {
+ logger.atWarning().log(
+ "Using legacy credentials in clear text in secure.config. Please encrypt your credentials using "
+ + "'java -jar gerrit.war passwd' for each remote, remove the gerrit.useLegacyCredentials in replication.config "
+ + "and then reload the replication plugin.");
+ }
}
- private long getSecureConfigLastEditTs() {
- if (!Files.exists(site.secure_config)) {
- return 0L;
+ private static CredentialsFactory newSecureCredentialsFactory(
+ SitePaths site, SecureStore secureStore, ReplicationConfig config)
+ throws ConfigInvalidException, IOException {
+ if (config.useLegacyCredentials()) {
+ return new LegacyCredentialsFactory(site);
}
- return lastModified(site.secure_config);
+ return new SecureCredentialsFactory(secureStore);
}
@Override
public CredentialsProvider create(String remoteName) {
try {
if (needsReload()) {
+ secureStore.reload();
secureCredentialsFactory.compareAndSet(
- secureCredentialsFactory.get(), new SecureCredentialsFactory(site));
- secureCredentialsFactoryLoadTs = getSecureConfigLastEditTs();
+ secureCredentialsFactory.get(), newSecureCredentialsFactory(site, secureStore, config));
logger.atInfo().log("secure.config reloaded as it was updated on the file system");
}
} catch (Exception e) {
@@ -69,7 +80,11 @@
}
private boolean needsReload() {
- return config.getConfig().getBoolean("gerrit", "autoReload", false)
- && getSecureConfigLastEditTs() != secureCredentialsFactoryLoadTs;
+ 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 c186493..3bb1d2d 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/FanoutConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
index 4220ddb..c634cce 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
@@ -17,17 +17,20 @@
import static com.google.common.io.Files.getNameWithoutExtension;
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
+import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Optional;
+import java.util.List;
import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.file.FileSnapshot;
@@ -40,6 +43,7 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final Path remoteConfigsDirPath;
+ private final Config fanoutConfig;
@Inject
FanoutConfigResource(SitePaths site) throws IOException, ConfigInvalidException {
@@ -48,16 +52,59 @@
removeRemotes(config);
try (Stream<Path> files = Files.list(remoteConfigsDirPath)) {
- files
- .filter(Files::isRegularFile)
- .filter(FanoutConfigResource::isConfig)
- .map(FanoutConfigResource::loadConfig)
- .filter(Optional::isPresent)
- .map(Optional::get)
- .filter(FanoutConfigResource::isValid)
- .forEach(cfg -> addRemoteConfig(cfg, config));
- } catch (IllegalStateException e) {
- throw new ConfigInvalidException(e.getMessage());
+ Stream<String> mergedConfigs =
+ files
+ .filter(Files::isRegularFile)
+ .filter(FanoutConfigResource::isConfig)
+ .flatMap(path -> getConfigLines(path, line -> addRemoteSubsection(line, path)));
+
+ fanoutConfig = new Config(config);
+ fanoutConfig.fromText(mergedConfigs.collect(Collectors.joining("\n")));
+ }
+ }
+
+ private static String addRemoteSubsection(String line, Path path) {
+ return line.contains("[remote]")
+ ? line.replace(
+ "[remote]", "[remote \"" + getNameWithoutExtension(path.toFile().getName()) + "\"]")
+ : line;
+ }
+
+ private static Stream<String> getConfigLines(
+ Path path, Function<String, String> configLineMapping) {
+ try {
+ List<String> configLines = Files.readAllLines(path);
+ if (!isValid(path, configLines)) {
+ return Stream.empty();
+ }
+ return configLines.stream().map(configLineMapping);
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log("Unable to access replication config %s", path);
+ return Stream.empty();
+ }
+ }
+
+ @Override
+ public void update(Config updates) throws IOException {
+ super.update(filterOutRemotes(updates));
+
+ Set<String> remotes = updates.getSubsections("remote");
+ for (String remote : remotes) {
+ File remoteFile = remoteConfigsDirPath.resolve(remote + ".config").toFile();
+ FileBasedConfig remoteConfig = new FileBasedConfig(remoteFile, FS.DETECTED);
+ try {
+ remoteConfig.load();
+ } catch (ConfigInvalidException e) {
+ throw new IOException(
+ String.format("Cannot parse configuration file: %s", remoteFile.getAbsoluteFile()), e);
+ }
+ Set<String> options = updates.getNames("remote", remote);
+ for (String option : options) {
+ List<String> values = List.of(updates.getStringList("remote", remote, option));
+ remoteConfig.setStringList("remote", null, option, values);
+ config.setStringList("remote", remote, option, values);
+ }
+ remoteConfig.save();
}
}
@@ -74,42 +121,67 @@
}
}
- private static void addRemoteConfig(FileBasedConfig source, Config destination) {
- String remoteName = getNameWithoutExtension(source.getFile().getName());
- for (String name : source.getNames("remote")) {
- destination.setStringList(
- "remote",
- remoteName,
- name,
- Lists.newArrayList(source.getStringList("remote", null, name)));
- }
+ private static Config filterOutRemotes(Config config) {
+ Config filteredConfig = new Config();
+ Set<String> sections = config.getSections();
+ sections.stream()
+ .filter(Predicate.not("remote"::equalsIgnoreCase))
+ .forEach(
+ section -> {
+ config
+ .getNames(section)
+ .forEach(
+ sectionName ->
+ filteredConfig.setStringList(
+ section,
+ null,
+ sectionName,
+ List.of(config.getStringList(section, null, sectionName))));
+ config
+ .getSubsections(section)
+ .forEach(
+ subsection ->
+ config
+ .getNames(section, subsection)
+ .forEach(
+ name ->
+ filteredConfig.setStringList(
+ section,
+ subsection,
+ name,
+ List.of(
+ config.getStringList(section, subsection, name)))));
+ });
+ return filteredConfig;
}
- private static boolean isValid(Config cfg) {
- if (cfg.getSections().size() != 1 || !cfg.getSections().contains("remote")) {
- logger.atSevere().log(
- "Remote replication configuration file %s must contain only one remote section.", cfg);
- return false;
- }
- if (cfg.getSubsections("remote").size() > 0) {
- logger.atSevere().log(
- "Remote replication configuration file %s cannot contain remote subsections.", cfg);
- return false;
+ private static boolean isValid(Path path, List<String> remoteConfigLines) {
+ int numRemoteSections = 0;
+ int numRemoteSectionsWithName = 0;
+ boolean valid = true;
+ for (String configLine : remoteConfigLines) {
+ if (configLine.contains("[remote]")) {
+ numRemoteSections++;
+ }
+
+ if (configLine.contains("[remote \"")) {
+ numRemoteSectionsWithName++;
+ }
}
- return true;
- }
-
- private static Optional<FileBasedConfig> loadConfig(Path path) {
- FileBasedConfig cfg = new FileBasedConfig(path.toFile(), FS.DETECTED);
- try {
- cfg.load();
- } catch (IOException | ConfigInvalidException e) {
- logger.atSevere().withCause(e).log(
- "Cannot load remote replication configuration file %s.", path);
- return Optional.empty();
+ if (numRemoteSectionsWithName > 0) {
+ logger.atSevere().log(
+ "Remote replication configuration file %s cannot contain remote subsections.", path);
+ valid = false;
}
- return Optional.of(cfg);
+
+ if (numRemoteSections != 1) {
+ logger.atSevere().log(
+ "Remote replication configuration file %s must contain only one remote section.", path);
+ valid = false;
+ }
+
+ return valid;
}
private static boolean isConfig(Path p) {
@@ -140,4 +212,9 @@
return parentVersion;
}
}
+
+ @Override
+ public Config getConfig() {
+ return fanoutConfig;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
index baccb83..7a14cd1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
@@ -17,6 +17,7 @@
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.UsedAt.Project;
import com.google.gerrit.server.config.SitePaths;
@@ -24,6 +25,7 @@
import com.googlesource.gerrit.plugins.replication.api.ConfigResource;
import java.io.IOException;
import java.nio.file.Path;
+import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -54,6 +56,25 @@
}
@Override
+ public void update(Config updates) throws IOException {
+ for (String section : updates.getSections()) {
+ for (String subsection : updates.getSubsections(section)) {
+ for (String name : updates.getNames(section, subsection, true)) {
+ List<String> values =
+ Lists.newArrayList(updates.getStringList(section, subsection, name));
+ config.setStringList(section, subsection, name, values);
+ }
+ }
+
+ for (String name : updates.getNames(section, true)) {
+ List<String> values = Lists.newArrayList(updates.getStringList(section, null, name));
+ config.setStringList(section, null, name, values);
+ }
+ }
+ config.save();
+ }
+
+ @Override
public String getVersion() {
return Long.toString(config.getFile().lastModified());
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/LegacyCredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/LegacyCredentialsFactory.java
new file mode 100644
index 0000000..a89c936
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/LegacyCredentialsFactory.java
@@ -0,0 +1,66 @@
+// Copyright (C) 2011 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication;
+
+import com.google.gerrit.server.config.SitePaths;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.Objects;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.eclipse.jgit.util.FS;
+
+/**
+ * Looks up a remote's password in secure.config.
+ *
+ * @deprecated This class is not secure and should no longer be used; it was allowing to record and
+ * read credentials in clear text in secure.config even though the Gerrit administrator
+ * installed a proper SecureStore implementation such as the secure-config.jar libModule.
+ */
+@Deprecated(forRemoval = true)
+class LegacyCredentialsFactory implements CredentialsFactory {
+ private final Config config;
+
+ @Inject
+ public LegacyCredentialsFactory(SitePaths site) throws ConfigInvalidException, IOException {
+ config = load(site);
+ }
+
+ private static Config load(SitePaths site) throws ConfigInvalidException, IOException {
+ FileBasedConfig cfg = new FileBasedConfig(site.secure_config.toFile(), FS.DETECTED);
+ if (cfg.getFile().exists() && cfg.getFile().length() > 0) {
+ try {
+ cfg.load();
+ } catch (ConfigInvalidException e) {
+ throw new ConfigInvalidException(
+ String.format("Config file %s is invalid: %s", cfg.getFile(), e.getMessage()), e);
+ } catch (IOException e) {
+ throw new IOException(
+ String.format("Cannot read %s: %s", cfg.getFile(), e.getMessage()), e);
+ }
+ }
+ return cfg;
+ }
+
+ @Override
+ public CredentialsProvider create(String remoteName) {
+ String user = Objects.toString(config.getString("remote", remoteName, "username"), "");
+ String pass = Objects.toString(config.getString("remote", remoteName, "password"), "");
+ return new UsernamePasswordCredentialsProvider(user, pass);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
index 43d90a5..58a10f0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
@@ -26,6 +26,7 @@
import com.google.inject.util.Providers;
import com.googlesource.gerrit.plugins.replication.api.ConfigResource;
import com.googlesource.gerrit.plugins.replication.api.ReplicationConfigOverrides;
+import java.io.IOException;
import java.util.function.Supplier;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
@@ -77,7 +78,15 @@
return baseVersion + overrides.get().getVersion();
}
- private boolean noOverrides() {
+ boolean noOverrides() {
return overrides == null || overrides.get() == null;
}
+
+ void update(Config remotesConfig) throws IOException {
+ if (noOverrides()) {
+ base.get().update(remotesConfig);
+ } else {
+ overrides.get().update(remotesConfig);
+ }
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
index 2b60145..a648845 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -79,7 +79,6 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.FetchConnection;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RefSpec;
@@ -119,7 +118,7 @@
private final Destination pool;
private final RemoteConfig config;
private final ReplicationConfig replConfig;
- private final CredentialsProvider credentialsProvider;
+ private final CredentialsFactory credentialsFactory;
private final PerThreadRequestScope.Scoper threadScoper;
private final Project.NameKey projectName;
@@ -166,7 +165,7 @@
pool = p;
config = c;
replConfig = rc;
- credentialsProvider = cpFactory.create(c.getName());
+ credentialsFactory = cpFactory;
threadScoper = ts;
projectName = d;
uri = u;
@@ -563,7 +562,7 @@
private PushResult pushVia(Transport tn) throws IOException, PermissionBackendException {
tn.applyConfig(config);
- tn.setCredentialsProvider(credentialsProvider);
+ tn.setCredentialsProvider(credentialsFactory.create(config.getName()));
List<RemoteRefUpdate> todo = generateUpdates(tn);
if (todo.isEmpty()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
index 1223db0..8f9b805 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
@@ -31,6 +31,7 @@
private final SitePaths site;
private final MergedConfigResource configResource;
+ private final boolean useLegacyCredentials;
private boolean replicateAllOnPluginStart;
private boolean defaultForceUpdate;
private int maxRefsToLog;
@@ -62,6 +63,7 @@
DEFAULT_SSH_CONNECTION_TIMEOUT_MS,
MILLISECONDS);
this.pluginDataDir = pluginDataDir;
+ this.useLegacyCredentials = config.getBoolean("gerrit", "useLegacyCredentials", false);
}
@Nullable
@@ -125,6 +127,11 @@
}
@Override
+ public boolean useLegacyCredentials() {
+ return useLegacyCredentials;
+ }
+
+ @Override
public String getVersion() {
return configResource.getVersion();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
index ac27280..e3e4021 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.events.HeadUpdatedListener;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.events.ProjectDeletedListener;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.events.EventTypes;
import com.google.inject.AbstractModule;
@@ -30,6 +31,7 @@
import com.google.inject.Scopes;
import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.google.inject.internal.UniqueAnnotations;
+import com.googlesource.gerrit.plugins.replication.api.ReplicationRemotesApi;
import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationDoneEvent;
import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationFailedEvent;
import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationScheduledEvent;
@@ -52,9 +54,7 @@
@Override
protected void configure() {
- install(new FactoryModuleBuilder().build(Destination.Factory.class));
install(configModule);
- bind(ReplicationQueue.class).in(Scopes.SINGLETON);
bind(ObservableQueue.class).to(ReplicationQueue.class);
bind(LifecycleListener.class)
.annotatedWith(UniqueAnnotations.create())
@@ -63,6 +63,7 @@
DynamicSet.bind(binder(), GitBatchRefUpdateListener.class).to(ReplicationQueue.class);
DynamicSet.bind(binder(), ProjectDeletedListener.class).to(ReplicationQueue.class);
DynamicSet.bind(binder(), HeadUpdatedListener.class).to(ReplicationQueue.class);
+ DynamicItem.bind(binder(), ReplicationRemotesApi.class).to(ReplicationRemotesApiImpl.class);
bind(OnStartStop.class).in(Scopes.SINGLETON);
bind(LifecycleListener.class).annotatedWith(UniqueAnnotations.create()).to(OnStartStop.class);
@@ -77,10 +78,8 @@
.to(StartReplicationCapability.class);
install(new FactoryModuleBuilder().build(PushAll.Factory.class));
- install(new FactoryModuleBuilder().build(ProjectDeletionState.Factory.class));
bind(EventBus.class).in(Scopes.SINGLETON);
- bind(ReplicationDestinations.class).to(DestinationsCollection.class);
bind(ConfigParser.class).to(DestinationConfigParser.class).in(Scopes.SINGLETON);
DynamicSet.setOf(binder(), ReplicationStateListener.class);
@@ -103,5 +102,11 @@
bind(TransportFactory.class).to(TransportFactoryImpl.class).in(Scopes.SINGLETON);
bind(CloseableHttpClient.class).toProvider(HttpClientProvider.class).in(Scopes.SINGLETON);
+
+ bind(ReplicationQueue.class).in(Scopes.SINGLETON);
+ bind(ReplicationDestinations.class).to(DestinationsCollection.class);
+
+ install(new FactoryModuleBuilder().build(Destination.Factory.class));
+ install(new FactoryModuleBuilder().build(ProjectDeletionState.Factory.class));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
new file mode 100644
index 0000000..cc6bcad
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
@@ -0,0 +1,100 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication;
+
+import com.google.gerrit.server.securestore.SecureStore;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.replication.api.ReplicationRemotesApi;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import org.eclipse.jgit.lib.Config;
+
+@Singleton
+class ReplicationRemotesApiImpl implements ReplicationRemotesApi {
+
+ private final SecureStore secureStore;
+ private final MergedConfigResource mergedConfigResource;
+
+ @Inject
+ ReplicationRemotesApiImpl(SecureStore secureStore, MergedConfigResource mergedConfigResource) {
+ this.secureStore = secureStore;
+ this.mergedConfigResource = mergedConfigResource;
+ }
+
+ @Override
+ public Config get(String... remoteNames) {
+ Config replicationConfig = mergedConfigResource.getConfig();
+ Config remoteConfig = new Config();
+ for (String remoteName : remoteNames) {
+ Set<String> configNames = replicationConfig.getNames("remote", remoteName);
+ for (String configName : configNames) {
+ String[] values = replicationConfig.getStringList("remote", remoteName, configName);
+ if (values.length > 0) {
+ remoteConfig.setStringList("remote", remoteName, configName, Arrays.asList(values));
+ }
+ }
+ }
+ return remoteConfig;
+ }
+
+ @Override
+ public void update(Config remoteConfig) throws IOException {
+ if (remoteConfig.getSubsections("remote").isEmpty()) {
+ throw new IllegalArgumentException(
+ "configuration update must have at least one 'remote' section");
+ }
+
+ SeparatedRemoteConfigs configs = onlyRemoteSectionsWithSeparatedCredentials(remoteConfig);
+ persistRemotesCredentials(configs);
+
+ mergedConfigResource.update(configs.remotes);
+ }
+
+ private SeparatedRemoteConfigs onlyRemoteSectionsWithSeparatedCredentials(Config configUpdates) {
+ SeparatedRemoteConfigs configs = new SeparatedRemoteConfigs();
+ for (String subSection : configUpdates.getSubsections("remote")) {
+ for (String name : configUpdates.getNames("remote", subSection)) {
+ List<String> values = List.of(configUpdates.getStringList("remote", subSection, name));
+ if ("password".equals(name) || "username".equals(name)) {
+ configs.credentials.setStringList("remote", subSection, name, values);
+ } else {
+ configs.remotes.setStringList("remote", subSection, name, values);
+ }
+ }
+ }
+
+ return configs;
+ }
+
+ private void persistRemotesCredentials(SeparatedRemoteConfigs configs) {
+ for (String subSection : configs.credentials.getSubsections("remote")) {
+ copyRemoteStringList(configs.credentials, subSection, "username");
+ copyRemoteStringList(configs.credentials, subSection, "password");
+ }
+ }
+
+ private void copyRemoteStringList(Config config, String subSection, String key) {
+ secureStore.setList(
+ "remote", subSection, key, List.of(config.getStringList("remote", subSection, key)));
+ }
+
+ private static class SeparatedRemoteConfigs {
+ private final Config remotes = new Config();
+ private final Config credentials = new Config();
+ }
+}
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 ed15b92..3e00089 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2011 The Android Open Source Project
+// Copyright (C) 2024 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -14,46 +14,43 @@
package com.googlesource.gerrit.plugins.replication;
-import com.google.gerrit.server.config.SitePaths;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.common.UsedAt.Project;
+import com.google.gerrit.server.securestore.SecureStore;
import com.google.inject.Inject;
-import java.io.IOException;
import java.util.Objects;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
-import org.eclipse.jgit.util.FS;
-/** Looks up a remote's password in secure.config. */
+/** Looks up a remote's password in SecureStore */
+@UsedAt(Project.PLUGIN_PULL_REPLICATION)
public class SecureCredentialsFactory implements CredentialsFactory {
- private final Config config;
+ private static final FluentLogger log = FluentLogger.forEnclosingClass();
+ private final SecureStore secureStore;
@Inject
- public SecureCredentialsFactory(SitePaths site) throws ConfigInvalidException, IOException {
- config = load(site);
- }
-
- private static Config load(SitePaths site) throws ConfigInvalidException, IOException {
- FileBasedConfig cfg = new FileBasedConfig(site.secure_config.toFile(), FS.DETECTED);
- if (cfg.getFile().exists() && cfg.getFile().length() > 0) {
- try {
- cfg.load();
- } catch (ConfigInvalidException e) {
- throw new ConfigInvalidException(
- String.format("Config file %s is invalid: %s", cfg.getFile(), e.getMessage()), e);
- } catch (IOException e) {
- throw new IOException(
- String.format("Cannot read %s: %s", cfg.getFile(), e.getMessage()), e);
- }
- }
- return cfg;
+ public SecureCredentialsFactory(SecureStore secureStore) {
+ this.secureStore = secureStore;
}
@Override
public CredentialsProvider create(String remoteName) {
- String user = Objects.toString(config.getString("remote", remoteName, "username"), "");
- String pass = Objects.toString(config.getString("remote", remoteName, "password"), "");
+ String user = Objects.toString(secureStore.get("remote", remoteName, "username"), "");
+ 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/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java
index 3764ede..44ec88c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java
@@ -22,5 +22,6 @@
protected void configure() {
DynamicItem.itemOf(binder(), ReplicationPushFilter.class);
DynamicItem.itemOf(binder(), ReplicationConfigOverrides.class);
+ DynamicItem.itemOf(binder(), ReplicationRemotesApi.class);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java
index 43733bc..0052fc2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.replication.api;
+import java.io.IOException;
import org.eclipse.jgit.lib.Config;
/**
@@ -37,6 +38,16 @@
Config getConfig();
/**
+ * Update the configuration resource.
+ *
+ * <p>Allows to persist changes to the configuration resource.
+ *
+ * @param config updated configuration
+ * @throws IOException when configuration cannot be persisted
+ */
+ void update(Config config) throws IOException;
+
+ /**
* Current logical version string of the current configuration on the persistent storage.
*
* @return latest logical version number on the persistent storage
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java
index 8ef4e2b..d5ccb02 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java
@@ -102,4 +102,16 @@
* @return the config.
*/
Config getConfig();
+
+ /**
+ * Use legacy credentials for migrating existing sites.
+ *
+ * <p>Existing installations may have used a mix of encrypted and clear text credentials in
+ * secure.config, leveraging the replication plugin bug that was not accessing it using the
+ * correct API. The legacy feature flag 'gerrit.useLegacyCredentials' allows Gerrit to still use
+ * direct access to secure.config without decrypting its values.
+ *
+ * @return true if the secure.config should be read always directly and without decryption
+ */
+ boolean useLegacyCredentials();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java
new file mode 100644
index 0000000..76d6d35
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java
@@ -0,0 +1,55 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication.api;
+
+import static com.google.gerrit.common.UsedAt.Project.PLUGIN_GITHUB;
+
+import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.securestore.SecureStore;
+import java.io.IOException;
+import org.eclipse.jgit.lib.Config;
+
+/** Public API to update replication plugin remotes configurations programmatically. */
+@UsedAt(PLUGIN_GITHUB)
+@DynamicItem.Final(implementedByPlugin = "replication")
+public interface ReplicationRemotesApi {
+
+ /**
+ * Retrieves the configuration for a remote by name.
+ *
+ * <p>Builds a JGit {@link Config} with the remote's configuration obtained by parsing the
+ * replication configuration sources.
+ *
+ * <p>NOTE: The remotes secrets are excluded for security reasons.
+ *
+ * @param remoteNames the remote names to retrieve.
+ * @return {@link Config} associated with the remoteName(s)
+ */
+ Config get(String... remoteNames);
+
+ /**
+ * Adds or updates the remote configuration for the replication plugin.
+ *
+ * <p>Provided JGit {@link Config} object should contain at least one named <em>remote</em>
+ * section. All other configurations will be ignored.
+ *
+ * <p>NOTE: The {@code remote.$name.password} will be stored using {@link SecureStore}.
+ *
+ * @param remoteConfig remotes to add or update
+ * @throws IOException when persisting fails
+ */
+ void update(Config remoteConfig) throws IOException;
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 6f9a49d..bb4be9a 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -170,6 +170,14 @@
: Timeout for SSH connections. If 0, there is no timeout and
the client waits indefinitely. By default, 2 minutes.
+gerrit.useLegacyCredentials
+: Use legacy credentials on secure.config which did not allow encryption
+ through a SecureStore provider. Legacy credentials should not be used in
+ production and are provided purely for backward compatibility with existing
+ secure.config files. When enabling the SecureStore in Gerrit, for instance,
+ the secure-config.jar libModule, make sure that all existing replication
+ credentials are encrypted. By default, false.
+
replication.distributionInterval
: Interval in seconds for running the replication distributor. When
run, the replication distributor will add all persisted waiting tasks
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 b8bbc08..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;
}
@@ -78,7 +79,7 @@
}
@Before
- public void setup() {
+ public void setup() throws IOException {
when(destinationFactoryMock.create(any(DestinationConfiguration.class)))
.thenAnswer(
new Answer<Destination>() {
@@ -103,6 +104,10 @@
return newReplicationConfig("replication.config");
}
+ protected FileBasedConfig newSecureConfig() {
+ return newReplicationConfig("secure.config");
+ }
+
protected FileBasedConfig newReplicationConfig(String path) {
FileBasedConfig replicationConfig =
new FileBasedConfig(sitePaths.etc_dir.resolve(path).toFile(), FS.DETECTED);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecoratorTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecoratorTest.java
new file mode 100644
index 0000000..c959cc9
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecoratorTest.java
@@ -0,0 +1,97 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.server.securestore.SecureStore;
+import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.transport.CredentialItem;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.URIish;
+import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class AutoReloadSecureCredentialsFactoryDecoratorTest extends AbstractConfigTest {
+ private static final String REMOTE_TEST = "remote-test";
+ private static final String USERNAME_CLEARTEXT = "username-clear";
+ private static final String PASSWORD_CLEARTEXT = "password-clear";
+ private static final String USERNAME_CIPHERTEXT = "username-encrypted";
+ private static final String PASSWORD_CIPHERTEXT = "password-encrypted";
+
+ @Mock private SecureStore secureStoreMock;
+ @Mock private ReplicationConfig replicationConfigMock;
+
+ public AutoReloadSecureCredentialsFactoryDecoratorTest() throws IOException {
+ super();
+ }
+
+ @Override
+ @Before
+ public void setup() throws IOException {
+ when(replicationConfigMock.getConfig()).thenReturn(new Config());
+
+ when(secureStoreMock.get("remote", REMOTE_TEST, "username")).thenReturn(USERNAME_CIPHERTEXT);
+ when(secureStoreMock.get("remote", REMOTE_TEST, "password")).thenReturn(PASSWORD_CIPHERTEXT);
+
+ FileBasedConfig secureConfig = newSecureConfig();
+ secureConfig.setString("remote", REMOTE_TEST, "username", USERNAME_CLEARTEXT);
+ secureConfig.setString("remote", REMOTE_TEST, "password", PASSWORD_CLEARTEXT);
+ secureConfig.save();
+ }
+
+ @Test
+ public void shouldCreateLegacyCredentials() throws Exception {
+ when(replicationConfigMock.useLegacyCredentials()).thenReturn(true);
+ assertUsernamePasswordCredentials(
+ getCredentialsProvider(), USERNAME_CLEARTEXT, PASSWORD_CLEARTEXT);
+ }
+
+ @Test
+ public void shouldCreateEncryptedCredentialsByDefault() throws Exception {
+ assertUsernamePasswordCredentials(
+ getCredentialsProvider(), USERNAME_CIPHERTEXT, PASSWORD_CIPHERTEXT);
+ }
+
+ private UsernamePasswordCredentialsProvider getCredentialsProvider()
+ throws ConfigInvalidException, IOException {
+ AutoReloadSecureCredentialsFactoryDecorator credFactory =
+ new AutoReloadSecureCredentialsFactoryDecorator(
+ sitePaths, secureStoreMock, replicationConfigMock);
+ CredentialsProvider legacyCredentials = credFactory.create(REMOTE_TEST);
+ assertThat(legacyCredentials).isInstanceOf(UsernamePasswordCredentialsProvider.class);
+ return (UsernamePasswordCredentialsProvider) legacyCredentials;
+ }
+
+ private static void assertUsernamePasswordCredentials(
+ UsernamePasswordCredentialsProvider credentials, String username, String password) {
+ CredentialItem.Username usernameItem = new CredentialItem.Username();
+ CredentialItem.Password passwordItem = new CredentialItem.Password();
+ assertThat(credentials.get(new URIish(), usernameItem, passwordItem)).isTrue();
+
+ assertThat(usernameItem.getValue()).isEqualTo(username);
+ assertThat(new String(passwordItem.getValue())).isEqualTo(password);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
index 9147ea1..a3d811f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
@@ -24,6 +24,7 @@
import java.io.IOException;
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.junit.Before;
@@ -285,9 +286,73 @@
assertThat(objectUnderTest.getVersion()).isEqualTo(replicationConfigVersion);
}
+ @Test
+ public void shouldAddConfigOptionToMainConfig() throws Exception {
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("new", null, "value", "set");
+
+ objectUnderTest.update(update);
+ Config updatedConfig = objectUnderTest.getConfig();
+
+ assertThat(updatedConfig.getString("new", null, "value")).isEqualTo("set");
+ }
+
+ @Test
+ public void shouldUpdateConfigOptionInMainConfig() throws Exception {
+ FileBasedConfig config = newReplicationConfig();
+ config.setString("updatable", null, "value", "orig");
+ config.save();
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("updatable", null, "value", "updated");
+
+ objectUnderTest.update(update);
+ Config updatedConfig = objectUnderTest.getConfig();
+
+ assertThat(updatedConfig.getString("updatable", null, "value")).isEqualTo("updated");
+ }
+
+ @Test
+ public void shouldAddNewRemoteFile() throws Exception {
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("remote", remoteName1, "url", remoteUrl1);
+
+ objectUnderTest.update(update);
+
+ assertThat(objectUnderTest.getConfig().getString("remote", remoteName1, "url"))
+ .isEqualTo(remoteUrl1);
+ Config actual = loadRemoteConfig(remoteName1);
+ assertThat(actual.getString("remote", null, "url")).isEqualTo(remoteUrl1);
+ }
+
+ @Test
+ public void shouldUpdateExistingRemote() throws Exception {
+ FileBasedConfig rawRemoteConfig = newRemoteConfig(remoteName1);
+ rawRemoteConfig.setString("remote", remoteName1, "url", remoteUrl1);
+ rawRemoteConfig.save();
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("remote", remoteName1, "url", remoteUrl2);
+
+ objectUnderTest.update(update);
+
+ assertThat(objectUnderTest.getConfig().getString("remote", remoteName1, "url"))
+ .isEqualTo(remoteUrl2);
+ Config actual = loadRemoteConfig(remoteName1);
+ assertThat(actual.getString("remote", null, "url")).isEqualTo(remoteUrl2);
+ }
+
protected FileBasedConfig newRemoteConfig(String configFileName) {
return new FileBasedConfig(
sitePaths.etc_dir.resolve("replication/" + configFileName + ".config").toFile(),
FS.DETECTED);
}
+
+ private Config loadRemoteConfig(String siteName) throws Exception {
+ FileBasedConfig config = newRemoteConfig(siteName);
+ config.load();
+ return config;
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java
new file mode 100644
index 0000000..92bf58d
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java
@@ -0,0 +1,121 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication;
+
+import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.io.MoreFiles;
+import com.google.gerrit.server.config.SitePaths;
+import com.googlesource.gerrit.plugins.replication.api.ConfigResource;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FileConfigResourceTest {
+ private static final String VALUE_KEY = "value";
+ private static final String INITIAL_KEY = "initial";
+ private static final String UPDATABLE_KEY = "updatable";
+
+ private Path testDir;
+ private SitePaths sitePaths;
+ private ConfigResource configResource;
+
+ @Before
+ public void setUp() throws Exception {
+ testDir = Files.createTempDirectory("fileConfigResourceTest");
+ sitePaths = new SitePaths(testDir);
+ configResource = newFileConfigResource();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ MoreFiles.deleteRecursively(testDir, ALLOW_INSECURE);
+ }
+
+ @Test
+ public void updateEmptyFile() throws Exception {
+ Config configUpdate = newConfigUpdate();
+
+ Config beforeUpdate = configResource.getConfig();
+ assertThat(beforeUpdate.getSections()).isEmpty();
+
+ configResource.update(configUpdate);
+ Config updatedConfig = newFileConfigResource().getConfig();
+
+ assertConfigUpdate(updatedConfig);
+ }
+
+ @Test
+ public void appendOptionToConfig() throws Exception {
+ FileBasedConfig rawConfig = getRawReplicationConfig();
+ rawConfig.setInt(INITIAL_KEY, null, VALUE_KEY, 10);
+ rawConfig.save();
+ configResource = newFileConfigResource();
+ Config configUpdate = newConfigUpdate();
+
+ configResource.update(configUpdate);
+ Config updatedConfig = configResource.getConfig();
+
+ assertConfigUpdate(updatedConfig);
+ assertThat(updatedConfig.getInt(INITIAL_KEY, null, VALUE_KEY, -1)).isEqualTo(10);
+ }
+
+ @Test
+ public void updateExistingOption() throws Exception {
+ int expectedValue = 20;
+ Config configUpdate = new Config();
+ configUpdate.setInt(UPDATABLE_KEY, null, VALUE_KEY, expectedValue);
+ FileBasedConfig rawConfig = getRawReplicationConfig(newConfigUpdate());
+ rawConfig.save();
+
+ configResource.update(configUpdate);
+ Config updatedConfig = configResource.getConfig();
+
+ assertConfigUpdate(updatedConfig, expectedValue);
+ }
+
+ private FileConfigResource newFileConfigResource() {
+ return new FileConfigResource(sitePaths);
+ }
+
+ private Config newConfigUpdate() {
+ Config configUpdate = new Config();
+ configUpdate.setInt(UPDATABLE_KEY, null, VALUE_KEY, 1);
+ return configUpdate;
+ }
+
+ private void assertConfigUpdate(Config config) {
+ assertConfigUpdate(config, 1);
+ }
+
+ private void assertConfigUpdate(Config config, int expectedValue) {
+ assertThat(config.getInt(UPDATABLE_KEY, null, VALUE_KEY, -1)).isEqualTo(expectedValue);
+ }
+
+ private FileBasedConfig getRawReplicationConfig() {
+ return getRawReplicationConfig(new Config());
+ }
+
+ private FileBasedConfig getRawReplicationConfig(Config base) {
+ Path configPath = sitePaths.etc_dir.resolve(FileConfigResource.CONFIG_NAME);
+ return new FileBasedConfig(base, configPath.toFile(), FS.DETECTED);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
index 18bb603..cae78e3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
@@ -22,6 +22,7 @@
import com.googlesource.gerrit.plugins.replication.api.ApiModule;
import com.googlesource.gerrit.plugins.replication.api.ConfigResource;
import com.googlesource.gerrit.plugins.replication.api.ReplicationConfigOverrides;
+import org.apache.commons.lang3.NotImplementedException;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@@ -78,6 +79,11 @@
}
@Override
+ public void update(Config config) {
+ throw new NotImplementedException("not implemented");
+ }
+
+ @Override
public String getVersion() {
return "base";
}
@@ -93,6 +99,11 @@
}
@Override
+ public void update(Config config) {
+ throw new NotImplementedException("not implemented");
+ }
+
+ @Override
public String getVersion() {
return "override";
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
new file mode 100644
index 0000000..40d9846
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
@@ -0,0 +1,212 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.replication;
+
+import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import com.google.common.io.MoreFiles;
+import com.google.common.truth.StringSubject;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.googlesource.gerrit.plugins.replication.api.ConfigResource;
+import com.googlesource.gerrit.plugins.replication.api.ReplicationConfigOverrides;
+import com.googlesource.gerrit.plugins.replication.api.ReplicationRemotesApi;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import org.eclipse.jgit.lib.Config;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ReplicationRemotesApiTest {
+
+ private Path testSite;
+ private SecureStore secureStoreMock;
+ private FileConfigResource baseConfig;
+ private Injector testInjector;
+ private AtomicReference<ReplicationConfigOverrides> testOverrides;
+ private String url1;
+ private String remoteName1;
+ private String url2;
+ private String remoteName2;
+
+ @Before
+ public void setUp() throws Exception {
+ testSite = Files.createTempDirectory("replicationRemotesUpdateTest");
+ secureStoreMock = mock(SecureStore.class);
+ baseConfig = new FileConfigResource(new SitePaths(testSite));
+ testOverrides = new AtomicReference<>(new TestReplicationConfigOverrides());
+ testInjector =
+ Guice.createInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ConfigResource.class).toInstance(baseConfig);
+ bind(SecureStore.class).toInstance(secureStoreMock);
+ bind(ReplicationRemotesApi.class).to(ReplicationRemotesApiImpl.class);
+ DynamicItem.itemOf(binder(), ReplicationConfigOverrides.class);
+ DynamicItem.bind(binder(), ReplicationConfigOverrides.class)
+ .toProvider(testOverrides::get);
+ }
+ });
+ url1 = "fake_url1";
+ remoteName1 = "site1";
+ url2 = "fake_url2";
+ remoteName2 = "site2";
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ MoreFiles.deleteRecursively(testSite, ALLOW_INSECURE);
+ }
+
+ @Test
+ public void shouldThrowWhenNoRemotesInTheUpdate() {
+ Config update = new Config();
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+ assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update));
+
+ update.setString("non-remote", null, "value", "one");
+
+ assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update));
+ }
+
+ @Test
+ public void shouldReturnEmptyConfigWhenNoRemotes() {
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+ assertThat(objectUnderTest.get("site").getSections()).isEmpty();
+ }
+
+ @Test
+ public void addRemoteSectionsToBaseConfigWhenNoOverrides() throws Exception {
+ testOverrides.set(null);
+
+ Config update = new Config();
+ setRemoteSite(update, remoteName1, "url", url1);
+ setRemoteSite(update, remoteName2, "url", url2);
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+ objectUnderTest.update(update);
+
+ assertRemoteSite(baseConfig.getConfig(), remoteName1, "url").isEqualTo(url1);
+ assertRemoteSite(baseConfig.getConfig(), remoteName2, "url").isEqualTo(url2);
+ }
+
+ @Test
+ public void shouldReturnRemotesFromBaseConfigWhenNoOverrides() {
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+ baseConfig.getConfig().setString("remote", remoteName1, "url", url1);
+ baseConfig.getConfig().setString("remote", remoteName2, "url", url2);
+
+ Config remotesConfig = objectUnderTest.get(remoteName1, remoteName2);
+ assertRemoteSite(remotesConfig, remoteName1, "url").isEqualTo(url1);
+ assertRemoteSite(remotesConfig, remoteName2, "url").isEqualTo(url2);
+ }
+
+ @Test
+ public void addRemotesSectionToBaseOverridesConfig() throws Exception {
+ Config update = new Config();
+ setRemoteSite(update, remoteName1, "url", url1);
+ setRemoteSite(update, remoteName2, "url", url2);
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+ objectUnderTest.update(update);
+
+ assertRemoteSite(testOverrides.get().getConfig(), remoteName1, "url").isEqualTo(url1);
+ assertRemoteSite(testOverrides.get().getConfig(), remoteName2, "url").isEqualTo(url2);
+ assertRemoteSite(baseConfig.getConfig(), remoteName1, "url").isNull();
+ assertRemoteSite(baseConfig.getConfig(), remoteName2, "url").isNull();
+
+ Config remotesConfig = objectUnderTest.get(remoteName1, remoteName2);
+ assertRemoteSite(remotesConfig, remoteName1, "url").isEqualTo(url1);
+ assertRemoteSite(remotesConfig, remoteName2, "url").isEqualTo(url2);
+ }
+
+ @Test
+ public void shouldSetPasswordViaSecureStoreButNotStoreInConfig() throws Exception {
+ Config update = new Config();
+ String password = "encrypted-password";
+ String remoteName = "site";
+ setRemoteSite(update, remoteName, "password", password);
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+ objectUnderTest.update(update);
+
+ verify(secureStoreMock).setList("remote", remoteName, "password", List.of(password));
+ assertRemoteSite(baseConfig.getConfig(), remoteName, "password").isNull();
+ assertRemoteSite(testOverrides.get().getConfig(), remoteName, "password").isNull();
+ assertRemoteSite(objectUnderTest.get(remoteName), remoteName, "password").isNull();
+ }
+
+ @Test
+ public void shouldSetUsernameViaSecureStoreButNotStoreInConfig() throws Exception {
+ Config update = new Config();
+ String username = "encrypted-username";
+ String remoteName = "site";
+ setRemoteSite(update, remoteName, "username", username);
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+ objectUnderTest.update(update);
+
+ verify(secureStoreMock).setList("remote", remoteName, "username", List.of(username));
+ assertRemoteSite(baseConfig.getConfig(), remoteName, "username").isNull();
+ assertRemoteSite(testOverrides.get().getConfig(), remoteName, "username").isNull();
+ assertRemoteSite(objectUnderTest.get(remoteName), remoteName, "username").isNull();
+ }
+
+ private void setRemoteSite(Config config, String remoteName, String name, String value) {
+ config.setString("remote", remoteName, name, value);
+ }
+
+ private StringSubject assertRemoteSite(Config config, String remoteName, String name) {
+ return assertThat(config.getString("remote", remoteName, name));
+ }
+
+ private ReplicationRemotesApi getReplicationRemotesApi() {
+ return testInjector.getInstance(ReplicationRemotesApi.class);
+ }
+
+ static class TestReplicationConfigOverrides implements ReplicationConfigOverrides {
+ private Config config = new Config();
+
+ @Override
+ public Config getConfig() {
+ return config;
+ }
+
+ @Override
+ public void update(Config update) throws IOException {
+ config = update;
+ }
+
+ @Override
+ public String getVersion() {
+ return "none";
+ }
+ }
+}
