From d7e4affe9812ad10f89ff6604f403f1225e9a15b Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 25 Apr 2022 11:28:40 +0100 Subject: [PATCH] Complete remote driver This patch completes the work started in creating a remote driver: - Renames the env driver to the remote driver (an alternative suggestion that should be more user-friendly) - Adds support for TLS to encrypt connections with buildkitd - Fixes outstanding review comments - Reworks the buildx create command endpoint construction to be clearer and include better support for this new driver. Signed-off-by: Justin Chadwell --- .github/workflows/e2e.yml | 17 +++++ cmd/buildx/main.go | 2 +- commands/create.go | 77 ++++++++++++------- commands/util.go | 29 +++++--- driver/docker-container/factory.go | 2 +- driver/docker/factory.go | 2 +- driver/env/factory.go | 52 ------------- driver/kubernetes/factory.go | 2 +- driver/manager.go | 19 ++--- driver/{env => remote}/driver.go | 38 ++++++---- driver/remote/factory.go | 115 +++++++++++++++++++++++++++++ hack/test-driver | 17 +++-- 12 files changed, 246 insertions(+), 126 deletions(-) delete mode 100644 driver/env/factory.go rename driver/{env => remote}/driver.go (66%) create mode 100644 driver/remote/factory.go diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 8ac2755d..6c9fe33e 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -53,6 +53,7 @@ jobs: - docker - docker-container - kubernetes + - remote buildkit: - moby/buildkit:buildx-stable-1 - moby/buildkit:master @@ -68,6 +69,8 @@ jobs: include: - driver: kubernetes driver-opt: qemu.install=true + - driver: remote + endpoint: tcp://localhost:1234 exclude: - driver: docker multi-node: mnode-true @@ -75,6 +78,10 @@ jobs: buildkit-cfg: bkcfg-true - driver: docker-container multi-node: mnode-true + - driver: remote + multi-node: mnode-true + - driver: remote + buildkit-cfg: bkcfg-true steps: - name: Checkout @@ -128,6 +135,15 @@ jobs: if: matrix.driver == 'kubernetes' run: | kubectl get nodes + - + name: Launch remote buildkitd + if: matrix.driver == 'remote' + run: | + docker run -d --privileged \ + --name=remote-buildkit \ + -p 1234:1234 \ + ${{ matrix.buildkit }} \ + --addr tcp://0.0.0.0:1234 - name: Test run: | @@ -136,4 +152,5 @@ jobs: BUILDKIT_IMAGE: ${{ matrix.buildkit }} DRIVER: ${{ matrix.driver }} DRIVER_OPT: ${{ matrix.driver-opt }} + ENDPOINT: ${{ matrix.endpoint }} PLATFORMS: ${{ matrix.platforms }} diff --git a/cmd/buildx/main.go b/cmd/buildx/main.go index 0a6295bc..c4e59be0 100644 --- a/cmd/buildx/main.go +++ b/cmd/buildx/main.go @@ -23,8 +23,8 @@ import ( _ "github.com/docker/buildx/driver/docker" _ "github.com/docker/buildx/driver/docker-container" - _ "github.com/docker/buildx/driver/env" _ "github.com/docker/buildx/driver/kubernetes" + _ "github.com/docker/buildx/driver/remote" ) func init() { diff --git a/commands/create.go b/commands/create.go index 1d4169fb..3fb72a49 100644 --- a/commands/create.go +++ b/commands/create.go @@ -60,16 +60,26 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { } } + buildkitHost := os.Getenv("BUILDKIT_HOST") + driverName := in.driver if driverName == "" { - f, err := driver.GetDefaultFactory(ctx, dockerCli.Client(), true) - if err != nil { - return err + if len(args) == 0 && buildkitHost != "" { + driverName = "remote" + } else { + var arg string + if len(args) > 0 { + arg = args[0] + } + f, err := driver.GetDefaultFactory(ctx, arg, dockerCli.Client(), true) + if err != nil { + return err + } + if f == nil { + return errors.Errorf("no valid drivers found") + } + driverName = f.Name() } - if f == nil { - return errors.Errorf("no valid drivers found") - } - driverName = f.Name() } if driver.GetFactory(driverName, true) == nil { @@ -129,44 +139,59 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { } var ep string + var setEp bool if in.actionLeave { if err := ng.Leave(in.nodeName); err != nil { return err } } else { - if len(args) > 0 { - ep, err = validateEndpoint(dockerCli, args[0]) - if err != nil { - return err - } - } else { - if dockerCli.CurrentContext() == "default" && dockerCli.DockerEndpoint().TLSData != nil { - return errors.Errorf("could not create a builder instance with TLS data loaded from environment. Please use `docker context create ` to create a context for current environment and then create a builder instance with `docker buildx create `") - } - - ep, err = storeutil.GetCurrentEndpoint(dockerCli) - if err != nil { - return err - } - } - - if in.driver == "kubernetes" { + switch { + case driverName == "kubernetes": // naming endpoint to make --append works ep = (&url.URL{ - Scheme: in.driver, + Scheme: driverName, Path: "/" + in.name, RawQuery: (&url.Values{ "deployment": {in.nodeName}, "kubeconfig": {os.Getenv("KUBECONFIG")}, }).Encode(), }).String() + setEp = false + case driverName == "remote": + if len(args) > 0 { + ep = args[0] + } else if buildkitHost != "" { + ep = buildkitHost + } else { + return errors.Errorf("no remote endpoint provided") + } + ep, err = validateBuildkitEndpoint(ep) + if err != nil { + return err + } + setEp = true + case len(args) > 0: + ep, err = validateEndpoint(dockerCli, args[0]) + if err != nil { + return err + } + setEp = true + default: + if dockerCli.CurrentContext() == "default" && dockerCli.DockerEndpoint().TLSData != nil { + return errors.Errorf("could not create a builder instance with TLS data loaded from environment. Please use `docker context create ` to create a context for current environment and then create a builder instance with `docker buildx create `") + } + ep, err = storeutil.GetCurrentEndpoint(dockerCli) + if err != nil { + return err + } + setEp = false } m, err := csvToMap(in.driverOpts) if err != nil { return err } - if err := ng.Update(in.nodeName, ep, in.platform, len(args) > 0, in.actionAppend, flags, in.configFile, m); err != nil { + if err := ng.Update(in.nodeName, ep, in.platform, setEp, in.actionAppend, flags, in.configFile, m); err != nil { return err } } diff --git a/commands/util.go b/commands/util.go index 1500ab73..5988ddde 100644 --- a/commands/util.go +++ b/commands/util.go @@ -6,8 +6,6 @@ import ( "os" "strings" - buildkitclient "github.com/moby/buildkit/client" - "github.com/docker/buildx/build" "github.com/docker/buildx/driver" ctxkube "github.com/docker/buildx/driver/kubernetes/context" @@ -43,6 +41,18 @@ func validateEndpoint(dockerCli command.Cli, ep string) (string, error) { return h, nil } +// validateBuildkitEndpoint validates that endpoint is a valid buildkit host +func validateBuildkitEndpoint(ep string) (string, error) { + endpoint, err := url.Parse(ep) + if err != nil { + return "", errors.Wrapf(err, "failed to parse endpoint %s", ep) + } + if endpoint.Scheme != "tcp" && endpoint.Scheme != "unix" { + return "", errors.Errorf("unrecognized url scheme %s", endpoint.Scheme) + } + return ep, nil +} + // driversForNodeGroup returns drivers for a nodegroup instance func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, contextPathHash string) ([]build.DriverInfo, error) { eg, _ := errgroup.WithContext(ctx) @@ -56,11 +66,12 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N return nil, errors.Errorf("failed to find driver %q", f) } } else { - dockerapi, err := clientForEndpoint(dockerCli, ng.Nodes[0].Endpoint) + ep := ng.Nodes[0].Endpoint + dockerapi, err := clientForEndpoint(dockerCli, ep) if err != nil { return nil, err } - f, err = driver.GetDefaultFactory(ctx, dockerapi, false) + f, err = driver.GetDefaultFactory(ctx, ep, dockerapi, false) if err != nil { return nil, err } @@ -83,12 +94,6 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N dis[i] = di }() - buildkitAPI, err := buildkitclient.New(ctx, n.Endpoint) - if err != nil { - di.Err = err - return nil - } - dockerapi, err := clientForEndpoint(dockerCli, n.Endpoint) if err != nil { di.Err = err @@ -127,7 +132,7 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N } } - d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, f, dockerapi, n.Endpoint, buildkitAPI, imageopt.Auth, kcc, n.Flags, n.Files, n.DriverOpts, n.Platforms, contextPathHash, ng.Driver) + d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, f, n.Endpoint, dockerapi, imageopt.Auth, kcc, n.Flags, n.Files, n.DriverOpts, n.Platforms, contextPathHash) if err != nil { di.Err = err return nil @@ -268,7 +273,7 @@ func getDefaultDrivers(ctx context.Context, dockerCli command.Cli, defaultOnly b return nil, err } - d, err := driver.GetDriver(ctx, "buildx_buildkit_default", nil, dockerCli.Client(), "", nil, imageopt.Auth, nil, nil, nil, nil, nil, contextPathHash, "") + d, err := driver.GetDriver(ctx, "buildx_buildkit_default", nil, "", dockerCli.Client(), imageopt.Auth, nil, nil, nil, nil, nil, contextPathHash) if err != nil { return nil, err } diff --git a/driver/docker-container/factory.go b/driver/docker-container/factory.go index 9b8212fd..94649b9e 100644 --- a/driver/docker-container/factory.go +++ b/driver/docker-container/factory.go @@ -29,7 +29,7 @@ func (*factory) Usage() string { return "docker-container" } -func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int { +func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int { if api == nil { return priorityUnsupported } diff --git a/driver/docker/factory.go b/driver/docker/factory.go index c960830e..8f2e37bc 100644 --- a/driver/docker/factory.go +++ b/driver/docker/factory.go @@ -26,7 +26,7 @@ func (*factory) Usage() string { return "docker" } -func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int { +func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int { if api == nil { return priorityUnsupported } diff --git a/driver/env/factory.go b/driver/env/factory.go deleted file mode 100644 index 309686dc..00000000 --- a/driver/env/factory.go +++ /dev/null @@ -1,52 +0,0 @@ -package env - -import ( - "context" - - "github.com/docker/buildx/driver" - dockerclient "github.com/docker/docker/client" - "github.com/pkg/errors" -) - -const prioritySupported = 50 -const priorityUnsupported = 90 - -func init() { - driver.Register(&factory{}) -} - -type factory struct { -} - -func (*factory) Name() string { - return "env" -} - -func (*factory) Usage() string { - return "env" -} - -func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int { - if api == nil { - return priorityUnsupported - } - - return prioritySupported -} - -func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver, error) { - if len(cfg.Files) > 0 { - return nil, errors.Errorf("setting config file is not supported for remote driver") - } - - return &Driver{ - factory: f, - InitConfig: cfg, - BuldkitdAddr: cfg.BuldkitdAddr, - BuildkitAPI: cfg.BuildkitAPI, - }, nil -} - -func (f *factory) AllowsInstances() bool { - return true -} diff --git a/driver/kubernetes/factory.go b/driver/kubernetes/factory.go index 04d8e105..6b831321 100644 --- a/driver/kubernetes/factory.go +++ b/driver/kubernetes/factory.go @@ -34,7 +34,7 @@ func (*factory) Usage() string { return DriverName } -func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int { +func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int { if api == nil { return priorityUnsupported } diff --git a/driver/manager.go b/driver/manager.go index 83854ca7..704b1228 100644 --- a/driver/manager.go +++ b/driver/manager.go @@ -11,7 +11,6 @@ import ( dockerclient "github.com/docker/docker/client" "github.com/moby/buildkit/client" - buildkitclient "github.com/moby/buildkit/client" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) @@ -19,7 +18,7 @@ import ( type Factory interface { Name() string Usage() string - Priority(context.Context, dockerclient.APIClient) int + Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int New(ctx context.Context, cfg InitConfig) (Driver, error) AllowsInstances() bool } @@ -51,13 +50,11 @@ func (k KubeClientConfigInCluster) Namespace() (string, bool, error) { type InitConfig struct { // This object needs updates to be generic for different drivers Name string + EndpointAddr string DockerAPI dockerclient.APIClient KubeClientConfig KubeClientConfig - BuldkitdAddr string - BuildkitAPI *buildkitclient.Client BuildkitFlags []string Files map[string][]byte - Driver string DriverOpts map[string]string Auth Auth Platforms []specs.Platform @@ -74,7 +71,7 @@ func Register(f Factory) { drivers[f.Name()] = f } -func GetDefaultFactory(ctx context.Context, c dockerclient.APIClient, instanceRequired bool) (Factory, error) { +func GetDefaultFactory(ctx context.Context, ep string, c dockerclient.APIClient, instanceRequired bool) (Factory, error) { if len(drivers) == 0 { return nil, errors.Errorf("no drivers available") } @@ -87,7 +84,7 @@ func GetDefaultFactory(ctx context.Context, c dockerclient.APIClient, instanceRe if instanceRequired && !f.AllowsInstances() { continue } - dd = append(dd, p{f: f, priority: f.Priority(ctx, c)}) + dd = append(dd, p{f: f, priority: f.Priority(ctx, ep, c)}) } sort.Slice(dd, func(i, j int) bool { return dd[i].priority < dd[j].priority @@ -107,16 +104,14 @@ func GetFactory(name string, instanceRequired bool) Factory { return nil } -func GetDriver(ctx context.Context, name string, f Factory, api dockerclient.APIClient, buldkitdAddr string, buildkitAPI *buildkitclient.Client, auth Auth, kcc KubeClientConfig, flags []string, files map[string][]byte, do map[string]string, platforms []specs.Platform, contextPathHash string, driver string) (Driver, error) { +func GetDriver(ctx context.Context, name string, f Factory, endpointAddr string, api dockerclient.APIClient, auth Auth, kcc KubeClientConfig, flags []string, files map[string][]byte, do map[string]string, platforms []specs.Platform, contextPathHash string) (Driver, error) { ic := InitConfig{ + EndpointAddr: endpointAddr, DockerAPI: api, KubeClientConfig: kcc, Name: name, - BuldkitdAddr: buldkitdAddr, - BuildkitAPI: buildkitAPI, BuildkitFlags: flags, DriverOpts: do, - Driver: driver, Auth: auth, Platforms: platforms, ContextPathHash: contextPathHash, @@ -124,7 +119,7 @@ func GetDriver(ctx context.Context, name string, f Factory, api dockerclient.API } if f == nil { var err error - f, err = GetDefaultFactory(ctx, api, false) + f, err = GetDefaultFactory(ctx, endpointAddr, api, false) if err != nil { return nil, err } diff --git a/driver/env/driver.go b/driver/remote/driver.go similarity index 66% rename from driver/env/driver.go rename to driver/remote/driver.go index acc1216d..ed52bea0 100644 --- a/driver/env/driver.go +++ b/driver/remote/driver.go @@ -1,21 +1,25 @@ -package env +package remote import ( "context" - "fmt" "github.com/docker/buildx/driver" "github.com/docker/buildx/util/progress" "github.com/moby/buildkit/client" - buildkitclient "github.com/moby/buildkit/client" "github.com/pkg/errors" ) type Driver struct { factory driver.Factory driver.InitConfig - BuldkitdAddr string - BuildkitAPI *buildkitclient.Client + *tlsOpts +} + +type tlsOpts struct { + serverName string + caCert string + cert string + key string } func (d *Driver) Bootstrap(ctx context.Context, l progress.Logger) error { @@ -23,11 +27,7 @@ func (d *Driver) Bootstrap(ctx context.Context, l progress.Logger) error { } func (d *Driver) Info(ctx context.Context) (*driver.Info, error) { - if d.BuldkitdAddr == "" && d.Driver == "env" { - return nil, errors.Errorf("buldkitd addr must not be empty") - } - - c, err := client.New(ctx, d.BuldkitdAddr) + c, err := d.Client(ctx) if err != nil { return nil, errors.Wrapf(driver.ErrNotConnecting, err.Error()) } @@ -42,19 +42,29 @@ func (d *Driver) Info(ctx context.Context) (*driver.Info, error) { } func (d *Driver) Stop(ctx context.Context, force bool) error { - return fmt.Errorf("stop command is not implemented for this driver") + return nil } func (d *Driver) Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error { - return fmt.Errorf("rm command is not implemented for this driver") + return nil } func (d *Driver) Client(ctx context.Context) (*client.Client, error) { - return client.New(ctx, d.BuldkitdAddr, client.WithSessionDialer(d.BuildkitAPI.Dialer())) + opts := []client.ClientOpt{} + if d.tlsOpts != nil { + opts = append(opts, client.WithCredentials(d.tlsOpts.serverName, d.tlsOpts.caCert, d.tlsOpts.cert, d.tlsOpts.key)) + } + + return client.New(ctx, d.InitConfig.EndpointAddr, opts...) } func (d *Driver) Features() map[driver.Feature]bool { - return map[driver.Feature]bool{} + return map[driver.Feature]bool{ + driver.OCIExporter: true, + driver.DockerExporter: false, + driver.CacheExport: true, + driver.MultiPlatform: true, + } } func (d *Driver) Factory() driver.Factory { diff --git a/driver/remote/factory.go b/driver/remote/factory.go new file mode 100644 index 00000000..cf256618 --- /dev/null +++ b/driver/remote/factory.go @@ -0,0 +1,115 @@ +package remote + +import ( + "context" + "net/url" + "path/filepath" + "regexp" + "strings" + + "github.com/docker/buildx/driver" + dockerclient "github.com/docker/docker/client" + "github.com/pkg/errors" +) + +const prioritySupported = 20 +const priorityUnsupported = 90 + +var schemeRegexp = regexp.MustCompile("^(tcp|unix)://") + +func init() { + driver.Register(&factory{}) +} + +type factory struct { +} + +func (*factory) Name() string { + return "remote" +} + +func (*factory) Usage() string { + return "remote" +} + +func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int { + if schemeRegexp.MatchString(endpoint) { + return prioritySupported + } + return priorityUnsupported +} + +func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver, error) { + if len(cfg.Files) > 0 { + return nil, errors.Errorf("setting config file is not supported for remote driver") + } + if len(cfg.BuildkitFlags) > 0 { + return nil, errors.Errorf("setting buildkit flags is not supported for remote driver") + } + + d := &Driver{ + factory: f, + InitConfig: cfg, + } + + tls := &tlsOpts{} + tlsEnabled := false + for k, v := range cfg.DriverOpts { + switch k { + case "servername": + tls.serverName = v + tlsEnabled = true + case "cacert": + if !filepath.IsAbs(v) { + return nil, errors.Errorf("non-absolute path '%s' provided for %s", v, k) + } + tls.caCert = v + tlsEnabled = true + case "cert": + if !filepath.IsAbs(v) { + return nil, errors.Errorf("non-absolute path '%s' provided for %s", v, k) + } + tls.cert = v + tlsEnabled = true + case "key": + if !filepath.IsAbs(v) { + return nil, errors.Errorf("non-absolute path '%s' provided for %s", v, k) + } + tls.key = v + tlsEnabled = true + default: + return nil, errors.Errorf("invalid driver option %s for remote driver", k) + } + } + + if tlsEnabled { + if tls.serverName == "" { + // guess servername as hostname of target address + uri, err := url.Parse(cfg.EndpointAddr) + if err != nil { + return nil, err + } + tls.serverName = uri.Hostname() + } + missing := []string{} + if tls.caCert == "" { + missing = append(missing, "cacert") + } + if tls.cert == "" { + missing = append(missing, "cert") + } + if tls.key == "" { + missing = append(missing, "key") + } + if len(missing) > 0 { + return nil, errors.Errorf("tls enabled, but missing keys %s", strings.Join(missing, ", ")) + } + d.tlsOpts = tls + } + + return d, nil +} + +func (f *factory) AllowsInstances() bool { + return true +} diff --git a/hack/test-driver b/hack/test-driver index 12818dc0..c0ce3f2e 100755 --- a/hack/test-driver +++ b/hack/test-driver @@ -7,6 +7,7 @@ set -eu -o pipefail : ${BUILDKIT_CFG=} : ${DRIVER=docker-container} : ${DRIVER_OPT=} +: ${ENDPOINT=} : ${MULTI_NODE=0} : ${PLATFORMS=linux/amd64,linux/arm64} @@ -34,9 +35,11 @@ else buildPlatformFlag=--platform="${PLATFORMS}" fi -driverOpt=image=${BUILDKIT_IMAGE} +if [ "$DRIVER" != "remote" ]; then + driverOpt=${driverOpt:+"${driverOpt},"}image=${BUILDKIT_IMAGE} +fi if [ -n "$DRIVER_OPT" ]; then - driverOpt=$driverOpt,$DRIVER_OPT + driverOpt=${driverOpt:+"${driverOpt},"}$DRIVER_OPT fi # create builder except for docker driver @@ -54,9 +57,10 @@ if [ "$DRIVER" != "docker" ]; then buildxCmd create ${createFlags} \ --name="${builderName}" \ --node="${builderName}-${platform/\//-}" \ + --platform="${platform}" \ --driver="${DRIVER}" \ - --driver-opt="${driverOpt}" \ - --platform="${platform}" + ${driverOpt:+"--driver-opt=${driverOpt}"} \ + ${ENDPOINT} firstNode=0 done else @@ -66,9 +70,10 @@ if [ "$DRIVER" != "docker" ]; then fi buildxCmd create ${createFlags} \ --name="${builderName}" \ + --platform="${PLATFORMS}" \ --driver="${DRIVER}" \ - --driver-opt="${driverOpt}" \ - --platform="${PLATFORMS}" + ${driverOpt:+"--driver-opt=${driverOpt}"} \ + ${ENDPOINT} fi fi