Skip to content

Commit bcec5d0

Browse files
shivaspeaksshivaspeaks
committed
address comments and add test for failure cases
1 parent e6be504 commit bcec5d0

3 files changed

Lines changed: 142 additions & 71 deletions

File tree

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import com.google.auth.oauth2.ComputeEngineCredentials;
2424
import com.google.auth.oauth2.IdTokenCredentials;
25+
import com.google.common.annotations.VisibleForTesting;
2526
import com.google.common.primitives.UnsignedLongs;
2627
import com.google.protobuf.Any;
2728
import com.google.protobuf.InvalidProtocolBufferException;
@@ -164,11 +165,7 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
164165
}
165166

166167
if (!xdsCluster.hasValue()) {
167-
return new FailingClientCall<>(
168-
xdsCluster.getStatus().withDescription(
169-
String.format(
170-
"GCP Authn for %s with %s - xds cluster does not contain cluster resource",
171-
filterInstanceName, clusterName)));
168+
return new FailingClientCall<>(xdsCluster.getStatus());
172169
}
173170

174171
Object audienceObj =
@@ -241,9 +238,11 @@ public String typeUrl() {
241238
}
242239

243240
/** An implementation of {@link ClientCall} that fails when started. */
244-
private static final class FailingClientCall<ReqT, RespT> extends ClientCall<ReqT, RespT> {
241+
@VisibleForTesting
242+
static final class FailingClientCall<ReqT, RespT> extends ClientCall<ReqT, RespT> {
245243

246-
private final Status error;
244+
@VisibleForTesting
245+
final Status error;
247246

248247
public FailingClientCall(Status error) {
249248
this.error = error;

‎xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java‎

Lines changed: 135 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static io.grpc.xds.XdsNameResolver.CLUSTER_SELECTION_KEY;
2121
import static io.grpc.xds.XdsNameResolver.XDS_CONFIG_CALL_OPTION_KEY;
22+
import static io.grpc.xds.XdsTestUtils.CLUSTER_NAME;
23+
import static io.grpc.xds.XdsTestUtils.EDS_NAME;
24+
import static io.grpc.xds.XdsTestUtils.ENDPOINT_HOSTNAME;
25+
import static io.grpc.xds.XdsTestUtils.ENDPOINT_PORT;
26+
import static io.grpc.xds.XdsTestUtils.RDS_NAME;
27+
import static io.grpc.xds.XdsTestUtils.buildRouteConfiguration;
28+
import static io.grpc.xds.XdsTestUtils.getWrrLbConfigAsMap;
2229
import static org.junit.Assert.assertEquals;
2330
import static org.junit.Assert.assertNotNull;
2431
import static org.junit.Assert.assertNull;
@@ -27,19 +34,37 @@
2734
import static org.mockito.ArgumentMatchers.eq;
2835
import static org.mockito.Mockito.mock;
2936

37+
import com.google.common.collect.ImmutableList;
38+
import com.google.common.collect.ImmutableMap;
3039
import com.google.protobuf.Any;
3140
import com.google.protobuf.Empty;
3241
import com.google.protobuf.Message;
3342
import com.google.protobuf.UInt64Value;
43+
import io.envoyproxy.envoy.config.route.v3.RouteConfiguration;
3444
import io.envoyproxy.envoy.extensions.filters.http.gcp_authn.v3.GcpAuthnFilterConfig;
3545
import io.envoyproxy.envoy.extensions.filters.http.gcp_authn.v3.TokenCacheConfig;
3646
import io.grpc.CallOptions;
3747
import io.grpc.Channel;
48+
import io.grpc.ClientCall;
3849
import io.grpc.ClientInterceptor;
3950
import io.grpc.MethodDescriptor;
51+
import io.grpc.Status;
52+
import io.grpc.StatusOr;
4053
import io.grpc.inprocess.InProcessServerBuilder;
4154
import io.grpc.testing.TestMethodDescriptors;
55+
import io.grpc.xds.Endpoints.LbEndpoint;
56+
import io.grpc.xds.Endpoints.LocalityLbEndpoints;
57+
import io.grpc.xds.GcpAuthenticationFilter.AudienceMetadataParser.AudienceWrapper;
58+
import io.grpc.xds.GcpAuthenticationFilter.FailingClientCall;
4259
import io.grpc.xds.GcpAuthenticationFilter.GcpAuthenticationConfig;
60+
import io.grpc.xds.XdsClusterResource.CdsUpdate;
61+
import io.grpc.xds.XdsConfig.XdsClusterConfig;
62+
import io.grpc.xds.XdsConfig.XdsClusterConfig.EndpointConfig;
63+
import io.grpc.xds.client.Locality;
64+
import io.grpc.xds.client.XdsResourceType;
65+
import java.util.Collections;
66+
import java.util.HashMap;
67+
import java.util.Map;
4368
import org.junit.Test;
4469
import org.junit.runner.RunWith;
4570
import org.junit.runners.JUnit4;
@@ -96,9 +121,52 @@ public void testParseFilterConfig_withInvalidMessageType() {
96121
}
97122

98123
@Test
99-
public void testClientInterceptor_createsAndReusesCachedCredentials() throws Exception {
124+
public void testClientInterceptor() throws Exception {
100125
String serverName = InProcessServerBuilder.generateName();
101-
XdsConfig defaultXdsConfig = XdsTestUtils.getDefaultXdsConfigWithCdsUpdate(serverName);
126+
XdsConfig.XdsConfigBuilder builder = new XdsConfig.XdsConfigBuilder();
127+
128+
Filter.NamedFilterConfig routerFilterConfig = new Filter.NamedFilterConfig(
129+
serverName, RouterFilter.ROUTER_CONFIG);
130+
131+
HttpConnectionManager httpConnectionManager = HttpConnectionManager.forRdsName(
132+
0L, RDS_NAME, Collections.singletonList(routerFilterConfig));
133+
XdsListenerResource.LdsUpdate ldsUpdate =
134+
XdsListenerResource.LdsUpdate.forApiListener(httpConnectionManager);
135+
136+
RouteConfiguration routeConfiguration =
137+
buildRouteConfiguration(serverName, RDS_NAME, CLUSTER_NAME);
138+
XdsResourceType.Args args = new XdsResourceType.Args(null, "0", "0", null, null, null);
139+
XdsRouteConfigureResource.RdsUpdate rdsUpdate =
140+
XdsRouteConfigureResource.getInstance().doParse(args, routeConfiguration);
141+
142+
// Take advantage of knowing that there is only 1 virtual host in the route configuration
143+
assertThat(rdsUpdate.virtualHosts).hasSize(1);
144+
VirtualHost virtualHost = rdsUpdate.virtualHosts.get(0);
145+
146+
// Need to create endpoints to create locality endpoints map to create edsUpdate
147+
Map<Locality, LocalityLbEndpoints> lbEndpointsMap = new HashMap<>();
148+
LbEndpoint lbEndpoint = LbEndpoint.create(
149+
serverName, ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
150+
lbEndpointsMap.put(
151+
Locality.create("", "", ""),
152+
LocalityLbEndpoints.create(ImmutableList.of(lbEndpoint), 10, 0, ImmutableMap.of()));
153+
154+
// Need to create EdsUpdate to create CdsUpdate to create XdsClusterConfig for builder
155+
XdsEndpointResource.EdsUpdate edsUpdate = new XdsEndpointResource.EdsUpdate(
156+
EDS_NAME, lbEndpointsMap, Collections.emptyList());
157+
158+
// Use ImmutableMap.Builder to construct the map
159+
ImmutableMap.Builder<String, Object> parsedMetadata = ImmutableMap.builder();
160+
parsedMetadata.put("FILTER_INSTANCE_NAME", new AudienceWrapper("TEST_AUDIENCE"));
161+
162+
CdsUpdate.Builder cdsUpdate = CdsUpdate.forEds(
163+
CLUSTER_NAME, EDS_NAME, null, null, null, null, false)
164+
.lbPolicyConfig(getWrrLbConfigAsMap());
165+
cdsUpdate.parsedMetadata(parsedMetadata.build());
166+
XdsConfig.XdsClusterConfig clusterConfig = new XdsConfig.XdsClusterConfig(
167+
CLUSTER_NAME,
168+
cdsUpdate.build(),
169+
new EndpointConfig(StatusOr.fromValue(edsUpdate)));
102170

103171
GcpAuthenticationConfig config = new GcpAuthenticationConfig(10);
104172
GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME");
@@ -109,17 +177,80 @@ public void testClientInterceptor_createsAndReusesCachedCredentials() throws Exc
109177

110178
// Mock channel and capture CallOptions
111179
Channel mockChannel = mock(Channel.class);
112-
ArgumentCaptor<CallOptions> callOptionsCaptor = ArgumentCaptor.forClass(CallOptions.class);
113180

114181
// Set CallOptions with required keys
115-
CallOptions callOptionsWithXds = CallOptions.DEFAULT
182+
CallOptions callOptionsWithXds = CallOptions.DEFAULT;
183+
184+
// Execute interception twice to check caching
185+
ClientCall<Void, Void> call = interceptor.interceptCall(
186+
methodDescriptor, callOptionsWithXds, mockChannel);
187+
assertTrue(call instanceof FailingClientCall);
188+
FailingClientCall<Void, Void> clientCall = (FailingClientCall<Void, Void>) call;
189+
assertThat(clientCall.error.getDescription()).contains("does not contain cluster resource");
190+
191+
callOptionsWithXds = CallOptions.DEFAULT
192+
.withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0");
193+
194+
// Execute interception twice to check caching
195+
call = interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel);
196+
assertTrue(call instanceof FailingClientCall);
197+
clientCall = (FailingClientCall<Void, Void>) call;
198+
assertThat(clientCall.error.getDescription()).contains("does not contain xds configuration");
199+
200+
XdsConfig defaultXdsConfig = builder
201+
.setListener(ldsUpdate)
202+
.setRoute(rdsUpdate)
203+
.setVirtualHost(virtualHost)
204+
.addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig)).build();
205+
callOptionsWithXds = CallOptions.DEFAULT
206+
.withOption(CLUSTER_SELECTION_KEY, "cluster:cluster")
207+
.withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig);
208+
209+
// Execute interception twice to check caching
210+
call = interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel);
211+
assertTrue(call instanceof FailingClientCall);
212+
clientCall = (FailingClientCall<Void, Void>) call;
213+
assertThat(clientCall.error.getDescription()).contains("does not contain xds cluster");
214+
215+
StatusOr<XdsClusterConfig> errorCluster =
216+
StatusOr.fromStatus(Status.NOT_FOUND.withDescription("Cluster resource not found"));
217+
defaultXdsConfig = builder
218+
.setListener(ldsUpdate)
219+
.setRoute(rdsUpdate)
220+
.setVirtualHost(virtualHost)
221+
.addCluster(CLUSTER_NAME, errorCluster).build();
222+
callOptionsWithXds = CallOptions.DEFAULT
223+
.withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0")
224+
.withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig);
225+
226+
// Create interceptor
227+
interceptor = filter.buildClientInterceptor(config, null, null);
228+
methodDescriptor = TestMethodDescriptors.voidMethod();
229+
230+
// Mock channel and capture CallOptions
231+
mockChannel = mock(Channel.class);
232+
call = interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel);
233+
assertTrue(call instanceof FailingClientCall);
234+
clientCall = (FailingClientCall<Void, Void>) call;
235+
assertThat(clientCall.error.getDescription())
236+
.contains("Cluster resource not found");
237+
238+
// Success case
239+
defaultXdsConfig = builder
240+
.setListener(ldsUpdate)
241+
.setRoute(rdsUpdate)
242+
.setVirtualHost(virtualHost)
243+
.addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig)).build();
244+
// Set CallOptions with required keys
245+
callOptionsWithXds = CallOptions.DEFAULT
116246
.withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0")
117247
.withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig);
118248

