From 68b68abd24111c757a7566c40949007a10c48dba Mon Sep 17 00:00:00 2001
From: Erik Berg <erikberg@usit.uio.no>
Date: Sat, 25 Jun 2022 14:05:27 +0200
Subject: [PATCH 1/3] fix permissions for /consul/extra-config

On openshift/okd you might not have permissions to create directories
everywhere. But you can introduce mounts.

Here we're just creating insignificant mount-points for the extra-config
to do it's thing, thus eliminating the need for creating the directory,
which the user running the container might not have permissions to do.

Fixes #1306
---
 charts/consul/templates/_helpers.tpl          |  1 -
 charts/consul/templates/client-daemonset.yaml |  4 ++++
 .../consul/templates/server-statefulset.yaml  |  4 ++++
 charts/consul/test/unit/client-daemonset.bats | 21 +++++++++++++++++++
 .../consul/test/unit/server-statefulset.bats  | 21 +++++++++++++++++++
 5 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl
index 56f4fbf128..e72cd14497 100644
--- a/charts/consul/templates/_helpers.tpl
+++ b/charts/consul/templates/_helpers.tpl
@@ -142,7 +142,6 @@ substitution for HOST_IP/POD_IP/HOSTNAME. Useful for dogstats telemetry. The out
 is passed to consul as a -config-file param on command line.
 */}}
 {{- define "consul.extraconfig" -}}
-              mkdir -p /consul/extra-config
               cp /consul/config/extra-from-values.json /consul/extra-config/extra-from-values.json
               [ -n "${HOST_IP}" ] && sed -Ei "s|HOST_IP|${HOST_IP?}|g" /consul/extra-config/extra-from-values.json
               [ -n "${POD_IP}" ] && sed -Ei "s|POD_IP|${POD_IP?}|g" /consul/extra-config/extra-from-values.json
diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml
index 33cc400a2f..19be330efb 100644
--- a/charts/consul/templates/client-daemonset.yaml
+++ b/charts/consul/templates/client-daemonset.yaml
@@ -126,6 +126,8 @@ spec:
         - name: config
           configMap:
             name: {{ template "consul.fullname" . }}-client-config
+        - name: extra-config
+          emptyDir: {}
         - name: consul-data
           emptyDir:
             medium: "Memory"
@@ -359,6 +361,8 @@ spec:
               mountPath: /consul/data
             - name: config
               mountPath: /consul/config
+            - name: extra-config
+              mountPath: /consul/extra-config
             - mountPath: /consul/login
               name: consul-data
               readOnly: true
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index 56a1904cd9..aa77e474b5 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -130,6 +130,8 @@ spec:
         - name: config
           configMap:
             name: {{ template "consul.fullname" . }}-server-config
+        - name: extra-config
+          emptyDir: {}
         {{- if (and .Values.global.tls.enabled (not .Values.global.secretsBackend.vault.enabled)) }}
         - name: consul-ca-cert
           secret:
@@ -303,6 +305,8 @@ spec:
               mountPath: /consul/data
             - name: config
               mountPath: /consul/config
+            - name: extra-config
+              mountPath: /consul/extra-config
             {{- if (and .Values.global.tls.enabled (not .Values.global.secretsBackend.vault.enabled)) }}
             - name: consul-ca-cert
               mountPath: /consul/tls/ca/
diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats
index 56743ab742..d7f6b8a7de 100755
--- a/charts/consul/test/unit/client-daemonset.bats
+++ b/charts/consul/test/unit/client-daemonset.bats
@@ -238,6 +238,27 @@ load _helpers
   [ "${actual}" = "bar" ]
 }
 
