Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Option to choose namespace destination for helm chart #100

Open
deuch opened this issue Feb 6, 2021 · 10 comments
Open

Option to choose namespace destination for helm chart #100

deuch opened this issue Feb 6, 2021 · 10 comments

Comments

@deuch
Copy link

deuch commented Feb 6, 2021

Hello,

We would like to use the template system of kiosk to let user install some packages without all the permissions to do it (like an operator)

For eg we need to install some helm chart in a specific namespace (created with the AKS cluster) with specific permissions (more restricted that the permissions in the space).
For example, our monitoring package need a service account with a ClusterRoleBinding+ClusterRole. This ClusterRole has more permissions than a normal user.
In the dedicated namespace named monitoring, user can not have access to secret or service account (namespace and permissions are not created by kiosk or the user and not managed by kiosk)

I’ve created a template for a helm chart with a value namespace: monitoring to force the installation in this namespace.
My issue is that if the user create the template-instance in, let say, project-dev namespace, the helm chart is installed in project-dev and not monitoring namespace.

Your helm template command render the files with « namespace: monitoring », but later in your code your replace the value of the namespace with an another one (the one use to create the template-instance) ...

Is it possible to have an option in the template to choose the namespace where the helm chart will be installed ?

Users can not modify template, so each time they want to install a template-instace, i can (for some of my charts with extended permission) set the target namespace per template if needed.

Let says : useTargetNamespace: true/false
If not set or set to true : same behaviour as before
If set to false : helm decision (so a forced namespace or the TargetNamespace)

Does it make sense ?

@LukasGentele
Copy link
Member

I don't think the spec of TemplateInstance should add any target namespace or so because that would be defying the purpose of using Templates to set up namespace isolation/restrictions. Users could freely break out of their namespace and Templates would become a security concern for many.

However, if a Template exposes a helm chart where a value can be changed to switch the namespaces of the templates resources, I think that would be more acceptable because it's more up to the admin to pick the chart and most isolation related things are probably not going to be implemented with charts and configurable values anyway. Still, it would be a security concern and very sensitive point of kiosk. I don't think we should turn kiosk into something like Tiller of v2 helm.

Btw there may he other Helm Operators out there that are more suitable for this use case though than kiosk.

@deuch
Copy link
Author

deuch commented Feb 7, 2021

If i understand correctly this is what i’m trying to do.
In the template, i set a helm chart to install and a value : namespace:monitoring.
So basically what you propose to do. But, after template rendering, Kiosk rewrite the namespace that i forced in the helm chart.

With your solution it doesn’t work and it’s worst for a security point of view.

My template is set to force the installation in the namespace monitoring.
A user create a template-instance of this template in it’s own namespace. The helm chart is not installed in the monitoring namespace, and not monitoring namespace. At the end, the service account and token that i do now want the user have is created in it’s own namespace ...
And this is because kiosk rewrite the namespace in the file after template rendering.
I can not see why the features i ask as a security issue. In the contraty, i enforce some of the template to not run in application namespace, but dedicated namespace with special permissions.

Template can not be edited or modify by the project, only admin.

@LukasGentele
Copy link
Member

The point that you are making about this being unintended by the admin because the namespace is explicitly set to a different one other than the user's current namespace is a very valid point. And I agree that uncertainty can be a security concern, so it may be better not to override the namespace in kiosk.

@FabianKramm
Copy link
Member

@deuch while I see this could be helpful in some cases, I think there are some points that seem problematic to me:

  1. How do you handle naming conflicts? If 2 two users install the same chart into another namespace, naming conflicts occur, except you somehow mingle the {{ .Release.Namespace }} into the kube name, which can be problematic for longer names.
  2. One of the primary features of template instances is that as soon as you delete a template instance, objects are automatically cleaned up that were deployed by it. This is done through ownerReferences in kubernetes objects, however you cannot set an owner reference to an object which lies in another namespace, which would mean these objects would not cleaned up, unless we implement our own custom cleanup mechanism.

