Allow the operator chart to deploy CRDs separately

gravitational · Mar 27, 2024 · d908458 · d908458
1 parent 5e338ee
commit d908458
Show file tree

Hide file tree

Showing 20 changed files with 161 additions and 13 deletions.
diff --git a/docs/pages/reference/helm-reference/includes/zz_generated.teleport-operator.mdx b/docs/pages/reference/helm-reference/includes/zz_generated.teleport-operator.mdx
@@ -10,6 +10,23 @@
 - When `true`, the chart creates both the `CustomResourceDefinition` and operator `Deployment` Kubernetes resources.
 - When `false`, the chart creates the `CustomResourceDefinition` resources without the operator `Deployment`.
 
+## `installCRDs`
+
+| Type | Default |
+|------|---------|
+| `string` | `"dynamic"` |
+
+`installCRDs` controls if the chart should install the CRDs.
+There are 3 possible values: dynamic, always, never.
+
+- "dynamic" means the CRDs are installed if the operator is enabled or if
+  the CRDs are already present in the cluster. The presence check is here to
+  avoid all CRDs to be removed if you temporarily disable the operator.
+  Removing CRDs triggers a cascading deletion, which removes CRs, and all the
+  related resources in Teleport.
+- "always" means the CRDs are always installed
+- "never" means the CRDs are never installed
+
 ## `teleportAddress`
 
 | Type | Default |

diff --git a/examples/chart/teleport-cluster/Chart.yaml b/examples/chart/teleport-cluster/Chart.yaml
@@ -12,5 +12,4 @@ keywords:
 dependencies:
   - name: teleport-operator
     version: *version
-    condition: installCRDs,operator.enabled
     alias: operator
diff --git a/...s/resources.teleport.dev_accesslists.yaml → ...s/resources.teleport.dev_accesslists.yaml b/...s/resources.teleport.dev_accesslists.yaml → ...s/resources.teleport.dev_accesslists.yaml
diff --git a/...ources.teleport.dev_githubconnectors.yaml → ...ources.teleport.dev_githubconnectors.yaml b/...ources.teleport.dev_githubconnectors.yaml → ...ources.teleport.dev_githubconnectors.yaml
diff --git a/...es/resources.teleport.dev_loginrules.yaml → ...ds/resources.teleport.dev_loginrules.yaml b/...es/resources.teleport.dev_loginrules.yaml → ...ds/resources.teleport.dev_loginrules.yaml
diff --git a/...esources.teleport.dev_oidcconnectors.yaml → ...esources.teleport.dev_oidcconnectors.yaml b/...esources.teleport.dev_oidcconnectors.yaml → ...esources.teleport.dev_oidcconnectors.yaml
diff --git a/...sources.teleport.dev_oktaimportrules.yaml → ...sources.teleport.dev_oktaimportrules.yaml b/...sources.teleport.dev_oktaimportrules.yaml → ...sources.teleport.dev_oktaimportrules.yaml
diff --git a/...es.teleport.dev_openssheiceserversv2.yaml → ...es.teleport.dev_openssheiceserversv2.yaml b/...es.teleport.dev_openssheiceserversv2.yaml → ...es.teleport.dev_openssheiceserversv2.yaml
diff --git a/...ources.teleport.dev_opensshserversv2.yaml → ...ources.teleport.dev_opensshserversv2.yaml b/...ources.teleport.dev_opensshserversv2.yaml → ...ources.teleport.dev_opensshserversv2.yaml
diff --git a/...sources.teleport.dev_provisiontokens.yaml → ...sources.teleport.dev_provisiontokens.yaml b/...sources.teleport.dev_provisiontokens.yaml → ...sources.teleport.dev_provisiontokens.yaml
diff --git a/...mplates/resources.teleport.dev_roles.yaml → ...or-crds/resources.teleport.dev_roles.yaml b/...mplates/resources.teleport.dev_roles.yaml → ...or-crds/resources.teleport.dev_roles.yaml
@@ -303,7 +303,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:
@@ -829,7 +829,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:
@@ -1635,7 +1635,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:
@@ -2161,7 +2161,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:

diff --git a/...lates/resources.teleport.dev_rolesv6.yaml → ...-crds/resources.teleport.dev_rolesv6.yaml b/...lates/resources.teleport.dev_rolesv6.yaml → ...-crds/resources.teleport.dev_rolesv6.yaml
@@ -306,7 +306,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:
@@ -832,7 +832,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:

diff --git a/...lates/resources.teleport.dev_rolesv7.yaml → ...-crds/resources.teleport.dev_rolesv7.yaml b/...lates/resources.teleport.dev_rolesv7.yaml → ...-crds/resources.teleport.dev_rolesv7.yaml
@@ -306,7 +306,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:
@@ -832,7 +832,7 @@ spec:
                           a mechanism to propagate extra information to plugins.  Since
                           these annotations support variable interpolation syntax,
                           they also offer a mechanism for forwarding claims from an
