address reviews

Author: mjudeikis

backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go

@@ -117,7 +117,7 @@ func (r *reconciler) getExportedSchemas(ctx context.Context, cl client.Client) (
 		return nil, err
 	}
 
-	var boundSchemas kubebindv1alpha2.ExportedSchemas = make(map[string]*kubebindv1alpha2.BoundSchema, len(list.Items))
+	boundSchemas := make(kubebindv1alpha2.ExportedSchemas, len(list.Items))
 	for _, item := range list.Items {
 		boundSchema, err := helpers.UnstructuredToBoundSchema(item)
 		if err != nil {
@@ -285,7 +285,7 @@ func (r *reconciler) validate(ctx context.Context, cl client.Client, req *kubebi
 		return fmt.Errorf("no exported schemas found")
 	}
 
-	scopes := make([]apiextensionsv1.ResourceScope, 0, len(exportedSchemas))
+	first := apiextensionsv1.ResourceScope("")
 	for _, res := range req.Spec.Resources {
 		boundSchema, ok := exportedSchemas[res.ResourceGroupName()]
 		if !ok {
@@ -299,23 +299,19 @@ func (r *reconciler) validate(ctx context.Context, cl client.Client, req *kubebi
 			)
 			return fmt.Errorf("schema %s not found", res.ResourceGroupName())
 		}
-		scopes = append(scopes, boundSchema.Spec.Scope)
-	}
-
-	// Check CRD scopes matches.
-	if len(scopes) > 1 {
-		first := scopes[0]
-		for _, scope := range scopes[1:] {
-			if scope != first {
-				conditions.MarkFalse(req,
-					kubebindv1alpha2.APIServiceExportRequestConditionExportsReady,
-					"DifferentScopes",
-					conditionsapi.ConditionSeverityError,
-					"Different scopes found: %v",
-					scopes,
-				)
-				return fmt.Errorf("different scopes found for claimed resources: %v", scopes)
-			}
+		if first == apiextensionsv1.ResourceScope("") {
+			first = boundSchema.Spec.Scope
+			continue
+		}
+		if boundSchema.Spec.Scope != first {
+			conditions.MarkFalse(req,
+				kubebindv1alpha2.APIServiceExportRequestConditionExportsReady,
+				"DifferentScopes",
+				conditionsapi.ConditionSeverityError,
+				"Different scopes found: %v",
+				boundSchema.Spec.Scope,
+			)
+			return fmt.Errorf("different scopes found for claimed resources: %v", boundSchema.Name)
 		}
 	}
 

backend/http/handler.go

@@ -555,12 +555,13 @@ func (h *handler) getBackendDynamicResource(ctx context.Context, cluster string)
 		Version: parts[1],
 		Group:   parts[2],
 	}
-	items, err := h.kubeManager.ListDynamicResources(ctx, cluster, gvk, labelSelector.AsSelector())
+	list, err := h.kubeManager.ListDynamicResources(ctx, cluster, gvk, labelSelector.AsSelector())
 	if err != nil {
 		return nil, fmt.Errorf("failed to list resources: %w", err)
 	}
-	var boundSchemas kubebindv1alpha2.ExportedSchemas = make(map[string]*kubebindv1alpha2.BoundSchema, len(items.Items))
-	for _, item := range items.Items {
+
+	boundSchemas := make(kubebindv1alpha2.ExportedSchemas, len(list.Items))
+	for _, item := range list.Items {
 		boundSchema, err := helpers.UnstructuredToBoundSchema(item)
 		if err != nil {
 			return nil, err

contrib/kcp/README.md

@@ -90,7 +90,7 @@ kubectl kcp bind apiexport root:provider:cowboys-stable
 ```bash
 kubectl get logicalcluster
 # NAME      PHASE   URL                                                    AGE
-# cluster   Ready   https://192.168.2.166:6443/clusters/crh63mkto0egdjca    
+# cluster   Ready   https://192.168.2.166:6443/clusters/xwhgz3kv8l67wx87    
 ```
 
 9. Now we gonna initiate consumer:
@@ -104,15 +104,15 @@ kubectl ws create consumer --enter
 10. Bind the thing:
 
 ```bash
-./bin/kubectl-bind http://127.0.0.1:8080/clusters/crh63mkto0egdjca/exports --dry-run -o yaml > apiserviceexport.yaml
+./bin/kubectl-bind http://127.0.0.1:8080/clusters/xwhgz3kv8l67wx87/exports --dry-run -o yaml > apiserviceexport.yaml
 
 # Extract secret for binding process. Note that secret name is not the same as output from command above. Check secret
 # name by running `kubectl get secret -n kube-bind` 
-kubectl get secret kubeconfig-npp6r -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
+kubectl get secret kubeconfig-ntgtl -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
 
-./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f contrib/kcp/deploy/examples/apiserviceexport-namespaced.yaml  --skip-konnector --remote-namespace kube-bind-w6gs4
+./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f contrib/kcp/deploy/examples/apiserviceexport-namespaced.yaml  --skip-konnector --remote-namespace kube-bind-wg2tb
 
-./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f contrib/kcp/deploy/examples/apiserviceexport-cluster.yaml  --skip-konnector --remote-namespace kube-bind-t6wg7
+./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f contrib/kcp/deploy/examples/apiserviceexport-cluster.yaml  --skip-konnector --remote-namespace kube-bind-wg2tb
 
 
 export KUBECONFIG=.kcp/consumer.kubeconfig

contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml

@@ -40,7 +40,7 @@ spec:
   - group: apis.kcp.io
     resource: apiresourceschemas
     verbs:
-    - "*"
+    - '*'
   resources:
   - group: kube-bind.io
     name: apiconversions
@@ -54,7 +54,7 @@ spec:
       crd: {}
   - group: kube-bind.io
     name: apiserviceexportrequests
-    schema: v250918-24ab10a.apiserviceexportrequests.kube-bind.io
+    schema: v250923-ec6651d.apiserviceexportrequests.kube-bind.io
     storage:
       crd: {}
   - group: kube-bind.io

contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml

@@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1
 kind: APIResourceSchema
 metadata:
   creationTimestamp: null
-  name: v250918-24ab10a.apiserviceexportrequests.kube-bind.io
+  name: v250923-ec6651d.apiserviceexportrequests.kube-bind.io
 spec:
   conversion:
     strategy: None
@@ -307,7 +307,7 @@ spec:
                 type: object
               type: array
               x-kubernetes-validations:
-              - message: Permission claim selector is immutable
+              - message: permissionClaims are immutable
                 rule: self == oldSelf
             resources:
               description: resources is a list of resources that should be exported.

deploy/crd/kube-bind.io_apiserviceexportrequests.yaml

@@ -311,7 +311,7 @@ spec:
                   type: object
                 type: array
                 x-kubernetes-validations:
-                - message: Permission claim selector is immutable
+                - message: permissionClaims are immutable
                   rule: self == oldSelf
               resources:
                 description: resources is a list of resources that should be exported.

pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go

@@ -46,7 +46,7 @@ type readReconciler struct {
 	deleteConsumerObject func(ctx context.Context, ns, name string) error
 }
 
-// reconcile syncs conmsumer claimed resources to provider.
+// reconcile syncs consumer claimed resources to provider.
 func (r *readReconciler) reconcile(ctx context.Context, providerNamespace, name string) error {
 	logger := klog.FromContext(ctx)
 	logger = logger.WithValues("name", name, "providerNamespace", providerNamespace)
@@ -128,7 +128,7 @@ func (r *readReconciler) reconcile(ctx context.Context, providerNamespace, name
 			return nil
 		}
 
-		if providerObj.GetDeletionTimestamp() != nil && !providerObj.GetDeletionTimestamp().IsZero() {
+		if !providerObj.GetDeletionTimestamp().IsZero() {
 			logger.Info("Deleting downstream object because it has been deleted upstream", "downstreamNamespace", consumerNS, "downstreamName", providerObj.GetName())
 			if err := r.deleteConsumerObject(ctx, consumerNS, providerObj.GetName()); err != nil {
 				return err

pkg/konnector/controllers/cluster/serviceexport/multinsinformer/informer.go

@@ -189,20 +189,19 @@ func (inf *DynamicMultiNamespaceInformer) enqueueServiceNamespace(obj any) {
 	logger.V(1).Info("starting dynamic informer", "namespace", sns.Status.Namespace)
 	ctx, cancel := context.WithCancel(context.Background())
 
-	var factory dynamicinformer.DynamicSharedInformerFactory
+	var tweakListOptions func(options *metav1.ListOptions)
 	if inf.labelSelector != nil {
-		tweakListOptions := func(options *metav1.ListOptions) {
+		tweakListOptions = func(options *metav1.ListOptions) {
 			selector, err := metav1.LabelSelectorAsSelector(inf.labelSelector)
 			if err != nil {
 				utilruntime.HandleError(fmt.Errorf("failed to convert label selector: %w", err))
 				return
 			}
 			options.LabelSelector = selector.String()
 		}
-		factory = dynamicinformer.NewFilteredDynamicSharedInformerFactory(inf.providerDynamicClient, time.Minute*30, sns.Status.Namespace, tweakListOptions)
-	} else {
-		factory = dynamicinformer.NewFilteredDynamicSharedInformerFactory(inf.providerDynamicClient, time.Minute*30, sns.Status.Namespace, nil)
 	}
+
+	factory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(inf.providerDynamicClient, time.Minute*30, sns.Status.Namespace, tweakListOptions)
 	gvrInf := factory.ForResource(inf.gvr)
 	gvrInf.Lister() // to wire the GVR up in the informer factory
 	inf.namespaceCancel[name] = cancel

pkg/konnector/controllers/cluster/serviceexport/serviceexport_reconcile.go

@@ -92,8 +92,8 @@ func (r *reconciler) ensureControllers(ctx context.Context, namespace, name stri
 	if export == nil {
 		// Clean up any controllers associated with this export
 		deleted := r.syncStore.BulkDeletePrefixed(exportKey)
-		for _, k := range deleted {
-			logger.V(1).Info("Stopping APIServiceExport sync", "key", k.Key(), "reason", "NoAPIServiceExport")
+		for _, c := range deleted {
+			logger.V(1).Info("Stopping APIServiceExport sync", "key", c.Key(), "reason", "NoAPIServiceExport")
 		}
 
 		return nil
@@ -362,8 +362,6 @@ func (r *reconciler) ensureControllerForPermissionClaim(
 ) error {
 	logger := klog.FromContext(ctx)
 
-	ctxWithCancel, cancel := context.WithCancel(ctx)
-
 	dynamicProviderClient := dynamicclient.NewForConfigOrDie(r.providerConfig)
 	dynamicConsumerClient := dynamicclient.NewForConfigOrDie(r.consumerConfig)
 
@@ -430,7 +428,6 @@ func (r *reconciler) ensureControllerForPermissionClaim(
 
 		if err != nil {
 			logger.Info("aborting", "error", err)
-			cancel()
 			return err
 		}
 	}
@@ -450,7 +447,6 @@ func (r *reconciler) ensureControllerForPermissionClaim(
 		serviceNamespaceChannel,
 	)
 	if err != nil {
-		cancel()
 		return err
 	}
 
@@ -465,14 +461,15 @@ func (r *reconciler) ensureControllerForPermissionClaim(
 		serviceNamespaceChannel,
 	)
 	if err != nil {
-		cancel()
 		return err
 	}
 
 	logger.Info("creating claim reconciler", "gvr", claimGVR, "key", claimKey)
 
+	ctxWithCancel, cancel := context.WithCancel(ctx)
 	// Start the informers and controllers in a goroutine
 	go func() {
+		defer r.syncStore.Delete(claimKey)
 		defaultConsumerInf.Start(ctxWithCancel.Done())
 
 		// Wait for consumer informers to sync

pkg/konnector/controllers/cluster/serviceexport/spec/spec_reconcile.go

@@ -52,7 +52,7 @@ type reconciler struct {
 func (r *reconciler) reconcile(ctx context.Context, obj *unstructured.Unstructured) error {
 	logger := klog.FromContext(ctx)
 	if r.apiServiceExport == nil { // Should never happen, but we check to make sure we dont regress in the future.
-		panic("apiServiceExport is nil")
+		return fmt.Errorf("internal error: apiServiceExport is nil")
 	}
 
 	ns := obj.GetNamespace()