Merge pull request #42835 from thaJeztah/proxy_daemon_config_carry2

Add http(s) proxy properties to daemon configuration (carry 42647)

cmd/dockerd/config.go

@@ -101,6 +101,10 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 
 	flags.StringVar(&conf.DefaultRuntime, "default-runtime", config.StockRuntimeName, "Default OCI runtime for containers")
 
+	flags.StringVar(&conf.HTTPProxy, "http-proxy", "", "HTTP proxy URL to use for outgoing traffic")
+	flags.StringVar(&conf.HTTPSProxy, "https-proxy", "", "HTTPS proxy URL to use for outgoing traffic")
+	flags.StringVar(&conf.NoProxy, "no-proxy", "", "Comma-separated list of hosts or IP addresses for which the proxy is skipped")
+
 	return nil
 }
 

cmd/dockerd/daemon.go

@@ -87,6 +87,8 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 		return nil
 	}
 
+	configureProxyEnv(cli.Config)
+
 	warnOnDeprecatedConfigOptions(cli.Config)
 
 	if err := configureDaemonLogs(cli.Config); err != nil {
@@ -779,3 +781,29 @@ func configureDaemonLogs(conf *config.Config) error {
 	})
 	return nil
 }
+
+func configureProxyEnv(conf *config.Config) {
+	if p := conf.HTTPProxy; p != "" {
+		overrideProxyEnv("HTTP_PROXY", p)
+		overrideProxyEnv("http_proxy", p)
+	}
+	if p := conf.HTTPSProxy; p != "" {
+		overrideProxyEnv("HTTPS_PROXY", p)
+		overrideProxyEnv("https_proxy", p)
+	}
+	if p := conf.NoProxy; p != "" {
+		overrideProxyEnv("NO_PROXY", p)
+		overrideProxyEnv("no_proxy", p)
+	}
+}
+
+func overrideProxyEnv(name, val string) {
+	if oldVal := os.Getenv(name); oldVal != "" && oldVal != val {
+		logrus.WithFields(logrus.Fields{
+			"name":      name,
+			"old-value": config.MaskCredentials(oldVal),
+			"new-value": config.MaskCredentials(val),
+		}).Warn("overriding existing proxy variable with value from configuration")
+	}
+	_ = os.Setenv(name, val)
+}

daemon/config/config.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"net/url"
 	"os"
 	"reflect"
 	"strings"
