redpanda: handle cert = ""

Prior to this commit, the redpanda chart would fail to render if the admin's internal TLS set cert to "" due to the order of operations in post_upgrade_job.go. The regression appeared during the conversion to go as there was previously no error handling if an invalid certificate was referenced. This failure surfaced during an attempted release of the operator as a helm test for the operator chart did exactly this. (See #1422) This commit corrects the issue by checking TLS.IsEnabled _before_ calling MustGet which will only trigger a failure if TLS is enabled and references an invalid certificate.
redpanda-data · Jul 22, 2024 · 71f4d8c · 71f4d8c
1 parent acdc6e7
commit 71f4d8c
Show file tree

Hide file tree

Showing 9 changed files with 1,465 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,16 @@
 
 ## Redpanda Chart
 
-### [Unreleased](https://github.com/redpanda-data/helm-charts/releases/tag/redpanda-FILLMEIN) - YYYY-MM-DD
+### [Unreleased](https://github.com/redpanda-data/helm-charts/releases/tag/redpanda-5.8.13) - YYYY-MM-DD
+#### Added
+#### Changed
+#### Fixed
+* Fixed a regression where `post_upgrade_job` would fail if TLS on the admin
+  listener was disabled but had `cert` set to an invalid cert (e.g. `""`)
+* Fixed mTLS configurations between Redpanda and Console [#1402](https://github.com/redpanda-data/helm-charts/pull/1402)
+#### Removed
+
+### [5.8.12](https://github.com/redpanda-data/helm-charts/releases/tag/redpanda-5.8.12) - 2024-07-10
 
 #### Added
 
@@ -61,6 +70,7 @@
 #### Added
 #### Changed
 #### Fixed
+* Added missing permissions for the NodeWatcher controller (`rbac.createAdditionalControllerCRs`)
 #### Removed
 
 ### [0.4.25](https://github.com/redpanda-data/helm-charts/releases/tag/operator-0.4.25) - 2024-07-17

diff --git a/charts/operator/files/three_node_redpanda.yaml b/charts/operator/files/three_node_redpanda.yaml
@@ -16,7 +16,7 @@ spec:
         external: {}
         port: 9644
         tls:
-          # TODO(chrisseto): Uncomment this once the redpanda chart is fixed.
+          # TODO(chrisseto): Uncomment once #1428 is released
           # cert: ""
           enabled: false
           requireClientAuth: false
@@ -27,7 +27,7 @@ spec:
         kafkaEndpoint: kafka-default
         port: 8082
         tls:
-          # TODO(chrisseto): Uncomment this once the redpanda chart is fixed.
+          # TODO(chrisseto): Uncomment once #1428 is released
           # cert: ""
           enabled: false
           requireClientAuth: false

diff --git a/charts/redpanda/ci/40-empty-string-tls-novalues.yaml b/charts/redpanda/ci/40-empty-string-tls-novalues.yaml
@@ -0,0 +1,57 @@
+# Copied from charts/operator/files/three_node_redpanda.yaml. The inclusion of
+# tls.enabled: false and tls.cert: "" triggered failures.
+console:
+  enabled: false
+image:
+  repository: docker.redpanda.com/redpandadata/redpanda
+  tag: v23.2.2
+listeners:
+  admin:
+    external: {}
+    port: 9644
+    tls:
+      cert: ""
+      enabled: false
+      requireClientAuth: false
+  http:
+    authenticationMethod: none
+    enabled: true
+    external: {}
+    kafkaEndpoint: kafka-default
+    port: 8082
+    tls:
+      cert: ""
+      enabled: false
+      requireClientAuth: false
+  kafka:
+    authenticationMethod: none
+    external: {}
+    port: 9092
+    tls:
+      cert: kafka-internal-0
+      enabled: true
+      requireClientAuth: true
+  rpc:
+    port: 33145
+logging:
+  logLevel: trace
+  usageStats:
+    enabled: false
+resources:
+  cpu:
+    cores: 1
+  memory:
+    container:
+      max: 2Gi
+      min: 2Gi
+statefulset:
+  replicas: 3
+storage:
+  persistentVolume:
+    enabled: true
+    size: 100Gi
+tls:
+  certs:
+    kafka-internal-0:
+      caEnabled: true
+  enabled: true
diff --git a/charts/redpanda/post_upgrade_job.go b/charts/redpanda/post_upgrade_job.go
@@ -112,16 +112,20 @@ func PostUpgradeJobScript(dot *helmette.Dot) string {
 
 	if RedpandaAtLeast_23_2_1(dot) {
 		service := values.Listeners.Admin
-		cert := values.TLS.Certs.MustGet(service.TLS.Cert)
 
 		caCert := ""
-		if cert.CAEnabled {
-			caCert = fmt.Sprintf("--cacert /etc/tls/certs/%s/ca.crt", service.TLS.Cert)
-		}
-
 		scheme := "http"
+
 		if service.TLS.IsEnabled(&values.TLS) {
 			scheme = "https"
+
+			// NB: Only call MustGet _after_ we've checked that TLS is enabled
+			// as setting cert to "" is a valid way to disable TLS.
+			cert := values.TLS.Certs.MustGet(service.TLS.Cert)
+
+			if cert.CAEnabled {
+				caCert = fmt.Sprintf("--cacert /etc/tls/certs/%s/ca.crt", service.TLS.Cert)
+			}
 		}
 
 		url := fmt.Sprintf("%s://%s:%d/v1/debug/restart_service?service=schema-registry", scheme, InternalDomain(dot), int64(service.Port))

diff --git a/charts/redpanda/templates/post_upgrade_job.yaml b/charts/redpanda/templates/post_upgrade_job.yaml
@@ -69,14 +69,14 @@
 {{- end -}}
 {{- if (get (fromJson (include "redpanda.RedpandaAtLeast_23_2_1" (dict "a" (list $dot) ))) "r") -}}
 {{- $service := $values.listeners.admin -}}
-{{- $cert := (get (fromJson (include "redpanda.TLSCertMap.MustGet" (dict "a" (list (deepCopy $values.tls.certs) $service.tls.cert) ))) "r") -}}
 {{- $caCert := "" -}}
-{{- if $cert.caEnabled -}}
-{{- $caCert = (printf "--cacert /etc/tls/certs/%s/ca.crt" $service.tls.cert) -}}
-{{- end -}}
 {{- $scheme := "http" -}}
 {{- if (get (fromJson (include "redpanda.InternalTLS.IsEnabled" (dict "a" (list $service.tls $values.tls) ))) "r") -}}
 {{- $scheme = "https" -}}
+{{- $cert := (get (fromJson (include "redpanda.TLSCertMap.MustGet" (dict "a" (list (deepCopy $values.tls.certs) $service.tls.cert) ))) "r") -}}
+{{- if $cert.caEnabled -}}
+{{- $caCert = (printf "--cacert /etc/tls/certs/%s/ca.crt" $service.tls.cert) -}}
+{{- end -}}
 {{- end -}}
 {{- $url := (printf "%s://%s:%d/v1/debug/restart_service?service=schema-registry" $scheme (get (fromJson (include "redpanda.InternalDomain" (dict "a" (list $dot) ))) "r") (($service.port | int) | int64)) -}}
 {{- $script = (concat (default (list ) $script) (list `if [ -d "/etc/secrets/users/" ]; then` `    IFS=":" read -r USER_NAME PASSWORD MECHANISM < <(grep "" $(find /etc/secrets/users/* -print))` `    curl -svm3 --fail --retry "120" --retry-max-time "120" --retry-all-errors --ssl-reqd \` (printf `    %s \` $caCert) `    -X PUT -u ${USER_NAME}:${PASSWORD} \` (printf `    %s || true` $url) `fi`)) -}}

diff --git a/charts/redpanda/testdata/ci/02-one-node-cluster-no-tls-no-sasl-values.yaml.golden b/charts/redpanda/testdata/ci/02-one-node-cluster-no-tls-no-sasl-values.yaml.golden
@@ -1055,7 +1055,7 @@ spec:
           if [ -d "/etc/secrets/users/" ]; then
               IFS=":" read -r USER_NAME PASSWORD MECHANISM < <(grep "" $(find /etc/secrets/users/* -print))
               curl -svm3 --fail --retry "120" --retry-max-time "120" --retry-all-errors --ssl-reqd \
-              --cacert /etc/tls/certs/default/ca.crt \
+               \
               -X PUT -u ${USER_NAME}:${PASSWORD} \
               http://redpanda.default.svc.cluster.local.:9644/v1/debug/restart_service?service=schema-registry || true
           fi

diff --git a/charts/redpanda/testdata/ci/04-one-node-cluster-no-tls-sasl-values.yaml.golden b/charts/redpanda/testdata/ci/04-one-node-cluster-no-tls-sasl-values.yaml.golden
@@ -1184,7 +1184,7 @@ spec:
           if [ -d "/etc/secrets/users/" ]; then
               IFS=":" read -r USER_NAME PASSWORD MECHANISM < <(grep "" $(find /etc/secrets/users/* -print))
               curl -svm3 --fail --retry "120" --retry-max-time "120" --retry-all-errors --ssl-reqd \
-              --cacert /etc/tls/certs/default/ca.crt \
+               \
               -X PUT -u ${USER_NAME}:${PASSWORD} \
               http://redpanda.default.svc.cluster.local.:9644/v1/debug/restart_service?service=schema-registry || true
           fi

diff --git a/charts/redpanda/testdata/ci/14-prometheus-no-tls-values.yaml.golden b/charts/redpanda/testdata/ci/14-prometheus-no-tls-values.yaml.golden
@@ -1100,7 +1100,7 @@ spec:
           if [ -d "/etc/secrets/users/" ]; then
               IFS=":" read -r USER_NAME PASSWORD MECHANISM < <(grep "" $(find /etc/secrets/users/* -print))
               curl -svm3 --fail --retry "120" --retry-max-time "120" --retry-all-errors --ssl-reqd \
-              --cacert /etc/tls/certs/default/ca.crt \
+               \
               -X PUT -u ${USER_NAME}:${PASSWORD} \
               http://redpanda.default.svc.cluster.local.:9644/v1/debug/restart_service?service=schema-registry || true
           fi