Skip to content

Commit

Permalink
xds: improve code readability of server FilterChain parsing
Browse files Browse the repository at this point in the history 
- Improve code flow and variable names
- Reduce nesting
- Add comments between logical blocks
- Add comments explaining some xDS/gRPC nuances
  • Loading branch information
@sergiitk
sergiitk authored Feb 11, 2025
1 parent 67fc2e1 commit bd6af59
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 72 deletions.
86 changes: 56 additions & 30 deletions xds/src/main/java/io/grpc/xds/XdsListenerResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,11 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
Listener proto, TlsContextManager tlsContextManager,
FilterRegistry filterRegistry, Set<String> certProviderInstances, XdsResourceType.Args args)
throws ResourceInvalidException {
if (!proto.getTrafficDirection().equals(TrafficDirection.INBOUND)
&& !proto.getTrafficDirection().equals(TrafficDirection.UNSPECIFIED)) {
TrafficDirection trafficDirection = proto.getTrafficDirection();
if (!trafficDirection.equals(TrafficDirection.INBOUND)
&& !trafficDirection.equals(TrafficDirection.UNSPECIFIED)) {
throw new ResourceInvalidException(
"Listener " + proto.getName() + " with invalid traffic direction: "
+ proto.getTrafficDirection());
"Listener " + proto.getName() + " with invalid traffic direction: " + trafficDirection);
}
if (!proto.getListenerFiltersList().isEmpty()) {
throw new ResourceInvalidException(
Expand Down Expand Up @@ -178,16 +178,29 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
}

ImmutableList.Builder<FilterChain> filterChains = ImmutableList.builder();
Set<FilterChainMatch> uniqueSet = new HashSet<>();
Set<FilterChainMatch> filterChainMatchSet = new HashSet<>();
int i = 0;
for (io.envoyproxy.envoy.config.listener.v3.FilterChain fc : proto.getFilterChainsList()) {
// May be empty. If it's not empty, required to be unique.
String filterChainName = fc.getName();
if (filterChainName.isEmpty()) {
// Generate a name, so we can identify it in the logs.
filterChainName = "chain_" + i;
}
filterChains.add(
parseFilterChain(fc, tlsContextManager, filterRegistry, uniqueSet,
certProviderInstances, args));
parseFilterChain(fc, filterChainName, tlsContextManager, filterRegistry,
filterChainMatchSet, certProviderInstances, args));
i++;
}

FilterChain defaultFilterChain = null;
if (proto.hasDefaultFilterChain()) {
String defaultFilterChainName = proto.getDefaultFilterChain().getName();
if (defaultFilterChainName.isEmpty()) {
defaultFilterChainName = "chain_default";
}
defaultFilterChain = parseFilterChain(
proto.getDefaultFilterChain(), tlsContextManager, filterRegistry,
proto.getDefaultFilterChain(), defaultFilterChainName, tlsContextManager, filterRegistry,
null, certProviderInstances, args);
}

Expand All @@ -198,36 +211,44 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
@VisibleForTesting
static FilterChain parseFilterChain(
io.envoyproxy.envoy.config.listener.v3.FilterChain proto,
TlsContextManager tlsContextManager, FilterRegistry filterRegistry,
Set<FilterChainMatch> uniqueSet, Set<String> certProviderInstances, XdsResourceType.Args args)
String filterChainName,
TlsContextManager tlsContextManager,
FilterRegistry filterRegistry,
// null disables FilterChainMatch uniqueness check, used for defaultFilterChain
@Nullable Set<FilterChainMatch> filterChainMatchSet,
Set<String> certProviderInstances,
XdsResourceType.Args args)
throws ResourceInvalidException {
// FilterChain contains L4 filters, so we ensure it contains only HCM.
if (proto.getFiltersCount() != 1) {
throw new ResourceInvalidException("FilterChain " + proto.getName()
throw new ResourceInvalidException("FilterChain " + filterChainName
+ " should contain exact one HttpConnectionManager filter");
}
io.envoyproxy.envoy.config.listener.v3.Filter filter = proto.getFiltersList().get(0);
if (!filter.hasTypedConfig()) {
io.envoyproxy.envoy.config.listener.v3.Filter l4Filter = proto.getFiltersList().get(0);
if (!l4Filter.hasTypedConfig()) {
throw new ResourceInvalidException(
"FilterChain " + proto.getName() + " contains filter " + filter.getName()
"FilterChain " + filterChainName + " contains filter " + l4Filter.getName()
+ " without typed_config");
}
Any any = filter.getTypedConfig();
// HttpConnectionManager is the only supported network filter at the moment.
Any any = l4Filter.getTypedConfig();
if (!any.getTypeUrl().equals(TYPE_URL_HTTP_CONNECTION_MANAGER)) {
throw new ResourceInvalidException(
"FilterChain " + proto.getName() + " contains filter " + filter.getName()
"FilterChain " + filterChainName + " contains filter " + l4Filter.getName()
+ " with unsupported typed_config type " + any.getTypeUrl());
}

// Parse HCM.
HttpConnectionManager hcmProto;
try {
hcmProto = any.unpack(HttpConnectionManager.class);
} catch (InvalidProtocolBufferException e) {
throw new ResourceInvalidException("FilterChain " + proto.getName() + " with filter "
+ filter.getName() + " failed to unpack message", e);
throw new ResourceInvalidException("FilterChain " + filterChainName + " with filter "
+ l4Filter.getName() + " failed to unpack message", e);
}
io.grpc.xds.HttpConnectionManager httpConnectionManager = parseHttpConnectionManager(
hcmProto, filterRegistry, false /* isForClient */, args);

// Parse Transport Socket.
EnvoyServerProtoData.DownstreamTlsContext downstreamTlsContext = null;
if (proto.hasTransportSocket()) {
if (!TRANSPORT_SOCKET_NAME_TLS.equals(proto.getTransportSocket().getName())) {
Expand All @@ -239,18 +260,23 @@ static FilterChain parseFilterChain(
downstreamTlsContextProto =
proto.getTransportSocket().getTypedConfig().unpack(DownstreamTlsContext.class);
} catch (InvalidProtocolBufferException e) {
throw new ResourceInvalidException("FilterChain " + proto.getName()
throw new ResourceInvalidException("FilterChain " + filterChainName
+ " failed to unpack message", e);
}
downstreamTlsContext =
EnvoyServerProtoData.DownstreamTlsContext.fromEnvoyProtoDownstreamTlsContext(
validateDownstreamTlsContext(downstreamTlsContextProto, certProviderInstances));
}

// Parse FilterChainMatch.
FilterChainMatch filterChainMatch = parseFilterChainMatch(proto.getFilterChainMatch());
checkForUniqueness(uniqueSet, filterChainMatch);
// null used to skip this check for defaultFilterChain.
if (filterChainMatchSet != null) {
validateFilterChainMatchForUniqueness(filterChainMatchSet, filterChainMatch);
}

return FilterChain.create(
proto.getName(),
filterChainName,
filterChainMatch,
httpConnectionManager,
downstreamTlsContext,
Expand Down Expand Up @@ -284,15 +310,15 @@ static DownstreamTlsContext validateDownstreamTlsContext(
return downstreamTlsContext;
}

private static void checkForUniqueness(Set<FilterChainMatch> uniqueSet,
private static void validateFilterChainMatchForUniqueness(
Set<FilterChainMatch> filterChainMatchSet,
FilterChainMatch filterChainMatch) throws ResourceInvalidException {
if (uniqueSet != null) {
List<FilterChainMatch> crossProduct = getCrossProduct(filterChainMatch);
for (FilterChainMatch cur : crossProduct) {
if (!uniqueSet.add(cur)) {
throw new ResourceInvalidException("FilterChainMatch must be unique. "
+ "Found duplicate: " + cur);
}
// Flattens complex FilterChainMatch into a list of simple FilterChainMatch'es.
List<FilterChainMatch> crossProduct = getCrossProduct(filterChainMatch);
for (FilterChainMatch cur : crossProduct) {
if (!filterChainMatchSet.add(cur)) {
throw new ResourceInvalidException("FilterChainMatch must be unique. "
+ "Found duplicate: " + cur);
}
}
}
Expand Down
77 changes: 49 additions & 28 deletions xds/src/main/java/io/grpc/xds/XdsServerWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import java.io.IOException;
import java.net.SocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -459,47 +458,69 @@ private void shutdown() {
}

private void updateSelector() {
Map<FilterChain, AtomicReference<ServerRoutingConfig>> filterChainRouting = new HashMap<>();
// This is regenerated in generateRoutingConfig() calls below.
savedRdsRoutingConfigRef.clear();

// Prepare server routing config map.
ImmutableMap.Builder<FilterChain, AtomicReference<ServerRoutingConfig>> routingConfigs =
ImmutableMap.builder();
for (FilterChain filterChain: filterChains) {
filterChainRouting.put(filterChain, generateRoutingConfig(filterChain));
routingConfigs.put(filterChain, generateRoutingConfig(filterChain));
}
FilterChainSelector selector = new FilterChainSelector(
Collections.unmodifiableMap(filterChainRouting),
defaultFilterChain == null ? null : defaultFilterChain.sslContextProviderSupplier(),
defaultFilterChain == null ? new AtomicReference<ServerRoutingConfig>() :
generateRoutingConfig(defaultFilterChain));
List<SslContextProviderSupplier> toRelease = getSuppliersInUse();

// Prepare the new selector.
FilterChainSelector selector;
if (defaultFilterChain != null) {
selector = new FilterChainSelector(
routingConfigs.build(),
defaultFilterChain.sslContextProviderSupplier(),
generateRoutingConfig(defaultFilterChain));
} else {
selector = new FilterChainSelector(routingConfigs.build(), null, new AtomicReference<>());
}

// Prepare the list of current selector's resources to close later.
List<SslContextProviderSupplier> oldSslSuppliers = getSuppliersInUse();

// Swap the selectors, initiate a graceful shutdown of the old one.
logger.log(Level.FINEST, "Updating selector {0}", selector);
filterChainSelectorManager.updateSelector(selector);
for (SslContextProviderSupplier e: toRelease) {
e.close();

// Release old resources.
for (SslContextProviderSupplier supplier: oldSslSuppliers) {
supplier.close();
}

// Now that we have valid Transport Socket config, we can start/restart listening on a port.
startDelegateServer();
}

private AtomicReference<ServerRoutingConfig> generateRoutingConfig(FilterChain filterChain) {
HttpConnectionManager hcm = filterChain.httpConnectionManager();
ImmutableMap<Route, ServerInterceptor> interceptors;

// Inlined routes.
if (hcm.virtualHosts() != null) {
ImmutableMap<Route, ServerInterceptor> interceptors = generatePerRouteInterceptors(
hcm.httpFilterConfigs(), hcm.virtualHosts());
return new AtomicReference<>(ServerRoutingConfig.create(hcm.virtualHosts(),interceptors));
interceptors = generatePerRouteInterceptors(hcm.httpFilterConfigs(), hcm.virtualHosts());
return new AtomicReference<>(ServerRoutingConfig.create(hcm.virtualHosts(), interceptors));
}

// Routes from RDS.
RouteDiscoveryState rds = routeDiscoveryStates.get(hcm.rdsName());
checkNotNull(rds, "rds");

ServerRoutingConfig routingConfig;
ImmutableList<VirtualHost> savedVhosts = rds.savedVirtualHosts;
if (savedVhosts != null) {
interceptors = generatePerRouteInterceptors(hcm.httpFilterConfigs(), savedVhosts);
routingConfig = ServerRoutingConfig.create(savedVhosts, interceptors);
} else {
RouteDiscoveryState rds = routeDiscoveryStates.get(hcm.rdsName());
checkNotNull(rds, "rds");
AtomicReference<ServerRoutingConfig> serverRoutingConfigRef = new AtomicReference<>();
if (rds.savedVirtualHosts != null) {
ImmutableMap<Route, ServerInterceptor> 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;
routingConfig = ServerRoutingConfig.FAILING_ROUTING_CONFIG;
}

AtomicReference<ServerRoutingConfig> routingConfigRef = new AtomicReference<>(routingConfig);
savedRdsRoutingConfigRef.put(filterChain, routingConfigRef);
return routingConfigRef;
}

private ImmutableMap<Route, ServerInterceptor> generatePerRouteInterceptors(
Expand Down
36 changes: 22 additions & 14 deletions xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2719,7 +2719,7 @@ public void parseFilterChain_noHcm() throws ResourceInvalidException {
thrown.expectMessage(
"FilterChain filter-chain-foo should contain exact one HttpConnectionManager filter");
XdsListenerResource.parseFilterChain(
filterChain, null, filterRegistry, null, null,
filterChain, "filter-chain-foo", null, filterRegistry, null, null,
getXdsResourceTypeArgs(true));
}

Expand All @@ -2738,7 +2738,7 @@ public void parseFilterChain_duplicateFilter() throws ResourceInvalidException {
thrown.expectMessage(
"FilterChain filter-chain-foo should contain exact one HttpConnectionManager filter");
XdsListenerResource.parseFilterChain(
filterChain, null, filterRegistry, null, null,
filterChain, "filter-chain-foo", null, filterRegistry, null, null,
getXdsResourceTypeArgs(true));
}

Expand All @@ -2757,7 +2757,7 @@ public void parseFilterChain_filterMissingTypedConfig() throws ResourceInvalidEx
"FilterChain filter-chain-foo contains filter envoy.http_connection_manager "
+ "without typed_config");
XdsListenerResource.parseFilterChain(
filterChain, null, filterRegistry, null, null,
filterChain, "filter-chain-foo", null, filterRegistry, null, null,
getXdsResourceTypeArgs(true));
}

Expand All @@ -2780,13 +2780,13 @@ public void parseFilterChain_unsupportedFilter() throws ResourceInvalidException
"FilterChain filter-chain-foo contains filter unsupported with unsupported "
+ "typed_config type unsupported-type-url");
XdsListenerResource.parseFilterChain(
filterChain, null, filterRegistry, null, null,
filterChain, "filter-chain-foo", null, filterRegistry, null, null,
getXdsResourceTypeArgs(true));
}

@Test
public void parseFilterChain_noName() throws ResourceInvalidException {
FilterChain filterChain1 =
FilterChain filterChain0 =
FilterChain.newBuilder()
.setFilterChainMatch(FilterChainMatch.getDefaultInstance())
.addFilters(buildHttpConnectionManagerFilter(
Expand All @@ -2796,9 +2796,11 @@ public void parseFilterChain_noName() throws ResourceInvalidException {
.setTypedConfig(Any.pack(Router.newBuilder().build()))
.build()))
.build();
FilterChain filterChain2 =

FilterChain filterChain1 =
FilterChain.newBuilder()
.setFilterChainMatch(FilterChainMatch.getDefaultInstance())
.setFilterChainMatch(
FilterChainMatch.newBuilder().addAllSourcePorts(Arrays.asList(443, 8080)))
.addFilters(buildHttpConnectionManagerFilter(
HttpFilter.newBuilder()
.setName("http-filter-bar")
Expand All @@ -2807,13 +2809,19 @@ public void parseFilterChain_noName() throws ResourceInvalidException {
.build()))
.build();

EnvoyServerProtoData.FilterChain parsedFilterChain1 = XdsListenerResource.parseFilterChain(
filterChain1, null, filterRegistry, null,
null, getXdsResourceTypeArgs(true));
EnvoyServerProtoData.FilterChain parsedFilterChain2 = XdsListenerResource.parseFilterChain(
filterChain2, null, filterRegistry, null,
null, getXdsResourceTypeArgs(true));
assertThat(parsedFilterChain1.name()).isEqualTo(parsedFilterChain2.name());
Listener listenerProto =
Listener.newBuilder()
.setName("listener1")
.setTrafficDirection(TrafficDirection.INBOUND)
.addAllFilterChains(Arrays.asList(filterChain0, filterChain1))
.setDefaultFilterChain(filterChain0)
.build();
EnvoyServerProtoData.Listener listener = XdsListenerResource.parseServerSideListener(
listenerProto, null, filterRegistry, null, getXdsResourceTypeArgs(true));

assertThat(listener.filterChains().get(0).name()).isEqualTo("chain_0");
assertThat(listener.filterChains().get(1).name()).isEqualTo("chain_1");
assertThat(listener.defaultFilterChain().name()).isEqualTo("chain_default");
}

@Test
Expand Down

0 comments on commit bd6af59

Please sign in to comment.