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 <me@jedevc.com>
2022-04-25 11:28:40 +01:00 · 2022-04-25 11:28:40 +01:00 · d7e4affe98
parent 3dc83e5dd8
commit d7e4affe98
12 changed files with 246 additions and 126 deletions
--- 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 }}
--- 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() {
--- a/commands/create.go
+++ b/commands/create.go
@ -60,9 +60,18 @@ 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 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
 			}
@ -71,6 +80,7 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
 			}
 			driverName = f.Name()
 		}
+	}

 	if driver.GetFactory(driverName, true) == nil {
 		return errors.Errorf("failed to find driver %q", in.driver)
@ -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 <context-name>` to create a context for current environment and then create a builder instance with `docker buildx create <context-name>`")
-			}
-
-			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 <context-name>` to create a context for current environment and then create a builder instance with `docker buildx create <context-name>`")
+			}
+			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
 		}
 	}
--- 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
 	}
--- 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
 	}
--- 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
 	}
--- a/driver/env/factory.go
+++ b/driver/env/factory.go
@ -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
-}
--- 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
 	}
--- 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
 		}
--- a/driver/remote/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 {
--- a/driver/remote/factory.go
+++ 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
+}
--- 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