Merge pull request #595 from aiyengar2/change_crd_ownership

Add generateCRDChart.assumeOwnershipOfCRDs flag

README.md

@@ -100,6 +100,11 @@ up-to-date with the latest chart. It also automatically build github pages to se
 
 ### Experimental: Splitting CRDs from an upstream package into a separate package
 
+**Note to Contributers:** This flag can only be used to create separate CRD charts if your chart meets the following three requirements:
+- The chart defines one or more CRDs
+- The chart is based on an upstream chart and includes a `package.yaml` (if this is not the case, you will need to manually create a separate CRD chart)
+- The chart tries to install some default CRs based on the CRDs that it defines (if this is not the case, you should place the CRDs directly within the `templates/` directory of the chart; using a CRD chart is only necessary since rendering the chart will fail since the `kind` of the default CRs cannot be found in the cluster as the CRD is not installed yet).
+
 There are cases in which upstream charts import CRDs into a cluster using the Helm 3 `crd/` directory, which allows a user to first install the CRDs before rendering the templates created by the chart. However, using this approach has certain caveats [as documented by Helm](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/), such as an inability to upgrade / delete those CRDs and or use the `--dry-run` flag on the package before installing the CRDs. As a result, it may be advised to move those CRDs into a separate chart under the `templates/` directory so that the lifecycle of those CRDs can continue to be managed by Helm.
 
 However, in the current `rancher/charts` model, this would require deleting the CRDs from the upstream chart (which introduces significant changes to the package's patch) and maintaining a separate CRD chart that also needs to be kept up to date with the upstream chart. This poses several challenges, including but not limited to:
@@ -114,6 +119,8 @@ To resolve this, `rancher/charts` has a flag that can be added to the `package.y
 
    After running the default prepare script to pull the chart from upstream and apply the patch, a new directory called `charts-crd` is also created alongside `charts`. This will represent your new CRD chart. Any CRDs located within the Rancher chart in `charts/crd/` will be relocated to `charts-crd/templates/` and a new `charts-crd/Chart.yaml` (with chart names `{CHART_NAME}-crd`) and `charts-crd/README.md` will be generated. The `charts-crd/Chart.yaml` and `charts/Chart.yaml` will also be updated with the relevant annotations used by Rancher to auto-install the CRD chart from Dashboard.
 
+   If you are using the `generateCRDChart.assumeOwnershipOfCRDs` flag, the CRDs will instead be located in `charts-crd/crd-manifest/*` and some additional resources (ConfigMap, Jobs, and RBAC resources) will be created in the `charts-crd/templates` directory instead. This option should only be enabled if the chart is expected to be deployed in a setting where all or a subset of your CRDs may or may not already exist, in which case your CRD chart may want to assume ownership of the CRDs to prevent a failure on deploy.
+
    In addition, a new file `charts/templates/validate-install-${CHART_NAME}-crd.yaml` will be added to your Rancher chart that is automatically configured to validate whether the CRDs that have been moved to the CRD chart are installed onto your cluster before trying to render the Rancher chart. For example, here is an error you might encounter if you try to install the Rancher chart first:
 
    ```
@@ -126,7 +133,7 @@ To resolve this, `rancher/charts` has a flag that can be added to the `package.y
  
    The experience of modifying values within the `charts` directory and making a new patch is unchanged. The same workflow also applies to the `charts-crd` directory with two caveats:
    - Changes to `charts/templates/validate-install-${CHART_NAME}-crd.yaml`, `charts-crd/Chart.yaml`, and `charts-crd/README.md` will be ignored / not be shown in the patch as they are not expected to be updated
-   - Any changes to `charts-crd/templates/*` will show up in the patch as if you had changed the relevant file within `charts/crd/*`.
+   - Any changes to `charts-crd/templates/*` (`charts-crd/crd-manifest/*` if you are using the `generateCRDChart.assumeOwnershipOfCRDs` flag) will show up in the patch as if you had changed the relevant file within `charts/crd/*`.
 
    Files added to the `overlay` directory will only overlay onto the Rancher chart, not the CRD chart.
    

packages/rancher-monitoring/package.yaml

@@ -2,3 +2,4 @@ url: https://kubernetes-charts.storage.googleapis.com/prometheus-operator-8.16.1
 packageVersion: 00
 generateCRDChart:
   enabled: true
+  assumeOwnershipOfCRDs: true

scripts/chart-templates/crd-assume-ownership/templates/_helpers.tpl

@@ -0,0 +1,7 @@
+# Rancher
+
+{{- define "system_default_registry" -}}
+{{- if .Values.global.cattle.systemDefaultRegistry -}}
+{{- printf "%s/" .Values.global.cattle.systemDefaultRegistry -}}
+{{- end -}}
+{{- end -}}

scripts/chart-templates/crd-assume-ownership/templates/jobs.yaml

@@ -0,0 +1,86 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: {{ .Chart.Name }}-create
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}
+  annotations:
+    "helm.sh/hook": post-install, post-upgrade, post-rollback
+    "helm.sh/hook-delete-policy": hook-succeeded
+spec:
+  template:
+    metadata:
+      name: {{ .Chart.Name }}-create
+      labels:
+        app: {{ .Chart.Name }}
+    spec:
+      serviceAccountName: {{ .Chart.Name }}-manager
+      containers:
+        - name: create-crds
+          image: {{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}
+          imagePullPolicy: IfNotPresent
+          command:
+          - /bin/kubectl
+          - apply
+          - -f
+          - /etc/config/crd-manifest.yaml
+          volumeMounts:
+            - name: crd-manifest
+              readOnly: true
+              mountPath: /etc/config
+      restartPolicy: OnFailure
+      volumes:
+      - name: crd-manifest
+        configMap:
+          name: {{ .Chart.Name }}-manifest
+---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: {{ .Chart.Name }}-delete
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}
+  annotations:
+    "helm.sh/hook": pre-delete
+    "helm.sh/hook-delete-policy": hook-succeeded
+spec:
+  template:
+    metadata:
+      name: {{ .Chart.Name }}-delete
+      labels:
+        app: {{ .Chart.Name }}
+    spec:
+      serviceAccountName: {{ .Chart.Name }}-manager
+      initContainers:
+        - name: remove-finalizers
+          image: {{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}
+          imagePullPolicy: IfNotPresent
+          command:
+          - /bin/kubectl
+          - apply
+          - -f
+          - /etc/config/crd-manifest.yaml
+          volumeMounts:
+            - name: crd-manifest
+              readOnly: true
+              mountPath: /etc/config
+      containers:
+        - name: delete-crds
+          image: {{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}
+          imagePullPolicy: IfNotPresent
+          command:
+          - /bin/kubectl
+          - delete
+          - -f
+          - /etc/config/crd-manifest.yaml
+          volumeMounts:
+            - name: crd-manifest
+              readOnly: true
+              mountPath: /etc/config
+      restartPolicy: OnFailure
+      volumes:
+      - name: crd-manifest
+        configMap:
+          name: {{ .Chart.Name }}-manifest

scripts/chart-templates/crd-assume-ownership/templates/manifest.yaml

@@ -0,0 +1,14 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: {{ .Chart.Name }}-manifest
+  namespace: {{ .Release.Namespace }}
+data:
+  crd-manifest.yaml: |
+    {{- \$currentScope := . -}}
+    {{- \$crds := (.Files.Glob  "crd-manifest/**.yaml") -}}
+    {{- range \$path, \$_ :=  \$crds -}}
+    {{- with \$currentScope -}}
+    {{ .Files.Get \$path | nindent 4 }}
+    ---
+    {{- end -}}{{- end -}}

scripts/chart-templates/crd-assume-ownership/templates/rbac.yaml

@@ -0,0 +1,35 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: {{ .Chart.Name }}-manager
+  labels:
+    app: {{ .Chart.Name }}-manager
+rules:
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs: ['create', 'get', 'patch', 'delete']
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: {{ .Chart.Name }}-manager
+  labels:
+    app: {{ .Chart.Name }}-manager
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ .Chart.Name }}-manager
+subjects:
+- kind: ServiceAccount
+  name: {{ .Chart.Name }}-manager
+  namespace: {{ .Release.Namespace }}
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{ .Chart.Name }}-manager
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}-manager

scripts/chart-templates/crd-assume-ownership/values.yaml

@@ -0,0 +1,11 @@
+# Default values for ${name}-crd.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+global:
+  cattle:
+    systemDefaultRegistry: ""
+
+image:
+  repository: rancher/kubectl
+  tag: v1.18.6

scripts/chart-templates/crd-base/Chart.yaml

@@ -0,0 +1,7 @@
+apiVersion: ${api_version}
+version: ${chart_version}
+description: Installs the CRDs for ${name}.
+name: ${name}-crd
+type: application
+annotations:
+  catalog.cattle.io/hidden: "true"

scripts/chart-templates/crd-base/README.md

@@ -0,0 +1,2 @@
+# ${name}-crd
+A Rancher chart that installs the CRDs used by [${name}](https://github.com/rancher/dev-charts/tree/master/packages/${name}).

scripts/chart-templates/crd-base/overlay-upstream/templates/validate-install-crd.yaml

@@ -0,0 +1,12 @@
+# {{- \$found := dict -}}
+${set_found_crd}
+# {{- range .Capabilities.APIVersions -}}
+# {{- if hasKey \$found (toString .) -}}
+# 	{{- set \$found (toString .) true -}}
+# {{- end -}}
+# {{- end -}}
+# {{- range \$_, \$exists := \$found -}}
+# {{- if (eq \$exists false) -}}
+# 	{{- required "Required CRDs are missing. Please install the ${name}-crd chart before installing this chart." "" -}}
+# {{- end -}}
+# {{- end -}}

scripts/clean-crds

@@ -16,7 +16,11 @@ fi
 
 # Move CRDs back into ${f}/charts/crd/ and remove ${f}/charts-crd
 mkdir -p ${f}/charts/crds/
-mv ${f}/charts-crd/templates/* ${f}/charts/crds/
+if [[ -d ${f}/charts-crd/crd-manifest ]]; then
+	mv ${f}/charts-crd/crd-manifest/* ${f}/charts/crds/
+else
+	mv ${f}/charts-crd/templates/* ${f}/charts/crds/
+fi
 rm -rf ${f}/charts-crd
 
 if ! [[ -d ${f}/charts ]]; then
@@ -26,7 +30,7 @@ fi
 
 # Remove the validate-install-${name}-crd.yaml
 name=$(cat ${f}/charts/Chart.yaml | yq r - 'name')
-rm ${f}/charts/templates/validate-install-${name}-crd.yaml
+rm ${f}/charts/templates/validate-install-crd.yaml
 # Remove additional annotations added to original chart if added
 if [[ "$(yq r ${f}/charts/Chart.yaml 'annotations[catalog.cattle.io/auto-install]')" == "${name}-crd=match" ]]; then
     yq d -i ${f}/charts/Chart.yaml 'annotations[catalog.cattle.io/auto-install]'

scripts/prepare-crds

@@ -2,6 +2,14 @@
 set -e
 
 # Split the provided package into a charts and charts-crd package
+# This script is split up into the following sections
+# - Initial checks to make sure that the current state of the directory is suitable for running this script
+# - Collecting information from the upstream chart that will be used to fill out a set of templates
+# - Copy templates stored in ./scripts/chart-templates into ${f}/charts-crd based on flags
+# - Apply templates based on information collected from the upstream chart
+# - Move anything from ${f}/charts-crd/overlay-upstream to the upstream chart
+# - Move any CRDs from the upstream chart into the appropriate directory for CRDs based on flags
+# - Apply any additional necessary annotation logic on the Chart.yaml of both the upstream and CRD charts
 
 if [[ -z $1 ]]; then
 	echo "No directory provided to initialize charts-crd within"
@@ -34,11 +42,6 @@ if ! [[ -d ${f}/charts/crds ]] || [[ $(ls ${f}/charts/crds | wc -l) -eq 0 ]]; th
 	exit 1
 fi
 
-# Create directory and move CRDs
-mkdir -p ${f}/charts-crd/templates
-mv ${f}/charts/crds/* ${f}/charts-crd/templates
-rm -rf ${f}/charts/crds
-
 # Collect information on chart
 name=$(cat ${f}/charts/Chart.yaml | yq r - 'name')
 api_version=$(cat ${f}/charts/Chart.yaml | yq r - 'apiVersion')
@@ -46,7 +49,7 @@ chart_version=$(cat ${f}/charts/Chart.yaml | yq r - 'version')
 
 # Collect information on CRDs
 crd_apis=()
-for crd_yaml in ${f}/charts-crd/templates/*; do
+for crd_yaml in ${f}/charts/crds/*; do
 	crd_group=$(yq r ${crd_yaml} 'spec.group')
 	crd_kind=$(yq r ${crd_yaml} 'spec.names.kind')
 	crd_version=$(yq r ${crd_yaml} 'spec.version')
@@ -56,25 +59,45 @@ for crd_yaml in ${f}/charts-crd/templates/*; do
 	crd_apis+=("${crd_group}/${crd_version}/${crd_kind}")
 done
 
-# Init Chart.yaml for CRD chart
+# Set environment variable to be used in validate-install-crd.yaml template
-cat << EOF > ${f}/charts-crd/Chart.yaml
+set_found_crd=$(
-apiVersion: ${api_version}
+for crd in ${crd_apis[@]}; do
-version: ${chart_version}
+  echo "# {{- set \$found \"${crd}\" false -}}"
-description: A Rancher chart that creates ${name} CRDs within a cluster.
+done
-name: ${name}-crd
+)
-type: application
+
-EOF
+# Copy base template and apply variables to the template
+mkdir -p ${f}/charts-crd
+cp -R ./scripts/chart-templates/crd-base/* ${f}/charts-crd
+assume_ownership=$(cat ${f}/package.yaml | yq r - generateCRDChart.assumeOwnershipOfCRDs)
+if [[ ${assume_ownership} == "true" ]]; then
+    cp -fR ./scripts/chart-templates/crd-assume-ownership/* ${f}/charts-crd
+fi
+for template_file in $(find ${f}/charts-crd -type f); do
+  # Applies any environment variables currently set onto your template file
+  eval "echo \"$(sed 's/"/\\"/g' ${template_file})\"" > ${template_file}
+done
+
+# Move anything from ${f}/charts-crd/overlay-upstream to the main chart
+cp -R ${f}/charts-crd/overlay-upstream/* ${f}/charts
+rm -rf ${f}/charts-crd/overlay-upstream
+
+# Move CRDs from main chart to CRD chart
+if [[ ${assume_ownership} == "true" ]]; then
+  mkdir -p ${f}/charts-crd/crd-manifest
+  mv ${f}/charts/crds/* ${f}/charts-crd/crd-manifest
+else
+  mkdir -p ${f}/charts-crd/templates
+  mv ${f}/charts/crds/* ${f}/charts-crd/templates
+fi
+rm -rf ${f}/charts/crds
 
 # Make the primary chart auto-install the CRD chart
 if [[ -z "$(yq r ${f}/charts/Chart.yaml 'annotations[catalog.cattle.io/auto-install]')" ]]; then
 	yq w -i ${f}/charts/Chart.yaml "annotations[catalog.cattle.io/auto-install]" "${name}-crd=match"
 fi
-# Hide the CRD chart
-if [[ -z "$(yq r ${f}/charts-crd/Chart.yaml 'annotations[catalog.cattle.io/hidden]')" ]]; then
-	yq w -i ${f}/charts-crd/Chart.yaml "annotations[catalog.cattle.io/hidden]" "true"
-fi
 
-# Add annotations to charts-crd/Chart.yaml
+# Add annotations to ${f}/charts-crd/Chart.yaml
 copyAnnotations=(catalog.cattle.io/release-name catalog.cattle.io/certified catalog.cattle.io/experimental catalog.cattle.io/namespace)
 for a in ${copyAnnotations[@]}; do
 	v=$(yq r ${f}/charts/Chart.yaml "annotations[${a}]")
@@ -85,29 +108,3 @@ for a in ${copyAnnotations[@]}; do
 		yq w -i ${f}/charts-crd/Chart.yaml "annotations[${a}]" "${v}"
 	fi
 done
-
-# Init README.yaml for CRD chart
-cat << EOF > ${f}/charts-crd/README.md
-# ${name}-crd
-A Rancher chart that installs the CRDs used by [${name}](https://github.com/rancher/dev-charts/tree/master/packages/${name}).
-EOF
-
-# Copy a YAML that triggers a failure on the original chart if the CRDs that are copied don't exist
-cat << EOF > ${f}/charts/templates/validate-install-${name}-crd.yaml
-# {{- \$found := dict -}}
-$(
-for crd in ${crd_apis[@]}; do
-echo "# {{- set \$found \"${crd}\" false -}}"
-done
-)
-# {{- range .Capabilities.APIVersions -}}
-# {{- if hasKey \$found (toString .) -}}
-# 	{{- set \$found (toString .) true -}}
-# {{- end -}}
-# {{- end -}}
-# {{- range \$_, \$exists := \$found -}}
-# {{- if (eq \$exists false) -}}
-# 	{{- required "Required CRDs are missing. Please install the ${name}-crd chart before installing this chart." "" -}}
-# {{- end -}}
-# {{- end -}}
-EOF