Merge pull request #38648 from sykesm/fake-propagates-ns

Automatic merge from submit-queue

Fake clientset propagates namespace to objects on create/update

**What this PR does / why we need it**:

Unlike the real Clientset, the fake Clientset requires creates and updates of namespaced objects to have the namespace specified in the target runtime object metadata. This difference forces API clients using the fake Clientset for test to propagate the namespaces to the runtime objects in the production code. This propagation is unnecessary and should be handled by the fake implementation.

**Which issue this PR fixes**: fixes kubernetes/client-go#48

**Special notes for your reviewer**:

**Release note**:
NONE

pkg/client/testing/core/fake_test.go

@@ -94,6 +94,65 @@ func TestFakeClientSetFiltering(t *testing.T) {
 	}
 }
 
+func TestFakeClientsetInheritsNamespace(t *testing.T) {
+	tc := clientsetfake.NewSimpleClientset(
+		testNamespace("nsA"),
+		testPod("nsA", "pod-1"),
+	)
+
+	_, err := tc.Core().Namespaces().Create(testNamespace("nsB"))
+	if err != nil {
+		t.Fatalf("Namespaces.Create: %s", err)
+	}
+
+	allNS, err := tc.Core().Namespaces().List(api.ListOptions{})
+	if err != nil {
+		t.Fatalf("Namespaces.List: %s", err)
+	}
+	if actual, expected := len(allNS.Items), 2; expected != actual {
+		t.Fatalf("Expected %d namespaces to match, got %d", expected, actual)
+	}
+
+	_, err = tc.Core().Pods("nsB").Create(testPod("", "pod-1"))
+	if err != nil {
+		t.Fatalf("Pods.Create nsB/pod-1: %s", err)
+	}
+
+	podB1, err := tc.Core().Pods("nsB").Get("pod-1", metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Pods.Get nsB/pod-1: %s", err)
+	}
+	if podB1 == nil {
+		t.Fatalf("Expected to find pod nsB/pod-1 but it wasn't found")
+	}
+	if podB1.Namespace != "nsB" || podB1.Name != "pod-1" {
+		t.Fatalf("Expected to find pod nsB/pod-1t, got %s/%s", podB1.Namespace, podB1.Name)
+	}
+
+	_, err = tc.Core().Pods("nsA").Create(testPod("", "pod-1"))
+	if err == nil {
+		t.Fatalf("Expected Pods.Create to fail with already exists error")
+	}
+
+	_, err = tc.Core().Pods("nsA").Update(testPod("", "pod-1"))
+	if err != nil {
+		t.Fatalf("Pods.Update nsA/pod-1: %s", err)
+	}
+
+	_, err = tc.Core().Pods("nsA").Create(testPod("nsB", "pod-2"))
+	if err == nil {
+		t.Fatalf("Expected Pods.Create to fail with bad request from namespace mismtach")
+	}
+	if err.Error() != `request namespace does not match object namespace, request: "nsA" object: "nsB"` {
+		t.Fatalf("Expected Pods.Create error to provide object and request namespaces, got %q", err)
+	}
+
+	_, err = tc.Core().Pods("nsA").Update(testPod("", "pod-3"))
+	if err == nil {
+		t.Fatalf("Expected Pods.Update nsA/pod-3 to fail with not found error")
+	}
+}
+
 func testSA(ns, name string) *api.ServiceAccount {
 	return &api.ServiceAccount{
 		ObjectMeta: api.ObjectMeta{
@@ -111,3 +170,11 @@ func testPod(ns, name string) *api.Pod {
 		},
 	}
 }
+
+func testNamespace(ns string) *api.Namespace {
+	return &api.Namespace{
+		ObjectMeta: api.ObjectMeta{
+			Name: ns,
+		},
+	}
+}

pkg/client/testing/core/fixture.go

@@ -41,8 +41,11 @@ type ObjectTracker interface {
 	// Get retrieves the object by its kind, namespace and name.
 	Get(gvk schema.GroupVersionKind, ns, name string) (runtime.Object, error)
 
-	// Update updates an existing object in the tracker.
-	Update(obj runtime.Object) error
+	// Create adds an object to the tracker in the specified namespace.
+	Create(obj runtime.Object, ns string) error
+
+	// Update updates an existing object in the tracker in the specified namespace.
+	Update(obj runtime.Object, ns string) error
 
 	// List retrieves all objects of a given kind in the given
 	// namespace. Only non-List kinds are accepted.
@@ -102,12 +105,12 @@ func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc
 				return true, nil, err
 			}
 			if action.GetSubresource() == "" {
-				err = tracker.Add(action.GetObject())
+				err = tracker.Create(action.GetObject(), ns)
 			} else {
 				// TODO: Currently we're handling subresource creation as an update
 				// on the enclosing resource. This works for some subresources but
 				// might not be generic enough.
-				err = tracker.Update(action.GetObject())
+				err = tracker.Update(action.GetObject(), ns)
 			}
 			if err != nil {
 				return true, nil, err
@@ -120,7 +123,7 @@ func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc
 			if err != nil {
 				return true, nil, err
 			}
-			err = tracker.Update(action.GetObject())
+			err = tracker.Update(action.GetObject(), ns)
 			if err != nil {
 				return true, nil, err
 			}
@@ -243,18 +246,26 @@ func (t *tracker) Get(gvk schema.GroupVersionKind, ns, name string) (runtime.Obj
 }
 
 func (t *tracker) Add(obj runtime.Object) error {
-	return t.add(obj, false)
-}
-
-func (t *tracker) Update(obj runtime.Object) error {
-	return t.add(obj, true)
-}
-
-func (t *tracker) add(obj runtime.Object, replaceExisting bool) error {
 	if meta.IsListType(obj) {
-		return t.addList(obj, replaceExisting)
+		return t.addList(obj, false)
 	}
 
+	objMeta, err := meta.Accessor(obj)
+	if err != nil {
+		return err
+	}
+	return t.add(obj, objMeta.GetNamespace(), false)
+}
+
+func (t *tracker) Create(obj runtime.Object, ns string) error {
+	return t.add(obj, ns, false)
+}
+
+func (t *tracker) Update(obj runtime.Object, ns string) error {
+	return t.add(obj, ns, true)
+}
+
+func (t *tracker) add(obj runtime.Object, ns string, replaceExisting bool) error {
 	gvks, _, err := t.scheme.ObjectKinds(obj)
 	if err != nil {
 		return err
@@ -286,6 +297,16 @@ func (t *tracker) add(obj runtime.Object, replaceExisting bool) error {
 			return err
 		}
 
+		// Propagate namespace to the new object if hasn't already been set.
+		if len(newMeta.GetNamespace()) == 0 {
+			newMeta.SetNamespace(ns)
+		}
+
+		if ns != newMeta.GetNamespace() {
+			msg := fmt.Sprintf("request namespace does not match object namespace, request: %q object: %q", ns, newMeta.GetNamespace())
+			return errors.NewBadRequest(msg)
+		}
+
 		if err := checkNamespace(gvk, newMeta.GetNamespace()); err != nil {
 			return err
 		}
@@ -325,7 +346,11 @@ func (t *tracker) addList(obj runtime.Object, replaceExisting bool) error {
 		return errs[0]
 	}
 	for _, obj := range list {
-		err := t.add(obj, replaceExisting)
+		objMeta, err := meta.Accessor(obj)
+		if err != nil {
+			return err
+		}
+		err = t.add(obj, objMeta.GetNamespace(), replaceExisting)
 		if err != nil {
 			return err
 		}