Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intercept any API/Kind #185

Closed
oliverbaehler opened this issue Feb 11, 2022 · 12 comments · Fixed by #389
Closed

Intercept any API/Kind #185

oliverbaehler opened this issue Feb 11, 2022 · 12 comments · Fixed by #389
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@oliverbaehler
Copy link
Collaborator

We talked to some of our developers and they ask why they can't execute commands like kubectl get task -A(Tekton Task CR) an just get all the tasks they have access to with that command.

And i thought it would be a great feature to dynamically configure which resources are intercepted for which users. For example something like this:

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - kind: User
    name: alice
    # ...
    proxySettings:
    - api: tekton.dev/v1beta1
      kind: Task
      operations:
      - List
    - api: tekton.dev/v1beta1
      kind: Pipeline
      operations:
      - List
    - kind: PriorityClasses
      operations:
      - List

So as little as I understand from the code it would be difficult to change the implementation for the currently proxied resources/modules. However that's why I added an api field so we could differ between supported modules and a generic api and kind. Those we would have to handle differently.

Before trying to create a solution I wanted to hear your opinions. For us this would help improve the customer experience tremendously and we need something like that in place.

@prometherion
Copy link
Member

Thanks for submitting a feature request, we really appreciated it!

Let me explain the root cause behind capsule-proxy: essentially, we wanted to solve the missing ACL control of the Kubernetes API Server. The Kubernetes RBAC has been designed around resources, designing the permissions on these, breaking what an end-user would expect, such as give me the list of Namespace resources belong to me, pretty common for web services or software user-centric.

I guess we found a smart and non-opinionated way to solve this and we were lucky (or good at, still unsure) on designing Capsule to overcome the list of owned Namespace resources: since all the Namespace resources are labeled with the key-value capsule.clastix.io/tenant=${TENANT_NAME} label, we're exploiting the Kubernetes API Server listing capabilities by decorating the HTTP request adding a label selector, such as list Namespaces which key capsule.clastix.io/tenant is in (oil, gas, etc) (that translated to query string, would be something as capsule.clastix.io/tenant in (oil,gas)).

Besides that, we overcame also the limitations of other cluster-scoped resources used by Tenant resources, such as PriorityClass, Node, IngressClass, and StorageClass, although with a limitation: these must be labeled with their name. The limitation is that we're using the Kubernetes API Server filtering capabilities in order to proxy-pass the requests and get two birds with one stone.

Finally, with more context, I can try to provide further analysis on the requested feature. Retrieving a Namespaced-scope resource for a specific Namespace is easy, we just need to issue kubectl get myresource -n mynamespace that will be translated to the following URL: https://127.0.0.1:43627/apis/mygroup.tld/v1/namespaces/mynamespace/myresources.

When trying to retrieve resources at the cluster-wide level the flag --all-namespace must be used, and the following request will be something like this: https://127.0.0.1:43627/apis/mygroup.tld/v1/myresource.

There's no way to retrieve resources in two or more specific Namespaces due to the API design: a workaround could be using the fields selector that allows using the key metadata.namespace, although the available operators are limited to a single value, and it means we cannot do the trick as we do with labels (e.g.: metadata.namespace in (gas-development, gas-staging, gas-etc)).

A possible solution for this would be implementing a response writer as the API Server would do, and this is tracked in #57: @MaxFedotov was working on that and encountered several issues, mostly because of all the filtering, listing, and methods must be re-implemented.

I don't have any idea on how to nail the requested feature according to the current design of capsule-proxy since it relies entirely on the HTTP proxy-pass with query-string mangling. With that said, I'm open to getting a discussion on this, since I strongly believe that a feature like that would be terrific from the DX standpoint.

@prometherion prometherion added enhancement New feature or request help wanted Extra attention is needed labels Feb 12, 2022
@oliverbaehler
Copy link
Collaborator Author

oliverbaehler commented Feb 14, 2022

Hi Dario, Thanks for your fast response. I am aware of your named points, let me elaborate on my use case a bit more:

We are currently trying to bootstrap a tekton dashboard as tenant feature (). The tekton dashboard is very simplistic since it just forwards requests. If you want to see the overview of all tasks, it tries to query /apis/tekton.dev/v1beta1/tasks. This obviously won't work since the serviceAccount running within the dashboard application does only have privileges to list all tasks in the tenant member namespaces. So that's where this feature would help: If I could just tell capsule: Hey! please also intercept only list request for this kind, on this api group/version it would solve my problem. Obviously any get request is not really relevant.

This is only speaking for namespaced resources as of now. Let me show you what i have tried so far (with my very limited experience):

I have added a new module called additional: oliverbaehler@55ecf7e#diff-ae36cf9a0e9906f5bc5faca429427ffe6c90e1c7c722694aacf4184621ff8432

It's basically the same as the namespace module, but allows as parameter to give any kind. The idea is to check the namespace of the kind with those which are allowed by the capsuleConfiguration.

