Skip to content

Commit

Permalink
Add configuration validation option and tests.
Browse files Browse the repository at this point in the history
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 <rjhorwood@apple.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Anca Iordache <anca.iordache@docker.com>
  • Loading branch information
Rich Horwood authored and aiordache committed Jun 23, 2021
1 parent 0f124ab commit 8f80e55
Show file tree
Hide file tree
Showing 12 changed files with 148 additions and 34 deletions.
13 changes: 10 additions & 3 deletions cmd/dockerd/daemon.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions cmd/dockerd/options.go
Expand Up @@ -41,6 +41,7 @@ type daemonOptions struct {
TLS bool
TLSVerify bool
TLSOptions *tlsconfig.Options
Validate bool
}

// newDaemonOptions returns a new daemonFlags
Expand All @@ -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")
Expand Down
15 changes: 9 additions & 6 deletions daemon/config/config.go
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net"
"os"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
23 changes: 0 additions & 23 deletions integration/config/config_test.go
Expand Up @@ -4,8 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"io/ioutil"
"path/filepath"
"sort"
"testing"
"time"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
101 changes: 101 additions & 0 deletions 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)
}
})
}
}
10 changes: 10 additions & 0 deletions 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())
}
1 change: 1 addition & 0 deletions integration/daemon/testdata/empty-config-1.json
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions integration/daemon/testdata/empty-config-2.json
@@ -0,0 +1 @@
{}
1 change: 1 addition & 0 deletions integration/daemon/testdata/invalid-config-1.json
@@ -0,0 +1 @@
{"unknown-option": true}
1 change: 1 addition & 0 deletions integration/daemon/testdata/malformed-config.json
@@ -0,0 +1 @@
{
1 change: 1 addition & 0 deletions integration/daemon/testdata/valid-config-1.json
@@ -0,0 +1 @@
{"debug": true}
13 changes: 11 additions & 2 deletions testutil/daemon/daemon.go
Expand Up @@ -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
Expand Down Expand Up @@ -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 == "" {
Expand Down

0 comments on commit 8f80e55

Please sign in to comment.