Reworked RBAC machinery for APIServiceExports by gman0 · Pull Request #451 · kbind-dev/kbind

gman0 · 2026-01-28T23:30:18Z

Summary

Added new APIServiceExportRBAC controller for managing APIServiceExport RBACs.

All (Cluster)RoleBindings use the same subject: the <APIServiceExport.Namespace>/kube-binder ServiceAccount.
The main ClusterRole kube-binder-exports aggregates all ClusterRoles of all service exports.
Each APIServiceExport has its own set of ClusterRoles:
- kube-binder-resources-<APIServiceExport.Namespace>-<APIServiceExport.Name> ClusterRole for its exported resources,
- and kube-binder-claims-<APIServiceExport.Namespace>-<APIServiceExport.Name> ClusterRole for its claimed resources.
  - If the informer scope is Cluster: this ClusterRole contains rules for all export's claims.
  - If the informer scope is Namespaced: this ClusterRole contains rules only for cluster-scoped claimed resources.
    - For namespace-scoped claims, a kube-binder-claims-<APIServiceExport.Name> is created in each respective <APIServiceNamespace.Status.Namespace> namespace.

Additionally:

kube-binder ClusterRoleBinding creation/update was moved to /bind handler
Redundant boundschemas and boundschemas/status rules in kube-binder-<ClusterBinding.Namespace> ClusterRole were removed, because they are already present in kube-binder ClusterRole.

What Type of PR Is This?

/kind feature

Related Issue(s)

Fixes #344

Release Notes

Reworked RBAC machinery for APIServiceExports

gman0 · 2026-01-28T23:32:04Z

-			// to be able to figure out what to bind.
-			{
-				APIGroups: []string{kubebindv1alpha2.GroupName},
-				Resources: []string{"boundschemas"},


^ These rules are already part of the kube-binder ClusterRole:

https://github.com/kube-bind/kube-bind/blob/d15b44c34d7d5844482cdb59ff6256912acc9c69/backend/kubernetes/resources/rbac.go#L105-L114

Probably no need to re-create them?

gman0 · 2026-01-28T23:37:21Z

 		return nil, err
 	}

+	err = kuberesources.EnsureBinderRoleBinding(ctx, c, ns, sa.Name)


Previously, kube-binder RoleBinding was created here:

https://github.com/kube-bind/kube-bind/blob/d15b44c34d7d5844482cdb59ff6256912acc9c69/backend/controllers/servicenamespace/servicenamespace_reconcile.go#L251-L275

IMHO it makes sense to move it to here to the /bind handler, since this is the place where kube-binder ClusterRole as well as the kube-bind-<Cluster> namespace is created.

gman0 · 2026-01-29T10:12:23Z

