From ec67b5dd45c60346f709630e0d9acbd6f538ac69 Mon Sep 17 00:00:00 2001
From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com>
Date: Thu, 8 Apr 2021 11:18:16 -0400
Subject: [PATCH] Add logLevel and logFormat values for Vault (#488)

* Add logLevel and logFormat values for Vault

* Add configurable tests

* Update order of log levels

* Update values.yaml

* Update per review

* Update test/unit/server-statefulset.bats

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

* Update test/unit/server-statefulset.bats

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
---
 templates/server-statefulset.yaml     |   8 ++
 test/unit/server-dev-statefulset.bats |  50 ++++-------
 test/unit/server-ha-statefulset.bats  | 115 +++++++++-----------------
 test/unit/server-statefulset.bats     |  96 ++++++++++++++-------
 values.yaml                           |  11 ++-
 5 files changed, 144 insertions(+), 136 deletions(-)

diff --git a/templates/server-statefulset.yaml b/templates/server-statefulset.yaml
index 62296af5e..69232ede8 100644
--- a/templates/server-statefulset.yaml
+++ b/templates/server-statefulset.yaml
@@ -117,6 +117,14 @@ spec:
             {{- end }}
             - name: HOME
               value: "/home/vault"
+            {{- if .Values.server.logLevel }}
+            - name: VAULT_LOG_LEVEL
+              value: "{{ .Values.server.logLevel }}"
+            {{- end }}
+            {{- if .Values.server.logFormat }}
+            - name: VAULT_LOG_FORMAT
+              value: "{{ .Values.server.logFormat }}"
+            {{- end }}
             {{ template "vault.envs" . }}
             {{- include "vault.extraEnvironmentVars" .Values.server | nindent 12 }}
             {{- include "vault.extraSecretEnvironmentVars" .Values.server | nindent 12 }}
diff --git a/test/unit/server-dev-statefulset.bats b/test/unit/server-dev-statefulset.bats
index 4e47fea69..3c5f9d8fb 100755
--- a/test/unit/server-dev-statefulset.bats
+++ b/test/unit/server-dev-statefulset.bats
@@ -246,13 +246,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-      yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_DEV_ROOT_TOKEN_ID" ]
-
-  local actual=$(echo $object |
-      yq -r '.[11].value' | tee /dev/stderr)
-  [ "${actual}" = "root" ]
+  local name=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_DEV_ROOT_TOKEN_ID")) | .[] .value' | tee /dev/stderr)
+  [ "${name}" = "root" ]
 }
 
 @test "server/dev-StatefulSet: set custom devRootToken" {
@@ -264,13 +260,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-      yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_DEV_ROOT_TOKEN_ID" ]
-
-  local actual=$(echo $object |
-      yq -r '.[11].value' | tee /dev/stderr)
-  [ "${actual}" = "customtoken" ]
+  local name=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_DEV_ROOT_TOKEN_ID")) | .[] .value' | tee /dev/stderr)
+  [ "${name}" = "customtoken" ]
 }
 
 #--------------------------------------------------------------------
