Ignore remote configurations without a fetch URL
Fix an issue where all push replication remotes were
incorrectly classified as fetch sources and configured
with the default fetch URLs.
Change-Id: I1f30a9926b89cd576250ce5160e633b658c6e3de
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
index c4c22c7..f7deaf4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
@@ -29,7 +29,6 @@
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.RemoteConfig;
import org.eclipse.jgit.transport.URIish;
@@ -64,12 +63,6 @@
continue;
}
- // fetch source has to be specified.
- if (c.getFetchRefSpecs().isEmpty()) {
- throw new ConfigInvalidException(
- String.format("You must specify a valid refSpec for this remote"));
- }
-
SourceConfiguration sourceConfig = new SourceConfiguration(c, config);
if (!sourceConfig.isSingleProjectMatch()) {
@@ -108,14 +101,9 @@
for (String name : names) {
try {
final RemoteConfig remoteConfig = new RemoteConfig(cfg, name);
- if (remoteConfig.getFetchRefSpecs().isEmpty()) {
- remoteConfig.setFetchRefSpecs(
- List.of(
- new RefSpec()
- .setSourceDestination("refs/*", "refs/*")
- .setForceUpdate(replicationConfigProvider.get().isDefaultForceUpdate())));
+ if (!remoteConfig.getFetchRefSpecs().isEmpty()) {
+ result.add(remoteConfig);
}
- result.add(remoteConfig);
} catch (URISyntaxException e) {
throw new ConfigInvalidException(
String.format("remote %s has invalid URL in %s", name, cfg));
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 1635e5c..74648eb 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -21,6 +21,7 @@
[remote "host-two"]
url = gerrit2@host-two.example.com:/some/path/${name}.git
apiUrl = http://host-two.example.com:8080
+ fetch = +refs/*:refs/*
[remote "pubmirror"]
url = mirror1.us.some.org:/pub/git/${name}.git
@@ -443,12 +444,8 @@
when `replicatePermissions` is true, even if the push refspec
is 'all refs'.
-> NOTE: When using the pull-replication and replication plugins together,
-> **NOT having** a `fetch` configuration for a remote, will also enable
-> the standard _push_ replication for that remote.
-
- Default: refs/*:refs/*, with the force fetch set to
- `gerrit.defaultForceUpdate`
+> NOTE: **NOT having** a `fetch` configuration for a remote, causes
+> the remote to be ignored.
[2]: #example_file
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java
index 6229e02..c7102ae 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java
@@ -18,9 +18,12 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.inject.util.Providers;
+import com.googlesource.gerrit.plugins.replication.RemoteConfiguration;
import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig;
+import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.transport.RefSpec;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -29,6 +32,10 @@
@RunWith(MockitoJUnitRunner.class)
public class SourceConfigParserTest {
+ private static final String TEST_REMOTE_URL = "http://git.foo.remote.url/${name}";
+ private static final String TEST_REMOTE_FETCH_REFSPEC = "refs/heads/mybranch:refs/heads/mybranch";
+ private static final String TEST_REMOTE_NAME = "testremote";
+
@Mock ReplicationConfig replicationConfigMock;
private SourceConfigParser objectUnderTest;
@@ -54,4 +61,29 @@
.getMessage();
assertThat(errorMessage).contains("invalid configuration");
}
+
+ @Test
+ public void shouldIgnoreRemoteWithoutFetchConfiguration() throws Exception {
+ Config config = new Config();
+ config.fromText("[remote \"foo\"]\n" + "url = http://git.foo.remote.url/repo");
+
+ assertThat(objectUnderTest.parseRemotes(config)).isEmpty();
+ }
+
+ @Test
+ public void shouldParseRemoteWithFetchConfiguration() throws Exception {
+ Config config = new Config();
+ config.fromText(
+ String.format(
+ "[remote \"%s\"]\n" + "url = %s\n" + "fetch = %s",
+ TEST_REMOTE_NAME, TEST_REMOTE_URL, TEST_REMOTE_FETCH_REFSPEC));
+
+ List<RemoteConfiguration> remotes = objectUnderTest.parseRemotes(config);
+ assertThat(remotes).hasSize(1);
+ RemoteConfiguration remoteConfig = remotes.get(0);
+
+ assertThat(remoteConfig.getUrls()).containsExactly(TEST_REMOTE_URL);
+ assertThat(remoteConfig.getRemoteConfig().getFetchRefSpecs())
+ .containsExactly(new RefSpec(TEST_REMOTE_FETCH_REFSPEC));
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourcesFetchPeriodicallyIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourcesFetchPeriodicallyIT.java
index 5ac1deb..f51376d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourcesFetchPeriodicallyIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourcesFetchPeriodicallyIT.java
@@ -51,6 +51,7 @@
List<String> fetchUrls =
buildReplicaURLs(replicaSuffixes, s -> gitPath.resolve("${name}" + s + ".git").toString());
config.setStringList("remote", remoteName, "url", fetchUrls);
+ config.setString("remote", remoteName, "fetch", "+refs/*:refs/*");
project.ifPresent(prj -> config.setString("remote", remoteName, "projects", prj));
config.setString("remote", remoteName, "fetchEvery", TEST_FETCH_FREQUENCY);
config.save();
