Skip to content

avoid policy dependency from builder loading #3998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
crazy-max wants to merge 1 commit into
base: master
Choose a base branch
from
Open

avoid policy dependency from builder loading #3998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion builder/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ type CreateOpts struct {
Endpoint string
Append bool
Timeout time.Duration
ImageVerifier driver.ImageVerifier
}

func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts CreateOpts) (*Builder, error) {
Expand Down Expand Up @@ -532,7 +533,7 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre
}
defer func() { cancel(errors.WithStack(context.Canceled)) }()

nodes, err := b.LoadNodes(timeoutCtx, WithData())
nodes, err := b.LoadNodes(timeoutCtx, WithData(), WithImageVerifier(opts.ImageVerifier))
if err != nil {
return nil, err
}
Expand Down
34 changes: 12 additions & 22 deletions builder/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,15 @@ import (

"github.com/containerd/platforms"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/store"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/dockerutil"
"github.com/docker/buildx/util/imagetools"
"github.com/docker/buildx/util/platformutil"
"github.com/moby/buildkit/client"
"github.com/moby/buildkit/util/grpcerrors"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc/codes"
)
Expand Down Expand Up @@ -51,10 +47,11 @@ func (b *Builder) Nodes() []Node {
type LoadNodesOption func(*loadNodesOptions)

type loadNodesOptions struct {
data bool
skipImageOpt bool
dialMeta map[string][]string
clientOpt []client.ClientOpt
data bool
skipImageOpt bool
dialMeta map[string][]string
clientOpt []client.ClientOpt
imageVerifier driver.ImageVerifier
}

func WithData() LoadNodesOption {
Expand All @@ -81,6 +78,12 @@ func WithClientOpt(clientOpt ...client.ClientOpt) LoadNodesOption {
}
}

func WithImageVerifier(imageVerifier driver.ImageVerifier) LoadNodesOption {
return func(o *loadNodesOptions) {
o.imageVerifier = imageVerifier
}
}

// LoadNodes loads and returns nodes for this builder.
// TODO: this should be a method on a Node object and lazy load data for each driver.
func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []Node, err error) {
Expand Down Expand Up @@ -113,19 +116,6 @@ func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []N
}
}

var imageVerifier driver.ImageVerifier
if policy.DefaultPolicyEnabled() {
pol := policy.DefaultPolicy(policy.Opt{
Log: func(_ logrus.Level, msg string) {
logrus.Debug(msg)
},
VerifierProvider: policy.SignatureVerifier(confutil.NewConfig(b.opts.dockerCli)),
})
imageVerifier = func(ctx context.Context, ref string, platform *ocispecs.Platform, resolver policy.SourceMetadataResolver) (digest.Digest, error) {
return pol.CheckSource(ctx, ref, platform, resolver)
}
}

