Introduce API for retrieving the remote config
Extend the ReplicationRemotesApi for retrieving the
configuration associated with multiple replication remotes.
NOTE: The configuration would not include passwords
for the sake of not exposing them when not necessarily needed.
Change-Id: I26b6f098dd7b6331adf929315b2cc1cebdf0eb4a
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 315c4a6..58a10f0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
@@ -78,7 +78,7 @@
return baseVersion + overrides.get().getVersion();
}
- private boolean noOverrides() {
+ boolean noOverrides() {
return overrides == null || overrides.get() == null;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
index dd4d5b0..e4ef318 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
@@ -14,34 +14,42 @@
package com.googlesource.gerrit.plugins.replication;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.server.securestore.SecureStore;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
-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.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 Provider<ConfigResource> baseConfigProvider;
- private final DynamicItem<ReplicationConfigOverrides> configOverridesItem;
+ private final MergedConfigResource mergedConfigResource;
@Inject
- ReplicationRemotesApiImpl(
- SecureStore secureStore,
- Provider<ConfigResource> baseConfigProvider,
- @Nullable DynamicItem<ReplicationConfigOverrides> configOverridesItem) {
+ ReplicationRemotesApiImpl(SecureStore secureStore, MergedConfigResource mergedConfigResource) {
this.secureStore = secureStore;
- this.baseConfigProvider = baseConfigProvider;
- this.configOverridesItem = configOverridesItem;
+ 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
@@ -54,11 +62,7 @@
SeparatedRemoteConfigs configs = onlyRemoteSectionsWithSeparatedPasswords(remoteConfig);
persistRemotesPasswords(configs);
- if (hasConfigOverrides()) {
- configOverridesItem.get().update(configs.remotes);
- } else {
- baseConfigProvider.get().update(configs.remotes);
- }
+ mergedConfigResource.update(configs.remotes);
}
private SeparatedRemoteConfigs onlyRemoteSectionsWithSeparatedPasswords(Config configUpdates) {
@@ -85,10 +89,6 @@
}
}
- private boolean hasConfigOverrides() {
- return configOverridesItem != null && configOverridesItem.get() != null;
- }
-
private static class SeparatedRemoteConfigs {
private final Config remotes = new Config();
private final Config passwords = new Config();
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
index fde2abc..76d6d35 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java
@@ -28,6 +28,19 @@
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>
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
index 3aa97e8..a806875 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
@@ -19,20 +19,23 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
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.util.Providers;
+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;
@@ -43,12 +46,36 @@
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
@@ -59,7 +86,7 @@
@Test
public void shouldThrowWhenNoRemotesInTheUpdate() {
Config update = new Config();
- ReplicationRemotesApi objectUnderTest = newReplicationConfigUpdater();
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update));
@@ -69,64 +96,83 @@
}
@Test
- public void addRemoteSectionToBaseConfigWhenNoOverrides() throws Exception {
- String url = "fake_url";
- Config update = new Config();
- setRemoteSite(update, "url", url);
- ReplicationRemotesApi objectUnderTest = newReplicationConfigUpdater();
-
- objectUnderTest.update(update);
-
- assertRemoteSite(baseConfig.getConfig(), "url").isEqualTo(url);
+ public void shouldReturnEmptyConfigWhenNoRemotes() {
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+ assertThat(objectUnderTest.get("site").getSections()).isEmpty();
}
@Test
- public void addRemoteSectionToBaseOverridesConfig() throws Exception {
- TestReplicationConfigOverrides testOverrides = new TestReplicationConfigOverrides();
- String url = "fake_url";
+ public void addRemoteSectionsToBaseConfigWhenNoOverrides() throws Exception {
+ testOverrides.set(null);
+
Config update = new Config();
- setRemoteSite(update, "url", url);
- ReplicationRemotesApi objectUnderTest = newReplicationConfigUpdater(testOverrides);
+ setRemoteSite(update, remoteName1, "url", url1);
+ setRemoteSite(update, remoteName2, "url", url2);
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
objectUnderTest.update(update);
- assertRemoteSite(testOverrides.getConfig(), "url").isEqualTo(url);
- assertRemoteSite(baseConfig.getConfig(), "url").isNull();
+ assertRemoteSite(baseConfig.getConfig(), remoteName1, "url").isEqualTo(url1);
+ assertRemoteSite(baseConfig.getConfig(), remoteName2, "url").isEqualTo(url2);
}
@Test
- public void encryptPassword() throws Exception {
- TestReplicationConfigOverrides testOverrides = new TestReplicationConfigOverrides();
+ 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 shouldEncryptPasswordButNotStoreInConfig() throws Exception {
Config update = new Config();
String password = "my_secret_password";
- setRemoteSite(update, "password", password);
- ReplicationRemotesApi objectUnderTest = newReplicationConfigUpdater(testOverrides);
+ String remoteName = "site";
+ setRemoteSite(update, remoteName, "password", password);
+ ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
objectUnderTest.update(update);
- verify(secureStoreMock).setList("remote", "site", "password", List.of(password));
- assertRemoteSite(baseConfig.getConfig(), "password").isNull();
- assertRemoteSite(testOverrides.getConfig(), "password").isNull();
+ 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();
}
- private ReplicationRemotesApi newReplicationConfigUpdater() {
- return newReplicationConfigUpdater(null);
+ private void setRemoteSite(Config config, String remoteName, String name, String value) {
+ config.setString("remote", remoteName, name, value);
}
- private void setRemoteSite(Config config, String name, String value) {
- config.setString("remote", "site", name, value);
+ private StringSubject assertRemoteSite(Config config, String remoteName, String name) {
+ return assertThat(config.getString("remote", remoteName, name));
}
- private StringSubject assertRemoteSite(Config config, String name) {
- return assertThat(config.getString("remote", "site", name));
- }
-
- private ReplicationRemotesApi newReplicationConfigUpdater(ReplicationConfigOverrides overrides) {
- DynamicItem<ReplicationConfigOverrides> dynamicItemMock = mock(DynamicItem.class);
- when(dynamicItemMock.get()).thenReturn(overrides);
-
- return new ReplicationRemotesApiImpl(
- secureStoreMock, Providers.of(baseConfig), dynamicItemMock);
+ private ReplicationRemotesApi getReplicationRemotesApi() {
+ return testInjector.getInstance(ReplicationRemotesApi.class);
}
static class TestReplicationConfigOverrides implements ReplicationConfigOverrides {
