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

[FR] Allow the Chart to create extra manifest #1230

Closed
pierluigilenoci opened this issue Mar 29, 2023 · 11 comments · May be fixed by #1288
Closed

[FR] Allow the Chart to create extra manifest #1230

pierluigilenoci opened this issue Mar 29, 2023 · 11 comments · May be fixed by #1288
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@pierluigilenoci
Copy link
Contributor

What would you like to be added:

It would be nice to be able to do this directly via the Chart using the approach that Grafana, Prometheus or OAuth2 Proxy also uses.

Why is this needed:

Sometimes, accessories to the installation may be manifests to be installed (for example, a SecretProviderClass or ConfigMap).

/kind feature

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 29, 2023
@logicalhan
Copy link
Contributor

/triage accepted
/assign @stevehipwell

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 6, 2023
@stevehipwell
Copy link
Contributor

@pierluigilenoci what exactly is your use case here? And why do you want/need Helm to manage arbitrary resources?

@pierluigilenoci
Copy link
Contributor Author

@stevehipwell more or less.
I was thinking of manifests that can be ancillary to the installation (like ConfigMaps, Secrets, SecretProviderClasses, or ServiceAccounts).
In my specific case, it is linked to configurations linked to Kyverno policies, which is irrelevant.
A manifest is a manifest regardless of the use in the specific case.

@stevehipwell
Copy link
Contributor

@pierluigilenoci I'm definitely of the opinion that we should be reducing the individual complexity of Helm charts and setting might tighter constraints on inputs. The general purpose resource input to a Helm chart really doesn't fit with this pattern and I'm still struggling to figure out the use case for this over raw YAML or even Kustomize? I don't think this is a pattern which we should be supporting in the Metrics Server Helm chart.

All that said, would a generic Helm chart able to render resources be of use to you if you can't use another solution?

@pierluigilenoci
Copy link
Contributor Author

@stevehipwell, let me explain the situation better.
In exceptional cases, there is a need to add extra manifests to those generated by a chart to adapt the functioning of the software.
In a GitOps (ArgoCD/Flux/any other tools) approach, a viable solution could be to use an umbrella chart with the primary chart as a dependency or add Kustomize on top.
Having the ability to add extra manifests (as some popular charts already do) allows you to reduce the complexity of this sort of thing.

@stevehipwell
Copy link
Contributor

@pierluigilenoci I don't think it's a good pattern to add generic Helm chart inputs, a Helm chart is a packages and as such should have constraints on what it can and will do, in fact I'd go as far as to categorise this as an anti-pattern. It sounds like your actual problem here is with the complexity and compromises needed to use "GitOps".

I won't be supporting this pattern in any Helm charts I maintain and will be advising the community to remove this pattern where possible; Karpenter has recently taken this step.

@pierluigilenoci
Copy link
Contributor Author

Of course, It is related to the complexity and tradeoffs of the GitOps approach.
But given how much it is increasingly present in our daily work, I was looking for a reasonable way to solve or reduce them.
However, I understand your thoughts on this, I share it in theory, but I try to be more pragmatic about it.

@stevehipwell
Copy link
Contributor

/close

@k8s-ci-robot
Copy link
Contributor

@stevehipwell: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mkilchhofer
Copy link

mkilchhofer commented Jul 17, 2023

Why closing this? Can you elaborate why this isn't a good pattern in your eyes?
There are so many tools around in the K8s ecosystem, a helm chart can never implement all variants/flavours, eg.:

  • NetworkPolicy (networking.k8s.io/v1) vs. NetworkPolicy (projectcalico.org/v3) vs. CiliumNetworkPolicy (cilium.io/v2)
  • Ingress (networking.k8s.io/v1) vs. HTTPRoute (Gateway API)
  • Secret handling (ExternalSecret, VaultSecret etc.)

Also I'd like to mention that this approach is adopted widely in the community:

  • all Argoproj charts (Argo CD, Argo workflows, ...)
  • flux2 community helm chart
  • grafana charts (Grafana, Promtail, Loki, etc.)
  • most Bitnami charts
  • VictoriaMetrics charts
  • oauth2-proxy
  • prometheus-community charts

just to mention some of them.

@stevehipwell
Copy link
Contributor

TL;DR - The Metrics Server Helm chart is optimised for simplicity without blocking extension for advanced users (open/closed architecture), if you have advanced use cases please feel free to extend on top of this Helm chart.

@mkilchhofer Helm is not a general purpose configuration management tool, it is a tool to package up complex applications in an easy to consume way. If an application needs resources creating to work then they can be supported in a strongly typed way, but just opening the chart up for arbitrary input makes no sense and offers only downsides.

For a focused community project such as Metrics Server there is no benefit to making our Helm chart everything to everyone, the tighter the constraints the smaller the TCO (it took a long time to get an official Helm chart added due to the potential for unsupportable engineering cost). We're following an open/closed architecture allowing anyone with advanced requirements to provide them themselves by extension and not put the cost onto the core project maintainers. This means that if you wish to use Helm for more than what this chart provides you can extend it by creating your own wrapper chart depending on this one.

FYI I the chart sources you listed above don't add anything to your argument and if anything are a strong case for tighter controls around chart scope and feature creep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants