Merge pull request #77252 from rojkov/rewrite-CreateServiceAccountKeyAndPublicKeyFiles

kubeadm: do unit testing of actual public function
This commit is contained in:
Kubernetes Prow Robot 2019-05-02 05:30:25 -07:00 committed by GitHub
commit 13885bf9fd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 109 deletions

View File

@ -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",
],

View File

@ -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.

View File

@ -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")
}
})
}
}