xds: Add header mutations library by sauravzg · Pull Request #12494 · grpc/grpc-java

sauravzg · 2025-11-11T08:51:30Z

This PR sits on top of #12690, so only the last commit + any fixups need to be reviewed.

This commit introduces a library for handling header mutations as specified by the xDS protocol. This library provides the core functionality for modifying request and response headers based on a set of rules.

The main components of this library are:

HeaderMutator: Applies header mutations to Metadata objects.
HeaderMutationFilter: Filters header mutations based on a set of configurable rules, such as disallowing mutations of system headers.
HeaderMutations: A value class that represents the set of mutations to be applied to request and response headers.
HeaderMutationDisallowedException: An exception that is thrown when a disallowed header mutation is attempted.

This commit also includes comprehensive unit tests for the new library.

sauravzg · 2026-03-12T13:49:24Z

@kannanjgithub PTAL, if the implementation mutation meets the spec for ext proc. You need to TAL at the last two commits only.
I'll eventually move it down the PR chain later to ensure that the dependencies for ext_proc are merged earlier.

kannanjgithub

Sending comments for headermutations.

kannanjgithub

Reviewed rest of the PR.

sauravzg · 2026-03-15T21:00:41Z

@kannanjgithub I've resolved or clarified the comments as applicable. PTAL

sauravzg · 2026-03-16T11:08:50Z

@kannanjgithub Addressed the comments. PTAL.

sauravzg · 2026-03-16T11:43:02Z

/gemini review

sauravzg · 2026-04-01T04:48:18Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces the core infrastructure for xDS external authorization (ext_authz), including parsers for ExtAuthz and GrpcService configurations, a CachedChannelManager for authorization service connections, and a comprehensive header mutation system. Feedback focuses on HeaderMutator.java, specifically identifying a bug where the keepEmptyValue logic does not correctly align with Envoy's behavior for the APPEND action, which could lead to incorrect header persistence. Additionally, it is recommended to refactor duplicated Metadata.Key creation logic into a helper method to improve maintainability.

sauravzg · 2026-04-22T12:22:38Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a CachedChannelManager to manage gRPC channels based on service configurations, alongside a new framework for processing xDS header mutations. The mutation framework includes components for filtering, applying, and validating header changes. Review feedback highlights the need to align the mutation order with the xDS specification, correct the logic for header allowance expressions, optimize performance by pre-calculating metadata keys, and improve error reporting with more descriptive exception messages.

sauravzg · 2026-04-22T13:17:08Z

/gemini review - Please don't repeat previously addressed comments.

gemini-code-assist

Code Review

This pull request introduces a CachedChannelManager for managing gRPC channels and a framework for xDS header mutations, including filtering and application logic. Key feedback includes aligning the order of header operations in HeaderMutator with Envoy by processing removals before additions and caching Metadata.Key objects to optimize performance. Furthermore, the filtering logic in HeaderMutationFilter needs adjustment to ensure disallowed headers properly trigger exceptions when strict validation is active.

This commit introduces a library for handling header mutations as specified by the xDS protocol. This library provides the core functionality for modifying request and response headers based on a set of rules. The main components of this library are: - `HeaderMutator`: Applies header mutations to `Metadata` objects. - `HeaderMutationFilter`: Filters header mutations based on a set of configurable rules, such as disallowing mutations of system headers. - `HeaderMutations`: A value class that represents the set of mutations to be applied to request and response headers. - `HeaderMutationDisallowedException`: An exception that is thrown when a disallowed header mutation is attempted. This commit also includes comprehensive unit tests for the new library.

…spec

… headermutations libraries

https://github.com/grpc/proposal/pull/481/changes#r3022089452

kannanjgithub

Some unit tests are missing.

kannanjgithub · 2026-04-23T10:52:18Z

+    assertThat(filtered.headers()).containsExactly(header("x-allowed-key", "value"));
+  }
+
+


nit: Remove extra empty line.

kannanjgithub · 2026-04-23T10:53:42Z

