Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [release-1.3] Prevent KCP to create many private keys for each reconcile #8626

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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