Skip to content

Commit 6f032de

Browse files
chrissetochrisseto
committed
charts/redpanda: fix mTLS
Prior to this commit the chart had a variety of bugs around mTLS. The majority of them were incorrect path construction and handling of `.clientSecretRef`. The primary issue, though, is that the chart incorrectly mints a single client certificate regardless of how many trust chains are in use. This commit moves all name and path references into helper methods onto the `TLSCert` itself and generates client certs per unique trust chain with client auth enabled. K8S-719 (cherry picked from commit 6c63e57)
1 parent d355ad0 commit 6f032de

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+1006
-2962
lines changed

‎.changes/unreleased/charts-redpanda-Changed-20250918-152741.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
project: charts/redpanda
2+
kind: Changed
3+
body: Client certificates are now named `$FULLNAME-$CERT-client-cert`.
4+
time: 2025-09-18T15:27:41.700988-04:00

‎.changes/unreleased/charts-redpanda-Fixed-20250918-152623.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
project: charts/redpanda
2+
kind: Fixed
3+
body: mTLS client certificates are now generated per certificate, as required, instead of using a single and potentially invalid certificate.
4+
time: 2025-09-18T15:26:23.232523-04:00

‎.changes/unreleased/operator-Changed-20250918-152741.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
project: operator
2+
kind: Changed
3+
body: Client certificates are now named `$FULLNAME-$CERT-client-cert`.
4+
time: 2025-09-18T15:27:41.700988-04:00

‎.changes/unreleased/operator-Fixed-20250918-152623.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
project: operator
2+
kind: Fixed
3+
body: mTLS client certificates are now generated per certificate, as required, instead of using a single and potentially invalid certificate.
4+
time: 2025-09-18T15:26:23.232523-04:00