+    assertThat(headers.get(keepEmptyKey)).isEqualTo("");
+    assertThat(headers.containsKey(keepEmptyOverwriteKey)).isTrue();
+    assertThat(headers.get(keepEmptyOverwriteKey)).isEqualTo("");
+


nit: Remove unnecessary blank line.

kannanjgithub · 2026-04-23T11:01:41Z

+  private void updateHeader(final HeaderValueOption option, Metadata mutableHeaders) {
+    HeaderValue header = option.header();
+    HeaderAppendAction action = option.appendAction();
+    boolean keepEmptyValue = option.keepEmptyValue();


The commit b9caa47 refactored how keepEmptyValue is handled in HeaderMutator.java, moving the check from a post-action cleanup (checkAndRemoveEmpty) to a pre-action filter. The following
cases are not covered by the existing tests:

Binary Headers: No tests exist for binary headers with empty values (empty ByteString) when keepEmptyValue is either true or false.

Specific Actions: The OVERWRITE_IF_EXISTS and ADD_IF_ABSENT actions are not tested with empty values and keepEmptyValue=false.

Behavior Change Verification: The new implementation skips the updateHeader call entirely if the value is empty and keepEmptyValue is false. This means existing values are preserved during an OVERWRITE action, whereas the old implementation would have removed them. This deliberate change in behavior needs explicit verification.

kannanjgithub · 2026-04-23T11:06:57Z

   * A generic helper to filter a collection based on a predicate.
   */
  private <T> ImmutableList<T> filterCollection(Collection<T> items,
      Predicate<T> isIgnoredPredicate, Predicate<T> isAllowedPredicate)


The commit 9640f87 updated HeaderMutationFilter.java to match Envoy's implementation, where headers that are either "ignored" (system headers like :path or grpc-*) or "disallowed" (by custom
rules) trigger a HeaderMutationDisallowedException when disallowIsError is enabled. Previously, ignored headers were skipped silently even if disallowIsError was true.

The following scenarios are missing coverage:

DisallowIsError = False: No test verifies that disallowed/ignored headers are still silently skipped when disallowIsError is false.

Rule-based Disallowance: No tests verify that headers disallowed by custom regex (disallowExpression) or disallowAll trigger the exception when disallowIsError is true.

Rule-based Allowance Override: No test verifies that allowExpression correctly overrides disallowAll even when disallowIsError is true.

System Header Rejection: While some tests were added in the commit for system headers, they don't cover the full range of HeaderValueValidationUtils logic (e.g., uppercase keys).

This was referenced Nov 11, 2025

feat(xds): Implement response handling for external authorization #12495

Open

feat(xds): Add ExtAuthzClientInterceptor and related components #12496

Open

Feat/server interceptor #12497

Open

feat(xds): Add ExternalAuthorizationFilter #12498

Open

sauravzg force-pushed the feat/header-mutations branch 5 times, most recently from 328631a to 86d68ac Compare November 17, 2025 10:36

kannanjgithub reviewed Mar 9, 2026

View reviewed changes

Comment thread xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java Outdated

sauravzg force-pushed the feat/header-mutations branch from 86d68ac to 759130e Compare March 12, 2026 13:46

sauravzg force-pushed the feat/header-mutations branch from 759130e to 4bfd5e6 Compare March 12, 2026 18:14

sauravzg changed the title ~~feat(xds): Add header mutations library~~ xds: Add header mutations library Mar 12, 2026

sauravzg force-pushed the feat/header-mutations branch from 4bfd5e6 to 6961989 Compare March 12, 2026 19:11

kannanjgithub requested changes Mar 13, 2026

View reviewed changes

sauravzg force-pushed the feat/header-mutations branch 2 times, most recently from 90e5491 to efb725f Compare March 15, 2026 21:44

kannanjgithub approved these changes Mar 16, 2026

View reviewed changes

kannanjgithub requested a review from AgraVator March 17, 2026 06:04

