Skip to content

Add xDS support for Athenz filters - #6853

Merged
jrhee17 merged 8 commits into
line:mainfrom
jrhee17:feat/athenz-filter
Jul 22, 2026
Merged

Add xDS support for Athenz filters#6853
jrhee17 merged 8 commits into
line:mainfrom
jrhee17:feat/athenz-filter

Conversation

@jrhee17

@jrhee17 jrhee17 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Motivation

Add Athenz xDS filter support for access token injection (outbound) and access token constraint authorization (inbound). This is added in the upstream repo rather than downstream since Athenz test-related infrastructure (containers, ZMS/ZTS setup, key provisioning) is already well established here.

Modifications

  • Defined a separate xds-athenz module since jdk version requirements are different for the Athenz integration
  • Define proto messages for Athenz access token metadata (athenz_access_token.proto):
    • AccessTokenTarget (outbound): domain/roles to fetch a token for
    • AccessTokenConstraint (inbound): domain to evaluate with optional action/resource mapping rules
    • WellKnownEndpointAttribute, EndpointAttributeMatch, AssertionMappingRule, AssertionMapping for flexible per-request action/resource resolution
    • All fields annotated with (armeria.xds.supported.field) for strict validation
  • Define separate filter config messages in armeria.xds.athenz package (athenz_filter_config.proto):
    • AccessTokenTargetConfig for outbound filter
    • AccessTokenConstraintConfig for inbound filter
  • Add AccessTokenTargetFilterFactory — outbound xDS HTTP filter that injects Athenz access tokens via AthenzTokenClient, supporting all four filter paths (httpPreprocessor, rpcPreprocessor, httpDecorator, rpcDecorator)
  • Add AccessTokenConstraintFilterFactory — inbound xDS HTTP filter that authorizes requests by evaluating Athenz access token constraints with configurable assertion mapping rules
  • Add MappingTemplate for parsing and resolving ${host}, ${method}, ${path}, ${match.<name>.<index>} placeholders in mapping rule templates
  • Move XdsStringMatcher to a shared location for reuse by both xDS routing and filter condition matching
  • Remove unsupportedPackage from SupportedFieldValidator; only skip google.protobuf messages so that Athenz-related fields can also be validated

Result

  • Outbound: requests routed through xDS clusters automatically acquire Athenz access tokens (HTTP and RPC, downstream and upstream)
  • Inbound: requests are authorized against Athenz policies using configurable action/resource mapping
  • Supported field validation is stricter — third-party proto messages (except google.protobuf) must have annotations

@jrhee17 jrhee17 added this to the 1.41.0 milestone Jul 14, 2026
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5f37fd43-37d3-4b93-a9b0-7b923f4c5b8d

📥 Commits

Reviewing files that changed from the base of the PR and between 17e030b and 012fdc7.

📒 Files selected for processing (1)
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java
💤 Files with no reviewable changes (1)
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java

📝 Walkthrough

Walkthrough

Adds Athenz protobuf contracts and a new xds-athenz module with outbound token injection and inbound authorization filters. Registers the filters through xDS discovery, reuses a public string matcher, updates validation, and adds Docker-gated HTTP, RPC, and authorization integration tests.

Changes

Athenz contracts and matcher support

