capsule

capsule copied to clipboard

We are using a specific struct, AllowedListSpec:

type AllowedListSpec struct {
	Exact []string `json:"allowed,omitempty"`
	Regex string   `json:"allowedRegex,omitempty"`
}

to allow tenant users to use only specific IngressClasses, StorageClasses or PriorityClasses.

While it allows additional fine-grained configuration, that is not a kubernetes way of specifying resources. Much better will be to use label selectors (the same way, as it is in nodeSelector option for tenant).

With this, tenant spec will look like:

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - name: alice
    kind: User
  storageClasses:
    selector:
       foo: bar  

This change will simplify webhook code, make it works much faster (no regexp check), and will simplify the code of a capsule-proxy (as we won't need to get all resources first and then perform regexp validation or array probing).

The downside of this change is that it is completely backward incompatible and there will be no way to automatically upgrade tenant spec to this new version.

@bsctl, @prometherion need your opinion on this.

@MaxFedotov sounds good to me since more Kubernetes like. What about labelSelector ?

For backward compatibility same concerns. Let’s see @prometherion point of view

@bsctl

What about labelSelector ?

Can you please explain in more details?

Can you please explain in more details?

just this:

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - name: alice
    kind: User
  storageClasses:
    labelSelector:
       foo: bar  

@MaxFedotov sounds good to me since more Kubernetes like. What about labelSelector ?

Couldn't agree more on this.

For backward compatibility same concerns. Let’s see @prometherion point of view

This change is absolutely breaking and not portable using a conversion. The unique way to use it would be adding labelSelector a third option in the AllowedList spec.

https://github.com/clastix/capsule/blob/d3e3b8a881b4ce25332e8d1a6904ff178b11242d/api/v1beta1/allowed_list.go#L12-L15

However, this would make things even more complicated on capsule-proxy side since we would need to add additional checks in the selection.

I got a plan:

1. introducing this feature for v1beta1 as annotation
2. plan landing on v1beta2
3. switch entirely towards selection for v1 API version that will be Capsule v1.0.0

Do you think this could be feasible?

@prometherion as we're approaching new API version for Tenant resource, what about to introduce this enhancement? To avoid breaking changes, we can just add another method of specifying Classes to make the transition smooth:

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - name: alice
    kind: User
  storageClasses:
    labelSelector: # <<< new way in v1beta2
       foo: bar
    allowed: # <<< old way deprecated but still supported in v1beta2, removed in v1
    - default
    allowedRegex: "^tier-.*$"  

@bsctl you're right, @MaxFedotov agreed on working on this.