feat!: migrate apollo server baseline to spring boot 4.0.x#5585
Merged
nobodyiam merged 3 commits intoapolloconfig:masterfrom Mar 28, 2026
Merged
feat!: migrate apollo server baseline to spring boot 4.0.x#5585nobodyiam merged 3 commits intoapolloconfig:masterfrom
nobodyiam merged 3 commits intoapolloconfig:masterfrom
Conversation
dosubot
bot
added
the
size:L
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:
📝 WalkthroughWalkthroughUpgrades Spring Boot/Cloud/Alibaba BOMs to Spring Boot 4, migrates discovery integrations to Spring Cloud starters, replaces many Hibernate Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions (external-discovery-smoke)
participant Build as build-services job
participant Art as Artifact store
participant Runner as provider matrix job
participant Provider as Discovery provider (nacos/consul/zookeeper)
participant Admin as apollo-adminservice (java -jar)
participant Config as apollo-configservice (java -jar)
participant E2E as run-smoke.sh
rect rgba(100,149,237,0.5)
GH->>Build: trigger (PR or workflow_dispatch)
Build->>Build: mvn assemble admin & config jars
Build->>Art: upload apollo-discovery-jars
end
rect rgba(60,179,113,0.5)
GH->>Runner: start matrix job (provider)
Runner->>Art: download apollo-discovery-jars
Runner->>Provider: provider.sh start <provider> -> Docker run
Provider-->>Runner: container ready + env file
Runner->>Admin: start java -jar adminservice (provider profile) & record PID
Runner->>Config: start java -jar configservice (provider profile) & record PID
Runner->>E2E: run-smoke.sh (DISCOVERY_PROVIDER, ARTIFACT_DIR)
E2E->>Provider: assert-service (verify registrations)
E2E->>Admin: fetch /services/admin
E2E->>Config: fetch /services/config
E2E-->>Runner: pass/fail
Runner->>Provider: provider.sh logs / stop
Runner->>Admin: kill PID
Runner->>Config: kill PID
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceHealthIndicator.java (1)
34-38:⚠️ Potential issue | 🟡 MinorHealth check silently swallows exceptions.
If
check()throws an exception (e.g., database unreachable), the method still returnsHealth.up(). Consider propagating failures:🛡️ Suggested fix to propagate health check failures
`@Override` public Health health() { - check(); - return Health.up().build(); + try { + check(); + return Health.up().build(); + } catch (Exception e) { + return Health.down(e).build(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceHealthIndicator.java` around lines 34 - 38, The health() method in AdminServiceHealthIndicator calls check() but always returns Health.up(), so any exception from check() is swallowed; wrap the check() call in a try/catch (catch Exception or Throwable) and on failure return Health.down().withDetail("error", e.getMessage() /* and optionally stack or class */).build(); on success return Health.up().build(); reference AdminServiceHealthIndicator.health() and the check() method when making the change.apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceHealthIndicator.java (1)
34-38:⚠️ Potential issue | 🟡 MinorSame exception-swallowing issue as AdminServiceHealthIndicator.
The
health()method does not propagate exceptions fromcheck(). Consider applying the same fix pattern suggested forAdminServiceHealthIndicator.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceHealthIndicator.java` around lines 34 - 38, The health() method in ConfigServiceHealthIndicator currently calls check() and always returns Health.up(), which swallows exceptions; update health() (in class ConfigServiceHealthIndicator) to follow the same pattern used for AdminServiceHealthIndicator: call check() inside a try/catch, return Health.up() when check() succeeds and return Health.down().withDetail("error", e.getMessage()) (or include exception details) when an exception is caught, ensuring exceptions from check() are not silently ignored but reflected in the Health response.apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditHttpInterceptorTest.java (1)
31-39:⚠️ Potential issue | 🟠 MajorRemove
@ContextConfigurationor add@ComponenttoApolloAuditHttpInterceptor.
ApolloAuditHttpInterceptoris not annotated with@Component,@Service, or any other Spring stereotype. Using@ContextConfiguration(classes = ApolloAuditHttpInterceptor.class)treats it as a configuration class rather than registering it as a bean in the Spring context. Since@MockitoSpyBeanrequires an existing bean instance to wrap, the test configuration will fail.Either add
@Componentto the class (if it should be a Spring-managed bean) or simplify the test by removing@ContextConfigurationand letting@MockitoSpyBeanand@MockitoBeanhandle bean creation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditHttpInterceptorTest.java` around lines 31 - 39, The test uses `@ContextConfiguration`(classes = ApolloAuditHttpInterceptor.class) but ApolloAuditHttpInterceptor is not a Spring stereotype, so `@MockitoSpyBean` cannot wrap it; either remove the `@ContextConfiguration` annotation from ApolloAuditHttpInterceptorTest so Mockito annotations create/mange the bean, or annotate the ApolloAuditHttpInterceptor class with `@Component` (or another Spring stereotype) so it is registered as a bean; update the test to keep `@MockitoSpyBean` ApolloAuditHttpInterceptor interceptor and `@MockitoBean` ApolloAuditTraceContext traceContext and ensure only one of these fixes is applied.apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.java (1)
88-89:⚠️ Potential issue | 🔴 CriticalFix:
@Captoris not initialized, causing NPE in tests.The pipeline failures show
influenceCaptoris null. The@Captorannotation requires Mockito initialization, which is not happening with the new@MockitoBean/@MockitoSpyBeanannotations.With
@SpringBootTest, you need to explicitly enable Mockito annotation processing for@Captor.🐛 Proposed fix: Initialize Mockito annotations in `@BeforeEach`
import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; import org.springframework.boot.test.context.SpringBootTest;+ private AutoCloseable mockitoCloseable; + `@BeforeEach` void beforeEach() { + mockitoCloseable = MockitoAnnotations.openMocks(this); Mockito.reset(traceContext, tracer); Mockito.when(traceContext.tracer()).thenReturn(tracer); } + + `@org.junit.jupiter.api.AfterEach` + void afterEach() throws Exception { + mockitoCloseable.close(); + }Alternatively, add
@ExtendWith(MockitoExtension.class)to the test class, though mixing with@SpringBootTestrequires care.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.java` around lines 88 - 89, The test class ApolloAuditLogApiJpaImplTest has an uninitialized `@Captor` field influenceCaptor causing NPEs; initialize Mockito annotations during test setup by adding a `@BeforeEach` setup method that calls MockitoAnnotations.openMocks(this) (or alternatively annotate the test class with `@ExtendWith`(MockitoExtension.class) if you prefer), ensuring influenceCaptor is properly created before tests run.
🧹 Nitpick comments (5)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/PortalOpenApiConfig.java (1)
33-43: Consider callingsuper.customize(factory)for consistency and maintainability.While the parent
WebMvcConfig.customize()method (which sets up MIME mappings) will still be invoked via the parent bean instance due to Spring's WebServerFactoryCustomizer chaining, callingsuper.customize(factory)here makes the inheritance and intended behavior explicit and improves maintainability if the parent configuration ever changes.Suggested improvement
`@Override` public void customize(TomcatServletWebServerFactory factory) { + super.customize(factory); final String relaxedChars = "<>[\\]^`{|}"; final String tomcatRelaxedPathCharsProperty = "relaxedPathChars"; final String tomcatRelaxedQueryCharsProperty = "relaxedQueryChars";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/PortalOpenApiConfig.java` around lines 33 - 43, The override of customize in PortalWebMvcConfig should explicitly invoke the parent behavior: add a call to super.customize(factory) at the start of PortalWebMvcConfig.customize(TomcatServletWebServerFactory factory) so WebMvcConfig.customize(...) (e.g., MIME mappings) runs before you set Tomcat relaxed chars; keep the existing connector.addConnectorCustomizers(...) logic after that call to preserve the current customizations.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.java (2)
192-200: Consider removing the wrapped trailing note for readability.The multi-line inline note after the assertion is noisy and repeats scenario context already documented in method comments.
Small cleanup
- .andExpect(header().string("Location", endsWith("/signin"))); // should - // be - // aligned - // with - // 2.1-2 - // portal - // calling - // portal + .andExpect(header().string("Location", endsWith("/signin")));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.java` around lines 192 - 200, Remove the wrapped trailing inline note that follows the header().string("Location", endsWith("/signin")) assertion in PortalOpenApiAuthenticationScenariosTest (the noisy multi-line comment block after the .andExpect(...) call); either delete it entirely or move any non-duplicate context into the method-level Javadoc so the assertion line stays clean and readable.
43-45: Migrate this touched test to JUnit 5.This file uses modern Spring test wiring (
@MockitoBean,@SpringBootTest) but still runs with JUnit 4 (@RunWith, org.junit.Test,@After, org.junit.Assert). Since the file is actively updated and the coding guidelines specify JUnit 5 as the default testing framework, aligning it would reduce mixed-runner drift.Migration changes needed
-import org.junit.After; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; ... -@RunWith(org.springframework.test.context.junit4.SpringRunner.class) `@SpringBootTest`(classes = PortalOpenApiAuthenticationScenariosTest.TestApplication.class) `@AutoConfigureMockMvc` ... - `@After` + `@AfterEach` public void tearDown() { reset(consumerAuthUtil, consumerAuditUtil); } ... - org.junit.Assert.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + Assertions.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.java` around lines 43 - 45, The test class still uses JUnit 4 annotations and imports; migrate it to JUnit 5 by replacing the JUnit 4 runner and annotations: remove `@RunWith` and org.junit imports and add `@ExtendWith`(SpringExtension.class) (or rely on `@SpringBootTest` which auto-registers the extension), change `@org.junit.Test` to `@org.junit.jupiter.api.Test`, replace `@After` with `@org.junit.jupiter.api.AfterEach`, and swap junit.framework/org.junit.Assert assertions to org.junit.jupiter.api.Assertions (e.g., Assertions.assertEquals). Also update import statements accordingly and ensure any Mockito-related annotations (like `@MockitoBean`) remain compatible with JUnit 5.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java (1)
70-95: HttpClient 5 timeout configuration is correctly structured.The timeout separation into
ConnectionConfig(socket connect + TTL) andRequestConfig(response/read timeout) aligns with HttpClient 5's configuration model.Minor observation: Line 91 reuses
portalConfig.connectTimeout()forconnectionRequestTimeout(time waiting for a pooled connection). While this works, these represent different concerns—consider whether a separate configuration property would provide better operational flexibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java` around lines 70 - 95, The code uses portalConfig.connectTimeout() for the connectionRequestTimeout in afterPropertiesSet; add a dedicated config property (e.g., portalConfig.connectionRequestTimeout() or portalConfig.connectPoolWaitTimeout()) and use it when calling requestFactory.setConnectionRequestTimeout(Duration.ofMillis(...)) instead of reusing portalConfig.connectTimeout(), keeping existing ConnectionConfig and RequestConfig behavior (references: afterPropertiesSet, ConnectionConfig.custom(), RequestConfig.custom(), requestFactory.setConnectionRequestTimeout). Update the portalConfig interface/implementation to expose the new property with a sensible default and wire it into configuration so operational teams can tune pooled-connection wait time separately from socket connect timeout.e2e/scripts/apollo-smoke-lib.sh (1)
91-97: Avoid repeated state-mutating warm-up writes during readiness polling.Line 146 calls
apollo_warm_up_portal_admin_pathon every retry, and Line 96 generates a new app id each time. During a slow startup window, this can create many warm-up apps/releases before readiness succeeds.♻️ Proposed refactor (reuse one app id across retries)
-apollo_warm_up_portal_admin_path() { - local app_id cookie_file app_payload item_payload release_payload +apollo_warm_up_portal_admin_path() { + local app_id cookie_file app_payload item_payload release_payload local app_status item_status release_status cookie_file="$(mktemp)" - app_id="warmup$(date +%s)$RANDOM" + app_id="${1:-warmup$(date +%s)$RANDOM}" @@ wait_for_apollo_portal_admin_path() { - local deadline + local deadline warmup_app_id @@ apollo_smoke_init_env deadline=$((SECONDS + WAIT_TIMEOUT_SECONDS)) + warmup_app_id="warmup$(date +%s)$RANDOM" @@ - elif apollo_probe "${PORTAL_URL}/signin" && apollo_warm_up_portal_admin_path; then + elif apollo_probe "${PORTAL_URL}/signin" && apollo_warm_up_portal_admin_path "$warmup_app_id"; then echo "Portal/Admin path is ready" return 0 fiAlso applies to: 139-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/scripts/apollo-smoke-lib.sh` around lines 91 - 97, The warm-up helper apollo_warm_up_portal_admin_path currently generates a new app_id and temp cookie on every call causing repeated state-mutating writes across readiness retries; refactor so a single app_id (and cookie_file if needed) is created once and reused across retries—either move the app_id/cookie generation out of apollo_warm_up_portal_admin_path into the readiness loop and pass them in as parameters, or make apollo_warm_up_portal_admin_path accept an optional app_id/cookie_file and only create them when not provided, ensuring the same app_id is reused for all retry attempts to prevent many warm-up apps/releases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apollo-adminservice/src/main/scripts/shutdown.sh`:
- Around line 33-36: pgrep -f $SERVICE_NAME can return multiple PIDs but the
current code assigns them to a single variable "pid" and then calls kill "$pid"
which treats the whole string as one PID; change shutdown.sh to handle multiple
PIDs returned by pgrep -f $SERVICE_NAME (the SERVICE_NAME/pid variables) by
either iterating over the PIDs (e.g., for p in $pid; do kill "$p"; done) or by
calling kill without quoting the PID list (kill $pid) so word-splitting passes
each PID to kill; ensure the script still checks for a non-empty result before
attempting to kill.
In
`@apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.java`:
- Around line 37-41: The test fails because `@MockitoSpyBean` is targeting
ApolloAuditOperatorDefaultSupplier but the application context only loads
ApolloAuditOperatorSupplier.class; update the test context so the implementation
bean exists by adding ApolloAuditOperatorDefaultSupplier.class to the
`@ContextConfiguration` (i.e. load both ApolloAuditOperatorSupplier and
ApolloAuditOperatorDefaultSupplier) so the `@MockitoSpyBean` can wrap the existing
bean.
In
`@apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/NamespaceLock.java`:
- Around line 27-29: The NamespaceLock entity is missing a soft-delete
annotation so deleteByNamespaceId() currently performs hard deletes; add an
`@SQLDelete` on the NamespaceLock entity (paired with the existing `@SQLRestriction`
and BaseEntity soft-delete pattern) that updates IsDeleted = true and sets
DeletedAt (timestamp) in the WHERE Id = ? clause so deletes mark records as
deleted instead of removing them; update the NamespaceLock class declaration to
include this `@SQLDelete` annotation analogous to other entities (Item, Cluster,
Audit) that implement soft-delete.
In `@apollo-configservice/src/main/scripts/shutdown.sh`:
- Around line 33-36: pgrep -f $SERVICE_NAME can return multiple PIDs, and the
current code assigns them to a single variable `pid` and calls `kill "$pid"`
which may fail; update the shutdown logic in shutdown.sh to handle multiple
results from `pgrep -f $SERVICE_NAME` by splitting on newlines and either piping
results to xargs (e.g., pgrep -f "$SERVICE_NAME" | xargs -r kill) or iterating
over each PID (readarray or while read -r pid; do kill "$pid"; done), ensure you
check for an empty result before killing and preserve use of the `SERVICE_NAME`
and `pid` identifiers in the updated logic.
In `@apollo-portal/pom.xml`:
- Around line 72-75: The pom includes the deprecated artifact
spring-boot-jackson2 which is only for Jackson 2 backwards-compatibility; either
remove this dependency or explicitly justify/lock it: if the project is
migrating to Spring Boot 4, remove spring-boot-jackson2 and adopt the Spring
Boot 4 default JSON handling (Jackson 3) or replace with the appropriate Jackson
3 dependency; if you must keep Jackson 2 while on Spring Boot 2.x, add a comment
and pin the dependency/version and include rationale (or a migration task) to
avoid accidental upgrade. Target symbols: artifactId "spring-boot-jackson2" and
the dependency block in the POM.
In `@apollo-portal/src/main/scripts/shutdown.sh`:
- Around line 33-36: The shutdown script currently assigns pid=$(pgrep -f
$SERVICE_NAME) and calls kill "$pid", which fails when pgrep returns multiple
newline-separated PIDs; change the logic in shutdown.sh to handle multiple PIDs
by capturing the output of pgrep -f "$SERVICE_NAME" and either piping it to
xargs (e.g., pgrep -f "$SERVICE_NAME" | xargs -r kill) or iterating over each
PID in a loop (for pid in $(pgrep -f "$SERVICE_NAME"); do kill "$pid"; done),
keeping SERVICE_NAME quoted and preserving the existing check for non-empty
results before attempting to kill.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AbstractControllerTest.java`:
- Line 23: The test currently imports and uses the deprecated
HttpMessageConverters; update the test to use the non-deprecated
ServerHttpMessageConvertersCustomizer (server-side) or
ClientHttpMessageConvertersCustomizer (client-side) depending on what
AbstractControllerTest exercises: replace usages of HttpMessageConverters with
the appropriate Customizer type, adjust any setup calls to register or configure
converters via ServerHttpMessageConvertersCustomizer (or
ClientHttpMessageConvertersCustomizer) in the test harness, or if you must keep
the deprecated class temporarily, add a clear comment and
`@SuppressWarnings`("deprecation") around the import and its usages; look for
references to HttpMessageConverters in AbstractControllerTest to locate where to
change the wiring.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.java`:
- Around line 19-21: The test class RestTemplateFactoryTest is using JUnit 4
imports and lacks class Javadoc; update imports and annotations to JUnit 5
(replace org.junit.Before with org.junit.jupiter.api.BeforeEach, org.junit.Test
with org.junit.jupiter.api.Test, and use
org.junit.jupiter.api.Assertions.assertEquals/assertSame) and remove any
Mockito/JUnit4-specific runners if present; add a brief class-level Javadoc
above the RestTemplateFactoryTest class that states its purpose (e.g., "Unit
tests for RestTemplateFactory behavior"), and update any setup methods (e.g.,
setUp) to use `@BeforeEach` and adjust static import statements accordingly.
In `@e2e/discovery-smoke/scripts/provider.sh`:
- Around line 105-113: The nacos case only publishes the HTTP port 8848; add an
explicit Docker port mapping for Nacos' gRPC port (main_port + 1000 => 9848) so
external clients can reach gRPC. Modify the nacos branch where docker run is
invoked (case on DISCOVERY_PROVIDER, variables PROVIDER_CONTAINER_NAME and
PROVIDER_PORT) to also include a -p binding for 9848 (for example map host
127.0.0.1:${PROVIDER_PORT_PLUS_1000}:9848 or compute $((PROVIDER_PORT+1000))) so
the container exposes the gRPC port alongside 8848. Ensure the added -p follows
the same 127.0.0.1 binding pattern as the existing HTTP port mapping.
In `@e2e/README.md`:
- Around line 112-118: Update the README's local smoke example to pick the
runnable jar explicitly for the two java -jar invocations (the lines invoking
apollo-configservice and apollo-adminservice) instead of using the ambiguous
glob patterns; change the selection to a deterministic executable artifact (for
example use the executable classifier such as -exec.jar or filter out
-*sources.jar and -*javadoc.jar when resolving the filename) so java -jar always
receives the actual runnable service JAR.
In `@e2e/scripts/apollo-smoke-lib.sh`:
- Around line 24-25: Remove the hardcoded fallback assignments for
PORTAL_USERNAME and PORTAL_PASSWORD (the lines that use
${PORTAL_USERNAME:-apollo} and ${PORTAL_PASSWORD:-admin}); instead read these
env vars without defaults and add a guard that exits with a clear error if
either PORTAL_USERNAME or PORTAL_PASSWORD is unset/empty so callers must provide
credentials via environment/local override (.env, CI secrets, etc.). Ensure the
check references the PORTAL_USERNAME and PORTAL_PASSWORD variables and prints a
short message guiding users to set them before continuing.
- Around line 18-37: The curl calls in apollo_probe and
apollo_portal_http_status can hang indefinitely; update both functions
(apollo_probe and apollo_portal_http_status) to pass explicit curl timeout flags
(e.g., --connect-timeout and --max-time) using the WAIT_TIMEOUT_SECONDS or a
smaller derived value so each curl invocation cannot block beyond a reasonable
period and the overall WAIT_TIMEOUT_SECONDS enforcement remains reliable.
---
Outside diff comments:
In
`@apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceHealthIndicator.java`:
- Around line 34-38: The health() method in AdminServiceHealthIndicator calls
check() but always returns Health.up(), so any exception from check() is
swallowed; wrap the check() call in a try/catch (catch Exception or Throwable)
and on failure return Health.down().withDetail("error", e.getMessage() /* and
optionally stack or class */).build(); on success return Health.up().build();
reference AdminServiceHealthIndicator.health() and the check() method when
making the change.
In
`@apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditHttpInterceptorTest.java`:
- Around line 31-39: The test uses `@ContextConfiguration`(classes =
ApolloAuditHttpInterceptor.class) but ApolloAuditHttpInterceptor is not a Spring
stereotype, so `@MockitoSpyBean` cannot wrap it; either remove the
`@ContextConfiguration` annotation from ApolloAuditHttpInterceptorTest so Mockito
annotations create/mange the bean, or annotate the ApolloAuditHttpInterceptor
class with `@Component` (or another Spring stereotype) so it is registered as a
bean; update the test to keep `@MockitoSpyBean` ApolloAuditHttpInterceptor
interceptor and `@MockitoBean` ApolloAuditTraceContext traceContext and ensure
only one of these fixes is applied.
In
`@apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.java`:
- Around line 88-89: The test class ApolloAuditLogApiJpaImplTest has an
uninitialized `@Captor` field influenceCaptor causing NPEs; initialize Mockito
annotations during test setup by adding a `@BeforeEach` setup method that calls
MockitoAnnotations.openMocks(this) (or alternatively annotate the test class
with `@ExtendWith`(MockitoExtension.class) if you prefer), ensuring
influenceCaptor is properly created before tests run.
In
`@apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceHealthIndicator.java`:
- Around line 34-38: The health() method in ConfigServiceHealthIndicator
currently calls check() and always returns Health.up(), which swallows
exceptions; update health() (in class ConfigServiceHealthIndicator) to follow
the same pattern used for AdminServiceHealthIndicator: call check() inside a
try/catch, return Health.up() when check() succeeds and return
Health.down().withDetail("error", e.getMessage()) (or include exception details)
when an exception is caught, ensuring exceptions from check() are not silently
ignored but reflected in the Health response.
---
Nitpick comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/PortalOpenApiConfig.java`:
- Around line 33-43: The override of customize in PortalWebMvcConfig should
explicitly invoke the parent behavior: add a call to super.customize(factory) at
the start of PortalWebMvcConfig.customize(TomcatServletWebServerFactory factory)
so WebMvcConfig.customize(...) (e.g., MIME mappings) runs before you set Tomcat
relaxed chars; keep the existing connector.addConnectorCustomizers(...) logic
after that call to preserve the current customizations.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java`:
- Around line 70-95: The code uses portalConfig.connectTimeout() for the
connectionRequestTimeout in afterPropertiesSet; add a dedicated config property
(e.g., portalConfig.connectionRequestTimeout() or
portalConfig.connectPoolWaitTimeout()) and use it when calling
requestFactory.setConnectionRequestTimeout(Duration.ofMillis(...)) instead of
reusing portalConfig.connectTimeout(), keeping existing ConnectionConfig and
RequestConfig behavior (references: afterPropertiesSet,
ConnectionConfig.custom(), RequestConfig.custom(),
requestFactory.setConnectionRequestTimeout). Update the portalConfig
interface/implementation to expose the new property with a sensible default and
wire it into configuration so operational teams can tune pooled-connection wait
time separately from socket connect timeout.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.java`:
- Around line 192-200: Remove the wrapped trailing inline note that follows the
header().string("Location", endsWith("/signin")) assertion in
PortalOpenApiAuthenticationScenariosTest (the noisy multi-line comment block
after the .andExpect(...) call); either delete it entirely or move any
non-duplicate context into the method-level Javadoc so the assertion line stays
clean and readable.
- Around line 43-45: The test class still uses JUnit 4 annotations and imports;
migrate it to JUnit 5 by replacing the JUnit 4 runner and annotations: remove
`@RunWith` and org.junit imports and add `@ExtendWith`(SpringExtension.class) (or
rely on `@SpringBootTest` which auto-registers the extension), change
`@org.junit.Test` to `@org.junit.jupiter.api.Test`, replace `@After` with
`@org.junit.jupiter.api.AfterEach`, and swap junit.framework/org.junit.Assert
assertions to org.junit.jupiter.api.Assertions (e.g., Assertions.assertEquals).
Also update import statements accordingly and ensure any Mockito-related
annotations (like `@MockitoBean`) remain compatible with JUnit 5.
In `@e2e/scripts/apollo-smoke-lib.sh`:
- Around line 91-97: The warm-up helper apollo_warm_up_portal_admin_path
currently generates a new app_id and temp cookie on every call causing repeated
state-mutating writes across readiness retries; refactor so a single app_id (and
cookie_file if needed) is created once and reused across retries—either move the
app_id/cookie generation out of apollo_warm_up_portal_admin_path into the
readiness loop and pass them in as parameters, or make
apollo_warm_up_portal_admin_path accept an optional app_id/cookie_file and only
create them when not provided, ensuring the same app_id is reused for all retry
attempts to prevent many warm-up apps/releases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a693d9d0-c689-4375-9f1f-3ab95306c464
📒 Files selected for processing (104)
.github/workflows/external-discovery-smoke.yml.github/workflows/portal-login-e2e.yml.github/workflows/portal-ui-e2e.ymlCHANGES.mdapollo-adminservice/pom.xmlapollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceApplication.javaapollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceHealthIndicator.javaapollo-adminservice/src/main/resources/application-consul-discovery.propertiesapollo-adminservice/src/main/resources/application-nacos-discovery.propertiesapollo-adminservice/src/main/resources/application-zookeeper-discovery.propertiesapollo-adminservice/src/main/scripts/shutdown.shapollo-adminservice/src/main/scripts/startup.shapollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/GracefulShutdownConfigurationTest.javaapollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/AbstractControllerTest.javaapollo-assembly/src/main/java/com/ctrip/framework/apollo/assembly/ApolloApplication.javaapollo-assembly/src/test/java/com/ctrip/framework/apollo/assembly/LocalApolloApplication.javaapollo-audit/apollo-audit-impl/pom.xmlapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/aop/ApolloAuditSpanAspectTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditHttpInterceptorTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditScopeManagerTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/context/ApolloAuditTraceContextTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/context/ApolloAuditTracerTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/controller/ApolloAuditControllerTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/ApolloBizAssemblyConfiguration.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/AccessKey.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Audit.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Cluster.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Commit.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/GrayReleaseRule.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Item.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Namespace.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/NamespaceLock.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Privilege.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Release.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ReleaseHistory.javaapollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ServerConfig.javaapollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemSetServiceTest.javaapollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/NamespaceServiceIntegrationTest.javaapollo-common/src/main/java/com/ctrip/framework/apollo/common/controller/HttpMessageConverterConfiguration.javaapollo-common/src/main/java/com/ctrip/framework/apollo/common/controller/WebMvcConfig.javaapollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/App.javaapollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/AppNamespace.javaapollo-configservice/pom.xmlapollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServerEurekaServerConfigure.javaapollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceApplication.javaapollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceHealthIndicator.javaapollo-configservice/src/main/java/com/ctrip/framework/apollo/metaservice/service/NacosDiscoveryService.javaapollo-configservice/src/main/java/com/ctrip/framework/apollo/metaservice/service/SpringCloudInnerDiscoveryService.javaapollo-configservice/src/main/resources/application-consul-discovery.propertiesapollo-configservice/src/main/resources/application-nacos-discovery.propertiesapollo-configservice/src/main/resources/application-zookeeper-discovery.propertiesapollo-configservice/src/main/scripts/shutdown.shapollo-configservice/src/main/scripts/startup.shapollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/GracefulShutdownConfigurationTest.javaapollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/integration/AbstractBaseIntegrationTest.javaapollo-configservice/src/test/java/com/ctrip/framework/apollo/metaservice/service/NacosDiscoveryServiceTest.javaapollo-portal/pom.xmlapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/PortalOpenApiConfig.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/Consumer.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerRole.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerToken.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/PortalApplication.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/PortalAssemblyConfiguration.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/AdminServiceAddressLocator.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/ConfigReleaseWebhookNotifier.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/PortalSettings.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RetryableRestTemplate.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/SystemInfoController.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Favorite.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Permission.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Role.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/RolePermission.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/ServerConfig.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/UserRole.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.javaapollo-portal/src/main/scripts/shutdown.shapollo-portal/src/main/scripts/startup.shapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AbstractControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AppControllerParamBindLowLevelTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AppControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterControllerParamBindLowLevelTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/EnvControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/AbstractIntegrationTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ClusterControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/InstanceControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ItemControllerAuthIntegrationTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PermissionControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SystemInfoControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/AppServiceTest.javae2e/README.mde2e/discovery-smoke/scripts/provider.she2e/discovery-smoke/scripts/run-smoke.she2e/portal-e2e/scripts/wait-for-ready.she2e/scripts/apollo-smoke-lib.shpom.xml
💤 Files with no reviewable changes (1)
- apollo-configservice/src/main/java/com/ctrip/framework/apollo/metaservice/service/NacosDiscoveryService.java
...impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/NamespaceLock.java
Show resolved
Hide resolved
...ortal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
e2e/discovery-smoke/scripts/provider.sh (1)
105-115:⚠️ Potential issue | 🟠 MajorNacos container still misses gRPC port publishing.
The
nacosbranch only binds8848. External clients typically need the gRPC port (main + 1000, i.e.9848) reachable as well, so discovery assertions can fail even when HTTP health checks pass.🛠️ Proposed fix
nacos) docker run -d \ --name "${PROVIDER_CONTAINER_NAME}" \ -p "127.0.0.1:${PROVIDER_PORT}:8848" \ + -p "127.0.0.1:$((PROVIDER_PORT + 1000)):9848" \ -e MODE=standalone \ -e NACOS_AUTH_ENABLE=false \ -e NACOS_AUTH_TOKEN="MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=" \#!/bin/bash set -euo pipefail # Verify current Nacos port bindings in this script. rg -n -C3 'nacos\)|-p "127\.0\.0\.1:\$\{PROVIDER_PORT\}:8848"|9848' e2e/discovery-smoke/scripts/provider.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/discovery-smoke/scripts/provider.sh` around lines 105 - 115, The nacos case in the DISCOVERY_PROVIDER switch starts the container binding only port 8848, but external clients also need the gRPC port (PROVIDER_PORT+1000, e.g. 9848) published; update the docker run invocation in the nacos branch (the block that references PROVIDER_CONTAINER_NAME and PROVIDER_PORT) to add a -p "127.0.0.1:${PROVIDER_PORT+1000}:9848" (or explicitly -p "127.0.0.1:${PROVIDER_PORT}:8848" and -p "127.0.0.1:${PROVIDER_PORT_GRPC}:9848") so the gRPC port is reachable for discovery assertions. Ensure the new port mapping follows the same host binding style as the existing 8848 mapping and keep the container image nacos/nacos-server:v3.1.1-slim unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/discovery-smoke/scripts/provider.sh`:
- Around line 110-114: Remove the hardcoded Nacos credential-like environment
variables from the script: delete the NACOS_AUTH_TOKEN, NACOS_AUTH_IDENTITY_KEY,
and NACOS_AUTH_IDENTITY_VALUE lines (and keep NACOS_AUTH_ENABLE=false) in the
env block where MODE is set, or change the script to source these values from a
local/CI-only override when NACOS_AUTH_ENABLE is true; locate the env block that
sets MODE, NACOS_AUTH_ENABLE, NACOS_AUTH_TOKEN, NACOS_AUTH_IDENTITY_KEY and
NACOS_AUTH_IDENTITY_VALUE and either remove the token/identity lines entirely or
add conditional logic to load them only from an external file/secure variable
when NACOS_AUTH_ENABLE is enabled.
---
Duplicate comments:
In `@e2e/discovery-smoke/scripts/provider.sh`:
- Around line 105-115: The nacos case in the DISCOVERY_PROVIDER switch starts
the container binding only port 8848, but external clients also need the gRPC
port (PROVIDER_PORT+1000, e.g. 9848) published; update the docker run invocation
in the nacos branch (the block that references PROVIDER_CONTAINER_NAME and
PROVIDER_PORT) to add a -p "127.0.0.1:${PROVIDER_PORT+1000}:9848" (or explicitly
-p "127.0.0.1:${PROVIDER_PORT}:8848" and -p
"127.0.0.1:${PROVIDER_PORT_GRPC}:9848") so the gRPC port is reachable for
discovery assertions. Ensure the new port mapping follows the same host binding
style as the existing 8848 mapping and keep the container image
nacos/nacos-server:v3.1.1-slim unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efaea8f9-52cf-4aed-b9bc-0ce8667b60a0
📒 Files selected for processing (6)
.github/workflows/external-discovery-smoke.ymlapollo-assembly/src/main/resources/application.ymlapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.javaapollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.javae2e/discovery-smoke/scripts/provider.sh
✅ Files skipped from review due to trivial changes (2)
- apollo-assembly/src/main/resources/application.yml
- .github/workflows/external-discovery-smoke.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.java
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.java
- apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.java
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/AbstractControllerTest.java (1)
20-23: Migrate lifecycle methods to JUnit 5 annotations.Per coding guidelines, test files should use JUnit 5 as the default testing framework. Since this file is being actively modified, replace
@Before/@Afterwith@BeforeEach/@AfterEachand replace@RunWith(SpringJUnit4ClassRunner.class)with JUnit 5 extension style. The project has JUnit 5 infrastructure (Vintage) available for supporting legacy JUnit 4 tests, so only truly legacy tests should remain on JUnit 4.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/AbstractControllerTest.java` around lines 20 - 23, Replace JUnit4 lifecycle and runner annotations in AbstractControllerTest: change `@Before` and `@After` to JUnit 5 equivalents `@BeforeEach` and `@AfterEach` and update their imports from org.junit.Before/org.junit.After to org.junit.jupiter.api.BeforeEach/org.junit.jupiter.api.AfterEach; remove the `@RunWith`(SpringJUnit4ClassRunner.class) usage and its import and instead annotate the class with `@ExtendWith`(SpringExtension.class) and add the import org.springframework.test.context.junit.jupiter.SpringExtension (or appropriate JUnit5 Spring extension); ensure any Mockito initialization using MockitoAnnotations.openMocks(this) remains called in the `@BeforeEach` method and close the AutoCloseable in `@AfterEach` if used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/AbstractControllerTest.java`:
- Around line 20-23: Replace JUnit4 lifecycle and runner annotations in
AbstractControllerTest: change `@Before` and `@After` to JUnit 5 equivalents
`@BeforeEach` and `@AfterEach` and update their imports from
org.junit.Before/org.junit.After to
org.junit.jupiter.api.BeforeEach/org.junit.jupiter.api.AfterEach; remove the
`@RunWith`(SpringJUnit4ClassRunner.class) usage and its import and instead
annotate the class with `@ExtendWith`(SpringExtension.class) and add the import
org.springframework.test.context.junit.jupiter.SpringExtension (or appropriate
JUnit5 Spring extension); ensure any Mockito initialization using
MockitoAnnotations.openMocks(this) remains called in the `@BeforeEach` method and
close the AutoCloseable in `@AfterEach` if used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f672f283-715d-4866-af36-a5619967c222
📒 Files selected for processing (5)
apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/AbstractControllerTest.javaapollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.javaapollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ReleaseHistoryServiceTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/AbstractIntegrationTest.javae2e/discovery-smoke/scripts/provider.sh
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
e2e/discovery-smoke/scripts/provider.sh (1)
121-123:⚠️ Potential issue | 🟠 MajorRemove hardcoded Nacos credential-like env values from the script.
Line 121 through Line 123 still commit token/identity-like values. Even with
NACOS_AUTH_ENABLE=false, these trigger secret scanners and weaken compliance posture. Keep these out of VCS; source them only from local/CI overrides when auth is enabled.🛠️ Proposed fix
nacos) docker run -d \ --name "${PROVIDER_CONTAINER_NAME}" \ -p "127.0.0.1:${PROVIDER_PORT}:8848" \ -p "127.0.0.1:${PROVIDER_GRPC_PORT}:9848" \ -p "127.0.0.1:${PROVIDER_RAFT_PORT}:9849" \ -e MODE=standalone \ -e NACOS_AUTH_ENABLE=false \ - -e NACOS_AUTH_TOKEN="MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=" \ - -e NACOS_AUTH_IDENTITY_KEY="serverIdentity" \ - -e NACOS_AUTH_IDENTITY_VALUE="security" \ nacos/nacos-server:v3.1.1-slim >/dev/nullAs per coding guidelines, "Do not commit secrets or environment-specific credentials; use local overrides instead".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/discovery-smoke/scripts/provider.sh` around lines 121 - 123, Remove the hardcoded credential-like environment variables NACOS_AUTH_TOKEN, NACOS_AUTH_IDENTITY_KEY, and NACOS_AUTH_IDENTITY_VALUE from provider.sh; instead, do not commit any secret values—either omit those -e entries entirely or set them to empty placeholders and document that callers must supply real values via local/CI environment overrides when NACOS_AUTH_ENABLE is true; ensure any logic that relies on NACOS_AUTH_TOKEN or the identity vars checks NACOS_AUTH_ENABLE (or the presence of the env) before using them so no secret-like literals remain in the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@e2e/discovery-smoke/scripts/provider.sh`:
- Around line 121-123: Remove the hardcoded credential-like environment
variables NACOS_AUTH_TOKEN, NACOS_AUTH_IDENTITY_KEY, and
NACOS_AUTH_IDENTITY_VALUE from provider.sh; instead, do not commit any secret
values—either omit those -e entries entirely or set them to empty placeholders
and document that callers must supply real values via local/CI environment
overrides when NACOS_AUTH_ENABLE is true; ensure any logic that relies on
NACOS_AUTH_TOKEN or the identity vars checks NACOS_AUTH_ENABLE (or the presence
of the env) before using them so no secret-like literals remain in the repo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e193328-3e88-4136-9d2c-7b96d6babf9b
📒 Files selected for processing (1)
e2e/discovery-smoke/scripts/provider.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/README.md (1)
123-126: Redundant env file sourcing in example.Lines 104-106 already source
/tmp/discovery-provider.env. The second sourcing at lines 123-125 is unnecessary since the environment variables persist in the same shell session. Consider removing the duplicate for cleaner documentation.📝 Suggested documentation fix
java -jar "$ADMIN_JAR" > /tmp/apollo-adminservice.log 2>&1 & -set -a -source /tmp/discovery-provider.env -set +a ARTIFACT_DIR=/tmp/external-discovery-smoke/consul ./e2e/discovery-smoke/scripts/run-smoke.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/README.md` around lines 123 - 126, Remove the duplicate environment sourcing in the second block: drop the redundant "set -a / source /tmp/discovery-provider.env / set +a" lines near the ARTIFACT_DIR invocation so only the first occurrence remains; leave the ARTIFACT_DIR=... ./e2e/discovery-smoke/scripts/run-smoke.sh command intact and ensure the initial "set -a" / "source /tmp/discovery-provider.env" / "set +a" sequence earlier in the file is preserved as the single sourcing step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apollo-adminservice/src/main/scripts/shutdown.sh`:
- Around line 26-35: The shutdown logic currently kills by PID or by pgrep -f
"$SERVICE_NAME" too broadly; update it to first check the PID file's cmdline
(read /proc/$pid/cmdline) and ensure it contains the expected jar name (e.g.,
the app's jar tied to APP_NAME or explicit JAR name) before calling kill, and if
it doesn't match treat the PID file as stale and remove it without killing; for
the fallback path replace the broad pgrep -f "$SERVICE_NAME" with a pgrep -f
that matches the specific jar name (narrower string) and only kill the returned
PIDs, not any other processes.
---
Nitpick comments:
In `@e2e/README.md`:
- Around line 123-126: Remove the duplicate environment sourcing in the second
block: drop the redundant "set -a / source /tmp/discovery-provider.env / set +a"
lines near the ARTIFACT_DIR invocation so only the first occurrence remains;
leave the ARTIFACT_DIR=... ./e2e/discovery-smoke/scripts/run-smoke.sh command
intact and ensure the initial "set -a" / "source /tmp/discovery-provider.env" /
"set +a" sequence earlier in the file is preserved as the single sourcing step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0812fba-eff4-4f63-b53e-2b409cd20597
📒 Files selected for processing (10)
.github/workflows/portal-ui-e2e.ymlapollo-adminservice/src/main/scripts/shutdown.shapollo-configservice/src/main/scripts/shutdown.shapollo-portal/pom.xmlapollo-portal/src/main/scripts/shutdown.shapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AbstractControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.javae2e/README.mde2e/discovery-smoke/scripts/provider.she2e/scripts/apollo-smoke-lib.sh
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/portal-ui-e2e.yml
- apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AbstractControllerTest.java
- apollo-configservice/src/main/scripts/shutdown.sh
- apollo-portal/src/main/scripts/shutdown.sh
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.java
- e2e/scripts/apollo-smoke-lib.sh
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the Apollo server baseline to Spring Boot 4.0.x (Spring Framework 7 / Spring Cloud 2025.1 / Spring Cloud Alibaba 2025.1) while keeping Java 17, updating discovery integrations and CI coverage accordingly.
Changes:
- Upgraded root dependency BOMs/plugins and refreshed module/test code to Spring Boot 4 APIs (incl. security, webmvc test, health, Hibernate).
- Migrated Nacos discovery to Spring Cloud Alibaba’s supported starter and aligned discovery handling across Nacos/Consul/ZooKeeper.
- Added external discovery smoke scripts + GitHub Actions workflow; updated service start/stop scripts to
java -jar+ pidfile handling.
Reviewed changes
Copilot reviewed 105 out of 107 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Bumps Boot/Cloud baselines; imports Spring Cloud Alibaba BOM; removes executable-jar config; updates plugin versions. |
| e2e/scripts/apollo-smoke-lib.sh | Adds shared bash helpers for readiness + portal warm-up. |
| e2e/portal-e2e/scripts/wait-for-ready.sh | Refactors readiness wait to shared smoke library. |
| e2e/discovery-smoke/scripts/run-smoke.sh | Adds external registry smoke runner for meta + provider assertions. |
| e2e/discovery-smoke/scripts/provider.sh | Adds provider lifecycle/query helpers for Nacos/Consul/ZooKeeper via Docker. |
| e2e/README.md | Documents external discovery smoke suite and workflow triggers. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/AppServiceTest.java | Migrates mocking to Boot 4 Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/filter/PortalOpenApiAuthenticationScenariosTest.java | Updates MockMvc + redirect assertions for Boot 4; centralizes expired-session assertions. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/SystemInfoControllerTest.java | Updates Health import to Boot 4 health contributor package. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/PermissionControllerTest.java | Updates ResponseEntity status assertions for new API. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ItemControllerAuthIntegrationTest.java | Replaces TestRestTemplate usage with RestTemplate. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/InstanceControllerTest.java | Updates ResponseEntity status assertions for new API. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ClusterControllerTest.java | Updates ResponseEntity status assertions for new API. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.java | Adds unit test validating HttpClient 5 timeout wiring. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/AbstractIntegrationTest.java | Replaces TestRestTemplate; adds explicit Mockito open/close for JUnit4 tests. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationControllerTest.java | Updates MockMvc test wiring + Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/EnvControllerTest.java | Updates MockMvc test wiring + Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterControllerTest.java | Updates MockMvc test wiring + Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterControllerParamBindLowLevelTest.java | Updates MockMvc test wiring + Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AppControllerTest.java | Updates MockMvc test wiring + Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AppControllerParamBindLowLevelTest.java | Updates MockMvc test wiring + Mockito override annotations. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AbstractControllerTest.java | Updates HttpMessageConverters import; replaces TestRestTemplate; documents deprecation usage. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/service/ConsumerServiceTest.java | Migrates to MockitoBean/MockitoSpyBean. |
| apollo-portal/src/main/scripts/startup.sh | Switches to java -jar start with pidfile support. |
| apollo-portal/src/main/scripts/shutdown.sh | Switches to pidfile/pgrep-based shutdown for non-executable jars. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java | Updates OAuth2 properties imports and config DSL for Boot 4/Security 7. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/UserRole.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/ServerConfig.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/RolePermission.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Role.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Permission.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Favorite.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/SystemInfoController.java | Updates Health import to Boot 4 health contributor package. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RetryableRestTemplate.java | Avoids CollectionUtils dependency for headers empty-check. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java | Moves HttpClient 5 timeout config to ConnectionConfig/RequestConfig; uses Duration for request factory. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/PortalSettings.java | Updates Health import to Boot 4 health contributor package. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/ConfigReleaseWebhookNotifier.java | Updates JSON content type constant. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/AdminServiceAddressLocator.java | Updates HttpMessageConverters import. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java | Updates Health import to Boot 4 health contributor package. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/PortalAssemblyConfiguration.java | Updates DataSourceProperties import location. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/PortalApplication.java | Removes explicit LDAP auto-config exclusion to align with Boot 4 changes. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerToken.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerRole.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/Consumer.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/PortalOpenApiConfig.java | Updates Tomcat factory import location. |
| apollo-portal/pom.xml | Adds Boot 4 migration deps; updates test starters; upgrades OpenAPI generator config. |
| apollo-configservice/src/test/java/com/ctrip/framework/apollo/metaservice/service/NacosDiscoveryServiceTest.java | Switches Nacos test to generic DiscoveryClient-backed implementation. |
| apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/integration/AbstractBaseIntegrationTest.java | Updates RestTemplateBuilder import; removes TestRestTemplate usage. |
| apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/GracefulShutdownConfigurationTest.java | Updates ServerProperties/Servlet context imports for Boot 4. |
| apollo-configservice/src/main/scripts/startup.sh | Switches to java -jar start with pidfile support. |
| apollo-configservice/src/main/scripts/shutdown.sh | Switches to pidfile/pgrep-based shutdown for non-executable jars. |
| apollo-configservice/src/main/resources/application-zookeeper-discovery.properties | Keeps zookeeper instance-id property alignment. |
| apollo-configservice/src/main/resources/application-nacos-discovery.properties | Aligns Nacos properties to Spring Cloud Alibaba discovery keys; enables discovery. |
| apollo-configservice/src/main/resources/application-consul-discovery.properties | Expands Consul discovery/service-registry config for explicit registration. |
| apollo-configservice/src/main/java/com/ctrip/framework/apollo/metaservice/service/SpringCloudInnerDiscoveryService.java | Extends DiscoveryClient-based impl to cover nacos profile too. |
| apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceHealthIndicator.java | Updates Health/HealthIndicator imports for Boot 4. |
| apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServiceApplication.java | Updates auto-config exclusion imports for Boot 4. |
| apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/ConfigServerEurekaServerConfigure.java | Updates DaoAuthenticationProvider construction per newer API. |
| apollo-configservice/pom.xml | Drops direct nacos-api dependency; uses Spring Cloud Alibaba starter under profile. |
| apollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/AppNamespace.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/App.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-common/src/main/java/com/ctrip/framework/apollo/common/controller/WebMvcConfig.java | Updates Tomcat factory import; removes deprecated content negotiation override. |
| apollo-common/src/main/java/com/ctrip/framework/apollo/common/controller/HttpMessageConverterConfiguration.java | Refactors converter bean creation; exposes Gson bean; uses new HttpMessageConverters ctor. |
| apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ReleaseHistoryServiceTest.java | Adds explicit Mockito open/close lifecycle for mocks. |
| apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/NamespaceServiceIntegrationTest.java | Migrates @MockBean usage to Boot 4 Mockito override. |
| apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemSetServiceTest.java | Migrates @MockBean usage to Boot 4 Mockito override. |
| apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java | Adds explicit Mockito open/close lifecycle for mocks. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ServerConfig.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ReleaseHistory.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Release.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Privilege.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/NamespaceLock.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Namespace.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Item.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/GrayReleaseRule.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Commit.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Cluster.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Audit.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/AccessKey.java | Replaces Hibernate @Where with @SQLRestriction. |
| apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/ApolloBizAssemblyConfiguration.java | Updates DataSourceProperties import location. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.java | Converts to lightweight unit test; manages RequestContextHolder cleanup. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/controller/ApolloAuditControllerTest.java | Updates WebMvcTest import + Mockito override annotations. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/context/ApolloAuditTracerTest.java | Migrates to MockitoBean/MockitoSpyBean. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/context/ApolloAuditTraceContextTest.java | Migrates to MockitoBean/MockitoSpyBean. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditScopeManagerTest.java | Migrates to MockitoSpyBean. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImplTest.java | Migrates Mockito overrides; avoids @Captor by local captor creation. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/component/ApolloAuditHttpInterceptorTest.java | Migrates to MockitoBean/MockitoSpyBean. |
| apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/aop/ApolloAuditSpanAspectTest.java | Migrates to MockitoBean/MockitoSpyBean. |
| apollo-audit/apollo-audit-impl/pom.xml | Adds Boot webmvc test starter for updated test infrastructure. |
| apollo-assembly/src/test/java/com/ctrip/framework/apollo/assembly/LocalApolloApplication.java | Updates auto-config imports for Boot 4. |
| apollo-assembly/src/main/resources/application.yml | Sets explicit session cookie name to SESSION. |
| apollo-assembly/src/main/java/com/ctrip/framework/apollo/assembly/ApolloApplication.java | Updates auto-config imports for Boot 4. |
| apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/AbstractControllerTest.java | Updates HttpMessageConverters import; replaces TestRestTemplate; adds Mockito open/close for JUnit4. |
| apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/GracefulShutdownConfigurationTest.java | Updates ServerProperties/Servlet context imports for Boot 4. |
| apollo-adminservice/src/main/scripts/startup.sh | Switches to java -jar start with pidfile support. |
| apollo-adminservice/src/main/scripts/shutdown.sh | Switches to pidfile/pgrep-based shutdown for non-executable jars. |
| apollo-adminservice/src/main/resources/application-zookeeper-discovery.properties | Keeps zookeeper instance-id property alignment. |
| apollo-adminservice/src/main/resources/application-nacos-discovery.properties | Aligns Nacos properties to Spring Cloud Alibaba discovery keys; enables discovery. |
| apollo-adminservice/src/main/resources/application-consul-discovery.properties | Expands Consul discovery/service-registry config for explicit registration. |
| apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceHealthIndicator.java | Updates Health/HealthIndicator imports for Boot 4. |
| apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceApplication.java | Updates auto-config exclusion imports for Boot 4. |
| apollo-adminservice/pom.xml | Migrates Nacos dependency to Spring Cloud Alibaba starter under profile. |
| CHANGES.md | Adds release note entry for the Boot 4 baseline migration + smoke workflow. |
| .github/workflows/portal-ui-e2e.yml | Ensures shared e2e scripts trigger CI; passes portal credentials to readiness step. |
| .github/workflows/portal-login-e2e.yml | Ensures shared e2e scripts trigger CI; passes auth mode + credentials to readiness step. |
| .github/workflows/external-discovery-smoke.yml | Adds CI workflow to validate discovery registrations against real external registries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dosubot
bot
added
size:XL
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 105 out of 107 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ortal/src/test/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactoryTest.java
Show resolved
Hide resolved
Contributor
|
@nobodyiam This pull request has conflicts with the target branch. Please resolve them and update the branch before merging. |
nobodyiam
force-pushed
the
codex/spring-boot-4-upgrade
branch
from
e2291d2 to
4ccda7d
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What's the purpose of this PR
This PR migrates the Apollo server baseline from Spring Boot 3.5.x to Spring Boot 4.0.x while keeping Java 17.
The change aligns the server with Spring Framework 7, Spring Cloud 2025.1.x, and Spring Cloud Alibaba 2025.1.x. It also removes legacy Boot-4-incompatible pieces and adds a real external discovery smoke workflow so the upgraded discovery paths can be exercised continuously.
Which issue(s) this PR fixes:
N/A
Brief changelog
com.alibaba.bootstarter to the supported Spring Cloud Alibaba discovery pathjava -jarbased startup/shutdown handlingexternal-discovery-smokeGitHub Actions workflow plus shell-based provider smoke scripts fornacos,consul, andzookeeperFollow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.mvn spotless:applyto format your code.CHANGESlog.Validation
./mvnw -B spotless:apply./mvnw -B -pl apollo-biz,apollo-configservice,apollo-adminservice,apollo-portal -am test-compile./mvnw -B -pl apollo-biz,apollo-configservice,apollo-adminservice,apollo-portal -am -Dsurefire.failIfNoSpecifiedTests=false -Dtest=NamespaceServiceIntegrationTest,NacosDiscoveryServiceTest,GracefulShutdownConfigurationTest,PortalOpenApiAuthenticationScenariosTest,SystemInfoControllerTest,RestTemplateFactoryTest testconsulandzookeeperwhile implementing the new workflowSummary by CodeRabbit
New Features
Chores
Tests
Documentation