Layer / File(s) Summary
Athenz filter contracts and validation
xds-api/src/main/proto/armeria/xds/athenz/*, xds-api/src/main/java/com/linecorp/armeria/xds/api/SupportedFieldValidator.java
Defines outbound and inbound Athenz filter configuration, assertion mappings, templates, endpoint attributes, and updated protobuf validation behavior.
Shared xDS string matching
xds/src/main/java/com/linecorp/armeria/xds/internal/XdsStringMatcher.java, xds/src/main/java/com/linecorp/armeria/xds/{RouteEntryMatcher,SanMatcher,CertificateValidationContextSnapshot}.java
Makes XdsStringMatcher public and uses it for route, SAN, and certificate matching paths.

Athenz filter factories

Layer / File(s) Summary
Reactive token filters
xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/*
Adds ZTS-backed outbound token injection and inbound constraint authorization, including mapping-template and capture evaluation.

Module discovery and build wiring

Layer / File(s) Summary
Module and service registration
settings.gradle, xds-athenz/build.gradle, xds-athenz/src/main/{java,resources}/...
Adds the :xds-athenz project, API dependencies, package metadata, type registry registration, and extension factory providers.
Integration resource preparation
it/xds-client/build.gradle
Adds Athenz test dependencies, copies Docker test resources, and wires the copy task into test compilation and resource-processing tasks.

Docker-backed integration coverage

Layer / File(s) Summary
HTTP and RPC token injection tests
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessToken{Filter,RpcFilter}Test.java
Verifies Bearer-token injection, replacement of existing authorization headers, upstream HTTP filters, and RPC preprocessing.
Constraint authorization tests
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java
Exercises literal, default, template, and no-prefix mappings with unauthenticated and authorized requests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • line/armeria#6607: Introduces or refactors the AthenzAuthorizer API used by the new constraint filter.
  • line/armeria#6691: Provides the AthenzTokenClient API used for outbound token acquisition.
  • line/armeria#6838: Introduces the xDS extension and type-registry discovery APIs used by this module.

Suggested labels: improvement

Suggested reviewers: minwoox, trustin, ikhoon

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding xDS support for Athenz filters.
Description check ✅ Passed The description is clearly related to the changeset and matches the added Athenz xDS filter support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
xds-api/src/main/proto/armeria/xds/athenz/athenz_filter_config.proto (1)

1-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Missing copyright header.

As per path instructions, all modified/added source files must include the LY Corporation copyright header. Please add the required header to this proto file.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@xds-api/src/main/proto/armeria/xds/athenz/athenz_filter_config.proto` around
lines 1 - 19, Add the repository-standard LY Corporation copyright header at the
beginning of the proto file, before the syntax declaration, while leaving the
package, imports, and message definitions unchanged.

Source: Path instructions

🧹 Nitpick comments (4)
it/xds-client/build.gradle (1)

18-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider testFixtures instead of copying .java sources across modules.

Copying AthenzDocker.java/AthenzExtension.java from :athenz at build time works, but duplicating source files via Copy is more fragile than consuming a java-test-fixtures artifact from :athenz (auto-refactors on rename, IDE navigation stays correct, avoids drift between copies). Low priority if this mirrors an existing convention elsewhere in the build.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@it/xds-client/build.gradle` around lines 18 - 30, Replace the
copyTestResources-based sharing of AthenzDocker.java and AthenzExtension.java
with consumption of the :athenz java-test-fixtures artifact. Configure :athenz
to publish its test fixtures if needed, then make the it/xds-client test source
set depend on those fixtures and remove the copy task and its task dependencies.
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java (1)

131-283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate ZTS/TLS YAML-scaffolding logic across both Athenz IT tests. Both test classes independently rebuild ATHENZ_RESOURCES-relative cert/key paths and near-identical zts-cluster TLS Bootstrap YAML fragments; the shared root cause is a missing common test helper for Athenz xDS bootstrap scaffolding.

  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java#L131-L283: extract the cert-path resolution (lines 135-139) and the zts-cluster TLS cluster YAML block (lines 248-273) into a shared helper/base class reusable by both tests.
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java#L101-L183: replace the equivalent cert-path resolution (lines 103-107) and zts-cluster TLS block (lines 152-176) with the same shared helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java`
around lines 131 - 283, Extract the shared Athenz certificate/key path
resolution and zts-cluster TLS Bootstrap YAML construction from bootstrapYaml in
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java#L131-L283
into a reusable helper or base class, then update both bootstrapYaml
implementations to use it. Apply the same replacement in
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java#L101-L183,
preserving each test’s existing listener and filter-specific configuration.
xds/src/main/java/com/linecorp/armeria/xds/internal/XdsStringMatcher.java (1)

41-43: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a null check to the now-public constructor.

XdsStringMatcher is now public and used across module boundaries; the constructor doesn't validate stringMatcher, so a null argument would surface as an unclear NPE deep in the switch rather than a clear error at the boundary.

♻️ Suggested fix
 public XdsStringMatcher(StringMatcher stringMatcher) {
+    requireNonNull(stringMatcher, "stringMatcher");
     ignoreCase = stringMatcher.getIgnoreCase();

As per path instructions, "do explicit null checks for user-facing public method parameters (use Objects.requireNonNull)".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@xds/src/main/java/com/linecorp/armeria/xds/internal/XdsStringMatcher.java`
around lines 41 - 43, Add an explicit Objects.requireNonNull validation for the
stringMatcher parameter in the public XdsStringMatcher constructor before
accessing it, producing a clear boundary error while preserving existing
initialization behavior.

Source: Path instructions

xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate create() boilerplate across both filter factories. Both create() overrides are byte-for-byte identical except for NAME; this pattern likely belongs as a default method on the shared HttpFilterFactory interface for stream-only factories.

  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java#L62-67: extract to a shared default/helper instead of repeating the UnsupportedOperationException throw.
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactory.java#L79-84: same extraction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java`
at line 1, Move the identical stream-only create() implementation from
AccessTokenTargetFilterFactory and AccessTokenConstraintFilterFactory into a
shared default/helper on HttpFilterFactory that throws
UnsupportedOperationException. Remove both duplicate overrides while preserving
each factory’s distinct NAME and other behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java`:
- Around line 99-106: Move the newAssertion(String action, String resource)
helper out of the anonymous AthenzExtension class into the enclosing test class,
preserving its private static signature and existing assertion setup.

In `@xds-api/src/main/proto/armeria/xds/athenz/athenz_access_token.proto`:
- Line 1: The documented syntax-version contract is not enforced by the consumer
factories. In AccessTokenTargetFilterFactory, validate target.getSyntaxVersion()
against the implemented maximum before constructing the token client; apply the
equivalent validation to constraint.getSyntaxVersion() in
AccessTokenConstraintFilterFactory before constructing the authorizer, rejecting
unsupported future versions while preserving supported-version behavior.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java`:
- Line 1: Add `@UnstableApi` to both public final classes
AccessTokenTargetFilterFactory and AccessTokenConstraintFilterFactory, including
the necessary import, while leaving their existing behavior unchanged.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactoryProvider.java`:
- Around line 22-25: Add the `@UnstableApi` annotation to the public
AccessTokenTargetFilterFactoryProvider class, including the required import if
absent, while preserving its existing XdsExtensionFactoryProvider
implementation.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/MappingTemplate.java`:
- Around line 88-97: Update the match placeholder parsing in MappingTemplate to
catch NumberFormatException from Integer.parseInt and rethrow it as an
IllegalArgumentException that includes the invalid index value and preserves the
original exception as the cause; keep valid indices and existing
unknown-placeholder handling unchanged.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/XdsZtsBaseClient.java`:
- Around line 39-42: Update XdsZtsBaseClient.webClient(Consumer<? super
WebClientBuilder>) so it does not silently ignore the configurer; throw
UnsupportedOperationException when customization is unsupported in the
xDS-driven flow, matching the explicit-failure behavior used by the related
filter factories.

---

Outside diff comments:
In `@xds-api/src/main/proto/armeria/xds/athenz/athenz_filter_config.proto`:
- Around line 1-19: Add the repository-standard LY Corporation copyright header
at the beginning of the proto file, before the syntax declaration, while leaving
the package, imports, and message definitions unchanged.

---

Nitpick comments:
In `@it/xds-client/build.gradle`:
- Around line 18-30: Replace the copyTestResources-based sharing of
AthenzDocker.java and AthenzExtension.java with consumption of the :athenz
java-test-fixtures artifact. Configure :athenz to publish its test fixtures if
needed, then make the it/xds-client test source set depend on those fixtures and
remove the copy task and its task dependencies.

In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java`:
- Around line 131-283: Extract the shared Athenz certificate/key path resolution
and zts-cluster TLS Bootstrap YAML construction from bootstrapYaml in
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java#L131-L283
into a reusable helper or base class, then update both bootstrapYaml
implementations to use it. Apply the same replacement in
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java#L101-L183,
preserving each test’s existing listener and filter-specific configuration.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java`:
- Line 1: Move the identical stream-only create() implementation from
AccessTokenTargetFilterFactory and AccessTokenConstraintFilterFactory into a
shared default/helper on HttpFilterFactory that throws
UnsupportedOperationException. Remove both duplicate overrides while preserving
each factory’s distinct NAME and other behavior.

In `@xds/src/main/java/com/linecorp/armeria/xds/internal/XdsStringMatcher.java`:
- Around line 41-43: Add an explicit Objects.requireNonNull validation for the
stringMatcher parameter in the public XdsStringMatcher constructor before
accessing it, producing a clear boundary error while preserving existing
initialization behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dcaedd81-59f9-4cd3-93b6-a047bda75d2c

📥 Commits

Reviewing files that changed from the base of the PR and between a953182 and 574c6fe.

📒 Files selected for processing (21)
  • it/xds-client/build.gradle
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java
  • settings.gradle
  • xds-api/src/main/proto/armeria/xds/athenz/athenz_access_token.proto
  • xds-api/src/main/proto/armeria/xds/athenz/athenz_filter_config.proto
  • xds-athenz/build.gradle
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactory.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactoryProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactoryProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AthenzTypeRegistryPackageProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/MappingTemplate.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/XdsZtsBaseClient.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/package-info.java
  • xds-athenz/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsExtensionFactoryProvider
  • xds-athenz/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProvider
  • xds/src/main/java/com/linecorp/armeria/xds/CertificateValidationContextSnapshot.java
  • xds/src/main/java/com/linecorp/armeria/xds/RouteEntryMatcher.java
  • xds/src/main/java/com/linecorp/armeria/xds/SanMatcher.java
  • xds/src/main/java/com/linecorp/armeria/xds/internal/XdsStringMatcher.java

Comment thread xds-api/src/main/proto/armeria/xds/athenz/athenz_access_token.proto

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/XdsZtsBaseClient.java (1)

39-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Past concern resolved; consider adding an explanatory message to the exception.

The previously-flagged silent-ignore issue on webClient(Consumer<...>) is now fixed by throwing UnsupportedOperationException. Consider adding a message so callers immediately understand why customization isn't supported for this internal ZTS client.

♻️ Suggested tweak
     public WebClient webClient(Consumer<? super WebClientBuilder> configurer) {
-        throw new UnsupportedOperationException();
+        throw new UnsupportedOperationException(
+                "Customizing the WebClientBuilder is not supported for XdsZtsBaseClient");
     }

As per path instructions, "throw exceptions with detailed messages including the offending value and expected constraints."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/XdsZtsBaseClient.java`
around lines 39 - 42, Update the webClient method in XdsZtsBaseClient to throw
UnsupportedOperationException with a clear message explaining that custom
WebClientBuilder configuration is not supported for this internal ZTS client.

Source: Path instructions

it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java (1)

197-285: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Extract shared Envoy bootstrap-YAML boilerplate for Athenz integration tests. All three test methods rebuild the same Athenz cert/key/CA path resolution and identical static echo-cluster/zts-cluster (with TLS) blocks, differing only in the listener's HTTP-filter wiring; the shared root cause is the lack of a common test helper for this boilerplate.

  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java#L197-L285: extract the cert/key/CA-path setup and static-cluster YAML block shared with bootstrapYaml() into a common helper/constant, keeping only the listener filter-chain shape method-specific.
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java#L114-L196: same extraction applies to this twin method.
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenRpcFilterTest.java#L85-L167: reuse the same shared helper instead of re-declaring the cert/cluster boilerplate in this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java`
around lines 197 - 285, Extract the shared Athenz certificate/key/CA path setup
and static echo-cluster/zts-cluster TLS YAML from upstreamBootstrapYaml(),
bootstrapYaml(), and the RPC test bootstrap method into one reusable test helper
or constant. Update all three affected
sites—AthenzAccessTokenFilterTest.java:197-285,
AthenzAccessTokenFilterTest.java:114-196, and
AthenzAccessTokenRpcFilterTest.java:85-167—to reuse it while retaining each
method’s listener and HTTP-filter-specific wiring.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java`:
- Around line 197-285: Extract the shared Athenz certificate/key/CA path setup
and static echo-cluster/zts-cluster TLS YAML from upstreamBootstrapYaml(),
bootstrapYaml(), and the RPC test bootstrap method into one reusable test helper
or constant. Update all three affected
sites—AthenzAccessTokenFilterTest.java:197-285,
AthenzAccessTokenFilterTest.java:114-196, and
AthenzAccessTokenRpcFilterTest.java:85-167—to reuse it while retaining each
method’s listener and HTTP-filter-specific wiring.

In
`@xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/XdsZtsBaseClient.java`:
- Around line 39-42: Update the webClient method in XdsZtsBaseClient to throw
UnsupportedOperationException with a clear message explaining that custom
WebClientBuilder configuration is not supported for this internal ZTS client.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f4f2a0e3-a217-443f-a3e6-53e75e67fd49

📥 Commits

Reviewing files that changed from the base of the PR and between 574c6fe and 34abb8d.

📒 Files selected for processing (14)
  • it/xds-client/build.gradle
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenFilterTest.java
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenRpcFilterTest.java
  • it/xds-client/src/test/thrift/echo.thrift
  • xds-api/src/main/java/com/linecorp/armeria/xds/api/SupportedFieldValidator.java
  • xds-api/src/main/proto/armeria/xds/athenz/athenz_access_token.proto
  • xds-api/src/main/proto/armeria/xds/athenz/athenz_filter_config.proto
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactory.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactoryProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactoryProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AthenzTypeRegistryPackageProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/XdsZtsBaseClient.java
🚧 Files skipped from review as they are similar to previous changes (8)
  • xds-api/src/main/proto/armeria/xds/athenz/athenz_filter_config.proto
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactoryProvider.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactoryProvider.java
  • it/xds-client/build.gradle
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenTargetFilterFactory.java
  • xds-api/src/main/proto/armeria/xds/athenz/athenz_access_token.proto
  • it/xds-client/src/test/java/com/linecorp/armeria/xds/it/athenz/AthenzAccessTokenConstraintFilterTest.java
  • xds-athenz/src/main/java/com/linecorp/armeria/xds/filter/athenz/AccessTokenConstraintFilterFactory.java

@jrhee17
jrhee17 marked this pull request as ready for review July 15, 2026 01:25
@jrhee17
jrhee17 requested review from ikhoon and minwoox as code owners July 15, 2026 01:25
@mergify

mergify Bot commented Jul 15, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.28112% with 74 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.12%. Comparing base (8150425) to head (17e030b).
⚠️ Report is 537 commits behind head on main.

Files with missing lines Patch % Lines
...ter/athenz/AccessTokenConstraintFilterFactory.java 71.66% 19 Missing and 15 partials ⚠️
...orp/armeria/xds/filter/athenz/MappingTemplate.java 59.64% 15 Missing and 8 partials ⚠️
.../filter/athenz/AccessTokenTargetFilterFactory.java 70.83% 13 Missing and 1 partial ⚠️
...rp/armeria/xds/filter/athenz/XdsZtsBaseClient.java 66.66% 2 Missing ⚠️
...inecorp/armeria/xds/internal/XdsStringMatcher.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6853      +/-   ##
============================================
+ Coverage     74.46%   75.12%   +0.65%     
- Complexity    22234    25396    +3162     
============================================
  Files          1963     2262     +299     
  Lines         82437    94335   +11898     
  Branches      10764    12336    +1572     
============================================
+ Hits          61385    70866    +9481     
- Misses        15918    17607    +1689     
- Partials       5134     5862     +728     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ikhoon ikhoon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍

CommonPools.blockingTaskExecutor().execute(() -> {
try {
final ZtsBaseClient ztsBaseClient =
new XdsZtsBaseClient(clusterSnapshot.preprocessor());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The abstraction in xDS is impressive.

@Override
public DecoratingHttpServiceFunction serviceDecorator() {
return (delegate, ctx, req) -> {
final AthenzTokenHeader tokenHeader = AthenzTokenHeader.ofAccessToken();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood only Authorization: Bearer <token> is supported in xDS-Athenz integration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been done for simplicity - if needed I think we can add a field at AccessTokenConstraintConfig/AccessTokenTargetConfig which denotes which headers to use

@Nullable
public String resolve(ServiceRequestContext ctx, Map<String, List<String>> captures) {
final List<String> groups = captures.get(name);
if (groups == null || index < 0 || index >= groups.size()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question) Would it make sense to reject a negative index in the constructor?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, done

}
final ImmutableList.Builder<String> groups = ImmutableList.builder();
for (int i = 0; i <= m.groupCount(); i++) {
if (m.group(i) == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted that capturing may fail if a regex contains an alternation such as regex: "(/a)|(/b)".

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

import com.linecorp.armeria.client.WebClientBuilder;
import com.linecorp.armeria.client.athenz.ZtsBaseClient;

final class XdsZtsBaseClient implements ZtsBaseClient {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay not overriding addTlsKeyPairListener?
It seems like the method is called from AccessTokenClient

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AccessTokenClient uses it to immediately retrieve an access token using when TLS is updated.
I don't think this is really necessarily to function correctly.

@jrhee17
jrhee17 merged commit 2975d98 into line:main Jul 22, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants