From 68c89ac7ef5fe53a3bb52eb56fba56e9cd6228a9 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Thu, 2 Dec 2021 10:27:41 -0500 Subject: [PATCH] Do not mount consul-ca-cert to partition init if externalServers.useSystemRoots is true (#885) * Do not mount consul-ca-cert to partition init if externalServers.useSystemRoots is true * Update CHANGELOG --- CHANGELOG.md | 4 ++ .../consul/templates/partition-init-job.yaml | 12 +++-- charts/consul/test/unit/client-daemonset.bats | 2 + .../consul/test/unit/partition-init-job.bats | 54 +++++++++++++++++++ 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddf6aee220..1165ade1f4 100644 --- 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 index 3bdfa58a23..fe5f26fd86 100644 --- 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 index 62c27a0106..20b5aac856 100755 --- 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 index 5dc60e9888..ca1a9a6d37 100644 --- 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" .