Address review comments

Author: Karthik-K-N

controllers/clustercache/cluster_accessor.go

@@ -18,7 +18,7 @@ package clustercache
 
 import (
 	"context"
-	"crypto"
+	"crypto/rsa"
 	"fmt"
 	"sync"
 	"time"
@@ -165,7 +165,7 @@ type clusterAccessorLockedState struct {
 	// cert to communicate with etcd.
 	// This private key is stored and cached in the ClusterCache because it's expensive to generate a new
 	// private key in every single Reconcile.
-	clientCertificatePrivateKey crypto.Signer
+	clientCertificatePrivateKey *rsa.PrivateKey
 
 	// healthChecking holds the health checking state (e.g. lastProbeSuccessTime, consecutiveFailures)
 	// of the clusterAccessor.
@@ -435,7 +435,7 @@ func (ca *clusterAccessor) GetRESTConfig(ctx context.Context) (*rest.Config, err
 	return ca.lockedState.connection.restConfig, nil
 }
 
-func (ca *clusterAccessor) GetClientCertificatePrivateKey(ctx context.Context) crypto.Signer {
+func (ca *clusterAccessor) GetClientCertificatePrivateKey(ctx context.Context) *rsa.PrivateKey {
 	ca.rLock(ctx)
 	defer ca.rUnlock(ctx)
 

controllers/clustercache/cluster_cache.go

@@ -18,7 +18,7 @@ package clustercache
 
 import (
 	"context"
-	"crypto"
+	"crypto/rsa"
 	"fmt"
 	"os"
 	"strings"
@@ -150,7 +150,7 @@ type ClusterCache interface {
 	//
 	// Deprecated: This method is deprecated and will be removed in a future release as caching a rsa.PrivateKey
 	// is outside the scope of the ClusterCache.
-	GetClientCertificatePrivateKey(ctx context.Context, cluster client.ObjectKey) (crypto.Signer, error)
+	GetClientCertificatePrivateKey(ctx context.Context, cluster client.ObjectKey) (*rsa.PrivateKey, error)
 
 	// Watch watches a workload cluster for events.
 	// Each unique watch (by input.Name) is only added once after a Connect (otherwise we return early).
@@ -417,7 +417,7 @@ func (cc *clusterCache) GetRESTConfig(ctx context.Context, cluster client.Object
 	return accessor.GetRESTConfig(ctx)
 }
 
-func (cc *clusterCache) GetClientCertificatePrivateKey(ctx context.Context, cluster client.ObjectKey) (crypto.Signer, error) {
+func (cc *clusterCache) GetClientCertificatePrivateKey(ctx context.Context, cluster client.ObjectKey) (*rsa.PrivateKey, error) {
 	accessor := cc.getClusterAccessor(cluster)
 	if accessor == nil {
 		return nil, errors.New("error getting client certificate private key: private key was not generated yet")

controlplane/kubeadm/internal/cluster_test.go

@@ -39,6 +39,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	bootstrapv1 "sigs.k8s.io/cluster-api/api/bootstrap/kubeadm/v1beta2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/cluster-api/controllers/clustercache"
 	"sigs.k8s.io/cluster-api/controllers/remote"
@@ -249,7 +250,7 @@ func TestGetWorkloadCluster(t *testing.T) {
 			})
 			g.Expect(err).ToNot(HaveOccurred())
 
-			workloadCluster, err := m.GetWorkloadCluster(ctx, tt.clusterKey, "")
+			workloadCluster, err := m.GetWorkloadCluster(ctx, tt.clusterKey, bootstrapv1.EncryptionAlgorithmRSA2048)
 			if tt.expectErr {
 				g.Expect(err).To(HaveOccurred())
 				g.Expect(workloadCluster).To(BeNil())

controlplane/kubeadm/internal/control_plane.go

@@ -469,6 +469,10 @@ func (c *ControlPlane) StatusToLogKeyAndValues(newMachine, deletedMachine *clust
 }
 
 // GetKeyEncryptionAlgorithm returns the control plane EncryptionAlgorithm.
+// If its unset the default encryption algorithm is returned.
 func (c *ControlPlane) GetKeyEncryptionAlgorithm() bootstrapv1.EncryptionAlgorithmType {
+	if c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.EncryptionAlgorithm == "" {
+		return bootstrapv1.EncryptionAlgorithmRSA2048
+	}
 	return c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.EncryptionAlgorithm
 }

controlplane/kubeadm/internal/workload_cluster.go

@@ -364,32 +364,17 @@ func generateClientCert(caCertEncoded, caKeyEncoded []byte, keyEncryptionAlgorit
 		return tls.Certificate{}, err
 	}
 
-	var x509Cert *x509.Certificate
-	var encodedClientKey []byte
-
-	if keyEncryptionAlgorithm != "" {
-		clientKey, err := certs.NewSigner(keyEncryptionAlgorithm)
-		if err != nil {
-			return tls.Certificate{}, err
-		}
-		x509Cert, err = newClientCert(caCert, clientKey, caKey)
-		if err != nil {
-			return tls.Certificate{}, err
-		}
-		encodedClientKey, err = certs.EncodePrivateKeyPEMFromSigner(clientKey)
-		if err != nil {
-			return tls.Certificate{}, err
-		}
-	} else {
-		clientKey, err := certs.NewPrivateKey()
-		if err != nil {
-			return tls.Certificate{}, err
-		}
-		x509Cert, err = newClientCert(caCert, clientKey, caKey)
-		if err != nil {
-			return tls.Certificate{}, err
-		}
-		encodedClientKey = certs.EncodePrivateKeyPEM(clientKey)
+	clientKey, err := certs.NewSigner(keyEncryptionAlgorithm)
+	if err != nil {
+		return tls.Certificate{}, err
+	}
+	x509Cert, err := newClientCert(caCert, clientKey, caKey)
+	if err != nil {
+		return tls.Certificate{}, err
+	}
+	encodedClientKey, err := certs.EncodePrivateKeyPEMFromSigner(clientKey)
+	if err != nil {
+		return tls.Certificate{}, err
 	}
 
 	return tls.X509KeyPair(certs.EncodeCertPEM(x509Cert), encodedClientKey)

util/kubeconfig/kubeconfig.go

@@ -55,8 +55,6 @@ func FromSecret(ctx context.Context, c client.Reader, cluster client.ObjectKey)
 
 // New creates a new Kubeconfig using the cluster name and specified endpoint.
 func New(clusterName, endpoint string, caCert *x509.Certificate, caKey crypto.Signer, options ...KubeConfigurationOption) (*api.Config, error) {
-	var clientCert *x509.Certificate
-	var encodedClientKey []byte
 	cfg := &certs.Config{
 		CommonName:   "kubernetes-admin",
 		Organization: []string{"system:masters"},
@@ -69,34 +67,19 @@ func New(clusterName, endpoint string, caCert *x509.Certificate, caKey crypto.Si
 	kubeConfigOptions := &KubeConfigurationOptions{}
 	kubeConfigOptions.ApplyOptions(options)
 
-	// Generate key based on the EncryptionAlgorithm if set.
-	if kubeConfigOptions.keyEncryptionAlgorithm != "" {
-		clientKey, err := certs.NewSigner(kubeConfigOptions.keyEncryptionAlgorithm)
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to create private key")
-		}
-
-		clientCert, err = cfg.NewSignedCert(clientKey, caCert, caKey)
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to sign certificate")
-		}
-
-		encodedClientKey, err = certs.EncodePrivateKeyPEMFromSigner(clientKey)
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to encode private key")
-		}
-	} else {
-		clientKey, err := certs.NewPrivateKey()
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to create private key")
-		}
+	clientKey, err := certs.NewSigner(kubeConfigOptions.keyEncryptionAlgorithm)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to create private key")
+	}
 
-		clientCert, err = cfg.NewSignedCert(clientKey, caCert, caKey)
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to sign certificate")
-		}
+	clientCert, err := cfg.NewSignedCert(clientKey, caCert, caKey)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to sign certificate")
+	}
 
-		encodedClientKey = certs.EncodeCertPEM(clientCert)
+	encodedClientKey, err := certs.EncodePrivateKeyPEMFromSigner(clientKey)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to encode private key")
 	}
 
 	return &api.Config{
@@ -139,7 +122,7 @@ func CreateSecretWithOwner(ctx context.Context, c client.Client, clusterName cli
 	if err != nil {
 		return err
 	}
-	out, err := generateKubeconfig(ctx, c, clusterName, server, options)
+	out, err := generateKubeconfig(ctx, c, clusterName, server, options...)
 	if err != nil {
 		return err
 	}
@@ -222,15 +205,15 @@ func RegenerateSecret(ctx context.Context, c client.Client, configSecret *corev1
 	}
 	endpoint := config.Clusters[clusterName].Server
 	key := client.ObjectKey{Name: clusterName, Namespace: configSecret.Namespace}
-	out, err := generateKubeconfig(ctx, c, key, endpoint, options)
+	out, err := generateKubeconfig(ctx, c, key, endpoint, options...)
 	if err != nil {
 		return err
 	}
 	configSecret.Data[secret.KubeconfigDataName] = out
 	return c.Update(ctx, configSecret)
 }
 
-func generateKubeconfig(ctx context.Context, c client.Client, clusterName client.ObjectKey, endpoint string, options []KubeConfigurationOption) ([]byte, error) {
+func generateKubeconfig(ctx context.Context, c client.Client, clusterName client.ObjectKey, endpoint string, options ...KubeConfigurationOption) ([]byte, error) {
 	clusterCA, err := secret.GetFromNamespacedName(ctx, c, clusterName, secret.ClusterCA)
 	if err != nil {
 		if apierrors.IsNotFound(err) {