Helm(hooks): honor imagePullPolicy and imagePullSecrets (#873)

chart/templates/deployment.yaml

@@ -11,7 +11,7 @@ metadata:
   labels:
     app.kubernetes.io/component: controller-manager
   {{- include "vso.chart.labels" . | nindent 4 }}
-{{ include "vso.imagePullSecrets" .}}
+{{ include "vso.imagePullSecrets" . }}
 ---
 apiVersion: apps/v1
 kind: Deployment
@@ -197,6 +197,7 @@ spec:
       containers:
       - name: pre-delete-controller-cleanup
         image: {{ .Values.controller.manager.image.repository }}:{{ .Values.controller.manager.image.tag }}
+        imagePullPolicy: {{ .Values.controller.manager.image.pullPolicy }}
         args:
         - --uninstall
         - --pre-delete-hook-timeout-seconds={{ .Values.controller.preDeleteHookTimeoutSeconds }}

chart/templates/hook-upgrade-crds.yaml

@@ -12,11 +12,12 @@ metadata:
   namespace: {{ .Release.Namespace }}
   labels:
     app.kubernetes.io/component: controller-manager
-{{ include "vso.chart.labels" . | indent 4 }}
+  {{- include "vso.chart.labels" . | nindent 4 }}
   annotations:
     helm.sh/hook: pre-upgrade
     helm.sh/hook-delete-policy: "hook-succeeded,before-hook-creation"
     helm.sh/hook-weight: "1"
+{{ include "vso.imagePullSecrets" . }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
@@ -91,6 +92,7 @@ spec:
       containers:
       - name: pre-upgrade-crds
         image: {{ .Values.controller.manager.image.repository }}:{{ .Values.controller.manager.image.tag }}
+        imagePullPolicy: {{ .Values.controller.manager.image.pullPolicy }}
         env:
         - name: VSO_UPGRADE_CRDS_TIMEOUT
           value: {{ .Values.hooks.upgradeCRDs.executionTimeout }}

test/unit/deployment.bats

@@ -1111,3 +1111,24 @@ load _helpers
    actual=$(echo "$object" | yq '.strategy.rollingUpdate.maxUnavailable' | tee /dev/stderr)
    [ "${actual}" = "1" ]
 }
+
+@test "controller/Deployment: hookPreDelete Job" {
+  cd "$(chart_dir)"
+  local job
+  job=$(helm template \
+      -s templates/deployment.yaml  \
+      . | tee /dev/stderr |
+      yq 'select(.kind == "Job")' | tee /dev/stderr)
+  [ "$(echo "${job}" | \
+    yq '(.spec.template.spec.containers | length) == "1"')" = "true" ]
+  [ "$(echo "${job}" | \
+    yq '.spec.template.spec.containers[0].command[0] == "/vault-secrets-operator"')" = "true" ]
+  [ "$(echo "${job}" | \
+    yq '(.spec.template.spec.containers[0].args | length) == "2"')" = "true" ]
+  [ "$(echo "${job}" | \
+    yq '.spec.template.spec.containers[0].args[0]  == "--uninstall"')" = "true" ]
+  [ "$(echo "${job}" | \
+    yq '.spec.template.spec.containers[0].args[1]  == "--pre-delete-hook-timeout-seconds=120"')" = "true" ]
+  [ "$(echo "${job}" | \
+    yq '(.spec.template.spec.containers[0].imagePullPolicy == "IfNotPresent")')" = "true" ]
+}

test/unit/hook-upgrade-crds.bats

@@ -26,7 +26,7 @@ load _helpers
         -s templates/hook-upgrade-crds.yaml \
         . | tee /dev/stderr)
 
-    # assert that we have 4 documents using an base 0 index
+    # assert that we have 4 documents using a base 0 index
     [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
 
     local sa
@@ -77,6 +77,8 @@ load _helpers
       yq '.metadata.annotations."helm.sh/hook-delete-policy" == "hook-succeeded,before-hook-creation"')" = "true" ]
     [ "$(echo "${job}"  | \
       yq '.metadata.annotations."helm.sh/hook-weight" == "99"')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '(.spec.template.spec.imagePullSecrets | length == 0)')" = "true" ]
 }
 
 
