diff --git a/cmd/kubeadm/app/phases/certs/BUILD b/cmd/kubeadm/app/phases/certs/BUILD index 0d3333e08b7..b9c566ec880 100644 --- a/cmd/kubeadm/app/phases/certs/BUILD +++ b/cmd/kubeadm/app/phases/certs/BUILD @@ -39,6 +39,7 @@ go_library( "//cmd/kubeadm/app/constants:go_default_library", "//cmd/kubeadm/app/util/pkiutil:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", + "//staging/src/k8s.io/client-go/util/keyutil:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index e03c719ae49..8d6dfa0a0bf 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -25,6 +25,7 @@ import ( "github.com/pkg/errors" certutil "k8s.io/client-go/util/cert" + "k8s.io/client-go/util/keyutil" "k8s.io/klog" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" pkiutil "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil" @@ -67,27 +68,31 @@ func CreatePKIAssets(cfg *kubeadmapi.InitConfiguration) error { // 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(certsDir string) error { klog.V(1).Infoln("creating a new public/private key files for signing service account users") - saSigningKey, err := NewServiceAccountSigningKey() + _, err := keyutil.PrivateKeyFromFile(filepath.Join(certsDir, kubeadmconstants.ServiceAccountPrivateKeyName)) + if err == nil { + // 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("[certs] Using the existing %q key\n", kubeadmconstants.ServiceAccountKeyBaseName) + return nil + } else if !os.IsNotExist(err) { + return errors.Wrapf(err, "file %s existed but it could not be loaded properly", kubeadmconstants.ServiceAccountPrivateKeyName) + } + + // The key does NOT exist, let's generate it now + key, err := pkiutil.NewPrivateKey() if err != nil { return err } - return writeKeyFilesIfNotExist( - certsDir, - kubeadmconstants.ServiceAccountKeyBaseName, - saSigningKey, - ) -} + // Write .key and .pub files to disk + fmt.Printf("[certs] Generating %q key and public key\n", kubeadmconstants.ServiceAccountKeyBaseName) -// NewServiceAccountSigningKey generate public/private key pairs for signing service account tokens. -func NewServiceAccountSigningKey() (crypto.Signer, error) { - // The key does NOT exist, let's generate it now - saSigningKey, err := pkiutil.NewPrivateKey() - if err != nil { - return nil, errors.Wrap(err, "failure while creating service account token signing key") + if err := pkiutil.WriteKey(certsDir, kubeadmconstants.ServiceAccountKeyBaseName, key); err != nil { + return err } - return saSigningKey, nil + return pkiutil.WritePublicKey(certsDir, kubeadmconstants.ServiceAccountKeyBaseName, key.Public()) } // CreateCACertAndKeyFiles generates and writes out a given certificate authority. @@ -246,42 +251,6 @@ func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert 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 crypto.Signer) 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 errors.Wrapf(err, "%s key existed but it could not be loaded properly", baseName) - } - - // 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("[certs] Using the existing %q key\n", baseName) - } else { - - // Write .key and .pub files to disk - fmt.Printf("[certs] Generating %q key and public key\n", baseName) - - if err := pkiutil.WriteKey(pkiDir, baseName, key); err != nil { - return errors.Wrapf(err, "failure while saving %s key", baseName) - } - - if err := pkiutil.WritePublicKey(pkiDir, baseName, key.Public()); err != nil { - return errors.Wrapf(err, "failure while saving %s public key", baseName) - } - } - - return nil -} - // writeCSRFilesIfNotExist writes a new CSR to the given path. // If there already is a CSR file at the given path; kubeadm tries to load it and check if it's a valid certificate. // otherwise this function returns an error. diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index 858cfab6cae..191f7aa950f 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -301,84 +301,65 @@ func TestWriteCSRFilesIfNotExist(t *testing.T) { } -func TestWriteKeyFilesIfNotExist(t *testing.T) { +func TestCreateServiceAccountKeyAndPublicKeyFiles(t *testing.T) { + setupKey, err := keyutil.MakeEllipticPrivateKeyPEM() + if err != nil { + t.Fatalf("Can't setup test: %v", err) + } - setupKey, _ := NewServiceAccountSigningKey() - key, _ := NewServiceAccountSigningKey() - - var tests = []struct { - setupFunc func(pkiDir string) error - expectedError bool - expectedKey crypto.Signer + tcases := []struct { + name string + setupFunc func(pkiDir string) error + expectedErr bool + expectedKey []byte }{ { // key does not exists > key written - expectedKey: key, + name: "generate successfully", }, { // key exists > existing key used + name: "use existing key", setupFunc: func(pkiDir string) error { - return writeKeyFilesIfNotExist(pkiDir, "dummy", setupKey) + err := keyutil.WriteKey(filepath.Join(pkiDir, kubeadmconstants.ServiceAccountPrivateKeyName), setupKey) + return err }, expectedKey: setupKey, }, { // some file exists, but it is not a valid key > err + name: "empty key", setupFunc: func(pkiDir string) error { - testutil.SetupEmptyFiles(t, pkiDir, "dummy.key") + testutil.SetupEmptyFiles(t, pkiDir, kubeadmconstants.ServiceAccountPrivateKeyName) return nil }, - expectedError: true, + expectedErr: true, }, } + for _, tt := range tcases { + t.Run(tt.name, func(t *testing.T) { + dir := testutil.SetupTempDir(t) + defer os.RemoveAll(dir) - 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 + if tt.setupFunc != nil { + if err := tt.setupFunc(dir); err != nil { + t.Fatalf("error executing setupFunc: %v", err) + } } - } - // executes create func - err := writeKeyFilesIfNotExist(tmpdir, "dummy", key) + err := CreateServiceAccountKeyAndPublicKeyFiles(dir) + if (err != nil) != tt.expectedErr { + t.Fatalf("expected error: %v, got: %v, error: %v", tt.expectedErr, err != nil, err) + } else if tt.expectedErr { + return + } - 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 || test.expectedKey == nil { - 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 - } - - resultingKeyPEM, err := keyutil.MarshalPrivateKeyToPEM(resultingKey) - if err != nil { - t.Errorf("failure marshaling created key: %v", err) - continue - } - - expectedKeyPEM, err := keyutil.MarshalPrivateKeyToPEM(test.expectedKey) - if err != nil { - t.Fatalf("Failed to marshal expected private key: %v", err) - } - - if !bytes.Equal(resultingKeyPEM, expectedKeyPEM) { - t.Error("created key does not match expected key") - } + resultingKeyPEM, wasGenerated, err := keyutil.LoadOrGenerateKeyFile(filepath.Join(dir, kubeadmconstants.ServiceAccountPrivateKeyName)) + if err != nil { + t.Errorf("Can't load created key: %v", err) + } else if wasGenerated { + t.Error("The key was not created") + } else if tt.expectedKey != nil && !bytes.Equal(resultingKeyPEM, tt.expectedKey) { + t.Error("Non-existing key is used") + } + }) } }