projectcapsule / capsule-proxy

Reverse proxy for Capsule Operator.
https://github.com/projectcapsule/capsule
Apache License 2.0
44 stars 40 forks source link

Custom resources list for --feature-gates=ProxyAllNamespaced=true. #530

Open anatolychernov opened 2 months ago

anatolychernov commented 2 months ago

Describe the feature

According to issue 303, capsule-proxy provides the ability to list all resources within a user’s tenants when the ProxyAllNamespaced feature flag is enabled, e.g. kubectl get pods --all-namespaces

It would be great if we could manage the list of resources where a user can use the cluster scope flag --all-namespaces, e.g.

ProxyAllNamespacedResourcesList:
  - pods/v1
  - secrets/v1
  - configmaps/v1

Some operators (like the Operator Lifecycle Manager) strictly control their resource labels, and if the capsule-proxy tries to add its own label, a conflict between the capsule-proxy and the operator may occur.

If you think this idea is good, I can try to prepare a merge request.

What would the new user story look like?

If ProxyAllNamespacedResourcesList is not empty, the user can apply the --all-namespaces flag only to the resources defined in ProxyAllNamespacedResourcesList. Otherwise, if ProxyAllNamespacedResourcesList is empty, the user can still list all types of resources under their tenants using the --all-namespaces flag.

Expected behavior

A clear and concise description of what you expect to happen.

oliverbaehler commented 1 month ago

hi @anatolychernov, it's a bit unclear to me what you are trying to solve. Do you want for users only to be able to list some namespaced resources via selectors, such as we are using for clusterscoped resources?

anatolychernov commented 1 month ago

Hi @oliverbaehler,

Thank you for reply!

As I understand, this code block collects all the APIs available in the cluster. I would like to suggest implementing a filter for collecting APIs.

In my example above, the filter should look like this:

ProxyAllNamespacedResourcesList:
  - pods/v1
  - secrets/v1
  - configmaps/v1

So, only the APIs listed above will be stored in the apis variable.

As an outcome, the user will be able to execute commands like kubectl get pods -A or kubectl get secrets -A, but it will not be allowed to execute commands such as kubectl get cvs -A, because cvs kind is outside the filter.

Do you want for users only to be able to list some namespaced resources via selectors, such as we are using for clusterscoped resources?

It seems, yes.

oliverbaehler commented 1 month ago

Gotchu, I would probably prefer using filter rules instead of skipping APIs. For Example:

---
apiVersion: capsule.clastix.io/v1beta1
kind: ProxySetting/GlobalProxySetting
metadata:
  name: argocd-serviceaccount
spec:
  rules:
  - subjects:
     - kind: ServiceAccount
        name: system:serviceaccount:capsule-argocd-addon:solar
    namespacedResources:
    - apiGroups:
      - '*'
      resources:
      - '*'

      # Optional, if not set, use tenant selector
      selector:
        matchExpressions:
         - { key: tier, operator: In, values: [cache] }

Your example would translate to the dollowing setting:

---
apiVersion: capsule.clastix.io/v1beta1
kind: GlobalProxySetting
metadata:
  name: selective-namespaced
spec:
  rules:
  - subjects:
     - kind: Group
        name: capsule-users
    namespacedResources:
    - apiGroups:
      - '*'
      resources:
      - 'pods'
      - 'secrets'
      - 'configmaps'

This would restrict listing the resources to pods, secrets, configmaps (for the tenants they are part of).

anatolychernov commented 1 month ago

@oliverbaehler,

Could you clarify, if I set:

namespacedResources:
- apiGroups:  
  - '*'  
  resources:  
  - 'pods'  
  - 'secrets'  
  - 'configmaps'  

Will the capsule add the capsule.clastix.io/tenant: "mytenant" label to other kinds of entities, such as ingress?(I'd like to prevent it).

oliverbaehler commented 1 month ago

Yes, but you could also add custom labels:

apiVersion: capsule.clastix.io/v1beta1
kind: GlobalProxySetting
metadata:
  name: selective-namespaced
spec:
  rules:
  - subjects:
     - kind: Group
        name: capsule-users
    namespacedResources:
    - apiGroups:
      - '*'
      resources:
      - 'pods'
      - 'secrets'
      - 'configmaps'
      selector:
        matchLabels:
          company-wide: label
          nonexisting: "label"

Effictively overwriting the tenant label. It's just a draft in my head btw.. :D Would you prefer not using labels at all?

anatolychernov commented 1 month ago

@oliverbaehler

Let me highlight the main problem I am trying to solve.

If I enable --feature-gates=ProxyAllNamespaced=true, Capsule adds the tenant label to all objects of any kind, and some operators, like the Operator Lifecycle Manager cannot work properly due to the presence of unknown labels in their kinds.

For example, OLM (Operator Lifecycle Manager) creates ClusterServiceVersions, and Capsule adds a label with the tenant. OLM discovers the label created by Capsule and tries to remove it, causing OLM to malfunction.

Can I enable --feature-gates=ProxyAllNamespaced=true and configure Capsule to not add the tenant label to ClusterServiceVersions entities or entities which I like to exclude?

oliverbaehler commented 1 month ago

Oh okay, yeah we can implement something like this. Not quiet sure yet if passing this via args is nice. I would probably do something like this:

/ So in your case v1/secrets etc. This way we could consider using regex pattern, so you could blacklist entire api groups, eg. addons.projectcapsule.dev/* Then just not registering the route with the proxy. I would probably also implement some sort of gc. eg. select resources of the blacklisted domain and removed the managed-by label. Probably we could also make the managed-by label variable via args to further avoid such conflicts. @prometherion WDYT
prometherion commented 1 month ago

If I enable --feature-gates=ProxyAllNamespaced=true, Capsule adds the tenant label to all objects of any kind, and some operators, like the Operator Lifecycle Manager cannot work properly due to the presence of unknown labels in their kinds.

This is the problem: external labels should not be withdrawn by other components since it would create such errors. Rather than dealing with Capsule, I would get in touch with the OLM community asking to avoid this wrong label management.

I'm not a fan of customising the supported API for such a feature, mostly because it puts more burden on our side for a wrong behaviour by other components: an Operator should only manage its own set of labels, and OLM is breaking this contract.

anatolychernov commented 1 month ago

If it is possible to fix the misbehavior on the OLM side, it would be great.