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); + } +}