xds: Reuse ext_authz header mutation helpers

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

@@ -291,7 +291,6 @@ private static class DataPlaneClientCall<ReqT, RespT>
         new ConcurrentLinkedQueue<>();
     @Nullable private volatile DataPlaneListener<RespT> wrappedListener;
     private final HeaderMutationFilter mutationFilter;
-    private final HeaderMutator mutator = HeaderMutator.create();
     private final AtomicInteger pendingRequests = new AtomicInteger(0);
     private final ProcessingMode currentProcessingMode;
 
@@ -606,7 +605,7 @@ public void onNext(ProcessingResponse response) {
                     requestHeaders,
                     response.getRequestHeaders().getResponse().getHeaderMutation(),
                     mutationFilter,
-                    mutator);
+                    HeaderMutator.HEADER_MUTATOR);
               }
               activateCall();
             }
@@ -632,7 +631,7 @@ else if (response.hasResponseHeaders()) {
                     target,
                     response.getResponseHeaders().getResponse().getHeaderMutation(),
                     mutationFilter,
-                    mutator);
+                    HeaderMutator.HEADER_MUTATOR);
               }
               if (wrappedListener.isTrailersOnly()) {
                 wrappedListener.proceedWithClose();
@@ -653,7 +652,7 @@ else if (response.hasResponseTrailers()) {
                     wrappedListener.getSavedTrailers(),
                     response.getResponseTrailers().getHeaderMutation(),
                     mutationFilter,
-                    mutator);
+                    HeaderMutator.HEADER_MUTATOR);
               }
               wrappedListener.proceedWithClose();
             }
@@ -1339,7 +1338,8 @@ private void handleImmediateResponse(
 
       Metadata trailers = new Metadata();
       if (immediate.hasHeaders()) {
-        applyHeaderMutations(trailers, immediate.getHeaders(), mutationFilter, mutator);
+        applyHeaderMutations(
+            trailers, immediate.getHeaders(), mutationFilter, HeaderMutator.HEADER_MUTATOR);
       }
 
       listener.setImmediateResponse(status, trailers);

xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzCallbackObserver.java

@@ -48,8 +48,6 @@ final class AuthzCallbackObserver<ReqT, RespT> implements StreamObserver<CheckRe
   private final CallOptions callOptions;
   private final Executor callExecutor;
   private final CheckResponseHandler responseHandler;
-  // TODO(sauravzg): Make this static seeing that it holds no state.
-  private final HeaderMutator headerMutator = HeaderMutator.create();
   private final ExtAuthzConfig config;
   private final Context.CancellableContext authzContext;
 
@@ -97,7 +95,7 @@ && isHeaderMutationsEmpty(authzResponse.responseHeaderMutations())) {
             delegate,
             authzResponse.requestHeaderMutations(),
             authzResponse.responseHeaderMutations(),
-            headerMutator);
+            HeaderMutator.HEADER_MUTATOR);
       }
       setCallAndDrain(finalCall);
     } else {
@@ -113,7 +111,7 @@ && isHeaderMutationsEmpty(authzResponse.responseHeaderMutations())) {
         failingCall = new FailingCallWithTrailerMutations<>(
             status,
             authzResponse.responseHeaderMutations(),
-            headerMutator);
+            HeaderMutator.HEADER_MUTATOR);
       }
       setCallAndDrain(failingCall);
     }
@@ -130,12 +128,9 @@ public void onError(Throwable t) {
                 HeaderValue.create(X_ENVOY_AUTH_FAILURE_MODE_ALLOWED.name(), "true"),
                 HeaderValueOption.HeaderAppendAction.APPEND_IF_EXISTS_OR_ADD)),
             ImmutableList.of());
-        // TODO(sauravz): Consider using a static EMPTY_MUTATIONS constant
-        // to avoid repeated allocation of empty HeaderMutations.
-        HeaderMutations responseMutations =
-            HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
         finalCall = new MutatingClientCall<>(
-            delegate, requestMutations, responseMutations, headerMutator);
+            delegate, requestMutations, HeaderMutations.EMPTY_MUTATIONS,
+            HeaderMutator.HEADER_MUTATOR);
       } else {
         finalCall = delegate;
       }

xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzResponse.java

@@ -17,7 +17,6 @@
 package io.grpc.xds.internal.extauthz;
 
 import com.google.auto.value.AutoValue;
-import com.google.common.collect.ImmutableList;
 import io.grpc.Status;
 import io.grpc.xds.internal.headermutations.HeaderMutations;
 import java.util.Optional;
@@ -37,15 +36,12 @@ public enum Decision {
     DENY,
   }
 
