Add support for overriding replication configuration
It is a next step in the direction of modifying the replicationn
configuraiton from an external source. Where the external source could
be a git repository or third party configuration managemnet system.
This adds a `ReplicationConfigOverrides` interface and a default bindng
for `DynamicItem<ReplicationConfigOverrides`. The added interface is
just an extension of `ConfigResource` as Guice was complaining about
duplicated binding for `ConfigResource` when it was used in
`DynamicItem`. Additinally `RepilcationConfigOverrides` conveys better
its purpose.
For now, the default implementation provides no overrides. As the
location of overrides files is still to be decided.
Finally, the `MergedConfigResource` will take both, the base
configuraiton for the file system, and configured overrides and merge
both JGit `Config` objects togehter.
With this approach, an implementation of `ReplicationConfigOverrides`
can provide a single confiuration option or the whole configuration
file.
Later we may consider, discarding some of the overrides, like
`gerrit.autoReload`, as disabling that option will effecively prevent
users from updating configuation.
Change-Id: I2e401c05571180719df33a4a094fbb4ccfb39f23
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 b6532d9..9d0c978 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
@@ -42,7 +42,7 @@
public AutoReloadConfigDecorator(
@PluginName String pluginName,
WorkQueue workQueue,
- FileReplicationConfig replicationConfig,
+ ReplicationConfigImpl replicationConfig,
AutoReloadRunnable reloadRunner,
EventBus eventBus) {
this.currentConfig = replicationConfig;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java
index 9ed43fe..ae07760 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java
@@ -27,14 +27,14 @@
private final Provider<ObservableQueue> queueObserverProvider;
private final ConfigParser configParser;
private ReplicationConfig loadedConfig;
- private Provider<FileReplicationConfig> replicationConfigProvider;
+ private Provider<ReplicationConfigImpl> replicationConfigProvider;
private String loadedConfigVersion;
private String lastFailedConfigVersion;
@Inject
public AutoReloadRunnable(
ConfigParser configParser,
- Provider<FileReplicationConfig> replicationConfigProvider,
+ Provider<ReplicationConfigImpl> replicationConfigProvider,
EventBus eventBus,
Provider<ObservableQueue> queueObserverProvider) {
this.replicationConfigProvider = replicationConfigProvider;
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 9c70aa6..008e50b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -15,7 +15,7 @@
package com.googlesource.gerrit.plugins.replication;
import static com.google.gerrit.server.project.ProjectCache.noSuchProject;
-import static com.googlesource.gerrit.plugins.replication.FileReplicationConfig.replaceName;
+import static com.googlesource.gerrit.plugins.replication.ReplicationConfigImpl.replaceName;
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.NON_EXISTING;
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON;
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 1d471fe..08f3477 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
@@ -17,7 +17,7 @@
import static com.googlesource.gerrit.plugins.replication.AdminApiFactory.isGerrit;
import static com.googlesource.gerrit.plugins.replication.AdminApiFactory.isGerritHttp;
import static com.googlesource.gerrit.plugins.replication.AdminApiFactory.isSSH;
-import static com.googlesource.gerrit.plugins.replication.FileReplicationConfig.replaceName;
+import static com.googlesource.gerrit.plugins.replication.ReplicationConfigImpl.replaceName;
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
import static java.util.stream.Collectors.toList;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
new file mode 100644
index 0000000..4cd0af0
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
@@ -0,0 +1,75 @@
+// 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.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.util.Providers;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+
+public class MergedConfigResource {
+ @VisibleForTesting
+ static MergedConfigResource withBaseOnly(ConfigResource base) {
+ return new MergedConfigResource(Providers.of(base), null);
+ }
+
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final Provider<ConfigResource> base;
+ @Nullable private final DynamicItem<ReplicationConfigOverrides> overrides;
+
+ @Inject
+ MergedConfigResource(
+ Provider<ConfigResource> base, @Nullable DynamicItem<ReplicationConfigOverrides> overrides) {
+ this.base = base;
+ this.overrides = overrides;
+ }
+
+ public Config getConfig() {
+ Config config = base.get().getConfig();
+ if (noOverrides()) {
+ return config;
+ }
+
+ String overridesText = overrides.get().getConfig().toText();
+ if (!overridesText.isEmpty()) {
+ try {
+ config.fromText(overridesText);
+ } catch (ConfigInvalidException e) {
+ logger.atWarning().withCause(e).log("Failed to merge replication config overrides");
+ }
+ }
+
+ return config;
+ }
+
+ public String getVersion() {
+ String baseVersion = base.get().getVersion();
+ if (noOverrides()) {
+ return baseVersion;
+ }
+
+ return baseVersion + overrides.get().getVersion();
+ }
+
+ private boolean noOverrides() {
+ return overrides == null || overrides.get() == null;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/FileReplicationConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
similarity index 89%
rename from src/main/java/com/googlesource/gerrit/plugins/replication/FileReplicationConfig.java
rename to src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
index b34ea54..59c6bb7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/FileReplicationConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.replication;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.annotations.PluginData;
@@ -22,7 +21,7 @@
import java.nio.file.Path;
import org.eclipse.jgit.lib.Config;
-public class FileReplicationConfig implements ReplicationConfig {
+public class ReplicationConfigImpl implements ReplicationConfig {
private static final int DEFAULT_SSH_CONNECTION_TIMEOUT_MS = 2 * 60 * 1000; // 2 minutes
private final SitePaths site;
@@ -32,19 +31,18 @@
private final int maxRefsToShow;
private int sshCommandTimeout;
private int sshConnectionTimeout = DEFAULT_SSH_CONNECTION_TIMEOUT_MS;
- private final ConfigResource configResource;
+ private final MergedConfigResource configResource;
private final Path pluginDataDir;
// TODO: remove in follow-up change, as this was added to reduce the diff size
// for change Ic6a5c5b8ab5
- @VisibleForTesting
- public FileReplicationConfig(SitePaths paths, @PluginData Path pluginDataDir) {
- this(new FileConfigResource(paths), paths, pluginDataDir);
+ public ReplicationConfigImpl(SitePaths paths, @PluginData Path pluginDataDir) {
+ this(MergedConfigResource.withBaseOnly(new FileConfigResource(paths)), paths, pluginDataDir);
}
@Inject
- public FileReplicationConfig(
- ConfigResource configResource, SitePaths site, @PluginData Path pluginDataDir) {
+ public ReplicationConfigImpl(
+ MergedConfigResource configResource, SitePaths site, @PluginData Path pluginDataDir) {
this.site = site;
this.configResource = configResource;
Config config = configResource.getConfig();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
index 220ec50..bcea543 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
@@ -18,6 +18,7 @@
import static com.googlesource.gerrit.plugins.replication.FileConfigResource.CONFIG_NAME;
import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
@@ -46,6 +47,7 @@
@Override
protected void configure() {
bind(ConfigResource.class).to(getConfigResourceClass());
+ DynamicItem.itemOf(binder(), ReplicationConfigOverrides.class);
if (getReplicationConfig().getBoolean("gerrit", "autoReload", false)) {
bind(ReplicationConfig.class).to(AutoReloadConfigDecorator.class).in(Scopes.SINGLETON);
@@ -53,7 +55,7 @@
.annotatedWith(UniqueAnnotations.create())
.to(AutoReloadConfigDecorator.class);
} else {
- bind(ReplicationConfig.class).to(FileReplicationConfig.class).in(Scopes.SINGLETON);
+ bind(ReplicationConfig.class).to(ReplicationConfigImpl.class).in(Scopes.SINGLETON);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigOverrides.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigOverrides.java
new file mode 100644
index 0000000..97a77bf
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigOverrides.java
@@ -0,0 +1,21 @@
+// 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;
+
+/**
+ * Provide a way to override or extend replication configuration from other sources, like git
+ * repository or external configuration management tool.
+ */
+public interface ReplicationConfigOverrides extends ConfigResource {}
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 4c43cb0..342d394 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java
@@ -131,7 +131,8 @@
protected DestinationsCollection newDestinationsCollections(ConfigResource configResource)
throws ConfigInvalidException {
return newDestinationsCollections(
- new FileReplicationConfig(configResource, sitePaths, pluginDataPath));
+ new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(configResource), sitePaths, pluginDataPath));
}
protected DestinationsCollection newDestinationsCollections(ReplicationConfig replicationConfig)
@@ -144,7 +145,7 @@
eventBus);
}
- protected FileReplicationConfig newReplicationFileBasedConfig() {
- return new FileReplicationConfig(sitePaths, pluginDataPath);
+ protected ReplicationConfigImpl newReplicationFileBasedConfig() {
+ return new ReplicationConfigImpl(sitePaths, pluginDataPath);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
index bb08830..aa0d5f3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
@@ -76,13 +76,18 @@
remoteConfig.save();
replicationConfig =
- new FileReplicationConfig(new FanoutConfigResource(sitePaths), sitePaths, pluginDataPath);
+ new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(new FanoutConfigResource(sitePaths)),
+ sitePaths,
+ pluginDataPath);
newAutoReloadConfig(
() -> {
try {
- return new FileReplicationConfig(
- new FanoutConfigResource(sitePaths), sitePaths, pluginDataPath);
+ return new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(new FanoutConfigResource(sitePaths)),
+ sitePaths,
+ pluginDataPath);
} catch (IOException | ConfigInvalidException e) {
throw new RuntimeException(e);
}
@@ -125,13 +130,18 @@
remoteConfig.save();
replicationConfig =
- new FileReplicationConfig(new FanoutConfigResource(sitePaths), sitePaths, pluginDataPath);
+ new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(new FanoutConfigResource(sitePaths)),
+ sitePaths,
+ pluginDataPath);
newAutoReloadConfig(
() -> {
try {
- return new FileReplicationConfig(
- new FanoutConfigResource(sitePaths), sitePaths, pluginDataPath);
+ return new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(new FanoutConfigResource(sitePaths)),
+ sitePaths,
+ pluginDataPath);
} catch (IOException | ConfigInvalidException e) {
throw new RuntimeException(e);
}
@@ -173,13 +183,18 @@
remoteConfig.save();
replicationConfig =
- new FileReplicationConfig(new FanoutConfigResource(sitePaths), sitePaths, pluginDataPath);
+ new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(new FanoutConfigResource(sitePaths)),
+ sitePaths,
+ pluginDataPath);
newAutoReloadConfig(
() -> {
try {
- return new FileReplicationConfig(
- new FanoutConfigResource(sitePaths), sitePaths, pluginDataPath);
+ return new ReplicationConfigImpl(
+ MergedConfigResource.withBaseOnly(new FanoutConfigResource(sitePaths)),
+ sitePaths,
+ pluginDataPath);
} catch (IOException | ConfigInvalidException e) {
throw new RuntimeException(e);
}
@@ -234,14 +249,14 @@
}
private AutoReloadConfigDecorator newAutoReloadConfig(
- Supplier<FileReplicationConfig> configSupplier) {
+ Supplier<ReplicationConfigImpl> configSupplier) {
AutoReloadRunnable autoReloadRunnable =
new AutoReloadRunnable(
configParser,
- new Provider<FileReplicationConfig>() {
+ new Provider<ReplicationConfigImpl>() {
@Override
- public FileReplicationConfig get() {
+ public ReplicationConfigImpl get() {
return configSupplier.get();
}
},
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnableTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnableTest.java
index 14307f8..8a1b85d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnableTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnableTest.java
@@ -81,11 +81,11 @@
autoReloadRunnable.run();
}
- private Provider<FileReplicationConfig> newVersionConfigProvider() {
+ private Provider<ReplicationConfigImpl> newVersionConfigProvider() {
return new Provider<>() {
@Override
- public FileReplicationConfig get() {
- return new FileReplicationConfig(sitePaths, sitePaths.data_dir) {
+ public ReplicationConfigImpl get() {
+ return new ReplicationConfigImpl(sitePaths, sitePaths.data_dir) {
@Override
public String getVersion() {
return String.format("%s", System.nanoTime());
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 34712a7..9a5ece9 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
@@ -266,7 +266,7 @@
FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
String replicationConfigVersion =
- new FileReplicationConfig(sitePaths, pluginDataPath).getVersion();
+ new ReplicationConfigImpl(sitePaths, pluginDataPath).getVersion();
MoreFiles.deleteRecursively(sitePaths.etc_dir.resolve("replication"), ALLOW_INSECURE);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
new file mode 100644
index 0000000..80ad449
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
@@ -0,0 +1,112 @@
+// 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 com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class MergedConfigResourceTest {
+ private static final int BASE_CONFIG_MAX_RETIRES = 10;
+ private static final int OVERRIDDEN_CONFIG_MAX_RETIRES = 5;
+
+ @Test
+ public void onlyUseBaseConfig() {
+ final MergedConfigResource configResource = newMergedConfigResource();
+
+ assertThat(configResource.getVersion()).isEqualTo("base");
+ assertThat(getMaxRetires(configResource)).isEqualTo(BASE_CONFIG_MAX_RETIRES);
+ }
+
+ @Test
+ public void overrideBaseConfig() {
+ final MergedConfigResource configResource =
+ newMergedConfigResource(TestReplicationConfigOverrides.class);
+
+ assertThat(configResource.getVersion()).isEqualTo("baseoverride");
+ assertThat(getMaxRetires(configResource)).isEqualTo(OVERRIDDEN_CONFIG_MAX_RETIRES);
+ assertThat(getUseGcClient(configResource)).isTrue();
+ }
+
+ private MergedConfigResource newMergedConfigResource() {
+ return newMergedConfigResource(null);
+ }
+
+ private MergedConfigResource newMergedConfigResource(
+ Class<? extends ReplicationConfigOverrides> overrides) {
+ return Guice.createInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ConfigResource.class).to(TestBaseConfigResource.class);
+
+ DynamicItem.itemOf(binder(), ReplicationConfigOverrides.class);
+ if (overrides != null) {
+ DynamicItem.bind(binder(), ReplicationConfigOverrides.class).to(overrides);
+ }
+ }
+ })
+ .getInstance(MergedConfigResource.class);
+ }
+
+ private static class TestBaseConfigResource implements ConfigResource {
+ @Override
+ public Config getConfig() {
+ Config config = new Config();
+ setMaxRetires(config, BASE_CONFIG_MAX_RETIRES);
+ return config;
+ }
+
+ @Override
+ public String getVersion() {
+ return "base";
+ }
+ }
+
+ private static class TestReplicationConfigOverrides implements ReplicationConfigOverrides {
+ @Override
+ public Config getConfig() {
+ Config config = new Config();
+ setMaxRetires(config, OVERRIDDEN_CONFIG_MAX_RETIRES);
+ setUseGcClient(config, true);
+ return config;
+ }
+
+ @Override
+ public String getVersion() {
+ return "override";
+ }
+ }
+
+ private static void setMaxRetires(Config config, int value) {
+ config.setInt("replication", null, "maxRetries", value);
+ }
+
+ private static void setUseGcClient(Config config, boolean value) {
+ config.setBoolean("replication", null, "useGcClient", value);
+ }
+
+ private static int getMaxRetires(MergedConfigResource resource) {
+ return resource.getConfig().getInt("replication", null, "maxRetries", -1);
+ }
+
+ private static boolean getUseGcClient(MergedConfigResource resource) {
+ return resource.getConfig().getBoolean("replication", null, "useGcClient", false);
+ }
+}
