From 40ada3ad03ff85d79bb03886d257288c50bb3571 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Mon, 13 Sep 2021 13:28:38 -0700 Subject: [PATCH 1/5] build per route interceptor --- .../java/io/grpc/xds/XdsServerWrapper.java | 152 ++++++++---- ...rChainMatchingProtocolNegotiatorsTest.java | 11 +- .../io/grpc/xds/XdsServerWrapperTest.java | 226 ++++++++++++------ 3 files changed, 270 insertions(+), 119 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index faa6e9d34b2..b4d7bae0e99 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -22,6 +22,7 @@ import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; import io.grpc.Attributes; import io.grpc.InternalServerInterceptors; @@ -55,6 +56,7 @@ import io.grpc.xds.XdsNameResolverProvider.XdsClientPoolFactory; import io.grpc.xds.XdsServerBuilder.XdsServingStatusListener; import io.grpc.xds.internal.sds.SslContextProviderSupplier; + import java.io.IOException; import java.net.SocketAddress; import java.util.ArrayList; @@ -345,6 +347,15 @@ private final class DiscoveryState implements LdsResourceWatcher { @Nullable private FilterChain defaultFilterChain; private boolean stopped; + private final Map>> + rdsPrebuiltInterceptorRef = new HashMap<>(); + private final ServerInterceptor noopInterceptor = new ServerInterceptor() { + @Override + public Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + return next.startCall(call, headers); + } + }; private DiscoveryState(String resourceName) { this.resourceName = checkNotNull(resourceName, "resourceName"); @@ -452,6 +463,7 @@ private void shutdown() { private void updateSelector() { Map filterChainRouting = new HashMap<>(); + rdsPrebuiltInterceptorRef.clear(); for (FilterChain filterChain: filterChains) { filterChainRouting.put(filterChain, generateRoutingConfig(filterChain)); } @@ -470,13 +482,78 @@ private void updateSelector() { private ServerRoutingConfig generateRoutingConfig(FilterChain filterChain) { HttpConnectionManager hcm = filterChain.getHttpConnectionManager(); if (hcm.virtualHosts() != null) { + AtomicReference> interceptorRef = + new AtomicReference<>(generatePerRouteInterceptors( + hcm.httpFilterConfigs(), hcm.virtualHosts())); return ServerRoutingConfig.create(hcm.httpFilterConfigs(), - new AtomicReference<>(hcm.virtualHosts())); + new AtomicReference<>(hcm.virtualHosts()), interceptorRef); } else { RouteDiscoveryState rds = routeDiscoveryStates.get(hcm.rdsName()); checkNotNull(rds, "rds"); - return ServerRoutingConfig.create(hcm.httpFilterConfigs(), rds.savedVirtualHosts); + AtomicReference> interceptorRef = + new AtomicReference<>(generatePerRouteInterceptors( + hcm.httpFilterConfigs(), rds.savedVirtualHosts.get())); + rdsPrebuiltInterceptorRef.put(filterChain, interceptorRef); + return ServerRoutingConfig.create(hcm.httpFilterConfigs(), rds.savedVirtualHosts, + interceptorRef); + } + } + + private ImmutableMap generatePerRouteInterceptors( + List namedFilterConfigs, @Nullable List virtualHosts) { + if (virtualHosts == null) { + return null; } + ImmutableMap.Builder perRouteInterceptors = + new ImmutableMap.Builder<>(); + for (VirtualHost virtualHost : virtualHosts) { + for (Route route : virtualHost.routes()) { + List filterInterceptors = new ArrayList<>(); + Map selectedOverrideConfigs = + new HashMap<>(virtualHost.filterConfigOverrides()); + selectedOverrideConfigs.putAll(route.filterConfigOverrides()); + for (NamedFilterConfig namedFilterConfig : namedFilterConfigs) { + FilterConfig filterConfig = namedFilterConfig.filterConfig; + Filter filter = filterRegistry.get(filterConfig.typeUrl()); + if (filter instanceof ServerInterceptorBuilder) { + ServerInterceptor interceptor = + ((ServerInterceptorBuilder) filter).buildServerInterceptor( + filterConfig, selectedOverrideConfigs.get(namedFilterConfig.name)); + if (interceptor != null) { + filterInterceptors.add(interceptor); + } + } else { + logger.log(Level.WARNING, "HttpFilterConfig(type URL: " + + filterConfig.typeUrl() + ") is not supported on server-side. " + + "Probably a bug at ClientXdsClient verification."); + } + } + ServerInterceptor interceptor = combineInterceptors(filterInterceptors); + perRouteInterceptors.put(route, interceptor); + } + } + return perRouteInterceptors.build(); + } + + private ServerInterceptor combineInterceptors(final List interceptors) { + if (interceptors.isEmpty()) { + return noopInterceptor; + } + if (interceptors.size() == 1) { + return interceptors.get(0); + } + return new ServerInterceptor() { + @Override + public Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + // intercept forward + for (int i = interceptors.size() - 1; i >= 0; i--) { + next = InternalServerInterceptors.interceptCallHandlerCreate( + interceptors.get(i), next); + } + return next.startCall(call, headers); + } + }; } private void handleConfigNotFound(StatusException exception) { @@ -507,6 +584,7 @@ private void cleanUpRouteDiscoveryStates() { xdsClient.cancelRdsResourceWatch(rdsName, rdsState); } routeDiscoveryStates.clear(); + rdsPrebuiltInterceptorRef.clear(); } private List getSuppliersInUse() { @@ -560,6 +638,7 @@ public void run() { return; } savedVirtualHosts.set(ImmutableList.copyOf(update.virtualHosts)); + updateRdsPrebuiltInterceptorRef(update.virtualHosts); maybeUpdateSelector(); } }); @@ -575,6 +654,7 @@ public void run() { } logger.log(Level.WARNING, "Rds {0} unavailable", resourceName); savedVirtualHosts.set(null); + updateRdsPrebuiltInterceptorRef(null); maybeUpdateSelector(); } }); @@ -595,6 +675,17 @@ public void run() { }); } + private void updateRdsPrebuiltInterceptorRef(@Nullable List virtualHosts) { + for (FilterChain filterChain : rdsPrebuiltInterceptorRef.keySet()) { + if (resourceName.equals(filterChain.getHttpConnectionManager().rdsName())) { + ImmutableMap updatedInterceptors = + generatePerRouteInterceptors( + filterChain.getHttpConnectionManager().httpFilterConfigs(), virtualHosts); + rdsPrebuiltInterceptorRef.get(filterChain).set(updatedInterceptors); + } + } + } + // Update the selector to use the most recently updated configs only after all rds have been // discovered for the first time. Later changes on rds will be applied through virtual host // list atomic ref. @@ -652,14 +743,11 @@ public Listener interceptCall(ServerCall call, return new Listener() {}; } Route selectedRoute = null; - Map selectedOverrideConfigs = - new HashMap<>(virtualHost.filterConfigOverrides()); MethodDescriptor method = call.getMethodDescriptor(); for (Route route : virtualHost.routes()) { if (RoutingUtils.matchRoute( route.routeMatch(), "/" + method.getFullMethodName(), headers, random)) { selectedRoute = route; - selectedOverrideConfigs.putAll(route.filterConfigOverrides()); break; } } @@ -669,48 +757,12 @@ public Listener interceptCall(ServerCall call, new Metadata()); return new ServerCall.Listener() {}; } - List filterInterceptors = new ArrayList<>(); - for (NamedFilterConfig namedFilterConfig : routingConfig.httpFilterConfigs()) { - FilterConfig filterConfig = namedFilterConfig.filterConfig; - Filter filter = filterRegistry.get(filterConfig.typeUrl()); - if (filter instanceof ServerInterceptorBuilder) { - ServerInterceptor interceptor = - ((ServerInterceptorBuilder) filter).buildServerInterceptor( - filterConfig, selectedOverrideConfigs.get(namedFilterConfig.name)); - if (interceptor != null) { - filterInterceptors.add(interceptor); - } - } else { - call.close( - Status.UNAVAILABLE.withDescription("HttpFilterConfig(type URL: " - + filterConfig.typeUrl() + ") is not supported on server-side."), - new Metadata()); - return new Listener() {}; - } + ServerInterceptor routeInterceptor = noopInterceptor; + Map perRouteInterceptors = routingConfig.interceptors().get(); + if (perRouteInterceptors != null && perRouteInterceptors.get(selectedRoute) != null) { + routeInterceptor = perRouteInterceptors.get(selectedRoute); } - ServerInterceptor interceptor = combineInterceptors(filterInterceptors); - return interceptor.interceptCall(call, headers, next); - } - - private ServerInterceptor combineInterceptors(final List interceptors) { - if (interceptors.isEmpty()) { - return noopInterceptor; - } - if (interceptors.size() == 1) { - return interceptors.get(0); - } - return new ServerInterceptor() { - @Override - public Listener interceptCall(ServerCall call, - Metadata headers, ServerCallHandler next) { - // intercept forward - for (int i = interceptors.size() - 1; i >= 0; i--) { - next = InternalServerInterceptors.interceptCallHandlerCreate( - interceptors.get(i), next); - } - return next.startCall(call, headers); - } - }; + return routeInterceptor.interceptCall(call, headers, next); } } @@ -724,15 +776,19 @@ abstract static class ServerRoutingConfig { abstract AtomicReference> virtualHosts(); + abstract AtomicReference> interceptors(); + /** * Server routing configuration. * */ public static ServerRoutingConfig create(List httpFilterConfigs, - AtomicReference> virtualHosts) { + AtomicReference> virtualHosts, + AtomicReference> interceptors) { checkNotNull(httpFilterConfigs, "httpFilterConfigs"); checkNotNull(virtualHosts, "virtualHosts"); + checkNotNull(interceptors, "interceptors"); return new AutoValue_XdsServerWrapper_ServerRoutingConfig( - ImmutableList.copyOf(httpFilterConfigs), virtualHosts); + ImmutableList.copyOf(httpFilterConfigs), virtualHosts, interceptors); } } } diff --git a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java index 167f3f03c6b..23cab060e84 100644 --- a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; +import io.grpc.ServerInterceptor; import io.grpc.internal.TestUtils.NoopChannelLogger; import io.grpc.netty.GrpcHttp2ConnectionHandler; import io.grpc.netty.InternalProtocolNegotiationEvent; @@ -92,7 +93,8 @@ public class FilterChainMatchingProtocolNegotiatorsTest { private static final String REMOTE_IP = "10.4.2.3"; // source private static final int PORT = 7000; private final ServerRoutingConfig noopConfig = ServerRoutingConfig.create( - new ArrayList(), new AtomicReference>()); + new ArrayList(), new AtomicReference>(), + new AtomicReference>()); @Test public void nofilterChainMatch_defaultSslContext() throws Exception { @@ -241,7 +243,8 @@ public void destPortFails_returnDefaultFilterChain() throws Exception { ServerRoutingConfig routingConfig = ServerRoutingConfig.create( new ArrayList(), new AtomicReference<>( - ImmutableList.of(createVirtualHost("virtual")))); + ImmutableList.of(createVirtualHost("virtual"))), + new AtomicReference>()); FilterChainSelector selector = new FilterChainSelector( ImmutableMap.of(filterChainWithDestPort, routingConfig), defaultFilterChain.getSslContextProviderSupplier(), noopConfig); @@ -1148,7 +1151,9 @@ private static VirtualHost createVirtualHost(String name) { private static ServerRoutingConfig randomConfig(String domain) { return ServerRoutingConfig.create( new ArrayList(), new AtomicReference<>( - ImmutableList.of(createVirtualHost(domain)))); + ImmutableList.of(createVirtualHost(domain))), + new AtomicReference<>(ImmutableMap.of()) + ); } private EnvoyServerProtoData.DownstreamTlsContext createTls() { diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index 876b0913742..006a2c433aa 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -727,9 +727,9 @@ public void run() { @Test @SuppressWarnings("unchecked") - public void interceptor_notServerInterceptor() throws Exception { + public void interceptor_success() throws Exception { ArgumentCaptor interceptorCaptor = - ArgumentCaptor.forClass(ConfigApplyingInterceptor.class); + ArgumentCaptor.forClass(ConfigApplyingInterceptor.class); final SettableFuture start = SettableFuture.create(); Executors.newSingleThreadExecutor().execute(new Runnable() { @Override @@ -744,26 +744,38 @@ public void run() { xdsClient.ldsResource.get(5, TimeUnit.SECONDS); verify(mockBuilder).intercept(interceptorCaptor.capture()); ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); - ServerRoutingConfig routingConfig = createRoutingConfig("/FooService/barMethod", - "foo.google.com", "filter-type-url"); + RouteMatch routeMatch = + RouteMatch.create( + PathMatcher.fromPath("/FooService/barMethod", true), + Collections.emptyList(), null); + Route route = Route.forAction(routeMatch, null, + ImmutableMap.of()); + VirtualHost virtualHost = VirtualHost.create( + "v1", Collections.singletonList("foo.google.com"), Arrays.asList(route), + ImmutableMap.of()); + final List interceptorTrace = new ArrayList<>(); + ServerInterceptor interceptor0 = new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + interceptorTrace.add(0); + return next.startCall(call, headers); + } + }; + ServerRoutingConfig routingConfig = ServerRoutingConfig.create( + Collections.emptyList(), + new AtomicReference<>(ImmutableList.of(virtualHost)), + new AtomicReference<>(ImmutableMap.of(route, interceptor0)) + ); ServerCall serverCall = mock(ServerCall.class); - when(serverCall.getAttributes()).thenReturn( - Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, routingConfig).build()); when(serverCall.getMethodDescriptor()).thenReturn(createMethod("FooService/barMethod")); + when(serverCall.getAttributes()).thenReturn( + Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, routingConfig).build()); when(serverCall.getAuthority()).thenReturn("foo.google.com"); - - Filter filter = mock(Filter.class); - when(filter.typeUrls()).thenReturn(new String[]{"filter-type-url"}); - filterRegistry.register(filter); ServerCallHandler next = mock(ServerCallHandler.class); interceptor.interceptCall(serverCall, new Metadata(), next); - verify(next, never()).startCall(any(ServerCall.class), any(Metadata.class)); - ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(Status.class); - verify(serverCall).close(statusCaptor.capture(), any(Metadata.class)); - Status status = statusCaptor.getValue(); - assertThat(status.getCode()).isEqualTo(Status.UNAVAILABLE.getCode()); - assertThat(status.getDescription()).isEqualTo( - "HttpFilterConfig(type URL: filter-type-url) is not supported on server-side."); + verify(next).startCall(eq(serverCall), any(Metadata.class)); + assertThat(interceptorTrace).isEqualTo(Arrays.asList(0)); } @Test @@ -865,7 +877,9 @@ public void run() { verify(mockBuilder).intercept(interceptorCaptor.capture()); ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); ServerRoutingConfig failingConfig = ServerRoutingConfig.create( - ImmutableList.of(), new AtomicReference>() + ImmutableList.of(), + new AtomicReference>(), + new AtomicReference>() ); ServerCall serverCall = mock(ServerCall.class); when(serverCall.getAttributes()).thenReturn( @@ -884,9 +898,7 @@ public void run() { @Test @SuppressWarnings("unchecked") - public void interceptors() throws Exception { - ArgumentCaptor interceptorCaptor = - ArgumentCaptor.forClass(ConfigApplyingInterceptor.class); + public void buildInterceptor_inline() throws Exception { final SettableFuture start = SettableFuture.create(); Executors.newSingleThreadExecutor().execute(new Runnable() { @Override @@ -899,14 +911,12 @@ public void run() { } }); xdsClient.ldsResource.get(5, TimeUnit.SECONDS); - verify(mockBuilder).intercept(interceptorCaptor.capture()); - final ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); RouteMatch routeMatch = - RouteMatch.create( - PathMatcher.fromPath("/FooService/barMethod", true), - Collections.emptyList(), null); + RouteMatch.create( + PathMatcher.fromPath("/FooService/barMethod", true), + Collections.emptyList(), null); Filter filter = mock(Filter.class, withSettings() - .extraInterfaces(ServerInterceptorBuilder.class)); + .extraInterfaces(ServerInterceptorBuilder.class)); when(filter.typeUrls()).thenReturn(new String[]{"filter-type-url"}); filterRegistry.register(filter); FilterConfig f0 = mock(FilterConfig.class); @@ -916,7 +926,7 @@ public void run() { ServerInterceptor interceptor0 = new ServerInterceptor() { @Override public ServerCall.Listener interceptCall(ServerCall call, - Metadata headers, ServerCallHandler next) { + Metadata headers, ServerCallHandler next) { interceptorTrace.add(0); return next.startCall(call, headers); } @@ -929,55 +939,130 @@ public ServerCall.Listener interceptCall(ServerCallof()); VirtualHost virtualHost = VirtualHost.create( - "v1", Collections.singletonList("foo.google.com"), - Arrays.asList(Route.forAction(routeMatch, null, - ImmutableMap.of())), - ImmutableMap.of("filter-config-name-0", f0Override)); - ServerRoutingConfig routingConfig = ServerRoutingConfig.create( - Arrays.asList(new NamedFilterConfig("filter-config-name-0", f0), - new NamedFilterConfig("filter-config-name-1", f0)), - new AtomicReference<>(ImmutableList.of(virtualHost)) - ); + "v1", Collections.singletonList("foo.google.com"), Arrays.asList(route), + ImmutableMap.of("filter-config-name-0", f0Override)); + HttpConnectionManager hcmVirtual = HttpConnectionManager.forVirtualHosts( + 0L, Collections.singletonList(virtualHost), + Arrays.asList(new NamedFilterConfig("filter-config-name-0", f0), + new NamedFilterConfig("filter-config-name-1", f0))); + EnvoyServerProtoData.FilterChain filterChain = createFilterChain("filter-chain-0", hcmVirtual); + xdsClient.deliverLdsUpdate(Collections.singletonList(filterChain), null); + start.get(5000, TimeUnit.MILLISECONDS); + verify(mockServer).start(); + assertThat(selectorRef.get().getRoutingConfigs().size()).isEqualTo(1); + ServerInterceptor realInterceptor = selectorRef.get().getRoutingConfigs() + .get(filterChain).interceptors().get().get(route); + assertThat(realInterceptor).isNotNull(); + ServerCall serverCall = mock(ServerCall.class); ServerCallHandler mockNext = mock(ServerCallHandler.class); final ServerCall.Listener listener = new ServerCall.Listener() {}; when(mockNext.startCall(any(ServerCall.class), any(Metadata.class))).thenReturn(listener); - when(serverCall.getAttributes()).thenReturn( - Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, routingConfig).build()); - when(serverCall.getMethodDescriptor()).thenReturn(createMethod("FooService/barMethod")); - when(serverCall.getAuthority()).thenReturn("foo.google.com"); + realInterceptor.interceptCall(serverCall, new Metadata(), mockNext); + assertThat(interceptorTrace).isEqualTo(Arrays.asList(1, 0)); + verify(mockNext).startCall(eq(serverCall), any(Metadata.class)); + } - when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, f0Override)) - .thenReturn(null); + @Test + @SuppressWarnings("unchecked") + public void buildInterceptor_rds() throws Exception { + final SettableFuture start = SettableFuture.create(); + Executors.newSingleThreadExecutor().execute(new Runnable() { + @Override + public void run() { + try { + start.set(xdsServerWrapper.start()); + } catch (Exception ex) { + start.setException(ex); + } + } + }); + xdsClient.ldsResource.get(5, TimeUnit.SECONDS); + + Filter filter = mock(Filter.class, withSettings() + .extraInterfaces(ServerInterceptorBuilder.class)); + when(filter.typeUrls()).thenReturn(new String[]{"filter-type-url"}); + filterRegistry.register(filter); + FilterConfig f0 = mock(FilterConfig.class); + FilterConfig f0Override = mock(FilterConfig.class); + when(f0.typeUrl()).thenReturn("filter-type-url"); + final List interceptorTrace = new ArrayList<>(); + ServerInterceptor interceptor0 = new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + interceptorTrace.add(0); + return next.startCall(call, headers); + } + }; + ServerInterceptor interceptor1 = new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + interceptorTrace.add(1); + return next.startCall(call, headers); + } + }; when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, null)) - .thenReturn(null); - ServerCall.Listener configApplyingInterceptorListener = - interceptor.interceptCall(serverCall, new Metadata(), mockNext); - assertThat(configApplyingInterceptorListener).isSameInstanceAs(listener); + .thenReturn(interceptor0); + when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, f0Override)) + .thenReturn(interceptor1); + RouteMatch routeMatch = + RouteMatch.create( + PathMatcher.fromPath("/FooService/barMethod", true), + Collections.emptyList(), null); + + HttpConnectionManager rdsHcm = HttpConnectionManager.forRdsName(0L, "r0", + Arrays.asList(new NamedFilterConfig("filter-config-name-0", f0), + new NamedFilterConfig("filter-config-name-1", f0))); + EnvoyServerProtoData.FilterChain filterChain = createFilterChain("filter-chain-0", rdsHcm); + xdsClient.deliverLdsUpdate(Collections.singletonList(filterChain), null); + Route route = Route.forAction(routeMatch, null, + ImmutableMap.of()); + VirtualHost virtualHost = VirtualHost.create( + "v1", Collections.singletonList("foo.google.com"), Arrays.asList(route), + ImmutableMap.of("filter-config-name-0", f0Override)); + xdsClient.deliverRdsUpdate("r0", Collections.singletonList(virtualHost)); + xdsClient.rdsCount.await(5, TimeUnit.SECONDS); + start.get(5000, TimeUnit.MILLISECONDS); + verify(mockServer).start(); + assertThat(selectorRef.get().getRoutingConfigs().size()).isEqualTo(1); + ServerInterceptor realInterceptor = selectorRef.get().getRoutingConfigs() + .get(filterChain).interceptors().get().get(route); + assertThat(realInterceptor).isNotNull(); + + ServerCall serverCall = mock(ServerCall.class); + ServerCallHandler mockNext = mock(ServerCallHandler.class); + final ServerCall.Listener listener = new ServerCall.Listener() {}; + when(mockNext.startCall(any(ServerCall.class), any(Metadata.class))).thenReturn(listener); + realInterceptor.interceptCall(serverCall, new Metadata(), mockNext); + assertThat(interceptorTrace).isEqualTo(Arrays.asList(1, 0)); verify(mockNext).startCall(eq(serverCall), any(Metadata.class)); - assertThat(interceptorTrace).isEqualTo(Arrays.asList()); - when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, f0Override)) - .thenReturn(null); - when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, null)) - .thenReturn(interceptor0); - configApplyingInterceptorListener = interceptor.interceptCall( - serverCall, new Metadata(), mockNext); - assertThat(configApplyingInterceptorListener).isSameInstanceAs(listener); + xdsClient.rdsCount = new CountDownLatch(1); + virtualHost = VirtualHost.create( + "v1", Collections.singletonList("foo.google.com"), Arrays.asList(route), + ImmutableMap.of()); + xdsClient.deliverRdsUpdate("r0", Collections.singletonList(virtualHost)); + xdsClient.rdsCount.await(5, TimeUnit.SECONDS); + realInterceptor = selectorRef.get().getRoutingConfigs() + .get(filterChain).interceptors().get().get(route); + assertThat(realInterceptor).isNotNull(); + interceptorTrace.clear(); + realInterceptor.interceptCall(serverCall, new Metadata(), mockNext); + assertThat(interceptorTrace).isEqualTo(Arrays.asList(0, 0)); verify(mockNext, times(2)).startCall(eq(serverCall), any(Metadata.class)); - assertThat(interceptorTrace).isEqualTo(Arrays.asList(0)); - when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, f0Override)) - .thenReturn(interceptor0); - when(((ServerInterceptorBuilder)filter).buildServerInterceptor(f0, null)) - .thenReturn(interceptor1); - configApplyingInterceptorListener = interceptor.interceptCall( - serverCall, new Metadata(), mockNext); - assertThat(configApplyingInterceptorListener).isSameInstanceAs(listener); - verify(mockNext, times(3)).startCall(eq(serverCall), any(Metadata.class)); - assertThat(interceptorTrace).isEqualTo(Arrays.asList(0, 0, 1)); + xdsClient.rdsWatchers.get("r0").onResourceDoesNotExist("r0"); + assertThat(selectorRef.get().getRoutingConfigs() + .get(filterChain).interceptors().get()).isNull(); } private static FilterChain createFilterChain(String name, HttpConnectionManager hcm) { @@ -992,8 +1077,12 @@ private static VirtualHost createVirtualHost(String name) { } private static HttpConnectionManager createRds(String name) { + return createRds(name, null); + } + + private static HttpConnectionManager createRds(String name, FilterConfig filterConfig) { return HttpConnectionManager.forRdsName(0L, name, - Arrays.asList(new NamedFilterConfig("named-config-" + name, null))); + Arrays.asList(new NamedFilterConfig("named-config-" + name, filterConfig))); } private static EnvoyServerProtoData.FilterChainMatch createMatch() { @@ -1023,7 +1112,8 @@ private static ServerRoutingConfig createRoutingConfig(String path, String domai when(f0.typeUrl()).thenReturn(filterType); return ServerRoutingConfig.create( Arrays.asList(new NamedFilterConfig("filter-config-name-0", f0)), - new AtomicReference<>(ImmutableList.of(virtualHost)) + new AtomicReference<>(ImmutableList.of(virtualHost)), + new AtomicReference<>(ImmutableMap.of()) ); } From 4fe99ff75c111e6bcf6f7ea1a9565484eb77a918 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Wed, 15 Sep 2021 10:02:18 -0700 Subject: [PATCH 2/5] remove unused http filters --- .../java/io/grpc/xds/XdsServerWrapper.java | 17 ++--- ...rChainMatchingProtocolNegotiatorsTest.java | 8 +-- .../io/grpc/xds/XdsServerWrapperTest.java | 65 +++++++------------ 3 files changed, 34 insertions(+), 56 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index b4d7bae0e99..f1cc7e2c34c 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -485,8 +485,8 @@ private ServerRoutingConfig generateRoutingConfig(FilterChain filterChain) { AtomicReference> interceptorRef = new AtomicReference<>(generatePerRouteInterceptors( hcm.httpFilterConfigs(), hcm.virtualHosts())); - return ServerRoutingConfig.create(hcm.httpFilterConfigs(), - new AtomicReference<>(hcm.virtualHosts()), interceptorRef); + return ServerRoutingConfig.create(new AtomicReference<>(hcm.virtualHosts()), + interceptorRef); } else { RouteDiscoveryState rds = routeDiscoveryStates.get(hcm.rdsName()); checkNotNull(rds, "rds"); @@ -494,8 +494,7 @@ private ServerRoutingConfig generateRoutingConfig(FilterChain filterChain) { new AtomicReference<>(generatePerRouteInterceptors( hcm.httpFilterConfigs(), rds.savedVirtualHosts.get())); rdsPrebuiltInterceptorRef.put(filterChain, interceptorRef); - return ServerRoutingConfig.create(hcm.httpFilterConfigs(), rds.savedVirtualHosts, - interceptorRef); + return ServerRoutingConfig.create(rds.savedVirtualHosts, interceptorRef); } } @@ -771,24 +770,20 @@ public Listener interceptCall(ServerCall call, */ @AutoValue abstract static class ServerRoutingConfig { - // Top level http filter configs. - abstract ImmutableList httpFilterConfigs(); - abstract AtomicReference> virtualHosts(); + // Prebuilt per route server interceptors from http filter configs. abstract AtomicReference> interceptors(); /** * Server routing configuration. * */ - public static ServerRoutingConfig create(List httpFilterConfigs, + public static ServerRoutingConfig create( AtomicReference> virtualHosts, AtomicReference> interceptors) { - checkNotNull(httpFilterConfigs, "httpFilterConfigs"); checkNotNull(virtualHosts, "virtualHosts"); checkNotNull(interceptors, "interceptors"); - return new AutoValue_XdsServerWrapper_ServerRoutingConfig( - ImmutableList.copyOf(httpFilterConfigs), virtualHosts, interceptors); + return new AutoValue_XdsServerWrapper_ServerRoutingConfig(virtualHosts, interceptors); } } } diff --git a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java index 23cab060e84..8ea28a0c038 100644 --- a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java @@ -93,7 +93,7 @@ public class FilterChainMatchingProtocolNegotiatorsTest { private static final String REMOTE_IP = "10.4.2.3"; // source private static final int PORT = 7000; private final ServerRoutingConfig noopConfig = ServerRoutingConfig.create( - new ArrayList(), new AtomicReference>(), + new AtomicReference>(), new AtomicReference>()); @Test @@ -241,8 +241,7 @@ public void destPortFails_returnDefaultFilterChain() throws Exception { "filter-chain-bar", null, HTTP_CONNECTION_MANAGER, tlsContextForDefaultFilterChain, tlsContextManager); - ServerRoutingConfig routingConfig = ServerRoutingConfig.create( - new ArrayList(), new AtomicReference<>( + ServerRoutingConfig routingConfig = ServerRoutingConfig.create(new AtomicReference<>( ImmutableList.of(createVirtualHost("virtual"))), new AtomicReference>()); FilterChainSelector selector = new FilterChainSelector( @@ -1149,8 +1148,7 @@ private static VirtualHost createVirtualHost(String name) { } private static ServerRoutingConfig randomConfig(String domain) { - return ServerRoutingConfig.create( - new ArrayList(), new AtomicReference<>( + return ServerRoutingConfig.create(new AtomicReference<>( ImmutableList.of(createVirtualHost(domain))), new AtomicReference<>(ImmutableMap.of()) ); diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index 006a2c433aa..56363322a79 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -380,7 +380,7 @@ public void run() { assertThat(selectorRef.get().getRoutingConfigs().size()).isEqualTo(1); ServerRoutingConfig realConfig = selectorRef.get().getRoutingConfigs().get(filterChain); assertThat(realConfig.virtualHosts().get()).isEqualTo(httpConnectionManager.virtualHosts()); - assertThat(realConfig.httpFilterConfigs()).isEqualTo(httpConnectionManager.httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); verify(listener).onServing(); verify(mockServer).start(); } @@ -429,19 +429,15 @@ public void run() { ServerRoutingConfig realConfig = selectorRef.get().getRoutingConfigs().get(f0); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f0.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); assertThat(selectorRef.get().getRoutingConfigs().size()).isEqualTo(2); realConfig = selectorRef.get().getRoutingConfigs().get(f2); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f2.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); realConfig = selectorRef.get().getDefaultRoutingConfig(); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-2"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f3.getHttpConnectionManager().httpFilterConfigs()); assertThat(selectorRef.get().getDefaultSslContextProviderSupplier()).isEqualTo( f3.getSslContextProviderSupplier()); } @@ -478,19 +474,17 @@ public void run() { ServerRoutingConfig realConfig = selectorRef.get().getRoutingConfigs().get(f0); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f0.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + realConfig = selectorRef.get().getRoutingConfigs().get(f1); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); realConfig = selectorRef.get().getDefaultRoutingConfig(); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f2.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); assertThat(selectorRef.get().getDefaultSslContextProviderSupplier()).isSameInstanceAs( f2.getSslContextProviderSupplier()); @@ -509,19 +503,17 @@ public void run() { realConfig = selectorRef.get().getRoutingConfigs().get(f5); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f5.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); realConfig = selectorRef.get().getRoutingConfigs().get(f3); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f3.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); realConfig = selectorRef.get().getDefaultRoutingConfig(); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f4.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(selectorRef.get().getDefaultSslContextProviderSupplier()).isSameInstanceAs( f4.getSslContextProviderSupplier()); verify(mockServer, times(1)).start(); @@ -559,33 +551,29 @@ public void run() { assertThat(selectorRef.get().getRoutingConfigs().size()).isEqualTo(2); ServerRoutingConfig realConfig = selectorRef.get().getRoutingConfigs().get(f1); assertThat(realConfig.virtualHosts().get()).isNull(); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isNull(); + realConfig = selectorRef.get().getRoutingConfigs().get(f0); assertThat(realConfig.virtualHosts().get()).isEqualTo(hcmVirtual.virtualHosts()); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f0.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); xdsClient.deliverRdsUpdate("r0", Collections.singletonList(createVirtualHost("virtual-host-1"))); realConfig = selectorRef.get().getRoutingConfigs().get(f1); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); xdsClient.rdsWatchers.get("r0").onError(Status.CANCELLED); realConfig = selectorRef.get().getRoutingConfigs().get(f1); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); xdsClient.rdsWatchers.get("r0").onResourceDoesNotExist("r0"); realConfig = selectorRef.get().getRoutingConfigs().get(f1); assertThat(realConfig.virtualHosts().get()).isNull(); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isNull(); } @Test @@ -638,8 +626,8 @@ public void run() { ServerRoutingConfig realConfig = selectorRef.get().getRoutingConfigs().get(filterChain1); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - filterChain1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + // xds update after start xdsClient.deliverRdsUpdate("rds", Collections.singletonList(createVirtualHost("virtual-host-2"))); @@ -652,8 +640,8 @@ public void run() { realConfig = selectorRef.get().getRoutingConfigs().get(filterChain1); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-2"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - filterChain1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(sslSupplier1.isShutdown()).isFalse(); // not serving after serving @@ -690,8 +678,8 @@ public void run() { realConfig = selectorRef.get().getRoutingConfigs().get(filterChain2); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - filterChain2.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(executor.numPendingTasks()).isEqualTo(1); xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); verify(mockServer, times(4)).shutdown(); @@ -716,8 +704,8 @@ public void run() { realConfig = selectorRef.get().getRoutingConfigs().get(filterChain3); assertThat(realConfig.virtualHosts().get()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - filterChain3.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + xdsServerWrapper.shutdown(); verify(mockServer, times(5)).shutdown(); assertThat(sslSupplier3.isShutdown()).isTrue(); @@ -763,7 +751,6 @@ public ServerCall.Listener interceptCall(ServerCallemptyList(), new AtomicReference<>(ImmutableList.of(virtualHost)), new AtomicReference<>(ImmutableMap.of(route, interceptor0)) ); @@ -877,7 +864,6 @@ public void run() { verify(mockBuilder).intercept(interceptorCaptor.capture()); ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); ServerRoutingConfig failingConfig = ServerRoutingConfig.create( - ImmutableList.of(), new AtomicReference>(), new AtomicReference>() ); @@ -1111,7 +1097,6 @@ private static ServerRoutingConfig createRoutingConfig(String path, String domai FilterConfig f0 = mock(FilterConfig.class); when(f0.typeUrl()).thenReturn(filterType); return ServerRoutingConfig.create( - Arrays.asList(new NamedFilterConfig("filter-config-name-0", f0)), new AtomicReference<>(ImmutableList.of(virtualHost)), new AtomicReference<>(ImmutableMap.of()) ); From f9dedd4b087422c2bf159789461fa15359d3ef9e Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Wed, 15 Sep 2021 14:03:43 -0700 Subject: [PATCH 3/5] atomic reference server routing config --- ...ilterChainMatchingProtocolNegotiators.java | 24 +-- .../java/io/grpc/xds/XdsServerWrapper.java | 111 +++++++------ ...rChainMatchingProtocolNegotiatorsTest.java | 81 ++++------ .../xds/FilterChainSelectorManagerTest.java | 12 +- .../io/grpc/xds/XdsServerWrapperTest.java | 152 +++++++++--------- 5 files changed, 185 insertions(+), 195 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/FilterChainMatchingProtocolNegotiators.java b/xds/src/main/java/io/grpc/xds/FilterChainMatchingProtocolNegotiators.java index 24cd4e9ae7e..b828b862454 100644 --- a/xds/src/main/java/io/grpc/xds/FilterChainMatchingProtocolNegotiators.java +++ b/xds/src/main/java/io/grpc/xds/FilterChainMatchingProtocolNegotiators.java @@ -59,6 +59,7 @@ import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -135,28 +136,29 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc static final class FilterChainSelector { public static final FilterChainSelector NO_FILTER_CHAIN = new FilterChainSelector( - Collections.emptyMap(), null, null); - private final Map routingConfigs; + Collections.>emptyMap(), + null, new AtomicReference()); + private final Map> routingConfigs; @Nullable private final SslContextProviderSupplier defaultSslContextProviderSupplier; @Nullable - private final ServerRoutingConfig defaultRoutingConfig; + private final AtomicReference defaultRoutingConfig; - FilterChainSelector(Map routingConfigs, + FilterChainSelector(Map> routingConfigs, @Nullable SslContextProviderSupplier defaultSslContextProviderSupplier, - @Nullable ServerRoutingConfig defaultRoutingConfig) { + @Nullable AtomicReference defaultRoutingConfig) { this.routingConfigs = checkNotNull(routingConfigs, "routingConfigs"); this.defaultSslContextProviderSupplier = defaultSslContextProviderSupplier; - this.defaultRoutingConfig = defaultRoutingConfig; + this.defaultRoutingConfig = checkNotNull(defaultRoutingConfig, "defaultRoutingConfig"); } @VisibleForTesting - Map getRoutingConfigs() { + Map> getRoutingConfigs() { return routingConfigs; } @VisibleForTesting - ServerRoutingConfig getDefaultRoutingConfig() { + AtomicReference getDefaultRoutingConfig() { return defaultRoutingConfig; } @@ -189,7 +191,7 @@ SelectedConfig select(InetSocketAddress localAddr, InetSocketAddress remoteAddr) return new SelectedConfig( routingConfigs.get(selected), selected.getSslContextProviderSupplier()); } - if (defaultRoutingConfig != null) { + if (defaultRoutingConfig.get() != null) { return new SelectedConfig(defaultRoutingConfig, defaultSslContextProviderSupplier); } return null; @@ -393,11 +395,11 @@ public void close() { * The FilterChain level configuration. */ private static final class SelectedConfig { - private final ServerRoutingConfig routingConfig; + private final AtomicReference routingConfig; @Nullable private final SslContextProviderSupplier sslContextProviderSupplier; - private SelectedConfig(ServerRoutingConfig routingConfig, + private SelectedConfig(AtomicReference routingConfig, @Nullable SslContextProviderSupplier sslContextProviderSupplier) { this.routingConfig = checkNotNull(routingConfig, "routingConfig"); this.sslContextProviderSupplier = sslContextProviderSupplier; diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index c019927d237..e28a2d3ec96 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -89,8 +89,9 @@ public void uncaughtException(Thread t, Throwable e) { } }); - public static final Attributes.Key ATTR_SERVER_ROUTING_CONFIG = - Attributes.Key.create("io.grpc.xds.ServerWrapper.serverRoutingConfig"); + public static final Attributes.Key> + ATTR_SERVER_ROUTING_CONFIG = + Attributes.Key.create("io.grpc.xds.ServerWrapper.serverRoutingConfig"); @VisibleForTesting static final long RETRY_DELAY_NANOS = TimeUnit.MINUTES.toNanos(1); @@ -348,8 +349,8 @@ private final class DiscoveryState implements LdsResourceWatcher { @Nullable private FilterChain defaultFilterChain; private boolean stopped; - private final Map>> - rdsPrebuiltInterceptorRef = new HashMap<>(); + private final Map> savedRdsRoutingConfigRef + = new HashMap<>(); private final ServerInterceptor noopInterceptor = new ServerInterceptor() { @Override public Listener interceptCall(ServerCall call, @@ -463,15 +464,16 @@ private void shutdown() { } private void updateSelector() { - Map filterChainRouting = new HashMap<>(); - rdsPrebuiltInterceptorRef.clear(); + Map> filterChainRouting = new HashMap<>(); + savedRdsRoutingConfigRef.clear(); for (FilterChain filterChain: filterChains) { filterChainRouting.put(filterChain, generateRoutingConfig(filterChain)); } FilterChainSelector selector = new FilterChainSelector( Collections.unmodifiableMap(filterChainRouting), defaultFilterChain == null ? null : defaultFilterChain.getSslContextProviderSupplier(), - defaultFilterChain == null ? null : generateRoutingConfig(defaultFilterChain)); + defaultFilterChain == null ? new AtomicReference() : + generateRoutingConfig(defaultFilterChain)); List toRelease = getSuppliersInUse(); filterChainSelectorManager.updateSelector(selector); for (SslContextProviderSupplier e: toRelease) { @@ -480,30 +482,32 @@ private void updateSelector() { startDelegateServer(); } - private ServerRoutingConfig generateRoutingConfig(FilterChain filterChain) { + private AtomicReference generateRoutingConfig(FilterChain filterChain) { HttpConnectionManager hcm = filterChain.getHttpConnectionManager(); if (hcm.virtualHosts() != null) { - AtomicReference> interceptorRef = - new AtomicReference<>(generatePerRouteInterceptors( - hcm.httpFilterConfigs(), hcm.virtualHosts())); - return ServerRoutingConfig.create(new AtomicReference<>(hcm.virtualHosts()), - interceptorRef); + ImmutableMap interceptors = generatePerRouteInterceptors( + hcm.httpFilterConfigs(), hcm.virtualHosts()); + return new AtomicReference<>(ServerRoutingConfig.create(hcm.virtualHosts(),interceptors)); } else { RouteDiscoveryState rds = routeDiscoveryStates.get(hcm.rdsName()); checkNotNull(rds, "rds"); - AtomicReference> interceptorRef = - new AtomicReference<>(generatePerRouteInterceptors( - hcm.httpFilterConfigs(), rds.savedVirtualHosts.get())); - rdsPrebuiltInterceptorRef.put(filterChain, interceptorRef); - return ServerRoutingConfig.create(rds.savedVirtualHosts, interceptorRef); + AtomicReference serverRoutingConfigRef = new AtomicReference<>(); + if (rds.savedVirtualHosts != null) { + ImmutableMap interceptors = generatePerRouteInterceptors( + hcm.httpFilterConfigs(), rds.savedVirtualHosts); + ServerRoutingConfig serverRoutingConfig = + ServerRoutingConfig.create(rds.savedVirtualHosts, interceptors); + serverRoutingConfigRef.set(serverRoutingConfig); + } else { + serverRoutingConfigRef.set(ServerRoutingConfig.FAILING_ROUTING_CONFIG); + } + savedRdsRoutingConfigRef.put(filterChain, serverRoutingConfigRef); + return serverRoutingConfigRef; } } private ImmutableMap generatePerRouteInterceptors( - List namedFilterConfigs, @Nullable List virtualHosts) { - if (virtualHosts == null) { - return null; - } + List namedFilterConfigs, List virtualHosts) { ImmutableMap.Builder perRouteInterceptors = new ImmutableMap.Builder<>(); for (VirtualHost virtualHost : virtualHosts) { @@ -584,7 +588,7 @@ private void cleanUpRouteDiscoveryStates() { xdsClient.cancelRdsResourceWatch(rdsName, rdsState); } routeDiscoveryStates.clear(); - rdsPrebuiltInterceptorRef.clear(); + savedRdsRoutingConfigRef.clear(); } private List getSuppliersInUse() { @@ -621,8 +625,7 @@ private void releaseSuppliersInFlight() { private final class RouteDiscoveryState implements RdsResourceWatcher { private final String resourceName; - private AtomicReference> savedVirtualHosts = - new AtomicReference<>(); + private ImmutableList savedVirtualHosts; private boolean isPending = true; private RouteDiscoveryState(String resourceName) { @@ -637,8 +640,8 @@ public void run() { if (!routeDiscoveryStates.containsKey(resourceName)) { return; } - savedVirtualHosts.set(ImmutableList.copyOf(update.virtualHosts)); - updateRdsPrebuiltInterceptorRef(update.virtualHosts); + savedVirtualHosts = ImmutableList.copyOf(update.virtualHosts); + updateRdsRoutingConfig(); maybeUpdateSelector(); } }); @@ -653,8 +656,8 @@ public void run() { return; } logger.log(Level.WARNING, "Rds {0} unavailable", resourceName); - savedVirtualHosts.set(null); - updateRdsPrebuiltInterceptorRef(null); + savedVirtualHosts = null; + updateRdsRoutingConfig(); maybeUpdateSelector(); } }); @@ -675,13 +678,21 @@ public void run() { }); } - private void updateRdsPrebuiltInterceptorRef(@Nullable List virtualHosts) { - for (FilterChain filterChain : rdsPrebuiltInterceptorRef.keySet()) { + private void updateRdsRoutingConfig() { + for (FilterChain filterChain : savedRdsRoutingConfigRef.keySet()) { if (resourceName.equals(filterChain.getHttpConnectionManager().rdsName())) { - ImmutableMap updatedInterceptors = - generatePerRouteInterceptors( - filterChain.getHttpConnectionManager().httpFilterConfigs(), virtualHosts); - rdsPrebuiltInterceptorRef.get(filterChain).set(updatedInterceptors); + ServerRoutingConfig updatedRoutingConfig; + if (savedVirtualHosts == null) { + updatedRoutingConfig = ServerRoutingConfig.FAILING_ROUTING_CONFIG; + } else { + ImmutableMap updatedInterceptors = + generatePerRouteInterceptors( + filterChain.getHttpConnectionManager().httpFilterConfigs(), + savedVirtualHosts); + updatedRoutingConfig = ServerRoutingConfig.create(savedVirtualHosts, + updatedInterceptors); + } + savedRdsRoutingConfigRef.get(filterChain).set(updatedRoutingConfig); } } } @@ -722,18 +733,16 @@ public Listener interceptCall(ServerCall call, @Override public Listener interceptCall(ServerCall call, Metadata headers, ServerCallHandler next) { - ServerRoutingConfig routingConfig = call.getAttributes().get(ATTR_SERVER_ROUTING_CONFIG); - if (routingConfig == null) { - String errorMsg = "Missing xDS routing config."; - call.close(Status.UNAVAILABLE.withDescription(errorMsg), new Metadata()); - return new Listener() {}; - } - List virtualHosts = routingConfig.virtualHosts().get(); - if (virtualHosts == null) { - String errorMsg = "Missing xDS routing config VirtualHosts due to RDS config unavailable."; + AtomicReference routingConfigRef = + call.getAttributes().get(ATTR_SERVER_ROUTING_CONFIG); + ServerRoutingConfig routingConfig = routingConfigRef == null ? null : + routingConfigRef.get(); + if (routingConfig == null || routingConfig == ServerRoutingConfig.FAILING_ROUTING_CONFIG) { + String errorMsg = "Missing or broken xDS routing config: RDS config unavailable."; call.close(Status.UNAVAILABLE.withDescription(errorMsg), new Metadata()); return new Listener() {}; } + List virtualHosts = routingConfig.virtualHosts(); VirtualHost virtualHost = RoutingUtils.findVirtualHostForHostName( virtualHosts, call.getAuthority()); if (virtualHost == null) { @@ -758,7 +767,7 @@ public Listener interceptCall(ServerCall call, return new ServerCall.Listener() {}; } ServerInterceptor routeInterceptor = noopInterceptor; - Map perRouteInterceptors = routingConfig.interceptors().get(); + Map perRouteInterceptors = routingConfig.interceptors(); if (perRouteInterceptors != null && perRouteInterceptors.get(selectedRoute) != null) { routeInterceptor = perRouteInterceptors.get(selectedRoute); } @@ -771,17 +780,21 @@ public Listener interceptCall(ServerCall call, */ @AutoValue abstract static class ServerRoutingConfig { - abstract AtomicReference> virtualHosts(); + @VisibleForTesting + static final ServerRoutingConfig FAILING_ROUTING_CONFIG = ServerRoutingConfig.create( + ImmutableList.of(), ImmutableMap.of()); + + abstract ImmutableList virtualHosts(); // Prebuilt per route server interceptors from http filter configs. - abstract AtomicReference> interceptors(); + abstract ImmutableMap interceptors(); /** * Server routing configuration. * */ public static ServerRoutingConfig create( - AtomicReference> virtualHosts, - AtomicReference> interceptors) { + ImmutableList virtualHosts, + ImmutableMap interceptors) { checkNotNull(virtualHosts, "virtualHosts"); checkNotNull(interceptors, "interceptors"); return new AutoValue_XdsServerWrapper_ServerRoutingConfig(virtualHosts, interceptors); diff --git a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java index e65ca38ede4..b223516465f 100644 --- a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java @@ -94,9 +94,12 @@ public class FilterChainMatchingProtocolNegotiatorsTest { private static final String LOCAL_IP = "10.1.2.3"; // dest private static final String REMOTE_IP = "10.4.2.3"; // source private static final int PORT = 7000; - private final ServerRoutingConfig noopConfig = ServerRoutingConfig.create( - new AtomicReference>(), - new AtomicReference>()); + private final AtomicReference noopConfig = new AtomicReference<>( + ServerRoutingConfig.create(ImmutableList.of(), + ImmutableMap.of())); + final SettableFuture sslSet = SettableFuture.create(); + final SettableFuture> routingSettable = + SettableFuture.create(); @After @SuppressWarnings("FutureReturnValueIgnored") @@ -110,15 +113,14 @@ public void tearDown() { @Test public void nofilterChainMatch_defaultSslContext() throws Exception { - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); SslContextProviderSupplier defaultSsl = new SslContextProviderSupplier(createTls(), tlsContextManager); selectorManager.updateSelector(new FilterChainSelector( - new HashMap(), defaultSsl, noopConfig)); + new HashMap>(), + defaultSsl, noopConfig)); FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); setupChannel("172.168.1.1", "172.168.1.2", 80, filterChainMatchingHandler); @@ -140,7 +142,8 @@ public void nofilterChainMatch_defaultSslContext() throws Exception { @Test public void noFilterChainMatch_noDefaultSslContext() { selectorManager.updateSelector(new FilterChainSelector( - new HashMap(), null, null)); + new HashMap>(), + null, new AtomicReference())); FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); setupChannel("172.168.1.1", "172.168.2.2", 90, filterChainMatchingHandler); @@ -158,7 +161,7 @@ public void filterSelectorChange_drainsConnection() { ChannelHandler next = new ChannelInboundHandlerAdapter(); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); selectorManager.updateSelector(new FilterChainSelector( - new HashMap(), null, noopConfig)); + new HashMap>(), null, noopConfig)); FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); setupChannel("172.168.1.1", "172.168.2.2", 90, filterChainMatchingHandler); @@ -175,7 +178,7 @@ public void filterSelectorChange_drainsConnection() { assertThat(channel.readOutbound()).isNull(); selectorManager.updateSelector(new FilterChainSelector( - new HashMap(), null, noopConfig)); + new HashMap>(), null, noopConfig)); assertThat(channel.readOutbound().getClass().getName()) .isEqualTo("io.grpc.netty.GracefulServerCloseCommand"); } @@ -199,11 +202,9 @@ public void singleFilterChainWithoutAlpn() throws Exception { tlsContextManager); selectorManager.updateSelector(new FilterChainSelector(ImmutableMap.of(filterChain, noopConfig), - null, null)); + null, new AtomicReference())); FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -243,8 +244,6 @@ public void singleFilterChainWithAlpn() throws Exception { FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -280,18 +279,17 @@ public void destPortFails_returnDefaultFilterChain() throws Exception { "filter-chain-bar", null, HTTP_CONNECTION_MANAGER, tlsContextForDefaultFilterChain, tlsContextManager); - ServerRoutingConfig routingConfig = ServerRoutingConfig.create(new AtomicReference<>( - ImmutableList.of(createVirtualHost("virtual"))), - new AtomicReference>()); + ServerRoutingConfig routingConfig = ServerRoutingConfig.create( + ImmutableList.of(createVirtualHost("virtual")), + ImmutableMap.of()); selectorManager.updateSelector(new FilterChainSelector( - ImmutableMap.of(filterChainWithDestPort, routingConfig), + ImmutableMap.of(filterChainWithDestPort, + new AtomicReference(routingConfig)), defaultFilterChain.getSslContextProviderSupplier(), noopConfig)); FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -333,8 +331,6 @@ public void destPrefixRangeMatch() throws Exception { FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -377,8 +373,6 @@ public void destPrefixRangeMismatch_returnDefaultFilterChain() FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -417,12 +411,11 @@ public void dest0LengthPrefixRange() selectorManager.updateSelector(new FilterChainSelector( ImmutableMap.of(filterChain0Length, noopConfig), - defaultFilterChain.getSslContextProviderSupplier(), null)); + defaultFilterChain.getSslContextProviderSupplier(), + new AtomicReference())); FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -480,8 +473,6 @@ public void destPrefixRange_moreSpecificWins() FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -538,8 +529,6 @@ public void destPrefixRange_emptyListLessSpecific() FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -596,8 +585,6 @@ public void destPrefixRangeIpv6_moreSpecificWins() FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); @@ -658,8 +645,6 @@ filterChainLessSpecific, randomConfig("no-match")), FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -700,8 +685,7 @@ public void sourceTypeMismatch_returnDefaultFilterChain() throws Exception { FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); + ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -714,8 +698,6 @@ public void sourceTypeMismatch_returnDefaultFilterChain() throws Exception { @Test public void sourceTypeLocal() throws Exception { - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); EnvoyServerProtoData.DownstreamTlsContext tlsContextMatch = @@ -755,8 +737,6 @@ public void sourceTypeLocal() throws Exception { @Test public void sourcePrefixRange_moreSpecificWith2Wins() throws Exception { - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); @@ -817,7 +797,6 @@ filterChainLessSpecific, randomConfig("no-match")), @Test public void sourcePrefixRange_2Matchers_expectException() throws UnknownHostException { - final SettableFuture sslSet = SettableFuture.create(); ChannelHandler next = new ChannelInboundHandlerAdapter() { @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { @@ -932,8 +911,6 @@ public void sourcePortMatch_exactMatchWinsOverEmptyList() throws Exception { FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -1074,7 +1051,7 @@ public void filterChain_5stepMatch() throws Exception { EnvoyServerProtoData.FilterChain defaultFilterChain = new EnvoyServerProtoData.FilterChain( "filter-chain-7", null, HTTP_CONNECTION_MANAGER, null, tlsContextManager); - Map map = new HashMap<>(); + Map> map = new HashMap<>(); map.put(filterChain1, randomConfig("1")); map.put(filterChain2, randomConfig("2")); map.put(filterChain3, randomConfig("3")); @@ -1087,8 +1064,6 @@ public void filterChain_5stepMatch() throws Exception { FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -1161,8 +1136,6 @@ public void filterChainMatch_unsupportedMatchers() throws Exception { FilterChainMatchingHandler filterChainMatchingHandler = new FilterChainMatchingHandler(grpcHandler, selectorManager, mockDelegate); - final SettableFuture sslSet = SettableFuture.create(); - final SettableFuture routingSettable = SettableFuture.create(); ChannelHandler next = captureAttrHandler(sslSet, routingSettable); when(mockDelegate.newHandler(grpcHandler)).thenReturn(next); setupChannel(LOCAL_IP, REMOTE_IP, 15000, filterChainMatchingHandler); @@ -1186,10 +1159,10 @@ private static VirtualHost createVirtualHost(String name) { ImmutableMap.of()); } - private static ServerRoutingConfig randomConfig(String domain) { - return ServerRoutingConfig.create(new AtomicReference<>( - ImmutableList.of(createVirtualHost(domain))), - new AtomicReference<>(ImmutableMap.of()) + private static AtomicReference randomConfig(String domain) { + return new AtomicReference<>( + ServerRoutingConfig.create(ImmutableList.of(createVirtualHost(domain)), + ImmutableMap.of()) ); } @@ -1219,7 +1192,7 @@ public SocketAddress remoteAddress() { private static ChannelHandler captureAttrHandler( final SettableFuture sslSet, - final SettableFuture routingSettable) { + final SettableFuture> routingSettable) { return new ChannelInboundHandlerAdapter() { @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { diff --git a/xds/src/test/java/io/grpc/xds/FilterChainSelectorManagerTest.java b/xds/src/test/java/io/grpc/xds/FilterChainSelectorManagerTest.java index b892a6548f1..a3a2218d4c3 100644 --- a/xds/src/test/java/io/grpc/xds/FilterChainSelectorManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/FilterChainSelectorManagerTest.java @@ -34,13 +34,15 @@ @RunWith(JUnit4.class) public final class FilterChainSelectorManagerTest { private FilterChainSelectorManager manager = new FilterChainSelectorManager(); - private ServerRoutingConfig noopConfig = ServerRoutingConfig.create( - new AtomicReference>(), - new AtomicReference>()); + private AtomicReference noopConfig = new AtomicReference<>( + ServerRoutingConfig.create(ImmutableList.of(), + ImmutableMap.of())); private FilterChainSelector selector1 = new FilterChainSelector( - Collections.emptyMap(), null, null); + Collections.>emptyMap(), + null, new AtomicReference()); private FilterChainSelector selector2 = new FilterChainSelector( - Collections.emptyMap(), null, noopConfig); + Collections.>emptyMap(), + null, noopConfig); private CounterRunnable runnable1 = new CounterRunnable(); private CounterRunnable runnable2 = new CounterRunnable(); diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index eb42fdd9270..6fa3664116f 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -104,6 +104,8 @@ public class XdsServerWrapperTest { private FakeXdsClient xdsClient = new FakeXdsClient(); private FilterRegistry filterRegistry = FilterRegistry.getDefaultRegistry(); private XdsServerWrapper xdsServerWrapper; + private ServerRoutingConfig noopConfig = ServerRoutingConfig.create( + ImmutableList.of(), ImmutableMap.of()); @Before public void setup() { @@ -380,9 +382,9 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(1); ServerRoutingConfig realConfig = - selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(filterChain); - assertThat(realConfig.virtualHosts().get()).isEqualTo(httpConnectionManager.virtualHosts()); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(filterChain).get(); + assertThat(realConfig.virtualHosts()).isEqualTo(httpConnectionManager.virtualHosts()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); verify(listener).onServing(); verify(mockServer).start(); } @@ -429,18 +431,18 @@ public void run() { start.get(5000, TimeUnit.MILLISECONDS); verify(mockServer).start(); ServerRoutingConfig realConfig = - selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(2); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f2); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f2).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); - realConfig = selectorManager.getSelectorToUpdateSelector().getDefaultRoutingConfig(); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + realConfig = selectorManager.getSelectorToUpdateSelector().getDefaultRoutingConfig().get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-2"))); assertThat(selectorManager.getSelectorToUpdateSelector().getDefaultSslContextProviderSupplier()) .isEqualTo(f3.getSslContextProviderSupplier()); @@ -476,20 +478,20 @@ public void run() { start.get(5000, TimeUnit.MILLISECONDS); verify(mockServer, times(1)).start(); ServerRoutingConfig realConfig = - selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); - realConfig = selectorManager.getSelectorToUpdateSelector().getDefaultRoutingConfig(); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getDefaultRoutingConfig().get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(selectorManager.getSelectorToUpdateSelector().getDefaultSslContextProviderSupplier()) .isSameInstanceAs(f2.getSslContextProviderSupplier()); @@ -506,19 +508,19 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(2); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f5); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f5).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f3); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f3).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); - realConfig = selectorManager.getSelectorToUpdateSelector().getDefaultRoutingConfig(); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getDefaultRoutingConfig().get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(selectorManager.getSelectorToUpdateSelector().getDefaultSslContextProviderSupplier()) .isSameInstanceAs(f4.getSslContextProviderSupplier()); @@ -557,31 +559,31 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(2); ServerRoutingConfig realConfig = - selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1); - assertThat(realConfig.virtualHosts().get()).isNull(); - assertThat(realConfig.interceptors().get()).isNull(); + selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEmpty(); + assertThat(realConfig.interceptors()).isEmpty(); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0); - assertThat(realConfig.virtualHosts().get()).isEqualTo(hcmVirtual.virtualHosts()); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0).get(); + assertThat(realConfig.virtualHosts()).isEqualTo(hcmVirtual.virtualHosts()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); xdsClient.deliverRdsUpdate("r0", Collections.singletonList(createVirtualHost("virtual-host-1"))); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); xdsClient.rdsWatchers.get("r0").onError(Status.CANCELLED); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); xdsClient.rdsWatchers.get("r0").onResourceDoesNotExist("r0"); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1); - assertThat(realConfig.virtualHosts().get()).isNull(); - assertThat(realConfig.interceptors().get()).isNull(); + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEmpty(); + assertThat(realConfig.interceptors()).isEmpty(); } @Test @@ -634,10 +636,10 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(1); ServerRoutingConfig realConfig = - selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(filterChain1); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(filterChain1).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); // xds update after start xdsClient.deliverRdsUpdate("rds", @@ -650,10 +652,10 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(1); realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs() - .get(filterChain1); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + .get(filterChain1).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-2"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(sslSupplier1.isShutdown()).isFalse(); @@ -691,10 +693,10 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(1); realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs() - .get(filterChain2); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + .get(filterChain2).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(executor.numPendingTasks()).isEqualTo(1); xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); @@ -719,10 +721,10 @@ public void run() { assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(1); realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs() - .get(filterChain3); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + .get(filterChain3).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); - assertThat(realConfig.interceptors().get()).isEqualTo(ImmutableMap.of()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); xdsServerWrapper.shutdown(); verify(mockServer, times(5)).shutdown(); @@ -768,14 +770,13 @@ public ServerCall.Listener interceptCall(ServerCall(ImmutableList.of(virtualHost)), - new AtomicReference<>(ImmutableMap.of(route, interceptor0)) - ); + ServerRoutingConfig realConfig = ServerRoutingConfig.create( + ImmutableList.of(virtualHost), ImmutableMap.of(route, interceptor0)); ServerCall serverCall = mock(ServerCall.class); when(serverCall.getMethodDescriptor()).thenReturn(createMethod("FooService/barMethod")); when(serverCall.getAttributes()).thenReturn( - Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, routingConfig).build()); + Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, + new AtomicReference<>(realConfig)).build()); when(serverCall.getAuthority()).thenReturn("foo.google.com"); ServerCallHandler next = mock(ServerCallHandler.class); interceptor.interceptCall(serverCall, new Metadata(), next); @@ -806,7 +807,8 @@ public void run() { "foo.google.com", "filter-type-url"); ServerCall serverCall = mock(ServerCall.class); when(serverCall.getAttributes()).thenReturn( - Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, routingConfig).build()); + Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, + new AtomicReference<>(routingConfig)).build()); when(serverCall.getAuthority()).thenReturn("not-match.google.com"); Filter filter = mock(Filter.class); @@ -845,7 +847,8 @@ public void run() { "foo.google.com", "filter-type-url"); ServerCall serverCall = mock(ServerCall.class); when(serverCall.getAttributes()).thenReturn( - Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, routingConfig).build()); + Attributes.newBuilder() + .set(ATTR_SERVER_ROUTING_CONFIG, new AtomicReference<>(routingConfig)).build()); when(serverCall.getMethodDescriptor()).thenReturn(createMethod("NotMatchMethod")); when(serverCall.getAuthority()).thenReturn("foo.google.com"); @@ -881,13 +884,11 @@ public void run() { xdsClient.ldsResource.get(5, TimeUnit.SECONDS); verify(mockBuilder).intercept(interceptorCaptor.capture()); ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); - ServerRoutingConfig failingConfig = ServerRoutingConfig.create( - new AtomicReference>(), - new AtomicReference>() - ); ServerCall serverCall = mock(ServerCall.class); + when(serverCall.getAttributes()).thenReturn( - Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, failingConfig).build()); + Attributes.newBuilder().set(ATTR_SERVER_ROUTING_CONFIG, + new AtomicReference<>(ServerRoutingConfig.FAILING_ROUTING_CONFIG)).build()); ServerCallHandler next = mock(ServerCallHandler.class); interceptor.interceptCall(serverCall, new Metadata(), next); @@ -897,7 +898,7 @@ public void run() { Status status = statusCaptor.getValue(); assertThat(status.getCode()).isEqualTo(Status.UNAVAILABLE.getCode()); assertThat(status.getDescription()).isEqualTo( - "Missing xDS routing config VirtualHosts due to RDS config unavailable."); + "Missing or broken xDS routing config: RDS config unavailable."); } @Test @@ -963,7 +964,7 @@ public ServerCall.Listener interceptCall(ServerCall serverCall = mock(ServerCall.class); @@ -1041,7 +1042,7 @@ public ServerCall.Listener interceptCall(ServerCall serverCall = mock(ServerCall.class); @@ -1059,7 +1060,7 @@ public ServerCall.Listener interceptCall(ServerCall ServerCall.Listener interceptCall(ServerCallemptyMap()); FilterConfig f0 = mock(FilterConfig.class); when(f0.typeUrl()).thenReturn(filterType); - return ServerRoutingConfig.create( - new AtomicReference<>(ImmutableList.of(virtualHost)), - new AtomicReference<>(ImmutableMap.of()) + return ServerRoutingConfig.create(ImmutableList.of(virtualHost), + ImmutableMap.of() ); } From e14ca9395aca15cee78239ca2d04ffc4131dd46b Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Wed, 15 Sep 2021 14:57:25 -0700 Subject: [PATCH 4/5] fix rds --- xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index 6fa3664116f..c109bb44a13 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -1035,8 +1035,8 @@ public ServerCall.Listener interceptCall(ServerCall ServerCall.Listener interceptCall(ServerCallof()); xdsClient.deliverRdsUpdate("r0", Collections.singletonList(virtualHost)); - xdsClient.rdsCount.await(5, TimeUnit.SECONDS); realInterceptor = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs() .get(filterChain).get().interceptors().get(route); assertThat(realInterceptor).isNotNull(); From d286dd007f7292a317b9dd9eefbd8593a2e9b29e Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Wed, 15 Sep 2021 17:11:35 -0700 Subject: [PATCH 5/5] remove empty line --- xds/src/main/java/io/grpc/xds/XdsServerWrapper.java | 1 - 1 file changed, 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index e28a2d3ec96..e7301500e0e 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -56,7 +56,6 @@ import io.grpc.xds.XdsNameResolverProvider.XdsClientPoolFactory; import io.grpc.xds.XdsServerBuilder.XdsServingStatusListener; import io.grpc.xds.internal.sds.SslContextProviderSupplier; - import java.io.IOException; import java.net.SocketAddress; import java.util.ArrayList;