From 5fac458f5f3065f563b69dfa8a0c5ff118df5331 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Fri, 18 Aug 2017 09:13:49 +0200 Subject: [PATCH 1/3] Main work -- refactor certs phase --- cmd/kubeadm/app/phases/certs/certs.go | 288 +++++++++++++- cmd/kubeadm/app/phases/certs/certs_test.go | 417 ++++++++++++++++++--- cmd/kubeadm/test/certs/util.go | 55 +++ cmd/kubeadm/test/util.go | 11 + 4 files changed, 704 insertions(+), 67 deletions(-) diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index 0fc6b5b27fa..a6b060a0da2 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -22,14 +22,164 @@ import ( "fmt" "net" - "k8s.io/apimachinery/pkg/util/validation" certutil "k8s.io/client-go/util/cert" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" "k8s.io/kubernetes/pkg/registry/core/service/ipallocator" + "k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/validation" ) +// CreatePKIAssets will create and write to disk all PKI assets necessary to establish the control plane. +// If the PKI assets already exists in the target folder, they are used only if evaluated equal; otherwise an error is returned. +func CreatePKIAssets(cfg *kubeadmapi.MasterConfiguration) error { + + certActions := []func(cfg *kubeadmapi.MasterConfiguration) error{ + CreateCACertAndKeyfiles, + CreateAPIServerCertAndKeyFiles, + CreateAPIServerKubeletClientCertAndKeyFiles, + CreateServiceAccountKeyAndPublicKeyFiles, + CreateFrontProxyCACertAndKeyFiles, + CreateFrontProxyClientCertAndKeyFiles, + } + + for _, action := range certActions { + err := action(cfg) + if err != nil { + return err + } + } + + fmt.Printf("[certificates] Valid certificates and keys now exist in %q\n", cfg.CertificatesDir) + + return nil +} + +// CreateCACertAndKeyfiles create a new self signed CA certificate and key files. +// If the CA certificate and key files already exists in the target folder, they are used only if evaluated equal; otherwise an error is returned. +func CreateCACertAndKeyfiles(cfg *kubeadmapi.MasterConfiguration) error { + + caCert, caKey, err := NewCACertAndKey() + if err != nil { + return err + } + + return writeCertificateAuthorithyFilesIfNotExist( + cfg.CertificatesDir, + kubeadmconstants.CACertAndKeyBaseName, + caCert, + caKey, + ) +} + +// CreateAPIServerCertAndKeyFiles create a new certificate and key files for the apiserver. +// If the apiserver certificate and key files already exists in the target folder, they are used only if evaluated equal; otherwise an error is returned. +// It assumes the cluster CA certificate and key files should exists into the CertificatesDir +func CreateAPIServerCertAndKeyFiles(cfg *kubeadmapi.MasterConfiguration) error { + + caCert, caKey, err := loadCertificateAuthorithy(cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName) + if err != nil { + return err + } + + apiCert, apiKey, err := NewAPIServerCertAndKey(cfg, caCert, caKey) + if err != nil { + return err + } + + return writeCertificateFilesIfNotExist( + cfg.CertificatesDir, + kubeadmconstants.APIServerCertAndKeyBaseName, + caCert, + apiCert, + apiKey, + ) +} + +// CreateAPIServerKubeletClientCertAndKeyFiles create a new CA certificate for kubelets calling apiserver +// If the apiserver-kubelet-client certificate and key files already exists in the target folder, they are used only if evaluated equals; otherwise an error is returned. +// It assumes the cluster CA certificate and key files should exists into the CertificatesDir +func CreateAPIServerKubeletClientCertAndKeyFiles(cfg *kubeadmapi.MasterConfiguration) error { + + caCert, caKey, err := loadCertificateAuthorithy(cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName) + if err != nil { + return err + } + + apiClientCert, apiClientKey, err := NewAPIServerKubeletClientCertAndKey(caCert, caKey) + if err != nil { + return err + } + + return writeCertificateFilesIfNotExist( + cfg.CertificatesDir, + kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName, + caCert, + apiClientCert, + apiClientKey, + ) +} + +// CreateServiceAccountKeyAndPublicKeyFiles create a new public/private key files for signing service account users. +// If the sa public/private key files already exists in the target folder, they are used only if evaluated equals; otherwise an error is returned. +func CreateServiceAccountKeyAndPublicKeyFiles(cfg *kubeadmapi.MasterConfiguration) error { + + saSigningKey, err := NewServiceAccountSigningKey() + if err != nil { + return err + } + + return writeKeyFilesIfNotExist( + cfg.CertificatesDir, + kubeadmconstants.ServiceAccountKeyBaseName, + saSigningKey, + ) +} + +// CreateFrontProxyCACertAndKeyFiles create a self signed front proxy CA certificate and key files. +// Front proxy CA and client certs are used to secure a front proxy authenticator which is used to assert identity +// without the client cert; This is a separte CA, so that front proxy identities cannot hit the API and normal client certs cannot be used +// as front proxies. +// If the front proxy CA certificate and key files already exists in the target folder, they are used only if evaluated equals; otherwise an error is returned. +func CreateFrontProxyCACertAndKeyFiles(cfg *kubeadmapi.MasterConfiguration) error { + + frontProxyCACert, frontProxyCAKey, err := NewFrontProxyCACertAndKey() + if err != nil { + return err + } + + return writeCertificateAuthorithyFilesIfNotExist( + cfg.CertificatesDir, + kubeadmconstants.FrontProxyCACertAndKeyBaseName, + frontProxyCACert, + frontProxyCAKey, + ) +} + +// CreateFrontProxyClientCertAndKeyFiles create a new certificate for proxy server client. +// If the front-proxy-client certificate and key files already exists in the target folder, they are used only if evaluated equals; otherwise an error is returned. +// It assumes the front proxy CAA certificate and key files should exists into the CertificatesDir +func CreateFrontProxyClientCertAndKeyFiles(cfg *kubeadmapi.MasterConfiguration) error { + + frontProxyCACert, frontProxyCAKey, err := loadCertificateAuthorithy(cfg.CertificatesDir, kubeadmconstants.FrontProxyCACertAndKeyBaseName) + if err != nil { + return err + } + + frontProxyClientCert, frontProxyClientKey, err := NewFrontProxyClientCertAndKey(frontProxyCACert, frontProxyCAKey) + if err != nil { + return err + } + + return writeCertificateFilesIfNotExist( + cfg.CertificatesDir, + kubeadmconstants.FrontProxyClientCertAndKeyBaseName, + frontProxyCACert, + frontProxyClientCert, + frontProxyClientKey, + ) +} + // NewCACertAndKey will generate a self signed CA. func NewCACertAndKey() (*x509.Certificate, *rsa.PrivateKey, error) { @@ -91,10 +241,6 @@ func NewServiceAccountSigningKey() (*rsa.PrivateKey, error) { } // NewFrontProxyCACertAndKey generate a self signed front proxy CA. -// Front proxy CA and client certs are used to secure a front proxy authenticator which is used to assert identity -// without the client cert. -// This is a separte CA, so that front proxy identities cannot hit the API and normal client certs cannot be used -// as front proxies. func NewFrontProxyCACertAndKey() (*x509.Certificate, *rsa.PrivateKey, error) { frontProxyCACert, frontProxyCAKey, err := pkiutil.NewCertificateAuthority() @@ -120,6 +266,138 @@ func NewFrontProxyClientCertAndKey(frontProxyCACert *x509.Certificate, frontProx return frontProxyClientCert, frontProxyClientKey, nil } +// loadCertificateAuthorithy loads certificate authorithy +func loadCertificateAuthorithy(pkiDir string, baseName string) (*x509.Certificate, *rsa.PrivateKey, error) { + // Checks if certificate authorithy exists in the PKI directory + if !pkiutil.CertOrKeyExist(pkiDir, baseName) { + return nil, nil, fmt.Errorf("couldn't load %s certificate authorithy from %s", baseName, pkiDir) + } + + // Try to load certificate authorithy .crt and .key from the PKI directory + caCert, caKey, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) + if err != nil { + return nil, nil, fmt.Errorf("failure loading %s certificate authorithy: %v", baseName, err) + } + + // Make sure the loaded CA cert actually is a CA + if !caCert.IsCA { + return nil, nil, fmt.Errorf("%s certificate is not a certificate authorithy", baseName) + } + + return caCert, caKey, nil +} + +// writeCertificateAuthorithyFilesIfNotExist write a new certificate Authorithy to the given path. +// If there already is a certificate file at the given path; kubeadm tries to load it and check if the values in the +// existing and the expected certificate equals. If they do; kubeadm will just skip writing the file as it's up-to-date, +// otherwise this function returns an error. +func writeCertificateAuthorithyFilesIfNotExist(pkiDir string, baseName string, caCert *x509.Certificate, caKey *rsa.PrivateKey) error { + + // If cert or key exists, we should try to load them + if pkiutil.CertOrKeyExist(pkiDir, baseName) { + + // Try to load .crt and .key from the PKI directory + caCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) + if err != nil { + return fmt.Errorf("failure loading %s certificate: %v", baseName, err) + } + + // Check if the existing cert is a CA + if !caCert.IsCA { + return fmt.Errorf("certificate %s is not a CA", baseName) + } + + // kubeadm doesn't validate the existing certificate Authorithy more than this; + // Basically, if we find a certificate file with the same path; and it is a CA + // kubeadm thinks those files are equal and doesn't bother writing a new file + fmt.Printf("[certificates] Using the existing %s certificate and key.\n", baseName) + } else { + + // Write .crt and .key files to disk + if err := pkiutil.WriteCertAndKey(pkiDir, baseName, caCert, caKey); err != nil { + return fmt.Errorf("failure while saving %s certificate and key: %v", baseName, err) + } + + fmt.Printf("[certificates] Generated %s certificate and key.\n", baseName) + } + return nil +} + +// writeCertificateFilesIfNotExist write a new certificate to the given path. +// If there already is a certificate file at the given path; kubeadm tries to load it and check if the values in the +// existing and the expected certificate equals. If they do; kubeadm will just skip writing the file as it's up-to-date, +// otherwise this function returns an error. +func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert *x509.Certificate, cert *x509.Certificate, key *rsa.PrivateKey) error { + + // Checks if the signed certificate exists in the PKI directory + if pkiutil.CertOrKeyExist(pkiDir, baseName) { + // Try to load signed certificate .crt and .key from the PKI directory + signedCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) + if err != nil { + return fmt.Errorf("failure loading %s certificate: %v", baseName, err) + } + + // Check if the existing cert is signed by the given CA + if err := signedCert.CheckSignatureFrom(signingCert); err != nil { + return fmt.Errorf("certificate %s is not signed by corresponding CA", baseName) + } + + // kubeadm doesn't validate the existing certificate more than this; + // Basically, if we find a certificate file with the same path; and it is signed by + // the expected certificate authorithy, kubeadm thinks those files are equal and + // doesn't bother writing a new file + fmt.Printf("[certificates] Using the existing %s certificate and key.\n", baseName) + } else { + + // Write .crt and .key files to disk + if err := pkiutil.WriteCertAndKey(pkiDir, baseName, cert, key); err != nil { + return fmt.Errorf("failure while saving %s certificate and key: %v", baseName, err) + } + + fmt.Printf("[certificates] Generated %s certificate and key.\n", baseName) + if pkiutil.HasServerAuth(cert) { + fmt.Printf("[certificates] %s serving cert is signed for DNS names %v and IPs %v\n", baseName, cert.DNSNames, cert.IPAddresses) + } + } + + return nil +} + +// writeKeyFilesIfNotExist write a new key to the given path. +// If there already is a key file at the given path; kubeadm tries to load it and check if the values in the +// existing and the expected key equals. If they do; kubeadm will just skip writing the file as it's up-to-date, +// otherwise this function returns an error. +func writeKeyFilesIfNotExist(pkiDir string, baseName string, key *rsa.PrivateKey) error { + + // Checks if the key exists in the PKI directory + if pkiutil.CertOrKeyExist(pkiDir, baseName) { + + // Try to load .key from the PKI directory + _, err := pkiutil.TryLoadKeyFromDisk(pkiDir, baseName) + if err != nil { + return fmt.Errorf("%s key existed but it could not be loaded properly: %v", baseName, err) + } + + // kubeadm doesn't validate the existing certificate key more than this; + // Basically, if we find a key file with the same path kubeadm thinks those files + // are equal and doesn't bother writing a new file + fmt.Printf("[certificates] Using the existing %s key.\n", baseName) + } else { + + // Write .key and .pub files to disk + if err := pkiutil.WriteKey(pkiDir, baseName, key); err != nil { + return fmt.Errorf("failure while saving %s key: %v", baseName, err) + } + + if err := pkiutil.WritePublicKey(pkiDir, baseName, &key.PublicKey); err != nil { + return fmt.Errorf("failure while saving %s public key: %v", baseName, err) + } + fmt.Printf("[certificates] Generated %s key and public key.\n", baseName) + } + + return nil +} + // getAltNames builds an AltNames object for to be used when generating apiserver certificate func getAltNames(cfg *kubeadmapi.MasterConfiguration) (*certutil.AltNames, error) { diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index 7a058373fe6..5840e648524 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -17,21 +17,298 @@ limitations under the License. package certs import ( + "crypto/rsa" "crypto/x509" "net" + "os" "testing" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" - "k8s.io/kubernetes/cmd/kubeadm/app/constants" + kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" + + testutil "k8s.io/kubernetes/cmd/kubeadm/test" + certstestutil "k8s.io/kubernetes/cmd/kubeadm/test/certs" ) +func TestWriteCertificateAuthorithyFilesIfNotExist(t *testing.T) { + + setupCert, setupKey, _ := NewCACertAndKey() + caCert, caKey, _ := NewCACertAndKey() + + var tests = []struct { + setupFunc func(pkiDir string) error + expectedError bool + expectedCa *x509.Certificate + }{ + { // ca cert does not exists > ca written + expectedCa: caCert, + }, + { // ca cert exists, is ca > existing ca used + setupFunc: func(pkiDir string) error { + return writeCertificateAuthorithyFilesIfNotExist(pkiDir, "dummy", setupCert, setupKey) + }, + expectedCa: setupCert, + }, + { // some file exists, but it is not a valid ca cert > err + setupFunc: func(pkiDir string) error { + testutil.SetupEmptyFiles(t, pkiDir, "dummy.crt") + return nil + }, + expectedError: true, + }, + { // cert exists, but it is not a ca > err + setupFunc: func(pkiDir string) error { + cert, key, _ := NewFrontProxyClientCertAndKey(setupCert, setupKey) + return writeCertificateFilesIfNotExist(pkiDir, "dummy", setupCert, cert, key) + }, + expectedError: true, + }, + } + + for _, test := range tests { + // Create temp folder for the test case + tmpdir := testutil.SetupTempDir(t) + defer os.RemoveAll(tmpdir) + + // executes setup func (if necessary) + if test.setupFunc != nil { + if err := test.setupFunc(tmpdir); err != nil { + t.Errorf("error executing setupFunc: %v", err) + continue + } + } + + // executes create func + err := writeCertificateAuthorithyFilesIfNotExist(tmpdir, "dummy", caCert, caKey) + + if !test.expectedError && err != nil { + t.Errorf("error writeCertificateAuthorithyFilesIfNotExist failed when not expected to fail: %v", err) + continue + } else if test.expectedError && err == nil { + t.Error("error writeCertificateAuthorithyFilesIfNotExist didn't failed when expected") + continue + } else if test.expectedError { + continue + } + + // asserts expected files are there + testutil.AssertFileExists(t, tmpdir, "dummy.key", "dummy.crt") + + // check created cert + resultingCaCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(tmpdir, "dummy") + if err != nil { + t.Errorf("failure reading created cert: %v", err) + continue + } + if !resultingCaCert.Equal(test.expectedCa) { + t.Error("created ca cert does not match expected ca cert") + } + } +} + +func TestWriteCertificateFilesIfNotExist(t *testing.T) { + + caCert, caKey, _ := NewFrontProxyCACertAndKey() + setupCert, setupKey, _ := NewFrontProxyClientCertAndKey(caCert, caKey) + cert, key, _ := NewFrontProxyClientCertAndKey(caCert, caKey) + + var tests = []struct { + setupFunc func(pkiDir string) error + expectedError bool + expectedCert *x509.Certificate + }{ + { // cert does not exists > cert written + expectedCert: cert, + }, + { // cert exists, is signed by the same ca > existing cert used + setupFunc: func(pkiDir string) error { + return writeCertificateFilesIfNotExist(pkiDir, "dummy", caCert, setupCert, setupKey) + }, + expectedCert: setupCert, + }, + { // some file exists, but it is not a valid cert > err + setupFunc: func(pkiDir string) error { + testutil.SetupEmptyFiles(t, pkiDir, "dummy.crt") + return nil + }, + expectedError: true, + }, + { // cert exists, is signed by another ca > err + setupFunc: func(pkiDir string) error { + anotherCaCert, anotherCaKey, _ := NewFrontProxyCACertAndKey() + anotherCert, anotherKey, _ := NewFrontProxyClientCertAndKey(anotherCaCert, anotherCaKey) + + return writeCertificateFilesIfNotExist(pkiDir, "dummy", anotherCaCert, anotherCert, anotherKey) + }, + expectedError: true, + }, + } + + for _, test := range tests { + // Create temp folder for the test case + tmpdir := testutil.SetupTempDir(t) + defer os.RemoveAll(tmpdir) + + // executes setup func (if necessary) + if test.setupFunc != nil { + if err := test.setupFunc(tmpdir); err != nil { + t.Errorf("error executing setupFunc: %v", err) + continue + } + } + + // executes create func + err := writeCertificateFilesIfNotExist(tmpdir, "dummy", caCert, cert, key) + + if !test.expectedError && err != nil { + t.Errorf("error writeCertificateFilesIfNotExist failed when not expected to fail: %v", err) + continue + } else if test.expectedError && err == nil { + t.Error("error writeCertificateFilesIfNotExist didn't failed when expected") + continue + } else if test.expectedError { + continue + } + + // asserts expected files are there + testutil.AssertFileExists(t, tmpdir, "dummy.key", "dummy.crt") + + // check created cert + resultingCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(tmpdir, "dummy") + if err != nil { + t.Errorf("failure reading created cert: %v", err) + continue + } + if !resultingCert.Equal(test.expectedCert) { + t.Error("created cert does not match expected cert") + } + } +} + +func TestWriteKeyFilesIfNotExist(t *testing.T) { + + setupKey, _ := NewServiceAccountSigningKey() + key, _ := NewServiceAccountSigningKey() + + var tests = []struct { + setupFunc func(pkiDir string) error + expectedError bool + expectedKey *rsa.PrivateKey + }{ + { // key does not exists > key written + expectedKey: key, + }, + { // key exists > existing key used + setupFunc: func(pkiDir string) error { + return writeKeyFilesIfNotExist(pkiDir, "dummy", setupKey) + }, + expectedKey: setupKey, + }, + { // some file exists, but it is not a valid key > err + setupFunc: func(pkiDir string) error { + testutil.SetupEmptyFiles(t, pkiDir, "dummy.key") + return nil + }, + expectedError: true, + }, + } + + for _, test := range tests { + // Create temp folder for the test case + tmpdir := testutil.SetupTempDir(t) + defer os.RemoveAll(tmpdir) + + // executes setup func (if necessary) + if test.setupFunc != nil { + if err := test.setupFunc(tmpdir); err != nil { + t.Errorf("error executing setupFunc: %v", err) + continue + } + } + + // executes create func + err := writeKeyFilesIfNotExist(tmpdir, "dummy", key) + + if !test.expectedError && err != nil { + t.Errorf("error writeKeyFilesIfNotExist failed when not expected to fail: %v", err) + continue + } else if test.expectedError && err == nil { + t.Error("error writeKeyFilesIfNotExist didn't failed when expected") + continue + } else if test.expectedError { + continue + } + + // asserts expected files are there + testutil.AssertFileExists(t, tmpdir, "dummy.key", "dummy.pub") + + // check created key + resultingKey, err := pkiutil.TryLoadKeyFromDisk(tmpdir, "dummy") + if err != nil { + t.Errorf("failure reading created key: %v", err) + continue + } + + //TODO: check if there is a better method to compare keys + if resultingKey.D == key.D { + t.Error("created key does not match expected key") + } + } +} + +func TestGetAltNames(t *testing.T) { + hostname := "valid-hostname" + advertiseIP := "1.2.3.4" + cfg := &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{AdvertiseAddress: advertiseIP}, + Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, + NodeName: hostname, + } + + altNames, err := getAltNames(cfg) + if err != nil { + t.Fatalf("failed calling getAltNames: %v", err) + } + + expectedDNSNames := []string{hostname, "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local"} + for _, DNSName := range expectedDNSNames { + found := false + for _, val := range altNames.DNSNames { + if val == DNSName { + found = true + break + } + } + + if !found { + t.Errorf("altNames does not contain DNSName %s", DNSName) + } + } + + expectedIPAddresses := []string{"10.96.0.1", advertiseIP} + for _, IPAddress := range expectedIPAddresses { + found := false + for _, val := range altNames.IPs { + if val.Equal(net.ParseIP(IPAddress)) { + found = true + break + } + } + + if !found { + t.Errorf("altNames does not contain IPAddress %s", IPAddress) + } + } +} + func TestNewCACertAndKey(t *testing.T) { caCert, _, err := NewCACertAndKey() if err != nil { t.Fatalf("failed call NewCACertAndKey: %v", err) } - assertIsCa(t, caCert) + certstestutil.AssertCertificateIsCa(t, caCert) } func TestNewAPIServerCertAndKey(t *testing.T) { @@ -51,15 +328,10 @@ func TestNewAPIServerCertAndKey(t *testing.T) { t.Fatalf("failed creation of cert and key: %v", err) } - assertIsSignedByCa(t, apiServerCert, caCert) - assertHasServerAuth(t, apiServerCert) - - for _, DNSName := range []string{hostname, "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local"} { - assertHasDNSNames(t, apiServerCert, DNSName) - } - for _, IPAddress := range []string{"10.96.0.1", addr} { - assertHasIPAddresses(t, apiServerCert, net.ParseIP(IPAddress)) - } + certstestutil.AssertCertificateIsSignedByCa(t, apiServerCert, caCert) + certstestutil.AssertCertificateHasServerAuthUsage(t, apiServerCert) + certstestutil.AssertCertificateHasDNSNames(t, apiServerCert, hostname, "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local") + certstestutil.AssertCertificateHasIPAddresses(t, apiServerCert, net.ParseIP("10.96.0.1"), net.ParseIP(addr)) } } @@ -71,9 +343,9 @@ func TestNewAPIServerKubeletClientCertAndKey(t *testing.T) { t.Fatalf("failed creation of cert and key: %v", err) } - assertIsSignedByCa(t, apiClientCert, caCert) - assertHasClientAuth(t, apiClientCert) - assertHasOrganization(t, apiClientCert, constants.MastersGroup) + certstestutil.AssertCertificateIsSignedByCa(t, apiClientCert, caCert) + certstestutil.AssertCertificateHasClientAuthUsage(t, apiClientCert) + certstestutil.AssertCertificateHasOrganizations(t, apiClientCert, kubeadmconstants.MastersGroup) } func TestNewNewServiceAccountSigningKey(t *testing.T) { @@ -94,7 +366,7 @@ func TestNewFrontProxyCACertAndKey(t *testing.T) { t.Fatalf("failed creation of cert and key: %v", err) } - assertIsCa(t, frontProxyCACert) + certstestutil.AssertCertificateIsCa(t, frontProxyCACert) } func TestNewFrontProxyClientCertAndKey(t *testing.T) { @@ -105,63 +377,84 @@ func TestNewFrontProxyClientCertAndKey(t *testing.T) { t.Fatalf("failed creation of cert and key: %v", err) } - assertIsSignedByCa(t, frontProxyClientCert, frontProxyCACert) - assertHasClientAuth(t, frontProxyClientCert) + certstestutil.AssertCertificateIsSignedByCa(t, frontProxyClientCert, frontProxyCACert) + certstestutil.AssertCertificateHasClientAuthUsage(t, frontProxyClientCert) } -func assertIsCa(t *testing.T, cert *x509.Certificate) { - if !cert.IsCA { - t.Error("cert is not a valida CA") +func TestCreateCertificateFilesMethods(t *testing.T) { + + var tests = []struct { + setupFunc func(cfg *kubeadmapi.MasterConfiguration) error + createFunc func(cfg *kubeadmapi.MasterConfiguration) error + expectedFiles []string + }{ + { + createFunc: CreatePKIAssets, + expectedFiles: []string{ + kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, + kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, + kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName, + kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName, + kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, + kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName, + }, + }, + { + createFunc: CreateCACertAndKeyfiles, + expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName}, + }, + { + setupFunc: CreateCACertAndKeyfiles, + createFunc: CreateAPIServerCertAndKeyFiles, + expectedFiles: []string{kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName}, + }, + { + setupFunc: CreateCACertAndKeyfiles, + createFunc: CreateAPIServerKubeletClientCertAndKeyFiles, + expectedFiles: []string{kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, + }, + { + createFunc: CreateServiceAccountKeyAndPublicKeyFiles, + expectedFiles: []string{kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName}, + }, + { + createFunc: CreateFrontProxyCACertAndKeyFiles, + expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName}, + }, + { + setupFunc: CreateFrontProxyCACertAndKeyFiles, + createFunc: CreateFrontProxyClientCertAndKeyFiles, + expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName}, + }, } -} -func assertIsSignedByCa(t *testing.T, cert *x509.Certificate, ca *x509.Certificate) { - if err := cert.CheckSignatureFrom(ca); err != nil { - t.Error("cert is not signed by ca") - } -} + for _, test := range tests { + // Create temp folder for the test case + tmpdir := testutil.SetupTempDir(t) + defer os.RemoveAll(tmpdir) -func assertHasClientAuth(t *testing.T, cert *x509.Certificate) { - for i := range cert.ExtKeyUsage { - if cert.ExtKeyUsage[i] == x509.ExtKeyUsageClientAuth { - return + cfg := &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4"}, + Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, + NodeName: "valid-hostname", + CertificatesDir: tmpdir, } - } - t.Error("cert is not a ClientAuth") -} -func assertHasServerAuth(t *testing.T, cert *x509.Certificate) { - for i := range cert.ExtKeyUsage { - if cert.ExtKeyUsage[i] == x509.ExtKeyUsageServerAuth { - return + // executes setup func (if necessary) + if test.setupFunc != nil { + if err := test.setupFunc(cfg); err != nil { + t.Errorf("error executing setupFunc: %v", err) + continue + } } - } - t.Error("cert is not a ServerAuth") -} -func assertHasOrganization(t *testing.T, cert *x509.Certificate, OU string) { - for i := range cert.Subject.Organization { - if cert.Subject.Organization[i] == OU { - return + // executes create func + if err := test.createFunc(cfg); err != nil { + t.Errorf("error executing createFunc: %v", err) + continue } - } - t.Errorf("cert does not contain OU %s", OU) -} -func assertHasDNSNames(t *testing.T, cert *x509.Certificate, DNSName string) { - for i := range cert.DNSNames { - if cert.DNSNames[i] == DNSName { - return - } + // asserts expected files are there + testutil.AssertFileExists(t, tmpdir, test.expectedFiles...) } - t.Errorf("cert does not contain DNSName %s", DNSName) -} - -func assertHasIPAddresses(t *testing.T, cert *x509.Certificate, IPAddress net.IP) { - for i := range cert.IPAddresses { - if cert.IPAddresses[i].Equal(IPAddress) { - return - } - } - t.Errorf("cert does not contain IPAddress %s", IPAddress) } diff --git a/cmd/kubeadm/test/certs/util.go b/cmd/kubeadm/test/certs/util.go index 30ef1d303ae..4f3235010cf 100644 --- a/cmd/kubeadm/test/certs/util.go +++ b/cmd/kubeadm/test/certs/util.go @@ -19,6 +19,7 @@ package certs import ( "crypto/rsa" "crypto/x509" + "net" "testing" "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" @@ -35,6 +36,13 @@ 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) { @@ -77,3 +85,50 @@ func AssertCertificateHasClientAuthUsage(t *testing.T, cert *x509.Certificate) { } t.Error("cert has not ClientAuth usage as expected") } + +// AssertCertificateHasServerAuthUsage is a utility function for kubeadm testing that asserts if a given certificate has +// the expected ExtKeyUsageServerAuth +func AssertCertificateHasServerAuthUsage(t *testing.T, cert *x509.Certificate) { + for i := range cert.ExtKeyUsage { + if cert.ExtKeyUsage[i] == x509.ExtKeyUsageServerAuth { + return + } + } + t.Error("cert is not a ServerAuth") +} + +// AssertCertificateHasDNSNames is a utility function for kubeadm testing that asserts if a given certificate has +// the expected DNSNames +func AssertCertificateHasDNSNames(t *testing.T, cert *x509.Certificate, DNSNames ...string) { + for _, DNSName := range DNSNames { + found := false + for _, val := range cert.DNSNames { + if val == DNSName { + found = true + break + } + } + + if !found { + t.Errorf("cert does not contain DNSName %s", DNSName) + } + } +} + +// AssertCertificateHasIPAddresses is a utility function for kubeadm testing that asserts if a given certificate has +// the expected IPAddresses +func AssertCertificateHasIPAddresses(t *testing.T, cert *x509.Certificate, IPAddresses ...net.IP) { + for _, IPAddress := range IPAddresses { + found := false + for _, val := range cert.IPAddresses { + if val.Equal(IPAddress) { + found = true + break + } + } + + if !found { + t.Errorf("cert does not contain IPAddress %s", IPAddress) + } + } +} diff --git a/cmd/kubeadm/test/util.go b/cmd/kubeadm/test/util.go index fe8ea38f781..916e72bf512 100644 --- a/cmd/kubeadm/test/util.go +++ b/cmd/kubeadm/test/util.go @@ -76,6 +76,17 @@ func SetupMasterConfigurationFile(t *testing.T, tmpdir string, cfg *kubeadmapi.M return cfgPath } +// SetupEmptyFiles is a utility function for kubeadm testing that creates one or more empty files (touch) +func SetupEmptyFiles(t *testing.T, tmpdir string, fileNames ...string) { + for _, fileName := range fileNames { + newFile, err := os.Create(filepath.Join(tmpdir, fileName)) + if err != nil { + t.Fatalf("Error creating file %s in %s: %v", fileName, tmpdir, err) + } + newFile.Close() + } +} + // SetupPkiDirWithCertificateAuthorithy is a utility function for kubeadm testing that creates a // CertificateAuthorithy cert/key pair into /pki subfolder of a given temporary directory. // The funtion returns the path of the created pki. From 00fa026b9d9c00076d1a64ed6c688355aac40615 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Fri, 18 Aug 2017 09:14:11 +0200 Subject: [PATCH 2/3] Main work -- cleanup certs CLI command --- cmd/kubeadm/app/cmd/init.go | 4 +- cmd/kubeadm/app/cmd/phases/certs.go | 251 ++--------------------- cmd/kubeadm/app/cmd/phases/certs_test.go | 106 +++++++--- 3 files changed, 92 insertions(+), 269 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index f1f88127397..39f1c8b7adb 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -31,13 +31,13 @@ import ( kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/features" - cmdphases "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" dnsaddonphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns" proxyaddonphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy" apiconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/apiconfig" clusterinfophase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo" nodebootstraptokenphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node" + certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" controlplanephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane" etcdphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/etcd" @@ -234,7 +234,7 @@ func (i *Init) Run(out io.Writer) error { } // PHASE 1: Generate certificates - if err := cmdphases.CreatePKIAssets(i.cfg); err != nil { + if err := certsphase.CreatePKIAssets(i.cfg); err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/phases/certs.go b/cmd/kubeadm/app/cmd/phases/certs.go index aa8c23ce1be..6e4344d74cd 100644 --- a/cmd/kubeadm/app/cmd/phases/certs.go +++ b/cmd/kubeadm/app/cmd/phases/certs.go @@ -17,17 +17,12 @@ limitations under the License. package phases import ( - "crypto/rsa" - "crypto/x509" - "fmt" - "github.com/spf13/cobra" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1" - kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" - certphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" - "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" + "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" + certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config" "k8s.io/kubernetes/pkg/api" @@ -64,37 +59,37 @@ func getCertsSubCommands() []*cobra.Command { { use: "all", short: "Generate all PKI assets necessary to establish the control plane", - cmdFunc: CreatePKIAssets, + cmdFunc: certsphase.CreatePKIAssets, }, { use: "ca", short: "Generate CA certificate and key for a Kubernetes cluster.", - cmdFunc: createOrUseCACertAndKey, + cmdFunc: certsphase.CreateCACertAndKeyfiles, }, { use: "apiserver", short: "Generate API Server serving certificate and key.", - cmdFunc: createOrUseAPIServerCertAndKey, + cmdFunc: certsphase.CreateAPIServerCertAndKeyFiles, }, { use: "apiserver-kubelet-client", short: "Generate a client certificate for the API Server to connect to the kubelets securely.", - cmdFunc: createOrUseAPIServerKubeletClientCertAndKey, + cmdFunc: certsphase.CreateAPIServerKubeletClientCertAndKeyFiles, }, { use: "sa", short: "Generate a private key for signing service account tokens along with its public key.", - cmdFunc: createOrUseServiceAccountKeyAndPublicKey, + cmdFunc: certsphase.CreateServiceAccountKeyAndPublicKeyFiles, }, { use: "front-proxy-ca", short: "Generate front proxy CA certificate and key for a Kubernetes cluster.", - cmdFunc: createOrUseFrontProxyCACertAndKey, + cmdFunc: certsphase.CreateFrontProxyCACertAndKeyFiles, }, { use: "front-proxy-client", short: "Generate front proxy CA client certificate and key for a Kubernetes cluster.", - cmdFunc: createOrUseFrontProxyClientCertAndKey, + cmdFunc: certsphase.CreateFrontProxyClientCertAndKeyFiles, }, } @@ -131,6 +126,9 @@ func runCmdFunc(cmdFunc func(cfg *kubeadmapi.MasterConfiguration) error, cfgPath // are shared between sub commands and gets access to current value e.g. flags value. return func(cmd *cobra.Command, args []string) { + if err := validation.ValidateMixedArguments(cmd.Flags()); err != nil { + kubeadmutil.CheckErr(err) + } // This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(*cfgPath, cfg) @@ -141,228 +139,3 @@ func runCmdFunc(cmdFunc func(cfg *kubeadmapi.MasterConfiguration) error, cfgPath kubeadmutil.CheckErr(err) } } - -// CreatePKIAssets will create and write to disk all PKI assets necessary to establish the control plane. -// Please note that this action is a bulk action calling all the atomic certphase actions -func CreatePKIAssets(cfg *kubeadmapi.MasterConfiguration) error { - - certActions := []func(cfg *kubeadmapi.MasterConfiguration) error{ - createOrUseCACertAndKey, - createOrUseAPIServerCertAndKey, - createOrUseAPIServerKubeletClientCertAndKey, - createOrUseServiceAccountKeyAndPublicKey, - createOrUseFrontProxyCACertAndKey, - createOrUseFrontProxyClientCertAndKey, - } - - for _, action := range certActions { - err := action(cfg) - if err != nil { - return err - } - } - - fmt.Printf("[certificates] Valid certificates and keys now exist in %q\n", cfg.CertificatesDir) - - return nil -} - -// createOrUseCACertAndKey create a new self signed CA, or use the existing one. -func createOrUseCACertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseCertificateAuthorithy( - cfg.CertificatesDir, - kubeadmconstants.CACertAndKeyBaseName, - "CA", - certphase.NewCACertAndKey, - ) -} - -// createOrUseAPIServerCertAndKey create a new CA certificate for apiserver, or use the existing one. -// It assumes the CA certificates should exists into the CertificatesDir -func createOrUseAPIServerCertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseSignedCertificate( - cfg.CertificatesDir, - kubeadmconstants.CACertAndKeyBaseName, - kubeadmconstants.APIServerCertAndKeyBaseName, - "API server", - func(caCert *x509.Certificate, caKey *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey, error) { - return certphase.NewAPIServerCertAndKey(cfg, caCert, caKey) - }, - ) -} - -// create a new CA certificate for kubelets calling apiserver, or use the existing one -// It assumes the CA certificates should exists into the CertificatesDir -func createOrUseAPIServerKubeletClientCertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseSignedCertificate( - cfg.CertificatesDir, - kubeadmconstants.CACertAndKeyBaseName, - kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName, - "API server kubelet client", - certphase.NewAPIServerKubeletClientCertAndKey, - ) -} - -// createOrUseServiceAccountKeyAndPublicKey create a new public/private key pairs for signing service account user, or use the existing one. -func createOrUseServiceAccountKeyAndPublicKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseKeyAndPublicKey( - cfg.CertificatesDir, - kubeadmconstants.ServiceAccountKeyBaseName, - "service account", - certphase.NewServiceAccountSigningKey, - ) -} - -// createOrUseFrontProxyCACertAndKey create a new self signed front proxy CA, or use the existing one. -func createOrUseFrontProxyCACertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseCertificateAuthorithy( - cfg.CertificatesDir, - kubeadmconstants.FrontProxyCACertAndKeyBaseName, - "front-proxy CA", - certphase.NewFrontProxyCACertAndKey, - ) -} - -// createOrUseFrontProxyClientCertAndKey create a new certificate for proxy server client, or use the existing one. -// It assumes the front proxy CA certificates should exists into the CertificatesDir -func createOrUseFrontProxyClientCertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseSignedCertificate( - cfg.CertificatesDir, - kubeadmconstants.FrontProxyCACertAndKeyBaseName, - kubeadmconstants.FrontProxyClientCertAndKeyBaseName, - "front-proxy client", - certphase.NewFrontProxyClientCertAndKey, - ) -} - -// createOrUseCertificateAuthorithy is a generic function that will create a new certificate Authorithy using the given newFunc, -// assign file names according to the given baseName, or use the existing one already present in pkiDir. -func createOrUseCertificateAuthorithy(pkiDir string, baseName string, UXName string, newFunc func() (*x509.Certificate, *rsa.PrivateKey, error)) error { - - // If cert or key exists, we should try to load them - if pkiutil.CertOrKeyExist(pkiDir, baseName) { - - // Try to load .crt and .key from the PKI directory - caCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) - if err != nil { - return fmt.Errorf("failure loading %s certificate: %v", UXName, err) - } - - // Check if the existing cert is a CA - if !caCert.IsCA { - return fmt.Errorf("certificate %s is not a CA", UXName) - } - - fmt.Printf("[certificates] Using the existing %s certificate and key.\n", UXName) - } else { - // The certificate and the key did NOT exist, let's generate them now - caCert, caKey, err := newFunc() - if err != nil { - return fmt.Errorf("failure while generating %s certificate and key: %v", UXName, err) - } - - // Write .crt and .key files to disk - if err = pkiutil.WriteCertAndKey(pkiDir, baseName, caCert, caKey); err != nil { - return fmt.Errorf("failure while saving %s certificate and key: %v", UXName, err) - } - - fmt.Printf("[certificates] Generated %s certificate and key.\n", UXName) - } - return nil -} - -// createOrUseSignedCertificate is a generic function that will create a new signed certificate using the given newFunc, -// assign file names according to the given baseName, or use the existing one already present in pkiDir. -func createOrUseSignedCertificate(pkiDir string, CABaseName string, baseName string, UXName string, newFunc func(*x509.Certificate, *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey, error)) error { - - // Checks if certificate authorithy exists in the PKI directory - if !pkiutil.CertOrKeyExist(pkiDir, CABaseName) { - return fmt.Errorf("couldn't load certificate authorithy for %s from certificate dir", UXName) - } - - // Try to load certificate authorithy .crt and .key from the PKI directory - caCert, caKey, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, CABaseName) - if err != nil { - return fmt.Errorf("failure loading certificate authorithy for %s: %v", UXName, err) - } - - // Make sure the loaded CA cert actually is a CA - if !caCert.IsCA { - return fmt.Errorf("certificate authorithy for %s is not a CA", UXName) - } - - // Checks if the signed certificate exists in the PKI directory - if pkiutil.CertOrKeyExist(pkiDir, baseName) { - // Try to load signed certificate .crt and .key from the PKI directory - signedCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) - if err != nil { - return fmt.Errorf("failure loading %s certificate: %v", UXName, err) - } - - // Check if the existing cert is signed by the given CA - if err := signedCert.CheckSignatureFrom(caCert); err != nil { - return fmt.Errorf("certificate %s is not signed by corresponding CA", UXName) - } - - fmt.Printf("[certificates] Using the existing %s certificate and key.\n", UXName) - } else { - // The certificate and the key did NOT exist, let's generate them now - signedCert, signedKey, err := newFunc(caCert, caKey) - if err != nil { - return fmt.Errorf("failure while generating %s key and certificate: %v", UXName, err) - } - - // Write .crt and .key files to disk - if err = pkiutil.WriteCertAndKey(pkiDir, baseName, signedCert, signedKey); err != nil { - return fmt.Errorf("failure while saving %s certificate and key: %v", UXName, err) - } - - fmt.Printf("[certificates] Generated %s certificate and key.\n", UXName) - if pkiutil.HasServerAuth(signedCert) { - fmt.Printf("[certificates] %s serving cert is signed for DNS names %v and IPs %v\n", UXName, signedCert.DNSNames, signedCert.IPAddresses) - } - } - - return nil -} - -// createOrUseKeyAndPublicKey is a generic function that will create a new public/private key pairs using the given newFunc, -// assign file names according to the given baseName, or use the existing one already present in pkiDir. -func createOrUseKeyAndPublicKey(pkiDir string, baseName string, UXName string, newFunc func() (*rsa.PrivateKey, error)) error { - - // Checks if the key exists in the PKI directory - if pkiutil.CertOrKeyExist(pkiDir, baseName) { - - // Try to load .key from the PKI directory - _, err := pkiutil.TryLoadKeyFromDisk(pkiDir, baseName) - if err != nil { - return fmt.Errorf("%s key existed but they could not be loaded properly: %v", UXName, err) - } - - fmt.Printf("[certificates] Using the existing %s key.\n", UXName) - } else { - // The key does NOT exist, let's generate it now - key, err := newFunc() - if err != nil { - return fmt.Errorf("failure while generating %s key: %v", UXName, err) - } - - // Write .key and .pub files to disk - if err = pkiutil.WriteKey(pkiDir, baseName, key); err != nil { - return fmt.Errorf("failure while saving %s key: %v", UXName, err) - } - - if err = pkiutil.WritePublicKey(pkiDir, baseName, &key.PublicKey); err != nil { - return fmt.Errorf("failure while saving %s public key: %v", UXName, err) - } - fmt.Printf("[certificates] Generated %s key and public key.\n", UXName) - } - - return nil -} diff --git a/cmd/kubeadm/app/cmd/phases/certs_test.go b/cmd/kubeadm/app/cmd/phases/certs_test.go index 8cb5e7414e2..abf67f69923 100644 --- a/cmd/kubeadm/app/cmd/phases/certs_test.go +++ b/cmd/kubeadm/app/cmd/phases/certs_test.go @@ -33,7 +33,61 @@ import ( cmdtestutil "k8s.io/kubernetes/cmd/kubeadm/test/cmd" ) -func TestSubCmdCertsCreateFiles(t *testing.T) { +func TestCertsSubCommandsHasFlags(t *testing.T) { + + subCmds := getCertsSubCommands() + + commonFlags := []string{ + "cert-dir", + "config", + } + + var tests = []struct { + command string + additionalFlags []string + }{ + { + command: "all", + additionalFlags: []string{ + "apiserver-advertise-address", + "apiserver-cert-extra-sans", + "service-cidr", + "service-dns-domain", + }, + }, + { + command: "ca", + }, + { + command: "apiserver", + additionalFlags: []string{ + "apiserver-advertise-address", + "apiserver-cert-extra-sans", + "service-cidr", + "service-dns-domain", + }, + }, + { + command: "apiserver-kubelet-client", + }, + { + command: "sa", + }, + { + command: "front-proxy-ca", + }, + { + command: "front-proxy-client", + }, + } + + for _, test := range tests { + expectedFlags := append(commonFlags, test.additionalFlags...) + cmdtestutil.AssertSubCommandHasFlags(t, subCmds, test.command, expectedFlags...) + } +} + +func TestSubCmdCertsCreateFilesWithFlags(t *testing.T) { subCmds := getCertsSubCommands() @@ -53,25 +107,13 @@ func TestSubCmdCertsCreateFiles(t *testing.T) { }, }, { - subCmds: []string{"ca"}, - expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName}, - }, - { - subCmds: []string{"ca", "apiserver"}, - expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName}, - }, - { - subCmds: []string{"ca", "apiserver-kubelet-client"}, - expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, + subCmds: []string{"ca", "apiserver", "apiserver-kubelet-client"}, + expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, }, { subCmds: []string{"sa"}, expectedFiles: []string{kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName}, }, - { - subCmds: []string{"front-proxy-ca"}, - expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName}, - }, { subCmds: []string{"front-proxy-ca", "front-proxy-client"}, expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName}, @@ -94,7 +136,7 @@ func TestSubCmdCertsCreateFiles(t *testing.T) { } } -func TestSubCmdApiServerFlags(t *testing.T) { +func TestSubCmdCertsApiServerForwardsFlags(t *testing.T) { subCmds := getCertsSubCommands() @@ -116,6 +158,7 @@ func TestSubCmdApiServerFlags(t *testing.T) { } cmdtestutil.RunSubCommand(t, subCmds, "apiserver", apiserverFlags...) + // asserts created cert has values from CLI flags APIserverCert, err := pkiutil.TryLoadCertFromDisk(tmpdir, kubeadmconstants.APIServerCertAndKeyBaseName) if err != nil { t.Fatalf("Error loading API server certificate: %v", err) @@ -135,29 +178,36 @@ func TestSubCmdApiServerFlags(t *testing.T) { } } -func TestSubCmdCertsReadsConfig(t *testing.T) { +func TestSubCmdCertsCreateFilesWithConfigFile(t *testing.T) { subCmds := getCertsSubCommands() var tests = []struct { - subCmds []string - expectedFileCount int + subCmds []string + expectedFiles []string }{ { - subCmds: []string{"sa"}, - expectedFileCount: 2, + subCmds: []string{"all"}, + expectedFiles: []string{ + kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, + kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, + kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName, + kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName, + kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, + kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName, + }, }, { - subCmds: []string{"front-proxy-ca", "front-proxy-client"}, - expectedFileCount: 4, + subCmds: []string{"ca", "apiserver", "apiserver-kubelet-client"}, + expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, }, { - subCmds: []string{"ca", "apiserver", "apiserver-kubelet-client"}, - expectedFileCount: 6, + subCmds: []string{"front-proxy-ca", "front-proxy-client"}, + expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName}, }, { - subCmds: []string{"all"}, - expectedFileCount: 12, + subCmds: []string{"sa"}, + expectedFiles: []string{kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName}, }, } @@ -182,6 +232,6 @@ func TestSubCmdCertsReadsConfig(t *testing.T) { } // verify expected files are there - testutil.AssertFilesCount(t, tmpdir, test.expectedFileCount) + testutil.AssertFileExists(t, tmpdir, test.expectedFiles...) } } From c6bb8fbb4aaa8981da8863b88e88ba7aed3ff3d0 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Fri, 18 Aug 2017 09:14:28 +0200 Subject: [PATCH 3/3] Autogenerated bazel files --- cmd/kubeadm/app/cmd/BUILD | 1 + cmd/kubeadm/app/cmd/phases/BUILD | 1 - cmd/kubeadm/app/phases/certs/BUILD | 5 ++++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/kubeadm/app/cmd/BUILD b/cmd/kubeadm/app/cmd/BUILD index e80b1b37996..6b4f40779f1 100644 --- a/cmd/kubeadm/app/cmd/BUILD +++ b/cmd/kubeadm/app/cmd/BUILD @@ -30,6 +30,7 @@ go_library( "//cmd/kubeadm/app/phases/apiconfig:go_default_library", "//cmd/kubeadm/app/phases/bootstraptoken/clusterinfo:go_default_library", "//cmd/kubeadm/app/phases/bootstraptoken/node:go_default_library", + "//cmd/kubeadm/app/phases/certs:go_default_library", "//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library", "//cmd/kubeadm/app/phases/controlplane:go_default_library", "//cmd/kubeadm/app/phases/etcd:go_default_library", diff --git a/cmd/kubeadm/app/cmd/phases/BUILD b/cmd/kubeadm/app/cmd/phases/BUILD index 8f683403e7e..787750a0322 100644 --- a/cmd/kubeadm/app/cmd/phases/BUILD +++ b/cmd/kubeadm/app/cmd/phases/BUILD @@ -29,7 +29,6 @@ go_library( "//cmd/kubeadm/app/phases/bootstraptoken/clusterinfo:go_default_library", "//cmd/kubeadm/app/phases/bootstraptoken/node:go_default_library", "//cmd/kubeadm/app/phases/certs:go_default_library", - "//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library", "//cmd/kubeadm/app/phases/controlplane:go_default_library", "//cmd/kubeadm/app/phases/etcd:go_default_library", "//cmd/kubeadm/app/phases/kubeconfig:go_default_library", diff --git a/cmd/kubeadm/app/phases/certs/BUILD b/cmd/kubeadm/app/phases/certs/BUILD index f144a02481a..16e44355db9 100644 --- a/cmd/kubeadm/app/phases/certs/BUILD +++ b/cmd/kubeadm/app/phases/certs/BUILD @@ -13,6 +13,9 @@ go_test( deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", "//cmd/kubeadm/app/constants:go_default_library", + "//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library", + "//cmd/kubeadm/test:go_default_library", + "//cmd/kubeadm/test/certs:go_default_library", ], ) @@ -27,7 +30,7 @@ go_library( "//cmd/kubeadm/app/constants:go_default_library", "//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library", "//pkg/registry/core/service/ipallocator:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//vendor/k8s.io/client-go/util/cert:go_default_library", ], )