-  private static final HeaderMutations EMPTY_MUTATIONS =
-      HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
-
   /**
    * Creates a builder for an ALLOW response, initializing with the specified request header
    * mutations.
    */
   static Builder allow(HeaderMutations requestHeaderMutations) {
-    return allow(requestHeaderMutations, EMPTY_MUTATIONS);
+    return allow(requestHeaderMutations, HeaderMutations.EMPTY_MUTATIONS);
   }
 
   /**
@@ -60,14 +56,14 @@ static Builder allow(
 
   /** Creates a builder for a DENY response, initializing with the specified status. */
   static Builder deny(Status status) {
-    return deny(status, EMPTY_MUTATIONS);
+    return deny(status, HeaderMutations.EMPTY_MUTATIONS);
   }
 
   /** Creates a builder for a DENY response with status and response trailer mutations. */
   static Builder deny(Status status, HeaderMutations responseHeaderMutations) {
     return new AutoValue_AuthzResponse.Builder().setDecision(Decision.DENY)
         .setResponseHeaderMutations(responseHeaderMutations)
-        .setRequestHeaderMutations(EMPTY_MUTATIONS)
+        .setRequestHeaderMutations(HeaderMutations.EMPTY_MUTATIONS)
         .setStatus(status);
   }
 

xds/src/main/java/io/grpc/xds/internal/extauthz/CheckResponseHandler.java

