diff --git a/daemon/commit.go b/daemon/commit.go index 9a75137a28..119355262f 100644 --- a/daemon/commit.go +++ b/daemon/commit.go @@ -13,15 +13,20 @@ import ( "github.com/docker/docker/api/types/events" "github.com/docker/docker/builder/dockerfile" "github.com/docker/docker/errdefs" + "github.com/docker/docker/opts" "github.com/pkg/errors" ) +type mergeOptions struct { + keepReservedLabels bool +} + // merge merges two Config, the image container configuration (defaults values), // and the user container configuration, either passed by the API or generated // by the cli. // It will mutate the specified user configuration (userConf) with the image // configuration where the user configuration is incomplete. -func merge(userConf, imageConf *containertypes.Config) error { +func merge(userConf, imageConf *containertypes.Config, options mergeOptions) error { if userConf.User == "" { userConf.User = imageConf.User } @@ -63,6 +68,9 @@ func merge(userConf, imageConf *containertypes.Config) error { userConf.Labels = map[string]string{} } for l, v := range imageConf.Labels { + if !options.keepReservedLabels && opts.IsReservedLabelNamespace(l) { + continue + } if _, ok := userConf.Labels[l]; !ok { userConf.Labels[l] = v } @@ -155,7 +163,7 @@ func (daemon *Daemon) CreateImageFromContainer(ctx context.Context, name string, if err != nil { return "", err } - if err := merge(newConfig, container.Config); err != nil { + if err := merge(newConfig, container.Config, mergeOptions{}); err != nil { return "", err } diff --git a/daemon/create.go b/daemon/create.go index 96f2ec800f..71ddc9a6c8 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -309,7 +309,7 @@ func (daemon *Daemon) generateSecurityOpt(hostConfig *containertypes.HostConfig) func (daemon *Daemon) mergeAndVerifyConfig(config *containertypes.Config, img *image.Image) error { if img != nil && img.Config != nil { - if err := merge(config, img.Config); err != nil { + if err := merge(config, img.Config, mergeOptions{keepReservedLabels: true}); err != nil { return err } } diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 85afcc41cc..522559e4a8 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -232,6 +232,7 @@ func TestMerge(t *testing.T) { ExposedPorts: portsImage, Env: []string{"VAR1=1", "VAR2=2"}, Volumes: volumesImage, + Labels: map[string]string{"com.docker.foo": "bar", "my-label": "my-value"}, } portsUser := make(nat.PortSet) @@ -245,7 +246,7 @@ func TestMerge(t *testing.T) { Volumes: volumesUser, } - if err := merge(configUser, configImage); err != nil { + if err := merge(configUser, configImage, mergeOptions{keepReservedLabels: true}); err != nil { t.Error(err) } @@ -279,22 +280,30 @@ func TestMerge(t *testing.T) { if err != nil { t.Error(err) } + + assert.DeepEqual(t, configUser.Labels, configImage.Labels) + configImage2 := &containertypes.Config{ ExposedPorts: ports, + Labels: map[string]string{"com.docker.foo": "bar", "my-label": "my-value"}, + } + configUser2 := &containertypes.Config{ + ExposedPorts: portsUser, } - if err := merge(configUser, configImage2); err != nil { + if err := merge(configUser2, configImage2, mergeOptions{}); err != nil { t.Error(err) } - if len(configUser.ExposedPorts) != 4 { + if len(configUser2.ExposedPorts) != 4 { t.Fatalf("Expected 4 ExposedPorts, 0000, 1111, 2222 and 3333, found %d", len(configUser.ExposedPorts)) } - for portSpecs := range configUser.ExposedPorts { + for portSpecs := range configUser2.ExposedPorts { if portSpecs.Port() != "0" && portSpecs.Port() != "1111" && portSpecs.Port() != "2222" && portSpecs.Port() != "3333" { t.Fatalf("Expected %q or %q or %q or %q, found %s", 0, 1111, 2222, 3333, portSpecs) } } + assert.DeepEqual(t, configUser2.Labels, map[string]string{"my-label": "my-value"}) } func TestValidateContainerIsolation(t *testing.T) { diff --git a/opts/opts.go b/opts/opts.go index 40b19c4bd9..4d0e95a552 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -337,13 +337,12 @@ func validateDomain(val string) (string, error) { // and returns it. // Labels are in the form on key=value. func ValidateLabel(val string) (string, error) { - if strings.Count(val, "=") < 1 { + kv := strings.SplitN(val, "=", 2) + if len(kv) != 2 { return "", fmt.Errorf("bad attribute format: %s", val) } - lowered := strings.ToLower(val) - if strings.HasPrefix(lowered, "com.docker.") || strings.HasPrefix(lowered, "io.docker.") || - strings.HasPrefix(lowered, "org.dockerproject.") { + if IsReservedLabelNamespace(kv[0]) { return "", fmt.Errorf( "label %s is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", val) @@ -352,6 +351,21 @@ func ValidateLabel(val string) (string, error) { return val, nil } +var reservedLabelNamespaces = []string{"com.docker", "io.docker", "org.dockerproject"} + +// IsReservedLabelNamespace checks if a given label uses a reserved namespace +// Reserved namespaces are com.docker.*, io.docker.*, and org.dockerproject.* +// (case insensitive). +func IsReservedLabelNamespace(name string) bool { + lowered := strings.ToLower(name) + for _, ns := range reservedLabelNamespaces { + if lowered == ns || strings.HasPrefix(lowered, ns+".") { + return true + } + } + return false +} + // ValidateSingleGenericResource validates that a single entry in the // generic resource list is valid. // i.e 'GPU=UID1' is valid however 'GPU:UID1' or 'UID1' isn't diff --git a/opts/opts_test.go b/opts/opts_test.go index 3f7f33a246..524c58a3a2 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -233,7 +233,7 @@ func TestValidateLabel(t *testing.T) { expectedErr string }{ { - name: "lable with bad attribute format", + name: "label with bad attribute format", label: "label", expectedErr: "bad attribute format: label", }, @@ -316,6 +316,86 @@ func TestValidateLabel(t *testing.T) { } } +func TestIsReservedLabelNamespace(t *testing.T) { + tests := []struct { + label string + expected bool + }{ + { + label: "my-label", + expected: false, + }, + { + label: "com.dockerpsychnotreserved.label", + expected: false, + }, + { + label: "io.dockerproject.not", + expected: false, + }, + { + label: "org.docker.not", + expected: false, + }, + { + label: "com.docker", + expected: true, + }, + { + label: "com.docker.", + expected: true, + }, + { + label: "com.docker.feature", + expected: true, + }, + { + label: "COM.docker.feature", + expected: true, + }, + { + label: "io.docker", + expected: true, + }, + { + label: "io.docker.", + expected: true, + }, + { + label: "io.docker.feature", + expected: true, + }, + { + label: "io.docker.feature", + expected: true, + }, + { + label: "org.dockerproject", + expected: true, + }, + { + label: "org.dockerproject.", + expected: true, + }, + { + label: "org.dockerproject.feature", + expected: true, + }, + { + label: "org.dockerproject.feature", + expected: true, + }, + } + + for _, tc := range tests { + testCase := tc + t.Run(testCase.label, func(t *testing.T) { + result := IsReservedLabelNamespace(testCase.label) + assert.Equal(t, result, testCase.expected) + }) + } +} + func logOptsValidator(val string) (string, error) { allowedKeys := map[string]string{"max-size": "1", "max-file": "2"} vals := strings.Split(val, "=")