From 266a4c314b506dbdb931e46424f56709fecc4e53 Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Wed, 30 Mar 2022 13:02:28 -0600
Subject: [PATCH 1/7] refactor: move server TLS config to configmap

---
 .../templates/server-config-configmap.yaml    |  30 +++++
 .../consul/templates/server-statefulset.yaml  |  23 ----
 .../test/unit/server-config-configmap.bats    | 112 +++++++++++++++++-
 .../consul/test/unit/server-statefulset.bats  |  72 +----------
 4 files changed, 142 insertions(+), 95 deletions(-)

diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml
index 3ac7a29fd4..71852b2dc5 100644
--- a/charts/consul/templates/server-config-configmap.yaml
+++ b/charts/consul/templates/server-config-configmap.yaml
@@ -62,6 +62,36 @@ data:
       }
     }
   {{- end }}
+  {{- if .Values.global.tls.enabled }}
+  tls-config.json: |-
+    {
+      {{- if .Values.global.secretsBackend.vault.enabled }}
+      "ca_file": "/vault/secrets/serverca.crt",
+      "cert_file": "/vault/secrets/servercert.crt",
+      "key_file": "/vault/secrets/servercert.key",
+      {{- else }}
+      "ca_file": "/consul/tls/ca/tls.crt",
+      "cert_file": "/consul/tls/server/tls.crt",
+      "key_file": "/consul/tls/server/tls.key",
+      {{- end }}
+      {{- if .Values.global.tls.enableAutoEncrypt }}
+      "auto_encrypt": {
+          "allow_tls": true
+      },
+      {{- end }}
+      {{- if .Values.global.tls.verify }}
+      "verify_incoming_rpc": true,
+      "verify_outgoing": true,
+      "verify_server_hostname": true,
+      {{- end }}
+      "ports": {
+         {{- if .Values.global.tls.httpsOnly }}
+         "http": -1,
+         {{- end }}
+         "https": 8501
+       }
+    }
+  {{- end }}
   {{- if (and .Values.ui.enabled (or (eq "true" (.Values.ui.metrics.enabled | toString) ) (and .Values.global.metrics.enabled (eq "-" (.Values.ui.metrics.enabled | toString))))) }}
   ui-config.json: |-
     {
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index d83e9d5c50..93586819d3 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -278,29 +278,6 @@ spec:
                 -advertise="${ADVERTISE_IP}" \
                 -bind=0.0.0.0 \
                 -bootstrap-expect={{ if .Values.server.bootstrapExpect }}{{ .Values.server.bootstrapExpect }}{{ else }}{{ .Values.server.replicas }}{{ end }} \
-                {{- if .Values.global.tls.enabled }}
-                {{- if .Values.global.secretsBackend.vault.enabled }}
-                -hcl='ca_file = "/vault/secrets/serverca.crt"' \
-                -hcl='cert_file = "/vault/secrets/servercert.crt"' \
-                -hcl='key_file = "/vault/secrets/servercert.key"' \
-                {{- else }}
-                -hcl='ca_file = "/consul/tls/ca/tls.crt"' \
-                -hcl='cert_file = "/consul/tls/server/tls.crt"' \
-                -hcl='key_file = "/consul/tls/server/tls.key"' \
-                {{- end }}
-                {{- if .Values.global.tls.enableAutoEncrypt }}
-                -hcl='auto_encrypt = {allow_tls = true}' \
-                {{- end }}
-                {{- if .Values.global.tls.verify }}
-                -hcl='verify_incoming_rpc = true' \
-                -hcl='verify_outgoing = true' \
-                -hcl='verify_server_hostname = true' \
-                {{- end }}
-                -hcl='ports { https = 8501 }' \
-                {{- if .Values.global.tls.httpsOnly }}
-                -hcl='ports { http = -1 }' \
-                {{- end }}
-                {{- end }}
                 -client=0.0.0.0 \
                 -config-dir=/consul/config \
                 -datacenter={{ .Values.global.datacenter }} \
diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats
index 6fddfe7be2..5a53a46902 100755
--- a/charts/consul/test/unit/server-config-configmap.bats
+++ b/charts/consul/test/unit/server-config-configmap.bats
@@ -506,7 +506,7 @@ load _helpers
 @test "server/ConfigMap: adds empty federation config when global.federation.enabled is true" {
   cd `chart_dir`
   local actual=$(helm template \
-      -s templates/server-config-configmap.yaml  \
+      -s templates/server-config-configmap.yaml \
       --set 'global.federation.enabled=true' \
       --set 'global.tls.enabled=true' \
       --set 'meshGateway.enabled=true' \
@@ -519,7 +519,7 @@ load _helpers
 @test "server/ConfigMap: can set primary dc and gateways when global.federation.enabled is true" {
   cd `chart_dir`
   local actual=$(helm template \
-      -s templates/server-config-configmap.yaml  \
+      -s templates/server-config-configmap.yaml \
       --set 'global.federation.enabled=true' \
       --set 'global.federation.primaryDatacenter=dc1' \
       --set 'global.federation.primaryGateways[0]=1.1.1.1:443' \
@@ -530,4 +530,112 @@ load _helpers
       . | tee /dev/stderr |
       yq '.data["federation-config.json"]' | tee /dev/stderr)
   [ "${actual}" = '"{\n  \"primary_datacenter\": \"dc1\",\n  \"primary_gateways\": [\"1.1.1.1:443\",\"2.2.2.2:443\"]\n}"' ]
+}
+
+#--------------------------------------------------------------------
+# TLS
+
+@test "server/ConfigMap: sets correct default configuration when global.tls.enabled" {
+  cd `chart_dir`
+  local config=$(helm template \
+      -s templates/server-config-configmap.yaml \
+      --set 'global.tls.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.data["tls-config.json"]' | tee /dev/stderr)
+
+  local actual
+  actual=$(echo $config | jq -r .ca_file | tee /dev/stderr)
+  [ "${actual}" = "/consul/tls/ca/tls.crt" ]
+
+  actual=$(echo $config | jq -r .cert_file | tee /dev/stderr)
+  [ "${actual}" = "/consul/tls/server/tls.crt" ]
+
+  actual=$(echo $config | jq -r .key_file | tee /dev/stderr)
+  [ "${actual}" = "/consul/tls/server/tls.key" ]
+
+  actual=$(echo $config | jq -r .verify_incoming_rpc | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+
+  actual=$(echo $config | jq -r .verify_outgoing | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+
+  actual=$(echo $config | jq -r .verify_server_hostname | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+
+  actual=$(echo $config | jq -c .ports | tee /dev/stderr)
+  [ "${actual}" = '{"http":-1,"https":8501}' ]
+}
+
+@test "server/ConfigMap: doesn't set verify_* configuration to true when global.tls.enabled and global.tls.verify is false" {
+  cd `chart_dir`
+  local config=$(helm template \
+      -s templates/server-config-configmap.yaml \
+      --set 'global.tls.enabled=true' \
+      --set 'global.tls.verify=false' \
+      . | tee /dev/stderr |
+      yq -r '.data["tls-config.json"]' | tee /dev/stderr)
+
+  local actual
+  actual=$(echo $config | jq -r .verify_incoming_rpc | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+
+  actual=$(echo $config | jq -r .verify_outgoing | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+
+  actual=$(echo $config | jq -r .verify_server_hostname | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ConfigMap: HTTP port is not set in when httpsOnly is false" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'global.tls.enabled=true' \
+      --set 'global.tls.httpsOnly=false' \
+      . | tee /dev/stderr |
+      yq -r '.data["tls-config.json"]' | jq -c .ports | tee /dev/stderr)
+  [ "${actual}" = '{"https":8501}' ]
+}
+
+#--------------------------------------------------------------------
+# global.tls.enableAutoEncrypt
+
+@test "server/ConfigMap: enables auto-encrypt for the servers when global.tls.enableAutoEncrypt is true" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml \
+      --set 'global.tls.enabled=true' \
+      --set 'global.tls.enableAutoEncrypt=true' \
+      . | tee /dev/stderr |
+      yq -r '.data["tls-config.json"]' | jq -c .auto_encrypt | tee /dev/stderr)
+  [ "${actual}" = '{"allow_tls":true}' ]
+}
+
+#--------------------------------------------------------------------
+# TLS + Vault
+
+@test "server/ConfigMap: sets TLS file paths point to vault secrets when Vault is enabled" {
+  cd `chart_dir`
+  local object=$(helm template \
+    -s templates/server-config-configmap.yaml  \
+    --set 'global.tls.enabled=true' \
+    --set 'global.tls.enableAutoEncrypt=true' \
+    --set 'global.datacenter=dc2' \
+    --set 'global.secretsBackend.vault.enabled=true' \
+    --set 'global.secretsBackend.vault.consulClientRole=test' \
+    --set 'global.secretsBackend.vault.consulServerRole=foo' \
+    --set 'global.secretsBackend.vault.consulCARole=test' \
+    --set 'global.tls.caCert.secretName=pki_int/cert/ca' \
+    --set 'server.serverCert.secretName=pki_int/issue/test' \
+    . | tee /dev/stderr |
+    yq -r '.data["tls-config.json"]' | tee /dev/stderr)
+
+  local actual=$(echo $object | jq -r .ca_file | tee /dev/stderr)
+  [ "${actual}" = "/vault/secrets/serverca.crt" ]
+
+  local actual=$(echo $object | jq -r .cert_file | tee /dev/stderr)
+  [ "${actual}" = "/vault/secrets/servercert.crt" ]
+
+  local actual=$(echo $object | jq -r .key_file | tee /dev/stderr)
+  [ "${actual}" = "/vault/secrets/servercert.key" ]
 }
\ No newline at end of file
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index 24b811e19f..6436fd8411 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -769,7 +769,7 @@ load _helpers
 }
 
 #--------------------------------------------------------------------
-# tolerations
+# topologySpreadConstraints
 
 @test "server/StatefulSet: topologySpreadConstraints not set by default" {
   cd `chart_dir`
@@ -994,7 +994,7 @@ load _helpers
   [ "${actual}" != "" ]
 }
 
-@test "server/StatefulSet: server volume present when TLS is enabled" {
+@test "server/StatefulSet: server cert volume present when TLS is enabled" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-statefulset.yaml  \
@@ -1086,17 +1086,6 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "server/StatefulSet: HTTP is disabled in agent when httpsOnly is enabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.tls.enabled=true' \
-      --set 'global.tls.httpsOnly=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("ports { http = -1 }")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 @test "server/StatefulSet: sets Consul environment variables when global.tls.enabled" {
   cd `chart_dir`
   local env=$(helm template \
@@ -1132,45 +1121,6 @@ load _helpers
     [ "${actual}" = "/vault/secrets/serverca.crt" ]
 }
 
-@test "server/StatefulSet: sets verify_* flags to true by default when global.tls.enabled" {
-  cd `chart_dir`
-  local command=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.tls.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ")' | tee /dev/stderr)
-
-  local actual
-  actual=$(echo $command | jq -r '. | contains("verify_incoming_rpc = true")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-
-  actual=$(echo $command | jq -r '. | contains("verify_outgoing = true")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-
-  actual=$(echo $command | jq -r '. | contains("verify_server_hostname = true")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "server/StatefulSet: doesn't set the verify_* flags by default when global.tls.enabled and global.tls.verify is false" {
-  cd `chart_dir`
-  local command=$(helm template \
-      -s templates/server-statefulset.yaml \
-      --set 'global.tls.enabled=true' \
-      --set 'global.tls.verify=false' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ")' | tee /dev/stderr)
-
-  local actual
-  actual=$(echo $command | jq -r '. | contains("verify_incoming_rpc = true")' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-
-  actual=$(echo $command | jq -r '. | contains("verify_outgoing = true")' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-
-  actual=$(echo $command | jq -r '. | contains("verify_server_hostname = true")' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
 @test "server/StatefulSet: can overwrite CA secret with the provided one" {
   cd `chart_dir`
   local ca_cert_volume=$(helm template \
@@ -1365,20 +1315,6 @@ load _helpers
   [ "${actual}" = '[{"name":"ACL_REPLICATION_TOKEN","valueFrom":{"secretKeyRef":{"name":"name","key":"key"}}}]' ]
 }
 
-#--------------------------------------------------------------------
-# global.tls.enableAutoEncrypt
-
-@test "server/StatefulSet: enables auto-encrypt for the servers when global.tls.enableAutoEncrypt is true" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.tls.enabled=true' \
-      --set 'global.tls.enableAutoEncrypt=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("auto_encrypt = {allow_tls = true}")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 #--------------------------------------------------------------------
 # -bootstrap-expect
 
@@ -1859,10 +1795,6 @@ load _helpers
       yq -r '.metadata.annotations["vault.hashicorp.com/agent-inject-template-servercert.key"]' | tee /dev/stderr)"
   local expected=$'{{- with secret \"pki_int/issue/test\" \"common_name=server.dc2.consul\"\n\"alt_names=localhost,RELEASE-NAME-consul-server,*.RELEASE-NAME-consul-server,*.RELEASE-NAME-consul-server.default,RELEASE-NAME-consul-server.default,*.RELEASE-NAME-consul-server.default.svc,RELEASE-NAME-consul-server.default.svc,*.server.dc2.consul\" \"ip_sans=127.0.0.1\" -}}\n{{- .Data.private_key -}}\n{{- end -}}'
   [ "${actual}" = "${expected}" ]
-
-  local actual=$(echo $object |
-    yq -r '.spec.containers[0].command | any(contains("ca_file = \"/vault/secrets/serverca.crt\""))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
 }
 
 @test "server/StatefulSet: tls related volumes not attached when tls is enabled on vault" {

From 74be34879d049eb77aaba40bf6d7106dbd1397c3 Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Wed, 30 Mar 2022 22:26:22 -0600
Subject: [PATCH 2/7] refactor: consolidate all ui config into server configmap

---
 .../templates/server-config-configmap.yaml    | 13 ++-
 .../consul/templates/server-statefulset.yaml  |  6 --
 .../test/unit/server-config-configmap.bats    | 91 +++++++++++++++----
 .../consul/test/unit/server-statefulset.bats  | 34 +------
 4 files changed, 84 insertions(+), 60 deletions(-)

diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml
index 71852b2dc5..54372a71ef 100644
--- a/charts/consul/templates/server-config-configmap.yaml
+++ b/charts/consul/templates/server-config-configmap.yaml
@@ -92,15 +92,22 @@ data:
        }
     }
   {{- end }}
-  {{- if (and .Values.ui.enabled (or (eq "true" (.Values.ui.metrics.enabled | toString) ) (and .Values.global.metrics.enabled (eq "-" (.Values.ui.metrics.enabled | toString))))) }}
+  {{- if .Values.ui.enabled }}
   ui-config.json: |-
     {
       "ui_config": {
-        "enabled": true,
+        {{- if (or (eq "true" (.Values.ui.metrics.enabled | toString) ) (and .Values.global.metrics.enabled (eq "-" (.Values.ui.metrics.enabled | toString)))) }}
         "metrics_provider": "{{ .Values.ui.metrics.provider }}",
         "metrics_proxy": {
           "base_url": "{{ .Values.ui.metrics.baseURL }}"
-        }
+        },
+        {{- end }}
+        {{- if .Values.ui.dashboardURLTemplates.service }}
+        "dashboard_url_templates": {
+          "service": "{{ .Values.ui.dashboardURLTemplates.service }}"
+        },
+        {{- end }}
+        "enabled": true
       }
     }
   {{- end }}
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index 93586819d3..c099880cfa 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -302,12 +302,6 @@ spec:
                 -hcl="acl { tokens { agent = \"${ACL_REPLICATION_TOKEN}\", replication = \"${ACL_REPLICATION_TOKEN}\" } }" \
                 {{- end }}
                 {{- end }}
-                {{- if .Values.ui.enabled }}
-                -ui \
-                {{- if .Values.ui.dashboardURLTemplates.service }}
-                -hcl='ui_config { dashboard_url_templates { service = "{{ .Values.ui.dashboardURLTemplates.service }}" } }' \
-                {{- end }}
-                {{- end }}
                 {{- $serverSerfLANPort  := .Values.server.ports.serflan.port -}}
                 {{- range $index := until (.Values.server.replicas | int) }}
                 -retry-join="${CONSUL_FULLNAME}-server-{{ $index }}.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:{{ $serverSerfLANPort }}" \
diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats
index 5a53a46902..766e4ab140 100755
--- a/charts/consul/test/unit/server-config-configmap.bats
+++ b/charts/consul/test/unit/server-config-configmap.bats
@@ -62,61 +62,84 @@ load _helpers
 }
 
 #--------------------------------------------------------------------
-# global.metrics.enabled & ui.enabled
+# ui.enabled
 
-@test "server/ConfigMap: creates ui config with .ui.enabled=true and .global.metrics.enabled=true" {
+@test "server/ConfigMap: creates ui config with .ui.enabled=true" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
-      --set 'global.metrics.enabled=true' \
       --set 'ui.enabled=true' \
       . | tee /dev/stderr |
-      yq '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config | tee /dev/stderr)
+  [ "${actual}" = '{"enabled":true}' ]
+}
+
+@test "server/ConfigMap: does not create ui config with .ui.enabled=false" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'ui.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.data["ui-config.json"] | length == 0' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 
-@test "server/ConfigMap: creates ui config with .ui.enabled=true and .ui.metrics.enabled=true" {
+@test "server/ConfigMap: adds metrics ui config with .global.metrics.enabled=true and ui.metrics.enabled=-" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
-      --set 'ui.metrics.enabled=true' \
+      --set 'global.metrics.enabled=true' \
       --set 'ui.enabled=true' \
       . | tee /dev/stderr |
-      yq '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config | tee /dev/stderr)
+  [ "${actual}" = '{"metrics_provider":"prometheus","metrics_proxy":{"base_url":"http://prometheus-server"},"enabled":true}' ]
 }
 
-@test "server/ConfigMap: does not create ui config when .ui.enabled=false and .ui.metrics.enabled=true" {
+@test "server/ConfigMap: adds metrics ui config with .global.metrics.enabled=false and .ui.metrics.enabled=true" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
-      --set 'ui.enabled=false' \
-      --set 'ui.metrics.enabled=false' \
+      --set 'ui.metrics.enabled=true' \
+      --set 'ui.enabled=true' \
       . | tee /dev/stderr |
-      yq -r '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config | tee /dev/stderr)
+  [ "${actual}" = '{"metrics_provider":"prometheus","metrics_proxy":{"base_url":"http://prometheus-server"},"enabled":true}' ]
 }
 
-@test "server/ConfigMap: does not create ui config when .ui.enabled=true and .global.metrics.enabled=false" {
+@test "server/ConfigMap: adds metrics ui config with .global.metrics.enabled=true and .ui.metrics.enabled=true" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
+      --set 'global.metrics.enabled=true' \
+      --set 'ui.metrics.enabled=true' \
       --set 'ui.enabled=true' \
-      --set 'global.metrics.enabled=false' \
       . | tee /dev/stderr |
-      yq -r '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config | tee /dev/stderr)
+  [ "${actual}" = '{"metrics_provider":"prometheus","metrics_proxy":{"base_url":"http://prometheus-server"},"enabled":true}' ]
 }
 
-@test "server/ConfigMap: does not create ui config when .ui.enabled=true and .ui.metrics.enabled=false" {
+@test "server/ConfigMap: doesn't add metrics ui config with .global.metrics.enabled=true and .ui.metrics.enabled=false" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
+      --set 'global.metrics.enabled=true' \
+      --set 'ui.metrics.enabled=false' \
       --set 'ui.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config | tee /dev/stderr)
+  [ "${actual}" = '{"enabled":true}' ]
+}
+
+@test "server/ConfigMap: doesn't add metrics ui config with .global.metrics.enabled=false and .ui.metrics.enabled=false" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'global.metrics.enabled=false' \
       --set 'ui.metrics.enabled=false' \
+      --set 'ui.enabled=true' \
       . | tee /dev/stderr |
-      yq -r '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config | tee /dev/stderr)
+  [ "${actual}" = '{"enabled":true}' ]
 }
 
 @test "server/ConfigMap: updates ui config with .ui.metrics.provider" {
@@ -143,6 +166,34 @@ load _helpers
   [ "${actual}" = "http://foo.bar" ]
 }
 
+#--------------------------------------------------------------------
+# ui.dashboardURLTemplates.service
+
+@test "server/ConfigMap: dashboard_url_templates not set by default" {
+  cd `chart_dir`
+
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      . | tee /dev/stderr |
+      yq -r '.data["ui-config.json"]' | jq .dashboard_url_templates | tee /dev/stderr)
+
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ConfigMap: ui.dashboardURLTemplates.service sets the template" {
+  cd `chart_dir`
+
+  local expected='-hcl='\''ui_config { dashboard_url_templates { service = \"http://localhost:3000/d/WkFEBmF7z/services?orgId=1&var-Service={{Service.Name}}\" } }'
+
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'ui.dashboardURLTemplates.service=http://localhost:3000/d/WkFEBmF7z/services?orgId=1&var-Service={{Service.Name}}' \
+      . | tee /dev/stderr |
+      yq -r '.data["ui-config.json"]' | jq -c .ui_config.dashboard_url_templates | tee /dev/stderr)
+
+  [ "${actual}" = '{"service":"http://localhost:3000/d/WkFEBmF7z/services?orgId=1&var-Service={{Service.Name}}"}' ]
+}
+
 #--------------------------------------------------------------------
 # connectInject.centralConfig [DEPRECATED]
 
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index 6436fd8411..889b8881f8 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -723,7 +723,7 @@ load _helpers
       -s templates/server-statefulset.yaml  \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = 2c5397272acdc6fe5b079bf25c846c5a17f474603c794c64e7226ce0690625f7 ]
+  [ "${actual}" = 97314eb896e88e23a6552ca812e1a3205243b1fa2f4d3e6e95e63c893a7c1f1e ]
 }
 
 @test "server/StatefulSet: adds config-checksum annotation when extraConfig is provided" {
@@ -733,7 +733,7 @@ load _helpers
       --set 'server.extraConfig="{\"hello\": \"world\"}"' \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = b0d22cb051216505edc0e61b57f9eacc0d7e15b24719d815842df88f06f1abe0 ]
+  [ "${actual}" = 004fe72210e383398b8e36de2e918d005dcd6b8e162d67ad23aef024a872dda8 ]
 }
 
 @test "server/StatefulSet: adds config-checksum annotation when config is updated" {
@@ -743,7 +743,7 @@ load _helpers
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = 7772975be982e25cc8df101375374e2ba672a55737f8f1580011e0d88d8752a8 ]
+  [ "${actual}" = 54aae134d108d934d0a10b527df62dfbb10074fffb27c196625ebf5da6b8206d ]
 }
 
 #--------------------------------------------------------------------
@@ -2040,31 +2040,3 @@ load _helpers
   local actual="$(echo $object | yq -r '.spec.containers[] | select(.name=="consul").command | any(contains("-config-file=/vault/secrets/replication-token-config.hcl"))' | tee /dev/stderr)"
   [ "${actual}" = "true" ]
 }
-
-#--------------------------------------------------------------------
-# ui.dashboardURLTemplates.service
-
-@test "server/StatefulSet: dashboard_url_templates not set by default" {
-  cd `chart_dir`
-
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      . | tee /dev/stderr |
-      yq -r ".spec.template.spec.containers[0].command | any(contains(\"dashboard_url_templates\"))" | tee /dev/stderr)
-
-  [ "${actual}" = "false" ]
-}
-
-@test "server/StatefulSet: ui.dashboardURLTemplates.service sets the template" {
-  cd `chart_dir`
-
-  local expected='-hcl='\''ui_config { dashboard_url_templates { service = \"http://localhost:3000/d/WkFEBmF7z/services?orgId=1&var-Service={{Service.Name}}\" } }'
-
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'ui.dashboardURLTemplates.service=http://localhost:3000/d/WkFEBmF7z/services?orgId=1&var-Service={{Service.Name}}' \
-      . | tee /dev/stderr |
-      yq -r ".spec.template.spec.containers[0].command | any(contains(\"$expected\"))" | tee /dev/stderr)
-
-  [ "${actual}" = "true" ]
-}

From 51240a2ce9279b2e1e17701b6ae04c17ecf1ef34 Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Thu, 31 Mar 2022 10:45:49 -0600
Subject: [PATCH 3/7] Use single DNS name for server's retry-join instead

We don't need to join individual server instances as the headless service
DNS will resolve to all the server pod IPs, and consul will use that
to join the other agents.
---
 charts/consul/templates/server-statefulset.yaml | 5 +----
 charts/consul/test/unit/server-statefulset.bats | 8 +-------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index c099880cfa..c1f0d9fbf0 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -302,10 +302,7 @@ spec:
                 -hcl="acl { tokens { agent = \"${ACL_REPLICATION_TOKEN}\", replication = \"${ACL_REPLICATION_TOKEN}\" } }" \
                 {{- end }}
                 {{- end }}
-                {{- $serverSerfLANPort  := .Values.server.ports.serflan.port -}}
-                {{- range $index := until (.Values.server.replicas | int) }}
-                -retry-join="${CONSUL_FULLNAME}-server-{{ $index }}.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:{{ $serverSerfLANPort }}" \
-                {{- end }}
+                -retry-join="${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:{{ .Values.server.ports.serflan.port }}" \
                 -serf-lan-port={{ .Values.server.ports.serflan.port }} \
                 {{- range $value := .Values.global.recursors }}
                 -recursor={{ quote $value }} \
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index 889b8881f8..f2e6e1d13f 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -365,13 +365,7 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
 
-  local actual=$(echo $command | jq -r ' . | any(contains("-retry-join=\"${CONSUL_FULLNAME}-server-0.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:9301\""))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-
-  local actual=$(echo $command | jq -r ' . | any(contains("-retry-join=\"${CONSUL_FULLNAME}-server-1.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:9301\""))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-
-  local actual=$(echo $command | jq -r ' . | any(contains("-retry-join=\"${CONSUL_FULLNAME}-server-2.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:9301\""))' | tee /dev/stderr)
+  local actual=$(echo $command | jq -r ' . | any(contains("-retry-join=\"${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:9301\""))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 

From 5be3b40f8809ab21cbcc84b331f50fcabfbf23a0 Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Thu, 31 Mar 2022 14:43:03 -0600
Subject: [PATCH 4/7] refactor: move general server config from cmd flags to
 configmap

---
 .../templates/server-config-configmap.yaml    | 17 +++++
 .../consul/templates/server-statefulset.yaml  | 20 +-----
 .../test/unit/server-config-configmap.bats    | 72 +++++++++++++++++++
 .../consul/test/unit/server-statefulset.bats  | 70 +++---------------
 4 files changed, 99 insertions(+), 80 deletions(-)

diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml
index 54372a71ef..c67f591867 100644
--- a/charts/consul/templates/server-config-configmap.yaml
+++ b/charts/consul/templates/server-config-configmap.yaml
@@ -12,6 +12,23 @@ metadata:
     release: {{ .Release.Name }}
     component: server
 data:
+  server.json: |
+    {
+      "bind_addr": "0.0.0.0",
+      "bootstrap_expect": {{ if .Values.server.bootstrapExpect }}{{ .Values.server.bootstrapExpect }}{{ else }}{{ .Values.server.replicas }}{{ end }},
+      "client_addr": "0.0.0.0",
+      "connect": {
+        "enabled": {{ .Values.server.connect }}
+      },
+      "datacenter": "{{ .Values.global.datacenter }}",
+      "data_dir": "/consul/data",
+      "domain": "{{ .Values.global.domain }}",
+      "ports": {
+        "serf_lan": {{ .Values.server.ports.serflan.port }}
+      },
+      "retry_join": ["{{template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:{{ .Values.server.ports.serflan.port }}"],
+      "server": true
+    }
   {{- $vaultConnectCAEnabled := and .Values.global.secretsBackend.vault.connectCA.address .Values.global.secretsBackend.vault.connectCA.rootPKIPath .Values.global.secretsBackend.vault.connectCA.intermediatePKIPath -}}
   {{- if and .Values.global.secretsBackend.vault.enabled $vaultConnectCAEnabled }}
   {{- with .Values.global.secretsBackend.vault }}
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index c1f0d9fbf0..e1255b51d6 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -205,10 +205,6 @@ spec:
               valueFrom:
                 fieldRef:
                   fieldPath: status.podIP
-            - name: NAMESPACE
-              valueFrom:
-                fieldRef:
-                  fieldPath: metadata.namespace
             - name: CONSUL_DISABLE_PERM_MGMT
               value: "true"
             {{- if (or .Values.global.gossipEncryption.autoGenerate (and .Values.global.gossipEncryption.secretName .Values.global.gossipEncryption.secretKey)) }}
@@ -262,8 +258,6 @@ spec:
             - "/bin/sh"
             - "-ec"
             - |
-              CONSUL_FULLNAME="{{template "consul.fullname" . }}"
-              
               {{- if and .Values.global.secretsBackend.vault.enabled .Values.global.gossipEncryption.secretName }}
               GOSSIP_KEY=`cat /vault/secrets/gossip.txt`
               {{- end }}
@@ -276,19 +270,10 @@ spec:
 
               exec /usr/local/bin/docker-entrypoint.sh consul agent \
                 -advertise="${ADVERTISE_IP}" \
-                -bind=0.0.0.0 \
-                -bootstrap-expect={{ if .Values.server.bootstrapExpect }}{{ .Values.server.bootstrapExpect }}{{ else }}{{ .Values.server.replicas }}{{ end }} \
-                -client=0.0.0.0 \
                 -config-dir=/consul/config \
-                -datacenter={{ .Values.global.datacenter }} \
-                -data-dir=/consul/data \
-                -domain={{ .Values.global.domain }} \
                 {{- if (or .Values.global.gossipEncryption.autoGenerate (and .Values.global.gossipEncryption.secretName .Values.global.gossipEncryption.secretKey)) }}
                 -encrypt="${GOSSIP_KEY}" \
                 {{- end }}
-                {{- if .Values.server.connect }}
-                -hcl="connect { enabled = true }" \
-                {{- end }}
                 {{- if (and .Values.global.metrics.enabled .Values.global.metrics.enableAgentMetrics) }}
                 -hcl='telemetry { prometheus_retention_time = "{{ .Values.global.metrics.agentMetricsRetentionTime }}" }' \
                 {{- end }}
@@ -302,8 +287,6 @@ spec:
                 -hcl="acl { tokens { agent = \"${ACL_REPLICATION_TOKEN}\", replication = \"${ACL_REPLICATION_TOKEN}\" } }" \
                 {{- end }}
                 {{- end }}
-                -retry-join="${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:{{ .Values.server.ports.serflan.port }}" \
-                -serf-lan-port={{ .Values.server.ports.serflan.port }} \
                 {{- range $value := .Values.global.recursors }}
                 -recursor={{ quote $value }} \
                 {{- end }}
@@ -323,8 +306,7 @@ spec:
                 -config-dir=/consul/userconfig/{{ .name }} \
                 {{- end }}
                 {{- end }}
-                -config-file=/consul/extra-config/extra-from-values.json \
-                -server
+                -config-file=/consul/extra-config/extra-from-values.json
           volumeMounts:
             - name: data-{{ .Release.Namespace | trunc 58 | trimSuffix "-" }}
               mountPath: /consul/data
diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats
index 766e4ab140..a20fbe666d 100755
--- a/charts/consul/test/unit/server-config-configmap.bats
+++ b/charts/consul/test/unit/server-config-configmap.bats
@@ -48,6 +48,78 @@ load _helpers
   [ ! -z "${actual}" ]
 }
 
+#--------------------------------------------------------------------
+# retry-join
+
+@test "server/ConfigMap: retry join gets populated" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'server.replicas=3' \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq -r .retry_join[0] | tee /dev/stderr)
+
+  [ "${actual}" = "RELEASE-NAME-consul-server.default.svc:8301" ]
+}
+
+#--------------------------------------------------------------------
+# serflan
+
+@test "server/ConfigMap: server.ports.serflan.port is set to 8301 by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq -r .ports.serf_lan | tee /dev/stderr)
+
+  [ "${actual}" = "8301" ]
+}
+
+@test "server/ConfigMap: server.ports.serflan.port can be customized" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'server.ports.serflan.port=9301' \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq -r .ports.serf_lan | tee /dev/stderr)
+
+  [ "${actual}" = "9301" ]
+}
+
+@test "server/ConfigMap: retry join uses server.ports.serflan.port" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'server.replicas=3' \
+      --set 'server.ports.serflan.port=9301' \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq -r .retry_join[0] | tee /dev/stderr)
+
+  [ "${actual}" = "RELEASE-NAME-consul-server.default.svc:9301" ]
+}
+
+#--------------------------------------------------------------------
+# bootstrap_expect
+
+@test "server/ConfigMap: bootstrap_expect defaults to replicas" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq .bootstrap_expect | tee /dev/stderr)
+  [ "${actual}" = "3" ]
+}
+
+@test "server/ConfigMap: bootstrap_expect can be set by server.bootstrapExpect" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'server.bootstrapExpect=5' \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq .bootstrap_expect | tee /dev/stderr)
+  [ "${actual}" = "5" ]
+}
+
 #--------------------------------------------------------------------
 # global.acls.manageSystemACLs
 
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index f2e6e1d13f..bd9bf66ece 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -39,17 +39,15 @@ load _helpers
 }
 
 #--------------------------------------------------------------------
