Skip to content

Commit e4c9079

Browse files
yongtangyongtang
committed
Remove duplicate keys in labels of docker info
This fix tries to address the issue raised in 24392 where labels with duplicate keys exist in `docker info`, which contradicts with the specifications in the docs. The reason for duplicate keys is that labels are stored as slice of strings in the format of `A=B` (and the input/output). This fix tries to address this issue by checking conflict labels when daemon started, and remove duplicate labels (K-V). The existing `/info` API has not been changed. An additional integration test has been added to cover the changes in this fix. This fix fixes 24392. Signed-off-by: Yong Tang <[email protected]>
1 parent 15ea28f commit e4c9079

5 files changed

Lines changed: 93 additions & 0 deletions

File tree

‎cli/command/system/info.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,21 @@ func prettyPrintInfo(dockerCli *command.DockerCli, info types.Info) error {
223223
for _, attribute := range info.Labels {
224224
fmt.Fprintf(dockerCli.Out(), " %s\n", attribute)
225225
}
226+
// TODO: Engine labels with duplicate keys has been deprecated in 1.13 and will be error out
227+
// after 3 release cycles (1.16). For now, a WARNING will be generated. The following will
228+
// be removed eventually.
229+
labelMap := map[string]string{}
230+
for _, label := range info.Labels {
231+
stringSlice := strings.SplitN(label, "=", 2)
232+
if len(stringSlice) > 1 {
233+
// If there is a conflict we will throw out an warning
234+
if v, ok := labelMap[stringSlice[0]]; ok && v != stringSlice[1] {
235+
fmt.Fprintln(dockerCli.Err(), "WARNING: labels with duplicate keys and conflicting values have been deprecated")
236+
break
237+
}
238+
labelMap[stringSlice[0]] = stringSlice[1]
239+
}
240+
}
226241
}
227242

228243
ioutils.FprintfIfTrue(dockerCli.Out(), "Experimental: %v\n", info.ExperimentalBuild)

‎cmd/dockerd/daemon.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,23 @@ func loadDaemonCliConfig(opts daemonOptions) (*daemon.Config, error) {
396396
return nil, err
397397
}
398398

399+
// Labels of the docker engine used to allow multiple values associated with the same key.
400+
// This is deprecated in 1.13, and, be removed after 3 release cycles.
401+
// The following will check the conflict of labels, and report a warning for deprecation.
402+
//
403+
// TODO: After 3 release cycles (1.16) an error will be returned, and labels will be
404+
// sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels):
405+
//
406+
// newLabels, err := daemon.GetConflictFreeLabels(config.Labels)
407+
// if err != nil {
408+
// return nil, err
409+
// }
410+
// config.Labels = newLabels
411+
//
412+
if _, err := daemon.GetConflictFreeLabels(config.Labels); err != nil {
413+
logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err)
414+
}
415+
399416
// Regardless of whether the user sets it to true or false, if they
400417
// specify TLSVerify at all then we need to turn on TLS
401418
if config.IsValueSet(cliflags.FlagTLSVerify) {

‎daemon/config.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,30 @@ func parseClusterAdvertiseSettings(clusterStore, clusterAdvertise string) (strin
220220
return advertise, nil
221221
}
222222

223+
// GetConflictFreeLabels validate Labels for conflict
224+
// In swarm the duplicates for labels are removed
225+
// so we only take same values here, no conflict values
226+
// If the key-value is the same we will only take the last label
227+
func GetConflictFreeLabels(labels []string) ([]string, error) {
228+
labelMap := map[string]string{}
229+
for _, label := range labels {
230+
stringSlice := strings.SplitN(label, "=", 2)
231+
if len(stringSlice) > 1 {
232+
// If there is a conflict we will return an error
233+
if v, ok := labelMap[stringSlice[0]]; ok && v != stringSlice[1] {
234+
return nil, fmt.Errorf("conflict labels for %s=%s and %s=%s", stringSlice[0], stringSlice[1], stringSlice[0], v)
235+
}
236+
labelMap[stringSlice[0]] = stringSlice[1]
237+
}
238+
}
239+
240+
newLabels := []string{}
241+
for k, v := range labelMap {
242+
newLabels = append(newLabels, fmt.Sprintf("%s=%s", k, v))
243+
}
244+
return newLabels, nil
245+
}
246+
223247
// ReloadConfiguration reads the configuration in the host and reloads the daemon and server.
224248
func ReloadConfiguration(configFile string, flags *pflag.FlagSet, reload func(*Config)) error {
225249
logrus.Infof("Got signal to reload configuration, reloading from: %s", configFile)
@@ -232,6 +256,23 @@ func ReloadConfiguration(configFile string, flags *pflag.FlagSet, reload func(*C
232256
return fmt.Errorf("file configuration validation failed (%v)", err)
233257
}
234258

259+
// Labels of the docker engine used to allow multiple values associated with the same key.
260+
// This is deprecated in 1.13, and, be removed after 3 release cycles.
261+
// The following will check the conflict of labels, and report a warning for deprecation.
262+
//
263+
// TODO: After 3 release cycles (1.16) an error will be returned, and labels will be
264+
// sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels):
265+
//
266+
// newLabels, err := GetConflictFreeLabels(newConfig.Labels)
267+
// if err != nil {
268+
// return err
269+
// }
270+
// newConfig.Labels = newLabels
271+
//
272+
if _, err := GetConflictFreeLabels(newConfig.Labels); err != nil {
273+
logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err)
274+
}
275+
235276
reload(newConfig)
236277
return nil
237278
}

‎docs/deprecated.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ see [Feature Deprecation Policy](index.md#feature-deprecation-policy).
2626

2727
The daemon is moved to a separate binary (`dockerd`), and should be used instead.
2828

29+
### Duplicate keys with conflicting values in engine labels
30+
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/)**
31+
32+
**Target For Removal In Release: v1.16**
33+
34+
Duplicate keys with conflicting values have been deprecated. A warning is displayed
35+
in the output, and an error will be returned in the future.
36+
2937
### Three argument form in `docker import`
3038
**Deprecated In Release: [v0.6.7](https://github.com/docker/docker/releases/tag/v0.6.7)**
3139

‎integration-cli/docker_cli_info_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,15 @@ func (s *DockerDaemonSuite) TestRegistryMirrors(c *check.C) {
219219
c.Assert(out, checker.Contains, fmt.Sprintf(" %s", registryMirror1))
220220
c.Assert(out, checker.Contains, fmt.Sprintf(" %s", registryMirror2))
221221
}
222+
223+
// Test case for #24392
224+
func (s *DockerDaemonSuite) TestInfoLabels(c *check.C) {
225+
testRequires(c, SameHostDaemon, DaemonIsLinux)
226+
227+
err := s.d.Start("--label", `test.empty=`, "--label", `test.empty=`, "--label", `test.label="1"`, "--label", `test.label="2"`)
228+
c.Assert(err, checker.IsNil)
229+
230+
out, err := s.d.Cmd("info")
231+
c.Assert(err, checker.IsNil)
232+
c.Assert(out, checker.Contains, "WARNING: labels with duplicate keys and conflicting values have been deprecated")
233+
}

0 commit comments

Comments
 (0)