From ded42fd57e4df85730140734ca3681cceef66742 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sat, 29 Jun 2024 19:47:16 +0200 Subject: [PATCH] fix: improve CRD install (Helm and standalone) --- Makefile | 3 +- charts/accurate/README.md | 36 +++++++++---------- charts/accurate/templates/NOTES.txt | 3 ++ charts/accurate/templates/_helpers.tpl | 14 ++++++++ charts/accurate/templates/generated/crds.yaml | 4 ++- charts/accurate/values.yaml | 13 +++++++ config/crd-only/crd_conversion_patch.yaml | 13 +++++++ config/crd-only/kustomization.yaml | 7 ++++ .../crds/crd_resource_policy_patch.yaml | 6 ++++ .../overlays/crds/kustomization.yaml | 1 + 10 files changed, 80 insertions(+), 20 deletions(-) create mode 100644 config/crd-only/crd_conversion_patch.yaml create mode 100644 config/crd-only/kustomization.yaml create mode 100644 config/kustomize-to-helm/overlays/crds/crd_resource_policy_patch.yaml diff --git a/Makefile b/Makefile index d211841..7e970dc 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,8 @@ HELM_CRDS_FILE := charts/accurate/templates/generated/crds.yaml .PHONY: manifests manifests: setup ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. controller-gen $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="{./api/..., ./controllers/..., ./hooks/...}" output:crd:artifacts:config=config/crd/bases - echo '{{- if .Values.installCRDs }}' > $(HELM_CRDS_FILE) + echo '{{- include "accurate.crd-check" . }}' > $(HELM_CRDS_FILE) + echo '{{- if or .Values.crds.enabled .Values.installCRDs }}' >> $(HELM_CRDS_FILE) kustomize build config/kustomize-to-helm/overlays/crds | yq e "." -p yaml - >> $(HELM_CRDS_FILE) echo '{{- end }}' >> $(HELM_CRDS_FILE) kustomize build config/kustomize-to-helm/overlays/templates | yq e "." -p yaml - > charts/accurate/templates/generated/generated.yaml diff --git a/charts/accurate/README.md b/charts/accurate/README.md index 71b02f2..7f44f53 100644 --- a/charts/accurate/README.md +++ b/charts/accurate/README.md @@ -19,17 +19,25 @@ $ curl -fsL https://github.com/jetstack/cert-manager/releases/latest/download/ce ### Installing CustomResourceDefinitions (optional) -You must now decide if Accurate CRDs are to be managed by Helm or not. Please read -[CRD considerations](#crd-considerations) and make sure you understand the pros and cons with the different approaches. +Accurate does not use the [official helm method](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/) of installing CRD resources. +This is because it makes upgrading CRDs impossible with helm CLI alone. +The helm team explain the limitations of their approach [here](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations). + +The Accurate Helm chart default is to install and manage CRDs with Helm and +add annotations preventing Helm from uninstalling the CRD when the Helm release is uninstalled. -The Accurate Helm chart default is to install and manage CRDs with Helm, but if you want to manage them yourself, -now is the time. +The recommended approach is to let helm manage CRDs, but if you want to manage CRDs yourself, now is the time. ```console -$ kubectl apply -k https://github.com/cybozu-go/accurate//config/crd/ +$ kubectl apply -k https://github.com/cybozu-go/accurate//config/crd-only/ ``` -If you decided to manage CRDs outside of Helm, make sure you set the `installCRDs` Helm value to `false`. +> NOTE: +> +> Since the CRDs contain configuration of conversion webhooks, you may have to tweak the webhook settings +> if installing the chart using non-standard values. + +If you decided to manage CRDs outside of Helm, make sure you set the `crds.enabled` Helm value to `false`. ### Installing the Chart @@ -54,7 +62,7 @@ $ helm install --create-namespace --namespace accurate accurate -f values.yaml a ## Values | Key | Type | Default | Description | -| ---------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +|------------------------------------------|--------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | controller.additionalRBAC.rules | list | `[]` | Specify the RBAC rules to be added to the controller. ClusterRole and ClusterRoleBinding are created with the names `{{ release name }}-additional-resources`. The rules defined here will be used for the ClusterRole rules. | | controller.additionalRBAC.clusterRoles | list | `[]` | Specify additional ClusterRoles to be granted to the accurate controller. "admin" is recommended to allow the controller to manage common namespace-scoped resources. | | controller.config.annotationKeys | list | `[]` | Annotations to be propagated to sub-namespaces. It is also possible to specify a glob pattern that can be interpreted by Go's "path.Match" func. | @@ -67,7 +75,9 @@ $ helm install --create-namespace --namespace accurate accurate -f values.yaml a | image.pullPolicy | string | `nil` | Accurate image pullPolicy. | | image.repository | string | `"ghcr.io/cybozu-go/accurate"` | Accurate image repository to use. | | image.tag | string | `{{ .Chart.AppVersion }}` | Accurate image tag to use. | -| installCRDs | bool | `true` | Controls if CRDs are automatically installed and managed as part of your Helm release. | +| crds.enabled | bool | `true` | Decides if the CRDs should be installed as part of the Helm installation. | +| crds.keep | bool | `true` | Setting this to `true` will prevent Helm from uninstalling the CRD when the Helm release is uninstalled. | +| installCRDs | bool | `true` | Controls if CRDs are automatically installed and managed as part of your Helm release. Deprecated: Use `crds.enabled` and `crds.keep` instead. | ## Generate Manifests @@ -76,13 +86,3 @@ You can use the `helm template` command to render manifests. ```console $ helm template --namespace accurate accurate accurate/accurate ``` - -## CRD considerations - -Accurate does not use the [official helm method](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/) of installing CRD resources. -This is because it makes upgrading CRDs impossible with helm CLI alone. -The helm team explain the limitations of their approach [here](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations). - -Managing CRDs with Helm is probably the easiest, but also has some drawbacks. -The [cert-manager documentation](https://cert-manager.io/docs/installation/helm/#crd-considerations) -debates some pros and cons that are worth reading. diff --git a/charts/accurate/templates/NOTES.txt b/charts/accurate/templates/NOTES.txt index e69de29..9605b4f 100644 --- a/charts/accurate/templates/NOTES.txt +++ b/charts/accurate/templates/NOTES.txt @@ -0,0 +1,3 @@ +{{- if not .Values.installCRDs }} +⚠️ WARNING: `installCRDs` is deprecated, use `crds.enabled` instead. +{{- end }} diff --git a/charts/accurate/templates/_helpers.tpl b/charts/accurate/templates/_helpers.tpl index c097929..dc5fc58 100644 --- a/charts/accurate/templates/_helpers.tpl +++ b/charts/accurate/templates/_helpers.tpl @@ -37,3 +37,17 @@ app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} {{- end }} app.kubernetes.io/managed-by: {{ .Release.Service }} {{- end }} + +{{/* +Check that the user has not unset both .installCRDs and .crds.enabled or +set .installCRDs and disabled .crds.keep. +.installCRDs is deprecated and users should use .crds.enabled and .crds.keep instead. +*/}} +{{- define "accurate.crd-check" -}} + {{- if and ( not .Values.installCRDs) (not .Values.crds.enabled) }} + {{- fail "ERROR: the deprecated .installCRDs option cannot be disabled at the same time as its replacement .crds.enabled" }} + {{- end }} + {{- if and (.Values.installCRDs) (not .Values.crds.keep) }} + {{- fail "ERROR: .crds.keep is not compatible with .installCRDs, please use .crds.enabled and .crds.keep instead" }} + {{- end }} +{{- end -}} diff --git a/charts/accurate/templates/generated/crds.yaml b/charts/accurate/templates/generated/crds.yaml index 317cda7..0810994 100644 --- a/charts/accurate/templates/generated/crds.yaml +++ b/charts/accurate/templates/generated/crds.yaml @@ -1,10 +1,12 @@ -{{- if .Values.installCRDs }} +{{- include "accurate.crd-check" . }} +{{- if or .Values.crds.enabled .Values.installCRDs }} apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: cert-manager.io/inject-ca-from: '{{ .Release.Namespace }}/{{ template "accurate.fullname" . }}-serving-cert' controller-gen.kubebuilder.io/version: v0.15.0 + helm.sh/resource-policy: '{{ .Values.crds.keep | ternary "keep" "delete" }}' labels: app.kubernetes.io/managed-by: '{{ .Release.Service }}' app.kubernetes.io/name: '{{ include "accurate.name" . }}' diff --git a/charts/accurate/values.yaml b/charts/accurate/values.yaml index 1429a24..98bc93e 100644 --- a/charts/accurate/values.yaml +++ b/charts/accurate/values.yaml @@ -1,5 +1,18 @@ +# Deprecated: Use crds.enabled and crds.keep instead. installCRDs: true +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 Accurate custom resources + # like subnamespaces will be removed too by the garbage collector. + keep: true + image: # image.repository -- Accurate image repository to use. repository: ghcr.io/cybozu-go/accurate diff --git a/config/crd-only/crd_conversion_patch.yaml b/config/crd-only/crd_conversion_patch.yaml new file mode 100644 index 0000000..ed3c25f --- /dev/null +++ b/config/crd-only/crd_conversion_patch.yaml @@ -0,0 +1,13 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: accurate/accurate-serving-cert + name: subnamespaces.accurate.cybozu.com +spec: + conversion: + webhook: + clientConfig: + service: + namespace: accurate + name: accurate-webhook-service diff --git a/config/crd-only/kustomization.yaml b/config/crd-only/kustomization.yaml new file mode 100644 index 0000000..67736ee --- /dev/null +++ b/config/crd-only/kustomization.yaml @@ -0,0 +1,7 @@ +labels: + - pairs: + app.kubernetes.io/name: accurate +resources: +- ../crd +patches: + - path: crd_conversion_patch.yaml \ No newline at end of file diff --git a/config/kustomize-to-helm/overlays/crds/crd_resource_policy_patch.yaml b/config/kustomize-to-helm/overlays/crds/crd_resource_policy_patch.yaml new file mode 100644 index 0000000..6349bad --- /dev/null +++ b/config/kustomize-to-helm/overlays/crds/crd_resource_policy_patch.yaml @@ -0,0 +1,6 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + helm.sh/resource-policy: '{{ .Values.crds.keep | ternary "keep" "delete" }}' + name: subnamespaces.accurate.cybozu.com diff --git a/config/kustomize-to-helm/overlays/crds/kustomization.yaml b/config/kustomize-to-helm/overlays/crds/kustomization.yaml index d06365a..5bf958d 100644 --- a/config/kustomize-to-helm/overlays/crds/kustomization.yaml +++ b/config/kustomize-to-helm/overlays/crds/kustomization.yaml @@ -3,6 +3,7 @@ resources: patchesStrategicMerge: - crd_conversion_patch.yaml + - crd_resource_policy_patch.yaml components: - ../../components/common-labels