diff --git a/pkg/api/validation/name.go b/pkg/api/validation/name.go index ffd547b43cc..e36775b601d 100644 --- a/pkg/api/validation/name.go +++ b/pkg/api/validation/name.go @@ -27,14 +27,11 @@ 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) - } +// IsValidPathSegmentName validates the name can be safely encoded as a path segment +func IsValidPathSegmentName(name string) (bool, string) { + for _, illegalName := range NameMayNotBe { + if name == illegalName { + return false, fmt.Sprintf(`name may not be %q`, illegalName) } } @@ -46,3 +43,24 @@ func ValidatePathSegmentName(name string, prefix bool) (bool, string) { return true, "" } + +// IsValidPathSegmentPrefix validates the name can be used as a prefix for a name which will be encoded as a path segment +// It does not check for exact matches with disallowed names, since an arbitrary suffix might make the name valid +func IsValidPathSegmentPrefix(name string) (bool, string) { + for _, illegalContent := range NameMayNotContain { + if strings.Contains(name, illegalContent) { + return false, fmt.Sprintf(`name may not contain %q`, illegalContent) + } + } + + return true, "" +} + +// ValidatePathSegmentName validates the name can be safely encoded as a path segment +func ValidatePathSegmentName(name string, prefix bool) (bool, string) { + if prefix { + return IsValidPathSegmentPrefix(name) + } else { + return IsValidPathSegmentName(name) + } +} diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index 28bcddc3adb..c687b23db6d 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -55,6 +55,33 @@ func validateHorizontalPodAutoscalerSpec(autoscaler extensions.HorizontalPodAuto if autoscaler.CPUUtilization != nil && autoscaler.CPUUtilization.TargetPercentage < 1 { allErrs = append(allErrs, errs.NewFieldInvalid("cpuUtilization.targetPercentage", autoscaler.CPUUtilization.TargetPercentage, `must be bigger or equal to 1`)) } + if refErrs := ValidateSubresourceReference(autoscaler.ScaleRef); len(refErrs) > 0 { + allErrs = append(allErrs, refErrs.Prefix("scaleRef")...) + } else if autoscaler.ScaleRef.Subresource != "scale" { + allErrs = append(allErrs, errs.NewFieldValueNotSupported("scaleRef.subresource", autoscaler.ScaleRef.Subresource, []string{"scale"})) + } + return allErrs +} + +func ValidateSubresourceReference(ref extensions.SubresourceReference) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + if len(ref.Kind) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("kind")) + } else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Kind); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("kind", ref.Kind, msg)) + } + + if len(ref.Name) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("name")) + } else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Name); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("name", ref.Name, msg)) + } + + if len(ref.Subresource) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("subresource")) + } else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Subresource); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("subresource", ref.Subresource, msg)) + } return allErrs } diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index c7e2ac04184..246e24d29eb 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -36,6 +36,8 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) { }, Spec: extensions.HorizontalPodAutoscalerSpec{ ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", Subresource: "scale", }, MinReplicas: newInt(1), @@ -50,6 +52,8 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) { }, Spec: extensions.HorizontalPodAutoscalerSpec{ ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", Subresource: "scale", }, MinReplicas: newInt(1), @@ -67,6 +71,90 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) { horizontalPodAutoscaler extensions.HorizontalPodAutoscaler msg string }{ + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Name: "myrc", Subresource: "scale"}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.kind: required", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Kind: "..", Name: "myrc", Subresource: "scale"}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.kind: invalid", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Kind: "ReplicationController", Subresource: "scale"}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.name: required", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Kind: "ReplicationController", Name: "..", Subresource: "scale"}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.name: invalid", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Kind: "ReplicationController", Name: "myrc", Subresource: ""}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.subresource: required", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Kind: "ReplicationController", Name: "myrc", Subresource: ".."}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.subresource: invalid", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{Name: "myautoscaler", Namespace: api.NamespaceDefault}, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{Kind: "ReplicationController", Name: "myrc", Subresource: "randomsubresource"}, + MinReplicas: newInt(1), + MaxReplicas: 5, + CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70}, + }, + }, + msg: "scaleRef.subresource: unsupported", + }, { horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ ObjectMeta: api.ObjectMeta{ diff --git a/pkg/client/unversioned/request.go b/pkg/client/unversioned/request.go index 1df7fcebf9c..9d9873a9e11 100644 --- a/pkg/client/unversioned/request.go +++ b/pkg/client/unversioned/request.go @@ -32,6 +32,7 @@ import ( "github.com/golang/glog" "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/api/unversioned" + "k8s.io/kubernetes/pkg/api/validation" "k8s.io/kubernetes/pkg/client/metrics" "k8s.io/kubernetes/pkg/conversion/queryparams" "k8s.io/kubernetes/pkg/fields" @@ -149,6 +150,10 @@ func (r *Request) Resource(resource string) *Request { r.err = fmt.Errorf("resource already set to %q, cannot change to %q", r.resource, resource) return r } + if ok, msg := validation.IsValidPathSegmentName(resource); !ok { + r.err = fmt.Errorf("invalid resource %q: %s", resource, msg) + return r + } r.resource = resource return r } @@ -164,6 +169,12 @@ func (r *Request) SubResource(subresources ...string) *Request { r.err = fmt.Errorf("subresource already set to %q, cannot change to %q", r.resource, subresource) return r } + for _, s := range subresources { + if ok, msg := validation.IsValidPathSegmentName(s); !ok { + r.err = fmt.Errorf("invalid subresource %q: %s", s, msg) + return r + } + } r.subresource = subresource return r } @@ -181,6 +192,10 @@ func (r *Request) Name(resourceName string) *Request { r.err = fmt.Errorf("resource name already set to %q, cannot change to %q", r.resourceName, resourceName) return r } + if ok, msg := validation.IsValidPathSegmentName(resourceName); !ok { + r.err = fmt.Errorf("invalid resource name %q: %s", resourceName, msg) + return r + } r.resourceName = resourceName return r } @@ -194,6 +209,10 @@ func (r *Request) Namespace(namespace string) *Request { r.err = fmt.Errorf("namespace already set to %q, cannot change to %q", r.namespace, namespace) return r } + if ok, msg := validation.IsValidPathSegmentName(namespace); !ok { + r.err = fmt.Errorf("invalid namespace %q: %s", namespace, msg) + return r + } r.namespaceSet = true r.namespace = namespace return r diff --git a/pkg/client/unversioned/request_test.go b/pkg/client/unversioned/request_test.go index 77573b8bc0a..7b6d9aaf65c 100644 --- a/pkg/client/unversioned/request_test.go +++ b/pkg/client/unversioned/request_test.go @@ -144,6 +144,25 @@ func TestRequestSetTwiceError(t *testing.T) { } } +func TestInvalidSegments(t *testing.T) { + invalidSegments := []string{".", "..", "test/segment", "test%2bsegment"} + setters := map[string]func(string, *Request){ + "namespace": func(s string, r *Request) { r.Namespace(s) }, + "resource": func(s string, r *Request) { r.Resource(s) }, + "name": func(s string, r *Request) { r.Name(s) }, + "subresource": func(s string, r *Request) { r.SubResource(s) }, + } + for _, invalidSegment := range invalidSegments { + for setterName, setter := range setters { + r := &Request{} + setter(invalidSegment, r) + if r.err == nil { + t.Errorf("%s: %s: expected error, got none", setterName, invalidSegment) + } + } + } +} + func TestRequestParam(t *testing.T) { r := (&Request{}).Param("foo", "a") if !reflect.DeepEqual(r.params, url.Values{"foo": []string{"a"}}) { diff --git a/pkg/registry/generic/etcd/etcd.go b/pkg/registry/generic/etcd/etcd.go index 42c994fe42f..47a7c3b2fb5 100644 --- a/pkg/registry/generic/etcd/etcd.go +++ b/pkg/registry/generic/etcd/etcd.go @@ -129,7 +129,7 @@ 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 { + if ok, msg := validation.IsValidPathSegmentName(name); !ok { return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg)) } key = key + "/" + name @@ -141,7 +141,7 @@ func NoNamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, er if len(name) == 0 { return "", kubeerr.NewBadRequest("Name parameter required.") } - if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { + if ok, msg := validation.IsValidPathSegmentName(name); !ok { return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg)) } key := prefix + "/" + name diff --git a/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go b/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go index 497e27480f2..188f9542c99 100644 --- a/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go +++ b/pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go @@ -44,6 +44,8 @@ func validNewHorizontalPodAutoscaler(name string) *extensions.HorizontalPodAutos }, Spec: extensions.HorizontalPodAutoscalerSpec{ ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", Subresource: "scale", }, MaxReplicas: 5, diff --git a/pkg/storage/util.go b/pkg/storage/util.go index 1e12684e427..c10a8ee2311 100644 --- a/pkg/storage/util.go +++ b/pkg/storage/util.go @@ -59,7 +59,7 @@ func NamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) { return "", err } name := meta.Name() - if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { + if ok, msg := validation.IsValidPathSegmentName(name); !ok { return "", fmt.Errorf("invalid name: %v", msg) } return prefix + "/" + meta.Namespace() + "/" + meta.Name(), nil @@ -71,7 +71,7 @@ func NoNamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) { return "", err } name := meta.Name() - if ok, msg := validation.ValidatePathSegmentName(name, false); !ok { + if ok, msg := validation.IsValidPathSegmentName(name); !ok { return "", fmt.Errorf("invalid name: %v", msg) } return prefix + "/" + meta.Name(), nil