feat: Support version history and rollback for traffic rules#1477
Open
mochengqian wants to merge 12 commits into
Open
feat: Support version history and rollback for traffic rules#1477mochengqian wants to merge 12 commits into
mochengqian wants to merge 12 commits into
Conversation
Running the §9.4 smoke drill end-to-end uncovered three real defects that the unit suite did not catch: - RuleVersionSubscriber recorded a duplicate UPSTREAM row whenever the registry echoed back a no-op change identical to the latest ledger row (typically right after BOOTSTRAP). Now dedupes upstream events whose content hash already matches the current head, with an explicit test in versioning_test.go. - writeVersioningResp mapped every bizerror to HTTP 200/UnknownError; bizerror.InvalidArgument (eg. empty rollback reason) now returns HTTP 400 with its original code so the frontend can act on it. Covered by a new handler/rule_version_test.go. - The 409 VERSION_CONFLICT toast auto-dismissed after the default duration; users could miss the Reload button entirely. Pinned with duration: 0 so the notification stays until acknowledged.
The §9.4 smoke drill expectation "after rollback, the rule should look the same on UI refresh" exposed a pre-existing edit-form regression: rollback was correct at the ledger and ZK levels, but the edit form silently dropped `priority`, `force`, and (for condition routes) `configVersion` because they were neither rendered in the GET response nor re-sent on save. This is not caused by versioning, but a true round-trip is the first flow that forces every field through the loop. Adds the missing fields to ConditionRuleResp / TagRuleResp on the backend, and reads/writes them in updateByFormView.vue on the frontend so a "save → rollback → reload" cycle is now lossless.
Add /task_plan.md /findings.md /progress.md to .gitignore so the planning-with-files workflow does not leak per-developer working memory into the repo.
6deb25e to
9596f7e
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.



Closes #1473.
Please view the detailed test report at https://notion-next-iota-amber-43.vercel.app/article/dubbo-admin-reviewer-report
1. What this PR delivers
Dubbo Admin currently saves traffic-rule edits with a destructive overwrite. There is no history, no audit trail for upstream-registry pushes, and no way to recover from a bad edit. This PR adds an immutable release ledger plus one-click rollback for the three governor-managed rule kinds — Condition Route, Tag Route, and Dynamic Config (Configurator).
The approach was picked after evaluating five candidate solutions during design:
User-visible capabilities
ADMIN/UPSTREAM/ROLLBACK/BOOTSTRAP), operator, timestamp and reason.ROLLBACKrow. The old row is never mutated.versioning.maxVersionsPerRule).2. How it works
(kind, resourceKey, contentHash).source=ROLLBACKandrolled_back_from_idpointing at the source version. The original row is never modified.version_nois monotonic and not reused after trim — users always see strictly increasing version labels.3. Scope
In this PR
Backend
pkg/core/versioningpackage: types, memory + GORM stores, service, hint registry, subscriber, component.pkg/config/versioningconfig block with sane defaults and validation.BOOTSTRAPbaseline row for every existing rule (idempotent).expectedVersionIdprecondition on existing PUT / POST / DELETE.events.SourceRegistryContextKeyconstant +ZKConfigEventSubscriberemits registry context.Frontend
ui-vue3/src/views/traffic/_shared/:RuleHistoryDrawer.vue,RuleHistoryPanel.vue,RuleDiffEditor.vue,ruleVersion.ts.notificationwithduration: 0and a Reload action.Tests
e2e_rollback_drill_test.go) covering bootstrap → admin edit → upstream push → rollback → overflow trim → audit-chain assertions.Out of scope for this PR
@mochengqianas an immediate follow-up. AffinityRoute is not currently ingovernor.RuleResourceKinds, so its write path bypasses the governor that this ledger hooks into. Once it is brought onto the governor path, all four kinds will share the same versioning code.4. Locked design decisions
governor.RuleResourceKinds; out of scope.expectedVersionIdis best-effort5. API surface
New endpoints (one set per rule kind:
condition-rule,tag-rule,configurator)Existing endpoints (backward-compatible)
PUT / POST / DELETE /api/v1/{kind}/:ruleNameaccept an optional?expectedVersionId=<id>.Omit → unchanged behavior. Provide → mismatch returns:
Feature flag
With
versioning.enabled=false, all new endpoints return503 + {"code":"FEATURE_DISABLED"}. Existing CRUD is completely untouched.6. Database
Two new tables, created via
AutoMigrateon the existing GORM connection (MySQL / Postgres). Withstore.type=memorya pure-Go in-memory implementation is used — no config changes needed.Upgrade path: on first start, scan every existing rule and write one
source=BOOTSTRAPbaseline row. Idempotent — safe to re-run.7. Verification
Automated tests
go test ./pkg/core/versioning/... \ ./pkg/config/versioning/... \ ./pkg/console/handler/... \ ./pkg/console/service/... \ ./pkg/store/... \ ./pkg/core/discovery/subscriber/... \ ./pkg/core/manager/...All green.
go vet ./pkg/...reports no new warnings.Frontend build
Passes.
npm run type-checkstill reports the pre-existing repo-wide TypeScript debt (home/index.vue, AuthUtil.ts, GrafanaPage, etc.) — that count does not increase under this branch.Manual smoke
A full bootstrap → admin edit → diff → rollback → optimistic-lock 409 → retention cap → upstream ZK push → cross-rule rollback sweep was run end-to-end against MySQL + ZooKeeper. Evidence (HTTP transcripts, JSON ledger dumps, UI screenshots) is in the PR comments.
8. Upgrade and rollback
AutoMigratecreates the two tables; the bootstrap scan inserts a baseline row per existing rule. Clients and registries need no changes.versioning.enabled=falseand restart. CRUD paths are unaffected; new endpoints return 503. The tables are left in place in case the feature is re-enabled later.feat(versioning)commits is sufficient; the two preparatory commits at the bottom of the ladder are harmless to keep.9. Test plan checklist
go test ./pkg/...passes locallycd ui-vue3 && npm run buildpassesmaxVersionsPerRule