WIP: Rebase 1.32 Pre.1 Diff

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go

@@ -263,10 +263,25 @@ const (
 	resourceCountPollPeriodJitter = 1.2
 )
 
+// NoNamespaceKeyRootFunc is the default function for constructing storage paths
+// to resource directories enforcing namespace rules.
+func NoNamespaceKeyRootFunc(ctx context.Context, prefix string) string {
+	key := prefix
+	cluster, err := genericapirequest.ValidClusterFrom(ctx)
+	if err != nil {
+		klog.Errorf("invalid context cluster value: %v", err)
+		return key
+	}
+	if !cluster.Wildcard {
+		key += "/" + cluster.Name.String()
+	}
+	return key
+}
+
 // NamespaceKeyRootFunc is the default function for constructing storage paths
 // to resource directories enforcing namespace rules.
 func NamespaceKeyRootFunc(ctx context.Context, prefix string) string {
-	key := prefix
+	key := NoNamespaceKeyRootFunc(ctx, prefix)
 	ns, ok := genericapirequest.NamespaceFrom(ctx)
 	if ok && len(ns) > 0 {
 		key = key + "/" + ns
@@ -278,7 +293,6 @@ func NamespaceKeyRootFunc(ctx context.Context, prefix string) string {
 // a resource relative to the given prefix enforcing namespace rules. If the
 // context does not contain a namespace, it errors.
 func NamespaceKeyFunc(ctx context.Context, prefix string, name string) (string, error) {
-	key := NamespaceKeyRootFunc(ctx, prefix)
 	ns, ok := genericapirequest.NamespaceFrom(ctx)
 	if !ok || len(ns) == 0 {
 		return "", apierrors.NewBadRequest("Namespace parameter required.")
@@ -289,8 +303,7 @@ func NamespaceKeyFunc(ctx context.Context, prefix string, name string) (string,
 	if msgs := path.IsValidPathSegmentName(name); len(msgs) != 0 {
 		return "", apierrors.NewBadRequest(fmt.Sprintf("Name parameter invalid: %q: %s", name, strings.Join(msgs, ";")))
 	}
-	key = key + "/" + name
-	return key, nil
+	return NoNamespaceKeyRootFunc(ctx, prefix) + "/" + ns + "/" + name, nil
 }
 
 // NoNamespaceKeyFunc is the default function for constructing storage paths
@@ -302,8 +315,7 @@ func NoNamespaceKeyFunc(ctx context.Context, prefix string, name string) (string
 	if msgs := path.IsValidPathSegmentName(name); len(msgs) != 0 {
 		return "", apierrors.NewBadRequest(fmt.Sprintf("Name parameter invalid: %q: %s", name, strings.Join(msgs, ";")))
 	}
-	key := prefix + "/" + name
-	return key, nil
+	return NoNamespaceKeyRootFunc(ctx, prefix) + "/" + name, nil
 }
 
 // New implements RESTStorage.New.
@@ -478,7 +490,8 @@ func (e *Store) create(ctx context.Context, obj runtime.Object, createValidation
 	var finishCreate FinishFunc = finishNothing
 
 	// Init metadata as early as possible.
-	if objectMeta, err := meta.Accessor(obj); err != nil {
+	objectMeta, err := meta.Accessor(obj)
+	if err != nil {
 		return nil, err
 	} else {
 		rest.FillObjectMetaSystemFields(objectMeta)
@@ -1504,6 +1517,9 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error {
 	if (e.KeyRootFunc == nil) != (e.KeyFunc == nil) {
 		return fmt.Errorf("store for %s must set both KeyRootFunc and KeyFunc or neither", e.DefaultQualifiedResource.String())
 	}
+	if e.KeyFunc != nil || e.KeyFunc != nil {
+		return fmt.Errorf("DEBUG: keyfunc must be non-nil for all resources", e.DefaultQualifiedResource.String())
+	}
 
 	if e.TableConvertor == nil {
 		return fmt.Errorf("store for %s must set TableConvertor; rest.NewDefaultTableConvertor(e.DefaultQualifiedResource) can be used to output just name/creation time", e.DefaultQualifiedResource.String())
@@ -1564,6 +1580,13 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error {
 		return fmt.Errorf("store for %s has an invalid prefix %q", e.DefaultQualifiedResource.String(), opts.ResourcePrefix)
 	}
 
+	if e.KeyFunc != nil {
+		panic(fmt.Sprintf("KeyFunc is illegal for %v: %T", e.DefaultQualifiedResource, e.KeyFunc))
+	}
+	if e.KeyRootFunc != nil {
+		panic(fmt.Sprintf("KeyRootFunc is illegal for %v: %T", e.DefaultQualifiedResource, e.KeyRootFunc))
+	}
+
 	// Set the default behavior for storage key generation
 	if e.KeyRootFunc == nil && e.KeyFunc == nil {
 		if isNamespaced {
@@ -1575,7 +1598,7 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error {
 			}
 		} else {
 			e.KeyRootFunc = func(ctx context.Context) string {
-				return prefix
+				return NoNamespaceKeyRootFunc(ctx, prefix)
 			}
 			e.KeyFunc = func(ctx context.Context, name string) (string, error) {
 				return NoNamespaceKeyFunc(ctx, prefix, name)

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

@@ -25,6 +25,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/kcp-dev/logicalcluster/v3"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -102,10 +103,14 @@ func (t *Tester) TestNamespace() string {
 	return "test"
 }
 
+func (t *Tester) TestCluster() logicalcluster.Name {
+	return logicalcluster.New("root")
+}
+
 // TestContext returns a namespaced context that will be used when making storage calls.
 // Namespace is determined by TestNamespace()
 func (t *Tester) TestContext() context.Context {
-	return genericapirequest.WithNamespace(genericapirequest.NewContext(), t.TestNamespace())
+	return genericapirequest.WithCluster(genericapirequest.WithNamespace(genericapirequest.NewContext(), t.TestNamespace()), genericapirequest.Cluster{Name: t.TestCluster()})
 }
 
 func (t *Tester) getObjectMetaOrFail(obj runtime.Object) metav1.Object {
@@ -314,6 +319,7 @@ func (t *Tester) testCreateDryRunEquals(obj runtime.Object) {
 	createdFakeMeta.SetResourceVersion("")
 	createdMeta.SetResourceVersion("")
 	createdMeta.SetUID(createdFakeMeta.GetUID())
+	createdMeta.SetZZZ_DeprecatedClusterName(createdFakeMeta.GetZZZ_DeprecatedClusterName())
 
 	if e, a := created, createdFake; !apiequality.Semantic.DeepEqual(e, a) {
 		t.Errorf("unexpected obj: %#v, expected %#v", e, a)
@@ -341,6 +347,7 @@ func (t *Tester) testCreateEquals(obj runtime.Object, getFn GetFunc) {
 	createdMeta := t.getObjectMetaOrFail(created)
 	gotMeta := t.getObjectMetaOrFail(got)
 	createdMeta.SetResourceVersion(gotMeta.GetResourceVersion())
+	createdMeta.SetZZZ_DeprecatedClusterName(gotMeta.GetZZZ_DeprecatedClusterName())
 
 	if e, a := created, got; !apiequality.Semantic.DeepEqual(e, a) {
 		t.Errorf("unexpected obj: %#v, expected %#v", e, a)
@@ -402,7 +409,7 @@ func (t *Tester) testCreateHasMetadata(valid runtime.Object) {
 
 func (t *Tester) testCreateIgnoresContextNamespace(valid runtime.Object, opts metav1.CreateOptions) {
 	// Ignore non-empty namespace in context
-	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), "not-default2")
+	ctx := genericapirequest.WithNamespace(genericapirequest.WithCluster(genericapirequest.NewContext(), genericapirequest.Cluster{Name: t.TestCluster()}), "not-default2")
 
 	// Ideally, we'd get an error back here, but at least verify the namespace wasn't persisted
 	created, err := t.storage.(rest.Creater).Create(ctx, valid.DeepCopyObject(), rest.ValidateAllObjectFunc, &opts)
@@ -421,7 +428,7 @@ func (t *Tester) testCreateIgnoresMismatchedNamespace(valid runtime.Object, opts
 
 	// Ignore non-empty namespace in object meta
 	objectMeta.SetNamespace("not-default")
-	ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), "not-default2")
+	ctx := genericapirequest.WithNamespace(genericapirequest.WithCluster(genericapirequest.NewContext(), genericapirequest.Cluster{Name: t.TestCluster()}), "not-default2")
 
 	// Ideally, we'd get an error back here, but at least verify the namespace wasn't persisted
 	created, err := t.storage.(rest.Creater).Create(ctx, valid.DeepCopyObject(), rest.ValidateAllObjectFunc, &opts)
@@ -1164,14 +1171,14 @@ func (t *Tester) testGetDifferentNamespace(obj runtime.Object) {
 	objMeta := t.getObjectMetaOrFail(obj)
 	objMeta.SetName(t.namer(5))
 
-	ctx1 := genericapirequest.WithNamespace(genericapirequest.NewContext(), "bar3")
+	ctx1 := genericapirequest.WithNamespace(genericapirequest.WithCluster(genericapirequest.NewContext(), genericapirequest.Cluster{Name: t.TestCluster()}), "bar3")
 	objMeta.SetNamespace(genericapirequest.NamespaceValue(ctx1))
 	_, err := t.storage.(rest.Creater).Create(ctx1, obj, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
 
-	ctx2 := genericapirequest.WithNamespace(genericapirequest.NewContext(), "bar4")
+	ctx2 := genericapirequest.WithNamespace(genericapirequest.WithCluster(genericapirequest.NewContext(), genericapirequest.Cluster{Name: t.TestCluster()}), "bar4")
 	objMeta.SetNamespace(genericapirequest.NamespaceValue(ctx2))
 	_, err = t.storage.(rest.Creater).Create(ctx2, obj, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 	if err != nil {
@@ -1225,8 +1232,8 @@ func (t *Tester) testGetFound(obj runtime.Object) {
 }
 
 func (t *Tester) testGetMimatchedNamespace(obj runtime.Object) {
-	ctx1 := genericapirequest.WithNamespace(genericapirequest.NewContext(), "bar1")
-	ctx2 := genericapirequest.WithNamespace(genericapirequest.NewContext(), "bar2")
+	ctx1 := genericapirequest.WithNamespace(genericapirequest.WithCluster(genericapirequest.NewContext(), genericapirequest.Cluster{Name: t.TestCluster()}), "bar1")
+	ctx2 := genericapirequest.WithNamespace(genericapirequest.WithCluster(genericapirequest.NewContext(), genericapirequest.Cluster{Name: t.TestCluster()}), "bar2")
 	objMeta := t.getObjectMetaOrFail(obj)
 	objMeta.SetName(t.namer(4))
 	objMeta.SetNamespace(genericapirequest.NamespaceValue(ctx1))
@@ -1319,7 +1326,7 @@ func (t *Tester) testListMatchLabels(obj runtime.Object, assignFn AssignFunc) {
 	foo4Meta.SetNamespace(genericapirequest.NamespaceValue(ctx))
 	foo4Meta.SetLabels(testLabels)
 
-	objs := ([]runtime.Object{foo3, foo4})
+	objs := []runtime.Object{foo3, foo4}
 
 	assignFn(objs)
 	filtered := []runtime.Object{objs[1]}
@@ -1373,7 +1380,7 @@ func (t *Tester) testListTableConversion(obj runtime.Object, assignFn AssignFunc
 	foo4Meta.SetNamespace(genericapirequest.NamespaceValue(ctx))
 	foo4Meta.SetLabels(testLabels)
 
-	objs := ([]runtime.Object{foo3, foo4})
+	objs := []runtime.Object{foo3, foo4}
 
 	assignFn(objs)