Allow to configure own URL instead of relying on httpd.listenUrl
The JGroupsPeerInfoProvider was using the value of httpd.listenUrl [1]
from the gerrit.config to detect its own URL, which is broadcast to
the jgroups channel for other peers to discover.
There were several problems with this approach.
- The httpd.listenUrl can be configured with a reverse proxy, for
example:
[httpd]
listenUrl = proxy-https://localhost:9080
When a site is configured in such a way, using the URL as-is causes
the peer's HTTP POST requests to fail with the error:
"proxy-https protocol is not supported"
- When configured with a reverse-proxy, the listenUrl's port might be
bound only to 127.0.0.1, which means it will be inaccessible from
the peer machine.
- The listenUrl's hostname may be configured as `*` to listen on all
local addresses. In this case the URL must be rewritten to replace
the `*` with a valid hostname.
- It is allowed to specify multiple values for httpd.listenUrl, but
the provider is only reading a single value from the configuration.
Rather than trying to handle these cases, allow to explicitly specify
the own URL in the JGroups peer info section of the plugin's config
file (etc/high-availability.config):
[peerInfo]
strategy = jgroups
[peerInfo "jgroups"]
myUrl = https://myserver:8080/
If the myUrl value is not specified, fall back to httpd.listenUrl, but
fail early if its value is not reasonable, i.e. require it to be a
single value with an explicit hostname.
[1] http://gerrit-documentation.storage.googleapis.com/Documentation/2.14/config-gerrit.html#httpd.listenUrl
Change-Id: Ifd97c751eea5e1eb99e70b3f02f5d80d8c368540
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
index 5bff79f..8cb57f3 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
@@ -20,6 +20,7 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.PluginConfigFactory;
@@ -51,6 +52,7 @@
static final String JGROUPS_SUBSECTION = PeerInfoStrategy.JGROUPS.name().toLowerCase();
static final String URL_KEY = "url";
static final String STRATEGY_KEY = "strategy";
+ static final String MY_URL_KEY = "myUrl";
// http section
static final String HTTP_SECTION = "http";
@@ -197,6 +199,11 @@
return ((value == null) ? defaultValue : value);
}
+ @Nullable
+ private static String trimTrailingSlash(@Nullable String in) {
+ return in == null ? null : CharMatcher.is('/').trimTrailingFrom(in);
+ }
+
public static class Main {
private final Path sharedDirectory;
@@ -235,10 +242,8 @@
private PeerInfoStatic(Config cfg) {
url =
- CharMatcher.is('/')
- .trimTrailingFrom(
- Strings.nullToEmpty(
- cfg.getString(PEER_INFO_SECTION, STATIC_SUBSECTION, URL_KEY)));
+ trimTrailingSlash(
+ Strings.nullToEmpty(cfg.getString(PEER_INFO_SECTION, STATIC_SUBSECTION, URL_KEY)));
}
public String url() {
@@ -249,6 +254,7 @@
public static class PeerInfoJGroups {
private final ImmutableList<String> skipInterface;
private final String clusterName;
+ private final String myUrl;
private PeerInfoJGroups(Config cfg) {
String[] skip = cfg.getStringList(PEER_INFO_SECTION, JGROUPS_SUBSECTION, SKIP_INTERFACE_KEY);
@@ -256,6 +262,7 @@
clusterName =
getString(
cfg, PEER_INFO_SECTION, JGROUPS_SUBSECTION, CLUSTER_NAME_KEY, DEFAULT_CLUSTER_NAME);
+ myUrl = trimTrailingSlash(cfg.getString(PEER_INFO_SECTION, JGROUPS_SUBSECTION, MY_URL_KEY));
}
public ImmutableList<String> skipInterface() {
@@ -265,6 +272,10 @@
public String clusterName() {
return clusterName;
}
+
+ public String myUrl() {
+ return myUrl;
+ }
}
public static class Http {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/JGroupsPeerInfoProvider.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/JGroupsPeerInfoProvider.java
index fd1781a..39ba578 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/JGroupsPeerInfoProvider.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/JGroupsPeerInfoProvider.java
@@ -11,21 +11,17 @@
// 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.ericsson.gerrit.plugins.highavailability.peers.jgroups;
import com.ericsson.gerrit.plugins.highavailability.Configuration;
import com.ericsson.gerrit.plugins.highavailability.peers.PeerInfo;
import com.google.gerrit.extensions.events.LifecycleListener;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.net.InetAddress;
-import java.net.URISyntaxException;
-import java.net.UnknownHostException;
import java.util.Optional;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.transport.URIish;
import org.jgroups.Address;
import org.jgroups.JChannel;
import org.jgroups.Message;
@@ -49,9 +45,9 @@
implements Provider<Optional<PeerInfo>>, LifecycleListener {
private static final Logger log = LoggerFactory.getLogger(JGroupsPeerInfoProvider.class);
- private final String myUrl;
private final Configuration.PeerInfoJGroups jgroupsConfig;
private final InetAddressFinder finder;
+ private final String myUrl;
private JChannel channel;
private Optional<PeerInfo> peerInfo = Optional.empty();
@@ -59,15 +55,10 @@
@Inject
JGroupsPeerInfoProvider(
- @GerritServerConfig Config srvConfig,
- Configuration pluginConfiguration,
- InetAddressFinder finder)
- throws UnknownHostException, URISyntaxException {
- String hostName = InetAddress.getLocalHost().getHostName();
- URIish u = new URIish(srvConfig.getString("httpd", null, "listenUrl"));
- this.myUrl = u.setHost(hostName).toString();
+ Configuration pluginConfiguration, InetAddressFinder finder, MyUrlProvider myUrlProvider) {
this.jgroupsConfig = pluginConfiguration.peerInfoJGroups();
this.finder = finder;
+ this.myUrl = myUrlProvider.get();
}
@Override
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/MyUrlProvider.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/MyUrlProvider.java
new file mode 100644
index 0000000..dbb5d1d
--- /dev/null
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/peers/jgroups/MyUrlProvider.java
@@ -0,0 +1,101 @@
+// Copyright (C) 2017 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.ericsson.gerrit.plugins.highavailability.peers.jgroups;
+
+import com.ericsson.gerrit.plugins.highavailability.Configuration;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.CharMatcher;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.ProvisionException;
+import com.google.inject.Singleton;
+import java.net.InetAddress;
+import java.net.URISyntaxException;
+import java.net.UnknownHostException;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.transport.URIish;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class MyUrlProvider implements Provider<String> {
+ private static final Logger log = LoggerFactory.getLogger(MyUrlProvider.class);
+
+ private static final String HTTPD_SECTION = "httpd";
+ private static final String LISTEN_URL_KEY = "listenUrl";
+ private static final String LISTEN_URL = HTTPD_SECTION + "." + LISTEN_URL_KEY;
+ private static final String PROXY_PREFIX = "proxy-";
+
+ private final String myUrl;
+
+ @Inject
+ @VisibleForTesting
+ public MyUrlProvider(@GerritServerConfig Config srvConfig, Configuration pluginConfiguration) {
+ String url = pluginConfiguration.peerInfoJGroups().myUrl();
+ if (url == null) {
+ log.info("myUrl not configured; attempting to determine from {}", LISTEN_URL);
+ try {
+ url = CharMatcher.is('/').trimTrailingFrom(getMyUrlFromListenUrl(srvConfig));
+ } catch (MyUrlProviderException e) {
+ throw new ProvisionException(e.getMessage());
+ }
+ }
+ this.myUrl = url;
+ }
+
+ @Override
+ public String get() {
+ return myUrl;
+ }
+
+ private String getMyUrlFromListenUrl(Config srvConfig) throws MyUrlProviderException {
+ String[] listenUrls = srvConfig.getStringList(HTTPD_SECTION, null, LISTEN_URL_KEY);
+ if (listenUrls.length != 1) {
+ throw new MyUrlProviderException(
+ String.format(
+ "Can only determine myUrl from %s when there is exactly 1 value configured; found %d",
+ LISTEN_URL, listenUrls.length));
+ }
+ String url = listenUrls[0];
+ if (url.startsWith(PROXY_PREFIX)) {
+ throw new MyUrlProviderException(
+ String.format(
+ "Cannot determine myUrl from %s when configured as reverse-proxy: %s",
+ LISTEN_URL, url));
+ }
+ if (url.contains("*")) {
+ throw new MyUrlProviderException(
+ String.format(
+ "Cannot determine myUrl from %s when configured with wildcard: %s", LISTEN_URL, url));
+ }
+ try {
+ URIish u = new URIish(url);
+ return u.setHost(InetAddress.getLocalHost().getHostName()).toString();
+ } catch (URISyntaxException | UnknownHostException e) {
+ throw new MyUrlProviderException(
+ String.format(
+ "Unable to determine myUrl from %s value [%s]: %s", LISTEN_URL, url, e.getMessage()));
+ }
+ }
+
+ private static class MyUrlProviderException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ MyUrlProviderException(String message) {
+ super(message);
+ }
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index e537545..1c7caaf 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -22,6 +22,7 @@
[peerInfo]
: strategy = jgroups
[peerInfo "jgroups"]
+: myUrl = local_instance_url
: cluster = foo
: skipInterface = lo*
: skipInterface = eth2
@@ -66,6 +67,14 @@
Defaults to the list of: `lo*`, `utun*`, `awdl*` which are known to be
inappropriate for JGroups communication.
+peerInfo.jgroups.myUrl
+: The URL of this instance to be broadcast to other peers. If not specified, the
+ URL is determined from the `httpd.listenUrl` in the `gerrit.config`.
+ If `httpd.listenUrl` is configured with multiple values, is configured to work
+ with a reverse proxy (i.e. uses `proxy-http` or `proxy-https` scheme), or is
+ configured to listen on all local addresses (i.e. using hostname `*`), then
+ the URL must be explicitly specified with `myUrl`.
+
NOTE: To work properly in certain environments, JGroups needs the System property
`java.net.preferIPv4Stack` to be set to `true`.
See (http://jgroups.org/tutorial/index.html#_trouble_shooting).
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java
index a9982f3..b1d9fcc 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java
@@ -33,6 +33,7 @@
import static com.ericsson.gerrit.plugins.highavailability.Configuration.JGROUPS_SUBSECTION;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.MAIN_SECTION;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.MAX_TRIES_KEY;
+import static com.ericsson.gerrit.plugins.highavailability.Configuration.MY_URL_KEY;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.PASSWORD_KEY;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.PATTERN_KEY;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.PEER_INFO_SECTION;
@@ -48,11 +49,15 @@
import static com.ericsson.gerrit.plugins.highavailability.Configuration.USER_KEY;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.WEBSESSION_SECTION;
import static com.google.common.truth.Truth.assertThat;
+import static java.net.InetAddress.getLocalHost;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.ericsson.gerrit.plugins.highavailability.cache.CachePatternMatcher;
+import com.ericsson.gerrit.plugins.highavailability.peers.jgroups.MyUrlProvider;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.config.SitePaths;
@@ -62,7 +67,9 @@
import java.nio.file.Paths;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
@@ -86,10 +93,13 @@
@Mock private PluginConfigFactory cfgFactoryMock;
@Mock private Config configMock;
+ @Mock private Config serverConfigMock;
private SitePaths site;
private Configuration configuration;
private String pluginName = "high-availability";
+ @Rule public ExpectedException exception = ExpectedException.none();
+
@Before
public void setUp() throws IOException {
when(cfgFactoryMock.getGlobalPluginConfig(pluginName)).thenReturn(configMock);
@@ -172,6 +182,121 @@
}
@Test
+ public void testGetJGroupsMyUrl() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ initializeConfiguration();
+ assertThat(configuration.peerInfoJGroups().myUrl()).isNull();
+
+ when(configMock.getString(PEER_INFO_SECTION, JGROUPS_SUBSECTION, MY_URL_KEY)).thenReturn(URL);
+ initializeConfiguration();
+ assertThat(configuration.peerInfoJGroups().myUrl()).isEqualTo(URL);
+
+ when(configMock.getString(PEER_INFO_SECTION, JGROUPS_SUBSECTION, MY_URL_KEY))
+ .thenReturn(URL + "/");
+ initializeConfiguration();
+ assertThat(configuration.peerInfoJGroups().myUrl()).isEqualTo(URL);
+ }
+
+ @Test
+ public void testGetJGroupsMyUrlFromListenUrlWhenNoListenUrlSpecified() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ initializeConfiguration();
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl")).thenReturn(new String[] {});
+
+ exception.expect(ProvisionException.class);
+ exception.expectMessage("exactly 1 value configured; found 0");
+ getMyUrlProvider();
+ }
+
+ @Test
+ public void testGetJGroupsMyUrlFromListenUrlWhenMultipleListenUrlsSpecified() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ initializeConfiguration();
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl"))
+ .thenReturn(new String[] {"a", "b"});
+
+ exception.expect(ProvisionException.class);
+ exception.expectMessage("exactly 1 value configured; found 2");
+ getMyUrlProvider();
+ }
+
+ @Test
+ public void testGetJGroupsMyUrlFromListenUrlWhenReverseProxyConfigured() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ initializeConfiguration();
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl"))
+ .thenReturn(new String[] {"proxy-https://foo"});
+
+ exception.expect(ProvisionException.class);
+ exception.expectMessage("when configured as reverse-proxy");
+ getMyUrlProvider();
+ }
+
+ @Test
+ public void testGetJGroupsMyUrlFromListenUrlWhenWildcardConfigured() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ initializeConfiguration();
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl"))
+ .thenReturn(new String[] {"https://*"});
+
+ exception.expect(ProvisionException.class);
+ exception.expectMessage("when configured with wildcard");
+ getMyUrlProvider();
+ }
+
+ @Test
+ public void testGetJGroupsMyUrlOverridesListenUrl() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ when(configMock.getString(PEER_INFO_SECTION, JGROUPS_SUBSECTION, MY_URL_KEY)).thenReturn(URL);
+ initializeConfiguration();
+ assertThat(configuration.peerInfoJGroups().myUrl()).isEqualTo(URL);
+
+ verify(serverConfigMock, never()).getStringList("httpd", null, "listenUrl");
+ assertThat(getMyUrlProvider().get()).isEqualTo(URL);
+ }
+
+ @Test
+ public void testGetJGroupsMyUrlFromListenUrl() throws Exception {
+ when(configMock.getEnum(PEER_INFO_SECTION, null, STRATEGY_KEY, DEFAULT_PEER_INFO_STRATEGY))
+ .thenReturn(Configuration.PeerInfoStrategy.JGROUPS);
+ when(configMock.getString(PEER_INFO_SECTION, JGROUPS_SUBSECTION, MY_URL_KEY)).thenReturn(null);
+ initializeConfiguration();
+ assertThat(configuration.peerInfoJGroups().myUrl()).isNull();
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl"))
+ .thenReturn(new String[] {"https://foo:8080"});
+
+ String hostName = getLocalHost().getHostName();
+ String expected = "https://" + hostName + ":8080";
+ assertThat(getMyUrlProvider().get()).isEqualTo(expected);
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl"))
+ .thenReturn(new String[] {"https://foo"});
+
+ expected = "https://" + hostName;
+ assertThat(getMyUrlProvider().get()).isEqualTo(expected);
+
+ when(serverConfigMock.getStringList("httpd", null, "listenUrl"))
+ .thenReturn(new String[] {"https://foo/"});
+
+ assertThat(getMyUrlProvider().get()).isEqualTo(expected);
+ }
+
+ private MyUrlProvider getMyUrlProvider() {
+ return new MyUrlProvider(serverConfigMock, configuration);
+ }
+
+ @Test
public void testGetUser() throws Exception {
initializeConfiguration();
assertThat(configuration.http().user()).isEqualTo(EMPTY);
