Merge pull request #3494 from xmudrii/vw-admission

Implement admission for Virtual Workspaces and label selectors for PermissionClaims

Author: kcp-ci-bot

cmd/virtual-workspaces/command/cmd.go

@@ -178,6 +178,12 @@ func Run(ctx context.Context, o *options.Options) error {
 		return err
 	}
 
+	if err := o.VirtualWorkspaceAdmission.ApplyTo(&recommendedConfig.Config, func() []virtualrootapiserver.NamedVirtualWorkspace {
+		return rootAPIServerConfig.Extra.VirtualWorkspaces
+	}); err != nil {
+		return err
+	}
+
 	sharedExternalURLGetter := func() string {
 		return o.ShardExternalURL
 	}

cmd/virtual-workspaces/options/options.go

@@ -55,7 +55,8 @@ type Options struct {
 
 	Logs *logs.Options
 
-	CoreVirtualWorkspaces corevwoptions.Options
+	CoreVirtualWorkspaces     corevwoptions.Options
+	VirtualWorkspaceAdmission corevwoptions.Admission
 
 	ProfilerAddress string
 }
@@ -74,8 +75,9 @@ func NewOptions() *Options {
 		Audit:          *genericapiserveroptions.NewAuditOptions(),
 		Logs:           logs.NewOptions(),
 
-		CoreVirtualWorkspaces: *corevwoptions.NewOptions(),
-		ProfilerAddress:       "",
+		CoreVirtualWorkspaces:     *corevwoptions.NewOptions(),
+		VirtualWorkspaceAdmission: *corevwoptions.NewAdmission(),
+		ProfilerAddress:           "",
 	}
 
 	opts.SecureServing.ServerCert.CertKey.CertFile = filepath.Join(".", ".kcp", "apiserver.crt")

config/crds/apis.kcp.io_apibindings.yaml

@@ -557,9 +557,6 @@ spec:
                       x-kubernetes-validations:
                       - message: Permission claim selector is immutable
                         rule: self == oldSelf
-                      - message: a selector is required. Only "matchAll" is currently
-                          implemented
-                        rule: (has(self.matchAll) && self.matchAll)
                     state:
                       enum:
                       - Accepted
@@ -710,9 +707,6 @@ spec:
                       x-kubernetes-validations:
                       - message: Permission claim selector is immutable
                         rule: self == oldSelf
