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

[epic] Add support for handling helm charts #962

Open
joelanford opened this issue Jun 21, 2024 · 2 comments
Open

[epic] Add support for handling helm charts #962

joelanford opened this issue Jun 21, 2024 · 2 comments
Labels
epic v1.x Issues related to OLMv1 features that come after 1.0

Comments

@joelanford
Copy link
Member

joelanford commented Jun 21, 2024

In OLMv1, we plan to support the registry+v1 bundle format in order to provide a means for migration from OLMv0. However, we recognize the many pain points and limitations of that format (to name a few: limited types of supported objects, lack of templating, too opinionated out how RBAC is generated)

In order to provide operator authors and cluster admins with more flexibility and control, this epic tracks the work necessary to support managing native helm charts in OLMv1.

Warning

A meta concern is that the below list of concerns has continued to grow as we have discovered more of Helm's behaviors. IMO, we need to do one of the following:

  1. Force cluster admins to acknowledge that they working with helm charts and helm behaviors, and then always do exactly what helm would do (is this even possible?).
  2. Don't use Helm as the backend engine that applies and lifecycles extension content.

Some items to consider:

  • Not all helm charts are conductive to being used in a declarative, always reconciling controller. Charts the use template functions that cause non-deterministic output will cause a reconciler to continuously detect and rollout changes.
  • Helm charts that use hooks may be problematic in a reconciler because they introduce phases that require waiting. If we need/want to support helm hooks, we may need to figure out how to make the helm install/upgrade/uninstall processes asynchronous. (see [epic] operator-controller rejects helm charts that use helm hooks #995)
  • Helm doesn't lifecycle CRDs in the ./crds directory when performing upgrades. It ignores them if the CRDs are already present on the cluster. If helm chart authors understand this behavior of helm and make assumptions about it, it is unsafe for OLMv1 to treat them differently. However one of the primary goals of OLMv1 is to lifecycle CRDs. This Helm behavior pits helm chart authors against ClusterExtension users who have opposing expectations of CRD lifecycling.
  • Helm supports a helm.sh/resource-policy: keep annotation, which gives chart authors the ability to control helm's behavior when uninstalling a chart. We specifically designed the ClusterExtension API to give cluster admins the ultimate control (see [epic] Support orphan deletion policy #775), not the extension author. So we currently use owner references to propogate deletions from the ClusterExtension, which means manifests that use this annotation would be deleted. How do we reconcile this ?
  • Helm supports chart dependencies, and the Helm CLI facilitates resolving, unpacking, and templating dependencies as part of a Helm release. There are multiple concerns with this:
    1. OLMv1 does not support dependencies. Helm's idea of a dependency could theoretically work in OLMv1 because the dependencies are resolved, included, and owned within the scope of a single ClusterExtension. However these nuances may be difficult to document and hard for users to understand.
    2. Helm dependencies might assume a certain distribution mechanism that is not supported in OLM. OLMv1 today supports only distribution via image registries, and is not configured to know anything about helm repositories. Would we try to build (what would likely be complex) solutions to these problems or would we simply reject Helm charts that specify dependencies? Perhaps there is an avenue to support helm charts whose dependencies are inlined.
  • Upstream Helm supports HTTPS and OCI based chart distribution. Downstream OLMv1 currently supports only OCI-based distribution. Additionally, OLM components have in the past relied on CRI-based pod unpacking, which relies on a container-runtime to pull, extract, and (critically) run images. Helm OCI Artifact charts would not be runnable (and therefore not extractable) if used with existing CRI-based unpacking techniques.
    • This concern could potentially be alleviated if/when the OCI VolumeSource KEP is implemented and graduates to stable.
@joelanford joelanford added epic v1.x Issues related to OLMv1 features that come after 1.0 labels Jun 21, 2024
@bentito
Copy link
Contributor

bentito commented Jun 27, 2024

Have you thought about which libraries might be good choices for helping with the validating of Helm charts to ensure we can work with them? We need to rule out chart with non-deterministic template functions and hooks, so something like:

customChecks := []*config.Check{
		{
			Name:        "detect-non-deterministic-functions",
			Description: "Detect non-deterministic template functions in Helm charts",
			Template:    "template-name",
			Params: map[string]interface{}{
				"functions": []string{"randAlphaNum", "randAlpha", "randNumeric", "randAlphaSpecial", "now"},
			},
		},
		{
			Name:        "detect-helm-hooks",
			Description: "Detect Helm hooks in Helm charts",
			Template:    "template-name",
			Params: map[string]interface{}{
				"hooks": []string{"pre-install", "post-install", "pre-delete", "post-delete", "pre-upgrade", "post-upgrade", "pre-rollback", "post-rollback"},
			},
		},

in Stackrox's kube-linter terms, though I can't quite make a working example yet. Maybe there are other libs that would work better?

@joelanford
Copy link
Member Author

Checks for non-deterministic functions would probably require use of the https://pkg.go.dev/text/template/parse package, since those functions disappear post-templating.

Checks for helm hooks should be straightforward because we can render a manifest, parse as normal kube objects, then search for the annotations. If we end up with a handful of checks like that, I could definitely see the value in using an off-the-shelf library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic v1.x Issues related to OLMv1 features that come after 1.0
Projects
Status: No status
Development

No branches or pull requests

2 participants