Change remote driver endpoint parsing

Remote drivers require an endpoint, so enforce this at the buildx creation level. Additionally, update the endpoint to be generic, so it can be easily shared between different drivers. Finally, ensure that an appropriate default driver is selected for the remote driver cases. Signed-off-by: Justin Chadwell <[email protected]>
docker · Apr 27, 2022 · 05f516d · 05f516d
1 parent 98cfecc
commit 05f516d
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 48 deletions.
diff --git 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 f == nil {
-			return errors.Errorf("no valid drivers found")
+		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()
 		}
-		driverName = f.Name()
 	}
 
 	if driver.GetFactory(driverName, true) == nil {
@@ -134,12 +144,35 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
 			return err
 		}
 	} else {
-		if len(args) > 0 {
+		switch {
+		case driverName == "kubernetes":
+			// naming endpoint to make --append works
+			ep = (&url.URL{
+				Scheme: driverName,
+				Path:   "/" + in.name,
+				RawQuery: (&url.Values{
+					"deployment": {in.nodeName},
+					"kubeconfig": {os.Getenv("KUBECONFIG")},
+				}).Encode(),
+			}).String()
+		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
+			}
+		case len(args) > 0:
 			ep, err = validateEndpoint(dockerCli, args[0])
 			if err != nil {
 				return err
 			}
-		} else {
+		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>`")
 			}
@@ -150,22 +183,11 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
 			}
 		}
 
-		if in.driver == "kubernetes" {
-			// naming endpoint to make --append works
-			ep = (&url.URL{
-				Scheme: in.driver,
-				Path:   "/" + in.name,
-				RawQuery: (&url.Values{
-					"deployment": {in.nodeName},
-					"kubeconfig": {os.Getenv("KUBECONFIG")},
-				}).Encode(),
-			}).String()
-		}
-
 		m, err := csvToMap(in.driverOpts)
 		if err != nil {
 			return err
 		}
+		// XXX: len(args) logic wrong
 		if err := ng.Update(in.nodeName, ep, in.platform, len(args) > 0, in.actionAppend, flags, in.configFile, m); err != nil {
 			return err
 		}

diff --git a/commands/util.go b/commands/util.go
@@ -41,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)
@@ -54,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
 		}
@@ -119,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, imageopt.Auth, kcc, n.Flags, n.Files, n.DriverOpts, n.Platforms, contextPathHash)
+				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
@@ -260,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(), "", 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
@@ -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
@@ -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/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
@@ -18,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
 }
@@ -50,9 +50,9 @@ 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
-	BuildkitAddr     string
 	BuildkitFlags    []string
 	Files            map[string][]byte
 	DriverOpts       map[string]string
@@ -71,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")
 	}
@@ -84,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
@@ -104,12 +104,12 @@ func GetFactory(name string, instanceRequired bool) Factory {
 	return nil
 }
 
-func GetDriver(ctx context.Context, name string, f Factory, api dockerclient.APIClient, buildkitAddr string, auth Auth, kcc KubeClientConfig, flags []string, files map[string][]byte, do map[string]string, platforms []specs.Platform, contextPathHash 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,
-		BuildkitAddr:     buildkitAddr,
 		BuildkitFlags:    flags,
 		DriverOpts:       do,
 		Auth:             auth,
@@ -119,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/remote/driver.go b/driver/remote/driver.go
@@ -13,7 +13,6 @@ type Driver struct {
 	factory driver.Factory
 	driver.InitConfig
 	*tlsOpts
-	buildkitAddr string
 }
 
 type tlsOpts struct {
@@ -28,10 +27,6 @@ func (d *Driver) Bootstrap(ctx context.Context, l progress.Logger) error {
 }
 
 func (d *Driver) Info(ctx context.Context) (*driver.Info, error) {
-	if d.buildkitAddr == "" {
-		return nil, errors.Errorf("buildkitd addr must not be empty")
-	}
-
 	c, err := d.Client(ctx)
 	if err != nil {
 		return nil, errors.Wrapf(driver.ErrNotConnecting, err.Error())
@@ -60,7 +55,7 @@ func (d *Driver) Client(ctx context.Context) (*client.Client, error) {
 		opts = append(opts, client.WithCredentials(d.tlsOpts.serverName, d.tlsOpts.caCert, d.tlsOpts.cert, d.tlsOpts.key))
 	}
 
-	return client.New(ctx, d.buildkitAddr, opts...)
+	return client.New(ctx, d.InitConfig.EndpointAddr, opts...)
 }
 
 func (d *Driver) Features() map[driver.Feature]bool {

diff --git a/driver/remote/factory.go b/driver/remote/factory.go
@@ -4,14 +4,18 @@ import (
 	"context"
 	"net/url"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/docker/buildx/driver"
 	dockerclient "github.com/docker/docker/client"
 	"github.com/pkg/errors"
 )
 
-const priority = 90
+const prioritySupported = 20
+const priorityUnsupported = 90
+
+var schemeRegexp = regexp.MustCompile("^(tcp|unix)://")
 
 func init() {
 	driver.Register(&factory{})
@@ -28,8 +32,11 @@ func (*factory) Usage() string {
 	return "remote"
 }
 
-func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int {
-	return priority
+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) {
@@ -41,9 +48,8 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 	}
 
 	d := &Driver{
-		factory:      f,
-		InitConfig:   cfg,
-		buildkitAddr: cfg.BuildkitAddr,
+		factory:    f,
+		InitConfig: cfg,
 	}
 
 	tls := &tlsOpts{}
@@ -79,7 +85,7 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 	if tlsEnabled {
 		if tls.serverName == "" {
 			// guess servername as hostname of target address
-			uri, err := url.Parse(d.buildkitAddr)
+			uri, err := url.Parse(cfg.EndpointAddr)
 			if err != nil {
 				return nil, err
 			}