From 5449b9d0459d83a685abe54830864de8f5a90296 Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Tue, 12 May 2026 08:54:52 +0300 Subject: [PATCH] Add RBAC permissions controller for kcp Signed-off-by: Mangirdas Judeikis On-behalf-of: @SAP mangirdas.judeikis@sap.com --- Makefile | 2 + .../apibindingtemplate/controller.go | 2 +- .../apiresourceschema/controller.go | 2 +- backend/provider/kcp/controllers/shared/vw.go | 57 ++-- .../kcp/controllers/vwrbac/controller.go | 296 ++++++++++++++++++ backend/server.go | 11 + 6 files changed, 346 insertions(+), 24 deletions(-) create mode 100644 backend/provider/kcp/controllers/vwrbac/controller.go diff --git a/Makefile b/Makefile index 72970c063..de54516da 100644 --- a/Makefile +++ b/Makefile @@ -435,6 +435,8 @@ image-local: export PLATFORMS ?= linux/$(shell go env GOARCH) image-local: ## Build images locally (single platform, --load) for the current commit @LDFLAGS="$(LDFLAGS)" hack/build-image.sh +# Dev example: +# make IMAGE_REPO=ghcr.io/ IMAGE_TAGS=v$(date -u +%Y%m%d) image-push .PHONY: image-push image-push: export PLATFORMS = $(IMAGE_PUSH_PLATFORMS) image-push: export PUSH = true diff --git a/backend/provider/kcp/controllers/apibindingtemplate/controller.go b/backend/provider/kcp/controllers/apibindingtemplate/controller.go index 28e3c8926..8e2d45515 100644 --- a/backend/provider/kcp/controllers/apibindingtemplate/controller.go +++ b/backend/provider/kcp/controllers/apibindingtemplate/controller.go @@ -73,7 +73,7 @@ func New( opts: opts, ignorePrefixes: ignorePrefixes, scheme: scheme, - vwCache: shared.NewVWClientCache(baseConfig, scheme), + vwCache: shared.NewVWClientCache(baseConfig), } return r, nil diff --git a/backend/provider/kcp/controllers/apiresourceschema/controller.go b/backend/provider/kcp/controllers/apiresourceschema/controller.go index 6055a94df..7bbaeaa3f 100644 --- a/backend/provider/kcp/controllers/apiresourceschema/controller.go +++ b/backend/provider/kcp/controllers/apiresourceschema/controller.go @@ -63,7 +63,7 @@ func New( manager: mgr, opts: opts, scheme: scheme, - vwCache: shared.NewVWClientCache(baseConfig, scheme), + vwCache: shared.NewVWClientCache(baseConfig), }, nil } diff --git a/backend/provider/kcp/controllers/shared/vw.go b/backend/provider/kcp/controllers/shared/vw.go index 4286c543c..90379a41e 100644 --- a/backend/provider/kcp/controllers/shared/vw.go +++ b/backend/provider/kcp/controllers/shared/vw.go @@ -25,8 +25,9 @@ import ( "sync" apisv1alpha1 "github.com/kcp-dev/sdk/apis/apis/v1alpha1" + apisv1alpha1client "github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha1" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -57,27 +58,27 @@ func ExtractClusterID(clusterConfig *rest.Config) (string, error) { return pathParts[5], nil } -// VWClientCache caches controller-runtime clients keyed by cluster ID to avoid -// creating a new client on every schema lookup. +// VWClientCache caches typed apis.kcp.io clients keyed by cluster ID. The +// clients target the apiresourceschema virtual workspace and skip discovery +// (which the VW does not fully serve), so we use the typed kcp REST client +// rather than controller-runtime's client.Client. type VWClientCache struct { mu sync.RWMutex - clients map[string]client.Client + clients map[string]apisv1alpha1client.ApisV1alpha1Interface baseConfig *rest.Config - scheme *runtime.Scheme } // NewVWClientCache creates a new VWClientCache. -func NewVWClientCache(baseConfig *rest.Config, scheme *runtime.Scheme) *VWClientCache { +func NewVWClientCache(baseConfig *rest.Config) *VWClientCache { return &VWClientCache{ - clients: make(map[string]client.Client), + clients: make(map[string]apisv1alpha1client.ApisV1alpha1Interface), baseConfig: baseConfig, - scheme: scheme, } } // GetClient returns a cached client for the given cluster ID, creating one if // necessary. -func (c *VWClientCache) GetClient(clusterID string) (client.Client, error) { +func (c *VWClientCache) GetClient(clusterID string) (apisv1alpha1client.ApisV1alpha1Interface, error) { c.mu.RLock() if cl, ok := c.clients[clusterID]; ok { c.mu.RUnlock() @@ -93,7 +94,7 @@ func (c *VWClientCache) GetClient(clusterID string) (client.Client, error) { return cl, nil } - cl, err := newVWClient(c.baseConfig, c.scheme, clusterID) + cl, err := newVWClient(c.baseConfig, clusterID) if err != nil { return nil, err } @@ -101,20 +102,32 @@ func (c *VWClientCache) GetClient(clusterID string) (client.Client, error) { return cl, nil } -// newVWClient creates a client pointing at the apiresourceschema virtual workspace -// for the given cluster ID: -// https://host:port/services/apiresourceschema/{clusterID}/clusters/*/ -func newVWClient(baseConfig *rest.Config, scheme *runtime.Scheme, clusterID string) (client.Client, error) { +// newVWClient creates a typed apis.kcp.io client pointing at the +// apiresourceschema virtual workspace for the given cluster ID: +// +// https://host:port/services/apiresourceschema/{clusterID}/clusters/{clusterID} +// +// Two important details: +// +// 1. We replace the path on baseConfig.Host (which typically points at a +// workspace such as .../clusters/) rather than appending — otherwise +// the URL ends up with two /clusters/ segments and never matches the VW root. +// +// 2. The kcp VW resolver accepts either "*" or the literal consumer cluster +// name in the /clusters// segment (see kcp pkg/virtual/apiresourceschema/ +// builder/build.go digestURL). We use the literal name because client-go's +// REST request builder percent-encodes "*" to "%2A" when constructing the +// final URL, and the resolver compares the path segment against the literal +// string "*" — so a wildcard never matches in practice. +func newVWClient(baseConfig *rest.Config, clusterID string) (apisv1alpha1client.ApisV1alpha1Interface, error) { cfg := rest.CopyConfig(baseConfig) - u, err := url.Parse(cfg.Host) + u, err := url.Parse(baseConfig.Host) if err != nil { return nil, fmt.Errorf("failed to parse base config host: %w", err) } + cfg.Host = fmt.Sprintf("%s://%s/services/apiresourceschema/%s/clusters/%s", u.Scheme, u.Host, clusterID, clusterID) - u.Path = fmt.Sprintf("/services/apiresourceschema/%s/clusters/*", clusterID) - cfg.Host = u.String() - - return client.New(cfg, client.Options{Scheme: scheme}) + return apisv1alpha1client.NewForConfig(cfg) } // SchemaGetterWithFallback returns a function that first tries to get an @@ -151,11 +164,11 @@ func SchemaGetterWithFallback( return nil, fmt.Errorf("failed to get VW client for cluster %q: %w", clusterID, err) } - var vwSchema apisv1alpha1.APIResourceSchema - if err := vwClient.Get(ctx, client.ObjectKey{Name: name}, &vwSchema); err != nil { + vwSchema, err := vwClient.APIResourceSchemas().Get(ctx, name, metav1.GetOptions{}) + if err != nil { return nil, fmt.Errorf("APIResourceSchema %q not found in workspace or VW: %w", name, err) } - return &vwSchema, nil + return vwSchema, nil } } diff --git a/backend/provider/kcp/controllers/vwrbac/controller.go b/backend/provider/kcp/controllers/vwrbac/controller.go new file mode 100644 index 000000000..b895d9db0 --- /dev/null +++ b/backend/provider/kcp/controllers/vwrbac/controller.go @@ -0,0 +1,296 @@ +/* +Copyright 2026 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 vwrbac is a TEMPORARY controller that bootstraps RBAC required by +// the kcp apiresourceschema virtual workspace into every consumer workspace +// that has a kube-bind APIBinding. The VW authorizer requires the caller to +// have read access to apibindings.apis.kcp.io in the consumer workspace +// before it will return APIResourceSchemas (see +// kcp/pkg/virtual/apiresourceschema/authorizer/authorizer.go). The kube-bind +// backend SA does not have that by default, so we plant a ClusterRole + +// ClusterRoleBinding once per consumer workspace. +// +// This package should be removed once kcp grants apiresourceschema VW access +// directly to the APIExport owner identity. +package vwrbac + +import ( + "context" + "fmt" + "strings" + + apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2" + authenticationv1 "k8s.io/api/authentication/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/log" + mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder" + mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" +) + +// kcpClusterNameExtraKey is the extra-key kcp injects on requests authenticated +// as a kcp-native ServiceAccount. Its value is the SA's home logical cluster +// name. It's the marker that lets us turn the bare SA username into the kcp +// "global SA" identity used for cross-workspace RBAC. +const kcpClusterNameExtraKey = "authentication.kcp.io/cluster-name" + +const ( + controllerName = "kube-bind-kcp-vwrbac" + + // roleName is the deterministic name for the ClusterRole + ClusterRoleBinding + // installed in every consumer workspace. + roleName = "kube-bind-vw-apibinding-reader" +) + +// VWRBACReconciler watches APIBindings across consumer workspaces and ensures +// each workspace has a ClusterRole + ClusterRoleBinding allowing the kube-bind +// backend identity to read apibindings.apis.kcp.io. This is what unlocks the +// kcp apiresourceschema VW for the backend. +type VWRBACReconciler struct { + manager mcmanager.Manager + opts controller.TypedOptions[mcreconcile.Request] + subject rbacv1.Subject +} + +// New returns a new VWRBACReconciler. It performs a SelfSubjectReview against +// baseConfig to discover the identity the backend will use against consumer +// workspaces, so the bootstrapped RBAC binds the right user. +func New( + ctx context.Context, + mgr mcmanager.Manager, + opts controller.TypedOptions[mcreconcile.Request], + baseConfig *rest.Config, +) (*VWRBACReconciler, error) { + subject, err := discoverSubject(ctx, baseConfig) + if err != nil { + return nil, fmt.Errorf("vwrbac: failed to discover backend identity: %w", err) + } + + log.FromContext(ctx).Info("vwrbac: bootstrapping consumer-workspace RBAC for identity", + "kind", subject.Kind, "name", subject.Name) + + return &VWRBACReconciler{ + manager: mgr, + opts: opts, + subject: subject, + }, nil +} + +// discoverSubject calls SelfSubjectReview against the supplied config and maps +// the resulting identity into the RBAC subject that consumer-workspace +// ClusterRoleBindings must use. +// +// Two cases: +// +// 1. kcp-native ServiceAccount (the recommended setup): the JWT carries the +// SA's home logical cluster, which kcp surfaces as the +// "authentication.kcp.io/cluster-name" extra. Cross-workspace RBAC binds +// it via the "global SA" identity: +// +// system:kcp:serviceaccount::: +// +// bound with Kind: User. See kcp e2e +// test/e2e/virtual/apiresourceschema/virtualworkspace_test.go. +// +// 2. Foreign identity (e.g. a local Kubernetes SA presented via OIDC, or any +// other User): use the bare username as Kind: User. kcp will not resolve +// it as a ServiceAccount object — `Kind: ServiceAccount` would never +// match. +func discoverSubject(ctx context.Context, cfg *rest.Config) (rbacv1.Subject, error) { + kc, err := kubernetes.NewForConfig(cfg) + if err != nil { + return rbacv1.Subject{}, err + } + rev, err := kc.AuthenticationV1().SelfSubjectReviews().Create( + ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return rbacv1.Subject{}, fmt.Errorf("SelfSubjectReview: %w", err) + } + + user := rev.Status.UserInfo.Username + if user == "" { + return rbacv1.Subject{}, fmt.Errorf("SelfSubjectReview returned empty username") + } + + name := user + const saPrefix = "system:serviceaccount:" + if clusters := rev.Status.UserInfo.Extra[kcpClusterNameExtraKey]; len(clusters) > 0 && strings.HasPrefix(user, saPrefix) { + nsName := strings.TrimPrefix(user, saPrefix) // ":" + name = fmt.Sprintf("system:kcp:serviceaccount:%s:%s", clusters[0], nsName) + } + + return rbacv1.Subject{ + Kind: rbacv1.UserKind, + APIGroup: rbacv1.GroupName, + Name: name, + }, nil +} + +// Reconcile ensures the bootstrap ClusterRole + ClusterRoleBinding exist in +// the consumer workspace where the APIBinding lives. It does not need to read +// the binding's contents — the existence of any binding in the cluster is the +// trigger. +func (r *VWRBACReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) { + logger := log.FromContext(ctx).WithValues("cluster", req.ClusterName, "binding", req.Name) + + cl, err := r.manager.GetCluster(ctx, req.ClusterName) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get client for cluster %q: %w", req.ClusterName, err) + } + c := cl.GetClient() + + if err := ensureClusterRole(ctx, c); err != nil { + return ctrl.Result{}, err + } + if err := ensureClusterRoleBinding(ctx, c, r.subject); err != nil { + return ctrl.Result{}, err + } + + logger.V(2).Info("vwrbac: bootstrap RBAC ensured") + return ctrl.Result{}, nil +} + +func ensureClusterRole(ctx context.Context, c client.Client) error { + desired := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: roleName}, + Rules: []rbacv1.PolicyRule{ + { + // Workspace access — required by kcp's workspaceContentAuthorizer + // for foreign / cross-workspace SAs to even enter the workspace. + NonResourceURLs: []string{"/"}, + Verbs: []string{"access"}, + }, + { + // The "invitation" the apiresourceschema VW authorizer checks. + APIGroups: []string{apisv1alpha2.SchemeGroupVersion.Group}, + Resources: []string{"apibindings"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + } + + existing := &rbacv1.ClusterRole{} + err := c.Get(ctx, client.ObjectKey{Name: roleName}, existing) + if errors.IsNotFound(err) { + if cerr := c.Create(ctx, desired); cerr != nil && !errors.IsAlreadyExists(cerr) { + return fmt.Errorf("create ClusterRole %q: %w", roleName, cerr) + } + return nil + } + if err != nil { + return fmt.Errorf("get ClusterRole %q: %w", roleName, err) + } + + // Update only if rules drifted. + if rulesEqual(existing.Rules, desired.Rules) { + return nil + } + existing.Rules = desired.Rules + if err := c.Update(ctx, existing); err != nil { + return fmt.Errorf("update ClusterRole %q: %w", roleName, err) + } + return nil +} + +func ensureClusterRoleBinding(ctx context.Context, c client.Client, subject rbacv1.Subject) error { + desired := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: roleName}, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: roleName, + }, + Subjects: []rbacv1.Subject{subject}, + } + + existing := &rbacv1.ClusterRoleBinding{} + err := c.Get(ctx, client.ObjectKey{Name: roleName}, existing) + if errors.IsNotFound(err) { + if cerr := c.Create(ctx, desired); cerr != nil && !errors.IsAlreadyExists(cerr) { + return fmt.Errorf("create ClusterRoleBinding %q: %w", roleName, cerr) + } + return nil + } + if err != nil { + return fmt.Errorf("get ClusterRoleBinding %q: %w", roleName, err) + } + + // RoleRef is immutable; only fix subjects if they drifted. + if subjectsEqual(existing.Subjects, desired.Subjects) { + return nil + } + existing.Subjects = desired.Subjects + if err := c.Update(ctx, existing); err != nil { + return fmt.Errorf("update ClusterRoleBinding %q: %w", roleName, err) + } + return nil +} + +func rulesEqual(a, b []rbacv1.PolicyRule) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if !stringSliceEqual(a[i].APIGroups, b[i].APIGroups) || + !stringSliceEqual(a[i].Resources, b[i].Resources) || + !stringSliceEqual(a[i].Verbs, b[i].Verbs) || + !stringSliceEqual(a[i].NonResourceURLs, b[i].NonResourceURLs) || + !stringSliceEqual(a[i].ResourceNames, b[i].ResourceNames) { + return false + } + } + return true +} + +func subjectsEqual(a, b []rbacv1.Subject) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func stringSliceEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +// SetupWithManager registers the controller with the multicluster-runtime Manager. +func (r *VWRBACReconciler) SetupWithManager(mgr mcmanager.Manager) error { + return mcbuilder.ControllerManagedBy(mgr). + For(&apisv1alpha2.APIBinding{}). + WithOptions(r.opts). + Named(controllerName). + Complete(r) +} diff --git a/backend/server.go b/backend/server.go index 4f8b75e4d..bd8b2fbf4 100644 --- a/backend/server.go +++ b/backend/server.go @@ -43,6 +43,7 @@ import ( kube "github.com/kube-bind/kube-bind/backend/kubernetes" "github.com/kube-bind/kube-bind/backend/provider/kcp/controllers/apibindingtemplate" "github.com/kube-bind/kube-bind/backend/provider/kcp/controllers/apiresourceschema" + "github.com/kube-bind/kube-bind/backend/provider/kcp/controllers/vwrbac" "github.com/kube-bind/kube-bind/backend/session/memory" "github.com/kube-bind/kube-bind/backend/session/redis" kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2" @@ -72,6 +73,7 @@ type Controllers struct { // Provider specific controllers - only set when provider is "kcp" APIBindingTemplate *apibindingtemplate.APIBindingTemplateReconciler APIResourceSchema *apiresourceschema.APIResourceSchemaReconciler + VWRBAC *vwrbac.VWRBACReconciler } func NewServer(ctx context.Context, c *Config) (*Server, error) { @@ -325,6 +327,15 @@ func NewServer(ctx context.Context, c *Config) (*Server, error) { return nil, fmt.Errorf("error setting up APIResourceSchema controller with manager: %w", err) } logger.Info("APIResourceSchema controller enabled for kcp provider") + + s.VWRBAC, err = vwrbac.New(ctx, s.Config.Manager, opts, c.ClientConfig) + if err != nil { + return nil, fmt.Errorf("error setting up VWRBAC Controller: %w", err) + } + if err := s.VWRBAC.SetupWithManager(s.Config.Manager); err != nil { + return nil, fmt.Errorf("error setting up VWRBAC controller with manager: %w", err) + } + logger.Info("VWRBAC controller enabled (temporary; bootstraps consumer-workspace RBAC for kcp apiresourceschema VW)") } return s, nil