Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kubeadm: do unit testing of actual public function #77252

Merged
merged 1 commit into from
May 2, 2019
Merged

kubeadm: do unit testing of actual public function #77252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kubeadm/app/phases/certs/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
69 changes: 19 additions & 50 deletions cmd/kubeadm/app/phases/certs/certs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
99 changes: 40 additions & 59 deletions cmd/kubeadm/app/phases/certs/certs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,84 +301,65 @@ func TestWriteCSRFilesIfNotExist(t *testing.T) {

}

func TestWriteKeyFilesIfNotExist(t *testing.T) {

setupKey, _ := NewServiceAccountSigningKey()
key, _ := NewServiceAccountSigningKey()
func TestCreateServiceAccountKeyAndPublicKeyFiles(t *testing.T) {
setupKey, err := keyutil.MakeEllipticPrivateKeyPEM()
if err != nil {
t.Fatalf("Can't setup test: %v", err)
}

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)

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

Expand Down