Vtadmin2 - #20946
Conversation
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Alisher <thealishh@gmail.com>
Signed-off-by: Alisher <thealishh@gmail.com>
vitess-bot
Bot
added
NeedsWebsiteDocsUpdate
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
github-actions
Bot
added
Component: VTAdmin
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 User-visible changes lack release notes
The new command, UI, flags, example startup, and transaction authorization change lack the deployment-note callout required for user-visible behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if tableSettings == "" { | ||
| s.renderFormError(w, r, title, "table settings are required") | ||
| return |
There was a problem hiding this comment.
🟡 Reference-table materializations always fail
Selecting a reference table with required table settings makes createMaterialize send mutually exclusive modes. The backend rejects the workflow.
Prompt for agents
The Materialize form and handler cannot represent the vtctld Materialize contract. go/vt/vtadmin/vtadmin2/workflows_create.go:createMaterialize requires non-empty table_settings, while the form also offers reference_table selections. vtctld validateMaterializeSettings requires exactly one of TableSettings or ReferenceTables, so selecting a reference table with the required JSON always fails, and reference-table-only workflows cannot be submitted. Update the form, validation, and tests to support either mode while rejecting requests that provide both or neither.
Was this helpful? React with 👍 or 👎 to provide feedback.
| allTables := r.Form.Get("all_tables") == "on" | ||
| includeTables := r.Form["table"] |
There was a problem hiding this comment.
🟡 All-table workflows reject prior selections
Selecting tables before “Copy all tables” leaves both choices in createMoveTables. The backend rejects the workflow instead of ignoring selections.
| allTables := r.Form.Get("all_tables") == "on" | |
| includeTables := r.Form["table"] | |
| allTables := r.Form.Get("all_tables") == "on" | |
| includeTables := r.Form["table"] | |
| if allTables { | |
| includeTables = nil | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| Uuid: strings.TrimSpace(r.Form.Get("uuid")), | ||
| SourceCells: splitFormList(r.Form.Get("source_cells")), | ||
| TargetCells: splitFormList(r.Form.Get("target_cells")), | ||
| Tables: splitFormList(r.Form.Get("tables")), | ||
| AutoRetry: r.Form.Get("auto_retry") == "on", | ||
| Wait: r.Form.Get("wait") == "on", |
There was a problem hiding this comment.
🟡 VDiff options are silently ignored
Entering a UUID or disabling auto-retry in workflowVDiffCreate has no effect because the shared API overwrites both values.
Prompt for agents
The new VDiff form exposes UUID and auto-retry controls, and workflowVDiffCreate forwards them in go/vt/vtadmin/vtadmin2/workflows_actions.go. However, go/vt/vtadmin/api.go:VDiffCreate unconditionally generates a new UUID and sets AutoRetry=true, discarding both submitted values. Preserve explicit caller values while applying defaults only when an option is absent. AutoRetry may need presence-aware representation at the VTAdmin boundary so an explicit false can be distinguished from omission. Add integration-level tests through the real VTAdmin API, not only the fake used by the handler tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return | ||
| } | ||
|
|
||
| redirectWithFlash(w, r, "/transactions?cluster_id="+url.QueryEscape(clusterID), Flash{ |
There was a problem hiding this comment.
🔍 Transaction conclusion redirects to error
After transactionConclude succeeds, its redirect omits the required keyspace. Operators land on a bad-request page instead of the transaction list.
Was this helpful? React with 👍 or 👎 to provide feedback.
| func run(cmd *cobra.Command, args []string) error { | ||
| cfg, err := buildRuntimeConfig() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| api, err := buildAPI(cmd.Context(), cfg.rbac) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| server, err := vtadmin2.NewServer(api, cfg.ui) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| addr := uiOpts.Addr | ||
| if addr == "" { | ||
| addr = ":15001" | ||
| } | ||
| log.Info("starting vtadmin2", slog.String("addr", addr)) | ||
| ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM) | ||
| defer stop() | ||
| return serveHTTPServer(ctx, buildHTTPServer(addr, server)) | ||
| } |
There was a problem hiding this comment.
🔍 Standalone shutdown skips API cleanup
When standalone HTTP serving ends, run returns without closing the API’s cluster connections and discovery resources.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return | ||
| } | ||
|
|
||
| _, err := s.api.VDiffCreate(r.Context(), &vtadminpb.VDiffCreateRequest{ |
There was a problem hiding this comment.
🟥 Cluster viewers can create VDiffs
VDiffCreate accepts cluster read permission for a mutating operation. Read-only users can launch VDiff jobs and modify cluster state.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Name: csrfCookieName, | ||
| Value: token, | ||
| Path: "/", | ||
| Secure: r.TLS != nil, |
There was a problem hiding this comment.
🟨 Proxied HTTPS produces insecure cookies
csrfToken marks cookies secure only when Go terminates TLS. HTTPS behind a proxy emits cookies that browsers can resend over plaintext HTTP.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
Introduces the experimental server-rendered VTAdmin2 interface described in #20107, reusing VTAdmin’s Go API while reducing frontend-toolchain dependencies.
Changes:
- Adds Go handlers, embedded templates/assets, administrative actions, and extensive tests.
- Adds standalone and integrated VTAdmin2 server modes.
- Adds example startup support and strengthens transaction-conclusion authorization.
Reviewed changes
Copilot reviewed 103 out of 104 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
examples/common/scripts/vtadmin-down.sh |
Stops VTAdmin2. |
examples/common/scripts/vtadmin2-up.sh |
Starts example VTAdmin2. |
examples/local/101_initial_cluster.sh |
Starts VTAdmin2 locally. |
go/cmd/vtadmin/main.go |
Integrates VTAdmin2 mode. |
go/cmd/vtadmin/main_test.go |
Tests UI option validation. |
go/cmd/vtadmin2/main.go |
Adds standalone command. |
go/cmd/vtadmin2/main_test.go |
Tests command lifecycle/configuration. |
go/flags/endtoend/flags_test.go |
Registers VTAdmin flag snapshot. |
go/flags/endtoend/vtadmin.txt |
Records updated VTAdmin flags. |
go/vt/vtadmin/api.go |
Tightens transaction RBAC. |
go/vt/vtadmin/api_authz_transaction_test.go |
Tests transaction authorization. |
go/vt/vtadmin/vtadmin2/actions.go |
Implements keyspace creation. |
go/vt/vtadmin/vtadmin2/actions_test.go |
Tests keyspace creation. |
go/vt/vtadmin/vtadmin2/api.go |
Defines required API interface. |
go/vt/vtadmin/vtadmin2/assets.go |
Embeds templates/assets. |
go/vt/vtadmin/vtadmin2/csrf_test.go |
Tests CSRF enforcement. |
go/vt/vtadmin/vtadmin2/debug_json_test.go |
Tests debug JSON rendering. |
go/vt/vtadmin/vtadmin2/http_server.go |
Configures HTTP timeouts. |
go/vt/vtadmin/vtadmin2/inventory.go |
Renders inventory pages. |
go/vt/vtadmin/vtadmin2/inventory_test.go |
Tests inventory handlers. |
go/vt/vtadmin/vtadmin2/keyspace_actions.go |
Implements keyspace actions. |
go/vt/vtadmin/vtadmin2/keyspace_actions_test.go |
Tests keyspace actions. |
go/vt/vtadmin/vtadmin2/operations.go |
Renders operational data. |
go/vt/vtadmin/vtadmin2/operations_test.go |
Tests operational pages. |
go/vt/vtadmin/vtadmin2/pages.go |
Renders core resource pages. |
go/vt/vtadmin/vtadmin2/pages_test.go |
Tests core pages. |
go/vt/vtadmin/vtadmin2/refresh_test.go |
Tests refresh behavior. |
go/vt/vtadmin/vtadmin2/render.go |
Implements template rendering. |
go/vt/vtadmin/vtadmin2/schema.go |
Renders schema details. |
go/vt/vtadmin/vtadmin2/schema_test.go |
Tests schema rendering. |
go/vt/vtadmin/vtadmin2/security.go |
Implements CSRF and flash cookies. |
go/vt/vtadmin/vtadmin2/server.go |
Registers server routes. |
go/vt/vtadmin/vtadmin2/server_test.go |
Tests routing/authentication. |
go/vt/vtadmin/vtadmin2/settings.go |
Implements browser settings. |
go/vt/vtadmin/vtadmin2/settings_test.go |
Tests settings persistence. |
go/vt/vtadmin/vtadmin2/shard.go |
Implements shard views/actions. |
go/vt/vtadmin/vtadmin2/shard_test.go |
Tests shard behavior. |
go/vt/vtadmin/vtadmin2/static/vitess-stacked.png |
Adds branded logo. |
go/vt/vtadmin/vtadmin2/static/vtadmin2.css |
Styles VTAdmin2. |
go/vt/vtadmin/vtadmin2/static/vtadmin2.js |
Adds progressive enhancements. |
go/vt/vtadmin/vtadmin2/stream.go |
Renders workflow streams. |
go/vt/vtadmin/vtadmin2/stream_test.go |
Tests stream pages. |
go/vt/vtadmin/vtadmin2/tablet.go |
Renders tablet details. |
go/vt/vtadmin/vtadmin2/tablet_actions.go |
Implements tablet actions. |
go/vt/vtadmin/vtadmin2/tablet_actions_test.go |
Tests tablet actions. |
go/vt/vtadmin/vtadmin2/tablet_test.go |
Tests tablet pages. |
go/vt/vtadmin/vtadmin2/templates/backups.html |
Displays backups. |
go/vt/vtadmin/vtadmin2/templates/cells.html |
Displays cells. |
go/vt/vtadmin/vtadmin2/templates/cells_aliases.html |
Displays cell aliases. |
go/vt/vtadmin/vtadmin2/templates/clusters.html |
Displays clusters. |
go/vt/vtadmin/vtadmin2/templates/gates.html |
Displays VTGates. |
go/vt/vtadmin/vtadmin2/templates/index.html |
Provides generic content/error page. |
go/vt/vtadmin/vtadmin2/templates/keyspace.html |
Displays keyspace actions/details. |
go/vt/vtadmin/vtadmin2/templates/keyspace_create.html |
Provides keyspace form. |
go/vt/vtadmin/vtadmin2/templates/keyspaces.html |
Lists keyspaces. |
go/vt/vtadmin/vtadmin2/templates/layout.html |
Defines shared layout. |
go/vt/vtadmin/vtadmin2/templates/migration_create.html |
Provides migration form. |
go/vt/vtadmin/vtadmin2/templates/migrations.html |
Displays migrations. |
go/vt/vtadmin/vtadmin2/templates/schema.html |
Displays schema details. |
go/vt/vtadmin/vtadmin2/templates/schemas.html |
Lists schemas. |
go/vt/vtadmin/vtadmin2/templates/settings.html |
Provides settings form. |
go/vt/vtadmin/vtadmin2/templates/shard.html |
Displays shard actions/details. |
go/vt/vtadmin/vtadmin2/templates/shards.html |
Lists shards. |
go/vt/vtadmin/vtadmin2/templates/srvkeyspaces.html |
Displays serving keyspaces. |
go/vt/vtadmin/vtadmin2/templates/srvvschemas.html |
Displays serving VSchemas. |
go/vt/vtadmin/vtadmin2/templates/stream.html |
Displays stream details. |
go/vt/vtadmin/vtadmin2/templates/tablet.html |
Displays tablet actions/details. |
go/vt/vtadmin/vtadmin2/templates/tablet_full_status.html |
Displays full tablet status. |
go/vt/vtadmin/vtadmin2/templates/tablets.html |
Lists tablets. |
go/vt/vtadmin/vtadmin2/templates/topology.html |
Displays topology selection. |
go/vt/vtadmin/vtadmin2/templates/topology_tree.html |
Displays topology hierarchy. |
go/vt/vtadmin/vtadmin2/templates/transaction.html |
Displays transaction details. |
go/vt/vtadmin/vtadmin2/templates/transactions.html |
Lists unresolved transactions. |
go/vt/vtadmin/vtadmin2/templates/vdiff_show.html |
Displays VDiff reports. |
go/vt/vtadmin/vtadmin2/templates/vexplain.html |
Provides VExplain UI. |
go/vt/vtadmin/vtadmin2/templates/vschema.html |
Displays VSchema details. |
go/vt/vtadmin/vtadmin2/templates/vschemas.html |
Lists VSchemas. |
go/vt/vtadmin/vtadmin2/templates/vtctlds.html |
Lists VTctlds. |
go/vt/vtadmin/vtadmin2/templates/vtexplain.html |
Provides VTExplain UI. |
go/vt/vtadmin/vtadmin2/templates/workflow.html |
Displays workflow actions/details. |
go/vt/vtadmin/vtadmin2/templates/workflow_materialize_create.html |
Provides Materialize form. |
go/vt/vtadmin/vtadmin2/templates/workflow_movetables_create.html |
Provides MoveTables form. |
go/vt/vtadmin/vtadmin2/templates/workflow_reshard_create.html |
Provides Reshard form. |
go/vt/vtadmin/vtadmin2/templates/workflow_status.html |
Displays workflow status. |
go/vt/vtadmin/vtadmin2/templates/workflows.html |
Lists workflows. |
go/vt/vtadmin/vtadmin2/tools.go |
Implements explain tools. |
go/vt/vtadmin/vtadmin2/tools_test.go |
Tests explain tools. |
go/vt/vtadmin/vtadmin2/topology.go |
Renders topology selection. |
go/vt/vtadmin/vtadmin2/topology_test.go |
Tests topology selection. |
go/vt/vtadmin/vtadmin2/topology_tree.go |
Builds topology hierarchy. |
go/vt/vtadmin/vtadmin2/topology_tree_test.go |
Tests topology hierarchy. |
go/vt/vtadmin/vtadmin2/transaction_actions.go |
Implements transaction conclusion. |
go/vt/vtadmin/vtadmin2/transaction_actions_test.go |
Tests transaction conclusion. |
go/vt/vtadmin/vtadmin2/view.go |
Provides template helpers. |
go/vt/vtadmin/vtadmin2/view_test.go |
Tests template helpers. |
go/vt/vtadmin/vtadmin2/vschema.go |
Renders serving schema pages. |
go/vt/vtadmin/vtadmin2/vschema_test.go |
Tests serving schema pages. |
go/vt/vtadmin/vtadmin2/workflows.go |
Renders workflow and VDiff pages. |
go/vt/vtadmin/vtadmin2/workflows_actions.go |
Implements workflow actions. |
go/vt/vtadmin/vtadmin2/workflows_actions_test.go |
Tests workflow actions. |
go/vt/vtadmin/vtadmin2/workflows_create.go |
Implements workflow creation. |
go/vt/vtadmin/vtadmin2/workflows_create_test.go |
Tests workflow creation. |
go/vt/vtadmin/vtadmin2/workflows_test.go |
Tests workflow pages. |
go/vt/vtadmin/vtctldclient/fakevtctldclient/vtctldclient.go |
Adds transaction fake support. |
Suppressed comments (1)
go/vt/vtadmin/vtadmin2/shard.go:273
API.EmergencyFailoverShardreturns(nil, nil)when the actor lacksEmergencyFailoverShardAction. Because the response is discarded, the UI claims the emergency failover completed even when authorization prevented it; that false signal can misdirect incident response. Treat a nil response as denial, or make the API return an authorization error.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| resp, err := s.api.GetSrvKeyspaces(r.Context(), &vtadminpb.GetSrvKeyspacesRequest{ | ||
| ClusterIds: queryValues(r, "cluster_id"), | ||
| Cells: queryValues(r, "cell"), | ||
| }) |
| redirect := "/vdiff/" + pathEscape(clusterID) + "/show?" + | ||
| "cluster_id=" + url.QueryEscape(clusterID) + | ||
| "&keyspace=" + url.QueryEscape(keyspace) + | ||
| "&workflow=" + url.QueryEscape(workflow) |
| Handler: handler, | ||
| ReadHeaderTimeout: 10 * time.Second, | ||
| ReadTimeout: 30 * time.Second, | ||
| WriteTimeout: 5 * time.Minute, |
| _, err := s.api.ReloadSchemaShard(r.Context(), &vtadminpb.ReloadSchemaShardRequest{ | ||
| ClusterId: clusterID, | ||
| Keyspace: keyspace, | ||
| Shard: shard, | ||
| IncludePrimary: r.Form.Get("include_primary") == "on", |
| _, err = s.api.PlannedFailoverShard(r.Context(), &vtadminpb.PlannedFailoverShardRequest{ | ||
| ClusterId: clusterID, | ||
| Options: options.Planned, | ||
| }) | ||
| if err != nil { | ||
| s.renderFormError(w, r, title, err.Error()) | ||
| return | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7a70d344b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| output "\n\033[1;32mStarting vtadmin2 on http://${case_insensitive_hostname}:${vtadmin2_port}\033[0m" | ||
|
|
||
| vtadmin2 \ |
There was a problem hiding this comment.
This script is now started by examples/local/101_initial_cluster.sh by default, but it invokes a standalone vtadmin2 binary. I checked the install and release copy lists (Makefile install/install-local and tools/make-release-packages.sh), and they still copy/package vtadmin but not vtadmin2, so users running the packaged examples from an install/release tree will fail here with vtadmin2: command not found unless they know to set SKIP_VTADMIN2.
Useful? React with 👍 / 👎.
| Data: createReshardData{ | ||
| Form: form, | ||
| PickCluster: pickCluster, | ||
| SelectedCluster: requestedCluster, |
There was a problem hiding this comment.
When /workflows/reshard/create is opened without an explicit cluster_id in a single-cluster setup, loadFormOptions has already selected the first/default cluster, but this stores the empty requested value instead. The template filters keyspace options with $d.SelectedCluster, so the normal initial reshard form renders an empty keyspace select and cannot be submitted without manually adding ?cluster_id=....
Useful? React with 👍 / 👎.
| "&workflow=" + url.QueryEscape(workflow) | ||
|
|
||
| if u := strings.TrimSpace(r.Form.Get("uuid")); u != "" { | ||
| redirect += "&uuid=" + url.QueryEscape(u) |
There was a problem hiding this comment.
When the user supplies a VDiff UUID, the redirect preserves it as uuid=..., but vdiffShow only reads the arg query parameter and otherwise defaults to last. As a result, the post-create page silently ignores the requested UUID and can show a different VDiff; the redirect needs to use the parameter consumed by the show handler.
Useful? React with 👍 / 👎.
| return | ||
| } | ||
|
|
||
| redirectWithFlash(w, r, "/transactions?cluster_id="+url.QueryEscape(clusterID), Flash{ |
There was a problem hiding this comment.
After a successful conclude this redirects to /transactions with only cluster_id, but the transactions handler treats any query string as a filtered request and requires both cluster_id and keyspace. Users therefore land on a 400 “keyspace query parameter is required” page immediately after the action succeeds; either include the keyspace in the form/redirect or redirect to the unfiltered transactions page.
Useful? React with 👍 / 👎.
| Request: &vtctldatapb.WorkflowSwitchTrafficRequest{ | ||
| Keyspace: keyspace, | ||
| Workflow: workflow, | ||
| TabletTypes: tabletTypes, |
There was a problem hiding this comment.
If an operator leaves the switch/reverse tablet-type checkboxes untouched, this sends an empty TabletTypes list. I checked the vtctldclient switchtraffic pre-run (go/cmd/vtctldclient/command/vreplication/common/switchtraffic.go:43-50), and the established CLI behavior defaults an omitted tablet-types flag to PRIMARY, REPLICA, and RDONLY; the workflow server only switches work inside the primary/replica/rdonly branches, so the empty UI request can return success while doing no traffic switch.
Useful? React with 👍 / 👎.
| Keyspace: keyspace, | ||
| Shard: shard, |
There was a problem hiding this comment.
This new shard action passes the selected keyspace and shard to vtadmin.API.ReloadSchemaShard, but that API currently drops both fields when constructing the vtctld request (go/vt/vtadmin/api.go:2305-2308), while vtctldata.ReloadSchemaShardRequest requires them. Clicking Reload schema on any shard therefore runs with empty keyspace/shard and can no-op or report a misleading success instead of reloading the selected shard.
Useful? React with 👍 / 👎.
| SourceKeyspace: sourceKeyspace, | ||
| TargetKeyspace: targetKeyspace, | ||
| AllTables: allTables, | ||
| IncludeTables: includeTables, |
There was a problem hiding this comment.
If a user checks some tables and then also checks “Copy all tables,” the browser submits both table=... and all_tables=on. This request still forwards the selected IncludeTables even when AllTables is true, but MoveTablesCreate rejects that exact combination (go/vt/vtctl/workflow/server.go:1101-1103), so the form fails despite the template saying individual selection is ignored; clear the include list when allTables is set.
Useful? React with 👍 / 👎.
| _, err = s.api.TabletExternallyPromoted(r.Context(), &vtadminpb.TabletExternallyPromotedRequest{ | ||
| Alias: alias, | ||
| ClusterIds: []string{clusterID}, | ||
| }) |
There was a problem hiding this comment.
On the shard detail page the route scopes this action to /{keyspace}/{shard}, but the request only passes the typed tablet alias; Cluster.TabletExternallyPromoted calls vtctld with that alias and updates the alias’s shard. If an operator enters an alias from a different shard, this page mutates that other shard while appearing as a current-shard action. Validate the alias tablet belongs to the route keyspace/shard before calling the API.
Useful? React with 👍 / 👎.
Promptless documentation updates
|
Description
Related Issue(s)
#20107
Checklist
Deployment Notes
AI Disclosure