Rewrite target routing logic #172
Conversation
cpuguy83
force-pushed
the
rewrite_router
branch
6 times, most recently
from
b169d44
to
c8c023f
Compare
|
Just noting here that we may want to account for cross-compilation in the structure of the routes themselves. See this discussion on the Windows PR: #108 (comment) tldr: currently the first path component refers to both the build toolchain and the output (for example, |
cpuguy83
force-pushed
the
rewrite_router
branch
3 times, most recently
from
cdce81e
to
58205e7
Compare
frontend/mux.go
Outdated
| // cases for `t` are as follows: | ||
| // 1. may have an exact match in the handlers (ideal) | ||
| // 2. may have a prefix match in the handlers, e.g. hander for `foo`, `target == "foo/bar"` (assume nested route) | ||
| // 3. No matching handler and `target == ""` and there is a default handler set (assume default handler) |
There was a problem hiding this comment.
nit: formatting
nit2: match the order of the cases in this list and their order in the code
| ) | ||
|
|
||
| const ( | ||
| targetKey = "windowscross" | ||
| outputKey = "windows" | ||
| DefaultTargetKey = "windowscross" |
There was a problem hiding this comment.
note: no action needed at this time
I am thinking ahead to when (or if?) we change the route to windows/cross/container (or possibly windows/container/cross). That is going to require some additional work in the targets part of the spec, because we will have one "dalec.target" for both cross-compiled and non-cross-compiled windows containers (both would have windows as the top-level path component).
There might be different requirements, like different build dependencies. What do we do about that?
There was a problem hiding this comment.
Could we specify dependencies specifically for windows/cross in the targets portion of the spec, i.e. list windows/cross as the target key?
There was a problem hiding this comment.
That would be difficult, because the client's opts is updated to have dalec.target set to the first path component. The rest of the target will have been gradually chomped away by the recursive calls to mux.Handle.
There was a problem hiding this comment.
Yeah, I see what you're saying. Trying to think about how this could be dealt with. One possible option is, in addition to dalec.target, we could keep an option that is updated with the current handler path. So, the handler registered at windows/cross/container would know the path that was taken to get to it. Then, when fetching dependencies for a target, we could first try the full handler path before falling back to dalec.target. Just an idea. It does kind of break the nested handler abstraction, though
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
cpuguy83
force-pushed
the
rewrite_router
branch
3 times, most recently
from
282e737
to
7cd95af
Compare
|
This should be all updated with requested items resolved. |
This breaks down routing logic into smaller pieces.
Introduces a `BuildMux` upon which all the target routing is handled.
`BuildMux` implements a buildkit `BuildFunc` via its `Handle` method.
With a `BuildMux` you register routes with `mux.Add("someKey",
SomeHandler, optionalTargetInfo)`.
When a build request is made, `BuildMux` tries to match the requested
build target to a registered handler via the registered handler.
The correct handler to use is determined with the following logic:
1. Build target is an exact match to a registered route
2. Build target is empty, check if a default handler is registered
3. Check if any of the registered handlers have a route that is a prefix
match to the build target
`BuildMux` route handlers also must be buildkit `BuildFunc`s.
As such a handler can itself also be a distinc `BuildMux` with its own
set of routes.
This allows handlers to also do their own routing.
All logic for *what* to route is handled outside of the `BuildMux`.
`BuildMux` and buildkit `BuildFunc` have a similar relationship as
`http.ServeMux` and `http.Handler` (where `http.ServeMux` *is* an
`http.Handler`).
In the same way `BuildMux` is a `BuildFunc` (or rather `BuildMux.Handle`
is).
So `BuildMux` can be nested.
When `BuildMux` calls a handler, it modifies the client to chomp off the
matched route prefix.
So a `BuildMux` with receiving a build target of `mariner2/container` will
match on the registered handler for `mariner2` then call the handler
with the build target changed to just `container`.
Finally, `BuildMux` sets an extra build option on the client
`dalec.target=<matched prefix>`.
This is only done if the `dalec.target` option is not already set, so
`dalec.target` is only modified once and then used in handlers to
determine which target in `spec.Targets` applies to them.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
frontend/mux.go
Outdated
|
|
||
| // Add adds a handler for the given target | ||
| // [targetKey] is the resource path to be handled | ||
| func (m *BuildMux) Add(targePath string, bf gwclient.BuildFunc, info *bktargets.Target) { |
|
One tiny typo to fix |
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This breaks down routing logic into smaller pieces. Introduces a
BuildMuxupon which all the target routing is handled.BuildMuximplements a buildkitBuildFuncvia itsHandlemethod. With aBuildMuxyou register routes withmux.Add("someKey", SomeHandler, optionalTargetInfo).When a build request is made,
BuildMuxtries to match the requested build target to a registered handler via the registered route. The correct handler to use is determined with the following logic:BuildMuxroute handlers also must be buildkitBuildFuncs. As such a handler can itself also be a distincBuildMuxwith its own set of routes.This allows handlers to also do their own routing. All logic for what to route is handled outside of the
BuildMux.BuildMuxand buildkitBuildFunchave a similar relationship ashttp.ServeMuxandhttp.Handler(wherehttp.ServeMuxis anhttp.Handler).In the same way
BuildMuxis aBuildFunc(or ratherBuildMux.Handleis).So
BuildMuxcan be nested.When
BuildMuxcalls a handler, it modifies the client to chomp off the matched route prefix.So a
BuildMuxwith receiving a build target ofmariner2/containerwill match on the registered handler formariner2then call the handler with the build target changed to justcontainer.Finally,
BuildMuxsets an extra build option on the clientdalec.target=<matched prefix>.This is only done if the
dalec.targetoption is not already set, sodalec.targetis only modified once and then used in handlers to determine which target inspec.Targetsapplies to them.