Here's the iteration which would add any additional interceptor: oliverbaehler@55ecf7e#diff-abd7ed06d2e103da6501fbaf553c64d33d62628dd46621aa8fbaab8452dac7f7

the dicts are just for testing, i would get it via the capsuelConfiguration https://github.com/oliverbaehler/capsule/blob/master/api/v1alpha1/capsuleconfiguration_types.go#L25

So if I create an object like this:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: hello
  labels:
    namespace: gas
  namespace: gas
spec:
  steps:
    - name: hello
      image: ubuntu
      command:
        - echo
      args:
        - "Hello World!"

It works:

curl -H "Authorization: Bearer $TOKEN" http://localhost:9001/apis/tekton.dev/v1beta1/tasks
{"apiVersion":"tekton.dev/v1beta1","items":[{"apiVersion":"tekton.dev/v1beta1","kind":"Task","metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"tekton.dev/v1beta1\",\"kind\":\"Task\",\"metadata\":{\"annotations\":{},\"labels\":{\"namespace\":\"gas\"},\"name\":\"hello\",\"namespace\":\"gas\"},\"spec\":{\"steps\":[{\"args\":[\"Hello World!\"],\"command\":[\"echo\"],\"image\":\"ubuntu\",\"name\":\"hello\"}]}}\n"},"creationTimestamp":"2022-02-14T11:06:00Z","generation":1,"labels":{"namespace":"gas"},"managedFields":[{"apiVersion":"tekton.dev/v1beta1","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:annotations":{".":{},"f:kubectl.kubernetes.io/last-applied-configuration":{}},"f:labels":{".":{},"f:namespace":{}}},"f:spec":{".":{},"f:steps":{}}},"manager":"kubectl-client-side-apply","operation":"Update","time":"2022-02-14T11:09:13Z"}],"name":"hello","namespace":"gas","resourceVersion":"423373","uid":"3e14bdd5-949f-4196-8d9e-1c8018666ea2"},"spec":{"steps":[{"args":["Hello World!"],"command":["echo"],"image":"ubuntu","name":"hello","resources":{}}]}}],"kind":"TaskList","metadata":{"continue":"","resourceVersion":"423416"}}

So I would just need to change the selector to metadata.namespace instead of a label, right? Which doesn't work since we would need the fieldSelector an there we have the one or any namespace problem you mentioned. dang it

