Merge branch 'main' into VAULT-27167/update-log-tests

hashicorp · Aug 8, 2024 · 6df170d · 6df170d
2 parents 1baa3c1 + 191e422
commit 6df170d
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,11 @@
 * Default `vault` version updated to 1.17.3
 * Default `vault-csi-provider` version updated to 1.5.0
 
+Bugs:
+
+* Properly handle JSON formatted server config [GH-1049](https://github.com/hashicorp/vault-helm/pull/1049)
+
+
 ## 0.28.1 (July 11, 2024)
 
 Changes:

diff --git a/templates/_helpers.tpl b/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 -}}
diff --git a/test/unit/server-configmap.bats b/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" {

diff --git a/values.schema.json b/values.schema.json
@@ -664,7 +664,7 @@
  }
  }
  },
- "configAnnotation": {
+ "includeConfigAnnotation": {
  "type": "boolean"
  },
  "dataStorage": {

diff --git a/values.yaml b/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