Feature/some refactoring

pkg/controller/kubeconfig.go

@@ -32,16 +32,19 @@ func (c *Controller) createBootstrapToken(name string) (string, error) {
 	tokenID := rand.String(6)
 	tokenSecret := rand.String(16)
 
-	secret := v1.Secret{}
-	secret.Name = fmt.Sprintf("bootstrap-token-%s", tokenID)
-	secret.Type = SecretTypeBootstrapToken
-	secret.StringData = map[string]string{
-		"description":                    "bootstrap token for " + name,
-		"token-id":                       tokenID,
-		"token-secret":                   tokenSecret,
-		"expiration":                     metav1.Now().Add(24 * time.Hour).Format(time.RFC3339),
-		"usage-bootstrap-authentication": "true",
-		"usage-bootstrap-signing":        "true",
+	secret := v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: fmt.Sprintf("bootstrap-token-%s", tokenID),
+		},
+		Type: SecretTypeBootstrapToken,
+		StringData: map[string]string{
+			"description":                    "bootstrap token for " + name,
+			"token-id":                       tokenID,
+			"token-secret":                   tokenSecret,
+			"expiration":                     metav1.Now().Add(24 * time.Hour).Format(time.RFC3339),
+			"usage-bootstrap-authentication": "true",
+			"usage-bootstrap-signing":        "true",
+		},
 	}
 
 	_, err := c.kubeClient.CoreV1().Secrets(metav1.NamespaceSystem).Create(&secret)

pkg/signals/signal.go

@@ -22,9 +22,10 @@ import (
 	"syscall"
 )
 
-var onlyOneSignalHandler = make(chan struct{})
-
-var shutdownSignals = []os.Signal{os.Interrupt, syscall.SIGTERM}
+var (
+	onlyOneSignalHandler = make(chan struct{})
+	shutdownSignals      = []os.Signal{os.Interrupt, syscall.SIGTERM}
+)
 
 // SetupSignalHandler registered for SIGTERM and SIGINT. A stop channel is returned
 // which is closed on one of these signals. If a second signal is caught, the program

pkg/ssh/configmap.go

@@ -16,53 +16,69 @@ import (
 
 const (
 	privateKeyDataIndex = "id_rsa"
-
-	secretName = "machine-controller-ssh-key"
+	secretName          = "machine-controller-ssh-key"
+	rsaPrivateKey       = "RSA PRIVATE KEY"
 )
 
+// EnsureSSHKeypairSecret
 func EnsureSSHKeypairSecret(client kubernetes.Interface) (*rsa.PrivateKey, error) {
+	if client == nil {
+		return nil, fmt.Errorf("got an nil k8s client")
+	}
 	secret, err := client.CoreV1().Secrets(metav1.NamespaceSystem).Get(secretName, metav1.GetOptions{})
-	if err != nil {
-		if errors.IsNotFound(err) {
-			glog.V(4).Info("generating master ssh keypair")
-			pk, err := NewPrivateKey()
-			if err != nil {
-				return nil, fmt.Errorf("failed to generate ssh keypair: %v", err)
-			}
+	if err == nil {
+		return keyFromSecret(secret)
+	}
+
+	if !errors.IsNotFound(err) {
+		return nil, err
+	}
 
-			privateKeyPEM := &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(pk)}
-			privBuf := bytes.Buffer{}
-			err = pem.Encode(&privBuf, privateKeyPEM)
-			if err != nil {
-				return nil, err
-			}
+	glog.V(4).Info("generating master ssh keypair")
+	pk, err := NewPrivateKey()
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate ssh keypair: %v", err)
+	}
 
-			secret := v1.Secret{}
-			secret.Name = secretName
-			secret.Type = v1.SecretTypeOpaque
+	privateKeyPEM := &pem.Block{Type: rsaPrivateKey, Bytes: x509.MarshalPKCS1PrivateKey(pk)}
+	privBuf := bytes.Buffer{}
+	err = pem.Encode(&privBuf, privateKeyPEM)
+	if err != nil {
+		return nil, err
+	}
 
-			secret.Data = map[string][]byte{
-				privateKeyDataIndex: privBuf.Bytes(),
-			}
+	secret = &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: secretName,
+		},
+		Type: v1.SecretTypeOpaque,
+		Data: map[string][]byte{
+			privateKeyDataIndex: privBuf.Bytes(),
+		},
+	}
 
-			_, err = client.CoreV1().Secrets(metav1.NamespaceSystem).Create(&secret)
-			if err != nil {
-				return nil, err
-			}
-			return pk, nil
-		}
+	_, err = client.CoreV1().Secrets(metav1.NamespaceSystem).Create(secret)
+	if err != nil {
 		return nil, err
 	}
+	return pk, nil
 
