From 9a0035f5910edf57569fa286c7029e445ba7e77c Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 26 Aug 2026 23:08:22 +0200 Subject: [PATCH 01/14] Allow resource/subresourcce in validation Signed-off-by: Nelo-T. Wallus --- .../kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go | 4 +++- .../kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go index 3e692e1bf9b..7f0433642eb 100644 --- a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go +++ b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go @@ -288,9 +288,11 @@ type GroupResource struct { Group string `json:"group,omitempty"` // resource is the name of the resource. + // A subresource may be claimed as "resource/subresource", + // e.g. "serviceaccounts/token", in the style of RBAC rules. // Note: it is worth noting that you can not ask for permissions for resource provided by a CRD // not provided by an api export. - // +kubebuilder:validation:Pattern=`^[a-z][-a-z0-9]*[a-z0-9]$` + // +kubebuilder:validation:Pattern=`^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$` // +required // +kubebuilder:validation:Required Resource string `json:"resource"` diff --git a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go index 8d8d5d547ee..0626d0dc988 100644 --- a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go +++ b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go @@ -319,10 +319,12 @@ type GroupResource struct { Group string `json:"group,omitempty"` // resource is the name of the resource. + // A subresource may be claimed as "resource/subresource", + // e.g. "serviceaccounts/token", in the style of RBAC rules. // Note: it is worth noting that you can not ask for permissions for resource provided by a CRD // not provided by an api export. // - // +kubebuilder:validation:Pattern=`^[a-z][-a-z0-9]*[a-z0-9]$` + // +kubebuilder:validation:Pattern=`^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$` // +required // +kubebuilder:validation:Required Resource string `json:"resource"` From c4e51af7aa88782120aeeef85657c151938bdd74 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 26 Aug 2026 23:09:06 +0200 Subject: [PATCH 02/14] codegen Signed-off-by: Nelo-T. Wallus --- config/crds/apis.kcp.io_apibindings.yaml | 24 ++++++++++++++----- config/crds/apis.kcp.io_apiexports.yaml | 8 +++++-- .../apis/v1alpha2/groupresource.go | 2 ++ .../sdk/openapi/zz_generated.openapi.go | 8 +++---- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/config/crds/apis.kcp.io_apibindings.yaml b/config/crds/apis.kcp.io_apibindings.yaml index fed2682014f..c53afe814fa 100644 --- a/config/crds/apis.kcp.io_apibindings.yaml +++ b/config/crds/apis.kcp.io_apibindings.yaml @@ -84,9 +84,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: description: resourceSelector is a list of claimed resource @@ -198,9 +200,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: description: resourceSelector is a list of claimed resource @@ -378,9 +382,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: description: resourceSelector is a list of claimed resource @@ -556,9 +562,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string selector: description: |- @@ -764,9 +772,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string selector: description: |- @@ -1042,9 +1052,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string verbs: description: |- diff --git a/config/crds/apis.kcp.io_apiexports.yaml b/config/crds/apis.kcp.io_apiexports.yaml index 2eb4371985b..0797157b961 100644 --- a/config/crds/apis.kcp.io_apiexports.yaml +++ b/config/crds/apis.kcp.io_apiexports.yaml @@ -171,9 +171,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: description: resourceSelector is a list of claimed resource @@ -472,9 +474,11 @@ spec: resource: description: |- resource is the name of the resource. + A subresource may be claimed as "resource/subresource", + e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. - pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string verbs: description: |- diff --git a/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/groupresource.go b/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/groupresource.go index a57a7f55751..b523dc14dfd 100644 --- a/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/groupresource.go +++ b/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/groupresource.go @@ -27,6 +27,8 @@ type GroupResourceApplyConfiguration struct { // For core groups this is the empty string '""'. Group *string `json:"group,omitempty"` // resource is the name of the resource. + // A subresource may be claimed as "resource/subresource", + // e.g. "serviceaccounts/token", in the style of RBAC rules. // Note: it is worth noting that you can not ask for permissions for resource provided by a CRD // not provided by an api export. Resource *string `json:"resource,omitempty"` diff --git a/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go b/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go index 4d22a854de7..d247e5f5f12 100644 --- a/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go +++ b/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go @@ -2364,7 +2364,7 @@ func schema_sdk_apis_apis_v1alpha2_AcceptablePermissionClaim(ref common.Referenc }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", @@ -2588,7 +2588,7 @@ func schema_sdk_apis_apis_v1alpha2_GroupResource(ref common.ReferenceCallback) c }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", @@ -2670,7 +2670,7 @@ func schema_sdk_apis_apis_v1alpha2_PermissionClaim(ref common.ReferenceCallback) }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", @@ -2924,7 +2924,7 @@ func schema_sdk_apis_apis_v1alpha2_ScopedPermissionClaim(ref common.ReferenceCal }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", From 012ebe15dfe8b1e0a762df7cbb2e3d19463ac108 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 26 Aug 2026 23:10:04 +0200 Subject: [PATCH 03/14] Add NamedCreaterFunc Signed-off-by: Nelo-T. Wallus --- .../pkg/forwardingregistry/functions.go | 11 ++++++++++ .../pkg/forwardingregistry/store.go | 21 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/functions.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/functions.go index 2b3cf846dcb..516587ff57c 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/functions.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/functions.go @@ -56,6 +56,17 @@ var _ rest.Creater = &struct { CreaterFunc }{} +type NamedCreaterFunc func(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) + +func (f NamedCreaterFunc) Create(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { + return f(ctx, name, obj, createValidation, options) +} + +var _ rest.NamedCreater = &struct { + FactoryFunc + NamedCreaterFunc +}{} + type GetterFunc func(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) func (f GetterFunc) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/store.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/store.go index 3eeebf03374..fd17dce5bb5 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/store.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/store.go @@ -49,6 +49,7 @@ type StoreFuncs struct { GetterFunc CreaterFunc + NamedCreaterFunc GracefulDeleterFunc CollectionDeleterFunc ListerFunc @@ -112,6 +113,26 @@ func DefaultDynamicDelegatedStoreFuncs( return delegate.Create(ctx, unstructuredObj, *options, subResources...) } + s.NamedCreaterFunc = func(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { + unstructuredObj, ok := obj.(*unstructured.Unstructured) + if !ok { + return nil, fmt.Errorf("not an Unstructured: %T", obj) + } + + if err := createValidation(ctx, obj); err != nil { + return nil, err + } + + delegate, err := client(ctx) + if err != nil { + return nil, err + } + + unstructuredObj = unstructuredObj.DeepCopy() + unstructuredObj.SetName(name) + + return delegate.Create(ctx, unstructuredObj, *options, subResources...) + } s.GracefulDeleterFunc = func(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { delegate, err := client(ctx) if err != nil { From 0381bad4cd5bb862f3eb27336075ba5738db1f0a Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 26 Aug 2026 23:14:52 +0200 Subject: [PATCH 04/14] Serve all subresources Signed-off-by: Nelo-T. Wallus --- .../pkg/dynamic/apiserver/handler.go | 16 ++- .../pkg/dynamic/apiserver/serving_info.go | 105 ++++++++++++------ 2 files changed, 80 insertions(+), 41 deletions(-) diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/handler.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/handler.go index bf8eae3746e..f314c05221d 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/handler.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/handler.go @@ -202,12 +202,11 @@ func (r *resourceHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } var handlerFunc http.HandlerFunc - subresources := apiResourceVersion.Subresources switch { - case subresource == "status" && subresources.Status != nil: - handlerFunc = r.serveStatus(w, req, requestInfo, apiDef, supportedTypes) case len(subresource) == 0: handlerFunc = r.serveResource(w, req, requestInfo, apiDef, supportedTypes) + case apiDef.GetSubResourceStorage(subresource) != nil: + handlerFunc = r.serveSubResource(w, req, requestInfo, apiDef, supportedTypes) default: responsewriters.ErrorNegotiated( apierrors.NewNotFound(schema.GroupResource{Group: requestInfo.APIGroup, Resource: requestInfo.Resource}, requestInfo.Name), @@ -275,15 +274,20 @@ func (r *resourceHandler) serveResource(w http.ResponseWriter, req *http.Request return nil } -func (r *resourceHandler) serveStatus(w http.ResponseWriter, req *http.Request, requestInfo *apirequest.RequestInfo, apiDef apidefinition.APIDefinition, supportedTypes []string) http.HandlerFunc { - requestScope := apiDef.GetSubResourceRequestScope("status") - storage := apiDef.GetSubResourceStorage("status") +func (r *resourceHandler) serveSubResource(w http.ResponseWriter, req *http.Request, requestInfo *apirequest.RequestInfo, apiDef apidefinition.APIDefinition, supportedTypes []string) http.HandlerFunc { + requestScope := apiDef.GetSubResourceRequestScope(requestInfo.Subresource) + storage := apiDef.GetSubResourceStorage(requestInfo.Subresource) switch requestInfo.Verb { case "get": if storage, isAble := storage.(rest.Getter); isAble { return handlers.GetResource(storage, requestScope) } + case "create": + if storage, isAble := storage.(rest.NamedCreater); isAble { + // Using CreateNamedResource since a subresource is created against a named parent and CreateResource would drop the name passed to .Create. + return handlers.CreateNamedResource(storage, requestScope, r.admission) + } case "update": if storage, isAble := storage.(rest.Updater); isAble { return handlers.UpdateResource(storage, requestScope, r.admission) diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go index 6abedb89e9f..38e67e09c22 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go @@ -19,6 +19,8 @@ package apiserver import ( "fmt" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" + apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver" @@ -264,40 +266,79 @@ func CreateServingInfoFor(genericConfig genericapiserver.CompletedConfig, apiRes return nil, fmt.Errorf("storage for resource %q should define GetResetFields", gvk.String()) } - var statusScope handlers.RequestScope - statusStorage, statusEnabled := subresourceStorages["status"] - if statusEnabled { + subresourceScopes := map[string]*handlers.RequestScope{} + for subresource, subresourceStorage := range subresourceStorages { + // A subresource may produce a gvk different from its parent (e.g. ServiceAccount and TokenRequest). + // Default to the gvk (e.g. status) but override with the gvk the storage builds. + subresourceGVK := gvk + if obj := subresourceStorage.New(); obj != nil { + if objGVK := obj.GetObjectKind().GroupVersionKind(); !objGVK.Empty() { + subresourceGVK = objGVK + } + } + // shallow copy - statusScope = *requestScope - statusScope.Subresource = "status" - statusScope.Namer = handlers.ContextBasedNaming{ + subresourceScope := *requestScope + subresourceScope.Subresource = subresource + subresourceScope.Namer = handlers.ContextBasedNaming{ Namer: runtime.Namer(meta.NewAccessor()), ClusterScoped: clusterScoped, } - if withResetFields, canGetResetFields := statusStorage.(rest.ResetFieldsStrategy); canGetResetFields { - resetFields := withResetFields.GetResetFields() - statusScope, err = apiextensionsapiserver.ScopeWithFieldManager( - typeConverter, - statusScope, - resetFields, - "status", + subresourceTypeConverter := typeConverter + if subresourceGVK != gvk { + // If the GKVs of resource and subresource differ the + // request scope needs to be adjusted to handle the + // subresource GVK correctly. + subresourceScope.Kind = subresourceGVK + subresourceScope.HubGroupVersion = subresourceGVK.GroupVersion() + subresourceScope.Serializer = apiextensionsapiserver.NewUnstructuredNegotiatedSerializer( + typer, + creator, + safeConverter, + nil, + subresourceGVK.GroupKind(), + true, ) - if err != nil { - return nil, err - } - } else { + subresourceScope.Defaulter = apiextensionsapiserver.NewUnstructuredDefaulter( + parameterScheme, + nil, + subresourceGVK.GroupKind(), + ) + + // Register the subresource in the registry. + equivalentResourceRegistry.RegisterKindFor(gvr, subresource, subresourceGVK) + // And build a new type converter for the subresource. + subresourceTypeConverter = managedfields.NewDeducedTypeConverter() + } + + var resetFields map[fieldpath.APIVersion]*fieldpath.Set + if withResetFields, canGetResetFields := subresourceStorage.(rest.ResetFieldsStrategy); canGetResetFields { + resetFields = withResetFields.GetResetFields() + } else if subresource == "status" { return nil, fmt.Errorf("storage for resource %q status should define GetResetFields", gvk.String()) } + + subresourceScope, err = apiextensionsapiserver.ScopeWithFieldManager( + subresourceTypeConverter, + subresourceScope, + resetFields, + subresource, + ) + if err != nil { + return nil, err + } + + subresourceScopes[subresource] = &subresourceScope } ret := &servingInfo{ - apiResourceSchema: apiResourceSchema, - storage: storage, - statusStorage: statusStorage, - requestScope: requestScope, - statusRequestScope: &statusScope, - logicalClusterName: logicalcluster.From(apiResourceSchema), + apiResourceSchema: apiResourceSchema, + storage: storage, + subresourceStorages: subresourceStorages, + requestScope: requestScope, + subresourceScopes: subresourceScopes, + logicalClusterName: logicalcluster.From(apiResourceSchema), } return ret, nil @@ -308,11 +349,11 @@ type servingInfo struct { logicalClusterName logicalcluster.Name apiResourceSchema *apisv1alpha1.APIResourceSchema - storage rest.Storage - statusStorage rest.Storage + storage rest.Storage + subresourceStorages map[string]rest.Storage - requestScope *handlers.RequestScope - statusRequestScope *handlers.RequestScope + requestScope *handlers.RequestScope + subresourceScopes map[string]*handlers.RequestScope } // Implement APIDefinition interface @@ -327,19 +368,13 @@ func (apiDef *servingInfo) GetStorage() rest.Storage { return apiDef.storage } func (apiDef *servingInfo) GetSubResourceStorage(subresource string) rest.Storage { - if subresource == "status" { - return apiDef.statusStorage - } - return nil + return apiDef.subresourceStorages[subresource] } func (apiDef *servingInfo) GetRequestScope() *handlers.RequestScope { return apiDef.requestScope } func (apiDef *servingInfo) GetSubResourceRequestScope(subresource string) *handlers.RequestScope { - if subresource == "status" { - return apiDef.statusRequestScope - } - return nil + return apiDef.subresourceScopes[subresource] } func (apiDef *servingInfo) TearDown() { } From f4aebc278cf707d3de56196fa7bfb114bb69d10c Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 26 Aug 2026 23:17:21 +0200 Subject: [PATCH 05/14] Handle subresources in claims Signed-off-by: Nelo-T. Wallus --- .../permissionclaimlabel_reconcile.go | 8 +- .../permissionclaimlabel_reconcile_test.go | 6 +- pkg/virtual/apiexport/authorizer/binding.go | 11 +- .../apiexport/authorizer/binding_test.go | 234 ++++++++++++++++++ pkg/virtual/apiexport/builder/forwarding.go | 49 ++++ .../apiexport_apireconciler_reconcile.go | 6 + .../apiexport/schemas/builtin/builtin.go | 28 ++- 7 files changed, 332 insertions(+), 10 deletions(-) diff --git a/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile.go b/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile.go index 7f8c2d413f5..0b4969d5587 100644 --- a/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile.go +++ b/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile.go @@ -117,6 +117,10 @@ func (c *controller) reconcile(ctx context.Context, apiBinding *apisv1alpha2.API for _, s := range sets.List[string](allChanges) { claim := claimFromSetKey(s) + // the claim references a subresource in rbac-style, the parent label applies + if strings.Contains(claim.Resource, "/") { + continue + } if _, nonPersisted := permissionclaim.NonPersistedResourcesClaimable[schema.GroupResource{Group: claim.Group, Resource: claim.Resource}]; nonPersisted { continue } @@ -297,11 +301,11 @@ func (c *controller) reconcile(ctx context.Context, apiBinding *apisv1alpha2.API } func setKeyForClaim(claim apisv1alpha2.PermissionClaim) string { - return fmt.Sprintf("%s/%s/%s", claim.Resource, claim.Group, claim.IdentityHash) + return fmt.Sprintf("%s|%s|%s", claim.Resource, claim.Group, claim.IdentityHash) } func claimFromSetKey(key string) apisv1alpha2.PermissionClaim { - parts := strings.SplitN(key, "/", 3) + parts := strings.SplitN(key, "|", 3) return apisv1alpha2.PermissionClaim{ GroupResource: apisv1alpha2.GroupResource{ Group: parts[1], diff --git a/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile_test.go b/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile_test.go index 5425caaea61..901863be89a 100644 --- a/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile_test.go +++ b/pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile_test.go @@ -43,7 +43,7 @@ func TestClaimSetKeys(t *testing.T) { }, IdentityHash: "", }, - key: "configmaps//", + key: "configmaps||", }, "non-core built-in gr": { claim: apisv1alpha2.PermissionClaim{ @@ -53,7 +53,7 @@ func TestClaimSetKeys(t *testing.T) { }, IdentityHash: "", }, - key: "roles/rbac.authorization.k8s.io/", + key: "roles|rbac.authorization.k8s.io|", }, "3rd party gr + hash": { claim: apisv1alpha2.PermissionClaim{ @@ -63,7 +63,7 @@ func TestClaimSetKeys(t *testing.T) { }, IdentityHash: "hash", }, - key: "apibindings/apis.kcp.io/hash", + key: "apibindings|apis.kcp.io|hash", }, } diff --git a/pkg/virtual/apiexport/authorizer/binding.go b/pkg/virtual/apiexport/authorizer/binding.go index d2c90fee18b..b950c1ce447 100644 --- a/pkg/virtual/apiexport/authorizer/binding.go +++ b/pkg/virtual/apiexport/authorizer/binding.go @@ -131,20 +131,25 @@ func (a *boundAPIAuthorizer) Authorize(ctx context.Context, attr authorizer.Attr // check if a resource claim for this resource has been accepted and has correct verbs. // normalize the requested group/resource to handle the events.k8s.io ↔ core/v1 equivalence. normalizedGR := permissionclaim.NormalizeEventGroupResource(schema.GroupResource{Group: attr.GetAPIGroup(), Resource: attr.GetResource()}) + // subresource status is implicitly granted with its parent resource. + // others must be claimed explicitly + claimedResource := normalizedGR.Resource + if sub := attr.GetSubresource(); sub != "" && sub != "status" { + claimedResource = normalizedGR.Resource + "/" + sub + } for _, permissionClaim := range apiBinding.Spec.PermissionClaims { if permissionClaim.State != apisv1alpha2.ClaimAccepted { // if the claim is not accepted it cannot be used. continue } - if permissionClaim.Group == normalizedGR.Group && permissionClaim.Resource == normalizedGR.Resource { + if permissionClaim.Group == normalizedGR.Group && permissionClaim.Resource == claimedResource { apiBindingVerbs := sets.New(permissionClaim.Verbs...) - apiExportVerbs := sets.New[string]() + apiExportVerbs := sets.New[string]() for _, exportPermpermissionClaim := range apiExport.Spec.PermissionClaims { if exportPermpermissionClaim.EqualGRI(permissionClaim.PermissionClaim) { apiExportVerbs.Insert(exportPermpermissionClaim.Verbs...) - break } } diff --git a/pkg/virtual/apiexport/authorizer/binding_test.go b/pkg/virtual/apiexport/authorizer/binding_test.go index 67041da63d6..781c968c871 100644 --- a/pkg/virtual/apiexport/authorizer/binding_test.go +++ b/pkg/virtual/apiexport/authorizer/binding_test.go @@ -695,6 +695,240 @@ func TestBoundAPIAuthorizer(t *testing.T) { }, expectedDecision: authorizer.DecisionAllow, }, + { + name: "create token subresource claimed by both APIExport and APIBinding", + attr: &authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + APIGroup: "", + Resource: "serviceaccounts", + Subresource: "token", + Verb: "create", + }, + apidomainKey: apidomainKey, + getAPIBindingByExport: func(clusterName, apiExportName, apiExportCluster string) (*apisv1alpha2.APIBinding, error) { + return &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIBindingSpec{ + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts/token", + }, + Verbs: []string{"create"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + MatchAll: true, + }, + }, + State: apisv1alpha2.ClaimAccepted, + }, + }, + }, + }, nil + }, + getAPIExport: func(clusterName, apiExportName string) (*apisv1alpha2.APIExport, error) { + return &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts/token", + }, + Verbs: []string{"create"}, + }, + }, + }, + }, nil + }, + expectedDecision: authorizer.DecisionAllow, + }, + { + name: "create token subresource denied when APIExport does not claim the subresource", + attr: &authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + APIGroup: "", + Resource: "serviceaccounts", + Subresource: "token", + Verb: "create", + }, + apidomainKey: apidomainKey, + getAPIBindingByExport: func(clusterName, apiExportName, apiExportCluster string) (*apisv1alpha2.APIBinding, error) { + return &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIBindingSpec{ + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts", + }, + Verbs: []string{"get", "create"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + MatchAll: true, + }, + }, + State: apisv1alpha2.ClaimAccepted, + }, + }, + }, + }, nil + }, + getAPIExport: func(clusterName, apiExportName string) (*apisv1alpha2.APIExport, error) { + return &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts", + }, + Verbs: []string{"get", "create"}, + }, + }, + }, + }, nil + }, + expectedDecision: authorizer.DecisionDeny, + expectedReason: "failed to find suitable reason to allow access in APIBinding", + }, + { + name: "create token subresource denied when APIBinding does not accept the subresource claim", + attr: &authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + APIGroup: "", + Resource: "serviceaccounts", + Subresource: "token", + Verb: "create", + }, + apidomainKey: apidomainKey, + getAPIBindingByExport: func(clusterName, apiExportName, apiExportCluster string) (*apisv1alpha2.APIBinding, error) { + return &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIBindingSpec{ + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts", + }, + Verbs: []string{"get"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + MatchAll: true, + }, + }, + State: apisv1alpha2.ClaimAccepted, + }, + }, + }, + }, nil + }, + getAPIExport: func(clusterName, apiExportName string) (*apisv1alpha2.APIExport, error) { + return &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts", + }, + Verbs: []string{"get"}, + }, + { + GroupResource: apisv1alpha2.GroupResource{ + Group: "", + Resource: "serviceaccounts/token", + }, + Verbs: []string{"create"}, + }, + }, + }, + }, nil + }, + expectedDecision: authorizer.DecisionDeny, + expectedReason: "failed to find suitable reason to allow access in APIBinding", + }, + { + // regression check for the status subresource that was/is implicit. + name: "update status subresource allowed by parent resource claim", + attr: &authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + APIGroup: "foo", + Resource: "bar", + Subresource: "status", + Verb: "update", + }, + apidomainKey: apidomainKey, + getAPIBindingByExport: func(clusterName, apiExportName, apiExportCluster string) (*apisv1alpha2.APIBinding, error) { + return &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIBindingSpec{ + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "foo", + Resource: "bar", + }, + Verbs: []string{"update"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + MatchAll: true, + }, + }, + State: apisv1alpha2.ClaimAccepted, + }, + }, + }, + }, nil + }, + getAPIExport: func(clusterName, apiExportName string) (*apisv1alpha2.APIExport, error) { + return &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Group: "foo", + Resource: "bar", + }, + Verbs: []string{"update"}, + }, + }, + }, + }, nil + }, + expectedDecision: authorizer.DecisionAllow, + }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() diff --git a/pkg/virtual/apiexport/builder/forwarding.go b/pkg/virtual/apiexport/builder/forwarding.go index aef698a3cfe..834d7af7730 100644 --- a/pkg/virtual/apiexport/builder/forwarding.go +++ b/pkg/virtual/apiexport/builder/forwarding.go @@ -25,16 +25,21 @@ import ( structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" "k8s.io/apiextensions-apiserver/pkg/registry/customresource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/client-go/util/retry" "github.com/kcp-dev/logicalcluster/v3" apisv1alpha1 "github.com/kcp-dev/sdk/apis/apis/v1alpha1" "github.com/kcp-dev/sdk/apis/apis/v1alpha2/permissionclaims" "github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver" registry "github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry" + + apiexportbuiltin "github.com/kcp-dev/kcp/pkg/virtual/apiexport/schemas/builtin" ) func provideAPIExportFilteredRestStorage(ctx context.Context, dynamicClusterClientFunc registry.DynamicClusterClientFunc, clusterName logicalcluster.Name, exportName string) (apiserver.RestProviderFunc, error) { @@ -119,6 +124,50 @@ func provideDelegatingRestStorage(ctx context.Context, dynamicClusterClientFunc // TODO(sttts): add scale subresource + for name, subresourceGVK := range apiexportbuiltin.BuiltInSubresources[resource.GroupResource()] { + factory := func() runtime.Object { + ret := &unstructured.Unstructured{} + ret.SetGroupVersionKind(subresourceGVK) + return ret + } + subresourceStore := registry.DefaultDynamicDelegatedStoreFuncs( + factory, + nil, + func() {}, + strategy, + tableConvertor, + resource, + apiExportIdentityHash, + nil, + dynamicClusterClientFunc, + []string{name}, + retry.DefaultRetry, + ctx.Done(), + ) + + // get the parent resource for the subresource so the parents' permissions are validated. + // prevents e.g. accessing the subresource of an unclaimed parent resource. + delegateCreate := subresourceStore.NamedCreaterFunc + subresourceStore.NamedCreaterFunc = func(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { + if _, err := storage.GetterFunc.Get(ctx, name, &metav1.GetOptions{}); err != nil { + return nil, err + } + return delegateCreate(ctx, name, obj, createValidation, options) + } + + subresourceStorages[name] = &struct { + registry.FactoryFunc + registry.DestroyerFunc + + registry.NamedCreaterFunc + }{ + FactoryFunc: subresourceStore.FactoryFunc, + DestroyerFunc: subresourceStore.DestroyerFunc, + + NamedCreaterFunc: subresourceStore.NamedCreaterFunc, + } + } + return &struct { registry.FactoryFunc registry.ListFactoryFunc diff --git a/pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_reconcile.go b/pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_reconcile.go index 26eee2276d3..6669c3648ef 100644 --- a/pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_reconcile.go +++ b/pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_reconcile.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sort" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" @@ -87,6 +88,11 @@ func (c *APIReconciler) reconcile(ctx context.Context, apiExport *apisv1alpha2.A logger := logger.WithValues("claim", pc.String()) logger.V(4).Info("evaluating claim") + // claim references a subresource, the API comes from the parent resource claim + if strings.Contains(pc.Resource, "/") { + continue + } + // APIExport resources have priority over claimed resources gr := schema.GroupResource{Group: pc.Group, Resource: pc.Resource} if _, found := apiResourceSchemas[gr]; found { diff --git a/pkg/virtual/apiexport/schemas/builtin/builtin.go b/pkg/virtual/apiexport/schemas/builtin/builtin.go index 9e742484496..1d5a3efc29c 100644 --- a/pkg/virtual/apiexport/schemas/builtin/builtin.go +++ b/pkg/virtual/apiexport/schemas/builtin/builtin.go @@ -18,6 +18,7 @@ package builtin import ( "fmt" + "strings" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" authenticationv1 "k8s.io/api/authentication/v1" @@ -70,13 +71,36 @@ func init() { // IsBuiltInAPI indicates whether the API identified by group and resource is // built-in. func IsBuiltInAPI(gr apis.GroupResource) bool { - _, exists := builtInAPIResourceSchemas[apisv1alpha1.GroupResource{ + resource, sub, hasSub := strings.Cut(gr.GetResource(), "/") + s, exists := builtInAPIResourceSchemas[apisv1alpha1.GroupResource{ Group: gr.GetGroup(), - Resource: gr.GetResource(), + Resource: resource, }] + if !exists || !hasSub { + return exists + } + if sub == "status" { + for _, v := range s.Spec.Versions { + if v.Subresources.Status != nil { + return true + } + } + return false + } + subresources, ok := BuiltInSubresources[schema.GroupResource{Group: gr.GetGroup(), Resource: resource}] + if !ok { + return false + } + _, exists = subresources[sub] return exists } +var BuiltInSubresources = map[schema.GroupResource]map[string]schema.GroupVersionKind{ + {Group: "", Resource: "serviceaccounts"}: { + "token": authenticationv1.SchemeGroupVersion.WithKind("TokenRequest"), + }, +} + // GetBuiltInAPISchema retrieves the APIResourceSchema for a built-in API. func GetBuiltInAPISchema(gr apis.GroupResource) (*apisv1alpha1.APIResourceSchema, error) { s, exists := builtInAPIResourceSchemas[apisv1alpha1.GroupResource{ From 7106c6418854257af2163fb518542f58f8cb1c8b Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Wed, 26 Aug 2026 23:17:33 +0200 Subject: [PATCH 06/14] Add e2e tests for ServiceAccount and TokenRequest Signed-off-by: Nelo-T. Wallus --- .../virtual/apiexport/serviceaccount_test.go | 410 ++++++++++++++++++ 1 file changed, 410 insertions(+) create mode 100644 test/e2e/virtual/apiexport/serviceaccount_test.go diff --git a/test/e2e/virtual/apiexport/serviceaccount_test.go b/test/e2e/virtual/apiexport/serviceaccount_test.go new file mode 100644 index 00000000000..62054960b1f --- /dev/null +++ b/test/e2e/virtual/apiexport/serviceaccount_test.go @@ -0,0 +1,410 @@ +/* +Copyright 2022 The kcp 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 apiexport + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilrand "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/rest" + + kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes" + "github.com/kcp-dev/logicalcluster/v3" + apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2" + "github.com/kcp-dev/sdk/apis/core" + kcpclientset "github.com/kcp-dev/sdk/client/clientset/versioned/cluster" + kcptesting "github.com/kcp-dev/sdk/testing" + kcptestinghelpers "github.com/kcp-dev/sdk/testing/helpers" + + "github.com/kcp-dev/kcp/test/e2e/framework" +) + +func TestMintServiceAccountTokenThroughVW(t *testing.T) { + t.Parallel() + framework.Suite(t, "control-plane") + + server := kcptesting.SharedKcpServer(t) + + cfg := server.BaseConfig(t) + + kcpClients, err := kcpclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kcp cluster client for server") + + kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kube cluster client for server") + + orgPath, _ := kcptesting.NewWorkspaceFixture(t, server, core.RootCluster.Path(), kcptesting.WithType(core.RootCluster.Path(), "organization")) + providerPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerPath, consumerWorkspace := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerClusterName := logicalcluster.Name(consumerWorkspace.Spec.Cluster) + + const providerSAClaimLabel = "custom.provider/label" + + randomStringKey := "test" + randomString := utilrand.String(8) + t.Logf("Create a ConfigMap in the consumer with content %q=%q", randomStringKey, randomString) + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "default", + }, + Data: map[string]string{ + randomStringKey: randomString, + }, + } + _, err = kubeClusterClient.Cluster(consumerPath).CoreV1().ConfigMaps(cm.Namespace).Create(t.Context(), cm, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Setup an unclaimed ServiceAccount in consumer with cluster-admin") + unclaimedSA := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unclaimed-sa", + Namespace: "default", + }, + } + _, err = kubeClusterClient.Cluster(consumerPath).CoreV1().ServiceAccounts("default").Create(t.Context(), unclaimedSA, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Setup an claimed ServiceAccount in consumer with cluster-admin") + claimedSA := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "claimed-sa", + Namespace: "default", + Labels: map[string]string{ + providerSAClaimLabel: "true", + }, + }, + } + _, err = kubeClusterClient.Cluster(consumerPath).CoreV1().ServiceAccounts("default").Create(t.Context(), claimedSA, metav1.CreateOptions{}) + require.NoError(t, err) + + crb := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimedSA.Name, + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: claimedSA.Name, + Namespace: claimedSA.Namespace, + }, + }, + RoleRef: rbacv1.RoleRef{ + Kind: "ClusterRole", + Name: "cluster-admin", + }, + } + _, err = kubeClusterClient.Cluster(consumerPath).RbacV1().ClusterRoleBindings().Create(t.Context(), crb, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Create APIExport in provider with claims for ServiceAccount") + apiExport := &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-token", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Resource: "serviceaccounts", + }, + Verbs: []string{"get", "list"}, + DefaultSelector: &apisv1alpha2.PermissionClaimSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + providerSAClaimLabel: "true", + }, + }, + }, + }, + { + GroupResource: apisv1alpha2.GroupResource{ + Resource: "serviceaccounts/token", + }, + Verbs: []string{"create"}, + DefaultSelector: &apisv1alpha2.PermissionClaimSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + providerSAClaimLabel: "true", + }, + }, + }, + }, + }, + }, + } + _, err = kcpClients.Cluster(providerPath).ApisV1alpha2().APIExports().Create(t.Context(), apiExport, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Bind APIExport in consumer") + apiBinding := &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: apiExport.Name, + }, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Path: providerPath.String(), + Name: apiExport.Name, + }, + }, + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + State: apisv1alpha2.ClaimAccepted, + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Resource: "serviceaccounts", + }, + Verbs: []string{"get", "list"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{providerSAClaimLabel: "true"}, + }, + }, + }, + }, + { + State: apisv1alpha2.ClaimAccepted, + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Resource: "serviceaccounts/token", + }, + Verbs: []string{"create"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{providerSAClaimLabel: "true"}, + }, + }, + }, + }, + }, + }, + } + _, err = kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Wait for VW URL in APIExportES") + apiExportVWCfg := rest.CopyConfig(cfg) + kcptestinghelpers.Eventually(t, func() (bool, string) { + apiExportEndpointSlice, err := kcpClients.Cluster(providerPath).ApisV1alpha1().APIExportEndpointSlices().Get(t.Context(), apiExport.Name, metav1.GetOptions{}) + if kcptestinghelpers.TolerateOrFail(t, err, apierrors.IsNotFound) { + return false, fmt.Sprintf("waiting on APIExportEndpointSlice to be available %v", err.Error()) + } + var found bool + apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(t.Context(), kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) + if err != nil { + return false, fmt.Sprintf("error getting VW URL: %v", err) + } + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) + }, wait.ForeverTestTimeout, time.Millisecond*100) + vwClient, err := kcpkubernetesclientset.NewForConfig(apiExportVWCfg) + require.NoError(t, err) + + t.Log("Verify the ServiceAccount is visible through the VW") + require.EventuallyWithT(t, func(c *assert.CollectT) { + vwServiceAccounts, err := vwClient.CoreV1().ServiceAccounts().List(t.Context(), metav1.ListOptions{}) + require.NoError(c, err) + require.Len(c, vwServiceAccounts.Items, 1, "expect listing exactly one ServiceAccount through the VW") + require.Equal(c, vwServiceAccounts.Items[0].Name, claimedSA.Name, "expect the ServieAccount to have the name %q", claimedSA.Name) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Run("Test a minted token for the claimed ServiceAccount", func(t *testing.T) { + t.Parallel() + t.Log("Mint a token for the ServiceAccount") + tokenRequest := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimedSA.Name, + Namespace: claimedSA.Namespace, + }, + } + trResponse, err := vwClient.CoreV1().ServiceAccounts().Cluster(consumerClusterName.Path()).Namespace(claimedSA.Namespace).CreateToken(t.Context(), claimedSA.Name, tokenRequest, metav1.CreateOptions{}) + require.NoError(t, err) + require.NotEmpty(t, trResponse.Status.Token) + + t.Log("Create a new client with the ServiceAccount identity") + saCfg := framework.ConfigWithToken(trResponse.Status.Token, server.BaseConfig(t)) + saClusterClient, err := kcpkubernetesclientset.NewForConfig(saCfg) + require.NoError(t, err) + + t.Log("Get test ConfigMap using the ServiceAccount identity") + saConfigMap, err := saClusterClient.Cluster(consumerPath).CoreV1().ConfigMaps(cm.Namespace).Get(t.Context(), cm.Name, metav1.GetOptions{}) + require.NoError(t, err) + + val := saConfigMap.Data[randomStringKey] + assert.Equal(t, randomString, val, "expect data to match random test string") + }) + + t.Run("Verify that minting a token for the unclaimed ServiceAccount fails", func(t *testing.T) { + t.Parallel() + unclaimedTokenRequest := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: unclaimedSA.Name, + Namespace: unclaimedSA.Namespace, + }, + } + utrResponse, err := vwClient.CoreV1().ServiceAccounts().Cluster(consumerClusterName.Path()).Namespace(unclaimedSA.Namespace).CreateToken(t.Context(), unclaimedSA.Name, unclaimedTokenRequest, metav1.CreateOptions{}) + assert.True(t, apierrors.IsNotFound(err)) + assert.Empty(t, utrResponse.Status.Token) + }) +} + +func TestMintServiceAccountTokenThroughVWFailsWithoutSubresoureClaim(t *testing.T) { + t.Parallel() + framework.Suite(t, "control-plane") + + server := kcptesting.SharedKcpServer(t) + + cfg := server.BaseConfig(t) + + kcpClients, err := kcpclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kcp cluster client for server") + + kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kube cluster client for server") + + orgPath, _ := kcptesting.NewWorkspaceFixture(t, server, core.RootCluster.Path(), kcptesting.WithType(core.RootCluster.Path(), "organization")) + providerPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerPath, consumerWorkspace := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerClusterName := logicalcluster.Name(consumerWorkspace.Spec.Cluster) + + const providerSAClaimLabel = "custom.provider/label" + + t.Log("Setup a claimed ServiceAccount in consumer") + claimedSA := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "claimed-sa", + Namespace: "default", + Labels: map[string]string{ + providerSAClaimLabel: "true", + }, + }, + } + _, err = kubeClusterClient.Cluster(consumerPath).CoreV1().ServiceAccounts("default").Create(t.Context(), claimedSA, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Create APIExport in provider with claims for ServiceAccount but no subresources") + apiExport := &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-token", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Resource: "serviceaccounts", + }, + Verbs: []string{"get", "list"}, + DefaultSelector: &apisv1alpha2.PermissionClaimSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + providerSAClaimLabel: "true", + }, + }, + }, + }, + }, + }, + } + _, err = kcpClients.Cluster(providerPath).ApisV1alpha2().APIExports().Create(t.Context(), apiExport, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Bind APIExport in consumer") + apiBinding := &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: apiExport.Name, + }, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Path: providerPath.String(), + Name: apiExport.Name, + }, + }, + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + State: apisv1alpha2.ClaimAccepted, + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Resource: "serviceaccounts", + }, + Verbs: []string{"get", "list"}, + }, + Selector: apisv1alpha2.PermissionClaimSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{providerSAClaimLabel: "true"}, + }, + }, + }, + }, + }, + }, + } + _, err = kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Wait for VW URL in APIExportES") + apiExportVWCfg := rest.CopyConfig(cfg) + kcptestinghelpers.Eventually(t, func() (bool, string) { + apiExportEndpointSlice, err := kcpClients.Cluster(providerPath).ApisV1alpha1().APIExportEndpointSlices().Get(t.Context(), apiExport.Name, metav1.GetOptions{}) + if kcptestinghelpers.TolerateOrFail(t, err, apierrors.IsNotFound) { + return false, fmt.Sprintf("waiting on APIExportEndpointSlice to be available %v", err.Error()) + } + var found bool + apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(t.Context(), kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) + if err != nil { + return false, fmt.Sprintf("error getting VW URL: %v", err) + } + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) + }, wait.ForeverTestTimeout, time.Millisecond*100) + vwClient, err := kcpkubernetesclientset.NewForConfig(apiExportVWCfg) + require.NoError(t, err) + + t.Log("Verify the ServiceAccount is visible through the VW") + require.EventuallyWithT(t, func(c *assert.CollectT) { + vwServiceAccounts, err := vwClient.CoreV1().ServiceAccounts().List(t.Context(), metav1.ListOptions{}) + require.NoError(c, err) + require.Len(c, vwServiceAccounts.Items, 1, "expect listing exactly one ServiceAccount through the VW") + require.Equal(c, vwServiceAccounts.Items[0].Name, claimedSA.Name, "expect the ServieAccount to have the name %q", claimedSA.Name) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Verify mint a token for the ServiceAccount fails") + tokenRequest := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimedSA.Name, + Namespace: claimedSA.Namespace, + }, + } + trResponse, err := vwClient.CoreV1().ServiceAccounts().Cluster(consumerClusterName.Path()).Namespace(claimedSA.Namespace).CreateToken(t.Context(), claimedSA.Name, tokenRequest, metav1.CreateOptions{}) + require.Error(t, err) + require.Empty(t, trResponse.Status.Token) +} From 5f3f5224aaddbe1e3e1f79ed44cfec52765396eb Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 00:06:50 +0200 Subject: [PATCH 07/14] Disallow setting selectors on subresource claims Signed-off-by: Nelo-T. Wallus --- .../sdk/apis/apis/v1alpha2/types_apibinding.go | 2 ++ .../sdk/apis/apis/v1alpha2/types_apiexport.go | 2 ++ test/e2e/virtual/apiexport/serviceaccount_test.go | 12 ------------ 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apibinding.go b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apibinding.go index de9b79516d4..a64a56ba6be 100644 --- a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apibinding.go +++ b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apibinding.go @@ -83,6 +83,8 @@ type APIBindingSpec struct { // ScopedPermissionClaim embeds a PermissionClaim and adds a selector to // scope down access to objects of the claimed resource. +// +// +kubebuilder:validation:XValidation:rule="!self.resource.contains('/') || (has(self.selector.matchAll) && self.selector.matchAll)",message="subresource claims must use selector.matchAll" type ScopedPermissionClaim struct { PermissionClaim `json:",inline"` diff --git a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go index 0626d0dc988..e5c0c6eda6a 100644 --- a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go +++ b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go @@ -230,6 +230,8 @@ type LocalAPIExportPolicy struct{} // PermissionClaim identifies an object by GR and identity hash. // Its purpose is to determine the added permissions that a service provider may // request and that a consumer may accept and allow the service provider access to. +// +// +kubebuilder:validation:XValidation:rule="!self.resource.contains('/') || !has(self.defaultSelector)",message="defaultSelector is not allowed on subresource claims" type PermissionClaim struct { GroupResource `json:",inline"` diff --git a/test/e2e/virtual/apiexport/serviceaccount_test.go b/test/e2e/virtual/apiexport/serviceaccount_test.go index 62054960b1f..c9023211d95 100644 --- a/test/e2e/virtual/apiexport/serviceaccount_test.go +++ b/test/e2e/virtual/apiexport/serviceaccount_test.go @@ -147,13 +147,6 @@ func TestMintServiceAccountTokenThroughVW(t *testing.T) { Resource: "serviceaccounts/token", }, Verbs: []string{"create"}, - DefaultSelector: &apisv1alpha2.PermissionClaimSelector{ - LabelSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - providerSAClaimLabel: "true", - }, - }, - }, }, }, }, @@ -199,11 +192,6 @@ func TestMintServiceAccountTokenThroughVW(t *testing.T) { }, Verbs: []string{"create"}, }, - Selector: apisv1alpha2.PermissionClaimSelector{ - LabelSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{providerSAClaimLabel: "true"}, - }, - }, }, }, }, From 07811b984de998a51a945b20e2975714217aed4e Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 00:08:37 +0200 Subject: [PATCH 08/14] codegen Signed-off-by: Nelo-T. Wallus --- config/crds/apis.kcp.io_apibindings.yaml | 15 +++++++++++++++ config/crds/apis.kcp.io_apiexports.yaml | 3 +++ .../apis/v1alpha1/groupresource.go | 2 ++ .../kcp-dev/sdk/openapi/zz_generated.openapi.go | 6 +++--- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/config/crds/apis.kcp.io_apibindings.yaml b/config/crds/apis.kcp.io_apibindings.yaml index c53afe814fa..58a7353ff86 100644 --- a/config/crds/apis.kcp.io_apibindings.yaml +++ b/config/crds/apis.kcp.io_apibindings.yaml @@ -644,6 +644,12 @@ spec: - state - verbs type: object + x-kubernetes-validations: + - message: subresource claims must use selector.matchAll + rule: '!self.resource.contains(''/'') || (has(self.selector.matchAll) + && self.selector.matchAll)' + - message: defaultSelector is not allowed on subresource claims + rule: '!self.resource.contains(''/'') || !has(self.defaultSelector)' type: array x-kubernetes-list-map-keys: - group @@ -848,6 +854,12 @@ spec: - selector - verbs type: object + x-kubernetes-validations: + - message: subresource claims must use selector.matchAll + rule: '!self.resource.contains(''/'') || (has(self.selector.matchAll) + && self.selector.matchAll)' + - message: defaultSelector is not allowed on subresource claims + rule: '!self.resource.contains(''/'') || !has(self.defaultSelector)' type: array x-kubernetes-list-map-keys: - group @@ -1072,6 +1084,9 @@ spec: - resource - verbs type: object + x-kubernetes-validations: + - message: defaultSelector is not allowed on subresource claims + rule: '!self.resource.contains(''/'') || !has(self.defaultSelector)' type: array phase: description: |- diff --git a/config/crds/apis.kcp.io_apiexports.yaml b/config/crds/apis.kcp.io_apiexports.yaml index 0797157b961..4986d05c198 100644 --- a/config/crds/apis.kcp.io_apiexports.yaml +++ b/config/crds/apis.kcp.io_apiexports.yaml @@ -494,6 +494,9 @@ spec: - resource - verbs type: object + x-kubernetes-validations: + - message: defaultSelector is not allowed on subresource claims + rule: '!self.resource.contains(''/'') || !has(self.defaultSelector)' type: array x-kubernetes-list-map-keys: - group diff --git a/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha1/groupresource.go b/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha1/groupresource.go index 33c329a416b..dc8088ff7c5 100644 --- a/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha1/groupresource.go +++ b/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha1/groupresource.go @@ -27,6 +27,8 @@ type GroupResourceApplyConfiguration struct { // For core groups this is the empty string '""'. Group *string `json:"group,omitempty"` // resource is the name of the resource. + // A subresource may be claimed as "resource/subresource", + // e.g. "serviceaccounts/token", in the style of RBAC rules. // Note: it is worth noting that you can not ask for permissions for resource provided by a CRD // not provided by an api export. Resource *string `json:"resource,omitempty"` diff --git a/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go b/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go index d247e5f5f12..f7377c8d671 100644 --- a/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go +++ b/staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go @@ -1393,7 +1393,7 @@ func schema_sdk_apis_apis_v1alpha1_AcceptablePermissionClaim(ref common.Referenc }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", @@ -1636,7 +1636,7 @@ func schema_sdk_apis_apis_v1alpha1_GroupResource(ref common.ReferenceCallback) c }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", @@ -1718,7 +1718,7 @@ func schema_sdk_apis_apis_v1alpha1_PermissionClaim(ref common.ReferenceCallback) }, "resource": { SchemaProps: spec.SchemaProps{ - Description: "resource is the name of the resource. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", + Description: "resource is the name of the resource. A subresource may be claimed as \"resource/subresource\", e.g. \"serviceaccounts/token\", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export.", Default: "", Type: []string{"string"}, Format: "", From 39a8008fa46de32e57c3df3113bc9e77d57bf8ff Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 00:36:27 +0200 Subject: [PATCH 09/14] Set matchall for subresource claim in admission Signed-off-by: Nelo-T. Wallus --- pkg/admission/apibinding/apibinding_admission.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/admission/apibinding/apibinding_admission.go b/pkg/admission/apibinding/apibinding_admission.go index bfb8a2c4f25..6031ce78706 100644 --- a/pkg/admission/apibinding/apibinding_admission.go +++ b/pkg/admission/apibinding/apibinding_admission.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "io" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -177,6 +178,17 @@ func (o *apiBindingAdmission) Admit(ctx context.Context, a admission.Attributes, ab.SetLabels(lbls) } + // subresource claims have no selector, set a matchAll so users + // don't have to repeat it for every subresource + if v2, ok := ab.(*apiBindingV1alpha2); ok { + for i := range v2.binding.Spec.PermissionClaims { + pc := &v2.binding.Spec.PermissionClaims[i] + if strings.Contains(pc.Resource, "/") { + pc.Selector.MatchAll = true + } + } + } + // write back raw, err := ab.ToUnstructured() if err != nil { From 610db7edc5c4552153b0124c1d5da18d1e6571c8 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 07:59:31 +0200 Subject: [PATCH 10/14] Add a maxmimum length for resource names to lessen estimated CEL cost 127 because a resource can have up to 63 bytes per segment, so 63 for the resource, 63 for the subresource and 1 for the `/`. Signed-off-by: Nelo-T. Wallus --- .../github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go | 1 + .../github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go | 1 + 2 files changed, 2 insertions(+) diff --git a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go index 7f0433642eb..8c8739c7b75 100644 --- a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go +++ b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha1/types_apiexport.go @@ -293,6 +293,7 @@ type GroupResource struct { // Note: it is worth noting that you can not ask for permissions for resource provided by a CRD // not provided by an api export. // +kubebuilder:validation:Pattern=`^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$` + // +kubebuilder:validation:MaxLength=127 // +required // +kubebuilder:validation:Required Resource string `json:"resource"` diff --git a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go index e5c0c6eda6a..6cc6c6638fd 100644 --- a/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go +++ b/staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexport.go @@ -327,6 +327,7 @@ type GroupResource struct { // not provided by an api export. // // +kubebuilder:validation:Pattern=`^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$` + // +kubebuilder:validation:MaxLength=127 // +required // +kubebuilder:validation:Required Resource string `json:"resource"` From 4aba907176a8530922b51da4ab4b499cfb3a48e0 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 08:01:54 +0200 Subject: [PATCH 11/14] codegen Signed-off-by: Nelo-T. Wallus --- config/crds/apis.kcp.io_apibindings.yaml | 6 ++++++ config/crds/apis.kcp.io_apiexports.yaml | 2 ++ 2 files changed, 8 insertions(+) diff --git a/config/crds/apis.kcp.io_apibindings.yaml b/config/crds/apis.kcp.io_apibindings.yaml index 58a7353ff86..b41250f62aa 100644 --- a/config/crds/apis.kcp.io_apibindings.yaml +++ b/config/crds/apis.kcp.io_apibindings.yaml @@ -88,6 +88,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: @@ -204,6 +205,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: @@ -386,6 +388,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: @@ -566,6 +569,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string selector: @@ -782,6 +786,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string selector: @@ -1068,6 +1073,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string verbs: diff --git a/config/crds/apis.kcp.io_apiexports.yaml b/config/crds/apis.kcp.io_apiexports.yaml index 4986d05c198..e219f670d01 100644 --- a/config/crds/apis.kcp.io_apiexports.yaml +++ b/config/crds/apis.kcp.io_apiexports.yaml @@ -175,6 +175,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string resourceSelector: @@ -478,6 +479,7 @@ spec: e.g. "serviceaccounts/token", in the style of RBAC rules. Note: it is worth noting that you can not ask for permissions for resource provided by a CRD not provided by an api export. + maxLength: 127 pattern: ^[a-z][-a-z0-9]*[a-z0-9](/[a-z][-a-z0-9]*[a-z0-9])?$ type: string verbs: From 42430153bbc4bf3372501c735b4e18373310d2b0 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 10:16:09 +0200 Subject: [PATCH 12/14] Retry creating APIBindings for sharded setups Signed-off-by: Nelo-T. Wallus --- test/e2e/virtual/apiexport/serviceaccount_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/e2e/virtual/apiexport/serviceaccount_test.go b/test/e2e/virtual/apiexport/serviceaccount_test.go index c9023211d95..735e69e4d7f 100644 --- a/test/e2e/virtual/apiexport/serviceaccount_test.go +++ b/test/e2e/virtual/apiexport/serviceaccount_test.go @@ -197,8 +197,10 @@ func TestMintServiceAccountTokenThroughVW(t *testing.T) { }, }, } - _, err = kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) - require.NoError(t, err) + kcptestinghelpers.Eventually(t, func() (bool, string) { + _, err = kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) + return err == nil, fmt.Sprintf("error creating APIBinding: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) t.Log("Wait for VW URL in APIExportES") apiExportVWCfg := rest.CopyConfig(cfg) @@ -357,8 +359,10 @@ func TestMintServiceAccountTokenThroughVWFailsWithoutSubresoureClaim(t *testing. }, }, } - _, err = kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) - require.NoError(t, err) + kcptestinghelpers.Eventually(t, func() (bool, string) { + _, err = kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) + return err == nil, fmt.Sprintf("error creating APIBinding: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) t.Log("Wait for VW URL in APIExportES") apiExportVWCfg := rest.CopyConfig(cfg) From caa2147511d60474c0abe14769f9d617daeaa0a4 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Thu, 27 Aug 2026 21:56:56 +0200 Subject: [PATCH 13/14] Add test for scale (and status) subresource through VW Signed-off-by: Nelo-T. Wallus --- .../apiresourceschema_cowboys_scale.yaml | 49 +++ test/e2e/virtual/apiexport/scale_test.go | 324 ++++++++++++++++++ 2 files changed, 373 insertions(+) create mode 100644 test/e2e/virtual/apiexport/apiresourceschema_cowboys_scale.yaml create mode 100644 test/e2e/virtual/apiexport/scale_test.go diff --git a/test/e2e/virtual/apiexport/apiresourceschema_cowboys_scale.yaml b/test/e2e/virtual/apiexport/apiresourceschema_cowboys_scale.yaml new file mode 100644 index 00000000000..389d73b0815 --- /dev/null +++ b/test/e2e/virtual/apiexport/apiresourceschema_cowboys_scale.yaml @@ -0,0 +1,49 @@ +apiVersion: apis.kcp.io/v1alpha1 +kind: APIResourceSchema +metadata: + name: today.cowboys.scale.wildwest.dev +spec: + group: scale.wildwest.dev + names: + kind: Cowboy + listKind: CowboyList + plural: cowboys + singular: cowboy + scope: Namespaced + versions: + - name: v1alpha1 + schema: + description: Cowboy is part of the wild west + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + description: CowboySpec holds the desired state of the Cowboy. + properties: + intent: + type: string + replicas: + type: integer + format: int32 + type: object + status: + description: CowboyStatus communicates the observed state of the Cowboy. + properties: + result: + type: string + replicas: + type: integer + format: int32 + type: object + type: object + served: true + storage: true + subresources: + status: {} + scale: + specReplicasPath: .spec.replicas + statusReplicasPath: .status.replicas diff --git a/test/e2e/virtual/apiexport/scale_test.go b/test/e2e/virtual/apiexport/scale_test.go new file mode 100644 index 00000000000..e7a0ba5e23e --- /dev/null +++ b/test/e2e/virtual/apiexport/scale_test.go @@ -0,0 +1,324 @@ +/* +Copyright 2026 The kcp 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 apiexport + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/discovery/cached/memory" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" + + kcpdynamic "github.com/kcp-dev/client-go/dynamic" + "github.com/kcp-dev/logicalcluster/v3" + apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2" + "github.com/kcp-dev/sdk/apis/core" + "github.com/kcp-dev/sdk/apis/third_party/conditions/util/conditions" + kcpclientset "github.com/kcp-dev/sdk/client/clientset/versioned/cluster" + kcptesting "github.com/kcp-dev/sdk/testing" + kcptestinghelpers "github.com/kcp-dev/sdk/testing/helpers" + + "github.com/kcp-dev/kcp/config/helpers" + "github.com/kcp-dev/kcp/test/e2e/framework" +) + +func TestScaleSubresourceThroughVW(t *testing.T) { + t.Parallel() + framework.Suite(t, "control-plane") + + server := kcptesting.SharedKcpServer(t) + cfg := server.BaseConfig(t) + + kcpClients, err := kcpclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kcp cluster client for server") + + dynamicClusterClient, err := kcpdynamic.NewForConfig(cfg) + require.NoError(t, err, "failed to construct dynamic cluster client for server") + + orgPath, _ := kcptesting.NewWorkspaceFixture(t, server, core.RootCluster.Path(), kcptesting.WithType(core.RootCluster.Path(), "organization")) + providerPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath) + claimerPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerPath, consumerWorkspace := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerClusterName := logicalcluster.Name(consumerWorkspace.Spec.Cluster) + + cowboysGVR := schema.GroupVersionResource{Group: "scale.wildwest.dev", Version: "v1alpha1", Resource: "cowboys"} + consumerCowboys := dynamicClusterClient.Cluster(consumerPath).Resource(cowboysGVR).Namespace("default") + + t.Log("Install cowboys APIResourceSchema with status and scale subresources into provider") + mapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(kcpClients.Cluster(providerPath).Discovery())) + err = helpers.CreateResourceFromFS(t.Context(), dynamicClusterClient.Cluster(providerPath), mapper, nil, "apiresourceschema_cowboys_scale.yaml", testFiles) + require.NoError(t, err) + + t.Log("Create APIExport in provider") + apiExport := &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "scale-cowboys", + }, + Spec: apisv1alpha2.APIExportSpec{ + Resources: []apisv1alpha2.ResourceSchema{ + { + Name: "cowboys", + Group: "scale.wildwest.dev", + Schema: "today.cowboys.scale.wildwest.dev", + Storage: apisv1alpha2.ResourceSchemaStorage{ + CRD: &apisv1alpha2.ResourceSchemaStorageCRD{}, + }, + }, + }, + }, + } + _, err = kcpClients.Cluster(providerPath).ApisV1alpha2().APIExports().Create(t.Context(), apiExport, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Bind provider APIExport in consumer") + apiBinding := &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: apiExport.Name, + }, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Path: providerPath.String(), + Name: apiExport.Name, + }, + }, + }, + } + kcptestinghelpers.Eventually(t, func() (bool, string) { + _, err := kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), apiBinding, metav1.CreateOptions{}) + return err == nil, fmt.Sprintf("error creating APIBinding: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Create a cowboy in the consumer workspace") + cowboy := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "scale.wildwest.dev/v1alpha1", + "kind": "Cowboy", + "metadata": map[string]interface{}{ + "name": "woody", + }, + "spec": map[string]interface{}{ + "intent": "yeehaw", + "replicas": int64(1), + }, + }, + } + kcptestinghelpers.Eventually(t, func() (bool, string) { + _, err := consumerCowboys.Create(t.Context(), cowboy, metav1.CreateOptions{}) + return err == nil, fmt.Sprintf("error creating cowboy: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Wait for VW URL of the provider APIExport") + providerVWCfg := rest.CopyConfig(cfg) + kcptestinghelpers.Eventually(t, func() (bool, string) { + apiExportEndpointSlice, err := kcpClients.Cluster(providerPath).ApisV1alpha1().APIExportEndpointSlices().Get(t.Context(), apiExport.Name, metav1.GetOptions{}) + if kcptestinghelpers.TolerateOrFail(t, err, apierrors.IsNotFound) { + return false, fmt.Sprintf("waiting on APIExportEndpointSlice to be available %v", err.Error()) + } + var found bool + providerVWCfg.Host, found, err = framework.VirtualWorkspaceURL(t.Context(), kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) + if err != nil { + return false, fmt.Sprintf("error getting VW URL: %v", err) + } + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) + }, wait.ForeverTestTimeout, time.Millisecond*100) + providerVWClient, err := kcpdynamic.NewForConfig(providerVWCfg) + require.NoError(t, err) + providerVWCowboys := providerVWClient.Cluster(consumerClusterName.Path()).Resource(cowboysGVR).Namespace("default") + + t.Log("Update the status subresource through the provider VW") + kcptestinghelpers.Eventually(t, func() (bool, string) { + vwCowboy, err := providerVWCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}) + if err != nil { + return false, fmt.Sprintf("error getting cowboy through VW: %v", err) + } + if err := unstructured.SetNestedField(vwCowboy.Object, "giddyup", "status", "result"); err != nil { + return false, err.Error() + } + _, err = providerVWCowboys.Update(t.Context(), vwCowboy, metav1.UpdateOptions{}, "status") + return err == nil, fmt.Sprintf("error updating cowboy status through VW: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Verify the status update is visible in the consumer workspace") + updated, err := consumerCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}) + require.NoError(t, err) + result, _, err := unstructured.NestedString(updated.Object, "status", "result") + require.NoError(t, err) + require.Equal(t, "giddyup", result) + + t.Log("Update the scale subresource through the provider VW") + kcptestinghelpers.Eventually(t, func() (bool, string) { + scale, err := providerVWCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}, "scale") + if err != nil { + return false, fmt.Sprintf("error getting scale through VW: %v", err) + } + if err := unstructured.SetNestedField(scale.Object, int64(3), "spec", "replicas"); err != nil { + return false, err.Error() + } + _, err = providerVWCowboys.Update(t.Context(), scale, metav1.UpdateOptions{}, "scale") + return err == nil, fmt.Sprintf("error updating scale through VW: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Verify the scale update is visible in the consumer workspace") + updated, err = consumerCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}) + require.NoError(t, err) + replicas, _, err := unstructured.NestedInt64(updated.Object, "spec", "replicas") + require.NoError(t, err) + require.Equal(t, int64(3), replicas) + + t.Log("Get the provider APIExport identity hash") + kcptestinghelpers.EventuallyCondition(t, func() (conditions.Getter, error) { + return kcpClients.Cluster(providerPath).ApisV1alpha2().APIExports().Get(t.Context(), apiExport.Name, metav1.GetOptions{}) + }, kcptestinghelpers.Is(apisv1alpha2.APIExportIdentityValid)) + export, err := kcpClients.Cluster(providerPath).ApisV1alpha2().APIExports().Get(t.Context(), apiExport.Name, metav1.GetOptions{}) + require.NoError(t, err) + identityHash := export.Status.IdentityHash + + t.Log("Create claimer APIExport in third workspace claiming cowboys and cowboys/scale") + cowboysClaim := apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "scale.wildwest.dev", + Resource: "cowboys", + }, + Verbs: []string{"get", "list", "watch", "update", "patch"}, + IdentityHash: identityHash, + } + scaleClaim := apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "scale.wildwest.dev", + Resource: "cowboys/scale", + }, + Verbs: []string{"get", "update", "patch"}, + IdentityHash: identityHash, + } + claimerExport := &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cowboy-wrangler", + }, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + cowboysClaim, + scaleClaim, + }, + }, + } + _, err = kcpClients.Cluster(claimerPath).ApisV1alpha2().APIExports().Create(t.Context(), claimerExport, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Log("Bind claimer APIExport in consumer, accepting the claims") + claimerBinding := &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimerExport.Name, + }, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Path: claimerPath.String(), + Name: claimerExport.Name, + }, + }, + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + State: apisv1alpha2.ClaimAccepted, + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: cowboysClaim, + Selector: apisv1alpha2.PermissionClaimSelector{ + MatchAll: true, + }, + }, + }, + { + State: apisv1alpha2.ClaimAccepted, + ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{ + PermissionClaim: scaleClaim, + }, + }, + }, + }, + } + kcptestinghelpers.Eventually(t, func() (bool, string) { + _, err := kcpClients.Cluster(consumerPath).ApisV1alpha2().APIBindings().Create(t.Context(), claimerBinding, metav1.CreateOptions{}) + return err == nil, fmt.Sprintf("error creating claimer APIBinding: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Wait for VW URL of the claimer APIExport") + claimerVWCfg := rest.CopyConfig(cfg) + kcptestinghelpers.Eventually(t, func() (bool, string) { + apiExportEndpointSlice, err := kcpClients.Cluster(claimerPath).ApisV1alpha1().APIExportEndpointSlices().Get(t.Context(), claimerExport.Name, metav1.GetOptions{}) + if kcptestinghelpers.TolerateOrFail(t, err, apierrors.IsNotFound) { + return false, fmt.Sprintf("waiting on APIExportEndpointSlice to be available %v", err.Error()) + } + var found bool + claimerVWCfg.Host, found, err = framework.VirtualWorkspaceURL(t.Context(), kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) + if err != nil { + return false, fmt.Sprintf("error getting VW URL: %v", err) + } + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) + }, wait.ForeverTestTimeout, time.Millisecond*100) + claimerVWClient, err := kcpdynamic.NewForConfig(claimerVWCfg) + require.NoError(t, err) + claimerVWCowboys := claimerVWClient.Cluster(consumerClusterName.Path()).Resource(cowboysGVR).Namespace("default") + + t.Log("Update the cowboy through the claimer VW") + kcptestinghelpers.Eventually(t, func() (bool, string) { + vwCowboy, err := claimerVWCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}) + if err != nil { + return false, fmt.Sprintf("error getting cowboy through claimer VW: %v", err) + } + if err := unstructured.SetNestedField(vwCowboy.Object, "howdy", "spec", "intent"); err != nil { + return false, err.Error() + } + _, err = claimerVWCowboys.Update(t.Context(), vwCowboy, metav1.UpdateOptions{}) + return err == nil, fmt.Sprintf("error updating cowboy through claimer VW: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Verify the cowboy update is visible in the consumer workspace") + updated, err = consumerCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}) + require.NoError(t, err) + intent, _, err := unstructured.NestedString(updated.Object, "spec", "intent") + require.NoError(t, err) + require.Equal(t, "howdy", intent) + + t.Log("Update the scale subresource through the claimer VW") + kcptestinghelpers.Eventually(t, func() (bool, string) { + scale, err := claimerVWCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}, "scale") + if err != nil { + return false, fmt.Sprintf("error getting scale through claimer VW: %v", err) + } + if err := unstructured.SetNestedField(scale.Object, int64(5), "spec", "replicas"); err != nil { + return false, err.Error() + } + _, err = claimerVWCowboys.Update(t.Context(), scale, metav1.UpdateOptions{}, "scale") + return err == nil, fmt.Sprintf("error updating scale through claimer VW: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + + t.Log("Verify the scale update is visible in the consumer workspace") + updated, err = consumerCowboys.Get(t.Context(), cowboy.GetName(), metav1.GetOptions{}) + require.NoError(t, err) + replicas, _, err = unstructured.NestedInt64(updated.Object, "spec", "replicas") + require.NoError(t, err) + require.Equal(t, int64(5), replicas) +} From ddf4fc51f649dc669374c69ce19c0ff8923fb2b6 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Fri, 28 Aug 2026 18:41:57 +0200 Subject: [PATCH 14/14] Implement scale subresource in virtual workspaces Signed-off-by: Nelo-T. Wallus --- pkg/virtual/apiexport/builder/forwarding.go | 33 ++++++++++++-- .../builder/forwarding.go | 31 ++++++++++++- .../builder/forwarding.go | 35 ++++++++++++++- .../pkg/dynamic/apiserver/serving_info.go | 19 +++++++- .../pkg/forwardingregistry/rest.go | 45 +++++++++++++++++-- .../pkg/forwardingregistry/rest_test.go | 20 ++++----- 6 files changed, 162 insertions(+), 21 deletions(-) diff --git a/pkg/virtual/apiexport/builder/forwarding.go b/pkg/virtual/apiexport/builder/forwarding.go index 834d7af7730..2f1596cf85e 100644 --- a/pkg/virtual/apiexport/builder/forwarding.go +++ b/pkg/virtual/apiexport/builder/forwarding.go @@ -64,8 +64,12 @@ func provideDelegatingRestStorage(ctx context.Context, dynamicClusterClientFunc statusSpec = &apiextensions.CustomResourceSubresourceStatus{} } + _, scaleEnabled := subresourcesSchemaValidator["scale"] + var scaleSpec *apiextensions.CustomResourceSubresourceScale - // TODO(sttts): implement scale subresource + if scaleEnabled { + scaleSpec = &apiextensions.CustomResourceSubresourceScale{} + } strategy := customresource.NewStrategy( typer, @@ -80,7 +84,7 @@ func provideDelegatingRestStorage(ctx context.Context, dynamicClusterClientFunc []apiextensionsv1.SelectableField{}, ) - storage, statusStorage := registry.NewStorage( + storage, statusStorage, scaleStorage := registry.NewStorage( ctx, resource, apiExportIdentityHash, @@ -122,7 +126,30 @@ func provideDelegatingRestStorage(ctx context.Context, dynamicClusterClientFunc } } - // TODO(sttts): add scale subresource + if scaleEnabled { + subresourceStorages["scale"] = &struct { + registry.FactoryFunc + registry.DestroyerFunc + + registry.GetterFunc + registry.UpdaterFunc + // patch is implicit as we have get + update + + registry.TableConvertorFunc + registry.CategoriesProviderFunc + registry.ResetFieldsStrategyFunc + }{ + FactoryFunc: scaleStorage.FactoryFunc, + DestroyerFunc: scaleStorage.DestroyerFunc, + + GetterFunc: scaleStorage.GetterFunc, + UpdaterFunc: scaleStorage.UpdaterFunc, + + TableConvertorFunc: scaleStorage.TableConvertorFunc, + CategoriesProviderFunc: scaleStorage.CategoriesProviderFunc, + ResetFieldsStrategyFunc: scaleStorage.ResetFieldsStrategyFunc, + } + } for name, subresourceGVK := range apiexportbuiltin.BuiltInSubresources[resource.GroupResource()] { factory := func() runtime.Object { diff --git a/pkg/virtual/initializingworkspaces/builder/forwarding.go b/pkg/virtual/initializingworkspaces/builder/forwarding.go index 0624fc0d80c..04ac8bcff14 100644 --- a/pkg/virtual/initializingworkspaces/builder/forwarding.go +++ b/pkg/virtual/initializingworkspaces/builder/forwarding.go @@ -106,7 +106,11 @@ func delegatingLogicalClusterReadOnlyRestStorage( statusSpec = &apiextensions.CustomResourceSubresourceStatus{} } + _, scaleEnabled := subresourcesSchemaValidator["scale"] var scaleSpec *apiextensions.CustomResourceSubresourceScale + if scaleEnabled { + scaleSpec = &apiextensions.CustomResourceSubresourceScale{} + } strategy := customresource.NewStrategy( typer, @@ -121,7 +125,7 @@ func delegatingLogicalClusterReadOnlyRestStorage( []apiextensionsv1.SelectableField{}, ) - storage, statusStorage := registry.NewStorage( + storage, statusStorage, scaleStorage := registry.NewStorage( ctx, resource, "", @@ -166,6 +170,31 @@ func delegatingLogicalClusterReadOnlyRestStorage( } } + if scaleEnabled { + subresourceStorages["scale"] = &struct { + registry.FactoryFunc + registry.DestroyerFunc + + registry.GetterFunc + registry.UpdaterFunc + // patch is implicit as we have get + update + + registry.TableConvertorFunc + registry.CategoriesProviderFunc + registry.ResetFieldsStrategyFunc + }{ + FactoryFunc: scaleStorage.FactoryFunc, + DestroyerFunc: scaleStorage.DestroyerFunc, + + GetterFunc: scaleStorage.GetterFunc, + UpdaterFunc: scaleStorage.UpdaterFunc, + + TableConvertorFunc: scaleStorage.TableConvertorFunc, + CategoriesProviderFunc: scaleStorage.CategoriesProviderFunc, + ResetFieldsStrategyFunc: scaleStorage.ResetFieldsStrategyFunc, + } + } + // only expose GET return &struct { registry.FactoryFunc diff --git a/pkg/virtual/terminatingworkspaces/builder/forwarding.go b/pkg/virtual/terminatingworkspaces/builder/forwarding.go index d80e0983350..f3fbf9ea246 100644 --- a/pkg/virtual/terminatingworkspaces/builder/forwarding.go +++ b/pkg/virtual/terminatingworkspaces/builder/forwarding.go @@ -97,6 +97,12 @@ func filteredLogicalClusterStatusWriteOnly( statusSpec = &apiextensions.CustomResourceSubresourceStatus{} } + _, scaleEnabled := subresourcesSchemaValidator["scale"] + var scaleSpec *apiextensions.CustomResourceSubresourceScale + if scaleEnabled { + scaleSpec = &apiextensions.CustomResourceSubresourceScale{} + } + strategy := customresource.NewStrategy( typer, namespaceScoped, @@ -106,11 +112,11 @@ func filteredLogicalClusterStatusWriteOnly( statusSchemaValidate, structuralSchema, statusSpec, - nil, // no scale subresource needed + scaleSpec, []apiextensionsv1.SelectableField{}, ) - storage, statusStorage := registry.NewStorage( + storage, statusStorage, scaleStorage := registry.NewStorage( ctx, resource, "", // no hash, as this is not backed by an APIExport @@ -156,6 +162,31 @@ func filteredLogicalClusterStatusWriteOnly( } } + if scaleEnabled { + subresourceStorages["scale"] = &struct { + registry.FactoryFunc + registry.DestroyerFunc + + registry.GetterFunc + registry.UpdaterFunc + // patch is implicit as we have get + update + + registry.TableConvertorFunc + registry.CategoriesProviderFunc + registry.ResetFieldsStrategyFunc + }{ + FactoryFunc: scaleStorage.FactoryFunc, + DestroyerFunc: scaleStorage.DestroyerFunc, + + GetterFunc: scaleStorage.GetterFunc, + UpdaterFunc: scaleStorage.UpdaterFunc, + + TableConvertorFunc: scaleStorage.TableConvertorFunc, + CategoriesProviderFunc: scaleStorage.CategoriesProviderFunc, + ResetFieldsStrategyFunc: scaleStorage.ResetFieldsStrategyFunc, + } + } + // only expose GET on the regular storage storages := &struct { registry.FactoryFunc diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go index 38e67e09c22..5879932f12c 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver/serving_info.go @@ -184,6 +184,21 @@ func CreateServingInfoFor(genericConfig genericapiserver.CompletedConfig, apiRes subResourcesValidators["status"] = statusValidator } + if scale := apiResourceVersion.Subresources.Scale; scale != nil { + var scaleValidator apiservervalidation.SchemaValidator + equivalentResourceRegistry.RegisterKindFor(gvr, "scale", gvk) + // for the scale subresource, validate only against the scale schema + if internalValidationSchema != nil && internalValidationSchema.OpenAPIV3Schema != nil && internalValidationSchema.OpenAPIV3Schema.Properties != nil { + if scaleSchema, ok := internalValidationSchema.OpenAPIV3Schema.Properties["scale"]; ok { + scaleValidator, _, err = apiservervalidation.NewSchemaValidator(&scaleSchema) + if err != nil { + return nil, err + } + } + } + subResourcesValidators["scale"] = scaleValidator + } + table, err := tableconvertor.New(apiResourceVersion.AdditionalPrinterColumns) if err != nil { klog.Background().V(2).WithValues("cluster", logicalcluster.From(apiResourceSchema), "gvk", gvk, "err", err).Info("the CRD has an invalid printer specification, falling back to default printing") @@ -315,8 +330,8 @@ func CreateServingInfoFor(genericConfig genericapiserver.CompletedConfig, apiRes var resetFields map[fieldpath.APIVersion]*fieldpath.Set if withResetFields, canGetResetFields := subresourceStorage.(rest.ResetFieldsStrategy); canGetResetFields { resetFields = withResetFields.GetResetFields() - } else if subresource == "status" { - return nil, fmt.Errorf("storage for resource %q status should define GetResetFields", gvk.String()) + } else if subresource == "status" || subresource == "scale" { + return nil, fmt.Errorf("storage for resource %q status/scale should define GetResetFields", gvk.String()) } subresourceScope, err = apiextensionsapiserver.ScopeWithFieldManager( diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest.go index ab8240f0aca..fcb18d6a957 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/managedfields" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/client-go/scale/scheme/autoscalingv1" "k8s.io/client-go/util/retry" "github.com/kcp-dev/virtual-workspace-framework/pkg/dynamic/apiserver" @@ -72,7 +73,7 @@ func NewStorage( dynamicClusterClientFunc DynamicClusterClientFunc, patchConflictRetryBackoff *wait.Backoff, wrapper StorageWrapper, -) (mainStorage, statusStorage *StoreFuncs) { +) (mainStorage, statusStorage, scaleStorage *StoreFuncs) { if patchConflictRetryBackoff == nil { patchConflictRetryBackoff = &retry.DefaultRetry } @@ -119,7 +120,45 @@ func NewStorage( if wrapper != nil { wrapper.Decorate(resource.GroupResource(), statusStore) } - return store, statusStore + + scaleFactory := func() runtime.Object { + ret := &unstructured.Unstructured{} + ret.SetGroupVersionKind(autoscalingv1.SchemeGroupVersion.WithKind("Scale")) + return ret + } + + scaleStore := DefaultDynamicDelegatedStoreFuncs( + scaleFactory, + nil, + func() {}, + strategy, + tableConvertor, + resource, + apiExportIdentityHash, + nil, + dynamicClusterClientFunc, + []string{"scale"}, + *patchConflictRetryBackoff, + ctx.Done(), + ) + + delegateScaleGet := scaleStore.GetterFunc + scaleStore.GetterFunc = func(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + if _, err := store.GetterFunc.Get(ctx, name, &metav1.GetOptions{}); err != nil { + return nil, err + } + return delegateScaleGet(ctx, name, options) + } + + delegateScaleUpdate := scaleStore.UpdaterFunc + scaleStore.UpdaterFunc = func(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { + if _, err := store.GetterFunc.Get(ctx, name, &metav1.GetOptions{}); err != nil { + return nil, false, err + } + return delegateScaleUpdate(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) + } + + return store, statusStore, scaleStore } // ProvideReadOnlyRestStorage returns a commonly used REST storage that forwards calls to a dynamic client, @@ -151,7 +190,7 @@ func ProvideReadOnlyRestStorage(ctx context.Context, dynamicClusterClientFunc Dy []apiextensionsv1.SelectableField{}, ) - storage, _ := NewStorage( + storage, _, _ := NewStorage( ctx, resource, identities[resource.GroupResource()], diff --git a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest_test.go b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest_test.go index dbd80978e21..efb7261fe0d 100644 --- a/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest_test.go +++ b/staging/src/github.com/kcp-dev/virtual-workspace-framework/pkg/forwardingregistry/rest_test.go @@ -55,7 +55,7 @@ import ( var noxusGVR = schema.GroupVersionResource{Group: "mygroup.example.com", Resource: "noxus", Version: "v1beta1"} -func newStorage(t *testing.T, clusterClient kcpdynamic.ClusterInterface, apiExportIdentityHash string, patchConflictRetryBackoff *wait.Backoff) (mainStorage, statusStorage rest.Storage) { +func newStorage(t *testing.T, clusterClient kcpdynamic.ClusterInterface, apiExportIdentityHash string, patchConflictRetryBackoff *wait.Backoff) (mainStorage, statusStorage, scaleStorage rest.Storage) { t.Helper() gvr := noxusGVR @@ -147,7 +147,7 @@ func createResource(namespace, name string) *unstructured.Unstructured { func TestGet(t *testing.T) { t.Parallel() fakeClient := kcpfakedynamic.NewSimpleDynamicClient(runtime.NewScheme()) - storage, _ := newStorage(t, fakeClient, "", nil) + storage, _, _ := newStorage(t, fakeClient, "", nil) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithCluster(ctx, request.Cluster{Name: "test"}) @@ -167,7 +167,7 @@ func TestList(t *testing.T) { t.Parallel() resources := []runtime.Object{createResource("default", "foo"), createResource("default", "foo2")} fakeClient := kcpfakedynamic.NewSimpleDynamicClient(runtime.NewScheme(), resources...) - storage, _ := newStorage(t, fakeClient, "", nil) + storage, _, _ := newStorage(t, fakeClient, "", nil) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithCluster(ctx, request.Cluster{Name: "test"}) @@ -199,7 +199,7 @@ func TestWildcardListWithAPIExportIdentity(t *testing.T) { _ = fakeClient.Tracker().Cluster(logicalcluster.NewPath("test")).Create(noxusGVRWithHash, resource, "default") } - storage, _ := newStorage(t, fakeClient, "apiExportIdentityHash", nil) + storage, _, _ := newStorage(t, fakeClient, "apiExportIdentityHash", nil) ctx := request.WithNamespace(context.Background(), "") ctx = request.WithCluster(ctx, request.Cluster{Wildcard: true}) @@ -251,7 +251,7 @@ func TestWatch(t *testing.T) { fakeWatcher := watch.NewFake() t.Cleanup(fakeWatcher.Stop) fakeClient.PrependWatchReactor("noxus", kcptesting.DefaultWatchReactor(fakeWatcher, nil)) - storage, _ := newStorage(t, fakeClient, "", nil) + storage, _, _ := newStorage(t, fakeClient, "", nil) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithCluster(ctx, request.Cluster{Name: "test"}) @@ -296,7 +296,7 @@ func TestWildcardWatchWithPIExportIdentity(t *testing.T) { fakeWatcher := watch.NewFake() t.Cleanup(fakeWatcher.Stop) fakeClient.PrependWatchReactor("noxus:apiExportIdentityHash", kcptesting.DefaultWatchReactor(fakeWatcher, nil)) - storage, _ := newStorage(t, fakeClient, "apiExportIdentityHash", nil) + storage, _, _ := newStorage(t, fakeClient, "apiExportIdentityHash", nil) ctx := request.WithNamespace(context.Background(), "") ctx = request.WithCluster(ctx, request.Cluster{Wildcard: true}) @@ -358,7 +358,7 @@ func TestUpdate(t *testing.T) { fakeClient := kcpfakedynamic.NewSimpleDynamicClient(runtime.NewScheme()) fakeClient.PrependReactor("update", "noxus", updateReactor(fakeClient)) - storage, _ := newStorage(t, fakeClient, "", nil) + storage, _, _ := newStorage(t, fakeClient, "", nil) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithCluster(ctx, request.Cluster{Name: "test"}) updated := resource.DeepCopy() @@ -405,7 +405,7 @@ func TestUpdateWithForceAllowCreate(t *testing.T) { fakeClient := kcpfakedynamic.NewSimpleDynamicClient(runtime.NewScheme()) fakeClient.PrependReactor("update", "noxus", updateReactor(fakeClient)) - storage, _ := newStorage(t, fakeClient, "", nil) + storage, _, _ := newStorage(t, fakeClient, "", nil) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithCluster(ctx, request.Cluster{Name: "test"}) updated := resource.DeepCopy() @@ -448,7 +448,7 @@ func TestStatusUpdate(t *testing.T) { fakeClient := kcpfakedynamic.NewSimpleDynamicClient(runtime.NewScheme(), resource) fakeClient.PrependReactor("update", "noxus", updateReactor(fakeClient)) - _, statusStorage := newStorage(t, fakeClient, "", nil) + _, statusStorage, _ := newStorage(t, fakeClient, "", nil) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithCluster(ctx, request.Cluster{Name: "test"}) statusUpdated := resource.DeepCopy() @@ -478,7 +478,7 @@ func TestPatch(t *testing.T) { backoff := retry.DefaultRetry backoff.Steps = 5 - storage, _ := newStorage(t, fakeClient, "", &backoff) + storage, _, _ := newStorage(t, fakeClient, "", &backoff) ctx := request.WithNamespace(context.Background(), "default") ctx = request.WithRequestInfo(ctx, &request.RequestInfo{Verb: "patch"}) ctx = request.WithCluster(ctx, request.Cluster{Name: "test"})