Merge pull request #3786 from olamilekan000/fix-resource-selctor-not0being-inherited

fix inherited APIBinding not inheriting default permission claim selector

Author: kcp-ci-bot

pkg/reconciler/tenancy/defaultapibindinglifecycle/default_apibinding_lifecycle_controller.go

@@ -60,6 +60,7 @@ const (
 func NewDefaultAPIBindingController(
 	kcpClusterClient kcpclientset.ClusterInterface,
 	logicalClusterInformer corev1alpha1informers.LogicalClusterClusterInformer,
+	globalLogicalClusterInformer corev1alpha1informers.LogicalClusterClusterInformer,
 	workspaceTypeInformer, globalWorkspaceTypeInformer tenancyv1alpha1informers.WorkspaceTypeClusterInformer,
 	apiBindingsInformer apisv1alpha2informers.APIBindingClusterInformer,
 	apiExportsInformer, globalAPIExportsInformer apisv1alpha2informers.APIExportClusterInformer,
@@ -76,6 +77,22 @@ func NewDefaultAPIBindingController(
 			return logicalClusterInformer.Lister().Cluster(clusterName).Get(corev1alpha1.LogicalClusterName)
 		},
 
+		getLogicalClusterByPath: func(path logicalcluster.Path) (*corev1alpha1.LogicalCluster, error) {
+			clusters, err := indexers.ByIndexWithFallback[*corev1alpha1.LogicalCluster](
+				logicalClusterInformer.Informer().GetIndexer(),
+				globalLogicalClusterInformer.Informer().GetIndexer(),
+				indexers.ByLogicalClusterPath,
+				path.String(),
+			)
+			if err != nil {
+				return nil, err
+			}
+			if len(clusters) == 0 {
+				return nil, apierrors.NewNotFound(corev1alpha1.Resource("logicalclusters"), path.String())
+			}
+			return clusters[0], nil
+		},
+
 		listLogicalClusters: func() ([]*corev1alpha1.LogicalCluster, error) {
 			return logicalClusterInformer.Lister().List(labels.Everything())
 		},
@@ -87,6 +104,17 @@ func NewDefaultAPIBindingController(
 		listAPIBindings: func(clusterName logicalcluster.Name) ([]*apisv1alpha2.APIBinding, error) {
 			return apiBindingsInformer.Lister().Cluster(clusterName).List(labels.Everything())
 		},
+		listAPIBindingsByPath: func(ctx context.Context, clusterPath logicalcluster.Path) ([]*apisv1alpha2.APIBinding, error) {
+			bindingList, err := kcpClusterClient.Cluster(clusterPath).ApisV1alpha2().APIBindings().List(ctx, metav1.ListOptions{})
+			if err != nil {
+				return nil, err
+			}
+			result := make([]*apisv1alpha2.APIBinding, len(bindingList.Items))
+			for i := range bindingList.Items {
+				result[i] = &bindingList.Items[i]
+			}
+			return result, nil
+		},
 		getAPIBinding: func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIBinding, error) {
 			return apiBindingsInformer.Lister().Cluster(clusterName).Get(name)
 		},
@@ -134,14 +162,16 @@ type logicalClusterResource = committer.Resource[*corev1alpha1.LogicalClusterSpe
 type DefaultAPIBindingController struct {
 	queue workqueue.TypedRateLimitingInterface[string]
 
-	getLogicalCluster   func(clusterName logicalcluster.Name) (*corev1alpha1.LogicalCluster, error)
-	getWorkspaceType    func(clusterName logicalcluster.Path, name string) (*tenancyv1alpha1.WorkspaceType, error)
-	listLogicalClusters func() ([]*corev1alpha1.LogicalCluster, error)
+	getLogicalCluster       func(clusterName logicalcluster.Name) (*corev1alpha1.LogicalCluster, error)
+	getLogicalClusterByPath func(path logicalcluster.Path) (*corev1alpha1.LogicalCluster, error)
+	getWorkspaceType        func(clusterName logicalcluster.Path, name string) (*tenancyv1alpha1.WorkspaceType, error)
+	listLogicalClusters     func() ([]*corev1alpha1.LogicalCluster, error)
 
-	listAPIBindings  func(clusterName logicalcluster.Name) ([]*apisv1alpha2.APIBinding, error)
-	getAPIBinding    func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIBinding, error)
-	createAPIBinding func(ctx context.Context, clusterName logicalcluster.Path, binding *apisv1alpha2.APIBinding) (*apisv1alpha2.APIBinding, error)
-	getAPIExport     func(clusterName logicalcluster.Path, name string) (*apisv1alpha2.APIExport, error)
+	listAPIBindings       func(clusterName logicalcluster.Name) ([]*apisv1alpha2.APIBinding, error)
+	listAPIBindingsByPath func(ctx context.Context, clusterPath logicalcluster.Path) ([]*apisv1alpha2.APIBinding, error)
+	getAPIBinding         func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIBinding, error)
+	createAPIBinding      func(ctx context.Context, clusterName logicalcluster.Path, binding *apisv1alpha2.APIBinding) (*apisv1alpha2.APIBinding, error)
+	getAPIExport          func(clusterName logicalcluster.Path, name string) (*apisv1alpha2.APIExport, error)
 
 	commitApiBinding     func(ctx context.Context, old, new *apiBindingResource) error
 	commitLogicalCluster func(ctx context.Context, old, new *logicalClusterResource) error
@@ -302,7 +332,13 @@ func (c *DefaultAPIBindingController) process(ctx context.Context, key string) e
 }
 
 // InstallIndexers adds the additional indexers that this controller requires to the informers.
-func InstallIndexers(apiExportInformer, globalApiExportInformer apisv1alpha2informers.APIExportClusterInformer) {
+func InstallIndexers(logicalClusterInformer, globalLogicalClusterInformer corev1alpha1informers.LogicalClusterClusterInformer, apiExportInformer, globalApiExportInformer apisv1alpha2informers.APIExportClusterInformer) {
+	indexers.AddIfNotPresentOrDie(logicalClusterInformer.Informer().GetIndexer(), cache.Indexers{
+		indexers.ByLogicalClusterPath: indexers.IndexByLogicalClusterPath,
+	})
+	indexers.AddIfNotPresentOrDie(globalLogicalClusterInformer.Informer().GetIndexer(), cache.Indexers{
+		indexers.ByLogicalClusterPath: indexers.IndexByLogicalClusterPath,
+	})
 	indexers.AddIfNotPresentOrDie(apiExportInformer.Informer().GetIndexer(), cache.Indexers{
 		indexers.ByLogicalClusterPathAndName: indexers.IndexByLogicalClusterPathAndName,
 	})

pkg/reconciler/tenancy/defaultapibindinglifecycle/default_apibinding_lifecycle_reconcile.go

@@ -30,6 +30,7 @@ import (
 
 	"github.com/kcp-dev/logicalcluster/v3"
 	apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2"
+	"github.com/kcp-dev/sdk/apis/core"
 	corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
 	tenancyv1alpha1 "github.com/kcp-dev/sdk/apis/tenancy/v1alpha1"
 	conditionsv1alpha1 "github.com/kcp-dev/sdk/apis/third_party/conditions/apis/conditions/v1alpha1"
@@ -39,6 +40,70 @@ import (
 	"github.com/kcp-dev/kcp/pkg/reconciler/tenancy/initialization"
 )
 
+func (c *DefaultAPIBindingController) findSelectorInWorkspace(ctx context.Context, workspacePath logicalcluster.Path, exportRef tenancyv1alpha1.APIExportReference, exportClaim apisv1alpha2.PermissionClaim, logger klog.Logger) *apisv1alpha2.PermissionClaimSelector {
+	if workspacePath.Empty() {
+		return nil
+	}
+
+	workspaceBindings, err := c.listAPIBindingsByPath(ctx, workspacePath)
+	if err != nil {
+		logger.V(4).Info("error listing workspace APIBindings by path", "error", err, "path", workspacePath)
+		return nil
+	}
+
+	exportRefPath := logicalcluster.NewPath(exportRef.Path)
+	if exportRefPath.Empty() {
+		exportRefPath = workspacePath
+	}
+
+	var matchingBindings []*apisv1alpha2.APIBinding
+	for _, binding := range workspaceBindings {
+		if binding.Spec.Reference.Export == nil {
+			continue
+		}
+
+		bindingExportPath := logicalcluster.NewPath(binding.Spec.Reference.Export.Path)
+		if bindingExportPath.Empty() {
+			bindingExportPath = workspacePath
+		}
+
+		if binding.Spec.Reference.Export.Name == exportRef.Export &&
+			bindingExportPath.String() == exportRefPath.String() {
+			matchingBindings = append(matchingBindings, binding)
+		}
+	}
+
+	if len(matchingBindings) == 0 {
+		return nil
+	}
+
+	var matchedSelector *apisv1alpha2.PermissionClaimSelector
+	for _, binding := range matchingBindings {
+		for _, claim := range binding.Spec.PermissionClaims {
+			if claim.Group == exportClaim.Group &&
+				claim.Resource == exportClaim.Resource &&
+				claim.IdentityHash == exportClaim.IdentityHash &&
+				claim.State == apisv1alpha2.ClaimAccepted {
+				if !claim.Selector.MatchAll {
+					logger.V(4).Info("found matching selector in workspace binding", "workspacePath", workspacePath, "selector", claim.Selector)
+					return &claim.Selector
+				}
+
+				if matchedSelector == nil {
+					matchedSelector = &claim.Selector
+				}
+			}
+		}
+	}
+
+	if matchedSelector != nil {
+		logger.V(4).Info("found matching selector in workspace binding", "workspacePath", workspacePath, "selector", matchedSelector)
+		return matchedSelector
+	}
+
+	return nil
+}
+
 func (c *DefaultAPIBindingController) reconcile(ctx context.Context, logicalCluster *corev1alpha1.LogicalCluster) error {
 	logger := klog.FromContext(ctx)
 
@@ -132,13 +197,26 @@ func (c *DefaultAPIBindingController) reconcile(ctx context.Context, logicalClus
 			}
 
 			for _, exportClaim := range apiExport.Spec.PermissionClaims {
-				// For now we automatically accept DefaultAPIBindings
+				var selector apisv1alpha2.PermissionClaimSelector
+				selector = apisv1alpha2.PermissionClaimSelector{
+					MatchAll: true,
+				}
+
+				var parentPath logicalcluster.Path
+				if annPath, found := logicalCluster.Annotations[core.LogicalClusterPathAnnotationKey]; found {
+					currentPath := logicalcluster.NewPath(annPath)
+					parentPath, _ = currentPath.Parent()
+				}
+
+				if parentSelector := c.findSelectorInWorkspace(ctx, parentPath, exportRef, exportClaim, logger); parentSelector != nil {
+					selector = *parentSelector
+					logger.V(3).Info("inheriting selector from parent workspace binding", "parentPath", parentPath, "selector", selector)
+				}
+
 				acceptedClaim := apisv1alpha2.AcceptablePermissionClaim{
 					ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{
 						PermissionClaim: exportClaim,
-						Selector: apisv1alpha2.PermissionClaimSelector{
-							MatchAll: true,
-						},
+						Selector:        selector,
 					},
 					State: apisv1alpha2.ClaimAccepted,
 				}