Repository navigation
feat(verifier): serve the admin console from the verifier factories - #1501
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Aggregator transport settings are lost, pre-flight validation is incomplete, and config access failures can silently disable the console.
3 open findings
What changed in this PR
Wires the admin console into committee and token verifier lifecycles and adds configuration validation through the CLI.
Changes:
- Starts and stops the in-process console with verifier jobs.
- Uses the committee verifier’s first aggregator by default.
- Adds
ccv admin check-config.
| File | Description |
|---|---|
cmd/verifier/tokenfactory.go |
Integrates console lifecycle into token verifiers. |
cmd/verifier/servicefactory.go |
Integrates console lifecycle and aggregator defaults. |
cmd/verifier/run_ccv_cli.go |
Registers admin CLI commands. |
cmd/verifier/adminconsole.go |
Implements shared console startup and shutdown. |
cli/admin/commands.go |
Adds console configuration validation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if len(resolvedAggregators) > 0 { | ||
| aggregatorAddress = resolvedAggregators[0].Address | ||
| } |
There was a problem hiding this comment.
Done — DialResultsClient now takes the transport mode, and the wiring passes the selected aggregator's insecure_connection through (startAdminConsole wraps it into admin.Deps.ResultsDialer). A config-file aggregator_address overrides the endpoint, not the transport mode, and the console's default dialer stays TLS.
| Action: func(c *cli.Context) error { | ||
| cfg, err := admin.LoadConfig(c.String("config")) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Done — admin.Command now receives the CLI-loaded verifier secrets, and check-config runs BasicAuthFromSecrets plus ValidateAccessPolicy, so a non-loopback bind without an identity source or a half-supplied [admin_ui] pair fails exactly as startup would. The guide's pre-flight section is updated to match.
| if _, err := os.Stat(path); err != nil { //nolint:gosec // G703: operator-provided config path, not request input. | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
Done — only fs.ErrNotExist means disabled; any other stat error is returned so an unreadable-but-present config fails the startup visibly.
|
Wiring LGTM - I think some changes will happen if some of my suggestions are adopted from #1500. |
|
Code coverage report:
Files added (in
|
Summary
Middle of a 3-PR stack splitting #1471 (base: #1500). Wires the admin console into the verifier lifecycle:
/etc/ccv-admin/config.toml(CCV_ADMIN_CONFIG_PATH) is present; no config file means disabled. The console shares the verifier's application DB and its[admin_ui]credential, and shuts down with the job.aggregator_addressin the config file overrides).ccv admin serveand the supervised sibling process are gone (per review);ccv admin check-configremains for pre-flight validation.Stack: #1500 → this PR → docs.