Merge pull request #30533 from liggitt/event-validation

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
+}

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,