From 7e01e38334f5590fbac623f38913bf43a69e6068 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 4 Sep 2020 13:36:10 -0400 Subject: [PATCH] Avoid mutating global variables in kubeadm certs phases --- cmd/kubeadm/app/cmd/alpha/certs_test.go | 54 +++++----- cmd/kubeadm/app/cmd/phases/init/certs_test.go | 2 +- cmd/kubeadm/app/phases/certs/certlist.go | 98 ++++++++++++------- cmd/kubeadm/app/phases/certs/certs_test.go | 4 +- cmd/kubeadm/app/phases/upgrade/staticpods.go | 14 +-- .../app/phases/upgrade/staticpods_test.go | 54 +++++----- cmd/kubeadm/test/cmd/init_test.go | 6 +- 7 files changed, 128 insertions(+), 104 deletions(-) diff --git a/cmd/kubeadm/app/cmd/alpha/certs_test.go b/cmd/kubeadm/app/cmd/alpha/certs_test.go index 2897cd0c40b..826cf9988b2 100644 --- a/cmd/kubeadm/app/cmd/alpha/certs_test.go +++ b/cmd/kubeadm/app/cmd/alpha/certs_test.go @@ -98,9 +98,9 @@ func TestRunRenewCommands(t *testing.T) { CACerts := map[string]*x509.Certificate{} CAKeys := map[string]crypto.Signer{} for _, ca := range []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertRootCA, - &certsphase.KubeadmCertFrontProxyCA, - &certsphase.KubeadmCertEtcdCA, + certsphase.KubeadmCertRootCA(), + certsphase.KubeadmCertFrontProxyCA(), + certsphase.KubeadmCertEtcdCA(), } { caCert, caKey, err := ca.CreateAsCA(cfg) if err != nil { @@ -112,13 +112,13 @@ func TestRunRenewCommands(t *testing.T) { // Generate all the signed certificates for _, cert := range []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertAPIServer, - &certsphase.KubeadmCertKubeletClient, - &certsphase.KubeadmCertFrontProxyClient, - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertEtcdServer, - &certsphase.KubeadmCertEtcdPeer, - &certsphase.KubeadmCertEtcdHealthcheck, + certsphase.KubeadmCertAPIServer(), + certsphase.KubeadmCertKubeletClient(), + certsphase.KubeadmCertFrontProxyClient(), + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertEtcdServer(), + certsphase.KubeadmCertEtcdPeer(), + certsphase.KubeadmCertEtcdHealthcheck(), } { caCert := CACerts[cert.CAName] caKey := CAKeys[cert.CAName] @@ -146,13 +146,13 @@ func TestRunRenewCommands(t *testing.T) { { command: "all", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertAPIServer, - &certsphase.KubeadmCertKubeletClient, - &certsphase.KubeadmCertFrontProxyClient, - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertEtcdServer, - &certsphase.KubeadmCertEtcdPeer, - &certsphase.KubeadmCertEtcdHealthcheck, + certsphase.KubeadmCertAPIServer(), + certsphase.KubeadmCertKubeletClient(), + certsphase.KubeadmCertFrontProxyClient(), + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertEtcdServer(), + certsphase.KubeadmCertEtcdPeer(), + certsphase.KubeadmCertEtcdHealthcheck(), }, KubeconfigFiles: []string{ kubeadmconstants.AdminKubeConfigFileName, @@ -163,43 +163,43 @@ func TestRunRenewCommands(t *testing.T) { { command: "apiserver", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertAPIServer, + certsphase.KubeadmCertAPIServer(), }, }, { command: "apiserver-kubelet-client", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertKubeletClient, + certsphase.KubeadmCertKubeletClient(), }, }, { command: "apiserver-etcd-client", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdAPIClient, + certsphase.KubeadmCertEtcdAPIClient(), }, }, { command: "front-proxy-client", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertFrontProxyClient, + certsphase.KubeadmCertFrontProxyClient(), }, }, { command: "etcd-server", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdServer, + certsphase.KubeadmCertEtcdServer(), }, }, { command: "etcd-peer", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdPeer, + certsphase.KubeadmCertEtcdPeer(), }, }, { command: "etcd-healthcheck-client", Certs: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdHealthcheck, + certsphase.KubeadmCertEtcdHealthcheck(), }, }, { @@ -271,14 +271,14 @@ func TestRunRenewCommands(t *testing.T) { func TestRenewUsingCSR(t *testing.T) { tmpDir := testutil.SetupTempDir(t) defer os.RemoveAll(tmpDir) - cert := &certsphase.KubeadmCertEtcdServer + cert := certsphase.KubeadmCertEtcdServer() cfg := testutil.GetDefaultInternalConfig(t) cfg.CertificatesDir = tmpDir - caCert, caKey, err := certsphase.KubeadmCertEtcdCA.CreateAsCA(cfg) + caCert, caKey, err := certsphase.KubeadmCertEtcdCA().CreateAsCA(cfg) if err != nil { - t.Fatalf("couldn't write out CA %s: %v", certsphase.KubeadmCertEtcdCA.Name, err) + t.Fatalf("couldn't write out CA %s: %v", certsphase.KubeadmCertEtcdCA().Name, err) } if err := cert.CreateFromCA(cfg, caCert, caKey); err != nil { diff --git a/cmd/kubeadm/app/cmd/phases/init/certs_test.go b/cmd/kubeadm/app/cmd/phases/init/certs_test.go index c28c8945d70..8cae580a2e1 100644 --- a/cmd/kubeadm/app/cmd/phases/init/certs_test.go +++ b/cmd/kubeadm/app/cmd/phases/init/certs_test.go @@ -44,7 +44,7 @@ func TestCertsWithCSRs(t *testing.T) { defer os.RemoveAll(csrDir) certDir := testutil.SetupTempDir(t) defer os.RemoveAll(certDir) - cert := &certs.KubeadmCertAPIServer + cert := certs.KubeadmCertAPIServer() certsData := &testCertsData{ cfg: testutil.GetDefaultInternalConfig(t), diff --git a/cmd/kubeadm/app/phases/certs/certlist.go b/cmd/kubeadm/app/phases/certs/certlist.go index 7b1a57c02fb..45f75ec33b4 100644 --- a/cmd/kubeadm/app/phases/certs/certlist.go +++ b/cmd/kubeadm/app/phases/certs/certlist.go @@ -212,36 +212,36 @@ func (c Certificates) AsMap() CertificateMap { // GetDefaultCertList returns all of the certificates kubeadm requires to function. func GetDefaultCertList() Certificates { return Certificates{ - &KubeadmCertRootCA, - &KubeadmCertAPIServer, - &KubeadmCertKubeletClient, + KubeadmCertRootCA(), + KubeadmCertAPIServer(), + KubeadmCertKubeletClient(), // Front Proxy certs - &KubeadmCertFrontProxyCA, - &KubeadmCertFrontProxyClient, + KubeadmCertFrontProxyCA(), + KubeadmCertFrontProxyClient(), // etcd certs - &KubeadmCertEtcdCA, - &KubeadmCertEtcdServer, - &KubeadmCertEtcdPeer, - &KubeadmCertEtcdHealthcheck, - &KubeadmCertEtcdAPIClient, + KubeadmCertEtcdCA(), + KubeadmCertEtcdServer(), + KubeadmCertEtcdPeer(), + KubeadmCertEtcdHealthcheck(), + KubeadmCertEtcdAPIClient(), } } // GetCertsWithoutEtcd returns all of the certificates kubeadm needs when etcd is hosted externally. func GetCertsWithoutEtcd() Certificates { return Certificates{ - &KubeadmCertRootCA, - &KubeadmCertAPIServer, - &KubeadmCertKubeletClient, + KubeadmCertRootCA(), + KubeadmCertAPIServer(), + KubeadmCertKubeletClient(), // Front Proxy certs - &KubeadmCertFrontProxyCA, - &KubeadmCertFrontProxyClient, + KubeadmCertFrontProxyCA(), + KubeadmCertFrontProxyClient(), } } -var ( - // KubeadmCertRootCA is the definition of the Kubernetes Root CA for the API Server and kubelet. - KubeadmCertRootCA = KubeadmCert{ +// KubeadmCertRootCA is the definition of the Kubernetes Root CA for the API Server and kubelet. +func KubeadmCertRootCA() *KubeadmCert { + return &KubeadmCert{ Name: "ca", LongName: "self-signed Kubernetes CA to provision identities for other Kubernetes components", BaseName: kubeadmconstants.CACertAndKeyBaseName, @@ -251,8 +251,11 @@ var ( }, }, } - // KubeadmCertAPIServer is the definition of the cert used to serve the Kubernetes API. - KubeadmCertAPIServer = KubeadmCert{ +} + +// KubeadmCertAPIServer is the definition of the cert used to serve the Kubernetes API. +func KubeadmCertAPIServer() *KubeadmCert { + return &KubeadmCert{ Name: "apiserver", LongName: "certificate for serving the Kubernetes API", BaseName: kubeadmconstants.APIServerCertAndKeyBaseName, @@ -267,8 +270,11 @@ var ( makeAltNamesMutator(pkiutil.GetAPIServerAltNames), }, } - // KubeadmCertKubeletClient is the definition of the cert used by the API server to access the kubelet. - KubeadmCertKubeletClient = KubeadmCert{ +} + +// KubeadmCertKubeletClient is the definition of the cert used by the API server to access the kubelet. +func KubeadmCertKubeletClient() *KubeadmCert { + return &KubeadmCert{ Name: "apiserver-kubelet-client", LongName: "certificate for the API server to connect to kubelet", BaseName: kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName, @@ -281,9 +287,11 @@ var ( }, }, } +} - // KubeadmCertFrontProxyCA is the definition of the CA used for the front end proxy. - KubeadmCertFrontProxyCA = KubeadmCert{ +// KubeadmCertFrontProxyCA is the definition of the CA used for the front end proxy. +func KubeadmCertFrontProxyCA() *KubeadmCert { + return &KubeadmCert{ Name: "front-proxy-ca", LongName: "self-signed CA to provision identities for front proxy", BaseName: kubeadmconstants.FrontProxyCACertAndKeyBaseName, @@ -293,9 +301,11 @@ var ( }, }, } +} - // KubeadmCertFrontProxyClient is the definition of the cert used by the API server to access the front proxy. - KubeadmCertFrontProxyClient = KubeadmCert{ +// KubeadmCertFrontProxyClient is the definition of the cert used by the API server to access the front proxy. +func KubeadmCertFrontProxyClient() *KubeadmCert { + return &KubeadmCert{ Name: "front-proxy-client", BaseName: kubeadmconstants.FrontProxyClientCertAndKeyBaseName, LongName: "certificate for the front proxy client", @@ -307,9 +317,11 @@ var ( }, }, } +} - // KubeadmCertEtcdCA is the definition of the root CA used by the hosted etcd server. - KubeadmCertEtcdCA = KubeadmCert{ +// KubeadmCertEtcdCA is the definition of the root CA used by the hosted etcd server. +func KubeadmCertEtcdCA() *KubeadmCert { + return &KubeadmCert{ Name: "etcd-ca", LongName: "self-signed CA to provision identities for etcd", BaseName: kubeadmconstants.EtcdCACertAndKeyBaseName, @@ -319,8 +331,11 @@ var ( }, }, } - // KubeadmCertEtcdServer is the definition of the cert used to serve etcd to clients. - KubeadmCertEtcdServer = KubeadmCert{ +} + +// KubeadmCertEtcdServer is the definition of the cert used to serve etcd to clients. +func KubeadmCertEtcdServer() *KubeadmCert { + return &KubeadmCert{ Name: "etcd-server", LongName: "certificate for serving etcd", BaseName: kubeadmconstants.EtcdServerCertAndKeyBaseName, @@ -339,8 +354,11 @@ var ( setCommonNameToNodeName(), }, } - // KubeadmCertEtcdPeer is the definition of the cert used by etcd peers to access each other. - KubeadmCertEtcdPeer = KubeadmCert{ +} + +// KubeadmCertEtcdPeer is the definition of the cert used by etcd peers to access each other. +func KubeadmCertEtcdPeer() *KubeadmCert { + return &KubeadmCert{ Name: "etcd-peer", LongName: "certificate for etcd nodes to communicate with each other", BaseName: kubeadmconstants.EtcdPeerCertAndKeyBaseName, @@ -355,8 +373,11 @@ var ( setCommonNameToNodeName(), }, } - // KubeadmCertEtcdHealthcheck is the definition of the cert used by Kubernetes to check the health of the etcd server. - KubeadmCertEtcdHealthcheck = KubeadmCert{ +} + +// KubeadmCertEtcdHealthcheck is the definition of the cert used by Kubernetes to check the health of the etcd server. +func KubeadmCertEtcdHealthcheck() *KubeadmCert { + return &KubeadmCert{ Name: "etcd-healthcheck-client", LongName: "certificate for liveness probes to healthcheck etcd", BaseName: kubeadmconstants.EtcdHealthcheckClientCertAndKeyBaseName, @@ -369,8 +390,11 @@ var ( }, }, } - // KubeadmCertEtcdAPIClient is the definition of the cert used by the API server to access etcd. - KubeadmCertEtcdAPIClient = KubeadmCert{ +} + +// KubeadmCertEtcdAPIClient is the definition of the cert used by the API server to access etcd. +func KubeadmCertEtcdAPIClient() *KubeadmCert { + return &KubeadmCert{ Name: "apiserver-etcd-client", LongName: "certificate the apiserver uses to access etcd", BaseName: kubeadmconstants.APIServerEtcdClientCertAndKeyBaseName, @@ -383,7 +407,7 @@ var ( }, }, } -) +} func makeAltNamesMutator(f func(*kubeadmapi.InitConfiguration) (*certutil.AltNames, error)) configMutatorsFunc { return func(mc *kubeadmapi.InitConfiguration, cc *pkiutil.CertConfig) error { diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index 660da8b0f59..c1c43e3e9b8 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -685,7 +685,7 @@ func TestValidateMethods(t *testing.T) { } func TestNewCSR(t *testing.T) { - kubeadmCert := KubeadmCertAPIServer + kubeadmCert := KubeadmCertAPIServer() cfg := testutil.GetDefaultInternalConfig(t) certConfig, err := kubeadmCert.GetConfig(cfg) @@ -693,7 +693,7 @@ func TestNewCSR(t *testing.T) { t.Fatalf("couldn't get cert config: %v", err) } - csr, _, err := NewCSR(&kubeadmCert, cfg) + csr, _, err := NewCSR(kubeadmCert, cfg) if err != nil { t.Errorf("invalid signature on CSR: %v", err) diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods.go b/cmd/kubeadm/app/phases/upgrade/staticpods.go index 673be53f3b3..60c5b1cc138 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods.go @@ -554,9 +554,9 @@ func renewCertsByComponent(cfg *kubeadmapi.InitConfiguration, component string, if component == constants.Etcd { if cfg.Etcd.Local != nil { certificates = []string{ - certsphase.KubeadmCertEtcdServer.Name, - certsphase.KubeadmCertEtcdPeer.Name, - certsphase.KubeadmCertEtcdHealthcheck.Name, + certsphase.KubeadmCertEtcdServer().Name, + certsphase.KubeadmCertEtcdPeer().Name, + certsphase.KubeadmCertEtcdHealthcheck().Name, } } } @@ -565,12 +565,12 @@ func renewCertsByComponent(cfg *kubeadmapi.InitConfiguration, component string, //if local etcd, renew also the etcd client certificate if component == constants.KubeAPIServer { certificates = []string{ - certsphase.KubeadmCertAPIServer.Name, - certsphase.KubeadmCertKubeletClient.Name, - certsphase.KubeadmCertFrontProxyClient.Name, + certsphase.KubeadmCertAPIServer().Name, + certsphase.KubeadmCertKubeletClient().Name, + certsphase.KubeadmCertFrontProxyClient().Name, } if cfg.Etcd.Local != nil { - certificates = append(certificates, certsphase.KubeadmCertEtcdAPIClient.Name) + certificates = append(certificates, certsphase.KubeadmCertEtcdAPIClient().Name) } } diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go index d206672dd71..be7c8a6ed8b 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go @@ -513,15 +513,15 @@ func TestStaticPodControlPlane(t *testing.T) { } // create the kubeadm etcd certs - caCert, caKey, err := certsphase.KubeadmCertEtcdCA.CreateAsCA(newcfg) + caCert, caKey, err := certsphase.KubeadmCertEtcdCA().CreateAsCA(newcfg) if err != nil { t.Fatalf("couldn't create new CA certificate: %v", err) } for _, cert := range []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdServer, - &certsphase.KubeadmCertEtcdPeer, - &certsphase.KubeadmCertEtcdHealthcheck, - &certsphase.KubeadmCertEtcdAPIClient, + certsphase.KubeadmCertEtcdServer(), + certsphase.KubeadmCertEtcdPeer(), + certsphase.KubeadmCertEtcdHealthcheck(), + certsphase.KubeadmCertEtcdAPIClient(), } { if err := cert.CreateFromCA(newcfg, caCert, caKey); err != nil { t.Fatalf("couldn't create certificate %s: %v", cert.Name, err) @@ -685,33 +685,33 @@ func TestRenewCertsByComponent(t *testing.T) { name: "all CA exist, all certs should be rotated for etcd", component: constants.Etcd, certsShouldExist: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdServer, - &certsphase.KubeadmCertEtcdPeer, - &certsphase.KubeadmCertEtcdHealthcheck, + certsphase.KubeadmCertEtcdServer(), + certsphase.KubeadmCertEtcdPeer(), + certsphase.KubeadmCertEtcdHealthcheck(), }, }, { name: "all CA exist, all certs should be rotated for apiserver", component: constants.KubeAPIServer, certsShouldExist: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertAPIServer, - &certsphase.KubeadmCertKubeletClient, - &certsphase.KubeadmCertFrontProxyClient, + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertAPIServer(), + certsphase.KubeadmCertKubeletClient(), + certsphase.KubeadmCertFrontProxyClient(), }, }, { name: "external CA, renew only certificates not signed by CA for apiserver", component: constants.KubeAPIServer, certsShouldExist: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertFrontProxyClient, - &certsphase.KubeadmCertAPIServer, - &certsphase.KubeadmCertKubeletClient, + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertFrontProxyClient(), + certsphase.KubeadmCertAPIServer(), + certsphase.KubeadmCertKubeletClient(), }, certsShouldBeRenewed: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertFrontProxyClient, + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertFrontProxyClient(), }, externalCA: true, }, @@ -719,15 +719,15 @@ func TestRenewCertsByComponent(t *testing.T) { name: "external front-proxy-CA, renew only certificates not signed by front-proxy-CA for apiserver", component: constants.KubeAPIServer, certsShouldExist: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertFrontProxyClient, - &certsphase.KubeadmCertAPIServer, - &certsphase.KubeadmCertKubeletClient, + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertFrontProxyClient(), + certsphase.KubeadmCertAPIServer(), + certsphase.KubeadmCertKubeletClient(), }, certsShouldBeRenewed: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdAPIClient, - &certsphase.KubeadmCertAPIServer, - &certsphase.KubeadmCertKubeletClient, + certsphase.KubeadmCertEtcdAPIClient(), + certsphase.KubeadmCertAPIServer(), + certsphase.KubeadmCertKubeletClient(), }, externalFrontProxyCA: true, }, @@ -750,8 +750,8 @@ func TestRenewCertsByComponent(t *testing.T) { component: constants.Etcd, shouldErrorOnRenew: true, certsShouldExist: []*certsphase.KubeadmCert{ - &certsphase.KubeadmCertEtcdServer, - &certsphase.KubeadmCertEtcdPeer, + certsphase.KubeadmCertEtcdServer(), + certsphase.KubeadmCertEtcdPeer(), }, }, { diff --git a/cmd/kubeadm/test/cmd/init_test.go b/cmd/kubeadm/test/cmd/init_test.go index 6a323923d9f..d120f87dec2 100644 --- a/cmd/kubeadm/test/cmd/init_test.go +++ b/cmd/kubeadm/test/cmd/init_test.go @@ -212,11 +212,11 @@ func TestCmdInitCertPhaseCSR(t *testing.T) { }{ { name: "generate CSR", - baseName: certs.KubeadmCertKubeletClient.BaseName, + baseName: certs.KubeadmCertKubeletClient().BaseName, }, { name: "fails on CSR", - baseName: certs.KubeadmCertRootCA.BaseName, + baseName: certs.KubeadmCertRootCA().BaseName, expectedError: "unknown flag: --csr-only", }, { @@ -229,7 +229,7 @@ func TestCmdInitCertPhaseCSR(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { csrDir := testutil.SetupTempDir(t) - cert := &certs.KubeadmCertKubeletClient + cert := certs.KubeadmCertKubeletClient() kubeadmPath := getKubeadmPath() _, stderr, _, err := RunCmd(kubeadmPath, "init",