-                      - message: a selector is required. Only "matchAll" is currently
-                          implemented
-                        rule: (has(self.matchAll) && self.matchAll)
                     verbs:
                       description: |-
                         verbs is a list of supported API operation types (this includes

docs/content/concepts/apis/exporting-apis.md

@@ -177,10 +177,12 @@ served to clients. See [Build Your Controller](#build-your-controller) for more
 When a consumer creates an `APIBinding` that binds to an `APIExport`, the API provider who owns the `APIExport`
 implicitly has access to instances of the exported APIs in the consuming workspace. There are also times when the API
 provider needs to access additional resource data in a consuming workspace. These resources might come from other
-APIExports the consumer has created `APIBindings` for, or from APIs that are built in to kcp. The API provider
-requests access to these additional resources by adding `PermissionClaims` for the desired API's group, resource, and
-identity hash to their `APIExport`. The API provider is responsible for specifying what operations are required for the
-requested resource by setting the appropriate [API verbs](https://kubernetes.io/docs/reference/using-api/api-concepts/#api-verbs).
+APIExports the consumer has created `APIBindings` for, or from APIs that are built in to kcp.
+
+The API provider requests access to these additional resources by adding `PermissionClaims` for the desired API's
+group, resource, and identity hash to their `APIExport`. The API provider is responsible for specifying what operations
+are required for the requested resource by setting the appropriate
+[API verbs](https://kubernetes.io/docs/reference/using-api/api-concepts/#api-verbs).
 Let's take the example `APIExport` from above and add permission claims for `ConfigMaps` and `Things`:
 
 ```yaml
@@ -211,10 +213,18 @@ spec:
 4. `"*"` is a special "verb" that matches any possible verb
 
 This is essentially a request from the API provider, asking each consumer to grant permission for the claimed
-resources. If the consumer does not accept a permission claim, the API provider is not allowed to access the claimed
-resources. Consumer acceptance of permission claims is part of the `APIBinding` spec. The operations allowed on the
-resource are the intersection of the verbs defined in the `APIExport` and the verbs accepted in the appropriate
-`APIBinding`. For more details, see the section on [APIBindings](#apibinding).
+resources. The consumer can, via `APIBinding`, accept or reject the service provider's request to access these
+resources, i.e. accept or reject PermissionClaims.
+
+Additionally, the consumer can choose between giving access to all instances (objects) of a claimed resource
+and giving access only to a subset of instances. In the latter case, the consumer can specify labels when accepting
+the PermissionClaim, so the service provider can only access instances which have these specified labels
+(this is also known as a label selector).
+
+The set of operations that the service provider can perform on the claimed resource is the intersection of the verbs
+defined in the `APIExport` and the verbs accepted in the appropriate `APIBinding`.
+
+For more details, see the section on [APIBindings](#apibinding).
 
 ### Maximal Permission Policy
 
@@ -420,8 +430,13 @@ spec:
 
 #### Permission Claims
 
-Furthermore, `APIBindings` provide the `APIExport` owner access to additional resources defined in an `APIExport`'s permission claims list. Permission claims must be accepted by the user explicitly, before this access is granted. The resources can be builtin Kubernetes resources or resources from other `APIExports`.
-When an `APIExport` is changed after workspaces have bound to it, new or changed APIs are automatically propagated to all `APIBindings`. New permission claims on the other hand are NOT automatically accepted.
+Furthermore, `APIBindings` provide the `APIExport` owner access to additional resources defined in an `APIExport`'s
+permission claims list. Permission claims must be accepted by the user explicitly, before this access is granted.
+The resources can be builtin Kubernetes resources or resources bound with other `APIBindings`.
+
+!!! information
+    When an `APIExport` is changed after workspaces have bound to it, new or changed APIs are automatically propagated
+    to all `APIBindings`. New permission claims on the other hand are NOT automatically accepted.
 
 Returning to our example, we can grant the requested permissions in the `APIBinding`:
 
@@ -450,7 +465,8 @@ spec:
       matchAll: true
 ```
 
-It should be noted that `APIBindings` do not create `CRDs` or `APIResourceSchemas`in the workspace. Instead APIs are directly bound using Kubernetes' internal binding mechanism behind the scenes.
+It should be noted that `APIBindings` do not create `CRDs` or `APIResourceSchemas` in the workspace.
+Instead APIs are directly bound using Kubernetes' internal binding mechanism behind the scenes.
 
 ##### Verbs
 
@@ -459,10 +475,51 @@ the verbs in the APIBinding and the verbs in the appropriate APIExport.
 
 ##### Selector
 
-`APIBindings` allow API consumers to scope an API provider's access to claimed resources via the `selector` field on a permission claim. This means that providers will only be able to see and access those objects matched by the `selector`.
+`APIBindings` allow API consumers to scope an API provider's access to claimed resources via the `selector` field on
+a permission claim. This means that providers will only be able to see and access those objects matched by
+the `selector`.
+
+There are two types of selectors at the moment:
+
+- `matchAll`: gives the service provider access to all objects of a claimed resource
+- label selector: gives the service provider access only to objects which are satisfying the given label selector
+
+The `matchAll` selector is shown in the example above.
+
+A label selector can be defined using `matchLabels` or `matchExpressions`:
+
+- `matchLabels` specifies a set of labels (key-value pairs). For the selector to match, **all** of the listed labels
+  must be present on the object.
+- `matchExpressions` specifies a set of expressions that are evaluated against object’s labels. If multiple expressions
+  are specified, **all must evaluate to `true`** for the selector to match.
+
+```yaml
+...
+  permissionClaims:
+  - resource: configmaps
+    verbs: ["get", "list", "create"]
+    state: Accepted
+    selector:
+      matchLabels:
+        env: prod
+        app: logbook
+  - resource: things
+    group: somegroup.kcp.io
+    identityHash: 5fdf7c7aaf407fd1594566869803f565bb84d22156cef5c445d2ee13ac2cfca6
+    verbs: ["*"]
+    state: Accepted
+    selector:
+      matchExpressions:
+        - key: env
+          operator: In
+          values: ["prod", "staging"]
+```
 
 !!! information
-    Currently, only `selector.matchAll=true` is supported, giving the provider that owns the `APIExport` full access to all objects of a claimed resource. Additional selectors are planned for upcoming releases.
+    Special attention is needed by the service provider when creating or updating an object via the APIExport Virtual
+    Workspace. If `matchLabels` is used, the specified labels will be automatically applied to the object that's being
+    applied even if not specified by the service provider. However, that's not the case for `matchExpressions`,
+    in which case the service provider needs to explicitly specify labels upon applying the object.
 
 ---
 

docs/content/concepts/quickstart-tenancy-and-apis.md

@@ -365,6 +365,8 @@ spec:
 
 Operations allowed on the resources for which permission claim is accepted is defined as the intersection of the verbs in the `APIBinding` and the verbs in the `APIExport`. Verbs in this case are matching the verbs used by the [Kubernetes API](https://kubernetes.io/docs/reference/using-api/api-concepts/#api-verbs). There is the possibility to further limit the access claim to single resources.
 
+PermissionClaims allows for additional selectors, for more details, check out the [APIBindings documentation](./apis/exporting-apis.md#apibinding).
+
 ## Dig Deeper into APIExports
 
 Switching back to the service provider persona:

pkg/permissionclaim/permissionclaim_labeler.go

@@ -22,6 +22,8 @@ import (
 
 	authenticationv1 "k8s.io/api/authentication/v1"
 	authorizationv1 "k8s.io/api/authorization/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	klabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
@@ -117,10 +119,23 @@ func (l *Labeler) LabelsFor(ctx context.Context, cluster logicalcluster.Name, gr
 		}
 
 		for _, claim := range binding.Spec.PermissionClaims {
-			if claim.State != apisv1alpha2.ClaimAccepted || claim.Group != normalizedGR.Group || claim.Resource != normalizedGR.Resource || !claim.Selector.MatchAll {
+			if claim.State != apisv1alpha2.ClaimAccepted || claim.Group != normalizedGR.Group || claim.Resource != normalizedGR.Resource {
 				continue
 			}
 
+			if !claim.Selector.MatchAll {
+				selector, err := metav1.LabelSelectorAsSelector(&claim.Selector.LabelSelector)
+				if err != nil {
+					logger.Error(err, "error calculating permission claim label key and value",
+						"claim", claim.String())
+					continue
+				}
+
+				if !selector.Matches(klabels.Set(resourceLabels)) {
+					continue
+				}
+			}
+
 			k, v, err := permissionclaims.ToLabelKeyAndValue(logicalcluster.From(export), export.Name, claim.PermissionClaim)
 			if err != nil {
 				// extremely unlikely to get an error here - it means the json marshaling failed

pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_resource_reconcile.go

@@ -34,7 +34,7 @@ import (
 
 	"github.com/kcp-dev/logicalcluster/v3"
 
-	apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
+	apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2"
 )
 
 func (c *resourceController) reconcile(ctx context.Context, obj *unstructured.Unstructured, gvr *schema.GroupVersionResource) error {
@@ -48,7 +48,7 @@ func (c *resourceController) reconcile(ctx context.Context, obj *unstructured.Un
 
 	actualClaimLabels := make(map[string]string)
 	for k, v := range obj.GetLabels() {
-		if strings.HasPrefix(k, apisv1alpha1.APIExportPermissionClaimLabelPrefix) {
+		if strings.HasPrefix(k, apisv1alpha2.APIExportPermissionClaimLabelPrefix) {
 			actualClaimLabels[k] = v
 		}
 	}

pkg/server/virtual.go

@@ -85,6 +85,13 @@ func newVirtualConfig(
 		return nil, err
 	}
 
+	admissionOptions := virtualoptions.NewAdmission()
+	if err := admissionOptions.ApplyTo(&recommendedConfig.Config, func() []virtualrootapiserver.NamedVirtualWorkspace {
+		return c.Extra.VirtualWorkspaces
+	}); err != nil {
+		return nil, err
+	}
+
 	c.Extra.VirtualWorkspaces, err = o.Virtual.VirtualWorkspaces.NewVirtualWorkspaces(
 		config,
 		virtualcommandoptions.DefaultRootPathPrefix,