-# retry-join
+# server.replicas and server.bootstrapExpect
 
-@test "server/StatefulSet: retry join gets populated" {
+@test "server/StatefulSet: errors if bootstrapExpect < replicas" {
   cd `chart_dir`
-  local actual=$(helm template \
+  run helm template \
       -s templates/server-statefulset.yaml  \
-      --set 'server.replicas=3' \
-      . | tee /dev/stderr |
-      yq -r '.spec.template.spec.containers[0].command | any(contains("-retry-join"))' | tee /dev/stderr)
-
-  [ "${actual}" = "true" ]
+      --set 'server.bootstrapExpect=1' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "server.bootstrapExpect cannot be less than server.replicas" ]]
 }
 
 #--------------------------------------------------------------------
@@ -329,9 +327,6 @@ load _helpers
 
   local command=$(echo "$object" |
       yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo $command | jq -r ' . | any(contains("-serf-lan-port=8301"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
 }
 
 @test "server/StatefulSet: server.ports.serflan.port can be customized" {
@@ -351,22 +346,6 @@ load _helpers
 
   local command=$(echo "$object" |
       yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo $command | jq -r ' . | any(contains("-serf-lan-port=9301"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "server/StatefulSet: retry join uses server.ports.serflan.port" {
-  cd `chart_dir`
-  local command=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'server.replicas=3' \
-      --set 'server.ports.serflan.port=9301' \
-      . | tee /dev/stderr |
-      yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo $command | jq -r ' . | any(contains("-retry-join=\"${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:9301\""))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
 }
 
 #--------------------------------------------------------------------
@@ -717,7 +696,7 @@ load _helpers
       -s templates/server-statefulset.yaml  \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = 97314eb896e88e23a6552ca812e1a3205243b1fa2f4d3e6e95e63c893a7c1f1e ]
