apis/apibindings: store served group resources consistently on Logica…

…lCluster

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

pkg/admission/crdnooverlappinggvr/crdnooverlappinggvr_admission.go

@@ -20,19 +20,24 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/kcp-dev/logicalcluster/v3"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	"k8s.io/apimachinery/pkg/labels"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/kubernetes/pkg/controlplane/apiserver"
+	"k8s.io/client-go/util/retry"
+	"k8s.io/utils/ptr"
 
 	"github.com/kcp-dev/kcp/pkg/reconciler/apis/apibinding"
-	apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
+	corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
+	kcpclientset "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/cluster"
 	kcpinformers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions"
-	apisv1alpha1listers "github.com/kcp-dev/kcp/sdk/client/listers/apis/v1alpha1"
+	corev1alpha1listers "github.com/kcp-dev/kcp/sdk/client/listers/core/v1alpha1"
 )
 
 const (
@@ -43,15 +48,22 @@ func Register(plugins *admission.Plugins) {
 	plugins.Register(PluginName,
 		func(_ io.Reader) (admission.Interface, error) {
 			return &crdNoOverlappingGVRAdmission{
-				Handler: admission.NewHandler(admission.Create),
+				Handler:                admission.NewHandler(admission.Create),
+				lockResourcesPartially: apibinding.LockResourcesPartially,
+				now:                    metav1.Now,
 			}, nil
 		})
 }
 
 type crdNoOverlappingGVRAdmission struct {
 	*admission.Handler
 
-	apiBindingClusterLister apisv1alpha1listers.APIBindingClusterLister
+	lockResourcesPartially func(ctx context.Context, kcpClusterCLient kcpclientset.ClusterInterface, crds []*apiextensionsv1.CustomResourceDefinition, lc *corev1alpha1.LogicalCluster, grs []schema.GroupResource, binding apibinding.ExpirableLock) ([]schema.GroupResource, map[schema.GroupResource]apibinding.Lock, error)
+
+	logicalclusterLister corev1alpha1listers.LogicalClusterClusterLister
+	kcpClusterClient     kcpclientset.ClusterInterface
+
+	now func() metav1.Time
 }
 
 // Ensure that the required admission interfaces are implemented.
@@ -60,12 +72,19 @@ var _ = admission.InitializationValidator(&crdNoOverlappingGVRAdmission{})
 
 func (p *crdNoOverlappingGVRAdmission) SetKcpInformers(local, global kcpinformers.SharedInformerFactory) {
 	p.SetReadyFunc(local.Apis().V1alpha1().APIBindings().Informer().HasSynced)
-	p.apiBindingClusterLister = local.Apis().V1alpha1().APIBindings().Lister()
+	p.logicalclusterLister = local.Core().V1alpha1().LogicalClusters().Lister()
+}
+
+func (p *crdNoOverlappingGVRAdmission) SetKcpClusterClient(c kcpclientset.ClusterInterface) {
+	p.kcpClusterClient = c
 }
 
 func (p *crdNoOverlappingGVRAdmission) ValidateInitialization() error {
-	if p.apiBindingClusterLister == nil {
-		return fmt.Errorf(PluginName + " plugin needs an APIBindings lister")
+	if p.logicalclusterLister == nil {
+		return fmt.Errorf(PluginName + " plugin needs an LogicalCluster lister")
+	}
+	if p.kcpClusterClient == nil {
+		return fmt.Errorf(PluginName + " plugin needs a KCP cluster client")
 	}
 	return nil
 }
@@ -78,35 +97,51 @@ func (p *crdNoOverlappingGVRAdmission) Validate(ctx context.Context, a admission
 	if a.GetKind().GroupKind() != apiextensions.Kind("CustomResourceDefinition") {
 		return nil
 	}
-	cluster, err := request.ClusterNameFrom(ctx)
+	if a.GetOperation() != admission.Create {
+		return nil
+	}
+
+	clusterName, err := request.ClusterNameFrom(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to retrieve cluster from context: %w", err)
 	}
-	clusterName := logicalcluster.Name(cluster.String()) // TODO(sttts): remove this cast once ClusterNameFrom returns a tenancy.Name
-	// ignore CRDs targeting system and non-root workspaces
-	if clusterName == apibinding.SystemBoundCRDsClusterName || clusterName == apiserver.LocalAdminCluster {
+
+	// ignore CRDs targeting system logical clusters.
+	if strings.HasPrefix(string(clusterName), "system:") {
 		return nil
 	}
 
 	crd, ok := a.GetObject().(*apiextensions.CustomResourceDefinition)
 	if !ok {
 		return fmt.Errorf("unexpected type %T", a.GetObject())
 	}
-	apiBindingsForCurrentClusterName, err := p.listAPIBindingsFor(clusterName)
+
+	lc, err := p.logicalclusterLister.Cluster(clusterName).Get(corev1alpha1.LogicalClusterName)
 	if err != nil {
+		return fmt.Errorf("failed to get LogicalCluster in logical cluster %q: %w", clusterName, err)
+	}
+
+	// (optimistically) lock group resource for LogicalCluster. If this request
+	// eventually fails, the logicalclustercleanup controller will clean them
+	// up eventually.
+	gr := schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Spec.Names.Plural}
+	var locked []schema.GroupResource
+	var skipped map[schema.GroupResource]apibinding.Lock
+	err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		var err error
+		locked, skipped, err = p.lockResourcesPartially(ctx, p.kcpClusterClient, nil, lc, []schema.GroupResource{gr}, apibinding.ExpirableLock{
+			Lock:      apibinding.Lock{CRD: true},
+			CRDExpiry: ptr.To(p.now()),
+		})
 		return err
+	})
+	if err != nil {
+		return fmt.Errorf("failed to lock resources %s in logical cluster %q: %w", gr, logicalcluster.From(lc), err)
 	}
-	for _, apiBindingForCurrentClusterName := range apiBindingsForCurrentClusterName {
-		for _, boundResource := range apiBindingForCurrentClusterName.Status.BoundResources {
-			if boundResource.Group == crd.Spec.Group && boundResource.Resource == crd.Spec.Names.Plural {
-				return admission.NewForbidden(a, fmt.Errorf("cannot create %q CustomResourceDefinition with %q group and %q resource because it overlaps with a bound CustomResourceDefinition for %q APIBinding in %q logical cluster",
-					crd.Name, crd.Spec.Group, crd.Spec.Names.Plural, apiBindingForCurrentClusterName.Name, clusterName))
-			}
-		}
+	if len(locked) == 0 {
+		return admission.NewForbidden(a, fmt.Errorf("cannot create CustomResourceDefinition %q because it overlaps with a bound CustomResourceDefinition for %v APIBinding in %q logical cluster",
+			crd.Name, skipped[gr], clusterName))
 	}
-	return nil
-}
 
-func (p *crdNoOverlappingGVRAdmission) listAPIBindingsFor(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) {
-	return p.apiBindingClusterLister.Cluster(clusterName).List(labels.Everything())
+	return nil
 }