From 76142b89cdb84c83bc986e4e67d7611ab3458fc0 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 30 Nov 2021 14:01:42 -0500 Subject: [PATCH 1/3] Do not mount consul-ca-cert to partition init if externalServers.useSystemRoots is true --- .../consul/templates/partition-init-job.yaml | 7 +++++-- .../consul/test/unit/partition-init-job.bats | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/charts/consul/templates/partition-init-job.yaml b/charts/consul/templates/partition-init-job.yaml index 3bdfa58a23..ab038c5cdc 100644 --- a/charts/consul/templates/partition-init-job.yaml +++ b/charts/consul/templates/partition-init-job.yaml @@ -31,6 +31,7 @@ spec: restartPolicy: Never serviceAccountName: {{ template "consul.fullname" . }}-partition-init {{- if .Values.global.tls.enabled }} + {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} volumes: - name: consul-ca-cert secret: @@ -43,6 +44,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,11 +61,13 @@ spec: key: {{ .Values.global.acls.bootstrapToken.secretKey }} {{- end }} {{- if .Values.global.tls.enabled }} + {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} volumeMounts: - name: consul-ca-cert mountPath: /consul/tls/ca readOnly: true {{- end }} + {{- end }} command: - "/bin/sh" - "-ec" @@ -82,9 +86,8 @@ spec: {{- if .Values.global.tls.enabled }} -use-https \ + {{- if not (and .Values.externalServers.enabled .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/partition-init-job.bats b/charts/consul/test/unit/partition-init-job.bats index 5dc60e9888..406e40bd04 100644 --- a/charts/consul/test/unit/partition-init-job.bats +++ b/charts/consul/test/unit/partition-init-job.bats @@ -71,6 +71,24 @@ 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.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 \ From 3996a051cd621e38d5ffe8ce97db0dbe004b2557 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 30 Nov 2021 14:07:01 -0500 Subject: [PATCH 2/3] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bd710d7b1..d5b77a5d45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ IMPROVEMENTS: * CLI * Pre-check in the `install` command to verify the correct license secret exists when using an enterprise Consul image. [[GH-875](https://github.com/hashicorp/consul-k8s/pull/875)] +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: From 691fe82834b4d9297cd4c939086390cf120fece2 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Wed, 1 Dec 2021 15:58:28 -0500 Subject: [PATCH 3/3] Add review changes suggested by Luke --- .../consul/templates/partition-init-job.yaml | 11 +++--- charts/consul/test/unit/client-daemonset.bats | 2 ++ .../consul/test/unit/partition-init-job.bats | 36 +++++++++++++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/charts/consul/templates/partition-init-job.yaml b/charts/consul/templates/partition-init-job.yaml index ab038c5cdc..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,7 +32,7 @@ spec: restartPolicy: Never serviceAccountName: {{ template "consul.fullname" . }}-partition-init {{- if .Values.global.tls.enabled }} - {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} + {{- if not .Values.externalServers.useSystemRoots }} volumes: - name: consul-ca-cert secret: @@ -61,7 +62,7 @@ spec: key: {{ .Values.global.acls.bootstrapToken.secretKey }} {{- end }} {{- if .Values.global.tls.enabled }} - {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} + {{- if not .Values.externalServers.useSystemRoots }} volumeMounts: - name: consul-ca-cert mountPath: /consul/tls/ca @@ -72,8 +73,6 @@ spec: - "/bin/sh" - "-ec" - | - CONSUL_FULLNAME="{{template "consul.fullname" . }}" - consul-k8s-control-plane partition-init \ -log-level={{ .Values.global.logLevel }} \ -log-json={{ .Values.global.logJSON }} \ @@ -86,7 +85,7 @@ spec: {{- if .Values.global.tls.enabled }} -use-https \ - {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} + {{- if not .Values.externalServers.useSystemRoots }} -consul-ca-cert=/consul/tls/ca/tls.crt \ {{- end }} {{- if .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 406e40bd04..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) @@ -77,6 +104,7 @@ load _helpers -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' \ @@ -95,6 +123,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.tls.enabled=true' \ --set 'global.tls.caCert.secretName=foo-ca-cert' \ --set 'global.tls.caCert.secretKey=key' \ @@ -122,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 | @@ -160,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" .