Browse code

enforce reserve internal labels.

The namespaces com.docker.*, io.docker.*, org.dockerproject.*
have been documented to be reserved for Docker's internal use.

Co-Authored-By: Sebastiaan van Stijn <thaJeztah@users.noreply.github.com>
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>

Jintao Zhang authored on 2020/02/12 13:03:35
Showing 5 changed files
... ...
@@ -326,19 +326,6 @@ func (cli *DaemonCli) reloadConfig() {
326 326
 		}
327 327
 		cli.authzMiddleware.SetPlugins(c.AuthorizationPlugins)
328 328
 
329
-		// The namespaces com.docker.*, io.docker.*, org.dockerproject.* have been documented
330
-		// to be reserved for Docker's internal use, but this was never enforced.  Allowing
331
-		// configured labels to use these namespaces are deprecated for 18.05.
332
-		//
333
-		// The following will check the usage of such labels, and report a warning for deprecation.
334
-		//
335
-		// TODO: At the next stable release, the validation should be folded into the other
336
-		// configuration validation functions and an error will be returned instead, and this
337
-		// block should be deleted.
338
-		if err := config.ValidateReservedNamespaceLabels(c.Labels); err != nil {
339
-			logrus.Warnf("Configured labels using reserved namespaces is deprecated: %s", err)
340
-		}
341
-
342 329
 		if err := cli.d.Reload(c); err != nil {
343 330
 			logrus.Errorf("Error reconfiguring the daemon: %v", err)
344 331
 			return
... ...
@@ -446,18 +433,6 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
446 446
 	if err != nil {
447 447
 		return nil, err
448 448
 	}
449
-	// The namespaces com.docker.*, io.docker.*, org.dockerproject.* have been documented
450
-	// to be reserved for Docker's internal use, but this was never enforced.  Allowing
451
-	// configured labels to use these namespaces are deprecated for 18.05.
452
-	//
453
-	// The following will check the usage of such labels, and report a warning for deprecation.
454
-	//
455
-	// TODO: At the next stable release, the validation should be folded into the other
456
-	// configuration validation functions and an error will be returned instead, and this
457
-	// block should be deleted.
458
-	if err := config.ValidateReservedNamespaceLabels(newLabels); err != nil {
459
-		logrus.Warnf("Configured labels using reserved namespaces is deprecated: %s", err)
460
-	}
461 449
 	conf.Labels = newLabels
462 450
 
463 451
 	// Regardless of whether the user sets it to true or false, if they
... ...
@@ -308,25 +308,6 @@ func GetConflictFreeLabels(labels []string) ([]string, error) {
308 308
 	return newLabels, nil
309 309
 }
310 310
 
311
-// ValidateReservedNamespaceLabels errors if the reserved namespaces com.docker.*,
312
-// io.docker.*, org.dockerproject.* are used in a configured engine label.
313
-//
314
-// TODO: This is a separate function because we need to warn users first of the
315
-// deprecation.  When we return an error, this logic can be added to Validate
316
-// or GetConflictFreeLabels instead of being here.
317
-func ValidateReservedNamespaceLabels(labels []string) error {
318
-	for _, label := range labels {
319
-		lowered := strings.ToLower(label)
320
-		if strings.HasPrefix(lowered, "com.docker.") || strings.HasPrefix(lowered, "io.docker.") ||
321
-			strings.HasPrefix(lowered, "org.dockerproject.") {
322
-			return fmt.Errorf(
323
-				"label %s not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for Docker's internal use",
324
-				label)
325
-		}
326
-	}
327
-	return nil
328
-}
329
-
330 311
 // Reload reads the configuration in the host and reloads the daemon and server.
331 312
 func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error {
332 313
 	logrus.Infof("Got signal to reload configuration, reloading from: %s", configFile)
... ...
@@ -188,40 +188,6 @@ func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
188 188
 	}
189 189
 }
190 190
 
