From 4848dc97b796e1e28c5bfdc94ffea8ea5d800377 Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Thu, 16 Oct 2025 17:07:14 +0300 Subject: [PATCH 1/4] delete old stuff --- .../serviceexport/serviceexport_controller.go | 9 --------- .../controllers/serviceexport/serviceexport_reconcile.go | 4 +--- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/backend/controllers/serviceexport/serviceexport_controller.go b/backend/controllers/serviceexport/serviceexport_controller.go index fa40c15c4..8d8e5efd8 100644 --- a/backend/controllers/serviceexport/serviceexport_controller.go +++ b/backend/controllers/serviceexport/serviceexport_controller.go @@ -22,7 +22,6 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -75,14 +74,6 @@ func NewAPIServiceExportReconciler( } return &schema, nil }, - deleteServiceExport: func(ctx context.Context, cl client.Client, ns, name string) error { - return cl.Delete(ctx, &kubebindv1alpha2.APIServiceExport{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - Name: name, - }, - }) - }, }, } diff --git a/backend/controllers/serviceexport/serviceexport_reconcile.go b/backend/controllers/serviceexport/serviceexport_reconcile.go index 00ffe55bb..727310342 100644 --- a/backend/controllers/serviceexport/serviceexport_reconcile.go +++ b/backend/controllers/serviceexport/serviceexport_reconcile.go @@ -23,7 +23,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/cache" - "sigs.k8s.io/controller-runtime/pkg/client" kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2" "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2/helpers" @@ -32,8 +31,7 @@ import ( ) type reconciler struct { - getBoundSchema func(ctx context.Context, cache cache.Cache, namespace, name string) (*kubebindv1alpha2.BoundSchema, error) - deleteServiceExport func(ctx context.Context, client client.Client, namespace, name string) error + getBoundSchema func(ctx context.Context, cache cache.Cache, namespace, name string) (*kubebindv1alpha2.BoundSchema, error) } func (r *reconciler) reconcile(ctx context.Context, cache cache.Cache, export *kubebindv1alpha2.APIServiceExport) error { From 584b5456fee4d7f71ec77eba022036c064726773 Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Fri, 17 Oct 2025 13:52:31 +0300 Subject: [PATCH 2/4] Add provider side namespaces claims Signed-off-by: Mangirdas Judeikis On-behalf-of: @SAP mangirdas.judeikis@sap.com --- .../serviceexport/serviceexport_controller.go | 2 + .../serviceexport/serviceexport_reconcile.go | 16 +++ .../serviceexportrequest_reconcile.go | 36 ++++++ .../servicenamespace_reconcile.go | 2 +- backend/server.go | 1 + contrib/kcp/README.md | 2 + .../resources/apiexport-kube-bind.io.yaml | 2 +- ...apiserviceexportrequests.kube-bind.io.yaml | 21 +++- ...kube-bind.io_apiserviceexportrequests.yaml | 19 +++ .../claimedresources_reconciler.go | 10 +- .../controllers/cluster/cluster_controller.go | 11 +- .../namespacelifecycle_controller.go} | 109 +++++++++++++++--- .../v1alpha2/apiserviceexport_types.go | 4 + .../v1alpha2/apiserviceexportrequest_types.go | 17 +++ .../kubebind/v1alpha2/helpers/namespaces.go | 39 +++++++ .../v1alpha2/zz_generated.deepcopy.go | 21 ++++ test/e2e/bind/happy-case_test.go | 94 +++++++++++++++ 17 files changed, 376 insertions(+), 30 deletions(-) rename pkg/konnector/controllers/cluster/{namespacedeletion/namespacedeletion_controller.go => namespacelifecycle/namespacelifecycle_controller.go} (62%) create mode 100644 sdk/apis/kubebind/v1alpha2/helpers/namespaces.go diff --git a/backend/controllers/serviceexport/serviceexport_controller.go b/backend/controllers/serviceexport/serviceexport_controller.go index 8d8e5efd8..21166e4ff 100644 --- a/backend/controllers/serviceexport/serviceexport_controller.go +++ b/backend/controllers/serviceexport/serviceexport_controller.go @@ -55,6 +55,7 @@ type APIServiceExportReconciler struct { func NewAPIServiceExportReconciler( ctx context.Context, mgr mcmanager.Manager, + scope kubebindv1alpha2.InformerScope, opts controller.TypedOptions[mcreconcile.Request], ) (*APIServiceExportReconciler, error) { if err := mgr.GetFieldIndexer().IndexField(ctx, &kubebindv1alpha2.APIServiceExport{}, indexers.ServiceExportByBoundSchema, @@ -66,6 +67,7 @@ func NewAPIServiceExportReconciler( manager: mgr, opts: opts, reconciler: reconciler{ + scope: scope, getBoundSchema: func(ctx context.Context, cache cache.Cache, namespace, name string) (*kubebindv1alpha2.BoundSchema, error) { var schema kubebindv1alpha2.BoundSchema key := types.NamespacedName{Namespace: namespace, Name: name} diff --git a/backend/controllers/serviceexport/serviceexport_reconcile.go b/backend/controllers/serviceexport/serviceexport_reconcile.go index 727310342..23061c530 100644 --- a/backend/controllers/serviceexport/serviceexport_reconcile.go +++ b/backend/controllers/serviceexport/serviceexport_reconcile.go @@ -31,6 +31,7 @@ import ( ) type reconciler struct { + scope kubebindv1alpha2.InformerScope getBoundSchema func(ctx context.Context, cache cache.Cache, namespace, name string) (*kubebindv1alpha2.BoundSchema, error) } @@ -41,6 +42,12 @@ func (r *reconciler) reconcile(ctx context.Context, cache cache.Cache, export *k errs = append(errs, err) } + if r.scope == kubebindv1alpha2.ClusterScope { + if err := r.ensureClusterPermissions(ctx, cache, export); err != nil { + errs = append(errs, err) + } + } + return utilerrors.NewAggregate(errs) } @@ -86,3 +93,12 @@ func (r *reconciler) ensureSchema(ctx context.Context, cache cache.Cache, export return nil } + +// ensureClusterPermissions ensures that the necessary cluster-wide permissions are in place for the exported APIs. +// This runs only when backend is operating in ClusterScope. In this scenario, there might not yet be a namespace so +// APIServiceNamespace reconciliation cannot be used. +func (r *reconciler) ensureClusterPermissions(ctx context.Context, cache cache.Cache, export *kubebindv1alpha2.APIServiceExport) error { + // TODO(mjudeikis): implement this for cluster scoped resources. + // https://github.com/kube-bind/kube-bind/issues/344 + return nil +} diff --git a/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go b/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go index 630cf4a8f..71bf2806f 100644 --- a/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go +++ b/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go @@ -70,6 +70,11 @@ func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cach return fmt.Errorf("failed to ensure exports: %w", err) } + if err := r.ensureAPIServiceNamespaces(ctx, cl, cache, req); err != nil { + conditions.SetSummary(req) + return fmt.Errorf("failed to ensure APIServiceNamespaces: %w", err) + } + // TODO(mjudeikis): we could potentially add finallizer to APIServiceExport above or "adopt" boundschemas // with owner references once export is created. // https://github.com/kube-bind/kube-bind/issues/297 @@ -367,3 +372,34 @@ func isClaimableAPI(claim kubebindv1alpha2.PermissionClaim) bool { } return false } + +func (r *reconciler) ensureAPIServiceNamespaces(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error { + logger := klog.FromContext(ctx) + + // TODO(mjudeikis): We have this object above already, pass it down to avoid extra get. + export := &kubebindv1alpha2.APIServiceExport{} + if err := cl.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: req.Name}, export); err != nil { + return fmt.Errorf("failed to get APIServiceExport %s/%s: %w", req.Namespace, req.Name, err) + } + + for _, ns := range req.Spec.Namespaces { + apiServiceNamespace := helpers.APIServiceNamespaceFromExport(export, ns.Name) + currentAPIServiceNamespace := &kubebindv1alpha2.APIServiceNamespace{} + err := cache.Get(ctx, client.ObjectKey{Namespace: apiServiceNamespace.Namespace, Name: apiServiceNamespace.Name}, currentAPIServiceNamespace) + if err != nil { + if apierrors.IsNotFound(err) { + logger.V(1).Info("Creating APIServiceNamespace", "name", apiServiceNamespace.Name, "namespace", apiServiceNamespace.Namespace) + if err := cl.Create(ctx, apiServiceNamespace); err != nil { + if apierrors.IsAlreadyExists(err) { + continue + } + return err + } + } else { + return err + } + } + } + + return nil +} diff --git a/backend/controllers/servicenamespace/servicenamespace_reconcile.go b/backend/controllers/servicenamespace/servicenamespace_reconcile.go index 98ad6d3dc..fe408588e 100644 --- a/backend/controllers/servicenamespace/servicenamespace_reconcile.go +++ b/backend/controllers/servicenamespace/servicenamespace_reconcile.go @@ -106,7 +106,7 @@ func (c *reconciler) reconcile(ctx context.Context, client client.Client, cache Rules: permissions, } // Create new ClusterRole - if err := client.Create(ctx, role); err != nil { + if err := client.Create(ctx, role); !errors.IsAlreadyExists(err) { return fmt.Errorf("failed to create ClusterRole %s: %w", name, err) } } else { diff --git a/backend/server.go b/backend/server.go index c36ad7b9e..d35e5c73d 100644 --- a/backend/server.go +++ b/backend/server.go @@ -147,6 +147,7 @@ func NewServer(ctx context.Context, c *Config) (*Server, error) { s.ServiceExport, err = serviceexport.NewAPIServiceExportReconciler( ctx, s.Config.Manager, + kubebindv1alpha2.InformerScope(c.Options.ConsumerScope), opts, ) if err != nil { diff --git a/contrib/kcp/README.md b/contrib/kcp/README.md index 804818261..2c70e571c 100644 --- a/contrib/kcp/README.md +++ b/contrib/kcp/README.md @@ -170,6 +170,8 @@ kubectl apply -f contrib/kcp/deploy/examples/sheriff.yaml ``` + + ## Debug ```bash diff --git a/contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml b/contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml index 5a8b03b89..f6269bb0a 100644 --- a/contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml +++ b/contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml @@ -54,7 +54,7 @@ spec: crd: {} - group: kube-bind.io name: apiserviceexportrequests - schema: v250929-62fc678.apiserviceexportrequests.kube-bind.io + schema: v251020-7892cc6.apiserviceexportrequests.kube-bind.io storage: crd: {} - group: kube-bind.io diff --git a/contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml b/contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml index 76a220263..d9a3f88f9 100644 --- a/contrib/kcp/deploy/resources/apiresourceschema-apiserviceexportrequests.kube-bind.io.yaml +++ b/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: v250929-62fc678.apiserviceexportrequests.kube-bind.io + name: v251020-7892cc6.apiserviceexportrequests.kube-bind.io spec: conversion: strategy: None @@ -208,6 +208,25 @@ spec: spec specifies how an API service from a service provider should be bound in the local consumer cluster. properties: + namespaces: + description: |- + namespaces specifies the namespaces to bootstrap as part of this request. + When objects originate from provider side, the consumer does not always know the necessary details. + This field allows provider to pre-heat the necessary namespaces on provider side by creating + APIServiceNamespace objects attached to the APIServiceExport. More namespaces can be created later by the consumer. + items: + properties: + name: + description: name is the name of the namespace to create on provider + side. + type: string + required: + - name + type: object + type: array + x-kubernetes-validations: + - message: namespaces are immutable + rule: self == oldSelf parameters: description: |- parameters holds service provider specific parameters for this binding diff --git a/deploy/crd/kube-bind.io_apiserviceexportrequests.yaml b/deploy/crd/kube-bind.io_apiserviceexportrequests.yaml index 2dc9adcfc..8a9c3eea0 100644 --- a/deploy/crd/kube-bind.io_apiserviceexportrequests.yaml +++ b/deploy/crd/kube-bind.io_apiserviceexportrequests.yaml @@ -212,6 +212,25 @@ spec: spec specifies how an API service from a service provider should be bound in the local consumer cluster. properties: + namespaces: + description: |- + namespaces specifies the namespaces to bootstrap as part of this request. + When objects originate from provider side, the consumer does not always know the necessary details. + This field allows provider to pre-heat the necessary namespaces on provider side by creating + APIServiceNamespace objects attached to the APIServiceExport. More namespaces can be created later by the consumer. + items: + properties: + name: + description: name is the name of the namespace to create on + provider side. + type: string + required: + - name + type: object + type: array + x-kubernetes-validations: + - message: namespaces are immutable + rule: self == oldSelf parameters: description: |- parameters holds service provider specific parameters for this binding diff --git a/pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go b/pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go index fc549305c..3d176d994 100644 --- a/pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go +++ b/pkg/konnector/controllers/cluster/claimedresources/claimedresources_reconciler.go @@ -31,8 +31,6 @@ import ( kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2" ) -const label = "kube-bind.io/owner" - type readReconciler struct { getServiceNamespace func(upstreamNamespace string) (*kubebindv1alpha2.APIServiceNamespace, error) getProviderObject func(ns, name string) (*unstructured.Unstructured, error) @@ -185,7 +183,7 @@ func (r readReconciler) makeConsumerOwner(obj *unstructured.Unstructured) { if a == nil { a = map[string]string{} } - a[label] = string(kubebindv1alpha2.OwnerConsumer) + a[kubebindv1alpha2.ObjectOwnerLabel] = string(kubebindv1alpha2.OwnerConsumer) obj.SetLabels(a) } @@ -194,7 +192,7 @@ func (r readReconciler) makeProviderOwner(obj *unstructured.Unstructured) { if a == nil { a = map[string]string{} } - a[label] = string(kubebindv1alpha2.OwnerProvider) + a[kubebindv1alpha2.ObjectOwnerLabel] = string(kubebindv1alpha2.OwnerProvider) obj.SetLabels(a) } @@ -237,7 +235,7 @@ func candidateFromOwnerObj(downstreamNS string, obj *unstructured.Unstructured) // consumer or provider side. func determineOwner(providerObj, consumerObj *unstructured.Unstructured) (kubebindv1alpha2.Owner, error) { if providerObj != nil { - ownerLabel := providerObj.GetLabels()[label] + ownerLabel := providerObj.GetLabels()[kubebindv1alpha2.ObjectOwnerLabel] switch ownerLabel { case kubebindv1alpha2.OwnerProvider.String(): return kubebindv1alpha2.OwnerProvider, nil @@ -250,7 +248,7 @@ func determineOwner(providerObj, consumerObj *unstructured.Unstructured) (kubebi } if consumerObj != nil { - ownerLabel := consumerObj.GetLabels()[label] + ownerLabel := consumerObj.GetLabels()[kubebindv1alpha2.ObjectOwnerLabel] switch ownerLabel { case kubebindv1alpha2.OwnerProvider.String(): return kubebindv1alpha2.OwnerProvider, nil diff --git a/pkg/konnector/controllers/cluster/cluster_controller.go b/pkg/konnector/controllers/cluster/cluster_controller.go index 59e90595e..1710b10be 100644 --- a/pkg/konnector/controllers/cluster/cluster_controller.go +++ b/pkg/konnector/controllers/cluster/cluster_controller.go @@ -35,7 +35,7 @@ import ( "github.com/kube-bind/kube-bind/pkg/indexers" "github.com/kube-bind/kube-bind/pkg/konnector/controllers/cluster/clusterbinding" - "github.com/kube-bind/kube-bind/pkg/konnector/controllers/cluster/namespacedeletion" + "github.com/kube-bind/kube-bind/pkg/konnector/controllers/cluster/namespacelifecycle" "github.com/kube-bind/kube-bind/pkg/konnector/controllers/cluster/servicebinding" "github.com/kube-bind/kube-bind/pkg/konnector/controllers/cluster/serviceexport" "github.com/kube-bind/kube-bind/pkg/konnector/controllers/dynamic" @@ -115,8 +115,9 @@ func NewController( if err != nil { return nil, err } - namespacedeletionCtrl, err := namespacedeletion.NewController( + namespacelifecycleCtrl, err := namespacelifecycle.NewController( providerConfig, + consumerConfig, providerNamespace, providerBindInformers.KubeBind().V1alpha2().APIServiceNamespaces(), namespaceInformer, @@ -167,7 +168,7 @@ func NewController( serviceBindingIndexer: serviceBindingInformer.Informer().GetIndexer(), clusterbindingCtrl: clusterbindingCtrl, - namespacedeletionCtrl: namespacedeletionCtrl, + namespacelifecycleCtrl: namespacelifecycleCtrl, servicebindingCtrl: servicebindingCtrl, serviceresourcebindingCtrl: serviceexportCtrl, }, nil @@ -194,7 +195,7 @@ type controller struct { factories []SharedInformerFactory clusterbindingCtrl GenericController - namespacedeletionCtrl GenericController + namespacelifecycleCtrl GenericController servicebindingCtrl GenericController serviceresourcebindingCtrl GenericController } @@ -248,7 +249,7 @@ func (c *controller) Start(ctx context.Context) { }) go c.clusterbindingCtrl.Start(ctx, 2) - go c.namespacedeletionCtrl.Start(ctx, 2) + go c.namespacelifecycleCtrl.Start(ctx, 2) go c.servicebindingCtrl.Start(ctx, 2) go c.serviceresourcebindingCtrl.Start(ctx, 2) diff --git a/pkg/konnector/controllers/cluster/namespacedeletion/namespacedeletion_controller.go b/pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go similarity index 62% rename from pkg/konnector/controllers/cluster/namespacedeletion/namespacedeletion_controller.go rename to pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go index 3f0ffb6be..e59d9203e 100644 --- a/pkg/konnector/controllers/cluster/namespacedeletion/namespacedeletion_controller.go +++ b/pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package namespacedeletion +package namespacelifecycle import ( "context" @@ -41,12 +41,13 @@ import ( ) const ( - controllerName = "kube-bind-konnector-namespacedeletion" + controllerName = "kube-bind-konnector-namespacelifecycle" ) -// NewController returns a new controller deleting old ServiceNamespaces. +// NewController returns a new controller lifecycle for ServiceNamespaces and Namespaces. func NewController( - config *rest.Config, + providerConfig *rest.Config, + consumerConfig *rest.Config, providerNamespace string, serviceNamespaceInformer bindinformers.APIServiceNamespaceInformer, namespaceInformer dynamic.Informer[corelisters.NamespaceLister], @@ -55,14 +56,17 @@ func NewController( logger := klog.Background().WithValues("controller", controllerName) - config = rest.CopyConfig(config) - config = rest.AddUserAgent(config, controllerName) + providerConfig = rest.CopyConfig(providerConfig) + providerConfig = rest.AddUserAgent(providerConfig, controllerName) - bindClient, err := bindclient.NewForConfig(config) + consumerConfig = rest.CopyConfig(consumerConfig) + consumerConfig = rest.AddUserAgent(consumerConfig, controllerName) + + bindClient, err := bindclient.NewForConfig(providerConfig) if err != nil { return nil, err } - kubeClient, err := kubernetesclient.NewForConfig(config) + consumerKubeClient, err := kubernetesclient.NewForConfig(consumerConfig) if err != nil { return nil, err } @@ -70,8 +74,8 @@ func NewController( c := &controller{ queue: queue, - bindClient: bindClient, - kubeClient: kubeClient, + providerBindClient: bindClient, + consumerKubeClient: consumerKubeClient, serviceNamespaceLister: serviceNamespaceInformer.Lister(), serviceNamespaceIndexer: serviceNamespaceInformer.Informer().GetIndexer(), @@ -112,8 +116,8 @@ func NewController( type controller struct { queue workqueue.TypedRateLimitingInterface[string] - bindClient bindclient.Interface - kubeClient kubernetesclient.Interface + providerBindClient bindclient.Interface + consumerKubeClient kubernetesclient.Interface namespaceInformer dynamic.Informer[corelisters.NamespaceLister] @@ -217,14 +221,87 @@ func (c *controller) process(ctx context.Context, key string) error { return nil // we cannot do anything } - if _, err := c.getNamespace(name); err != nil && !errors.IsNotFound(err) { + current, err := c.getServiceNamespace(snsNamespace, name) + if err != nil && !errors.IsNotFound(err) { + return err + } + + switch { + case errors.IsNotFound(err): + return c.handleNamespaceDeletion(ctx, snsNamespace, name) + default: + return c.handleNamespaceLifecycle(ctx, current) + } +} + +// handleNamespaceDeletion ensures that when an APIServiceNamespace is deleted, +// the corresponding Namespace is also deleted, does not matter who owns it. +func (c *controller) handleNamespaceDeletion(ctx context.Context, snsNamespace, name string) error { + logger := klog.FromContext(ctx).WithValues("APIServiceNamespace", fmt.Sprintf("%s/%s", snsNamespace, name)) + logger.Info("handling deletion of APIServiceNamespace") + + _, err := c.getNamespace(name) + if err != nil && !errors.IsNotFound(err) { return err } else if errors.IsNotFound(err) { - if err := c.deleteServiceNamespace(ctx, snsNamespace, name); err != nil && !errors.IsNotFound(err) { - return err - } + // Namespace already deleted, lets check who owns return nil } + // We should not delete namespaces in this case as we can't be sure who owns it. + // This might leave hanging namespaces, but it's safer than deleting something we don't own. + logger.Info("not deleting Namespace as we can't be sure who owns it") + return nil +} +func (c *controller) handleNamespaceLifecycle(ctx context.Context, current *kubebindv1alpha2.APIServiceNamespace) error { + logger := klog.FromContext(ctx).WithValues("APIServiceNamespace", fmt.Sprintf("%s/%s", current.Namespace, current.Name)) + logger.Info("handling creation of APIServiceNamespace") + + // At this point we know that the APIServiceNamespace exists, and we need to see who owns it and + // act accordingly. + // If provider owned, ensure Namespace exists. If consumer owned, and local namespace exists, delete it. + consumerOwned := isConsumerOwned(current) + + switch consumerOwned { + case true: + // Consumer owned, ensure Namespace is deleted if exists + _, err := c.getNamespace(current.Name) + if errors.IsNotFound(err) { + // No local namespace - delete APIServiceNamespace if exists. + return c.deleteServiceNamespace(ctx, current.Namespace, current.Name) + } else if err != nil { + return err + } + case false: + _, err := c.getNamespace(current.Name) + if errors.IsNotFound(err) { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: current.Name, + Labels: map[string]string{ + kubebindv1alpha2.ObjectOwnerLabel: kubebindv1alpha2.OwnerProvider.String(), + }, + }, + } + if _, err := c.consumerKubeClient.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}); err != nil { + return err + } + logger.Info("created Namespace for APIServiceNamespace") + return nil + } else if err != nil { + return err + } + } return nil } + +// isConsumerOwned checks if the APIServiceNamespace is owned by consumer. +// If the label is missing, it is considered consumer owned. +func isConsumerOwned(sns *kubebindv1alpha2.APIServiceNamespace) bool { + switch sns.Labels[kubebindv1alpha2.ObjectOwnerLabel] { + case kubebindv1alpha2.OwnerProvider.String(): + return false + default: + return true + } +} diff --git a/sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go b/sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go index 2237c787b..5cd389bdb 100644 --- a/sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go +++ b/sdk/apis/kubebind/v1alpha2/apiserviceexport_types.go @@ -24,6 +24,10 @@ import ( const ( SourceSpecHashAnnotationKey = "kube-bind.io/source-spec-hash" + + // Label is used to indicate which side owns the resource. By changing the label value, + // ownership can be transferred. + ObjectOwnerLabel = "kube-bind.io/owner" ) const ( diff --git a/sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go b/sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go index 5c3ad11b2..d95e589e0 100644 --- a/sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go +++ b/sdk/apis/kubebind/v1alpha2/apiserviceexportrequest_types.go @@ -115,6 +115,23 @@ type APIServiceExportRequestSpec struct { // // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="permissionClaims are immutable" PermissionClaims []PermissionClaim `json:"permissionClaims,omitempty"` + + // namespaces specifies the namespaces to bootstrap as part of this request. + // When objects originate from provider side, the consumer does not always know the necessary details. + // This field allows provider to pre-heat the necessary namespaces on provider side by creating + // APIServiceNamespace objects attached to the APIServiceExport. More namespaces can be created later by the consumer. + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="namespaces are immutable" + // +optional + // +kubebuilder:validation:Optional + Namespaces []Namespaces `json:"namespaces,omitempty"` +} + +type Namespaces struct { + // name is the name of the namespace to create on provider side. + // + // +required + // +kubebuilder:validation:Required + Name string `json:"name"` } type APIServiceExportRequestResource struct { diff --git a/sdk/apis/kubebind/v1alpha2/helpers/namespaces.go b/sdk/apis/kubebind/v1alpha2/helpers/namespaces.go new file mode 100644 index 000000000..c055d36a2 --- /dev/null +++ b/sdk/apis/kubebind/v1alpha2/helpers/namespaces.go @@ -0,0 +1,39 @@ +/* +Copyright 2025 The Kube Bind Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helpers + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2" +) + +func APIServiceNamespaceFromExport(export *kubebindv1alpha2.APIServiceExport, namespaceName string) *kubebindv1alpha2.APIServiceNamespace { + return &kubebindv1alpha2.APIServiceNamespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + Namespace: export.Namespace, + Labels: map[string]string{ + kubebindv1alpha2.ObjectOwnerLabel: kubebindv1alpha2.OwnerProvider.String(), + }, + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(export, kubebindv1alpha2.SchemeGroupVersion.WithKind("APIServiceExport")), + }, + }, + Spec: kubebindv1alpha2.APIServiceNamespaceSpec{}, + } +} diff --git a/sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go b/sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go index f952ef835..eee7f0729 100644 --- a/sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go +++ b/sdk/apis/kubebind/v1alpha2/zz_generated.deepcopy.go @@ -392,6 +392,11 @@ func (in *APIServiceExportRequestSpec) DeepCopyInto(out *APIServiceExportRequest (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Namespaces != nil { + in, out := &in.Namespaces, &out.Namespaces + *out = make([]Namespaces, len(*in)) + copy(*out, *in) + } return } @@ -1083,6 +1088,22 @@ func (in *NamedResource) DeepCopy() *NamedResource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Namespaces) DeepCopyInto(out *Namespaces) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Namespaces. +func (in *Namespaces) DeepCopy() *Namespaces { + if in == nil { + return nil + } + out := new(Namespaces) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OAuth2CodeGrant) DeepCopyInto(out *OAuth2CodeGrant) { *out = *in diff --git a/test/e2e/bind/happy-case_test.go b/test/e2e/bind/happy-case_test.go index 4bfb2d6f4..59b934f77 100644 --- a/test/e2e/bind/happy-case_test.go +++ b/test/e2e/bind/happy-case_test.go @@ -37,6 +37,7 @@ import ( "k8s.io/client-go/util/retry" "sigs.k8s.io/yaml" + kuberesources "github.com/kube-bind/kube-bind/backend/kubernetes/resources" clusterscoped "github.com/kube-bind/kube-bind/pkg/konnector/controllers/cluster/serviceexport/cluster-scoped" kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2" providerfixtures "github.com/kube-bind/kube-bind/test/e2e/bind/fixtures/provider" @@ -148,6 +149,12 @@ spec: var request kubebindv1alpha2.APIServiceExportRequest err := json.Unmarshal(inv.Stdin, &request) require.NoError(t, err) + // Pre-seed provider side namespaces for secret management + request.Spec.Namespaces = []kubebindv1alpha2.Namespaces{ + { + Name: "consumer-secrets-ns", + }, + } request.Spec.PermissionClaims = []kubebindv1alpha2.PermissionClaim{ { GroupResource: kubebindv1alpha2.GroupResource{ @@ -373,6 +380,93 @@ spec: } }, }, + { + name: "verify provider side namespace pre-seeding and RBAC management", + step: func(t *testing.T) { + if !withPermissionClaims { + t.Skip("Skipping provider side namespace test when permission claims are disabled") + return + } + + t.Logf("Verifying APIServiceNamespace was created from pre-seeded namespace spec") + var foundPreSeededNamespace bool + require.Eventually(t, func() bool { + namespaces, err := providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(providerNS).List(ctx, metav1.ListOptions{}) + if err != nil { + return false + } + for _, ns := range namespaces.Items { + if ns.Name == "consumer-secrets-ns" && ns.Status.Namespace != "" { + foundPreSeededNamespace = true + return true + } + } + return false + }, wait.ForeverTestTimeout, time.Millisecond*100, "waiting for pre-seeded APIServiceNamespace to be created on provider side") + require.True(t, foundPreSeededNamespace, "Pre-seeded namespace 'consumer-secrets-ns' should be created via APIServiceExportRequest.Spec.Namespaces") + + t.Logf("Verifying provider side physical namespace exists for pre-seeded namespace") + var actualProviderNamespace string + namespaces, err := providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(providerNS).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + for _, ns := range namespaces.Items { + if ns.Name == "consumer-secrets-ns" { + actualProviderNamespace = ns.Status.Namespace + break + } + } + require.NotEmpty(t, actualProviderNamespace, "Pre-seeded namespace should have a physical namespace assigned") + + providerCoreClient := framework.KubeClient(t, providerConfig).CoreV1() + _, err = providerCoreClient.Namespaces().Get(ctx, actualProviderNamespace, metav1.GetOptions{}) + require.NoError(t, err, "Physical provider side namespace should exist") + + if informerScope == kubebindv1alpha2.ClusterScope { + t.Logf("Verifying RBAC resources were created for secret management in cluster scope") + rbacClient := framework.KubeClient(t, providerConfig).RbacV1() + + clusterRoles, err := rbacClient.ClusterRoles().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + var foundSecretClusterRole bool + for _, cr := range clusterRoles.Items { + if strings.Contains(cr.Name, "kube-binder-") && strings.Contains(cr.Name, "-export-") { + for _, rule := range cr.Rules { + for _, resource := range rule.Resources { + if resource == "secrets" { + foundSecretClusterRole = true + require.Contains(t, rule.Verbs, "*", "ClusterRole should have * permissions for secrets") + require.Contains(t, rule.APIGroups, "", "ClusterRole should target core API group") + break + } + } + } + } + } + require.True(t, foundSecretClusterRole, "ClusterRole for secrets should be created") + + t.Logf("Verifying ClusterRoleBinding was created for pre-seeded namespace secret access") + clusterRoleBindings, err := rbacClient.ClusterRoleBindings().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + var foundSecretClusterRoleBinding bool + for _, crb := range clusterRoleBindings.Items { + if strings.Contains(crb.Name, "kube-binder-") && strings.Contains(crb.Name, "-export-") { + for _, subject := range crb.Subjects { + if subject.Kind == "ServiceAccount" && subject.Name == kuberesources.ServiceAccountName { + foundSecretClusterRoleBinding = true + require.Equal(t, "ClusterRole", crb.RoleRef.Kind, "Should reference ClusterRole") + break + } + } + } + } + require.True(t, foundSecretClusterRoleBinding, "ClusterRoleBinding for ServiceAccount should be created") + } + + t.Logf("Provider side namespace pre-seeding and secret management RBAC verified successfully") + }, + }, { name: "verify secrets and configmaps are synced to provider", step: func(t *testing.T) { From fa7fd4f58565fea629c1cc6cd71ad9e569d01051 Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Tue, 21 Oct 2025 14:46:14 +0300 Subject: [PATCH 3/4] Update test & simplify logic --- .../clusterbinding_controller.go | 5 +- .../servicenamespace_reconcile.go | 4 +- contrib/kcp/README.md | 4 +- .../serviceexport/spec/spec_controller.go | 7 +- test/e2e/bind/happy-case_test.go | 357 +++++++++--------- test/e2e/framework/kcp.go | 7 + 6 files changed, 209 insertions(+), 175 deletions(-) diff --git a/backend/controllers/clusterbinding/clusterbinding_controller.go b/backend/controllers/clusterbinding/clusterbinding_controller.go index 029ff22de..d524d8c54 100644 --- a/backend/controllers/clusterbinding/clusterbinding_controller.go +++ b/backend/controllers/clusterbinding/clusterbinding_controller.go @@ -229,7 +229,10 @@ func mapAPIResourceSchema(clusterName string, cl cluster.Cluster) handler.TypedE return []mcreconcile.Request{ { Request: reconcile.Request{ - NamespacedName: client.ObjectKeyFromObject(serviceExport), + NamespacedName: types.NamespacedName{ + Namespace: serviceExport.GetNamespace(), + Name: "cluster", + }, }, ClusterName: clusterName, }, diff --git a/backend/controllers/servicenamespace/servicenamespace_reconcile.go b/backend/controllers/servicenamespace/servicenamespace_reconcile.go index fe408588e..b916efe56 100644 --- a/backend/controllers/servicenamespace/servicenamespace_reconcile.go +++ b/backend/controllers/servicenamespace/servicenamespace_reconcile.go @@ -83,7 +83,7 @@ func (c *reconciler) reconcile(ctx context.Context, client client.Client, cache } for _, export := range apiServiceExports.Items { - name := fmt.Sprintf("kube-binder-%s-export-%s", sns.Name, export.Name) // per-sns unique name + name := fmt.Sprintf("kube-binder-export-%s-%s", sns.Name, export.Name) // per-sns unique name permissions := []rbacv1.PolicyRule{} for _, claim := range export.Spec.PermissionClaims { permissions = append(permissions, rbacv1.PolicyRule{ @@ -106,7 +106,7 @@ func (c *reconciler) reconcile(ctx context.Context, client client.Client, cache Rules: permissions, } // Create new ClusterRole - if err := client.Create(ctx, role); !errors.IsAlreadyExists(err) { + if err := client.Create(ctx, role); err != nil && !errors.IsAlreadyExists(err) { return fmt.Errorf("failed to create ClusterRole %s: %w", name, err) } } else { diff --git a/contrib/kcp/README.md b/contrib/kcp/README.md index 2c70e571c..79d9b2239 100644 --- a/contrib/kcp/README.md +++ b/contrib/kcp/README.md @@ -104,7 +104,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/1d5vpxvdpy0opbj1 +# cluster Ready https://192.168.2.166:6443/clusters/1f4roigyt6meiaf8 ``` ## Consumer @@ -120,7 +120,7 @@ kubectl ws create consumer --enter 10. Bind the thing: ```bash -./bin/kubectl-bind http://127.0.0.1:8080/clusters/1d5vpxvdpy0opbj1/exports --dry-run -o yaml > apiserviceexport.yaml +./bin/kubectl-bind http://127.0.0.1:8080/clusters/1f4roigyt6meiaf8/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` diff --git a/pkg/konnector/controllers/cluster/serviceexport/spec/spec_controller.go b/pkg/konnector/controllers/cluster/serviceexport/spec/spec_controller.go index db794909b..aef040809 100644 --- a/pkg/konnector/controllers/cluster/serviceexport/spec/spec_controller.go +++ b/pkg/konnector/controllers/cluster/serviceexport/spec/spec_controller.go @@ -129,7 +129,12 @@ func NewController( }, createProviderObject: func(ctx context.Context, obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { if ns := obj.GetNamespace(); ns != "" { - return providerClient.Resource(gvr).Namespace(obj.GetNamespace()).Create(ctx, obj, metav1.CreateOptions{}) + copy := obj.DeepCopy() + err := clusterscoped.InjectClusterNs(copy, providerNamespace, providerNamespaceUID) + if err != nil { + return nil, err + } + return providerClient.Resource(gvr).Namespace(copy.GetNamespace()).Create(ctx, copy, metav1.CreateOptions{}) } err := clusterscoped.TranslateFromDownstream(obj, providerNamespace, providerNamespaceUID) if err != nil { diff --git a/test/e2e/bind/happy-case_test.go b/test/e2e/bind/happy-case_test.go index 59b934f77..43306c430 100644 --- a/test/e2e/bind/happy-case_test.go +++ b/test/e2e/bind/happy-case_test.go @@ -46,32 +46,33 @@ import ( func TestClusterScoped(t *testing.T) { t.Parallel() - testHappyCase(t, apiextensionsv1.ClusterScoped, kubebindv1alpha2.ClusterScope, false) - testHappyCase(t, apiextensionsv1.ClusterScoped, kubebindv1alpha2.ClusterScope, true) + // name & test type defined by letters - cc - cluster-cluster so its easier to identify failures in the logs. + testHappyCase(t, "cc", apiextensionsv1.ClusterScoped, kubebindv1alpha2.ClusterScope) } func TestNamespacedScoped(t *testing.T) { t.Parallel() - testHappyCase(t, apiextensionsv1.NamespaceScoped, kubebindv1alpha2.NamespacedScope, false) - testHappyCase(t, apiextensionsv1.NamespaceScoped, kubebindv1alpha2.NamespacedScope, true) - testHappyCase(t, apiextensionsv1.NamespaceScoped, kubebindv1alpha2.ClusterScope, false) - testHappyCase(t, apiextensionsv1.NamespaceScoped, kubebindv1alpha2.ClusterScope, true) + testHappyCase(t, "nn", apiextensionsv1.NamespaceScoped, kubebindv1alpha2.NamespacedScope) + testHappyCase(t, "nc", apiextensionsv1.NamespaceScoped, kubebindv1alpha2.ClusterScope) } func testHappyCase( t *testing.T, + name string, resourceScope apiextensionsv1.ResourceScope, informerScope kubebindv1alpha2.InformerScope, - withPermissionClaims bool, ) { ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) + // t.Cleanup(cancel) // Commented out to prevent cleanup of kcp assets + _ = cancel // Prevent unused variable warning framework.StartDex(t) + suffix := framework.RandomString(4) + t.Logf("Creating provider workspace") - providerConfig, providerKubeconfig := framework.NewWorkspace(t, framework.ClientConfig(t), framework.WithGenerateName("test-happy-case-provider")) + providerConfig, providerKubeconfig := framework.NewWorkspace(t, framework.ClientConfig(t), framework.WithName("%s-provider-%s", name, suffix)) t.Logf("Installing kubebind CRDs") framework.InstallKubebindCRDs(t, providerConfig) @@ -83,7 +84,7 @@ func testHappyCase( providerfixtures.Bootstrap(t, framework.DiscoveryClient(t, providerConfig), framework.DynamicClient(t, providerConfig), nil) t.Logf("Creating consumer workspace and starting konnector") - consumerConfig, consumerKubeconfig := framework.NewWorkspace(t, framework.ClientConfig(t), framework.WithGenerateName("test-happy-case-consumer")) + consumerConfig, consumerKubeconfig := framework.NewWorkspace(t, framework.ClientConfig(t), framework.WithName("%s-consumer-%s", name, suffix)) framework.StartKonnector(t, consumerConfig, "--kubeconfig="+consumerKubeconfig) serviceGVR := schema.GroupVersionResource{Group: "mangodb.com", Version: "v1alpha1", Resource: "mangodbs"} @@ -144,65 +145,65 @@ spec: inv := <-invocations requireEqualSlicePattern(t, []string{"apiservice", "--remote-kubeconfig-namespace", "*", "--remote-kubeconfig-name", "*", "-f", "-", "--kubeconfig=" + consumerKubeconfig, "--skip-konnector=true", "--no-banner"}, inv.Args) + var request kubebindv1alpha2.APIServiceExportRequest + err := json.Unmarshal(inv.Stdin, &request) + require.NoError(t, err) + // Pre-seed provider side namespaces for secret management + request.Spec.Namespaces = []kubebindv1alpha2.Namespaces{ + { + Name: "consumer-secrets-ns", + }, + } + // If we are in permissions claims mode - add configmaps & secrets - if withPermissionClaims { - var request kubebindv1alpha2.APIServiceExportRequest - err := json.Unmarshal(inv.Stdin, &request) - require.NoError(t, err) - // Pre-seed provider side namespaces for secret management - request.Spec.Namespaces = []kubebindv1alpha2.Namespaces{ - { - Name: "consumer-secrets-ns", + request.Spec.PermissionClaims = []kubebindv1alpha2.PermissionClaim{ + { + GroupResource: kubebindv1alpha2.GroupResource{ + Group: "", + Resource: "configmaps", }, - } - request.Spec.PermissionClaims = []kubebindv1alpha2.PermissionClaim{ - { - GroupResource: kubebindv1alpha2.GroupResource{ - Group: "", - Resource: "configmaps", - }, - Selector: kubebindv1alpha2.Selector{ - NamedResource: []kubebindv1alpha2.NamedResource{ - { - Name: "named-configmap-only", - Namespace: consumerNS, - }, + Selector: kubebindv1alpha2.Selector{ + NamedResource: []kubebindv1alpha2.NamedResource{ + { + Name: "named-configmap-only", + Namespace: consumerNS, }, }, }, - { - GroupResource: kubebindv1alpha2.GroupResource{ - Group: "", - Resource: "secrets", + }, + { + GroupResource: kubebindv1alpha2.GroupResource{ + Group: "", + Resource: "secrets", + }, + Selector: kubebindv1alpha2.Selector{ + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "secrets", + }, }, - Selector: kubebindv1alpha2.Selector{ - LabelSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "secrets", - }, + NamedResource: []kubebindv1alpha2.NamedResource{ + { + Name: "test-secret", + Namespace: consumerNS, + }, + { + Name: "named-secret-1", + Namespace: consumerNS, }, - NamedResource: []kubebindv1alpha2.NamedResource{ - { - Name: "test-secret", - Namespace: consumerNS, - }, - { - Name: "named-secret-1", - Namespace: consumerNS, - }, - { - Name: "named-secret-2", - Namespace: consumerNS, - }, + { + Name: "named-secret-2", + Namespace: consumerNS, }, }, }, - } - payload, err := json.Marshal(request) - require.NoError(t, err) - inv.Stdin = payload + }, } + payload, err := json.Marshal(request) + require.NoError(t, err) + inv.Stdin = payload + framework.BindAPIService(t, inv.Stdin, "", inv.Args...) t.Logf("Waiting for %s CRD to be created on consumer side", serviceGVR.Resource) @@ -241,20 +242,132 @@ spec: // these are used everywhere further down providerNS = instances.Items[0].GetNamespace() - if resourceScope == apiextensionsv1.ClusterScoped { - clusterNs, _ = clusterscoped.ExtractClusterNs(&instances.Items[0]) - clusterScopedUpInsName = clusterscoped.Prepend("test", clusterNs) - } + // Cluster namespace represent binding contract namespace, and is stored on every object. + clusterNs, _ = clusterscoped.ExtractClusterNs(&instances.Items[0]) + clusterScopedUpInsName = clusterscoped.Prepend("test", clusterNs) }, }, + // Request included namespace, so we check it first { - name: "create secrets and configmaps if permission claims enabled", + name: "verify provider side namespace pre-seeding and RBAC management", step: func(t *testing.T) { - if !withPermissionClaims { - t.Skip("Skipping permission claims test when permission claims are disabled") - return + t.Logf("Verifying APIServiceNamespace was created from pre-seeded namespace spec") + var foundPreSeededNamespace bool + var actualProviderNamespace string + require.Eventually(t, func() bool { + // If are operating namespaced resources - namespace will be set, if cluster - we need to use + // extraced one from the cluster-scoped object. + namespaces, err := providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(clusterNs).List(ctx, metav1.ListOptions{}) + if err != nil { + return false + } + + for _, ns := range namespaces.Items { + if ns.Name == "consumer-secrets-ns" && ns.Status.Namespace != "" { + actualProviderNamespace = ns.Status.Namespace + foundPreSeededNamespace = true + return true + } + } + return false + }, wait.ForeverTestTimeout, time.Millisecond*100, "waiting for pre-seeded APIServiceNamespace to be created on provider side") + require.True(t, foundPreSeededNamespace, "Pre-seeded namespace 'consumer-secrets-ns' should be created via APIServiceExportRequest.Spec.Namespaces") + + providerCoreClient := framework.KubeClient(t, providerConfig).CoreV1() + _, err := providerCoreClient.Namespaces().Get(ctx, actualProviderNamespace, metav1.GetOptions{}) + require.NoError(t, err, "Physical provider side namespace should exist") + + switch informerScope { + case kubebindv1alpha2.ClusterScope: + t.Logf("Verifying RBAC resources were created for secret management in cluster scope") + rbacClient := framework.KubeClient(t, providerConfig).RbacV1() + + clusterRoles, err := rbacClient.ClusterRoles().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + var foundSecretClusterRole bool + for _, cr := range clusterRoles.Items { + if strings.Contains(cr.Name, "kube-binder-") && strings.Contains(cr.Name, "-export-") { + for _, rule := range cr.Rules { + for _, resource := range rule.Resources { + if resource == "secrets" { + foundSecretClusterRole = true + require.Contains(t, rule.Verbs, "*", "ClusterRole should have * permissions for secrets") + require.Contains(t, rule.APIGroups, "", "ClusterRole should target core API group") + break + } + } + } + } + } + require.True(t, foundSecretClusterRole, "ClusterRole for secrets should be created") + + t.Logf("Verifying ClusterRoleBinding was created for pre-seeded namespace secret access") + clusterRoleBindings, err := rbacClient.ClusterRoleBindings().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + var foundSecretClusterRoleBinding bool + for _, crb := range clusterRoleBindings.Items { + if strings.Contains(crb.Name, "kube-binder-export") { + for _, subject := range crb.Subjects { + if subject.Kind == "ServiceAccount" && subject.Name == kuberesources.ServiceAccountName { + foundSecretClusterRoleBinding = true + require.Equal(t, "ClusterRole", crb.RoleRef.Kind, "Should reference ClusterRole") + break + } + } + } + } + require.True(t, foundSecretClusterRoleBinding, "ClusterRoleBinding for ServiceAccount should be created") + case kubebindv1alpha2.NamespacedScope: + t.Logf("Verifying RBAC resources were created for secret management in namespace scope") + rbacClient := framework.KubeClient(t, providerConfig).RbacV1() + + roles, err := rbacClient.Roles(providerNS).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + var foundSecretRole bool + for _, cr := range roles.Items { + if strings.Contains(cr.Name, "kube-binder-export") { + for _, rule := range cr.Rules { + for _, resource := range rule.Resources { + if resource == "secrets" { + foundSecretRole = true + require.Contains(t, rule.Verbs, "*", "Role should have * permissions for secrets") + require.Contains(t, rule.APIGroups, "", "Role should target core API group") + break + } + } + } + } + } + require.True(t, foundSecretRole, "Role for secrets should be created") + + t.Logf("Verifying RoleBinding was created for pre-seeded namespace secret access") + roleBindings, err := rbacClient.RoleBindings(providerNS).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + var foundSecretRoleBinding bool + for _, crb := range roleBindings.Items { + if strings.Contains(crb.Name, "kube-binder-") && strings.Contains(crb.Name, "-export-") { + for _, subject := range crb.Subjects { + if subject.Kind == "ServiceAccount" && subject.Name == kuberesources.ServiceAccountName { + foundSecretRoleBinding = true + require.Equal(t, "Role", crb.RoleRef.Kind, "Should reference Role") + break + } + } + } + } + require.True(t, foundSecretRoleBinding, "RoleBinding for ServiceAccount should be created") } + t.Logf("Provider side namespace pre-seeding and secret management RBAC verified successfully") + }, + }, + { + name: "create secrets and configmaps if permission claims enabled", + step: func(t *testing.T) { t.Logf("Creating named-only configmap on consumer side") namedConfigMapData := map[string]string{ "named-config.yaml": "named: value", @@ -356,125 +469,36 @@ spec: step: func(t *testing.T) { // We need to establish namespace only in cluster scope for cluster scoped resources. // Else we can trust sync object namespace as it will be the same. - if withPermissionClaims && - informerScope == kubebindv1alpha2.ClusterScope && + if informerScope == kubebindv1alpha2.ClusterScope && resourceScope == apiextensionsv1.ClusterScoped { if providerNS == "unknown" { t.Fatal("providerNS is not set. Programming error in the test.") } - var namespaces *kubebindv1alpha2.APIServiceNamespaceList - t.Logf("Waiting for APIServiceNamespace to be created on provider side") + t.Logf("Waiting for APIServiceNamespace to be created on provider side: %s", clusterNs) require.Eventually(t, func() bool { var err error - namespaces, err = providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(providerNS).List(ctx, metav1.ListOptions{}) + namespaces, err := providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(clusterNs).List(ctx, metav1.ListOptions{}) if err != nil { return false } - return len(namespaces.Items) == 1 && namespaces.Items[0].Status.Namespace != "" - }, wait.ForeverTestTimeout, time.Millisecond*100, "waiting for APIServiceNamespace to be created on provider side") - - providerNS = namespaces.Items[0].Status.Namespace - require.NotEmpty(t, providerNS, "No cluster namespaces found") - } - }, - }, - { - name: "verify provider side namespace pre-seeding and RBAC management", - step: func(t *testing.T) { - if !withPermissionClaims { - t.Skip("Skipping provider side namespace test when permission claims are disabled") - return - } - - t.Logf("Verifying APIServiceNamespace was created from pre-seeded namespace spec") - var foundPreSeededNamespace bool - require.Eventually(t, func() bool { - namespaces, err := providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(providerNS).List(ctx, metav1.ListOptions{}) - if err != nil { - return false - } - for _, ns := range namespaces.Items { - if ns.Name == "consumer-secrets-ns" && ns.Status.Namespace != "" { - foundPreSeededNamespace = true - return true - } - } - return false - }, wait.ForeverTestTimeout, time.Millisecond*100, "waiting for pre-seeded APIServiceNamespace to be created on provider side") - require.True(t, foundPreSeededNamespace, "Pre-seeded namespace 'consumer-secrets-ns' should be created via APIServiceExportRequest.Spec.Namespaces") - - t.Logf("Verifying provider side physical namespace exists for pre-seeded namespace") - var actualProviderNamespace string - namespaces, err := providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(providerNS).List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - for _, ns := range namespaces.Items { - if ns.Name == "consumer-secrets-ns" { - actualProviderNamespace = ns.Status.Namespace - break - } - } - require.NotEmpty(t, actualProviderNamespace, "Pre-seeded namespace should have a physical namespace assigned") - - providerCoreClient := framework.KubeClient(t, providerConfig).CoreV1() - _, err = providerCoreClient.Namespaces().Get(ctx, actualProviderNamespace, metav1.GetOptions{}) - require.NoError(t, err, "Physical provider side namespace should exist") - - if informerScope == kubebindv1alpha2.ClusterScope { - t.Logf("Verifying RBAC resources were created for secret management in cluster scope") - rbacClient := framework.KubeClient(t, providerConfig).RbacV1() - - clusterRoles, err := rbacClient.ClusterRoles().List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - - var foundSecretClusterRole bool - for _, cr := range clusterRoles.Items { - if strings.Contains(cr.Name, "kube-binder-") && strings.Contains(cr.Name, "-export-") { - for _, rule := range cr.Rules { - for _, resource := range rule.Resources { - if resource == "secrets" { - foundSecretClusterRole = true - require.Contains(t, rule.Verbs, "*", "ClusterRole should have * permissions for secrets") - require.Contains(t, rule.APIGroups, "", "ClusterRole should target core API group") - break - } - } + for _, namespace := range namespaces.Items { + if strings.Contains(namespace.Name, consumerNS) && namespace.Status.Namespace != "" { + providerNS = namespace.Status.Namespace + return true } } - } - require.True(t, foundSecretClusterRole, "ClusterRole for secrets should be created") - t.Logf("Verifying ClusterRoleBinding was created for pre-seeded namespace secret access") - clusterRoleBindings, err := rbacClient.ClusterRoleBindings().List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - - var foundSecretClusterRoleBinding bool - for _, crb := range clusterRoleBindings.Items { - if strings.Contains(crb.Name, "kube-binder-") && strings.Contains(crb.Name, "-export-") { - for _, subject := range crb.Subjects { - if subject.Kind == "ServiceAccount" && subject.Name == kuberesources.ServiceAccountName { - foundSecretClusterRoleBinding = true - require.Equal(t, "ClusterRole", crb.RoleRef.Kind, "Should reference ClusterRole") - break - } - } - } - } - require.True(t, foundSecretClusterRoleBinding, "ClusterRoleBinding for ServiceAccount should be created") + return false + }, wait.ForeverTestTimeout, time.Millisecond*100, "waiting for APIServiceNamespace to be created on provider side") + require.NotEmpty(t, providerNS, "No cluster namespaces found") } - - t.Logf("Provider side namespace pre-seeding and secret management RBAC verified successfully") }, }, { name: "verify secrets and configmaps are synced to provider", step: func(t *testing.T) { - if !withPermissionClaims { - t.Skip("Skipping permission claims test when permission claims are disabled") - return - } - t.Logf("Waiting for named-only configmap to be synced to provider side") require.Eventually(t, func() bool { _, err := providerCoreClient.ConfigMaps(providerNS).Get(ctx, "named-configmap-only", metav1.GetOptions{}) @@ -531,11 +555,6 @@ spec: { name: "verify secrets and configmaps are deleted when removed from consumer", step: func(t *testing.T) { - if !withPermissionClaims { - t.Skip("Skipping permission claims test when permission claims are disabled") - return - } - t.Logf("Deleting named-only configmap from consumer side") err := consumerCoreClient.ConfigMaps(consumerNS).Delete(ctx, "named-configmap-only", metav1.DeleteOptions{}) require.NoError(t, err) diff --git a/test/e2e/framework/kcp.go b/test/e2e/framework/kcp.go index 84be8e8ae..d89c8a3a4 100644 --- a/test/e2e/framework/kcp.go +++ b/test/e2e/framework/kcp.go @@ -63,6 +63,13 @@ func WithName(s string, formatArgs ...any) ClusterWorkspaceOption { } } +func RandomString(length int) string { + token := make([]byte, length) + rand.Read(token) //nolint:errcheck + base36hash := strings.ToLower(base36.EncodeBytes(token)) + return base36hash[:length+1] +} + func WithGenerateName(s string, formatArgs ...any) ClusterWorkspaceOption { return func(ws *tenancyv1alpha1.Workspace) { s = fmt.Sprintf(s, formatArgs...) From 26aed569f05a15532a4484c3c1712765323a282f Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Tue, 21 Oct 2025 15:34:23 +0300 Subject: [PATCH 4/4] review nits --- .../serviceexportrequest_reconcile.go | 4 ++-- contrib/kcp/README.md | 5 +--- .../namespacelifecycle_controller.go | 24 ++++++------------- test/e2e/bind/happy-case_test.go | 3 +-- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go b/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go index 71bf2806f..59c3584ef 100644 --- a/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go +++ b/backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go @@ -378,14 +378,14 @@ func (r *reconciler) ensureAPIServiceNamespaces(ctx context.Context, cl client.C // TODO(mjudeikis): We have this object above already, pass it down to avoid extra get. export := &kubebindv1alpha2.APIServiceExport{} - if err := cl.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: req.Name}, export); err != nil { + if err := cl.Get(ctx, client.ObjectKeyFromObject(req), export); err != nil { return fmt.Errorf("failed to get APIServiceExport %s/%s: %w", req.Namespace, req.Name, err) } for _, ns := range req.Spec.Namespaces { apiServiceNamespace := helpers.APIServiceNamespaceFromExport(export, ns.Name) currentAPIServiceNamespace := &kubebindv1alpha2.APIServiceNamespace{} - err := cache.Get(ctx, client.ObjectKey{Namespace: apiServiceNamespace.Namespace, Name: apiServiceNamespace.Name}, currentAPIServiceNamespace) + err := cache.Get(ctx, client.ObjectKeyFromObject(apiServiceNamespace), currentAPIServiceNamespace) if err != nil { if apierrors.IsNotFound(err) { logger.V(1).Info("Creating APIServiceNamespace", "name", apiServiceNamespace.Name, "namespace", apiServiceNamespace.Namespace) diff --git a/contrib/kcp/README.md b/contrib/kcp/README.md index 79d9b2239..686437b46 100644 --- a/contrib/kcp/README.md +++ b/contrib/kcp/README.md @@ -104,7 +104,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/1f4roigyt6meiaf8 +# cluster Ready https://192.168.2.166:6443/clusters/1f4roigyt6meiaf8 ``` ## Consumer @@ -169,9 +169,6 @@ kubectl apply -f contrib/kcp/deploy/examples/cowboy.yaml kubectl apply -f contrib/kcp/deploy/examples/sheriff.yaml ``` - - - ## Debug ```bash diff --git a/pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go b/pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go index e59d9203e..7edc6172b 100644 --- a/pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go +++ b/pkg/konnector/controllers/cluster/namespacelifecycle/namespacelifecycle_controller.go @@ -221,15 +221,11 @@ func (c *controller) process(ctx context.Context, key string) error { return nil // we cannot do anything } - current, err := c.getServiceNamespace(snsNamespace, name) - if err != nil && !errors.IsNotFound(err) { - return err - } - - switch { - case errors.IsNotFound(err): + if current, err := c.getServiceNamespace(snsNamespace, name); errors.IsNotFound(err) { return c.handleNamespaceDeletion(ctx, snsNamespace, name) - default: + } else if err != nil { + return err + } else { return c.handleNamespaceLifecycle(ctx, current) } } @@ -262,8 +258,7 @@ func (c *controller) handleNamespaceLifecycle(ctx context.Context, current *kube // If provider owned, ensure Namespace exists. If consumer owned, and local namespace exists, delete it. consumerOwned := isConsumerOwned(current) - switch consumerOwned { - case true: + if consumerOwned { // Consumer owned, ensure Namespace is deleted if exists _, err := c.getNamespace(current.Name) if errors.IsNotFound(err) { @@ -272,7 +267,7 @@ func (c *controller) handleNamespaceLifecycle(ctx context.Context, current *kube } else if err != nil { return err } - case false: + } else { _, err := c.getNamespace(current.Name) if errors.IsNotFound(err) { ns := &corev1.Namespace{ @@ -298,10 +293,5 @@ func (c *controller) handleNamespaceLifecycle(ctx context.Context, current *kube // isConsumerOwned checks if the APIServiceNamespace is owned by consumer. // If the label is missing, it is considered consumer owned. func isConsumerOwned(sns *kubebindv1alpha2.APIServiceNamespace) bool { - switch sns.Labels[kubebindv1alpha2.ObjectOwnerLabel] { - case kubebindv1alpha2.OwnerProvider.String(): - return false - default: - return true - } + return sns.Labels[kubebindv1alpha2.ObjectOwnerLabel] != kubebindv1alpha2.OwnerProvider.String() } diff --git a/test/e2e/bind/happy-case_test.go b/test/e2e/bind/happy-case_test.go index 43306c430..780937e55 100644 --- a/test/e2e/bind/happy-case_test.go +++ b/test/e2e/bind/happy-case_test.go @@ -64,8 +64,7 @@ func testHappyCase( informerScope kubebindv1alpha2.InformerScope, ) { ctx, cancel := context.WithCancel(context.Background()) - // t.Cleanup(cancel) // Commented out to prevent cleanup of kcp assets - _ = cancel // Prevent unused variable warning + t.Cleanup(cancel) // Commented out to prevent cleanup of kcp assets framework.StartDex(t)