From d1c6550f71567c9a16bbf271273150067711560f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 8 Feb 2025 14:25:52 +0100 Subject: [PATCH] daemon: use structured logs for printing reloaded config, move to cli - Move logging out of config.Reload and daemon.Reload itself, as it was not the right place to know whether it was a "signal" that triggered the reload. - Use Daemon.Config() to get the new config after reloading. This returns an immutable copy of the daemon's config, so we can redact fields without having to use an ad-hoc struct to shadow the underlying fields. - Use structured logs for logging config reload events. Before this (plain text): INFO[2025-02-08T12:13:53.389649297Z] Got signal to reload configuration, reloading from: /etc/docker/daemon.json INFO[2025-02-08T12:30:34.857691260Z] Reloaded configuration: {"pidfile":"/var/run/docker.pid","data-root":"/var/lib/docker","exec-root":"/var/run/docker","group":"docker","max-concurrent-downloads":3,"max-concurrent-uploads":5,"max-download-attempts":5,"shutdown-timeout":15,"hosts":["unix:///var/run/docker.sock"],"log-level":"info","log-format":"text","swarm-default-advertise-addr":"","swarm-raft-heartbeat-tick":0,"swarm-raft-election-tick":0,"metrics-addr":"","host-gateway-ips":[""],"log-driver":"json-file","mtu":1500,"ip":"0.0.0.0","icc":true,"iptables":true,"ip6tables":true,"ip-forward":true,"ip-masq":true,"userland-proxy":true,"userland-proxy-path":"/usr/local/bin/docker-proxy","default-address-pools":{"Values":null},"network-control-plane-mtu":1500,"experimental":false,"containerd":"/var/run/docker/containerd/containerd.sock","features":{"containerd-snapshotter":false},"builder":{"GC":{},"Entitlements":{}},"containerd-namespace":"moby","containerd-plugin-namespace":"plugins.moby","default-runtime":"runc","runtimes":{"crun":{"path":"/usr/local/bin/crun"}},"seccomp-profile":"builtin","default-shm-size":67108864,"default-ipc-mode":"private","default-cgroupns-mode":"private","resolv-conf":"/etc/resolv.conf","proxies":{}} Before this (JSON logs): {"level":"info","msg":"Reloaded configuration: {\"pidfile\":\"/var/run/docker.pid\",\"data-root\":\"/var/lib/docker\",\"exec-root\":\"/var/run/docker\",\"group\":\"docker\",\"max-concurrent-downloads\":3,\"max-concurrent-uploads\":5,\"max-download-attempts\":5,\"shutdown-timeout\":15,\"hosts\":[\"unix:///var/run/docker.sock\"],\"log-level\":\"info\",\"log-format\":\"json\",\"swarm-default-advertise-addr\":\"\",\"swarm-raft-heartbeat-tick\":0,\"swarm-raft-election-tick\":0,\"metrics-addr\":\"\",\"host-gateway-ips\":[\"\"],\"log-driver\":\"json-file\",\"mtu\":1500,\"ip\":\"0.0.0.0\",\"icc\":true,\"iptables\":true,\"ip6tables\":true,\"ip-forward\":true,\"ip-masq\":true,\"userland-proxy\":true,\"userland-proxy-path\":\"/usr/local/bin/docker-proxy\",\"default-address-pools\":{\"Values\":null},\"network-control-plane-mtu\":1500,\"experimental\":false,\"containerd\":\"/var/run/docker/containerd/containerd.sock\",\"features\":{\"containerd-snapshotter\":false},\"builder\":{\"GC\":{},\"Entitlements\":{}},\"containerd-namespace\":\"moby\",\"containerd-plugin-namespace\":\"plugins.moby\",\"default-runtime\":\"runc\",\"runtimes\":{\"crun\":{\"path\":\"/usr/local/bin/crun\"}},\"seccomp-profile\":\"builtin\",\"default-shm-size\":67108864,\"default-ipc-mode\":\"private\",\"default-cgroupns-mode\":\"private\",\"resolv-conf\":\"/etc/resolv.conf\",\"proxies\":{}}","time":"2025-02-08T12:24:38.600761054Z"} After this (plain text): INFO[2025-02-08T12:30:34.835953594Z] Got signal to reload configuration config-file=/etc/docker/daemon.json INFO[2025-02-08T12:30:34.857614135Z] Reloaded configuration config="{\"pidfile\":\"/var/run/docker.pid\",\"data-root\":\"/var/lib/docker\",\"exec-root\":\"/var/run/docker\",\"group\":\"docker\",\"max-concurrent-downloads\":3,\"max-concurrent-uploads\":5,\"max-download-attempts\":5,\"shutdown-timeout\":15,\"hosts\":[\"unix:///var/run/docker.sock\"],\"log-level\":\"info\",\"log-format\":\"text\",\"swarm-default-advertise-addr\":\"\",\"swarm-raft-heartbeat-tick\":0,\"swarm-raft-election-tick\":0,\"metrics-addr\":\"\",\"host-gateway-ips\":[\"\"],\"log-driver\":\"json-file\",\"mtu\":1500,\"ip\":\"0.0.0.0\",\"icc\":true,\"iptables\":true,\"ip6tables\":true,\"ip-forward\":true,\"ip-masq\":true,\"userland-proxy\":true,\"userland-proxy-path\":\"/usr/local/bin/docker-proxy\",\"default-address-pools\":{\"Values\":null},\"network-control-plane-mtu\":1500,\"experimental\":false,\"containerd\":\"/var/run/docker/containerd/containerd.sock\",\"features\":{\"containerd-snapshotter\":false},\"builder\":{\"GC\":{},\"Entitlements\":{}},\"containerd-namespace\":\"moby\",\"containerd-plugin-namespace\":\"plugins.moby\",\"default-runtime\":\"runc\",\"runtimes\":{\"crun\":{\"path\":\"/usr/local/bin/crun\"}},\"seccomp-profile\":\"builtin\",\"default-shm-size\":67108864,\"default-ipc-mode\":\"private\",\"default-cgroupns-mode\":\"private\",\"resolv-conf\":\"/etc/resolv.conf\",\"proxies\":{}}" After this (JSON logs): {"config-file":"/etc/docker/daemon.json","level":"info","msg":"Got signal to reload configuration","time":"2025-02-08T12:24:38.589955637Z"} {"config":"{\"pidfile\":\"/var/run/docker.pid\",\"data-root\":\"/var/lib/docker\",\"exec-root\":\"/var/run/docker\",\"group\":\"docker\",\"max-concurrent-downloads\":3,\"max-concurrent-uploads\":5,\"max-download-attempts\":5,\"shutdown-timeout\":15,\"hosts\":[\"unix:///var/run/docker.sock\"],\"log-level\":\"info\",\"log-format\":\"json\",\"swarm-default-advertise-addr\":\"\",\"swarm-raft-heartbeat-tick\":0,\"swarm-raft-election-tick\":0,\"metrics-addr\":\"\",\"host-gateway-ips\":[\"\"],\"log-driver\":\"json-file\",\"mtu\":1500,\"ip\":\"0.0.0.0\",\"icc\":true,\"iptables\":true,\"ip6tables\":true,\"ip-forward\":true,\"ip-masq\":true,\"userland-proxy\":true,\"userland-proxy-path\":\"/usr/local/bin/docker-proxy\",\"default-address-pools\":{\"Values\":null},\"network-control-plane-mtu\":1500,\"experimental\":false,\"containerd\":\"/var/run/docker/containerd/containerd.sock\",\"features\":{\"containerd-snapshotter\":false},\"builder\":{\"GC\":{},\"Entitlements\":{}},\"containerd-namespace\":\"moby\",\"containerd-plugin-namespace\":\"plugins.moby\",\"default-runtime\":\"runc\",\"runtimes\":{\"crun\":{\"path\":\"/usr/local/bin/crun\"}},\"seccomp-profile\":\"builtin\",\"default-shm-size\":67108864,\"default-ipc-mode\":\"private\",\"default-cgroupns-mode\":\"private\",\"resolv-conf\":\"/etc/resolv.conf\",\"proxies\":{}}","level":"info","msg":"Reloaded configuration","time":"2025-02-08T12:24:38.600736179Z"} Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/daemon.go | 18 +++++++++++++++--- daemon/config/config.go | 13 +++++++++++-- daemon/config/config_test.go | 23 +++++++++++++++++++++++ daemon/reload.go | 19 +------------------ integration/daemon/daemon_test.go | 2 +- 5 files changed, 51 insertions(+), 24 deletions(-) diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 38a52a10b2..10769fbee6 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -3,6 +3,7 @@ package main import ( "context" "crypto/tls" + "encoding/json" "fmt" "net" "net/http" @@ -469,14 +470,15 @@ type builderOptions struct { func (cli *daemonCLI) reloadConfig() { ctx := context.TODO() + log.G(ctx).WithField("config-file", *cli.configFile).Info("Got signal to reload configuration") reload := func(c *config.Config) { if err := validateAuthzPlugins(c.AuthorizationPlugins, cli.d.PluginStore); err != nil { - log.G(ctx).Fatalf("Error validating authorization plugin: %v", err) + log.G(ctx).WithError(err).Fatal("Error validating authorization plugin") return } if err := cli.d.Reload(c); err != nil { - log.G(ctx).Errorf("Error reconfiguring the daemon: %v", err) + log.G(ctx).WithError(err).Error("Error reconfiguring the daemon") return } @@ -497,7 +499,17 @@ func (cli *daemonCLI) reloadConfig() { } if err := config.Reload(*cli.configFile, cli.flags, reload); err != nil { - log.G(ctx).Error(err) + log.G(ctx).WithError(err).Error("Error reloading configuration") + return + } + + sanitizedConfig := config.Sanitize(cli.d.Config()) + jsonData, err := json.Marshal(sanitizedConfig) + if err != nil { + log.G(context.TODO()).WithError(err).Warn("Error when marshaling configuration for printing") + log.G(context.TODO()).Info("Reloaded configuration") + } else { + log.G(context.TODO()).WithField("config", string(jsonData)).Info("Reloaded configuration") } } diff --git a/daemon/config/config.go b/daemon/config/config.go index 2f6e7f6151..ca533b5618 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -2,7 +2,6 @@ package config // import "github.com/docker/docker/daemon/config" import ( "bytes" - "context" "encoding/json" stderrors "errors" "fmt" @@ -380,7 +379,6 @@ func GetConflictFreeLabels(labels []string) ([]string, error) { // Reload reads the configuration in the host and reloads the daemon and server. func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error { - log.G(context.TODO()).Infof("Got signal to reload configuration, reloading from: %s", configFile) newConfig, err := getConflictFreeConfiguration(configFile, flags) if err != nil { if flags.Changed("config-file") || !os.IsNotExist(err) { @@ -801,3 +799,14 @@ func migrateHostGatewayIP(config *Config) { config.HostGatewayIP = nil //nolint:staticcheck // ignore SA1019: clearing old value. } } + +// Sanitize sanitizes the config for printing. It is currently limited to +// masking usernames and passwords from Proxy URLs. +func Sanitize(cfg Config) Config { + cfg.CommonConfig.Proxies = Proxies{ + HTTPProxy: MaskCredentials(cfg.HTTPProxy), + HTTPSProxy: MaskCredentials(cfg.HTTPSProxy), + NoProxy: MaskCredentials(cfg.NoProxy), + } + return cfg +} diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index 2a9a662470..85570873a0 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -794,3 +794,26 @@ func TestMaskURLCredentials(t *testing.T) { assert.Equal(t, maskedURL, test.maskedURL) } } + +func TestSanitize(t *testing.T) { + const ( + userPass = "myuser:mypassword@" + proxyRawURL = "https://" + userPass + "example.org" + proxyURL = "https://xxxxx:xxxxx@example.org" + ) + sanitizedCfg := Sanitize(Config{ + CommonConfig: CommonConfig{ + Proxies: Proxies{ + HTTPProxy: proxyRawURL, + HTTPSProxy: proxyRawURL, + NoProxy: proxyRawURL, + }, + }, + }) + expectedProxies := Proxies{ + HTTPProxy: proxyURL, + HTTPSProxy: proxyURL, + NoProxy: proxyURL, + } + assert.Check(t, is.DeepEqual(sanitizedCfg.Proxies, expectedProxies)) +} diff --git a/daemon/reload.go b/daemon/reload.go index 2c28551d95..bb8b818ffd 100644 --- a/daemon/reload.go +++ b/daemon/reload.go @@ -96,6 +96,7 @@ func (daemon *Daemon) Reload(conf *config.Config) error { var txn reloadTxn for _, reload := range []func(txn *reloadTxn, newCfg *configStore, conf *config.Config, attributes map[string]string) error{ + // TODO(thaJeztah): most of these are defined as method, but don't use the daemon receiver; consider making them regular functions. daemon.reloadPlatform, daemon.reloadDebug, daemon.reloadMaxConcurrentDownloadsAndUploads, @@ -115,24 +116,6 @@ func (daemon *Daemon) Reload(conf *config.Config) error { } } - redactedConfig := struct { - *config.Config - config.Proxies `json:"proxies"` - }{ - Config: &newCfg.Config, - Proxies: config.Proxies{ - HTTPProxy: config.MaskCredentials(newCfg.HTTPProxy), - HTTPSProxy: config.MaskCredentials(newCfg.HTTPSProxy), - NoProxy: config.MaskCredentials(newCfg.NoProxy), - }, - } - jsonData, err := json.Marshal(&redactedConfig) - if err != nil { - log.G(context.TODO()).WithError(err).Warn("Error when marshaling configuration for printing") - log.G(context.TODO()).Info("Reloaded configuration") - } else { - log.G(context.TODO()).Infof("Reloaded configuration: %s", jsonData) - } daemon.configStore.Store(newCfg) daemon.LogDaemonEventWithAttributes(events.ActionReload, attributes) return txn.Commit() diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index f8c2b2ca16..78169a9372 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -440,7 +440,7 @@ func TestDaemonProxy(t *testing.T) { err := d.Signal(syscall.SIGHUP) assert.NilError(t, err) - poll.WaitOn(t, d.PollCheckLogs(ctx, daemon.ScanLogsMatchAll("Reloaded configuration:", proxyURL))) + poll.WaitOn(t, d.PollCheckLogs(ctx, daemon.ScanLogsMatchAll("Reloaded configuration", proxyURL))) ok, logs := d.ScanLogsT(ctx, t, daemon.ScanLogsMatchString(userPass)) assert.Assert(t, !ok, "logs should not contain the non-sanitized proxy URL: %s", logs)