From 580513ed668065a7266a1aa22a4531f6a63b6989 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Thu, 18 Apr 2019 16:31:20 +0300 Subject: [PATCH] kubeadm: drop duplicate function NewCACertAndKey The function certs.NewCACertAndKey() is just a wrapper around pkiutil.NewCertificateAuthority() which doesn't add any additional functionality. Instead use pkiutil.NewCertificateAuthority() directly. --- cmd/kubeadm/app/phases/certs/certlist.go | 4 ++-- cmd/kubeadm/app/phases/certs/certs.go | 13 +------------ cmd/kubeadm/app/phases/certs/certs_test.go | 10 ---------- cmd/kubeadm/app/phases/certs/renewal/BUILD | 1 - .../phases/certs/renewal/filerenewal_test.go | 4 ++-- .../app/phases/certs/renewal/renewal_test.go | 5 ++--- cmd/kubeadm/app/util/certs/util.go | 7 ------- cmd/kubeadm/app/util/pkiutil/pki_helpers.go | 4 ++-- .../app/util/pkiutil/pki_helpers_test.go | 17 +++++++---------- 9 files changed, 16 insertions(+), 49 deletions(-) diff --git a/cmd/kubeadm/app/phases/certs/certlist.go b/cmd/kubeadm/app/phases/certs/certlist.go index ec969d5e4da..08d42046cfb 100644 --- a/cmd/kubeadm/app/phases/certs/certlist.go +++ b/cmd/kubeadm/app/phases/certs/certlist.go @@ -85,7 +85,7 @@ func (k *KubeadmCert) CreateAsCA(ic *kubeadmapi.InitConfiguration) (*x509.Certif if err != nil { return nil, nil, errors.Wrapf(err, "couldn't get configuration for %q CA certificate", k.Name) } - caCert, caKey, err := NewCACertAndKey(cfg) + caCert, caKey, err := pkiutil.NewCertificateAuthority(cfg) if err != nil { return nil, nil, errors.Wrapf(err, "couldn't generate %q CA certificate", k.Name) } @@ -141,7 +141,7 @@ func (t CertificateTree) CreateTree(ic *kubeadmapi.InitConfiguration) error { // CA key exists; just use that to create new certificates. } else { // CACert doesn't already exist, create a new cert and key. - caCert, caKey, err = NewCACertAndKey(cfg) + caCert, caKey, err = pkiutil.NewCertificateAuthority(cfg) if err != nil { return err } diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index 2cac3f16e72..6d3da11d63e 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -90,17 +90,6 @@ func NewServiceAccountSigningKey() (*rsa.PrivateKey, error) { return saSigningKey, nil } -// NewCACertAndKey will generate a self signed CA. -func NewCACertAndKey(certSpec *certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) { - - caCert, caKey, err := pkiutil.NewCertificateAuthority(certSpec) - if err != nil { - return nil, nil, errors.Wrap(err, "failure while generating CA certificate and key") - } - - return caCert, caKey, nil -} - // CreateCACertAndKeyFiles generates and writes out a given certificate authority. // The certSpec should be one of the variables from this package. func CreateCACertAndKeyFiles(certSpec *KubeadmCert, cfg *kubeadmapi.InitConfiguration) error { @@ -114,7 +103,7 @@ func CreateCACertAndKeyFiles(certSpec *KubeadmCert, cfg *kubeadmapi.InitConfigur return err } - caCert, caKey, err := NewCACertAndKey(certConfig) + caCert, caKey, err := pkiutil.NewCertificateAuthority(certConfig) if err != nil { return err } diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index a77a33f95de..8924c72bd49 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -370,16 +370,6 @@ func TestWriteKeyFilesIfNotExist(t *testing.T) { } } -func TestNewCACertAndKey(t *testing.T) { - certCfg := &certutil.Config{CommonName: "kubernetes"} - caCert, _, err := NewCACertAndKey(certCfg) - if err != nil { - t.Fatalf("failed call NewCACertAndKey: %v", err) - } - - certstestutil.AssertCertificateIsCa(t, caCert) -} - func TestSharedCertificateExists(t *testing.T) { caCert, caKey := certstestutil.CreateCACert(t) _, key, _ := certstestutil.CreateTestCert(t, caCert, caKey, certutil.AltNames{}) diff --git a/cmd/kubeadm/app/phases/certs/renewal/BUILD b/cmd/kubeadm/app/phases/certs/renewal/BUILD index 98da00fd170..626aad15750 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/BUILD +++ b/cmd/kubeadm/app/phases/certs/renewal/BUILD @@ -30,7 +30,6 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//cmd/kubeadm/app/phases/certs:go_default_library", "//cmd/kubeadm/app/util/certs:go_default_library", "//cmd/kubeadm/app/util/pkiutil:go_default_library", "//cmd/kubeadm/test:go_default_library", diff --git a/cmd/kubeadm/app/phases/certs/renewal/filerenewal_test.go b/cmd/kubeadm/app/phases/certs/renewal/filerenewal_test.go index 3c8a9e58f35..29d92e78c30 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/filerenewal_test.go +++ b/cmd/kubeadm/app/phases/certs/renewal/filerenewal_test.go @@ -21,12 +21,12 @@ import ( "testing" certutil "k8s.io/client-go/util/cert" - "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" + "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil" ) func TestFileRenew(t *testing.T) { caCertCfg := &certutil.Config{CommonName: "kubernetes"} - caCert, caKey, err := certs.NewCACertAndKey(caCertCfg) + caCert, caKey, err := pkiutil.NewCertificateAuthority(caCertCfg) if err != nil { t.Fatalf("couldn't create CA: %v", err) } diff --git a/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go b/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go index bf651e00d0a..0d9c6df54a1 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go +++ b/cmd/kubeadm/app/phases/certs/renewal/renewal_test.go @@ -32,7 +32,6 @@ import ( fakecerts "k8s.io/client-go/kubernetes/typed/certificates/v1beta1/fake" k8stesting "k8s.io/client-go/testing" certutil "k8s.io/client-go/util/cert" - "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" certtestutil "k8s.io/kubernetes/cmd/kubeadm/app/util/certs" "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil" testutil "k8s.io/kubernetes/cmd/kubeadm/test" @@ -40,7 +39,7 @@ import ( func TestRenewImplementations(t *testing.T) { caCertCfg := &certutil.Config{CommonName: "kubernetes"} - caCert, caKey, err := certs.NewCACertAndKey(caCertCfg) + caCert, caKey, err := pkiutil.NewCertificateAuthority(caCertCfg) if err != nil { t.Fatalf("couldn't create CA: %v", err) } @@ -198,7 +197,7 @@ func TestRenewExistingCert(t *testing.T) { } caCertCfg := &certutil.Config{CommonName: "kubernetes"} - caCert, caKey, err := certs.NewCACertAndKey(caCertCfg) + caCert, caKey, err := pkiutil.NewCertificateAuthority(caCertCfg) if err != nil { t.Fatalf("couldn't create CA: %v", err) } diff --git a/cmd/kubeadm/app/util/certs/util.go b/cmd/kubeadm/app/util/certs/util.go index bf0d0210b71..cc0731f87e3 100644 --- a/cmd/kubeadm/app/util/certs/util.go +++ b/cmd/kubeadm/app/util/certs/util.go @@ -39,13 +39,6 @@ func SetupCertificateAuthorithy(t *testing.T) (*x509.Certificate, *rsa.PrivateKe return caCert, caKey } -// AssertCertificateIsCa is a utility function for kubeadm testing that asserts if a given certificate is a CA -func AssertCertificateIsCa(t *testing.T, cert *x509.Certificate) { - if !cert.IsCA { - t.Error("cert is not a valida CA") - } -} - // AssertCertificateIsSignedByCa is a utility function for kubeadm testing that asserts if a given certificate is signed // by the expected CA func AssertCertificateIsSignedByCa(t *testing.T, cert *x509.Certificate, signingCa *x509.Certificate) { diff --git a/cmd/kubeadm/app/util/pkiutil/pki_helpers.go b/cmd/kubeadm/app/util/pkiutil/pki_helpers.go index 06f15c06774..6f68a01cbc8 100644 --- a/cmd/kubeadm/app/util/pkiutil/pki_helpers.go +++ b/cmd/kubeadm/app/util/pkiutil/pki_helpers.go @@ -61,12 +61,12 @@ const ( func NewCertificateAuthority(config *certutil.Config) (*x509.Certificate, *rsa.PrivateKey, error) { key, err := NewPrivateKey() if err != nil { - return nil, nil, errors.Wrap(err, "unable to create private key") + return nil, nil, errors.Wrap(err, "unable to create private key while generating CA certificate") } cert, err := certutil.NewSelfSignedCACert(*config, key) if err != nil { - return nil, nil, errors.Wrap(err, "unable to create self-signed certificate") + return nil, nil, errors.Wrap(err, "unable to create self-signed CA certificate") } return cert, key, nil diff --git a/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go b/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go index 3d162e6e30d..b4542a2ecc6 100644 --- a/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go +++ b/cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go @@ -33,20 +33,17 @@ func TestNewCertificateAuthority(t *testing.T) { cert, key, err := NewCertificateAuthority(&certutil.Config{CommonName: "kubernetes"}) if cert == nil { - t.Errorf( - "failed NewCertificateAuthority, cert == nil", - ) + t.Error("failed NewCertificateAuthority, cert == nil") + } else if !cert.IsCA { + t.Error("cert is not a valida CA") } + if key == nil { - t.Errorf( - "failed NewCertificateAuthority, key == nil", - ) + t.Error("failed NewCertificateAuthority, key == nil") } + if err != nil { - t.Errorf( - "failed NewCertificateAuthority with an error: %v", - err, - ) + t.Errorf("failed NewCertificateAuthority with an error: %+v", err) } }