ContextQMD
Back to Cert Manager

Proposal: add "helm.sh/resource-policy: keep" CRD annotation and uniformise CRD options.

design/20240206.helm-resource-policy.md

1.20.26.5 KB
Original Source
<!-- This template is adapted from Kubernetes Enhancements KEP template https://raw.githubusercontent.com/kubernetes/enhancements/a86942e8ba802d0035ec7d4a9c992f03bca7dce9/keps/NNNN-kep-template/README.md -->

Proposal: add "helm.sh/resource-policy: keep" CRD annotation and uniformise CRD options.

<!-- toc --> <!-- /toc -->

Release Signoff Checklist

This checklist contains actions which must be completed before a PR implementing this design can be merged.

  • This design doc has been discussed and approved
  • Test plan has been agreed upon and the tests implemented
  • Feature gate status has been agreed upon (whether the new functionality will be placed behind a feature gate or not)
  • Graduation criteria is in place if required (if the new functionality is placed behind a feature gate, how will it graduate between stages)
  • User-facing documentation has been PR-ed against the release branch in [cert-manager/website]

Summary

Using Helm to install CRDs is difficult. We cannot use the Helm crds/ folder to install CRDs because then CRDs are not upgraded when the Helm chart is upgraded. For that reason, we use the templates/ folder to install CRDs. However, this means that the CRDs are removed when the Helm chart is uninstalled. This is not ideal because it means that all custom resources are removed too.

Motivation

<!-- This section is for explicitly listing the motivation, goals, and non-goals of the proposed enhancement. Describe why the change is important and the benefits to users. The motivation section can optionally provide links to demonstrate the interest in this functionality amongst the community. -->

Goals

<!-- List specific goals. What is this proposal trying to achieve? How will we know that this has succeeded? -->

There are two use cases we want to support:

  • install CRDs with Helm; safely and up-to-date
  • manage CRDs with a tool different from Helm

Right now, we have different Helm chart CRD options for the different cert-manager projects, we want a standardised solution across most of these projects:

  • cert-manager: "installCRDs"
  • trust-manager: "crds.enabled"
  • approver-policy, istio-csr, csi-driver(-spiffe): <none>

Non-Goals

<!-- What is out of scope for this proposal? Listing non-goals helps to focus discussion and make progress. -->

/

Proposal

<!-- This is where we get down to the specifics of what the proposal actually is. What is the desired outcome and how do we measure success? This should have enough detail that reviewers can understand exactly what you're proposing, but should not include things like API designs or implementation - those should go into "Design Details" below. -->

I would like to introduce the following options to all Helm charts that install CRDs (based on https://github.com/cert-manager/cert-manager/pull/5777):

yaml
crds:
  # This option decides if the CRDs should be installed
  # as part of the Helm installation.
  enabled: true


  # This option makes it so that the "helm.sh/resource-policy": keep
  # annotation is added to the CRD. This will prevent Helm from uninstalling
  # the CRD when the Helm release is uninstalled.
  # WARNING: when the CRDs are removed, all cert-manager custom resources
  # (Certificates, Issuers, ...) will be removed too by the garbage collector.
  keep: true

NOTE 1: For backwards compatibility, the crds.enabled option would be false for the cert-manager chart.

NOTE 2: For the cert-manager chart, instead of introducing two new options, we could use the existing installCRDs option and add a new keepCRDs option.

Design Details

<!-- This section should contain enough information that the specifics of your change are understandable. This may include API specs (though not always required) or even code snippets. If there's any ambiguity about HOW your proposal will be implemented, this is the place to discuss them. -->

Possible breaking change: This change will change the default uninstall behavior of the Helm chart. Before, the CRDs were removed when the Helm chart was uninstalled. Now, the CRDs will be kept by default. If the user wants to remove the CRDs too, they will have to manually delete them. This is a breaking change because it changes the default behavior of the Helm chart, but it will also make the lives of a lot of users much easier. I think the benefits outweigh the costs.

Info about the "helm.sh/resource-policy" annotation: Since we are using the templates/ folder to manage CRDs, which is required to allow templating and up-dating, the CRDs will be removed when we uninstall the chart. However, this annotation allows us to keep the resource even after the chart was uninstalled. We want to keep the CRDs to prevent accidental deletion of the custom resources.

The challenge with having only CRDs in a cluster, no webhooks: After uninstalling the Helm chart, we are left with only the CRDs. The ValidatingWebhookConfiguration and the MutatingWebhookConfiguration are removed too. This means that the CRs will be freely editable, potentially causing inconsistencies. Also, the cmctl check api command will still return successfully, because it can create CRs without any issues. A potential fix for the second problem would be to check that the webhook performs the required mutations/ validations.

Drawbacks

<!-- Why should this proposal _not_ be implemented? -->

This change will introduce new required steps in the following scenarios:

  • To fully uninstall the Helm chart, we now need to additionally run kubectl delete <crd1> <crd2> …
  • To re-install a Helm chart, if the new install has the same name and namespace, the CRDs are adopted automatically, otherwise, the CRDs have to be updated to match the name and namespace of the new release.

Alternatives

<!-- What other approaches did you consider, and why did you rule them out? These do not need to be as detailed as the proposal, but should include enough information to express the idea and why it was not acceptable. -->

Install CRDs separately (e.g., using kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.14.1/cert-manager.crds.yaml or using a separate Helm chart) and manage them separately from the Helm chart. This would require us to publish a separate Helm chart for the CRDs or a static manifest for the CRDs.