@@ -340,27 +332,21 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-      yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "ENV_FOO_0" ]
-  local actual=$(echo $object |
-      yq -r '.[11].valueFrom.secretKeyRef.name' | tee /dev/stderr)
-  [ "${actual}" = "secret_name_0" ]
-  local actual=$(echo $object |
-      yq -r '.[11].valueFrom.secretKeyRef.key' | tee /dev/stderr)
-  [ "${actual}" = "secret_key_0" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_0")) | .[] .valueFrom.secretKeyRef.name' | tee /dev/stderr)
+  [ "${value}" = "secret_name_0" ]
 
-  local actual=$(echo $object |
-      yq -r '.[12].name' | tee /dev/stderr)
-  [ "${actual}" = "ENV_FOO_1" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_0")) | .[] .valueFrom.secretKeyRef.key' | tee /dev/stderr)
+  [ "${value}" = "secret_key_0" ]
 
-  local actual=$(echo $object |
-      yq -r '.[12].valueFrom.secretKeyRef.name' | tee /dev/stderr)
-  [ "${actual}" = "secret_name_1" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_1")) | .[] .valueFrom.secretKeyRef.name' | tee /dev/stderr)
+  [ "${value}" = "secret_name_1" ]
 
-  local actual=$(echo $object |
-      yq -r '.[12].valueFrom.secretKeyRef.key' | tee /dev/stderr)
-  [ "${actual}" = "secret_key_1" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_1")) | .[] .valueFrom.secretKeyRef.key' | tee /dev/stderr)
+  [ "${value}" = "secret_key_1" ]
 }
 
 #--------------------------------------------------------------------
diff --git a/test/unit/server-ha-statefulset.bats b/test/unit/server-ha-statefulset.bats
index 6df4dfe30..43e1acef2 100755
--- a/test/unit/server-ha-statefulset.bats
+++ b/test/unit/server-ha-statefulset.bats
@@ -70,14 +70,11 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[4].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_ADDR" ]
-
-  local actual=$(echo $object |
-     yq -r '.[4].value' | tee /dev/stderr)
-  [ "${actual}" = "http://127.0.0.1:8200" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_ADDR")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "http://127.0.0.1:8200" ]
 }
+
 @test "server/ha-StatefulSet: tls enabled" {
   cd `chart_dir`
   local object=$(helm template \
@@ -86,13 +83,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[4].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_ADDR" ]
-
-  local actual=$(echo $object |
-     yq -r '.[4].value' | tee /dev/stderr)
-  [ "${actual}" = "https://127.0.0.1:8200" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_ADDR")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "https://127.0.0.1:8200" ]
 }
 
 #--------------------------------------------------------------------
@@ -348,21 +341,13 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "FOO" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="FOO")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "bar" ]
 
-  local actual=$(echo $object |
-      yq -r '.[11].value' | tee /dev/stderr)
-  [ "${actual}" = "bar" ]
-
-  local actual=$(echo $object |
-      yq -r '.[12].name' | tee /dev/stderr)
-  [ "${actual}" = "FOOBAR" ]
-
-  local actual=$(echo $object |
-      yq -r '.[12].value' | tee /dev/stderr)
-  [ "${actual}" = "foobar" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="FOOBAR")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "foobar" ]
 }
 
 #--------------------------------------------------------------------
@@ -382,25 +367,21 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-      yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "ENV_FOO_0" ]
-  local actual=$(echo $object |
-      yq -r '.[11].valueFrom.secretKeyRef.name' | tee /dev/stderr)
-  [ "${actual}" = "secret_name_0" ]
-  local actual=$(echo $object |
-      yq -r '.[11].valueFrom.secretKeyRef.key' | tee /dev/stderr)
-  [ "${actual}" = "secret_key_0" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_0")) | .[] .valueFrom.secretKeyRef.name' | tee /dev/stderr)
+  [ "${value}" = "secret_name_0" ]
 
-  local actual=$(echo $object |
-      yq -r '.[12].name' | tee /dev/stderr)
-  [ "${actual}" = "ENV_FOO_1" ]
-  local actual=$(echo $object |
-      yq -r '.[12].valueFrom.secretKeyRef.name' | tee /dev/stderr)
-  [ "${actual}" = "secret_name_1" ]
-  local actual=$(echo $object |
-      yq -r '.[12].valueFrom.secretKeyRef.key' | tee /dev/stderr)
-  [ "${actual}" = "secret_key_1" ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_0")) | .[] .valueFrom.secretKeyRef.key' | tee /dev/stderr)
+  [ "${value}" = "secret_key_0" ]
+
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_1")) | .[] .valueFrom.secretKeyRef.name' | tee /dev/stderr)
+  [ "${value}" = "secret_name_1" ]
+
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="ENV_FOO_1")) | .[] .valueFrom.secretKeyRef.key' | tee /dev/stderr)
+  [ "${value}" = "secret_key_1" ]
 }
 
 #--------------------------------------------------------------------
@@ -414,16 +395,12 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[5].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_API_ADDR" ]
-
-  local actual=$(echo $object |
-     yq -r '.[5].value' | tee /dev/stderr)
-  [ "${actual}" = 'http://$(POD_IP):8200' ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_API_ADDR")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = 'http://$(POD_IP):8200' ]
 }
 