+#--------------------------------------------------------------------
+# extra-config
+
+@test "client/DaemonSet: has extra-config volume" {
+  cd `chart_dir`
+
+  # check that the extra-config volume is defined
+  local volume_name=$(helm template \
+      -s templates/client-daemonset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.volumes[] | select(.name == "extra-config") | .name' | tee /dev/stderr)
+  [ "${volume_name}" = "extra-config" ]
+
+  # check that the consul container mounts the volume at /consul/extra-config
+  local mount_path=$(helm template \
+      -s templates/client-daemonset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[] | select(.name == "consul") | .volumeMounts[] | select(.name == "extra-config") | .mountPath' | tee /dev/stderr)
+  [ "${mount_path}" = "/consul/extra-config" ]
+}
+
 #--------------------------------------------------------------------
 # extraVolumes
 
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index fbac54eca9..0a2c79bc73 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -348,6 +348,27 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
 }
 
+#--------------------------------------------------------------------
+# extra-config
+
+@test "server/StatefulSet: has extra-config volume" {
+  cd `chart_dir`
+
+  # check that the extra-config volume is defined
+  local volume_name=$(helm template \
+      -s templates/server-statefulset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.volumes[] | select(.name == "extra-config") | .name' | tee /dev/stderr)
+  [ "${volume_name}" = "extra-config" ]
+
+  # check that the consul container mounts the volume at /consul/extra-config
+  local mount_path=$(helm template \
+      -s templates/server-statefulset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[] | select(.name == "consul") | .volumeMounts[] | select(.name == "extra-config") | .mountPath' | tee /dev/stderr)
+  [ "${mount_path}" = "/consul/extra-config" ]
+}
+
 #--------------------------------------------------------------------
 # extraVolumes
 

From 22fc7ea0fa79c6a25a1e5d48547a5ed3a72507d0 Mon Sep 17 00:00:00 2001
From: Erik Berg <erikberg@usit.uio.no>
Date: Mon, 4 Jul 2022 11:02:35 +0200
Subject: [PATCH 2/3] fix serverCert tests

Assuming that consul-server-cert will always be the 3rd volume in
a list of volumes won't work in the long run. Use `select(.name ...`
to find it.
---
 charts/consul/test/unit/server-statefulset.bats | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index 0a2c79bc73..fe9f25aef4 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -207,7 +207,7 @@ load _helpers
         . | tee /dev/stderr )
 
     local actual=$(echo "$object" |
-        yq -r '.spec.template.spec.volumes[2].secret.secretName' | tee /dev/stderr)
+        yq -r '.spec.template.spec.volumes[] | select(.name == "consul-server-cert") | .secret.secretName' | tee /dev/stderr)
     [ "${actual}" = "release-name-consul-server-cert" ]
 }
 
@@ -221,7 +221,7 @@ load _helpers
         . | tee /dev/stderr )
 
     local actual=$(echo "$object" |
-        yq -r '.spec.template.spec.volumes[2].secret.secretName' | tee /dev/stderr)
+        yq -r '.spec.template.spec.volumes[] | select(.name == "consul-server-cert") | .secret.secretName' | tee /dev/stderr)
     [ "${actual}" = "server-cert" ]
 }
 

From 193f40a3a828581b58fc9fc32e3a53700d59292b Mon Sep 17 00:00:00 2001
From: Erik Berg <erikberg@usit.uio.no>
Date: Mon, 4 Jul 2022 11:10:25 +0200
Subject: [PATCH 3/3] fix global.acls.manageSystemACLs tests

Assuming that volumes will always be at a fixed positions in a list
won't work in the long run. Use `select(.name ...` to find
them.

Removing redundant queries/checks, and also change the names of
local variables so some tests read easier.
---
 charts/consul/test/unit/client-daemonset.bats | 48 ++++++++-----------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats
index d7f6b8a7de..00369be52e 100755
--- a/charts/consul/test/unit/client-daemonset.bats
+++ b/charts/consul/test/unit/client-daemonset.bats
@@ -1093,29 +1093,22 @@ load _helpers
 
 @test "client/DaemonSet: aclconfig volume is created when global.acls.manageSystemACLs=true" {
   cd `chart_dir`
-  local actual=$(helm template \
+  local volume_name=$(helm template \
       -s templates/client-daemonset.yaml  \
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
-      yq '.spec.template.spec.volumes[3].name == "aclconfig"' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
+      yq -r '.spec.template.spec.volumes[] | select(.name == "aclconfig") | .name' | tee /dev/stderr)
+  [ "${volume_name}" = "aclconfig" ]
 }
 
 @test "client/DaemonSet: aclconfig volumeMount is created when global.acls.manageSystemACLs=true" {
   cd `chart_dir`
-  local object=$(helm template \
+  local mount_path=$(helm template \
       -s templates/client-daemonset.yaml  \
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].volumeMounts[3]' | tee /dev/stderr)
-
-  local actual=$(echo $object |
-      yq -r '.name' | tee /dev/stderr)
-  [ "${actual}" = "aclconfig" ]
-
-  local actual=$(echo $object |
-      yq -r '.mountPath' | tee /dev/stderr)
-  [ "${actual}" = "/consul/aclconfig" ]
+      yq -r '.spec.template.spec.containers[] | select(.name == "consul") | .volumeMounts[] | select(.name == "aclconfig") | .mountPath' | tee /dev/stderr)
+  [ "${mount_path}" = "/consul/aclconfig" ]
 }
 
 @test "client/DaemonSet: command includes aclconfig dir when global.acls.manageSystemACLs=true" {
@@ -1253,37 +1246,34 @@ local actual=$(echo $object |
 
 @test "client/DaemonSet: Adds consul login volume when ACLs are enabled" {
   cd `chart_dir`
-  local object=$(helm template \
+  local volume=$(helm template \
       -s templates/client-daemonset.yaml  \
       --set 'global.acls.manageSystemACLs=true' \
-      . | yq '.spec.template.spec.volumes[2]' | tee /dev/stderr)
-  local actual=$(echo $object |
+      . | yq '.spec.template.spec.volumes[] | select(.name == "consul-data")' | tee /dev/stderr)
+
+  local volume_name=$(echo $volume |
       yq -r '.name' | tee /dev/stderr)
-  [ "${actual}" = "consul-data" ]
+  [ "${volume_name}" = "consul-data" ]
 
-  local actual=$(echo $object |
+  local volume_emptydir_medium=$(echo $volume |
       yq -r '.emptyDir.medium' | tee /dev/stderr)
-  [ "${actual}" = "Memory" ]
+  [ "${volume_emptydir_medium}" = "Memory" ]
 }
 
 @test "client/DaemonSet: Adds consul login volumeMount to client container when ACLs are enabled" {
   cd `chart_dir`
-  local object=$(helm template \
+  local volume_mount=$(helm template \
       -s templates/client-daemonset.yaml  \
       --set 'global.acls.manageSystemACLs=true' \
-      . | yq '.spec.template.spec.containers[0].volumeMounts[2]' | tee /dev/stderr)
-
-  local actual=$(echo $object |
-      yq -r '.name' | tee /dev/stderr)
-  [ "${actual}" = "consul-data" ]
+      . | yq '.spec.template.spec.containers[] | select(.name == "consul") | .volumeMounts[] | select(.name == "consul-data")' | tee /dev/stderr)
 
-  local actual=$(echo $object |
+  local volume_mount_path=$(echo $volume_mount |
       yq -r '.mountPath' | tee /dev/stderr)
-  [ "${actual}" = "/consul/login" ]
+  [ "${volume_mount_path}" = "/consul/login" ]
 
-  local actual=$(echo $object |
+  local volume_mount_ro=$(echo $volume_mount |
       yq -r '.readOnly' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
+  [ "${volume_mount_ro}" = "true" ]
 }
 
 @test "client/DaemonSet: Adds consul login volumeMount to acl-init init container when ACLs are enabled" {