Do not mount consul-ca-cert to partition init if externalServers.useS…

…ystemRoots is true (#885) * Do not mount consul-ca-cert to partition init if externalServers.useSystemRoots is true * Update CHANGELOG
hashicorp · Dec 2, 2021 · 68c89ac · 68c89ac
1 parent 9c55e45
commit 68c89ac
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,10 @@ BUG FIXES:
 * Control Plane:
    * Add a workaround to check that the ACL token is replicated to other Consul servers. [[GH-862](https://github.com/hashicorp/consul-k8s/issues/862)]
 
+BUG FIXES:
+* Helm Chart
+  * Admin Partitions **(Consul Enterprise only)**: Do not mount Consul CA certs to partition-init job if `externalServers.useSystemRoots` is `true`. [[GH-885](https://github.com/hashicorp/consul-k8s/pull/885)]
+
 ## 0.37.0 (November 18, 2021)
 
 BREAKING CHANGES:

diff --git a/charts/consul/templates/partition-init-job.yaml b/charts/consul/templates/partition-init-job.yaml
@@ -1,6 +1,7 @@
 {{- $serverEnabled := (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) -}}
-{{- if (and .Values.global.adminPartitions.enabled (not $serverEnabled)) }}
+{{- if (and .Values.global.adminPartitions.enabled (not $serverEnabled) (ne .Values.global.adminPartitions.name "default")) }}
 {{- template "consul.reservedNamesFailer" (list .Values.global.adminPartitions.name "global.adminPartitions.name") }}
+{{- if and (not .Values.externalServers.enabled) (ne .Values.global.adminPartitions.name "default") }}{{ fail "externalServers.enabled needs to be true and configured to create a non-default partition." }}{{ end -}}
 apiVersion: batch/v1
 kind: Job
 metadata:
@@ -31,6 +32,7 @@ spec:
       restartPolicy: Never
       serviceAccountName: {{ template "consul.fullname" . }}-partition-init
       {{- if .Values.global.tls.enabled }}
+      {{- if not .Values.externalServers.useSystemRoots }}
       volumes:
         - name: consul-ca-cert
           secret:
@@ -43,6 +45,7 @@ spec:
               - key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }}
                 path: tls.crt
       {{- end }}
+      {{- end }}
       containers:
         - name: partition-init-job
           image: {{ .Values.global.imageK8S }}
@@ -59,17 +62,17 @@ spec:
                   key: {{ .Values.global.acls.bootstrapToken.secretKey }}
             {{- end }}
           {{- if .Values.global.tls.enabled  }}
+          {{- if not .Values.externalServers.useSystemRoots }}
           volumeMounts:
             - name: consul-ca-cert
               mountPath: /consul/tls/ca
               readOnly: true
           {{- end }}
+          {{- end }}
           command:
             - "/bin/sh"
             - "-ec"
             - |
-              CONSUL_FULLNAME="{{template "consul.fullname" . }}"
-
               consul-k8s-control-plane partition-init \
                 -log-level={{ .Values.global.logLevel }} \
                 -log-json={{ .Values.global.logJSON }} \
@@ -82,9 +85,8 @@ spec:
 
                 {{- if .Values.global.tls.enabled }}
                 -use-https \
+                {{- if not .Values.externalServers.useSystemRoots }}
                 -consul-ca-cert=/consul/tls/ca/tls.crt \
-                {{- if not .Values.externalServers.enabled }}
-                -server-port=8501 \
                 {{- end }}
                 {{- if .Values.externalServers.tlsServerName }}
                 -consul-tls-server-name={{ .Values.externalServers.tlsServerName }} \

diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats
@@ -1446,6 +1446,8 @@ rollingUpdate:
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.adminPartitions.name=test' \
       --set 'server.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=bar' \
       . | tee /dev/stderr |
       yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("partition = \"test\"")' | tee /dev/stderr)
   [ "${actual}" = "true" ]

diff --git a/charts/consul/test/unit/partition-init-job.bats b/charts/consul/test/unit/partition-init-job.bats
@@ -15,6 +15,9 @@ load _helpers
       -s templates/partition-init-job.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=false' \
+      --set 'global.adminPartitions.name=bar' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=foo' \
       . | tee /dev/stderr |
       yq 'length > 0' | tee /dev/stderr)
   [ "${actual}" = "true" ]
@@ -29,6 +32,15 @@ load _helpers
       .
 }
 
+@test "partitionInit/Job: disabled with global.adminPartitions.enabled=true and adminPartition.name = default" {
+  cd `chart_dir`
+  assert_empty helm template \
+      -s templates/partition-init-job.yaml  \
+      --set 'global.adminPartitions.enabled=true' \
+      --set 'server.enabled=false' \
+      .
+}
+
 @test "partitionInit/Job: disabled with global.adminPartitions.enabled=true and global.enabled = true" {
   cd `chart_dir`
   assert_empty helm template \
@@ -47,6 +59,18 @@ load _helpers
       .
 }
 
+@test "partitionInit/Job: fails if externalServers.enabled = false with non-default adminPartition" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/partition-init-job.yaml  \
+      --set 'global.adminPartitions.enabled=true' \
+      --set 'global.adminPartitions.name=bar' \
+      --set 'server.enabled=false' \
+      --set 'externalServers.enabled=false' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "externalServers.enabled needs to be true and configured to create a non-default partition." ]]
+}
+
 #--------------------------------------------------------------------
 # global.tls.enabled
 
