Introduce an extension point for specifying custom ConfigSource#6747
Merged
Introduce an extension point for specifying custom ConfigSource#6747
ConfigSource#6747Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors xDS config-source handling to a handler+subscription model, adds SOTW/Path/gRPC subscription factories and callbacks, centralizes YAML/JSON→protobuf parsing, extends proto for custom_config_source, and updates tests and integrations. ChangesxDS Config-Source Handler & Subscription Model
Sequence DiagramsequenceDiagram
participant App as Application
participant CPCM as ControlPlaneClientManager
participant CSH as ConfigSourceHandler
participant SF as ConfigSourceFactory
participant SUB as ConfigSourceSubscription
participant SC as StateCoordinator
participant STREAM as XdsStream
participant WATCH as ResourceWatcher
App->>CPCM: subscribe(configSource, type, name, watcher)
CPCM->>CPCM: clientMap.computeIfAbsent(configSource, createClient)
CPCM->>SF: create(configSource, ...)
SF->>SC: new StateCoordinator(configSource,...)
SF->>SUB: create subscription (Grpc or Path)
SF-->>CPCM: return ConfigSourceHandler(SC, SUB)
CPCM->>CSH: addSubscriber(type, name, watcher)
CSH->>SC: register(type, name, watcher)
CSH->>SUB: updateInterests(type, interestedResources)
SUB->>STREAM: resourcesUpdated(type)
STREAM->>STREAM: send/adjust discovery requests
alt update arrives (file or gRPC)
SUB->>SC: onDiscoveryResponse(response)
SC->>SC: onSotwConfigUpdate(type, parsedResources)
SC->>WATCH: notify resource updates / missing
end
App->>CPCM: unsubscribe(...)
CPCM->>CSH: removeSubscriber(...)
CSH->>SUB: updateInterests(...)
CSH->>SC: unregister(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
xds/src/main/java/com/linecorp/armeria/xds/XdsExtensionRegistry.java (1)
61-73: 💤 Low valueSPI factory name collisions will throw at startup.
If an SPI-discovered
SotwConfigSourceSubscriptionFactoryhas the samename()as the built-inPathSotwConfigSourceSubscriptionFactoryorGrpcConfigSourceStreamFactory, theImmutableMap.Builderwill throwIllegalArgumentExceptionon duplicate keys. This may be intentional to prevent accidental overrides, but consider documenting this behavior or adding a more descriptive error message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xds/src/main/java/com/linecorp/armeria/xds/XdsExtensionRegistry.java` around lines 61 - 73, SPI-discovered SotwConfigSourceSubscriptionFactory instances can have the same name() as built-in factories (PathSotwConfigSourceSubscriptionFactory or GrpcConfigSourceStreamFactory), causing ImmutableMap.Builder to throw on duplicate keys; add an explicit duplicate-name check before calling register (or inside register) that detects if byName already contains the factory.name() and throw a clear IllegalStateException (or log+skip) that includes the duplicate name and both factory class names (e.g., reference PathSotwConfigSourceSubscriptionFactory, GrpcConfigSourceStreamFactory, SotwConfigSourceSubscriptionFactory, register, byName, byTypeUrl) so the startup failure message is descriptive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/PathConfigSourceTest.java`:
- Around line 127-145: The watcher lambda in verifyClusterFromPathConfigSource
silently ignores throwable t which causes tests to hang; update the
SnapshotWatcher lambda (watcher) so that if t is non-null it immediately fails
the test by rethrowing or wrapping t (e.g., throw new
AssertionError("SnapshotWatcher error", t)), otherwise continue to set
snapshotRef when snapshot is a ClusterSnapshot; keep existing await/assert logic
that checks snapshotRef and xdsResource name and still call
xdsBootstrap.clusterRoot("path-cluster") as before.
In `@xds/src/main/java/com/linecorp/armeria/xds/SotwSubscriptionCallbacks.java`:
- Around line 37-44: The onDiscoveryResponse(DiscoveryResponse) Javadoc is
missing the threading requirement; document that implementations must be invoked
on the event-loop thread used by the subscription: state that callers must call
onDiscoveryResponse from the EventExecutor passed into
SotwConfigSourceSubscriptionFactory.create (i.e., the same EventExecutor used by
StateCoordinator), and note that StateCoordinator.onDiscoveryResponse enforces
this with checkArgument(eventLoop.inEventLoop(), ...), so calling from another
thread will throw IllegalArgumentException; add this requirement to the method
Javadoc so third-party implementors know to run their callback on that
EventExecutor.
In `@xds/src/main/java/com/linecorp/armeria/xds/StateCoordinator.java`:
- Around line 61-72: The initialFetchTimeoutMillis method incorrectly treats
protobuf Duration as an Instant; replace the manual Instant.ofEpochSecond
conversion with Durations.toMillis(timeoutDuration) (import
com.google.protobuf.util.Durations) to get a proper millisecond conversion and
validation; update checkArgument to validate the resulting millis (e.g.,
checkArgument(millis >= 0, ...)) and return the millis from
initialFetchTimeoutMillis (refer to method initialFetchTimeoutMillis and symbol
timeoutDuration).
---
Nitpick comments:
In `@xds/src/main/java/com/linecorp/armeria/xds/XdsExtensionRegistry.java`:
- Around line 61-73: SPI-discovered SotwConfigSourceSubscriptionFactory
instances can have the same name() as built-in factories
(PathSotwConfigSourceSubscriptionFactory or GrpcConfigSourceStreamFactory),
causing ImmutableMap.Builder to throw on duplicate keys; add an explicit
duplicate-name check before calling register (or inside register) that detects
if byName already contains the factory.name() and throw a clear
IllegalStateException (or log+skip) that includes the duplicate name and both
factory class names (e.g., reference PathSotwConfigSourceSubscriptionFactory,
GrpcConfigSourceStreamFactory, SotwConfigSourceSubscriptionFactory, register,
byName, byTypeUrl) so the startup failure message is descriptive.
🪄 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: 5a5f9373-0bee-49e8-af19-77ca66aa1754
📒 Files selected for processing (26)
it/xds-client/src/main/java/com/linecorp/armeria/xds/it/XdsResourceReader.javait/xds-client/src/test/java/com/linecorp/armeria/xds/it/PathConfigSourceTest.javaxds-api/src/main/proto/envoy/config/core/v3/config_source.protoxds/build.gradlexds/src/main/java/com/linecorp/armeria/xds/AdsXdsStream.javaxds/src/main/java/com/linecorp/armeria/xds/ConfigSourceClient.javaxds/src/main/java/com/linecorp/armeria/xds/ConfigSourceHandler.javaxds/src/main/java/com/linecorp/armeria/xds/ConfigSourceMapper.javaxds/src/main/java/com/linecorp/armeria/xds/ConfigSourceSubscription.javaxds/src/main/java/com/linecorp/armeria/xds/ControlPlaneClientManager.javaxds/src/main/java/com/linecorp/armeria/xds/DeltaActualStream.javaxds/src/main/java/com/linecorp/armeria/xds/GrpcConfigSourceStreamFactory.javaxds/src/main/java/com/linecorp/armeria/xds/PathConfigSourceLifecycleObserver.javaxds/src/main/java/com/linecorp/armeria/xds/PathSotwConfigSourceSubscriptionFactory.javaxds/src/main/java/com/linecorp/armeria/xds/ResourceNodeAdapter.javaxds/src/main/java/com/linecorp/armeria/xds/SotwActualStream.javaxds/src/main/java/com/linecorp/armeria/xds/SotwConfigSourceSubscriptionFactory.javaxds/src/main/java/com/linecorp/armeria/xds/SotwSubscriptionCallbacks.javaxds/src/main/java/com/linecorp/armeria/xds/StateCoordinator.javaxds/src/main/java/com/linecorp/armeria/xds/XdsBootstrapImpl.javaxds/src/main/java/com/linecorp/armeria/xds/XdsExtensionRegistry.javaxds/src/main/java/com/linecorp/armeria/xds/XdsResourceReader.javaxds/src/test/java/com/linecorp/armeria/xds/StateCoordinatorTest.javaxds/src/test/java/com/linecorp/armeria/xds/XdsClientCleanupTest.javaxds/src/test/java/com/linecorp/armeria/xds/XdsExtensionRegistryTest.javaxds/src/test/resources/META-INF/services/com.linecorp.armeria.xds.validator.XdsValidatorIndex
💤 Files with no reviewable changes (1)
- xds/src/main/java/com/linecorp/armeria/xds/ConfigSourceClient.java
Contributor
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/PathConfigSourceTest.java (1)
127-152: ⚖️ Poor tradeoffConsider adding a test for dynamic file-update delivery.
The helper
verifyClusterFromPathConfigSourceonly verifies the initial load path. The primary value ofpath_config_sourceis reacting to file changes at runtime. A complementary test that overwrites the file afterXdsBootstrapis running and asserts the new snapshot is delivered would give meaningful coverage of the watch loop inPathConfigSourceSubscription.parseAndPush.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/PathConfigSourceTest.java` around lines 127 - 152, The test helper verifyClusterFromPathConfigSource only asserts initial load; add a new test that starts XdsBootstrap with verifyClusterFromPathConfigSource (or reuse its setup) and then updates the file backing the PathConfigSource to a different cluster name, then await until the SnapshotWatcher (set as defaultSnapshotWatcher) observes a new ClusterSnapshot with the new resource name. Specifically, reuse the AtomicReference<ClusterSnapshot> and SnapshotWatcher logic from verifyClusterFromPathConfigSource, perform a file overwrite of the path-config file after xdsBootstrap.clusterRoot("path-cluster") is active, and assert the watcher receives the updated snapshot (verifying PathConfigSourceSubscription.parseAndPush reacts to the file change and pushes the new snapshot).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/PathConfigSourceTest.java`:
- Around line 127-152: The test helper verifyClusterFromPathConfigSource only
asserts initial load; add a new test that starts XdsBootstrap with
verifyClusterFromPathConfigSource (or reuse its setup) and then updates the file
backing the PathConfigSource to a different cluster name, then await until the
SnapshotWatcher (set as defaultSnapshotWatcher) observes a new ClusterSnapshot
with the new resource name. Specifically, reuse the
AtomicReference<ClusterSnapshot> and SnapshotWatcher logic from
verifyClusterFromPathConfigSource, perform a file overwrite of the path-config
file after xdsBootstrap.clusterRoot("path-cluster") is active, and assert the
watcher receives the updated snapshot (verifying
PathConfigSourceSubscription.parseAndPush reacts to the file change and pushes
the new snapshot).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 861fe2d6-d3fa-4888-9e78-bb9fb52cc2d6
📒 Files selected for processing (4)
it/xds-client/src/test/java/com/linecorp/armeria/xds/it/PathConfigSourceTest.javaxds/src/main/java/com/linecorp/armeria/xds/SotwConfigSourceSubscriptionFactory.javaxds/src/main/java/com/linecorp/armeria/xds/SotwSubscriptionCallbacks.javaxds/src/main/java/com/linecorp/armeria/xds/StateCoordinator.java
✅ Files skipped from review due to trivial changes (1)
- xds/src/main/java/com/linecorp/armeria/xds/StateCoordinator.java
🚧 Files skipped from review as they are similar to previous changes (2)
- xds/src/main/java/com/linecorp/armeria/xds/SotwSubscriptionCallbacks.java
- xds/src/main/java/com/linecorp/armeria/xds/SotwConfigSourceSubscriptionFactory.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
it/xds-controlplane-api/src/test/java/com/linecorp/armeria/xds/it/ControlPlaneApiRegressionTest.java (1)
115-119: ⚡ Quick winFail fast on watcher callback errors instead of silently ignoring them.
At Line 115 and Line 186, the callback drops
t. If the watcher reports an error, these tests may only fail by timeout. Capturetand assert it is null insideawait()for clearer failures.Suggested change
- final AtomicReference<ClusterSnapshot> snapshotRef = new AtomicReference<>(); + final AtomicReference<ClusterSnapshot> snapshotRef = new AtomicReference<>(); + final AtomicReference<Throwable> errorRef = new AtomicReference<>(); try (XdsBootstrap xdsBootstrap = XdsBootstrap.of(bootstrap)) { xdsBootstrap.clusterRoot("my-cluster") .addSnapshotWatcher((snapshot, t) -> { + if (t != null) { + errorRef.set(t); + return; + } if (snapshot != null) { snapshotRef.set(snapshot); } }); await().untilAsserted(() -> { + assertThat(errorRef.get()).isNull(); assertThat(snapshotRef.get()).isNotNull(); assertThat(snapshotRef.get().xdsResource().resource().getName()) .isEqualTo("my-cluster"); }); }Also applies to: 186-190
🤖 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-controlplane-api/src/test/java/com/linecorp/armeria/xds/it/ControlPlaneApiRegressionTest.java` around lines 115 - 119, The watcher callback passed to addSnapshotWatcher currently ignores the throwable parameter t, causing test failures to surface only via timeouts; modify the callback used in ControlPlaneApiRegressionTest (the lambda passed to addSnapshotWatcher that updates snapshotRef) to capture the throwable (t) into a shared AtomicReference or similar (e.g., errorRef) and then, in the await() assertion after the watcher runs, assert that that errorRef is null (or rethrow/assert the throwable) so any watcher error fails fast and surfaces immediately; apply the same change to the second occurrence around lines referenced (the other addSnapshotWatcher callback).
🤖 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
`@xds/src/main/java/com/linecorp/armeria/xds/PathSotwConfigSourceSubscriptionFactory.java`:
- Around line 102-114: The watchService.register(...) call in
PathSotwConfigSourceSubscriptionFactory is unprotected and can throw, leaving
the subscription inactive; wrap that registration in a try/catch inside the
CommonPools.blockingTaskExecutor() runnable, catch Throwable from
watchService.register(PathWatcher.ofFile(...)), and on error schedule an
eventLoop.execute to surface the failure (e.g., call
close0(cancellableCandidate) or a dedicated failure handler) and log/report the
exception; ensure watchCancellable is only set on success and that
close0/watchCancellable cleanup paths are invoked from the event loop so the
subscription doesn't silently stall (refer to symbols: watchService.register,
PathWatcher.ofFile, parseAndPush, watchCancellable, close0,
CommonPools.blockingTaskExecutor, eventLoop).
- Around line 61-67: The factory method
PathSotwConfigSourceSubscriptionFactory.create currently assumes
configSource.getPathConfigSource() exists and contains a non-empty path; add
validation in create(ConfigSource, SotwSubscriptionCallbacks, EventExecutor) to
call configSource.hasPathConfigSource() and verify the returned
PathConfigSource's path is non-null and not blank (e.g., path() or getPath()),
and if invalid throw an IllegalArgumentException (or another appropriate runtime
exception) with a clear message describing the missing/empty path instead of
constructing a PathConfigSourceSubscription; this ensures
PathConfigSourceSubscription is only created with a valid PathConfigSource.
---
Nitpick comments:
In
`@it/xds-controlplane-api/src/test/java/com/linecorp/armeria/xds/it/ControlPlaneApiRegressionTest.java`:
- Around line 115-119: The watcher callback passed to addSnapshotWatcher
currently ignores the throwable parameter t, causing test failures to surface
only via timeouts; modify the callback used in ControlPlaneApiRegressionTest
(the lambda passed to addSnapshotWatcher that updates snapshotRef) to capture
the throwable (t) into a shared AtomicReference or similar (e.g., errorRef) and
then, in the await() assertion after the watcher runs, assert that that errorRef
is null (or rethrow/assert the throwable) so any watcher error fails fast and
surfaces immediately; apply the same change to the second occurrence around
lines referenced (the other addSnapshotWatcher callback).
🪄 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: 0b7dd28e-f645-49f9-9eb6-a464ded20e24
📒 Files selected for processing (5)
it/xds-controlplane-api/build.gradleit/xds-controlplane-api/src/test/java/com/linecorp/armeria/xds/it/ControlPlaneApiRegressionTest.javasettings.gradlexds/src/main/java/com/linecorp/armeria/xds/ConfigSourceMapper.javaxds/src/main/java/com/linecorp/armeria/xds/PathSotwConfigSourceSubscriptionFactory.java
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
xds/src/main/java/com/linecorp/armeria/xds/PathSotwConfigSourceSubscriptionFactory.java (2)
101-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWatcher-registration failure handling still incomplete
The
catchblock silently callslifecycleObserver.close()from the blocking executor thread (not the event loop), which is inconsistent with every other lifecycle-observer call in this class (all of which happen on the event loop viaparseAndPushorclose0). Additionally, the failure is still not surfaced tocallbacks, leaving the subscription silently inoperative.🤖 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/PathSotwConfigSourceSubscriptionFactory.java` around lines 101 - 121, In startWatching(), the catch block currently closes lifecycleObserver from the blocking executor and doesn't notify the subscription callbacks; instead, schedule the failure handling onto the eventLoop: inside the catch, capture the exception and call eventLoop.execute(() -> { lifecycleObserver.close(); close0(null) or the appropriate cleanup; invoke the subscription callbacks with the error (e.g., callbacks.onError / callbacks.onSubscriptionFailed or the class's existing error-reporting callback) so the failure is surfaced; ensure watchCancellable remains null. Reference: startWatching, watchService.register, lifecycleObserver, eventLoop, close0, watchCancellable, callbacks, parseAndPush.
61-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidation still missing in
create()
configSource.getPathConfigSource()is called without first checkingconfigSource.hasPathConfigSource(), and neitherconfigSourcenorcallbacksnoreventLoopare null-checked. This is unchanged since the prior review.🤖 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/PathSotwConfigSourceSubscriptionFactory.java` around lines 61 - 67, In create(), validate inputs before using them: check configSource is non-null and configSource.hasPathConfigSource() is true before calling configSource.getPathConfigSource(), and null-check callbacks and eventLoop; if any validation fails, throw an appropriate exception (e.g., IllegalArgumentException or NullPointerException) with a clear message. After these guards, construct and return the PathConfigSourceSubscription using the verified configSource.getPathConfigSource(), watchService, callbacks, eventLoop, meterRegistry, and meterIdPrefix.
🤖 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
`@xds/src/main/java/com/linecorp/armeria/xds/PathSotwConfigSourceSubscriptionFactory.java`:
- Around line 101-121: In startWatching(), the catch block currently closes
lifecycleObserver from the blocking executor and doesn't notify the subscription
callbacks; instead, schedule the failure handling onto the eventLoop: inside the
catch, capture the exception and call eventLoop.execute(() -> {
lifecycleObserver.close(); close0(null) or the appropriate cleanup; invoke the
subscription callbacks with the error (e.g., callbacks.onError /
callbacks.onSubscriptionFailed or the class's existing error-reporting callback)
so the failure is surfaced; ensure watchCancellable remains null. Reference:
startWatching, watchService.register, lifecycleObserver, eventLoop, close0,
watchCancellable, callbacks, parseAndPush.
- Around line 61-67: In create(), validate inputs before using them: check
configSource is non-null and configSource.hasPathConfigSource() is true before
calling configSource.getPathConfigSource(), and null-check callbacks and
eventLoop; if any validation fails, throw an appropriate exception (e.g.,
IllegalArgumentException or NullPointerException) with a clear message. After
these guards, construct and return the PathConfigSourceSubscription using the
verified configSource.getPathConfigSource(), watchService, callbacks, eventLoop,
meterRegistry, and meterIdPrefix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e60e99b-7932-40cc-9b1c-02146db71b35
📒 Files selected for processing (1)
xds/src/main/java/com/linecorp/armeria/xds/PathSotwConfigSourceSubscriptionFactory.java
minwoox
approved these changes
May 7, 2026
|
|
||
| // custom_config_source is an Armeria extension (field 1000) that may not exist | ||
| // when a different proto artifact (e.g. java-control-plane api) is used at runtime. | ||
| static final boolean HAS_CUSTOM_CONFIG_SOURCE; |
|
|
||
| @Override | ||
| public void onDiscoveryResponse(DiscoveryResponse response) { | ||
| checkArgument(eventLoop.inEventLoop(), "eventLoop must be inEventLoop"); |
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:
The xDS
ConfigSourceClientwas a monolithic class that tightly coupled gRPC streaming, timeout handling, and response parsing into a single unit. This made it impossible to support non-gRPC config sources like Envoy'spath_config_source(file-based) or user-definedcustom_config_sourcebackends. The class also mixed concerns — gRPC client construction, initial fetch timeout computation, and subscriber management all lived in one place.Modifications:
ConfigSourceClientand decomposed it into focused components:ConfigSourceSubscription— a public interface representing a subscription that delivers xDS resources from any config sourceConfigSourceHandler— internal glue that wires aConfigSourceSubscriptionto aStateCoordinatorSotwConfigSourceSubscriptionFactory— a public SPI interface for creating subscriptions for non-gRPC config sourcesSotwSubscriptionCallbacks— the callback interface implementations use to pushDiscoveryResponsedata backGrpcConfigSourceStreamFactory— extracts the gRPC-specific client construction and stream logic from the oldConfigSourceClient, implementsSotwConfigSourceSubscriptionFactoryPathSotwConfigSourceSubscriptionFactorywithPathConfigSourceSubscriptionthat watches a file viaDirectoryWatchService/PathWatcherand pushes parsedDiscoveryResponseon every changeXdsResourceReader— a public utility for parsing xDS protobuf messages from YAML or JSON strings/files, with aTypeRegistrythat auto-discovers allio.envoyproxyprotobuf types via classpath scanningXdsExtensionRegistryto register built-inPathSotwConfigSourceSubscriptionFactoryandGrpcConfigSourceStreamFactory, and load SPI-discoveredSotwConfigSourceSubscriptionFactoryimplementationsResult:
path_config_sourcein their xDS bootstrap to load xDS resources from local YAML or JSON files, with automatic file watching for updates.SotwConfigSourceSubscriptionFactoryto plug in custom non-gRPC config sources (e.g., etcd, Consul, ZooKeeper) via ServiceLoader SPI.XdsResourceReaderis available as a public utility for parsing Envoy protobuf types from YAML/JSON.