From 5b7edc1aa673a1e69835c48087bfa1a6ee42564b Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 12 Aug 2016 15:38:22 -0400 Subject: [PATCH] Validate involvedObject.Namespace matches event.Namespace --- pkg/api/validation/events.go | 57 +++++++++-- pkg/api/validation/events_test.go | 162 +++++++++++++++++++++++++++++- 2 files changed, 207 insertions(+), 12 deletions(-) diff --git a/pkg/api/validation/events.go b/pkg/api/validation/events.go index 754cf883001..589fe919f2d 100644 --- a/pkg/api/validation/events.go +++ b/pkg/api/validation/events.go @@ -17,7 +17,13 @@ limitations under the License. package validation import ( + "fmt" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/meta" + "k8s.io/kubernetes/pkg/api/unversioned" + apiutil "k8s.io/kubernetes/pkg/api/util" + "k8s.io/kubernetes/pkg/apimachinery/registered" "k8s.io/kubernetes/pkg/util/validation" "k8s.io/kubernetes/pkg/util/validation/field" ) @@ -25,21 +31,50 @@ import ( // ValidateEvent makes sure that the event makes sense. func ValidateEvent(event *api.Event) field.ErrorList { allErrs := field.ErrorList{} - // There is no namespace required for node or persistent volume. - // However, older client code accidentally sets event.Namespace - // to api.NamespaceDefault, so we accept that too, but "" is preferred. - if (event.InvolvedObject.Kind == "Node" || event.InvolvedObject.Kind == "PersistentVolume") && - event.Namespace != api.NamespaceDefault && - event.Namespace != "" { - allErrs = append(allErrs, field.Invalid(field.NewPath("involvedObject", "namespace"), event.InvolvedObject.Namespace, "not allowed for node")) + + // Make sure event.Namespace and the involvedObject.Namespace agree + if len(event.InvolvedObject.Namespace) == 0 { + // event.Namespace must also be empty (or "default", for compatibility with old clients) + if event.Namespace != api.NamespaceNone && event.Namespace != api.NamespaceDefault { + allErrs = append(allErrs, field.Invalid(field.NewPath("involvedObject", "namespace"), event.InvolvedObject.Namespace, "does not match event.namespace")) + } + } else { + // event namespace must match + if event.Namespace != event.InvolvedObject.Namespace { + allErrs = append(allErrs, field.Invalid(field.NewPath("involvedObject", "namespace"), event.InvolvedObject.Namespace, "does not match event.namespace")) + } } - if event.InvolvedObject.Kind != "Node" && - event.InvolvedObject.Kind != "PersistentVolume" && - event.Namespace != event.InvolvedObject.Namespace { - allErrs = append(allErrs, field.Invalid(field.NewPath("involvedObject", "namespace"), event.InvolvedObject.Namespace, "does not match involvedObject")) + + // For kinds we recognize, make sure involvedObject.Namespace is set for namespaced kinds + if namespaced, err := isNamespacedKind(event.InvolvedObject.Kind, event.InvolvedObject.APIVersion); err == nil { + if namespaced && len(event.InvolvedObject.Namespace) == 0 { + allErrs = append(allErrs, field.Required(field.NewPath("involvedObject", "namespace"), fmt.Sprintf("required for kind %s", event.InvolvedObject.Kind))) + } + if !namespaced && len(event.InvolvedObject.Namespace) > 0 { + allErrs = append(allErrs, field.Invalid(field.NewPath("involvedObject", "namespace"), event.InvolvedObject.Namespace, fmt.Sprintf("not allowed for kind %s", event.InvolvedObject.Kind))) + } } + for _, msg := range validation.IsDNS1123Subdomain(event.Namespace) { allErrs = append(allErrs, field.Invalid(field.NewPath("namespace"), event.Namespace, msg)) } return allErrs } + +// Check whether the kind in groupVersion is scoped at the root of the api hierarchy +func isNamespacedKind(kind, groupVersion string) (bool, error) { + group := apiutil.GetGroup(groupVersion) + g, err := registered.Group(group) + if err != nil { + return false, err + } + restMapping, err := g.RESTMapper.RESTMapping(unversioned.GroupKind{Group: group, Kind: kind}, apiutil.GetVersion(groupVersion)) + if err != nil { + return false, err + } + scopeName := restMapping.Scope.Name() + if scopeName == meta.RESTScopeNameNamespace { + return true, nil + } + return false, nil +} diff --git a/pkg/api/validation/events_test.go b/pkg/api/validation/events_test.go index 61f0d6e4e70..3747a30f864 100644 --- a/pkg/api/validation/events_test.go +++ b/pkg/api/validation/events_test.go @@ -35,17 +35,177 @@ func TestValidateEvent(t *testing.T) { }, InvolvedObject: api.ObjectReference{ Namespace: "bar", + Kind: "Pod", }, }, false, }, { &api.Event{ ObjectMeta: api.ObjectMeta{ - Name: "test1", + Name: "test2", Namespace: "aoeu-_-aoeu", }, InvolvedObject: api.ObjectReference{ Namespace: "aoeu-_-aoeu", + Kind: "Pod", + }, + }, + false, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test3", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "v1", + Kind: "Node", + }, + }, + true, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test4", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "v1", + Kind: "Namespace", + }, + }, + true, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test5", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "extensions/v1beta1", + Kind: "NoKind", + Namespace: api.NamespaceDefault, + }, + }, + true, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test6", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "extensions/v1beta1", + Kind: "Job", + Namespace: "foo", + }, + }, + false, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test7", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "extensions/v1beta1", + Kind: "Job", + Namespace: api.NamespaceDefault, + }, + }, + true, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test8", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "other/v1beta1", + Kind: "Job", + Namespace: "foo", + }, + }, + false, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test9", + Namespace: "foo", + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "other/v1beta1", + Kind: "Job", + Namespace: "foo", + }, + }, + true, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test10", + Namespace: api.NamespaceDefault, + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "extensions", + Kind: "Job", + Namespace: "foo", + }, + }, + false, + }, { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test11", + Namespace: "foo", + }, + InvolvedObject: api.ObjectReference{ + // must register in v1beta1 to be true + APIVersion: "extensions/v1beta1", + Kind: "Job", + Namespace: "foo", + }, + }, + true, + }, + { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test12", + Namespace: "foo", + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "other/v1beta1", + Kind: "FooBar", + Namespace: "bar", + }, + }, + false, + }, + { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test13", + Namespace: "", + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "other/v1beta1", + Kind: "FooBar", + Namespace: "bar", + }, + }, + false, + }, + { + &api.Event{ + ObjectMeta: api.ObjectMeta{ + Name: "test14", + Namespace: "foo", + }, + InvolvedObject: api.ObjectReference{ + APIVersion: "other/v1beta1", + Kind: "FooBar", + Namespace: "", }, }, false,