xds: Expose filter names to filter instances (#11971)

This is to support gRFC A83 xDS GCP Authentication Filter:
> Otherwise, the filter will look in the CDS resource's metadata for a
> key corresponding to the filter's instance name.
This commit is contained in:
Eric Anderson 2025-03-20 22:31:16 -07:00 committed by GitHub
parent bb120a8cbb
commit d2d72cda83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 21 additions and 17 deletions

View File

@ -99,7 +99,7 @@ final class FaultFilter implements Filter {
} }
@Override @Override
public FaultFilter newInstance() { public FaultFilter newInstance(String name) {
return INSTANCE; return INSTANCE;
} }

View File

@ -87,7 +87,7 @@ interface Filter extends Closeable {
* <li>Filter name+typeUrl in FilterChain's HCM.http_filters.</li> * <li>Filter name+typeUrl in FilterChain's HCM.http_filters.</li>
* </ol> * </ol>
*/ */
Filter newInstance(); Filter newInstance(String name);
/** /**
* Parses the top-level filter config from raw proto message. The message may be either a {@link * Parses the top-level filter config from raw proto message. The message may be either a {@link

View File

@ -64,7 +64,7 @@ final class GcpAuthenticationFilter implements Filter {
} }
@Override @Override
public GcpAuthenticationFilter newInstance() { public GcpAuthenticationFilter newInstance(String name) {
return new GcpAuthenticationFilter(); return new GcpAuthenticationFilter();
} }

View File

@ -33,7 +33,7 @@ public final class InternalRbacFilter {
throw new IllegalArgumentException( throw new IllegalArgumentException(
String.format("Failed to parse Rbac policy: %s", filterConfig.errorDetail)); String.format("Failed to parse Rbac policy: %s", filterConfig.errorDetail));
} }
return new RbacFilter.Provider().newInstance() return new RbacFilter.Provider().newInstance("internalRbacFilter")
.buildServerInterceptor(filterConfig.config, null); .buildServerInterceptor(filterConfig.config, null);
} }
} }

View File

@ -89,7 +89,7 @@ final class RbacFilter implements Filter {
} }
@Override @Override
public RbacFilter newInstance() { public RbacFilter newInstance(String name) {
return INSTANCE; return INSTANCE;
} }

View File

@ -56,7 +56,7 @@ final class RouterFilter implements Filter {
} }
@Override @Override
public RouterFilter newInstance() { public RouterFilter newInstance(String name) {
return INSTANCE; return INSTANCE;
} }

View File

@ -704,7 +704,8 @@ final class XdsNameResolver extends NameResolver {
Filter.Provider provider = filterRegistry.get(typeUrl); Filter.Provider provider = filterRegistry.get(typeUrl);
checkNotNull(provider, "provider %s", typeUrl); checkNotNull(provider, "provider %s", typeUrl);
Filter filter = activeFilters.computeIfAbsent(filterKey, k -> provider.newInstance()); Filter filter = activeFilters.computeIfAbsent(
filterKey, k -> provider.newInstance(namedFilter.name));
checkNotNull(filter, "filter %s", filterKey); checkNotNull(filter, "filter %s", filterKey);
filtersToShutdown.remove(filterKey); filtersToShutdown.remove(filterKey);
} }

View File

@ -560,7 +560,8 @@ final class XdsServerWrapper extends Server {
Filter.Provider provider = filterRegistry.get(typeUrl); Filter.Provider provider = filterRegistry.get(typeUrl);
checkNotNull(provider, "provider %s", typeUrl); checkNotNull(provider, "provider %s", typeUrl);
Filter filter = chainFilters.computeIfAbsent(filterKey, k -> provider.newInstance()); Filter filter = chainFilters.computeIfAbsent(
filterKey, k -> provider.newInstance(namedFilter.name));
checkNotNull(filter, "filter %s", filterKey); checkNotNull(filter, "filter %s", filterKey);
filtersToShutdown.remove(filterKey); filtersToShutdown.remove(filterKey);
} }

View File

@ -1267,7 +1267,7 @@ public class GrpcXdsClientImplDataTest {
} }
@Override @Override
public TestFilter newInstance() { public TestFilter newInstance(String name) {
return new TestFilter(); return new TestFilter();
} }

View File

@ -80,6 +80,8 @@ public class RbacFilterTest {
StringMatcher.newBuilder().setExact("/" + PATH).setIgnoreCase(true).build(); StringMatcher.newBuilder().setExact("/" + PATH).setIgnoreCase(true).build();
private static final RbacFilter.Provider FILTER_PROVIDER = new RbacFilter.Provider(); private static final RbacFilter.Provider FILTER_PROVIDER = new RbacFilter.Provider();
private final String name = "theFilterName";
@Test @Test
public void filterType_serverOnly() { public void filterType_serverOnly() {
assertThat(FILTER_PROVIDER.isClientFilter()).isFalse(); assertThat(FILTER_PROVIDER.isClientFilter()).isFalse();
@ -259,7 +261,7 @@ public class RbacFilterTest {
OrMatcher.create(AlwaysTrueMatcher.INSTANCE)); OrMatcher.create(AlwaysTrueMatcher.INSTANCE));
AuthConfig authconfig = AuthConfig.create(Collections.singletonList(policyMatcher), AuthConfig authconfig = AuthConfig.create(Collections.singletonList(policyMatcher),
GrpcAuthorizationEngine.Action.ALLOW); GrpcAuthorizationEngine.Action.ALLOW);
FILTER_PROVIDER.newInstance().buildServerInterceptor(RbacConfig.create(authconfig), null) FILTER_PROVIDER.newInstance(name).buildServerInterceptor(RbacConfig.create(authconfig), null)
.interceptCall(mockServerCall, new Metadata(), mockHandler); .interceptCall(mockServerCall, new Metadata(), mockHandler);
verify(mockHandler, never()).startCall(eq(mockServerCall), any(Metadata.class)); verify(mockHandler, never()).startCall(eq(mockServerCall), any(Metadata.class));
ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class); ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
@ -271,7 +273,7 @@ public class RbacFilterTest {
authconfig = AuthConfig.create(Collections.singletonList(policyMatcher), authconfig = AuthConfig.create(Collections.singletonList(policyMatcher),
GrpcAuthorizationEngine.Action.DENY); GrpcAuthorizationEngine.Action.DENY);
FILTER_PROVIDER.newInstance().buildServerInterceptor(RbacConfig.create(authconfig), null) FILTER_PROVIDER.newInstance(name).buildServerInterceptor(RbacConfig.create(authconfig), null)
.interceptCall(mockServerCall, new Metadata(), mockHandler); .interceptCall(mockServerCall, new Metadata(), mockHandler);
verify(mockHandler).startCall(eq(mockServerCall), any(Metadata.class)); verify(mockHandler).startCall(eq(mockServerCall), any(Metadata.class));
} }
@ -322,7 +324,7 @@ public class RbacFilterTest {
RbacConfig override = FILTER_PROVIDER.parseFilterConfigOverride(Any.pack(rbacPerRoute)).config; RbacConfig override = FILTER_PROVIDER.parseFilterConfigOverride(Any.pack(rbacPerRoute)).config;
assertThat(override).isEqualTo(RbacConfig.create(null)); assertThat(override).isEqualTo(RbacConfig.create(null));
ServerInterceptor interceptor = ServerInterceptor interceptor =
FILTER_PROVIDER.newInstance().buildServerInterceptor(original, override); FILTER_PROVIDER.newInstance(name).buildServerInterceptor(original, override);
assertThat(interceptor).isNull(); assertThat(interceptor).isNull();
policyMatcher = PolicyMatcher.create("policy-matcher-override", policyMatcher = PolicyMatcher.create("policy-matcher-override",
@ -332,7 +334,7 @@ public class RbacFilterTest {
GrpcAuthorizationEngine.Action.ALLOW); GrpcAuthorizationEngine.Action.ALLOW);
override = RbacConfig.create(authconfig); override = RbacConfig.create(authconfig);
FILTER_PROVIDER.newInstance().buildServerInterceptor(original, override) FILTER_PROVIDER.newInstance(name).buildServerInterceptor(original, override)
.interceptCall(mockServerCall, new Metadata(), mockHandler); .interceptCall(mockServerCall, new Metadata(), mockHandler);
verify(mockHandler).startCall(eq(mockServerCall), any(Metadata.class)); verify(mockHandler).startCall(eq(mockServerCall), any(Metadata.class));
verify(mockServerCall).getAttributes(); verify(mockServerCall).getAttributes();

View File

@ -108,7 +108,7 @@ class StatefulFilter implements Filter {
} }
@Override @Override
public synchronized StatefulFilter newInstance() { public synchronized StatefulFilter newInstance(String name) {
StatefulFilter filter = new StatefulFilter(counter++); StatefulFilter filter = new StatefulFilter(counter++);
instances.put(filter.idx, filter); instances.put(filter.idx, filter);
return filter; return filter;

View File

@ -219,7 +219,7 @@ public class XdsNameResolverTest {
// Lenient: suppress [MockitoHint] Unused warning, only used in resolved_fault* tests. // Lenient: suppress [MockitoHint] Unused warning, only used in resolved_fault* tests.
lenient() lenient()
.doReturn(new FaultFilter(mockRandom, new AtomicLong())) .doReturn(new FaultFilter(mockRandom, new AtomicLong()))
.when(faultFilterProvider).newInstance(); .when(faultFilterProvider).newInstance(any(String.class));
FilterRegistry filterRegistry = FilterRegistry.newRegistry().register( FilterRegistry filterRegistry = FilterRegistry.newRegistry().register(
ROUTER_FILTER_PROVIDER, ROUTER_FILTER_PROVIDER,

View File

@ -1135,7 +1135,7 @@ public class XdsServerWrapperTest {
Filter.Provider filterProvider = mock(Filter.Provider.class); Filter.Provider filterProvider = mock(Filter.Provider.class);
when(filterProvider.typeUrls()).thenReturn(new String[]{"filter-type-url"}); when(filterProvider.typeUrls()).thenReturn(new String[]{"filter-type-url"});
when(filterProvider.isServerFilter()).thenReturn(true); when(filterProvider.isServerFilter()).thenReturn(true);
when(filterProvider.newInstance()).thenReturn(filter); when(filterProvider.newInstance(any(String.class))).thenReturn(filter);
filterRegistry.register(filterProvider); filterRegistry.register(filterProvider);
FilterConfig f0 = mock(FilterConfig.class); FilterConfig f0 = mock(FilterConfig.class);
@ -1208,7 +1208,7 @@ public class XdsServerWrapperTest {
Filter.Provider filterProvider = mock(Filter.Provider.class); Filter.Provider filterProvider = mock(Filter.Provider.class);
when(filterProvider.typeUrls()).thenReturn(new String[]{"filter-type-url"}); when(filterProvider.typeUrls()).thenReturn(new String[]{"filter-type-url"});
when(filterProvider.isServerFilter()).thenReturn(true); when(filterProvider.isServerFilter()).thenReturn(true);
when(filterProvider.newInstance()).thenReturn(filter); when(filterProvider.newInstance(any(String.class))).thenReturn(filter);
filterRegistry.register(filterProvider); filterRegistry.register(filterProvider);
FilterConfig f0 = mock(FilterConfig.class); FilterConfig f0 = mock(FilterConfig.class);