diff --git a/xds/build.gradle b/xds/build.gradle index d594b2e9d0..d86750929b 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -24,7 +24,8 @@ dependencies { project(':grpc-core'), project(':grpc-services'), project(path: ':grpc-alts', configuration: 'shadow'), - libraries.gson + libraries.gson, + libraries.re2j def nettyDependency = implementation project(':grpc-netty') implementation (libraries.opencensus_proto) { diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index 2bda30a8ed..7e1cd803c5 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -16,9 +16,14 @@ package io.grpc.xds; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; +import com.google.common.base.MoreObjects.ToStringHelper; import com.google.common.collect.ImmutableList; +import com.google.re2j.Pattern; +import com.google.re2j.PatternSyntaxException; import io.envoyproxy.envoy.type.FractionalPercent; import io.envoyproxy.envoy.type.FractionalPercent.DenominatorType; import io.grpc.EquivalentAddressGroup; @@ -38,6 +43,10 @@ import javax.annotation.Nullable; * *

For data types that need to be sent as protobuf messages, a {@code toEnvoyProtoXXX} instance * method is defined to convert an instance to Envoy proto message. + * + *

Data conversion should follow the invariant: converted data is guaranteed to be valid for + * gRPC. If the protobuf message contains invalid data, the conversion should fail and no object + * should be instantiated. */ final class EnvoyProtoData { @@ -45,6 +54,83 @@ final class EnvoyProtoData { private EnvoyProtoData() { } + static final class StructOrError { + + /** + * Returns a {@link StructOrError} for the successfully converted data object. + */ + static StructOrError fromStruct(T struct) { + return new StructOrError<>(struct); + } + + /** + * Returns a {@link StructOrError} for the failure to convert the data object. + */ + static StructOrError fromError(String errorDetail) { + return new StructOrError<>(errorDetail); + } + + private final String errorDetail; + private final T struct; + + private StructOrError(T struct) { + this.struct = checkNotNull(struct, "struct"); + this.errorDetail = null; + } + + private StructOrError(String errorDetail) { + this.struct = null; + this.errorDetail = checkNotNull(errorDetail, "errorDetail"); + } + + /** + * Returns struct if exists, otherwise null. + */ + @Nullable + public T getStruct() { + return struct; + } + + /** + * Returns error detail if exists, otherwise null. + */ + @Nullable + String getErrorDetail() { + return errorDetail; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + StructOrError that = (StructOrError) o; + return Objects.equals(errorDetail, that.errorDetail) && Objects.equals(struct, that.struct); + } + + @Override + public int hashCode() { + return Objects.hash(errorDetail, struct); + } + + @Override + public String toString() { + if (struct != null) { + return MoreObjects.toStringHelper(this) + .add("struct", struct) + .toString(); + } else { + assert errorDetail != null; + return MoreObjects.toStringHelper(this) + .add("error", errorDetail) + .toString(); + } + } + } + /** * See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.core.Locality}. */ @@ -342,7 +428,6 @@ final class EnvoyProtoData { /** See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.route.Route}. */ static final class Route { private final RouteMatch routeMatch; - @Nullable private final RouteAction routeAction; @VisibleForTesting @@ -355,11 +440,14 @@ final class EnvoyProtoData { return routeMatch; } - @Nullable RouteAction getRouteAction() { return routeAction; } + boolean isDefaultRoute() { + return routeMatch.isMatchAll(); + } + @Override public boolean equals(Object o) { if (this == o) { @@ -386,55 +474,96 @@ final class EnvoyProtoData { .toString(); } - static Route fromEnvoyProtoRoute(io.envoyproxy.envoy.api.v2.route.Route proto) { - RouteMatch routeMatch = RouteMatch.fromEnvoyProtoRouteMatch(proto.getMatch()); - RouteAction routeAction = null; - if (proto.hasRoute()) { - routeAction = RouteAction.fromEnvoyProtoRouteAction(proto.getRoute()); + @Nullable + static StructOrError fromEnvoyProtoRoute(io.envoyproxy.envoy.api.v2.route.Route proto) { + StructOrError routeMatch = RouteMatch.fromEnvoyProtoRouteMatch(proto.getMatch()); + if (routeMatch == null) { + return null; } - return new Route(routeMatch, routeAction); + if (routeMatch.getErrorDetail() != null) { + return StructOrError.fromError( + "Invalid route [" + proto.getName() + "]: " + routeMatch.getErrorDetail()); + } + + StructOrError routeAction; + switch (proto.getActionCase()) { + case ROUTE: + routeAction = RouteAction.fromEnvoyProtoRouteAction(proto.getRoute()); + break; + case REDIRECT: + return StructOrError.fromError("Unsupported action type: redirect"); + case DIRECT_RESPONSE: + return StructOrError.fromError("Unsupported action type: direct_response"); + case FILTER_ACTION: + return StructOrError.fromError("Unsupported action type: filter_action"); + case ACTION_NOT_SET: + default: + return StructOrError.fromError("Unknown action type: " + proto.getActionCase()); + } + if (routeAction.getErrorDetail() != null) { + return StructOrError.fromError( + "Invalid route [" + proto.getName() + "]: " + routeAction.getErrorDetail()); + } + return StructOrError.fromStruct(new Route(routeMatch.getStruct(), routeAction.getStruct())); } } /** See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.route.RouteMatch}. */ static final class RouteMatch { - private final String prefix; - private final String path; - private final boolean hasRegex; - private final boolean caseSensitive; + // Exactly one of the following fields is non-null. + @Nullable + private final String pathPrefixMatch; + @Nullable + private final String pathExactMatch; + @Nullable + private final Pattern pathSafeRegExMatch; + + private final List headerMatchers; + @Nullable + private final Fraction fractionMatch; @VisibleForTesting - RouteMatch(String prefix, String path, boolean hasRegex, boolean caseSensitive) { - this.prefix = prefix; - this.path = path; - this.hasRegex = hasRegex; - this.caseSensitive = caseSensitive; + RouteMatch( + @Nullable String pathPrefixMatch, @Nullable String pathExactMatch, + @Nullable Pattern pathSafeRegExMatch, @Nullable Fraction fractionMatch, + List headerMatchers) { + this.pathPrefixMatch = pathPrefixMatch; + this.pathExactMatch = pathExactMatch; + this.pathSafeRegExMatch = pathSafeRegExMatch; + this.fractionMatch = fractionMatch; + this.headerMatchers = headerMatchers; } - String getPrefix() { - return prefix; + RouteMatch(@Nullable String pathPrefixMatch, @Nullable String pathExactMatch) { + this( + pathPrefixMatch, pathExactMatch, null, null, + Collections.emptyList()); } - String getPath() { - return path; + @Nullable + String getPathPrefixMatch() { + return pathPrefixMatch; } - boolean hasRegex() { - return hasRegex; + @Nullable + String getPathExactMatch() { + return pathExactMatch; } - boolean isCaseSensitive() { - return caseSensitive; - } - - boolean isDefaultMatcher() { - if (hasRegex) { + boolean isMatchAll() { + if (pathSafeRegExMatch != null || fractionMatch != null) { return false; } - if (!path.isEmpty()) { + if (!headerMatchers.isEmpty()) { return false; } - return prefix.isEmpty() || prefix.equals("/"); + if (pathExactMatch != null) { + return false; + } + if (pathPrefixMatch != null) { + return pathPrefixMatch.isEmpty() || pathPrefixMatch.equals("/"); + } + return false; } @Override @@ -446,62 +575,388 @@ final class EnvoyProtoData { return false; } RouteMatch that = (RouteMatch) o; - return hasRegex == that.hasRegex - && caseSensitive == that.caseSensitive - && Objects.equals(prefix, that.prefix) - && Objects.equals(path, that.path); + return Objects.equals(pathPrefixMatch, that.pathPrefixMatch) + && Objects.equals(pathExactMatch, that.pathExactMatch) + && Objects.equals( + pathSafeRegExMatch == null ? null : pathSafeRegExMatch.pattern(), + that.pathSafeRegExMatch == null ? null : that.pathSafeRegExMatch.pattern()) + && Objects.equals(fractionMatch, that.fractionMatch) + && Objects.equals(headerMatchers, that.headerMatchers); } @Override public int hashCode() { - return Objects.hash(prefix, path, caseSensitive, hasRegex); + return Objects.hash( + pathPrefixMatch, pathExactMatch, + pathSafeRegExMatch == null ? null : pathSafeRegExMatch.pattern(), headerMatchers, + fractionMatch); } @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("prefix", prefix) - .add("path", path) - .add("hasRegex", hasRegex) - .add("caseSensitive", caseSensitive) - .toString(); + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this); + if (pathPrefixMatch != null) { + toStringHelper.add("pathPrefixMatch", pathPrefixMatch); + } + if (pathExactMatch != null) { + toStringHelper.add("pathExactMatch", pathExactMatch); + } + if (pathSafeRegExMatch != null) { + toStringHelper.add("pathSafeRegExMatch",pathSafeRegExMatch.pattern()); + } + if (fractionMatch != null) { + toStringHelper.add("fractionMatch", fractionMatch); + } + return toStringHelper.add("headerMatchers", headerMatchers).toString(); } @VisibleForTesting - static RouteMatch fromEnvoyProtoRouteMatch( + @SuppressWarnings("deprecation") + @Nullable + static StructOrError fromEnvoyProtoRouteMatch( io.envoyproxy.envoy.api.v2.route.RouteMatch proto) { - return new RouteMatch( - /* prefix= */ proto.getPrefix(), - /* path= */ proto.getPath(), - /* hasRegex= */ !proto.getRegex().isEmpty() || proto.hasSafeRegex(), - // case_sensitive defaults to true if the field is not set - /*caseSensitive= */ !proto.hasCaseSensitive() || proto.getCaseSensitive().getValue()); + if (proto.getQueryParametersCount() != 0) { + return null; + } + if (proto.hasCaseSensitive() && !proto.getCaseSensitive().getValue()) { + return StructOrError.fromError("Unsupported match option: case insensitive"); + } + + Fraction fraction = null; + if (proto.hasRuntimeFraction()) { + io.envoyproxy.envoy.type.FractionalPercent percent = + proto.getRuntimeFraction().getDefaultValue(); + int numerator = percent.getNumerator(); + int denominator = 0; + switch (percent.getDenominator()) { + case HUNDRED: + denominator = 100; + break; + case TEN_THOUSAND: + denominator = 10_000; + break; + case MILLION: + denominator = 1_000_000; + break; + case UNRECOGNIZED: + default: + return StructOrError.fromError( + "Unrecognized fractional percent denominator: " + percent.getDenominator()); + } + fraction = new Fraction(numerator, denominator); + } + + String prefixPathMatch = null; + String exactPathMatch = null; + Pattern safeRegExPathMatch = null; + switch (proto.getPathSpecifierCase()) { + case PREFIX: + prefixPathMatch = proto.getPrefix(); + // Supported prefix match format: + // "", "/" (default) + // "/service/" + if (!prefixPathMatch.isEmpty() && !prefixPathMatch.equals("/")) { + if (!prefixPathMatch.startsWith("/") || !prefixPathMatch.endsWith("/") + || prefixPathMatch.length() < 3) { + return StructOrError.fromError( + "Invalid format of prefix path match: " + prefixPathMatch); + } + } + break; + case PATH: + exactPathMatch = proto.getPath(); + int lastSlash = exactPathMatch.lastIndexOf('/'); + // Supported exact match format: + // "/service/method" + if (!exactPathMatch.startsWith("/") || lastSlash == 0 + || lastSlash == exactPathMatch.length() - 1) { + return StructOrError.fromError( + "Invalid format of exact path match: " + exactPathMatch); + } + break; + case REGEX: + return StructOrError.fromError("Unsupported path match type: regex"); + case SAFE_REGEX: + String rawPattern = proto.getSafeRegex().getRegex(); + try { + safeRegExPathMatch = Pattern.compile(rawPattern); + } catch (PatternSyntaxException e) { + return StructOrError.fromError("Malformed safe regex pattern: " + e.getMessage()); + } + break; + case PATHSPECIFIER_NOT_SET: + default: + return StructOrError.fromError("Unknown path match type"); + } + + List headerMatchers = new ArrayList<>(); + for (io.envoyproxy.envoy.api.v2.route.HeaderMatcher hmProto : proto.getHeadersList()) { + StructOrError headerMatcher = + HeaderMatcher.fromEnvoyProtoHeaderMatcher(hmProto); + if (headerMatcher.getErrorDetail() != null) { + return StructOrError.fromError(headerMatcher.getErrorDetail()); + } + headerMatchers.add(headerMatcher.getStruct()); + } + + return StructOrError.fromStruct( + new RouteMatch( + prefixPathMatch, exactPathMatch, safeRegExPathMatch, fraction, + Collections.unmodifiableList(headerMatchers))); + } + + static final class Fraction { + private final int numerator; + private final int denominator; + + @VisibleForTesting + Fraction(int numerator, int denominator) { + this.numerator = numerator; + this.denominator = denominator; + } + + @Override + public int hashCode() { + return Objects.hash(numerator, denominator); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Fraction that = (Fraction) o; + return Objects.equals(numerator, that.numerator) + && Objects.equals(denominator, that.denominator); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("numerator", numerator) + .add("denominator", denominator) + .toString(); + } + } + } + + /** + * See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.route.HeaderMatcher}. + */ + @SuppressWarnings("unused") + static final class HeaderMatcher { + private final String name; + + // Exactly one of the following fields is non-null. + @Nullable + private final String exactMatch; + @Nullable + private final Pattern safeRegExMatch; + @Nullable + private final Range rangeMatch; + @Nullable + private final Boolean presentMatch; + @Nullable + private final String prefixMatch; + @Nullable + private final String suffixMatch; + + private final boolean isInvertedMatch; + + @VisibleForTesting + HeaderMatcher( + String name, + @Nullable String exactMatch, @Nullable Pattern safeRegExMatch, @Nullable Range rangeMatch, + @Nullable Boolean presentMatch, @Nullable String prefixMatch, @Nullable String suffixMatch, + boolean isInvertedMatch) { + this.name = name; + this.exactMatch = exactMatch; + this.safeRegExMatch = safeRegExMatch; + this.rangeMatch = rangeMatch; + this.presentMatch = presentMatch; + this.prefixMatch = prefixMatch; + this.suffixMatch = suffixMatch; + this.isInvertedMatch = isInvertedMatch; + } + + // TODO (chengyuanzhang): add getters when needed. + + @VisibleForTesting + @SuppressWarnings("deprecation") + static StructOrError fromEnvoyProtoHeaderMatcher( + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto) { + String exactMatch = null; + Pattern safeRegExMatch = null; + Range rangeMatch = null; + Boolean presentMatch = null; + String prefixMatch = null; + String suffixMatch = null; + + switch (proto.getHeaderMatchSpecifierCase()) { + case EXACT_MATCH: + exactMatch = proto.getExactMatch(); + break; + case REGEX_MATCH: + return StructOrError.fromError( + "HeaderMatcher [" + proto.getName() + "] has unsupported match type: regex"); + case SAFE_REGEX_MATCH: + String rawPattern = proto.getSafeRegexMatch().getRegex(); + try { + safeRegExMatch = Pattern.compile(rawPattern); + } catch (PatternSyntaxException e) { + return StructOrError.fromError( + "HeaderMatcher [" + proto.getName() + "] contains malformed safe regex pattern: " + + e.getMessage()); + } + break; + case RANGE_MATCH: + rangeMatch = new Range(proto.getRangeMatch().getStart(), proto.getRangeMatch().getEnd()); + break; + case PRESENT_MATCH: + presentMatch = proto.getPresentMatch(); + break; + case PREFIX_MATCH: + prefixMatch = proto.getPrefixMatch(); + break; + case SUFFIX_MATCH: + suffixMatch = proto.getSuffixMatch(); + break; + case HEADERMATCHSPECIFIER_NOT_SET: + default: + return StructOrError.fromError("Unknown header matcher type"); + } + return StructOrError.fromStruct( + new HeaderMatcher( + proto.getName(), exactMatch, safeRegExMatch, rangeMatch, presentMatch, + prefixMatch, suffixMatch, proto.getInvertMatch())); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + HeaderMatcher that = (HeaderMatcher) o; + return Objects.equals(name, that.name) + && Objects.equals(exactMatch, that.exactMatch) + && Objects.equals( + safeRegExMatch == null ? null : safeRegExMatch.pattern(), + that.safeRegExMatch == null ? null : that.safeRegExMatch.pattern()) + && Objects.equals(rangeMatch, that.rangeMatch) + && Objects.equals(presentMatch, that.presentMatch) + && Objects.equals(prefixMatch, that.prefixMatch) + && Objects.equals(suffixMatch, that.suffixMatch) + && Objects.equals(isInvertedMatch, that.isInvertedMatch); + } + + @Override + public int hashCode() { + return Objects.hash( + name, exactMatch, safeRegExMatch == null ? null : safeRegExMatch.pattern(), + rangeMatch, presentMatch, prefixMatch, suffixMatch, isInvertedMatch); + } + + @Override + public String toString() { + ToStringHelper toStringHelper = + MoreObjects.toStringHelper(this).add("name", name); + if (exactMatch != null) { + toStringHelper.add("exactMatch", exactMatch); + } + if (safeRegExMatch != null) { + toStringHelper.add("safeRegExMatch", safeRegExMatch.pattern()); + } + if (rangeMatch != null) { + toStringHelper.add("rangeMatch", rangeMatch); + } + if (presentMatch != null) { + toStringHelper.add("presentMatch", presentMatch); + } + if (prefixMatch != null) { + toStringHelper.add("prefixMatch", prefixMatch); + } + if (suffixMatch != null) { + toStringHelper.add("suffixMatch", suffixMatch); + } + return toStringHelper.add("isInvertedMatch", isInvertedMatch).toString(); + } + + static final class Range { + private final long start; + private final long end; + + @VisibleForTesting + Range(long start, long end) { + this.start = start; + this.end = end; + } + + @Override + public int hashCode() { + return Objects.hash(start, end); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Range that = (Range) o; + return Objects.equals(start, that.start) + && Objects.equals(end, that.end); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("start", start) + .add("end", end) + .toString(); + } } } /** See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.route.RouteAction}. */ static final class RouteAction { + // Exactly one of the following fields is non-null. + @Nullable private final String cluster; + @Nullable private final String clusterHeader; - private final List weightedCluster; + @Nullable + private final List weightedClusters; @VisibleForTesting - RouteAction(String cluster, String clusterHeader, List weightedCluster) { + RouteAction( + @Nullable String cluster, @Nullable String clusterHeader, + @Nullable List weightedClusters) { this.cluster = cluster; this.clusterHeader = clusterHeader; - this.weightedCluster = Collections.unmodifiableList(weightedCluster); + this.weightedClusters = weightedClusters; } + @Nullable String getCluster() { return cluster; } + @Nullable String getClusterHeader() { return clusterHeader; } + @Nullable List getWeightedCluster() { - return weightedCluster; + return weightedClusters; } @Override @@ -515,33 +970,57 @@ final class EnvoyProtoData { RouteAction that = (RouteAction) o; return Objects.equals(cluster, that.cluster) && Objects.equals(clusterHeader, that.clusterHeader) - && Objects.equals(weightedCluster, that.weightedCluster); + && Objects.equals(weightedClusters, that.weightedClusters); } @Override public int hashCode() { - return Objects.hash(cluster, clusterHeader, weightedCluster); + return Objects.hash(cluster, clusterHeader, weightedClusters); } @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("cluster", cluster) - .add("clusterHeader", clusterHeader) - .add("weightedCluster", weightedCluster) - .toString(); + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this); + if (cluster != null) { + toStringHelper.add("cluster", cluster); + } + if (clusterHeader != null) { + toStringHelper.add("clusterHeader", clusterHeader); + } + if (weightedClusters != null) { + toStringHelper.add("weightedClusters", weightedClusters); + } + return toStringHelper.toString(); } - private static RouteAction fromEnvoyProtoRouteAction( + @VisibleForTesting + static StructOrError fromEnvoyProtoRouteAction( io.envoyproxy.envoy.api.v2.route.RouteAction proto) { - List weightedCluster = new ArrayList<>(); - List clusterWeights - = proto.getWeightedClusters().getClustersList(); - for (io.envoyproxy.envoy.api.v2.route.WeightedCluster.ClusterWeight clusterWeight - : clusterWeights) { - weightedCluster.add(ClusterWeight.fromEnvoyProtoClusterWeight(clusterWeight)); + String cluster = null; + String clusterHeader = null; + List weightedClusters = null; + switch (proto.getClusterSpecifierCase()) { + case CLUSTER: + cluster = proto.getCluster(); + break; + case CLUSTER_HEADER: + clusterHeader = proto.getClusterHeader(); + break; + case WEIGHTED_CLUSTERS: + List clusterWeights + = proto.getWeightedClusters().getClustersList(); + weightedClusters = new ArrayList<>(); + for (io.envoyproxy.envoy.api.v2.route.WeightedCluster.ClusterWeight clusterWeight + : clusterWeights) { + weightedClusters.add(ClusterWeight.fromEnvoyProtoClusterWeight(clusterWeight)); + } + break; + case CLUSTERSPECIFIER_NOT_SET: + default: + return StructOrError.fromError( + "Unknown cluster specifier: " + proto.getClusterSpecifierCase()); } - return new RouteAction(proto.getCluster(), proto.getClusterHeader(), weightedCluster); + return StructOrError.fromStruct(new RouteAction(cluster, clusterHeader, weightedClusters)); } } @@ -592,7 +1071,8 @@ final class EnvoyProtoData { .toString(); } - private static ClusterWeight fromEnvoyProtoClusterWeight( + @VisibleForTesting + static ClusterWeight fromEnvoyProtoClusterWeight( io.envoyproxy.envoy.api.v2.route.WeightedCluster.ClusterWeight proto) { return new ClusterWeight(proto.getName(), proto.getWeight().getValue()); } diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index fe40ed7896..ea056755af 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -63,12 +63,12 @@ import io.grpc.xds.Bootstrapper.ServerInfo; import io.grpc.xds.EnvoyProtoData.DropOverload; import io.grpc.xds.EnvoyProtoData.Locality; import io.grpc.xds.EnvoyProtoData.LocalityLbEndpoints; -import io.grpc.xds.EnvoyProtoData.RouteAction; -import io.grpc.xds.EnvoyProtoData.RouteMatch; +import io.grpc.xds.EnvoyProtoData.StructOrError; import io.grpc.xds.LoadReportClient.LoadReportCallback; import io.grpc.xds.XdsLogger.XdsLogLevel; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -601,14 +601,13 @@ final class XdsClientImpl extends XdsClient { // data or one supersedes the other. TBD. if (requestedHttpConnManager.hasRouteConfig()) { RouteConfiguration rc = requestedHttpConnManager.getRouteConfig(); - routes = findRoutesInRouteConfig(rc, ldsResourceName); - String errorDetail = validateRoutes(routes); - if (errorDetail != null) { + try { + routes = findRoutesInRouteConfig(rc, ldsResourceName); + } catch (InvalidProtoDataException e) { errorMessage = "Listener " + ldsResourceName + " : cannot find a valid cluster name in any " - + "virtual hosts inside RouteConfiguration with domains matching: " - + ldsResourceName - + " with the reason : " + errorDetail; + + "virtual hosts domains matching: " + ldsResourceName + + " with the reason : " + e.getMessage(); } } else if (requestedHttpConnManager.hasRds()) { Rds rds = requestedHttpConnManager.getRds(); @@ -643,23 +642,10 @@ final class XdsClientImpl extends XdsClient { } if (routes != null) { // Found routes in the in-lined RouteConfiguration. - ConfigUpdate configUpdate; - if (!enableExperimentalRouting) { - EnvoyProtoData.Route defaultRoute = Iterables.getLast(routes); - configUpdate = - ConfigUpdate.newBuilder() - .addRoutes(ImmutableList.of(defaultRoute)) - .build(); - logger.log( - XdsLogLevel.INFO, - "Found cluster name (inlined in route config): {0}", - defaultRoute.getRouteAction().getCluster()); - } else { - configUpdate = ConfigUpdate.newBuilder().addRoutes(routes).build(); - logger.log( - XdsLogLevel.INFO, - "Found routes (inlined in route config): {0}", routes); - } + logger.log( + XdsLogLevel.DEBUG, + "Found routes (inlined in route config): {0}", routes); + ConfigUpdate configUpdate = ConfigUpdate.newBuilder().addRoutes(routes).build(); configWatcher.onConfigChanged(configUpdate); } else if (rdsRouteConfigName != null) { // Send an RDS request if the resource to request has changed. @@ -800,9 +786,10 @@ final class XdsClientImpl extends XdsClient { // Resolved cluster name for the requested resource, if exists. List routes = null; if (requestedRouteConfig != null) { - routes = findRoutesInRouteConfig(requestedRouteConfig, ldsResourceName); - String errorDetail = validateRoutes(routes); - if (errorDetail != null) { + try { + routes = findRoutesInRouteConfig(requestedRouteConfig, ldsResourceName); + } catch (InvalidProtoDataException e) { + String errorDetail = e.getMessage(); adsStream.sendNackRequest( ADS_TYPE_URL_RDS, ImmutableList.of(adsStream.rdsResourceName), rdsResponse.getVersionInfo(), @@ -824,24 +811,9 @@ final class XdsClientImpl extends XdsClient { rdsRespTimer.cancel(); rdsRespTimer = null; } - - // Found routes in the in-lined RouteConfiguration. - ConfigUpdate configUpdate; - if (!enableExperimentalRouting) { - EnvoyProtoData.Route defaultRoute = Iterables.getLast(routes); - configUpdate = - ConfigUpdate.newBuilder() - .addRoutes(ImmutableList.of(defaultRoute)) - .build(); - logger.log( - XdsLogLevel.INFO, - "Found cluster name: {0}", - defaultRoute.getRouteAction().getCluster()); - } else { - configUpdate = ConfigUpdate.newBuilder().addRoutes(routes).build(); - logger.log(XdsLogLevel.INFO, "Found {0} routes", routes.size()); - logger.log(XdsLogLevel.DEBUG, "Found routes: {0}", routes); - } + logger.log(XdsLogLevel.DEBUG, "Found routes: {0}", routes); + ConfigUpdate configUpdate = + ConfigUpdate.newBuilder().addRoutes(routes).build(); configWatcher.onConfigChanged(configUpdate); } } @@ -849,9 +821,79 @@ final class XdsClientImpl extends XdsClient { /** * Processes a RouteConfiguration message to find the routes that requests for the given host will * be routed to. + * + * @throws InvalidProtoDataException if the message contains invalid data. */ + private static List findRoutesInRouteConfig( + RouteConfiguration config, String hostName) throws InvalidProtoDataException { + VirtualHost targetVirtualHost = findVirtualHostForHostName(config, hostName); + if (targetVirtualHost == null) { + throw new InvalidProtoDataException("Unable to find virtual host for " + hostName); + } + + // 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. + return populateRoutesInVirtualHost(targetVirtualHost); + } + @VisibleForTesting - static List findRoutesInRouteConfig( + static List populateRoutesInVirtualHost(VirtualHost virtualHost) + throws InvalidProtoDataException { + List routes = new ArrayList<>(); + List routesProto = virtualHost.getRoutesList(); + for (Route routeProto : routesProto) { + StructOrError route = + EnvoyProtoData.Route.fromEnvoyProtoRoute(routeProto); + if (route == null) { + continue; + } else if (route.getErrorDetail() != null) { + throw new InvalidProtoDataException( + "Virtual host [" + virtualHost.getName() + "] contains invalid route : " + + route.getErrorDetail()); + } + routes.add(route.getStruct()); + } + if (routes.isEmpty()) { + throw new InvalidProtoDataException( + "Virtual host [" + virtualHost.getName() + "] contains no usable route"); + } + // The last route must be a default route. + if (!Iterables.getLast(routes).isDefaultRoute()) { + throw new InvalidProtoDataException( + "Virtual host [" + virtualHost.getName() + + "] contains non-default route as the last route"); + } + // We only validate the default route unless path matching is enabled. + if (!enableExperimentalRouting) { + EnvoyProtoData.Route defaultRoute = Iterables.getLast(routes); + if (defaultRoute.getRouteAction().getCluster() == null) { + throw new InvalidProtoDataException( + "Virtual host [" + virtualHost.getName() + + "] default route contains no cluster name"); + } + return Collections.singletonList(defaultRoute); + } + + // We do more validation if path matching is enabled, but whether every single route is + // required to be valid for grpc is TBD. + // For now we consider the whole list invalid if anything invalid for grpc is found. + // TODO(zdapeng): Fix it if the decision is different from current implementation. + // TODO(zdapeng): Add test for validation. + for (EnvoyProtoData.Route route : routes) { + if (route.getRouteAction().getCluster() == null + && route.getRouteAction().getWeightedCluster() == null) { + throw new InvalidProtoDataException( + "Virtual host [" + virtualHost.getName() + + "] contains route without cluster or weighted cluster"); + } + } + return Collections.unmodifiableList(routes); + } + + @VisibleForTesting + @Nullable + static VirtualHost findVirtualHostForHostName( RouteConfiguration config, String hostName) { List virtualHosts = config.getVirtualHostsList(); // Domain search order: @@ -889,91 +931,7 @@ final class XdsClientImpl extends XdsClient { break; } } - - List routes = new ArrayList<>(); - // 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) { - List routesProto = targetVirtualHost.getRoutesList(); - for (Route route : routesProto) { - routes.add(EnvoyProtoData.Route.fromEnvoyProtoRoute(route)); - } - } - return routes; - } - - /** - * Validates the given list of routes and returns error details if there's any error. - */ - @Nullable - private static String validateRoutes(List routes) { - if (routes.isEmpty()) { - return "No routes found"; - } - - // We only validate the default route unless path matching is enabled. - if (!enableExperimentalRouting) { - EnvoyProtoData.Route route = routes.get(routes.size() - 1); - RouteMatch routeMatch = route.getRouteMatch(); - if (!routeMatch.isDefaultMatcher()) { - return "The last route must be the default route"; - } - if (!routeMatch.isCaseSensitive()) { - return "Case-insensitive route match not supported"; - } - if (route.getRouteAction() == null) { - return "Route action is not specified for the default route"; - } - if (route.getRouteAction().getCluster().isEmpty()) { - return "Cluster is not specified for the default route"; - } - return null; - } - - // We do more validation if path matching is enabled, but whether every single route is required - // to be valid for grpc is TBD. - // For now we consider the whole list invalid if anything invalid for grpc is found. - // TODO(zdapeng): Fix it if the decision is different from current implementation. - // TODO(zdapeng): Add test for validation. - for (int i = 0; i < routes.size(); i++) { - EnvoyProtoData.Route route = routes.get(i); - RouteAction routeAction = route.getRouteAction(); - if (routeAction == null) { - return "Route action is not specified for one of the routes"; - } - RouteMatch routeMatch = route.getRouteMatch(); - if (!routeMatch.isCaseSensitive()) { - return "Case-insensitive route match not supported"; - } - if (!routeMatch.isDefaultMatcher()) { - String prefix = routeMatch.getPrefix(); - String path = routeMatch.getPath(); - if (!prefix.isEmpty()) { - if (!prefix.startsWith("/") || !prefix.endsWith("/") || prefix.length() < 3) { - return "Prefix route match must be in the format of '/service/'"; - } - } else if (!path.isEmpty()) { - int lastSlash = path.lastIndexOf('/'); - if (!path.startsWith("/") || lastSlash == 0 || lastSlash == path.length() - 1) { - return "Path route match must be in the format of '/service/method'"; - } - } else if (routeMatch.hasRegex()) { - return "Regex route match not supported"; - } - } - if (i == routes.size() - 1) { - if (!routeMatch.isDefaultMatcher()) { - return "The last route must be the default route"; - } - } - if (routeAction.getCluster().isEmpty() && routeAction.getWeightedCluster().isEmpty()) { - return "Either cluster or weighted cluster route action must be provided"; - } - } - return null; + return targetVirtualHost; } /** @@ -1792,4 +1750,13 @@ final class XdsClientImpl extends XdsClient { return res; } } + + @VisibleForTesting + static final class InvalidProtoDataException extends RuntimeException { + private static final long serialVersionUID = 1L; + + private InvalidProtoDataException(String message) { + super(message, null, false, false); + } + } } diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 5532fec112..78ed2fda1b 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -159,21 +159,27 @@ final class XdsNameResolver extends NameResolver { rawLbConfig = generateXdsRoutingRawConfig(update.getRoutes()); } else { Route defaultRoute = Iterables.getOnlyElement(update.getRoutes()); + RouteAction action = defaultRoute.getRouteAction(); String clusterName = defaultRoute.getRouteAction().getCluster(); - if (!clusterName.isEmpty()) { + if (action.getCluster() != null) { logger.log( XdsLogLevel.INFO, "Received config update from xDS client {0}: cluster_name={1}", xdsClient, clusterName); rawLbConfig = generateCdsRawConfig(clusterName); - } else { + } else if (action.getWeightedCluster() != null) { logger.log( XdsLogLevel.INFO, "Received config update with one weighted cluster route from xDS client {0}", xdsClient); List clusterWeights = defaultRoute.getRouteAction().getWeightedCluster(); rawLbConfig = generateWeightedTargetRawConfig(clusterWeights); + } else { + // TODO (chengyuanzhang): route with cluster_header + logger.log( + XdsLogLevel.WARNING, "Route action with cluster_header is not implemented"); + return; } } @@ -229,15 +235,18 @@ final class XdsNameResolver extends NameResolver { for (Route route : routesUpdate) { String service = ""; String method = ""; - if (!route.getRouteMatch().isDefaultMatcher()) { - String prefix = route.getRouteMatch().getPrefix(); - String path = route.getRouteMatch().getPath(); - if (!prefix.isEmpty()) { + if (!route.isDefaultRoute()) { + String prefix = route.getRouteMatch().getPathPrefixMatch(); + String path = route.getRouteMatch().getPathExactMatch(); + if (prefix != null) { service = prefix.substring(1, prefix.length() - 1); - } else if (!path.isEmpty()) { + } else if (path != null) { int splitIndex = path.lastIndexOf('/'); service = path.substring(1, splitIndex); method = path.substring(splitIndex + 1); + } else { + // TODO (chengyuanzhang): match with regex. + continue; } } Map methodName = ImmutableMap.of("service", service, "method", method); @@ -247,10 +256,10 @@ final class XdsNameResolver extends NameResolver { if (exitingActions.containsKey(routeAction)) { actionName = exitingActions.get(routeAction); } else { - if (!routeAction.getCluster().isEmpty()) { + if (routeAction.getCluster() != null) { actionName = "cds:" + routeAction.getCluster(); actionPolicy = generateCdsRawConfig(routeAction.getCluster()); - } else { + } else if (routeAction.getWeightedCluster() != null) { StringBuilder sb = new StringBuilder("weighted:"); List clusterWeights = routeAction.getWeightedCluster(); for (ClusterWeight clusterWeight : clusterWeights) { @@ -266,6 +275,9 @@ final class XdsNameResolver extends NameResolver { actionName = actionName + "_" + exitingActions.size(); } actionPolicy = generateWeightedTargetRawConfig(clusterWeights); + } else { + // TODO (chengyuanzhang): route with cluster_header. + continue; } exitingActions.put(routeAction, actionName); List childPolicies = ImmutableList.of(actionPolicy); diff --git a/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java b/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java index 6bf93f39a5..86668c1011 100644 --- a/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java +++ b/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java @@ -20,8 +20,20 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.testing.EqualsTester; import com.google.protobuf.BoolValue; -import io.envoyproxy.envoy.api.v2.route.RouteMatch; +import com.google.protobuf.UInt32Value; +import com.google.re2j.Pattern; +import io.envoyproxy.envoy.api.v2.route.QueryParameterMatcher; +import io.envoyproxy.envoy.api.v2.route.RedirectAction; +import io.grpc.xds.EnvoyProtoData.ClusterWeight; +import io.grpc.xds.EnvoyProtoData.HeaderMatcher; import io.grpc.xds.EnvoyProtoData.Locality; +import io.grpc.xds.EnvoyProtoData.Route; +import io.grpc.xds.EnvoyProtoData.RouteAction; +import io.grpc.xds.EnvoyProtoData.RouteMatch; +import io.grpc.xds.EnvoyProtoData.StructOrError; +import java.util.Arrays; +import java.util.Collections; +import javax.annotation.Nullable; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -75,24 +87,386 @@ public class EnvoyProtoDataTest { // TODO(chengyuanzhang): add test for other data types. @Test - public void routeMatchCaseSensitive() { - assertThat( - EnvoyProtoData.RouteMatch.fromEnvoyProtoRouteMatch(RouteMatch.newBuilder().build()) - .isCaseSensitive()) - .isTrue(); - assertThat( - EnvoyProtoData.RouteMatch.fromEnvoyProtoRouteMatch( - RouteMatch.newBuilder() - .setCaseSensitive(BoolValue.newBuilder().setValue(true)) - .build()) - .isCaseSensitive()) - .isTrue(); - assertThat( - EnvoyProtoData.RouteMatch.fromEnvoyProtoRouteMatch( - RouteMatch.newBuilder() - .setCaseSensitive(BoolValue.newBuilder().setValue(false)) - .build()) - .isCaseSensitive()) - .isFalse(); + public void convertRoute() { + io.envoyproxy.envoy.api.v2.route.Route proto1 = + io.envoyproxy.envoy.api.v2.route.Route.newBuilder() + .setName("route-blade") + .setMatch( + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setPath("/service/method")) + .setRoute( + io.envoyproxy.envoy.api.v2.route.RouteAction.newBuilder() + .setCluster("cluster-foo")) + .build(); + StructOrError struct1 = Route.fromEnvoyProtoRoute(proto1); + assertThat(struct1.getErrorDetail()).isNull(); + assertThat(struct1.getStruct()) + .isEqualTo( + new Route( + new RouteMatch( + null, "/service/method", null, null, Collections.emptyList()), + new RouteAction("cluster-foo", null, null))); + + io.envoyproxy.envoy.api.v2.route.Route unsupportedProto = + io.envoyproxy.envoy.api.v2.route.Route.newBuilder() + .setName("route-blade") + .setMatch(io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder().setPath("")) + .setRedirect(RedirectAction.getDefaultInstance()) + .build(); + StructOrError unsupportedStruct = Route.fromEnvoyProtoRoute(unsupportedProto); + assertThat(unsupportedStruct.getErrorDetail()).isNotNull(); + assertThat(unsupportedStruct.getStruct()).isNull(); + } + + @Test + public void isDefaultRoute() { + StructOrError struct1 = Route.fromEnvoyProtoRoute(buildSimpleRouteProto("", null)); + StructOrError struct2 = Route.fromEnvoyProtoRoute(buildSimpleRouteProto("/", null)); + StructOrError struct3 = + Route.fromEnvoyProtoRoute(buildSimpleRouteProto("/service/", null)); + StructOrError struct4 = + Route.fromEnvoyProtoRoute(buildSimpleRouteProto(null, "/service/method")); + + assertThat(struct1.getStruct().isDefaultRoute()).isTrue(); + assertThat(struct2.getStruct().isDefaultRoute()).isTrue(); + assertThat(struct3.getStruct().isDefaultRoute()).isFalse(); + assertThat(struct4.getStruct().isDefaultRoute()).isFalse(); + } + + private static io.envoyproxy.envoy.api.v2.route.Route buildSimpleRouteProto( + @Nullable String pathPrefix, @Nullable String path) { + io.envoyproxy.envoy.api.v2.route.Route.Builder routeBuilder = + io.envoyproxy.envoy.api.v2.route.Route.newBuilder() + .setName("simple-route") + .setRoute(io.envoyproxy.envoy.api.v2.route.RouteAction.newBuilder() + .setCluster("simple-cluster")); + if (pathPrefix != null) { + routeBuilder.setMatch(io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setPrefix(pathPrefix)); + } else if (path != null) { + routeBuilder.setMatch(io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setPath(path)); + } + return routeBuilder.build(); + } + + @Test + public void convertRouteMatch_pathMatching() { + // path_specifier = prefix + io.envoyproxy.envoy.api.v2.route.RouteMatch proto1 = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder().setPrefix("/").build(); + StructOrError struct1 = RouteMatch.fromEnvoyProtoRouteMatch(proto1); + assertThat(struct1.getErrorDetail()).isNull(); + assertThat(struct1.getStruct()).isEqualTo( + new RouteMatch("/", null, null, null, Collections.emptyList())); + + // path_specifier = path + io.envoyproxy.envoy.api.v2.route.RouteMatch proto2 = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder().setPath("/service/method").build(); + StructOrError struct2 = RouteMatch.fromEnvoyProtoRouteMatch(proto2); + assertThat(struct2.getErrorDetail()).isNull(); + assertThat(struct2.getStruct()).isEqualTo( + new RouteMatch( + null, "/service/method", null, null, Collections.emptyList())); + + // path_specifier = regex + @SuppressWarnings("deprecation") + io.envoyproxy.envoy.api.v2.route.RouteMatch proto3 = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder().setRegex("*").build(); + StructOrError struct3 = RouteMatch.fromEnvoyProtoRouteMatch(proto3); + assertThat(struct3.getErrorDetail()).isNotNull(); + assertThat(struct3.getStruct()).isNull(); + + // path_specifier = safe_regex + io.envoyproxy.envoy.api.v2.route.RouteMatch proto4 = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setSafeRegex( + io.envoyproxy.envoy.type.matcher.RegexMatcher.newBuilder().setRegex(".")).build(); + StructOrError struct4 = RouteMatch.fromEnvoyProtoRouteMatch(proto4); + assertThat(struct4.getErrorDetail()).isNull(); + assertThat(struct4.getStruct()).isEqualTo( + new RouteMatch( + null, null, Pattern.compile("."), null, Collections.emptyList())); + + // case_sensitive = false + io.envoyproxy.envoy.api.v2.route.RouteMatch proto5 = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setCaseSensitive(BoolValue.newBuilder().setValue(false)) + .build(); + StructOrError struct5 = RouteMatch.fromEnvoyProtoRouteMatch(proto5); + assertThat(struct5.getErrorDetail()).isNotNull(); + assertThat(struct5.getStruct()).isNull(); + + // query_parameters is set + io.envoyproxy.envoy.api.v2.route.RouteMatch proto6 = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .addQueryParameters(QueryParameterMatcher.getDefaultInstance()) + .build(); + StructOrError struct6 = RouteMatch.fromEnvoyProtoRouteMatch(proto6); + assertThat(struct6).isNull(); + + // path_specifier unset + io.envoyproxy.envoy.api.v2.route.RouteMatch unsetProto = + io.envoyproxy.envoy.api.v2.route.RouteMatch.getDefaultInstance(); + StructOrError unsetStruct = RouteMatch.fromEnvoyProtoRouteMatch(unsetProto); + assertThat(unsetStruct.getErrorDetail()).isNotNull(); + assertThat(unsetStruct.getStruct()).isNull(); + } + + @Test + public void convertRouteMatch_pathMatchFormat() { + StructOrError struct1 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto("", null)); + StructOrError struct2 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto("/", null)); + StructOrError struct3 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto("/service", null)); + StructOrError struct4 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto("/service/", null)); + StructOrError struct5 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto(null, "")); + StructOrError struct6 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto(null, "/service/method")); + StructOrError struct7 = + RouteMatch.fromEnvoyProtoRouteMatch(buildSimpleRouteMatchProto(null, "/service/method/")); + StructOrError struct8 = + RouteMatch.fromEnvoyProtoRouteMatch( + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setSafeRegex( + io.envoyproxy.envoy.type.matcher.RegexMatcher.newBuilder().setRegex("[")) + .build()); + + assertThat(struct1.getStruct()).isNotNull(); + assertThat(struct2.getStruct()).isNotNull(); + assertThat(struct3.getStruct()).isNull(); + assertThat(struct4.getStruct()).isNotNull(); + assertThat(struct5.getStruct()).isNull(); + assertThat(struct6.getStruct()).isNotNull(); + assertThat(struct7.getStruct()).isNull(); + assertThat(struct8.getStruct()).isNull(); + } + + private static io.envoyproxy.envoy.api.v2.route.RouteMatch buildSimpleRouteMatchProto( + @Nullable String pathPrefix, @Nullable String path) { + io.envoyproxy.envoy.api.v2.route.RouteMatch.Builder builder = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder(); + if (pathPrefix != null) { + builder.setPrefix(pathPrefix); + } else if (path != null) { + builder.setPath(path); + } + return builder.build(); + } + + @Test + public void convertRouteMatch_withHeaderMatching() { + io.envoyproxy.envoy.api.v2.route.RouteMatch proto = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setPrefix("") + .addHeaders( + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName(":scheme") + .setPrefixMatch("http")) + .addHeaders( + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName(":method") + .setExactMatch("PUT")) + .build(); + StructOrError struct = RouteMatch.fromEnvoyProtoRouteMatch(proto); + assertThat(struct.getErrorDetail()).isNull(); + assertThat(struct.getStruct()) + .isEqualTo( + new RouteMatch("", null, null, null, + Arrays.asList( + new HeaderMatcher(":scheme", null, null, null, null, "http", null, false), + new HeaderMatcher(":method", "PUT", null, null, null, null, null, false)))); + } + + @Test + public void convertRouteMatch_withRuntimeFraction() { + io.envoyproxy.envoy.api.v2.route.RouteMatch proto = + io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setPrefix("") + .setRuntimeFraction( + io.envoyproxy.envoy.api.v2.core.RuntimeFractionalPercent.newBuilder() + .setDefaultValue( + io.envoyproxy.envoy.type.FractionalPercent.newBuilder() + .setNumerator(30) + .setDenominator( + io.envoyproxy.envoy.type.FractionalPercent.DenominatorType + .HUNDRED))) + .build(); + StructOrError struct = RouteMatch.fromEnvoyProtoRouteMatch(proto); + assertThat(struct.getErrorDetail()).isNull(); + assertThat(struct.getStruct()) + .isEqualTo( + new RouteMatch( + "", null, null, new RouteMatch.Fraction(30, 100), + Collections.emptyList())); + } + + @Test + public void convertRouteAction() { + // cluster_specifier = cluster + io.envoyproxy.envoy.api.v2.route.RouteAction proto1 = + io.envoyproxy.envoy.api.v2.route.RouteAction.newBuilder() + .setCluster("cluster-foo") + .build(); + StructOrError struct1 = RouteAction.fromEnvoyProtoRouteAction(proto1); + assertThat(struct1.getErrorDetail()).isNull(); + assertThat(struct1.getStruct().getCluster()).isEqualTo("cluster-foo"); + assertThat(struct1.getStruct().getClusterHeader()).isNull(); + assertThat(struct1.getStruct().getWeightedCluster()).isNull(); + + // cluster_specifier = cluster_header + io.envoyproxy.envoy.api.v2.route.RouteAction proto2 = + io.envoyproxy.envoy.api.v2.route.RouteAction.newBuilder() + .setClusterHeader("cluster-bar") + .build(); + StructOrError struct2 = RouteAction.fromEnvoyProtoRouteAction(proto2); + assertThat(struct2.getErrorDetail()).isNull(); + assertThat(struct2.getStruct().getCluster()).isNull(); + assertThat(struct2.getStruct().getClusterHeader()).isEqualTo("cluster-bar"); + assertThat(struct2.getStruct().getWeightedCluster()).isNull(); + + // cluster_specifier = weighted_cluster + io.envoyproxy.envoy.api.v2.route.RouteAction proto3 = + io.envoyproxy.envoy.api.v2.route.RouteAction.newBuilder() + .setWeightedClusters( + io.envoyproxy.envoy.api.v2.route.WeightedCluster.newBuilder() + .addClusters( + io.envoyproxy.envoy.api.v2.route.WeightedCluster.ClusterWeight.newBuilder() + .setName("cluster-baz") + .setWeight(UInt32Value.newBuilder().setValue(100)))) + .build(); + StructOrError struct3 = RouteAction.fromEnvoyProtoRouteAction(proto3); + assertThat(struct3.getErrorDetail()).isNull(); + assertThat(struct3.getStruct().getCluster()).isNull(); + assertThat(struct3.getStruct().getClusterHeader()).isNull(); + assertThat(struct3.getStruct().getWeightedCluster()) + .containsExactly(new ClusterWeight("cluster-baz", 100)); + + // cluster_specifier unset + io.envoyproxy.envoy.api.v2.route.RouteAction unsetProto = + io.envoyproxy.envoy.api.v2.route.RouteAction.getDefaultInstance(); + StructOrError unsetStruct = RouteAction.fromEnvoyProtoRouteAction(unsetProto); + assertThat(unsetStruct.getErrorDetail()).isNotNull(); + assertThat(unsetStruct.getStruct()).isNull(); + } + + @Test + public void convertHeaderMatcher() { + // header_match_specifier = exact_match + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto1 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName(":method") + .setExactMatch("PUT") + .build(); + StructOrError struct1 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto1); + assertThat(struct1.getErrorDetail()).isNull(); + assertThat(struct1.getStruct()).isEqualTo( + new HeaderMatcher(":method", "PUT", null, null, null, null, null, false)); + + // header_match_specifier = regex_match + @SuppressWarnings("deprecation") + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto2 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName(":method") + .setRegexMatch("*") + .build(); + StructOrError struct2 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto2); + assertThat(struct2.getErrorDetail()).isNotNull(); + assertThat(struct2.getStruct()).isNull(); + + // header_match_specifier = safe_regex_match + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto3 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName(":method") + .setSafeRegexMatch( + io.envoyproxy.envoy.type.matcher.RegexMatcher.newBuilder().setRegex("P*")) + .build(); + StructOrError struct3 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto3); + assertThat(struct3.getErrorDetail()).isNull(); + assertThat(struct3.getStruct()).isEqualTo( + new HeaderMatcher(":method", null, Pattern.compile("P*"), null, null, null, null, false)); + + // header_match_specifier = range_match + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto4 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName("timeout") + .setRangeMatch( + io.envoyproxy.envoy.type.Int64Range.newBuilder().setStart(10L).setEnd(20L)) + .build(); + StructOrError struct4 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto4); + assertThat(struct4.getErrorDetail()).isNull(); + assertThat(struct4.getStruct()).isEqualTo( + new HeaderMatcher( + "timeout", null, null, new HeaderMatcher.Range(10L, 20L), null, null, null, false)); + + // header_match_specifier = present_match + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto5 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName("user-agent") + .setPresentMatch(true) + .build(); + StructOrError struct5 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto5); + assertThat(struct5.getErrorDetail()).isNull(); + assertThat(struct5.getStruct()).isEqualTo( + new HeaderMatcher("user-agent", null, null, null, true, null, null, false)); + + // header_match_specifier = prefix_match + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto6 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName("authority") + .setPrefixMatch("service-foo") + .build(); + StructOrError struct6 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto6); + assertThat(struct6.getErrorDetail()).isNull(); + assertThat(struct6.getStruct()).isEqualTo( + new HeaderMatcher("authority", null, null, null, null, "service-foo", null, false)); + + // header_match_specifier = suffix_match + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto7 = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName("authority") + .setSuffixMatch("googleapis.com") + .build(); + StructOrError struct7 = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto7); + assertThat(struct7.getErrorDetail()).isNull(); + assertThat(struct7.getStruct()).isEqualTo( + new HeaderMatcher( + "authority", null, null, null, null, null, "googleapis.com", false)); + + // header_match_specifier unset + io.envoyproxy.envoy.api.v2.route.HeaderMatcher unsetProto = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.getDefaultInstance(); + StructOrError unsetStruct = + HeaderMatcher.fromEnvoyProtoHeaderMatcher(unsetProto); + assertThat(unsetStruct.getErrorDetail()).isNotNull(); + assertThat(unsetStruct.getStruct()).isNull(); + } + + @Test + public void convertHeaderMatcher_malformedRegExPattern() { + io.envoyproxy.envoy.api.v2.route.HeaderMatcher proto = + io.envoyproxy.envoy.api.v2.route.HeaderMatcher.newBuilder() + .setName(":method") + .setSafeRegexMatch( + io.envoyproxy.envoy.type.matcher.RegexMatcher.newBuilder().setRegex("[")) + .build(); + StructOrError struct = HeaderMatcher.fromEnvoyProtoHeaderMatcher(proto); + assertThat(struct.getErrorDetail()).isNotNull(); + assertThat(struct.getStruct()).isNull(); + } + + @Test + public void convertClusterWeight() { + io.envoyproxy.envoy.api.v2.route.WeightedCluster.ClusterWeight proto = + io.envoyproxy.envoy.api.v2.route.WeightedCluster.ClusterWeight.newBuilder() + .setName("cluster-foo") + .setWeight(UInt32Value.newBuilder().setValue(30)).build(); + ClusterWeight struct = ClusterWeight.fromEnvoyProtoClusterWeight(proto); + assertThat(struct.getName()).isEqualTo("cluster-foo"); + assertThat(struct.getWeight()).isEqualTo(30); } } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java index 0200f1e91a..6781338616 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java @@ -113,6 +113,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; @@ -177,6 +178,8 @@ public class XdsClientImplTest { @Rule public final GrpcCleanupRule cleanupRule = new GrpcCleanupRule(); + @Rule + public ExpectedException thrown = ExpectedException.none(); private final SynchronizationContext syncContext = new SynchronizationContext( new Thread.UncaughtExceptionHandler() { @@ -722,25 +725,21 @@ public class XdsClientImplTest { new EnvoyProtoData.Route( // path match with cluster route new EnvoyProtoData.RouteMatch( - /* prefix= */ "", - /* path= */ "/service1/method1", - /* hasRegex= */ false, - /* caseSensitive= */ true), + /* prefix= */ null, + /* path= */ "/service1/method1"), new EnvoyProtoData.RouteAction( "cl1.googleapis.com", - "", - ImmutableList.of()))); + null, + null))); assertThat(routes.get(1)).isEqualTo( new EnvoyProtoData.Route( // path match with weighted cluster route new EnvoyProtoData.RouteMatch( - /* prefix= */ "", - /* path= */ "/service2/method2", - /* hasRegex= */ false, - /* caseSensitive= */ true), + /* prefix= */ null, + /* path= */ "/service2/method2"), new EnvoyProtoData.RouteAction( - "", - "", + null, + null, ImmutableList.of( new EnvoyProtoData.ClusterWeight("cl21.googleapis.com", 30), new EnvoyProtoData.ClusterWeight("cl22.googleapis.com", 70) @@ -750,25 +749,21 @@ public class XdsClientImplTest { // prefix match with cluster route new EnvoyProtoData.RouteMatch( /* prefix= */ "/service1/", - /* path= */ "", - /* hasRegex= */ false, - /* caseSensitive= */ true), + /* path= */ null), new EnvoyProtoData.RouteAction( "cl1.googleapis.com", - "", - ImmutableList.of()))); + null, + null))); assertThat(routes.get(3)).isEqualTo( new EnvoyProtoData.Route( // default match with cluster route new EnvoyProtoData.RouteMatch( /* prefix= */ "", - /* path= */ "", - /* hasRegex= */ false, - /* caseSensitive= */ true), + /* path= */ null), new EnvoyProtoData.RouteAction( "cluster.googleapis.com", - "", - ImmutableList.of()))); + null, + null))); } /** @@ -904,97 +899,6 @@ public class XdsClientImplTest { assertThat(fakeClock.getPendingTasks(RDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); } - /** - * Client receives an RDS response (after a previous LDS request-response) containing a - * RouteConfiguration message for the requested resource. But the RouteConfiguration message - * is invalid as the VirtualHost with domains matching the requested hostname contains a route - * with a case-insensitive matcher. - * The RDS response is NACKed, as if the XdsClient has not received this response. - * The config watcher is NOT notified with an error. - */ - @Test - public void matchingVirtualHostWithCaseInsensitiveAndSensitiveRouteMatch() { - xdsClient.watchConfigData(TARGET_AUTHORITY, configWatcher); - StreamObserver responseObserver = responseObservers.poll(); - StreamObserver requestObserver = requestObservers.poll(); - - Rds rdsConfig = - Rds.newBuilder() - // Must set to use ADS. - .setConfigSource( - ConfigSource.newBuilder().setAds(AggregatedConfigSource.getDefaultInstance())) - .setRouteConfigName("route-foo.googleapis.com") - .build(); - - List listeners = ImmutableList.of( - Any.pack(buildListener(TARGET_AUTHORITY, /* matching resource */ - Any.pack(HttpConnectionManager.newBuilder().setRds(rdsConfig).build()))) - ); - DiscoveryResponse response = - buildDiscoveryResponse("0", listeners, XdsClientImpl.ADS_TYPE_URL_LDS, "0000"); - responseObserver.onNext(response); - - // Client sends an ACK LDS request and an RDS request for "route-foo.googleapis.com". (Omitted) - - assertThat(fakeClock.getPendingTasks(RDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).hasSize(1); - - // A VirtualHost with a Route with a case-insensitive matcher. - VirtualHost virtualHost = - VirtualHost.newBuilder() - .setName("virtualhost00.googleapis.com") // don't care - .addDomains(TARGET_AUTHORITY) - .addRoutes( - Route.newBuilder() - .setRoute(RouteAction.newBuilder().setCluster("cluster.googleapis.com")) - .setMatch(RouteMatch.newBuilder().setPrefix("").setCaseSensitive( - BoolValue.newBuilder().setValue(false)))) - .build(); - - List routeConfigs = ImmutableList.of( - Any.pack( - buildRouteConfiguration("route-foo.googleapis.com", - ImmutableList.of(virtualHost)))); - response = buildDiscoveryResponse("0", routeConfigs, XdsClientImpl.ADS_TYPE_URL_RDS, "0000"); - responseObserver.onNext(response); - - // Client sent an NACK RDS request. - verify(requestObserver) - .onNext( - argThat(new DiscoveryRequestMatcher("", "route-foo.googleapis.com", - XdsClientImpl.ADS_TYPE_URL_RDS, "0000"))); - - // A VirtualHost with a Route with a case-sensitive matcher. - virtualHost = - VirtualHost.newBuilder() - .setName("virtualhost00.googleapis.com") // don't care - .addDomains(TARGET_AUTHORITY) - .addRoutes( - Route.newBuilder() - .setRoute(RouteAction.newBuilder().setCluster("cluster.googleapis.com")) - .setMatch(RouteMatch.newBuilder().setPrefix("").setCaseSensitive( - BoolValue.newBuilder().setValue(true)))) - .build(); - - routeConfigs = ImmutableList.of( - Any.pack( - buildRouteConfiguration("route-foo.googleapis.com", - ImmutableList.of(virtualHost)))); - response = buildDiscoveryResponse("0", routeConfigs, XdsClientImpl.ADS_TYPE_URL_RDS, "0000"); - responseObserver.onNext(response); - - // Client sent an ACK RDS request. - verify(requestObserver) - .onNext( - argThat(new DiscoveryRequestMatcher( - "0", - ImmutableList.of("route-foo.googleapis.com"), - XdsClientImpl.ADS_TYPE_URL_RDS, - "0000"))); - - verify(configWatcher).onConfigChanged(any(ConfigUpdate.class)); - verifyNoMoreInteractions(configWatcher); - } - /** * Client receives LDS/RDS responses for updating resources previously received. * @@ -3464,117 +3368,108 @@ public class XdsClientImplTest { } @Test - public void findClusterNameInRouteConfig_exactMatchFirst() { + public void findVirtualHostForHostName_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)); - List routes = - XdsClientImpl.findRoutesInRouteConfig(routeConfig, hostname); - assertThat(routes).hasSize(1); - assertThat(routes.get(0).getRouteAction().getCluster()) - .isEqualTo(targetClusterName); + assertThat(XdsClientImpl.findVirtualHostForHostName(routeConfig, hostname)).isEqualTo(vHost1); } @Test - public void findClusterNameInRouteConfig_preferSuffixDomainOverPrefixDomain() { + public void findVirtualHostForHostName_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)); - List routes = - XdsClientImpl.findRoutesInRouteConfig(routeConfig, hostname); - assertThat(routes).hasSize(1); - assertThat(routes.get(0).getRouteAction().getCluster()) - .isEqualTo(targetClusterName); + assertThat(XdsClientImpl.findVirtualHostForHostName(routeConfig, hostname)).isEqualTo(vHost1); } @Test - public void findClusterNameInRouteConfig_asteriskMatchAnyDomain() { + public void findVirtualHostForHostName_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)); - List routes = - XdsClientImpl.findRoutesInRouteConfig(routeConfig, hostname); - assertThat(routes).hasSize(1); - assertThat(routes.get(0).getRouteAction().getCluster()) - .isEqualTo(targetClusterName); + assertThat(XdsClientImpl.findVirtualHostForHostName(routeConfig, hostname)).isEqualTo(vHost1); + } + + @Test + public void populateRoutesInVirtualHost_routeWithCaseInsensitiveMatch() { + VirtualHost virtualHost = + VirtualHost.newBuilder() + .setName("virtualhost00.googleapis.com") // don't care + .addDomains(TARGET_AUTHORITY) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster.googleapis.com")) + .setMatch( + RouteMatch.newBuilder() + .setPrefix("") + .setCaseSensitive(BoolValue.newBuilder().setValue(false)))) + .build(); + + thrown.expect(XdsClientImpl.InvalidProtoDataException.class); + XdsClientImpl.populateRoutesInVirtualHost(virtualHost); + } + + @Test + public void populateRoutesInVirtualHost_lastRouteIsNotDefaultRoute() { + VirtualHost virtualHost = + VirtualHost.newBuilder() + .setName("virtualhost00.googleapis.com") // don't care + .addDomains(TARGET_AUTHORITY) + .addRoutes( + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("cluster.googleapis.com")) + .setMatch( + RouteMatch.newBuilder() + .setPrefix("/service/method") + .setCaseSensitive(BoolValue.newBuilder().setValue(true)))) + .build(); + + thrown.expect(XdsClientImpl.InvalidProtoDataException.class); + XdsClientImpl.populateRoutesInVirtualHost(virtualHost); } @Test diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index cf72fc098c..66e17cad96 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -355,23 +355,23 @@ public class XdsNameResolverTest { ImmutableList.of( // path match, routed to cluster Route.newBuilder() - .setMatch(buildPathMatch("fooSvc", "hello")) + .setMatch(buildPathExactMatch("fooSvc", "hello")) .setRoute(buildClusterRoute("cluster-hello.googleapis.com")) .build(), // prefix match, routed to cluster Route.newBuilder() - .setMatch(buildPrefixMatch("fooSvc")) + .setMatch(buildPathPrefixMatch("fooSvc")) .setRoute(buildClusterRoute("cluster-foo.googleapis.com")) .build(), // path match, routed to weighted clusters Route.newBuilder() - .setMatch(buildPathMatch("barSvc", "hello")) + .setMatch(buildPathExactMatch("barSvc", "hello")) .setRoute(buildWeightedClusterRoute(ImmutableMap.of( "cluster-hello.googleapis.com", 40, "cluster-hello2.googleapis.com", 60))) .build(), // prefix match, routed to weighted clusters Route.newBuilder() - .setMatch(buildPrefixMatch("barSvc")) + .setMatch(buildPathPrefixMatch("barSvc")) .setRoute( buildWeightedClusterRoute( ImmutableMap.of( @@ -451,6 +451,7 @@ public class XdsNameResolverTest { // with a route resolution for a single weighted cluster route. Route weightedClustersDefaultRoute = Route.newBuilder() + .setMatch(RouteMatch.newBuilder().setPrefix("")) .setRoute(buildWeightedClusterRoute( ImmutableMap.of( "cluster-foo.googleapis.com", 20, "cluster-bar.googleapis.com", 80))) @@ -496,23 +497,23 @@ public class XdsNameResolverTest { ImmutableList.of( // path match, routed to cluster Route.newBuilder() - .setMatch(buildPathMatch("fooSvc", "hello")) + .setMatch(buildPathExactMatch("fooSvc", "hello")) .setRoute(buildClusterRoute("cluster-hello.googleapis.com")) .build(), // prefix match, routed to cluster Route.newBuilder() - .setMatch(buildPrefixMatch("fooSvc")) + .setMatch(buildPathPrefixMatch("fooSvc")) .setRoute(buildClusterRoute("cluster-foo.googleapis.com")) .build(), // duplicate path match, routed to weighted clusters Route.newBuilder() - .setMatch(buildPathMatch("fooSvc", "hello")) + .setMatch(buildPathExactMatch("fooSvc", "hello")) .setRoute(buildWeightedClusterRoute(ImmutableMap.of( "cluster-hello.googleapis.com", 40, "cluster-hello2.googleapis.com", 60))) .build(), - // duplicage prefix match, routed to weighted clusters + // duplicate prefix match, routed to weighted clusters Route.newBuilder() - .setMatch(buildPrefixMatch("fooSvc")) + .setMatch(buildPathPrefixMatch("fooSvc")) .setRoute( buildWeightedClusterRoute( ImmutableMap.of( @@ -520,6 +521,7 @@ public class XdsNameResolverTest { .build(), // default, routed to cluster Route.newBuilder() + .setMatch(RouteMatch.newBuilder().setPrefix("")) .setRoute(buildClusterRoute("cluster-hello.googleapis.com")) .build()); List routeConfigs = ImmutableList.of( @@ -722,11 +724,11 @@ public class XdsNameResolverTest { return buildDiscoveryResponse(versionInfo, routeConfigs, XdsClientImpl.ADS_TYPE_URL_RDS, nonce); } - private static RouteMatch buildPrefixMatch(String service) { + private static RouteMatch buildPathPrefixMatch(String service) { return RouteMatch.newBuilder().setPrefix("/" + service + "/").build(); } - private static RouteMatch buildPathMatch(String service, String method) { + private static RouteMatch buildPathExactMatch(String service, String method) { return RouteMatch.newBuilder().setPath("/" + service + "/" + method).build(); }