diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 22803e22b0..5b267f03f3 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -326,19 +326,6 @@ func (cli *DaemonCli) reloadConfig() { } cli.authzMiddleware.SetPlugins(c.AuthorizationPlugins) - // The namespaces com.docker.*, io.docker.*, org.dockerproject.* have been documented - // to be reserved for Docker's internal use, but this was never enforced. Allowing - // configured labels to use these namespaces are deprecated for 18.05. - // - // The following will check the usage of such labels, and report a warning for deprecation. - // - // TODO: At the next stable release, the validation should be folded into the other - // configuration validation functions and an error will be returned instead, and this - // block should be deleted. - if err := config.ValidateReservedNamespaceLabels(c.Labels); err != nil { - logrus.Warnf("Configured labels using reserved namespaces is deprecated: %s", err) - } - if err := cli.d.Reload(c); err != nil { logrus.Errorf("Error reconfiguring the daemon: %v", err) return @@ -446,18 +433,6 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { if err != nil { return nil, err } - // The namespaces com.docker.*, io.docker.*, org.dockerproject.* have been documented - // to be reserved for Docker's internal use, but this was never enforced. Allowing - // configured labels to use these namespaces are deprecated for 18.05. - // - // The following will check the usage of such labels, and report a warning for deprecation. - // - // TODO: At the next stable release, the validation should be folded into the other - // configuration validation functions and an error will be returned instead, and this - // block should be deleted. - if err := config.ValidateReservedNamespaceLabels(newLabels); err != nil { - logrus.Warnf("Configured labels using reserved namespaces is deprecated: %s", err) - } conf.Labels = newLabels // Regardless of whether the user sets it to true or false, if they diff --git a/daemon/config/config.go b/daemon/config/config.go index b780f66106..77f8731350 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -308,25 +308,6 @@ func GetConflictFreeLabels(labels []string) ([]string, error) { return newLabels, nil } -// ValidateReservedNamespaceLabels errors if the reserved namespaces com.docker.*, -// io.docker.*, org.dockerproject.* are used in a configured engine label. -// -// TODO: This is a separate function because we need to warn users first of the -// deprecation. When we return an error, this logic can be added to Validate -// or GetConflictFreeLabels instead of being here. -func ValidateReservedNamespaceLabels(labels []string) error { - for _, label := range labels { - lowered := strings.ToLower(label) - if strings.HasPrefix(lowered, "com.docker.") || strings.HasPrefix(lowered, "io.docker.") || - strings.HasPrefix(lowered, "org.dockerproject.") { - return fmt.Errorf( - "label %s not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for Docker's internal use", - label) - } - } - return nil -} - // Reload reads the configuration in the host and reloads the daemon and server. func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error { logrus.Infof("Got signal to reload configuration, reloading from: %s", configFile) diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index 76a0119909..24d8093fc4 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -188,40 +188,6 @@ func TestFindConfigurationConflictsWithMergedValues(t *testing.T) { } } -func TestValidateReservedNamespaceLabels(t *testing.T) { - for _, validLabels := range [][]string{ - nil, // no error if there are no labels - { // no error if there aren't any reserved namespace labels - "hello=world", - "label=me", - }, - { // only reserved namespaces that end with a dot are invalid - "com.dockerpsychnotreserved.label=value", - "io.dockerproject.not=reserved", - "org.docker.not=reserved", - }, - } { - assert.Check(t, ValidateReservedNamespaceLabels(validLabels)) - } - - for _, invalidLabel := range []string{ - "com.docker.feature=enabled", - "io.docker.configuration=0", - "org.dockerproject.setting=on", - // casing doesn't matter - "COM.docker.feature=enabled", - "io.DOCKER.CONFIGURATION=0", - "Org.Dockerproject.Setting=on", - } { - err := ValidateReservedNamespaceLabels([]string{ - "valid=label", - invalidLabel, - "another=valid", - }) - assert.Check(t, is.ErrorContains(err, invalidLabel)) - } -} - func TestValidateConfigurationErrors(t *testing.T) { intPtr := func(i int) *int { return &i } diff --git a/opts/opts.go b/opts/opts.go index c3bf5cd7c1..60a093f28c 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -254,12 +254,23 @@ func validateDomain(val string) (string, error) { return "", fmt.Errorf("%s is not a valid domain", val) } -// ValidateLabel validates that the specified string is a valid label, and returns it. +// ValidateLabel validates that the specified string is a valid label, +// it does not use the reserved namespaces com.docker.*, io.docker.*, org.dockerproject.* +// and returns it. // Labels are in the form on key=value. func ValidateLabel(val string) (string, error) { if strings.Count(val, "=") < 1 { 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.") { + return "", fmt.Errorf( + "label %s is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + val) + } + return val, nil } diff --git a/opts/opts_test.go b/opts/opts_test.go index 577395edcb..2249cc1054 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -4,6 +4,9 @@ import ( "fmt" "strings" "testing" + + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) func TestValidateIPAddress(t *testing.T) { @@ -174,19 +177,94 @@ func TestValidateDNSSearch(t *testing.T) { } func TestValidateLabel(t *testing.T) { - if _, err := ValidateLabel("label"); err == nil || err.Error() != "bad attribute format: label" { - t.Fatalf("Expected an error [bad attribute format: label], go %v", err) + testCases := []struct { + name string + label string + expectedResult string + expectedErr string + }{ + { + name: "lable with bad attribute format", + label: "label", + expectedErr: "bad attribute format: label", + }, + { + name: "label with general format", + label: "key1=value1", + expectedResult: "key1=value1", + }, + { + name: "label with more than one =", + label: "key1=value1=value2", + expectedResult: "key1=value1=value2", + }, + { + name: "label with one more", + label: "key1=value1=value2=value3", + expectedResult: "key1=value1=value2=value3", + }, + { + name: "label with no reserved com.docker.*", + label: "com.dockerpsychnotreserved.label=value", + expectedResult: "com.dockerpsychnotreserved.label=value", + }, + { + name: "label with no reserved io.docker.*", + label: "io.dockerproject.not=reserved", + expectedResult: "io.dockerproject.not=reserved", + }, + { + name: "label with no reserved org.dockerproject.*", + label: "org.docker.not=reserved", + expectedResult: "org.docker.not=reserved", + }, + { + name: "label with reserved com.docker.*", + label: "com.docker.feature=enabled", + expectedErr: "label com.docker.feature=enabled is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + }, + { + name: "label with reserved upcase com.docker.* ", + label: "COM.docker.feature=enabled", + expectedErr: "label COM.docker.feature=enabled is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + }, + { + name: "label with reserved io.docker.*", + label: "io.docker.configuration=0", + expectedErr: "label io.docker.configuration=0 is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + }, + { + name: "label with reserved upcase io.docker.*", + label: "io.DOCKER.CONFIGURATion=0", + expectedErr: "label io.DOCKER.CONFIGURATion=0 is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + }, + { + name: "label with reserved org.dockerproject.*", + label: "org.dockerproject.setting=on", + expectedErr: "label org.dockerproject.setting=on is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + }, + { + name: "label with reserved upcase org.dockerproject.*", + label: "Org.Dockerproject.Setting=on", + expectedErr: "label Org.Dockerproject.Setting=on is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use", + }, } - if actual, err := ValidateLabel("key1=value1"); err != nil || actual != "key1=value1" { - t.Fatalf("Expected [key1=value1], got [%v,%v]", actual, err) - } - // Validate it's working with more than one = - if actual, err := ValidateLabel("key1=value1=value2"); err != nil { - t.Fatalf("Expected [key1=value1=value2], got [%v,%v]", actual, err) - } - // Validate it's working with one more - if actual, err := ValidateLabel("key1=value1=value2=value3"); err != nil { - t.Fatalf("Expected [key1=value1=value2=value2], got [%v,%v]", actual, err) + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + result, err := ValidateLabel(testCase.label) + + if testCase.expectedErr != "" { + assert.Error(t, err, testCase.expectedErr) + } else { + assert.NilError(t, err) + } + if testCase.expectedResult != "" { + assert.Check(t, is.Equal(result, testCase.expectedResult)) + } + }) + } }