Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Dalec’s frontend routing from a hierarchical, nested BuildMux to a single flat frontend.Router with fully-qualified routes. It also changes the plugin model to provide routes at request time (enabling spec-aware route availability), and extends the targets listing payload with dalec-specific metadata (specDefined, hidden) to help clients.
Changes:
- Replace
frontend.BuildMuxwith a flatfrontend.Routerand update dispatch/listing/subrequest handling accordingly. - Switch target plugins from “build-target” handlers to “route-provider” plugins returning
[]frontend.Route. - Add
internal/gwutilwrappers to preserveCurrentFrontend/SpecLoaderacross gwclient wrappers; update docs and tooling to the new targets and routing flow.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/examples/targets.md | Updates the documented target list to include new RPM debug subroutes. |
| test/testenv/buildx.go | Wraps gateway client wrappers with gwutil.WithCurrentFrontend to preserve interfaces. |
| test/fixtures/phony/main.go | Migrates test fixture frontend from BuildMux to Router. |
| targets/windows/handler.go | Converts Windows target handling into a route provider (Routes) returning flat routes. |
| targets/register.go | Replaces build-target registration with route-provider registration. |
| targets/plugin/init.go | Registers distro route providers instead of build-target handlers. |
| targets/linux/rpm/rockylinux/common.go | Removes old Handlers/builtin mux target map usage. |
| targets/linux/rpm/distro/distro.go | Converts RPM distro config from mux-based handling to flat Routes, includes debug routes. |
| targets/linux/rpm/distro/debug.go | Replaces nested debug mux with DebugRoutes returning flat debug routes. |
| targets/linux/rpm/azlinux/common.go | Removes old Handlers/builtin mux target map usage. |
| targets/linux/rpm/almalinux/common.go | Removes old Handlers/builtin mux target map usage. |
| targets/linux/distro_handler.go | Refactors base image config resolution helper (resolveBaseConfig). |
| targets/linux/deb/ubuntu/common.go | Removes old Handlers/builtin mux target map usage. |
| targets/linux/deb/distro/distro.go | Converts DEB distro config from mux-based handling to flat Routes. |
| targets/linux/deb/debian/common.go | Removes old Handlers/builtin mux target map usage. |
| internal/plugins/types.go | Changes plugin type contract to TypeRouteProvider with RouteProvider interface. |
| internal/gwutil/gwutil.go | Introduces helper to preserve CurrentFrontend and SpecLoader across client wrapping. |
| internal/frontendapi/router.go | Replaces build-router creation with NewRouter that loads route providers and caches spec loads. |
| frontend/router_test.go | Adds tests for router dispatch (exact match, prefix match, default route, not-found). |
| frontend/router.go | Adds the new flat router, extended target list response, and spec-loading helper. |
| frontend/mux_test.go | Removes old mux tests (replaced by router tests). |
| frontend/mux.go | Removes the old hierarchical BuildMux implementation. |
| frontend/debug/plugin/init.go | Removes debug plugin registration via the old build-target plugin type. |
| frontend/debug/handler.go | Converts debug routing to flat debug.Routes(prefix) route definitions. |
| frontend/client.go | Extracts shared client/opts helpers and subrequest handlers from the removed mux. |
| frontend/build.go | Updates platform wrapper to preserve CurrentFrontend via gwutil. |
| cmd/worker-img-matrix/main.go | Updates worker matrix generation to load route providers and discover /worker routes. |
| cmd/gen-doc-targets/main.go | Updates doc target generation to use the new router and provide a stub client for empty-spec loading. |
| cmd/frontend/main.go | Creates the router per request and dispatches through Router.Handler(WithTargetForwardingHandler). |
2da7548 to
5c5da9a
Compare
5c5da9a to
aeebc40
Compare
aeebc40 to
6aec426
Compare
6aec426 to
6417397
Compare
| Target: bktargets.Target{ | ||
| Name: frontendKey, | ||
| Description: fmt.Sprintf("Forwarded to custom frontend: %s", frontendTarget.Frontend.Image), | ||
| }, |
There was a problem hiding this comment.
The forwarding route metadata (Info) does not set SpecDefined. If the forwarded frontend came from spec.Targets (which is the only way this route is added), setting SpecDefined=true here would make the target list output more accurate, especially in the fallback path when querying the forwarded frontend fails.
| Target: bktargets.Target{ | |
| Name: frontendKey, | |
| Description: fmt.Sprintf("Forwarded to custom frontend: %s", frontendTarget.Frontend.Image), | |
| }, | |
| Target: bktargets.Target{ | |
| Name: frontendKey, | |
| Description: fmt.Sprintf("Forwarded to custom frontend: %s", frontendTarget.Frontend.Image), | |
| }, | |
| SpecDefined: true, |
invidian
left a comment
There was a problem hiding this comment.
Comments for frontend: replace BuildMux with flat Router.
| @@ -11,23 +12,49 @@ import ( | |||
| ) | |||
|
|
|||
| func init() { | |||
There was a problem hiding this comment.
While we're at it, if we use plugins only for building routes and we already have some statically defined routes, I wonder if we could load those explicitly in internal/frontendapi/router.go or somewhere along the way in the composition tree, while I guess still keeping logic for loading from plugins from some external sources (although perhaps exposing Dalec as a library would still be better so wrappers can do their own transparent composition?)
That might be out of scope of course.
invidian
left a comment
There was a problem hiding this comment.
For frontend: make route registration spec-aware
Move standalone types, constants, and functions from mux.go into a dedicated client.go file. This is a pure code movement with no behavioral changes, separating client/request helpers from the BuildMux routing logic to prepare for replacing BuildMux with a flat Router. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
6417397 to
11b3e2c
Compare
Replace the hierarchical BuildMux (nested sub-muxes) with a single flat Router that uses fully-qualified route paths like "azlinux3/container" and "debug/resolve". All routes are registered at startup via RouteProvider plugins instead of being lazily discovered through handler invocation. Key changes: - New Router type in frontend/router.go with exact and longest-prefix match dispatch - New RouteProvider plugin interface replaces BuildHandler/BuildTarget - Distro packages export Routes(prefix) instead of Handle() methods - Target list API returns SpecDefined annotation without invoking handlers - Remove all dead BuildMux code, unused Handlers() funcs, and the debug/plugin/init.go registration shim Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Load the spec once in NewRouter via LoadSpecForRouting and pass the *dalec.Spec to each RouteProvider.Routes() call. This enables: - SpecDefined annotation on each route (whether the target key appears in spec.Targets), used by the target list API - Per-request NewRouter(ctx, client) in the BuildFunc, since route registration now needs the gateway client - LoadSpecForRouting() helper in frontend/router.go that performs lenient arg substitution for routing purposes only - Each distro registers as a separate RouteProvider via RegisterRouteProvider() - Bare distro prefix routes (e.g. "mariner2") as hidden aliases for the default container handler - Hidden field on frontend.Target to exclude alias routes from the target list while keeping them dispatchable - DebugRoutes now registers individual sub-routes (buildroot, sources, spec) instead of a single prefix handler - gen-doc-targets uses a stub gwclient.Client to set up routes for documentation generation Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
11b3e2c to
1e4d420
Compare
This is in preparation of #986
It also just generally fixes issues with routing for routes with sub-routers (e.g. see that targets.md now populates certain debug routes).
It moves to using a single, flat router for all routes.
Route registration happens at request time since available routes can be spec-dependent.
For the targets json API (docker build --call frontend.targets,format=json) it now includes a field to give extra hints to clients, such as the vscode extension, on if the targets are explicit in the spec or not.