119249
// Execute interception twice to check caching
120250
interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel);
121251
interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel);
122252

253+
ArgumentCaptor<CallOptions> callOptionsCaptor = ArgumentCaptor.forClass(CallOptions.class);
123254
// Capture and verify CallOptions for CallCredentials presence
124255
Mockito.verify(mockChannel, Mockito.times(2))
125256
.newCall(eq(methodDescriptor), callOptionsCaptor.capture());

‎xds/src/test/java/io/grpc/xds/XdsTestUtils.java‎

Lines changed: 1 addition & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@
5656
import io.grpc.stub.StreamObserver;
5757
import io.grpc.xds.Endpoints.LbEndpoint;
5858
import io.grpc.xds.Endpoints.LocalityLbEndpoints;
59-
import io.grpc.xds.GcpAuthenticationFilter.AudienceMetadataParser.AudienceWrapper;
60-
import io.grpc.xds.XdsClusterResource.CdsUpdate;
6159
import io.grpc.xds.XdsConfig.XdsClusterConfig.EndpointConfig;
6260
import io.grpc.xds.client.Bootstrapper;
6361
import io.grpc.xds.client.Locality;
@@ -282,63 +280,6 @@ static XdsConfig getDefaultXdsConfig(String serverHostName)
282280
return builder.build();
283281
}
284282

