Properly handle JSON server config

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Unreleased
 
+Bugs:
+
+* Properly handle JSON formatted server config [GH-1049](https://github.com/hashicorp/vault-helm/pull/1049)
+
 ## 0.28.1 (July 11, 2024)
 
 Changes:

templates/_helpers.tpl

@@ -183,7 +183,7 @@ defined a custom configuration.  Additionally iterates over any
 extra volumes the user may have specified (such as a secret with TLS).
 */}}
 {{- define "vault.volumes" -}}
-  {{- if and (ne .mode "dev") (or (.Values.server.standalone.config) (.Values.server.ha.config)) }}
+  {{- if and (ne .mode "dev") (or (.Values.server.standalone.config) (.Values.server.ha.config) (.Values.server.ha.raft.config)) }}
         - name: config
           configMap:
             name: {{ template "vault.fullname" . }}-config
@@ -1083,23 +1083,32 @@ Supported inputs are Values.ui
 config file from values
 */}}
 {{- define "vault.config" -}}
-  {{- if or (eq .mode "ha") (eq .mode "standalone") }}
-  {{- $type := typeOf (index .Values.server .mode).config }}
-  {{- if eq $type "string" }}
-    disable_mlock = true
-  {{- if eq .mode "standalone" }}
-    {{ tpl .Values.server.standalone.config . | nindent 4 | trim }}
-  {{- else if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "false") }}
-    {{ tpl .Values.server.ha.config . | nindent 4 | trim }}
-  {{- else if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "true") }}
-    {{ tpl .Values.server.ha.raft.config . | nindent 4 | trim }}
-  {{ end }}
-  {{- else }}
-  {{- if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "true") }}
-{{ merge (dict "disable_mlock" true) (index .Values.server .mode).raft.config | toPrettyJson | indent 4 }}
-  {{- else }}
-{{ merge (dict "disable_mlock" true) (index .Values.server .mode).config | toPrettyJson | indent 4 }}
-  {{- end }}
-  {{- end }}
-  {{- end }}
-{{- end -}}
+{{- if or (eq .mode "ha") (eq .mode "standalone") }}
+{{- $config := (index .Values.server .mode).config -}}
+{{- if .Values.server.ha.raft.enabled -}}
+{{- $config = .Values.server.ha.raft.config -}}
+{{- end -}}
+{{- $type := typeOf $config -}}
+{{- if eq $type "string" -}}
+{{/* Vault supports both HCL and JSON as its configuration format */}}
+{{- $json := $config | fromJson -}}
+{{/*
+Helm's fromJson does not behave according to the corresponding sprig function nor Helm docs,
+which claim that it should return empty string on invalid JSON, it actually returns
+a map containing a single 'Error' element.
+https://github.com/helm/helm/blob/50c22ed7f953fadb32755e5881ba95a92da852b2/pkg/engine/funcs.go#L158
+ */}}
+{{- if or (and (eq ($json | len) 1) (hasKey $json "Error")) (eq ($json | len) 0) -}}
+{{- $config = printf "%s\n%s" $config "disable_mlock = true" -}}
+{{- else -}}
+{{- if not (hasKey $json "disable_mlock") -}}
+{{- $_ := set $json "disable_mlock" true -}}
+{{- end -}}
+{{- $config = $json | mustToJson -}}
+{{- end -}}
+{{- else }}
+{{- fail "structured server config is not supported, value must be a string"}}
+{{- end }}
+{{- $config | nindent 4 | trim }}
+{{- end -}}
+{{- end -}}

test/unit/server-configmap.bats

@@ -4,28 +4,29 @@ load _helpers
 
 @test "server/ConfigMap: enabled by default" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local actual
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       . | tee /dev/stderr |
       yq 'length > 0' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
