Skip to content

Commit

Permalink
refactor: remove duplicated tls code (#961)
Browse files Browse the repository at this point in the history 
* remove dup log

Signed-off-by: drivebyer <[email protected]>

* remove dup tsl replication code

Signed-off-by: drivebyer <[email protected]>

---------

Signed-off-by: drivebyer <[email protected]>
  • Loading branch information
@drivebyer
drivebyer authored Jun 2, 2024
1 parent 5554025 commit 0fd0e42
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 255 deletions.
62 changes: 26 additions & 36 deletions k8sutils/redis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,28 +371,23 @@ func configureRedisClient(client kubernetes.Interface, logger logr.Logger, cr *r
PodName: podName,
Namespace: cr.Namespace,
}
var redisClient *redis.Client

var err error
var pass string
if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
pass, err := getRedisPassword(client, logger, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
pass, err = getRedisPassword(client, logger, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
if err != nil {
logger.Error(err, "Error in getting redis password")
}
redisClient = redis.NewClient(&redis.Options{
Addr: getRedisServerAddress(client, logger, redisInfo, *cr.Spec.Port),
Password: pass,
DB: 0,
TLSConfig: getRedisTLSConfig(client, logger, cr, redisInfo),
})
} else {
redisClient = redis.NewClient(&redis.Options{
Addr: getRedisServerAddress(client, logger, redisInfo, *cr.Spec.Port),
Password: "",
DB: 0,
TLSConfig: getRedisTLSConfig(client, logger, cr, redisInfo),
})
}
return redisClient
}
opts := &redis.Options{
Addr: getRedisServerAddress(client, logger, redisInfo, *cr.Spec.Port),
Password: pass,
DB: 0,
}
if cr.Spec.TLS != nil {
opts.TLSConfig = getRedisTLSConfig(client, logger, cr.Namespace, cr.Spec.TLS.Secret.SecretName, redisInfo.PodName)
}
return redis.NewClient(opts)
}

// executeCommand will execute the commands in pod
Expand Down Expand Up @@ -498,28 +493,23 @@ func configureRedisReplicationClient(client kubernetes.Interface, logger logr.Lo
PodName: podName,
Namespace: cr.Namespace,
}
var redisClient *redis.Client

var err error
var pass string
if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
pass, err := getRedisPassword(client, logger, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
pass, err = getRedisPassword(client, logger, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
if err != nil {
logger.Error(err, "Error in getting redis password")
}
redisClient = redis.NewClient(&redis.Options{
Addr: getRedisServerAddress(client, logger, redisInfo, 6379),
Password: pass,
DB: 0,
TLSConfig: getRedisReplicationTLSConfig(client, logger, cr, redisInfo),
})
} else {
redisClient = redis.NewClient(&redis.Options{
Addr: getRedisServerAddress(client, logger, redisInfo, 6379),
Password: "",
DB: 0,
TLSConfig: getRedisReplicationTLSConfig(client, logger, cr, redisInfo),
})
}
return redisClient
}
opts := &redis.Options{
Addr: getRedisServerAddress(client, logger, redisInfo, 6379),
Password: pass,
DB: 0,
}
if cr.Spec.TLS != nil {
opts.TLSConfig = getRedisTLSConfig(client, logger, cr.Namespace, cr.Spec.TLS.Secret.SecretName, podName)
}
return redis.NewClient(opts)
}

// Get Redis nodes by it's role i.e. master, slave and sentinel
Expand Down
109 changes: 29 additions & 80 deletions k8sutils/secrets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"strings"

redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2"
"github.com/go-logr/logr"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand All @@ -33,90 +32,40 @@ func getRedisPassword(client kubernetes.Interface, logger logr.Logger, namespace
return "", nil
}