191
-func TestValidateReservedNamespaceLabels(t *testing.T) {
192
-	for _, validLabels := range [][]string{
193
-		nil, // no error if there are no labels
194
-		{ // no error if there aren't any reserved namespace labels
195
-			"hello=world",
196
-			"label=me",
197
-		},
198
-		{ // only reserved namespaces that end with a dot are invalid
199
-			"com.dockerpsychnotreserved.label=value",
200
-			"io.dockerproject.not=reserved",
201
-			"org.docker.not=reserved",
202
-		},
203
-	} {
204
-		assert.Check(t, ValidateReservedNamespaceLabels(validLabels))
205
-	}
206
-
207
-	for _, invalidLabel := range []string{
208
-		"com.docker.feature=enabled",
209
-		"io.docker.configuration=0",
210
-		"org.dockerproject.setting=on",
211
-		// casing doesn't matter
212
-		"COM.docker.feature=enabled",
213
-		"io.DOCKER.CONFIGURATION=0",
214
-		"Org.Dockerproject.Setting=on",
215
-	} {
216
-		err := ValidateReservedNamespaceLabels([]string{
217
-			"valid=label",
218
-			invalidLabel,
219
-			"another=valid",
220
-		})
221
-		assert.Check(t, is.ErrorContains(err, invalidLabel))
222
-	}
223
-}
224
-
225 191
 func TestValidateConfigurationErrors(t *testing.T) {
226 192
 	intPtr := func(i int) *int { return &i }
227 193
 
... ...
@@ -254,12 +254,23 @@ func validateDomain(val string) (string, error) {
254 254
 	return "", fmt.Errorf("%s is not a valid domain", val)
255 255
 }
256 256
 
257
-// ValidateLabel validates that the specified string is a valid label, and returns it.
257
+// ValidateLabel validates that the specified string is a valid label,
258
+// it does not use the reserved namespaces com.docker.*, io.docker.*, org.dockerproject.*
259
+// and returns it.
258 260
 // Labels are in the form on key=value.
259 261
 func ValidateLabel(val string) (string, error) {
260 262
 	if strings.Count(val, "=") < 1 {
261 263
 		return "", fmt.Errorf("bad attribute format: %s", val)
262 264
 	}
265
+
266
+	lowered := strings.ToLower(val)
267
+	if strings.HasPrefix(lowered, "com.docker.") || strings.HasPrefix(lowered, "io.docker.") ||
268
+		strings.HasPrefix(lowered, "org.dockerproject.") {
269
+		return "", fmt.Errorf(
270
+			"label %s is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
271
+			val)
272
+	}
273
+
263 274
 	return val, nil
264 275
 }
265 276
 
... ...
@@ -4,6 +4,9 @@ import (
4 4
 	"fmt"
5 5
 	"strings"
6 6
 	"testing"
7
+
8
+	"gotest.tools/v3/assert"
9
+	is "gotest.tools/v3/assert/cmp"
7 10
 )
8 11
 
9 12
 func TestValidateIPAddress(t *testing.T) {
... ...
@@ -174,19 +177,94 @@ func TestValidateDNSSearch(t *testing.T) {
174 174
 }
175 175
 
176 176
 func TestValidateLabel(t *testing.T) {
177
-	if _, err := ValidateLabel("label"); err == nil || err.Error() != "bad attribute format: label" {
178
-		t.Fatalf("Expected an error [bad attribute format: label], go %v", err)
179
-	}
180
-	if actual, err := ValidateLabel("key1=value1"); err != nil || actual != "key1=value1" {
181
-		t.Fatalf("Expected [key1=value1], got [%v,%v]", actual, err)
182
-	}
183
-	// Validate it's working with more than one =
184
-	if actual, err := ValidateLabel("key1=value1=value2"); err != nil {
185
-		t.Fatalf("Expected [key1=value1=value2], got [%v,%v]", actual, err)
177
+	testCases := []struct {
178
+		name           string
179
+		label          string
180
+		expectedResult string
181
+		expectedErr    string
182
+	}{
183
+		{
184
+			name:        "lable with bad attribute format",
185
+			label:       "label",
186
+			expectedErr: "bad attribute format: label",
187
+		},
188
+		{
189
+			name:           "label with general format",
190
+			label:          "key1=value1",
191
+			expectedResult: "key1=value1",
192
+		},
193
+		{
194
+			name:           "label with more than one =",
195
+			label:          "key1=value1=value2",
196
+			expectedResult: "key1=value1=value2",
197
+		},
198
+		{
199
+			name:           "label with one more",
200
+			label:          "key1=value1=value2=value3",
201
+			expectedResult: "key1=value1=value2=value3",
202
+		},
203
+		{
204
+			name:           "label with no reserved com.docker.*",
205
+			label:          "com.dockerpsychnotreserved.label=value",
206
+			expectedResult: "com.dockerpsychnotreserved.label=value",
207
+		},
208
+		{
209
+			name:           "label with no reserved io.docker.*",
210
+			label:          "io.dockerproject.not=reserved",
211
+			expectedResult: "io.dockerproject.not=reserved",
212
+		},
213
+		{
214
+			name:           "label with no reserved org.dockerproject.*",
215
+			label:          "org.docker.not=reserved",
216
+			expectedResult: "org.docker.not=reserved",
217
+		},
218
+		{
219
+			name:        "label with reserved com.docker.*",
220
+			label:       "com.docker.feature=enabled",
221
+			expectedErr: "label com.docker.feature=enabled is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
222
+		},
223
+		{
224
+			name:        "label with reserved upcase com.docker.* ",
225
+			label:       "COM.docker.feature=enabled",
226
+			expectedErr: "label COM.docker.feature=enabled is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
227
+		},
228
+		{
229
+			name:        "label with reserved io.docker.*",
230
+			label:       "io.docker.configuration=0",
231
+			expectedErr: "label io.docker.configuration=0 is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
232
+		},
233
+		{
234
+			name:        "label with reserved upcase io.docker.*",
235
+			label:       "io.DOCKER.CONFIGURATion=0",
236
+			expectedErr: "label io.DOCKER.CONFIGURATion=0 is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
237
+		},
238
+		{
239
+			name:        "label with reserved org.dockerproject.*",
240
+			label:       "org.dockerproject.setting=on",
241
+			expectedErr: "label org.dockerproject.setting=on is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
242
+		},
243
+		{
244
+			name:        "label with reserved upcase org.dockerproject.*",
245
+			label:       "Org.Dockerproject.Setting=on",
246
+			expectedErr: "label Org.Dockerproject.Setting=on is not allowed: the namespaces com.docker.*, io.docker.*, and org.dockerproject.* are reserved for internal use",
247
+		},
186 248
 	}
187
-	// Validate it's working with one more
188
-	if actual, err := ValidateLabel("key1=value1=value2=value3"); err != nil {
189
-		t.Fatalf("Expected [key1=value1=value2=value2], got [%v,%v]", actual, err)
249
+
250
+	for _, testCase := range testCases {
251
+		testCase := testCase
252
+		t.Run(testCase.name, func(t *testing.T) {
253
+			result, err := ValidateLabel(testCase.label)
254
+
255
+			if testCase.expectedErr != "" {
256
+				assert.Error(t, err, testCase.expectedErr)
257
+			} else {
258
+				assert.NilError(t, err)
259
+			}
260
+			if testCase.expectedResult != "" {
261
+				assert.Check(t, is.Equal(result, testCase.expectedResult))
262
+			}
263
+		})
264
+
190 265
 	}
191 266
 }
192 267