@@ -165,6 +166,7 @@ type CommonConfig struct {
 	ExecRoot              string                    `json:"exec-root,omitempty"`
 	SocketGroup           string                    `json:"group,omitempty"`
 	CorsHeaders           string                    `json:"api-cors-header,omitempty"`
+	ProxyConfig
 
 	// TrustKeyPath is used to generate the daemon ID and for signing schema 1 manifests
 	// when pushing to a registry which does not support schema 2. This field is marked as
@@ -275,6 +277,13 @@ type CommonConfig struct {
 	DefaultRuntime string `json:"default-runtime,omitempty"`
 }
 
+// ProxyConfig holds the proxy-configuration for the daemon.
+type ProxyConfig struct {
+	HTTPProxy  string `json:"http-proxy,omitempty"`
+	HTTPSProxy string `json:"https-proxy,omitempty"`
+	NoProxy    string `json:"no-proxy,omitempty"`
+}
+
 // IsValueSet returns true if a configuration value
 // was explicitly set in the configuration file.
 func (conf *Config) IsValueSet(name string) bool {
@@ -525,6 +534,11 @@ func findConfigurationConflicts(config map[string]interface{}, flags *pflag.Flag
 
 	var conflicts []string
 	printConflict := func(name string, flagValue, fileValue interface{}) string {
+		switch name {
+		case "http-proxy", "https-proxy":
+			flagValue = MaskCredentials(flagValue.(string))
+			fileValue = MaskCredentials(fileValue.(string))
+		}
 		return fmt.Sprintf("%s: (from flag: %v, from file: %v)", name, flagValue, fileValue)
 	}
 
@@ -645,3 +659,13 @@ func (conf *Config) GetDefaultRuntimeName() string {
 
 	return rt
 }
+
+// MaskCredentials masks credentials that are in an URL.
+func MaskCredentials(rawURL string) string {
+	parsedURL, err := url.Parse(rawURL)
+	if err != nil || parsedURL.User == nil {
+		return rawURL
+	}
+	parsedURL.User = url.UserPassword("xxxxx", "xxxxx")
+	return parsedURL.String()
+}

daemon/config/config_test.go

@@ -578,3 +578,49 @@ func TestReloadWithDuplicateLabels(t *testing.T) {
 	err := Reload(configFile, flags, func(c *Config) {})
 	assert.Check(t, err)
 }
+
+func TestMaskURLCredentials(t *testing.T) {
+	tests := []struct {
+		rawURL    string
+		maskedURL string
+	}{
+		{
+			rawURL:    "",
+			maskedURL: "",
+		}, {
+			rawURL:    "invalidURL",
+			maskedURL: "invalidURL",
+		}, {
+			rawURL:    "http://proxy.example.com:80/",
+			maskedURL: "http://proxy.example.com:80/",
+		}, {
+			rawURL:    "http://USER:PASSWORD@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://PASSWORD:PASSWORD@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://USER:@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://:PASSWORD@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://USER@docker:password@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://USER%40docker:password@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://USER%40docker:pa%3Fsword@proxy.example.com:80/",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
+		}, {
+			rawURL:    "http://USER%40docker:pa%3Fsword@proxy.example.com:80/hello%20world",
+			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/hello%20world",
+		},
+	}
+	for _, test := range tests {
+		maskedURL := MaskCredentials(test.rawURL)
+		assert.Equal(t, maskedURL, test.maskedURL)
+	}
+}

daemon/info.go

@@ -2,7 +2,6 @@ package daemon // import "github.com/docker/docker/daemon"
 
 import (
 	"fmt"
-	"net/url"
 	"os"
 	"runtime"
 	"strings"
@@ -64,9 +63,9 @@ func (daemon *Daemon) SystemInfo() *types.Info {
 		Labels:             daemon.configStore.Labels,
 		ExperimentalBuild:  daemon.configStore.Experimental,
 		ServerVersion:      dockerversion.Version,
-		HTTPProxy:          maskCredentials(getEnvAny("HTTP_PROXY", "http_proxy")),
-		HTTPSProxy:         maskCredentials(getEnvAny("HTTPS_PROXY", "https_proxy")),
-		NoProxy:            getEnvAny("NO_PROXY", "no_proxy"),
+		HTTPProxy:          config.MaskCredentials(getConfigOrEnv(daemon.configStore.HTTPProxy, "HTTP_PROXY", "http_proxy")),
+		HTTPSProxy:         config.MaskCredentials(getConfigOrEnv(daemon.configStore.HTTPSProxy, "HTTPS_PROXY", "https_proxy")),
+		NoProxy:            getConfigOrEnv(daemon.configStore.NoProxy, "NO_PROXY", "no_proxy"),
 		LiveRestoreEnabled: daemon.configStore.LiveRestoreEnabled,
 		Isolation:          daemon.defaultIsolation,
 	}
@@ -289,16 +288,6 @@ func osVersion() (version string) {
 	return version
 }
 
-func maskCredentials(rawURL string) string {
-	parsedURL, err := url.Parse(rawURL)
-	if err != nil || parsedURL.User == nil {
-		return rawURL
-	}
-	parsedURL.User = url.UserPassword("xxxxx", "xxxxx")
-	maskedURL := parsedURL.String()
-	return maskedURL
-}
-
 func getEnvAny(names ...string) string {
 	for _, n := range names {
 		if val := os.Getenv(n); val != "" {
@@ -307,3 +296,10 @@ func getEnvAny(names ...string) string {
 	}
 	return ""
 }
+
+func getConfigOrEnv(config string, env ...string) string {
+	if config != "" {
+		return config
+	}
+	return getEnvAny(env...)
+}

daemon/info_test.go

@@ -1,53 +0,0 @@
-package daemon
-
-import (
-	"testing"
-
-	"gotest.tools/v3/assert"
-)
-
-func TestMaskURLCredentials(t *testing.T) {
-	tests := []struct {
-		rawURL    string
-		maskedURL string
-	}{
-		{
-			rawURL:    "",
-			maskedURL: "",
-		}, {
-			rawURL:    "invalidURL",
-			maskedURL: "invalidURL",
-		}, {
-			rawURL:    "http://proxy.example.com:80/",
-			maskedURL: "http://proxy.example.com:80/",
-		}, {
-			rawURL:    "http://USER:PASSWORD@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://PASSWORD:PASSWORD@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://USER:@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://:PASSWORD@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://USER@docker:password@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://USER%40docker:password@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://USER%40docker:pa%3Fsword@proxy.example.com:80/",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/",
-		}, {
-			rawURL:    "http://USER%40docker:pa%3Fsword@proxy.example.com:80/hello%20world",
-			maskedURL: "http://xxxxx:xxxxx@proxy.example.com:80/hello%20world",
-		},
-	}
-	for _, test := range tests {
-		maskedURL := maskCredentials(test.rawURL)
-		assert.Equal(t, maskedURL, test.maskedURL)
-	}
-}

daemon/reload.go

@@ -28,14 +28,26 @@ func (daemon *Daemon) Reload(conf *config.Config) (err error) {
 	attributes := map[string]string{}
 
 	defer func() {
-		jsonString, _ := json.Marshal(daemon.configStore)
+		if err == nil {
+			jsonString, _ := json.Marshal(&struct {
+				*config.Config
+				config.ProxyConfig
+			}{
+				Config: daemon.configStore,
+				ProxyConfig: config.ProxyConfig{
+					HTTPProxy:  config.MaskCredentials(daemon.configStore.HTTPProxy),
+					HTTPSProxy: config.MaskCredentials(daemon.configStore.HTTPSProxy),
+					NoProxy:    config.MaskCredentials(daemon.configStore.NoProxy),
+				},
+			})
+			logrus.Infof("Reloaded configuration: %s", jsonString)
+		}
 
 		// we're unlocking here, because
 		// LogDaemonEventWithAttributes() -> SystemInfo() -> GetAllRuntimes()
 		// holds that lock too.
 		daemon.configStore.Unlock()
 		if err == nil {
-			logrus.Infof("Reloaded configuration: %s", jsonString)
 			daemon.LogDaemonEventWithAttributes("reload", attributes)
 		}
 	}()

integration/daemon/daemon_test.go

@@ -1,21 +1,29 @@
 package daemon // import "github.com/docker/docker/integration/daemon"
 
 import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strings"
+	"syscall"
 	"testing"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/testutil/daemon"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
+	"gotest.tools/v3/env"
 	"gotest.tools/v3/skip"
 )
 
 func TestConfigDaemonLibtrustID(t *testing.T) {
-	skip.If(t, runtime.GOOS != "linux")
+	skip.If(t, runtime.GOOS == "windows")
 
 	d := daemon.New(t)
 	defer d.Stop(t)
@@ -34,7 +42,7 @@ func TestConfigDaemonLibtrustID(t *testing.T) {
 }
 
 func TestDaemonConfigValidation(t *testing.T) {
-	skip.If(t, runtime.GOOS != "linux")
+	skip.If(t, runtime.GOOS == "windows")
 
 	d := daemon.New(t)
 	dockerBinary, err := d.BinaryPath()
@@ -100,7 +108,7 @@ func TestDaemonConfigValidation(t *testing.T) {
 }
 
 func TestConfigDaemonSeccompProfiles(t *testing.T) {
-	skip.If(t, runtime.GOOS != "linux")
+	skip.If(t, runtime.GOOS == "windows")
 
 	d := daemon.New(t)
 	defer d.Stop(t)
@@ -146,3 +154,186 @@ func TestConfigDaemonSeccompProfiles(t *testing.T) {
 		})
 	}
 }
+
+func TestDaemonProxy(t *testing.T) {
+	skip.If(t, runtime.GOOS == "windows", "cannot start multiple daemons on windows")
+	skip.If(t, os.Getenv("DOCKER_ROOTLESS") != "", "cannot connect to localhost proxy in rootless environment")
+
+	var received string
+	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		received = r.Host
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte("OK"))
+	}))
+	defer proxyServer.Close()
+
+	const userPass = "myuser:mypassword@"
+
+	// Configure proxy through env-vars
+	t.Run("environment variables", func(t *testing.T) {
+		defer env.Patch(t, "HTTP_PROXY", proxyServer.URL)()
+		defer env.Patch(t, "HTTPS_PROXY", proxyServer.URL)()
+		defer env.Patch(t, "NO_PROXY", "example.com")()
+
+		d := daemon.New(t)
+		c := d.NewClientT(t)
+		defer func() { _ = c.Close() }()
+		ctx := context.Background()
+		d.Start(t)
+
+		_, err := c.ImagePull(ctx, "example.org:5000/some/image:latest", types.ImagePullOptions{})
+		assert.ErrorContains(t, err, "", "pulling should have failed")
+		assert.Equal(t, received, "example.org:5000")
+
+		// Test NoProxy: example.com should not hit the proxy, and "received" variable should not be changed.
+		_, err = c.ImagePull(ctx, "example.com/some/image:latest", types.ImagePullOptions{})
+		assert.ErrorContains(t, err, "", "pulling should have failed")
+		assert.Equal(t, received, "example.org:5000", "should not have used proxy")
+
+		info := d.Info(t)
+		assert.Equal(t, info.HTTPProxy, proxyServer.URL)
+		assert.Equal(t, info.HTTPSProxy, proxyServer.URL)
+		assert.Equal(t, info.NoProxy, "example.com")
+		d.Stop(t)
+	})
+
+	// Configure proxy through command-line flags
+	t.Run("command-line options", func(t *testing.T) {
+		defer env.Patch(t, "HTTP_PROXY", "http://"+userPass+"from-env-http.invalid")()
+		defer env.Patch(t, "http_proxy", "http://"+userPass+"from-env-http.invalid")()
+		defer env.Patch(t, "HTTPS_PROXY", "https://"+userPass+"myuser:mypassword@from-env-https.invalid")()
+		defer env.Patch(t, "https_proxy", "https://"+userPass+"myuser:mypassword@from-env-https.invalid")()
+		defer env.Patch(t, "NO_PROXY", "ignore.invalid")()
+		defer env.Patch(t, "no_proxy", "ignore.invalid")()
+
+		d := daemon.New(t)
+		d.Start(t, "--http-proxy", proxyServer.URL, "--https-proxy", proxyServer.URL, "--no-proxy", "example.com")
+
+		logs, err := d.ReadLogFile()
+		assert.NilError(t, err)
+		assert.Assert(t, is.Contains(string(logs), "overriding existing proxy variable with value from configuration"))
+		for _, v := range []string{"http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY", "no_proxy", "NO_PROXY"} {
+			assert.Assert(t, is.Contains(string(logs), "name="+v))
+			assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs))
+		}
+
+		c := d.NewClientT(t)
+		defer func() { _ = c.Close() }()
+		ctx := context.Background()
+
+		_, err = c.ImagePull(ctx, "example.org:5001/some/image:latest", types.ImagePullOptions{})
+		assert.ErrorContains(t, err, "", "pulling should have failed")
+		assert.Equal(t, received, "example.org:5001")
+
+		// Test NoProxy: example.com should not hit the proxy, and "received" variable should not be changed.
+		_, err = c.ImagePull(ctx, "example.com/some/image:latest", types.ImagePullOptions{})
+		assert.ErrorContains(t, err, "", "pulling should have failed")
+		assert.Equal(t, received, "example.org:5001", "should not have used proxy")
+
+		info := d.Info(t)
+		assert.Equal(t, info.HTTPProxy, proxyServer.URL)
+		assert.Equal(t, info.HTTPSProxy, proxyServer.URL)
+		assert.Equal(t, info.NoProxy, "example.com")
+
+		d.Stop(t)
+	})
+
+	// Configure proxy through configuration file
+	t.Run("configuration file", func(t *testing.T) {
+		defer env.Patch(t, "HTTP_PROXY", "http://"+userPass+"from-env-http.invalid")()
+		defer env.Patch(t, "http_proxy", "http://"+userPass+"from-env-http.invalid")()
+		defer env.Patch(t, "HTTPS_PROXY", "https://"+userPass+"myuser:mypassword@from-env-https.invalid")()
+		defer env.Patch(t, "https_proxy", "https://"+userPass+"myuser:mypassword@from-env-https.invalid")()
+		defer env.Patch(t, "NO_PROXY", "ignore.invalid")()
+		defer env.Patch(t, "no_proxy", "ignore.invalid")()
+
+		d := daemon.New(t)
+		c := d.NewClientT(t)
+		defer func() { _ = c.Close() }()
+		ctx := context.Background()
+
+		configFile := filepath.Join(d.RootDir(), "daemon.json")
+		configJSON := fmt.Sprintf(`{"http-proxy":%[1]q, "https-proxy": %[1]q, "no-proxy": "example.com"}`, proxyServer.URL)
+		assert.NilError(t, os.WriteFile(configFile, []byte(configJSON), 0644))
+
+		d.Start(t, "--config-file", configFile)
+
+		logs, err := d.ReadLogFile()
+		assert.NilError(t, err)
+		assert.Assert(t, is.Contains(string(logs), "overriding existing proxy variable with value from configuration"))
+		for _, v := range []string{"http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY", "no_proxy", "NO_PROXY"} {
+			assert.Assert(t, is.Contains(string(logs), "name="+v))
+			assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs))
+		}
+
+		_, err = c.ImagePull(ctx, "example.org:5002/some/image:latest", types.ImagePullOptions{})
+		assert.ErrorContains(t, err, "", "pulling should have failed")
+		assert.Equal(t, received, "example.org:5002")
+
+		// Test NoProxy: example.com should not hit the proxy, and "received" variable should not be changed.
+		_, err = c.ImagePull(ctx, "example.com/some/image:latest", types.ImagePullOptions{})
+		assert.ErrorContains(t, err, "", "pulling should have failed")
+		assert.Equal(t, received, "example.org:5002", "should not have used proxy")
+
+		info := d.Info(t)
+		assert.Equal(t, info.HTTPProxy, proxyServer.URL)
+		assert.Equal(t, info.HTTPSProxy, proxyServer.URL)
+		assert.Equal(t, info.NoProxy, "example.com")
+
+		d.Stop(t)
+	})
+
+	// Conflicting options (passed both through command-line options and config file)
+	t.Run("conflicting options", func(t *testing.T) {
+		const (
+			proxyRawURL = "https://" + userPass + "example.org"
+			proxyURL    = "https://xxxxx:xxxxx@example.org"
+		)
+
+		d := daemon.New(t)
+
+		configFile := filepath.Join(d.RootDir(), "daemon.json")
+		configJSON := fmt.Sprintf(`{"http-proxy":%[1]q, "https-proxy": %[1]q, "no-proxy": "example.com"}`, proxyRawURL)
+		assert.NilError(t, os.WriteFile(configFile, []byte(configJSON), 0644))
+
+		err := d.StartWithError("--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com", "--config-file", configFile, "--validate")
+		assert.ErrorContains(t, err, "daemon exited during startup")
+		logs, err := d.ReadLogFile()
+		assert.NilError(t, err)
+		expected := fmt.Sprintf(
+			`the following directives are specified both as a flag and in the configuration file: http-proxy: (from flag: %[1]s, from file: %[1]s), https-proxy: (from flag: %[1]s, from file: %[1]s), no-proxy: (from flag: example.com, from file: example.com)`,
+			proxyURL,
+		)
+		assert.Assert(t, is.Contains(string(logs), expected))
+	})
+
+	// Make sure values are sanitized when reloading the daemon-config
+	t.Run("reload sanitized", func(t *testing.T) {
+		const (
+			proxyRawURL = "https://" + userPass + "example.org"
+			proxyURL    = "https://xxxxx:xxxxx@example.org"
+		)
+
+		d := daemon.New(t)
+		d.Start(t, "--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com")
+		defer d.Stop(t)
+		err := d.Signal(syscall.SIGHUP)
+		assert.NilError(t, err)
+
+		logs, err := d.ReadLogFile()
+		assert.NilError(t, err)
+
+		// FIXME: there appears to ba a race condition, which causes ReadLogFile
+		//        to not contain the full logs after signaling the daemon to reload,
+		//        causing the test to fail here. As a workaround, check if we
+		//        received the "reloaded" message after signaling, and only then
+		//        check that it's sanitized properly. For more details on this
+		//        issue, see https://github.com/moby/moby/pull/42835/files#r713120315
+		if !strings.Contains(string(logs), "Reloaded configuration:") {
+			t.Skip("Skipping test, because we did not find 'Reloaded configuration' in the logs")
+		}
+
+		assert.Assert(t, is.Contains(string(logs), proxyURL))
+		assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs))
+	})
+}

testutil/daemon/daemon.go

@@ -262,6 +262,7 @@ func (d *Daemon) LogFileName() string {
 
 // ReadLogFile returns the content of the daemon log file
 func (d *Daemon) ReadLogFile() ([]byte, error) {
+	_ = d.logFile.Sync()
 	return os.ReadFile(d.logFile.Name())
 }