Skip to content

Commit 374ddf9

Browse files
committed
Fix accidental overwrite of existing url and push specs
When importing additional repositories on an existing organisation, the GitHub plugin preserves the legacy 'url' and 'push' specs for allowing custom ad-hoc settings, such as the use of stable SSH keys for replication or more restrictive push specs. The abstraction of the replication configuration update implemented with I9bde7e1148 has accidental broken the current behaviour causing a regression in how the remotes are configured. Reintroduce the correct behaviour by reading the existing config when updating a remote and avoid overwriting the url and push replication settings. Change-Id: Idd7113f6c0549d752b1dd31bf52d960aee1f77b1
1 parent 40ef0cb commit 374ddf9

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicationRemoteConfigBuilder.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414
package com.googlesource.gerrit.plugins.github.git;
1515

16+
import com.google.common.base.Strings;
1617
import com.google.gerrit.extensions.registration.DynamicItem;
1718
import com.google.inject.Inject;
1819
import com.googlesource.gerrit.plugins.github.GitHubURL;
@@ -44,23 +45,30 @@ class ReplicationRemoteConfigBuilder {
4445
}
4546

4647
Config build(String repositoryName) {
47-
Config remoteConfig = new Config();
48+
Config remoteConfig = replicationConfigItem.get().get(username);
4849

4950
remoteConfig.setString("remote", username, "username", username);
5051
remoteConfig.setString("remote", username, "password", authToken);
5152

52-
remoteConfig.setString("remote", username, "url", gitHubUrl + "/${name}.git");
53+
setRemoteConfigIfNotSet(remoteConfig, "url", gitHubUrl + "/${name}.git");
5354

5455
String[] existingProjects = getProjects();
5556
List<String> projects = new ArrayList<>(List.of(existingProjects));
5657
projects.add(repositoryName);
5758

5859
remoteConfig.setStringList("remote", username, "projects", projects);
59-
remoteConfig.setString("remote", username, "push", "refs/*:refs/*");
60+
setRemoteConfigIfNotSet(remoteConfig, "push", "refs/*:refs/*");
6061

6162
return remoteConfig;
6263
}
6364

65+
private void setRemoteConfigIfNotSet(Config remoteConfig, String key, String value) {
66+
String existingValue = remoteConfig.getString("remote", username, key);
67+
if (Strings.isNullOrEmpty(existingValue)) {
68+
remoteConfig.setString("remote", username, key, value);
69+
}
70+
}
71+
6472
private String[] getProjects() {
6573
ReplicationRemotesApi config = replicationConfigItem.get();
6674
if (config != null) {

github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/git/ReplicationRemoteConfigBuilderTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ public void shouldAppendProjectToConfig() throws Exception {
6161
assertThat(actual.getString("remote", username, "push")).isEqualTo("refs/*:refs/*");
6262
}
6363

64+
@Test
65+
public void shoudKeepCustomUrlAndPushRefSpecInRemoteConfig() throws Exception {
66+
Config currentConfig = new Config();
67+
String customPushRefSpec = "+refs/heads/myheads/*:refs/heads/myheads/*";
68+
String customUrl = "ssh://[email protected]/myuser/${name}.git";
69+
currentConfig.setString("remote", username, "push", customPushRefSpec);
70+
currentConfig.setString("remote", username, "url", customUrl);
71+
72+
ReplicationRemoteConfigBuilder builder = newReplicationRemoteConfigBuilder(currentConfig);
73+
Config actual = builder.build(repoName);
74+
75+
assertThat(actual.getString("remote", username, "push")).isEqualTo(customPushRefSpec);
76+
assertThat(actual.getString("remote", username, "url")).isEqualTo(customUrl);
77+
}
78+
6479
private ReplicationRemoteConfigBuilder newReplicationRemoteConfigBuilder() throws Exception {
6580
return newReplicationRemoteConfigBuilder(new Config());
6681
}

0 commit comments

Comments
 (0)