Add Spring Boot xDS integration module boot4-xds - #6891
Open
jrhee17 wants to merge 2 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds Spring Boot 4 xDS integration backed by Spring ChangesSpring xDS configuration integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SpringConfigServer
participant SpringConfigClient
participant SpringXdsAutoConfiguration
participant SpringConfigSourceFactory
participant XdsWebClient
SpringConfigClient->>SpringConfigServer: import configserver settings
SpringConfigClient->>SpringXdsAutoConfiguration: create XdsBootstrap
SpringXdsAutoConfiguration->>SpringConfigSourceFactory: configure Environment-backed source
SpringConfigSourceFactory->>SpringConfigClient: publish cluster and listener resources
SpringConfigClient->>XdsWebClient: create xDS-enabled client
XdsWebClient->>SpringConfigServer: request configured endpoint
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
spring/boot4-xds-cloud-config/src/test/java/com/linecorp/armeria/spring/cloud/config/xds/SpringCloudConfigXdsAutoConfigurationTest.java (1)
66-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise refresh through
EnvironmentChangeEvent.The factory test only calls
factory.refresh()directly. Add an auto-configuration test that updates the property and publishes anEnvironmentChangeEvent; otherwise a broken listener inSpringCloudConfigXdsAutoConfiguration.java:88-92is not covered.🤖 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 `@spring/boot4-xds-cloud-config/src/test/java/com/linecorp/armeria/spring/cloud/config/xds/SpringCloudConfigXdsAutoConfigurationTest.java` around lines 66 - 84, Add a test alongside loadClusterViaAutoConfig that changes the relevant environment property and publishes an EnvironmentChangeEvent through the application context, then await and assert the XDS configuration refreshes as expected. Exercise the auto-configuration listener rather than calling factory.refresh() directly, using the existing xdsBootstrap and cluster snapshot assertions to verify the updated 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
`@spring/boot4-xds-cloud-config/src/main/java/com/linecorp/armeria/spring/cloud/config/xds/package-info.java`:
- Around line 1-2: Replace the copyright header in package-info.java with the
repository-required LY copyright header, preserving the remaining package
documentation and file contents unchanged.
In
`@spring/boot4-xds-cloud-config/src/main/java/com/linecorp/armeria/spring/cloud/config/xds/SpringCloudConfigXdsAutoConfiguration.java`:
- Around line 64-66: Add the required `@UnstableApi` annotation to the public
SpringCloudConfigXdsAutoConfiguration class, preserving its existing
`@AutoConfiguration` and `@ConditionalOnClass` annotations.
In
`@spring/boot4-xds-cloud-config/src/main/java/com/linecorp/armeria/spring/cloud/config/xds/SpringConfigSourceFactory.java`:
- Around line 103-111: Add Javadoc to the public interface implementations
name() and create() in SpringConfigSourceFactory, documenting each method’s
purpose, parameters, return value, and any relevant behavior. Keep the
implementation unchanged and follow the project’s existing Javadoc style.
- Around line 160-166: Update the property-key construction in the
resource-loading loop of SpringConfigSourceFactory to include a stable xDS
resource-type segment before the resource name, ensuring different XdsType
values use distinct keys. Apply the same namespaced key contract wherever these
properties are written or read, and add coverage for identical resource names
requested by different types.
- Around line 112-116: Update the prefix initialization in
SpringConfigSourceFactory to use DEFAULT_PREFIX when the unpacked typed config’s
rawPrefix is empty, then apply the existing trailing-dot normalization to that
effective value. Add a test covering an unset protobuf prefix and verify lookups
use the default armeria.xds.test-cluster prefix.
- Line 79: Annotate the new public class SpringConfigSourceFactory with
`@UnstableApi`, adding the appropriate import if needed; the class-level
annotation should cover its public constructor and methods.
---
Nitpick comments:
In
`@spring/boot4-xds-cloud-config/src/test/java/com/linecorp/armeria/spring/cloud/config/xds/SpringCloudConfigXdsAutoConfigurationTest.java`:
- Around line 66-84: Add a test alongside loadClusterViaAutoConfig that changes
the relevant environment property and publishes an EnvironmentChangeEvent
through the application context, then await and assert the XDS configuration
refreshes as expected. Exercise the auto-configuration listener rather than
calling factory.refresh() directly, using the existing xdsBootstrap and cluster
snapshot assertions to verify the updated 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 Plus
Run ID: f7d046de-46c2-48bb-ad8e-b575356d1443
📒 Files selected for processing (11)
dependencies.tomlsettings.gradlespring/boot4-xds-cloud-config/build.gradlespring/boot4-xds-cloud-config/src/main/java/com/linecorp/armeria/spring/cloud/config/xds/SpringCloudConfigXdsAutoConfiguration.javaspring/boot4-xds-cloud-config/src/main/java/com/linecorp/armeria/spring/cloud/config/xds/SpringConfigSourceFactory.javaspring/boot4-xds-cloud-config/src/main/java/com/linecorp/armeria/spring/cloud/config/xds/package-info.javaspring/boot4-xds-cloud-config/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.importsspring/boot4-xds-cloud-config/src/test/java/com/linecorp/armeria/spring/cloud/config/xds/SpringCloudConfigXdsAutoConfigurationTest.javaspring/boot4-xds-cloud-config/src/test/java/com/linecorp/armeria/spring/cloud/config/xds/SpringConfigSourceFactoryTest.javaxds-api/src/main/proto/armeria/xds/spring/spring_config_source.protoxds/src/main/java/com/linecorp/armeria/xds/XdsType.java
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6891 +/- ##
============================================
+ Coverage 74.46% 75.15% +0.69%
- Complexity 22234 25530 +3296
============================================
Files 1963 2274 +311
Lines 82437 94653 +12216
Branches 10764 12382 +1618
============================================
+ Hits 61385 71137 +9752
- Misses 15918 17638 +1720
- Partials 5134 5878 +744 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
examples/spring-cloud-config-xds/src/main/resources/config/application-client.yml (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider enabling fail-fast for the config client.
If the config server is unreachable, the client starts without the
armeria.xds.*properties, and the xDS listener stays empty. The failure then appears later as a request error.spring.cloud.config.fail-fast: truemakes the startup failure explicit in the example.♻️ Proposed configuration
spring: config: "import": "configserver:" cloud: config: uri: http://localhost:8888 + fail-fast: true🤖 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 `@examples/spring-cloud-config-xds/src/main/resources/config/application-client.yml` around lines 1 - 6, Update the Spring Cloud Config settings in application-client.yml to enable spring.cloud.config.fail-fast, ensuring startup fails immediately when the config server is unreachable instead of continuing without the armeria.xds.* properties.examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java (2)
32-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd coverage for the
/relayendpoint.
ClientMainexposesGET /relay, and no test calls it. The test only uses the injectedWebClient. Consider@SpringBootTest(webEnvironment = RANDOM_PORT)and one request to/relayso the example endpoint is verified.🤖 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 `@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java` around lines 32 - 38, Update SpringCloudConfigXdsExampleTest to run with a random web environment and add a test using the injected WebClient that sends one GET request to /relay, verifying the ClientMain endpoint is reachable.
43-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
@DynamicPropertySourceover a global system property.
startConfigServermutates the JVM-widespring.cloud.config.uriproperty. This works only because the Spring test context loads after@BeforeAll, and it leaks state into any other test class that shares the JVM.@DynamicPropertySourcesupplies the value directly to the test context and needs no cleanup.♻️ Proposed refactor
`@BeforeAll` static void startConfigServer() { server = ConfigServerMain.createApplication().run("--server.port=0"); - configPort = ((WebServerApplicationContext) server).getWebServer().getPort(); - System.setProperty("spring.cloud.config.uri", "http://localhost:" + configPort); } + `@DynamicPropertySource` + static void configServerUri(DynamicPropertyRegistry registry) { + registry.add("spring.cloud.config.uri", () -> "http://localhost:" + configPort); + } + `@AfterAll` static void stopConfigServer() { - System.clearProperty("spring.cloud.config.uri"); if (server != null) { server.close(); } }🤖 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 `@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java` around lines 43 - 57, Replace the global spring.cloud.config.uri mutation in startConfigServer and stopConfigServer with a static `@DynamicPropertySource` method that registers the URI using the dynamically assigned configPort. Remove the System.setProperty and System.clearProperty calls while preserving ConfigServerMain startup and server shutdown.examples/spring-cloud-config-xds/src/main/resources/config-repo/application.yml (1)
24-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that the port must match the config server port.
port_value: 8888must matchserver.portinexamples/spring-cloud-config-xds/src/main/resources/config/application-server.yml.SpringCloudConfigXdsExampleTestreplaces this cluster because the test server uses a random port. Add a short comment so a reader knows why the value is duplicated.🤖 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 `@examples/spring-cloud-config-xds/src/main/resources/config-repo/application.yml` around lines 24 - 35, Add a short YAML comment next to the test-cluster port_value in the test-cluster configuration explaining that 8888 must match the server.port setting in application-server.yml, while noting that SpringCloudConfigXdsExampleTest replaces this cluster because its test server uses a random port.examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.java (1)
13-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename or externalize the hard-coded listener name.
"test-listener"is a literal in main source code, and it must stay in sync withexamples/spring-cloud-config-xds/src/main/resources/config-repo/application.yml. Thetest-prefix also suggests test-only data inside an example application. Bind the name from a property, or use a neutral name such asrelay-listenerin both files.🤖 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 `@examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.java` around lines 13 - 16, Update XdsClientConfig.xdsHttpPreprocessor to remove the hard-coded "test-listener" value: bind the listener name from configuration, or replace it with a neutral name such as "relay-listener". Ensure the corresponding listener name in application.yml matches the selected value.examples/spring-cloud-config-xds/build.gradle (1)
9-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd descriptions and grouping to the run tasks.
runConfigServerandrunClienthave nogroupordescription, so they do not appear meaningfully in./gradlew tasks. Other example modules in this repository usually set both.♻️ Proposed task metadata
tasks.register('runConfigServer', JavaExec) { + group = 'Execution' + description = 'Runs the Spring Cloud Config Server example.' classpath = sourceSets.main.runtimeClasspath mainClass = 'example.springframework.boot.xds.cloudconfig.server.ConfigServerMain' } tasks.register('runClient', JavaExec) { + group = 'Execution' + description = 'Runs the xDS client example.' classpath = sourceSets.main.runtimeClasspath mainClass = 'example.springframework.boot.xds.cloudconfig.client.ClientMain' }🤖 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 `@examples/spring-cloud-config-xds/build.gradle` around lines 9 - 17, Update the runConfigServer and runClient task registrations to include the standard task group and clear descriptions, matching the metadata conventions used by other example modules so both tasks are meaningfully listed by Gradle.examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java (1)
36-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing Javadoc on the public members of the new example main classes. Both classes document the class but leave their public constructor and public static methods undocumented.
examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java#L36-L51: add Javadoc for the public constructor,main, andcreateApplication.examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.java#L21-L29: add Javadoc formainandcreateApplication, and note the activatedserverandnativeprofiles.As per path instructions: "provide Javadoc for all public classes and public/protected methods".
🤖 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 `@examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java` around lines 36 - 51, Document the public constructor, main, and createApplication methods in ClientMain.java with Javadoc describing their roles. Also document main and createApplication in ConfigServerMain.java, explicitly noting that the server and native profiles are activated; cover every listed public member without changing behavior.Source: Path instructions
🤖 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
`@examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java`:
- Around line 53-57: Update ClientMain.relay to allow only the intended upstream
path(s) before calling xdsWebClient.get, rejecting any unapproved path with the
existing application error mechanism. Replace the blocking aggregate().join()
flow with a CompletableFuture-based return that maps the completed
AggregatedHttpResponse to the current status-and-body string without blocking
the request thread.
In
`@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java`:
- Around line 68-81: Update
SpringCloudConfigXdsExampleTest.webClientViaXdsPreprocessor to use Awaitility
when invoking the xDS-resolved health endpoint, retrying until the asynchronous
cluster update is applied and the response status is HttpStatus.OK. Add the
Awaitility test dependency in the example build configuration if it is not
already available, reusing the version-catalog alias.
---
Nitpick comments:
In `@examples/spring-cloud-config-xds/build.gradle`:
- Around line 9-17: Update the runConfigServer and runClient task registrations
to include the standard task group and clear descriptions, matching the metadata
conventions used by other example modules so both tasks are meaningfully listed
by Gradle.
In
`@examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java`:
- Around line 36-51: Document the public constructor, main, and
createApplication methods in ClientMain.java with Javadoc describing their
roles. Also document main and createApplication in ConfigServerMain.java,
explicitly noting that the server and native profiles are activated; cover every
listed public member without changing behavior.
In
`@examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.java`:
- Around line 13-16: Update XdsClientConfig.xdsHttpPreprocessor to remove the
hard-coded "test-listener" value: bind the listener name from configuration, or
replace it with a neutral name such as "relay-listener". Ensure the
corresponding listener name in application.yml matches the selected value.
In
`@examples/spring-cloud-config-xds/src/main/resources/config-repo/application.yml`:
- Around line 24-35: Add a short YAML comment next to the test-cluster
port_value in the test-cluster configuration explaining that 8888 must match the
server.port setting in application-server.yml, while noting that
SpringCloudConfigXdsExampleTest replaces this cluster because its test server
uses a random port.
In
`@examples/spring-cloud-config-xds/src/main/resources/config/application-client.yml`:
- Around line 1-6: Update the Spring Cloud Config settings in
application-client.yml to enable spring.cloud.config.fail-fast, ensuring startup
fails immediately when the config server is unreachable instead of continuing
without the armeria.xds.* properties.
In
`@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java`:
- Around line 32-38: Update SpringCloudConfigXdsExampleTest to run with a random
web environment and add a test using the injected WebClient that sends one GET
request to /relay, verifying the ClientMain endpoint is reachable.
- Around line 43-57: Replace the global spring.cloud.config.uri mutation in
startConfigServer and stopConfigServer with a static `@DynamicPropertySource`
method that registers the URI using the dynamically assigned configPort. Remove
the System.setProperty and System.clearProperty calls while preserving
ConfigServerMain startup and server shutdown.
🪄 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 Plus
Run ID: a8d11ac0-fad7-4f8d-b0f0-45a6ccdecf35
📒 Files selected for processing (10)
dependencies.tomlexamples/spring-cloud-config-xds/build.gradleexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.javaexamples/spring-cloud-config-xds/src/main/resources/config-repo/application.ymlexamples/spring-cloud-config-xds/src/main/resources/config/application-client.ymlexamples/spring-cloud-config-xds/src/main/resources/config/application-server.ymlexamples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.javasettings.gradle
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.java`:
- Around line 23-30: Add `@UnstableApi` to the public
SpringXdsTypeRegistryPackageProvider class unless its package is already marked
unstable. In
spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.java
at lines 31-40, add `@UnstableApi` and Javadoc to createPropertySource,
documenting the optional name, YAML resource, and returned PropertySource.
In
`@spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.java`:
- Around line 34-37: Update createPropertySource(...) to validate the
EncodedResource parameter with requireNonNull(resource, "resource") before
calling resource.getResource(), preserving the existing factory setup and
property handling.
In
`@spring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.yml`:
- Around line 11-22: Remove the custom-cluster resource under the default
armeria.xds key from the test fixture, leaving only
custom.prefix.custom-cluster; alternatively, change its endpoint and update
assertions to verify the custom-prefixed value is used.
🪄 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 Plus
Run ID: 0d08bbf5-2057-4228-8bc3-861f703f61a9
📒 Files selected for processing (18)
examples/spring-cloud-config-xds/build.gradleexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.javasettings.gradlespring/boot4-xds/build.gradlespring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/package-info.javaspring/boot4-xds/src/main/resources/META-INF/armeria/xds/default-bootstrap.ymlspring/boot4-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProviderspring/boot4-xds/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.importsspring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsCustomBootstrapTest.javaspring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsYamlFileTest.javaspring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.ymlspring/boot4-xds/src/test/resources/application-xds-file-test.ymlxds-api/src/main/proto/armeria/xds/spring/spring_config_source.proto
🚧 Files skipped from review as they are similar to previous changes (4)
- xds-api/src/main/proto/armeria/xds/spring/spring_config_source.proto
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.java
- examples/spring-cloud-config-xds/build.gradle
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.java`:
- Around line 89-91: Update the public create method to call
Objects.requireNonNull on configSource, factoryContext, and interestedResources
before any dereferencing, using each parameter name as the explicit null-check
message.
🪄 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 Plus
Run ID: 2beac055-3e3f-45e1-a34f-72e8a5ecae90
📒 Files selected for processing (5)
examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.javaspring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.yml
💤 Files with no reviewable changes (1)
- spring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.java
- examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.java
jrhee17
force-pushed
the
poc/spring-cloud-config
branch
from
August 4, 2026 07:39
8ee0b9e to
10244f0
Compare
jrhee17
force-pushed
the
poc/spring-cloud-config
branch
from
August 4, 2026 07:41
10244f0 to
7149a6a
Compare
jrhee17
changed the title
spring-boot4-xds
jrhee17
changed the title
spring-boot4-xdsspring/boot4-xds
jrhee17
changed the title
spring/boot4-xdsboot4-xds
jrhee17
changed the title
boot4-xdsboot4-xds
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java (1)
58-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow and reorder the injected fields.
xdsWebClient,xdsBootstrap,environment, andapplicationContextare package-private and appear after the lifecycle methods. Declare them asprivateand place them with the other fields beforestartConfigServer.As per path instructions, use the narrowest possible visibility and organize members top-down.
🤖 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 `@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java` around lines 58 - 68, Update the injected fields xdsWebClient, xdsBootstrap, environment, and applicationContext in SpringCloudConfigXdsExampleTest to use private visibility, and move them before the startConfigServer lifecycle method alongside the other fields. Preserve their existing injection annotations and types.Source: Path instructions
🤖 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
`@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java`:
- Line 1: Add the repository’s standard LY Corporation Apache 2.0 copyright
header at the beginning of SpringCloudConfigXdsExampleTest.java, before the
package declaration; leave the test implementation unchanged.
---
Nitpick comments:
In
`@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java`:
- Around line 58-68: Update the injected fields xdsWebClient, xdsBootstrap,
environment, and applicationContext in SpringCloudConfigXdsExampleTest to use
private visibility, and move them before the startConfigServer lifecycle method
alongside the other fields. Preserve their existing injection annotations and
types.
🪄 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 Plus
Run ID: b65e04a4-69e0-4d89-9779-cb6c7f820f37
📒 Files selected for processing (25)
dependencies.tomlexamples/spring-cloud-config-xds/build.gradleexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.javaexamples/spring-cloud-config-xds/src/main/resources/config-repo/application.ymlexamples/spring-cloud-config-xds/src/main/resources/config/application-client.ymlexamples/spring-cloud-config-xds/src/main/resources/config/application-server.ymlexamples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.javasettings.gradlespring/boot4-xds/build.gradlespring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/package-info.javaspring/boot4-xds/src/main/resources/META-INF/armeria/xds/default-bootstrap.ymlspring/boot4-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProviderspring/boot4-xds/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.importsspring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsCustomBootstrapTest.javaspring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsYamlFileTest.javaspring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.ymlspring/boot4-xds/src/test/resources/application-xds-file-test.ymlxds-api/src/main/proto/armeria/xds/spring/spring_config_source.protoxds/src/main/java/com/linecorp/armeria/xds/XdsType.java
🚧 Files skipped from review as they are similar to previous changes (23)
- spring/boot4-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProvider
- spring/boot4-xds/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports
- xds-api/src/main/proto/armeria/xds/spring/spring_config_source.proto
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.java
- spring/boot4-xds/src/test/resources/application-xds-file-test.yml
- spring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsYamlFileTest.java
- spring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.yml
- spring/boot4-xds/src/main/resources/META-INF/armeria/xds/default-bootstrap.yml
- spring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsCustomBootstrapTest.java
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/package-info.java
- examples/spring-cloud-config-xds/src/main/resources/config/application-client.yml
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.java
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.java
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.java
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.java
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.java
- examples/spring-cloud-config-xds/build.gradle
- dependencies.toml
- examples/spring-cloud-config-xds/src/main/resources/config/application-server.yml
- settings.gradle
- examples/spring-cloud-config-xds/src/main/resources/config-repo/application.yml
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java
- xds/src/main/java/com/linecorp/armeria/xds/XdsType.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@examples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.java`:
- Around line 47-52: Update startConfigServer() to save the existing
spring.cloud.config.uri system property before replacing it, then update
stopConfigServer() to restore that saved value instead of unconditionally
clearing the property; preserve the property’s absence by clearing it only when
no prior value existed.
In
`@spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.java`:
- Line 34: Add Javadoc and the `@UnstableApi` annotation to the public override
createPropertySource in YamlPropertySourceFactory, placing both directly on the
method declaration while preserving its existing behavior.
- Around line 38-40: Update the source-name construction in
YamlPropertySourceFactory so an explicit name is used without first calling
resource.getResource().getFilename(); only resolve the filename when name is
null, and fall back to the resource description if that filename is unavailable.
Add a regression test using a filename-less resource with an explicit source
name.
🪄 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 Plus
Run ID: 340ea62c-df9e-43b1-a98f-e710224468c6
📒 Files selected for processing (25)
dependencies.tomlexamples/spring-cloud-config-xds/build.gradleexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.javaexamples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.javaexamples/spring-cloud-config-xds/src/main/resources/config-repo/application.ymlexamples/spring-cloud-config-xds/src/main/resources/config/application-client.ymlexamples/spring-cloud-config-xds/src/main/resources/config/application-server.ymlexamples/spring-cloud-config-xds/src/test/java/example/springframework/boot/xds/cloudconfig/SpringCloudConfigXdsExampleTest.javasettings.gradlespring/boot4-xds/build.gradlespring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/YamlPropertySourceFactory.javaspring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/package-info.javaspring/boot4-xds/src/main/resources/META-INF/armeria/xds/default-bootstrap.ymlspring/boot4-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProviderspring/boot4-xds/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.importsspring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsCustomBootstrapTest.javaspring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsYamlFileTest.javaspring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.ymlspring/boot4-xds/src/test/resources/application-xds-file-test.ymlxds-api/src/main/proto/armeria/xds/spring/spring_config_source.protoxds/src/main/java/com/linecorp/armeria/xds/XdsType.java
🚧 Files skipped from review as they are similar to previous changes (23)
- spring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsYamlFileTest.java
- examples/spring-cloud-config-xds/src/main/resources/config/application-client.yml
- examples/spring-cloud-config-xds/build.gradle
- examples/spring-cloud-config-xds/src/main/resources/config/application-server.yml
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/package-info.java
- spring/boot4-xds/src/test/resources/application-xds-custom-bootstrap-test.yml
- spring/boot4-xds/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports
- spring/boot4-xds/src/main/resources/META-INF/armeria/xds/default-bootstrap.yml
- settings.gradle
- spring/boot4-xds/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProvider
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsTypeRegistryPackageProvider.java
- spring/boot4-xds/src/test/resources/application-xds-file-test.yml
- spring/boot4-xds/build.gradle
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/XdsClientConfig.java
- xds-api/src/main/proto/armeria/xds/spring/spring_config_source.proto
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringXdsAutoConfiguration.java
- spring/boot4-xds/src/test/java/com/linecorp/armeria/spring/xds/SpringXdsCustomBootstrapTest.java
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/client/ClientMain.java
- dependencies.toml
- examples/spring-cloud-config-xds/src/main/resources/config-repo/application.yml
- xds/src/main/java/com/linecorp/armeria/xds/XdsType.java
- spring/boot4-xds/src/main/java/com/linecorp/armeria/spring/xds/SpringConfigSourceFactory.java
- examples/spring-cloud-config-xds/src/main/java/example/springframework/boot/xds/cloudconfig/server/ConfigServerMain.java
jrhee17
marked this pull request as ready for review
August 4, 2026 09:13
|
Tick the box to add this pull request to the merge queue (same as
|
minwoox
approved these changes
Aug 5, 2026
|
|
||
| @Override | ||
| public Iterable<String> packages() { | ||
| return List.of("com.linecorp.armeria.spring.xds"); |
| final YamlPropertiesFactoryBean factory = new YamlPropertiesFactoryBean(); | ||
| factory.setResources(resource.getResource()); | ||
| final Properties properties = requireNonNull(factory.getObject(), "properties"); | ||
| final String filename = requireNonNull(resource.getResource().getFilename(), "filename"); |
Contributor
There was a problem hiding this comment.
Can retrieve filename only when the name is null?
| * updated resources to subscribers. | ||
| */ | ||
| public void refresh() { | ||
| refreshSignal.push(); |
Contributor
There was a problem hiding this comment.
Is this okay for calling push from another thread?
| final SpringConfigSource springConfigSource = | ||
| factoryContext.validator().unpack(configSource.getCustomConfigSource().getTypedConfig(), | ||
| SpringConfigSource.class); | ||
| final String rawPrefix = springConfigSource.getPrefix(); |
|
|
||
| package armeria.xds.spring; | ||
|
|
||
| option java_package = "com.linecorp.armeria.spring.xds"; |
Contributor
There was a problem hiding this comment.
xds.spring like we did for Athenz?
ikhoon
approved these changes
Aug 11, 2026
| xds: | ||
| listener: | ||
| test-listener: | | ||
| name: test-listener |
Contributor
There was a problem hiding this comment.
Question) Could we use nested YAML syntax here instead?
| for (InterestedResources interested : interests.values()) { | ||
| try { | ||
| final DiscoveryResponse response = buildResponse(environment, prefix, interested); | ||
| watcher.onUpdate(response, null); |
Contributor
There was a problem hiding this comment.
I understand that dedup is already handled by ResourceStateStore.
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:
Users who want to define xDS resources (Listeners, Clusters, etc.) via Spring
Environmentproperties — such as those served by a Spring Cloud Config Server — currently have no built-in integration. They must manually parse bootstrap YAML, create anXdsBootstrap, and wire up property refresh. This PR provides a turnkey Spring Boot auto-configuration that handles all of this.Modifications:
spring/boot4-xdsmodule with:SpringXdsAutoConfiguration— auto-createsSpringConfigSourceFactory,XdsBootstrap, and anEnvironmentChangeEventlistener that triggers xDS refresh on property changesSpringConfigSourceFactory— aSotwConfigSourceSubscriptionFactorythat reads xDS resources from SpringEnvironmentproperties keyed by<prefix>.<resource-name>and pushes updates to subscribersarmeria.xds.spring.SpringConfigSourceprotobuf definition inxds-apiXdsType.resourceClass()to support type-driven proto parsingexamples/spring-cloud-config-xdsexample module demonstrating a two-server setup (Config Server + Client) with xDS-resolvedWebClientResult:
application.yml(or via Spring Cloud Config Server) and have them automatically loaded into anXdsBootstrapwith zero boilerplate