🌱 [scanner] introduce handlers.Deps and Registrar; port admin subpackage - #23738
Conversation
…e (Refs #23725) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: GitHub Copilot <223556219+Copilot@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
|
👋 Hey @hivecommons-hive[bot] — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
There was a problem hiding this comment.
Read the diff against the tree at merge-base 47b5c855 and verified this is the pure wiring refactor the body claims.
Route parity verified. The admin registrar (pkg/api/handlers/admin/registrar.go) registers the same 24 paths/methods, in the same order, as the blocks removed from route_group_api_core.go (settings, was lines ~142–146) and route_group_governance.go (teams/RBAC/rate-limit, was lines ~26–68). Constructor args are unchanged: NewRBACHandler(deps.Store, deps.K8sClient), NewAdminHandler(deps.FailureTracker, deps.Store) — and FailureTracker is still sourced from s.auth.failureTracker (routes_api_core.go:13), the same value governance previously passed.
Middleware parity verified. The registrar mounts on the same routes.api group (route_group_api_core.go:52) that the governance group used, created at routes_auth.go:292 with apiLimiterWithSkip, bodyGuard, csrfGuard, jwtAuth — no auth surface changed.
Alias deletion is safe. Grepped the PR head: no remaining references to the deleted admin_aliases.go re-exports outside pkg/api/handlers/admin; the one non-registrar call site (/api/me) now imports admin.NewUserHandler directly.
Ran locally at head c85eb16: go build ./pkg/... clean; go test ./pkg/api/ ./pkg/api/handlers/... all pass, including the new registrar route-surface lock test and the full-server routes_registration_test.go.
Looks correct to me. Refs #23725 (partial) is appropriate — slices 2b (remaining subpackages) and 2c (route-group collapse) are explicitly deferred.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
…e 2a+2b) (#23742) Slices 2a and 2b landed in #23738; this completes 2b by porting the last hand-wired admin-domain routes (/api/me) to the Registrar contract and sharing a single handlers.Deps value across the api-core route group. Signed-off-by: GitHub Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: GitHub Copilot <223556219+Copilot@users.noreply.github.com>
Refs #23725 (partial: slice 2a + admin template per the issue's plan; remaining 2b ports of k8s/proxy/ops/dashboards/persistence and 2c route-group collapse are follow-up PRs)
What changed
Slice 2a —
handlers.Deps+handlers.Registrar(newpkg/api/handlers/registrar.go)Depscarries the shared dependency set with the repo's real types:store.Store,*transport.Hub,*k8s.MultiClusterClient,*store.PersistenceStore,*notifications.Service,*middleware.FailureTracker,GitHubToken string.Config *config.Configfield;Configlives in packageapi, which importshandlers, so config-derived values are carried as explicit fields (GitHubTokentoday) to avoid an import cycle. This is documented in the file.RegistrarisRegister(router fiber.Router, deps Deps). No behavior change; every existing constructor still works.Slice 2b —
admin/ported as the template (newpkg/api/handlers/admin/registrar.go)admin.NewRegistrar() handlers.RegistrarbuildsSettingsHandler,TeamHandler,RBACHandlerandAdminHandlerfromDepsand registers the exact same paths/methods/order previously hand-wired inroute_group_api_core.go(settings) androute_group_governance.go(teams, RBAC,/admin/rate-limit-status).admin.NewRegistrar().Register(api, handlers.Deps{...})in the api-core route group (it runs first and now receivesfailureTracker); the governance group drops those blocks and its now-unusedfailureTrackerdependency./api/mestays in the route group because it is mounted on the app withbodyGuard/csrfGuard/jwtAuth— auth middleware deliberately stays in the route group per the issue. It now importsadmin.NewUserHandlerdirectly.pkg/api/handlers/admin_aliases.godeleted:go build ./...confirms nothing else referenced the root re-exports (the only 5 call sites were the two route groups). Deleting it also removes thehandlers -> adminedge, soadmincan importhandlersforDeps/Registrarwithout a cycle.No route paths, methods, middleware or handler semantics changed — this is a pure wiring refactor.
Verification
go build ./...— clean.go vet ./pkg/api/...— clean.go test ./pkg/api/...— all packages pass, includingroutes_registration_test.go(full-server route table: asserts/api/settingsis registered exactly once, plus all/api/teams*,/api/users*,/api/rbac/*,/api/admin/rate-limit-statusroutes) androute_group_api_core_test.go.pkg/api/handlers/admin/registrar_test.golocks the 24-route surface the registrar owns so later ports can't silently drop a route.— hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88