diff --git a/cmd/kubeadm/app/cmd/phases/join/preflight.go b/cmd/kubeadm/app/cmd/phases/join/preflight.go index e810cfe822f..6cd8a452819 100644 --- a/cmd/kubeadm/app/cmd/phases/join/preflight.go +++ b/cmd/kubeadm/app/cmd/phases/join/preflight.go @@ -147,7 +147,7 @@ func checkIfReadyForAdditionalControlPlane(initConfiguration *kubeadmapi.Cluster } if !hasCertificateKey { - // checks if the certificates that must be equal across controlplane instances are provided + // checks if the certificates are provided and are still valid, not expired yet. if ret, err := certs.SharedCertificateExists(initConfiguration); !ret { return err } diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index 2ebd58b836e..7e8a60bce91 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -26,6 +26,7 @@ import ( "github.com/pkg/errors" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/util/keyutil" "k8s.io/klog/v2" @@ -304,30 +305,32 @@ type certKeyLocation struct { uxName string } -// SharedCertificateExists verifies if the shared certificates - the certificates that must be -// equal across control-plane nodes: ca.key, ca.crt, sa.key, sa.pub + etcd/ca.key, etcd/ca.crt if local/stacked etcd -// Missing keys are non-fatal and produce warnings. +// SharedCertificateExists verifies if the shared certificates exist and are still valid - the certificates must be +// equal across control-plane nodes: ca.key, ca.crt, sa.key, sa.pub, front-proxy-ca.key, front-proxy-ca.crt and etcd/ca.key, etcd/ca.crt if local/stacked etcd +// Missing private keys of CA are non-fatal and produce warnings. func SharedCertificateExists(cfg *kubeadmapi.ClusterConfiguration) (bool, error) { - + var errs []error if err := validateCACertAndKey(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, "", "CA"}); err != nil { - return false, err + errs = append(errs, err) } if err := validatePrivatePublicKey(certKeyLocation{cfg.CertificatesDir, "", kubeadmconstants.ServiceAccountKeyBaseName, "service account"}); err != nil { - return false, err + errs = append(errs, err) } if err := validateCACertAndKey(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.FrontProxyCACertAndKeyBaseName, "", "front-proxy CA"}); err != nil { - return false, err + errs = append(errs, err) } // in case of local/stacked etcd if cfg.Etcd.External == nil { if err := validateCACertAndKey(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.EtcdCACertAndKeyBaseName, "", "etcd CA"}); err != nil { - return false, err + errs = append(errs, err) } } - + if len(errs) != 0 { + return false, utilerrors.NewAggregate(errs) + } return true, nil } diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index 51e8afa5ce5..c00269fed19 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -31,6 +31,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" + utilerrors "k8s.io/apimachinery/pkg/util/errors" certutil "k8s.io/client-go/util/cert" "k8s.io/client-go/util/keyutil" @@ -373,9 +374,9 @@ func TestSharedCertificateExists(t *testing.T) { publicKey := key.Public() var tests = []struct { - name string - files certstestutil.PKIFiles - expectedError bool + name string + files certstestutil.PKIFiles + expectedErrors int }{ { name: "success", @@ -401,7 +402,7 @@ func TestSharedCertificateExists(t *testing.T) { "etcd/ca.crt": caCert, "etcd/ca.key": caKey, }, - expectedError: true, + expectedErrors: 1, }, { name: "missing ca.key", @@ -414,7 +415,6 @@ func TestSharedCertificateExists(t *testing.T) { "etcd/ca.crt": caCert, "etcd/ca.key": caKey, }, - expectedError: false, }, { name: "missing sa.key", @@ -427,7 +427,7 @@ func TestSharedCertificateExists(t *testing.T) { "etcd/ca.crt": caCert, "etcd/ca.key": caKey, }, - expectedError: true, + expectedErrors: 1, }, { name: "missing front-proxy.crt", @@ -440,20 +440,32 @@ func TestSharedCertificateExists(t *testing.T) { "etcd/ca.crt": caCert, "etcd/ca.key": caKey, }, - expectedError: true, + expectedErrors: 1, }, { name: "missing etcd/ca.crt", files: certstestutil.PKIFiles{ "ca.crt": caCert, "ca.key": caKey, + "front-proxy-ca.crt": caCert, "front-proxy-ca.key": caKey, "sa.pub": publicKey, "sa.key": key, - "etcd/ca.crt": caCert, "etcd/ca.key": caKey, }, - expectedError: true, + expectedErrors: 1, + }, + { + name: "missing multiple certs (ca.crt and etcd/ca.crt)", + files: certstestutil.PKIFiles{ + "ca.key": caKey, + "front-proxy-ca.crt": caCert, + "front-proxy-ca.key": caKey, + "sa.pub": publicKey, + "sa.key": key, + "etcd/ca.key": caKey, + }, + expectedErrors: 2, }, } @@ -472,12 +484,13 @@ func TestSharedCertificateExists(t *testing.T) { // executes create func ret, err := SharedCertificateExists(cfg) - switch { - case !test.expectedError && err != nil: - t.Errorf("error SharedCertificateExists failed when not expected to fail: %v", err) - case test.expectedError && err == nil: - t.Errorf("error SharedCertificateExists didn't failed when expected") + case err != nil: + if agg, ok := err.(utilerrors.Aggregate); ok && len(agg.Errors()) != test.expectedErrors { + t.Errorf("SharedCertificateExists didn't fail with the expected number of errors, expected: %v, got: %v", test.expectedErrors, len(agg.Errors())) + } + case err == nil && test.expectedErrors != 0: + t.Errorf("error SharedCertificateExists didn't fail when expected") case ret != (err == nil): t.Errorf("error SharedCertificateExists returned %v when expected to return %v", ret, err == nil) }