@@ -87,7 +89,7 @@ load _helpers
         -s templates/hook-upgrade-crds.yaml \
         . )
 
-    # assert that we have 4 documents using an base 0 index
+    # assert that we have 4 documents using a base 0 index
     [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
 
     local cr
@@ -114,7 +116,7 @@ load _helpers
         -s templates/hook-upgrade-crds.yaml \
         . )
 
-    # assert that we have 4 documents using an base 0 index
+    # assert that we have 4 documents using a base 0 index
     [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
 
     local job
@@ -130,6 +132,8 @@ load _helpers
       yq '(.spec.template.spec.containers[0].resources | length) == 2')" = "true" ]
     [ "$(echo "${job}" | \
       yq '(.spec.template.spec.containers[0].env | length == 1)')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '.spec.template.spec.containers[0].imagePullPolicy == "IfNotPresent"')" = "true" ]
 }
 
 @test "hookUpgradeCRDs: Job extended with defaults" {
@@ -139,7 +143,7 @@ load _helpers
         -s templates/hook-upgrade-crds.yaml \
         . )
 
-    # assert that we have 4 documents using an base 0 index
+    # assert that we have 4 documents using a base 0 index
     [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
 
     local job
@@ -170,7 +174,7 @@ load _helpers
         --set hooks.resources.requests.memory='65Mi' \
         . )
 
-    # assert that we have 4 documents using an base 0 index
+    # assert that we have 4 documents using a base 0 index
     [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
 
     local job
@@ -199,3 +203,57 @@ load _helpers
     [ "$(echo "${job}" | \
       yq '.spec.template.spec.containers[0].env[0].value == "64s"')" = "true" ]
 }
+
+@test "hookUpgradeCRDs: Job extended with imagePullPolicy" {
+    pushd "$(chart_dir)" > /dev/stderr
+    local object
+    object=$(helm template \
+        --set controller.manager.image.pullPolicy='_Always_' \
+        -s templates/hook-upgrade-crds.yaml \
+        . )
+
+    # assert that we have 4 documents using a base 0 index
+    [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
+
+    local job
+    job="$(echo "${object}" | yq 'select(di == 3)' | tee /dev/stderr)"
+    [ "$(echo "${job}" | yq '.kind == "Job"')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '.spec.backoffLimit == 5')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '(.spec.template.spec.containers | length) == "1"')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '.spec.template.spec.containers[0].command[0] == "/scripts/upgrade-crds"')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '(.spec.template.spec.containers[0].resources | length) == 2')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '(.spec.template.spec.containers[0].env | length == 1)')" = "true" ]
+    [ "$(echo "${job}" | \
+      yq '(.spec.template.spec.containers[0].imagePullPolicy == "_Always_")')" = "true" ]
+    popd > /dev/stderr
+}
+
+@test "hookUpgradeCRDs: ServiceAccount extended with imagePullSecrets" {
+    pushd "$(chart_dir)" > /dev/stderr
+    local object
+    object=$(helm template \
+        --set controller.imagePullSecrets[0].name='pullSecret1' \
+        --set controller.imagePullSecrets[1].name='pullSecret2' \
+        -s templates/hook-upgrade-crds.yaml \
+        . )
+
+    # assert that we have 4 documents using a base 0 index
+    [ "$(echo "${object}" | yq '. | di' | tee /dev/stderr | tail -n1)" = "3" ]
+
+    local sa
+    sa="$(echo "${object}" | yq 'select(di == 0)' | tee /dev/stderr)"
+    [ "$(echo "${sa}" | \
+      yq '.kind == "ServiceAccount"')" = "true" ]
+    [ "$(echo "${sa}" | \
+      yq '(.imagePullSecrets | length) == "2"')" = "true" ]
+    [ "$(echo "${sa}" | \
+      yq '(.imagePullSecrets[0].name == "pullSecret1")')" = "true" ]
+    [ "$(echo "${sa}" | \
+      yq '(.imagePullSecrets[1].name == "pullSecret2")')" = "true" ]
+    popd > /dev/stderr
+}