285-
static XdsConfig getDefaultXdsConfigWithCdsUpdate(String serverHostName)
286-
throws XdsResourceType.ResourceInvalidException, IOException {
287-
XdsConfig.XdsConfigBuilder builder = new XdsConfig.XdsConfigBuilder();
288-
289-
Filter.NamedFilterConfig routerFilterConfig = new Filter.NamedFilterConfig(
290-
serverHostName, RouterFilter.ROUTER_CONFIG);
291-
292-
HttpConnectionManager httpConnectionManager = HttpConnectionManager.forRdsName(
293-
0L, RDS_NAME, Collections.singletonList(routerFilterConfig));
294-
XdsListenerResource.LdsUpdate ldsUpdate =
295-
XdsListenerResource.LdsUpdate.forApiListener(httpConnectionManager);
296-
297-
RouteConfiguration routeConfiguration =
298-
buildRouteConfiguration(serverHostName, RDS_NAME, CLUSTER_NAME);
299-
Bootstrapper.ServerInfo serverInfo = null;
300-
XdsResourceType.Args args = new XdsResourceType.Args(serverInfo, "0", "0", null, null, null);
301-
XdsRouteConfigureResource.RdsUpdate rdsUpdate =
302-
XdsRouteConfigureResource.getInstance().doParse(args, routeConfiguration);
303-
304-
// Take advantage of knowing that there is only 1 virtual host in the route configuration
305-
assertThat(rdsUpdate.virtualHosts).hasSize(1);
306-
VirtualHost virtualHost = rdsUpdate.virtualHosts.get(0);
307-
308-
// Need to create endpoints to create locality endpoints map to create edsUpdate
309-
Map<Locality, LocalityLbEndpoints> lbEndpointsMap = new HashMap<>();
310-
LbEndpoint lbEndpoint = LbEndpoint.create(
311-
serverHostName, ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
312-
lbEndpointsMap.put(
313-
Locality.create("", "", ""),
314-
LocalityLbEndpoints.create(ImmutableList.of(lbEndpoint), 10, 0, ImmutableMap.of()));
315-
316-
// Need to create EdsUpdate to create CdsUpdate to create XdsClusterConfig for builder
317-
XdsEndpointResource.EdsUpdate edsUpdate = new XdsEndpointResource.EdsUpdate(
318-
EDS_NAME, lbEndpointsMap, Collections.emptyList());
319-
320-
// Use ImmutableMap.Builder to construct the map
321-
ImmutableMap.Builder<String, Object> parsedMetadata = ImmutableMap.builder();
322-
parsedMetadata.put("FILTER_INSTANCE_NAME", new AudienceWrapper("TEST_AUDIENCE"));
323-
324-
CdsUpdate.Builder cdsUpdate = CdsUpdate.forEds(
325-
CLUSTER_NAME, EDS_NAME, null, null, null, null, false)
326-
.lbPolicyConfig(getWrrLbConfigAsMap());
327-
cdsUpdate.parsedMetadata(parsedMetadata.build());
328-
XdsConfig.XdsClusterConfig clusterConfig = new XdsConfig.XdsClusterConfig(
329-
CLUSTER_NAME,
330-
cdsUpdate.build(),
331-
new EndpointConfig(StatusOr.fromValue(edsUpdate)));
332-
333-
builder
334-
.setListener(ldsUpdate)
335-
.setRoute(rdsUpdate)
336-
.setVirtualHost(virtualHost)
337-
.addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig));
338-
339-
return builder.build();
340-
}
341-
342283
static Map<Locality, LocalityLbEndpoints> createMinimalLbEndpointsMap(String serverHostName) {
343284
Map<Locality, LocalityLbEndpoints> lbEndpointsMap = new HashMap<>();
344285
LbEndpoint lbEndpoint = LbEndpoint.create(
@@ -350,7 +291,7 @@ static Map<Locality, LocalityLbEndpoints> createMinimalLbEndpointsMap(String ser
350291
}
351292

352293
@SuppressWarnings("unchecked")
353-
private static ImmutableMap<String, ?> getWrrLbConfigAsMap() throws IOException {
294+
static ImmutableMap<String, ?> getWrrLbConfigAsMap() throws IOException {
354295
String lbConfigStr = "{\"wrr_locality_experimental\" : "
355296
+ "{ \"childPolicy\" : [{\"round_robin\" : {}}]}}";
356297

0 commit comments

Comments
 (0)