Allow setting hostPath for client data directory

client.dataDirectoryHostPath can be set to Mount client data to a host path. This is necessary for the clients to retain any service/check registrations made to them via API. Without this, if a Consul client Pod is deleted, e.g. during a Consul version upgrade, all its Consul Connect registrations are lost. Also allow configuring client dnsPolicy and updateStrategy.
ndhanushkodi · Dec 10, 2019 · 59e3ba3 · 59e3ba3
1 parent 0efda86
commit 59e3ba3
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,19 @@
 ## Unreleased
 
+IMPROVEMENTS:
+
+  * Consul client DaemonSet can now use a [hostPath mount](https://kubernetes.io/docs/concepts/storage/volumes/#hostpath)
+    for its data directory by setting the `client.dataDirectoryHostPath` value.
+    This setting is currently necessary to ensure that when a Consul client Pod is deleted,
+    e.g. during a Consul version upgrade, it does not lose its Connect service
+    registrations. In the next version, we plan to have services automatically
+    re-register which will remove the need for this. [[GH-298](https://github.com/hashicorp/consul-helm/pull/298)]
+
+    **Security Warning:** If using this setting, Pod Security Policies *must* be enabled on your cluster
+     and in this Helm chart (via the `global.enablePodSecurityPolicies` setting)
+     to prevent other Pods from mounting the same host path and gaining
+     access to all of Consul's data. Consul's data is not encrypted at rest.
+
 ## 0.13.0 (Dec 5, 2019)
 
 BREAKING CHANGES:

diff --git a/templates/client-daemonset.yaml b/templates/client-daemonset.yaml
@@ -13,7 +13,7 @@ metadata:
 spec:
   {{- if .Values.client.updateStrategy }}
   updateStrategy:
-    {{ toYaml .Values.client.updateStrategy | nindent 4 | trim }}
+    {{ tpl .Values.client.updateStrategy . | nindent 4 | trim }}
   {{- end }}
   selector:
     matchLabels:
@@ -55,15 +55,15 @@ spec:
       dnsPolicy: {{ .Values.client.dnsPolicy }}
       {{- end }}
 
-      # Consul agents require a directory for data, even clients. The data
-      # is okay to be wiped though if the Pod is removed, so just use an
-      # emptyDir volume.
+      # Consul clients require a directory for data.
+      # We use a hostPath so that if a Pod is restarted it will retain its
+      # service and checks registrations. This is important for Consul Connect
+      # because each Connect Pod registers with the local Consul client.
       volumes:
         - name: data
-        {{- if .Values.client.hostPath }}
+        {{- if .Values.client.dataDirectoryHostPath }}
           hostPath:
-            # directory location on host
-            path: {{ .Values.client.hostPath }}
+            path: {{ .Values.client.dataDirectoryHostPath }}
             type: DirectoryOrCreate
         {{- else }}
           emptyDir: {}

diff --git a/templates/client-podsecuritypolicy.yaml b/templates/client-podsecuritypolicy.yaml
@@ -23,6 +23,9 @@ spec:
     - 'projected'
     - 'secret'
     - 'downwardAPI'
+    {{- if .Values.client.dataDirectoryHostPath }}
+    - 'hostPath'
+    {{- end }}
   hostNetwork: false
   hostPorts:
   # HTTP Port
@@ -47,4 +50,9 @@ spec:
   fsGroup:
     rule: 'RunAsAny'
   readOnlyRootFilesystem: false
+  {{- if .Values.client.dataDirectoryHostPath }}
+  allowedHostPaths:
+  - pathPrefix: {{ .Values.client.dataDirectoryHostPath | quote }}
+    readOnly: false
+  {{- end }}
 {{- end }}
diff --git a/test/unit/client-daemonset.bats b/test/unit/client-daemonset.bats
@@ -557,3 +557,81 @@ load _helpers
       tee /dev/stderr)
   [ "${actual}" = "8301" ]
 }
+
+#--------------------------------------------------------------------
+# dataDirectoryHostPath
+
+@test "client/DaemonSet: data directory is emptyDir by defaut" {
+  cd `chart_dir`
+  # Test that hostPath is set to null.
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml  \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.volumes[0].hostPath == null' | tee /dev/stderr )
+  [ "${actual}" = "true" ]
+
+  # Test that emptyDir is set instead.
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml  \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.volumes[0].emptyDir == {}' | tee /dev/stderr )
+  [ "${actual}" = "true" ]
+}
+
+@test "client/DaemonSet: hostPath data directory can be set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml  \
+      --set 'client.dataDirectoryHostPath=/opt/consul' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.volumes[0].hostPath.path == "/opt/consul"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+#--------------------------------------------------------------------
+# dnsPolicy
+
+@test "client/DaemonSet: dnsPolicy not set by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.dnsPolicy == null' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "client/DaemonSet: dnsPolicy can be set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml \
+      --set 'client.dnsPolicy=ClusterFirstWithHostNet' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.dnsPolicy == "ClusterFirstWithHostNet"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+#--------------------------------------------------------------------
+# updateStrategy
+
+@test "client/DaemonSet: updateStrategy not set by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml \
+      . | tee /dev/stderr | \
+      yq '.spec.updateStrategy == null' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "client/DaemonSet: updateStrategy can be set" {
+  cd `chart_dir`
+  local updateStrategy="type: RollingUpdate
+rollingUpdate:
+  maxUnavailable: 5
+"
+  local actual=$(helm template \
+      -x templates/client-daemonset.yaml \
+      --set "client.updateStrategy=${updateStrategy}" \
+      . | tee /dev/stderr | \
+      yq -c '.spec.updateStrategy == {"type":"RollingUpdate","rollingUpdate":{"maxUnavailable":5}}' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
diff --git a/test/unit/client-podsecuritypolicy.bats b/test/unit/client-podsecuritypolicy.bats
@@ -45,3 +45,37 @@ load _helpers
       yq -c '.spec.hostPorts' | tee /dev/stderr)
   [ "${actual}" = '[{"min":8500,"max":8500},{"min":8502,"max":8502},{"min":8301,"max":8301}]' ]
 }
