Fix server-side namespace handling for events; add validation

justinsb · Nov 14, 2014 · de75e5a · de75e5a
1 parent 3cf0227
commit de75e5a
Show file tree

Hide file tree

Showing 5 changed files with 173 additions and 39 deletions.
diff --git a/pkg/api/validation/events.go b/pkg/api/validation/events.go
@@ -0,0 +1,35 @@
+/*
+Copyright 2014 Google Inc. 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 (
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	errs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+)
+
+// ValidateEvent makes sure that the event makes sense.
+func ValidateEvent(event *api.Event) errs.ValidationErrorList {
+	allErrs := errs.ValidationErrorList{}
+	if event.Namespace != event.InvolvedObject.Namespace {
+		allErrs = append(allErrs, errs.NewFieldInvalid("involvedObject.namespace", event.InvolvedObject.Namespace))
+	}
+	if !util.IsDNSSubdomain(event.Namespace) {
+		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", event.Namespace))
+	}
+	return allErrs
+}
diff --git a/pkg/api/validation/events_test.go b/pkg/api/validation/events_test.go
@@ -0,0 +1,60 @@
+/*
+Copyright 2014 Google Inc. 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 (
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+)
+
+func TestValidateEvent(t *testing.T) {
+	table := []struct {
+		*api.Event
+		valid bool
+	}{
+		{
+			&api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "test1",
+					Namespace: "foo",
+				},
+				InvolvedObject: api.ObjectReference{
+					Namespace: "bar",
+				},
+			},
+			false,
+		}, {
+			&api.Event{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "test1",
+					Namespace: "aoeu-_-aoeu",
+				},
+				InvolvedObject: api.ObjectReference{
+					Namespace: "aoeu-_-aoeu",
+				},
+			},
+			false,
+		},
+	}
+
+	for _, item := range table {
+		if e, a := item.valid, len(ValidateEvent(item.Event)) == 0; e != a {
+			t.Errorf("%v: expected %v, got %v", item.Event.Name, e, a)
+		}
+	}
+}
diff --git a/pkg/registry/event/rest.go b/pkg/registry/event/rest.go
@@ -20,6 +20,8 @@ import (
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic"
@@ -45,7 +47,14 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE
 	if !ok {
 		return nil, fmt.Errorf("invalid object type")
 	}
-
+	if api.Namespace(ctx) != "" {
+		if !api.ValidNamespace(ctx, &event.ObjectMeta) {
+			return nil, errors.NewConflict("event", event.Namespace, fmt.Errorf("event.namespace does not match the provided context"))
+		}
+	}
+	if errs := validation.ValidateEvent(event); len(errs) > 0 {
+		return nil, errors.NewInvalid("event", event.Name, errs)
+	}
 	api.FillObjectMetaSystemFields(ctx, &event.ObjectMeta)
 
 	return apiserver.MakeAsync(func() (runtime.Object, error) {

diff --git a/pkg/registry/event/rest_test.go b/pkg/registry/event/rest_test.go
@@ -38,38 +38,74 @@ func NewTestREST() (testRegistry, *REST) {
 	return reg, NewREST(reg)
 }
 
-func TestRESTCreate(t *testing.T) {
-	_, rest := NewTestREST()
-	eventA := &api.Event{
-		ObjectMeta: api.ObjectMeta{Name: "foo"},
-		Reason:     "forTesting",
-	}
-	c, err := rest.Create(api.NewContext(), eventA)
-	if err != nil {
-		t.Fatalf("Unexpected error %v", err)
+func testEvent(name string) *api.Event {
+	return &api.Event{
+		ObjectMeta: api.ObjectMeta{
+			Name:      name,
+			Namespace: "default",
+		},
+		InvolvedObject: api.ObjectReference{
+			Namespace: "default",
+		},
+		Reason: "forTesting",
 	}
-	if !api.HasObjectMetaSystemFieldValues(&eventA.ObjectMeta) {
-		t.Errorf("storage did not populate object meta field values")
+}
+
+func TestRESTCreate(t *testing.T) {
+	table := []struct {
+		ctx   api.Context
+		event *api.Event
+		valid bool
+	}{
+		{
+			ctx:   api.NewDefaultContext(),
+			event: testEvent("foo"),
+			valid: true,
+		}, {
+			ctx:   api.NewContext(),
+			event: testEvent("bar"),
+			valid: true,
+		}, {
+			ctx:   api.WithNamespace(api.NewContext(), "nondefault"),
+			event: testEvent("bazzzz"),
+			valid: false,
+		},
 	}
-	if e, a := eventA, (<-c).Object; !reflect.DeepEqual(e, a) {
-		t.Errorf("diff: %s", util.ObjectDiff(e, a))
+
+	for _, item := range table {
+		_, rest := NewTestREST()
+		c, err := rest.Create(item.ctx, item.event)
+		if !item.valid {
+			if err == nil {
+				ctxNS := api.Namespace(item.ctx)
+				t.Errorf("unexpected non-error for %v (%v, %v)", item.event.Name, ctxNS, item.event.Namespace)
+			}
+			continue
+		}
+		if err != nil {
+			t.Errorf("%v: Unexpected error %v", item.event.Name, err)
+			continue
+		}
+		if !api.HasObjectMetaSystemFieldValues(&item.event.ObjectMeta) {
+			t.Errorf("storage did not populate object meta field values")
+		}
+		if e, a := item.event, (<-c).Object; !reflect.DeepEqual(e, a) {
+			t.Errorf("diff: %s", util.ObjectDiff(e, a))
+		}
+		// Ensure we implement the interface
+		_ = apiserver.ResourceWatcher(rest)
 	}
-	// Ensure we implement the interface
-	_ = apiserver.ResourceWatcher(rest)
 }
 
 func TestRESTDelete(t *testing.T) {
 	_, rest := NewTestREST()
-	eventA := &api.Event{
-		ObjectMeta: api.ObjectMeta{Name: "foo"},
-		Reason:     "forTesting",
-	}
-	c, err := rest.Create(api.NewContext(), eventA)
+	eventA := testEvent("foo")
+	c, err := rest.Create(api.NewDefaultContext(), eventA)
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}
 	<-c
-	c, err = rest.Delete(api.NewContext(), eventA.Name)
+	c, err = rest.Delete(api.NewDefaultContext(), eventA.Name)
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}
@@ -80,16 +116,13 @@ func TestRESTDelete(t *testing.T) {
 
 func TestRESTGet(t *testing.T) {
 	_, rest := NewTestREST()
-	eventA := &api.Event{
-		ObjectMeta: api.ObjectMeta{Name: "foo"},
-		Reason:     "forTesting",
-	}
-	c, err := rest.Create(api.NewContext(), eventA)
+	eventA := testEvent("foo")
+	c, err := rest.Create(api.NewDefaultContext(), eventA)
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}
 	<-c
-	got, err := rest.Get(api.NewContext(), eventA.Name)
+	got, err := rest.Get(api.NewDefaultContext(), eventA.Name)
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}
@@ -140,16 +173,13 @@ func TestRESTgetAttrs(t *testing.T) {
 
 func TestRESTUpdate(t *testing.T) {
 	_, rest := NewTestREST()
-	eventA := &api.Event{
-		ObjectMeta: api.ObjectMeta{Name: "foo"},
-		Reason:     "forTesting",
-	}
-	c, err := rest.Create(api.NewContext(), eventA)
+	eventA := testEvent("foo")
+	c, err := rest.Create(api.NewDefaultContext(), eventA)
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}
 	<-c
-	_, err = rest.Update(api.NewContext(), eventA)
+	_, err = rest.Update(api.NewDefaultContext(), eventA)
 	if err == nil {
 		t.Errorf("unexpected non-error")
 	}

diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go
@@ -202,13 +202,13 @@ var aEvent string = `
 {
   "kind": "Event",
   "apiVersion": "v1beta1",
+  "namespace": "default",
   "id": "a",
   "involvedObject": {
-    {
-      "kind": "Minion",
-      "name": "a",
-      "apiVersion": "v1beta1",
-    }
+    "kind": "Minion",
+    "name": "a",
+    "namespace": "default",
+    "apiVersion": "v1beta1",
   }
 }
 `