+  [ "${actual}" = 6e60da3b41ca20684b887b27c188eebfefacd77ecb47491b83f55c19b1ba9374 ]
 }
 
 @test "server/StatefulSet: adds config-checksum annotation when extraConfig is provided" {
@@ -727,7 +706,7 @@ load _helpers
       --set 'server.extraConfig="{\"hello\": \"world\"}"' \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = 004fe72210e383398b8e36de2e918d005dcd6b8e162d67ad23aef024a872dda8 ]
+  [ "${actual}" = e9595e03c13c8208168fbbe251768bb8660021d6df8114363512012024b06bdb ]
 }
 
 @test "server/StatefulSet: adds config-checksum annotation when config is updated" {
@@ -737,7 +716,7 @@ load _helpers
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = 54aae134d108d934d0a10b527df62dfbb10074fffb27c196625ebf5da6b8206d ]
+  [ "${actual}" = d5b7cfe1ff7a5835a5702d88aeca8e2683d236daa6f67f6bfdf1ab9ba61f2a92 ]
 }
 
 #--------------------------------------------------------------------
@@ -1309,37 +1288,6 @@ load _helpers
   [ "${actual}" = '[{"name":"ACL_REPLICATION_TOKEN","valueFrom":{"secretKeyRef":{"name":"name","key":"key"}}}]' ]
 }
 