func getRedisTLSConfig(client kubernetes.Interface, logger logr.Logger, cr *redisv1beta2.RedisCluster, redisInfo RedisDetails) *tls.Config {
if cr.Spec.TLS != nil {
secret, err := client.CoreV1().Secrets(cr.Namespace).Get(context.TODO(), cr.Spec.TLS.Secret.SecretName, metav1.GetOptions{})
if err != nil {
logger.Error(err, "Failed in getting TLS secret for redis cluster")
logger.V(1).Error(err, "Failed in getting TLS secret for redis cluster", "secretName", cr.Spec.TLS.Secret.SecretName, "namespace", cr.Namespace, "redisClusterName", cr.Name)
return nil
}

tlsClientCert, certExists := secret.Data["tls.crt"]
tlsClientKey, keyExists := secret.Data["tls.key"]
tlsCaCertificate, caExists := secret.Data["ca.crt"]

if !certExists || !keyExists || !caExists {
logger.Error(errors.New("required TLS keys are missing in the secret"), "Missing TLS keys in the secret")
return nil
}

cert, err := tls.X509KeyPair(tlsClientCert, tlsClientKey)
if err != nil {
logger.Error(err, "Couldn't load TLS client key pair")
logger.V(1).Error(err, "Couldn't load TLS client key pair", "secretName", cr.Spec.TLS.Secret.SecretName, "namespace", cr.Namespace, "redisClusterName", cr.Name)
return nil
}

tlsCaCertificates := x509.NewCertPool()
ok := tlsCaCertificates.AppendCertsFromPEM(tlsCaCertificate)
if !ok {
logger.Error(errors.New("failed to load CA Certificates from secret"), "Invalid CA Certificates")
logger.V(1).Error(err, "Invalid CA Certificates", "secretName", cr.Spec.TLS.Secret.SecretName, "namespace", cr.Namespace, "redisClusterName", cr.Name)
return nil
}

return &tls.Config{
Certificates: []tls.Certificate{cert},
ServerName: redisInfo.PodName,
RootCAs: tlsCaCertificates,
MinVersion: tls.VersionTLS12,
ClientAuth: tls.NoClientCert,
}
func getRedisTLSConfig(client kubernetes.Interface, logger logr.Logger, namespace, tlsSecretName, podName string) *tls.Config {
secret, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), tlsSecretName, metav1.GetOptions{})
if err != nil {
logger.V(1).Error(err, "Failed in getting TLS secret", "secretName", tlsSecretName, "namespace", namespace)
return nil
}
return nil
}

func getRedisReplicationTLSConfig(client kubernetes.Interface, logger logr.Logger, cr *redisv1beta2.RedisReplication, redisInfo RedisDetails) *tls.Config {
if cr.Spec.TLS != nil {
secret, err := client.CoreV1().Secrets(cr.Namespace).Get(context.TODO(), cr.Spec.TLS.Secret.SecretName, metav1.GetOptions{})
if err != nil {
logger.Error(err, "Failed in getting TLS secret for redis replication")
logger.V(1).Error(err, "Failed in getting TLS secret for redis replication", "secretName", cr.Spec.TLS.Secret.SecretName, "namespace", cr.Namespace, "redisReplicationName", cr.Name)
return nil
}

tlsClientCert, certExists := secret.Data["tls.crt"]
tlsClientKey, keyExists := secret.Data["tls.key"]
tlsCaCertificate, caExists := secret.Data["ca.crt"]
tlsClientCert, certExists := secret.Data["tls.crt"]
tlsClientKey, keyExists := secret.Data["tls.key"]
tlsCaCertificate, caExists := secret.Data["ca.crt"]

if !certExists || !keyExists || !caExists {
logger.Error(errors.New("required TLS keys are missing in the secret"), "Missing TLS keys in the secret")
return nil
}
if !certExists || !keyExists || !caExists {
logger.Error(errors.New("required TLS keys are missing in the secret"), "Missing TLS keys in the secret")
return nil
}

cert, err := tls.X509KeyPair(tlsClientCert, tlsClientKey)
if err != nil {
logger.Error(err, "Couldn't load TLS client key pair")
logger.V(1).Error(err, "Couldn't load TLS client key pair", "secretName", cr.Spec.TLS.Secret.SecretName, "namespace", cr.Namespace, "redisReplicationName", cr.Name)
return nil
}
cert, err := tls.X509KeyPair(tlsClientCert, tlsClientKey)
if err != nil {
logger.V(1).Error(err, "Couldn't load TLS client key pair", "secretName", tlsSecretName, "namespace", namespace)
return nil
}

tlsCaCertificates := x509.NewCertPool()
ok := tlsCaCertificates.AppendCertsFromPEM(tlsCaCertificate)
if !ok {
logger.Error(errors.New("failed to load CA Certificates from secret"), "Invalid CA Certificates")
logger.V(1).Error(err, "Invalid CA Certificates", "secretName", cr.Spec.TLS.Secret.SecretName, "namespace", cr.Namespace, "redisReplicationName", cr.Name)
return nil
}
tlsCaCertificates := x509.NewCertPool()
ok := tlsCaCertificates.AppendCertsFromPEM(tlsCaCertificate)
if !ok {
logger.V(1).Error(err, "Invalid CA Certificates", "secretName", tlsSecretName, "namespace", namespace)
return nil
}

return &tls.Config{
Certificates: []tls.Certificate{cert},
ServerName: redisInfo.PodName,
RootCAs: tlsCaCertificates,
MinVersion: tls.VersionTLS12,
ClientAuth: tls.NoClientCert,
}
return &tls.Config{
Certificates: []tls.Certificate{cert},
ServerName: podName,
RootCAs: tlsCaCertificates,
MinVersion: tls.VersionTLS12,
ClientAuth: tls.NoClientCert,
}
return nil
}
140 changes: 1 addition & 139 deletions k8sutils/secrets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,145 +222,7 @@ func Test_getRedisTLSConfig(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
client := tt.setup()
logger := testr.New(t)
tlsConfig := getRedisTLSConfig(client, logger, tt.redisCluster, tt.redisInfo)

if tt.expectTLS {
require.NotNil(t, tlsConfig, "Expected TLS configuration but got nil")
require.NotEmpty(t, tlsConfig.Certificates, "TLS Certificates should not be empty")
require.NotNil(t, tlsConfig.RootCAs, "Root CAs should not be nil")
} else {
assert.Nil(t, tlsConfig, "Expected no TLS configuration but got one")
}
})
}
}