@@ -57,6 +81,9 @@ load _helpers
       --set 'global.enabled=false' \
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.tls.enabled=true' \
+      --set 'global.adminPartitions.name=bar' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=foo' \
       . | tee /dev/stderr |
       yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
 
@@ -71,12 +98,34 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
+@test "partitionInit/Job: does not set consul ca cert or server-port when .externalServers.useSystemRoots is true" {
+  cd `chart_dir`
+  local command=$(helm template \
+      -s templates/partition-init-job.yaml  \
+      --set 'global.enabled=false' \
+      --set 'global.adminPartitions.enabled=true' \
+      --set 'global.adminPartitions.name=bar' \
+      --set 'global.tls.enabled=true' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=foo' \
+      --set 'externalServers.useSystemRoots=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.containers[0].command' | tee /dev/stderr)
+
+  local actual
+  actual=$(echo $command | jq -r '. | any(contains("-consul-ca-cert=/consul/tls/ca/tls.crt"))' | tee /dev/stderr)
+  [ "${actual}" = "false" ]
+}
+
 @test "partitionInit/Job: can overwrite CA secret with the provided one" {
   cd `chart_dir`
   local ca_cert_volume=$(helm template \
       -s templates/partition-init-job.yaml  \
       --set 'global.enabled=false' \
       --set 'global.adminPartitions.enabled=true' \
+      --set 'global.adminPartitions.name=bar' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=foo' \
       --set 'global.tls.enabled=true' \
       --set 'global.tls.caCert.secretName=foo-ca-cert' \
       --set 'global.tls.caCert.secretKey=key' \
@@ -104,6 +153,9 @@ load _helpers
       -s templates/partition-init-job.yaml  \
       --set 'global.enabled=false' \
       --set 'global.adminPartitions.enabled=true' \
+      --set 'global.adminPartitions.name=bar' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=foo' \
       --set 'global.acls.bootstrapToken.secretName=partition-token' \
       --set 'global.acls.bootstrapToken.secretKey=token' \
       . | tee /dev/stderr |
@@ -142,6 +194,8 @@ reservedNameTest() {
 		run helm template \
 				-s templates/partition-init-job.yaml  \
 				--set 'global.enabled=false' \
+				--set 'externalServers.enabled=true' \
+                --set 'externalServers.hosts[0]=foo' \
 				--set 'global.adminPartitions.enabled=true' \
 				--set "global.adminPartitions.name=$name" .