From f13c08246d93dd5aae200d5881a3a374e6cac876 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 29 Feb 2024 18:16:38 -0800 Subject: [PATCH] Add feature to daemon flags Signed-off-by: Derek McGowan Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/config.go | 2 + daemon/config/config.go | 1 + daemon/config/config_linux_test.go | 67 +++++++++++++++++++++++++ integration/daemon/daemon_test.go | 68 +++++++++++++++++++++++++ internal/opts/opts.go | 80 ++++++++++++++++++++++++++++++ internal/opts/opts_test.go | 46 +++++++++++++++++ 6 files changed, 264 insertions(+) create mode 100644 internal/opts/opts.go create mode 100644 internal/opts/opts_test.go diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index 50852401a7..2dda3ef981 100644 --- a/cmd/dockerd/config.go +++ b/cmd/dockerd/config.go @@ -4,6 +4,7 @@ import ( "runtime" "github.com/docker/docker/daemon/config" + dopts "github.com/docker/docker/internal/opts" "github.com/docker/docker/opts" "github.com/docker/docker/registry" "github.com/spf13/pflag" @@ -28,6 +29,7 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) { flags.StringVar(&conf.ExecRoot, "exec-root", conf.ExecRoot, "Root directory for execution state files") flags.StringVar(&conf.ContainerdAddr, "containerd", "", "containerd grpc address") flags.BoolVar(&conf.CriContainerd, "cri-containerd", false, "start containerd with cri") + flags.Var(dopts.NewNamedSetOpts("features", conf.Features), "feature", "Enable feature in the daemon") flags.Var(opts.NewNamedMapMapOpts("default-network-opts", conf.DefaultNetworkOpts, nil), "default-network-opt", "Default network options") flags.IntVar(&conf.MTU, "mtu", conf.MTU, `Set the MTU for the default "bridge" network`) diff --git a/daemon/config/config.go b/daemon/config/config.go index 809727d4e6..6083a88a30 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -305,6 +305,7 @@ func New() (*Config, error) { }, ContainerdNamespace: DefaultContainersNamespace, ContainerdPluginNamespace: DefaultPluginNamespace, + Features: make(map[string]bool), DefaultRuntime: StockRuntimeName, MinAPIVersion: defaultMinAPIVersion, }, diff --git a/daemon/config/config_linux_test.go b/daemon/config/config_linux_test.go index 4b81be30b7..d3f96b2aed 100644 --- a/daemon/config/config_linux_test.go +++ b/daemon/config/config_linux_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/docker/docker/api/types/container" + dopts "github.com/docker/docker/internal/opts" "github.com/docker/docker/opts" "github.com/spf13/pflag" "gotest.tools/v3/assert" @@ -121,6 +122,72 @@ func TestDaemonConfigurationMergeShmSize(t *testing.T) { assert.Check(t, is.Equal(int64(expectedValue), cc.ShmSize.Value())) } +func TestDaemonConfigurationFeatures(t *testing.T) { + tests := []struct { + name, config, flags string + expectedValue map[string]bool + expectedErr string + }{ + { + name: "enable from file", + config: `{"features": {"containerd-snapshotter": true}}`, + expectedValue: map[string]bool{"containerd-snapshotter": true}, + }, + { + name: "enable from flags", + config: `{}`, + flags: "containerd-snapshotter=true", + expectedValue: map[string]bool{"containerd-snapshotter": true}, + }, + { + name: "disable from file", + config: `{"features": {"containerd-snapshotter": false}}`, + expectedValue: map[string]bool{"containerd-snapshotter": false}, + }, + { + name: "disable from flags", + config: `{}`, + flags: "containerd-snapshotter=false", + expectedValue: map[string]bool{"containerd-snapshotter": false}, + }, + { + name: "conflict", + config: `{"features": {"containerd-snapshotter": true}}`, + flags: "containerd-snapshotter=true", + expectedErr: `the following directives are specified both as a flag and in the configuration file: features: (from flag: map[containerd-snapshotter:true], from file: map[containerd-snapshotter:true])`, + }, + { + name: "invalid config value", + config: `{"features": {"containerd-snapshotter": "not-a-boolean"}}`, + expectedErr: `json: cannot unmarshal string into Go struct field Config.features of type bool`, + }, + } + + for _, tc := range tests { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + c, err := New() + assert.NilError(t, err) + + configFile := makeConfigFile(t, tc.config) + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.Var(dopts.NewNamedSetOpts("features", c.Features), "feature", "Enable feature in the daemon") + if tc.flags != "" { + err = flags.Set("feature", tc.flags) + assert.NilError(t, err) + } + cc, err := MergeDaemonConfigurations(c, flags, configFile) + if tc.expectedErr != "" { + assert.Error(t, err, tc.expectedErr) + } else { + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(tc.expectedValue, cc.Features)) + } + }) + } +} + func TestUnixGetInitPath(t *testing.T) { testCases := []struct { config *Config diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index 275a7b3010..64589c38b6 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -179,6 +179,74 @@ func TestConfigDaemonSeccompProfiles(t *testing.T) { } } +func TestDaemonConfigFeatures(t *testing.T) { + skip.If(t, runtime.GOOS == "windows") + ctx := testutil.StartSpan(baseContext, t) + + 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() + _ = testutil.StartSpan(ctx, t) + 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) + } + }) + } +} + 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") diff --git a/internal/opts/opts.go b/internal/opts/opts.go new file mode 100644 index 0000000000..71a0173120 --- /dev/null +++ b/internal/opts/opts.go @@ -0,0 +1,80 @@ +package opts + +import ( + "fmt" + "strconv" + "strings" + + "github.com/docker/docker/opts" +) + +// SetOpts holds a map of values and a validation function. +type SetOpts struct { + values map[string]bool +} + +// Set validates if needed the input value and add it to the +// internal map, by splitting on '='. +func (opts *SetOpts) Set(value string) error { + k, v, found := strings.Cut(value, "=") + var isSet bool + if !found { + isSet = true + k = value + } else { + var err error + isSet, err = strconv.ParseBool(v) + if err != nil { + return err + } + } + opts.values[k] = isSet + return nil +} + +// GetAll returns the values of SetOpts as a map. +func (opts *SetOpts) GetAll() map[string]bool { + return opts.values +} + +func (opts *SetOpts) String() string { + return fmt.Sprintf("%v", opts.values) +} + +// Type returns a string name for this Option type +func (opts *SetOpts) Type() string { + return "map" +} + +// NewSetOpts creates a new SetOpts with the specified set of values as a map of string to bool. +func NewSetOpts(values map[string]bool) *SetOpts { + if values == nil { + values = make(map[string]bool) + } + return &SetOpts{ + values: values, + } +} + +// NamedSetOpts is a SetOpts struct with a configuration name. +// This struct is useful to keep reference to the assigned +// field name in the internal configuration struct. +type NamedSetOpts struct { + SetOpts + name string +} + +var _ opts.NamedOption = &NamedSetOpts{} + +// NewNamedSetOpts creates a reference to a new NamedSetOpts struct. +func NewNamedSetOpts(name string, values map[string]bool) *NamedSetOpts { + return &NamedSetOpts{ + SetOpts: *NewSetOpts(values), + name: name, + } +} + +// Name returns the name of the NamedSetOpts in the configuration. +func (o *NamedSetOpts) Name() string { + return o.name +} diff --git a/internal/opts/opts_test.go b/internal/opts/opts_test.go new file mode 100644 index 0000000000..954c5baab3 --- /dev/null +++ b/internal/opts/opts_test.go @@ -0,0 +1,46 @@ +package opts + +import ( + "testing" + + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestSetOpts(t *testing.T) { + tmpMap := make(map[string]bool) + o := NewSetOpts(tmpMap) + assert.NilError(t, o.Set("feature-a=1")) + assert.NilError(t, o.Set("feature-b=true")) + assert.NilError(t, o.Set("feature-c=0")) + assert.NilError(t, o.Set("feature-d=false")) + + expected := "map[feature-a:true feature-b:true feature-c:false feature-d:false]" + assert.Check(t, is.Equal(expected, o.String())) + + expectedValue := map[string]bool{"feature-a": true, "feature-b": true, "feature-c": false, "feature-d": false} + assert.Check(t, is.DeepEqual(expectedValue, o.GetAll())) + + err := o.Set("feature=not-a-bool") + assert.Check(t, is.Error(err, `strconv.ParseBool: parsing "not-a-bool": invalid syntax`)) +} + +func TestNamedSetOpts(t *testing.T) { + tmpMap := make(map[string]bool) + o := NewNamedSetOpts("features", tmpMap) + assert.Check(t, is.Equal("features", o.Name())) + + assert.NilError(t, o.Set("feature-a=1")) + assert.NilError(t, o.Set("feature-b=true")) + assert.NilError(t, o.Set("feature-c=0")) + assert.NilError(t, o.Set("feature-d=false")) + + expected := "map[feature-a:true feature-b:true feature-c:false feature-d:false]" + assert.Check(t, is.Equal(expected, o.String())) + + expectedValue := map[string]bool{"feature-a": true, "feature-b": true, "feature-c": false, "feature-d": false} + assert.Check(t, is.DeepEqual(expectedValue, o.GetAll())) + + err := o.Set("feature=not-a-bool") + assert.Check(t, is.Error(err, `strconv.ParseBool: parsing "not-a-bool": invalid syntax`)) +}