for i, n := range b.NodeGroup.Nodes {
func(i int, n store.Node) {
eg.Go(func() error {
Expand Down Expand Up @@ -154,7 +144,7 @@ func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []N
Files: n.Files,
DriverOpts: n.DriverOpts,
Auth: imageopt.Auth,
ImageVerifier: imageVerifier,
ImageVerifier: lno.imageVerifier,
Platforms: n.Platforms,
ContextPathHash: b.opts.contextPathHash,
DialMeta: lno.dialMeta,
Expand Down
4 changes: 3 additions & 1 deletion commands/bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/localstate"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/buildflags"
"github.com/docker/buildx/util/cobrautil"
"github.com/docker/buildx/util/cobrautil/completion"
Expand Down Expand Up @@ -168,7 +169,8 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba
if err = updateLastActivity(dockerCli, b.NodeGroup); err != nil {
return errors.Wrapf(err, "failed to update builder last activity time")
}
nodes, err = b.LoadNodes(ctx)
imageVerifier := policy.DefaultImageVerifier(confutil.NewConfig(dockerCli))
nodes, err = b.LoadNodes(ctx, builder.WithImageVerifier(imageVerifier))
if err != nil {
return err
}
Expand Down
7 changes: 5 additions & 2 deletions commands/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/containerd/containerd/v2/pkg/epoch"
"github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/store"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/buildflags"
Expand Down Expand Up @@ -356,7 +357,8 @@ func runBuild(ctx context.Context, dockerCli command.Cli, debugOpts debuggerOpti
if err != nil {
return err
}
_, err = b.LoadNodes(ctx)
imageVerifier := policy.DefaultImageVerifier(confutil.NewConfig(dockerCli))
_, err = b.LoadNodes(ctx, builder.WithImageVerifier(imageVerifier))
if err != nil {
return err
}
Expand Down Expand Up @@ -1193,7 +1195,8 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *BuildOptions, inSt
if err = updateLastActivity(dockerCli, b.NodeGroup); err != nil {
return nil, nil, errors.Wrapf(err, "failed to update builder last activity time")
}
nodes, err := b.LoadNodes(ctx)
imageVerifier := policy.DefaultImageVerifier(confutil.NewConfig(dockerCli))
nodes, err := b.LoadNodes(ctx, builder.WithImageVerifier(imageVerifier))
if err != nil {
return nil, nil, err
}
Expand Down
3 changes: 3 additions & 0 deletions commands/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (

"github.com/docker/buildx/builder"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/cobrautil"
"github.com/docker/buildx/util/cobrautil/completion"
"github.com/docker/buildx/util/confutil"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -64,6 +66,7 @@ func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, arg
Endpoint: ep,
Append: in.actionAppend,
Timeout: in.timeout,
ImageVerifier: policy.DefaultImageVerifier(confutil.NewConfig(dockerCli)),
})
if err != nil {
return err
Expand Down
5 changes: 4 additions & 1 deletion commands/dial_stdio.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/containerd/platforms"
"github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/progress"
"github.com/docker/cli/cli/command"
"github.com/moby/buildkit/util/appcontext"
Expand Down Expand Up @@ -39,7 +41,8 @@ func runDialStdio(dockerCli command.Cli, opts stdioOptions) error {
if err = updateLastActivity(dockerCli, b.NodeGroup); err != nil {
return errors.Wrapf(err, "failed to update builder last activity time")
}
nodes, err := b.LoadNodes(ctx)
imageVerifier := policy.DefaultImageVerifier(confutil.NewConfig(dockerCli))
nodes, err := b.LoadNodes(ctx, builder.WithImageVerifier(imageVerifier))
if err != nil {
return err
}
Expand Down
7 changes: 5 additions & 2 deletions commands/history/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"time"

"github.com/docker/buildx/builder"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/confutil"
"github.com/docker/cli/cli/command"
"github.com/docker/go-units"
controlapi "github.com/moby/buildkit/api/services/control"
Expand Down Expand Up @@ -356,14 +358,15 @@ func loadNodes(ctx context.Context, dockerCli command.Cli, builderName string) (
if err != nil {
return nil, err
}
nodes, err := b.LoadNodes(ctx, builder.WithData())
imageVerifier := policy.DefaultImageVerifier(confutil.NewConfig(dockerCli))
nodes, err := b.LoadNodes(ctx, builder.WithData(), builder.WithImageVerifier(imageVerifier))
if err != nil {
return nil, err
}
if ok, err := b.Boot(ctx); err != nil {
return nil, err
} else if ok {
nodes, err = b.LoadNodes(ctx, builder.WithData())
nodes, err = b.LoadNodes(ctx, builder.WithData(), builder.WithImageVerifier(imageVerifier))
if err != nil {
return nil, err
}
Expand Down
7 changes: 5 additions & 2 deletions commands/inspect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (

"github.com/docker/buildx/builder"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/cobrautil/completion"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/platformutil"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
Expand Down Expand Up @@ -42,15 +44,16 @@ func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions) e
}
defer func() { cancel(errors.WithStack(context.Canceled)) }()

nodes, err := b.LoadNodes(timeoutCtx, builder.WithData())
imageVerifier := policy.DefaultImageVerifier(confutil.NewConfig(dockerCli))
nodes, err := b.LoadNodes(timeoutCtx, builder.WithData(), builder.WithImageVerifier(imageVerifier))
if in.bootstrap {
var ok bool
ok, err = b.Boot(ctx)
if err != nil {
return err
}
if ok {
nodes, err = b.LoadNodes(timeoutCtx, builder.WithData())
nodes, err = b.LoadNodes(timeoutCtx, builder.WithData(), builder.WithImageVerifier(imageVerifier))
}
}

Expand Down
4 changes: 2 additions & 2 deletions driver/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (

"github.com/distribution/reference"
"github.com/docker/buildx/driver/bkimage"
"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/sourcemeta"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
Expand All @@ -20,7 +20,7 @@ import (
// moby/buildkit repository (unmanaged images pass through the default policy
// unchanged). A tagged canonical reference is still verified because its tag
// carries the release identity checked by the policy.
func VerifyImageRef(ctx context.Context, l progress.SubLogger, ref string, platform *ocispecs.Platform, resolver policy.SourceMetadataResolver, verify ImageVerifier) (string, bool, error) {
func VerifyImageRef(ctx context.Context, l progress.SubLogger, ref string, platform *ocispecs.Platform, resolver *sourcemeta.Resolver, verify ImageVerifier) (string, bool, error) {
if verify == nil {
return ref, false, nil
}
Expand Down
6 changes: 3 additions & 3 deletions driver/image_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"testing"

"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/sourcemeta"
"github.com/moby/buildkit/client"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
Expand All @@ -22,7 +22,7 @@ func TestVerifyImageRefTaggedCanonical(t *testing.T) {
ref := "moby/buildkit:v0.31.2@" + dgst.String()
var verifiedRef string

pinned, applied, err := VerifyImageRef(context.Background(), nopSubLogger{}, ref, nil, nil, func(_ context.Context, ref string, _ *ocispecs.Platform, _ policy.SourceMetadataResolver) (digest.Digest, error) {
pinned, applied, err := VerifyImageRef(context.Background(), nopSubLogger{}, ref, nil, nil, func(_ context.Context, ref string, _ *ocispecs.Platform, _ *sourcemeta.Resolver) (digest.Digest, error) {
verifiedRef = ref
return dgst, nil
})
Expand All @@ -36,7 +36,7 @@ func TestVerifyImageRefDigestOnly(t *testing.T) {
dgst := digest.FromString("buildkit")
called := false

pinned, applied, err := VerifyImageRef(context.Background(), nopSubLogger{}, "moby/buildkit@"+dgst.String(), nil, nil, func(_ context.Context, _ string, _ *ocispecs.Platform, _ policy.SourceMetadataResolver) (digest.Digest, error) {
pinned, applied, err := VerifyImageRef(context.Background(), nopSubLogger{}, "moby/buildkit@"+dgst.String(), nil, nil, func(_ context.Context, _ string, _ *ocispecs.Platform, _ *sourcemeta.Resolver) (digest.Digest, error) {

@thaJeztah thaJeztah Aug 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're breaking the api / signature, perhaps as a follow-up also look if we need all these arguments to be positional arguments or if it could be an option-struct or variadic functional args.

called = true
return dgst, nil
})
Expand Down
4 changes: 2 additions & 2 deletions driver/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"sort"
"sync"

"github.com/docker/buildx/policy"
"github.com/docker/buildx/util/sourcemeta"
"github.com/docker/cli/cli/context/store"
"github.com/moby/buildkit/client"
"github.com/moby/buildkit/session/auth/authprovider"
Expand Down Expand Up @@ -36,7 +36,7 @@ type BuildkitConfig struct {
// Source metadata is resolved through the given resolver, which drivers back
// with a BuildKit instance they have access to so that registry access
// happens where the image is pulled from.
type ImageVerifier func(ctx context.Context, ref string, platform *ocispecs.Platform, resolver policy.SourceMetadataResolver) (digest.Digest, error)
type ImageVerifier func(ctx context.Context, ref string, platform *ocispecs.Platform, resolver *sourcemeta.Resolver) (digest.Digest, error)

type InitConfig struct {
Name string
Expand Down
24 changes: 24 additions & 0 deletions policy/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package policy

import (
"context"
_ "embed"
"os"
"strconv"
"sync"

"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/sourcemeta"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus"
)

// DefaultPolicyFilename is the synthetic filename used for the embedded
Expand Down Expand Up @@ -43,3 +50,20 @@ var DefaultPolicyEnabled = sync.OnceValue(func() bool {
}
return false
})

// DefaultImageVerifier returns an image verifier backed by the builtin default
// policy, or nil when the default policy is disabled.
func DefaultImageVerifier(cfg *confutil.Config) func(context.Context, string, *ocispecs.Platform, *sourcemeta.Resolver) (digest.Digest, error) {
if !DefaultPolicyEnabled() {
return nil
}
pol := DefaultPolicy(Opt{
Log: func(_ logrus.Level, msg string) {
logrus.Debug(msg)
},
VerifierProvider: SignatureVerifier(cfg),
})
return func(ctx context.Context, ref string, platform *ocispecs.Platform, resolver *sourcemeta.Resolver) (digest.Digest, error) {
return pol.CheckSource(ctx, ref, platform, resolver)
}
}
Loading