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 29821f2cba8..e7301500e0e 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; @@ -87,8 +88,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); @@ -346,6 +348,15 @@ private final class DiscoveryState implements LdsResourceWatcher { @Nullable private FilterChain defaultFilterChain; private boolean stopped; + private final Map> savedRdsRoutingConfigRef + = 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,14 +463,16 @@ private void shutdown() { } private void updateSelector() { - Map filterChainRouting = new HashMap<>(); + 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) { @@ -468,18 +481,84 @@ private void updateSelector() { startDelegateServer(); } - private ServerRoutingConfig generateRoutingConfig(FilterChain filterChain) { + private AtomicReference generateRoutingConfig(FilterChain filterChain) { HttpConnectionManager hcm = filterChain.getHttpConnectionManager(); if (hcm.virtualHosts() != null) { - return ServerRoutingConfig.create(hcm.httpFilterConfigs(), - new AtomicReference<>(hcm.virtualHosts())); + 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"); - return ServerRoutingConfig.create(hcm.httpFilterConfigs(), rds.savedVirtualHosts); + 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, List virtualHosts) { + 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) { cleanUpRouteDiscoveryStates(); List toRelease = getSuppliersInUse(); @@ -508,6 +587,7 @@ private void cleanUpRouteDiscoveryStates() { xdsClient.cancelRdsResourceWatch(rdsName, rdsState); } routeDiscoveryStates.clear(); + savedRdsRoutingConfigRef.clear(); } private List getSuppliersInUse() { @@ -544,8 +624,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) { @@ -560,7 +639,8 @@ public void run() { if (!routeDiscoveryStates.containsKey(resourceName)) { return; } - savedVirtualHosts.set(ImmutableList.copyOf(update.virtualHosts)); + savedVirtualHosts = ImmutableList.copyOf(update.virtualHosts); + updateRdsRoutingConfig(); maybeUpdateSelector(); } }); @@ -575,7 +655,8 @@ public void run() { return; } logger.log(Level.WARNING, "Rds {0} unavailable", resourceName); - savedVirtualHosts.set(null); + savedVirtualHosts = null; + updateRdsRoutingConfig(); maybeUpdateSelector(); } }); @@ -596,6 +677,25 @@ public void run() { }); } + private void updateRdsRoutingConfig() { + for (FilterChain filterChain : savedRdsRoutingConfigRef.keySet()) { + if (resourceName.equals(filterChain.getHttpConnectionManager().rdsName())) { + 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); + } + } + } + // 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. @@ -632,18 +732,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) { @@ -653,14 +751,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; } } @@ -670,48 +765,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(); + 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); } } @@ -720,20 +779,24 @@ public Listener interceptCall(ServerCall call, */ @AutoValue abstract static class ServerRoutingConfig { - // Top level http filter configs. - abstract ImmutableList httpFilterConfigs(); + @VisibleForTesting + static final ServerRoutingConfig FAILING_ROUTING_CONFIG = ServerRoutingConfig.create( + ImmutableList.of(), ImmutableMap.of()); + + abstract ImmutableList virtualHosts(); - abstract AtomicReference> virtualHosts(); + // Prebuilt per route server interceptors from http filter configs. + abstract ImmutableMap interceptors(); /** * Server routing configuration. * */ - public static ServerRoutingConfig create(List httpFilterConfigs, - AtomicReference> virtualHosts) { - checkNotNull(httpFilterConfigs, "httpFilterConfigs"); + public static ServerRoutingConfig create( + ImmutableList virtualHosts, + ImmutableMap interceptors) { checkNotNull(virtualHosts, "virtualHosts"); - return new AutoValue_XdsServerWrapper_ServerRoutingConfig( - ImmutableList.copyOf(httpFilterConfigs), 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 891dec322c0..b223516465f 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; @@ -93,8 +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 ArrayList(), 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") @@ -108,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); @@ -138,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); @@ -156,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); @@ -173,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"); } @@ -197,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); @@ -241,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); @@ -279,17 +280,16 @@ public void destPortFails_returnDefaultFilterChain() throws Exception { tlsContextForDefaultFilterChain, tlsContextManager); ServerRoutingConfig routingConfig = ServerRoutingConfig.create( - new ArrayList(), new AtomicReference<>( - ImmutableList.of(createVirtualHost("virtual")))); + 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); @@ -331,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); @@ -375,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); @@ -415,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); @@ -478,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); @@ -536,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); @@ -594,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); @@ -656,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); @@ -698,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); @@ -712,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 = @@ -753,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); @@ -815,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) { @@ -930,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); @@ -1072,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")); @@ -1085,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); @@ -1159,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); @@ -1184,10 +1159,11 @@ private static VirtualHost createVirtualHost(String name) { ImmutableMap.of()); } - private static ServerRoutingConfig randomConfig(String domain) { - return ServerRoutingConfig.create( - new ArrayList(), new AtomicReference<>( - ImmutableList.of(createVirtualHost(domain)))); + private static AtomicReference randomConfig(String domain) { + return new AtomicReference<>( + ServerRoutingConfig.create(ImmutableList.of(createVirtualHost(domain)), + ImmutableMap.of()) + ); } private EnvoyServerProtoData.DownstreamTlsContext createTls() { @@ -1216,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 d7b883f1941..a3a2218d4c3 100644 --- a/xds/src/test/java/io/grpc/xds/FilterChainSelectorManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/FilterChainSelectorManagerTest.java @@ -19,8 +19,9 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.grpc.ServerInterceptor; import io.grpc.xds.EnvoyServerProtoData.FilterChain; -import io.grpc.xds.Filter.NamedFilterConfig; import io.grpc.xds.FilterChainMatchingProtocolNegotiators.FilterChainMatchingHandler.FilterChainSelector; import io.grpc.xds.FilterChainSelectorManager.Closer; import io.grpc.xds.XdsServerWrapper.ServerRoutingConfig; @@ -33,13 +34,15 @@ @RunWith(JUnit4.class) public final class FilterChainSelectorManagerTest { private FilterChainSelectorManager manager = new FilterChainSelectorManager(); - private ServerRoutingConfig noopConfig = ServerRoutingConfig.create( - Collections.emptyList(), - 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 d4421361158..c109bb44a13 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.httpFilterConfigs()).isEqualTo(httpConnectionManager.httpFilterConfigs()); + 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,26 +431,21 @@ 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.httpFilterConfigs()).isEqualTo( - f0.getHttpConnectionManager().httpFilterConfigs()); + 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.httpFilterConfigs()).isEqualTo( - f2.getHttpConnectionManager().httpFilterConfigs()); - 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(realConfig.httpFilterConfigs()).isEqualTo( - f3.getHttpConnectionManager().httpFilterConfigs()); assertThat(selectorManager.getSelectorToUpdateSelector().getDefaultSslContextProviderSupplier()) - .isEqualTo( - f3.getSslContextProviderSupplier()); + .isEqualTo(f3.getSslContextProviderSupplier()); } @Test @@ -481,22 +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.httpFilterConfigs()).isEqualTo( - f0.getHttpConnectionManager().httpFilterConfigs()); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1); - assertThat(realConfig.virtualHosts().get()).isEqualTo( + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-0"))); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + 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.httpFilterConfigs()).isEqualTo( - f2.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(selectorManager.getSelectorToUpdateSelector().getDefaultSslContextProviderSupplier()) .isSameInstanceAs(f2.getSslContextProviderSupplier()); @@ -513,25 +508,22 @@ 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.httpFilterConfigs()).isEqualTo( - f5.getHttpConnectionManager().httpFilterConfigs()); - 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.httpFilterConfigs()).isEqualTo( - f3.getHttpConnectionManager().httpFilterConfigs()); + 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.httpFilterConfigs()).isEqualTo( - f4.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + assertThat(selectorManager.getSelectorToUpdateSelector().getDefaultSslContextProviderSupplier()) - .isSameInstanceAs( - f4.getSslContextProviderSupplier()); + .isSameInstanceAs(f4.getSslContextProviderSupplier()); verify(mockServer, times(1)).start(); xdsServerWrapper.shutdown(); verify(mockServer, times(1)).shutdown(); @@ -567,35 +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.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); - realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f0); - assertThat(realConfig.virtualHosts().get()).isEqualTo(hcmVirtual.virtualHosts()); - assertThat(realConfig.httpFilterConfigs()).isEqualTo( - f0.getHttpConnectionManager().httpFilterConfigs()); + selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEmpty(); + assertThat(realConfig.interceptors()).isEmpty(); + + 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.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + 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.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + 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.httpFilterConfigs()).isEqualTo( - f1.getHttpConnectionManager().httpFilterConfigs()); + realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); + assertThat(realConfig.virtualHosts()).isEmpty(); + assertThat(realConfig.interceptors()).isEmpty(); } @Test @@ -648,11 +636,11 @@ 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.httpFilterConfigs()).isEqualTo( - filterChain1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + // xds update after start xdsClient.deliverRdsUpdate("rds", Collections.singletonList(createVirtualHost("virtual-host-2"))); @@ -664,11 +652,11 @@ 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.httpFilterConfigs()).isEqualTo( - filterChain1.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + assertThat(sslSupplier1.isShutdown()).isFalse(); // not serving after serving @@ -705,11 +693,11 @@ 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.httpFilterConfigs()).isEqualTo( - filterChain2.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + assertThat(executor.numPendingTasks()).isEqualTo(1); xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); verify(mockServer, times(4)).shutdown(); @@ -733,11 +721,11 @@ 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.httpFilterConfigs()).isEqualTo( - filterChain3.getHttpConnectionManager().httpFilterConfigs()); + assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); + xdsServerWrapper.shutdown(); verify(mockServer, times(5)).shutdown(); assertThat(sslSupplier3.isShutdown()).isTrue(); @@ -747,9 +735,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 @@ -764,26 +752,36 @@ 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 realConfig = ServerRoutingConfig.create( + ImmutableList.of(virtualHost), 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, + new AtomicReference<>(realConfig)).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 @@ -809,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); @@ -848,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"); @@ -884,12 +884,11 @@ public void run() { xdsClient.ldsResource.get(5, TimeUnit.SECONDS); verify(mockBuilder).intercept(interceptorCaptor.capture()); ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); - ServerRoutingConfig failingConfig = ServerRoutingConfig.create( - ImmutableList.of(), 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); @@ -899,14 +898,12 @@ 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 @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 @@ -919,14 +916,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); @@ -936,7 +931,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); } @@ -949,55 +944,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(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) + .isEqualTo(1); + ServerInterceptor realInterceptor = selectorManager.getSelectorToUpdateSelector() + .getRoutingConfigs().get(filterChain).get().interceptors().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.rdsCount.await(5, TimeUnit.SECONDS); + xdsClient.deliverRdsUpdate("r0", Collections.singletonList(virtualHost)); + start.get(5000, TimeUnit.MILLISECONDS); + verify(mockServer).start(); + assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) + .isEqualTo(1); + ServerInterceptor realInterceptor = selectorManager.getSelectorToUpdateSelector() + .getRoutingConfigs().get(filterChain).get().interceptors().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); + virtualHost = VirtualHost.create( + "v1", Collections.singletonList("foo.google.com"), Arrays.asList(route), + ImmutableMap.of()); + xdsClient.deliverRdsUpdate("r0", Collections.singletonList(virtualHost)); + realInterceptor = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs() + .get(filterChain).get().interceptors().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(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs() + .get(filterChain).get()).isEqualTo(noopConfig); } private static FilterChain createFilterChain(String name, HttpConnectionManager hcm) { @@ -1012,8 +1082,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() { @@ -1041,9 +1115,8 @@ private static ServerRoutingConfig createRoutingConfig(String path, String domai Collections.emptyMap()); 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)) + return ServerRoutingConfig.create(ImmutableList.of(virtualHost), + ImmutableMap.of() ); }