Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

xds: ignore routes with unsupported optional cluster specifiers #9168

Merged
merged 1 commit into from May 13, 2022
Merged

xds: ignore routes with unsupported optional cluster specifiers #9168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 33 additions & 13 deletions xds/src/main/java/io/grpc/xds/ClientXdsClient.java
View file
Open in desktop
Expand Up @@ -981,12 +981,13 @@ static StructOrError<FilterConfig> parseHttpFilter(

private static StructOrError<VirtualHost> parseVirtualHost(
io.envoyproxy.envoy.config.route.v3.VirtualHost proto, FilterRegistry filterRegistry,
boolean parseHttpFilter, Map<String, PluginConfig> pluginConfigMap) {
boolean parseHttpFilter, Map<String, PluginConfig> pluginConfigMap,
Set<String> optionalPlugins) {
String name = proto.getName();
List<Route> routes = new ArrayList<>(proto.getRoutesCount());
for (io.envoyproxy.envoy.config.route.v3.Route routeProto : proto.getRoutesList()) {
StructOrError<Route> route = parseRoute(
routeProto, filterRegistry, parseHttpFilter, pluginConfigMap);
routeProto, filterRegistry, parseHttpFilter, pluginConfigMap, optionalPlugins);
if (route == null) {
continue;
}
Expand Down Expand Up @@ -1071,7 +1072,8 @@ static StructOrError<Map<String, FilterConfig>> parseOverrideFilterConfigs(
@Nullable
static StructOrError<Route> parseRoute(
io.envoyproxy.envoy.config.route.v3.Route proto, FilterRegistry filterRegistry,
boolean parseHttpFilter, Map<String, PluginConfig> pluginConfigMap) {
boolean parseHttpFilter, Map<String, PluginConfig> pluginConfigMap,
Set<String> optionalPlugins) {
StructOrError<RouteMatch> routeMatch = parseRouteMatch(proto.getMatch());
if (routeMatch == null) {
return null;
Expand All @@ -1097,7 +1099,8 @@ static StructOrError<Route> parseRoute(
switch (proto.getActionCase()) {
case ROUTE:
StructOrError<RouteAction> routeAction =
parseRouteAction(proto.getRoute(), filterRegistry, parseHttpFilter, pluginConfigMap);
parseRouteAction(proto.getRoute(), filterRegistry, parseHttpFilter, pluginConfigMap,
optionalPlugins);
if (routeAction == null) {
return null;
}
Expand Down Expand Up @@ -1250,7 +1253,8 @@ static StructOrError<HeaderMatcher> parseHeaderMatcher(
@Nullable
static StructOrError<RouteAction> parseRouteAction(
io.envoyproxy.envoy.config.route.v3.RouteAction proto, FilterRegistry filterRegistry,
boolean parseHttpFilter, Map<String, PluginConfig> pluginConfigMap) {
boolean parseHttpFilter, Map<String, PluginConfig> pluginConfigMap,
Set<String> optionalPlugins) {
Long timeoutNano = null;
if (proto.hasMaxStreamDuration()) {
io.envoyproxy.envoy.config.route.v3.RouteAction.MaxStreamDuration maxStreamDuration
Expand Down Expand Up @@ -1334,6 +1338,10 @@ static StructOrError<RouteAction> parseRouteAction(
String pluginName = proto.getClusterSpecifierPlugin();
PluginConfig pluginConfig = pluginConfigMap.get(pluginName);
if (pluginConfig == null) {
// Skip route if the plugin is not registered, but it's optional.
if (optionalPlugins.contains(pluginName)) {
return null;
}
return StructOrError.fromError(
"ClusterSpecifierPlugin for [" + pluginName + "] not found");
}
Expand Down Expand Up @@ -1491,23 +1499,30 @@ private static List<VirtualHost> extractVirtualHosts(
RouteConfiguration routeConfig, FilterRegistry filterRegistry, boolean parseHttpFilter)
throws ResourceInvalidException {
Map<String, PluginConfig> pluginConfigMap = new HashMap<>();
ImmutableSet.Builder<String> optionalPlugins = ImmutableSet.builder();

if (enableRouteLookup) {
List<ClusterSpecifierPlugin> plugins = routeConfig.getClusterSpecifierPluginsList();
for (ClusterSpecifierPlugin plugin : plugins) {
PluginConfig existing = pluginConfigMap.put(
plugin.getExtension().getName(), parseClusterSpecifierPlugin(plugin));
if (existing != null) {
throw new ResourceInvalidException(
"Multiple ClusterSpecifierPlugins with the same name: "
+ plugin.getExtension().getName());
String pluginName = plugin.getExtension().getName();
PluginConfig pluginConfig = parseClusterSpecifierPlugin(plugin);
if (pluginConfig != null) {
if (pluginConfigMap.put(pluginName, pluginConfig) != null) {
throw new ResourceInvalidException(
"Multiple ClusterSpecifierPlugins with the same name: " + pluginName);
}
} else {
// The plugin parsed successfully, and it's not supported, but it's marked as optional.
optionalPlugins.add(pluginName);
}
}
}
List<VirtualHost> virtualHosts = new ArrayList<>(routeConfig.getVirtualHostsCount());
for (io.envoyproxy.envoy.config.route.v3.VirtualHost virtualHostProto
: routeConfig.getVirtualHostsList()) {
StructOrError<VirtualHost> virtualHost =
parseVirtualHost(virtualHostProto, filterRegistry, parseHttpFilter, pluginConfigMap);
parseVirtualHost(virtualHostProto, filterRegistry, parseHttpFilter, pluginConfigMap,
optionalPlugins.build());
if (virtualHost.getErrorDetail() != null) {
throw new ResourceInvalidException(
"RouteConfiguration contains invalid virtual host: " + virtualHost.getErrorDetail());
Expand All @@ -1517,12 +1532,14 @@ private static List<VirtualHost> extractVirtualHosts(
return virtualHosts;
}

@Nullable // null if the plugin is not supported, but it's marked as optional.
private static PluginConfig parseClusterSpecifierPlugin(ClusterSpecifierPlugin pluginProto)
throws ResourceInvalidException {
return parseClusterSpecifierPlugin(
pluginProto, ClusterSpecifierPluginRegistry.getDefaultRegistry());
}

@Nullable // null if the plugin is not supported, but it's marked as optional.
@VisibleForTesting
static PluginConfig parseClusterSpecifierPlugin(
ClusterSpecifierPlugin pluginProto, ClusterSpecifierPluginRegistry registry)
Expand All @@ -1545,7 +1562,10 @@ static PluginConfig parseClusterSpecifierPlugin(
}
io.grpc.xds.ClusterSpecifierPlugin plugin = registry.get(typeUrl);
if (plugin == null) {
throw new ResourceInvalidException("Unsupported ClusterSpecifierPlugin type: " + typeUrl);
if (!pluginProto.getIsOptional()) {
throw new ResourceInvalidException("Unsupported ClusterSpecifierPlugin type: " + typeUrl);
}
return null;
}
ConfigOrError<? extends PluginConfig> pluginConfigOrError = plugin.parsePlugin(rawConfig);
if (pluginConfigOrError.errorDetail != null) {
Expand Down