-@test "server/ha-StatefulSet: api addr can be overriden" {
+@test "server/ha-StatefulSet: api addr is configurable" {
   cd `chart_dir`
   local object=$(helm template \
       --show-only templates/server-statefulset.yaml  \
@@ -432,13 +409,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[5].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_API_ADDR" ]
-
-  local actual=$(echo $object |
-     yq -r '.[5].value' | tee /dev/stderr)
-  [ "${actual}" = 'https://example.com:8200' ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_API_ADDR")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "https://example.com:8200" ]
 }
 
 #--------------------------------------------------------------------
@@ -453,13 +426,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[9].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_CLUSTER_ADDR" ]
-
-  local actual=$(echo $object |
-     yq -r '.[9].value' | tee /dev/stderr)
-  [ "${actual}" = 'https://$(HOSTNAME).RELEASE-NAME-vault-internal:8201' ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_CLUSTER_ADDR")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = 'https://$(HOSTNAME).RELEASE-NAME-vault-internal:8201' ]
 }
 
 #--------------------------------------------------------------------
@@ -475,13 +444,9 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[10].name' | tee /dev/stderr)
-  [ "${actual}" = "VAULT_RAFT_NODE_ID" ]
-
-  local actual=$(echo $object |
-     yq -r '.[10].valueFrom.fieldRef.fieldPath' | tee /dev/stderr)
-  [ "${actual}" = 'metadata.name' ]
+  local value=$(echo $object |
+      yq -r 'map(select(.name=="VAULT_RAFT_NODE_ID")) | .[] .valueFrom.fieldRef.fieldPath' | tee /dev/stderr)
+  [ "${value}" = "metadata.name" ]
 }
 
 #--------------------------------------------------------------------
diff --git a/test/unit/server-statefulset.bats b/test/unit/server-statefulset.bats
index b34636262..0c674d6c9 100755
--- a/test/unit/server-statefulset.bats
+++ b/test/unit/server-statefulset.bats
@@ -448,6 +448,62 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
+#--------------------------------------------------------------------
+# log level
+
+@test "server/standalone-StatefulSet: default log level to empty" {
+  cd `chart_dir`
+  local objects=$(helm template \
+      --show-only templates/server-statefulset.yaml  \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
+
+  local value=$(echo $objects |
+      yq -r 'map(select(.name=="VAULT_LOG_LEVEL")) | .[] .name' | tee /dev/stderr)
+  [ "${value}" = "" ]
+}
+
+@test "server/standalone-StatefulSet: log level can be changed" {
+  cd `chart_dir`
+  local objects=$(helm template \
+      --show-only templates/server-statefulset.yaml  \
+      --set='server.logLevel=debug' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
+
+  local value=$(echo $objects |
+      yq -r 'map(select(.name=="VAULT_LOG_LEVEL")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "debug" ]
+}
+
+#--------------------------------------------------------------------
+# log format
+
+@test "server/standalone-StatefulSet: default log format to empty" {
+  cd `chart_dir`
+  local objects=$(helm template \
+      --show-only templates/server-statefulset.yaml  \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
+
+  local value=$(echo $objects |
+      yq -r 'map(select(.name=="VAULT_LOG_FORMAT")) | .[] .name' | tee /dev/stderr)
+  [ "${value}" = "" ]
+}
+
+@test "server/standalone-StatefulSet: can set log format" {
+  cd `chart_dir`
+  local objects=$(helm template \
+      --show-only templates/server-statefulset.yaml  \
+      --set='server.logFormat=json' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
+
+  local value=$(echo $objects |
+      yq -r 'map(select(.name=="VAULT_LOG_FORMAT")) | .[] .value' | tee /dev/stderr)
+  [ "${value}" = "json" ]
+}
+
 #--------------------------------------------------------------------
 # extraEnvironmentVars
 
@@ -461,21 +517,13 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "FOO" ]
+  local name=$(echo $object |
+      yq -r 'map(select(.name=="FOO")) | .[] .value' | tee /dev/stderr)
+  [ "${name}" = "bar" ]
 
-  local actual=$(echo $object |
-      yq -r '.[11].value' | tee /dev/stderr)
-  [ "${actual}" = "bar" ]
-
-  local actual=$(echo $object |
-      yq -r '.[12].name' | tee /dev/stderr)
-  [ "${actual}" = "FOOBAR" ]
-
-  local actual=$(echo $object |
-      yq -r '.[12].value' | tee /dev/stderr)
-  [ "${actual}" = "foobar" ]
+  local name=$(echo $object |
+      yq -r 'map(select(.name=="FOOBAR")) | .[] .value' | tee /dev/stderr)
+  [ "${name}" = "foobar" ]
 
   local object=$(helm template \
       --show-only templates/server-statefulset.yaml  \
@@ -484,21 +532,13 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
-  local actual=$(echo $object |
-     yq -r '.[11].name' | tee /dev/stderr)
-  [ "${actual}" = "FOO" ]
+  local name=$(echo $object |
+      yq -r 'map(select(.name=="FOO")) | .[] .value' | tee /dev/stderr)
+  [ "${name}" = "bar" ]
 
-  local actual=$(echo $object |
-      yq -r '.[11].value' | tee /dev/stderr)
-  [ "${actual}" = "bar" ]
-
-  local actual=$(echo $object |
-      yq -r '.[12].name' | tee /dev/stderr)
-  [ "${actual}" = "FOOBAR" ]
-
-  local actual=$(echo $object |
-      yq -r '.[12].value' | tee /dev/stderr)
-  [ "${actual}" = "foobar" ]
+  local name=$(echo $object |
+      yq -r 'map(select(.name=="FOOBAR")) | .[] .value' | tee /dev/stderr)
+  [ "${name}" = "foobar" ]
 }
 
 #--------------------------------------------------------------------
diff --git a/values.yaml b/values.yaml
index c08e9acf2..89d713cf8 100644
--- a/values.yaml
+++ b/values.yaml
@@ -62,7 +62,8 @@ injector:
   # Mount Path of the Vault Kubernetes Auth Method.
   authPath: "auth/kubernetes"
 
-  # Configures the log verbosity of the injector. Supported log levels: Trace, Debug, Error, Warn, Info
+  # Configures the log verbosity of the injector.
+  # Supported log levels include: trace, debug, info, warn, error
   logLevel: "info"
 
   # Configures the log format of the injector. Supported log formats: "standard", "json".
@@ -195,6 +196,14 @@ server:
   # See https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#update-strategies
   updateStrategyType: "OnDelete"
 
+  # Configure the logging verbosity for the Vault server.
+  # Supported log levels include: trace, debug, info, warn, error
+  logLevel: ""
+
+  # Configure the logging format for the Vault server.
+  # Supported log formats include: standard, json
+  logFormat: ""
+
   resources: {}
   # resources:
   #   requests: