Skip to content

Commit d36b90c

Browse files
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 0fd710c commit d36b90c

Some content is hidden

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

45 files changed

+1002
-2958
lines changed
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
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
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
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
@@ -29,6 +29,7 @@ import (
2929
"github.com/twmb/franz-go/pkg/sr"
3030
corev1 "k8s.io/api/core/v1"
3131
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32+
"k8s.io/utils/ptr"
3233
"sigs.k8s.io/controller-runtime/pkg/client"
3334

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

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

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

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

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

0 commit comments

Comments
 (0)