@@ -58,8 +58,7 @@ AuthzResponse handleResponse(final CheckResponse response) {
   private AuthzResponse handleOkResponse(final CheckResponse response)
       throws HeaderMutationDisallowedException {
     if (!response.hasOkResponse()) {
-      return AuthzResponse.allow(
-          HeaderMutations.create(ImmutableList.of(), ImmutableList.of())).build();
+      return AuthzResponse.allow(HeaderMutations.EMPTY_MUTATIONS).build();
     }
     CheckResponseMutations allowedMutations =
         buildHeaderMutationsFromOkResponse(response.getOkResponse());

xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutations.java

@@ -23,6 +23,10 @@
 @AutoValue
 public abstract class HeaderMutations {
 
+  /** Shared empty header mutations. */
+  public static final HeaderMutations EMPTY_MUTATIONS =
+      create(ImmutableList.of(), ImmutableList.of());
+
   public static HeaderMutations create(ImmutableList<HeaderValueOption> headers,
       ImmutableList<String> headersToRemove) {
     return new AutoValue_HeaderMutations(headers, headersToRemove);

xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java

@@ -29,15 +29,20 @@
 public class HeaderMutator {
 
   private static final Logger logger = Logger.getLogger(HeaderMutator.class.getName());
+  /** Shared stateless header mutator. */
+  public static final HeaderMutator HEADER_MUTATOR = new HeaderMutator();
 
   /**
    * Creates a new instance of {@code HeaderMutator}.
+   *
+   * @deprecated Use {@link #HEADER_MUTATOR} instead.
    */
+  @Deprecated
   public static HeaderMutator create() {
     return new HeaderMutator();
   }
 
-  HeaderMutator() {}
+  private HeaderMutator() {}
 
   /**
    * Applies the given header mutations to the provided metadata headers.

xds/src/test/java/io/grpc/xds/internal/extauthz/AuthzResponseTest.java

@@ -32,7 +32,7 @@ public class AuthzResponseTest {
   @Test
   public void testAllow() {
     HeaderMutations requestMutations =
-        HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
+        HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse response = AuthzResponse.allow(requestMutations).build();
     assertThat(response.decision()).isEqualTo(Decision.ALLOW);
     assertThat(response.requestHeaderMutations()).isEqualTo(requestMutations);
@@ -43,7 +43,7 @@ public void testAllow() {
   @Test
   public void testAllowWithHeaderMutations() {
     HeaderMutations requestMutations =
-        HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
+        HeaderMutations.EMPTY_MUTATIONS;
     HeaderMutations responseMutations =
         HeaderMutations.create(
             ImmutableList.of(

xds/src/test/java/io/grpc/xds/internal/extauthz/CheckResponseHandlerTest.java

@@ -71,7 +71,7 @@ public void handleResponse_ok() {
     AuthzResponse authzResponse = responseHandler.handleResponse(checkResponse);
     assertThat(authzResponse.decision()).isEqualTo(Decision.ALLOW);
     assertThat(authzResponse.requestHeaderMutations())
-        .isEqualTo(HeaderMutations.create(ImmutableList.of(), ImmutableList.of()));
+        .isEqualTo(HeaderMutations.EMPTY_MUTATIONS);
   }
 
   @Test
@@ -192,7 +192,7 @@ public void handleResponse_deniedResponseWithStatusOverride() {
     assertThat(status.getDescription())
         .isEqualTo("RPC denied by external authorization server");
     HeaderMutations expectedMutations =
-        HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
+        HeaderMutations.EMPTY_MUTATIONS;
     assertThat(authzResponse.responseHeaderMutations()).isEqualTo(expectedMutations);
   }
 

xds/src/test/java/io/grpc/xds/internal/extauthz/ExtAuthzClientCallTest.java

@@ -207,8 +207,7 @@ public void cancel_whileCheckInFlight_responseAfterCancelDoesNotForwardToBackend
       return null;
     }).when(authzService).check(any(CheckRequest.class), any());
 
-    HeaderMutations emptyMutations = HeaderMutations.create(
-        ImmutableList.of(), ImmutableList.of());
+    HeaderMutations emptyMutations = HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse authzResponse =
         AuthzResponse.allow(emptyMutations)
             .setResponseHeaderMutations(emptyMutations)
@@ -309,8 +308,7 @@ public void authzCheckCompletes_contextIsCancelled()
         .check(any(CheckRequest.class),
             ArgumentMatchers.any());
 
-    HeaderMutations emptyMutations = HeaderMutations.create(
-        ImmutableList.of(), ImmutableList.of());
+    HeaderMutations emptyMutations = HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse authzResponse =
         AuthzResponse.allow(emptyMutations)
             .setResponseHeaderMutations(emptyMutations)
@@ -375,8 +373,7 @@ public void authzCheckCompletes_allow_forwardsCall() throws Exception {
       return null;
     }).when(authzService).check(any(CheckRequest.class), ArgumentMatchers.any());
 
-    HeaderMutations emptyMutations = HeaderMutations.create(
-        ImmutableList.of(), ImmutableList.of());
+    HeaderMutations emptyMutations = HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse authzResponse = AuthzResponse.allow(emptyMutations)
         .setResponseHeaderMutations(emptyMutations)
         .build();
@@ -411,8 +408,7 @@ public void authzCheckCompletes_allowWithMutations_wrapsCall() throws Exception
             HeaderValue.create("foo", "bar"),
             HeaderValueOption.HeaderAppendAction.APPEND_IF_EXISTS_OR_ADD)),
         ImmutableList.of());
-    HeaderMutations responseMutations = HeaderMutations.create(
-        ImmutableList.of(), ImmutableList.of());
+    HeaderMutations responseMutations = HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse authzResponse = AuthzResponse.allow(requestMutations)
         .setResponseHeaderMutations(responseMutations)
         .build();
@@ -443,8 +439,7 @@ public void authzCheckCompletes_deny_failsCall() throws Exception {
       return null;
     }).when(authzService).check(any(CheckRequest.class), ArgumentMatchers.any());
 
-    HeaderMutations emptyMutations = HeaderMutations.create(
-        ImmutableList.of(), ImmutableList.of());
+    HeaderMutations emptyMutations = HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse authzResponse = AuthzResponse.deny(Status.PERMISSION_DENIED)
         .setResponseHeaderMutations(emptyMutations)
         .build();
@@ -613,8 +608,7 @@ public void drain_executesOnCallExecutor() throws Exception {
       return null;
     }).when(authzService).check(any(CheckRequest.class), ArgumentMatchers.any());
 
-    HeaderMutations emptyMutations = HeaderMutations.create(
-        ImmutableList.of(), ImmutableList.of());
+    HeaderMutations emptyMutations = HeaderMutations.EMPTY_MUTATIONS;
     AuthzResponse authzResponse = AuthzResponse.allow(emptyMutations)
         .setResponseHeaderMutations(emptyMutations)
         .build();

xds/src/test/java/io/grpc/xds/internal/extauthz/MutatingClientCallTest.java

@@ -131,7 +131,7 @@ public void listenerCallbacks_areDelegatedCorrectly() {
   @Test
   public void emptyRequestMutations_nonEmptyResponseMutations_wrapsListener() {
     HeaderMutations emptyRequest =
-        HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
+        HeaderMutations.EMPTY_MUTATIONS;
     CapturingClientCall<Void, Void> delegate = new CapturingClientCall<>();
     MutatingClientCall<Void, Void> call = new MutatingClientCall<>(
         delegate, emptyRequest, responseMutations, mockHeaderMutator);
@@ -159,7 +159,7 @@ public void emptyRequestMutations_nonEmptyResponseMutations_wrapsListener() {
   @Test
   public void start_emptyResponseMutations_headersPassThroughUnmodified() {
     HeaderMutations emptyResponseMutations =
-        HeaderMutations.create(ImmutableList.of(), ImmutableList.of());
+        HeaderMutations.EMPTY_MUTATIONS;
     CapturingClientCall<Void, Void> delegate = new CapturingClientCall<>();
     MutatingClientCall<Void, Void> call = new MutatingClientCall<>(
         delegate, requestMutations, emptyResponseMutations, mockHeaderMutator);

xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutatorTest.java

@@ -55,7 +55,7 @@ public class HeaderMutatorTest {
   private static final Metadata.Key<String> OVERWRITE_IF_EXISTS_ABSENT_KEY =
       Metadata.Key.of("overwrite-if-exists-absent-key", Metadata.ASCII_STRING_MARSHALLER);
 
-  private final HeaderMutator headerMutator = HeaderMutator.create();
+  private final HeaderMutator headerMutator = HeaderMutator.HEADER_MUTATOR;
 
   private static final TestLogHandler logHandler = new TestLogHandler();
   private static final Logger logger = Logger.getLogger(HeaderMutator.class.getName());