Tenant owner can list the namespace not managed by Capsule

[sagar-jadhav]

Bug description

As a Tenant Owner I am able to list the namespace not managed by Capsule.

How to reproduce

Create a Tenant oil as a cluster admin

apiVersion: capsule.clastix.io/v1beta2
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - name: alice
    kind: User

Create a namespace db as a cluster admin

kubectl create ns db
kubectl label ns name=db

Create a RoleBinding admin as a cluster admin

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  name: admin
  namespace: db
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: admin
subjects:
- apiGroup: rbac.authorization.k8s.io
  kind: User
  name: alice

Note: Issue is not tied to any Role you can refer any Role or ClusterRole in the RoleBinding but in subject user needs to be Alice.

Create a namespace oil-frontend as a Alice user

kubectl create ns oil-frontend

List the namespace as a Alice user

kubectl get ns

NAME           STATUS   AGE
db             Active   3h
oil-frontend   Active   1h

Expected behaviour

As a Tenant owner I should only see the namespaces managed by Capsule. In the above e.g. only oil-frontend should be listed.

Additional context

• Helm Chart version: v0.4.2

[sagar-jadhav]

@prometherion I can work on this issue please assign it to me. I have debugged this issue and found the root cause.

capsule-proxy cached the RoleBindings using RoleBindingReflector to get the mapping of user and namespaces and then set the label selector in the request to show only those namespaces which belongs to that user. I think the issue here is capsule-proxy cached all the RoleBindings present in the cluster.

As the purpose of RoleBindingReflector is to get the mapping of user and a capsule managed namespace it should only cache the RoleBinding created by Capsule.

The fix for this issue is in two places:

1. Capsule controller will add default label let say capsule.clastix.io/managed-by: capsule to a capsule-<tenant_name>-0-admin RoleBinding when syncing the RoleBindings. This RoleBinding is created by default in the namespace managed by Capsule.
2. RoleBindingReflector will use label selector instead of fields.Everything() while creating watcher in the capsule-proxy.

optionsModifier := func(options *metav1.ListOptions) {
     options.LabelSelector = "capsule.clastix.io/managed-by: capsule"
}
watcher := cache.NewFilteredListWatchFromClient(clientset.RbacV1().RESTClient(), "rolebindings", "", optionsModifier)

In this way only Capsule managed RoleBindings are cached.

Please provide the feedback on the root cause and the proposed solution.

[MaxFedotov]

Hi @sagar-jadhav
It’s not a bug, it is a feature that was added in #149

The main idea behind this feature was following: if a person had permissions in any namespace (it doesn’t actually matters if is managed by capsule or not), it means that this permission was explicitly granted for him and he should be able to see this namespaces using namespace list operation (because he already have some permissions it makes no sense to hide it)

One of the use cases for this feature would be, for example, allowing tenant admins from all cluster tenants to have permissions only for reading logs on a shared ingress located in some other namespace (which can be an another tenant or standalone namespace)

[prometherion]

Thanks for the prompt response, Max!

The role binding reflector has been implemented by Max prior to the introduction of a capsule-proxy specific CustomResourceDefinition, such as the ProxySetting resource.

My idea is, in the longer term, to drop the role-binding reflector since it's hard-coded to the admin role (that could be misleading, it depends) since the purpose of the ProxySetting custom resource definition.

If this is blocking you, we introduced a new flag that is "disabling" the role binding reflector and relies only on the remote state of the Kubernetes cluster, bypassing the cache, such as the --disable-caching flag: it has been designed for a different purpose (#266) but it can work for you.

[prometherion]

Closing, also considering that discussion about the role binding reflector deprecation has started with #268.

[sagar-jadhav]

If it is designed to list all the namespaces (namespaces managed by capsule + others) for a user then YES it makes sense to work as it is. I thought it is designed to list only namespaces managed by Capsule hence pointed it out as Bug. Thank you @MaxFedotov for the information.

@prometherion I have used --disable-caching flag and found that it works as expected. But I am not sure how will it impact the overall behaviour? Can you provide some insights on it.

Thanks