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();