-#--------------------------------------------------------------------
-# -bootstrap-expect
-
-@test "server/StatefulSet: -bootstrap-expect defaults to replicas" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("-bootstrap-expect=3")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "server/StatefulSet: -bootstrap-expect can be set by server.bootstrapExpect" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'server.bootstrapExpect=5' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("-bootstrap-expect=5")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "server/StatefulSet: errors if bootstrapExpect < replicas" {
-  cd `chart_dir`
-  run helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'server.bootstrapExpect=1' .
-  [ "$status" -eq 1 ]
-  [[ "$output" =~ "server.bootstrapExpect cannot be less than server.replicas" ]]
-}
-
 #--------------------------------------------------------------------
 # license-autoload
 

From 70b78d2a9560aba8831a5068fe91a5c5d904078c Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Thu, 31 Mar 2022 16:22:29 -0600
Subject: [PATCH 5/7] refactor: move recursors to server configmap

---
 .../templates/server-config-configmap.yaml       |  1 +
 charts/consul/templates/server-statefulset.yaml  |  3 ---
 .../test/unit/server-config-configmap.bats       | 11 +++++++++++
 charts/consul/test/unit/server-statefulset.bats  | 16 +++-------------
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml
index c67f591867..d953d13f6a 100644
--- a/charts/consul/templates/server-config-configmap.yaml
+++ b/charts/consul/templates/server-config-configmap.yaml
@@ -26,6 +26,7 @@ data:
       "ports": {
         "serf_lan": {{ .Values.server.ports.serflan.port }}
       },
