From 8b9ae27cdff9c3204f17faf7649aa87988de06c6 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Wed, 19 Apr 2023 17:26:26 -0700 Subject: [PATCH] xds: remove environmental variables (#10055) --- .../grpc/xds/LoadBalancerConfigFactory.java | 4 +- .../java/io/grpc/xds/XdsClusterResource.java | 4 +- .../java/io/grpc/xds/XdsListenerResource.java | 95 ++++++----- .../java/io/grpc/xds/XdsResourceType.java | 12 -- .../grpc/xds/XdsRouteConfigureResource.java | 51 +++--- .../xds/LoadBalancerConfigFactoryTest.java | 58 +++---- .../io/grpc/xds/XdsClientImplDataTest.java | 154 ++++++++++-------- .../io/grpc/xds/XdsClientImplTestBase.java | 51 ------ 8 files changed, 176 insertions(+), 253 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java b/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java index 4b919a4e6f..f1ea5a2780 100644 --- a/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java +++ b/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java @@ -92,11 +92,11 @@ class LoadBalancerConfigFactory { * @throws ResourceInvalidException If the {@link Cluster} has an invalid LB configuration. */ static ImmutableMap newConfig(Cluster cluster, boolean enableLeastRequest, - boolean enableCustomLbConfig, boolean enableWrr) + boolean enableWrr) throws ResourceInvalidException { // The new load_balancing_policy will always be used if it is set, but for backward // compatibility we will fall back to using the old lb_policy field if the new field is not set. - if (cluster.hasLoadBalancingPolicy() && enableCustomLbConfig) { + if (cluster.hasLoadBalancingPolicy()) { try { return LoadBalancingPolicyConverter.convertToServiceConfig(cluster.getLoadBalancingPolicy(), 0, enableWrr); diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index 1dc59feb8b..b2917cffda 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -133,7 +133,7 @@ class XdsClusterResource extends XdsResourceType { CdsUpdate.Builder updateBuilder = structOrError.getStruct(); ImmutableMap lbPolicyConfig = LoadBalancerConfigFactory.newConfig(cluster, - enableLeastRequest, enableCustomLbConfig, enableWrr); + enableLeastRequest, enableWrr); // Validate the LB config by trying to parse it with the corresponding LB provider. LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(lbPolicyConfig); @@ -216,7 +216,7 @@ class XdsClusterResource extends XdsResourceType { } } - if (cluster.hasOutlierDetection() && enableOutlierDetection) { + if (cluster.hasOutlierDetection()) { try { outlierDetection = OutlierDetection.fromEnvoyOutlierDetection( validateOutlierDetection(cluster.getOutlierDetection())); diff --git a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java index 789f78ba5b..62e71a1cbd 100644 --- a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java @@ -103,14 +103,14 @@ class XdsListenerResource extends XdsResourceType { if (listener.hasApiListener()) { return processClientSideListener( - listener, args, enableFaultInjection); + listener, args); } else { return processServerSideListener( - listener, args, enableRbac); + listener, args); } } - private LdsUpdate processClientSideListener(Listener listener, Args args, boolean parseHttpFilter) + private LdsUpdate processClientSideListener(Listener listener, Args args) throws ResourceInvalidException { // Unpack HttpConnectionManager from the Listener. HttpConnectionManager hcm; @@ -123,23 +123,23 @@ class XdsListenerResource extends XdsResourceType { "Could not parse HttpConnectionManager config from ApiListener", e); } return LdsUpdate.forApiListener(parseHttpConnectionManager( - hcm, args.filterRegistry, parseHttpFilter, true /* isForClient */)); + hcm, args.filterRegistry, true /* isForClient */)); } - private LdsUpdate processServerSideListener(Listener proto, Args args, boolean parseHttpFilter) + private LdsUpdate processServerSideListener(Listener proto, Args args) throws ResourceInvalidException { Set certProviderInstances = null; if (args.bootstrapInfo != null && args.bootstrapInfo.certProviders() != null) { certProviderInstances = args.bootstrapInfo.certProviders().keySet(); } return LdsUpdate.forTcpListener(parseServerSideListener(proto, args.tlsContextManager, - args.filterRegistry, certProviderInstances, parseHttpFilter)); + args.filterRegistry, certProviderInstances)); } @VisibleForTesting static EnvoyServerProtoData.Listener parseServerSideListener( Listener proto, TlsContextManager tlsContextManager, - FilterRegistry filterRegistry, Set certProviderInstances, boolean parseHttpFilter) + FilterRegistry filterRegistry, Set certProviderInstances) throws ResourceInvalidException { if (!proto.getTrafficDirection().equals(TrafficDirection.INBOUND) && !proto.getTrafficDirection().equals(TrafficDirection.UNSPECIFIED)) { @@ -177,13 +177,13 @@ class XdsListenerResource extends XdsResourceType { for (io.envoyproxy.envoy.config.listener.v3.FilterChain fc : proto.getFilterChainsList()) { filterChains.add( parseFilterChain(fc, tlsContextManager, filterRegistry, uniqueSet, - certProviderInstances, parseHttpFilter)); + certProviderInstances)); } FilterChain defaultFilterChain = null; if (proto.hasDefaultFilterChain()) { defaultFilterChain = parseFilterChain( proto.getDefaultFilterChain(), tlsContextManager, filterRegistry, - null, certProviderInstances, parseHttpFilter); + null, certProviderInstances); } return EnvoyServerProtoData.Listener.create( @@ -194,7 +194,7 @@ class XdsListenerResource extends XdsResourceType { static FilterChain parseFilterChain( io.envoyproxy.envoy.config.listener.v3.FilterChain proto, TlsContextManager tlsContextManager, FilterRegistry filterRegistry, - Set uniqueSet, Set certProviderInstances, boolean parseHttpFilters) + Set uniqueSet, Set certProviderInstances) throws ResourceInvalidException { if (proto.getFiltersCount() != 1) { throw new ResourceInvalidException("FilterChain " + proto.getName() @@ -221,7 +221,7 @@ class XdsListenerResource extends XdsResourceType { + filter.getName() + " failed to unpack message", e); } io.grpc.xds.HttpConnectionManager httpConnectionManager = parseHttpConnectionManager( - hcmProto, filterRegistry, parseHttpFilters, false /* isForClient */); + hcmProto, filterRegistry, false /* isForClient */); EnvoyServerProtoData.DownstreamTlsContext downstreamTlsContext = null; if (proto.hasTransportSocket()) { @@ -453,12 +453,12 @@ class XdsListenerResource extends XdsResourceType { @VisibleForTesting static io.grpc.xds.HttpConnectionManager parseHttpConnectionManager( HttpConnectionManager proto, FilterRegistry filterRegistry, - boolean parseHttpFilter, boolean isForClient) throws ResourceInvalidException { - if (enableRbac && proto.getXffNumTrustedHops() != 0) { + boolean isForClient) throws ResourceInvalidException { + if (proto.getXffNumTrustedHops() != 0) { throw new ResourceInvalidException( "HttpConnectionManager with xff_num_trusted_hops unsupported"); } - if (enableRbac && !proto.getOriginalIpDetectionExtensionsList().isEmpty()) { + if (!proto.getOriginalIpDetectionExtensionsList().isEmpty()) { throw new ResourceInvalidException("HttpConnectionManager with " + "original_ip_detection_extensions unsupported"); } @@ -472,48 +472,45 @@ class XdsListenerResource extends XdsResourceType { } // Parse http filters. - List filterConfigs = null; - if (parseHttpFilter) { - if (proto.getHttpFiltersList().isEmpty()) { - throw new ResourceInvalidException("Missing HttpFilter in HttpConnectionManager."); + if (proto.getHttpFiltersList().isEmpty()) { + throw new ResourceInvalidException("Missing HttpFilter in HttpConnectionManager."); + } + List filterConfigs = new ArrayList<>(); + Set names = new HashSet<>(); + for (int i = 0; i < proto.getHttpFiltersCount(); i++) { + io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter + httpFilter = proto.getHttpFiltersList().get(i); + String filterName = httpFilter.getName(); + if (!names.add(filterName)) { + throw new ResourceInvalidException( + "HttpConnectionManager contains duplicate HttpFilter: " + filterName); } - filterConfigs = new ArrayList<>(); - Set names = new HashSet<>(); - for (int i = 0; i < proto.getHttpFiltersCount(); i++) { - io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter - httpFilter = proto.getHttpFiltersList().get(i); - String filterName = httpFilter.getName(); - if (!names.add(filterName)) { - throw new ResourceInvalidException( - "HttpConnectionManager contains duplicate HttpFilter: " + filterName); - } - StructOrError filterConfig = - parseHttpFilter(httpFilter, filterRegistry, isForClient); - if ((i == proto.getHttpFiltersCount() - 1) - && (filterConfig == null || !isTerminalFilter(filterConfig.getStruct()))) { - throw new ResourceInvalidException("The last HttpFilter must be a terminal filter: " - + filterName); - } - if (filterConfig == null) { - continue; - } - if (filterConfig.getErrorDetail() != null) { - throw new ResourceInvalidException( - "HttpConnectionManager contains invalid HttpFilter: " - + filterConfig.getErrorDetail()); - } - if ((i < proto.getHttpFiltersCount() - 1) && isTerminalFilter(filterConfig.getStruct())) { - throw new ResourceInvalidException("A terminal HttpFilter must be the last filter: " - + filterName); - } - filterConfigs.add(new Filter.NamedFilterConfig(filterName, filterConfig.getStruct())); + StructOrError filterConfig = + parseHttpFilter(httpFilter, filterRegistry, isForClient); + if ((i == proto.getHttpFiltersCount() - 1) + && (filterConfig == null || !isTerminalFilter(filterConfig.getStruct()))) { + throw new ResourceInvalidException("The last HttpFilter must be a terminal filter: " + + filterName); } + if (filterConfig == null) { + continue; + } + if (filterConfig.getErrorDetail() != null) { + throw new ResourceInvalidException( + "HttpConnectionManager contains invalid HttpFilter: " + + filterConfig.getErrorDetail()); + } + if ((i < proto.getHttpFiltersCount() - 1) && isTerminalFilter(filterConfig.getStruct())) { + throw new ResourceInvalidException("A terminal HttpFilter must be the last filter: " + + filterName); + } + filterConfigs.add(new Filter.NamedFilterConfig(filterName, filterConfig.getStruct())); } // Parse inlined RouteConfiguration or RDS. if (proto.hasRouteConfig()) { List virtualHosts = extractVirtualHosts( - proto.getRouteConfig(), filterRegistry, parseHttpFilter); + proto.getRouteConfig(), filterRegistry); return io.grpc.xds.HttpConnectionManager.forVirtualHosts( maxStreamDuration, virtualHosts, filterConfigs); } diff --git a/xds/src/main/java/io/grpc/xds/XdsResourceType.java b/xds/src/main/java/io/grpc/xds/XdsResourceType.java index 4c19ebf776..0304d39b5d 100644 --- a/xds/src/main/java/io/grpc/xds/XdsResourceType.java +++ b/xds/src/main/java/io/grpc/xds/XdsResourceType.java @@ -47,12 +47,6 @@ abstract class XdsResourceType { @VisibleForTesting static final String HASH_POLICY_FILTER_STATE_KEY = "io.grpc.channel_id"; @VisibleForTesting - static boolean enableFaultInjection = getFlag("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION", true); - @VisibleForTesting - static boolean enableRetry = getFlag("GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY", true); - @VisibleForTesting - static boolean enableRbac = getFlag("GRPC_XDS_EXPERIMENTAL_RBAC", true); - @VisibleForTesting static boolean enableRouteLookup = getFlag("GRPC_EXPERIMENTAL_XDS_RLS_LB", false); @VisibleForTesting static boolean enableLeastRequest = @@ -62,12 +56,6 @@ abstract class XdsResourceType { @VisibleForTesting static boolean enableWrr = getFlag("GRPC_EXPERIMENTAL_XDS_WRR_LB", false); - - @VisibleForTesting - static boolean enableCustomLbConfig = getFlag("GRPC_EXPERIMENTAL_XDS_CUSTOM_LB_CONFIG", true); - @VisibleForTesting - static boolean enableOutlierDetection = getFlag("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION", - true); static final String TYPE_URL_CLUSTER_CONFIG = "type.googleapis.com/envoy.extensions.clusters.aggregate.v3.ClusterConfig"; static final String TYPE_URL_TYPED_STRUCT_UDPA = diff --git a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java index 6ae23406d6..47f6f6beee 100644 --- a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java @@ -119,17 +119,17 @@ class XdsRouteConfigureResource extends XdsResourceType { throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass()); } return processRouteConfiguration((RouteConfiguration) unpackedMessage, - args.filterRegistry, enableFaultInjection); + args.filterRegistry); } private static RdsUpdate processRouteConfiguration( - RouteConfiguration routeConfig, FilterRegistry filterRegistry, boolean parseHttpFilter) + RouteConfiguration routeConfig, FilterRegistry filterRegistry) throws ResourceInvalidException { - return new RdsUpdate(extractVirtualHosts(routeConfig, filterRegistry, parseHttpFilter)); + return new RdsUpdate(extractVirtualHosts(routeConfig, filterRegistry)); } static List extractVirtualHosts( - RouteConfiguration routeConfig, FilterRegistry filterRegistry, boolean parseHttpFilter) + RouteConfiguration routeConfig, FilterRegistry filterRegistry) throws ResourceInvalidException { Map pluginConfigMap = new HashMap<>(); ImmutableSet.Builder optionalPlugins = ImmutableSet.builder(); @@ -154,7 +154,7 @@ class XdsRouteConfigureResource extends XdsResourceType { for (io.envoyproxy.envoy.config.route.v3.VirtualHost virtualHostProto : routeConfig.getVirtualHostsList()) { StructOrError virtualHost = - parseVirtualHost(virtualHostProto, filterRegistry, parseHttpFilter, pluginConfigMap, + parseVirtualHost(virtualHostProto, filterRegistry, pluginConfigMap, optionalPlugins.build()); if (virtualHost.getErrorDetail() != null) { throw new ResourceInvalidException( @@ -167,13 +167,13 @@ class XdsRouteConfigureResource extends XdsResourceType { private static StructOrError parseVirtualHost( io.envoyproxy.envoy.config.route.v3.VirtualHost proto, FilterRegistry filterRegistry, - boolean parseHttpFilter, Map pluginConfigMap, + Map pluginConfigMap, Set optionalPlugins) { String name = proto.getName(); List routes = new ArrayList<>(proto.getRoutesCount()); for (io.envoyproxy.envoy.config.route.v3.Route routeProto : proto.getRoutesList()) { StructOrError route = parseRoute( - routeProto, filterRegistry, parseHttpFilter, pluginConfigMap, optionalPlugins); + routeProto, filterRegistry, pluginConfigMap, optionalPlugins); if (route == null) { continue; } @@ -183,10 +183,6 @@ class XdsRouteConfigureResource extends XdsResourceType { } routes.add(route.getStruct()); } - if (!parseHttpFilter) { - return StructOrError.fromStruct(VirtualHost.create( - name, proto.getDomainsList(), routes, new HashMap())); - } StructOrError> overrideConfigs = parseOverrideFilterConfigs(proto.getTypedPerFilterConfigMap(), filterRegistry); if (overrideConfigs.getErrorDetail() != null) { @@ -258,7 +254,7 @@ class XdsRouteConfigureResource extends XdsResourceType { @Nullable static StructOrError parseRoute( io.envoyproxy.envoy.config.route.v3.Route proto, FilterRegistry filterRegistry, - boolean parseHttpFilter, Map pluginConfigMap, + Map pluginConfigMap, Set optionalPlugins) { StructOrError routeMatch = parseRouteMatch(proto.getMatch()); if (routeMatch == null) { @@ -270,22 +266,19 @@ class XdsRouteConfigureResource extends XdsResourceType { + routeMatch.getErrorDetail()); } - Map overrideConfigs = Collections.emptyMap(); - if (parseHttpFilter) { - StructOrError> overrideConfigsOrError = - parseOverrideFilterConfigs(proto.getTypedPerFilterConfigMap(), filterRegistry); - if (overrideConfigsOrError.getErrorDetail() != null) { - return StructOrError.fromError( - "Route [" + proto.getName() + "] contains invalid HttpFilter config: " - + overrideConfigsOrError.getErrorDetail()); - } - overrideConfigs = overrideConfigsOrError.getStruct(); + StructOrError> overrideConfigsOrError = + parseOverrideFilterConfigs(proto.getTypedPerFilterConfigMap(), filterRegistry); + if (overrideConfigsOrError.getErrorDetail() != null) { + return StructOrError.fromError( + "Route [" + proto.getName() + "] contains invalid HttpFilter config: " + + overrideConfigsOrError.getErrorDetail()); } + Map overrideConfigs = overrideConfigsOrError.getStruct(); switch (proto.getActionCase()) { case ROUTE: StructOrError routeAction = - parseRouteAction(proto.getRoute(), filterRegistry, parseHttpFilter, pluginConfigMap, + parseRouteAction(proto.getRoute(), filterRegistry, pluginConfigMap, optionalPlugins); if (routeAction == null) { return null; @@ -411,7 +404,7 @@ class XdsRouteConfigureResource extends XdsResourceType { @Nullable static StructOrError parseRouteAction( io.envoyproxy.envoy.config.route.v3.RouteAction proto, FilterRegistry filterRegistry, - boolean parseHttpFilter, Map pluginConfigMap, + Map pluginConfigMap, Set optionalPlugins) { Long timeoutNano = null; if (proto.hasMaxStreamDuration()) { @@ -424,7 +417,7 @@ class XdsRouteConfigureResource extends XdsResourceType { } } RetryPolicy retryPolicy = null; - if (enableRetry && proto.hasRetryPolicy()) { + if (proto.hasRetryPolicy()) { StructOrError retryPolicyOrError = parseRetryPolicy(proto.getRetryPolicy()); if (retryPolicyOrError != null) { if (retryPolicyOrError.getErrorDetail() != null) { @@ -482,7 +475,7 @@ class XdsRouteConfigureResource extends XdsResourceType { for (io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight clusterWeight : clusterWeights) { StructOrError clusterWeightOrError = - parseClusterWeight(clusterWeight, filterRegistry, parseHttpFilter); + parseClusterWeight(clusterWeight, filterRegistry); if (clusterWeightOrError.getErrorDetail() != null) { return StructOrError.fromError("RouteAction contains invalid ClusterWeight: " + clusterWeightOrError.getErrorDetail()); @@ -586,11 +579,7 @@ class XdsRouteConfigureResource extends XdsResourceType { @VisibleForTesting static StructOrError parseClusterWeight( io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight proto, - FilterRegistry filterRegistry, boolean parseHttpFilter) { - if (!parseHttpFilter) { - return StructOrError.fromStruct(ClusterWeight.create(proto.getName(), - proto.getWeight().getValue(), new HashMap())); - } + FilterRegistry filterRegistry) { StructOrError> overrideConfigs = parseOverrideFilterConfigs(proto.getTypedPerFilterConfigMap(), filterRegistry); if (overrideConfigs.getErrorDetail() != null) { diff --git a/xds/src/test/java/io/grpc/xds/LoadBalancerConfigFactoryTest.java b/xds/src/test/java/io/grpc/xds/LoadBalancerConfigFactoryTest.java index 884f04b2f2..60086d7279 100644 --- a/xds/src/test/java/io/grpc/xds/LoadBalancerConfigFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/LoadBalancerConfigFactoryTest.java @@ -142,14 +142,14 @@ public class LoadBalancerConfigFactoryTest { public void roundRobin() throws ResourceInvalidException { Cluster cluster = newCluster(buildWrrPolicy(ROUND_ROBIN_POLICY)); - assertThat(newLbConfig(cluster, true, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); + assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); } @Test public void weightedRoundRobin() throws ResourceInvalidException { Cluster cluster = newCluster(buildWrrPolicy(WRR_POLICY)); - assertThat(newLbConfig(cluster, true, true, true)).isEqualTo(VALID_WRR_CONFIG); + assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_WRR_CONFIG); } @Test @@ -166,7 +166,7 @@ public class LoadBalancerConfigFactoryTest { .build()) .build())); - assertResourceInvalidExceptionThrown(cluster, true, true, true, + assertResourceInvalidExceptionThrown(cluster, true, true, "Invalid duration in weighted round robin config"); } @@ -174,14 +174,14 @@ public class LoadBalancerConfigFactoryTest { public void weightedRoundRobin_fallback_roundrobin() throws ResourceInvalidException { Cluster cluster = newCluster(buildWrrPolicy(WRR_POLICY, ROUND_ROBIN_POLICY)); - assertThat(newLbConfig(cluster, true, true, false)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); + assertThat(newLbConfig(cluster, true, false)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); } @Test public void roundRobin_legacy() throws ResourceInvalidException { Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.ROUND_ROBIN).build(); - assertThat(newLbConfig(cluster, true, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); + assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); } @Test @@ -190,7 +190,7 @@ public class LoadBalancerConfigFactoryTest { .setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(RING_HASH_POLICY)) .build(); - assertThat(newLbConfig(cluster, true, true, true)).isEqualTo(VALID_RING_HASH_CONFIG); + assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_RING_HASH_CONFIG); } @Test @@ -200,7 +200,7 @@ public class LoadBalancerConfigFactoryTest { .setMaximumRingSize(UInt64Value.of(RING_HASH_MAX_RING_SIZE)) .setHashFunction(HashFunction.XX_HASH)).build(); - assertThat(newLbConfig(cluster, true, true, true)).isEqualTo(VALID_RING_HASH_CONFIG); + assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_RING_HASH_CONFIG); } @Test @@ -212,7 +212,7 @@ public class LoadBalancerConfigFactoryTest { .setMaximumRingSize(UInt64Value.of(RING_HASH_MAX_RING_SIZE)) .setHashFunction(RingHash.HashFunction.MURMUR_HASH_2).build()))).build()); - assertResourceInvalidExceptionThrown(cluster, true, true, true, "Invalid ring hash function"); + assertResourceInvalidExceptionThrown(cluster, true, true, "Invalid ring hash function"); } @Test @@ -220,7 +220,7 @@ public class LoadBalancerConfigFactoryTest { Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.RING_HASH).setRingHashLbConfig( RingHashLbConfig.newBuilder().setHashFunction(HashFunction.MURMUR_HASH_2)).build(); - assertResourceInvalidExceptionThrown(cluster, true, true, true, "invalid ring hash function"); + assertResourceInvalidExceptionThrown(cluster, true, true, "invalid ring hash function"); } @Test @@ -229,7 +229,7 @@ public class LoadBalancerConfigFactoryTest { .setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(LEAST_REQUEST_POLICY)) .build(); - assertThat(newLbConfig(cluster, true, true, true)).isEqualTo(VALID_LEAST_REQUEST_CONFIG); + assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_LEAST_REQUEST_CONFIG); } @Test @@ -241,7 +241,7 @@ public class LoadBalancerConfigFactoryTest { LeastRequestLbConfig.newBuilder() .setChoiceCount(UInt32Value.of(LEAST_REQUEST_CHOICE_COUNT))).build(); - LbConfig lbConfig = newLbConfig(cluster, true, true, true); + LbConfig lbConfig = newLbConfig(cluster, true, true); assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( @@ -258,14 +258,14 @@ public class LoadBalancerConfigFactoryTest { Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.LEAST_REQUEST).build(); - assertResourceInvalidExceptionThrown(cluster, false, true, true, "unsupported lb policy"); + assertResourceInvalidExceptionThrown(cluster, false, true, "unsupported lb policy"); } @Test public void customRootLb_providerRegistered() throws ResourceInvalidException { LoadBalancerRegistry.getDefaultRegistry().register(CUSTOM_POLICY_PROVIDER); - assertThat(newLbConfig(newCluster(CUSTOM_POLICY), false, true, + assertThat(newLbConfig(newCluster(CUSTOM_POLICY), false, true)).isEqualTo(VALID_CUSTOM_CONFIG); } @@ -275,7 +275,7 @@ public class LoadBalancerConfigFactoryTest { .setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(CUSTOM_POLICY)) .build(); - assertResourceInvalidExceptionThrown(cluster, false, true, true,"Invalid LoadBalancingPolicy"); + assertResourceInvalidExceptionThrown(cluster, false, true,"Invalid LoadBalancingPolicy"); } // When a provider for the endpoint picking custom policy is available, the configuration should @@ -287,7 +287,7 @@ public class LoadBalancerConfigFactoryTest { Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder() .addPolicies(buildWrrPolicy(CUSTOM_POLICY, ROUND_ROBIN_POLICY))).build(); - assertThat(newLbConfig(cluster, false, true, true)).isEqualTo(VALID_CUSTOM_CONFIG_IN_WRR); + assertThat(newLbConfig(cluster, false, true)).isEqualTo(VALID_CUSTOM_CONFIG_IN_WRR); } // When a provider for the endpoint picking custom policy is available, the configuration should @@ -299,7 +299,7 @@ public class LoadBalancerConfigFactoryTest { Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder() .addPolicies(buildWrrPolicy(CUSTOM_POLICY_UDPA, ROUND_ROBIN_POLICY))).build(); - assertThat(newLbConfig(cluster, false, true, true)).isEqualTo(VALID_CUSTOM_CONFIG_IN_WRR); + assertThat(newLbConfig(cluster, false, true)).isEqualTo(VALID_CUSTOM_CONFIG_IN_WRR); } // When a provider for the custom wrr_locality child policy is NOT available, we should fall back @@ -309,7 +309,7 @@ public class LoadBalancerConfigFactoryTest { Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder() .addPolicies(buildWrrPolicy(CUSTOM_POLICY, ROUND_ROBIN_POLICY))).build(); - assertThat(newLbConfig(cluster, false, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); + assertThat(newLbConfig(cluster, false, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); } // When a provider for the custom wrr_locality child policy is NOT available and no alternative @@ -319,18 +319,7 @@ public class LoadBalancerConfigFactoryTest { Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy( LoadBalancingPolicy.newBuilder().addPolicies(buildWrrPolicy(CUSTOM_POLICY))).build(); - assertResourceInvalidExceptionThrown(cluster, false, true, true, "Invalid LoadBalancingPolicy"); - } - - @Test - public void customConfig_notEnabled() throws ResourceInvalidException { - Cluster cluster = Cluster.newBuilder() - .setLoadBalancingPolicy( - LoadBalancingPolicy.newBuilder().addPolicies(RING_HASH_POLICY)) - .build(); - - // Custom LB flag not set, so we use old logic that will default to round_robin. - assertThat(newLbConfig(cluster, true, false, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); + assertResourceInvalidExceptionThrown(cluster, false, true, "Invalid LoadBalancingPolicy"); } @Test @@ -357,7 +346,7 @@ public class LoadBalancerConfigFactoryTest { buildWrrPolicy( ROUND_ROBIN_POLICY))))))))))))))))))).build(); - assertResourceInvalidExceptionThrown(cluster, false, true, true, + assertResourceInvalidExceptionThrown(cluster, false, true, "Maximum LB config recursion depth reached"); } @@ -373,18 +362,17 @@ public class LoadBalancerConfigFactoryTest { .build()))).build(); } - private LbConfig newLbConfig(Cluster cluster, boolean enableLeastRequest, - boolean enableCustomConfig, boolean enableWrr) + private LbConfig newLbConfig(Cluster cluster, boolean enableLeastRequest, boolean enableWrr) throws ResourceInvalidException { return ServiceConfigUtil.unwrapLoadBalancingConfig( - LoadBalancerConfigFactory.newConfig(cluster, enableLeastRequest, enableCustomConfig, + LoadBalancerConfigFactory.newConfig(cluster, enableLeastRequest, enableWrr)); } private void assertResourceInvalidExceptionThrown(Cluster cluster, boolean enableLeastRequest, - boolean enableCustomConfig, boolean enableWrr, String expectedMessage) { + boolean enableWrr, String expectedMessage) { try { - newLbConfig(cluster, enableLeastRequest, enableCustomConfig, enableWrr); + newLbConfig(cluster, enableLeastRequest, enableWrr); } catch (ResourceInvalidException e) { assertThat(e).hasMessageThat().contains(expectedMessage); return; diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java index a39ecb46fa..f4ed0a19b9 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java @@ -164,18 +164,12 @@ public class XdsClientImplDataTest { @Rule public final ExpectedException thrown = ExpectedException.none(); private final FilterRegistry filterRegistry = FilterRegistry.getDefaultRegistry(); - private boolean originalEnableRetry; - private boolean originalEnableRbac; private boolean originalEnableRouteLookup; private boolean originalEnableLeastRequest; private boolean originalEnableWrr; @Before public void setUp() { - originalEnableRetry = XdsResourceType.enableRetry; - assertThat(originalEnableRetry).isTrue(); - originalEnableRbac = XdsResourceType.enableRbac; - assertThat(originalEnableRbac).isTrue(); originalEnableRouteLookup = XdsResourceType.enableRouteLookup; assertThat(originalEnableRouteLookup).isFalse(); originalEnableLeastRequest = XdsResourceType.enableLeastRequest; @@ -186,8 +180,6 @@ public class XdsClientImplDataTest { @After public void tearDown() { - XdsResourceType.enableRetry = originalEnableRetry; - XdsResourceType.enableRbac = originalEnableRbac; XdsResourceType.enableRouteLookup = originalEnableRouteLookup; XdsResourceType.enableLeastRequest = originalEnableLeastRequest; XdsResourceType.enableWrr = originalEnableWrr; @@ -206,7 +198,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo")) .build(); StructOrError struct = XdsRouteConfigureResource.parseRoute( - proto, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of()); + proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()).isNull(); assertThat(struct.getStruct()) .isEqualTo( @@ -229,7 +221,7 @@ public class XdsClientImplDataTest { .setNonForwardingAction(NonForwardingAction.getDefaultInstance()) .build(); StructOrError struct = XdsRouteConfigureResource.parseRoute( - proto, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of()); + proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct()) .isEqualTo( Route.forNonForwardingAction( @@ -248,7 +240,7 @@ public class XdsClientImplDataTest { .setRedirect(RedirectAction.getDefaultInstance()) .build(); res = XdsRouteConfigureResource.parseRoute( - redirectRoute, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of()); + redirectRoute, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(res.getStruct()).isNull(); assertThat(res.getErrorDetail()) .isEqualTo("Route [route-blade] with unknown action type: REDIRECT"); @@ -260,7 +252,7 @@ public class XdsClientImplDataTest { .setDirectResponse(DirectResponseAction.getDefaultInstance()) .build(); res = XdsRouteConfigureResource.parseRoute( - directResponseRoute, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of()); + directResponseRoute, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(res.getStruct()).isNull(); assertThat(res.getErrorDetail()) .isEqualTo("Route [route-blade] with unknown action type: DIRECT_RESPONSE"); @@ -272,7 +264,7 @@ public class XdsClientImplDataTest { .setFilterAction(FilterAction.getDefaultInstance()) .build(); res = XdsRouteConfigureResource.parseRoute( - filterRoute, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of()); + filterRoute, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(res.getStruct()).isNull(); assertThat(res.getErrorDetail()) .isEqualTo("Route [route-blade] with unknown action type: FILTER_ACTION"); @@ -294,7 +286,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo")) .build(); assertThat(XdsRouteConfigureResource.parseRoute( - proto, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of())) + proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of())) .isNull(); } @@ -311,7 +303,7 @@ public class XdsClientImplDataTest { .setClusterHeader("cluster header")) // cluster_header action not supported .build(); assertThat(XdsRouteConfigureResource.parseRoute( - proto, filterRegistry, false, ImmutableMap.of(), ImmutableSet.of())) + proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of())) .isNull(); } @@ -520,7 +512,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()).isNull(); assertThat(struct.getStruct().cluster()).isEqualTo("cluster-foo"); @@ -544,7 +536,7 @@ public class XdsClientImplDataTest { .setWeight(UInt32Value.newBuilder().setValue(70)))) .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()).isNull(); assertThat(struct.getStruct().cluster()).isNull(); @@ -570,7 +562,7 @@ public class XdsClientImplDataTest { .setWeight(UInt32Value.newBuilder().setValue(0)))) .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()).isEqualTo("Sum of cluster weights should be above 0."); } @@ -586,7 +578,7 @@ public class XdsClientImplDataTest { .setMaxStreamDuration(Durations.fromMillis(20L))) .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().timeoutNano()).isEqualTo(TimeUnit.SECONDS.toNanos(5L)); } @@ -601,7 +593,7 @@ public class XdsClientImplDataTest { .setMaxStreamDuration(Durations.fromSeconds(5L))) .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().timeoutNano()).isEqualTo(TimeUnit.SECONDS.toNanos(5L)); } @@ -613,14 +605,13 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().timeoutNano()).isNull(); } @Test public void parseRouteAction_withRetryPolicy() { - XdsResourceType.enableRetry = true; RetryPolicy.Builder builder = RetryPolicy.newBuilder() .setNumRetries(UInt32Value.of(3)) .setRetryBackOff( @@ -636,7 +627,7 @@ public class XdsClientImplDataTest { .setRetryPolicy(builder.build()) .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); RouteAction.RetryPolicy retryPolicy = struct.getStruct().retryPolicy(); assertThat(retryPolicy.maxAttempts()).isEqualTo(4); @@ -660,7 +651,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder.build()) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().retryPolicy()).isNotNull(); assertThat(struct.getStruct().retryPolicy().retryableStatusCodes()).isEmpty(); @@ -673,7 +664,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()).isEqualTo("No base_interval specified in retry_backoff"); @@ -683,7 +674,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); retryPolicy = struct.getStruct().retryPolicy(); assertThat(retryPolicy.maxBackoff()).isEqualTo(Durations.fromMillis(500 * 10)); @@ -694,7 +685,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()) .isEqualTo("base_interval in retry_backoff must be positive"); @@ -707,7 +698,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()) .isEqualTo("max_interval in retry_backoff cannot be less than base_interval"); @@ -720,7 +711,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getErrorDetail()) .isEqualTo("max_interval in retry_backoff cannot be less than base_interval"); @@ -733,7 +724,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().retryPolicy().initialBackoff()) .isEqualTo(Durations.fromMillis(1)); @@ -749,7 +740,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); retryPolicy = struct.getStruct().retryPolicy(); assertThat(retryPolicy.initialBackoff()).isEqualTo(Durations.fromMillis(25)); @@ -768,7 +759,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().retryPolicy().retryableStatusCodes()) .containsExactly(Code.CANCELLED); @@ -786,7 +777,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().retryPolicy().retryableStatusCodes()) .containsExactly(Code.CANCELLED); @@ -804,7 +795,7 @@ public class XdsClientImplDataTest { .setCluster("cluster-foo") .setRetryPolicy(builder) .build(); - struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + struct = XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct.getStruct().retryPolicy().retryableStatusCodes()) .containsExactly(Code.CANCELLED); @@ -843,7 +834,7 @@ public class XdsClientImplDataTest { QueryParameter.newBuilder().setName("param"))) // unsupported .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); List policies = struct.getStruct().hashPolicies(); assertThat(policies).hasSize(2); @@ -863,7 +854,7 @@ public class XdsClientImplDataTest { io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct).isNull(); } @@ -876,7 +867,7 @@ public class XdsClientImplDataTest { .setClusterSpecifierPlugin(CLUSTER_SPECIFIER_PLUGIN.name()) .build(); StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false, + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, ImmutableMap.of(), ImmutableSet.of()); assertThat(struct).isNull(); } @@ -889,7 +880,7 @@ public class XdsClientImplDataTest { .setWeight(UInt32Value.newBuilder().setValue(30)) .build(); ClusterWeight clusterWeight = - XdsRouteConfigureResource.parseClusterWeight(proto, filterRegistry, false).getStruct(); + XdsRouteConfigureResource.parseClusterWeight(proto, filterRegistry).getStruct(); assertThat(clusterWeight.name()).isEqualTo("cluster-foo"); assertThat(clusterWeight.weight()).isEqualTo(30); } @@ -1354,7 +1345,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager with xff_num_trusted_hops unsupported"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, false /* does not matter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1368,7 +1359,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager with original_ip_detection_extensions unsupported"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, false /* does not matter */, false); + hcm, filterRegistry, false); } @Test @@ -1379,11 +1370,14 @@ public class XdsClientImplDataTest { .setCommonHttpProtocolOptions( HttpProtocolOptions.newBuilder() .setMaxStreamDuration(Durations.fromNanos(1000L))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager neither has inlined route_config nor RDS"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, false /* does not matter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1402,7 +1396,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager contains duplicate HttpFilter: envoy.filter.foo"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1420,7 +1414,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("The last HttpFilter must be a terminal filter: envoy.filter.bar"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1438,7 +1432,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("A terminal HttpFilter must be the last filter: terminal"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true); } @@ -1454,7 +1448,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("The last HttpFilter must be a terminal filter: envoy.filter.bar"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1466,7 +1460,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Missing HttpFilter in HttpConnectionManager."); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1509,10 +1503,13 @@ public class XdsClientImplDataTest { .addVirtualHosts(io.envoyproxy.envoy.config.route.v3.VirtualHost.newBuilder() .setName("virtual-host-1") .addRoutes(route))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); io.grpc.xds.HttpConnectionManager parsedHcm = XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, false /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); VirtualHost virtualHost = Iterables.getOnlyElement(parsedHcm.virtualHosts()); @@ -1583,13 +1580,16 @@ public class XdsClientImplDataTest { .addVirtualHosts(io.envoyproxy.envoy.config.route.v3.VirtualHost.newBuilder() .setName("virtual-host-1") .addRoutes(route))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Multiple ClusterSpecifierPlugins with the same name: rls-plugin-1"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, false /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1632,13 +1632,16 @@ public class XdsClientImplDataTest { .addVirtualHosts(io.envoyproxy.envoy.config.route.v3.VirtualHost.newBuilder() .setName("virtual-host-1") .addRoutes(route))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("ClusterSpecifierPlugin for [invalid-plugin-name] not found"); XdsListenerResource.parseHttpConnectionManager( - hcm, filterRegistry, false /* parseHttpFilter */, + hcm, filterRegistry, true /* does not matter */); } @@ -1708,8 +1711,12 @@ public class XdsClientImplDataTest { .addRoutes(optionalRoute)) .build(); io.grpc.xds.HttpConnectionManager parsedHcm = XdsListenerResource.parseHttpConnectionManager( - HttpConnectionManager.newBuilder().setRouteConfig(routeConfig).build(), filterRegistry, - false /* parseHttpFilter */, true /* does not matter */); + HttpConnectionManager.newBuilder().setRouteConfig(routeConfig) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) + .build(), filterRegistry, + true /* does not matter */); // Verify that the only route left is the one with the registered RLS plugin `rls-plugin-1`, // while the route with unregistered optional `optional-plugin-`1 has been skipped. @@ -1731,9 +1738,12 @@ public class XdsClientImplDataTest { .setRouteConfigName("rds-config-foo") .setConfigSource( ConfigSource.newBuilder().setAds(AggregatedConfigSource.getDefaultInstance()))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); XdsListenerResource.parseHttpConnectionManager( - hcm1, filterRegistry, false /* parseHttpFilter */, + hcm1, filterRegistry, true /* does not matter */); HttpConnectionManager hcm2 = @@ -1742,9 +1752,12 @@ public class XdsClientImplDataTest { .setRouteConfigName("rds-config-foo") .setConfigSource( ConfigSource.newBuilder().setSelf(SelfConfigSource.getDefaultInstance()))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); XdsListenerResource.parseHttpConnectionManager( - hcm2, filterRegistry, false /* parseHttpFilter */, + hcm2, filterRegistry, true /* does not matter */); HttpConnectionManager hcm3 = @@ -1754,12 +1767,15 @@ public class XdsClientImplDataTest { .setConfigSource( ConfigSource.newBuilder() .setPathConfigSource(PathConfigSource.newBuilder().setPath("foo-path")))) + .addHttpFilters( + HttpFilter.newBuilder().setName("terminal").setTypedConfig( + Any.pack(Router.newBuilder().build())).setIsOptional(true)) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage( "HttpConnectionManager contains invalid RDS: must specify ADS or self ConfigSource"); XdsListenerResource.parseHttpConnectionManager( - hcm3, filterRegistry, false /* parseHttpFilter */, + hcm3, filterRegistry, true /* does not matter */); } @@ -2117,7 +2133,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Listener listener1 with invalid traffic direction: OUTBOUND"); XdsListenerResource.parseServerSideListener( - listener, null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null); } @Test @@ -2127,7 +2143,7 @@ public class XdsClientImplDataTest { .setName("listener1") .build(); XdsListenerResource.parseServerSideListener( - listener, null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null); } @Test @@ -2141,7 +2157,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Listener listener1 cannot have listener_filters"); XdsListenerResource.parseServerSideListener( - listener, null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null); } @Test @@ -2155,7 +2171,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Listener listener1 cannot have use_original_dst set to true"); XdsListenerResource.parseServerSideListener( - listener,null, filterRegistry, null, true /* does not matter */); + listener,null, filterRegistry, null); } @Test @@ -2204,7 +2220,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("FilterChainMatch must be unique. Found duplicate:"); XdsListenerResource.parseServerSideListener( - listener, null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null); } @Test @@ -2253,7 +2269,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("FilterChainMatch must be unique. Found duplicate:"); XdsListenerResource.parseServerSideListener( - listener,null, filterRegistry, null, true /* does not matter */); + listener,null, filterRegistry, null); } @Test @@ -2302,7 +2318,7 @@ public class XdsClientImplDataTest { .addAllFilterChains(Arrays.asList(filterChain1, filterChain2)) .build(); XdsListenerResource.parseServerSideListener( - listener, null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null); } @Test @@ -2317,8 +2333,7 @@ public class XdsClientImplDataTest { thrown.expectMessage( "FilterChain filter-chain-foo should contain exact one HttpConnectionManager filter"); XdsListenerResource.parseFilterChain( - filterChain, null, filterRegistry, null, null, - true /* does not matter */); + filterChain, null, filterRegistry, null, null); } @Test @@ -2336,8 +2351,7 @@ public class XdsClientImplDataTest { thrown.expectMessage( "FilterChain filter-chain-foo should contain exact one HttpConnectionManager filter"); XdsListenerResource.parseFilterChain( - filterChain, null, filterRegistry, null, null, - true /* does not matter */); + filterChain, null, filterRegistry, null, null); } @Test @@ -2355,8 +2369,7 @@ public class XdsClientImplDataTest { "FilterChain filter-chain-foo contains filter envoy.http_connection_manager " + "without typed_config"); XdsListenerResource.parseFilterChain( - filterChain, null, filterRegistry, null, null, - true /* does not matter */); + filterChain, null, filterRegistry, null, null); } @Test @@ -2378,8 +2391,7 @@ public class XdsClientImplDataTest { "FilterChain filter-chain-foo contains filter unsupported with unsupported " + "typed_config type unsupported-type-url"); XdsListenerResource.parseFilterChain( - filterChain, null, filterRegistry, null, null, - true /* does not matter */); + filterChain, null, filterRegistry, null, null); } @Test @@ -2407,10 +2419,10 @@ public class XdsClientImplDataTest { EnvoyServerProtoData.FilterChain parsedFilterChain1 = XdsListenerResource.parseFilterChain( filterChain1, null, filterRegistry, null, - null, true /* does not matter */); + null); EnvoyServerProtoData.FilterChain parsedFilterChain2 = XdsListenerResource.parseFilterChain( filterChain2, null, filterRegistry, null, - null, true /* does not matter */); + null); assertThat(parsedFilterChain1.name()).isEqualTo(parsedFilterChain2.name()); } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java index 23364bbd96..a5d7f09794 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java @@ -283,8 +283,6 @@ public abstract class XdsClientImplTestBase { private ManagedChannel channelForCustomAuthority; private ManagedChannel channelForEmptyAuthority; private XdsClientImpl xdsClient; - private boolean originalEnableFaultInjection; - private boolean originalEnableRbac; private boolean originalEnableLeastRequest; private boolean originalEnableFederation; private Server xdsServer; @@ -301,10 +299,6 @@ public abstract class XdsClientImplTestBase { when(backoffPolicy2.nextBackoffNanos()).thenReturn(20L, 200L); // Start the server and the client. - originalEnableFaultInjection = XdsResourceType.enableFaultInjection; - XdsResourceType.enableFaultInjection = true; - originalEnableRbac = XdsResourceType.enableRbac; - assertThat(originalEnableRbac).isTrue(); originalEnableLeastRequest = XdsResourceType.enableLeastRequest; XdsResourceType.enableLeastRequest = true; originalEnableFederation = BootstrapperImpl.enableFederation; @@ -380,8 +374,6 @@ public abstract class XdsClientImplTestBase { @After public void tearDown() { - XdsResourceType.enableFaultInjection = originalEnableFaultInjection; - XdsResourceType.enableRbac = originalEnableRbac; XdsResourceType.enableLeastRequest = originalEnableLeastRequest; BootstrapperImpl.enableFederation = originalEnableFederation; xdsClient.shutdown(); @@ -2255,8 +2247,6 @@ public abstract class XdsClientImplTestBase { @SuppressWarnings("deprecation") public void cdsResponseWithOutlierDetection() { Assume.assumeTrue(useProtocolV3()); - XdsClusterResource.enableOutlierDetection = true; - DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, cdsResourceWatcher); @@ -2320,46 +2310,6 @@ public abstract class XdsClientImplTestBase { verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); } - /** - * CDS response containing OutlierDetection for a cluster, but support has not been enabled. - */ - @Test - @SuppressWarnings("deprecation") - public void cdsResponseWithOutlierDetection_supportDisabled() { - Assume.assumeTrue(useProtocolV3()); - XdsClusterResource.enableOutlierDetection = false; - - DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, - cdsResourceWatcher); - - OutlierDetection outlierDetectionXds = OutlierDetection.newBuilder() - .setInterval(Durations.fromNanos(100)).build(); - - // Management server sends back CDS response with UpstreamTlsContext. - Any clusterEds = - Any.pack(mf.buildEdsCluster(CDS_RESOURCE, "eds-cluster-foo.googleapis.com", "round_robin", - null, null, true, - mf.buildUpstreamTlsContext("cert-instance-name", "cert1"), - "envoy.transport_sockets.tls", null, outlierDetectionXds)); - List clusters = ImmutableList.of( - Any.pack(mf.buildLogicalDnsCluster("cluster-bar.googleapis.com", - "dns-service-bar.googleapis.com", 443, "round_robin", null, null,false, null, null)), - clusterEds, - Any.pack(mf.buildEdsCluster("cluster-baz.googleapis.com", null, "round_robin", null, null, - false, null, "envoy.transport_sockets.tls", null, outlierDetectionXds))); - call.sendResponse(CDS, clusters, VERSION_1, "0000"); - - // Client sent an ACK CDS request. - call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher, times(1)).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - - assertThat(cdsUpdate.outlierDetection()).isNull(); - - verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterEds, VERSION_1, TIME_INCREMENT); - verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); - } - /** * CDS response containing OutlierDetection for a cluster. */ @@ -2367,7 +2317,6 @@ public abstract class XdsClientImplTestBase { @SuppressWarnings("deprecation") public void cdsResponseWithInvalidOutlierDetectionNacks() { Assume.assumeTrue(useProtocolV3()); - XdsClusterResource.enableOutlierDetection = true; DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, cdsResourceWatcher);