Skip to content

Commit c32eeac

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) # Conflicts: # charts/redpanda/cert_issuers.go # charts/redpanda/certs.go # charts/redpanda/client/client_test.go # charts/redpanda/client_test.go # charts/redpanda/configmap.tpl.go # charts/redpanda/helpers.go # charts/redpanda/notes.go # charts/redpanda/render_state_nogotohelm.go # charts/redpanda/secrets.go # charts/redpanda/templates/_cert-issuers.go.tpl # charts/redpanda/templates/_certs.go.tpl # charts/redpanda/templates/_configmap.go.tpl # charts/redpanda/templates/_helpers.go.tpl # charts/redpanda/templates/_notes.go.tpl # charts/redpanda/templates/_secrets.go.tpl # charts/redpanda/templates/_values.go.tpl # charts/redpanda/testdata/template-cases.golden.txtar # charts/redpanda/values.go # gotohelm/transpiler.go # pkg/helm/helm.go
1 parent 0fd710c commit c32eeac

26 files changed

+3756
-263
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: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,25 @@ func certIssuersAndCAs(dot *helmette.Dot) ([]*certmanagerv1.Issuer, []*certmanag
3737
var issuers []*certmanagerv1.Issuer
3838
var certs []*certmanagerv1.Certificate
3939

40+
<<<<<<< HEAD
4041
if !TLSEnabled(dot) {
4142
return issuers, certs
4243
}
4344

4445
for name, data := range helmette.SortedMap(values.TLS.Certs) {
46+
=======
47+
inUseCerts := map[string]bool{}
48+
for _, name := range state.Values.Listeners.InUseServerCerts(&state.Values.TLS) {
49+
inUseCerts[name] = true
50+
}
51+
for _, name := range state.Values.Listeners.InUseClientCerts(&state.Values.TLS) {
52+
inUseCerts[name] = true
53+
}
54+
55+
for name := range helmette.SortedMap(inUseCerts) {
56+
data := state.Values.TLS.Certs.MustGet(name)
57+
58+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS)
4559
// If this certificate is disabled (.Enabled), provided directly by the
4660
// end user (.SecretRef), or has an issuer provided (.IssuerRef), we
4761
// don't need to bootstrap an issuer.
@@ -130,7 +144,11 @@ func certIssuersAndCAs(dot *helmette.Dot) ([]*certmanagerv1.Issuer, []*certmanag
130144
Spec: certmanagerv1.IssuerSpec{
131145
IssuerConfig: certmanagerv1.IssuerConfig{
132146
CA: &certmanagerv1.CAIssuer{
147+
<<<<<<< HEAD
133148
SecretName: fmt.Sprintf(`%s-%s-root-certificate`, Fullname(dot), name),
149+
=======
150+
SecretName: data.RootSecretName(state, name),
151+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS)
134152
},
135153
},
136154
},

charts/redpanda/certs.go

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/redpanda-data/redpanda-operator/gotohelm/helmette"
2323
)
2424

25+
<<<<<<< HEAD
2526
func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
2627
if !TLSEnabled(dot) {
2728
return []*certmanagerv1.Certificate{}
@@ -32,13 +33,27 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
3233
fullname := Fullname(dot)
3334
service := ServiceName(dot)
3435
ns := dot.Release.Namespace
36+
=======
37+
func ClientCerts(state *RenderState) []*certmanagerv1.Certificate {
38+
fullname := Fullname(state)
39+
service := ServiceName(state)
40+
ns := state.Release.Namespace
41+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS)
3542
// Trailing .'s don't play nice with TLS/SNI: https://datatracker.ietf.org/doc/html/rfc6066#section-3
3643
// So we trim it when generating certificates.
3744
domain := strings.TrimSuffix(values.ClusterDomain, ".")
3845

3946
var certs []*certmanagerv1.Certificate
47+
<<<<<<< HEAD
4048
for name, data := range helmette.SortedMap(values.TLS.Certs) {
4149
if !helmette.Empty(data.SecretRef) || !ptr.Deref(data.Enabled, true) {
50+
=======
51+
for _, name := range state.Values.Listeners.InUseServerCerts(&state.Values.TLS) {
52+
data := state.Values.TLS.Certs.MustGet(name)
53+
54+
// Don't generate server Certificates if a secret is provided.
55+
if !helmette.Empty(data.SecretRef) {
56+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS)
4257
continue
4358
}
4459

@@ -85,7 +100,7 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
85100
Duration: helmette.MustDuration(duration),
86101
IsCA: false,
87102
IssuerRef: issuerRef,
88-
SecretName: fmt.Sprintf("%s-%s-cert", fullname, name),
103+
SecretName: data.ServerSecretName(state, name),
89104
PrivateKey: &certmanagerv1.CertificatePrivateKey{
90105
Algorithm: "ECDSA",
91106
Size: 256,
@@ -94,6 +109,7 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
94109
})
95110
}
96111

112+
<<<<<<< HEAD
97113
name := values.Listeners.Kafka.TLS.Cert
98114

99115
data, ok := values.TLS.Certs[name]
@@ -104,20 +120,34 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
104120
if !helmette.Empty(data.SecretRef) || !ClientAuthRequired(dot) {
105121
return certs
106122
}
123+
=======
124+
for _, name := range state.Values.Listeners.InUseClientCerts(&state.Values.TLS) {
125+
data := state.Values.TLS.Certs.MustGet(name)
107126

108-
issuerRef := cmmetav1.ObjectReference{
109-
Group: "cert-manager.io",
110-
Kind: "Issuer",
111-
Name: fmt.Sprintf("%s-%s-root-issuer", fullname, name),
112-
}
127+
if data.SecretRef != nil && data.ClientSecretRef == nil {
128+
panic(fmt.Sprintf(".clientSecretRef MUST be set if .secretRef is set and require_client_auth is true: Cert %q", name))
129+
}
113130

114-
if data.IssuerRef != nil {
115-
issuerRef = *data.IssuerRef
116-
issuerRef.Group = "cert-manager.io"
117-
}
131+
// Don't generate a client Certificate if a client secret is provided.
132+
if data.ClientSecretRef != nil {
133+
continue
134+
}
135+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS)
136+
137+
issuerRef := cmmetav1.ObjectReference{
138+
Group: "cert-manager.io",
139+
Kind: "Issuer",
140+
Name: fmt.Sprintf("%s-%s-root-issuer", fullname, name),
141+
}
142+
143+
if data.IssuerRef != nil {
144+
issuerRef = *data.IssuerRef
145+
issuerRef.Group = "cert-manager.io"
146+
}
118147

119-
duration := helmette.Default("43800h", data.Duration)
148+
duration := helmette.Default("43800h", data.Duration)
120149

150+
<<<<<<< HEAD
121151
return append(certs, &certmanagerv1.Certificate{
122152
TypeMeta: metav1.TypeMeta{
123153
APIVersion: "cert-manager.io/v1",
@@ -135,8 +165,31 @@ func ClientCerts(dot *helmette.Dot) []*certmanagerv1.Certificate {
135165
PrivateKey: &certmanagerv1.CertificatePrivateKey{
136166
Algorithm: "ECDSA",
137167
Size: 256,
168+
=======
169+
certs = append(certs, &certmanagerv1.Certificate{
170+
TypeMeta: metav1.TypeMeta{
171+
APIVersion: "cert-manager.io/v1",
172+
Kind: "Certificate",
173+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS)
138174
},
139-
IssuerRef: issuerRef,
140-
},
141-
})
175+
ObjectMeta: metav1.ObjectMeta{
176+
Name: fmt.Sprintf("%s-%s-client", fullname, name),
177+
Namespace: state.Release.Namespace,
178+
Labels: FullLabels(state),
179+
},
180+
Spec: certmanagerv1.CertificateSpec{
181+
CommonName: fmt.Sprintf("%s--%s-client", fullname, name),
182+
Duration: helmette.MustDuration(duration),
183+
IsCA: false,
184+
SecretName: data.ClientSecretName(state, name),
185+
PrivateKey: &certmanagerv1.CertificatePrivateKey{
186+
Algorithm: "ECDSA",
187+
Size: 256,
188+
},
189+
IssuerRef: issuerRef,
190+
},
191+
})
192+
}
193+
194+
return certs
142195
}

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_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,104 @@ import (
2121
"github.com/redpanda-data/redpanda-operator/gotohelm/helmette"
2222
)
2323

24+
<<<<<<< HEAD:charts/redpanda/client/client_test.go
25+
=======
26+
func TestCertificates(t *testing.T) {
27+
cases := map[string]struct {
28+
Cert *TLSCert
29+
CertificateName string
30+
ExpectedRootCertName string
31+
ExpectedRootCertKey string
32+
ExpectedClientCertName string
33+
}{
34+
"default": {
35+
CertificateName: "default",
36+
ExpectedRootCertName: "redpanda-default-root-certificate",
37+
ExpectedRootCertKey: "tls.crt",
38+
ExpectedClientCertName: "redpanda-default-client-cert",
39+
},
40+
"default with non-enabled global cert": {
41+
Cert: &TLSCert{
42+
Enabled: ptr.To(false),
43+
SecretRef: &corev1.LocalObjectReference{
44+
Name: "some-cert",
45+
},
46+
},
47+
CertificateName: "default",
48+
ExpectedRootCertName: "redpanda-default-root-certificate",
49+
ExpectedRootCertKey: "tls.crt",
50+
ExpectedClientCertName: "redpanda-default-client-cert",
51+
},
52+
"certificate with secret ref": {
53+
Cert: &TLSCert{
54+
SecretRef: &corev1.LocalObjectReference{
55+
Name: "some-cert",
56+
},
57+
},
58+
CertificateName: "default",
59+
ExpectedRootCertName: "some-cert",
60+
ExpectedRootCertKey: "tls.crt",
61+
ExpectedClientCertName: "redpanda-default-client-cert",
62+
},
63+
"certificate with CA": {
64+
Cert: &TLSCert{
65+
CAEnabled: true,
66+
SecretRef: &corev1.LocalObjectReference{
67+
Name: "some-cert",
68+
},
69+
},
70+
CertificateName: "default",
71+
ExpectedRootCertName: "some-cert",
72+
ExpectedRootCertKey: "ca.crt",
73+
ExpectedClientCertName: "redpanda-default-client-cert",
74+
},
75+
"certificate with client certificate": {
76+
Cert: &TLSCert{
77+
CAEnabled: true,
78+
SecretRef: &corev1.LocalObjectReference{
79+
Name: "some-cert",
80+
},
81+
ClientSecretRef: &corev1.LocalObjectReference{
82+
Name: "client-cert",
83+
},
84+
},
85+
CertificateName: "default",
86+
ExpectedRootCertName: "some-cert",
87+
ExpectedRootCertKey: "ca.crt",
88+
ExpectedClientCertName: "client-cert",
89+
},
90+
}
91+
92+
for name, c := range cases {
93+
t.Run(name, func(t *testing.T) {
94+
certMap := TLSCertMap{}
95+
96+
if c.Cert != nil {
97+
certMap[c.CertificateName] = *c.Cert
98+
}
99+
100+
dot, err := Chart.Dot(nil, helmette.Release{
101+
Name: "redpanda",
102+
Namespace: "redpanda",
103+
Service: "Helm",
104+
}, Values{
105+
TLS: TLS{
106+
Certs: certMap,
107+
},
108+
})
109+
require.NoError(t, err)
110+
state, err := RenderStateFromDot(dot)
111+
require.NoError(t, err)
112+
113+
actualRootCertName, actualRootCertKey, actualClientCertName := certificatesFor(state, c.CertificateName)
114+
require.Equal(t, c.ExpectedRootCertName, actualRootCertName)
115+
require.Equal(t, c.ExpectedRootCertKey, actualRootCertKey)
116+
require.Equal(t, c.ExpectedClientCertName, actualClientCertName)
117+
})
118+
}
119+
}
120+
121+
>>>>>>> 6c63e57d (charts/redpanda: fix mTLS):charts/redpanda/render_state_test.go
24122
func TestFirstUser(t *testing.T) {
25123
cases := []struct {
26124
In string

0 commit comments

Comments
 (0)