-  local actual=$(helm template \
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'server.ha.enabled=true' \
       . | tee /dev/stderr |
       yq 'length > 0' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
-  local actual=$(helm template \
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'server.ha.enabled=true' \
       --set 'server.ha.raft.enabled=true' \
       . | tee /dev/stderr |
       yq 'length > 0' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
-  local actual=$(helm template \
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'server.standalone.enabled=true' \
       . | tee /dev/stderr |
@@ -35,7 +36,8 @@ load _helpers
 
 @test "server/ConfigMap: raft config disabled by default" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local actual
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'server.ha.enabled=true' \
       . | tee /dev/stderr |
@@ -45,7 +47,8 @@ load _helpers
 
 @test "server/ConfigMap: raft config can be enabled" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local actual
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'server.ha.enabled=true' \
       --set 'server.ha.raft.enabled=true' \
@@ -57,7 +60,8 @@ load _helpers
 
 @test "server/ConfigMap: disabled by server.dev.enabled true" {
   cd `chart_dir`
-  local actual=$( (helm template \
+  local actual
+  actual=$( (helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'server.dev.enabled=true' \
       . || echo "---") | tee /dev/stderr |
@@ -67,7 +71,8 @@ load _helpers
 
 @test "server/ConfigMap: disable with global.enabled" {
   cd `chart_dir`
-  local actual=$( (helm template \
+  local actual
+  actual=$( (helm template \
       --show-only templates/server-config-configmap.yaml  \
       --set 'global.enabled=false' \
       . || echo "---") | tee /dev/stderr |
@@ -77,13 +82,14 @@ load _helpers
 
 @test "server/ConfigMap: namespace" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local actual
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --namespace foo \
       . | tee /dev/stderr |
       yq -r '.metadata.namespace' | tee /dev/stderr)
   [ "${actual}" = "foo" ]
-  local actual=$(helm template \
+  actual=$(helm template \
       --show-only templates/server-config-configmap.yaml \
       --set 'global.namespace=bar' \
       --namespace foo \
@@ -92,42 +98,96 @@ load _helpers
   [ "${actual}" = "bar" ]
 }
 
-@test "server/ConfigMap: standalone extraConfig is set" {
+@test "server/ConfigMap: standalone extraConfig is set as JSON" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local data
+  data=$(helm template \
       --show-only templates/server-config-configmap.yaml  \
       --set 'server.standalone.enabled=true' \
-      --set 'server.standalone.config="{\"hello\": \"world\"}"' \
+      --set 'server.standalone.config=\{\"hello\": \"world\"\}' \
       . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | match("world") | length' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
+      yq '.data')
+  [ "$(echo "${data}" | \
+    yq '(. | length) == 1')" = "true" ]
+  [ "$(echo "${data}" | \
+    yq '."extraconfig-from-values.hcl" == "{\"disable_mlock\":true,\"hello\":\"world\"}"')" = 'true' ]
 
-  local actual=$(helm template \
+  data=$(helm template \
+      --show-only templates/server-config-configmap.yaml  \
+      --set 'server.standalone.enabled=true' \
+      --set 'server.standalone.config=\{\"foo\": \"bar\"\}' \
+      . | tee /dev/stderr |
+      yq '.data' | tee /dev/stderr)
+  [ "$(echo "${data}" | \
+    yq '(. | length) == 1')" = "true" ]
+  [ "$(echo "${data}" | \
+    yq '."extraconfig-from-values.hcl" == "{\"disable_mlock\":true,\"foo\":\"bar\"}"')" = 'true' ]
+
+  data=$(helm template \
       --show-only templates/server-config-configmap.yaml  \
       --set 'server.standalone.enabled=true' \
-      --set 'server.standalone.config="{\"foo\": \"bar\"}"' \
+      --set 'server.standalone.config=\{\"disable_mlock\": false\,\"foo\":\"bar\"\}' \
       . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | match("bar") | length' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
+      yq '.data' | tee /dev/stderr)
+  [ "$(echo "${data}" | \
+    yq '(. | length) == 1')" = "true" ]
+  [ "$(echo "${data}" | \
+    yq '."extraconfig-from-values.hcl" == "{\"disable_mlock\":false,\"foo\":\"bar\"}"')" = 'true' ]
 }
 
-@test "server/ConfigMap: ha extraConfig is set" {
+@test "server/ConfigMap: standalone extraConfig is set as not JSON" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local data
+  data=$(helm template \
+      --show-only templates/server-config-configmap.yaml  \
+      --set 'server.standalone.enabled=true' \
+      --set 'server.standalone.config=baz = false' \
+      . | tee /dev/stderr |
+      yq '.data')
+  [ "$(echo "${data}" | \
+    yq '(. | length) == 1')" = "true" ]
+  [ "$(echo "${data}" | \
+    yq '."extraconfig-from-values.hcl" == "baz = false\ndisable_mlock = true"')" = 'true' ]
+}
+
+@test "server/ConfigMap: standalone structured extraConfig fails" {
+  cd "$(chart_dir)"
+  local ret
+  ret=0
+  local output
+  output="$(helm template \
+        --show-only templates/server-config-configmap.yaml  \
+        --set 'server.standalone.enabled=true' \
+        --set 'server.standalone.config.key1=value1' \
+        . 2>&1)" || ret=$?
+  [ "${ret}" -ne 0 ]
+  echo "${output}" | grep -q "structured server config is not supported, value must be a string"
+}
+
+@test "server/ConfigMap: ha extraConfig is set as JSON" {
+  cd `chart_dir`
+  local data
+  data=$(helm template \
       --show-only templates/server-config-configmap.yaml  \
       --set 'server.ha.enabled=true' \
-      --set 'server.ha.config="{\"hello\": \"world\"}"' \
+      --set 'server.ha.config=\{\"hello\": \"ha-world\"\}' \
       . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | match("world") | length' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
+      yq '.data' | tee /dev/stderr)
+  [ "$(echo "${data}" | \
+    yq '(. | length) == 1')" = "true" ]
+  [ "$(echo "${data}" | \
+    yq '."extraconfig-from-values.hcl" == "{\"disable_mlock\":true,\"hello\":\"ha-world\"}"')" = 'true' ]
 
-  local actual=$(helm template \
+ data=$(helm template \
       --show-only templates/server-config-configmap.yaml  \
       --set 'server.ha.enabled=true' \
-      --set 'server.ha.config="{\"foo\": \"bar\"}"' \
+      --set 'server.ha.config=\{\"foo\": \"bar\"\,\"disable_mlock\":false\}' \
       . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | match("bar") | length' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
+      yq '.data' | tee /dev/stderr)
+  [ "$(echo "${data}" | \
+    yq '(. | length) == 1')" = "true" ]
+  [ "$(echo "${data}" | \
+    yq '."extraconfig-from-values.hcl" == "{\"disable_mlock\":false,\"foo\":\"bar\"}"')" = 'true' ]
 }
 
 @test "server/ConfigMap: disabled by injector.externalVaultAddr" {

values.schema.json

@@ -664,7 +664,7 @@
                         }
                     }
                 },
-                "configAnnotation": {
+                "includeConfigAnnotation": {
                     "type": "boolean"
                 },
                 "dataStorage": {

values.yaml

@@ -673,7 +673,7 @@ server:
   # This can be used together with an OnDelete deployment strategy to help
   # identify which pods still need to be deleted during a deployment to pick up
   # any configuration changes.
-  configAnnotation: false
+  includeConfigAnnotation: false
 
   # Enables a headless service to be used by the Vault Statefulset
   service:
@@ -829,13 +829,13 @@ server:
     # config is a raw string of default configuration when using a Stateful
     # deployment. Default is to use a PersistentVolumeClaim mounted at /vault/data
     # and store data there. This is only used when using a Replica count of 1, and
-    # using a stateful set. This should be HCL.
+    # using a stateful set. Supported formats are HCL and JSON.
 
     # Note: Configuration files are stored in ConfigMaps so sensitive data
     # such as passwords should be either mounted through extraSecretEnvironmentVars
-    # or through a Kube secret.  For more information see:
+    # or through a Kube secret. For more information see:
     # https://developer.hashicorp.com/vault/docs/platform/k8s/helm/run#protecting-sensitive-vault-configurations
-    config: |
+    config: |-
       ui = true
 
       listener "tcp" {
@@ -901,6 +901,7 @@ server:
       # such as passwords should be either mounted through extraSecretEnvironmentVars
       # or through a Kube secret.  For more information see:
       # https://developer.hashicorp.com/vault/docs/platform/k8s/helm/run#protecting-sensitive-vault-configurations
+      # Supported formats are HCL and JSON.
       config: |
         ui = true
 
@@ -922,11 +923,11 @@ server:
 
     # config is a raw string of default configuration when using a Stateful
     # deployment. Default is to use a Consul for its HA storage backend.
-    # This should be HCL.
+    # Supported formats are HCL and JSON.
 
     # Note: Configuration files are stored in ConfigMaps so sensitive data
     # such as passwords should be either mounted through extraSecretEnvironmentVars
-    # or through a Kube secret.  For more information see:
+    # or through a Kube secret. For more information see:
     # https://developer.hashicorp.com/vault/docs/platform/k8s/helm/run#protecting-sensitive-vault-configurations
     config: |
       ui = true