+      "recursors": {{ .Values.global.recursors | toJson }},
       "retry_join": ["{{template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:{{ .Values.server.ports.serflan.port }}"],
       "server": true
     }
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index e1255b51d6..9a4855c189 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -287,9 +287,6 @@ spec:
                 -hcl="acl { tokens { agent = \"${ACL_REPLICATION_TOKEN}\", replication = \"${ACL_REPLICATION_TOKEN}\" } }" \
                 {{- end }}
                 {{- end }}
-                {{- range $value := .Values.global.recursors }}
-                -recursor={{ quote $value }} \
-                {{- end }}
                 {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }}
                 $recursor_flags \
                 {{- end }}
diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats
index a20fbe666d..9e0c7afa7c 100755
--- a/charts/consul/test/unit/server-config-configmap.bats
+++ b/charts/consul/test/unit/server-config-configmap.bats
@@ -98,6 +98,17 @@ load _helpers
   [ "${actual}" = "RELEASE-NAME-consul-server.default.svc:9301" ]
 }
 
+@test "server/StatefulSet: recursors can be set by global.recursors" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'global.recursors[0]=1.1.1.1' \
+      --set 'global.recursors[1]=2.2.2.2' \
+      . | tee /dev/stderr |
+      yq -r '.data["server.json"]' | jq -c .recursors | tee /dev/stderr)
+  [ "${actual}" = '["1.1.1.1","2.2.2.2"]' ]
+}
+
 #--------------------------------------------------------------------
 # bootstrap_expect
 
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index bd9bf66ece..90ef553a90 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -696,7 +696,7 @@ load _helpers
       -s templates/server-statefulset.yaml  \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = 6e60da3b41ca20684b887b27c188eebfefacd77ecb47491b83f55c19b1ba9374 ]
