From 4b5e0a27ab5f56f47c0a698d2e0044df0f24b372 Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:06:40 +0200 Subject: [PATCH] driver: propagate feature detection errors Return feature detection errors through the driver interface and cache only successful Docker worker probes so transient failures can be retried. Resolve features before constructing build options and reuse that result for capability checks. This prevents failed probes from silently disabling default provenance before a later successful probe allows the build to proceed. Report detection failures in debug inspection and cover retries, caching, and provenance preservation. Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> --- build/build.go | 6 ++- build/opt.go | 22 +++++---- build/opt_test.go | 51 ++++++++++++++++++-- commands/inspect.go | 20 ++++---- driver/cloud/driver.go | 4 +- driver/docker-container/driver.go | 4 +- driver/docker/driver.go | 36 +++++++-------- driver/docker/driver_test.go | 77 +++++++++++++++++++++++++++++++ driver/driver.go | 2 +- driver/kubernetes/driver.go | 4 +- driver/remote/driver.go | 4 +- 11 files changed, 182 insertions(+), 48 deletions(-) create mode 100644 driver/docker/driver_test.go diff --git a/build/build.go b/build/build.go index f1b90ea0230a..345782102786 100644 --- a/build/build.go +++ b/build/build.go @@ -984,8 +984,12 @@ func Build(ctx context.Context, nodes []builder.Node, opts map[string]Options, d } node := dp.Node().Driver if node.IsMobyDriver() { + features, err := node.Features(ctx) + if err != nil { + return errors.Wrap(err, "failed to detect driver features") + } for _, e := range so.Exports { - if e.Type == "moby" && e.Attrs["push"] != "" && !node.Features(ctx)[driver.DirectPush] { + if e.Type == "moby" && e.Attrs["push"] != "" && !features[driver.DirectPush] { if ok, _ := strconv.ParseBool(e.Attrs["push"]); ok { pushNames = e.Attrs["name"] if pushNames == "" { diff --git a/build/opt.go b/build/opt.go index 67a524286bf3..d0f07788ddf9 100644 --- a/build/opt.go +++ b/build/opt.go @@ -246,6 +246,10 @@ func isPolicyEvaluationError(policies []*policy.Policy, err error) bool { func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver bool, opt *Options, bopts gateway.BuildOpts, cfg *confutil.Config, pw progress.Writer, docker *dockerutil.Client) (_ *client.SolveOpt, release func(error), err error) { node := np.Node() nodeDriver := node.Driver + driverFeatures, err := nodeDriver.Features(ctx) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to detect driver features") + } defers := make([]func(error), 0, 2) releaseF := func(inErr error) { for _, f := range defers { @@ -270,7 +274,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver } for _, e := range opt.CacheTo { - if e.Type != "inline" && !nodeDriver.Features(ctx)[driver.CacheExport] { + if e.Type != "inline" && !driverFeatures[driver.CacheExport] { return nil, nil, notSupported(driver.CacheExport, nodeDriver, "https://docs.docker.com/go/build-cache-backends/") } } @@ -347,10 +351,10 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver } } - supportAttestations := bopts.LLBCaps.Contains(apicaps.CapID("exporter.image.attestations")) && nodeDriver.Features(ctx)[driver.MultiPlatform] + supportAttestations := bopts.LLBCaps.Contains(apicaps.CapID("exporter.image.attestations")) && driverFeatures[driver.MultiPlatform] if len(attests) > 0 { if !supportAttestations { - if !nodeDriver.Features(ctx)[driver.MultiPlatform] { + if !driverFeatures[driver.MultiPlatform] { return nil, nil, notSupported("Attestation", nodeDriver, "https://docs.docker.com/go/attestations/") } return nil, nil, errors.Errorf("Attestations are not supported by the current BuildKit daemon") @@ -392,7 +396,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver // backwards compat for docker driver only: // this ensures the build results in a docker image. opt.Exports = []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}} - } else if nodeDriver.Features(ctx)[driver.DefaultLoad] { + } else if driverFeatures[driver.DefaultLoad] { opt.Exports = []client.ExportEntry{{Type: "docker", Attrs: map[string]string{}}} } } @@ -403,7 +407,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver } // check if index annotations are supported by docker driver - if len(opt.Exports) > 0 && opt.CallFunc == nil && len(opt.Annotations) > 0 && nodeDriver.IsMobyDriver() && !nodeDriver.Features(ctx)[driver.MultiPlatform] { + if len(opt.Exports) > 0 && opt.CallFunc == nil && len(opt.Annotations) > 0 && nodeDriver.IsMobyDriver() && !driverFeatures[driver.MultiPlatform] { for _, exp := range opt.Exports { if exp.Type == "image" || exp.Type == "docker" { for ak := range opt.Annotations { @@ -477,7 +481,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver // set up exporters for i, e := range so.Exports { - if e.Type == "oci" && !nodeDriver.Features(ctx)[driver.OCIExporter] { + if e.Type == "oci" && !driverFeatures[driver.OCIExporter] { return nil, nil, notSupported(driver.OCIExporter, nodeDriver, "https://docs.docker.com/go/build-exporters/") } if e.Type == "docker" { @@ -525,14 +529,14 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver so.Exports[i].Attrs["prefer-image-digest"] = "true" } } - } else if !nodeDriver.Features(ctx)[driver.DockerExporter] { + } else if !driverFeatures[driver.DockerExporter] { return nil, nil, notSupported(driver.DockerExporter, nodeDriver, "https://docs.docker.com/go/build-exporters/") } } if e.Type == "image" && nodeDriver.IsMobyDriver() { so.Exports[i].Type = "moby" // The containerd image store resolves images by manifest or index digest. - if nodeDriver.Features(ctx)[driver.PreferImageDigest] { + if driverFeatures[driver.PreferImageDigest] { so.Exports[i].Attrs["prefer-image-digest"] = "true" } if e.Attrs["push"] != "" { @@ -619,7 +623,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver for i, p := range opt.Platforms { pp[i] = platforms.FormatAll(p) } - if len(pp) > 1 && !nodeDriver.Features(ctx)[driver.MultiPlatform] { + if len(pp) > 1 && !driverFeatures[driver.MultiPlatform] { return nil, nil, notSupported(driver.MultiPlatform, nodeDriver, "https://docs.docker.com/go/build-multi-platform/") } so.FrontendAttrs["platform"] = strings.Join(pp, ",") diff --git a/build/opt_test.go b/build/opt_test.go index 7dd8bd16b33a..b7442c2dec4f 100644 --- a/build/opt_test.go +++ b/build/opt_test.go @@ -36,7 +36,8 @@ import ( type exporterTestDriver struct { driver.Driver - moby bool + moby bool + features func(context.Context) (map[driver.Feature]bool, error) } func (d exporterTestDriver) Info(context.Context) (*driver.Info, error) { @@ -51,8 +52,11 @@ func (d exporterTestDriver) IsMobyDriver() bool { return d.moby } -func (d exporterTestDriver) Features(context.Context) map[driver.Feature]bool { - return map[driver.Feature]bool{driver.DockerExporter: true} +func (d exporterTestDriver) Features(ctx context.Context) (map[driver.Feature]bool, error) { + if d.features != nil { + return d.features(ctx) + } + return map[driver.Feature]bool{driver.DockerExporter: true}, nil } type exporterTestCLI struct { @@ -159,6 +163,47 @@ func TestDockerExporterFeatureProbe(t *testing.T) { } } +func TestDriverFeatureFailurePreservesProvenance(t *testing.T) { + t.Setenv(noDefaultAttestationsEnv, "false") + probeErr := errors.New("worker is starting") + calls := 0 + d := exporterTestDriver{ + moby: true, + features: func(context.Context) (map[driver.Feature]bool, error) { + calls++ + if calls == 1 { + return nil, probeErr + } + return map[driver.Feature]bool{driver.MultiPlatform: true}, nil + }, + } + nodes, err := noderesolver.Resolve(t.Context(), []builder.Node{{ + Driver: &driver.DriverHandle{Driver: d}, + }}, nil, nil) + require.NoError(t, err) + require.Len(t, nodes, 1) + opt := &Options{ + Inputs: Inputs{ContextPath: "https://example.com/context.tar.gz"}, + Exports: []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}}, + Platforms: []ocispecs.Platform{{OS: "linux", Architecture: "amd64"}, {OS: "linux", Architecture: "arm64"}}, + Policy: []buildflags.PolicyConfig{{Disabled: true}}, + } + cfg := confutil.NewConfig(nil, confutil.WithDir(t.TempDir())) + bopts := buildOptsWithCaps(apicaps.CapID("exporter.image.attestations")) + so, release, err := toSolveOpt(t.Context(), nodes[0], false, opt, bopts, cfg, testProgressWriter{}, nil) + require.ErrorIs(t, err, probeErr) + require.ErrorContains(t, err, "failed to detect driver features") + require.Nil(t, so) + require.Nil(t, release) + require.Equal(t, 1, calls) + + so, release, err = toSolveOpt(t.Context(), nodes[0], false, opt, bopts, cfg, testProgressWriter{}, nil) + require.NoError(t, err) + defer release(nil) + require.Equal(t, "mode=min,inline-only=true", so.FrontendAttrs["attest:provenance"]) + require.Equal(t, 2, calls) +} + func TestCacheOptions_DerivedVars(t *testing.T) { t.Setenv("ACTIONS_RUNTIME_TOKEN", "sensitive_token") t.Setenv("ACTIONS_CACHE_URL", "https://cache.github.com") diff --git a/commands/inspect.go b/commands/inspect.go index 537cccab7f5c..bf004c4fa8bc 100644 --- a/commands/inspect.go +++ b/commands/inspect.go @@ -106,14 +106,18 @@ func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions) e } if debug.IsEnabled() { fmt.Fprintf(w, "Features:\n") - features := nodes[i].Driver.Features(ctx) - featKeys := make([]string, 0, len(features)) - for k := range features { - featKeys = append(featKeys, string(k)) - } - sort.Strings(featKeys) - for _, k := range featKeys { - fmt.Fprintf(w, "\t%s:\t%t\n", k, features[driver.Feature(k)]) + features, err := nodes[i].Driver.Features(ctx) + if err != nil { + fmt.Fprintf(w, "\tError:\t%s\n", err.Error()) + } else { + featKeys := make([]string, 0, len(features)) + for k := range features { + featKeys = append(featKeys, string(k)) + } + sort.Strings(featKeys) + for _, k := range featKeys { + fmt.Fprintf(w, "\t%s:\t%t\n", k, features[driver.Feature(k)]) + } } } if len(nodes[i].Labels) > 0 { diff --git a/driver/cloud/driver.go b/driver/cloud/driver.go index c2e61f75eea4..bde1fff09fe7 100644 --- a/driver/cloud/driver.go +++ b/driver/cloud/driver.go @@ -176,7 +176,7 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client. return c, nil } -func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { +func (d *Driver) Features(_ context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: false, @@ -184,7 +184,7 @@ func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) Factory() driver.Factory { diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index 5800311f40d5..833dfc0f3cdc 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -581,7 +581,7 @@ func (d *Driver) Factory() driver.Factory { return d.factory } -func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { +func (d *Driver) Features(ctx context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: true, @@ -589,7 +589,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) HostGatewayIP(ctx context.Context) (net.IP, error) { diff --git a/driver/docker/driver.go b/driver/docker/driver.go index 402509b613b7..a805b521bd2c 100644 --- a/driver/docker/driver.go +++ b/driver/docker/driver.go @@ -9,6 +9,7 @@ import ( "github.com/docker/buildx/driver" "github.com/docker/buildx/util/progress" "github.com/moby/buildkit/client" + "github.com/moby/buildkit/util/flightcontrol" dockerclient "github.com/moby/moby/client" "github.com/pkg/errors" ) @@ -19,7 +20,7 @@ type Driver struct { // if you add fields, remember to update docs: // https://github.com/docker/docs/blob/main/content/build/drivers/docker.md - features features + features flightcontrol.CachedGroup[map[driver.Feature]bool] hostGateway hostGateway } @@ -74,24 +75,24 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client. return client.New(ctx, "", opts...) } -type features struct { - once sync.Once - list map[driver.Feature]bool -} - -func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { - d.features.once.Do(func() { +func (d *Driver) Features(ctx context.Context) (map[driver.Feature]bool, error) { + return d.features.Do(ctx, "", func(ctx context.Context) (map[driver.Feature]bool, error) { + c, err := d.Client(ctx) + if err != nil { + return nil, err + } + defer c.Close() + workers, err := c.ListWorkers(ctx) + if err != nil { + return nil, errors.Wrap(err, "listing workers") + } var useContainerdSnapshotter bool - if c, err := d.Client(ctx); err == nil { - workers, _ := c.ListWorkers(ctx) - for _, w := range workers { - if _, ok := w.Labels["org.mobyproject.buildkit.worker.snapshotter"]; ok { - useContainerdSnapshotter = true - } + for _, w := range workers { + if _, ok := w.Labels["org.mobyproject.buildkit.worker.snapshotter"]; ok { + useContainerdSnapshotter = true } - c.Close() } - d.features.list = map[driver.Feature]bool{ + return map[driver.Feature]bool{ driver.OCIExporter: useContainerdSnapshotter, driver.DockerExporter: useContainerdSnapshotter, driver.CacheExport: useContainerdSnapshotter, @@ -99,9 +100,8 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.DirectPush: useContainerdSnapshotter, driver.PreferImageDigest: useContainerdSnapshotter, driver.DefaultLoad: true, - } + }, nil }) - return d.features.list } type hostGateway struct { diff --git a/driver/docker/driver_test.go b/driver/docker/driver_test.go new file mode 100644 index 000000000000..107771514128 --- /dev/null +++ b/driver/docker/driver_test.go @@ -0,0 +1,77 @@ +package docker + +import ( + "context" + "net" + "sync/atomic" + "testing" + "time" + + "github.com/docker/buildx/driver" + control "github.com/moby/buildkit/api/services/control" + types "github.com/moby/buildkit/api/types" + dockerclient "github.com/moby/moby/client" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +type featureTestAPI struct { + dockerclient.APIClient + addr string +} + +func (a featureTestAPI) DialHijack(ctx context.Context, _, _ string, _ map[string][]string) (net.Conn, error) { + var dialer net.Dialer + return dialer.DialContext(ctx, "tcp", a.addr) +} + +type featureTestControl struct { + control.UnimplementedControlServer + calls atomic.Int32 + supported bool +} + +func (c *featureTestControl) ListWorkers(context.Context, *control.ListWorkersRequest) (*control.ListWorkersResponse, error) { + if c.calls.Add(1) == 1 { + return nil, status.Error(codes.Unavailable, "worker is starting") + } + w := &types.WorkerRecord{} + if c.supported { + w.Labels = map[string]string{"org.mobyproject.buildkit.worker.snapshotter": "overlayfs"} + } + return &control.ListWorkersResponse{Record: []*types.WorkerRecord{w}}, nil +} + +func TestFeaturesRetry(t *testing.T) { + for _, supported := range []bool{false, true} { + t.Run(map[bool]string{false: "unsupported", true: "supported"}[supported], func(t *testing.T) { + ctx, cancel := context.WithTimeoutCause(t.Context(), 10*time.Second, context.DeadlineExceeded) + defer cancel() + var lc net.ListenConfig + listener, err := lc.Listen(ctx, "tcp", "127.0.0.1:0") + require.NoError(t, err) + defer listener.Close() + server := grpc.NewServer() + defer server.Stop() + ctl := &featureTestControl{supported: supported} + control.RegisterControlServer(server, ctl) + go server.Serve(listener) + d := &Driver{InitConfig: driver.InitConfig{DockerAPI: featureTestAPI{addr: listener.Addr().String()}}} + features, err := d.Features(ctx) + require.ErrorContains(t, err, "listing workers") + require.Equal(t, codes.Unavailable, status.Code(err)) + require.Nil(t, features) + for range 2 { + features, err = d.Features(ctx) + require.NoError(t, err) + require.True(t, features[driver.DefaultLoad]) + for _, f := range []driver.Feature{driver.OCIExporter, driver.DockerExporter, driver.CacheExport, driver.MultiPlatform, driver.DirectPush, driver.PreferImageDigest} { + require.Equal(t, supported, features[f], f) + } + } + require.EqualValues(t, 2, ctl.calls.Load()) + }) + } +} diff --git a/driver/driver.go b/driver/driver.go index 0fb1264464ca..7996cb0635f4 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -70,7 +70,7 @@ type Driver interface { Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error Dial(ctx context.Context) (net.Conn, error) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) - Features(ctx context.Context) map[Feature]bool + Features(ctx context.Context) (map[Feature]bool, error) HostGatewayIP(ctx context.Context) (net.IP, error) IsMobyDriver() bool Config() InitConfig diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go index 4a59a5d372a0..e0206785e399 100644 --- a/driver/kubernetes/driver.go +++ b/driver/kubernetes/driver.go @@ -416,7 +416,7 @@ func (d *Driver) Factory() driver.Factory { return d.factory } -func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { +func (d *Driver) Features(_ context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: d.DockerAPI != nil, @@ -424,7 +424,7 @@ func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, // Untested (needs multiple Driver instances) driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) HostGatewayIP(_ context.Context) (net.IP, error) { diff --git a/driver/remote/driver.go b/driver/remote/driver.go index 62ed7646f6cb..8dda8a13c7cf 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -187,7 +187,7 @@ func loadTLS(opts *tlsOpts) (*tls.Config, error) { return cfg, nil } -func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { +func (d *Driver) Features(ctx context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: true, @@ -195,7 +195,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) HostGatewayIP(ctx context.Context) (net.IP, error) {