Merge pull request #47000 from thaJeztah/userland_proxy_validation

portmapper: move userland-proxy lookup to daemon config

daemon/config/config_linux.go

@@ -1,12 +1,14 @@
 package config // import "github.com/docker/docker/daemon/config"
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"os/exec"
 	"path/filepath"
 
 	"github.com/containerd/cgroups/v3"
+	"github.com/containerd/log"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/system"
 	"github.com/docker/docker/libnetwork/drivers/bridge"
@@ -33,6 +35,10 @@ const (
 
 	// minAPIVersion represents Minimum REST API version supported
 	minAPIVersion = "1.12"
+
+	// userlandProxyBinary is the name of the userland-proxy binary.
+	// In rootless-mode, [rootless.RootlessKitDockerProxyBinary] is used instead.
+	userlandProxyBinary = "docker-proxy"
 )
 
 // BridgeConfig stores all the parameters for both the bridge driver and the default bridge network.
@@ -175,6 +181,21 @@ func (conf *Config) ValidatePlatformConfig() error {
 	if conf.OOMScoreAdjust != 0 {
 		return errors.New(`DEPRECATED: The "oom-score-adjust" config parameter and the dockerd "--oom-score-adjust" options have been removed.`)
 	}
+
+	if conf.EnableUserlandProxy {
+		if conf.UserlandProxyPath == "" {
+			return errors.New("invalid userland-proxy-path: userland-proxy is enabled, but userland-proxy-path is not set")
+		}
+		if !filepath.IsAbs(conf.UserlandProxyPath) {
+			return errors.New("invalid userland-proxy-path: must be an absolute path: " + conf.UserlandProxyPath)
+		}
+		// Using exec.LookPath here, because it also produces an error if the
+		// given path is not a valid executable or a directory.
+		if _, err := exec.LookPath(conf.UserlandProxyPath); err != nil {
+			return errors.Wrap(err, "invalid userland-proxy-path")
+		}
+	}
+
 	if err := verifyDefaultIpcMode(conf.IpcMode); err != nil {
 		return err
 	}
@@ -227,6 +248,17 @@ func setPlatformDefaults(cfg *Config) error {
 		cfg.ExecRoot = filepath.Join(runtimeDir, "docker")
 		cfg.Pidfile = filepath.Join(runtimeDir, "docker.pid")
 	} else {
+		var err error
+		cfg.BridgeConfig.UserlandProxyPath, err = exec.LookPath(userlandProxyBinary)
+		if err != nil {
+			// Log a warning, but don't error here. This allows running a daemon
+			// with userland-proxy disabled (which does not require the binary
+			// to be present).
+			//
+			// An error is still produced by [Config.ValidatePlatformConfig] if
+			// userland-proxy is enabled in the configuration.
+			log.G(context.TODO()).WithError(err).Warn("failed to lookup default userland-proxy binary")
+		}
 		cfg.Root = "/var/lib/docker"
 		cfg.ExecRoot = "/var/run/docker"
 		cfg.Pidfile = "/var/run/docker.pid"

integration-cli/docker_cli_daemon_test.go

@@ -2469,6 +2469,7 @@ func (s *DockerDaemonSuite) TestDaemonRestartSaveContainerExitCode(c *testing.T)
 
 func (s *DockerDaemonSuite) TestDaemonWithUserlandProxyPath(c *testing.T) {
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)
+	ctx := context.TODO()
 
 	dockerProxyPath, err := exec.LookPath("docker-proxy")
 	assert.NilError(c, err)
@@ -2490,11 +2491,20 @@ func (s *DockerDaemonSuite) TestDaemonWithUserlandProxyPath(c *testing.T) {
 	assert.NilError(c, err, out)
 
 	// not exist
-	s.d.Restart(c, "--userland-proxy-path", "/does/not/exist")
-	out, err = s.d.Cmd("run", "-p", "5000:5000", "busybox:latest", "true")
-	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "driver failed programming external connectivity on endpoint"))
-	assert.Assert(c, strings.Contains(out, "/does/not/exist: no such file or directory"))
+	s.d.Stop(c)
+	err = s.d.StartWithError("--userland-proxy-path", "/does/not/exist")
+	assert.ErrorContains(c, err, "", "daemon should fail to start")
+	expected := "invalid userland-proxy-path"
+	ok, _ := s.d.ScanLogsT(ctx, c, testdaemon.ScanLogsMatchString(expected))
+	assert.Assert(c, ok, "logs did not contain: %s", expected)
+
+	// not an absolute path
+	s.d.Stop(c)
+	err = s.d.StartWithError("--userland-proxy-path", "docker-proxy")
+	assert.ErrorContains(c, err, "", "daemon should fail to start")
+	expected = "invalid userland-proxy-path: must be an absolute path: docker-proxy"
+	ok, _ = s.d.ScanLogsT(ctx, c, testdaemon.ScanLogsMatchString(expected))
+	assert.Assert(c, ok, "logs did not contain: %s", expected)
 }
 
 // Test case for #22471

libnetwork/drivers/bridge/bridge_linux_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"os/exec"
 	"regexp"
 	"strconv"
 	"testing"
@@ -644,9 +645,18 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
 	d := newDriver()
 	d.portAllocator = portallocator.NewInstance()
 
+	var proxyBinary string
+	var err error
+	if ulPxyEnabled {
+		proxyBinary, err = exec.LookPath("docker-proxy")
+		if err != nil {
+			t.Fatalf("failed to lookup userland-proxy binary: %v", err)
+		}
+	}
 	config := &configuration{
 		EnableIPTables:      true,
 		EnableUserlandProxy: ulPxyEnabled,
+		UserlandProxyPath:   proxyBinary,
 	}
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config
@@ -663,7 +673,7 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
 	genericOption[netlabel.GenericData] = netconfig
 
 	ipdList := getIPv4Data(t)
-	err := d.CreateNetwork("net1", genericOption, nil, ipdList, nil)
+	err = d.CreateNetwork("net1", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}

libnetwork/portmapper/proxy_linux.go

@@ -12,31 +12,22 @@ import (
 	"time"
 )
 
-const userlandProxyCommandName = "docker-proxy"
-
 func newProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.IP, containerPort int, proxyPath string) (userlandProxy, error) {
-	path := proxyPath
 	if proxyPath == "" {
-		cmd, err := exec.LookPath(userlandProxyCommandName)
-		if err != nil {
-			return nil, err
-		}
-		path = cmd
-	}
-
-	args := []string{
-		path,
-		"-proto", proto,
-		"-host-ip", hostIP.String(),
-		"-host-port", strconv.Itoa(hostPort),
-		"-container-ip", containerIP.String(),
-		"-container-port", strconv.Itoa(containerPort),
+		return nil, fmt.Errorf("no path provided for userland-proxy binary")
 	}
 
 	return &proxyCommand{
 		cmd: &exec.Cmd{
-			Path: path,
-			Args: args,
+			Path: proxyPath,
+			Args: []string{
+				proxyPath,
+				"-proto", proto,
+				"-host-ip", hostIP.String(),
+				"-host-port", strconv.Itoa(hostPort),
+				"-container-ip", containerIP.String(),
+				"-container-port", strconv.Itoa(containerPort),
+			},
 			SysProcAttr: &syscall.SysProcAttr{
 				Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the creating thread in the daemon process dies (https://go.dev/issue/27505)
 			},