+  [ "${actual}" = 0413362d01133dcd9c1da159e01c20143fb1fe4e2e2ade27bd3b85645653d7cf ]
 }
 
 @test "server/StatefulSet: adds config-checksum annotation when extraConfig is provided" {
@@ -706,7 +706,7 @@ load _helpers
       --set 'server.extraConfig="{\"hello\": \"world\"}"' \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = e9595e03c13c8208168fbbe251768bb8660021d6df8114363512012024b06bdb ]
+  [ "${actual}" = 7394f1cb933e3501be41dd0225d8d2248f2d592e0241876035127e5b9540ec31 ]
 }
 
 @test "server/StatefulSet: adds config-checksum annotation when config is updated" {
@@ -716,7 +716,7 @@ load _helpers
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
       yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
-  [ "${actual}" = d5b7cfe1ff7a5835a5702d88aeca8e2683d236daa6f67f6bfdf1ab9ba61f2a92 ]
+  [ "${actual}" = 62db9fd78a8dd95db274586c7d372af85c899b8c9db942a2a0bb691b25c87f55 ]
 }
 
 #--------------------------------------------------------------------
@@ -1324,16 +1324,6 @@ load _helpers
       [ "${actual}" = '{"name":"CONSUL_LICENSE_PATH","value":"/consul/license/bar"}' ]
 }
 