+
+	errs = append(errs,
+		r.ensureAggregatingClusteRoleAndClusterRoleBinding(ctx, client, cache, kubebinderSubject),
+		// We always use cluster-scoped RBACs for exported resources.


RBACs for exported resources always use cluster-scoped permissions. This is how it was before too.

We could in theory use namespaced Roles & RoleBindings for resources too when Isolation==Namespaced && Scope==Namespaced.

does this means that if I have prefix isolation, and claim secrets, I get access to all secrets in the cluster?

@mjudeikis that decision (whether konnector has cluster- or namespace-scoped access for claimed resources) is based on:

Resource claim scope Informer scope What happens

cluster cluster Creates ClusterRole & ClusterRoleBinding

namespace cluster Creates ClusterRole & ClusterRoleBinding

--- --- ---

cluster namespace Creates ClusterRole & ClusterRoleBinding

namespace namespace Creates Role & RoleBinding

So konnector gets namespaced access to Secrets only if the informer scope itself is namespaced.

And this is the behavior we already have btw:

If c.scope == kubebindv1alpha2.ClusterScope:

https://github.com/kube-bind/kube-bind/blob/d84c3f0bf4b68bdb7038117f0d8a5992949f53a2/backend/controllers/servicenamespace/servicenamespace_reconcile.go#L100-L124

... otherwise (namespaced):

https://github.com/kube-bind/kube-bind/blob/d84c3f0bf4b68bdb7038117f0d8a5992949f53a2/backend/controllers/servicenamespace/servicenamespace_reconcile.go#L175-L189

My comment above ( #451 (comment) ) was just a suggestion that we could add (yet another :/) mode of operation -- this time for exported resources (APIServiceExport.Spec.Resources), where also backend isolation would be considered.

Right now exported resources always get cluster-scoped access, unconditionally.

Can we add the table to the docs somewhere?

@mjudeikis Added in 27f9533

ntnn

Otherwise lgtm, but leaving final approve to @mjudeikis

ntnn · 2026-02-02T08:43:40Z

+	err = kuberesources.EnsureBinderRoleBinding(ctx, c, ns, sa.Name)
+	if err != nil {
+		return nil, err
+	}


Suggested change

err = kuberesources.EnsureBinderRoleBinding(ctx, c, ns, sa.Name)

if err != nil {

return nil, err

}

if err := kuberesources.EnsureBinderRoleBinding(ctx, c, ns, sa.Name); err != nil {

return nil, err

}

mjudeikis · 2026-02-02T08:40:18Z

+func (r *APIServiceExportRBACReconciler) SetupWithManager(mgr mcmanager.Manager) error {
+	return mcbuilder.ControllerManagedBy(mgr).
+		For(&kubebindv1alpha2.APIServiceExport{}).
+		Watches(


Fo we need Owns here ?

Actually I've ended up removing Controller: true in the owner reference ( c4472ee ).

I'm not sure Owns() would do much: the owner is the Namespace object, so we wouldn't get notified :/

So that means we don't get notifications on RBACs at all. This is a bit meh, on the other hand we didn't have that before either.

We could add it if it's necessary -- maybe in a follow-up PR? If a care-free admin messes up the RBACs they could trigger a reconcile by nudging the APIServiceExport or APIServiceNamespace so it's not that critical...

mjudeikis · 2026-02-02T08:44:00Z

+
+	errs = append(errs,
+		r.ensureAggregatingClusteRoleAndClusterRoleBinding(ctx, client, cache, kubebinderSubject),
+		// We always use cluster-scoped RBACs for exported resources.


does this means that if I have prefix isolation, and claim secrets, I get access to all secrets in the cluster?

gman0 · 2026-02-02T12:56:42Z

+}
+
+func compareMap[V any](t *testing.T, kind string, a, b map[string]V) {
+	require.Equal(t, slices.Sorted(maps.Keys(a)), slices.Sorted(maps.Keys(b)), "got unexpected entries after %s reconciliation", kind)


Should call t.Helper().

cnvergence · 2026-02-02T20:14:41Z

-		}
-	}
-
-	if err := c.cleanupRBACResources(ctx, client, cache, sns, apiServiceExports); err != nil {


are we dropping cleanup of RBAC resources completely?

All created RBAC-related objects have the owner set to kube-binder-<Consumer's cluster name>. So when that namespace is gone, the GC will do the clean up.

Do you think it's not enough? It's not perfect, I agree: there is one caveat in that if a consumer un-binds an export, they'd retain their RBACs until the ClusterBinding goes away (and with that the whole namespace, which cleans up the RBACs).

The question is if this actually does have any impact, and I'd hazard to say no, because the consumer/konnector could gain these permissions again by simply re-binding the service export.

cnvergence · 2026-02-02T20:22:31Z

+	}
+	if clusterRole == nil {
+		err = r.createClusterRole(ctx, client, &expectedClusterRole)
+		if err != nil && apierrors.IsAlreadyExists(err) {


Suggested change

if err != nil && apierrors.IsAlreadyExists(err) {

if err != nil && !apierrors.IsAlreadyExists(err) {

cnvergence · 2026-02-02T20:39:41Z

+		Rules: rules,
+	}
+
+	role, err := r.getRole(ctx, cache, namespace, name)


Suggested change

role, err := r.getRole(ctx, cache, namespace, name)

role, err := r.getRole(ctx, cache, namespace, name)

if err != nil {

if !apierrors.IsNotFound(err) {

return err

}

if err := r.createRole(ctx, client, &expectedRole); err != nil && !apierrors.IsAlreadyExists(err) {

return err

}

role = &expectedRole

}

let's maybe simplify it to something like

I've moved this whole mess into getOrCreate function: 5d08fa1

Added new APIServiceExportRBAC controller for managing APIServiceExport RBACs. Additionally: * kube-binder ClusterRoleBinding creation/update was moved to /bind handler * Redundant boundschemas and boundschemas/status rules in `kube-binder-<ClusterBinding.Namespace>` ClusterRole were removed, because they are already present in `kube-binder` ClusterRole. On-behalf-of: @SAP robert.vasek@sap.com Signed-off-by: Robert Vasek <robert.vasek@clyso.com>

* Reworked RBAC machinery for APIServiceExports Added new APIServiceExportRBAC controller for managing APIServiceExport RBACs. Additionally: * kube-binder ClusterRoleBinding creation/update was moved to /bind handler * Redundant boundschemas and boundschemas/status rules in `kube-binder-<ClusterBinding.Namespace>` ClusterRole were removed, because they are already present in `kube-binder` ClusterRole. * e2e: adjusted tests for the new APIServiceExport RBAC machinery * cleanup manager * serviceexportrbac reconciler: don't set Controller=True in owner refs * serviceexportrbac reconciler unit test: add missing t.Helper() * code cleaning * added docs into developers/architecture.md On-behalf-of: @SAP robert.vasek@sap.com Signed-off-by: Robert Vasek <robert.vasek@clyso.com>

gman0 requested a review from a team as a code owner January 28, 2026 23:30

gman0 commented Jan 28, 2026

View reviewed changes

gman0 commented Jan 29, 2026

View reviewed changes

ntnn reviewed Feb 2, 2026

View reviewed changes

mjudeikis requested changes Feb 2, 2026

View reviewed changes

gman0 commented Feb 2, 2026

View reviewed changes

gman0 force-pushed the backend-rbac-controller branch 2 times, most recently from 2b5d05c to b426cb5 Compare February 2, 2026 17:52

cnvergence reviewed Feb 2, 2026

View reviewed changes

gman0 added 7 commits February 3, 2026 16:31

e2e: adjusted tests for the new APIServiceExport RBAC machinery

5555534

cleanup manager

456b1c6

serviceexportrbac reconciler: don't set Controller=True in owner refs

9d808bf

serviceexportrbac reconciler unit test: add missing t.Helper()

f6d0978

code cleaning

5d08fa1

added docs into developers/architecture.md

27f9533

gman0 force-pushed the backend-rbac-controller branch from b426cb5 to 27f9533 Compare February 4, 2026 09:21

mjudeikis approved these changes Feb 5, 2026

View reviewed changes

mjudeikis merged commit 8b330f5 into kbind-dev:main Feb 5, 2026
11 checks passed

gman0 deleted the backend-rbac-controller branch February 27, 2026 08:04

Resource claim scope	Informer scope	What happens
cluster	cluster	Creates ClusterRole & ClusterRoleBinding
namespace	cluster	Creates ClusterRole & ClusterRoleBinding
---	---	---
cluster	namespace	Creates ClusterRole & ClusterRoleBinding
namespace	namespace	Creates Role & RoleBinding

	if err != nil && apierrors.IsAlreadyExists(err) {
	if err != nil && !apierrors.IsAlreadyExists(err) {

-	role, err := r.getRole(ctx, cache, namespace, name)
+role, err := r.getRole(ctx, cache, namespace, name)
+if err != nil {
+	if !apierrors.IsNotFound(err) {
+		return err
+	}
+	if err := r.createRole(ctx, client, &expectedRole); err != nil && !apierrors.IsAlreadyExists(err) {
+		return err
+	}
+	role = &expectedRole
+}

Uh oh!

Conversation

gman0 commented Jan 28, 2026 • edited by mjudeikis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Type of PR Is This?

Related Issue(s)

Release Notes

Uh oh!

gman0 Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ntnn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gman0 commented Jan 28, 2026 •

edited by mjudeikis

Loading

gman0 Jan 28, 2026 •

edited

Loading