Skip to content

Commit

Permalink
xds: support case insensitive path matching (#7506)
Browse files Browse the repository at this point in the history
  • Loading branch information
@voidzcy
voidzcy committed Oct 15, 2020
1 parent 67b5460 commit ef90da0
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 140 deletions.
31 changes: 6 additions & 25 deletions xds/src/main/java/io/grpc/xds/EnvoyProtoData.java
View file
Expand Up @@ -919,16 +919,6 @@ RouteAction getRouteAction() {
return routeAction;
}

// TODO(chengyuanzhang): delete and do not use after routing feature is always ON.
boolean isDefaultRoute() {
// For backward compatibility, all the other matchers are ignored.
String prefix = routeMatch.getPathMatch().getPrefix();
if (prefix != null) {
return prefix.isEmpty() || prefix.equals("/");
}
return false;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down Expand Up @@ -993,17 +983,12 @@ static StructOrError<Route> fromEnvoyProtoRoute(
}

@VisibleForTesting
@SuppressWarnings("deprecation")
@Nullable
static StructOrError<RouteMatch> convertEnvoyProtoRouteMatch(
io.envoyproxy.envoy.config.route.v3.RouteMatch proto) {
if (proto.getQueryParametersCount() != 0) {
return null;
}
if (proto.hasCaseSensitive() && !proto.getCaseSensitive().getValue()) {
return StructOrError.fromError("Unsupported match option: case insensitive");
}

StructOrError<PathMatcher> pathMatch = convertEnvoyProtoPathMatcher(proto);
if (pathMatch.getErrorDetail() != null) {
return StructOrError.fromError(pathMatch.getErrorDetail());
Expand Down Expand Up @@ -1033,32 +1018,28 @@ static StructOrError<RouteMatch> convertEnvoyProtoRouteMatch(
pathMatch.getStruct(), Collections.unmodifiableList(headerMatchers), fractionMatch));
}

@SuppressWarnings("deprecation")
private static StructOrError<PathMatcher> convertEnvoyProtoPathMatcher(
io.envoyproxy.envoy.config.route.v3.RouteMatch proto) {
String path = null;
String prefix = null;
Pattern safeRegEx = null;
boolean caseSensitive = proto.getCaseSensitive().getValue();
switch (proto.getPathSpecifierCase()) {
case PREFIX:
prefix = proto.getPrefix();
break;
return StructOrError.fromStruct(
PathMatcher.fromPrefix(proto.getPrefix(), caseSensitive));
case PATH:
path = proto.getPath();
break;
return StructOrError.fromStruct(PathMatcher.fromPath(proto.getPath(), caseSensitive));
case SAFE_REGEX:
String rawPattern = proto.getSafeRegex().getRegex();
Pattern safeRegEx;
try {
safeRegEx = Pattern.compile(rawPattern);
} catch (PatternSyntaxException e) {
return StructOrError.fromError("Malformed safe regex pattern: " + e.getMessage());
}
break;
return StructOrError.fromStruct(PathMatcher.fromRegEx(safeRegEx));
case PATHSPECIFIER_NOT_SET:
default:
return StructOrError.fromError("Unknown path match type");
}
return StructOrError.fromStruct(new PathMatcher(path, prefix, safeRegEx));
}

private static StructOrError<FractionMatcher> convertEnvoyProtoFraction(
Expand Down
62 changes: 25 additions & 37 deletions xds/src/main/java/io/grpc/xds/RouteMatch.java
View file
Expand Up @@ -45,10 +45,8 @@ final class RouteMatch {
this.headerMatchers = headerMatchers;
}

@VisibleForTesting
RouteMatch(@Nullable String pathPrefixMatch, @Nullable String pathExactMatch) {
this(
new PathMatcher(pathExactMatch, pathPrefixMatch, null),
static RouteMatch withPathExactOnly(String pathExact) {
return new RouteMatch(PathMatcher.fromPath(pathExact, true),
Collections.<HeaderMatcher>emptyList(), null);
}

Expand Down Expand Up @@ -82,19 +80,6 @@ boolean matches(String path, Map<String, Iterable<String>> headers) {
return fractionMatch == null || fractionMatch.matches();
}

PathMatcher getPathMatch() {
return pathMatch;
}

List<HeaderMatcher> getHeaderMatchers() {
return Collections.unmodifiableList(headerMatchers);
}

@Nullable
FractionMatcher getFractionMatch() {
return fractionMatch;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down Expand Up @@ -132,35 +117,37 @@ static final class PathMatcher {
private final String prefix;
@Nullable
private final Pattern regEx;
private final boolean caseSensitive;

PathMatcher(@Nullable String path, @Nullable String prefix, @Nullable Pattern regEx) {
private PathMatcher(@Nullable String path, @Nullable String prefix, @Nullable Pattern regEx,
boolean caseSensitive) {
this.path = path;
this.prefix = prefix;
this.regEx = regEx;
this.caseSensitive = caseSensitive;
}

private boolean matches(String fullMethodName) {
if (path != null) {
return path.equals(fullMethodName);
} else if (prefix != null) {
return fullMethodName.startsWith(prefix);
}
return regEx.matches(fullMethodName);
static PathMatcher fromPath(String path, boolean caseSensitive) {
return new PathMatcher(path, null, null, caseSensitive);
}

@Nullable
String getPath() {
return path;
static PathMatcher fromPrefix(String prefix, boolean caseSensitive) {
return new PathMatcher(null, prefix, null, caseSensitive);
}

@Nullable
String getPrefix() {
return prefix;
static PathMatcher fromRegEx(Pattern regEx) {
return new PathMatcher(null, null, regEx, false /* doesn't matter */);
}

@Nullable
Pattern getRegEx() {
return regEx;
boolean matches(String fullMethodName) {
if (path != null) {
return caseSensitive ? path.equals(fullMethodName) : path.equalsIgnoreCase(fullMethodName);
} else if (prefix != null) {
return caseSensitive
? fullMethodName.startsWith(prefix)
: fullMethodName.toLowerCase().startsWith(prefix.toLowerCase());
}
return regEx.matches(fullMethodName);
}

@Override
Expand All @@ -174,25 +161,26 @@ public boolean equals(Object o) {
PathMatcher that = (PathMatcher) o;
return Objects.equals(path, that.path)
&& Objects.equals(prefix, that.prefix)
&& Objects.equals(caseSensitive, that.caseSensitive)
&& Objects.equals(
regEx == null ? null : regEx.pattern(),
that.regEx == null ? null : that.regEx.pattern());
}

@Override
public int hashCode() {
return Objects.hash(path, prefix, regEx == null ? null : regEx.pattern());
return Objects.hash(path, prefix, caseSensitive, regEx == null ? null : regEx.pattern());
}

@Override
public String toString() {
ToStringHelper toStringHelper =
MoreObjects.toStringHelper(this);
if (path != null) {
toStringHelper.add("path", path);
toStringHelper.add("path", path).add("caseSensitive", caseSensitive);
}
if (prefix != null) {
toStringHelper.add("prefix", prefix);
toStringHelper.add("prefix", prefix).add("caseSensitive", caseSensitive);
}
if (regEx != null) {
toStringHelper.add("regEx", regEx.pattern());
Expand Down
67 changes: 19 additions & 48 deletions xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java
View file
Expand Up @@ -50,7 +50,6 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -211,7 +210,7 @@ public void convertRoute() {
assertThat(struct1.getStruct())
.isEqualTo(
new Route(
new RouteMatch(new PathMatcher("/service/method", null, null),
new RouteMatch(PathMatcher.fromPath("/service/method", false),
Collections.<HeaderMatcher>emptyList(), null),
new RouteAction(TimeUnit.SECONDS.toNanos(15L), "cluster-foo", null)));

Expand Down Expand Up @@ -259,38 +258,6 @@ public void convertRoute_skipWithUnsupportedAction() {
assertThat(Route.fromEnvoyProtoRoute(proto)).isNull();
}

@Test
public void isDefaultRoute() {
StructOrError<Route> struct1 = Route.fromEnvoyProtoRoute(buildSimpleRouteProto("", null));
StructOrError<Route> struct2 = Route.fromEnvoyProtoRoute(buildSimpleRouteProto("/", null));
StructOrError<Route> struct3 =
Route.fromEnvoyProtoRoute(buildSimpleRouteProto("/service/", null));
StructOrError<Route> 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.config.route.v3.Route buildSimpleRouteProto(
@Nullable String pathPrefix, @Nullable String path) {
io.envoyproxy.envoy.config.route.v3.Route.Builder routeBuilder =
io.envoyproxy.envoy.config.route.v3.Route.newBuilder()
.setName("simple-route")
.setRoute(io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder()
.setCluster("simple-cluster"));
if (pathPrefix != null) {
routeBuilder.setMatch(io.envoyproxy.envoy.config.route.v3.RouteMatch.newBuilder()
.setPrefix(pathPrefix));
} else if (path != null) {
routeBuilder.setMatch(io.envoyproxy.envoy.config.route.v3.RouteMatch.newBuilder()
.setPath(path));
}
return routeBuilder.build();
}

@Test
public void convertRouteMatch_pathMatching() {
// path_specifier = prefix
Expand All @@ -300,7 +267,13 @@ public void convertRouteMatch_pathMatching() {
assertThat(struct1.getErrorDetail()).isNull();
assertThat(struct1.getStruct()).isEqualTo(
new RouteMatch(
new PathMatcher(null, "/", null), Collections.<HeaderMatcher>emptyList(), null));
PathMatcher.fromPrefix("/", false), Collections.<HeaderMatcher>emptyList(), null));

proto1 = proto1.toBuilder().setCaseSensitive(BoolValue.newBuilder().setValue(true)).build();
struct1 = Route.convertEnvoyProtoRouteMatch(proto1);
assertThat(struct1.getStruct()).isEqualTo(
new RouteMatch(
PathMatcher.fromPrefix("/", true), Collections.<HeaderMatcher>emptyList(), null));

// path_specifier = path
io.envoyproxy.envoy.config.route.v3.RouteMatch proto2 =
Expand All @@ -311,7 +284,14 @@ public void convertRouteMatch_pathMatching() {
assertThat(struct2.getErrorDetail()).isNull();
assertThat(struct2.getStruct()).isEqualTo(
new RouteMatch(
new PathMatcher("/service/method", null, null),
PathMatcher.fromPath("/service/method", false),
Collections.<HeaderMatcher>emptyList(), null));

proto2 = proto2.toBuilder().setCaseSensitive(BoolValue.newBuilder().setValue(true)).build();
struct2 = Route.convertEnvoyProtoRouteMatch(proto2);
assertThat(struct2.getStruct()).isEqualTo(
new RouteMatch(
PathMatcher.fromPath("/service/method", true),
Collections.<HeaderMatcher>emptyList(), null));

// path_specifier = safe_regex
Expand All @@ -323,18 +303,9 @@ public void convertRouteMatch_pathMatching() {
assertThat(struct4.getErrorDetail()).isNull();
assertThat(struct4.getStruct()).isEqualTo(
new RouteMatch(
new PathMatcher(null, null, Pattern.compile(".")),
PathMatcher.fromRegEx(Pattern.compile(".")),
Collections.<HeaderMatcher>emptyList(), null));

// case_sensitive = false
io.envoyproxy.envoy.config.route.v3.RouteMatch proto5 =
io.envoyproxy.envoy.config.route.v3.RouteMatch.newBuilder()
.setCaseSensitive(BoolValue.newBuilder().setValue(false))
.build();
StructOrError<RouteMatch> struct5 = Route.convertEnvoyProtoRouteMatch(proto5);
assertThat(struct5.getErrorDetail()).isNotNull();
assertThat(struct5.getStruct()).isNull();

// query_parameters is set
io.envoyproxy.envoy.config.route.v3.RouteMatch proto6 =
io.envoyproxy.envoy.config.route.v3.RouteMatch.newBuilder()
Expand Down Expand Up @@ -370,7 +341,7 @@ public void convertRouteMatch_withHeaderMatching() {
assertThat(struct.getStruct())
.isEqualTo(
new RouteMatch(
new PathMatcher(null, "", null),
PathMatcher.fromPrefix("", false),
Arrays.asList(
new HeaderMatcher(":scheme", null, null, null, null, "http", null, false),
new HeaderMatcher(":method", "PUT", null, null, null, null, null, false)),
Expand All @@ -394,7 +365,7 @@ public void convertRouteMatch_withRuntimeFraction() {
assertThat(struct.getStruct())
.isEqualTo(
new RouteMatch(
new PathMatcher(null, "", null), Collections.<HeaderMatcher>emptyList(),
PathMatcher.fromPrefix( "", false), Collections.<HeaderMatcher>emptyList(),
new FractionMatcher(30, 100)));
}

Expand Down

0 comments on commit ef90da0

Please sign in to comment.