Skip to content

Commit

Permalink
Merge pull request #8626 from fabriziopandini/cache-etcd-client-key-r…
Browse files Browse the repository at this point in the history
…elease-1.3

🐛 [release-1.3] Prevent KCP to create many private keys for each reconcile
  • Loading branch information
k8s-ci-robot authored May 9, 2023
2 parents 6a8ee61 + dee90a7 commit 1d746cd
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 16 deletions.
38 changes: 30 additions & 8 deletions controllers/remote/cluster_cache_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package remote

import (
"context"
"crypto/rsa"
"fmt"
"os"
"sync"
Expand Down Expand Up @@ -47,6 +48,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/certs"
"sigs.k8s.io/cluster-api/util/conditions"
)

Expand Down Expand Up @@ -165,12 +167,23 @@ func (t *ClusterCacheTracker) GetRESTConfig(ctc context.Context, cluster client.
return accessor.config, nil
}

// GetEtcdClientCertificateKey returns a cached certificate key to be used for generating certificates for accessing etcd in the given cluster.
func (t *ClusterCacheTracker) GetEtcdClientCertificateKey(ctx context.Context, cluster client.ObjectKey) (*rsa.PrivateKey, error) {
accessor, err := t.getClusterAccessor(ctx, cluster, t.indexes...)
if err != nil {
return nil, err
}

return accessor.etcdClientCertificateKey, nil
}

// clusterAccessor represents the combination of a delegating client, cache, and watches for a remote cluster.
type clusterAccessor struct {
cache *stoppableCache
client client.Client
watches sets.String
config *rest.Config
cache *stoppableCache
client client.Client
watches sets.String
config *rest.Config
etcdClientCertificateKey *rsa.PrivateKey
}

// clusterAccessorExists returns true if a clusterAccessor exists for cluster.
Expand Down Expand Up @@ -334,11 +347,20 @@ func (t *ClusterCacheTracker) newClusterAccessor(ctx context.Context, cluster cl
return nil, err
}

// Generating a new private key to be used for generating temporary certificates to connect to
// etcd on the target cluster.
// NOTE: Generating a private key is an expensive operation, so we store it in the cluster accessor.
etcdKey, err := certs.NewPrivateKey()
if err != nil {
return nil, errors.Wrapf(err, "error creating etcd client key for remote cluster %q", cluster.String())
}

return &clusterAccessor{
cache: cache,
config: config,
client: delegatingClient,
watches: sets.NewString(),
cache: cache,
config: config,
client: delegatingClient,
watches: sets.NewString(),
etcdClientCertificateKey: etcdKey,
}, nil
}

Expand Down
7 changes: 6 additions & 1 deletion controlplane/kubeadm/internal/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ 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 {
clientCert, err = generateClientCert(crtData, keyData)
clientKey, err := m.Tracker.GetEtcdClientCertificateKey(ctx, clusterKey)
if err != nil {
return nil, err
}

clientCert, err = generateClientCert(crtData, keyData, clientKey)
if err != nil {
return nil, err
}
Expand Down
10 changes: 3 additions & 7 deletions controlplane/kubeadm/internal/workload_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,7 @@ func calculateAPIServerPort(config *bootstrapv1.KubeadmConfig) int32 {
return 6443
}

func generateClientCert(caCertEncoded, caKeyEncoded []byte) (tls.Certificate, error) {
privKey, err := certs.NewPrivateKey()
if err != nil {
return tls.Certificate{}, err
}
func generateClientCert(caCertEncoded, caKeyEncoded []byte, clientKey *rsa.PrivateKey) (tls.Certificate, error) {
caCert, err := certs.DecodeCertPEM(caCertEncoded)
if err != nil {
return tls.Certificate{}, err
Expand All @@ -502,11 +498,11 @@ func generateClientCert(caCertEncoded, caKeyEncoded []byte) (tls.Certificate, er
if err != nil {
return tls.Certificate{}, err
}
x509Cert, err := newClientCert(caCert, privKey, caKey)
x509Cert, err := newClientCert(caCert, clientKey, caKey)
if err != nil {
return tls.Certificate{}, err
}
return tls.X509KeyPair(certs.EncodeCertPEM(x509Cert), certs.EncodePrivateKeyPEM(privKey))
return tls.X509KeyPair(certs.EncodeCertPEM(x509Cert), certs.EncodePrivateKeyPEM(clientKey))
}

func newClientCert(caCert *x509.Certificate, key *rsa.PrivateKey, caKey crypto.Signer) (*x509.Certificate, error) {
Expand Down

0 comments on commit 1d746cd

Please sign in to comment.