I think in your case an helm operator (like https://github.com/fluxcd/helm-operator) might be a better solution: you would install the operator and then create a kiosk template with the following manifest:

apiVersion: helm.fluxcd.io/v1
kind: HelmRelease
metadata:
  name: my-chart
spec:
  chart:
    repository: https://my-repo.com
    name: my-chart
    version: 3.2.0

This would be deployed by kiosk automatically during space creation and you could essentially install that chart into a different namespace.

@deuch
Copy link
Author

deuch commented Feb 8, 2021

Helm-operator seems to be a dead project, at least for the moment.
Installing helm-operator will not solve my problem if the user can create an helmrelease with the same chart in it’s own namespace ... he will have access to some secret and service account with cluster role installed by the helm operator in a forbidden namespace for this chart.

Maybe an another approach is to add a field in the Template with a list of authorized namespace to install it ? It will act like a ValidatingWebhook for any templatinstace of this template that will be deployed ?

On application onboarding i can create this as namespace :
Project-dev (space)
Project-uat (space)
Project-monitoring (namespace with restricted permission)

You can install the template many time you want (one per project of course, the helm chart will ne be installed twice in project-monitoring) and i ensure that this product (and more like argocd which create a service Account with basically cluster-admin priviledge ...) will be installed in a namespace set end choose by the admin.

It will not modify the actual behaviour, just deny the template-instance creation before rendering any yaml file from the configured chart in the template. And the helm chart installation is not compromised or altered.

What do you think about it ?

@deuch
Copy link
Author

deuch commented Feb 16, 2021

Hi, no thoughts ?

@FabianKramm
Copy link
Member

@deuch sorry, seems I missed your response. I'm not sure about the authorized namespaces field, since this would only work for helm chart templates were all resource names in the helm chart are based on the release name (and not for any non helm templates), because otherwise you would try to create a resource in a namespace that would already exist.

So I guess in your case it is probably best to indirectly deploy that helm chart into another namespace with the template instance, by either using some sort of helm chart operator (I think there are multiple out there not only the one I suggested) or a validating webhook with side effects that installs this chart as part of the template instance creation, a custom kubernetes controller that watches specific template instances or a simple kubernetes job that just runs helm install with appropriate rights the space owner doesn't have to install that helm release.

However, I also think this should be possible with kiosk, but I think there is no simple fix here, rahter we should probably introduce a completely new CRD (which we also already planned) that is able to parameterize existing templates (which is currently not possible, but would solve the naming issue I mentioned earlier) as well as install those into other namespaces, which would solve your problem as well.

@deuch
Copy link
Author

deuch commented Feb 18, 2021

For the naming issue, it’s already the case with or without forcing the namespace where to install :)

Do you have a ETA for those new CRDs ?

In the meantime i just need an admission controller with a ValidatingWebbook with a list of template-name:destination-namespace. If a template instance for a specific template is created, i check the destination namespace (if enforced) and if it does not match it’s destination namespace it will failed.
This list can be in a configmap an read for each call (this wehbook will be call less often that the one which check pods for eg ...)

Basically it’s what i proposed with a list of namespace for a kiosk helm template :)

But i think that kiosk must not handle how the chart is done. If it’s a bad one, let the admin make it good.

@FabianKramm
Copy link
Member

@deuch currently there is no official ETA for those CRDs, its still in the planning phase and there are some other things that are important as well as for example high availability of kiosk, so I'm not exactly sure when this will land in kiosk.

Okay great, sounds like your workaround is working already, I'll update this issue as soon as there is a release plan for the new CRD.

@deuch
Copy link
Author

deuch commented Mar 25, 2021

Hello,

I've seen that now we can add parameters to the template, nice feature 👍
We can also check parameters :)

But for my issue, can we also do some validation/check of the ${NAMESPACE} or ${ACCOUNT} value in the Template ? In this way i can restrict the template instance to be created only in the namespace set in the template !

Thank you !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants