diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 44e16677e7..02a03141df 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -480,22 +480,12 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { logrus.Warnf(`The "-g / --graph" flag is deprecated. Please use "--data-root" instead`) } - // Labels of the docker engine used to allow multiple values associated with the same key. - // This is deprecated in 1.13, and, be removed after 3 release cycles. - // The following will check the conflict of labels, and report a warning for deprecation. - // - // TODO: After 3 release cycles (17.12) an error will be returned, and labels will be - // sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels): - // - // newLabels, err := daemon.GetConflictFreeLabels(config.Labels) - // if err != nil { - // return nil, err - // } - // config.Labels = newLabels - // - if _, err := config.GetConflictFreeLabels(conf.Labels); err != nil { - logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err) + // Check if duplicate label-keys with different values are found + newLabels, err := config.GetConflictFreeLabels(conf.Labels) + if err != nil { + return nil, err } + conf.Labels = newLabels // Regardless of whether the user sets it to true or false, if they // specify TLSVerify at all then we need to turn on TLS diff --git a/cmd/dockerd/daemon_test.go b/cmd/dockerd/daemon_test.go index c559ee82a5..e5e4aa34e2 100644 --- a/cmd/dockerd/daemon_test.go +++ b/cmd/dockerd/daemon_test.go @@ -61,6 +61,28 @@ func TestLoadDaemonCliConfigWithConflicts(t *testing.T) { testutil.ErrorContains(t, err, "as a flag and in the configuration file: labels") } +func TestLoadDaemonCliWithConflictingLabels(t *testing.T) { + opts := defaultOptions("") + flags := opts.flags + + assert.NoError(t, flags.Set("label", "foo=bar")) + assert.NoError(t, flags.Set("label", "foo=baz")) + + _, err := loadDaemonCliConfig(opts) + assert.EqualError(t, err, "conflict labels for foo=baz and foo=bar") +} + +func TestLoadDaemonCliWithDuplicateLabels(t *testing.T) { + opts := defaultOptions("") + flags := opts.flags + + assert.NoError(t, flags.Set("label", "foo=the-same")) + assert.NoError(t, flags.Set("label", "foo=the-same")) + + _, err := loadDaemonCliConfig(opts) + assert.NoError(t, err) +} + func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) { tempFile := fs.NewFile(t, "config", fs.WithContent(`{"tlsverify": true}`)) defer tempFile.Remove() diff --git a/daemon/config/config.go b/daemon/config/config.go index f7a0df58ed..1e22a6fea7 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -258,22 +258,12 @@ func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error return fmt.Errorf("file configuration validation failed (%v)", err) } - // Labels of the docker engine used to allow multiple values associated with the same key. - // This is deprecated in 1.13, and, be removed after 3 release cycles. - // The following will check the conflict of labels, and report a warning for deprecation. - // - // TODO: After 3 release cycles (17.12) an error will be returned, and labels will be - // sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels): - // - // newLabels, err := GetConflictFreeLabels(newConfig.Labels) - // if err != nil { - // return err - // } - // newConfig.Labels = newLabels - // - if _, err := GetConflictFreeLabels(newConfig.Labels); err != nil { - logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err) + // Check if duplicate label-keys with different values are found + newLabels, err := GetConflictFreeLabels(newConfig.Labels) + if err != nil { + return err } + newConfig.Labels = newLabels reload(newConfig) return nil diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index bdd046bcdc..cb7fb00a72 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/daemon/discovery" "github.com/docker/docker/internal/testutil" "github.com/docker/docker/opts" + "github.com/gotestyourself/gotestyourself/fs" "github.com/spf13/pflag" "github.com/stretchr/testify/assert" ) @@ -459,3 +460,29 @@ func TestReloadBadDefaultConfig(t *testing.T) { assert.Error(t, err) testutil.ErrorContains(t, err, "unable to configure the Docker daemon with file") } + +func TestReloadWithConflictingLabels(t *testing.T) { + tempFile := fs.NewFile(t, "config", fs.WithContent(`{"labels":["foo=bar","foo=baz"]}`)) + defer tempFile.Remove() + configFile := tempFile.Path() + + var lbls []string + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.String("config-file", configFile, "") + flags.StringSlice("labels", lbls, "") + err := Reload(configFile, flags, func(c *Config) {}) + testutil.ErrorContains(t, err, "conflict labels for foo=baz and foo=bar") +} + +func TestReloadWithDuplicateLabels(t *testing.T) { + tempFile := fs.NewFile(t, "config", fs.WithContent(`{"labels":["foo=the-same","foo=the-same"]}`)) + defer tempFile.Remove() + configFile := tempFile.Path() + + var lbls []string + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.String("config-file", configFile, "") + flags.StringSlice("labels", lbls, "") + err := Reload(configFile, flags, func(c *Config) {}) + assert.NoError(t, err) +} diff --git a/integration-cli/docker_cli_info_test.go b/integration-cli/docker_cli_info_test.go index b6f867373b..d7ce238bfd 100644 --- a/integration-cli/docker_cli_info_test.go +++ b/integration-cli/docker_cli_info_test.go @@ -233,17 +233,6 @@ func (s *DockerDaemonSuite) TestRegistryMirrors(c *check.C) { c.Assert(out, checker.Contains, fmt.Sprintf(" %s", registryMirror2)) } -// Test case for #24392 -func (s *DockerDaemonSuite) TestInfoLabels(c *check.C) { - testRequires(c, SameHostDaemon, DaemonIsLinux) - - s.d.Start(c, "--label", `test.empty=`, "--label", `test.empty=`, "--label", `test.label="1"`, "--label", `test.label="2"`) - - out, err := s.d.Cmd("info") - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Contains, "WARNING: labels with duplicate keys and conflicting values have been deprecated") -} - func existingContainerStates(c *check.C) map[string]int { out, _ := dockerCmd(c, "info", "--format", "{{json .}}") var m map[string]interface{}