-                          external identity provider, to a plugin via {{ `{{external.trait_name}}` }}
+                          external identity provider, to a plugin via `{{external.trait_name}}`
                           style substitutions.
                         type: object
                       claims_to_roles:

diff --git a/...esources.teleport.dev_samlconnectors.yaml → ...esources.teleport.dev_samlconnectors.yaml b/...esources.teleport.dev_samlconnectors.yaml → ...esources.teleport.dev_samlconnectors.yaml
diff --git a/...mplates/resources.teleport.dev_users.yaml → ...or-crds/resources.teleport.dev_users.yaml b/...mplates/resources.teleport.dev_users.yaml → ...or-crds/resources.teleport.dev_users.yaml
diff --git a/examples/chart/teleport-cluster/charts/teleport-operator/templates/_helpers.tpl b/examples/chart/teleport-cluster/charts/teleport-operator/templates/_helpers.tpl
@@ -57,6 +57,7 @@ helm.sh/chart: '{{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}'
 app.kubernetes.io/managed-by: '{{ .Release.Service }}'
 app.kubernetes.io/version: '{{ include "teleport-cluster.version" . }}'
 teleport.dev/majorVersion: '{{ include "teleport-cluster.majorVersion" . }}'
+teleport.dev/release: '{{ include "teleport-cluster.operator.namespacedRelease" . }}'
 {{- end -}}
 
 {{/* Teleport auth or proxy address */}}
@@ -75,3 +76,56 @@ teleport.dev/majorVersion: '{{ include "teleport-cluster.majorVersion" . }}'
 {{- /* This template is a placeholder.
 If we are imported by the main chart "teleport-cluster" it is overridden*/ -}}
 {{- define "teleport-cluster.auth.serviceFQDN" -}}{{- end }}