-@test "server/StatefulSet: -recursor can be set by global.recursors" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.recursors[0]=1.2.3.4' \
-      . | tee /dev/stderr |
-      yq -r -c '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"1.2.3.4\"")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 @test "server/StatefulSet: when global.enterpriseLicense.secretKey!=null and global.enterpriseLicense.secretName=null, fail" {
     cd `chart_dir`
     run helm template \

From b6d9bab681566f234f4bac6e8cbf72174210ae4b Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Thu, 31 Mar 2022 16:36:26 -0600
Subject: [PATCH 6/7] refactor: move server connect wan fed config to configmap

---
 .../templates/server-config-configmap.yaml    |  5 +++-
 .../consul/templates/server-statefulset.yaml  |  3 ---
 .../test/unit/server-config-configmap.bats    | 14 +++++------
 .../consul/test/unit/server-statefulset.bats  | 23 -------------------
 4 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml
index d953d13f6a..30101de4fd 100644
--- a/charts/consul/templates/server-config-configmap.yaml
+++ b/charts/consul/templates/server-config-configmap.yaml
@@ -137,7 +137,10 @@ data:
   federation-config.json: |-
     {
       "primary_datacenter": "{{ .Values.global.federation.primaryDatacenter }}",
-      "primary_gateways": {{ .Values.global.federation.primaryGateways | toJson }}
+      "primary_gateways": {{ .Values.global.federation.primaryGateways | toJson }},
+      "connect": {
+        "enable_mesh_gateway_wan_federation": true
+      }
     }
   {{- end }}
 {{- end }}
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index 9a4855c189..95e54cabb6 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -277,9 +277,6 @@ spec:
                 {{- if (and .Values.global.metrics.enabled .Values.global.metrics.enableAgentMetrics) }}
                 -hcl='telemetry { prometheus_retention_time = "{{ .Values.global.metrics.agentMetricsRetentionTime }}" }' \
                 {{- end }}
-                {{- if .Values.global.federation.enabled }}
-                -hcl="connect { enable_mesh_gateway_wan_federation = true }" \
-                {{- end }}
                 {{- if (and .Values.global.acls.replicationToken.secretName .Values.global.acls.replicationToken.secretKey) }}
                 {{- if (and .Values.global.secretsBackend.vault.enabled (not .Values.global.acls.createReplicationToken)) }}
                 -config-file=/vault/secrets/replication-token-config.hcl \
diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats
index 9e0c7afa7c..746bfbe92d 100755
--- a/charts/consul/test/unit/server-config-configmap.bats
+++ b/charts/consul/test/unit/server-config-configmap.bats
@@ -98,7 +98,7 @@ load _helpers
   [ "${actual}" = "RELEASE-NAME-consul-server.default.svc:9301" ]
 }
 
-@test "server/StatefulSet: recursors can be set by global.recursors" {
+@test "server/ConfigMap: recursors can be set by global.recursors" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
@@ -628,7 +628,7 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "server/ConfigMap: doesn't add federation config by default" {
+@test "server/ConfigMap: doesn't add federation config when global.federation.enabled is false (default)" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml  \
@@ -637,7 +637,7 @@ load _helpers
   [ "${actual}" = "false" ]
 }
 
-@test "server/ConfigMap: adds empty federation config when global.federation.enabled is true" {
+@test "server/ConfigMap: adds default federation config when global.federation.enabled is true" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-config-configmap.yaml \
@@ -646,8 +646,8 @@ load _helpers
       --set 'meshGateway.enabled=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
-      yq '.data["federation-config.json"]' | tee /dev/stderr)
-  [ "${actual}" = '"{\n  \"primary_datacenter\": \"\",\n  \"primary_gateways\": []\n}"' ]
+      yq -r '.data["federation-config.json"]' | jq -c . | tee /dev/stderr)
+  [ "${actual}" = '{"primary_datacenter":"","primary_gateways":[],"connect":{"enable_mesh_gateway_wan_federation":true}}' ]
 }
 
 @test "server/ConfigMap: can set primary dc and gateways when global.federation.enabled is true" {
@@ -662,8 +662,8 @@ load _helpers
       --set 'meshGateway.enabled=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
-      yq '.data["federation-config.json"]' | tee /dev/stderr)
-  [ "${actual}" = '"{\n  \"primary_datacenter\": \"dc1\",\n  \"primary_gateways\": [\"1.1.1.1:443\",\"2.2.2.2:443\"]\n}"' ]
+      yq -r '.data["federation-config.json"]' | jq -c . | tee /dev/stderr)
+  [ "${actual}" = '{"primary_datacenter":"dc1","primary_gateways":["1.1.1.1:443","2.2.2.2:443"],"connect":{"enable_mesh_gateway_wan_federation":true}}' ]
 }
 
 #--------------------------------------------------------------------
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index 90ef553a90..b62d6f2f4c 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -1128,29 +1128,6 @@ load _helpers
   [[ "$output" =~ "If global.federation.enabled is true, global.tls.enabled must be true because federation is only supported with TLS enabled" ]]
 }
 