func Test_getRedisReplicationTLSConfig(t *testing.T) {
tests := []struct {
name string
setup func() *k8sClientFake.Clientset
redisReplication *redisv1beta2.RedisReplication
redisInfo RedisDetails
expectTLS bool
}{
{
name: "TLS enabled and successful configuration",
setup: func() *k8sClientFake.Clientset {
tlsSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "redis-tls-secret",
Namespace: "default",
},
Data: map[string][]byte{
"ca.crt": helperReadFile(filepath.Join("..", "tests", "testdata", "secrets", "ca.crt")),
"tls.crt": helperReadFile(filepath.Join("..", "tests", "testdata", "secrets", "tls.crt")),
"tls.key": helperReadFile(filepath.Join("..", "tests", "testdata", "secrets", "tls.key")),
},
}
client := k8sClientFake.NewSimpleClientset(tlsSecret)
return client
},
redisReplication: &redisv1beta2.RedisReplication{
ObjectMeta: metav1.ObjectMeta{
Name: "redis-cluster",
Namespace: "default",
},
Spec: redisv1beta2.RedisReplicationSpec{
TLS: &redisv1beta2.TLSConfig{
TLSConfig: common.TLSConfig{
CaKeyFile: "ca.crt",
CertKeyFile: "tls.crt",
KeyFile: "tls.key",
Secret: corev1.SecretVolumeSource{
SecretName: "redis-tls-secret",
},
},
},
},
},
redisInfo: RedisDetails{
PodName: "redis-pod",
Namespace: "default",
},
expectTLS: true,
},
{
name: "TLS enabled but secret not found",
setup: func() *k8sClientFake.Clientset {
client := k8sClientFake.NewSimpleClientset()
return client
},
redisReplication: &redisv1beta2.RedisReplication{
ObjectMeta: metav1.ObjectMeta{
Name: "redis-cluster",
Namespace: "default",
},
Spec: redisv1beta2.RedisReplicationSpec{
TLS: &redisv1beta2.TLSConfig{
TLSConfig: common.TLSConfig{
CaKeyFile: "ca.crt",
CertKeyFile: "tls.crt",
KeyFile: "tls.key",
Secret: corev1.SecretVolumeSource{
SecretName: "redis-tls-secret",
},
},
},
},
},
redisInfo: RedisDetails{
PodName: "redis-pod",
Namespace: "default",
},
expectTLS: false,
},
{
name: "TLS enabled but incomplete secret",
setup: func() *k8sClientFake.Clientset {
tlsSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "redis-tls-secret",
Namespace: "default",
},
Data: map[string][]byte{
"ca.crt": helperReadFile(filepath.Join("..", "tests", "testdata", "secrets", "ca.crt")),
// Missing tls.crt and tls.key
},
}
client := k8sClientFake.NewSimpleClientset(tlsSecret)
return client
},
redisReplication: &redisv1beta2.RedisReplication{
ObjectMeta: metav1.ObjectMeta{
Name: "redis-cluster",
Namespace: "default",
},
Spec: redisv1beta2.RedisReplicationSpec{
TLS: &redisv1beta2.TLSConfig{
TLSConfig: common.TLSConfig{
CaKeyFile: "ca.crt",
CertKeyFile: "tls.crt",
KeyFile: "tls.key",
Secret: corev1.SecretVolumeSource{
SecretName: "redis-tls-secret",
},
},
},
},
},
redisInfo: RedisDetails{
PodName: "redis-pod",
Namespace: "default",
},
expectTLS: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := tt.setup()
logger := testr.New(t)
tlsConfig := getRedisReplicationTLSConfig(client, logger, tt.redisReplication, tt.redisInfo)
tlsConfig := getRedisTLSConfig(client, logger, tt.redisCluster.Namespace, tt.redisCluster.Spec.TLS.Secret.SecretName, tt.redisInfo.PodName)

if tt.expectTLS {
require.NotNil(t, tlsConfig, "Expected TLS configuration but got nil")
Expand Down

0 comments on commit 0fd0e42

Please sign in to comment.