Skip to content

Commit d2d72cd

Browse files
authored
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.
1 parent bb120a8 commit d2d72cd

13 files changed

+21
-17
lines changed

xds/src/main/java/io/grpc/xds/FaultFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public boolean isClientFilter() {
9999
}
100100

101101
@Override
102-
public FaultFilter newInstance() {
102+
public FaultFilter newInstance(String name) {
103103
return INSTANCE;
104104
}
105105

xds/src/main/java/io/grpc/xds/Filter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ default boolean isServerFilter() {
8787
* <li>Filter name+typeUrl in FilterChain's HCM.http_filters.</li>
8888
* </ol>
8989
*/
90-
Filter newInstance();
90+
Filter newInstance(String name);
9191

9292
/**
9393
* Parses the top-level filter config from raw proto message. The message may be either a {@link

xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public boolean isClientFilter() {
6464
}
6565

6666
@Override
67-
public GcpAuthenticationFilter newInstance() {
67+
public GcpAuthenticationFilter newInstance(String name) {
6868
return new GcpAuthenticationFilter();
6969
}
7070

xds/src/main/java/io/grpc/xds/InternalRbacFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public static ServerInterceptor createInterceptor(RBAC rbac) {
3333
throw new IllegalArgumentException(
3434
String.format("Failed to parse Rbac policy: %s", filterConfig.errorDetail));
3535
}
36-
return new RbacFilter.Provider().newInstance()
36+
return new RbacFilter.Provider().newInstance("internalRbacFilter")
3737
.buildServerInterceptor(filterConfig.config, null);
3838
}
3939
}

xds/src/main/java/io/grpc/xds/RbacFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public boolean isServerFilter() {
8989
}
9090

9191
@Override
92-
public RbacFilter newInstance() {
92+
public RbacFilter newInstance(String name) {
9393
return INSTANCE;
9494
}
9595

xds/src/main/java/io/grpc/xds/RouterFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public boolean isServerFilter() {
5656
}
5757

5858
@Override
59-
public RouterFilter newInstance() {
59+
public RouterFilter newInstance(String name) {
6060
return INSTANCE;
6161
}
6262

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,8 @@ private void updateActiveFilters(@Nullable List<NamedFilterConfig> filterConfigs
704704

705705
Filter.Provider provider = filterRegistry.get(typeUrl);
706706
checkNotNull(provider, "provider %s", typeUrl);
707-
Filter filter = activeFilters.computeIfAbsent(filterKey, k -> provider.newInstance());
707+
Filter filter = activeFilters.computeIfAbsent(
708+
filterKey, k -> provider.newInstance(namedFilter.name));
708709
checkNotNull(filter, "filter %s", filterKey);
709710
filtersToShutdown.remove(filterKey);
710711
}

xds/src/main/java/io/grpc/xds/XdsServerWrapper.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,8 @@ private void updateActiveFiltersForChain(
560560

561561
Filter.Provider provider = filterRegistry.get(typeUrl);
562562
checkNotNull(provider, "provider %s", typeUrl);
563-
Filter filter = chainFilters.computeIfAbsent(filterKey, k -> provider.newInstance());
563+
Filter filter = chainFilters.computeIfAbsent(
564+
filterKey, k -> provider.newInstance(namedFilter.name));
564565
checkNotNull(filter, "filter %s", filterKey);
565566
filtersToShutdown.remove(filterKey);
566567
}

xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1267,7 +1267,7 @@ public boolean isClientFilter() {
12671267
}
12681268

12691269
@Override
1270-
public TestFilter newInstance() {
1270+
public TestFilter newInstance(String name) {
12711271
return new TestFilter();
12721272
}
12731273

xds/src/test/java/io/grpc/xds/RbacFilterTest.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ public class RbacFilterTest {
8080
StringMatcher.newBuilder().setExact("/" + PATH).setIgnoreCase(true).build();
8181
private static final RbacFilter.Provider FILTER_PROVIDER = new RbacFilter.Provider();
8282

83+
private final String name = "theFilterName";
84+
8385
@Test
8486
public void filterType_serverOnly() {
8587
assertThat(FILTER_PROVIDER.isClientFilter()).isFalse();
@@ -259,7 +261,7 @@ public void testAuthorizationInterceptor() {
259261
OrMatcher.create(AlwaysTrueMatcher.INSTANCE));
260262
AuthConfig authconfig = AuthConfig.create(Collections.singletonList(policyMatcher),
261263
GrpcAuthorizationEngine.Action.ALLOW);
262-
FILTER_PROVIDER.newInstance().buildServerInterceptor(RbacConfig.create(authconfig), null)
264+
FILTER_PROVIDER.newInstance(name).buildServerInterceptor(RbacConfig.create(authconfig), null)
263265
.interceptCall(mockServerCall, new Metadata(), mockHandler);
264266
verify(mockHandler, never()).startCall(eq(mockServerCall), any(Metadata.class));
265267
ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
@@ -271,7 +273,7 @@ public void testAuthorizationInterceptor() {
271273

272274
authconfig = AuthConfig.create(Collections.singletonList(policyMatcher),
273275
GrpcAuthorizationEngine.Action.DENY);
274-
FILTER_PROVIDER.newInstance().buildServerInterceptor(RbacConfig.create(authconfig), null)
276+
FILTER_PROVIDER.newInstance(name).buildServerInterceptor(RbacConfig.create(authconfig), null)
275277
.interceptCall(mockServerCall, new Metadata(), mockHandler);
276278
verify(mockHandler).startCall(eq(mockServerCall), any(Metadata.class));
277279
}
@@ -322,7 +324,7 @@ public void overrideConfig() {
322324
RbacConfig override = FILTER_PROVIDER.parseFilterConfigOverride(Any.pack(rbacPerRoute)).config;
323325
assertThat(override).isEqualTo(RbacConfig.create(null));
324326
ServerInterceptor interceptor =
325-
FILTER_PROVIDER.newInstance().buildServerInterceptor(original, override);
327+
FILTER_PROVIDER.newInstance(name).buildServerInterceptor(original, override);
326328
assertThat(interceptor).isNull();
327329

328330
policyMatcher = PolicyMatcher.create("policy-matcher-override",
@@ -332,7 +334,7 @@ public void overrideConfig() {
332334
GrpcAuthorizationEngine.Action.ALLOW);
333335
override = RbacConfig.create(authconfig);
334336

335-
FILTER_PROVIDER.newInstance().buildServerInterceptor(original, override)
337+
FILTER_PROVIDER.newInstance(name).buildServerInterceptor(original, override)
336338
.interceptCall(mockServerCall, new Metadata(), mockHandler);
337339
verify(mockHandler).startCall(eq(mockServerCall), any(Metadata.class));
338340
verify(mockServerCall).getAttributes();

xds/src/test/java/io/grpc/xds/StatefulFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public boolean isServerFilter() {
108108
}
109109

110110
@Override
111-
public synchronized StatefulFilter newInstance() {
111+
public synchronized StatefulFilter newInstance(String name) {
112112
StatefulFilter filter = new StatefulFilter(counter++);
113113
instances.put(filter.idx, filter);
114114
return filter;

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ public void setUp() {
219219
// Lenient: suppress [MockitoHint] Unused warning, only used in resolved_fault* tests.
220220
lenient()
221221
.doReturn(new FaultFilter(mockRandom, new AtomicLong()))
222-
.when(faultFilterProvider).newInstance();
222+
.when(faultFilterProvider).newInstance(any(String.class));
223223

224224
FilterRegistry filterRegistry = FilterRegistry.newRegistry().register(
225225
ROUTER_FILTER_PROVIDER,

xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1135,7 +1135,7 @@ public void run() {
11351135
Filter.Provider filterProvider = mock(Filter.Provider.class);
11361136
when(filterProvider.typeUrls()).thenReturn(new String[]{"filter-type-url"});
11371137
when(filterProvider.isServerFilter()).thenReturn(true);
1138-
when(filterProvider.newInstance()).thenReturn(filter);
1138+
when(filterProvider.newInstance(any(String.class))).thenReturn(filter);
11391139
filterRegistry.register(filterProvider);
11401140

11411141
FilterConfig f0 = mock(FilterConfig.class);
@@ -1208,7 +1208,7 @@ public void run() {
12081208
Filter.Provider filterProvider = mock(Filter.Provider.class);
12091209
when(filterProvider.typeUrls()).thenReturn(new String[]{"filter-type-url"});
12101210
when(filterProvider.isServerFilter()).thenReturn(true);
1211-
when(filterProvider.newInstance()).thenReturn(filter);
1211+
when(filterProvider.newInstance(any(String.class))).thenReturn(filter);
12121212
filterRegistry.register(filterProvider);
12131213

12141214
FilterConfig f0 = mock(FilterConfig.class);

0 commit comments

Comments
 (0)