diff --git a/pkg/registry/core/namespace/storage/storage_test.go b/pkg/registry/core/namespace/storage/storage_test.go index fc31c809cc2..a1bfd9e9af8 100644 --- a/pkg/registry/core/namespace/storage/storage_test.go +++ b/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) } diff --git a/pkg/registry/core/persistentvolume/storage/storage_test.go b/pkg/registry/core/persistentvolume/storage/storage_test.go index 62f15d35e70..c03fa3248a9 100644 --- a/pkg/registry/core/persistentvolume/storage/storage_test.go +++ b/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) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go index b981c3a66a9..141bfb17412 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go +++ b/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) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 001fed6b249..6cb17dfdb03 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/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)) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 2809d9f6200..878c2cfa42a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/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) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 17919e4aa17..a951c1de7a8 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/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 } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index 7009ae73869..dced59404d4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/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 diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go index 942ee4774c0..da203464d68 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/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) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go index add6044ab0c..6741b3da342 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go +++ b/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 } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/meta_test.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/meta_test.go index cec33b67a95..feafa56495e 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/meta_test.go +++ b/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) + } + }) } } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go index ffcca33c05e..3a98c4c4de2 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go +++ b/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 diff --git a/test/integration/auth/svcaccttoken_test.go b/test/integration/auth/svcaccttoken_test.go index 85bfc0fc46d..c02511b43b2 100644 --- a/test/integration/auth/svcaccttoken_test.go +++ b/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()