Merge pull request #94637 from liggitt/namespace-before-admission

set/validate object namespace before admission

pkg/registry/core/namespace/storage/storage_test.go

@@ -74,7 +74,7 @@ func TestCreateSetsFields(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	namespace := validNewNamespace()
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	_, err := storage.Create(ctx, namespace, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -152,7 +152,7 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -167,6 +167,7 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) {
 	if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	obj, immediate, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil)
 	if err != nil {
 		t.Fatalf("unexpected error")
@@ -188,7 +189,7 @@ func TestUpdateDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -204,6 +205,7 @@ func TestUpdateDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T) {
 	if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	ns, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -223,7 +225,7 @@ func TestUpdateDeletingNamespaceWithIncompleteSpecFinalizers(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -242,6 +244,7 @@ func TestUpdateDeletingNamespaceWithIncompleteSpecFinalizers(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	if _, _, err = storage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -257,7 +260,7 @@ func TestUpdateDeletingNamespaceWithCompleteFinalizers(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -275,6 +278,7 @@ func TestUpdateDeletingNamespaceWithCompleteFinalizers(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 	ns.(*api.Namespace).Finalizers = nil
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	if _, _, err = storage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -298,7 +302,7 @@ func TestFinalizeDeletingNamespaceWithCompleteFinalizers(t *testing.T) {
 	defer storage.store.DestroyFunc()
 	defer finalizeStorage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -318,6 +322,7 @@ func TestFinalizeDeletingNamespaceWithCompleteFinalizers(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 	ns.(*api.Namespace).Spec.Finalizers = nil
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	if _, _, err = finalizeStorage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -341,7 +346,7 @@ func TestFinalizeDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T)
 	defer storage.store.DestroyFunc()
 	defer finalizeStorage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -362,6 +367,7 @@ func TestFinalizeDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T)
 		t.Fatalf("unexpected error: %v", err)
 	}
 	ns.(*api.Namespace).Spec.Finalizers = nil
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	if _, _, err = finalizeStorage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -377,7 +383,7 @@ func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	key := "namespaces/foo"
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	now := metav1.Now()
 	namespace := &api.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -392,6 +398,7 @@ func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) {
 	if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+	ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 	if _, _, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil); err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -579,7 +586,7 @@ func TestDeleteWithGCFinalizers(t *testing.T) {
 
 	for _, test := range tests {
 		key := "namespaces/" + test.name
-		ctx := genericapirequest.NewContext()
+		ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 		namespace := &api.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:       test.name,
@@ -595,6 +602,7 @@ func TestDeleteWithGCFinalizers(t *testing.T) {
 		}
 		var obj runtime.Object
 		var err error
+		ctx = genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace.Name)
 		if obj, _, err = storage.Delete(ctx, test.name, rest.ValidateAllObjectFunc, test.deleteOptions); err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}

pkg/registry/core/persistentvolume/storage/storage_test.go

@@ -169,7 +169,7 @@ func TestUpdateStatus(t *testing.T) {
 	storage, statusStorage, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
-	ctx := genericapirequest.NewContext()
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
 	key, _ := storage.KeyFunc(ctx, "foo")
 	pvStart := validNewPersistentVolume("foo")
 	err := storage.Storage.Create(ctx, key, pvStart, nil, 0, false)

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go

@@ -50,6 +50,7 @@ import (
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/endpoints/discovery"
 	apirequest "k8s.io/apiserver/pkg/endpoints/request"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
 	"k8s.io/apiserver/pkg/registry/rest"
@@ -470,7 +471,10 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 	defer server.Terminate(t)
 
 	crd := multiVersionFixture.DeepCopy()
-	if _, err := cl.ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{}); err != nil {
+	// Create a context with metav1.NamespaceNone as the namespace since multiVersionFixture
+	// is a cluster scoped CRD.
+	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceNone)
+	if _, err := cl.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{}); err != nil {
 		t.Fatal(err)
 	}
 	if err := crdInformer.Informer().GetStore().Add(crd); err != nil {
@@ -526,12 +530,12 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 		u := &unstructured.Unstructured{Object: map[string]interface{}{}}
 		u.SetGroupVersionKind(schema.GroupVersionKind{Group: "stable.example.com", Version: "v1beta1", Kind: "MultiVersion"})
 		u.SetName("marker")
-		if item, err := crdInfo.storages["v1beta1"].CustomResource.Create(context.TODO(), u, validateFunc, &metav1.CreateOptions{}); err != nil {
+		if item, err := crdInfo.storages["v1beta1"].CustomResource.Create(ctx, u, validateFunc, &metav1.CreateOptions{}); err != nil {
 			t.Fatal(err)
 		} else {
 			startResourceVersion = item.(*unstructured.Unstructured).GetResourceVersion()
 		}
-		if _, _, err := crdInfo.storages["v1beta1"].CustomResource.Delete(context.TODO(), u.GetName(), validateFunc, &metav1.DeleteOptions{}); err != nil {
+		if _, _, err := crdInfo.storages["v1beta1"].CustomResource.Delete(ctx, u.GetName(), validateFunc, &metav1.DeleteOptions{}); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -545,7 +549,7 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 		unstructured.SetNestedField(u.Object, int64(1), "spec", "num")
 
 		// Create
-		if item, err := crdInfo.storages[version.Name].CustomResource.Create(context.TODO(), u, validateFunc, &metav1.CreateOptions{}); err != nil {
+		if item, err := crdInfo.storages[version.Name].CustomResource.Create(ctx, u, validateFunc, &metav1.CreateOptions{}); err != nil {
 			t.Fatal(err)
 		} else if item.GetObjectKind().GroupVersionKind() != expectGVK {
 			t.Errorf("expected create result to be %#v, got %#v", expectGVK, item.GetObjectKind().GroupVersionKind())
@@ -555,14 +559,14 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 
 		// Update
 		u.SetAnnotations(map[string]string{"updated": "true"})
-		if item, _, err := crdInfo.storages[version.Name].CustomResource.Update(context.TODO(), u.GetName(), rest.DefaultUpdatedObjectInfo(u), validateFunc, updateValidateFunc, false, &metav1.UpdateOptions{}); err != nil {
+		if item, _, err := crdInfo.storages[version.Name].CustomResource.Update(ctx, u.GetName(), rest.DefaultUpdatedObjectInfo(u), validateFunc, updateValidateFunc, false, &metav1.UpdateOptions{}); err != nil {
 			t.Fatal(err)
 		} else if item.GetObjectKind().GroupVersionKind() != expectGVK {
 			t.Errorf("expected update result to be %#v, got %#v", expectGVK, item.GetObjectKind().GroupVersionKind())
 		}
 
 		// Get
-		if item, err := crdInfo.storages[version.Name].CustomResource.Get(context.TODO(), u.GetName(), &metav1.GetOptions{}); err != nil {
+		if item, err := crdInfo.storages[version.Name].CustomResource.Get(ctx, u.GetName(), &metav1.GetOptions{}); err != nil {
 			t.Fatal(err)
 		} else if item.GetObjectKind().GroupVersionKind() != expectGVK {
 			t.Errorf("expected get result to be %#v, got %#v", expectGVK, item.GetObjectKind().GroupVersionKind())
@@ -572,7 +576,7 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 			// Allow time to propagate the create into the cache
 			time.Sleep(time.Second)
 			// Get cached
-			if item, err := crdInfo.storages[version.Name].CustomResource.Get(context.TODO(), u.GetName(), &metav1.GetOptions{ResourceVersion: "0"}); err != nil {
+			if item, err := crdInfo.storages[version.Name].CustomResource.Get(ctx, u.GetName(), &metav1.GetOptions{ResourceVersion: "0"}); err != nil {
 				t.Fatal(err)
 			} else if item.GetObjectKind().GroupVersionKind() != expectGVK {
 				t.Errorf("expected cached get result to be %#v, got %#v", expectGVK, item.GetObjectKind().GroupVersionKind())
@@ -584,7 +588,7 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 	for _, version := range crd.Spec.Versions {
 		expectGVK := schema.GroupVersionKind{Group: "stable.example.com", Version: version.Name, Kind: "MultiVersion"}
 
-		if list, err := crdInfo.storages[version.Name].CustomResource.List(context.TODO(), &metainternalversion.ListOptions{}); err != nil {
+		if list, err := crdInfo.storages[version.Name].CustomResource.List(ctx, &metainternalversion.ListOptions{}); err != nil {
 			t.Fatal(err)
 		} else {
 			for _, item := range list.(*unstructured.UnstructuredList).Items {
@@ -596,7 +600,7 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 
 		if enableWatchCache {
 			// List from watch cache
-			if list, err := crdInfo.storages[version.Name].CustomResource.List(context.TODO(), &metainternalversion.ListOptions{ResourceVersion: "0"}); err != nil {
+			if list, err := crdInfo.storages[version.Name].CustomResource.List(ctx, &metainternalversion.ListOptions{ResourceVersion: "0"}); err != nil {
 				t.Fatal(err)
 			} else {
 				for _, item := range list.(*unstructured.UnstructuredList).Items {
@@ -607,7 +611,7 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 			}
 		}
 
-		watch, err := crdInfo.storages[version.Name].CustomResource.Watch(context.TODO(), &metainternalversion.ListOptions{ResourceVersion: startResourceVersion})
+		watch, err := crdInfo.storages[version.Name].CustomResource.Watch(ctx, &metainternalversion.ListOptions{ResourceVersion: startResourceVersion})
 		if err != nil {
 			t.Fatal(err)
 		}

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

@@ -3510,6 +3510,7 @@ func TestNamedCreaterWithGenerateName(t *testing.T) {
 
 	itemOut.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
 	simple.Name = populateName
+	simple.Namespace = "default" // populated by create handler to match request URL
 	if !reflect.DeepEqual(&itemOut, simple) {
 		t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simple, string(body))
 	}
@@ -3588,6 +3589,7 @@ func TestCreate(t *testing.T) {
 	}
 
 	itemOut.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	simple.Namespace = "default" // populated by create handler to match request URL
 	if !reflect.DeepEqual(&itemOut, simple) {
 		t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simple, string(body))
 	}
@@ -3649,6 +3651,7 @@ func TestCreateYAML(t *testing.T) {
 	}
 
 	itemOut.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	simple.Namespace = "default" // populated by create handler to match request URL
 	if !reflect.DeepEqual(&itemOut, simple) {
 		t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simple, string(body))
 	}
@@ -3700,6 +3703,7 @@ func TestCreateInNamespace(t *testing.T) {
 	}
 
 	itemOut.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	simple.Namespace = "other" // populated by create handler to match request URL
 	if !reflect.DeepEqual(&itemOut, simple) {
 		t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simple, string(body))
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go

@@ -47,7 +47,7 @@ import (
 	utiltrace "k8s.io/utils/trace"
 )
 
-var namespaceGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"}
+var namespaceGVR = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}
 
 func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Interface, includeName bool) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
@@ -152,7 +152,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 		if len(name) == 0 {
 			_, name, _ = scope.Namer.ObjectName(obj)
 		}
-		if len(namespace) == 0 && *gvk == namespaceGVK {
+		if len(namespace) == 0 && scope.Resource == namespaceGVR {
 			namespace = name
 		}
 		ctx = request.WithNamespace(ctx, namespace)
@@ -163,6 +163,15 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 
 		userInfo, _ := request.UserFrom(ctx)
 
+		// if this object supports namespace info
+		if objectMeta, err := meta.Accessor(obj); err == nil {
+			// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
+			if err := rest.EnsureObjectNamespaceMatchesRequestNamespace(rest.ExpectedNamespaceForResource(namespace, scope.Resource), objectMeta); err != nil {
+				scope.err(err, w, req)
+				return
+			}
+		}
+
 		trace.Step("About to store object in database")
 		admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, options, dryrun.IsDryRun(options.DryRun), userInfo)
 		requestFunc := func() (runtime.Object, error) {

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -576,6 +576,14 @@ func (p *patcher) applyPatch(ctx context.Context, _, currentObject runtime.Objec
 		return nil, errors.NewConflict(p.resource.GroupResource(), p.name, fmt.Errorf("uid mismatch: the provided object specified uid %s, and no existing object was found", accessor.GetUID()))
 	}
 
+	// if this object supports namespace info
+	if objectMeta, err := meta.Accessor(objToUpdate); err == nil {
+		// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
+		if err := rest.EnsureObjectNamespaceMatchesRequestNamespace(rest.ExpectedNamespaceForResource(p.namespace, p.resource), objectMeta); err != nil {
+			return nil, err
+		}
+	}
+
 	if err := checkName(objToUpdate, p.name, p.namespace, p.namer); err != nil {
 		return nil, err
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go

@@ -139,6 +139,15 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 		audit.LogRequestObject(req.Context(), obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
 		admit = admission.WithAudit(admit, ae)
 
+		// if this object supports namespace info
+		if objectMeta, err := meta.Accessor(obj); err == nil {
+			// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
+			if err := rest.EnsureObjectNamespaceMatchesRequestNamespace(rest.ExpectedNamespaceForResource(namespace, scope.Resource), objectMeta); err != nil {
+				scope.err(err, w, req)
+				return
+			}
+		}
+
 		if err := checkName(obj, name, namespace, scope.Namer); err != nil {
 			scope.err(err, w, req)
 			return

staging/src/k8s.io/apiserver/pkg/registry/rest/create.go

@@ -18,6 +18,7 @@ package rest
 
 import (
 	"context"
+	"fmt"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -99,13 +100,15 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime.
 		return kerr
 	}
 
-	if strategy.NamespaceScoped() {
-		if !ValidNamespace(ctx, objectMeta) {
-			return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request")
-		}
-	} else if len(objectMeta.GetNamespace()) > 0 {
-		objectMeta.SetNamespace(metav1.NamespaceNone)
+	// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
+	requestNamespace, ok := genericapirequest.NamespaceFrom(ctx)
+	if !ok {
+		return errors.NewInternalError(fmt.Errorf("no namespace information found in request context"))
 	}
+	if err := EnsureObjectNamespaceMatchesRequestNamespace(ExpectedNamespaceForScope(requestNamespace, strategy.NamespaceScoped()), objectMeta); err != nil {
+		return err
+	}
+
 	objectMeta.SetDeletionTimestamp(nil)
 	objectMeta.SetDeletionGracePeriodSeconds(nil)
 	strategy.PrepareForCreate(ctx, obj)

staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go

@@ -17,11 +17,10 @@ limitations under the License.
 package rest
 
 import (
-	"context"
-
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/uuid"
-	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
 // FillObjectMetaSystemFields populates fields that are managed by the system on ObjectMeta.
@@ -31,13 +30,44 @@ func FillObjectMetaSystemFields(meta metav1.Object) {
 	meta.SetSelfLink("")
 }
 
-// ValidNamespace returns false if the namespace on the context differs from
-// the resource.  If the resource has no namespace, it is set to the value in
-// the context.
-func ValidNamespace(ctx context.Context, resource metav1.Object) bool {
-	ns, ok := genericapirequest.NamespaceFrom(ctx)
-	if len(resource.GetNamespace()) == 0 {
-		resource.SetNamespace(ns)
+// EnsureObjectNamespaceMatchesRequestNamespace returns an error if obj.Namespace and requestNamespace
+// are both populated and do not match. If either is unpopulated, it modifies obj as needed to ensure
+// obj.GetNamespace() == requestNamespace.
+func EnsureObjectNamespaceMatchesRequestNamespace(requestNamespace string, obj metav1.Object) error {
+	objNamespace := obj.GetNamespace()
+	switch {
+	case objNamespace == requestNamespace:
+		// already matches, no-op
+		return nil
+
+	case objNamespace == metav1.NamespaceNone:
+		// unset, default to request namespace
+		obj.SetNamespace(requestNamespace)
+		return nil
+
+	case requestNamespace == metav1.NamespaceNone:
+		// cluster-scoped, clear namespace
+		obj.SetNamespace(metav1.NamespaceNone)
+		return nil
+
+	default:
+		// mismatch, error
+		return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request")
 	}
-	return ns == resource.GetNamespace() && ok
+}
+
+// ExpectedNamespaceForScope returns the expected namespace for a resource, given the request namespace and resource scope.
+func ExpectedNamespaceForScope(requestNamespace string, namespaceScoped bool) string {
+	if namespaceScoped {
+		return requestNamespace
+	}
+	return ""
+}
+
+// ExpectedNamespaceForResource returns the expected namespace for a resource, given the request namespace.
+func ExpectedNamespaceForResource(requestNamespace string, resource schema.GroupVersionResource) string {
+	if resource.Resource == "namespaces" && resource.Group == "" {
+		return ""
+	}
+	return requestNamespace
 }

staging/src/k8s.io/apiserver/pkg/registry/rest/meta_test.go

@@ -21,8 +21,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apiserver/pkg/apis/example"
-	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
 // TestFillObjectMetaSystemFields validates that system populated fields are set on an object
@@ -55,30 +53,65 @@ func TestHasObjectMetaSystemFieldValues(t *testing.T) {
 	}
 }
 
-// TestValidNamespace validates that namespace rules are enforced on a resource prior to create or update
-func TestValidNamespace(t *testing.T) {
-	ctx := genericapirequest.NewDefaultContext()
-	namespace, _ := genericapirequest.NamespaceFrom(ctx)
-	// TODO: use some genericapiserver type here instead of clientapiv1
-	resource := example.Pod{}
-	if !ValidNamespace(ctx, &resource.ObjectMeta) {
-		t.Fatalf("expected success")
-	}
-	if namespace != resource.Namespace {
-		t.Fatalf("expected resource to have the default namespace assigned during validation")
-	}
-	resource = example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "other"}}
-	if ValidNamespace(ctx, &resource.ObjectMeta) {
-		t.Fatalf("Expected error that resource and context errors do not match because resource has different namespace")
-	}
-	ctx = genericapirequest.NewContext()
-	if ValidNamespace(ctx, &resource.ObjectMeta) {
-		t.Fatalf("Expected error that resource and context errors do not match since context has no namespace")
+func TestEnsureObjectNamespaceMatchesRequestNamespace(t *testing.T) {
+	testcases := []struct {
+		name        string
+		reqNS       string
+		objNS       string
+		expectErr   bool
+		expectObjNS string
+	}{
+		{
+			name:        "cluster-scoped req, cluster-scoped obj",
+			reqNS:       "",
+			objNS:       "",
+			expectErr:   false,
+			expectObjNS: "",
+		},
+		{
+			name:        "cluster-scoped req, namespaced obj",
+			reqNS:       "",
+			objNS:       "foo",
+			expectErr:   false,
+			expectObjNS: "", // no error, object is forced to cluster-scoped for backwards compatibility
+		},
+		{
+			name:        "namespaced req, no-namespace obj",
+			reqNS:       "foo",
+			objNS:       "",
+			expectErr:   false,
+			expectObjNS: "foo", // no error, object is updated to match request for backwards compatibility
+		},
+		{
+			name:        "namespaced req, matching obj",
+			reqNS:       "foo",
+			objNS:       "foo",
+			expectErr:   false,
+			expectObjNS: "foo",
+		},
+		{
+			name:      "namespaced req, mis-matched obj",
+			reqNS:     "foo",
+			objNS:     "bar",
+			expectErr: true,
+		},
 	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			obj := metav1.ObjectMeta{Namespace: tc.objNS}
+			err := EnsureObjectNamespaceMatchesRequestNamespace(tc.reqNS, &obj)
+			if tc.expectErr {
+				if err == nil {
+					t.Fatal("expected err, got none")
+				}
+				return
+			} else if err != nil {
+				t.Fatalf("unexpected err: %v", err)
+			}
 
-	ctx = genericapirequest.NewContext()
-	ns := genericapirequest.NamespaceValue(ctx)
-	if ns != "" {
-		t.Fatalf("Expected the empty string")
+			if obj.Namespace != tc.expectObjNS {
+				t.Fatalf("expected obj ns %q, got %q", tc.expectObjNS, obj.Namespace)
+			}
+		})
 	}
 }

staging/src/k8s.io/apiserver/pkg/registry/rest/update.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/features"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/apiserver/pkg/warning"
@@ -110,12 +111,14 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx context.Context, obj, old run
 	if kerr != nil {
 		return kerr
 	}
-	if strategy.NamespaceScoped() {
-		if !ValidNamespace(ctx, objectMeta) {
-			return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request")
-		}
-	} else if len(objectMeta.GetNamespace()) > 0 {
-		objectMeta.SetNamespace(metav1.NamespaceNone)
+
+	// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
+	requestNamespace, ok := genericapirequest.NamespaceFrom(ctx)
+	if !ok {
+		return errors.NewInternalError(fmt.Errorf("no namespace information found in request context"))
+	}
+	if err := EnsureObjectNamespaceMatchesRequestNamespace(ExpectedNamespaceForScope(requestNamespace, strategy.NamespaceScoped()), objectMeta); err != nil {
+		return err
 	}
 
 	// Ensure requests cannot update generation

test/integration/auth/svcaccttoken_test.go

@@ -203,8 +203,8 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 
 		treqWithBadNamespace := treq.DeepCopy()
 		treqWithBadNamespace.Namespace = "invalid-namespace"
-		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treqWithBadNamespace, metav1.CreateOptions{}); err == nil || !strings.Contains(err.Error(), "must match the service account namespace") {
-			t.Fatalf("expected err creating token with mismatched namespace but got: %#v", resp)
+		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treqWithBadNamespace, metav1.CreateOptions{}); err == nil || !strings.Contains(err.Error(), "does not match the namespace") {
+			t.Fatalf("expected err creating token with mismatched namespace but got: %#v, %v", resp, err)
 		}
 
 		warningHandler.clear()