xds: precise logic for selecting the virtual host in RDS responses (#6661)
Implements the precise logic for choosing the virtual host in RouteConfiguration of RDS responses. Specifically, fixes logic for domain search order. Minor fix for checking match field in RouteConfiguration. See RouteConfiguration Proto section in gRPC Client xDS API Flow design doc for specification.
diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java
index f1c8f4d..83340d0 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 @@
// 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 @@
// 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 @@
}
/**
- * 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.
- *
- * <p>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<VirtualHost> 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<Route> 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 738b510..55dccdd 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.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;
@@ -3197,6 +3199,111 @@
}
@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();
List<Any> listeners = ImmutableList.of(
@@ -3232,10 +3339,16 @@
+ " \"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 @@
+ " \"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 a31406d..6ef308f 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.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 @@
}
static VirtualHost buildVirtualHost(List<String> 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,
