Add CentralDogmaConfigSource for direct xDS resource fetching from repository#1318
Open
jrhee17 wants to merge 3 commits into
Open
Add CentralDogmaConfigSource for direct xDS resource fetching from repository#1318jrhee17 wants to merge 3 commits into
CentralDogmaConfigSource for direct xDS resource fetching from repository#1318jrhee17 wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a Central Dogma-backed xDS config source with JSON, YAML, and FreeMarker resource support, bearer-token and mTLS integration coverage, xDS SPI registration, protobuf API definitions, and supporting Gradle module and dependency wiring. ChangesCentral Dogma xDS Config Source
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant XdsBootstrap
participant CentralDogmaSotwConfigSourceSubscriptionFactory
participant CentralDogmaClientStream
participant WatcherStream
participant DiscoveryResponse
XdsBootstrap->>CentralDogmaSotwConfigSourceSubscriptionFactory: create config-source subscription
CentralDogmaSotwConfigSourceSubscriptionFactory->>CentralDogmaClientStream: initialize authenticated client
XdsBootstrap->>WatcherStream: subscribe to resource
WatcherStream->>CentralDogmaClientStream: watch Central Dogma resource
WatcherStream->>DiscoveryResponse: emit watched JSON update
CentralDogmaSotwConfigSourceSubscriptionFactory->>XdsBootstrap: emit DiscoveryResponse
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@build.gradle`:
- Line 21: The Sonatype snapshots repository at line 21 is unrestricted,
allowing any artifact to resolve from snapshots, which increases supply-chain
risk. Modify the maven repository declaration to restrict it to only artifacts
from the com.linecorp.armeria group and snapshots using a repositoryContent
block. This ensures only the required armeria snapshots use this repository
while preventing unrelated modules from resolving arbitrary snapshot artifacts.
In
`@client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.java`:
- Around line 72-77: After unpacking the CentralDogmaConfigSource with the
validator unpack method, validate that the cluster name obtained from
cdConfig.getClusterName() is not null or empty before passing it to
factoryContext.clusterStream(). Add validation logic immediately after the
unpacking of cdConfig and before the clusterStream creation to fail fast with a
clear configuration error message when the cluster_name is missing or empty,
rather than allowing an invalid value to propagate to downstream stream/watcher
operations.
In
`@client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.java`:
- Around line 61-67: The current code accepts query strings with multiple
parameters (such as ?profile=/x.yaml&foo=1) without validation, silently
treating the entire suffix as a profile value. After extracting the queryString,
add validation to ensure it only contains the expected PROFILE_PARAM and nothing
else. Specifically, check that the queryString does not contain additional
parameters or unsupported characters (such as the ampersand character), and
throw an IllegalArgumentException with a descriptive message if unexpected
content is detected. This validation should occur in the conditional block that
checks if queryString.startsWith(PROFILE_PARAM) to reject malformed resource
names early.
In
`@client/java-armeria-xds/src/main/proto/com/github/xds/centraldogma/v1/centraldogma_config_source.proto`:
- Line 16: The package declaration in centraldogma_config_source.proto does not
align with its file location, causing a Buf PACKAGE_DIRECTORY_MATCH lint error.
Either adjust the directory structure to match the package
com.github.xds.centraldogma.v1 (ensuring the file path from the Buf module root
corresponds to the package hierarchy), or update the buf.yaml configuration to
set the correct module root so the relative path from that root aligns with the
declared package name. Verify the fix by running Buf lint to confirm the
PACKAGE_DIRECTORY_MATCH error is resolved.
In
`@it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.java`:
- Around line 47-57: The scaffold method writes test data to
`/clusters/my-cluster.json` that is shared across all tests in the class, and
since the CentralDogmaExtension runs once per class with no per-test reset, the
file gets modified by different tests (both scaffold for STATIC type and
fullResourceChain for EDS type) causing test execution order to determine which
schema subsequent tests observe. Fix this by either creating unique resource
names for each test method (for example by including the test method name in the
cluster name used in CLUSTER_CHANGE) or by implementing a per-test reset
mechanism that clears or restores the repository state between test methods
using a `@BeforeEach` annotated method that either deletes and recreates the xds
repository or specifically removes the conflicting cluster file before each
test.
In
`@server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.java`:
- Around line 285-287: The conditional check for injecting the profile value
into cdContext uses only `variableFile != null`, which allows empty strings to
pass through and be published as the profile value. This can cause downstream
resource parsing failures. Update the condition to also verify that variableFile
is not empty or blank before adding it to cdContext with the "profile" key,
ensuring only non-empty profile values are injected.
🪄 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: 27fe8666-b625-4893-ab34-a348d48b8f4d
📒 Files selected for processing (14)
build.gradleclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/PreprocessorCentralDogmaBuilder.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/package-info.javaclient/java-armeria-xds/src/main/proto/com/github/xds/centraldogma/v1/centraldogma_config_source.protoclient/java-armeria-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.configsource.SotwConfigSourceSubscriptionFactorydependencies.tomlit/xds-client/build.gradleit/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTemplateTest.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/ToJsonMethod.javasettings.gradle
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1318 +/- ##
============================================
- Coverage 69.52% 69.34% -0.18%
- Complexity 5540 5545 +5
============================================
Files 531 535 +4
Lines 23413 23616 +203
Branches 2675 2692 +17
============================================
+ Hits 16277 16377 +100
- Misses 5666 5742 +76
- Partials 1470 1497 +27 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jrhee17
force-pushed
the
feat/centraldogma-configsource
branch
from
c4aacb3 to
1bcaf83
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.java (1)
351-359: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winShared resource path
/clusters/my-cluster.jsoncauses order-dependent tests.
scaffold()writes/clusters/my-cluster.jsonas aSTATICcluster, whilefullResourceChain(ext="json")overwrites the same path with anEDScluster. SinceCentralDogmaExtensionruns once per class with no per-test reset, the execution order determines whetherfetchClusterFromCentralDogmaobserves theSTATICschema it asserts on. Use a unique resource name for thejsonvariant (e.g., include the test name) or reset the repository between tests.🤖 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/centraldogma/xds/it/CentralDogmaConfigSourceTest.java` around lines 351 - 359, The shared cluster resource path in fullResourceChain() is causing order-dependent test failures because it overwrites the STATIC cluster written by scaffold() at the same /clusters/my-cluster.json location. Update fullResourceChain(String ext, ChangeFactory factory) to use a unique path for the json variant (for example, a test-specific cluster name) or add repository reset logic so fetchClusterFromCentralDogma always sees the intended cluster schema regardless of test order.server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.java (1)
285-287: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid injecting empty
centraldogma.variableFile.
variableFile != nullstill allows"", and downstream templates check only existence (??), which can emit?profile=and break resource-name parsing.Suggested fix
- if (variableFile != null) { + if (!Strings.isNullOrEmpty(variableFile)) { cdContext.put("variableFile", variableFile); }🤖 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 `@server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.java` around lines 285 - 287, Avoid injecting centraldogma.variableFile when it is empty, since the current Templater logic only checks variableFile != null and still stores blank strings. Update the conditional in Templater so it only puts variableFile into cdContext when it has a real value, preserving downstream ?? checks from emitting empty query parameters and breaking parsing.
🤖 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.
Duplicate comments:
In
`@it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.java`:
- Around line 351-359: The shared cluster resource path in fullResourceChain()
is causing order-dependent test failures because it overwrites the STATIC
cluster written by scaffold() at the same /clusters/my-cluster.json location.
Update fullResourceChain(String ext, ChangeFactory factory) to use a unique path
for the json variant (for example, a test-specific cluster name) or add
repository reset logic so fetchClusterFromCentralDogma always sees the intended
cluster schema regardless of test order.
In
`@server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.java`:
- Around line 285-287: Avoid injecting centraldogma.variableFile when it is
empty, since the current Templater logic only checks variableFile != null and
still stores blank strings. Update the conditional in Templater so it only puts
variableFile into cdContext when it has a real value, preserving downstream ??
checks from emitting empty query parameters and breaking parsing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 57c33ec0-97ba-456b-9496-8a8a417ac2fb
📒 Files selected for processing (13)
client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CachingStream.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/PreprocessorCentralDogmaBuilder.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/package-info.javaclient/java-armeria-xds/src/main/proto/com/github/xds/centraldogma/v1/centraldogma_config_source.protoclient/java-armeria-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.configsource.SotwConfigSourceSubscriptionFactoryit/xds-client/build.gradleit/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTemplateTest.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/ToJsonMethod.javasettings.gradle
✅ Files skipped from review due to trivial changes (5)
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CachingStream.java
- server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/ToJsonMethod.java
- it/xds-client/build.gradle
- settings.gradle
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/package-info.java
🚧 Files skipped from review as they are similar to previous changes (3)
- client/java-armeria-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.configsource.SotwConfigSourceSubscriptionFactory
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.java
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.java
ikhoon
approved these changes
Jun 30, 2026
| // under the License. | ||
| syntax = "proto3"; | ||
|
|
||
| package com.github.xds.centraldogma.v1; |
Contributor
There was a problem hiding this comment.
Is there a convention for the package names? I'm asking because I'm not sure whether we can use com.github.xds.
Contributor
Author
There was a problem hiding this comment.
This is due to the current limitation at https://github.com/line/armeria/blob/8de1a038a3aef5e143a542329af786d7f0a1e21e/xds/src/main/java/com/linecorp/armeria/xds/XdsResourceReader.java#L77-L79
I'm planning on updating to add a fixed namespace com.linecorp.armeria.xds.extensions
Contributor
There was a problem hiding this comment.
How about com.linecorp.centraldogma.xds.v1; for java_package?
Contributor
Author
There was a problem hiding this comment.
I'll add an API at XdsResourceReader for customizing which packages to scan, and also add to the FactoryContext for general usage.
| allprojects { | ||
| repositories { | ||
| mavenCentral() | ||
| maven { url "https://central.sonatype.com/repository/maven-snapshots/" } |
Contributor
There was a problem hiding this comment.
It looks like this was added for testing with the Armeria snapshot but it should be removed before merging.
Contributor
Author
There was a problem hiding this comment.
5ab0d65 will be reverted once 1.41.0 is released
minwoox
reviewed
Jul 1, 2026
| Clients.builder(ClientPreprocessors.of(preprocessor)); | ||
| builder.decorator(DecodingClient.newDecorator()); | ||
| final WebClient client = builder.build(WebClient.class); | ||
| return new ArmeriaCentralDogma(CommonPools.blockingTaskExecutor(), client, "anonymous", |
Contributor
There was a problem hiding this comment.
Is this okay for using anonymous?
| * </ul> | ||
| */ | ||
| Query<JsonNode> query() { | ||
| if (path.endsWith(".yaml") || path.endsWith(".yml")) { |
Contributor
There was a problem hiding this comment.
nit: guarding like basePath()?
if (ftl) {}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.java (1)
141-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate TLS client-builder logic.
configureWebClientBuilderandwebClient()build the identicalTlsKeyPair/ClientTlsConfigtwice. Could extract a shared helper.♻️ Proposed consolidation
+ private static ClientFactory tlsClientFactory() { + final TlsKeyPair tlsKeyPair = TlsKeyPair.of(clientCert.privateKey(), clientCert.certificate()); + final ClientTlsConfig tlsConfig = + ClientTlsConfig.builder() + .tlsCustomizer(b -> b.trustManager(serverCert.certificate())) + .build(); + return ClientFactory.builder() + .tlsProvider(TlsProvider.of(tlsKeyPair), tlsConfig) + .build(); + } + private static void configureWebClientBuilder(WebClientBuilder builder) { - final TlsKeyPair tlsKeyPair = TlsKeyPair.of(clientCert.privateKey(), - clientCert.certificate()); - final ClientTlsConfig tlsConfig = - ClientTlsConfig.builder() - .tlsCustomizer(b -> b.trustManager(serverCert.certificate())) - .build(); - builder.factory(ClientFactory.builder() - .tlsProvider(TlsProvider.of(tlsKeyPair), tlsConfig) - .build()); + builder.factory(tlsClientFactory()); } private static WebClient webClient() { - final TlsKeyPair tlsKeyPair = TlsKeyPair.of(clientCert.privateKey(), - clientCert.certificate()); - final ClientTlsConfig tlsConfig = - ClientTlsConfig.builder() - .tlsCustomizer(b -> b.trustManager(serverCert.certificate())) - .build(); return WebClient.builder("https://127.0.0.1:" + dogma.serverAddress().getPort()) - .factory(ClientFactory.builder() - .tlsProvider(TlsProvider.of(tlsKeyPair), tlsConfig) - .build()) + .factory(tlsClientFactory()) .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/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.java` around lines 141 - 165, The TLS client setup is duplicated in configureWebClientBuilder and webClient(), where both build the same TlsKeyPair and ClientTlsConfig. Extract that shared TLS/client factory construction into a common helper and have both methods call it, keeping the existing clientCert and serverCert wiring unchanged while removing the repeated builder logic.
🤖 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/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.java`:
- Around line 141-165: The TLS client setup is duplicated in
configureWebClientBuilder and webClient(), where both build the same TlsKeyPair
and ClientTlsConfig. Extract that shared TLS/client factory construction into a
common helper and have both methods call it, keeping the existing clientCert and
serverCert wiring unchanged while removing the repeated builder logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98426cc0-1a3d-43ad-89ab-bc672b59e1f3
📒 Files selected for processing (7)
client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/PreprocessorBasedCentralDogma.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceBearerTokenTest.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.javaxds-api/build.gradlexds-api/src/main/proto/com/github/xds/centraldogma/v1/centraldogma_config_source.proto
🚧 Files skipped from review as they are similar to previous changes (2)
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.java
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.java
minwoox
approved these changes
Jul 1, 2026
minwoox
left a comment
minwoox
left a comment
Contributor
There was a problem hiding this comment.
👍 👍
Please fix the CI failure.
jrhee17
force-pushed
the
feat/centraldogma-configsource
branch
from
fd24349 to
b6f9119
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.java (1)
141-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate TLS client-setup logic.
configureWebClientBuilderandwebClientbuild the sameTlsKeyPair/ClientTlsConfiginline; consider factoring the shared TLS config construction into one helper to avoid drift if cert wiring changes.♻️ Suggested consolidation
+ private static ClientFactory tlsClientFactory() { + final TlsKeyPair tlsKeyPair = TlsKeyPair.of(clientCert.privateKey(), clientCert.certificate()); + final ClientTlsConfig tlsConfig = + ClientTlsConfig.builder() + .tlsCustomizer(b -> b.trustManager(serverCert.certificate())) + .build(); + return ClientFactory.builder() + .tlsProvider(TlsProvider.of(tlsKeyPair), tlsConfig) + .build(); + } + private static void configureWebClientBuilder(WebClientBuilder builder) { - final TlsKeyPair tlsKeyPair = TlsKeyPair.of(clientCert.privateKey(), - clientCert.certificate()); - final ClientTlsConfig tlsConfig = - ClientTlsConfig.builder() - .tlsCustomizer(b -> b.trustManager(serverCert.certificate())) - .build(); - builder.factory(ClientFactory.builder() - .tlsProvider(TlsProvider.of(tlsKeyPair), tlsConfig) - .build()); + builder.factory(tlsClientFactory()); } private static WebClient webClient() { - final TlsKeyPair tlsKeyPair = TlsKeyPair.of(clientCert.privateKey(), - clientCert.certificate()); - final ClientTlsConfig tlsConfig = - ClientTlsConfig.builder() - .tlsCustomizer(b -> b.trustManager(serverCert.certificate())) - .build(); return WebClient.builder("https://127.0.0.1:" + dogma.serverAddress().getPort()) - .factory(ClientFactory.builder() - .tlsProvider(TlsProvider.of(tlsKeyPair), tlsConfig) - .build()) + .factory(tlsClientFactory()) .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/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.java` around lines 141 - 165, The TLS client setup is duplicated between configureWebClientBuilder and webClient, which can drift if cert wiring changes. Extract the shared TlsKeyPair and ClientTlsConfig construction into a single helper used by both methods, then have each method only apply that shared TLS configuration through ClientFactory/TlsProvider.
🤖 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/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.java`:
- Around line 141-165: The TLS client setup is duplicated between
configureWebClientBuilder and webClient, which can drift if cert wiring changes.
Extract the shared TlsKeyPair and ClientTlsConfig construction into a single
helper used by both methods, then have each method only apply that shared TLS
configuration through ClientFactory/TlsProvider.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba98a828-76ee-4e4a-836b-638bfe2b7955
📒 Files selected for processing (18)
build.gradleclient/java-armeria-xds/build.gradleclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/PreprocessorBasedCentralDogma.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.javaclient/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/package-info.javaclient/java-armeria-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.configsource.SotwConfigSourceSubscriptionFactorydependencies.tomlit/xds-client/build.gradleit/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceBearerTokenTest.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceMtlsTest.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTemplateTest.javait/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/ToJsonMethod.javasettings.gradlexds-api/build.gradlexds-api/src/main/proto/com/github/xds/centraldogma/v1/centraldogma_config_source.proto
✅ Files skipped from review due to trivial changes (2)
- build.gradle
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/package-info.java
🚧 Files skipped from review as they are similar to previous changes (13)
- xds-api/build.gradle
- dependencies.toml
- client/java-armeria-xds/build.gradle
- it/xds-client/build.gradle
- server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/ToJsonMethod.java
- settings.gradle
- client/java-armeria-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.configsource.SotwConfigSourceSubscriptionFactory
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/PreprocessorBasedCentralDogma.java
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/ResourcePath.java
- server/src/main/java/com/linecorp/centraldogma/server/internal/api/variable/Templater.java
- it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTest.java
- it/xds-client/src/test/java/com/linecorp/centraldogma/xds/it/CentralDogmaConfigSourceTemplateTest.java
- client/java-armeria-xds/src/main/java/com/linecorp/centraldogma/client/armeria/xds/configsource/CentralDogmaSotwConfigSourceSubscriptionFactory.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Central Dogma already provides an xDS control plane via its built-in xDS plugin. This change adds a supplementary approach: a custom
ConfigSourcebacked by Central Dogma's watch API that allows clients to subscribe to xDS resources (clusters, listeners, routes, endpoints) directly from the repository — without going through the gRPC-based xDS control plane. This is useful for simpler setups where users want to manage xDS resources as plain files in Central Dogma and get live updates via the watcher mechanism.Additionally, FreeMarker templates (
.ftlfiles) allow users to define reusable xDS resource templates parameterized via profile files. For example, a shared listener template can have its routes injected from a small YAML profile, eliminating the need for separate Listener and Route resources per service:/listeners/my-listener.yaml(per-service profile):The listener is subscribed to as:
myproject/xds/listeners/default-outbound.json.ftl?profile=/listeners/my-listener.yamlModifications:
client/java-armeria-xdsmodule with aCentralDogmaConfigSourceprotobuf message and aSotwConfigSourceSubscriptionFactorySPI implementation:CentralDogmaSotwConfigSourceSubscriptionFactory— subscribes to Central Dogma resources via the watch API, emittingDiscoveryResponsesnapshots that integrate with Armeria's xDS bootstrapResourcePath— parses xDS resource names encoded as{project}/{repo}/{path}[?profile={variableFile}]PreprocessorCentralDogmaBuilder— creates aCentralDogmaclient from aClusterSnapshot's preprocessorMETA-INF/servicesToJsonMethod— a custom FreeMarker method (${toJson(obj)}) that serializes template variables to JSON, enabling structured objects (e.g., route match rules) to be embedded in both JSON and YAML templatescentraldogmanamespace into the FreeMarker template context (centraldogma.project,centraldogma.repo,centraldogma.path,centraldogma.profile) so templates can derive their own xDS resource namesResult:
CentralDogmaConfigSourcein their bootstrap configuration to fetch xDS resources directly from Central Dogma without going through the xDS control plane.json.ftl,.yaml.ftl) with profile-based parameterization