Remove trailing slash in proxyTo and externalUrl

trinodb · Feb 7, 2025 · 91ea383 · 91ea383
1 parent bd2f513
commit 91ea383
Show file tree

Hide file tree

Showing 10 changed files with 68 additions and 21 deletions.
diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsHttpMonitor.java b/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsHttpMonitor.java
@@ -37,6 +37,7 @@
 import static io.trino.gateway.ha.handler.HttpUtils.UI_API_QUEUED_LIST_PATH;
 import static io.trino.gateway.ha.handler.HttpUtils.UI_API_STATS_PATH;
 import static io.trino.gateway.ha.handler.HttpUtils.UI_LOGIN_PATH;
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
 import static java.util.Objects.requireNonNull;
 
 public class ClusterStatsHttpMonitor
@@ -74,8 +75,8 @@ public ClusterStats monitor(ProxyBackendConfiguration backend)
                     .queuedQueryCount((int) result.get("queuedQueries"))
                     .runningQueryCount((int) result.get("runningQueries"))
                     .trinoStatus(activeWorkers > 0 ? TrinoStatus.HEALTHY : TrinoStatus.UNHEALTHY)
-                    .proxyTo(backend.getProxyTo())
-                    .externalUrl(backend.getExternalUrl())
+                    .proxyTo(removeTrailingSlash(backend.getProxyTo()))
+                    .externalUrl(removeTrailingSlash(backend.getExternalUrl()))
                     .routingGroup(backend.getRoutingGroup());
         }
         catch (Exception e) {
@@ -129,14 +130,14 @@ private OkHttpClient acquireClientWithCookie(String loginUrl)
 
     private String queryCluster(ProxyBackendConfiguration backend, String path)
     {
-        String loginUrl = backend.getProxyTo() + UI_LOGIN_PATH;
+        String loginUrl = removeTrailingSlash(backend.getProxyTo()) + UI_LOGIN_PATH;
         OkHttpClient client = acquireClientWithCookie(loginUrl);
         if (client == null) {
             log.error("Client received is null");
             return null;
         }
 
-        String targetUrl = backend.getProxyTo() + path;
+        String targetUrl = removeTrailingSlash(backend.getProxyTo()) + path;
         Request request = new Request.Builder()
                 .url(HttpUrl.parse(targetUrl))
                 .get()

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsInfoApiMonitor.java b/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsInfoApiMonitor.java
@@ -28,6 +28,7 @@
 import static io.airlift.http.client.Request.Builder.prepareGet;
 import static io.airlift.json.JsonCodec.jsonCodec;
 import static io.trino.gateway.ha.clustermonitor.MonitorUtils.shouldRetry;
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
 import static java.util.Objects.requireNonNull;
 
 public class ClusterStatsInfoApiMonitor
@@ -47,9 +48,11 @@ public ClusterStatsInfoApiMonitor(HttpClient client, MonitorConfiguration monito
     @Override
     public ClusterStats monitor(ProxyBackendConfiguration backend)
     {
-        return ClusterStats.builder(backend.getName()).trinoStatus(checkStatus(backend.getProxyTo()))
-                .proxyTo(backend.getProxyTo())
-                .externalUrl(backend.getExternalUrl())
+        String backendProxyToUrl = removeTrailingSlash(backend.getProxyTo());
+        String backendExternalUrl = removeTrailingSlash(backend.getExternalUrl());
+        return ClusterStats.builder(backend.getName()).trinoStatus(checkStatus(backendProxyToUrl))
+                .proxyTo(backendProxyToUrl)
+                .externalUrl(backendExternalUrl)
                 .routingGroup(backend.getRoutingGroup()).build();
     }
 

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java b/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java
@@ -32,6 +32,7 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeoutException;
 
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 public class ClusterStatsJdbcMonitor
@@ -65,7 +66,7 @@ public ClusterStatsJdbcMonitor(BackendStateConfiguration backendStateConfigurati
     @Override
     public ClusterStats monitor(ProxyBackendConfiguration backend)
     {
-        String url = backend.getProxyTo();
+        String url = removeTrailingSlash(backend.getProxyTo());
         ClusterStats.Builder clusterStats = ClusterStatsMonitor.getClusterStatsBuilder(backend);
         String jdbcUrl;
         try {

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsMonitor.java b/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsMonitor.java
@@ -15,15 +15,17 @@
 
 import io.trino.gateway.ha.config.ProxyBackendConfiguration;
 
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
+
 public interface ClusterStatsMonitor
 {
     ClusterStats monitor(ProxyBackendConfiguration backend);
 
     static ClusterStats.Builder getClusterStatsBuilder(ProxyBackendConfiguration backend)
     {
         ClusterStats.Builder builder = ClusterStats.builder(backend.getName());
-        builder.proxyTo(backend.getProxyTo());
-        builder.externalUrl(backend.getExternalUrl());
+        builder.proxyTo(removeTrailingSlash(backend.getProxyTo()));
+        builder.externalUrl(removeTrailingSlash(backend.getExternalUrl()));
         builder.routingGroup(backend.getRoutingGroup());
         return builder;
     }

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java b/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java
@@ -122,4 +122,9 @@ public static String buildUriWithNewBackend(String backendHost, HttpServletReque
     {
         return backendHost + request.getRequestURI() + (request.getQueryString() != null ? "?" + request.getQueryString() : "");
     }
+
+    public static String removeTrailingSlash(String url)
+    {
+        return url.replaceAll("/$", "");
+    }
 }
diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayViewResource.java b/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayViewResource.java
@@ -32,6 +32,7 @@
 import java.util.Map;
 import java.util.Optional;
 
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
 import static java.util.Objects.requireNonNull;
 
 @Path("/trino-gateway")
@@ -97,7 +98,7 @@ public Map<String, Integer> getQueryHistoryDistribution(@Context SecurityContext
                 .getAllBackends()
                 .forEach(
                         backend -> {
-                            urlToNameMap.put(backend.getProxyTo(), backend.getName());
+                            urlToNameMap.put(removeTrailingSlash(backend.getProxyTo()), backend.getName());
                         });
 
         Map<String, Integer> clusterToQueryCount = new HashMap<>();

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java b/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java
@@ -55,6 +55,7 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
 import static java.util.Objects.requireNonNull;
 import static java.util.Objects.requireNonNullElse;
 
@@ -95,7 +96,7 @@ public Response getAllBackends()
             backendResponse.setQueued(backendState.queuedQueryCount());
             backendResponse.setRunning(backendState.runningQueryCount());
             backendResponse.setName(b.getName());
-            backendResponse.setProxyTo(b.getProxyTo());
+            backendResponse.setProxyTo(removeTrailingSlash(b.getProxyTo()));
             backendResponse.setActive(b.isActive());
             backendResponse.setStatus(backendState.trinoStatus().toString());
             backendResponse.setRoutingGroup(b.getRoutingGroup());

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java b/gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java
@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Optional;
 
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
 import static java.util.Objects.requireNonNull;
 
 public class HaGatewayManager
@@ -94,19 +95,23 @@ public void activateBackend(String backendName)
     @Override
     public ProxyBackendConfiguration addBackend(ProxyBackendConfiguration backend)
     {
-        dao.create(backend.getName(), backend.getRoutingGroup(), backend.getProxyTo(), backend.getExternalUrl(), backend.isActive());
+        String backendProxyTo = removeTrailingSlash(backend.getProxyTo());
+        String backendExternalUrl = removeTrailingSlash(backend.getExternalUrl());
+        dao.create(backend.getName(), backend.getRoutingGroup(), backendProxyTo, backendExternalUrl, backend.isActive());
         return backend;
     }
 
     @Override
     public ProxyBackendConfiguration updateBackend(ProxyBackendConfiguration backend)
     {
+        String backendProxyTo = removeTrailingSlash(backend.getProxyTo());
+        String backendExternalUrl = removeTrailingSlash(backend.getExternalUrl());
         GatewayBackend model = dao.findFirstByName(backend.getName());
         if (model == null) {
-            dao.create(backend.getName(), backend.getRoutingGroup(), backend.getProxyTo(), backend.getExternalUrl(), backend.isActive());
+            dao.create(backend.getName(), backend.getRoutingGroup(), backendProxyTo, backendExternalUrl, backend.isActive());
         }
         else {
-            dao.update(backend.getName(), backend.getRoutingGroup(), backend.getProxyTo(), backend.getExternalUrl(), backend.isActive());
+            dao.update(backend.getName(), backend.getRoutingGroup(), backendProxyTo, backendExternalUrl, backend.isActive());
         }
         return backend;
     }

diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/router/RoutingManager.java b/gateway-ha/src/main/java/io/trino/gateway/ha/router/RoutingManager.java
@@ -35,6 +35,8 @@
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
+import static io.trino.gateway.ha.handler.ProxyUtils.removeTrailingSlash;
+
 /**
  * This class performs health check, stats counts for each backend and provides a backend given
  * request object. Default implementation comes here.
@@ -89,7 +91,7 @@ public String provideAdhocBackend(String user)
             throw new IllegalStateException("Number of active backends found zero");
         }
         int backendId = Math.abs(RANDOM.nextInt()) % backends.size();
-        return backends.get(backendId).getProxyTo();
+        return removeTrailingSlash(backends.get(backendId).getProxyTo());
     }
 
     /**
@@ -105,7 +107,7 @@ public String provideBackendForRoutingGroup(String routingGroup, String user)
             return provideAdhocBackend(user);
         }
         int backendId = Math.abs(RANDOM.nextInt()) % backends.size();
-        return backends.get(backendId).getProxyTo();
+        return removeTrailingSlash(backends.get(backendId).getProxyTo());
     }
 
     /**
@@ -144,11 +146,11 @@ public void updateBackEndStats(List<ClusterStats> stats)
     protected String findBackendForUnknownQueryId(String queryId)
     {
         List<ProxyBackendConfiguration> backends = gatewayBackendManager.getAllBackends();
-
         Map<String, Future<Integer>> responseCodes = new HashMap<>();
         try {
             for (ProxyBackendConfiguration backend : backends) {
-                String target = backend.getProxyTo() + "/v1/query/" + queryId;
+                String backendProxyToUrl = removeTrailingSlash(backend.getProxyTo());
+                String target = backendProxyToUrl + "/v1/query/" + queryId;
 
                 Future<Integer> call =
                         executorService.submit(
@@ -160,7 +162,7 @@ protected String findBackendForUnknownQueryId(String queryId)
                                     conn.setRequestMethod(HttpMethod.HEAD);
                                     return conn.getResponseCode();
                                 });
-                responseCodes.put(backend.getProxyTo(), call);
+                responseCodes.put(backendProxyToUrl, call);
             }
             for (Map.Entry<String, Future<Integer>> entry : responseCodes.entrySet()) {
                 if (entry.getValue().isDone()) {
@@ -177,7 +179,7 @@ protected String findBackendForUnknownQueryId(String queryId)
             log.warn("Query id [%s] not found", queryId);
         }
         // Fallback on first active backend if queryId mapping not found.
-        return gatewayBackendManager.getActiveAdhocBackends().get(0).getProxyTo();
+        return removeTrailingSlash(gatewayBackendManager.getActiveAdhocBackends().get(0).getProxyTo());
     }
 
     // Predicate helper function to remove the backends from the list

diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/router/TestHaGatewayManager.java b/gateway-ha/src/test/java/io/trino/gateway/ha/router/TestHaGatewayManager.java
@@ -83,4 +83,30 @@ void testGatewayManager()
                 .extracting(ProxyBackendConfiguration::getRoutingGroup)
                 .containsExactly("adhoc");
     }
+
+    @Test
+    void testRemoveTrailingSlashInUrl()
+    {
+        ProxyBackendConfiguration etl = new ProxyBackendConfiguration();
+        etl.setActive(false);
+        etl.setRoutingGroup("etl");
+        etl.setName("new-etl1");
+        etl.setProxyTo("https://etl1.trino.gateway.io:443/");
+        etl.setExternalUrl("https://etl1.trino.gateway.io:443/");
+        haGatewayManager.addBackend(etl);
+
+        assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
+        assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getExternalUrl).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
+
+        ProxyBackendConfiguration etl2 = new ProxyBackendConfiguration();
+        etl2.setActive(false);
+        etl2.setRoutingGroup("etl2");
+        etl2.setName("new-etl1");
+        etl2.setProxyTo("https://etl2.trino.gateway.io:443/");
+        etl2.setExternalUrl("https://etl2.trino.gateway.io:443/");
+        haGatewayManager.updateBackend(etl2);
+
+        assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl2.trino.gateway.io:443");
+        assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getExternalUrl).orElseThrow()).isEqualTo("https://etl2.trino.gateway.io:443");
+    }
 }