Skip to content

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

Open
yangjj-iso wants to merge 11 commits into
apache:masterfrom
yangjj-iso:feat/7652-file-configuration
Open

yangjj-iso wants to merge 11 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.
./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

@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

This branch has not been deployed

No deployments
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