fix: improve CRD install (Helm and standalone)

cybozu-go · Jun 29, 2024 · ded42fd · ded42fd
1 parent e09d427
commit ded42fd
Show file tree

Hide file tree

Showing 10 changed files with 80 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -0,0 +1,7 @@
+labels:
+  - pairs:
+      app.kubernetes.io/name: accurate
+resources:
+- ../crd
+patches:
+  - path: crd_conversion_patch.yaml
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
@@ -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
@@ -3,6 +3,7 @@ resources:
 
 patchesStrategicMerge:
   - crd_conversion_patch.yaml
+  - crd_resource_policy_patch.yaml
 
 components:
   - ../../components/common-labels