sauravzg force-pushed the feat/header-mutations branch 2 times, most recently from 399e233 to 677b251 Compare March 17, 2026 08:40

AgraVator reviewed Mar 23, 2026

View reviewed changes

Comment thread xds/src/main/java/io/grpc/xds/GrpcServiceConfigParser.java Outdated

Comment thread xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutationFilter.java Outdated

sauravzg force-pushed the feat/header-mutations branch from 677b251 to 4ca257e Compare March 23, 2026 18:03

kannanjgithub requested a review from Copilot March 24, 2026 17:20

Copilot started reviewing on behalf of kannanjgithub March 24, 2026 17:21 View session

kannanjgithub reviewed Mar 31, 2026

View reviewed changes

Comment thread xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java Outdated

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Mar 31, 2026

Fixup grpc#12494: Fix documentation

0a3efdf

sauravzg force-pushed the feat/header-mutations branch from d999d4b to 0a3efdf Compare March 31, 2026 13:03

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Mar 31, 2026

Fixup grpc#12494: Fix documentation

5261286

sauravzg force-pushed the feat/header-mutations branch from 0a3efdf to 5261286 Compare March 31, 2026 13:07

gemini-code-assist Bot reviewed Apr 1, 2026

View reviewed changes

Comment thread xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java Outdated

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

Comment thread xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java Outdated

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Apr 1, 2026

Fixup grpc#12494: Fix documentation

9ec272a

sauravzg force-pushed the feat/header-mutations branch from 5261286 to 9ec272a Compare April 1, 2026 13:31

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Apr 2, 2026

Fixup grpc#12494: Fix documentation

951b761

sauravzg force-pushed the feat/header-mutations branch from 9ec272a to 951b761 Compare April 2, 2026 11:58

gemini-code-assist Bot reviewed Apr 22, 2026

View reviewed changes

sauravzg force-pushed the feat/header-mutations branch from 8d6cd46 to 312f166 Compare April 22, 2026 13:16

gemini-code-assist Bot reviewed Apr 22, 2026

View reviewed changes

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

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

Comment thread xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutationFilter.java

sauravzg force-pushed the feat/header-mutations branch from c7777c2 to 46c951d Compare April 22, 2026 14:10

sauravzg added 11 commits April 22, 2026 14:18

Fixup: 12494 address comments and bring back up to updated ext authz …

24b736e

…spec

Fixup 12494: Fixes for logging and additional comment

65eb24f

Fixup 12494: Remove Authz specific abstractions away from the generic…

6653d2e

… headermutations libraries

Fixup 12494: Rename variable

b96db79

Fixup 12494: Address copilot comments

d866186

Fixup 12494: Improve test coverage for headermutations

c17c86a

Fixup grpc#12494: Fix documentation

e046a49

Make keepEmptyvalue consistent with envoy impl

b9caa47

https://github.com/grpc/proposal/pull/481/changes#r3022089452

Handle gemini review comments.

d2757fb

Change HeaderMutationFilter to match envoy's implementation

9640f87

sauravzg force-pushed the feat/header-mutations branch from 46c951d to 9640f87 Compare April 22, 2026 14:27

kannanjgithub requested changes Apr 23, 2026

View reviewed changes

		assertThat(filtered.headers()).containsExactly(header("x-allowed-key", "value"));
		}

Conversation

sauravzg commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sauravzg commented Mar 12, 2026

Uh oh!

kannanjgithub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kannanjgithub left a comment

Choose a reason for hiding this comment

Uh oh!

sauravzg commented Mar 15, 2026

Uh oh!

sauravzg commented Mar 16, 2026

Uh oh!

sauravzg commented Mar 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sauravzg commented Apr 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sauravzg commented Apr 22, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sauravzg commented Apr 22, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kannanjgithub left a comment

Choose a reason for hiding this comment

Uh oh!

kannanjgithub Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

kannanjgithub Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

kannanjgithub Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

kannanjgithub Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sauravzg commented Nov 11, 2025 •

edited

Loading