Skip to content

Commit 85758e3

Browse files
committed
xds: Make RawMessageClientInterceptor conditional on ext_proc flags
Only add RawMessageClientInterceptor to the interceptor chain in XdsNameResolver when either GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT or GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER is true. This prevents RawMessageClientInterceptor from corrupting method descriptors and causing empty payloads on retry attempts for regular xDS channels. Fixes grpc#13010 TAG=agy CONV=56c87717-243d-4f12-af9e-c532e509892a
1 parent 9799887 commit 85758e3

2 files changed

Lines changed: 85 additions & 3 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,10 @@ private ClientInterceptor createFilters(
907907
}
908908

909909
ImmutableList.Builder<ClientInterceptor> withRawMessage = ImmutableList.builder();
910-
withRawMessage.add(new RawMessageClientInterceptor());
910+
if (GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", false)
911+
|| GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", false)) {
912+
withRawMessage.add(new RawMessageClientInterceptor());
913+
}
911914
withRawMessage.addAll(filterInterceptors.build());
912915
return combineInterceptors(withRawMessage.build());
913916
}

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

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2957,7 +2957,7 @@ private final class TestChannel extends Channel {
29572957
@Override
29582958
public <ReqT, RespT> ClientCall<ReqT, RespT> newCall(
29592959
MethodDescriptor<ReqT, RespT> methodDescriptor, CallOptions callOptions) {
2960-
TestCall<ReqT, RespT> call = new TestCall<>(callOptions);
2960+
TestCall<ReqT, RespT> call = new TestCall<>(methodDescriptor, callOptions);
29612961
testCall = call;
29622962
return call;
29632963
}
@@ -2969,11 +2969,13 @@ public String authority() {
29692969
}
29702970

29712971
private static final class TestCall<ReqT, RespT> extends NoopClientCall<ReqT, RespT> {
2972+
final MethodDescriptor<ReqT, RespT> methodDescriptor;
29722973
// CallOptions actually received from the channel when the call is created.
29732974
final CallOptions callOptions;
29742975
ClientCall.Listener<RespT> listener;
29752976

2976-
TestCall(CallOptions callOptions) {
2977+
TestCall(MethodDescriptor<ReqT, RespT> methodDescriptor, CallOptions callOptions) {
2978+
this.methodDescriptor = methodDescriptor;
29772979
this.callOptions = callOptions;
29782980
}
29792981

@@ -3098,4 +3100,81 @@ public void onMessage(String message) {
30983100
channel, METHOD_SAY_HELLO, CallOptions.DEFAULT, "World");
30993101
assertThat(response).isEqualTo("Hello World");
31003102
}
3103+
3104+
@Test
3105+
public void rawMessageClientInterceptor_flagFalse() {
3106+
String origClientProp = System.getProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT");
3107+
String origServerProp = System.getProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER");
3108+
System.setProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", "false");
3109+
System.setProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", "false");
3110+
try {
3111+
filterStateTestSetupResolver();
3112+
FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient();
3113+
VirtualHost vhost = filterStateTestVhost();
3114+
3115+
xdsClient.deliverLdsUpdateWithFilters(vhost, filterStateTestConfigs(STATEFUL_1));
3116+
createAndDeliverClusterUpdates(xdsClient, cluster1);
3117+
3118+
// When flags are false, RawMessageClientInterceptor is not added.
3119+
assertClusterResolutionResult(call1, cluster1);
3120+
assertThat(testCall.methodDescriptor).isSameInstanceAs(call1.methodDescriptor);
3121+
} finally {
3122+
restoreProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", origClientProp);
3123+
restoreProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", origServerProp);
3124+
}
3125+
}
3126+
3127+
@Test
3128+
public void rawMessageClientInterceptor_flagTrue() {
3129+
String origClientProp = System.getProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT");
3130+
String origServerProp = System.getProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER");
3131+
3132+
// When GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT is true, RawMessageClientInterceptor is added.
3133+
System.setProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", "true");
3134+
System.setProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", "false");
3135+
try {
3136+
filterStateTestSetupResolver();
3137+
FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient();
3138+
VirtualHost vhost = filterStateTestVhost();
3139+
3140+
xdsClient.deliverLdsUpdateWithFilters(vhost, filterStateTestConfigs(STATEFUL_1));
3141+
createAndDeliverClusterUpdates(xdsClient, cluster1);
3142+
3143+
assertClusterResolutionResult(call1, cluster1);
3144+
assertThat(testCall.methodDescriptor).isNotSameInstanceAs(call1.methodDescriptor);
3145+
} finally {
3146+
restoreProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", origClientProp);
3147+
restoreProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", origServerProp);
3148+
}
3149+
3150+
resolver.shutdown();
3151+
reset(mockListener);
3152+
when(mockListener.onResult2(any())).thenReturn(Status.OK);
3153+
3154+
// When GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER is true, RawMessageClientInterceptor is added.
3155+
System.setProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", "false");
3156+
System.setProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", "true");
3157+
try {
3158+
filterStateTestSetupResolver();
3159+
FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient();
3160+
VirtualHost vhost = filterStateTestVhost();
3161+
3162+
xdsClient.deliverLdsUpdateWithFilters(vhost, filterStateTestConfigs(STATEFUL_1));
3163+
createAndDeliverClusterUpdates(xdsClient, cluster1);
3164+
3165+
assertClusterResolutionResult(call1, cluster1);
3166+
assertThat(testCall.methodDescriptor).isNotSameInstanceAs(call1.methodDescriptor);
3167+
} finally {
3168+
restoreProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT", origClientProp);
3169+
restoreProperty("GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER", origServerProp);
3170+
}
3171+
}
3172+
3173+
private static void restoreProperty(String key, @Nullable String value) {
3174+
if (value == null) {
3175+
System.clearProperty(key);
3176+
} else {
3177+
System.setProperty(key, value);
3178+
}
3179+
}
31013180
}

0 commit comments

Comments
 (0)