Merge pull request hashicorp#298 from hashicorp/persistent-storage

Allow setting hostPath for client data directory

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:

templates/client-daemonset.yaml

@@ -11,6 +11,10 @@ metadata:
     heritage: {{ .Release.Service }}
     release: {{ .Release.Name }}
 spec:
+  {{- if .Values.client.updateStrategy }}
+  updateStrategy:
+    {{ tpl .Values.client.updateStrategy . | nindent 4 | trim }}
+  {{- end }}
   selector:
     matchLabels:
       app: {{ template "consul.name" . }}
@@ -47,12 +51,23 @@ spec:
       priorityClassName: {{ .Values.client.priorityClassName | quote }}
       {{- 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.
+      {{- if .Values.client.dnsPolicy }}
+      dnsPolicy: {{ .Values.client.dnsPolicy }}
+      {{- end }}
+
+      # 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.dataDirectoryHostPath }}
+          hostPath:
+            path: {{ .Values.client.dataDirectoryHostPath }}
+            type: DirectoryOrCreate
+        {{- else }}
           emptyDir: {}
+        {{- end }}
         - name: config
           configMap:
             name: {{ template "consul.fullname" . }}-client-config

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 }}

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" ]
+}

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' ]
+}

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.