-@test "server/StatefulSet: mesh gateway federation enabled when federation.enabled=true" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.federation.enabled=true' \
-      --set 'global.tls.enabled=true' \
-      --set 'meshGateway.enabled=true' \
-      --set 'connectInject.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("connect { enable_mesh_gateway_wan_federation = true }")' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "server/StatefulSet: mesh gateway federation not enabled when federation.enabled=false" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.federation.enabled=false' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("connect { enable_mesh_gateway_wan_federation = true }")' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
 #--------------------------------------------------------------------
 # global.acls.bootstrapToken
 

From 69995ea0ea6706717507093306fd4ec46caf0ca3 Mon Sep 17 00:00:00 2001
From: Iryna Shustava <iryna@hashicorp.com>
Date: Thu, 31 Mar 2022 16:51:08 -0600
Subject: [PATCH 7/7] refactor: move server telemetry config to configmap

---
 .../templates/server-config-configmap.yaml    |  8 ++++++
 .../consul/templates/server-statefulset.yaml  |  3 ---
 .../test/unit/server-config-configmap.bats    | 25 +++++++++++++++++++
 .../consul/test/unit/server-statefulset.bats  | 25 -------------------
 4 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml
index 30101de4fd..2fa1842048 100644
--- a/charts/consul/templates/server-config-configmap.yaml
+++ b/charts/consul/templates/server-config-configmap.yaml
@@ -143,4 +143,12 @@ data:
       }
     }
   {{- end }}
+  {{- if (and .Values.global.metrics.enabled .Values.global.metrics.enableAgentMetrics) }}
+  telemetry-config.json: |-
+    {
+      "telemetry": {
+        "prometheus_retention_time": "{{ .Values.global.metrics.agentMetricsRetentionTime }}"
+      }
+    }
+  {{- end }}
 {{- end }}
diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
index 95e54cabb6..56a1904cd9 100644
--- a/charts/consul/templates/server-statefulset.yaml
+++ b/charts/consul/templates/server-statefulset.yaml
@@ -274,9 +274,6 @@ spec:
                 {{- if (or .Values.global.gossipEncryption.autoGenerate (and .Values.global.gossipEncryption.secretName .Values.global.gossipEncryption.secretKey)) }}
                 -encrypt="${GOSSIP_KEY}" \
                 {{- end }}
-                {{- if (and .Values.global.metrics.enabled .Values.global.metrics.enableAgentMetrics) }}
-                -hcl='telemetry { prometheus_retention_time = "{{ .Values.global.metrics.agentMetricsRetentionTime }}" }' \
-                {{- end }}
                 {{- if (and .Values.global.acls.replicationToken.secretName .Values.global.acls.replicationToken.secretKey) }}
                 {{- if (and .Values.global.secretsBackend.vault.enabled (not .Values.global.acls.createReplicationToken)) }}
                 -config-file=/vault/secrets/replication-token-config.hcl \
diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats
index 746bfbe92d..c7a250ffa3 100755
--- a/charts/consul/test/unit/server-config-configmap.bats
+++ b/charts/consul/test/unit/server-config-configmap.bats
@@ -772,4 +772,29 @@ load _helpers
 
   local actual=$(echo $object | jq -r .key_file | tee /dev/stderr)
   [ "${actual}" = "/vault/secrets/servercert.key" ]
+}
+
+@test "server/ConfigMap: when global.metrics.enableAgentMetrics=true, sets telemetry config" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'global.metrics.enabled=true'  \
+      --set 'global.metrics.enableAgentMetrics=true'  \
+      . | tee /dev/stderr |
+      yq -r '.data["telemetry-config.json"]' | jq -r .telemetry.prometheus_retention_time | tee /dev/stderr)
+
+  [ "${actual}" = "1m" ]
+}
+
+@test "server/ConfigMap: when global.metrics.enableAgentMetrics=true and global.metrics.agentMetricsRetentionTime is set, sets telemetry config with updated retention time" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-config-configmap.yaml  \
+      --set 'global.metrics.enabled=true'  \
+      --set 'global.metrics.enableAgentMetrics=true'  \
+      --set 'global.metrics.agentMetricsRetentionTime=5m'  \
+      . | tee /dev/stderr |
+      yq -r '.data["telemetry-config.json"]' | jq -r .telemetry.prometheus_retention_time | tee /dev/stderr)
+
+  [ "${actual}" = "5m" ]
 }
\ No newline at end of file
diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats
index b62d6f2f4c..51fc130261 100755
--- a/charts/consul/test/unit/server-statefulset.bats
+++ b/charts/consul/test/unit/server-statefulset.bats
@@ -648,31 +648,6 @@ load _helpers
   [ "${actual}" = "/v1/agent/metrics" ]
 }
 
-@test "server/StatefulSet: when global.metrics.enableAgentMetrics=true, sets telemetry flag" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.metrics.enabled=true'  \
-      --set 'global.metrics.enableAgentMetrics=true'  \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("telemetry { prometheus_retention_time = \"1m\" }")' | tee /dev/stderr)
-
-  [ "${actual}" = "true" ]
-}
-
-@test "server/StatefulSet: when global.metrics.enableAgentMetrics=true and global.metrics.agentMetricsRetentionTime is set, sets telemetry flag with updated retention time" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.metrics.enabled=true'  \
-      --set 'global.metrics.enableAgentMetrics=true'  \
-      --set 'global.metrics.agentMetricsRetentionTime=5m'  \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | join(" ") | contains("telemetry { prometheus_retention_time = \"5m\" }")' | tee /dev/stderr)
-
-  [ "${actual}" = "true" ]
-}
-
 @test "server/StatefulSet: when global.metrics.enableAgentMetrics=true, global.tls.enabled=true and global.tls.httpsOnly=true, fail" {
   cd `chart_dir`
   run helm template \