Sorry I might be completely wrong .. (https://www.youtube.com/watch?v=rR4n-0KYeKQ)

@oliverbaehler
Copy link
Collaborator Author

So I guess really the only solution would be to tag resources with the namespace as label the live in? Do you think that makes sense?

@prometherion
Copy link
Member

Yes, unfortunately we cannot list with complex query against metadata.namespace.

With that said, labelling resources with their tenant name is mandatory, we can plan a feature!

@oliverbaehler
Copy link
Collaborator Author

oliverbaehler commented Mar 21, 2022

Hi @prometherion. I could use your opinion on this one. I would have added the following configuration Option to the capsuleConfigurationKind of the capsule project:

// CapsuleConfigurationSpec defines the Capsule configuration
type CapsuleConfigurationSpec struct {
	// Names of the groups for Capsule users.
	// +kubebuilder:default={capsule.clastix.io}
	UserGroups []string `json:"userGroups,omitempty"`

	// Enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix,
	// separated by a dash. This is useful to avoid Namespace name collision in a public CaaS environment.
	// +kubebuilder:default=false
	ForceTenantPrefix bool `json:"forceTenantPrefix,omitempty"`

	// Disallow creation of namespaces, whose name matches this regexp
	ProtectedNamespaceRegexpString string `json:"protectedNamespaceRegex,omitempty"`

	// Additional api endpoints which are intercepted by the capsule-proxy
	AdditionalInterceptors []InterceptorReference `json:"additionalInterceptors,omitempty"`
}
// InterceptorReference defines an kubernetes api which is intercepted
type InterceptorReference struct {

	// API Group to intercept
	// +kubebuilder:validation:Required
	Group string `json:"group,omitempty"`

	// API Version
	// +kubebuilder:default: v1
	Version string `json:"version,omitempty"`

	// Kinds to intercept on given api group
	// +kubebuilder:validation:Required
	Kinds []string `json:"kind,omitempty"`

}

But I don't know if it makes sense to add it to the capsuleConfiguration. Maybe it would make more sense to make it's own Kind a la capsuleProxyConfigurationso that we don't interfere too much with the capsule project.

How would you approach this?

About labeling: I would have approached this with a kyverno policy, or were you thinking about adding this to capsule, so capsule would label all resources which are in a tenant namespace with the namespace's name? Would be nice as well.

@prometherion
Copy link
Member

If I understood correctly, you would like to put in place a MutatingWebhookConfiguration allowing to intercept the kinds declared in the CapsuleConfiguration so we can mutate them adding the desired label, required for capsule-proxy lately to perform the aggregated selection through the label selector, isn't it?

@prometherion
Copy link
Member

BTW, since the last effort with #196, wondering if we should provide this feature taking advantage of CRDs: what are your thoughts about this, @oliverbaehler?

The rationale behind this proposal is to decouple Capsule Proxy features from Capsule.

@oliverbaehler
Copy link
Collaborator Author

@prometherion Thanks for the feedback

Use-Cases

I think we need to differ the use-cases based on if a resource if namespaced or not

Cluster Scoped Resources

The currently implemented option to proxy permissions to cluster scoped resources. So #196 makes sense to me. Here it would be interesting to add the capability to proxy any other cluster scoped resource (eg clusterinterceptors.triggers.tekton.dev/v1alpha1)

apiVersion: capsule.clastix.io/v1beta1
kind: ProxySettings
metadata:
  name: sre-readers
  namespace: solar-management
spec:
  subjects:
  - name: sre
    kind: Group
    proxySettings:
    - kind: clusterinterceptors.triggers.tekton.dev
      operations:
      - List

Would be interesting if we could also add a label selector to each of the settings, what do you think?

apiVersion: capsule.clastix.io/v1beta1
kind: ProxySettings
metadata:
  name: sre-readers
  namespace: solar-management
spec:
  subjects:
  - name: sre
    kind: Group
    proxySettings:
    - kind: clusterinterceptors.triggers.tekton.dev
      selector:
        shared: "true"
      operations:
      - List

Namespace Scoped Resources

We have some crd, which are not cluster scoped, that end users want to be able to list, and where we don't necessary need to map proxySettings but just let them access the not namespaced api endpoint of that resource. They then get returned what they are allowed to see but for me it's not required to assign certain resources to users.

But that's just how I think about it. It comes from our devs which ask me:

  • I would like to execute kubectl get appprojects.argoproj.io -A and see all appprojects.argoproj.io across all namespaces they have access to.
  • I would like to execute kubectl get tasks.tekton.dev -A and see all tasks.tekton.dev across all namespaces they have access to.

Since we don't need to map cluster scoped resources, but just allow list operations for for namespaced resources, I think it makes sense to configure these on the capsule-proxy instance level, instead of having something like a ProxySettings crd in place.

Combining

We should think of a good combination of both features could be a huge win. Here's my very amateur attempt for a draft:

apiVersion: capsule.clastix.io/v1alpha1
kind: CapsuleProxyConfiguration
metadata:
  name: default
spec:

  # 1: For Namespace Scoped resources
  interceptors:
    - group: tekton.dev
       version: v1beta1
       kinds: [ "taskruns", "tasks" ]
    - group: argoproj.io
       version: v1beta1
       kinds: [ "appprojects"  ]

  # 2: For Cluster Scoped resources
  cluster_interceptors:
    - group: triggers.tekton.dev
       version: v1beta1
       kinds: [ "clustertriggerbindings", "clusterinterceptors" ]
  1. interceptors: For each of the interceptors we initialise the api endpoint in the capsule proxy. For those we would also add a MutatingWebhookConfiguration to label them with a namespace label, so they become selectable upon creation etc. And then I think we are already good for the namespaced resource. Because we then just select those based on namespace label. (I don't think we should bootstrap the rbac here, that should be left to tenant owners to decide)

  2. cluster_interceptors: For each of the cluster interceptors we initialise the api endpoint. Based on the proxySettings the role_binding controller provisions the rbac. This assumes there's a implementation of ProxySettings which would be considered.

I hope it makes some sense what I am trying to say.. :D

@oliverbaehler
Copy link
Collaborator Author

I would simply start by adding a Option for interceptAllNamespacedResources or some kind. Which then would intercept any namespaced resource within the capsule proxy.

@oliverbaehler
Copy link
Collaborator Author

I would change the scope of this feature to just by default intercept all namespaced resources. My best idea right now is:

  1. Add a mutating webhook in capsule which labels any resource in the tenant with the actual tenant (Will have to work out how to reconcile existing resources in a tenant).
  2. In capsule-proxy, intercept all calls for namespaced resources (list, get), gather all tenants of user making request, create a single query for which checks all resources for their tenant label if in users tenants.

Not sure if this works but I will give it at try :D

@carpenterm
Copy link
Contributor

@oliverbaehler @prometherion This sounds like it would solve some limitations we're hitting with Capsule/Capsule Proxy, mainly where we have a team that wants to develop a k8s operator. Right now what is happening is folks are developing using kubebuilder and everything of course works fine on kind, but then they come to a cluster with Capsule on it and they no longer have access to cluster-scoped get/list/watch/etc so are faced with re-writing a bunch of code to process namespaces individually. Ideally we want them to be able to have the same experience they can have on their own local cluster but limited to only their namespaces. It sounds like this work would help get us towards that? Let us know if you need any help/input :)

@oliverbaehler
Copy link
Collaborator Author

Hi @carpenterm, i have a working draft. But we need to make the code a bit more production ready. I havent had time lately but we are working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants