From a192aa4fa8acc1475ce60f2d716fd794c1b233f7 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Sun, 24 Jan 2021 19:51:56 -0800 Subject: [PATCH 1/7] xds: parse HttpFault filter from LDS/RDS response --- .../java/io/grpc/xds/ClientXdsClient.java | 32 +- .../main/java/io/grpc/xds/EnvoyProtoData.java | 460 +++++++++++++++--- xds/src/main/java/io/grpc/xds/XdsClient.java | 38 +- .../java/io/grpc/xds/EnvoyProtoDataTest.java | 4 +- .../java/io/grpc/xds/XdsNameResolverTest.java | 10 +- 5 files changed, 458 insertions(+), 86 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index 9b734cfaa11..76a303a97bf 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -17,7 +17,10 @@ package io.grpc.xds; import static com.google.common.base.Preconditions.checkArgument; +import static io.grpc.xds.EnvoyProtoData.HTTP_FAULT_FILTER_NAME; import static io.grpc.xds.EnvoyProtoData.TRANSPORT_SOCKET_NAME_TLS; +import static io.grpc.xds.EnvoyProtoData.decodeFaultFilterConfig; +import static io.grpc.xds.EnvoyProtoData.parseHttpFaultFilter; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; @@ -38,12 +41,14 @@ import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; import io.envoyproxy.envoy.config.route.v3.VirtualHost; import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager; +import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter; import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.Rds; import io.grpc.ManagedChannel; import io.grpc.Status; import io.grpc.SynchronizationContext.ScheduledHandle; import io.grpc.internal.BackoffPolicy; import io.grpc.xds.EnvoyProtoData.DropOverload; +import io.grpc.xds.EnvoyProtoData.HttpFault; import io.grpc.xds.EnvoyProtoData.Locality; import io.grpc.xds.EnvoyProtoData.LocalityLbEndpoints; import io.grpc.xds.EnvoyProtoData.Node; @@ -163,6 +168,28 @@ protected void handleLdsResponse(String versionInfo, List resources, String maxStreamDuration = Durations.toNanos(options.getMaxStreamDuration()); } } + boolean hasFaultInjection = false; + HttpFault httpFault = null; + List httpFilters = hcm.getHttpFiltersList(); + if (parseHttpFaultFilter()) { + for (HttpFilter httpFilter : httpFilters) { + if (HTTP_FAULT_FILTER_NAME.equals(httpFilter.getName())) { + hasFaultInjection = true; + if (httpFilter.hasTypedConfig()) { + StructOrError httpFaultOrError = + decodeFaultFilterConfig(httpFilter.getTypedConfig()); + if (httpFaultOrError.getErrorDetail() != null) { + nackResponse(ResourceType.LDS, nonce, + "Listener " + listenerName + " contains invalid HttpFault filter: " + + httpFaultOrError.getErrorDetail()); + return; + } + httpFault = httpFaultOrError.getStruct(); + } + break; + } + } + } if (hcm.hasRouteConfig()) { List virtualHosts = new ArrayList<>(); for (VirtualHost virtualHostProto : hcm.getRouteConfig().getVirtualHostsList()) { @@ -176,7 +203,7 @@ protected void handleLdsResponse(String versionInfo, List resources, String } virtualHosts.add(virtualHost.getStruct()); } - update = new LdsUpdate(maxStreamDuration, virtualHosts); + update = new LdsUpdate(maxStreamDuration, virtualHosts, hasFaultInjection, httpFault); } else if (hcm.hasRds()) { Rds rds = hcm.getRds(); if (!rds.getConfigSource().hasAds()) { @@ -184,7 +211,8 @@ protected void handleLdsResponse(String versionInfo, List resources, String "Listener " + listenerName + " with RDS config_source not set to ADS"); return; } - update = new LdsUpdate(maxStreamDuration, rds.getRouteConfigName()); + update = new LdsUpdate( + maxStreamDuration, rds.getRouteConfigName(), hasFaultInjection, httpFault); rdsNames.add(rds.getRouteConfigName()); } else { nackResponse(ResourceType.LDS, nonce, diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index 633bd6af91a..b2b6aebef5d 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -22,6 +22,8 @@ import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects.ToStringHelper; import com.google.common.collect.ImmutableList; +import com.google.protobuf.Any; +import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.ListValue; import com.google.protobuf.NullValue; import com.google.protobuf.Struct; @@ -29,9 +31,12 @@ import com.google.protobuf.util.Durations; import com.google.re2j.Pattern; import com.google.re2j.PatternSyntaxException; +import io.envoyproxy.envoy.extensions.filters.http.fault.v3.HTTPFault; import io.envoyproxy.envoy.type.v3.FractionalPercent; import io.envoyproxy.envoy.type.v3.FractionalPercent.DenominatorType; import io.grpc.EquivalentAddressGroup; +import io.grpc.Status; +import io.grpc.Status.Code; import io.grpc.xds.RouteMatch.FractionMatcher; import io.grpc.xds.RouteMatch.HeaderMatcher; import io.grpc.xds.RouteMatch.PathMatcher; @@ -61,6 +66,10 @@ // TODO(chengyuanzhang): put data types into smaller categories. final class EnvoyProtoData { static final String TRANSPORT_SOCKET_NAME_TLS = "envoy.transport_sockets.tls"; + static final String HTTP_FAULT_FILTER_NAME = "envoy.fault"; + @VisibleForTesting + static boolean enableFaultInjection = + Boolean.parseBoolean(System.getenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION")); // Prevent instantiation. private EnvoyProtoData() { @@ -756,54 +765,7 @@ static final class DropOverload { static DropOverload fromEnvoyProtoDropOverload( io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment.Policy.DropOverload proto) { FractionalPercent percent = proto.getDropPercentage(); - int numerator = percent.getNumerator(); - DenominatorType type = percent.getDenominator(); - switch (type) { - case TEN_THOUSAND: - numerator *= 100; - break; - case HUNDRED: - numerator *= 10_000; - break; - case MILLION: - break; - case UNRECOGNIZED: - default: - throw new IllegalArgumentException("Unknown denominator type of " + percent); - } - - if (numerator > 1_000_000) { - numerator = 1_000_000; - } - - return new DropOverload(proto.getCategory(), numerator); - } - - @VisibleForTesting - static DropOverload fromEnvoyProtoDropOverloadV2( - io.envoyproxy.envoy.api.v2.ClusterLoadAssignment.Policy.DropOverload proto) { - io.envoyproxy.envoy.type.FractionalPercent percent = proto.getDropPercentage(); - int numerator = percent.getNumerator(); - io.envoyproxy.envoy.type.FractionalPercent.DenominatorType type = percent.getDenominator(); - switch (type) { - case TEN_THOUSAND: - numerator *= 100; - break; - case HUNDRED: - numerator *= 10_000; - break; - case MILLION: - break; - case UNRECOGNIZED: - default: - throw new IllegalArgumentException("Unknown denominator type of " + percent); - } - - if (numerator > 1_000_000) { - numerator = 1_000_000; - } - - return new DropOverload(proto.getCategory(), numerator); + return new DropOverload(proto.getCategory(), getRatePerMillion(percent)); } String getCategory() { @@ -849,12 +811,19 @@ static final class VirtualHost { private final List domains; // The list of routes that will be matched, in order, for incoming requests. private final List routes; + @Nullable + private final HttpFault httpFault; @VisibleForTesting VirtualHost(String name, List domains, List routes) { + this(name, domains, routes, null); + } + + VirtualHost(String name, List domains, List routes, HttpFault httpFault) { this.name = name; this.domains = domains; this.routes = routes; + this.httpFault = httpFault; } String getName() { @@ -869,13 +838,21 @@ List getRoutes() { return routes; } + @Nullable + HttpFault getHttpFault() { + return httpFault; + } + @Override public String toString() { - return MoreObjects.toStringHelper(this) + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) .add("name", name) .add("domains", domains) - .add("routes", routes) - .toString(); + .add("routes", routes); + if (httpFault != null) { + toStringHelper.add("httpFault", httpFault); + } + return toStringHelper.toString(); } static StructOrError fromEnvoyProtoVirtualHost( @@ -893,9 +870,25 @@ static StructOrError fromEnvoyProtoVirtualHost( } routes.add(route.getStruct()); } + HttpFault httpFault = null; + if (parseHttpFaultFilter()) { + Map filterConfigMap = proto.getTypedPerFilterConfigMap(); + if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { + Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); + StructOrError httpFaultOrError = decodeFaultFilterConfig(rawFaultFilterConfig); + if (httpFaultOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "Virtual host [" + name + "] contains invalid HttpFault filter : " + + httpFaultOrError.getErrorDetail()); + } + httpFault = httpFaultOrError.getStruct(); + } + } return StructOrError.fromStruct( - new VirtualHost(name, Collections.unmodifiableList(proto.getDomainsList()), - Collections.unmodifiableList(routes))); + new VirtualHost( + name, Collections.unmodifiableList(proto.getDomainsList()), + Collections.unmodifiableList(routes), + httpFault)); } } @@ -903,11 +896,18 @@ static StructOrError fromEnvoyProtoVirtualHost( static final class Route { private final RouteMatch routeMatch; private final RouteAction routeAction; + @Nullable + private final HttpFault httpFault; @VisibleForTesting - Route(RouteMatch routeMatch, @Nullable RouteAction routeAction) { + Route(RouteMatch routeMatch, RouteAction routeAction) { + this(routeMatch, routeAction, null); + } + + Route(RouteMatch routeMatch, RouteAction routeAction, @Nullable HttpFault httpFault) { this.routeMatch = routeMatch; this.routeAction = routeAction; + this.httpFault = httpFault; } RouteMatch getRouteMatch() { @@ -918,6 +918,11 @@ RouteAction getRouteAction() { return routeAction; } + @Nullable + HttpFault getHttpFault() { + return httpFault; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -928,20 +933,24 @@ public boolean equals(Object o) { } Route route = (Route) o; return Objects.equals(routeMatch, route.routeMatch) - && Objects.equals(routeAction, route.routeAction); + && Objects.equals(routeAction, route.routeAction) + && Objects.equals(httpFault, route.httpFault); } @Override public int hashCode() { - return Objects.hash(routeMatch, routeAction); + return Objects.hash(routeMatch, routeAction, httpFault); } @Override public String toString() { - return MoreObjects.toStringHelper(this) + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) .add("routeMatch", routeMatch) - .add("routeAction", routeAction) - .toString(); + .add("routeAction", routeAction); + if (httpFault != null) { + toStringHelper.add("httpFault", httpFault); + } + return toStringHelper.toString(); } @Nullable @@ -978,7 +987,23 @@ static StructOrError fromEnvoyProtoRoute( return StructOrError.fromError( "Invalid route [" + proto.getName() + "]: " + routeAction.getErrorDetail()); } - return StructOrError.fromStruct(new Route(routeMatch.getStruct(), routeAction.getStruct())); + + HttpFault httpFault = null; + if (parseHttpFaultFilter()) { + Map filterConfigMap = proto.getTypedPerFilterConfigMap(); + if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { + Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); + StructOrError httpFaultOrError = decodeFaultFilterConfig(rawFaultFilterConfig); + if (httpFaultOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "Route [" + proto.getName() + "] contains invalid HttpFault filter: " + + httpFaultOrError.getErrorDetail()); + } + httpFault = httpFaultOrError.getStruct(); + } + } + return StructOrError.fromStruct( + new Route(routeMatch.getStruct(), routeAction.getStruct(), httpFault)); } @VisibleForTesting @@ -1113,6 +1138,270 @@ static StructOrError convertEnvoyProtoHeaderMatcher( } } + static StructOrError decodeFaultFilterConfig(Any rawFaultFilterConfig) { + if (rawFaultFilterConfig.getTypeUrl().equals( + "type.googleapis.com/envoy.config.filter.http.fault.v2.HTTPFault")) { + rawFaultFilterConfig = rawFaultFilterConfig.toBuilder().setTypeUrl( + "type.googleapis.com/envoy.extensions.filter.http.fault.v3.HTTPFault").build(); + } + if (rawFaultFilterConfig.getTypeUrl().equals( + "type.googleapis.com/envoy.extensions.filter.http.fault.v3.HTTPFault")) { + HTTPFault httpFaultProto; + try { + httpFaultProto = rawFaultFilterConfig.unpack(HTTPFault.class); + } catch (InvalidProtocolBufferException e) { + return StructOrError.fromError("Invalid proto: " + e); + } + return HttpFault.fromEnvoyProtoHttpFault(httpFaultProto); + } + return StructOrError.fromError( + "Invalid type url for envoy.fault filter: " + rawFaultFilterConfig.getTypeUrl()); + } + + static boolean parseHttpFaultFilter() { + return enableFaultInjection; + } + + /** + * See corresponding Envoy proto message {@link + * io.envoyproxy.envoy.extensions.filters.http.fault.v3.HTTPFault}. + */ + static final class HttpFault { + @Nullable + final FaultDelay faultDelay; + @Nullable + final FaultAbort faultAbort; + + HttpFault(@Nullable FaultDelay faultDelay, @Nullable FaultAbort faultAbort) { + this.faultDelay = faultDelay; + this.faultAbort = faultAbort; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + HttpFault httpFault = (HttpFault) o; + return Objects.equals(faultDelay, httpFault.faultDelay) + && Objects.equals(faultAbort, httpFault.faultAbort); + } + + @Override + public int hashCode() { + return Objects.hash(faultDelay, faultAbort); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("faultDelay", faultDelay) + .add("faultAbort", faultAbort) + .toString(); + } + + static StructOrError fromEnvoyProtoHttpFault(HTTPFault httpFault) { + FaultDelay faultDelay = null; + FaultAbort faultAbort = null; + if (httpFault.hasDelay()) { + faultDelay = FaultDelay.fromEnvoyProtoFaultDelay(httpFault.getDelay()); + } + if (httpFault.hasAbort()) { + StructOrError faultAbortOrError = + FaultAbort.fromEnvoyProtoFaultAbort(httpFault.getAbort()); + if (faultAbortOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "HttpFault contains invalid FaultAbort: " + faultAbortOrError.getErrorDetail()); + } + faultAbort = faultAbortOrError.getStruct(); + } + if (faultDelay == null && faultAbort == null) { + return StructOrError.fromError( + "Invalid HttpFault: neither fault_delay nor fault_abort is specified"); + } + return StructOrError.fromStruct(new HttpFault(faultDelay, faultAbort)); + } + } + + /** + * See corresponding Envoy proto message {@link + * io.envoyproxy.envoy.extensions.filters.common.fault.v3.FaultDelay}. + */ + static final class FaultDelay { + @Nullable + final Long delayNanos; + final int ratePerMillion; + + FaultDelay(@Nullable Long delayNanos, int ratePerMillion) { + this.delayNanos = delayNanos; + this.ratePerMillion = ratePerMillion; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + FaultDelay that = (FaultDelay) o; + return ratePerMillion == that.ratePerMillion + && Objects.equals(delayNanos, that.delayNanos); + } + + @Override + public int hashCode() { + return Objects.hash(delayNanos, ratePerMillion); + } + + @Override + public String toString() { + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) + .add("ratePerMillion", ratePerMillion); + if (delayNanos != null) { + toStringHelper.add("type", "fixed delay"); + toStringHelper.add("delayNanos", delayNanos); + } else { + toStringHelper.add("type", "header delay"); + } + return toStringHelper.toString(); + } + + private static FaultDelay fromEnvoyProtoFaultDelay( + io.envoyproxy.envoy.extensions.filters.common.fault.v3.FaultDelay faultDelay) { + int rate = getRatePerMillion(faultDelay.getPercentage()); + if (faultDelay.hasHeaderDelay()) { + return new FaultDelay(null, rate); + } + long delay = Durations.toNanos(faultDelay.getFixedDelay()); + return new FaultDelay(delay, rate); + } + } + + /** + * See corresponding Envoy proto message {@link + * io.envoyproxy.envoy.extensions.filters.http.fault.v3.FaultAbort}. + */ + static final class FaultAbort { + @Nullable + final Status status; + final int ratePerMillion; + + FaultAbort(@Nullable Status status, int ratePerMillion) { + this.status = status; + this.ratePerMillion = ratePerMillion; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + FaultAbort that = (FaultAbort) o; + return ratePerMillion == that.ratePerMillion + && Objects.equals(status, that.status); + } + + @Override + public int hashCode() { + return Objects.hash(status, ratePerMillion); + } + + @Override + public String toString() { + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) + .add("ratePerMillion", ratePerMillion); + if (status != null) { + toStringHelper.add("type", "fixed status"); + toStringHelper.add("status", status); + } else { + toStringHelper.add("type", "header abort"); + } + return toStringHelper.toString(); + } + + private static StructOrError fromEnvoyProtoFaultAbort( + io.envoyproxy.envoy.extensions.filters.http.fault.v3.FaultAbort faultAbort) { + int rate = getRatePerMillion(faultAbort.getPercentage()); + switch (faultAbort.getErrorTypeCase()) { + case HEADER_ABORT: + return StructOrError.fromStruct(new FaultAbort(null, rate)); + case HTTP_STATUS: + Status status = convertHttpStatus(faultAbort.getHttpStatus()); + return StructOrError.fromStruct(new FaultAbort(status, rate)); + case GRPC_STATUS: + Code code; + try { + code = Code.values()[faultAbort.getGrpcStatus()]; + } catch (ArrayIndexOutOfBoundsException e) { + return StructOrError.fromError("Unknown GRPC_STATUS: " + faultAbort.getGrpcStatus()); + } + return StructOrError.fromStruct(new FaultAbort(code.toStatus(), rate)); + case ERRORTYPE_NOT_SET: + default: + return StructOrError.fromError( + "Unknown error type case: " + faultAbort.getErrorTypeCase()); + } + } + + private static Status convertHttpStatus(int httpCode) { + Status status; + switch (httpCode) { + case 400: + status = Status.INTERNAL; + break; + case 401: + status = Status.UNAUTHENTICATED; + break; + case 403: + status = Status.PERMISSION_DENIED; + break; + case 404: + status = Status.UNIMPLEMENTED; + break; + case 429: + case 502: + case 503: + case 504: + status = Status.UNAVAILABLE; + break; + default: + status = Status.UNKNOWN; + } + return status.withDescription("HTTP code: " + httpCode); + } + } + + private static int getRatePerMillion(FractionalPercent percent) { + int numerator = percent.getNumerator(); + DenominatorType type = percent.getDenominator(); + switch (type) { + case TEN_THOUSAND: + numerator *= 100; + break; + case HUNDRED: + numerator *= 10_000; + break; + case MILLION: + break; + case UNRECOGNIZED: + default: + throw new IllegalArgumentException("Unknown denominator type of " + percent); + } + + if (numerator > 1_000_000 || numerator < 0) { + numerator = 1_000_000; + } + return numerator; + } + /** * See corresponding Envoy proto message {@link io.envoyproxy.envoy.config.route.v3.RouteAction}. */ @@ -1203,7 +1492,13 @@ static StructOrError fromEnvoyProtoRouteAction( weightedClusters = new ArrayList<>(); for (io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight clusterWeight : clusterWeights) { - weightedClusters.add(ClusterWeight.fromEnvoyProtoClusterWeight(clusterWeight)); + StructOrError clusterWeightOrError = + ClusterWeight.fromEnvoyProtoClusterWeight(clusterWeight); + if (clusterWeightOrError.getErrorDetail() != null) { + return StructOrError.fromError("RouteAction contains invalid ClusterWeight: " + + clusterWeightOrError.getErrorDetail()); + } + weightedClusters.add(clusterWeightOrError.getStruct()); } // TODO(chengyuanzhang): validate if the sum of weights equals to total weight. break; @@ -1233,11 +1528,14 @@ static StructOrError fromEnvoyProtoRouteAction( static final class ClusterWeight { private final String name; private final int weight; + @Nullable + private final HttpFault httpFault; @VisibleForTesting - ClusterWeight(String name, int weight) { + ClusterWeight(String name, int weight, @Nullable HttpFault httpFault) { this.name = name; this.weight = weight; + this.httpFault = httpFault; } String getName() { @@ -1248,6 +1546,11 @@ int getWeight() { return weight; } + @Nullable + HttpFault getHttpFault() { + return httpFault; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -1257,26 +1560,45 @@ public boolean equals(Object o) { return false; } ClusterWeight that = (ClusterWeight) o; - return weight == that.weight && Objects.equals(name, that.name); + return weight == that.weight && Objects.equals(name, that.name) + && Objects.equals(httpFault, that.httpFault); } @Override public int hashCode() { - return Objects.hash(name, weight); + return Objects.hash(name, weight, httpFault); } @Override public String toString() { - return MoreObjects.toStringHelper(this) + ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) .add("name", name) - .add("weight", weight) - .toString(); + .add("weight", weight); + if (httpFault != null) { + toStringHelper.add("httpFault", httpFault); + } + return toStringHelper.toString(); } @VisibleForTesting - static ClusterWeight fromEnvoyProtoClusterWeight( + static StructOrError fromEnvoyProtoClusterWeight( io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight proto) { - return new ClusterWeight(proto.getName(), proto.getWeight().getValue()); + HttpFault httpFault = null; + if (parseHttpFaultFilter()) { + Map filterConfigMap = proto.getTypedPerFilterConfigMap(); + if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { + Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); + StructOrError httpFaultOrError = decodeFaultFilterConfig(rawFaultFilterConfig); + if (httpFaultOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "ClusterWeight [" + proto.getName() + "] contains invalid HttpFault filter: " + + httpFaultOrError.getErrorDetail()); + } + httpFault = httpFaultOrError.getStruct(); + } + } + return StructOrError.fromStruct( + new ClusterWeight(proto.getName(), proto.getWeight().getValue(), httpFault)); } } diff --git a/xds/src/main/java/io/grpc/xds/XdsClient.java b/xds/src/main/java/io/grpc/xds/XdsClient.java index 8da3601fd11..809558b088b 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClient.java +++ b/xds/src/main/java/io/grpc/xds/XdsClient.java @@ -23,6 +23,7 @@ import com.google.common.base.MoreObjects.ToStringHelper; import io.grpc.Status; import io.grpc.xds.EnvoyProtoData.DropOverload; +import io.grpc.xds.EnvoyProtoData.HttpFault; import io.grpc.xds.EnvoyProtoData.Locality; import io.grpc.xds.EnvoyProtoData.LocalityLbEndpoints; import io.grpc.xds.EnvoyProtoData.VirtualHost; @@ -54,26 +55,39 @@ static final class LdsUpdate implements ResourceUpdate { // The list virtual hosts that make up the route table. @Nullable final List virtualHosts; - - LdsUpdate(long httpMaxStreamDurationNano, String rdsName) { - this(httpMaxStreamDurationNano, rdsName, null); + // Listener contains the HttpFault filter. + final boolean hasFaultInjection; + @Nullable // Can be null even if hasFaultInjection is true. + final HttpFault httpFault; + + LdsUpdate( + long httpMaxStreamDurationNano, String rdsName, boolean hasFaultInjection, + @Nullable HttpFault httpFault) { + this(httpMaxStreamDurationNano, rdsName, null, hasFaultInjection, httpFault); } - LdsUpdate(long httpMaxStreamDurationNano, List virtualHosts) { - this(httpMaxStreamDurationNano, null, virtualHosts); + LdsUpdate( + long httpMaxStreamDurationNano, List virtualHosts, + boolean hasFaultInjection, @Nullable HttpFault httpFault) { + this(httpMaxStreamDurationNano, null, virtualHosts, hasFaultInjection, httpFault); } - private LdsUpdate(long httpMaxStreamDurationNano, @Nullable String rdsName, - @Nullable List virtualHosts) { + private LdsUpdate( + long httpMaxStreamDurationNano, @Nullable String rdsName, + @Nullable List virtualHosts, boolean hasFaultInjection, + @Nullable HttpFault httpFault) { this.httpMaxStreamDurationNano = httpMaxStreamDurationNano; this.rdsName = rdsName; this.virtualHosts = virtualHosts == null ? null : Collections.unmodifiableList(new ArrayList<>(virtualHosts)); + this.hasFaultInjection = hasFaultInjection; + this.httpFault = httpFault; } @Override public int hashCode() { - return Objects.hash(httpMaxStreamDurationNano, rdsName, virtualHosts); + return Objects.hash( + httpMaxStreamDurationNano, rdsName, virtualHosts, hasFaultInjection, httpFault); } @Override @@ -87,7 +101,9 @@ public boolean equals(Object o) { LdsUpdate that = (LdsUpdate) o; return httpMaxStreamDurationNano == that.httpMaxStreamDurationNano && Objects.equals(rdsName, that.rdsName) - && Objects.equals(virtualHosts, that.virtualHosts); + && Objects.equals(virtualHosts, that.virtualHosts) + && hasFaultInjection == that.hasFaultInjection + && Objects.equals(httpFault, that.httpFault); } @Override @@ -99,6 +115,10 @@ public String toString() { } else { toStringHelper.add("virtualHosts", virtualHosts); } + if (hasFaultInjection) { + toStringHelper.add("faultInjectionEnabled", true) + .add("httpFault", httpFault); + } return toStringHelper.toString(); } } diff --git a/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java b/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java index 600075fc41c..ae10beb66f2 100644 --- a/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java +++ b/xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java @@ -402,7 +402,7 @@ public void convertRouteAction_weightedCluster() { assertThat(struct.getErrorDetail()).isNull(); assertThat(struct.getStruct().getCluster()).isNull(); assertThat(struct.getStruct().getWeightedCluster()).containsExactly( - new ClusterWeight("cluster-foo", 30), new ClusterWeight("cluster-bar", 70)); + new ClusterWeight("cluster-foo", 30, null), new ClusterWeight("cluster-bar", 70, null)); } @Test @@ -547,7 +547,7 @@ public void convertClusterWeight() { io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight.newBuilder() .setName("cluster-foo") .setWeight(UInt32Value.newBuilder().setValue(30)).build(); - ClusterWeight struct = ClusterWeight.fromEnvoyProtoClusterWeight(proto); + ClusterWeight struct = ClusterWeight.fromEnvoyProtoClusterWeight(proto).getStruct(); assertThat(struct.getName()).isEqualTo("cluster-foo"); assertThat(struct.getWeight()).isEqualTo(30); } diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 7df19c0be1e..219a4aee5f4 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -437,7 +437,8 @@ public void resolved_simpleCallSucceeds_routeToWeightedCluster() { new RouteAction( TimeUnit.SECONDS.toNanos(20L), null, Arrays.asList( - new ClusterWeight(cluster1, 20), new ClusterWeight(cluster2, 80)))))); + new ClusterWeight(cluster1, 20, null), + new ClusterWeight(cluster2, 80, null)))))); verify(mockListener).onResult(resolutionResultCaptor.capture()); ResolutionResult result = resolutionResultCaptor.getValue(); assertThat(result.getAddresses()).isEmpty(); @@ -744,7 +745,7 @@ public void run() { if (!resourceName.equals(ldsResource)) { return; } - ldsWatcher.onChanged(new LdsUpdate(httpMaxStreamDurationNano, virtualHosts)); + ldsWatcher.onChanged(new LdsUpdate(httpMaxStreamDurationNano, virtualHosts, false, null)); } }); } @@ -758,7 +759,8 @@ public void run() { } VirtualHost virtualHost = new VirtualHost("virtual-host", Collections.singletonList(AUTHORITY), routes); - ldsWatcher.onChanged(new LdsUpdate(0, Collections.singletonList(virtualHost))); + ldsWatcher.onChanged( + new LdsUpdate(0, Collections.singletonList(virtualHost), false, null)); } }); } @@ -770,7 +772,7 @@ public void run() { if (!resourceName.equals(ldsResource)) { return; } - ldsWatcher.onChanged(new LdsUpdate(0, rdsName)); + ldsWatcher.onChanged(new LdsUpdate(0, rdsName, false, null)); } }); } From 5e8c3e1deb3ea94bfd315392bfc5540a406a8ba7 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Mon, 25 Jan 2021 00:01:45 -0800 Subject: [PATCH 2/7] add tests --- .../main/java/io/grpc/xds/EnvoyProtoData.java | 4 +- .../io/grpc/xds/ClientXdsClientTestBase.java | 75 +++++++++++++++- .../io/grpc/xds/ClientXdsClientV2Test.java | 89 ++++++++++++++++++- .../io/grpc/xds/ClientXdsClientV3Test.java | 85 +++++++++++++++++- 4 files changed, 245 insertions(+), 8 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index b2b6aebef5d..96b35bad6b6 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -1142,10 +1142,10 @@ static StructOrError decodeFaultFilterConfig(Any rawFaultFilterConfig if (rawFaultFilterConfig.getTypeUrl().equals( "type.googleapis.com/envoy.config.filter.http.fault.v2.HTTPFault")) { rawFaultFilterConfig = rawFaultFilterConfig.toBuilder().setTypeUrl( - "type.googleapis.com/envoy.extensions.filter.http.fault.v3.HTTPFault").build(); + "type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault").build(); } if (rawFaultFilterConfig.getTypeUrl().equals( - "type.googleapis.com/envoy.extensions.filter.http.fault.v3.HTTPFault")) { + "type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault")) { HTTPFault httpFaultProto; try { httpFaultProto = rawFaultFilterConfig.unpack(HTTPFault.class); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index 5395e778517..e00de7f3f5d 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -25,9 +25,11 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.protobuf.Any; import com.google.protobuf.Message; +import com.google.protobuf.StringValue; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.SdsSecretConfig; import io.grpc.BindableService; import io.grpc.ManagedChannel; @@ -42,6 +44,7 @@ import io.grpc.testing.GrpcCleanupRule; import io.grpc.xds.AbstractXdsClient.ResourceType; import io.grpc.xds.EnvoyProtoData.DropOverload; +import io.grpc.xds.EnvoyProtoData.HttpFault; import io.grpc.xds.EnvoyProtoData.LbEndpoint; import io.grpc.xds.EnvoyProtoData.Locality; import io.grpc.xds.EnvoyProtoData.LocalityLbEndpoints; @@ -65,6 +68,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Queue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -169,9 +173,11 @@ public boolean shouldAccept(Runnable command) { private ManagedChannel channel; private ClientXdsClient xdsClient; + private boolean originalIsFaultInjectionEnabled; @Before public void setUp() throws IOException { + originalIsFaultInjectionEnabled = EnvoyProtoData.enableFaultInjection; MockitoAnnotations.initMocks(this); when(backoffPolicyProvider.get()).thenReturn(backoffPolicy1, backoffPolicy2); when(backoffPolicy1.nextBackoffNanos()).thenReturn(10L, 100L); @@ -204,6 +210,7 @@ public void setUp() throws IOException { @After public void tearDown() { + EnvoyProtoData.enableFaultInjection = originalIsFaultInjectionEnabled; xdsClient.shutdown(); channel.shutdown(); // channel not owned by XdsClient assertThat(adsEnded.get()).isTrue(); @@ -328,6 +335,55 @@ public void ldsResourceUpdated() { assertThat(ldsUpdateCaptor.getValue().rdsName).isEqualTo(RDS_RESOURCE); } + @Test + public void ldsResourceUpdate_withFaultInjection() { + EnvoyProtoData.enableFaultInjection = true; + + DiscoveryRpcCall call = + startResourceWatcher(ResourceType.LDS, LDS_RESOURCE, ldsResourceWatcher); + List listeners = ImmutableList.of( + Any.pack(mf.buildListener( + LDS_RESOURCE, + mf.buildRouteConfiguration( + "do not care", + ImmutableList.of( + mf.buildVirtualHost( + mf.buildOpaqueRoutes(1), + ImmutableMap.of( + "irrelevant", + Any.pack(StringValue.of("irrelevant")), + "envoy.fault", + mf.buildHttpFaultTypedConfig(300L, 1000, null, null, null))), + mf.buildVirtualHost( + mf.buildOpaqueRoutes(2), + ImmutableMap.of( + "envoy.fault", + mf.buildHttpFaultTypedConfig(null, null, null, 503, 2000))) + )), + ImmutableList.of( + mf.buildHttpFilter("irrelevant", null), + mf.buildHttpFilter("envoy.fault", null) + )))); + call.sendResponse("0", listeners, ResourceType.LDS, "0000"); + + // Client sends an ACK LDS request. + call.verifyRequest(NODE, "0", Collections.singletonList(LDS_RESOURCE), ResourceType.LDS, + "0000"); + verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); + LdsUpdate ldsUpdate = ldsUpdateCaptor.getValue(); + assertThat(ldsUpdate.virtualHosts).hasSize(2); + assertThat(ldsUpdate.hasFaultInjection).isTrue(); + assertThat(ldsUpdate.httpFault).isNull(); + HttpFault httpFault = ldsUpdate.virtualHosts.get(0).getHttpFault(); + assertThat(httpFault.faultDelay.delayNanos).isEqualTo(300); + assertThat(httpFault.faultDelay.ratePerMillion).isEqualTo(1000); + assertThat(httpFault.faultAbort).isNull(); + httpFault = ldsUpdate.virtualHosts.get(1).getHttpFault(); + assertThat(httpFault.faultDelay).isNull(); + assertThat(httpFault.faultAbort.status.getCode()).isEqualTo(Status.Code.UNAVAILABLE); + assertThat(httpFault.faultAbort.ratePerMillion).isEqualTo(2000); + } + @Test public void ldsResourceDeleted() { DiscoveryRpcCall call = @@ -1445,15 +1501,32 @@ protected abstract static class LrsRpcCall { protected abstract static class MessageFactory { - protected abstract Message buildListener(String name, Message routeConfiguration); + protected final Message buildListener(String name, Message routeConfiguration) { + return buildListener(name, routeConfiguration, Collections.emptyList()); + } + + @SuppressWarnings("unchecked") + protected abstract Message buildListener( + String name, Message routeConfiguration, List httpFilters); protected abstract Message buildListenerForRds(String name, String rdsResourceName); + protected abstract Message buildHttpFilter(String name, @Nullable Any typedConfig); + + protected abstract Any buildHttpFaultTypedConfig( + @Nullable Long delayNanos, @Nullable Integer delayRate, + @Nullable Status status, @Nullable Integer httpCode, @Nullable Integer abortRate); + protected abstract Message buildRouteConfiguration(String name, List virtualHostList); protected abstract List buildOpaqueVirtualHosts(int num); + protected abstract Message buildVirtualHost( + List routes, Map typedConfigMap); + + protected abstract List buildOpaqueRoutes(int num); + protected abstract Message buildEdsCluster(String clusterName, @Nullable String edsServiceName, boolean enableLrs, @Nullable Message upstreamTlsContext, @Nullable Message circuitBreakers); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java index a2bfb8ee964..b7f2a372ad6 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java @@ -65,9 +65,16 @@ 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.cluster.aggregate.v2alpha.ClusterConfig; +import io.envoyproxy.envoy.config.filter.fault.v2.FaultDelay; +import io.envoyproxy.envoy.config.filter.fault.v2.FaultDelay.HeaderDelay; +import io.envoyproxy.envoy.config.filter.http.fault.v2.FaultAbort; +import io.envoyproxy.envoy.config.filter.http.fault.v2.FaultAbort.HeaderAbort; +import io.envoyproxy.envoy.config.filter.http.fault.v2.HTTPFault; import io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager; +import io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2.HttpFilter; import io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2.Rds; import io.envoyproxy.envoy.config.listener.v2.ApiListener; import io.envoyproxy.envoy.service.discovery.v2.AggregatedDiscoveryServiceGrpc.AggregatedDiscoveryServiceImplBase; @@ -79,12 +86,14 @@ import io.grpc.BindableService; import io.grpc.Context; import io.grpc.Context.CancellationListener; +import io.grpc.Status; import io.grpc.stub.StreamObserver; import io.grpc.xds.AbstractXdsClient.ResourceType; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import javax.annotation.Nullable; import org.junit.runner.RunWith; @@ -232,8 +241,10 @@ protected void sendResponse(List clusters, long loadReportIntervalNano) private static class MessageFactoryV2 extends MessageFactory { + @SuppressWarnings("unchecked") @Override - protected Message buildListener(String name, Message routeConfiguration) { + protected Message buildListener( + String name, Message routeConfiguration, List httpFilters) { return Listener.newBuilder() .setName(name) .setAddress(Address.getDefaultInstance()) @@ -241,7 +252,9 @@ protected Message buildListener(String name, Message routeConfiguration) { .setApiListener( ApiListener.newBuilder().setApiListener(Any.pack( HttpConnectionManager.newBuilder() - .setRouteConfig((RouteConfiguration) routeConfiguration).build()))) + .setRouteConfig((RouteConfiguration) routeConfiguration) + .addAllHttpFilters((List) httpFilters) + .build()))) .build(); } @@ -264,6 +277,50 @@ protected Message buildListenerForRds(String name, String rdsResourceName) { .build(); } + @Override + protected Message buildHttpFilter(String name, @Nullable Any typedConfig) { + HttpFilter.Builder builder = HttpFilter.newBuilder().setName(name); + if (typedConfig != null) { + builder.setTypedConfig(typedConfig); + } + return builder.build(); + } + + @Override + protected Any buildHttpFaultTypedConfig( + @Nullable Long delayNanos, @Nullable Integer delayRate, + @Nullable Status status, @Nullable Integer httpCode, @Nullable Integer abortRate) { + HTTPFault.Builder builder = HTTPFault.newBuilder(); + if (delayRate != null) { + FaultDelay.Builder delayBuilder + = FaultDelay.newBuilder(); + delayBuilder.setPercentage( + FractionalPercent.newBuilder() + .setNumerator(delayRate).setDenominator(DenominatorType.MILLION)); + if (delayNanos != null) { + delayBuilder.setFixedDelay(Durations.fromNanos(delayNanos)); + } else { + delayBuilder.setHeaderDelay(HeaderDelay.newBuilder()); + } + builder.setDelay(delayBuilder); + } + if (abortRate != null) { + FaultAbort.Builder abortBuilder = FaultAbort.newBuilder(); + abortBuilder.setPercentage( + FractionalPercent.newBuilder() + .setNumerator(abortRate).setDenominator(DenominatorType.MILLION)); + if (status != null) { + throw new UnsupportedOperationException(); + } else if (httpCode != null) { + abortBuilder.setHttpStatus(httpCode); + } else { + abortBuilder.setHeaderAbort(HeaderAbort.newBuilder()); + } + builder.setAbort(abortBuilder); + } + return Any.pack(builder.build()); + } + @Override protected Message buildRouteConfiguration(String name, List virtualHostList) { RouteConfiguration.Builder builder = RouteConfiguration.newBuilder(); @@ -285,7 +342,7 @@ protected List buildOpaqueVirtualHosts(int num) { .addRoutes( Route.newBuilder() .setRoute(RouteAction.newBuilder().setCluster("do not care")) - .setMatch(io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder() + .setMatch(RouteMatch.newBuilder() .setPrefix("do not care"))) .build(); virtualHosts.add(virtualHost); @@ -293,6 +350,32 @@ protected List buildOpaqueVirtualHosts(int num) { return virtualHosts; } + @SuppressWarnings("unchecked") + @Override + protected Message buildVirtualHost( + List routes, Map typedConfigMap) { + return VirtualHost.newBuilder() + .setName("do not care") + .addDomains("do not care") + .addAllRoutes((List) routes) + .putAllTypedPerFilterConfig(typedConfigMap) + .build(); + } + + @Override + protected List buildOpaqueRoutes(int num) { + List routes = new ArrayList<>(num); + for (int i = 0; i < num; i++) { + Route route = + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("do not care")) + .setMatch(RouteMatch.newBuilder().setPrefix("do not care")) + .build(); + routes.add(route); + } + return routes; + } + @Override protected Message buildEdsCluster(String clusterName, @Nullable String edsServiceName, boolean enableLrs, @Nullable Message upstreamTlsContext, diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java index b9de6bd97d1..d7145a585d2 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java @@ -64,7 +64,13 @@ import io.envoyproxy.envoy.config.route.v3.RouteMatch; import io.envoyproxy.envoy.config.route.v3.VirtualHost; import io.envoyproxy.envoy.extensions.clusters.aggregate.v3.ClusterConfig; +import io.envoyproxy.envoy.extensions.filters.common.fault.v3.FaultDelay; +import io.envoyproxy.envoy.extensions.filters.common.fault.v3.FaultDelay.HeaderDelay; +import io.envoyproxy.envoy.extensions.filters.http.fault.v3.FaultAbort; +import io.envoyproxy.envoy.extensions.filters.http.fault.v3.FaultAbort.HeaderAbort; +import io.envoyproxy.envoy.extensions.filters.http.fault.v3.HTTPFault; import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager; +import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter; import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.Rds; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.SdsSecretConfig; @@ -80,12 +86,14 @@ import io.grpc.BindableService; import io.grpc.Context; import io.grpc.Context.CancellationListener; +import io.grpc.Status; import io.grpc.stub.StreamObserver; import io.grpc.xds.AbstractXdsClient.ResourceType; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import javax.annotation.Nullable; import org.junit.runner.RunWith; @@ -233,8 +241,10 @@ protected void sendResponse(List clusters, long loadReportIntervalNano) private static class MessageFactoryV3 extends MessageFactory { + @SuppressWarnings("unchecked") @Override - protected Message buildListener(String name, Message routeConfiguration) { + protected Message buildListener( + String name, Message routeConfiguration, List httpFilters) { return Listener.newBuilder() .setName(name) .setAddress(Address.getDefaultInstance()) @@ -242,7 +252,9 @@ protected Message buildListener(String name, Message routeConfiguration) { .setApiListener( ApiListener.newBuilder().setApiListener(Any.pack( HttpConnectionManager.newBuilder() - .setRouteConfig((RouteConfiguration) routeConfiguration).build()))) + .setRouteConfig((RouteConfiguration) routeConfiguration) + .addAllHttpFilters((List) httpFilters) + .build()))) .build(); } @@ -265,6 +277,49 @@ protected Message buildListenerForRds(String name, String rdsResourceName) { .build(); } + @Override + protected Message buildHttpFilter(String name, @Nullable Any typedConfig) { + HttpFilter.Builder builder = HttpFilter.newBuilder().setName(name); + if (typedConfig != null) { + builder.setTypedConfig(typedConfig); + } + return builder.build(); + } + + @Override + protected Any buildHttpFaultTypedConfig( + @Nullable Long delayNanos, @Nullable Integer delayRate, + @Nullable Status status, @Nullable Integer httpCode, @Nullable Integer abortRate) { + HTTPFault.Builder builder = HTTPFault.newBuilder(); + if (delayRate != null) { + FaultDelay.Builder delayBuilder = FaultDelay.newBuilder(); + delayBuilder.setPercentage( + FractionalPercent.newBuilder() + .setNumerator(delayRate).setDenominator(DenominatorType.MILLION)); + if (delayNanos != null) { + delayBuilder.setFixedDelay(Durations.fromNanos(delayNanos)); + } else { + delayBuilder.setHeaderDelay(HeaderDelay.newBuilder()); + } + builder.setDelay(delayBuilder); + } + if (abortRate != null) { + FaultAbort.Builder abortBuilder = FaultAbort.newBuilder(); + abortBuilder.setPercentage( + FractionalPercent.newBuilder() + .setNumerator(abortRate).setDenominator(DenominatorType.MILLION)); + if (status != null) { + abortBuilder.setGrpcStatus(status.getCode().value()); + } else if (httpCode != null) { + abortBuilder.setHttpStatus(httpCode); + } else { + abortBuilder.setHeaderAbort(HeaderAbort.newBuilder()); + } + builder.setAbort(abortBuilder); + } + return Any.pack(builder.build()); + } + @Override protected Message buildRouteConfiguration(String name, List virtualHostList) { RouteConfiguration.Builder builder = RouteConfiguration.newBuilder(); @@ -293,6 +348,32 @@ protected List buildOpaqueVirtualHosts(int num) { return virtualHosts; } + @SuppressWarnings("unchecked") + @Override + protected Message buildVirtualHost( + List routes, Map typedConfigMap) { + return VirtualHost.newBuilder() + .setName("do not care") + .addDomains("do not care") + .addAllRoutes((List) routes) + .putAllTypedPerFilterConfig(typedConfigMap) + .build(); + } + + @Override + protected List buildOpaqueRoutes(int num) { + List routes = new ArrayList<>(num); + for (int i = 0; i < num; i++) { + Route route = + Route.newBuilder() + .setRoute(RouteAction.newBuilder().setCluster("do not care")) + .setMatch(RouteMatch.newBuilder().setPrefix("do not care")) + .build(); + routes.add(route); + } + return routes; + } + @Override protected Message buildEdsCluster(String clusterName, @Nullable String edsServiceName, boolean enableLrs, @Nullable Message upstreamTlsContext, From 502e0a3e592c7abbad5920a3fd2d0c4529ee710c Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Wed, 27 Jan 2021 10:54:01 -0800 Subject: [PATCH 3/7] add missing fields maxActiveFaults etc --- .../main/java/io/grpc/xds/EnvoyProtoData.java | 54 ++++++++++++++++--- .../io/grpc/xds/ClientXdsClientTestBase.java | 17 ++++-- .../io/grpc/xds/ClientXdsClientV2Test.java | 10 +++- .../io/grpc/xds/ClientXdsClientV3Test.java | 10 +++- 4 files changed, 77 insertions(+), 14 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index 96b35bad6b6..d4132d3bd4b 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -1171,10 +1171,22 @@ static final class HttpFault { final FaultDelay faultDelay; @Nullable final FaultAbort faultAbort; + String upstreamCluster; + List downstreamNodes; + List headers; + @Nullable + Integer maxActiveFaults; - HttpFault(@Nullable FaultDelay faultDelay, @Nullable FaultAbort faultAbort) { + private HttpFault( + @Nullable FaultDelay faultDelay, @Nullable FaultAbort faultAbort, String upstreamCluster, + List downstreamNodes, List headers, + @Nullable Integer maxActiveFaults) { this.faultDelay = faultDelay; this.faultAbort = faultAbort; + this.upstreamCluster = upstreamCluster; + this.downstreamNodes = downstreamNodes; + this.headers = headers; + this.maxActiveFaults = maxActiveFaults; } @Override @@ -1187,12 +1199,17 @@ public boolean equals(Object o) { } HttpFault httpFault = (HttpFault) o; return Objects.equals(faultDelay, httpFault.faultDelay) - && Objects.equals(faultAbort, httpFault.faultAbort); + && Objects.equals(faultAbort, httpFault.faultAbort) + && Objects.equals(upstreamCluster, httpFault.upstreamCluster) + && Objects.equals(downstreamNodes, httpFault.downstreamNodes) + && Objects.equals(headers, httpFault.headers) + && Objects.equals(maxActiveFaults, httpFault.maxActiveFaults); } @Override public int hashCode() { - return Objects.hash(faultDelay, faultAbort); + return Objects.hash( + faultDelay, faultAbort, upstreamCluster, downstreamNodes, headers, maxActiveFaults); } @Override @@ -1200,6 +1217,10 @@ public String toString() { return MoreObjects.toStringHelper(this) .add("faultDelay", faultDelay) .add("faultAbort", faultAbort) + .add("upstreamCluster", upstreamCluster) + .add("downstreamNodes", downstreamNodes) + .add("headers", headers) + .add("maxActiveFaults", maxActiveFaults) .toString(); } @@ -1222,7 +1243,28 @@ static StructOrError fromEnvoyProtoHttpFault(HTTPFault httpFault) { return StructOrError.fromError( "Invalid HttpFault: neither fault_delay nor fault_abort is specified"); } - return StructOrError.fromStruct(new HttpFault(faultDelay, faultAbort)); + String upstreamCluster = httpFault.getUpstreamCluster(); + List downstreamNodes = httpFault.getDownstreamNodesList(); + List headers = new ArrayList<>(); + for (io.envoyproxy.envoy.config.route.v3.HeaderMatcher proto : httpFault.getHeadersList()) { + StructOrError headerMatcherOrError = + Route.convertEnvoyProtoHeaderMatcher(proto); + if (headerMatcherOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "HttpFault contains invalid header matcher: " + + headerMatcherOrError.getErrorDetail()); + } + headers.add(headerMatcherOrError.getStruct()); + } + Integer maxActiveFaults = null; + if (httpFault.hasMaxActiveFaults()) { + maxActiveFaults = httpFault.getMaxActiveFaults().getValue(); + if (maxActiveFaults < 0) { + maxActiveFaults = Integer.MAX_VALUE; + } + } + return StructOrError.fromStruct(new HttpFault( + faultDelay, faultAbort, upstreamCluster, downstreamNodes, headers, maxActiveFaults)); } } @@ -1235,7 +1277,7 @@ static final class FaultDelay { final Long delayNanos; final int ratePerMillion; - FaultDelay(@Nullable Long delayNanos, int ratePerMillion) { + private FaultDelay(@Nullable Long delayNanos, int ratePerMillion) { this.delayNanos = delayNanos; this.ratePerMillion = ratePerMillion; } @@ -1291,7 +1333,7 @@ static final class FaultAbort { final Status status; final int ratePerMillion; - FaultAbort(@Nullable Status status, int ratePerMillion) { + private FaultAbort(@Nullable Status status, int ratePerMillion) { this.status = status; this.ratePerMillion = ratePerMillion; } diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index e00de7f3f5d..479690cb075 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -353,12 +353,16 @@ public void ldsResourceUpdate_withFaultInjection() { "irrelevant", Any.pack(StringValue.of("irrelevant")), "envoy.fault", - mf.buildHttpFaultTypedConfig(300L, 1000, null, null, null))), + mf.buildHttpFaultTypedConfig( + 300L, 1000, "cluster1", ImmutableList.of(), 100, null, null, + null))), mf.buildVirtualHost( mf.buildOpaqueRoutes(2), ImmutableMap.of( "envoy.fault", - mf.buildHttpFaultTypedConfig(null, null, null, 503, 2000))) + mf.buildHttpFaultTypedConfig( + null, null, "cluster2", ImmutableList.of(), 101, null, 503, + 2000))) )), ImmutableList.of( mf.buildHttpFilter("irrelevant", null), @@ -378,10 +382,14 @@ public void ldsResourceUpdate_withFaultInjection() { assertThat(httpFault.faultDelay.delayNanos).isEqualTo(300); assertThat(httpFault.faultDelay.ratePerMillion).isEqualTo(1000); assertThat(httpFault.faultAbort).isNull(); + assertThat(httpFault.upstreamCluster).isEqualTo("cluster1"); + assertThat(httpFault.maxActiveFaults).isEqualTo(100); httpFault = ldsUpdate.virtualHosts.get(1).getHttpFault(); assertThat(httpFault.faultDelay).isNull(); assertThat(httpFault.faultAbort.status.getCode()).isEqualTo(Status.Code.UNAVAILABLE); assertThat(httpFault.faultAbort.ratePerMillion).isEqualTo(2000); + assertThat(httpFault.upstreamCluster).isEqualTo("cluster2"); + assertThat(httpFault.maxActiveFaults).isEqualTo(101); } @Test @@ -1514,8 +1522,9 @@ protected abstract Message buildListener( protected abstract Message buildHttpFilter(String name, @Nullable Any typedConfig); protected abstract Any buildHttpFaultTypedConfig( - @Nullable Long delayNanos, @Nullable Integer delayRate, - @Nullable Status status, @Nullable Integer httpCode, @Nullable Integer abortRate); + @Nullable Long delayNanos, @Nullable Integer delayRate, String upstreamCluster, + List downstreamNodes, @Nullable Integer maxActiveFaults, @Nullable Status status, + @Nullable Integer httpCode, @Nullable Integer abortRate); protected abstract Message buildRouteConfiguration(String name, List virtualHostList); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java index b7f2a372ad6..341bac7a720 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java @@ -288,8 +288,9 @@ protected Message buildHttpFilter(String name, @Nullable Any typedConfig) { @Override protected Any buildHttpFaultTypedConfig( - @Nullable Long delayNanos, @Nullable Integer delayRate, - @Nullable Status status, @Nullable Integer httpCode, @Nullable Integer abortRate) { + @Nullable Long delayNanos, @Nullable Integer delayRate, String upstreamCluster, + List downstreamNodes, @Nullable Integer maxActiveFaults, @Nullable Status status, + @Nullable Integer httpCode, @Nullable Integer abortRate) { HTTPFault.Builder builder = HTTPFault.newBuilder(); if (delayRate != null) { FaultDelay.Builder delayBuilder @@ -318,6 +319,11 @@ protected Any buildHttpFaultTypedConfig( } builder.setAbort(abortBuilder); } + builder.setUpstreamCluster(upstreamCluster); + builder.addAllDownstreamNodes(downstreamNodes); + if (maxActiveFaults != null) { + builder.setMaxActiveFaults(UInt32Value.of(maxActiveFaults)); + } return Any.pack(builder.build()); } diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java index d7145a585d2..dca11b39846 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java @@ -288,8 +288,9 @@ protected Message buildHttpFilter(String name, @Nullable Any typedConfig) { @Override protected Any buildHttpFaultTypedConfig( - @Nullable Long delayNanos, @Nullable Integer delayRate, - @Nullable Status status, @Nullable Integer httpCode, @Nullable Integer abortRate) { + @Nullable Long delayNanos, @Nullable Integer delayRate, String upstreamCluster, + List downstreamNodes, @Nullable Integer maxActiveFaults, @Nullable Status status, + @Nullable Integer httpCode, @Nullable Integer abortRate) { HTTPFault.Builder builder = HTTPFault.newBuilder(); if (delayRate != null) { FaultDelay.Builder delayBuilder = FaultDelay.newBuilder(); @@ -317,6 +318,11 @@ protected Any buildHttpFaultTypedConfig( } builder.setAbort(abortBuilder); } + builder.setUpstreamCluster(upstreamCluster); + builder.addAllDownstreamNodes(downstreamNodes); + if (maxActiveFaults != null) { + builder.setMaxActiveFaults(UInt32Value.of(maxActiveFaults)); + } return Any.pack(builder.build()); } From d91cb37725067f45c7e94eee87b1faac40912e35 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Wed, 27 Jan 2021 11:23:06 -0800 Subject: [PATCH 4/7] use Status.fromCodeValue --- .../main/java/io/grpc/xds/EnvoyProtoData.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index d4132d3bd4b..be4877be7f3 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -36,7 +36,6 @@ import io.envoyproxy.envoy.type.v3.FractionalPercent.DenominatorType; import io.grpc.EquivalentAddressGroup; import io.grpc.Status; -import io.grpc.Status.Code; import io.grpc.xds.RouteMatch.FractionMatcher; import io.grpc.xds.RouteMatch.HeaderMatcher; import io.grpc.xds.RouteMatch.PathMatcher; @@ -1372,25 +1371,23 @@ public String toString() { private static StructOrError fromEnvoyProtoFaultAbort( io.envoyproxy.envoy.extensions.filters.http.fault.v3.FaultAbort faultAbort) { int rate = getRatePerMillion(faultAbort.getPercentage()); + Status status; switch (faultAbort.getErrorTypeCase()) { case HEADER_ABORT: - return StructOrError.fromStruct(new FaultAbort(null, rate)); + status = null; + break; case HTTP_STATUS: - Status status = convertHttpStatus(faultAbort.getHttpStatus()); - return StructOrError.fromStruct(new FaultAbort(status, rate)); + status = convertHttpStatus(faultAbort.getHttpStatus()); + break; case GRPC_STATUS: - Code code; - try { - code = Code.values()[faultAbort.getGrpcStatus()]; - } catch (ArrayIndexOutOfBoundsException e) { - return StructOrError.fromError("Unknown GRPC_STATUS: " + faultAbort.getGrpcStatus()); - } - return StructOrError.fromStruct(new FaultAbort(code.toStatus(), rate)); + status = Status.fromCodeValue(faultAbort.getGrpcStatus()); + break; case ERRORTYPE_NOT_SET: default: return StructOrError.fromError( "Unknown error type case: " + faultAbort.getErrorTypeCase()); } + return StructOrError.fromStruct(new FaultAbort(status, rate)); } private static Status convertHttpStatus(int httpCode) { From 35121ca70e08e114f4f2dbe3a6011340ef2d2d2e Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Wed, 27 Jan 2021 11:46:13 -0800 Subject: [PATCH 5/7] refactor decodeFaultFilterConfig --- .../java/io/grpc/xds/ClientXdsClient.java | 3 +- .../main/java/io/grpc/xds/EnvoyProtoData.java | 46 +++++++++---------- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index 76a303a97bf..c16da4df6a3 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static io.grpc.xds.EnvoyProtoData.HTTP_FAULT_FILTER_NAME; import static io.grpc.xds.EnvoyProtoData.TRANSPORT_SOCKET_NAME_TLS; -import static io.grpc.xds.EnvoyProtoData.decodeFaultFilterConfig; import static io.grpc.xds.EnvoyProtoData.parseHttpFaultFilter; import com.google.common.annotations.VisibleForTesting; @@ -177,7 +176,7 @@ protected void handleLdsResponse(String versionInfo, List resources, String hasFaultInjection = true; if (httpFilter.hasTypedConfig()) { StructOrError httpFaultOrError = - decodeFaultFilterConfig(httpFilter.getTypedConfig()); + HttpFault.decodeFaultFilterConfig(httpFilter.getTypedConfig()); if (httpFaultOrError.getErrorDetail() != null) { nackResponse(ResourceType.LDS, nonce, "Listener " + listenerName + " contains invalid HttpFault filter: " diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index be4877be7f3..d62fe401461 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -874,7 +874,8 @@ static StructOrError fromEnvoyProtoVirtualHost( Map filterConfigMap = proto.getTypedPerFilterConfigMap(); if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); - StructOrError httpFaultOrError = decodeFaultFilterConfig(rawFaultFilterConfig); + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); if (httpFaultOrError.getErrorDetail() != null) { return StructOrError.fromError( "Virtual host [" + name + "] contains invalid HttpFault filter : " @@ -992,7 +993,8 @@ static StructOrError fromEnvoyProtoRoute( Map filterConfigMap = proto.getTypedPerFilterConfigMap(); if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); - StructOrError httpFaultOrError = decodeFaultFilterConfig(rawFaultFilterConfig); + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); if (httpFaultOrError.getErrorDetail() != null) { return StructOrError.fromError( "Route [" + proto.getName() + "] contains invalid HttpFault filter: " @@ -1137,26 +1139,6 @@ static StructOrError convertEnvoyProtoHeaderMatcher( } } - static StructOrError decodeFaultFilterConfig(Any rawFaultFilterConfig) { - if (rawFaultFilterConfig.getTypeUrl().equals( - "type.googleapis.com/envoy.config.filter.http.fault.v2.HTTPFault")) { - rawFaultFilterConfig = rawFaultFilterConfig.toBuilder().setTypeUrl( - "type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault").build(); - } - if (rawFaultFilterConfig.getTypeUrl().equals( - "type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault")) { - HTTPFault httpFaultProto; - try { - httpFaultProto = rawFaultFilterConfig.unpack(HTTPFault.class); - } catch (InvalidProtocolBufferException e) { - return StructOrError.fromError("Invalid proto: " + e); - } - return HttpFault.fromEnvoyProtoHttpFault(httpFaultProto); - } - return StructOrError.fromError( - "Invalid type url for envoy.fault filter: " + rawFaultFilterConfig.getTypeUrl()); - } - static boolean parseHttpFaultFilter() { return enableFaultInjection; } @@ -1223,7 +1205,22 @@ public String toString() { .toString(); } - static StructOrError fromEnvoyProtoHttpFault(HTTPFault httpFault) { + static StructOrError decodeFaultFilterConfig(Any rawFaultFilterConfig) { + if (rawFaultFilterConfig.getTypeUrl().equals( + "type.googleapis.com/envoy.config.filter.http.fault.v2.HTTPFault")) { + rawFaultFilterConfig = rawFaultFilterConfig.toBuilder().setTypeUrl( + "type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault").build(); + } + HTTPFault httpFaultProto; + try { + httpFaultProto = rawFaultFilterConfig.unpack(HTTPFault.class); + } catch (InvalidProtocolBufferException e) { + return StructOrError.fromError("Invalid proto: " + e); + } + return fromEnvoyProtoHttpFault(httpFaultProto); + } + + private static StructOrError fromEnvoyProtoHttpFault(HTTPFault httpFault) { FaultDelay faultDelay = null; FaultAbort faultAbort = null; if (httpFault.hasDelay()) { @@ -1627,7 +1624,8 @@ static StructOrError fromEnvoyProtoClusterWeight( Map filterConfigMap = proto.getTypedPerFilterConfigMap(); if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); - StructOrError httpFaultOrError = decodeFaultFilterConfig(rawFaultFilterConfig); + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); if (httpFaultOrError.getErrorDetail() != null) { return StructOrError.fromError( "ClusterWeight [" + proto.getName() + "] contains invalid HttpFault filter: " From f8a26e9d81f011d5e80a86459a190445c6ee1ee3 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Wed, 27 Jan 2021 11:54:43 -0800 Subject: [PATCH 6/7] use explicit boolean for headerDelay and headerAbort --- .../main/java/io/grpc/xds/EnvoyProtoData.java | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index d62fe401461..fc20322ffae 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -1271,10 +1271,12 @@ private static StructOrError fromEnvoyProtoHttpFault(HTTPFault httpFa static final class FaultDelay { @Nullable final Long delayNanos; + final boolean headerDelay; final int ratePerMillion; - private FaultDelay(@Nullable Long delayNanos, int ratePerMillion) { + private FaultDelay(@Nullable Long delayNanos, boolean headerDelay, int ratePerMillion) { this.delayNanos = delayNanos; + this.headerDelay = headerDelay; this.ratePerMillion = ratePerMillion; } @@ -1288,23 +1290,24 @@ public boolean equals(Object o) { } FaultDelay that = (FaultDelay) o; return ratePerMillion == that.ratePerMillion + && headerDelay == that.headerDelay && Objects.equals(delayNanos, that.delayNanos); } @Override public int hashCode() { - return Objects.hash(delayNanos, ratePerMillion); + return Objects.hash(delayNanos, headerDelay, ratePerMillion); } @Override public String toString() { ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) .add("ratePerMillion", ratePerMillion); - if (delayNanos != null) { + if (headerDelay) { + toStringHelper.add("type", "header delay"); + } else { toStringHelper.add("type", "fixed delay"); toStringHelper.add("delayNanos", delayNanos); - } else { - toStringHelper.add("type", "header delay"); } return toStringHelper.toString(); } @@ -1313,10 +1316,10 @@ private static FaultDelay fromEnvoyProtoFaultDelay( io.envoyproxy.envoy.extensions.filters.common.fault.v3.FaultDelay faultDelay) { int rate = getRatePerMillion(faultDelay.getPercentage()); if (faultDelay.hasHeaderDelay()) { - return new FaultDelay(null, rate); + return new FaultDelay(null, true, rate); } long delay = Durations.toNanos(faultDelay.getFixedDelay()); - return new FaultDelay(delay, rate); + return new FaultDelay(delay, false, rate); } } @@ -1327,10 +1330,12 @@ private static FaultDelay fromEnvoyProtoFaultDelay( static final class FaultAbort { @Nullable final Status status; + final boolean headerAbort; final int ratePerMillion; - private FaultAbort(@Nullable Status status, int ratePerMillion) { + private FaultAbort(@Nullable Status status, boolean headerAbort, int ratePerMillion) { this.status = status; + this.headerAbort = headerAbort; this.ratePerMillion = ratePerMillion; } @@ -1344,23 +1349,24 @@ public boolean equals(Object o) { } FaultAbort that = (FaultAbort) o; return ratePerMillion == that.ratePerMillion + && headerAbort == that.headerAbort && Objects.equals(status, that.status); } @Override public int hashCode() { - return Objects.hash(status, ratePerMillion); + return Objects.hash(status, headerAbort, ratePerMillion); } @Override public String toString() { ToStringHelper toStringHelper = MoreObjects.toStringHelper(this) .add("ratePerMillion", ratePerMillion); - if (status != null) { + if (headerAbort) { + toStringHelper.add("type", "header abort"); + } else { toStringHelper.add("type", "fixed status"); toStringHelper.add("status", status); - } else { - toStringHelper.add("type", "header abort"); } return toStringHelper.toString(); } @@ -1368,10 +1374,11 @@ public String toString() { private static StructOrError fromEnvoyProtoFaultAbort( io.envoyproxy.envoy.extensions.filters.http.fault.v3.FaultAbort faultAbort) { int rate = getRatePerMillion(faultAbort.getPercentage()); - Status status; + boolean headerAbort = false; + Status status = null; switch (faultAbort.getErrorTypeCase()) { case HEADER_ABORT: - status = null; + headerAbort = true; break; case HTTP_STATUS: status = convertHttpStatus(faultAbort.getHttpStatus()); @@ -1384,7 +1391,7 @@ private static StructOrError fromEnvoyProtoFaultAbort( return StructOrError.fromError( "Unknown error type case: " + faultAbort.getErrorTypeCase()); } - return StructOrError.fromStruct(new FaultAbort(status, rate)); + return StructOrError.fromStruct(new FaultAbort(status, headerAbort, rate)); } private static Status convertHttpStatus(int httpCode) { From 537477dcf1c946446d1707fdde4cf8817651a222 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Wed, 27 Jan 2021 14:36:45 -0800 Subject: [PATCH 7/7] parse regardless of feature flag --- .../java/io/grpc/xds/ClientXdsClient.java | 29 ++++---- .../main/java/io/grpc/xds/EnvoyProtoData.java | 73 ++++++++----------- .../io/grpc/xds/ClientXdsClientTestBase.java | 5 -- 3 files changed, 43 insertions(+), 64 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index c16da4df6a3..9344e2ec90e 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static io.grpc.xds.EnvoyProtoData.HTTP_FAULT_FILTER_NAME; import static io.grpc.xds.EnvoyProtoData.TRANSPORT_SOCKET_NAME_TLS; -import static io.grpc.xds.EnvoyProtoData.parseHttpFaultFilter; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; @@ -170,23 +169,21 @@ protected void handleLdsResponse(String versionInfo, List resources, String boolean hasFaultInjection = false; HttpFault httpFault = null; List httpFilters = hcm.getHttpFiltersList(); - if (parseHttpFaultFilter()) { - for (HttpFilter httpFilter : httpFilters) { - if (HTTP_FAULT_FILTER_NAME.equals(httpFilter.getName())) { - hasFaultInjection = true; - if (httpFilter.hasTypedConfig()) { - StructOrError httpFaultOrError = - HttpFault.decodeFaultFilterConfig(httpFilter.getTypedConfig()); - if (httpFaultOrError.getErrorDetail() != null) { - nackResponse(ResourceType.LDS, nonce, - "Listener " + listenerName + " contains invalid HttpFault filter: " - + httpFaultOrError.getErrorDetail()); - return; - } - httpFault = httpFaultOrError.getStruct(); + for (HttpFilter httpFilter : httpFilters) { + if (HTTP_FAULT_FILTER_NAME.equals(httpFilter.getName())) { + hasFaultInjection = true; + if (httpFilter.hasTypedConfig()) { + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(httpFilter.getTypedConfig()); + if (httpFaultOrError.getErrorDetail() != null) { + nackResponse(ResourceType.LDS, nonce, + "Listener " + listenerName + " contains invalid HttpFault filter: " + + httpFaultOrError.getErrorDetail()); + return; } - break; + httpFault = httpFaultOrError.getStruct(); } + break; } } if (hcm.hasRouteConfig()) { diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index fc20322ffae..b7fe420485e 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -66,9 +66,6 @@ final class EnvoyProtoData { static final String TRANSPORT_SOCKET_NAME_TLS = "envoy.transport_sockets.tls"; static final String HTTP_FAULT_FILTER_NAME = "envoy.fault"; - @VisibleForTesting - static boolean enableFaultInjection = - Boolean.parseBoolean(System.getenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION")); // Prevent instantiation. private EnvoyProtoData() { @@ -870,19 +867,17 @@ static StructOrError fromEnvoyProtoVirtualHost( routes.add(route.getStruct()); } HttpFault httpFault = null; - if (parseHttpFaultFilter()) { - Map filterConfigMap = proto.getTypedPerFilterConfigMap(); - if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { - Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); - StructOrError httpFaultOrError = - HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); - if (httpFaultOrError.getErrorDetail() != null) { - return StructOrError.fromError( - "Virtual host [" + name + "] contains invalid HttpFault filter : " - + httpFaultOrError.getErrorDetail()); - } - httpFault = httpFaultOrError.getStruct(); + Map filterConfigMap = proto.getTypedPerFilterConfigMap(); + if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { + Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); + if (httpFaultOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "Virtual host [" + name + "] contains invalid HttpFault filter : " + + httpFaultOrError.getErrorDetail()); } + httpFault = httpFaultOrError.getStruct(); } return StructOrError.fromStruct( new VirtualHost( @@ -989,19 +984,17 @@ static StructOrError fromEnvoyProtoRoute( } HttpFault httpFault = null; - if (parseHttpFaultFilter()) { - Map filterConfigMap = proto.getTypedPerFilterConfigMap(); - if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { - Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); - StructOrError httpFaultOrError = - HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); - if (httpFaultOrError.getErrorDetail() != null) { - return StructOrError.fromError( - "Route [" + proto.getName() + "] contains invalid HttpFault filter: " - + httpFaultOrError.getErrorDetail()); - } - httpFault = httpFaultOrError.getStruct(); + Map filterConfigMap = proto.getTypedPerFilterConfigMap(); + if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { + Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); + if (httpFaultOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "Route [" + proto.getName() + "] contains invalid HttpFault filter: " + + httpFaultOrError.getErrorDetail()); } + httpFault = httpFaultOrError.getStruct(); } return StructOrError.fromStruct( new Route(routeMatch.getStruct(), routeAction.getStruct(), httpFault)); @@ -1139,10 +1132,6 @@ static StructOrError convertEnvoyProtoHeaderMatcher( } } - static boolean parseHttpFaultFilter() { - return enableFaultInjection; - } - /** * See corresponding Envoy proto message {@link * io.envoyproxy.envoy.extensions.filters.http.fault.v3.HTTPFault}. @@ -1627,19 +1616,17 @@ public String toString() { static StructOrError fromEnvoyProtoClusterWeight( io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight proto) { HttpFault httpFault = null; - if (parseHttpFaultFilter()) { - Map filterConfigMap = proto.getTypedPerFilterConfigMap(); - if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { - Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); - StructOrError httpFaultOrError = - HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); - if (httpFaultOrError.getErrorDetail() != null) { - return StructOrError.fromError( - "ClusterWeight [" + proto.getName() + "] contains invalid HttpFault filter: " - + httpFaultOrError.getErrorDetail()); - } - httpFault = httpFaultOrError.getStruct(); + Map filterConfigMap = proto.getTypedPerFilterConfigMap(); + if (filterConfigMap.containsKey(HTTP_FAULT_FILTER_NAME)) { + Any rawFaultFilterConfig = filterConfigMap.get(HTTP_FAULT_FILTER_NAME); + StructOrError httpFaultOrError = + HttpFault.decodeFaultFilterConfig(rawFaultFilterConfig); + if (httpFaultOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "ClusterWeight [" + proto.getName() + "] contains invalid HttpFault filter: " + + httpFaultOrError.getErrorDetail()); } + httpFault = httpFaultOrError.getStruct(); } return StructOrError.fromStruct( new ClusterWeight(proto.getName(), proto.getWeight().getValue(), httpFault)); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index 479690cb075..c47494f73e5 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -173,11 +173,9 @@ public boolean shouldAccept(Runnable command) { private ManagedChannel channel; private ClientXdsClient xdsClient; - private boolean originalIsFaultInjectionEnabled; @Before public void setUp() throws IOException { - originalIsFaultInjectionEnabled = EnvoyProtoData.enableFaultInjection; MockitoAnnotations.initMocks(this); when(backoffPolicyProvider.get()).thenReturn(backoffPolicy1, backoffPolicy2); when(backoffPolicy1.nextBackoffNanos()).thenReturn(10L, 100L); @@ -210,7 +208,6 @@ public void setUp() throws IOException { @After public void tearDown() { - EnvoyProtoData.enableFaultInjection = originalIsFaultInjectionEnabled; xdsClient.shutdown(); channel.shutdown(); // channel not owned by XdsClient assertThat(adsEnded.get()).isTrue(); @@ -337,8 +334,6 @@ public void ldsResourceUpdated() { @Test public void ldsResourceUpdate_withFaultInjection() { - EnvoyProtoData.enableFaultInjection = true; - DiscoveryRpcCall call = startResourceWatcher(ResourceType.LDS, LDS_RESOURCE, ldsResourceWatcher); List listeners = ImmutableList.of(