Skip to content

Commit 230a32a

Browse files
andrewstuckiandrewstucki
authored
Fix TLS params for clients (#1597)
* Fix up TLS initialization for clients * Add tests
1 parent c7c21fe commit 230a32a

File tree

2 files changed

+141
-11
lines changed

2 files changed

+141
-11
lines changed

‎pkg/redpanda/client.go

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ func AdminClient(dot *helmette.Dot, dialer DialContextFunc) (*rpadmin.AdminAPI,
5656
var tlsConfig *tls.Config
5757
var err error
5858

59-
if redpanda.TLSEnabled(dot) {
59+
if values.Listeners.Admin.TLS.IsEnabled(&values.TLS) {
6060
prefix = "https://"
6161

62-
tlsConfig, err = tlsConfigFromDot(dot, values.Listeners.Admin.TLS.Cert)
62+
tlsConfig, err = tlsConfigFromDot(dot, values.Listeners.Admin.TLS)
6363
if err != nil {
6464
return nil, err
6565
}
@@ -113,10 +113,10 @@ func SchemaRegistryClient(dot *helmette.Dot, dialer DialContextFunc, opts ...sr.
113113
}).DialContext
114114
}
115115

116-
if redpanda.TLSEnabled(dot) {
116+
if values.Listeners.SchemaRegistry.TLS.IsEnabled(&values.TLS) {
117117
prefix = "https://"
118118

119-
tlsConfig, err := tlsConfigFromDot(dot, values.Listeners.SchemaRegistry.TLS.Cert)
119+
tlsConfig, err := tlsConfigFromDot(dot, values.Listeners.SchemaRegistry.TLS)
120120
if err != nil {
121121
return nil, err
122122
}
@@ -156,8 +156,8 @@ func KafkaClient(dot *helmette.Dot, dialer DialContextFunc, opts ...kgo.Opt) (*k
156156

157157
opts = append(opts, kgo.SeedBrokers(brokers...))
158158

159-
if redpanda.TLSEnabled(dot) {
160-
tlsConfig, err := tlsConfigFromDot(dot, values.Listeners.Kafka.TLS.Cert)
159+
if values.Listeners.Kafka.TLS.IsEnabled(&values.TLS) {
160+
tlsConfig, err := tlsConfigFromDot(dot, values.Listeners.Kafka.TLS)
161161
if err != nil {
162162
return nil, err
163163
}
@@ -237,14 +237,44 @@ func authFromDot(dot *helmette.Dot) (username string, password string, mechanism
237237
return
238238
}
239239

240-
func tlsConfigFromDot(dot *helmette.Dot, cert string) (*tls.Config, error) {
240+
func certificatesFor(dot *helmette.Dot, cert string) (certSecret, certKey, clientSecret string) {
241+
values := helmette.Unwrap[redpanda.Values](dot.Values)
242+
241243
name := redpanda.Fullname(dot)
244+
245+
// default to cert manager issued names and tls.crt which is
246+
// where cert-manager outputs the root CA
247+
certKey = corev1.TLSCertKey
248+
certSecret = fmt.Sprintf("%s-%s-root-certificate", name, cert)
249+
clientSecret = fmt.Sprintf("%s-client", name)
250+
251+
if certificate, ok := values.TLS.Certs[cert]; ok {
252+
// if this references a non-enabled certificate, just return
253+
// the default cert-manager issued names
254+
if certificate.Enabled != nil && !*certificate.Enabled {
255+
return certSecret, certKey, clientSecret
256+
}
257+
258+
if certificate.ClientSecretRef != nil {
259+
clientSecret = certificate.ClientSecretRef.Name
260+
}
261+
if certificate.SecretRef != nil {
262+
certSecret = certificate.SecretRef.Name
263+
if certificate.CAEnabled {
264+
certKey = "ca.crt"
265+
}
266+
}
267+
}
268+
return certSecret, certKey, clientSecret
269+
}
270+
271+
func tlsConfigFromDot(dot *helmette.Dot, listener redpanda.InternalTLS) (*tls.Config, error) {
242272
namespace := dot.Release.Namespace
243273
serviceName := redpanda.ServiceName(dot)
244-
clientCertName := fmt.Sprintf("%s-client", name)
245-
rootCertName := fmt.Sprintf("%s-%s-root-certificate", name, cert)
246274
serverName := fmt.Sprintf("%s.%s.svc", serviceName, namespace)
247275

276+
rootCertName, rootCertKey, clientCertName := certificatesFor(dot, listener.Cert)
277+
248278
serverTLSError := func(err error) error {
249279
return fmt.Errorf("error fetching server root CA %s/%s: %w", namespace, rootCertName, err)
250280
}
@@ -263,7 +293,7 @@ func tlsConfigFromDot(dot *helmette.Dot, cert string) (*tls.Config, error) {
263293
return nil, serverTLSError(ErrServerCertificateNotFound)
264294
}
265295

266-
serverPublicKey, found := serverCert.Data[corev1.TLSCertKey]
296+
serverPublicKey, found := serverCert.Data[rootCertKey]
267297
if !found {
268298
return nil, serverTLSError(ErrServerCertificatePublicKeyNotFound)
269299
}
@@ -278,7 +308,7 @@ func tlsConfigFromDot(dot *helmette.Dot, cert string) (*tls.Config, error) {
278308

279309
tlsConfig.RootCAs = pool
280310

281-
if redpanda.ClientAuthRequired(dot) {
311+
if listener.RequireClientAuth {
282312
clientCert, found, lookupErr := helmette.SafeLookup[corev1.Secret](dot, namespace, clientCertName)
283313
if lookupErr != nil {
284314
return nil, clientTLSError(lookupErr)
@@ -288,6 +318,7 @@ func tlsConfigFromDot(dot *helmette.Dot, cert string) (*tls.Config, error) {
288318
return nil, clientTLSError(ErrServerCertificateNotFound)
289319
}
290320

321+
// we always use tls.crt for client certs
291322
clientPublicKey, found := clientCert.Data[corev1.TLSCertKey]
292323
if !found {
293324
return nil, clientTLSError(ErrClientCertificatePublicKeyNotFound)

‎pkg/redpanda/client_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ package redpanda
33
import (
44
"testing"
55

6+
"github.com/redpanda-data/helm-charts/charts/redpanda"
7+
"github.com/redpanda-data/helm-charts/pkg/gotohelm/helmette"
8+
"github.com/redpanda-data/helm-charts/pkg/kube"
69
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
v1 "k8s.io/api/core/v1"
12+
"k8s.io/utils/ptr"
713
)
814

915
func TestFirstUser(t *testing.T) {
@@ -30,3 +36,96 @@ func TestFirstUser(t *testing.T) {
3036
assert.Equal(t, [3]string{user, password, mechanism}, c.Out)
3137
}
3238
}
39+
40+
func TestCertificates(t *testing.T) {
41+
cases := map[string]struct {
42+
Cert *redpanda.TLSCert
43+
CertificateName string
44+
ExpectedRootCertName string
45+
ExpectedRootCertKey string
46+
ExpectedClientCertName string
47+
}{
48+
"default": {
49+
CertificateName: "default",
50+
ExpectedRootCertName: "redpanda-default-root-certificate",
51+
ExpectedRootCertKey: "tls.crt",
52+
ExpectedClientCertName: "redpanda-client",
53+
},
54+
"default with non-enabled global cert": {
55+
Cert: &redpanda.TLSCert{
56+
Enabled: ptr.To(false),
57+
SecretRef: &v1.LocalObjectReference{
58+
Name: "some-cert",
59+
},
60+
},
61+
CertificateName: "default",
62+
ExpectedRootCertName: "redpanda-default-root-certificate",
63+
ExpectedRootCertKey: "tls.crt",
64+
ExpectedClientCertName: "redpanda-client",
65+
},
66+
"certificate with secret ref": {
67+
Cert: &redpanda.TLSCert{
68+
SecretRef: &v1.LocalObjectReference{
69+
Name: "some-cert",
70+
},
71+
},
72+
CertificateName: "default",
73+
ExpectedRootCertName: "some-cert",
74+
ExpectedRootCertKey: "tls.crt",
75+
ExpectedClientCertName: "redpanda-client",
76+
},
77+
"certificate with CA": {
78+
Cert: &redpanda.TLSCert{
79+
CAEnabled: true,
80+
SecretRef: &v1.LocalObjectReference{
81+
Name: "some-cert",
82+
},
83+
},
84+
CertificateName: "default",
85+
ExpectedRootCertName: "some-cert",
86+
ExpectedRootCertKey: "ca.crt",
87+
ExpectedClientCertName: "redpanda-client",
88+
},
89+
"certificate with client certificate": {
90+
Cert: &redpanda.TLSCert{
91+
CAEnabled: true,
92+
SecretRef: &v1.LocalObjectReference{
93+
Name: "some-cert",
94+
},
95+
ClientSecretRef: &v1.LocalObjectReference{
96+
Name: "client-cert",
97+
},
98+
},
99+
CertificateName: "default",
100+
ExpectedRootCertName: "some-cert",
101+
ExpectedRootCertKey: "ca.crt",
102+
ExpectedClientCertName: "client-cert",
103+
},
104+
}
105+
106+
for name, c := range cases {
107+
t.Run(name, func(t *testing.T) {
108+
certMap := redpanda.TLSCertMap{}
109+
110+
if c.Cert != nil {
111+
certMap[c.CertificateName] = *c.Cert
112+
}
113+
114+
dot, err := redpanda.Chart.Dot(kube.Config{}, helmette.Release{
115+
Name: "redpanda",
116+
Namespace: "redpanda",
117+
Service: "Helm",
118+
}, redpanda.Values{
119+
TLS: redpanda.TLS{
120+
Certs: certMap,
121+
},
122+
})
123+
require.NoError(t, err)
124+
125+
actualRootCertName, actualRootCertKey, actualClientCertName := certificatesFor(dot, c.CertificateName)
126+
require.Equal(t, c.ExpectedRootCertName, actualRootCertName)
127+
require.Equal(t, c.ExpectedRootCertKey, actualRootCertKey)
128+
require.Equal(t, c.ExpectedClientCertName, actualClientCertName)
129+
})
130+
}
131+
}

0 commit comments

Comments
 (0)