+
+#--------------------------------------------------------------------
+# client.dataDirectoryHostPath
+
+@test "client/PodSecurityPolicy: disallows hostPath volume by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/client-podsecuritypolicy.yaml  \
+      --set 'global.enablePodSecurityPolicies=true' \
+      . | tee /dev/stderr |
+      yq '.spec.volumes | any(contains("hostPath"))' | tee /dev/stderr)
+  [ "${actual}" = 'false' ]
+}
+
+@test "client/PodSecurityPolicy: allows hostPath volume when dataDirectoryHostPath is set" {
+  cd `chart_dir`
+  # Test that hostPath is an allowed volume type.
+  local actual=$(helm template \
+      -x templates/client-podsecuritypolicy.yaml  \
+      --set 'global.enablePodSecurityPolicies=true' \
+      --set 'client.dataDirectoryHostPath=/opt/consul' \
+      . | tee /dev/stderr |
+      yq '.spec.volumes | any(contains("hostPath"))' | tee /dev/stderr)
+  [ "${actual}" = 'true' ]
+
+  # Test that the path we're allowed to write to is the right one.
+  local actual=$(helm template \
+      -x templates/client-podsecuritypolicy.yaml  \
+      --set 'global.enablePodSecurityPolicies=true' \
+      --set 'client.dataDirectoryHostPath=/opt/consul' \
+      . | tee /dev/stderr |
+      yq -r '.spec.allowedHostPaths[0].pathPrefix' | tee /dev/stderr)
+  [ "${actual}" = '/opt/consul' ]
+}
diff --git a/values.yaml b/values.yaml
@@ -167,6 +167,20 @@ client:
   image: null
   join: null
 
+  # dataDirectoryHostPath is an absolute path to a directory on the host machine
+  # to use as the Consul client data directory.
+  # If set to the empty string or null, the Consul agent will store its data
+  # in the Pod's local filesystem (which will be lost if the Pod is deleted).
+  # If using Consul Connect, this directory must be set. Otherwise when the Consul
+  # agent Pod is deleted, e.g. during an upgrade, all the Connect-injected Pods
+  # on that node will be de-registered and will need to be restarted to be
+  # re-registered.
+  # Security Warning: If setting this, Pod Security Policies *must* be enabled on your cluster
+  # and in this Helm chart (via the global.enablePodSecurityPolicies setting)
+  # to prevent other Pods from mounting the same host path and gaining
+  # access to all of Consul's data. Consul's data is not encrypted at rest.
+  dataDirectoryHostPath: null
+
   # If true, Consul's gRPC port will be exposed (see https://www.consul.io/docs/agent/options.html#grpc_port).
   # This should be set to true if connectInject or meshGateway is enabled.
   grpc: true
@@ -242,6 +256,19 @@ client:
     # https_proxy: http://localhost:3128,
     # no_proxy: internal.domain.com
 
+  # dnsPolicy to use.
+  dnsPolicy: null
+
+  # updateStrategy for the DaemonSet.
+  # See https://kubernetes.io/docs/tasks/manage-daemon/update-daemon-set/#daemonset-update-strategy.
+  # This should be a multi-line string mapping directly to the updateStrategy
+  # Example:
+  #  updateStrategy: |
+  #    rollingUpdate:
+  #      maxUnavailable: 5
+  #    type: RollingUpdate
+  updateStrategy: null
+
   # snaphotAgent contains settings for setting up and running snapshot agents
   # within the Consul clusters. They are required to be co-located with Consul
   # clients, so will inherit the clients' nodeSelector, tolerations and affinity.
@@ -263,25 +290,6 @@ client:
       secretName: null
       secretKey: null
 
-    # updateStrategy for the DaemonSet.
-    # See https://kubernetes.io/docs/tasks/manage-daemon/update-daemon-set/#daemonset-update-strategy.
-    # This should be a multi-line string mapping directly to the updateStrategy
-    # Example:
-    #  updateStrategy:
-    #    rollingUpdate:
-    #      maxUnavailable: 5
-    #    type: RollingUpdate
-    updateStrategy: null
-
-    # hostPath is fullpath to folder on host machine to mount as /consul/data folder. consul agent stores
-    # its configuration in this folder. By default its created as emptyDir: {}. To save data between pod restarts
-    # specify folder name on host machine to be mounted as /consul/data.
-    # Example:
-    # hostPath: '/var/consul-data'
-    hostPath: null
-
-    dnsPolicy: null
-
 # Configuration for DNS configuration within the Kubernetes cluster.
 # This creates a service that routes to all agents (client or server)
 # for serving DNS requests. This DOES NOT automatically configure kube-dns