‎charts/redpanda/cert_issuers.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,17 @@ func certIssuersAndCAs(dot *helmette.Dot) ([]*certmanagerv1.Issuer, []*certmanag
3737
var issuers []*certmanagerv1.Issuer
3838
var certs []*certmanagerv1.Certificate
3939

40-
if !TLSEnabled(dot) {
41-
return issuers, certs
40+
inUseCerts := map[string]bool{}
41+
for _, name := range values.Listeners.InUseServerCerts(&values.TLS) {
42+
inUseCerts[name] = true
4243
}
44+
for _, name := range values.Listeners.InUseClientCerts(&values.TLS) {
45+
inUseCerts[name] = true
46+
}
47+
48+
for name := range helmette.SortedMap(inUseCerts) {
49+
data := values.TLS.Certs.MustGet(name)
4350

44-
for name, data := range helmette.SortedMap(values.TLS.Certs) {
4551
// If this certificate is disabled (.Enabled), provided directly by the
4652
// end user (.SecretRef), or has an issuer provided (.IssuerRef), we
4753
// don't need to bootstrap an issuer.
@@ -130,7 +136,7 @@ func certIssuersAndCAs(dot *helmette.Dot) ([]*certmanagerv1.Issuer, []*certmanag
130136
Spec: certmanagerv1.IssuerSpec{
131137
IssuerConfig: certmanagerv1.IssuerConfig{
132138
CA: &certmanagerv1.CAIssuer{
133-
SecretName: fmt.Sprintf(`%s-%s-root-certificate`, Fullname(dot), name),
139+
SecretName: data.RootSecretName(dot, name),
134140
},
135141
},
136142
},

‎charts/redpanda/certs.go‎

Lines changed: 50 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ import (
2323
)
2424

2525
func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
26-
if !TLSEnabled(dot) {
27-
return []*certmanagerv1.Certificate{}
28-
}
29-
3026
values := helmette.Unwrap[Values](dot.Values)
3127

3228
fullname := Fullname(dot)
@@ -37,8 +33,11 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
3733
domain := strings.TrimSuffix(values.ClusterDomain, ".")
3834

3935
var certs []*certmanagerv1.Certificate
40-
for name, data := range helmette.SortedMap(values.TLS.Certs) {
41-
if !helmette.Empty(data.SecretRef) || !ptr.Deref(data.Enabled, true) {
36+
for _, name := range values.Listeners.InUseServerCerts(&values.TLS) {
37+
data := values.TLS.Certs.MustGet(name)
38+
39+
// Don't generate server Certificates if a secret is provided.
40+
if !helmette.Empty(data.SecretRef) {
4241
continue
4342
}
4443

@@ -85,7 +84,7 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
8584
Duration: helmette.MustDuration(duration),
8685
IsCA: false,
8786
IssuerRef: issuerRef,
88-
SecretName: fmt.Sprintf("%s-%s-cert", fullname, name),
87+
SecretName: data.ServerSecretName(dot, name),
8988
PrivateKey: &certmanagerv1.CertificatePrivateKey{
9089
Algorithm: "ECDSA",
9190
Size: 256,
@@ -94,49 +93,54 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
9493
})
9594
}
9695

97-
name := values.Listeners.Kafka.TLS.Cert
96+
for _, name := range values.Listeners.InUseClientCerts(&values.TLS) {
97+
data := values.TLS.Certs.MustGet(name)
9898

99-
data, ok := values.TLS.Certs[name]
100-
if !ok {
101-
panic(fmt.Sprintf("Certificate %q referenced but not defined", name))
102-
}
99+
if data.SecretRef != nil && data.ClientSecretRef == nil {
100+
panic(fmt.Sprintf(".clientSecretRef MUST be set if .secretRef is set and require_client_auth is true: Cert %q", name))
101+
}
103102

104-
if !helmette.Empty(data.SecretRef) || !ClientAuthRequired(dot) {
105-
return certs
106-
}
103+
// Don't generate a client Certificate if a client secret is provided.
104+
if data.ClientSecretRef != nil {
105+
continue
106+
}
107107

108-
issuerRef := cmmetav1.ObjectReference{
109-
Group: "cert-manager.io",
110-
Kind: "Issuer",
111-
Name: fmt.Sprintf("%s-%s-root-issuer", fullname, name),
112-
}
108+
issuerRef := cmmetav1.ObjectReference{
109+
Group: "cert-manager.io",
110+
Kind: "Issuer",
111+
Name: fmt.Sprintf("%s-%s-root-issuer", fullname, name),
112+
}
113113

114-
if data.IssuerRef != nil {
115-
issuerRef = *data.IssuerRef
116-
issuerRef.Group = "cert-manager.io"
117-
}
114+
if data.IssuerRef != nil {
115+
issuerRef = *data.IssuerRef
116+
issuerRef.Group = "cert-manager.io"
117+
}
118+
119+
duration := helmette.Default("43800h", data.Duration)
118120

119-
duration := helmette.Default("43800h", data.Duration)
120-
121-
return append(certs, &certmanagerv1.Certificate{
122-
TypeMeta: metav1.TypeMeta{
123-
APIVersion: "cert-manager.io/v1",
124-
Kind: "Certificate",
125-
},
126-
ObjectMeta: metav1.ObjectMeta{
127-
Name: fmt.Sprintf("%s-client", fullname),
128-
Labels: FullLabels(dot),
129-
},
130-
Spec: certmanagerv1.CertificateSpec{
131-
CommonName: fmt.Sprintf("%s-client", fullname),
132-
Duration: helmette.MustDuration(duration),
133-
IsCA: false,
134-
SecretName: fmt.Sprintf("%s-client", fullname),
135-
PrivateKey: &certmanagerv1.CertificatePrivateKey{
136-
Algorithm: "ECDSA",
137-
Size: 256,
121+
certs = append(certs, &certmanagerv1.Certificate{
122+
TypeMeta: metav1.TypeMeta{
123+
APIVersion: "cert-manager.io/v1",
124+
Kind: "Certificate",
125+
},
126+
ObjectMeta: metav1.ObjectMeta{
127+
Name: fmt.Sprintf("%s-%s-client", fullname, name),
128+
Namespace: dot.Release.Namespace,
129+
Labels: FullLabels(dot),
130+
},
131+
Spec: certmanagerv1.CertificateSpec{
132+
CommonName: fmt.Sprintf("%s--%s-client", fullname, name),
133+
Duration: helmette.MustDuration(duration),
134+
IsCA: false,
135+
SecretName: data.ClientSecretName(dot, name),
136+
PrivateKey: &certmanagerv1.CertificatePrivateKey{
137+
Algorithm: "ECDSA",
138+
Size: 256,
139+
},
140+
IssuerRef: issuerRef,
138141
},
139-
IssuerRef: issuerRef,
140-
},
141-
})
142+
})
143+
}
144+
145+
return certs
142146
}

‎charts/redpanda/chart_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,26 +749,50 @@ func httpProxyListenerTest(ctx context.Context, rpk *Client) error {
749749

750750
func mTLSValuesUsingCertManager() *redpanda.PartialValues {
751751
return minimalValues(&redpanda.PartialValues{
752+
TLS: &redpanda.PartialTLS{
753+
Certs: redpanda.PartialTLSCertMap{
754+
"kafka": redpanda.PartialTLSCert{
755+
Enabled: ptr.To(true),
756+
CAEnabled: ptr.To(true),
757+
},
758+
"http": redpanda.PartialTLSCert{
759+
Enabled: ptr.To(true),
760+
CAEnabled: ptr.To(true),
761+
},
762+
"rpc": redpanda.PartialTLSCert{
763+
Enabled: ptr.To(true),
764+
CAEnabled: ptr.To(true),
765+
},
766+
"schema": redpanda.PartialTLSCert{
767+
Enabled: ptr.To(true),
768+
CAEnabled: ptr.To(true),
769+
},
770+
},
771+
},
752772
External: &redpanda.PartialExternalConfig{Enabled: ptr.To(false)},
753773
ClusterDomain: ptr.To("cluster.local"),
754774
Listeners: &redpanda.PartialListeners{
755775
Admin: &redpanda.PartialListenerConfig[redpanda.NoAuth]{
756776
TLS: &redpanda.PartialInternalTLS{
777+
// Uses default by default.
757778
RequireClientAuth: ptr.To(true),
758779
},
759780
},
760781
HTTP: &redpanda.PartialListenerConfig[redpanda.HTTPAuthenticationMethod]{
761782
TLS: &redpanda.PartialInternalTLS{
783+
Cert: ptr.To("http"),
762784
RequireClientAuth: ptr.To(true),
763785
},
764786
},
765787
Kafka: &redpanda.PartialListenerConfig[redpanda.KafkaAuthenticationMethod]{
766788
TLS: &redpanda.PartialInternalTLS{
789+
Cert: ptr.To("kafka"),
767790
RequireClientAuth: ptr.To(true),
768791
},
769792
},
770793
SchemaRegistry: &redpanda.PartialListenerConfig[redpanda.NoAuth]{
771794
TLS: &redpanda.PartialInternalTLS{
795+
Cert: ptr.To("schema"),
772796
RequireClientAuth: ptr.To(true),
773797
},
774798
},
@@ -777,6 +801,7 @@ func mTLSValuesUsingCertManager() *redpanda.PartialValues {
777801
TLS *redpanda.PartialInternalTLS `json:"tls,omitempty" jsonschema:"required"`
778802
}{
779803
TLS: &redpanda.PartialInternalTLS{
804+
Cert: ptr.To("rpc"),
780805
RequireClientAuth: ptr.To(true),
781806
},
782807
},

‎charts/redpanda/client/client.go‎

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/twmb/franz-go/pkg/sasl/scram"
2929
"github.com/twmb/franz-go/pkg/sr"
3030
corev1 "k8s.io/api/core/v1"
31+
"k8s.io/utils/ptr"
3132
"sigs.k8s.io/controller-runtime/pkg/client"
3233

3334
"github.com/redpanda-data/redpanda-operator/charts/redpanda/v5"
@@ -286,35 +287,21 @@ func authFromDot(dot *helmette.Dot) (username string, password string, mechanism
286287
return
287288
}
288289

289-
func certificatesFor(dot *helmette.Dot, cert string) (certSecret, certKey, clientSecret string) {
290+
func certificatesFor(dot *helmette.Dot, name string) (certSecret, certKey, clientSecret string) {
290291
values := helmette.Unwrap[redpanda.Values](dot.Values)
291292

292-
name := redpanda.Fullname(dot)
293+
cert, ok := values.TLS.Certs[name]
294+
if !ok || !ptr.Deref(cert.Enabled, true) {
295+
// TODO this isn't correct but it matches historical behavior.
296+
fullname := redpanda.Fullname(dot)
297+
certSecret = fmt.Sprintf("%s-%s-root-certificate", fullname, name)
298+
clientSecret = fmt.Sprintf("%s-default-client-cert", fullname)
293299

294-
// default to cert manager issued names and tls.crt which is
295-
// where cert-manager outputs the root CA
296-
certKey = corev1.TLSCertKey
297-
certSecret = fmt.Sprintf("%s-%s-root-certificate", name, cert)
298-
clientSecret = fmt.Sprintf("%s-client", name)
299-
300-
if certificate, ok := values.TLS.Certs[cert]; ok {
301-
// if this references a non-enabled certificate, just return
302-
// the default cert-manager issued names
303-
if certificate.Enabled != nil && !*certificate.Enabled {
304-
return certSecret, certKey, clientSecret
305-
}
306-
307-
if certificate.ClientSecretRef != nil {
308-
clientSecret = certificate.ClientSecretRef.Name
309-
}
310-
if certificate.SecretRef != nil {
311-
certSecret = certificate.SecretRef.Name
312-
if certificate.CAEnabled {
313-
certKey = "ca.crt"
314-
}
315-
}
300+
return certSecret, corev1.TLSCertKey, clientSecret
316301
}
317-
return certSecret, certKey, clientSecret
302+
303+
ref := cert.CASecretRef(dot, name)
304+
return ref.LocalObjectReference.Name, ref.Key, cert.ClientSecretName(dot, name)
318305
}
319306

320307
func tlsConfigFromDot(dot *helmette.Dot, listener redpanda.InternalTLS) (*tls.Config, error) {

‎charts/redpanda/client/client_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestCertificates(t *testing.T) {
5858
CertificateName: "default",
5959
ExpectedRootCertName: "redpanda-default-root-certificate",
6060
ExpectedRootCertKey: "tls.crt",
61-
ExpectedClientCertName: "redpanda-client",
61+
ExpectedClientCertName: "redpanda-default-client-cert",
6262
},
6363
"default with non-enabled global cert": {
6464
Cert: &redpanda.TLSCert{
@@ -70,7 +70,7 @@ func TestCertificates(t *testing.T) {
7070
CertificateName: "default",
7171
ExpectedRootCertName: "redpanda-default-root-certificate",
7272
ExpectedRootCertKey: "tls.crt",
73-
ExpectedClientCertName: "redpanda-client",
73+
ExpectedClientCertName: "redpanda-default-client-cert",
7474
},
7575
"certificate with secret ref": {
7676
Cert: &redpanda.TLSCert{
@@ -81,7 +81,7 @@ func TestCertificates(t *testing.T) {
8181
CertificateName: "default",
8282
ExpectedRootCertName: "some-cert",
8383
ExpectedRootCertKey: "tls.crt",
84-
ExpectedClientCertName: "redpanda-client",
84+
ExpectedClientCertName: "redpanda-default-client-cert",
8585
},
8686
"certificate with CA": {
8787
Cert: &redpanda.TLSCert{
@@ -93,7 +93,7 @@ func TestCertificates(t *testing.T) {
9393
CertificateName: "default",
9494
ExpectedRootCertName: "some-cert",
9595
ExpectedRootCertKey: "ca.crt",
96-
ExpectedClientCertName: "redpanda-client",
96+
ExpectedClientCertName: "redpanda-default-client-cert",
9797
},
9898
"certificate with client certificate": {
9999
Cert: &redpanda.TLSCert{

0 commit comments

Comments
 (0)