ClusterCache: Deprecate GetClientCertificatePrivateKey and stop using it in KCP

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

controllers/clustercache/cluster_cache.go

@@ -147,6 +147,9 @@ type ClusterCache interface {
 	// 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.
+	//
+	// 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) (*rsa.PrivateKey, error)
 
 	// Watch watches a workload cluster for events.

controlplane/kubeadm/internal/cluster.go

@@ -32,6 +32,7 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/cluster-api/controllers/clustercache"
+	"sigs.k8s.io/cluster-api/util/cache"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/secret"
 )
@@ -53,6 +54,18 @@ type Management struct {
 	EtcdDialTimeout     time.Duration
 	EtcdCallTimeout     time.Duration
 	EtcdLogger          *zap.Logger
+	ClientCertCache     cache.Cache[ClientCertEntry]
+}
+
+// ClientCertEntry is an Entry for the Cache that stores the client cert.
+type ClientCertEntry struct {
+	Cluster    client.ObjectKey
+	ClientCert *tls.Certificate
+}
+
+// Key returns the cache key of a ClientCertEntry.
+func (r ClientCertEntry) Key() string {
+	return r.Cluster.String()
 }
 
 // RemoteClusterConnectionError represents a failure to connect to a remote cluster.
@@ -126,14 +139,18 @@ func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.O
 	// TODO: consider if we can detect if we are using external etcd in a more explicit way (e.g. looking at the config instead of deriving from the existing certificates)
 	var clientCert tls.Certificate
 	if keyData != nil {
-		clientKey, err := m.ClusterCache.GetClientCertificatePrivateKey(ctx, clusterKey)
-		if err != nil {
-			return nil, err
-		}
-
-		clientCert, err = generateClientCert(crtData, keyData, clientKey)
-		if err != nil {
-			return nil, err
+		// Get client cert from cache if possible, otherwise generate it and add it to the cache.
+		// TODO: When we implement ClusterConfiguration.EncryptionAlgorithm we should add it to
+		//       the ClientCertEntries and make it part of the key.
+		if entry, ok := m.ClientCertCache.Has(ClientCertEntry{Cluster: clusterKey}.Key()); ok {
+			clientCert = *entry.ClientCert
+		} else {
+			// The client cert expires after 10 years, but that's okay as the cache has a TTL of 1 day.
+			clientCert, err = generateClientCert(crtData, keyData)
+			if err != nil {
+				return nil, err
+			}
+			m.ClientCertCache.Add(ClientCertEntry{Cluster: clusterKey, ClientCert: &clientCert})
 		}
 	} else {
 		clientCert, err = m.getAPIServerEtcdClientCert(ctx, clusterKey)

controlplane/kubeadm/internal/cluster_test.go

@@ -42,6 +42,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/cluster-api/controllers/clustercache"
 	"sigs.k8s.io/cluster-api/controllers/remote"
+	"sigs.k8s.io/cluster-api/util/cache"
 	"sigs.k8s.io/cluster-api/util/certs"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/kubeconfig"
@@ -239,6 +240,7 @@ func TestGetWorkloadCluster(t *testing.T) {
 				Client:              env.GetClient(),
 				SecretCachingClient: secretCachingClient,
 				ClusterCache:        clusterCache,
+				ClientCertCache:     cache.New[ClientCertEntry](24 * time.Hour),
 			}
 
 			// Ensure the ClusterCache reconciled at least once (and if possible created a clusterAccessor).

controlplane/kubeadm/internal/controllers/controller.go

@@ -50,6 +50,7 @@ import (
 	"sigs.k8s.io/cluster-api/internal/contract"
 	"sigs.k8s.io/cluster-api/internal/util/ssa"
 	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/util/cache"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1"
@@ -149,6 +150,7 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
 			EtcdDialTimeout:     r.EtcdDialTimeout,
 			EtcdCallTimeout:     r.EtcdCallTimeout,
 			EtcdLogger:          r.EtcdLogger,
+			ClientCertCache:     cache.New[internal.ClientCertEntry](24 * time.Hour),
 		}
 	}
 

controlplane/kubeadm/internal/workload_cluster.go

@@ -347,7 +347,7 @@ func calculateAPIServerPort(config *bootstrapv1.KubeadmConfig) int32 {
 	return 6443
 }
 
-func generateClientCert(caCertEncoded, caKeyEncoded []byte, clientKey *rsa.PrivateKey) (tls.Certificate, error) {
+func generateClientCert(caCertEncoded, caKeyEncoded []byte) (tls.Certificate, error) {
 	caCert, err := certs.DecodeCertPEM(caCertEncoded)
 	if err != nil {
 		return tls.Certificate{}, err
@@ -356,6 +356,10 @@ func generateClientCert(caCertEncoded, caKeyEncoded []byte, clientKey *rsa.Priva
 	if err != nil {
 		return tls.Certificate{}, err
 	}
+	clientKey, err := certs.NewPrivateKey()
+	if err != nil {
+		return tls.Certificate{}, err
+	}
 	x509Cert, err := newClientCert(caCert, clientKey, caKey)
 	if err != nil {
 		return tls.Certificate{}, err