diff --git a/xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java b/xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java index 93a2230b9a4..b3466e2f5ea 100644 --- a/xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java +++ b/xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java @@ -291,7 +291,6 @@ private static class DataPlaneClientCall new ConcurrentLinkedQueue<>(); @Nullable private volatile DataPlaneListener 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); diff --git a/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzCallbackObserver.java b/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzCallbackObserver.java index 35110dae809..be1e0d564b3 100644 --- a/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzCallbackObserver.java +++ b/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzCallbackObserver.java @@ -48,8 +48,6 @@ final class AuthzCallbackObserver implements StreamObserver( 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; } diff --git a/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzResponse.java b/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzResponse.java index c432fff1d09..d945fbf6903 100644 --- a/xds/src/main/java/io/grpc/xds/internal/extauthz/AuthzResponse.java +++ b/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); } diff --git a/xds/src/main/java/io/grpc/xds/internal/extauthz/CheckResponseHandler.java b/xds/src/main/java/io/grpc/xds/internal/extauthz/CheckResponseHandler.java index 6b2f89a1312..1468a2d1dcb 100644 --- a/xds/src/main/java/io/grpc/xds/internal/extauthz/CheckResponseHandler.java +++ b/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()); diff --git a/xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutations.java b/xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutations.java index a456413c899..152e778a812 100644 --- a/xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutations.java +++ b/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 headers, ImmutableList headersToRemove) { return new AutoValue_HeaderMutations(headers, headersToRemove); diff --git a/xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java b/xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java index ef13590e2c0..7c30513813c 100644 --- a/xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java +++ b/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. diff --git a/xds/src/test/java/io/grpc/xds/internal/extauthz/AuthzResponseTest.java b/xds/src/test/java/io/grpc/xds/internal/extauthz/AuthzResponseTest.java index 6e2f089c078..22797f91a62 100644 --- a/xds/src/test/java/io/grpc/xds/internal/extauthz/AuthzResponseTest.java +++ b/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( diff --git a/xds/src/test/java/io/grpc/xds/internal/extauthz/CheckResponseHandlerTest.java b/xds/src/test/java/io/grpc/xds/internal/extauthz/CheckResponseHandlerTest.java index 9385dae5616..eaee889f7b7 100644 --- a/xds/src/test/java/io/grpc/xds/internal/extauthz/CheckResponseHandlerTest.java +++ b/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); } 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 11c04a182fc..18e77d7bff1 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 @@ -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(); diff --git a/xds/src/test/java/io/grpc/xds/internal/extauthz/MutatingClientCallTest.java b/xds/src/test/java/io/grpc/xds/internal/extauthz/MutatingClientCallTest.java index 45cfbb5bfae..a2a95b4c5f3 100644 --- a/xds/src/test/java/io/grpc/xds/internal/extauthz/MutatingClientCallTest.java +++ b/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 delegate = new CapturingClientCall<>(); MutatingClientCall 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 delegate = new CapturingClientCall<>(); MutatingClientCall call = new MutatingClientCall<>( delegate, requestMutations, emptyResponseMutations, mockHeaderMutator); diff --git a/xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutatorTest.java b/xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutatorTest.java index ff426931700..bf7c6d27d99 100644 --- a/xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutatorTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutatorTest.java @@ -55,7 +55,7 @@ public class HeaderMutatorTest { private static final Metadata.Key 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());