From e09de0dc636e8a8ee277d1e729a6f967867c62d8 Mon Sep 17 00:00:00 2001
From: Theron Voran <tvoran@users.noreply.github.com>
Date: Mon, 27 Apr 2020 08:28:50 -0700
Subject: [PATCH] Allow both yaml and multi-line string annotations (#272)

Changed/added helper functions to detect if the annotations value
is a string or yaml, and apply `tpl` or `toYaml`
accordingly. Defaults are left as `{}` since yaml is more likely
to be used with helm on the command line. This means a warning
will be shown when setting an annotation to a multi-line
string (which has been the existing behavior).
---
 templates/_helpers.tpl                        | 42 +++++++++++++++--
 templates/server-ha-active-service.yaml       |  4 +-
 templates/server-ha-standby-service.yaml      |  4 +-
 templates/server-headless-service.yaml        |  4 +-
 templates/server-service.yaml                 |  4 +-
 test/acceptance/server-annotations.bats       | 46 +++++++++++++++++++
 .../server-test/annotations-overrides.yaml    |  9 ++++
 test/unit/server-ha-standby-service.bats      | 13 +++++-
 test/unit/server-ingress.bats                 | 14 +++++-
 test/unit/server-serviceaccount.bats          |  8 ++++
 test/unit/server-statefulset.bats             | 22 +++++++++
 test/unit/ui-service.bats                     | 10 ++++
 values.yaml                                   | 22 +++++----
 13 files changed, 175 insertions(+), 27 deletions(-)
 create mode 100644 test/acceptance/server-annotations.bats
 create mode 100644 test/acceptance/server-test/annotations-overrides.yaml

diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl
index 12a006a..bab233b 100644
--- a/templates/_helpers.tpl
+++ b/templates/_helpers.tpl
@@ -264,7 +264,12 @@ Sets extra pod annotations
 {{- define "vault.annotations" -}}
   {{- if and (ne .mode "dev") .Values.server.annotations }}
       annotations:
-        {{- tpl .Values.server.annotations . | nindent 8 }}
+        {{- $tp := typeOf .Values.server.annotations }}
+        {{- if eq $tp "string" }}
+          {{- tpl .Values.server.annotations . | nindent 8 }}
+        {{- else }}
+          {{- toYaml .Values.server.annotations | nindent 8 }}
+        {{- end }}
   {{- end }}
 {{- end -}}
 
@@ -274,7 +279,12 @@ Sets extra ui service annotations
 {{- define "vault.ui.annotations" -}}
   {{- if .Values.ui.annotations }}
   annotations:
-    {{- tpl .Values.ui.annotations . | nindent 4 }}
+    {{- $tp := typeOf .Values.ui.annotations }}
+    {{- if eq $tp "string" }}
+      {{- tpl .Values.ui.annotations . | nindent 4 }}
+    {{- else }}
+      {{- toYaml .Values.ui.annotations | nindent 4 }}
+    {{- end }}
   {{- end }}
 {{- end -}}
 
@@ -284,7 +294,12 @@ Sets extra service account annotations
 {{- define "vault.serviceAccount.annotations" -}}
   {{- if and (ne .mode "dev") .Values.server.serviceAccount.annotations }}
   annotations:
-    {{- tpl .Values.server.serviceAccount.annotations . | nindent 4 }}
+    {{- $tp := typeOf .Values.server.serviceAccount.annotations }}
+    {{- if eq $tp "string" }}
+      {{- tpl .Values.server.serviceAccount.annotations . | nindent 4 }}
+    {{- else }}
+      {{- toYaml .Values.server.serviceAccount.annotations | nindent 4 }}
+    {{- end }}
   {{- end }}
 {{- end -}}
 
@@ -294,7 +309,26 @@ Sets extra ingress annotations
 {{- define "vault.ingress.annotations" -}}
   {{- if .Values.server.ingress.annotations }}
   annotations:
-    {{- tpl .Values.server.ingress.annotations . | nindent 4 }}
+    {{- $tp := typeOf .Values.server.ingress.annotations }}
+    {{- if eq $tp "string" }}
+      {{- tpl .Values.server.ingress.annotations . | nindent 4 }}
+    {{- else }}
+      {{- toYaml .Values.server.ingress.annotations | nindent 4 }}
+    {{- end }}
+  {{- end }}
+{{- end -}}
+
+{{/*
+Sets extra vault server Service annotations
+*/}}
+{{- define "vault.service.annotations" -}}
+  {{- if .Values.server.service.annotations }}
+    {{- $tp := typeOf .Values.server.service.annotations }}
+    {{- if eq $tp "string" }}
+      {{- tpl .Values.server.service.annotations . | nindent 4 }}
+    {{- else }}
+      {{- toYaml .Values.server.service.annotations | nindent 4 }}
+    {{- end }}
   {{- end }}
 {{- end -}}
 
diff --git a/templates/server-ha-active-service.yaml b/templates/server-ha-active-service.yaml
index 0333df1..01f962d 100644
--- a/templates/server-ha-active-service.yaml
+++ b/templates/server-ha-active-service.yaml
@@ -13,9 +13,7 @@ metadata:
     app.kubernetes.io/instance: {{ .Release.Name }}
     app.kubernetes.io/managed-by: {{ .Release.Service }}
   annotations:
-{{- if .Values.server.service.annotations }}
-{{ tpl .Values.server.service.annotations . | indent 4 }}
-{{- end }}
+{{ template "vault.service.annotations" .}}
 spec:
   type: ClusterIP
   publishNotReadyAddresses: true
diff --git a/templates/server-ha-standby-service.yaml b/templates/server-ha-standby-service.yaml
index d8df9e7..302627a 100644
--- a/templates/server-ha-standby-service.yaml
+++ b/templates/server-ha-standby-service.yaml
@@ -13,9 +13,7 @@ metadata:
     app.kubernetes.io/instance: {{ .Release.Name }}
     app.kubernetes.io/managed-by: {{ .Release.Service }}
   annotations:
-{{- if .Values.server.service.annotations }}
-{{ tpl .Values.server.service.annotations . | indent 4 }}
-{{- end }}
+{{ template "vault.service.annotations" .}}
 spec:
   type: ClusterIP
   publishNotReadyAddresses: true
diff --git a/templates/server-headless-service.yaml b/templates/server-headless-service.yaml
index cced609..4bb276b 100644
--- a/templates/server-headless-service.yaml
+++ b/templates/server-headless-service.yaml
@@ -14,9 +14,7 @@ metadata:
     app.kubernetes.io/managed-by: {{ .Release.Service }}
   annotations:
     service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"
-{{- if .Values.server.service.annotations }}
-{{ tpl .Values.server.service.annotations . | indent 4 }}
-{{- end }}
+{{ template "vault.service.annotations" .}}
 spec:
   clusterIP: None
   publishNotReadyAddresses: true
diff --git a/templates/server-service.yaml b/templates/server-service.yaml
index 4d0e289..6d50584 100644
--- a/templates/server-service.yaml
+++ b/templates/server-service.yaml
@@ -17,9 +17,7 @@ metadata:
     # to an open issue where it may not work:
     # https://github.com/kubernetes/kubernetes/issues/58662
     service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"
-{{- if .Values.server.service.annotations }}
-{{ tpl .Values.server.service.annotations . | indent 4 }}
-{{- end }}
+{{ template "vault.service.annotations" .}}
 spec:
   {{- if .Values.server.service.type}}
   type: {{ .Values.server.service.type }}
diff --git a/test/acceptance/server-annotations.bats b/test/acceptance/server-annotations.bats
new file mode 100644
index 0000000..d382788
--- /dev/null
+++ b/test/acceptance/server-annotations.bats
@@ -0,0 +1,46 @@
+#!/usr/bin/env bats
+
+load _helpers
+
+@test "server/annotations: testing yaml and yaml-formatted string formats" {
+  cd `chart_dir`
+  kubectl delete namespace acceptance --ignore-not-found=true
+  kubectl create namespace acceptance
+  kubectl config set-context --current --namespace=acceptance
+
+  helm install "$(name_prefix)" -f ./test/acceptance/server-test/annotations-overrides.yaml .
+  wait_for_running $(name_prefix)-0
+
+  # service annotations
+  local awesome=$(kubectl get service "$(name_prefix)" --output json |
+    jq -r '.metadata.annotations.active')
+  [ "${awesome}" == "sometimes" ]
+
+  local pickMe=$(kubectl get service "$(name_prefix)" --output json |
+    jq -r '.metadata.annotations.pickMe')
+  [ "${pickMe}" == "please" ]
+
+  local environment=$(kubectl get statefulset "$(name_prefix)" --output json |
+    jq -r '.spec.template.metadata.annotations.environment')
+  [ "${environment}" == "production" ]
+
+  local milk=$(kubectl get statefulset "$(name_prefix)" --output json |
+    jq -r '.spec.template.metadata.annotations.milk')
+  [ "${milk}" == "oat" ]
+
+  local myName=$(kubectl get statefulset "$(name_prefix)" --output json |
+    jq -r '.spec.template.metadata.annotations.myName')
+  [ "${myName}" == "$(name_prefix)" ]
+
+}
+
+# Clean up
+teardown() {
+  if [[ ${CLEANUP:-true} == "true" ]]
+  then
+      echo "helm/pvc teardown"
+      helm delete $(name_prefix)
+      kubectl delete --all pvc
+      kubectl delete namespace acceptance --ignore-not-found=true
+  fi
+}
diff --git a/test/acceptance/server-test/annotations-overrides.yaml b/test/acceptance/server-test/annotations-overrides.yaml
new file mode 100644
index 0000000..459576a
--- /dev/null
+++ b/test/acceptance/server-test/annotations-overrides.yaml
@@ -0,0 +1,9 @@
+server:
+  annotations: |
+    environment: production
+    milk: oat
+    myName: "{{ .Release.Name }}"
+  service:
+    annotations:
+      active: sometimes
+      pickMe: please
diff --git a/test/unit/server-ha-standby-service.bats b/test/unit/server-ha-standby-service.bats
index 7630ac5..f2f0043 100644
--- a/test/unit/server-ha-standby-service.bats
+++ b/test/unit/server-ha-standby-service.bats
@@ -2,7 +2,7 @@
 
 load _helpers
 
-@test "server/ha-standby-Service: generic annotations" {
+@test "server/ha-standby-Service: generic annotations string" {
   cd `chart_dir`
   local actual=$(helm template \
       --show-only templates/server-ha-standby-service.yaml \
@@ -12,3 +12,14 @@ load _helpers
       yq -r '.metadata.annotations["vaultIsAwesome"]' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
+
+@test "server/ha-standby-Service: generic annotations yaml" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.annotations.vaultIsAwesome=true' \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations["vaultIsAwesome"]' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
diff --git a/test/unit/server-ingress.bats b/test/unit/server-ingress.bats
index 9f54e5c..8660920 100755
--- a/test/unit/server-ingress.bats
+++ b/test/unit/server-ingress.bats
@@ -70,7 +70,7 @@ load _helpers
   [ "${actual}" = "external" ]
 }
 
-@test "server/ingress: annotations added to object" {
+@test "server/ingress: annotations added to object - string" {
   cd `chart_dir`
 
   local actual=$(helm template \
@@ -81,3 +81,15 @@ load _helpers
       yq -r '.metadata.annotations["kubernetes.io/ingress.class"]' | tee /dev/stderr)
   [ "${actual}" = "nginx" ]
 }
+
+@test "server/ingress: annotations added to object - yaml" {
+  cd `chart_dir`
+
+  local actual=$(helm template \
+      --show-only templates/server-ingress.yaml \
+      --set 'server.ingress.enabled=true' \
+      --set server.ingress.annotations."kubernetes\.io/ingress\.class"=nginx \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations["kubernetes.io/ingress.class"]' | tee /dev/stderr)
+  [ "${actual}" = "nginx" ]
+}
diff --git a/test/unit/server-serviceaccount.bats b/test/unit/server-serviceaccount.bats
index 5b8744a..fe09c2a 100755
--- a/test/unit/server-serviceaccount.bats
+++ b/test/unit/server-serviceaccount.bats
@@ -20,6 +20,14 @@ load _helpers
       yq -r '.metadata.annotations["foo"]' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 
+  local actual=$(helm template \
+      --show-only templates/server-serviceaccount.yaml  \
+      --set 'server.ha.enabled=true' \
+      --set 'server.serviceAccount.annotations.foo=bar' \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations["foo"]' | tee /dev/stderr)
+  [ "${actual}" = "bar" ]
+
   local actual=$(helm template \
       --show-only templates/server-serviceaccount.yaml  \
       --set 'server.ha.enabled=true' \
diff --git a/test/unit/server-statefulset.bats b/test/unit/server-statefulset.bats
index 3d08925..8e80119 100755
--- a/test/unit/server-statefulset.bats
+++ b/test/unit/server-statefulset.bats
@@ -936,3 +936,25 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].ports | map(select(.containerPort==8202)) | .[] .name' | tee /dev/stderr)
   [ "${actual}" = "https-rep" ]
 }
+
+#--------------------------------------------------------------------
+# annotations
+@test "server/standalone-StatefulSet: generic annotations string" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-statefulset.yaml \
+      --set 'server.annotations=vaultIsAwesome: true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.metadata.annotations["vaultIsAwesome"]' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "server/ha-standby-Service: generic annotations yaml" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-statefulset.yaml \
+      --set 'server.annotations.vaultIsAwesome=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.metadata.annotations["vaultIsAwesome"]' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
diff --git a/test/unit/ui-service.bats b/test/unit/ui-service.bats
index 042e141..b92160b 100755
--- a/test/unit/ui-service.bats
+++ b/test/unit/ui-service.bats
@@ -205,6 +205,16 @@ load _helpers
       yq -r '.metadata.annotations["foo"]' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 
+  local actual=$(helm template \
+      --show-only templates/ui-service.yaml  \
+      --set 'server.ha.enabled=true' \
+      --set 'ui.serviceType=LoadBalancer' \
+      --set 'ui.enabled=true' \
+      --set 'ui.annotations.foo=bar' \
+      . | tee /dev/stderr |
+      yq -r '.metadata.annotations["foo"]' | tee /dev/stderr)
+  [ "${actual}" = "bar" ]
+
   local actual=$(helm template \
       --show-only templates/ui-service.yaml  \
       --set 'server.ha.enabled=true' \
diff --git a/values.yaml b/values.yaml
index 54ca6d0..b0b303c 100644
--- a/values.yaml
+++ b/values.yaml
@@ -137,6 +137,9 @@ server:
       # |
       # kubernetes.io/ingress.class: nginx
       # kubernetes.io/tls-acme: "true"
+      #   or
+      # kubernetes.io/ingress.class: nginx
+      # kubernetes.io/tls-acme: "true"
     hosts:
       - host: chart-example.local
         paths: []
@@ -230,8 +233,8 @@ server:
   extraLabels: {}
 
   # Extra annotations to attach to the server pods
-  # This should be a multi-line string mapping directly to the a map of
-  # the annotations to apply to the server pods
+  # This can either be YAML or a YAML-formatted multi-line templated string map
+  # of the annotations to apply to the server pods
   annotations: {}
 
   # Enables a headless service to be used by the Vault Statefulset
@@ -257,8 +260,9 @@ server:
     port: 8200
     # Target port to which the service should be mapped to
     targetPort: 8200
-    # Extra annotations for the service definition. This should be a multi-line
-    # string formatted as a map of the annotations to apply to the service.
+    # Extra annotations for the service definition. This can either be YAML or a
+    # YAML-formatted multi-line templated string map of the annotations to apply
+    # to the service.
     annotations: {}
 
   # This configures the Vault Statefulset to create a PVC for data
@@ -400,9 +404,9 @@ server:
 
   # Definition of the serviceAccount used to run Vault.
   serviceAccount:
-    # Extra annotations for the serviceAccount definition. This should be a
-    # multi-line string formatted as a map of the annotations to apply to the
-    # serviceAccount.
+    # Extra annotations for the serviceAccount definition. This can either be
+    # YAML or a YAML-formatted multi-line templated string map of the
+    # annotations to apply to the serviceAccount.
     annotations: {}
 
 # Vault UI
@@ -424,6 +428,6 @@ ui:
   # loadBalancerIP:
 
   # Extra annotations to attach to the ui service
-  # This should be a multi-line string mapping directly to the a map of
-  # the annotations to apply to the ui service
+  # This can either be YAML or a YAML-formatted multi-line templated string map
+  # of the annotations to apply to the ui service
   annotations: {}