Skip to content

Commit 497d77c

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) # 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
1 parent 3c5ad97 commit 497d77c

Some content is hidden

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

47 files changed

+3034
-2704
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
@@ -807,26 +807,50 @@ func httpProxyListenerTest(ctx context.Context, rpk *Client) error {
807807

808808
func mTLSValuesUsingCertManager() *redpanda.PartialValues {
809809
return minimalValues(&redpanda.PartialValues{
810+
TLS: &redpanda.PartialTLS{
811+
Certs: redpanda.PartialTLSCertMap{
812+
"kafka": redpanda.PartialTLSCert{
813+
Enabled: ptr.To(true),
814+
CAEnabled: ptr.To(true),
815+
},
816+
"http": redpanda.PartialTLSCert{
817+
Enabled: ptr.To(true),
818+
CAEnabled: ptr.To(true),
819+
},
820+
"rpc": redpanda.PartialTLSCert{
821+
Enabled: ptr.To(true),
822+
CAEnabled: ptr.To(true),
823+
},
824+
"schema": redpanda.PartialTLSCert{
825+
Enabled: ptr.To(true),
826+
CAEnabled: ptr.To(true),
827+
},
828+
},
829+
},
810830
External: &redpanda.PartialExternalConfig{Enabled: ptr.To(false)},
811831
ClusterDomain: ptr.To("cluster.local"),
812832
Listeners: &redpanda.PartialListeners{
813833
Admin: &redpanda.PartialListenerConfig[redpanda.NoAuth]{
814834
TLS: &redpanda.PartialInternalTLS{
835+
// Uses default by default.
815836
RequireClientAuth: ptr.To(true),
816837
},
817838
},
818839
HTTP: &redpanda.PartialListenerConfig[redpanda.HTTPAuthenticationMethod]{
819840
TLS: &redpanda.PartialInternalTLS{
841+
Cert: ptr.To("http"),
820842
RequireClientAuth: ptr.To(true),
821843
},
822844
},
823845
Kafka: &redpanda.PartialListenerConfig[redpanda.KafkaAuthenticationMethod]{
824846
TLS: &redpanda.PartialInternalTLS{
847+
Cert: ptr.To("kafka"),
825848
RequireClientAuth: ptr.To(true),
826849
},
827850
},
828851
SchemaRegistry: &redpanda.PartialListenerConfig[redpanda.NoAuth]{
829852
TLS: &redpanda.PartialInternalTLS{
853+
Cert: ptr.To("schema"),
830854
RequireClientAuth: ptr.To(true),
831855
},
832856
},
@@ -835,6 +859,7 @@ func mTLSValuesUsingCertManager() *redpanda.PartialValues {
835859
TLS *redpanda.PartialInternalTLS `json:"tls,omitempty" jsonschema:"required"`
836860
}{
837861
TLS: &redpanda.PartialInternalTLS{
862+
Cert: ptr.To("rpc"),
838863
RequireClientAuth: ptr.To(true),
839864
},
840865
},

‎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/v25"
@@ -301,35 +302,21 @@ func authFromDot(dot *helmette.Dot) (username string, password string, mechanism
301302
return
302303
}
303304

304-
func certificatesFor(dot *helmette.Dot, cert string) (certSecret, certKey, clientSecret string) {
305+
func certificatesFor(dot *helmette.Dot, name string) (certSecret, certKey, clientSecret string) {
305306
values := helmette.Unwrap[redpanda.Values](dot.Values)
306307

307-
name := redpanda.Fullname(dot)
308+
cert, ok := values.TLS.Certs[name]
309+
if !ok || !ptr.Deref(cert.Enabled, true) {
310+
// TODO this isn't correct but it matches historical behavior.
311+
fullname := redpanda.Fullname(dot)
312+
certSecret = fmt.Sprintf("%s-%s-root-certificate", fullname, name)
313+
clientSecret = fmt.Sprintf("%s-default-client-cert", fullname)
308314

309-
// default to cert manager issued names and tls.crt which is
310-
// where cert-manager outputs the root CA
311-
certKey = corev1.TLSCertKey
312-
certSecret = fmt.Sprintf("%s-%s-root-certificate", name, cert)
313-
clientSecret = fmt.Sprintf("%s-client", name)
314-
315-
if certificate, ok := values.TLS.Certs[cert]; ok {
316-
// if this references a non-enabled certificate, just return
317-
// the default cert-manager issued names
318-
if certificate.Enabled != nil && !*certificate.Enabled {
319-
return certSecret, certKey, clientSecret
320-
}
321-
322-
if certificate.ClientSecretRef != nil {
323-
clientSecret = certificate.ClientSecretRef.Name
324-
}
325-
if certificate.SecretRef != nil {
326-
certSecret = certificate.SecretRef.Name
327-
if certificate.CAEnabled {
328-
certKey = "ca.crt"
329-
}
330-
}
315+
return certSecret, corev1.TLSCertKey, clientSecret
331316
}
332-
return certSecret, certKey, clientSecret
317+
318+
ref := cert.CASecretRef(dot, name)
319+
return ref.LocalObjectReference.Name, ref.Key, cert.ClientSecretName(dot, name)
333320
}
334321

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

‎charts/redpanda/client/client_test.go‎

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

24-
func TestFirstUser(t *testing.T) {
25-
cases := []struct {
26-
In string
27-
Out [3]string
28-
}{
29-
{
30-
In: "hello:world:SCRAM-SHA-256",
31-
Out: [3]string{"hello", "world", "SCRAM-SHA-256"},
32-
},
33-
{
34-
In: "name:password\n#Intentionally Blank\n",
35-
Out: [3]string{"name", "password", "SCRAM-SHA-512"},
36-
},
37-
{
38-
In: "name:password:SCRAM-MD5-999",
39-
Out: [3]string{"", "", ""},
40-
},
41-
}
42-
43-
for _, c := range cases {
44-
user, password, mechanism := firstUser([]byte(c.In))
45-
assert.Equal(t, [3]string{user, password, mechanism}, c.Out)
46-
}
47-
}
48-
4924
func TestCertificates(t *testing.T) {
5025
cases := map[string]struct {
5126
Cert *redpanda.TLSCert
@@ -58,7 +33,7 @@ func TestCertificates(t *testing.T) {
5833
CertificateName: "default",
5934
ExpectedRootCertName: "redpanda-default-root-certificate",
6035
ExpectedRootCertKey: "tls.crt",
61-
ExpectedClientCertName: "redpanda-client",
36+
ExpectedClientCertName: "redpanda-default-client-cert",
6237
},
6338
"default with non-enabled global cert": {
6439
Cert: &redpanda.TLSCert{
@@ -70,7 +45,7 @@ func TestCertificates(t *testing.T) {
7045
CertificateName: "default",
7146
ExpectedRootCertName: "redpanda-default-root-certificate",
7247
ExpectedRootCertKey: "tls.crt",
73-
ExpectedClientCertName: "redpanda-client",
48+
ExpectedClientCertName: "redpanda-default-client-cert",
7449
},
7550
"certificate with secret ref": {
7651
Cert: &redpanda.TLSCert{
@@ -81,7 +56,7 @@ func TestCertificates(t *testing.T) {
8156
CertificateName: "default",
8257
ExpectedRootCertName: "some-cert",
8358
ExpectedRootCertKey: "tls.crt",
84-
ExpectedClientCertName: "redpanda-client",
59+
ExpectedClientCertName: "redpanda-default-client-cert",
8560
},
8661
"certificate with CA": {
8762
Cert: &redpanda.TLSCert{
@@ -93,7 +68,7 @@ func TestCertificates(t *testing.T) {
9368
CertificateName: "default",
9469
ExpectedRootCertName: "some-cert",
9570
ExpectedRootCertKey: "ca.crt",
96-
ExpectedClientCertName: "redpanda-client",
71+
ExpectedClientCertName: "redpanda-default-client-cert",
9772
},
9873
"certificate with client certificate": {
9974
Cert: &redpanda.TLSCert{
@@ -138,3 +113,28 @@ func TestCertificates(t *testing.T) {
138113
})
139114
}
140115
}
116+
117+
func TestFirstUser(t *testing.T) {
118+
cases := []struct {
119+
In string
120+
Out [3]string
121+
}{
122+
{
123+
In: "hello:world:SCRAM-SHA-256",
124+
Out: [3]string{"hello", "world", "SCRAM-SHA-256"},
125+
},
126+
{
127+
In: "name:password\n#Intentionally Blank\n",
128+
Out: [3]string{"name", "password", "SCRAM-SHA-512"},
129+
},
130+
{
131+
In: "name:password:SCRAM-MD5-999",
132+
Out: [3]string{"", "", ""},
133+
},
134+
}
135+
136+
for _, c := range cases {
137+
user, password, mechanism := firstUser([]byte(c.In))
138+
assert.Equal(t, [3]string{user, password, mechanism}, c.Out)
139+
}
140+
}

0 commit comments

Comments
 (0)