Helm support: long term plan

natasha41575 commented 2 years ago

There are many open helm-related issues in kustomize: https://github.com/kubernetes-sigs/kustomize/issues?q=is%3Aissue+is%3Aopen++label%3Aarea%2Fhelm+

Many of these issues are requesting new options in the helm chart generator configuration that will become flags to helm template. Other issues are related to problems with the --enable-helm flag and wanting more flexibility in how to enable helm.

For some context, there are limited options in the plugin configuration and the --enable-helm flag exists because of a past security issue that has since been addressed. However, it is clear that the helm chart generator built into kustomize has limitations and we need a plan for long term support.

What we would like to do is introduce a new function in the new krm-functions-registry that can inflate a helm chart. This new function will be treated as a third party extension by kustomize so it will not need any special treatment. Additionally, because it will be a containerized function, there will be fewer security problems to worry about.

Once this function is ready, we can start recommending it to users and deprecate the existing helm-related fields in kustomize.

natasha41575 commented 2 years ago

/triage accepted /kind deprecation

natasha41575 commented 2 years ago

/label area/helm

k8s-ci-robot commented 2 years ago

@natasha41575: The label(s) /label area/helm cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor

In response to [this](https://github.com/kubernetes-sigs/kustomize/issues/4401#issuecomment-1016889963): >/label area/helm Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

james-callahan commented 2 years ago

FWIW we don't consider krm functions usable in most of the environments we use kustomize.

On the other hand, I think the deprecation would help if the plugin is not going to get further support in kustomize itself. It might be worth directing people to https://github.com/mgoltzsche/khelm

natasha41575 commented 2 years ago

Would you mind providing some more feedback for why krm functions are not usable for you? We would like to address friction points as we move toward more krm-function-oriented solutions and extensions.

james-callahan commented 2 years ago

Would you mind providing some more feedback for why krm functions are not usable for you?

We require kustomize to be run on various end user devices that won't have docker installed (and/or aren't supported by docker)
We run kustomize from within CI pipelines that are unable to spawn a container
We run kustomize from within containers such as ArgoCD where they will not (and should not) have permissions to spawn a container

khrisrichardson commented 2 years ago

It's also worth mentioning that flux2's kustomize-controller doesn't natively support KRM config functions by way of plugin enablement, nor does it include the helm binary in the container image, and thus the current options are to fork or pre-hydrate (which still sounds like it would be a challenge for @james-callahan until other runtimes are available for KRM config functions)

snuggie12 commented 2 years ago

In #3536 I did get a special built version of podman working after renaming it to docker though would have preferred simply being able to name my runtime.

Our need matches what was previously stated since we use argocd; we did not want to install docker onto their repo-server. It would be a lot of work if the helm plugin was moved to a krm function and we'd likely stay on an older version or fork for quite some time.

Side note, in some prototyping we got around the whole --enable-helm by altering viper to accept it as an env var. Older versions of kustomize would ignore the env var and newer ones would process it correctly. I'm all in favor of getting rid of the flag if it is not needed but if that helps as a solution for any other flags I figured I'd mention it.

KlavsKlavsen commented 2 years ago

