From bfb8cc0ab634979b953b32c091c6b7b2286f00ce Mon Sep 17 00:00:00 2001 From: Saurav Date: Fri, 11 Sep 2026 11:04:25 +0000 Subject: [PATCH] xds: apply ext_authz check deadline per-RPC withDeadlineAfter() resolves an absolute Deadline when it is called, so applying it to the shared AuthorizationStub at interceptor construction fixed one deadline for every RPC. Once the timeout elapsed, all later check RPCs failed immediately with DEADLINE_EXCEEDED. Apply the timeout to the per-RPC ExtAuthzClientCall instead. --- .../main/java/io/grpc/xds/ExtAuthzFilter.java | 6 --- .../internal/extauthz/ExtAuthzClientCall.java | 5 +- .../java/io/grpc/xds/ExtAuthzFilterTest.java | 7 ++- .../extauthz/ExtAuthzClientCallTest.java | 52 +++++++++++++++++++ 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ExtAuthzFilter.java b/xds/src/main/java/io/grpc/xds/ExtAuthzFilter.java index cf015ca129f..f77587d4454 100644 --- a/xds/src/main/java/io/grpc/xds/ExtAuthzFilter.java +++ b/xds/src/main/java/io/grpc/xds/ExtAuthzFilter.java @@ -42,7 +42,6 @@ import io.grpc.xds.internal.headermutations.HeaderMutationFilter; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import javax.annotation.concurrent.ThreadSafe; @@ -218,11 +217,6 @@ public ClientInterceptor buildClientInterceptor(FilterConfig config, stub = stub.withCallCredentials( extAuthzConfig.grpcService().googleGrpc().callCredentials().get()); } - if (extAuthzConfig.grpcService().timeout().isPresent()) { - stub = stub.withDeadlineAfter( - extAuthzConfig.grpcService().timeout().get().toMillis(), - TimeUnit.MILLISECONDS); - } return new ExtAuthzClientInterceptor(extAuthzConfig, stub, random, new CheckRequestBuilder(extAuthzConfig), new CheckResponseHandler( diff --git a/xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCall.java b/xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCall.java index eb5ddd7ed60..ea45ceb7765 100644 --- a/xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCall.java +++ b/xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCall.java @@ -31,6 +31,7 @@ import io.grpc.internal.DelayedClientCall; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; /** @@ -64,7 +65,9 @@ public ExtAuthzClientCall( this.callOptions = callOptions; this.next = next; this.method = method; - this.authzStub = authzStub; + this.authzStub = config.grpcService().timeout() + .map(t -> authzStub.withDeadlineAfter(t.toMillis(), TimeUnit.MILLISECONDS)) + .orElse(authzStub); this.checkRequestBuilder = checkRequestBuilder; this.responseHandler = responseHandler; this.config = config; diff --git a/xds/src/test/java/io/grpc/xds/ExtAuthzFilterTest.java b/xds/src/test/java/io/grpc/xds/ExtAuthzFilterTest.java index 7cbdd316dff..2d7e273fb34 100644 --- a/xds/src/test/java/io/grpc/xds/ExtAuthzFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/ExtAuthzFilterTest.java @@ -174,7 +174,7 @@ public void buildClientInterceptor_success() { } @Test - public void buildClientInterceptor_withTimeout_appliesDeadline() { + public void buildClientInterceptor_withTimeout_doesNotBakeDeadlineIntoSharedStub() { GrpcServiceConfig.GoogleGrpcConfig googleGrpc = GrpcServiceConfig.GoogleGrpcConfig.builder() .target("test-cluster") .configuredChannelCredentials(io.grpc.xds.client.ConfiguredChannelCredentials.create( @@ -204,7 +204,10 @@ public void buildClientInterceptor_withTimeout_appliesDeadline() { assertThat(created).isInstanceOf(ExtAuthzFilter.ExtAuthzClientInterceptor.class); ExtAuthzFilter.ExtAuthzClientInterceptor interceptor = (ExtAuthzFilter.ExtAuthzClientInterceptor) created; - assertThat(interceptor.getAuthzStubForTest().getCallOptions().getDeadline()).isNotNull(); + // The stub is shared by every RPC this interceptor handles. Resolving the timeout into an + // absolute Deadline here would expire it for all later calls, so it is applied per-RPC by + // ExtAuthzClientCall instead. + assertThat(interceptor.getAuthzStubForTest().getCallOptions().getDeadline()).isNull(); } @Test diff --git a/xds/src/test/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCallTest.java b/xds/src/test/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCallTest.java index 762011ddd5b..11c04a182fc 100644 --- a/xds/src/test/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCallTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCallTest.java @@ -34,6 +34,7 @@ import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; import io.grpc.Context; +import io.grpc.Deadline; import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -56,6 +57,7 @@ import io.grpc.xds.internal.grpcservice.HeaderValue; import io.grpc.xds.internal.headermutations.HeaderMutations; import io.grpc.xds.internal.headermutations.HeaderValueOption; +import java.time.Duration; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -627,6 +629,28 @@ public void drain_executesOnCallExecutor() throws Exception { verify(mockExecutor, org.mockito.Mockito.atLeastOnce()).execute(any(Runnable.class)); } + @Test + public void start_withTimeout_appliesDeadlineToCheckRpc() throws Exception { + ExtAuthzConfig configWithTimeout = buildConfigWithTimeout(Duration.ofSeconds(10)); + AtomicReference observedDeadline = new AtomicReference<>(); + CountDownLatch checkReceived = new CountDownLatch(1); + doAnswer(invocation -> { + observedDeadline.set(Context.current().getDeadline()); + checkReceived.countDown(); + return null; + }).when(authzService).check(any(CheckRequest.class), ArgumentMatchers.any()); + + createCall(com.google.common.util.concurrent.MoreExecutors.directExecutor(), channel, + configWithTimeout).start(new CapturingListener<>(), new Metadata()); + + assertThat(checkReceived.await(5, TimeUnit.SECONDS)).isTrue(); + Deadline deadline = observedDeadline.get(); + // ExtAuthzFilterTest asserts the shared stub carries no deadline, so one observed here can + // only have been derived for this individual call. + assertThat(deadline).isNotNull(); + assertThat(deadline.timeRemaining(TimeUnit.MILLISECONDS)).isAtMost(10_000L); + } + private ExtAuthzClientCall createCall() { return createCall(channel, config); @@ -675,4 +699,32 @@ private static ExtAuthzConfig buildConfig() { .statusOnError(Status.PERMISSION_DENIED) .build(); } + + private static ExtAuthzConfig buildConfigWithTimeout(Duration timeout) { + GrpcServiceConfig.GoogleGrpcConfig googleGrpc = + GrpcServiceConfig.GoogleGrpcConfig.builder() + .target("test-cluster") + .configuredChannelCredentials( + ConfiguredChannelCredentials.create( + mock(ChannelCredentials.class), + mock(ConfiguredChannelCredentials + .ChannelCredsConfig.class))) + .build(); + GrpcServiceConfig grpcServiceConfig = + GrpcServiceConfig.builder() + .googleGrpc(googleGrpc) + .initialMetadata(ImmutableList.of()) + .timeout(timeout) + .build(); + return ExtAuthzConfig.builder() + .grpcService(grpcServiceConfig) + .failureModeAllow(false) + .failureModeAllowHeaderAdd(false) + .includePeerCertificate(false) + .denyAtDisable(false) + .filterEnabled( + Matchers.FractionMatcher.create(100, 100)) + .statusOnError(Status.PERMISSION_DENIED) + .build(); + } }