From cabbd25bc37d1ed49604253201a2f2d1c679c73f Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 16 Sep 2021 12:35:09 -0700 Subject: [PATCH 1/2] xds, rbac: build per route serverInterceptor for httpConfig (#8524) --- ...ilterChainMatchingProtocolNegotiators.java | 24 +- .../java/io/grpc/xds/XdsServerWrapper.java | 209 ++++++---- ...rChainMatchingProtocolNegotiatorsTest.java | 80 ++-- .../xds/FilterChainSelectorManagerTest.java | 15 +- .../io/grpc/xds/XdsServerWrapperTest.java | 383 +++++++++++------- 5 files changed, 414 insertions(+), 297 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 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() ); } From 57cdfa6206d826db3dcddde7563a8c528655a307 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 16 Sep 2021 16:12:52 -0700 Subject: [PATCH 2/2] xds: implement RBAC gRFC misc cases (#8518) --- .../java/io/grpc/xds/ClientXdsClient.java | 12 +++- xds/src/main/java/io/grpc/xds/RbacFilter.java | 16 +++++ .../java/io/grpc/xds/XdsServerWrapper.java | 15 +++-- .../rbac/engine/GrpcAuthorizationEngine.java | 56 +++++++++++++++- .../io/grpc/xds/ClientXdsClientDataTest.java | 17 +++++ .../test/java/io/grpc/xds/RbacFilterTest.java | 43 ++++++++++++ .../io/grpc/xds/XdsServerWrapperTest.java | 63 +++++++++++++++-- .../engine/GrpcAuthorizationEngineTest.java | 67 +++++++++++++++++++ 8 files changed, 275 insertions(+), 14 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index d490c9861b9..3bef4c416e0 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -136,6 +136,10 @@ final class ClientXdsClient extends AbstractXdsClient { static boolean enableRetry = Strings.isNullOrEmpty(System.getenv("GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY")) || Boolean.parseBoolean(System.getenv("GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY")); + @VisibleForTesting + static boolean enableRbac = + Strings.isNullOrEmpty(System.getenv("GRPC_XDS_EXPERIMENTAL_RBAC")) + || Boolean.parseBoolean(System.getenv("GRPC_XDS_EXPERIMENTAL_RBAC")); private static final String TYPE_URL_HTTP_CONNECTION_MANAGER_V2 = "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2" @@ -218,7 +222,7 @@ protected void handleLdsResponse(String versionInfo, List resources, String listener, retainedRdsResources, enableFaultInjection && isResourceV3); } else { ldsUpdate = processServerSideListener( - listener, retainedRdsResources, enableFaultInjection && isResourceV3); + listener, retainedRdsResources, enableRbac); } } catch (ResourceInvalidException e) { errors.add( @@ -729,10 +733,14 @@ private static FilterChainMatch parseFilterChainMatch( static io.grpc.xds.HttpConnectionManager parseHttpConnectionManager( HttpConnectionManager proto, Set rdsResources, FilterRegistry filterRegistry, boolean parseHttpFilter, boolean isForClient) throws ResourceInvalidException { - if (proto.getXffNumTrustedHops() != 0) { + if (enableRbac && proto.getXffNumTrustedHops() != 0) { throw new ResourceInvalidException( "HttpConnectionManager with xff_num_trusted_hops unsupported"); } + if (enableRbac && !proto.getOriginalIpDetectionExtensionsList().isEmpty()) { + throw new ResourceInvalidException("HttpConnectionManager with " + + "original_ip_detection_extensions unsupported"); + } // Obtain max_stream_duration from Http Protocol Options. long maxStreamDuration = 0; if (proto.hasCommonHttpProtocolOptions()) { diff --git a/xds/src/main/java/io/grpc/xds/RbacFilter.java b/xds/src/main/java/io/grpc/xds/RbacFilter.java index 48b4954767a..39f91b475ae 100644 --- a/xds/src/main/java/io/grpc/xds/RbacFilter.java +++ b/xds/src/main/java/io/grpc/xds/RbacFilter.java @@ -28,6 +28,7 @@ import io.envoyproxy.envoy.config.rbac.v3.Principal; import io.envoyproxy.envoy.extensions.filters.http.rbac.v3.RBAC; import io.envoyproxy.envoy.extensions.filters.http.rbac.v3.RBACPerRoute; +import io.envoyproxy.envoy.type.v3.Int32Range; import io.grpc.Metadata; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -45,6 +46,7 @@ import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.AuthenticatedMatcher; import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.DestinationIpMatcher; import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.DestinationPortMatcher; +import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.DestinationPortRangeMatcher; import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.InvertMatcher; import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.Matcher; import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.OrMatcher; @@ -216,6 +218,8 @@ private static Matcher parsePermission(Permission permission) { return createDestinationIpMatcher(permission.getDestinationIp()); case DESTINATION_PORT: return createDestinationPortMatcher(permission.getDestinationPort()); + case DESTINATION_PORT_RANGE: + return parseDestinationPortRangeMatcher(permission.getDestinationPortRange()); case NOT_RULE: return new InvertMatcher(parsePermission(permission.getNotRule())); case METADATA: // hard coded, never match. @@ -291,6 +295,14 @@ private static RequestedServerNameMatcher parseRequestedServerNameMatcher( private static AuthHeaderMatcher parseHeaderMatcher( io.envoyproxy.envoy.config.route.v3.HeaderMatcher proto) { + if (proto.getName().startsWith("grpc-")) { + throw new IllegalArgumentException("Invalid header matcher config: [grpc-] prefixed " + + "header name is not allowed."); + } + if (":scheme".equals(proto.getName())) { + throw new IllegalArgumentException("Invalid header matcher config: header name [:scheme] " + + "is not allowed."); + } return new AuthHeaderMatcher(MatcherParser.parseHeaderMatcher(proto)); } @@ -304,6 +316,10 @@ private static DestinationPortMatcher createDestinationPortMatcher(int port) { return new DestinationPortMatcher(port); } + private static DestinationPortRangeMatcher parseDestinationPortRangeMatcher(Int32Range range) { + return new DestinationPortRangeMatcher(range.getStart(), range.getEnd()); + } + private static DestinationIpMatcher createDestinationIpMatcher(CidrRange cidrRange) { return new DestinationIpMatcher(Matchers.CidrMatcher.create( resolve(cidrRange), cidrRange.getPrefixLen().getValue())); diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index e7301500e0e..fdc5f099bfe 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -639,6 +639,9 @@ public void run() { if (!routeDiscoveryStates.containsKey(resourceName)) { return; } + if (savedVirtualHosts == null && !isPending) { + logger.log(Level.WARNING, "Received valid Rds {0} configuration.", resourceName); + } savedVirtualHosts = ImmutableList.copyOf(update.virtualHosts); updateRdsRoutingConfig(); maybeUpdateSelector(); @@ -746,8 +749,8 @@ public Listener interceptCall(ServerCall call, virtualHosts, call.getAuthority()); if (virtualHost == null) { call.close( - Status.UNAVAILABLE.withDescription("Could not find xDS virtual host matching RPC"), - new Metadata()); + Status.UNAVAILABLE.withDescription("Could not find xDS virtual host matching RPC"), + new Metadata()); return new Listener() {}; } Route selectedRoute = null; @@ -760,11 +763,15 @@ public Listener interceptCall(ServerCall call, } } if (selectedRoute == null) { - call.close( - Status.UNAVAILABLE.withDescription("Could not find xDS route matching RPC"), + call.close(Status.UNAVAILABLE.withDescription("Could not find xDS route matching RPC"), new Metadata()); return new ServerCall.Listener() {}; } + if (selectedRoute.routeAction() != null) { + call.close(Status.UNAVAILABLE.withDescription("Invalid xDS route action for matching " + + "route: only Route.non_forwarding_action should be allowed."), new Metadata()); + return new ServerCall.Listener() {}; + } ServerInterceptor routeInterceptor = noopInterceptor; Map perRouteInterceptors = routingConfig.interceptors(); if (perRouteInterceptors != null && perRouteInterceptors.get(selectedRoute) != null) { diff --git a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngine.java b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngine.java index 6d275d322a2..bb911461a27 100644 --- a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngine.java +++ b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngine.java @@ -20,6 +20,7 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Joiner; +import com.google.common.io.BaseEncoding; import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.ServerCall; @@ -35,6 +36,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -234,6 +236,23 @@ public boolean matches(EvaluateArgs args) { } } + public static final class DestinationPortRangeMatcher implements Matcher { + private final int start; + private final int end; + + /** Start of the range is inclusive. End of the range is exclusive.*/ + public DestinationPortRangeMatcher(int start, int end) { + this.start = start; + this.end = end; + } + + @Override + public boolean matches(EvaluateArgs args) { + int port = args.getDestinationPort(); + return port >= start && port < end; + } + } + public static final class RequestedServerNameMatcher implements Matcher { private final Matchers.StringMatcher delegate; @@ -316,9 +335,44 @@ private Collection getPrincipalNames() { @Nullable private String getHeader(String headerName) { - if (headerName.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { + headerName = headerName.toLowerCase(Locale.ROOT); + if ("te".equals(headerName)) { return null; } + if (":authority".equals(headerName)) { + headerName = "host"; + } + if ("host".equals(headerName)) { + return serverCall.getAuthority(); + } + if (":path".equals(headerName)) { + return getPath(); + } + if (":method".equals(headerName)) { + return "POST"; + } + return deserializeHeader(headerName); + } + + @Nullable + private String deserializeHeader(String headerName) { + if (headerName.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { + Metadata.Key key; + try { + key = Metadata.Key.of(headerName, Metadata.BINARY_BYTE_MARSHALLER); + } catch (IllegalArgumentException e) { + return null; + } + Iterable values = metadata.getAll(key); + if (values == null) { + return null; + } + List encoded = new ArrayList<>(); + for (byte[] v : values) { + encoded.add(BaseEncoding.base64().omitPadding().encode(v)); + } + return Joiner.on(",").join(encoded); + } Metadata.Key key; try { key = Metadata.Key.of(headerName, Metadata.ASCII_STRING_MARSHALLER); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java index 876615d0b39..597ca7df2d9 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java @@ -131,16 +131,20 @@ public class ClientXdsClientDataTest { public final ExpectedException thrown = ExpectedException.none(); private final FilterRegistry filterRegistry = FilterRegistry.getDefaultRegistry(); private boolean originalEnableRetry; + private boolean originalEnableRbac; @Before public void setUp() { originalEnableRetry = ClientXdsClient.enableRetry; assertThat(originalEnableRetry).isTrue(); + originalEnableRbac = ClientXdsClient.enableRbac; + assertThat(originalEnableRbac).isTrue(); } @After public void tearDown() { ClientXdsClient.enableRetry = originalEnableRetry; + ClientXdsClient.enableRbac = originalEnableRbac; } @Test @@ -1108,6 +1112,19 @@ public void parseHttpConnectionManager_xffNumTrustedHopsUnsupported() hcm, new HashSet(), filterRegistry, false /* does not matter */, true /* does not matter */); } + + @Test + public void parseHttpConnectionManager_OriginalIpDetectionExtensionsMustEmpty() + throws ResourceInvalidException { + @SuppressWarnings("deprecation") + HttpConnectionManager hcm = HttpConnectionManager.newBuilder() + .addOriginalIpDetectionExtensions(TypedExtensionConfig.newBuilder().build()) + .build(); + thrown.expect(ResourceInvalidException.class); + thrown.expectMessage("HttpConnectionManager with original_ip_detection_extensions unsupported"); + ClientXdsClient.parseHttpConnectionManager( + hcm, new HashSet(), filterRegistry, false /* does not matter */, false); + } @Test public void parseHttpConnectionManager_missingRdsAndInlinedRouteConfiguration() diff --git a/xds/src/test/java/io/grpc/xds/RbacFilterTest.java b/xds/src/test/java/io/grpc/xds/RbacFilterTest.java index d8f1d8aa825..082c49ef665 100644 --- a/xds/src/test/java/io/grpc/xds/RbacFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/RbacFilterTest.java @@ -41,6 +41,7 @@ import io.envoyproxy.envoy.type.matcher.v3.MetadataMatcher; import io.envoyproxy.envoy.type.matcher.v3.PathMatcher; import io.envoyproxy.envoy.type.matcher.v3.StringMatcher; +import io.envoyproxy.envoy.type.v3.Int32Range; import io.grpc.Attributes; import io.grpc.Grpc; import io.grpc.Metadata; @@ -109,6 +110,33 @@ public void ipPortParser() { assertThat(decision.decision()).isEqualTo(GrpcAuthorizationEngine.Action.DENY); } + @Test + @SuppressWarnings({"unchecked", "deprecation"}) + public void portRangeParser() { + List permissionList = Arrays.asList( + Permission.newBuilder().setDestinationPortRange( + Int32Range.newBuilder().setStart(1010).setEnd(65535).build() + ).build()); + List principalList = Arrays.asList( + Principal.newBuilder().setRemoteIp( + CidrRange.newBuilder().setAddressPrefix("10.10.10.0") + .setPrefixLen(UInt32Value.of(24)).build() + ).build()); + ConfigOrError result = parse(permissionList, principalList); + assertThat(result.errorDetail).isNull(); + ServerCall serverCall = mock(ServerCall.class); + Attributes attributes = Attributes.newBuilder() + .set(Grpc.TRANSPORT_ATTR_REMOTE_ADDR, new InetSocketAddress("10.10.10.0", 1)) + .set(Grpc.TRANSPORT_ATTR_LOCAL_ADDR, new InetSocketAddress("10.10.10.0",9090)) + .build(); + when(serverCall.getAttributes()).thenReturn(attributes); + when(serverCall.getMethodDescriptor()).thenReturn(method().build()); + GrpcAuthorizationEngine engine = + new GrpcAuthorizationEngine(((RbacConfig)result.config).authConfig()); + AuthDecision decision = engine.evaluate(new Metadata(), serverCall); + assertThat(decision.decision()).isEqualTo(GrpcAuthorizationEngine.Action.DENY); + } + @Test @SuppressWarnings("unchecked") public void pathParser() { @@ -172,6 +200,21 @@ public void headerParser() { assertThat(decision.decision()).isEqualTo(GrpcAuthorizationEngine.Action.DENY); } + @Test + @SuppressWarnings("deprecation") + public void headerParser_headerName() { + HeaderMatcher headerMatcher = HeaderMatcher.newBuilder() + .setName("grpc--feature").setExactMatch("win").build(); + List permissionList = Arrays.asList( + Permission.newBuilder().setHeader(headerMatcher).build()); + HeaderMatcher headerMatcher2 = HeaderMatcher.newBuilder() + .setName(":scheme").setExactMatch("win").build(); + List principalList = Arrays.asList( + Principal.newBuilder().setHeader(headerMatcher2).build()); + ConfigOrError result = parseOverride(permissionList, principalList); + assertThat(result.errorDetail).isNotNull(); + } + @Test @SuppressWarnings("unchecked") public void compositeRules() { diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index c109bb44a13..f2b6e9e4790 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -865,6 +865,50 @@ public void run() { assertThat(status.getDescription()).isEqualTo("Could not find xDS route matching RPC"); } + @Test + @SuppressWarnings("unchecked") + public void interceptor_invalidRouteAction() throws Exception { + ArgumentCaptor interceptorCaptor = + ArgumentCaptor.forClass(ConfigApplyingInterceptor.class); + 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); + verify(mockBuilder).intercept(interceptorCaptor.capture()); + ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue(); + ServerRoutingConfig routingConfig = createRoutingConfig("/FooService/barMethod", + "foo.google.com", "filter-type-url", Route.RouteAction.forCluster( + "cluster", Collections.emptyList(), null, null + )); + ServerCall serverCall = mock(ServerCall.class); + when(serverCall.getAttributes()).thenReturn( + Attributes.newBuilder() + .set(ATTR_SERVER_ROUTING_CONFIG, new AtomicReference<>(routingConfig)).build()); + when(serverCall.getMethodDescriptor()).thenReturn(createMethod("FooService/barMethod")); + 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("Invalid xDS route action for matching " + + "route: only Route.non_forwarding_action should be allowed."); + } + @Test @SuppressWarnings("unchecked") public void interceptor_failingRouterConfig() throws Exception { @@ -1104,15 +1148,20 @@ private static EnvoyServerProtoData.FilterChainMatch createMatch() { private static ServerRoutingConfig createRoutingConfig(String path, String domain, String filterType) { + return createRoutingConfig(path, domain, filterType, null); + } + + private static ServerRoutingConfig createRoutingConfig(String path, String domain, + String filterType, Route.RouteAction action) { RouteMatch routeMatch = - RouteMatch.create( - PathMatcher.fromPath(path, true), - Collections.emptyList(), null); + RouteMatch.create( + PathMatcher.fromPath(path, true), + Collections.emptyList(), null); VirtualHost virtualHost = VirtualHost.create( - "v1", Collections.singletonList(domain), - Arrays.asList(Route.forAction(routeMatch, null, - ImmutableMap.of())), - Collections.emptyMap()); + "v1", Collections.singletonList(domain), + Arrays.asList(Route.forAction(routeMatch, action, + ImmutableMap.of())), + Collections.emptyMap()); FilterConfig f0 = mock(FilterConfig.class); when(f0.typeUrl()).thenReturn(filterType); return ServerRoutingConfig.create(ImmutableList.of(virtualHost), diff --git a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngineTest.java b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngineTest.java index 504c9e8df2a..626a4cfc275 100644 --- a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngineTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/GrpcAuthorizationEngineTest.java @@ -16,12 +16,14 @@ package io.grpc.xds.internal.rbac.engine; +import static com.google.common.base.Charsets.US_ASCII; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; +import com.google.common.io.BaseEncoding; import io.grpc.Attributes; import io.grpc.Grpc; import io.grpc.Metadata; @@ -177,6 +179,71 @@ public void headerMatcher() { assertThat(decision.decision()).isEqualTo(Action.DENY); } + @Test + public void headerMatcher_binaryHeader() { + AuthHeaderMatcher headerMatcher = new AuthHeaderMatcher(Matchers.HeaderMatcher + .forExactValue(HEADER_KEY + Metadata.BINARY_HEADER_SUFFIX, + BaseEncoding.base64().omitPadding().encode(HEADER_VALUE.getBytes(US_ASCII)), false)); + OrMatcher principal = OrMatcher.create(headerMatcher); + OrMatcher permission = OrMatcher.create( + new InvertMatcher(new DestinationPortMatcher(PORT + 1))); + PolicyMatcher policyMatcher = new PolicyMatcher(POLICY_NAME, permission, principal); + GrpcAuthorizationEngine engine = new GrpcAuthorizationEngine( + new AuthConfig(Collections.singletonList(policyMatcher), Action.ALLOW)); + Metadata metadata = new Metadata(); + metadata.put(Metadata.Key.of(HEADER_KEY + Metadata.BINARY_HEADER_SUFFIX, + Metadata.BINARY_BYTE_MARSHALLER), HEADER_VALUE.getBytes(US_ASCII)); + AuthDecision decision = engine.evaluate(metadata, serverCall); + assertThat(decision.decision()).isEqualTo(Action.ALLOW); + assertThat(decision.matchingPolicyName()).isEqualTo(POLICY_NAME); + } + + @Test + public void headerMatcher_hardcodePostMethod() { + AuthHeaderMatcher headerMatcher = new AuthHeaderMatcher(Matchers.HeaderMatcher + .forExactValue(":method", "POST", false)); + OrMatcher principal = OrMatcher.create(headerMatcher); + OrMatcher permission = OrMatcher.create( + new InvertMatcher(new DestinationPortMatcher(PORT + 1))); + PolicyMatcher policyMatcher = new PolicyMatcher(POLICY_NAME, permission, principal); + GrpcAuthorizationEngine engine = new GrpcAuthorizationEngine( + new AuthConfig(Collections.singletonList(policyMatcher), Action.ALLOW)); + AuthDecision decision = engine.evaluate(new Metadata(), serverCall); + assertThat(decision.decision()).isEqualTo(Action.ALLOW); + assertThat(decision.matchingPolicyName()).isEqualTo(POLICY_NAME); + } + + @Test + public void headerMatcher_pathHeader() { + AuthHeaderMatcher headerMatcher = new AuthHeaderMatcher(Matchers.HeaderMatcher + .forExactValue(":path", "/" + PATH, false)); + OrMatcher principal = OrMatcher.create(headerMatcher); + OrMatcher permission = OrMatcher.create( + new InvertMatcher(new DestinationPortMatcher(PORT + 1))); + PolicyMatcher policyMatcher = new PolicyMatcher(POLICY_NAME, permission, principal); + GrpcAuthorizationEngine engine = new GrpcAuthorizationEngine( + new AuthConfig(Collections.singletonList(policyMatcher), Action.ALLOW)); + AuthDecision decision = engine.evaluate(HEADER, serverCall); + assertThat(decision.decision()).isEqualTo(Action.ALLOW); + assertThat(decision.matchingPolicyName()).isEqualTo(POLICY_NAME); + } + + @Test + public void headerMatcher_aliasAuthorityAndHost() { + AuthHeaderMatcher headerMatcher = new AuthHeaderMatcher(Matchers.HeaderMatcher + .forExactValue("Host", "google.com", false)); + OrMatcher principal = OrMatcher.create(headerMatcher); + OrMatcher permission = OrMatcher.create( + new InvertMatcher(new DestinationPortMatcher(PORT + 1))); + PolicyMatcher policyMatcher = new PolicyMatcher(POLICY_NAME, permission, principal); + GrpcAuthorizationEngine engine = new GrpcAuthorizationEngine( + new AuthConfig(Collections.singletonList(policyMatcher), Action.ALLOW)); + when(serverCall.getAuthority()).thenReturn("google.com"); + AuthDecision decision = engine.evaluate(new Metadata(), serverCall); + assertThat(decision.decision()).isEqualTo(Action.ALLOW); + assertThat(decision.matchingPolicyName()).isEqualTo(POLICY_NAME); + } + @Test public void pathMatcher() { PathMatcher pathMatcher = new PathMatcher(STRING_MATCHER);