We are also looking to use this in argocd (as soon as kostumize gains OCI support - which was just released as stable with Helm 3.8.0) - so it would be bad, to limit it like this :(

natasha41575 commented 2 years ago

We are looking at ways to support alternatives to docker, including containerized alternatives as well as simple and safe ways to run exec KRM functions. The latter would run very similarly to the existing builtin plugin.

natasha41575 commented 2 years ago

Since I have been receiving several messages about this, I would like to reiterate again that we will be supporting the new KRM Helm function as executable, meaning that you would not need to run docker in order to use it. In terms of execution, this will be almost exactly the same as running the current builtin Helm plugin, except that the code lives elsewhere and is compiled separately.

QuinnBast commented 2 years ago

I noticed that the helm chart inflation has been moved to the kubernetes-sigs repository and now exists in both repositories. Which is the officially supported version, and how would I use kubernetes-sigs with kustomize?

natasha41575 commented 2 years ago

I noticed that the helm chart inflation has been moved to the kubernetes-sigs repository and now exists in both repositories. Which is the officially supported version, and how would I use kubernetes-sigs with kustomize?

@QuinnBast Could you elaborate by what you mean by kubernetes-sigs repository? kubernetes-sigs is the org, and has many repositories.

Edited to add: If you are referring to the code in the krm functions registry, it is not ready yet. We are still working on setting up the repo infrastructure.

mwmahlberg commented 2 years ago

I have noticed that the lack of the possibility to pass extra-args to helm makes it impossible to properly use helm charts that check capabilities, e.g. cert-manager's in-built Prometheus ServiceMonitor creation, see https://github.com/cert-manager/cert-manager/blob/9887baac3357beb1ad949f9b62f25dc3eea27fb7/deploy/charts/cert-manager/templates/servicemonitor.yaml#L1

My thoughts on that:

It makes sense for the upstream projects to check whether a resource is accepted by the target cluster, so the capability check is vital, especially when helm's --atomic flag is used (when helm is used directly, that is)
Speaking of which: One cannot pass the --atomic flag, either.
Imho, it does NOT make sense to implement the functionality and/or logic of helm's respective flags in kustomize -- that would couple the implementations tightly.
The easiest solution would have to (re-)introduce extraArgs to the helmCharts members and pass them verbatim to the function call

james-callahan commented 2 years ago

I have noticed that the lack of the possibility to pass extra-args to helm makes it impossible to properly use helm charts that check capabilities

See https://github.com/kubernetes-sigs/kustomize/issues/3816

mwmahlberg commented 2 years ago

@james-callahan You sure are persistent in your efforts. +1

Some thoughts

I guess this shows a wider problem: the divergence of the various means to deploy to k8s.
One could argue that kubectl and by extension kustomize should be considered the canonical way and as such other tools need to adapt accordingly.
I think, but I am not sure about it, that we have an edge case here: overriding the capabilities as determined by Helm is only necessary if the logic in the template in question does come to a fail safe conclusion.

KnVerey commented 2 years ago

Please note that in light of slow progress on the KRM Function version, we've updated our mid-term plan around the current built-in. The new policy is outlined in our docs and copied below for ease of reference.

Re: extra-args in particular: We used to have such a field, but we removed it in https://github.com/kubernetes-sigs/kustomize/pull/3784 because it represented a security risk. As such we will not be considering adding it again.

Long term support

The helm chart inflation generator in kustomize is intended to be a limited subset of helm features to help with getting started with kustomize, and we cannot support the entire helm feature set.

The current builtin

For enhancements to the helm chart inflation generator feature, we will only support the following changes:

bug fixes
critical security issues
additional fields that are analogous to flags passed to helm template, except for flags such as post-renderer that allow arbitrary commands to be executed

We will not add support for:

private repository or registry authentication
OCI registries
other large features that increase the complexity of the feature and/or have significant security implications

Future support

The next iteration of the helm inflation generator will take the form of a KRM function, which will have no such restrictions on what types of features we can add and support. You can see more details in the Helm support long term plan.

jmmclean commented 2 years ago

Its kind of a poor Developer Experience that the KRM repo is referenced for helm inflation, but no contributions have put on it for about 6 months - this is typically an indicator of a dying project in the open source world. Im specifically thinking of https://github.com/kubernetes-sigs/kustomize/issues/4335 where contributions were made by the community, but disregarded for the below option (clearly not an active project)

https://github.com/kubernetes-sigs/krm-functions-registry

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

KnVerey commented 1 year ago

/lifecycle frozen

boldandbusted commented 1 year ago

I have noticed that the lack of the possibility to pass extra-args to helm makes it impossible to properly use helm charts that check capabilities, e.g. cert-manager's in-built Prometheus ServiceMonitor creation, see https://github.com/cert-manager/cert-manager/blob/9887baac3357beb1ad949f9b62f25dc3eea27fb7/deploy/charts/cert-manager/templates/servicemonitor.yaml#L1

My thoughts on that:

It makes sense for the upstream projects to check whether a resource is accepted by the target cluster, so the capability check is vital, especially when helm's --atomic flag is used (when helm is used directly, that is)

Speaking of which: One cannot pass the --atomic flag, either.

Imho, it does NOT make sense to implement the functionality and/or logic of helm's respective flags in kustomize -- that would couple the implementations tightly.

The easiest solution would have to (re-)introduce extraArgs to the helmCharts members and pass them verbatim to the function call

As a downstream user, I'd like to be able to make use of Helm's --atomic flag. I searched for an Issue that covers adding that flag, but came up empty. With https://github.com/kubernetes-sigs/kustomize/issues/4401#issuecomment-1213360696, and no extra-args: key, there's now apparently no path towards adding support for Helm features like --atomic. Thanks in advance for any background info or links to deeper discussions! :)

Jorricks commented 1 year ago

Is this still an active plan? I am seeing no activity in the mentioned repository, nor am I seeing any significant contributions in this repository to refactor the HELM setup. IMO the HELM functionality works well, of course it has bugs, but all software does. Furthermore I think it's good to keep active contributions in this area until the entire refactor is complete.

Furthermore, I would like to extend upon the ideas of @mwmahlberg here.

I've come across another implementation that will allow you to specify the args in the kustomize command, which would be the preferred way for me. The command would become something like; kustomize build --enable-alpha-plugins --enable-helm --load-restrictor=LoadRestrictionsNone --helm-templated-args '--api-versions my-api-version' /path/to/deployment

But I see the main reason for me to upgrade has been merged recently(I was looking for --api-versions support); https://github.com/kubernetes-sigs/kustomize/pull/4926

james-callahan commented 1 year ago

As a downstream user, I'd like to be able to make use of Helm's --atomic flag

The --atomic flag doesn't make any sense in the context of kustomize. Infact the usual way to apply kustomize is via kustomize build | kubectl apply -f - ==> the kustomize binary has no idea where it's output is going; whether it worked; or how to role it back.

boldandbusted commented 1 year ago

As a downstream user, I'd like to be able to make use of Helm's --atomic flag

The --atomic flag doesn't make any sense in the context of kustomize. Infact the usual way to apply kustomize is via kustomize build | kubectl apply -f - ==> the kustomize binary has no idea where it's output is going; whether it worked; or how to role it back.

Yep, I was an ignorant first-time Kustomize user when I posted that... and realize that using Kustomize as a way to apply Helm charts kind of removes a lot of the nice features of Helm as a K8s-equivalent of a OS package-manager.

hawkesn commented 1 year ago

There is some movement on this PR: https://github.com/kubernetes-sigs/kustomize/pull/5167 Very excited to see support for OCI!

lindhe commented 1 year ago

Is the plan to keep requiring the --enable-helm flag once this feature is complete, or can we get rid of it then?

vladfr commented 1 year ago

@KnVerey, @natasha41575 I've reached this page from the docs. It doesn't seem that this Long-term plan for Helm is actually happening, as it's been a year since anyone has contributed to the KRM repo. It does seem like problems with the --enable-helm option are being addressed.

Is it time to revisit the ideas outlined in this issue? Namely, there are PRs already done with support for private repos, OCI registry, and a few bugs. It would be great to reopen this.

Use-case: Cluster setup with ArgoCD

Helm in an Argo app can be a bit cumbersome to do with GitOps. There is a Chicken & Egg problem, where you need an App of Apps to initialize an Argo repo from Git, but Helm charts don't fit well with that. Argo UI also puts some limits in how it displays Helm charts too, with some nice advantages such as viewing values and overrides. But for the cluster admin use-case, it can get a bit cumbersome.

There is a case to be made that kustomize should be used instead. If we can nail support for Helm inflate within kustomize it would enable a nice use-case: Apply a chart and kustomizations in the same Argo app. This could be useful for cluster admins who want to consume org charts, or even public charts, and need to add or modify resources.

Examples:

install cert-manager and configure some ClusterIssuers
add Ingress resources for a Grafana chart

While this can be achieved in other ways, having kustomize apply these will enable a cleaner directory structure in the git repo, and enable the App-of-Apps pattern, which is a de-facto standard with ArgoCD and GitOps. (While I haven't fully tried it, the same can be said for Flux, although with Flux, the problem is smaller in scope because it doesn't extend to the UI)

KlavsKlavsen commented 1 year ago

For our open source project ( https://github.com/Obmondo/k8id ) - we only use helm charts and the 2.6+ feature with having values files in diff. repo - and in the very few instances where we need more, we add templates to the "wrapper helm chart" - and we also have a "static yaml" application which files are in the same "sepeate config repo" - or we extend helm chart and do upstream PR.. But helm + kustomize would give an extra option, which is definely not bad, but AFAIK kostumize's biggest strength is more the ability to handle patching of resources.. and a helm chart would "bark" at that afterwards? (unless argocd does helm template and runs kostumize locally - so it knows the end state - and there will then not be a diff.. I assume thats hot it would work). I still prefer upstream patch - but its definetely a good flexibility to have, to patch "resut of helm charts" - instead of the helm chart itself if necessary.

So if you use Helm umbrella model (you have a parent helm chart of your own - that simply depends on upstream chart) - then you have an easy way to extend helm charts (not quite as flexible as kustomize) that works too. You can see in k8id project - under argocd-helm-charts folder for examples :)

GreenCappuccino commented 1 year ago

There is a case to be made that kustomize should be used instead. If we can nail support for Helm inflate within kustomize it would enable a nice use-case: Apply a chart and kustomizations in the same Argo app. This could be useful for cluster admins who want to consume org charts, or even public charts, and need to add or modify resources.

I actually do this, but instead of in the GitOps application, it's done at the pre-commit step. By computing the combined hash of all files in each directory to find which releases have changed, I can automatically generate the relevant Kustomize output, with inflated charts in a build.yaml file, onto which Kustomization patches/overlays can be applied onto. build.yaml is committed into version control.

It's not a very comprehensive solution, since it's in a single pre-commit python script, but I think that such a model has the potential to be compatible with both Argo and Flux, and also enable static-analysis based CI tooling, like for example to upgrade helm charts like Dependabot.

Here is how I manage my lab cluster (I'm planning to refactor the folder structure to support kustomize localize, but I haven't had the time): https://github.com/ShamrockTeam/kubernetes

koba1t commented 6 months ago

Hi all. I'm currently the subproject lead of kustomize. I feel we need to improve helm support in this project because I think many users use this function in view of some issues and PRs.

In the first step, I plan to import helm lib in kustomize because it currently functions by directly running the helm binary on the user's computer. Other functions and improvements are added after that are completed.

Current status: I'm waiting for this PR to be merged. https://github.com/helm/helm/pull/12725

Skaronator commented 2 weeks ago

Just a heads-up: the Helm PR has been closed, and it looks like we might not see a solution to that issue in the near future. It might be best to explore solutions within Kustomize instead of relying on Helm.

I also gave it a shot myself with a Helm feature implementation. Even though I’m not a Golang expert, I didn’t get any feedback from the dev team. I even put together a Helm improvement proposal here but didn’t get a response. So, I wouldn’t expect any updates from the Helm side anytime soon.

GreenCappuccino commented 2 weeks ago

Note that Kustomize supports KRM fn generators like the kpt helm chart rendering generator: https://catalog.kpt.dev/render-helm-chart/v0.1/ So you can use any arbitrary generator as long as it's containerized.

Skaronator commented 1 week ago

While KRM does exist, I think it's a flawed approach. Unlike Helm plugins or kubectl plugins, KRMs depend on a container runtime, which is usually unavailable in CI pipelines or CD tools such as ArgoCD.

I understand that supporting Helm involves a lot of overhead, and I can see why Kustomize might not want to handle Helm-related issues. Personally, I think it would be ideal if Kustomize adopted a plugin approach similar to kubectl or Helm and outsourced all Helm-related functionality.

QuinnBast commented 1 week ago

It's pretty sad to see that this has been stalled for over 2 years, and that we STILL can't login to private helm registries despite that being a very basic and necessary functionality to use helm at all...

Additionally, the docs for Kustomize's helm chart inflator have been completely pulled and link to this github issue and the docs no longer explain the basic functionality that Kustomize currently supports through the helm inflation generator.

Edit: After browsing, I found the actual docs for the helm inflation generator but they are pretty buried, and the first result on google links to here which seems to indicate a lack of support for helm altogether...

kubernetes-sigs / kustomize