Support Gerrit Replicas listening to a URL with a URI prefix
The healthcheck plugin was failing to serve the healthcheck in a Gerrit
replica that had a URI prefix defined in the listenUrl, e.g.
https://*:8080/gerrit/. This was because the filter checking for the
healthcheck endpoint did not match the URI in that case.
Change-Id: I778c9cc2805a983a1d952d50ae6fda66a684e5e3
diff --git a/BUILD b/BUILD
index 5077015..1c88192 100644
--- a/BUILD
+++ b/BUILD
@@ -29,6 +29,7 @@
resources = glob(["src/test/resources/**/*"]),
deps = [
":healthcheck__plugin_test_deps",
+ "//javatests/com/google/gerrit/util/http/testutil",
],
)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java
index a6f33ea..1b52d4c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java
@@ -14,17 +14,21 @@
package com.googlesource.gerrit.plugins.healthcheck.filter;
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.httpd.AllRequestFilter;
import com.google.gerrit.httpd.restapi.RestApiServlet;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.config.ConfigResource;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gson.Gson;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.healthcheck.api.HealthCheckStatusEndpoint;
import java.io.IOException;
import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
@@ -32,18 +36,65 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.lib.Config;
public class HealthCheckStatusFilter extends AllRequestFilter {
private final HealthCheckStatusEndpoint statusEndpoint;
private final Gson gson;
private final String pluginName;
+ private final Config cfg;
+ private final String uriPattern;
@Inject
public HealthCheckStatusFilter(
- HealthCheckStatusEndpoint statusEndpoint, @PluginName String pluginName) {
+ HealthCheckStatusEndpoint statusEndpoint,
+ @PluginName String pluginName,
+ @GerritServerConfig Config cfg) {
this.statusEndpoint = statusEndpoint;
this.gson = OutputFormat.JSON.newGsonBuilder().create();
this.pluginName = pluginName;
+ this.cfg = cfg;
+ this.uriPattern = getUriPattern();
+ }
+
+ private static List<String> extractUriPrefixes(String[] listenUrls) {
+ List<String> prefixes = new ArrayList<>();
+ if (listenUrls.length == 0) {
+ return prefixes;
+ }
+ for (String listenUrl : listenUrls) {
+ String[] parts = listenUrl.split("/", 4);
+ if (parts.length < 4) {
+ continue;
+ }
+ String uriPrefix = parts[3];
+ if (uriPrefix.endsWith("/")) {
+ uriPrefix = uriPrefix.substring(0, uriPrefix.length() - 1);
+ }
+ prefixes.add(uriPrefix);
+ }
+ return prefixes;
+ }
+
+ private String getUriPattern() {
+ List<String> uriPrefixes = extractUriPrefixes(cfg.getStringList("httpd", null, "listenUrl"));
+ String prefixPattern = "";
+ if (uriPrefixes.size() == 1 && !uriPrefixes.get(0).isEmpty()) {
+ prefixPattern = "(?:/" + uriPrefixes.get(0) + ")";
+ } else if (uriPrefixes.size() > 1) {
+ prefixPattern = "(?:/";
+ for (String prefix : uriPrefixes) {
+ if (!prefix.isEmpty()) {
+ prefixPattern += prefix + "|";
+ }
+ }
+ prefixPattern = prefixPattern.substring(0, prefixPattern.length() - 1);
+ prefixPattern += ")";
+ if (uriPrefixes.contains("")) {
+ prefixPattern += "?";
+ }
+ }
+ return prefixPattern + "(?:/a)?/config/server/" + pluginName + "~status";
}
@Override
@@ -64,10 +115,9 @@
}
}
- private boolean isStatusCheck(HttpServletRequest httpServletRequest) {
- return httpServletRequest
- .getRequestURI()
- .matches("(?:/a)?/config/server/" + pluginName + "~status");
+ @VisibleForTesting
+ protected boolean isStatusCheck(HttpServletRequest httpServletRequest) {
+ return httpServletRequest.getRequestURI().matches(uriPattern);
}
private void doStatusCheck(HttpServletResponse httpResponse) throws ServletException {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilterTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilterTest.java
new file mode 100644
index 0000000..9d44f21
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilterTest.java
@@ -0,0 +1,67 @@
+// Copyright (C) 2024 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.googlesource.gerrit.plugins.healthcheck.filter;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class HealthCheckStatusFilterTest {
+ private static final String HEALTHCHECK_PATH = "/config/server/healthcheck~status";
+
+ @Test
+ public void statusRequestsShouldBeHandled() {
+ HealthCheckStatusFilter filterWithoutPrefix = createFilter(List.of("http://*:8080/"));
+ assertThat(filterWithoutPrefix.isStatusCheck(createRequest(HEALTHCHECK_PATH))).isTrue();
+ assertThat(filterWithoutPrefix.isStatusCheck(createRequest("/a" + HEALTHCHECK_PATH))).isTrue();
+ assertThat(filterWithoutPrefix.isStatusCheck(createRequest("/a/config/server/version")))
+ .isFalse();
+
+ String prefix = "/gerrit";
+ HealthCheckStatusFilter filterWithPrefix =
+ createFilter(List.of("http://*:8080" + prefix + "/"));
+ assertThat(filterWithPrefix.isStatusCheck(createRequest(HEALTHCHECK_PATH))).isFalse();
+ assertThat(filterWithPrefix.isStatusCheck(createRequest("/a" + HEALTHCHECK_PATH))).isFalse();
+ assertThat(filterWithPrefix.isStatusCheck(createRequest(prefix + HEALTHCHECK_PATH))).isTrue();
+ assertThat(filterWithPrefix.isStatusCheck(createRequest(prefix + "/a" + HEALTHCHECK_PATH)))
+ .isTrue();
+ assertThat(filterWithPrefix.isStatusCheck(createRequest(prefix + "/a/config/server/version")))
+ .isFalse();
+
+ HealthCheckStatusFilter filterWithPrefixes =
+ createFilter(List.of("http://*:8080/", "http://*:8080" + prefix + "/"));
+ assertThat(filterWithPrefixes.isStatusCheck(createRequest(HEALTHCHECK_PATH))).isTrue();
+ assertThat(filterWithPrefixes.isStatusCheck(createRequest("/a" + HEALTHCHECK_PATH))).isTrue();
+ assertThat(filterWithPrefixes.isStatusCheck(createRequest(prefix + HEALTHCHECK_PATH))).isTrue();
+ assertThat(filterWithPrefixes.isStatusCheck(createRequest(prefix + "/a" + HEALTHCHECK_PATH)))
+ .isTrue();
+ assertThat(filterWithPrefixes.isStatusCheck(createRequest(prefix + "/a/config/server/version")))
+ .isFalse();
+ }
+
+ private HealthCheckStatusFilter createFilter(List<String> listenUrl) {
+ Config cfg = new Config();
+ cfg.setStringList("httpd", null, "listenUrl", listenUrl);
+ return new HealthCheckStatusFilter(null, "healthcheck", cfg);
+ }
+
+ private HttpServletRequest createRequest(String path) {
+ return new FakeHttpServletRequest("gerrit.example.com", 8080, "", "").setPathInfo(path);
+ }
+}
