solver: route gateway dockerfile.v0 source to builtin frontend#6643
solver: route gateway dockerfile.v0 source to builtin frontend#6643tonistiigi merged 1 commit intomoby:masterfrom
Conversation
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
2 times, most recently
from
7067599 to
75e48b4
Compare
| return nil, errors.Errorf("no source specified for gateway") | ||
| } | ||
|
|
||
| if cmdline := strings.TrimSpace(opts["cmdline"]); cmdline == "dockerfile.v0" { |
There was a problem hiding this comment.
why is this checking cmdline instead of just source == "dockerfile.v0
There was a problem hiding this comment.
I think cmdline is stricter than source. Buildx sets source to the first token of BUILDKIT_SYNTAX and keeps the original value in cmdline, so checking only source would also treat values like dockerfile.v0 foo as the builtin selector. I keyed it off strings.TrimSpace(cmdline) == "dockerfile.v0" to keep the special case limited to the exact selector and avoid changing behavior for malformed values. If you prefer source == "dockerfile.v0" for simplicity, I can switch it.
There was a problem hiding this comment.
The contract is that gateway will load whatever is in the source option. cmdline is just a way to pass arguments that are not parsed by buildkit itself. So I think we should only use source.
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
from
75e48b4 to
cc113e9
Compare
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
from
cc113e9 to
80b841c
Compare
crazy-max
changed the title
|
|
||
| var pred provenancetypes.ProvenancePredicateSLSA1 | ||
| require.NoError(t, json.Unmarshal(dt, &pred)) | ||
| require.Empty(t, pred.BuildDefinition.ExternalParameters.Request.Frontend) |
There was a problem hiding this comment.
Why is empty correct here? Shouldn't it be dockerfile.v0?
There was a problem hiding this comment.
Hum yeah you're right, I changed this. The previous test was going through c.Build(..., frontendFunc), so it was the client-frontend wrapper path and Request.Frontend stayed empty for that reason, not because the gateway handoff was correct. I changed the test to run a top-level gateway.v0 solve with source=dockerfile.v0 so the recorded frontend is dockerfile.v0, which is the thing we actually want here.
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
from
80b841c to
f68d2a8
Compare
|
Check CI. For this to be correct in provenance I think the switch needs to be before the request is routed to frontend. |
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
from
f68d2a8 to
2d1526c
Compare
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
3 times, most recently
from
d212c08 to
8351b9a
Compare
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
2 times, most recently
from
318a5bf to
a94c493
Compare
crazy-max
changed the title
| } | ||
|
|
||
| func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req frontend.SolveRequest, exp ExporterRequest, ent []entitlements.Entitlement, post []Processor, internal bool, srcPol *spb.Policy, policySession string) (_ *client.SolveResponse, err error) { | ||
| if req.Frontend == "gateway.v0" && req.FrontendOpt[frontend.KeySource] == "dockerfile.v0" { |
There was a problem hiding this comment.
maybe we should check if there is no named context set for dockerfile.v0?
There was a problem hiding this comment.
good catch! I added a guard so the shortcut only applies when there is no context:dockerfile.v0 override, including the platform context:dockerfile.v0::... form
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
crazy-max
force-pushed
the
gateway-builtin-syntax
branch
from
a94c493 to
3f3c957
Compare
2 participants
alternative to and closes #6594
BuildKit treats
gateway.v0requests withsource=dockerfile.v0as requests for the built-in Dockerfile frontend. The request is normalized before frontend dispatch, so the solve is handled and recorded asdockerfile.v0instead of remaining agateway.v0request.This keeps the behavior aligned with the actual request shape that Buildx sends today. It also makes provenance and history reflect the built-in Dockerfile frontend correctly.