Validate names in BeforeCreate
This commit is contained in:
		| @@ -19,6 +19,7 @@ package rest | ||||
| import ( | ||||
| 	"k8s.io/kubernetes/pkg/api" | ||||
| 	"k8s.io/kubernetes/pkg/api/errors" | ||||
| 	"k8s.io/kubernetes/pkg/api/validation" | ||||
| 	"k8s.io/kubernetes/pkg/runtime" | ||||
| 	"k8s.io/kubernetes/pkg/util/fielderrors" | ||||
| ) | ||||
| @@ -68,6 +69,14 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Obje | ||||
| 	if errs := strategy.Validate(ctx, obj); len(errs) > 0 { | ||||
| 		return errors.NewInvalid(kind, objectMeta.Name, errs) | ||||
| 	} | ||||
|  | ||||
| 	// Custom validation (including name validation) passed | ||||
| 	// Now run common validation on object meta | ||||
| 	// Do this *after* custom validation so that specific error messages are shown whenever possible | ||||
| 	if errs := validation.ValidateObjectMeta(objectMeta, strategy.NamespaceScoped(), validation.ValidatePathSegmentName); len(errs) > 0 { | ||||
| 		return errors.NewInvalid(kind, objectMeta.Name, errs) | ||||
| 	} | ||||
|  | ||||
| 	return nil | ||||
| } | ||||
|  | ||||
|   | ||||
| @@ -27,6 +27,7 @@ import ( | ||||
| 	"k8s.io/kubernetes/pkg/api/errors" | ||||
| 	"k8s.io/kubernetes/pkg/api/rest" | ||||
| 	"k8s.io/kubernetes/pkg/api/unversioned" | ||||
| 	"k8s.io/kubernetes/pkg/api/validation" | ||||
| 	"k8s.io/kubernetes/pkg/conversion" | ||||
| 	"k8s.io/kubernetes/pkg/fields" | ||||
| 	"k8s.io/kubernetes/pkg/labels" | ||||
| @@ -153,6 +154,7 @@ func (t *Tester) TestCreate(valid runtime.Object, setFn SetFunc, getFn GetFunc, | ||||
| 		t.testCreateRejectsMismatchedNamespace(copyOrDie(valid)) | ||||
| 	} | ||||
| 	t.testCreateInvokesValidation(invalid...) | ||||
| 	t.testCreateValidatesNames(copyOrDie(valid)) | ||||
| } | ||||
|  | ||||
| // Test updating an object. | ||||
| @@ -346,6 +348,32 @@ func (t *Tester) testCreateIgnoresMismatchedNamespace(valid runtime.Object) { | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func (t *Tester) testCreateValidatesNames(valid runtime.Object) { | ||||
| 	for _, invalidName := range validation.NameMayNotBe { | ||||
| 		objCopy := copyOrDie(valid) | ||||
| 		objCopyMeta := t.getObjectMetaOrFail(objCopy) | ||||
| 		objCopyMeta.Name = invalidName | ||||
|  | ||||
| 		ctx := t.TestContext() | ||||
| 		_, err := t.storage.(rest.Creater).Create(ctx, objCopy) | ||||
| 		if !errors.IsInvalid(err) { | ||||
| 			t.Errorf("%s: Expected to get an invalid resource error, got %v", invalidName, err) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	for _, invalidSuffix := range validation.NameMayNotContain { | ||||
| 		objCopy := copyOrDie(valid) | ||||
| 		objCopyMeta := t.getObjectMetaOrFail(objCopy) | ||||
| 		objCopyMeta.Name += invalidSuffix | ||||
|  | ||||
| 		ctx := t.TestContext() | ||||
| 		_, err := t.storage.(rest.Creater).Create(ctx, objCopy) | ||||
| 		if !errors.IsInvalid(err) { | ||||
| 			t.Errorf("%s: Expected to get an invalid resource error, got %v", invalidSuffix, err) | ||||
| 		} | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func (t *Tester) testCreateInvokesValidation(invalid ...runtime.Object) { | ||||
| 	for i, obj := range invalid { | ||||
| 		ctx := t.TestContext() | ||||
|   | ||||
							
								
								
									
										48
									
								
								pkg/api/validation/name.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										48
									
								
								pkg/api/validation/name.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,48 @@ | ||||
| /* | ||||
| Copyright 2015 The Kubernetes Authors All rights reserved. | ||||
|  | ||||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||||
| you may not use this file except in compliance with the License. | ||||
| You may obtain a copy of the License at | ||||
|  | ||||
|     http://www.apache.org/licenses/LICENSE-2.0 | ||||
|  | ||||
| Unless required by applicable law or agreed to in writing, software | ||||
| distributed under the License is distributed on an "AS IS" BASIS, | ||||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||
| See the License for the specific language governing permissions and | ||||
| limitations under the License. | ||||
| */ | ||||
|  | ||||
| package validation | ||||
|  | ||||
| import ( | ||||
| 	"fmt" | ||||
| 	"strings" | ||||
| ) | ||||
|  | ||||
| // NameMayNotBe specifies strings that cannot be used as names specified as path segments (like the REST API or etcd store) | ||||
| var NameMayNotBe = []string{".", ".."} | ||||
|  | ||||
| // NameMayNotContain specifies substrings that cannot be used in names specified as path segments (like the REST API or etcd store) | ||||
| var NameMayNotContain = []string{"/", "%"} | ||||
|  | ||||
| // ValidatePathSegmentName validates the name can be used as a path segment | ||||
| func ValidatePathSegmentName(name string, prefix bool) (bool, string) { | ||||
| 	// Only check for exact matches if this is the full name (not a prefix) | ||||
| 	if prefix == false { | ||||
| 		for _, illegalName := range NameMayNotBe { | ||||
| 			if name == illegalName { | ||||
| 				return false, fmt.Sprintf(`name may not be %q`, illegalName) | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	for _, illegalContent := range NameMayNotContain { | ||||
| 		if strings.Contains(name, illegalContent) { | ||||
| 			return false, fmt.Sprintf(`name may not contain %q`, illegalContent) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	return true, "" | ||||
| } | ||||
							
								
								
									
										149
									
								
								pkg/api/validation/name_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										149
									
								
								pkg/api/validation/name_test.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,149 @@ | ||||
| /* | ||||
| Copyright 2015 The Kubernetes Authors All rights reserved. | ||||
|  | ||||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||||
| you may not use this file except in compliance with the License. | ||||
| You may obtain a copy of the License at | ||||
|  | ||||
|     http://www.apache.org/licenses/LICENSE-2.0 | ||||
|  | ||||
| Unless required by applicable law or agreed to in writing, software | ||||
| distributed under the License is distributed on an "AS IS" BASIS, | ||||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||
| See the License for the specific language governing permissions and | ||||
| limitations under the License. | ||||
| */ | ||||
|  | ||||
| package validation | ||||
|  | ||||
| import ( | ||||
| 	"strings" | ||||
| 	"testing" | ||||
| ) | ||||
|  | ||||
| func TestValidatePathSegmentName(t *testing.T) { | ||||
| 	testcases := map[string]struct { | ||||
| 		Name        string | ||||
| 		Prefix      bool | ||||
| 		ExpectedOK  bool | ||||
| 		ExpectedMsg string | ||||
| 	}{ | ||||
| 		"empty": { | ||||
| 			Name:        "", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
| 		"empty,prefix": { | ||||
| 			Name:        "", | ||||
| 			Prefix:      true, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
|  | ||||
| 		"valid": { | ||||
| 			Name:        "foo.bar.baz", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
| 		"valid,prefix": { | ||||
| 			Name:        "foo.bar.baz", | ||||
| 			Prefix:      true, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
|  | ||||
| 		// Make sure mixed case, non DNS subdomain characters are tolerated | ||||
| 		"valid complex": { | ||||
| 			Name:        "sha256:ABCDEF012345@ABCDEF012345", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
| 		// Make sure non-ascii characters are tolerated | ||||
| 		"valid extended charset": { | ||||
| 			Name:        "Iñtërnâtiônàlizætiøn", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
|  | ||||
| 		"dot": { | ||||
| 			Name:        ".", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  false, | ||||
| 			ExpectedMsg: ".", | ||||
| 		}, | ||||
| 		"dot leading": { | ||||
| 			Name:        ".test", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
| 		"dot,prefix": { | ||||
| 			Name:        ".", | ||||
| 			Prefix:      true, | ||||
| 			ExpectedOK:  true, // allowed because a suffix could make it valid | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
|  | ||||
| 		"dot dot": { | ||||
| 			Name:        "..", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  false, | ||||
| 			ExpectedMsg: "..", | ||||
| 		}, | ||||
| 		"dot dot leading": { | ||||
| 			Name:        "..test", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  true, | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
| 		"dot dot,prefix": { | ||||
| 			Name:        "..", | ||||
| 			Prefix:      true, | ||||
| 			ExpectedOK:  true, // allowed because a suffix could make it valid | ||||
| 			ExpectedMsg: "", | ||||
| 		}, | ||||
|  | ||||
| 		"slash": { | ||||
| 			Name:        "foo/bar", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  false, | ||||
| 			ExpectedMsg: "/", | ||||
| 		}, | ||||
| 		"slash,prefix": { | ||||
| 			Name:        "foo/bar", | ||||
| 			Prefix:      true, | ||||
| 			ExpectedOK:  false, | ||||
| 			ExpectedMsg: "/", | ||||
| 		}, | ||||
|  | ||||
| 		"percent": { | ||||
| 			Name:        "foo%bar", | ||||
| 			Prefix:      false, | ||||
| 			ExpectedOK:  false, | ||||
| 			ExpectedMsg: "%", | ||||
| 		}, | ||||
| 		"percent,prefix": { | ||||
| 			Name:        "foo%bar", | ||||
| 			Prefix:      true, | ||||
| 			ExpectedOK:  false, | ||||
| 			ExpectedMsg: "%", | ||||
| 		}, | ||||
| 	} | ||||
|  | ||||
| 	for k, tc := range testcases { | ||||
| 		ok, msg := ValidatePathSegmentName(tc.Name, tc.Prefix) | ||||
| 		if ok != tc.ExpectedOK { | ||||
| 			t.Errorf("%s: expected ok=%v, got %v", k, tc.ExpectedOK, ok) | ||||
| 		} | ||||
| 		if len(tc.ExpectedMsg) == 0 && len(msg) > 0 { | ||||
| 			t.Errorf("%s: expected no message, got %v", k, msg) | ||||
| 		} | ||||
| 		if len(tc.ExpectedMsg) > 0 && !strings.Contains(msg, tc.ExpectedMsg) { | ||||
| 			t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msg) | ||||
| 		} | ||||
| 	} | ||||
| } | ||||
| @@ -26,6 +26,7 @@ import ( | ||||
| 	etcderr "k8s.io/kubernetes/pkg/api/errors/etcd" | ||||
| 	"k8s.io/kubernetes/pkg/api/rest" | ||||
| 	"k8s.io/kubernetes/pkg/api/unversioned" | ||||
| 	"k8s.io/kubernetes/pkg/api/validation" | ||||
| 	"k8s.io/kubernetes/pkg/fields" | ||||
| 	"k8s.io/kubernetes/pkg/labels" | ||||
| 	"k8s.io/kubernetes/pkg/registry/generic" | ||||
| @@ -128,10 +129,25 @@ func NamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, erro | ||||
| 	if len(name) == 0 { | ||||
| 		return "", kubeerr.NewBadRequest("Name parameter required.") | ||||
| 	} | ||||
| 	if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { | ||||
| 		return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg)) | ||||
| 	} | ||||
| 	key = key + "/" + name | ||||
| 	return key, nil | ||||
| } | ||||
|  | ||||
| // NoNamespaceKeyFunc is the default function for constructing etcd paths to a resource relative to prefix without a namespace | ||||
| func NoNamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, error) { | ||||
| 	if len(name) == 0 { | ||||
| 		return "", kubeerr.NewBadRequest("Name parameter required.") | ||||
| 	} | ||||
| 	if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { | ||||
| 		return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg)) | ||||
| 	} | ||||
| 	key := prefix + "/" + name | ||||
| 	return key, nil | ||||
| } | ||||
|  | ||||
| // New implements RESTStorage | ||||
| func (e *Etcd) New() runtime.Object { | ||||
| 	return e.NewFunc() | ||||
|   | ||||
| @@ -18,7 +18,6 @@ package etcd | ||||
|  | ||||
| import ( | ||||
| 	"fmt" | ||||
| 	"path" | ||||
|  | ||||
| 	"k8s.io/kubernetes/pkg/api" | ||||
| 	apierrors "k8s.io/kubernetes/pkg/api/errors" | ||||
| @@ -58,7 +57,7 @@ func NewREST(s storage.Interface) (*REST, *StatusREST, *FinalizeREST) { | ||||
| 			return prefix | ||||
| 		}, | ||||
| 		KeyFunc: func(ctx api.Context, name string) (string, error) { | ||||
| 			return path.Join(prefix, name), nil | ||||
| 			return etcdgeneric.NoNamespaceKeyFunc(ctx, prefix, name) | ||||
| 		}, | ||||
| 		ObjectNameFunc: func(obj runtime.Object) (string, error) { | ||||
| 			return obj.(*api.Namespace).Name, nil | ||||
|   | ||||
| @@ -75,7 +75,7 @@ func NewREST(s storage.Interface, useCacher bool, connection client.ConnectionIn | ||||
| 			return prefix | ||||
| 		}, | ||||
| 		KeyFunc: func(ctx api.Context, name string) (string, error) { | ||||
| 			return prefix + "/" + name, nil | ||||
| 			return etcdgeneric.NoNamespaceKeyFunc(ctx, prefix, name) | ||||
| 		}, | ||||
| 		ObjectNameFunc: func(obj runtime.Object) (string, error) { | ||||
| 			return obj.(*api.Node).Name, nil | ||||
|   | ||||
| @@ -17,8 +17,6 @@ limitations under the License. | ||||
| package etcd | ||||
|  | ||||
| import ( | ||||
| 	"path" | ||||
|  | ||||
| 	"k8s.io/kubernetes/pkg/api" | ||||
| 	"k8s.io/kubernetes/pkg/fields" | ||||
| 	"k8s.io/kubernetes/pkg/labels" | ||||
| @@ -43,7 +41,7 @@ func NewREST(s storage.Interface) (*REST, *StatusREST) { | ||||
| 			return prefix | ||||
| 		}, | ||||
| 		KeyFunc: func(ctx api.Context, name string) (string, error) { | ||||
| 			return path.Join(prefix, name), nil | ||||
| 			return etcdgeneric.NoNamespaceKeyFunc(ctx, prefix, name) | ||||
| 		}, | ||||
| 		ObjectNameFunc: func(obj runtime.Object) (string, error) { | ||||
| 			return obj.(*api.PersistentVolume).Name, nil | ||||
|   | ||||
| @@ -17,10 +17,12 @@ limitations under the License. | ||||
| package storage | ||||
|  | ||||
| import ( | ||||
| 	"fmt" | ||||
| 	"strconv" | ||||
|  | ||||
| 	"k8s.io/kubernetes/pkg/api/errors" | ||||
| 	"k8s.io/kubernetes/pkg/api/meta" | ||||
| 	"k8s.io/kubernetes/pkg/api/validation" | ||||
| 	"k8s.io/kubernetes/pkg/runtime" | ||||
| 	"k8s.io/kubernetes/pkg/util/fielderrors" | ||||
| ) | ||||
| @@ -56,6 +58,10 @@ func NamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) { | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	} | ||||
| 	name := meta.Name() | ||||
| 	if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { | ||||
| 		return "", fmt.Errorf("invalid name: %v", msg) | ||||
| 	} | ||||
| 	return prefix + "/" + meta.Namespace() + "/" + meta.Name(), nil | ||||
| } | ||||
|  | ||||
| @@ -64,5 +70,9 @@ func NoNamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) { | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	} | ||||
| 	name := meta.Name() | ||||
| 	if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { | ||||
| 		return "", fmt.Errorf("invalid name: %v", msg) | ||||
| 	} | ||||
| 	return prefix + "/" + meta.Name(), nil | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Jordan Liggitt
					Jordan Liggitt