From 8f80e55111b3e4b31590c29cdbe68ca6e1110dab Mon Sep 17 00:00:00 2001 From: Rich Horwood Date: Mon, 5 Nov 2018 11:52:26 +1100 Subject: [PATCH] Add configuration validation option and tests. Fixes #36911 If config file is invalid we'll exit anyhow, so this just prevents the daemon from starting if the configuration is fine. Mainly useful for making config changes and restarting the daemon iff the config is valid. Signed-off-by: Rich Horwood Signed-off-by: Sebastiaan van Stijn Signed-off-by: Anca Iordache --- cmd/dockerd/daemon.go | 13 ++- cmd/dockerd/options.go | 2 + daemon/config/config.go | 15 +-- integration/config/config_test.go | 23 ---- integration/daemon/daemon_test.go | 101 ++++++++++++++++++ integration/daemon/main_test.go | 10 ++ .../daemon/testdata/empty-config-1.json | 1 + .../daemon/testdata/empty-config-2.json | 1 + .../daemon/testdata/invalid-config-1.json | 1 + .../daemon/testdata/malformed-config.json | 1 + .../daemon/testdata/valid-config-1.json | 1 + testutil/daemon/daemon.go | 13 ++- 12 files changed, 148 insertions(+), 34 deletions(-) create mode 100644 integration/daemon/daemon_test.go create mode 100644 integration/daemon/main_test.go create mode 100644 integration/daemon/testdata/empty-config-1.json create mode 100644 integration/daemon/testdata/empty-config-2.json create mode 100644 integration/daemon/testdata/invalid-config-1.json create mode 100644 integration/daemon/testdata/malformed-config.json create mode 100644 integration/daemon/testdata/valid-config-1.json diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index bb3d72ab38..d163eec51f 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -75,14 +75,18 @@ func NewDaemonCli() *DaemonCli { } func (cli *DaemonCli) start(opts *daemonOptions) (err error) { - stopc := make(chan bool) - defer close(stopc) - opts.SetDefaultOptions(opts.flags) if cli.Config, err = loadDaemonCliConfig(opts); err != nil { return err } + + if opts.Validate { + // If config wasn't OK we wouldn't have made it this far. + fmt.Fprintln(os.Stderr, "configuration OK") + return nil + } + warnOnDeprecatedConfigOptions(cli.Config) if err := configureDaemonLogs(cli.Config); err != nil { @@ -178,6 +182,9 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { } defer cancel() + stopc := make(chan bool) + defer close(stopc) + signal.Trap(func() { cli.stop() <-stopc // wait for daemonCli.start() to return diff --git a/cmd/dockerd/options.go b/cmd/dockerd/options.go index e8e8fd41e8..6bdc97a14a 100644 --- a/cmd/dockerd/options.go +++ b/cmd/dockerd/options.go @@ -41,6 +41,7 @@ type daemonOptions struct { TLS bool TLSVerify bool TLSOptions *tlsconfig.Options + Validate bool } // newDaemonOptions returns a new daemonFlags @@ -59,6 +60,7 @@ func (o *daemonOptions) InstallFlags(flags *pflag.FlagSet) { } flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode") + flags.BoolVar(&o.Validate, "validate", false, "Validate configuration file and exit") flags.StringVarP(&o.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`) flags.BoolVar(&o.TLS, FlagTLS, DefaultTLSValue, "Use TLS; implied by --tlsverify") flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify || DefaultTLSValue, "Use TLS and verify the remote") diff --git a/daemon/config/config.go b/daemon/config/config.go index 4990727597..fb78c06e7c 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/json" "fmt" - "io" "io/ioutil" "net" "os" @@ -394,11 +393,16 @@ func getConflictFreeConfiguration(configFile string, flags *pflag.FlagSet) (*Con } var config Config - var reader io.Reader + + b = bytes.TrimSpace(b) + if len(b) == 0 { + // empty config file + return &config, nil + } + if flags != nil { var jsonConfig map[string]interface{} - reader = bytes.NewReader(b) - if err := json.NewDecoder(reader).Decode(&jsonConfig); err != nil { + if err := json.Unmarshal(b, &jsonConfig); err != nil { return nil, err } @@ -441,8 +445,7 @@ func getConflictFreeConfiguration(configFile string, flags *pflag.FlagSet) (*Con config.ValuesSet = configSet } - reader = bytes.NewReader(b) - if err := json.NewDecoder(reader).Decode(&config); err != nil { + if err := json.Unmarshal(b, &config); err != nil { return nil, err } diff --git a/integration/config/config_test.go b/integration/config/config_test.go index 7f56fab112..5df1bfc3c7 100644 --- a/integration/config/config_test.go +++ b/integration/config/config_test.go @@ -4,8 +4,6 @@ import ( "bytes" "context" "encoding/json" - "io/ioutil" - "path/filepath" "sort" "testing" "time" @@ -17,7 +15,6 @@ import ( "github.com/docker/docker/errdefs" "github.com/docker/docker/integration/internal/swarm" "github.com/docker/docker/pkg/stdcopy" - "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/poll" @@ -379,26 +376,6 @@ func TestConfigCreateResolve(t *testing.T) { assert.Assert(t, is.Equal(0, len(entries))) } -func TestConfigDaemonLibtrustID(t *testing.T) { - skip.If(t, testEnv.DaemonInfo.OSType != "linux") - defer setupTest(t)() - - d := daemon.New(t) - defer d.Stop(t) - - trustKey := filepath.Join(d.RootDir(), "key.json") - err := ioutil.WriteFile(trustKey, []byte(`{"crv":"P-256","d":"dm28PH4Z4EbyUN8L0bPonAciAQa1QJmmyYd876mnypY","kid":"WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB","kty":"EC","x":"Mh5-JINSjaa_EZdXDttri255Z5fbCEOTQIZjAcScFTk","y":"eUyuAjfxevb07hCCpvi4Zi334Dy4GDWQvEToGEX4exQ"}`), 0644) - assert.NilError(t, err) - - config := filepath.Join(d.RootDir(), "daemon.json") - err = ioutil.WriteFile(config, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644) - assert.NilError(t, err) - - d.Start(t, "--config-file", config) - info := d.Info(t) - assert.Equal(t, info.ID, "WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB") -} - func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) { buf := bytes.NewBuffer(nil) _, err := stdcopy.StdCopy(buf, buf, attach.Reader) diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go new file mode 100644 index 0000000000..661f1ae91e --- /dev/null +++ b/integration/daemon/daemon_test.go @@ -0,0 +1,101 @@ +package daemon // import "github.com/docker/docker/integration/daemon" + +import ( + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/docker/docker/testutil/daemon" + "gotest.tools/v3/assert" + "gotest.tools/v3/skip" + + is "gotest.tools/v3/assert/cmp" +) + +func TestConfigDaemonLibtrustID(t *testing.T) { + skip.If(t, runtime.GOOS != "linux") + + d := daemon.New(t) + defer d.Stop(t) + + trustKey := filepath.Join(d.RootDir(), "key.json") + err := ioutil.WriteFile(trustKey, []byte(`{"crv":"P-256","d":"dm28PH4Z4EbyUN8L0bPonAciAQa1QJmmyYd876mnypY","kid":"WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB","kty":"EC","x":"Mh5-JINSjaa_EZdXDttri255Z5fbCEOTQIZjAcScFTk","y":"eUyuAjfxevb07hCCpvi4Zi334Dy4GDWQvEToGEX4exQ"}`), 0644) + assert.NilError(t, err) + + config := filepath.Join(d.RootDir(), "daemon.json") + err = ioutil.WriteFile(config, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644) + assert.NilError(t, err) + + d.Start(t, "--config-file", config) + info := d.Info(t) + assert.Equal(t, info.ID, "WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB") +} + +func TestDaemonConfigValidation(t *testing.T) { + skip.If(t, runtime.GOOS != "linux") + + d := daemon.New(t) + dockerBinary, err := d.BinaryPath() + assert.NilError(t, err) + params := []string{"--validate", "--config-file"} + + dest := os.Getenv("DOCKER_INTEGRATION_DAEMON_DEST") + if dest == "" { + dest = os.Getenv("DEST") + } + testdata := filepath.Join(dest, "..", "..", "integration", "daemon", "testdata") + + const ( + validOut = "configuration OK" + failedOut = "unable to configure the Docker daemon with file" + ) + + tests := []struct { + name string + args []string + expectedOut string + }{ + { + name: "config with no content", + args: append(params, filepath.Join(testdata, "empty-config-1.json")), + expectedOut: validOut, + }, + { + name: "config with {}", + args: append(params, filepath.Join(testdata, "empty-config-2.json")), + expectedOut: validOut, + }, + { + name: "invalid config", + args: append(params, filepath.Join(testdata, "invalid-config-1.json")), + expectedOut: failedOut, + }, + { + name: "malformed config", + args: append(params, filepath.Join(testdata, "malformed-config.json")), + expectedOut: failedOut, + }, + { + name: "valid config", + args: append(params, filepath.Join(testdata, "valid-config-1.json")), + expectedOut: validOut, + }, + } + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cmd := exec.Command(dockerBinary, tc.args...) + out, err := cmd.CombinedOutput() + assert.Check(t, is.Contains(string(out), tc.expectedOut)) + if tc.expectedOut == failedOut { + assert.ErrorContains(t, err, "", "expected an error, but got none") + } else { + assert.NilError(t, err) + } + }) + } +} diff --git a/integration/daemon/main_test.go b/integration/daemon/main_test.go new file mode 100644 index 0000000000..74a342e298 --- /dev/null +++ b/integration/daemon/main_test.go @@ -0,0 +1,10 @@ +package daemon // import "github.com/docker/docker/integration/daemon" + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + os.Exit(m.Run()) +} diff --git a/integration/daemon/testdata/empty-config-1.json b/integration/daemon/testdata/empty-config-1.json new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/integration/daemon/testdata/empty-config-1.json @@ -0,0 +1 @@ + diff --git a/integration/daemon/testdata/empty-config-2.json b/integration/daemon/testdata/empty-config-2.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/integration/daemon/testdata/empty-config-2.json @@ -0,0 +1 @@ +{} diff --git a/integration/daemon/testdata/invalid-config-1.json b/integration/daemon/testdata/invalid-config-1.json new file mode 100644 index 0000000000..f3c7781e52 --- /dev/null +++ b/integration/daemon/testdata/invalid-config-1.json @@ -0,0 +1 @@ +{"unknown-option": true} diff --git a/integration/daemon/testdata/malformed-config.json b/integration/daemon/testdata/malformed-config.json new file mode 100644 index 0000000000..98232c64fc --- /dev/null +++ b/integration/daemon/testdata/malformed-config.json @@ -0,0 +1 @@ +{ diff --git a/integration/daemon/testdata/valid-config-1.json b/integration/daemon/testdata/valid-config-1.json new file mode 100644 index 0000000000..b1608d8f8a --- /dev/null +++ b/integration/daemon/testdata/valid-config-1.json @@ -0,0 +1 @@ +{"debug": true} diff --git a/testutil/daemon/daemon.go b/testutil/daemon/daemon.go index 52882e4dcb..bcf09892a5 100644 --- a/testutil/daemon/daemon.go +++ b/testutil/daemon/daemon.go @@ -217,6 +217,15 @@ func New(t testing.TB, ops ...Option) *Daemon { return d } +// BinaryPath returns the binary and its arguments. +func (d *Daemon) BinaryPath() (string, error) { + dockerdBinary, err := exec.LookPath(d.dockerdBinary) + if err != nil { + return "", errors.Wrapf(err, "[%s] could not find docker binary in $PATH", d.id) + } + return dockerdBinary, nil +} + // ContainersNamespace returns the containerd namespace used for containers. func (d *Daemon) ContainersNamespace() string { return d.id @@ -307,9 +316,9 @@ func (d *Daemon) StartWithError(args ...string) error { // StartWithLogFile will start the daemon and attach its streams to a given file. func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error { d.handleUserns() - dockerdBinary, err := exec.LookPath(d.dockerdBinary) + dockerdBinary, err := d.BinaryPath() if err != nil { - return errors.Wrapf(err, "[%s] could not find docker binary in $PATH", d.id) + return err } if d.pidFile == "" {