-	return keyFromSecret(secret)
 }
 
 func keyFromSecret(secret *v1.Secret) (*rsa.PrivateKey, error) {
 	b, exists := secret.Data[privateKeyDataIndex]
 	if !exists {
 		return nil, fmt.Errorf("key data not found in secret '%s/%s' (secret.data['%s']). remove it and a new one will be created", secret.Namespace, secret.Name, privateKeyDataIndex)
 	}
+	if len(b) == 0 {
+		return nil, fmt.Errorf("key data not found in secret '%s/%s' (secret.data['%s']). remove it and a new one will be created", secret.Namespace, secret.Name, privateKeyDataIndex)
+	}
 	decoded, _ := pem.Decode(b)
+
+	if decoded == nil {
+		return nil, fmt.Errorf("invalid PEM in secret '%s/%s'. remove it and a new one will be created", secret.Namespace, secret.Name)
+	}
+
 	pk, err := x509.ParsePKCS1PrivateKey(decoded.Bytes)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse private key: %v", err)

pkg/ssh/configmap_test.go

@@ -0,0 +1,125 @@
+package ssh
+
+import (
+	"bytes"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/pem"
+	"testing"
+
+	"k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/fake"
+
+	"k8s.io/client-go/kubernetes"
+)
+
+func generateByteSlice(n int) []byte {
+	b := make([]byte, n)
+	rand.Read(b)
+	return b
+}
+
+func generateValidPEM() []byte {
+	b := &bytes.Buffer{}
+	pk, _ := NewPrivateKey()
+
+	_ = pem.Encode(b, &pem.Block{Type: rsaPrivateKey, Bytes: x509.MarshalPKCS1PrivateKey(pk)})
+	return b.Bytes()
+}
+
+func generateSecretWithCustomNameAndIndex(name, index string, b []byte) runtime.Object {
+	return &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: metav1.NamespaceSystem,
+		},
+		Data: map[string][]byte{
+			index: b,
+		},
+	}
+}
+
+func generateSecretWithCustomName(name string, b []byte) runtime.Object {
+	return generateSecretWithCustomNameAndIndex(name, privateKeyDataIndex, b)
+}
+
+func generateSecretWithKey(b []byte) runtime.Object {
+	return generateSecretWithCustomName(secretName, b)
+}
+
+func fakeClientFactory(objs ...runtime.Object) kubernetes.Interface {
+	return fake.NewSimpleClientset(objs...)
+}
+
+func TestEnsureSSHKeypairSecret(t *testing.T) {
+	type args struct {
+		client kubernetes.Interface
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *rsa.PrivateKey
+		wantErr bool
+	}{
+		{
+			name: "nil client",
+			args: args{
+				client: nil,
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "fake basis client without a key",
+			args: args{
+				client: fakeClientFactory(generateSecretWithKey(nil)),
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "fake basis client with a malformed key",
+			args: args{
+				client: fakeClientFactory(generateSecretWithKey(generateByteSlice(2048))),
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "fake basis client with a valid key",
+			args: args{
+				client: fakeClientFactory(generateSecretWithKey(generateValidPEM())),
+			},
+			want:    nil,
+			wantErr: false,
+		},
+		{
+			name: "fake basis client with a valid key, but the wrong secrete name",
+			args: args{
+				client: fakeClientFactory(generateSecretWithCustomName("blah", generateValidPEM())),
+			},
+			want:    nil,
+			wantErr: false,
+		},
+		{
+			name: "fake basis client with a valid key, but the wrong index",
+			args: args{
+				client: fakeClientFactory(generateSecretWithCustomNameAndIndex(secretName, "blah", generateValidPEM())),
+			},
+			want:    nil,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := EnsureSSHKeypairSecret(tt.args.client)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("EnsureSSHKeypairSecret() error = %+v, wantErr %+v", err, tt.wantErr)
+				return
+			}
+		})
+	}
+}

pkg/ssh/key.go

@@ -6,8 +6,11 @@ import (
 	"fmt"
 )
 
+const privateKeyBitSize = 2048
+
+// NewPrivateKey generates a new private key
 func NewPrivateKey() (key *rsa.PrivateKey, err error) {
-	priv, err := rsa.GenerateKey(rand.Reader, 2048)
+	priv, err := rsa.GenerateKey(rand.Reader, privateKeyBitSize)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create private key: %v", err)
 	}

pkg/userdata/coreos/userdata.go

@@ -89,7 +89,7 @@ func (p Provider) UserData(spec machinesv1alpha1.MachineSpec, kubeconfig string,
 	return string(out), nil
 }
 
-var ctTemplate = `
+const ctTemplate = `
 passwd:
   users:
     - name: core

pkg/userdata/ubuntu/userdata.go

@@ -75,7 +75,7 @@ func (p Provider) UserData(spec machinesv1alpha1.MachineSpec, kubeconfig string,
 	return string(b.String()), nil
 }
 
-var ctTemplate string = `#cloud-config
+const ctTemplate = `#cloud-config
 hostname: {{ .MachineSpec.Name }}
 
 package_update: false