diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index f1c8f4d516..83340d070b 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -508,7 +508,7 @@ final class XdsClientImpl extends XdsClient { // data or one supersedes the other. TBD. if (requestedHttpConnManager.hasRouteConfig()) { RouteConfiguration rc = requestedHttpConnManager.getRouteConfig(); - clusterName = processRouteConfig(rc); + clusterName = findClusterNameInRouteConfig(rc, hostName); if (clusterName == null) { errorMessage = "Cannot find a valid cluster name in VirtualHost inside " + "RouteConfiguration with domains matching: " + hostName + "."; @@ -599,7 +599,7 @@ final class XdsClientImpl extends XdsClient { // Resolved cluster name for the requested resource, if exists. String clusterName = null; if (requestedRouteConfig != null) { - clusterName = processRouteConfig(requestedRouteConfig); + clusterName = findClusterNameInRouteConfig(requestedRouteConfig, hostName); if (clusterName == null) { adsStream.sendNackRequest(ADS_TYPE_URL_RDS, ImmutableList.of(adsStream.rdsResourceName), "Cannot find a valid cluster name in VirtualHost inside " @@ -624,38 +624,66 @@ final class XdsClientImpl extends XdsClient { } /** - * Processes RouteConfiguration message (from an resource information in an LDS or RDS - * response), which may contain a VirtualHost with domains matching the "xds:" - * URI hostname directly in-line. Returns the clusterName found in that VirtualHost - * message. Returns {@code null} if such a clusterName cannot be resolved. - * - *

Note we only validate VirtualHosts with domains matching the "xds:" URI hostname. + * Processes a RouteConfiguration message to find the name of upstream cluster that requests + * for the given host will be routed to. Returns the clusterName if found. + * Otherwise, returns {@code null}. */ + @VisibleForTesting @Nullable - private String processRouteConfig(RouteConfiguration config) { + static String findClusterNameInRouteConfig(RouteConfiguration config, String hostName) { List virtualHosts = config.getVirtualHostsList(); - int matchingLen = -1; // longest length of wildcard pattern that matches host name + // Domain search order: + // 1. Exact domain names: ``www.foo.com``. + // 2. Suffix domain wildcards: ``*.foo.com`` or ``*-bar.foo.com``. + // 3. Prefix domain wildcards: ``foo.*`` or ``foo-*``. + // 4. Special wildcard ``*`` matching any domain. + // + // The longest wildcards match first. + // Assuming only a single virtual host in the entire route configuration can match + // on ``*`` and a domain must be unique across all virtual hosts. + int matchingLen = -1; // longest length of wildcard pattern that matches host name + boolean exactMatchFound = false; // true if a virtual host with exactly matched domain found VirtualHost targetVirtualHost = null; // target VirtualHost with longest matched domain for (VirtualHost vHost : virtualHosts) { for (String domain : vHost.getDomainsList()) { - if (matchHostName(hostName, domain) && domain.length() > matchingLen) { + boolean selected = false; + if (matchHostName(hostName, domain)) { // matching + if (!domain.contains("*")) { // exact matching + exactMatchFound = true; + targetVirtualHost = vHost; + break; + } else if (domain.length() > matchingLen) { // longer matching pattern + selected = true; + } else if (domain.length() == matchingLen && domain.startsWith("*")) { // suffix matching + selected = true; + } + } + if (selected) { matchingLen = domain.length(); targetVirtualHost = vHost; } } + if (exactMatchFound) { + break; + } } // Proceed with the virtual host that has longest wildcard matched domain name with the // hostname in original "xds:" URI. + // Note we would consider upstream cluster not found if the virtual host is not configured + // correctly for gRPC, even if there exist other virtual hosts with (lower priority) + // matching domains. if (targetVirtualHost != null) { // The client will look only at the last route in the list (the default route), - // whose match field must be empty and whose route field must be set. + // whose match field must contain a prefix field whose value is empty string + // and whose route field must be set. List routes = targetVirtualHost.getRoutesList(); if (!routes.isEmpty()) { Route route = routes.get(routes.size() - 1); - // TODO(chengyuanzhang): check the match field must be empty. - if (route.hasRoute()) { - return route.getRoute().getCluster(); + if (route.getMatch().getPrefix().equals("")) { + if (route.hasRoute()) { + return route.getRoute().getCluster(); + } } } } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java index 738b510af8..55dccdd6fe 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java @@ -58,6 +58,8 @@ import io.envoyproxy.envoy.api.v2.core.Node; import io.envoyproxy.envoy.api.v2.endpoint.ClusterStats; import io.envoyproxy.envoy.api.v2.route.RedirectAction; import io.envoyproxy.envoy.api.v2.route.Route; +import io.envoyproxy.envoy.api.v2.route.RouteAction; +import io.envoyproxy.envoy.api.v2.route.RouteMatch; import io.envoyproxy.envoy.api.v2.route.VirtualHost; import io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager; import io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2.Rds; @@ -3196,6 +3198,111 @@ public class XdsClientImplTest { assertThat(XdsClientImpl.matchHostName("foo-bar", pattern)).isTrue(); } + @Test + public void findClusterNameInRouteConfig_exactMatchFirst() { + String hostname = "a.googleapis.com"; + String targetClusterName = "cluster-hello.googleapis.com"; + VirtualHost vHost1 = + VirtualHost.newBuilder() + .setName("virtualhost01.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("a.googleapis.com", "b.googleapis.com")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster(targetClusterName)) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + VirtualHost vHost2 = + VirtualHost.newBuilder() + .setName("virtualhost02.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("*.googleapis.com")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster-hi.googleapis.com")) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + VirtualHost vHost3 = + VirtualHost.newBuilder() + .setName("virtualhost03.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("*")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster-hey.googleapis.com")) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + RouteConfiguration routeConfig = + buildRouteConfiguration( + "route-foo.googleapis.com", ImmutableList.of(vHost1, vHost2, vHost3)); + String result = XdsClientImpl.findClusterNameInRouteConfig(routeConfig, hostname); + assertThat(result).isEqualTo(targetClusterName); + } + + @Test + public void findClusterNameInRouteConfig_preferSuffixDomainOverPrefixDomain() { + String hostname = "a.googleapis.com"; + String targetClusterName = "cluster-hello.googleapis.com"; + VirtualHost vHost1 = + VirtualHost.newBuilder() + .setName("virtualhost01.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("*.googleapis.com", "b.googleapis.com")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster(targetClusterName)) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + VirtualHost vHost2 = + VirtualHost.newBuilder() + .setName("virtualhost02.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("a.googleapis.*")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster-hi.googleapis.com")) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + VirtualHost vHost3 = + VirtualHost.newBuilder() + .setName("virtualhost03.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("*")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster-hey.googleapis.com")) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + RouteConfiguration routeConfig = + buildRouteConfiguration( + "route-foo.googleapis.com", ImmutableList.of(vHost1, vHost2, vHost3)); + String result = XdsClientImpl.findClusterNameInRouteConfig(routeConfig, hostname); + assertThat(result).isEqualTo(targetClusterName); + } + + @Test + public void findClusterNameInRouteConfig_asteriskMatchAnyDomain() { + String hostname = "a.googleapis.com"; + String targetClusterName = "cluster-hello.googleapis.com"; + VirtualHost vHost1 = + VirtualHost.newBuilder() + .setName("virtualhost01.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("*")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster(targetClusterName)) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + VirtualHost vHost2 = + VirtualHost.newBuilder() + .setName("virtualhost02.googleapis.com") // don't care + .addAllDomains(ImmutableList.of("b.googleapis.com")) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster-hi.googleapis.com")) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); + RouteConfiguration routeConfig = + buildRouteConfiguration( + "route-foo.googleapis.com", ImmutableList.of(vHost1, vHost2)); + String result = XdsClientImpl.findClusterNameInRouteConfig(routeConfig, hostname); + assertThat(result).isEqualTo(targetClusterName); + } + @Test public void messagePrinter_printLdsResponse() { MessagePrinter printer = new MessagePrinter(); @@ -3232,10 +3339,16 @@ public class XdsClientImplTest { + " \"name\": \"virtualhost00.googleapis.com\",\n" + " \"domains\": [\"foo.googleapis.com\", \"bar.googleapis.com\"],\n" + " \"routes\": [{\n" + + " \"match\": {\n" + + " \"prefix\": \"\"\n" + + " },\n" + " \"route\": {\n" + " \"cluster\": \"whatever cluster\"\n" + " }\n" + " }, {\n" + + " \"match\": {\n" + + " \"prefix\": \"\"\n" + + " },\n" + " \"route\": {\n" + " \"cluster\": \"cluster.googleapis.com\"\n" + " }\n" @@ -3276,10 +3389,16 @@ public class XdsClientImplTest { + " \"name\": \"virtualhost00.googleapis.com\",\n" + " \"domains\": [\"foo.googleapis.com\", \"bar.googleapis.com\"],\n" + " \"routes\": [{\n" + + " \"match\": {\n" + + " \"prefix\": \"\"\n" + + " },\n" + " \"route\": {\n" + " \"cluster\": \"whatever cluster\"\n" + " }\n" + " }, {\n" + + " \"match\": {\n" + + " \"prefix\": \"\"\n" + + " },\n" + " \"route\": {\n" + " \"cluster\": \"cluster.googleapis.com\"\n" + " }\n" diff --git a/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java index a31406d6a7..6ef308f0e3 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java @@ -45,6 +45,7 @@ import io.envoyproxy.envoy.api.v2.core.SocketAddress; import io.envoyproxy.envoy.api.v2.listener.FilterChain; import io.envoyproxy.envoy.api.v2.route.Route; import io.envoyproxy.envoy.api.v2.route.RouteAction; +import io.envoyproxy.envoy.api.v2.route.RouteMatch; import io.envoyproxy.envoy.api.v2.route.VirtualHost; import io.envoyproxy.envoy.config.listener.v2.ApiListener; import io.envoyproxy.envoy.type.FractionalPercent; @@ -104,17 +105,19 @@ class XdsClientTestHelper { } static VirtualHost buildVirtualHost(List domains, String clusterName) { - return - VirtualHost.newBuilder() - .setName("virtualhost00.googleapis.com") // don't care - .addAllDomains(domains) - .addRoutes(Route.newBuilder() - .setRoute(RouteAction.newBuilder().setCluster("whatever cluster"))) - .addRoutes( - // Only the last (default) route matters. - Route.newBuilder() - .setRoute(RouteAction.newBuilder().setCluster(clusterName))) - .build(); + return VirtualHost.newBuilder() + .setName("virtualhost00.googleapis.com") // don't care + .addAllDomains(domains) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("whatever cluster")) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .addRoutes( + // Only the last (default) route matters. + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster(clusterName)) + .setMatch(RouteMatch.newBuilder().setPrefix(""))) + .build(); } static Cluster buildCluster(String clusterName, @Nullable String edsServiceName,