Skip to content

Commit

Permalink
Fix TLS params for clients (#1597)
Browse files Browse the repository at this point in the history 
* Fix up TLS initialization for clients

* Add tests
  • Loading branch information
@andrewstucki
andrewstucki authored Nov 13, 2024
1 parent c7c21fe commit 230a32a
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 11 deletions.
53 changes: 42 additions & 11 deletions pkg/redpanda/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ func AdminClient(dot *helmette.Dot, dialer DialContextFunc) (*rpadmin.AdminAPI,
var tlsConfig *tls.Config
var err error

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

tlsConfig, err = tlsConfigFromDot(dot, values.Listeners.Admin.TLS.Cert)
tlsConfig, err = tlsConfigFromDot(dot, values.Listeners.Admin.TLS)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -113,10 +113,10 @@ func SchemaRegistryClient(dot *helmette.Dot, dialer DialContextFunc, opts ...sr.
}).DialContext
}

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

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

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

if redpanda.TLSEnabled(dot) {
tlsConfig, err := tlsConfigFromDot(dot, values.Listeners.Kafka.TLS.Cert)
if values.Listeners.Kafka.TLS.IsEnabled(&values.TLS) {
tlsConfig, err := tlsConfigFromDot(dot, values.Listeners.Kafka.TLS)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -237,14 +237,44 @@ func authFromDot(dot *helmette.Dot) (username string, password string, mechanism
return
}

func tlsConfigFromDot(dot *helmette.Dot, cert string) (*tls.Config, error) {
func certificatesFor(dot *helmette.Dot, cert string) (certSecret, certKey, clientSecret string) {
values := helmette.Unwrap[redpanda.Values](dot.Values)

name := redpanda.Fullname(dot)

// default to cert manager issued names and tls.crt which is
// where cert-manager outputs the root CA
certKey = corev1.TLSCertKey
certSecret = fmt.Sprintf("%s-%s-root-certificate", name, cert)
clientSecret = fmt.Sprintf("%s-client", name)

if certificate, ok := values.TLS.Certs[cert]; ok {
// if this references a non-enabled certificate, just return
// the default cert-manager issued names
if certificate.Enabled != nil && !*certificate.Enabled {
return certSecret, certKey, clientSecret
}

if certificate.ClientSecretRef != nil {
clientSecret = certificate.ClientSecretRef.Name
}
if certificate.SecretRef != nil {
certSecret = certificate.SecretRef.Name
if certificate.CAEnabled {
certKey = "ca.crt"
}
}
}
return certSecret, certKey, clientSecret
}

func tlsConfigFromDot(dot *helmette.Dot, listener redpanda.InternalTLS) (*tls.Config, error) {
namespace := dot.Release.Namespace
serviceName := redpanda.ServiceName(dot)
clientCertName := fmt.Sprintf("%s-client", name)
rootCertName := fmt.Sprintf("%s-%s-root-certificate", name, cert)
serverName := fmt.Sprintf("%s.%s.svc", serviceName, namespace)

rootCertName, rootCertKey, clientCertName := certificatesFor(dot, listener.Cert)

serverTLSError := func(err error) error {
return fmt.Errorf("error fetching server root CA %s/%s: %w", namespace, rootCertName, err)
}
Expand All @@ -263,7 +293,7 @@ func tlsConfigFromDot(dot *helmette.Dot, cert string) (*tls.Config, error) {
return nil, serverTLSError(ErrServerCertificateNotFound)
}

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

tlsConfig.RootCAs = pool

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

// we always use tls.crt for client certs
clientPublicKey, found := clientCert.Data[corev1.TLSCertKey]
if !found {
return nil, clientTLSError(ErrClientCertificatePublicKeyNotFound)
Expand Down
99 changes: 99 additions & 0 deletions pkg/redpanda/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ package redpanda
import (
"testing"

"github.com/redpanda-data/helm-charts/charts/redpanda"
"github.com/redpanda-data/helm-charts/pkg/gotohelm/helmette"
"github.com/redpanda-data/helm-charts/pkg/kube"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
"k8s.io/utils/ptr"
)

func TestFirstUser(t *testing.T) {
Expand All @@ -30,3 +36,96 @@ func TestFirstUser(t *testing.T) {
assert.Equal(t, [3]string{user, password, mechanism}, c.Out)
}
}

func TestCertificates(t *testing.T) {
cases := map[string]struct {
Cert *redpanda.TLSCert
CertificateName string
ExpectedRootCertName string
ExpectedRootCertKey string
ExpectedClientCertName string
}{
"default": {
CertificateName: "default",
ExpectedRootCertName: "redpanda-default-root-certificate",
ExpectedRootCertKey: "tls.crt",
ExpectedClientCertName: "redpanda-client",
},
"default with non-enabled global cert": {
Cert: &redpanda.TLSCert{
Enabled: ptr.To(false),
SecretRef: &v1.LocalObjectReference{
Name: "some-cert",
},
},
CertificateName: "default",
ExpectedRootCertName: "redpanda-default-root-certificate",
ExpectedRootCertKey: "tls.crt",
ExpectedClientCertName: "redpanda-client",
},
"certificate with secret ref": {
Cert: &redpanda.TLSCert{
SecretRef: &v1.LocalObjectReference{
Name: "some-cert",
},
},
CertificateName: "default",
ExpectedRootCertName: "some-cert",
ExpectedRootCertKey: "tls.crt",
ExpectedClientCertName: "redpanda-client",
},
"certificate with CA": {
Cert: &redpanda.TLSCert{
CAEnabled: true,
SecretRef: &v1.LocalObjectReference{
Name: "some-cert",
},
},
CertificateName: "default",
ExpectedRootCertName: "some-cert",
ExpectedRootCertKey: "ca.crt",
ExpectedClientCertName: "redpanda-client",
},
"certificate with client certificate": {
Cert: &redpanda.TLSCert{
CAEnabled: true,
SecretRef: &v1.LocalObjectReference{
Name: "some-cert",
},
ClientSecretRef: &v1.LocalObjectReference{
Name: "client-cert",
},
},
CertificateName: "default",
ExpectedRootCertName: "some-cert",
ExpectedRootCertKey: "ca.crt",
ExpectedClientCertName: "client-cert",
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
certMap := redpanda.TLSCertMap{}

if c.Cert != nil {
certMap[c.CertificateName] = *c.Cert
}

dot, err := redpanda.Chart.Dot(kube.Config{}, helmette.Release{
Name: "redpanda",
Namespace: "redpanda",
Service: "Helm",
}, redpanda.Values{
TLS: redpanda.TLS{
Certs: certMap,
},
})
require.NoError(t, err)

actualRootCertName, actualRootCertKey, actualClientCertName := certificatesFor(dot, c.CertificateName)
require.Equal(t, c.ExpectedRootCertName, actualRootCertName)
require.Equal(t, c.ExpectedRootCertKey, actualRootCertKey)
require.Equal(t, c.ExpectedClientCertName, actualClientCertName)
})
}
}

0 comments on commit 230a32a

Please sign in to comment.