+
+{{- /* This templates returns "true" or "false"  describing if the CRDs should be deployed.
+If we have an explicit requirement ("always" or "never") things are easy.
+If we don't we check if the operator is enabled.
+However, we cannot just trash the CRDs if the operator is disabled, this causes
+a mass CR deletion and users will shoot themselves in the foot whith this
+(temporarily disabling the operator would cause havoc).
+So we check if there's a CRD already deployed, it that's the case, we keep the CRDs.
+*/ -}}
+{{- define "teleport-cluster.operator.shouldInstallCRDs" -}}
+  {{- if eq .Values.installCRDs "always" -}}
+    true
+  {{- else if eq .Values.installCRDs "never" -}}
+    false
+  {{- else if eq .Values.installCRDs "dynamic" -}}
+    {{- if .Values.enabled -}}
+      true
+    {{- else -}}
+      {{- include "teleport-cluster.operator.checkExistingCRDs" . -}}
+    {{- end -}}
+  {{- else -}}
+    {{- fail ".Values.installCRDs must be 'never', 'always' or 'dynamic'." -}}
+  {{- end -}}
+{{- end -}}
+
+{{- /* This template checks if a known CRD is depployed (rolev7) and owned by
+the release. As CRDs are not namespaced, we must use a custom annotation to avoid
+a conflict when two releases are deployed with the same name in different namespaces. */ -}}
+{{- define "teleport-cluster.operator.checkExistingCRDs" -}}
+  {{ $existingCRD := lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "teleportrolesv7.resources.teleport.dev"}}
+  {{- if not $existingCRD -}}
+    false
+  {{- else -}}
+    {{- $release := index $existingCRD.metadata.labels "teleport.dev/release" }}
+    {{- if eq $release (include "teleport-cluster.operator.namespacedRelease" .) -}}
+      true
+    {{- else -}}
+    false
+    {{- end -}}
+  {{- end -}}
+{{- end -}}
+
+{{- /* This is a custom label containing the namespaced release.
+This is used to avoid conflicts for non-namespaced resources like CRDs. */ -}}
+{{- define "teleport-cluster.operator.namespacedRelease" -}}
+  {{ .Release.Namespace }}_{{ .Release.Name }}
+{{- end -}}
+
+{{- /* This is the object merged with CRDs manifests to enrich them (add labels). */ -}}
+{{- define "teleport-cluster.operator.crdOverrides" -}}
+metadata:
+  labels: {{- include "teleport-cluster.operator.labels" . | nindent 4 }}
+{{- end -}}
diff --git a/examples/chart/teleport-cluster/charts/teleport-operator/templates/crds.yaml b/examples/chart/teleport-cluster/charts/teleport-operator/templates/crds.yaml
@@ -0,0 +1,24 @@
+{{- /* This template iterates over every CRD in the `operator-crds/` directory
+and creates them if needed. It also adds common labels, like any other
+Helm-deployed resource.
+
+We cannot rely on the "crds/" Helm directory as Helm's startegy is "fire and forget".
+We have no way to update the CRDs after the initial deployment. As Teleport keeps
+adding new field to existing CRs, we need a deployment strategy that supports
+updating CRDs.
+
+The obvious solution would be to have a separate chart for CRs but we wanted to
+have everything functional in a single "helm install", hence the rube goldberg
+mechanism to try to guess what to do with the CRDs (see the implementation of
+shouldInstallCRDs in _helpers.yaml for more details). */ -}}
+{{- if eq (include "teleport-cluster.operator.shouldInstallCRDs" . ) "true" -}}
+{{ $currentScope := .}}
+{{ range $path, $_ :=  .Files.Glob  "operator-crds/*" }}
+  {{- with $currentScope}}
+    {{- $crd := (.Files.Get $path | fromYaml) -}}
+    {{- $injectedCRD := mustMergeOverwrite $crd (include "teleport-cluster.operator.crdOverrides" $currentScope | fromYaml) -}}
+    {{- toYaml $injectedCRD -}}
+  {{- end }}
+---
+{{ end }}
+{{- end -}}
diff --git a/examples/chart/teleport-cluster/charts/teleport-operator/tests/crds_test.yaml b/examples/chart/teleport-cluster/charts/teleport-operator/tests/crds_test.yaml
@@ -0,0 +1,44 @@
+suite: Operator CRDs
+templates:
+  - crds.yaml
+tests:
+  - it: creates no CRDs when installCRDs is "never"
+    set:
+      installCRDs: "never"
+      enabled: true
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: creates CRDs when installCRDs is "always"
+    set:
+      installCRDs: "always"
+      enabled: false
+    asserts:
+      - containsDocument:
+          apiVersion: apiextensions.k8s.io/v1
+          kind: CustomResourceDefinition
+          name: teleportrolesv7.resources.teleport.dev
+  - it: labels CRDs
+    set:
+      installCRDs: "always"
+      enabled: false
+    asserts:
+      - equal:
+          path: metadata.labels.[teleport.dev/release]
+          value: NAMESPACE_RELEASE-NAME
+  - it: creates CRDs when installCRDs is "dynamic" and operator enabled
+    set:
+      installCRDs: "dynamic"
+      enabled: true
+    asserts:
+      - containsDocument:
+          apiVersion: apiextensions.k8s.io/v1
+          kind: CustomResourceDefinition
+          name: teleportrolesv7.resources.teleport.dev
+  - it: creates no CRDs when installCRDs is "dynamic" and operator disabled (and no existing CRD)
+    set:
+      installCRDs: "dynamic"
+      enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/examples/chart/teleport-cluster/charts/teleport-operator/values.yaml b/examples/chart/teleport-cluster/charts/teleport-operator/values.yaml
@@ -4,6 +4,18 @@
 # - When `false`, the chart creates the `CustomResourceDefinition` resources without the operator `Deployment`.
 enabled: true
 
+# installCRDs(string) -- controls if the chart should install the CRDs.
+# There are 3 possible values: dynamic, always, never.
+#
+# - "dynamic" means the CRDs are installed if the operator is enabled or if
+#   the CRDs are already present in the cluster. The presence check is here to
+#   avoid all CRDs to be removed if you temporarily disable the operator.
+#   Removing CRDs triggers a cascading deletion, which removes CRs, and all the
+#   related resources in Teleport.
+# - "always" means the CRDs are always installed
+# - "never" means the CRDs are never installed
+installCRDs: "dynamic"
+
 # teleportAddress(string) -- is the address of the Teleport cluster whose resources
 # are managed by the operator. The address must contain both the domain name and
 # the port of the Teleport cluster. It can be either the address of the Auth Service

diff --git a/integrations/operator/Makefile b/integrations/operator/Makefile
@@ -98,10 +98,8 @@ manifests: crdgen controller-gen ## Generate WebhookConfiguration, ClusterRole a
 			"$${proto}"; \
 	done
 
-# Some comments have `{{xyz}}` which triggers the golang templating engine when installing the chart.
-# We don't want that, so we escape them.
-	$(foreach file, $(wildcard $(CURDIR)/config/crd/bases/*), \
-		$(shell sed 's/`{{/{{ `{{/g;s/}}`/}}` }}/g' $(file) > $(CURDIR)/../../examples/chart/teleport-cluster/charts/teleport-operator/templates/$(notdir $(file))))
+	rm -r ../../examples/chart/teleport-cluster/charts/teleport-operator/operator-crds/
+	cp -r ./config/crd/bases/ ../../examples/chart/teleport-cluster/charts/teleport-operator/operator-crds
 
 .PHONY: generate
 generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.