Skip to content

[ISSUE #7652] Support standalone Bootstrap configuration from a local file - #7673

Merged
Aias00 merged 15 commits into
apache:masterfrom
yangjj-iso:feat/7652-file-configuration
Oct 20, 2026
Merged

Aias00 merged 15 commits into
apache:masterfrom
yangjj-iso:feat/7652-file-configuration

Conversation

@yangjj-iso

@yangjj-iso yangjj-iso commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #7652. Bootstrap can start from an external declarative YAML/JSON file and serve HTTP routes without Admin, an Admin database or an external configuration center. The opt-in shenyu-spring-boot-starter-sync-data-file is included in Bootstrap and reuses the existing configuration DTOs, subscribers, plugin handlers and request chain. Merge dependency: #7677 must land first. This branch includes its RuleData.logged rename at 6ffa0d8107b182d3ff03c8082500cc0671b5b649, and the file parser uses .logged(false). Selector and rule logging defaults are asserted by the parser test. The rename's SQL/Admin changes remain attributable to #7677 and will leave this PR's diff when that dependency lands. The branch is rebased onto master f00c15712, which already includes #7680. The competing Apollo selector-deletion test commit has been dropped; ApolloDataServiceTest.java is identical to master. Integration preserves the latest Admin dashboard/CORS changes and adapts the new base-plugin tests to the renamed setter. Behavior: - Validate the complete versioned document before publication: explicit plugin enablement, deterministic identities, references, ordering, matching conditions, rule handles and static HTTP origins. Reject unknown fields, duplicate mapping keys, unsupported metadata/auth entries and unavailable plugins/handlers without echoing configuration values. - Establish exclusive file ownership before clients are created. Suppress the shipped Admin WebSocket URL, heartbeat, WebSocket readiness and external plugin loader; reject other sync sources and explicitly enabled local API/registration/alert writers. Undeclared plugins stay disabled. File mode is disabled by default. - Apply plugins, selectors, rules and upstreams before the server starts; await asynchronous handler work with a bounded timeout. Abort startup on failure, gate traffic until application succeeds, and remove owned configuration on shutdown/failure. - Document the schema, environment placeholders, mounted-file/container launch and switching back to Admin mode. The documented sample is exercised by tests. Initial scope is static HTTP routing through global/divide/uri/nettyHttpClient/response, with optional contextPath. Metadata/auth entries and other declared plugins are rejected. Hot reload and multi-source merging are outside this change. The existing shared E2E repairs remain: a healthchecked local HTTP upstream for storage/cluster scenarios, RocketMQ log consumption with unique request/group IDs and topic/queue readiness, Spring Cloud route readiness using the actual controller response, and a local HTTP server for wait-helper tests. PR builds use the Maven wrapper. Validation on Java 17: - 150 selected tests pass, with zero failures/errors/skips: the complete file parser/loader/environment/real-HTTP suites, Apollo data service, RuleData, base-plugin matching, local plugin controller and common plugin subscriber. - The real-HTTP suite starts from the stock Bootstrap settings, forwards to a real upstream, rejects unmatched/local-management requests, checks disabled Admin integrations, restarts with stable identities and fails startup on invalid configuration/handler failure. - The complete 161-module Admin + Bootstrap reactor packages successfully, including test compilation; test execution is skipped for this packaging check. - Checkstyle, Apache RAT and git diff --check pass. - Container E2E/native database execution is left to CI; this local follow-up does not claim those suites passed. sh ./mvnw -B -ntp -pl shenyu-spring-boot-starter/shenyu-spring-boot-starter-sync-data-center/shenyu-spring-boot-starter-sync-data-file,shenyu-sync-data-center/shenyu-sync-data-apollo -am test \ '-Dtest=FileConfiguration*Test,FileMode*Test,ApolloDataServiceTest,RuleDataTest,AbstractShenyuPluginTest,LocalPluginControllerTest,CommonPluginDataSubscriberTest' \ -Dsurefire.failIfNoSpecifiedTests=false -DskipRemoteResources=true \ -Djacoco.skip=true -Dmaven.javadoc.skip=true ./mvnw -B -ntp -pl shenyu-admin,shenyu-bootstrap -am package \ -DskipTests -DskipRemoteResources=true -Djacoco.skip=true -Dmaven.javadoc.skip=true ./mvnw -B -ntp -pl shenyu-admin,shenyu-bootstrap -am apache-rat:check -DskipRemoteResources=true

Validation after the October 19 synchronization

Local Java validation used Temurin 21.0.10 through the repository Maven wrapper.

Maintainer review follow-up — October 20

Complete the three missed RuleData API updates identified in review: both key-auth authentication tests now call setLogged(false), and the MCP key-auth fixture uses .logged(false). A whole-tree Java search finds no remaining calls to setLoged, getLoged or the loged builder method. Historical SQL migration names and JSON compatibility aliases remain intentional.

Validation on Microsoft OpenJDK 17.0.18 through the Maven wrapper: all 44 affected reactor modules compile, and 210 selected tests pass with zero failures/errors/skips. This includes both formerly failing key-auth test classes, key-auth and principal MCP HTTP/security tests, starter wiring, RuleData compatibility, and the complete file parser/loader/environment/HTTP suites. Checkstyle, Apache RAT and git diff --check pass.

./mvnw -B -ntp \
  -pl shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway,shenyu-spring-boot-starter/shenyu-spring-boot-starter-sync-data-center/shenyu-spring-boot-starter-sync-data-file \
  -am test apache-rat:check \
  '-Dtest=KeyAuth*Test,PrincipalMcp*Test,AgentGatewayPluginConfigurationTest,FileConfiguration*Test,FileMode*Test,RuleDataTest' \
  -Dsurefire.failIfNoSpecifiedTests=false -DskipRemoteResources=true \
  -Djacoco.skip=true -Dmaven.javadoc.skip=true

The branch is mergeable with current master. The previously documented merge dependency remains: #7677 should land first; then refresh this branch so its shared rename changes leave this PR's diff. This follow-up does not claim that the new GitHub CI run has completed.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review — Standalone Bootstrap file configuration

Thanks for this — it's a large, well-structured contribution and the security posture of FileModeEnvironmentPostProcessor is genuinely good work. I verified the full head tree against the current master and found two blocking issues, both of which are collisions with other in-flight PRs rather than defects in your logic.


🔴 Blocking 1 — FileConfigurationParser.java:194 will not compile once #7677 lands

shenyu-sync-data-center/shenyu-sync-data-file/src/main/java/org/apache/shenyu/sync/data/file/FileConfigurationParser.java line 194:

RuleData.builder()... .enabled(...).sort(sort).matchMode(matchMode(entry, at)).loged(false).matchRestful(false)

On master today, RuleData still declares the field as loged (shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/RuleData.java:39 — private Boolean loged;), so this compiles. But #7677 (open) renames it to logged across the whole tree. SelectorData was already renamed in v2.5.0 (SelectorData.java:47 — private Boolean logged;), which is why line 173 of your own parser is already correct:

SelectorData.builder()... .logged(false).continued(true)...   // line 173 — correct

So the moment #7677 merges, this line breaks the build for the file-sync module. Note the asymmetry is easy to miss because it is not uniform within your own file.

Please rebase onto #7677 (or at minimum change loged → logged and add a note) so the two land in a compilable order. Merging #7673 before #7677 is also fine, but then #7677 must carry the fix — worth calling out in whichever PR merges second.

🔴 Blocking 2 — ApolloDataServiceTest collides with the already-approved #7680

shenyu-sync-data-center/shenyu-sync-data-apollo/src/test/java/org/apache/shenyu/sync/data/apollo/ApolloDataServiceTest.java line 283. You and #7680 are fixing the same master bug in the same two lines by different means.

The underlying defect: AbstractNodeDataSyncService#getDataKeys() returns {"test-plugin", "selector-id", ...} style arrays and AbstractNodeDataSyncService only calls setId(keys[4]) / setName(...) on one of them, leaving getName() null.

  • #7680 changes the input key to shenyu/test-plugin/selector-id and asserts getId() equals selector-id.
  • #7673 keeps the input and asserts getId() equals test-proxy.

Both are defensible readings of intent, but they touch identical lines, so this will be a guaranteed merge conflict. Since #7680 is already approved, please rebase onto it and drop your duplicate fix rather than keeping both. If you believe the test-proxy assertion is the more faithful one, that is a separate discussion to have on #7680 — but this PR should not carry a second, divergent fix for the same defect.


✅ What I verified as correct

I went through the whole module against the dependency sources; the design holds up.

FileModeEnvironmentPostProcessor

  • Ordering is right: ConfigDataEnvironmentPostProcessor.ORDER + 1 places you after ConfigDataEnvironmentPostProcessor has populated shenyu.sync.* from application.yaml, so environment.getProperty(key, "false") genuinely sees user configuration rather than the bootstrap default. You are early enough (EnvironmentPostProcessor) that no bean has been created yet, which is what makes the hard fail safe.
  • The bootstrap-default carve-out (ws://localhost:9095/websocket) is exactly the right refinement — it distinguishes "user left the shipped default alone" from "user explicitly configured another sync source". Without it, every fresh standalone deployment would be rejected.
  • Rejecting before overriding to false is the correct ordering; the IllegalStateException message naming the offending channel is actionable.

FileSyncDataConfiguration

  • environment.getPropertySources().contains("shenyuFileMode") at lines 66-68 is a good guard against the post-processor not having registered (e.g. when the starter is on the classpath but spring.factories registration was stripped or overridden by a custom spring.factories). This avoids a confusing NPE-less misconfiguration.

Filtered plugin subscriber

  • CommonPluginDataSubscriber holds handlerMap as an instance field (shenyu-plugin-base/.../CommonPluginDataSubscriber.java:53), so constructing a second filtered subscriber does not corrupt the shared admin-side instance. Correct.
  • I checked the startup path: updateCacheData calls handlerPlugin before cachePluginData/notifyPluginData, and subscribeDataHandler's catch rethrows while InitialSyncApplication.isActive() is true. Your SmartInitializingSingleton.afterSingletonsInstantiated() runs before finishRefresh(), hence before Netty binds — so a bad file fails startup rather than after the port is open. The extra 503 filter is reasonable defence in depth.

FileConfigurationParser strictness

  • Rejecting unknown fields, multiple documents, and >1 MiB files; resolving placeholders before inserting into the node (so a placeholder value cannot alter the document's shape); and not echoing values in diagnostics — all good, and the diagnostic messages keep the at path which is what makes them usable.
  • I diffed the documented defaults against DivideRuleHandle and they line up exactly: timeout = 3000 = Constants.TIME_OUT (shenyu-common/.../Constants.java:317), retry = 3, loadBalance = random, headerMaxSize/requestMaxSize = 0 (no initial value in the DTO, so 0 is the right "unset" default). ContextMappingRuleHandle.addPrefix is genuinely a String (ContextMappingRuleHandle.java:29-33), so requiring a leading / matches README.md:87.

FileConfigurationLoader

  • InitialSyncApplication.run(this::apply).get(timeout.toMillis(), MILLISECONDS) correctly blocks startup on initial sync, and close() unsubscribes in reverse dependency order (upstreams → rules → selectors → plugins) with per-entry cleanup isolation. Correct shape.

⭐ E2E infrastructure improvements (this is the part I'd most like to keep)

These are easy to overlook because they are "just CI", but they fix real, recurring pain:

  1. Local httpbin fixture — the compose network alias httpbin.org plus a healthcheck removes the external httpbin.org dependency. This is directly relevant: while reviewing #7677 I found the e2e storage suite red on endpoint '/anything/xx/yy' not exists, which traced back to external-site flakiness. You've eliminated that class of failure at the source.
  2. RocketMQ consumer hardening — DefaultLitePullConsumer, unique group and request id, seekToBegin, and tolerating TOPIC_NOT_EXIST are all the right changes for a suite that needs to be deterministic from a cold topic.
  3. WaitForHelperTest — replacing a wall-clock-dependent test with a local HttpServer + latch, and asserting the delayed request actually arrived, turns a timing guess into a deterministic assertion. Much better.
  4. Dropping mvnd for ./mvnw in ci.yml — justified by daemon disconnect flakiness.

Summary

Request changes for the two cross-PR collisions above; nothing else I found is blocking. Please rebase onto #7680 and reconcile the RuleData field name with #7677, and I'll re-review.

@yangjj-iso
yangjj-iso force-pushed the feat/7652-file-configuration branch from d611e62 to 1079512 Compare October 18, 2026 01:10
@yangjj-iso

Copy link
Copy Markdown
Contributor Author

@Aias00 Both original coordination items are addressed: the file parser uses logged(false) on the included #7677 dependency, and ApolloDataServiceTest matches the version now on master from #7680. I also resolved the current CI/generated-dashboard conflicts against master in c989867, preserving the upstream bundle and retry helper. All 74 selected RuleData and file-mode tests pass locally.

Could you please re-review this head and confirm the intended ordering with #7677? The rename dependency remains included; this does not claim that #7677 itself has merged.

@Aias00

Aias00 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Thanks for the update. This PR currently shows merge conflicts with master (GitHub reports mergeable = CONFLICTING). Please rebase/update your branch onto the latest master and resolve the conflicts, then push so CI can re-run. I will re-review and re-test once it is mergeable. Note: CI must be green before this can be merged.

Resolve the dashboard modify/delete conflict by retaining the current upstream bundle and index. Preserve the RuleData rename dependency and standalone file configuration changes.
…ertions

The initialization signal can reach block() before doFinally releases the handshake request. Await the existing idle barrier before measuring the expired-deadline call, preserving the no-request and zero-pending assertions.
@Aias00

Aias00 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

This PR still has a merge conflict with master (mergeable: false). Several new commits were pushed after my earlier review, but the branch has not been rebased onto the current master, so the conflict persists and it cannot be merged.

Please rebase onto the latest master and resolve the conflicts, then force-push the branch:

git fetch upstream
git rebase upstream/master
# resolve conflicts, then:
git push --force-with-lease

Once the conflict is cleared (and mergeable returns true), I will re-review the resolved code. Thanks!

@Aias00

Aias00 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Re-review: both previous blocking items are resolved, but pr_build is now red

Thanks for the rework. I verified the two items from my previous review against head c9f142aa and both are genuinely fixed:

  1. loged → logged — the parser, RuleData, SelectorData, the admin DTO/DO/VO layer, the MyBatis maps, the H2/MySQL/Oracle/PG schemas and all five 2.7.2-upgrade-2.7.3-*.sql scripts are now consistent. Good.
  2. ApolloDataServiceTest collision with [ISSUE #6602] Enforce Maven and Java build requirements #7680 — the duplicate fix is gone from the diff. Correct call.
  3. The module relocation into shenyu-spring-boot-starter-sync-data-center/shenyu-spring-boot-starter-sync-data-file is wired up correctly: parent <module>, spring.factories for the EnvironmentPostProcessor, AutoConfiguration.imports for the configuration, the shenyu-bootstrap dependency and the commented application.yml example all line up.

🔴 Blocking — the rename is incomplete, so pr_build fails to compile

pr_build (run 39357244520) fails at testCompile of shenyu-plugin-key-auth:

[ERROR] KeyAuthAuthenticationHttpTest.java:[151,17] cannot find symbol
[ERROR]   symbol:   method setLoged(boolean)
[ERROR]   location: variable rule of type org.apache.shenyu.common.dto.RuleData
[ERROR] KeyAuthAuthenticationTest.java:[311,13] cannot find symbol
[ERROR]   symbol:   method setLoged(boolean)

RuleData.setLoged(...) no longer exists after this PR, but these call sites were not updated. I cross-checked every remaining loged occurrence in the tree against your changed-file list, and exactly three files were missed:

File Line Current code
shenyu-plugin/shenyu-plugin-security/shenyu-plugin-key-auth/src/test/java/org/apache/shenyu/plugin/key/auth/KeyAuthAuthenticationHttpTest.java 151 rule.setLoged(false);
shenyu-plugin/shenyu-plugin-security/shenyu-plugin-key-auth/src/test/java/org/apache/shenyu/plugin/key/auth/KeyAuthAuthenticationTest.java 311 rule.setLoged(false);
shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/test/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/KeyAuthMcpTestFixture.java 310 .loged(false) on the RuleData builder

The build stops at shenyu-plugin-key-auth, so the third one has not surfaced in CI yet but will fail the same way once the first two are fixed. Please update all three (setLoged → setLogged, builder loged → logged) and push so pr_build can go green.

⚠️ Please coordinate the merge order with #7677

#7677 ("rename misspelled loged column/field to logged") is still open and currently CONFLICTING. This PR now carries the same rename, so the two will collide in RuleDO/SelectorDO/RuleDTO/SelectorDTO/RuleData, the MyBatis maps and the SQL scripts.

One of them should land first and the other should drop its copy — please agree on the order on the relevant issue rather than merging both. If #7677 lands first, this PR only needs to drop the rename hunks; if this PR lands first, #7677 becomes redundant and should be closed.

Nothing else I reviewed is blocking: FileModeEnvironmentPostProcessor, FileSyncDataConfiguration and FileConfigurationLoader are unchanged in substance from the version I looked at before and still hold up.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-review: Approved

Thank you for addressing the review comments in commit 076c1f137a (test(file-sync): complete RuleData logging API rename after master sync).

All three call sites identified in the previous review have been properly updated:

  • KeyAuthAuthenticationHttpTest.java:151: rule.setLogged(false);
  • KeyAuthAuthenticationTest.java:311: rule.setLogged(false);
  • KeyAuthMcpTestFixture.java:310: RuleData.builder()...logged(false)...

CI check results:

  • pr_build passed in 8m47s.
  • All 53 checks succeeded with 0 failures.

The standalone bootstrap file configuration feature is complete, well tested, and compliant with all project standards. Approved!

@Aias00
Aias00 merged commit 3f6733d into apache:master Oct 20, 2026
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Support standalone bootstrap startup from a local configuration file without Admin

3 participants