Skip to content

Commit ca79e36

Browse files
committed
xds: default to UNKNOWN status when ImmediateResponse status is unset
TAG=agy CONV=581f86a6-097d-44a2-9d6e-6f9d2e83cbc0
1 parent 7208ff8 commit ca79e36

2 files changed

Lines changed: 110 additions & 1 deletion

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,9 @@ void drainPendingUpstreamBodyMessages() {
13291329

13301330
private void handleImmediateResponse(ImmediateResponse immediate, DataPlaneListener listener)
13311331
throws HeaderMutationDisallowedException {
1332-
Status status = Status.fromCodeValue(immediate.getGrpcStatus().getStatus());
1332+
Status status = immediate.hasGrpcStatus()
1333+
? Status.fromCodeValue(immediate.getGrpcStatus().getStatus())
1334+
: Status.UNKNOWN;
13331335
if (!immediate.getDetails().isEmpty()) {
13341336
status = status.withDescription(immediate.getDetails());
13351337
}

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

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11439,6 +11439,113 @@ public void onClose(Status status, Metadata trailers) {
1143911439
channelManager.close();
1144011440
}
1144111441

11442+
@Test
11443+
@SuppressWarnings("unchecked")
11444+
public void givenImmediateResponseWithUnsetStatus_whenReceived_thenStatusUnknown()
11445+
throws Exception {
11446+
ExternalProcessor proto = ExternalProcessor.newBuilder()
11447+
.setGrpcService(GrpcService.newBuilder()
11448+
.setGoogleGrpc(GrpcService.GoogleGrpc.newBuilder()
11449+
.setTargetUri("in-process:///" + extProcServerName)
11450+
.addChannelCredentialsPlugin(Any.newBuilder()
11451+
.setTypeUrl("type.googleapis.com/envoy.extensions.grpc_service."
11452+
+ "channel_credentials.insecure.v3.InsecureCredentials")
11453+
.build())
11454+
.build())
11455+
.build())
11456+
.setProcessingMode(ProcessingMode.getDefaultInstance())
11457+
.build();
11458+
ConfigOrError<ExternalProcessorFilterConfig> configOrError =
11459+
provider.parseFilterConfig(Any.pack(proto), filterContext);
11460+
assertThat(configOrError.errorDetail).isNull();
11461+
ExternalProcessorFilterConfig filterConfig = configOrError.config;
11462+
11463+
// External Processor Server
11464+
ExternalProcessorGrpc.ExternalProcessorImplBase extProcImpl;
11465+
extProcImpl = new ExternalProcessorGrpc.ExternalProcessorImplBase() {
11466+
@Override
11467+
@SuppressWarnings("unchecked")
11468+
public StreamObserver<ProcessingRequest> process(
11469+
final StreamObserver<ProcessingResponse> responseObserver) {
11470+
((ServerCallStreamObserver<ProcessingResponse>) responseObserver).request(100);
11471+
return new StreamObserver<ProcessingRequest>() {
11472+
@Override
11473+
public void onNext(ProcessingRequest request) {
11474+
if (request.hasRequestHeaders()) {
11475+
responseObserver.onNext(ProcessingResponse.newBuilder()
11476+
.setImmediateResponse(ImmediateResponse.newBuilder()
11477+
.setDetails("Custom security rejection")
11478+
.build())
11479+
.build());
11480+
responseObserver.onCompleted();
11481+
}
11482+
}
11483+
11484+
@Override
11485+
public void onError(Throwable t) {
11486+
}
11487+
11488+
@Override
11489+
public void onCompleted() {
11490+
}
11491+
};
11492+
}
11493+
};
11494+
grpcCleanup.register(InProcessServerBuilder.forName(extProcServerName)
11495+
.addService(extProcImpl)
11496+
.directExecutor()
11497+
.build().start());
11498+
11499+
CachedChannelManager channelManager = new CachedChannelManager(config -> {
11500+
return grpcCleanup.register(
11501+
InProcessChannelBuilder.forName(extProcServerName)
11502+
.directExecutor()
11503+
.build());
11504+
});
11505+
11506+
ExternalProcessorClientInterceptor interceptor = new ExternalProcessorClientInterceptor(
11507+
filterConfig, channelManager, scheduler, FAKE_CONTEXT);
11508+
11509+
final AtomicBoolean dataPlaneStarted = new AtomicBoolean(false);
11510+
dataPlaneServiceRegistry.addService(ServerServiceDefinition.builder("test.TestService")
11511+
.addMethod(METHOD_SAY_HELLO, ServerCalls.asyncUnaryCall(
11512+
(request, responseObserver) -> {
11513+
dataPlaneStarted.set(true);
11514+
responseObserver.onNext("Hello " + request);
11515+
responseObserver.onCompleted();
11516+
}))
11517+
.build());
11518+
11519+
ManagedChannel dataPlaneChannel = grpcCleanup.register(
11520+
InProcessChannelBuilder.forName(dataPlaneServerName).directExecutor().build());
11521+
11522+
final AtomicReference<Status> closedStatus = new AtomicReference<>();
11523+
final CountDownLatch closedLatch = new CountDownLatch(1);
11524+
ClientCall.Listener<String> appListener = new ClientCall.Listener<String>() {
11525+
@Override
11526+
public void onClose(Status status, Metadata trailers) {
11527+
closedStatus.set(status);
11528+
closedLatch.countDown();
11529+
}
11530+
};
11531+
11532+
CallOptions callOptions = DEFAULT_CALL_OPTIONS.withExecutor(MoreExecutors.directExecutor());
11533+
ClientCall<String, String> proxyCall =
11534+
interceptCall(interceptor, METHOD_SAY_HELLO, callOptions, dataPlaneChannel);
11535+
proxyCall.start(appListener, new Metadata());
11536+
11537+
// Verify app listener notified with the correct status and details
11538+
assertThat(closedLatch.await(5, TimeUnit.SECONDS)).isTrue();
11539+
assertThat(closedStatus.get().getCode()).isEqualTo(Status.Code.UNKNOWN);
11540+
assertThat(closedStatus.get().getDescription()).isEqualTo("Custom security rejection");
11541+
11542+
// Data plane call should NOT have been started as sidecar rejected immediately on headers
11543+
assertThat(dataPlaneStarted.get()).isFalse();
11544+
11545+
proxyCall.cancel("Cleanup", null);
11546+
channelManager.close();
11547+
}
11548+
1144211549
@Test
1144311550
@SuppressWarnings("unchecked")
1144411551
public void givenImmediateResponseAndObservabilityTrue_whenReceived_thenImmediateResponseIgnored()

0 commit comments

Comments
 (0)