From 68a9b83d44d5ec73fc27271dbd2a4417dbaaeb38 Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Sat, 6 Jan 2018 21:42:27 +0100 Subject: [PATCH 01/10] some small refactoring --- pkg/controller/kubeconfig.go | 23 +++++++++++++---------- pkg/signals/signal.go | 7 ++++--- pkg/ssh/key.go | 5 ++++- pkg/userdata/coreos/userdata.go | 2 +- pkg/userdata/ubuntu/userdata.go | 4 ++++ 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/pkg/controller/kubeconfig.go b/pkg/controller/kubeconfig.go index c19062cff..56cb7ee26 100644 --- a/pkg/controller/kubeconfig.go +++ b/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) diff --git a/pkg/signals/signal.go b/pkg/signals/signal.go index 92e294e99..393a285fc 100644 --- a/pkg/signals/signal.go +++ b/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 diff --git a/pkg/ssh/key.go b/pkg/ssh/key.go index 4b9ea693d..4c0dd48cf 100644 --- a/pkg/ssh/key.go +++ b/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) } diff --git a/pkg/userdata/coreos/userdata.go b/pkg/userdata/coreos/userdata.go index 1ecc4c49a..a8c49e500 100644 --- a/pkg/userdata/coreos/userdata.go +++ b/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 diff --git a/pkg/userdata/ubuntu/userdata.go b/pkg/userdata/ubuntu/userdata.go index 507992c6e..4eb598495 100644 --- a/pkg/userdata/ubuntu/userdata.go +++ b/pkg/userdata/ubuntu/userdata.go @@ -75,7 +75,11 @@ func (p Provider) UserData(spec machinesv1alpha1.MachineSpec, kubeconfig string, return string(b.String()), nil } +<<<<<<< HEAD var ctTemplate string = `#cloud-config +======= +const ctTemplate = `#cloud-config +>>>>>>> some small refactoring hostname: {{ .MachineSpec.Name }} package_update: false From 9e89ac8a9e1986d214f1193fd22dadea7ff66d0d Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Sat, 6 Jan 2018 21:46:35 +0100 Subject: [PATCH 02/10] do not need to export everything --- pkg/client/clientset/versioned/scheme/register.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/client/clientset/versioned/scheme/register.go b/pkg/client/clientset/versioned/scheme/register.go index 298df6329..5fe4290f2 100644 --- a/pkg/client/clientset/versioned/scheme/register.go +++ b/pkg/client/clientset/versioned/scheme/register.go @@ -24,13 +24,15 @@ import ( serializer "k8s.io/apimachinery/pkg/runtime/serializer" ) -var Scheme = runtime.NewScheme() -var Codecs = serializer.NewCodecFactory(Scheme) -var ParameterCodec = runtime.NewParameterCodec(Scheme) +var ( + scheme = runtime.NewScheme() + codecs = serializer.NewCodecFactory(scheme) + parameterCodec = runtime.NewParameterCodec(scheme) +) func init() { - v1.AddToGroupVersion(Scheme, schema.GroupVersion{Version: "v1"}) - AddToScheme(Scheme) + v1.AddToGroupVersion(scheme, schema.GroupVersion{Version: "v1"}) + AddToScheme(scheme) } // AddToScheme adds all types of this clientset into the given scheme. This allows composition From e1ebeb36a1328a59b2eb6f4198e197c9143ce1bf Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Sat, 6 Jan 2018 22:06:37 +0100 Subject: [PATCH 03/10] early exit --- pkg/ssh/configmap.go | 57 ++++++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/pkg/ssh/configmap.go b/pkg/ssh/configmap.go index 10c243397..41d8eef89 100644 --- a/pkg/ssh/configmap.go +++ b/pkg/ssh/configmap.go @@ -20,41 +20,46 @@ const ( secretName = "machine-controller-ssh-key" ) +// EnsureSSHKeypairSecret func EnsureSSHKeypairSecret(client kubernetes.Interface) (*rsa.PrivateKey, error) { 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) + } - 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 - } + if !errors.IsNotFound(err) { + return nil, err + } - secret := v1.Secret{} - secret.Name = secretName - secret.Type = v1.SecretTypeOpaque + 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.Data = map[string][]byte{ - privateKeyDataIndex: privBuf.Bytes(), - } + 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 + } + + 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) { From ed3623b91ed3e0c7066cdffeab81116e2bf479b5 Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Sat, 6 Jan 2018 22:35:46 +0100 Subject: [PATCH 04/10] scheme.Codecs & scheme.ParameterCodec has to be exported --- pkg/client/clientset/versioned/scheme/register.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/client/clientset/versioned/scheme/register.go b/pkg/client/clientset/versioned/scheme/register.go index 5fe4290f2..6c9904d3e 100644 --- a/pkg/client/clientset/versioned/scheme/register.go +++ b/pkg/client/clientset/versioned/scheme/register.go @@ -26,8 +26,8 @@ import ( var ( scheme = runtime.NewScheme() - codecs = serializer.NewCodecFactory(scheme) - parameterCodec = runtime.NewParameterCodec(scheme) + Codecs = serializer.NewCodecFactory(scheme) + ParameterCodec = runtime.NewParameterCodec(scheme) ) func init() { From 26e3d467118dd6970aa3066991a9b83c618841ea Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Wed, 10 Jan 2018 11:04:22 +0100 Subject: [PATCH 05/10] do not touch generated code --- pkg/client/clientset/versioned/scheme/register.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/client/clientset/versioned/scheme/register.go b/pkg/client/clientset/versioned/scheme/register.go index 6c9904d3e..298df6329 100644 --- a/pkg/client/clientset/versioned/scheme/register.go +++ b/pkg/client/clientset/versioned/scheme/register.go @@ -24,15 +24,13 @@ import ( serializer "k8s.io/apimachinery/pkg/runtime/serializer" ) -var ( - scheme = runtime.NewScheme() - Codecs = serializer.NewCodecFactory(scheme) - ParameterCodec = runtime.NewParameterCodec(scheme) -) +var Scheme = runtime.NewScheme() +var Codecs = serializer.NewCodecFactory(Scheme) +var ParameterCodec = runtime.NewParameterCodec(Scheme) func init() { - v1.AddToGroupVersion(scheme, schema.GroupVersion{Version: "v1"}) - AddToScheme(scheme) + v1.AddToGroupVersion(Scheme, schema.GroupVersion{Version: "v1"}) + AddToScheme(Scheme) } // AddToScheme adds all types of this clientset into the given scheme. This allows composition From afe46ca65f68d6d2479fa4cc0f6bb0f5df6c72f7 Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Wed, 10 Jan 2018 11:04:48 +0100 Subject: [PATCH 06/10] fix merge conflict --- pkg/userdata/ubuntu/userdata.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/userdata/ubuntu/userdata.go b/pkg/userdata/ubuntu/userdata.go index 4eb598495..93b8a7334 100644 --- a/pkg/userdata/ubuntu/userdata.go +++ b/pkg/userdata/ubuntu/userdata.go @@ -75,11 +75,7 @@ func (p Provider) UserData(spec machinesv1alpha1.MachineSpec, kubeconfig string, return string(b.String()), nil } -<<<<<<< HEAD -var ctTemplate string = `#cloud-config -======= const ctTemplate = `#cloud-config ->>>>>>> some small refactoring hostname: {{ .MachineSpec.Name }} package_update: false From 435a49713bb59093148c8c1149c69e0342d9c4f6 Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Wed, 10 Jan 2018 11:05:18 +0100 Subject: [PATCH 07/10] add tests --- pkg/ssh/configmap.go | 6 +++ pkg/ssh/configmap_test.go | 79 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 pkg/ssh/configmap_test.go diff --git a/pkg/ssh/configmap.go b/pkg/ssh/configmap.go index 41d8eef89..406c2de3d 100644 --- a/pkg/ssh/configmap.go +++ b/pkg/ssh/configmap.go @@ -22,6 +22,9 @@ const ( // 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 { return keyFromSecret(secret) @@ -67,6 +70,9 @@ func keyFromSecret(secret *v1.Secret) (*rsa.PrivateKey, error) { 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) pk, err := x509.ParsePKCS1PrivateKey(decoded.Bytes) if err != nil { diff --git a/pkg/ssh/configmap_test.go b/pkg/ssh/configmap_test.go new file mode 100644 index 000000000..f6515557d --- /dev/null +++ b/pkg/ssh/configmap_test.go @@ -0,0 +1,79 @@ +package ssh + +import ( + "crypto/rsa" + "reflect" + "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 generateSecretWithKey(b []byte) runtime.Object { + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: metav1.NamespaceSystem, + }, + Data: map[string][]byte{ + privateKeyDataIndex: 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 without a malformed key", + args: args{ + client: fakeClientFactory(generateSecretWithKey(nil)), + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := EnsureSSHKeypairSecret(tt.args.client) + if (err != nil) != tt.wantErr { + t.Errorf("EnsureSSHKeypairSecret() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("EnsureSSHKeypairSecret() = %v, want %v", got, tt.want) + } + }) + } +} From d8c491ce246491f9434bb65a8a1dbe696fed5571 Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Thu, 11 Jan 2018 19:21:45 +0100 Subject: [PATCH 08/10] add test cases --- pkg/ssh/configmap.go | 9 ++++- pkg/ssh/configmap_test.go | 71 +++++++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/pkg/ssh/configmap.go b/pkg/ssh/configmap.go index 406c2de3d..5e6252110 100644 --- a/pkg/ssh/configmap.go +++ b/pkg/ssh/configmap.go @@ -18,6 +18,8 @@ const ( privateKeyDataIndex = "id_rsa" secretName = "machine-controller-ssh-key" + + rsaPrivateKey = "RSA PRIVATE KEY" ) // EnsureSSHKeypairSecret @@ -40,7 +42,7 @@ func EnsureSSHKeypairSecret(client kubernetes.Interface) (*rsa.PrivateKey, error return nil, fmt.Errorf("failed to generate ssh keypair: %v", err) } - privateKeyPEM := &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(pk)} + privateKeyPEM := &pem.Block{Type: rsaPrivateKey, Bytes: x509.MarshalPKCS1PrivateKey(pk)} privBuf := bytes.Buffer{} err = pem.Encode(&privBuf, privateKeyPEM) if err != nil { @@ -74,6 +76,11 @@ func keyFromSecret(secret *v1.Secret) (*rsa.PrivateKey, error) { 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) diff --git a/pkg/ssh/configmap_test.go b/pkg/ssh/configmap_test.go index f6515557d..ca7085867 100644 --- a/pkg/ssh/configmap_test.go +++ b/pkg/ssh/configmap_test.go @@ -1,8 +1,11 @@ package ssh import ( + "bytes" + "crypto/rand" "crypto/rsa" - "reflect" + "crypto/x509" + "encoding/pem" "testing" "k8s.io/api/core/v1" @@ -13,18 +16,40 @@ import ( "k8s.io/client-go/kubernetes" ) -func generateSecretWithKey(b []byte) runtime.Object { +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: secretName, + Name: name, Namespace: metav1.NamespaceSystem, }, Data: map[string][]byte{ - privateKeyDataIndex: b, + 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...) } @@ -56,9 +81,33 @@ func TestEnsureSSHKeypairSecret(t *testing.T) { wantErr: true, }, { - name: "fake basis client without a malformed key", + name: "fake basis client with a malformed key", args: args{ - client: fakeClientFactory(generateSecretWithKey(nil)), + 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, @@ -66,14 +115,14 @@ func TestEnsureSSHKeypairSecret(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := EnsureSSHKeypairSecret(tt.args.client) + _, err := EnsureSSHKeypairSecret(tt.args.client) if (err != nil) != tt.wantErr { - t.Errorf("EnsureSSHKeypairSecret() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("EnsureSSHKeypairSecret() error = %+v, wantErr %+v", err, tt.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("EnsureSSHKeypairSecret() = %v, want %v", got, tt.want) - } + // if !reflect.DeepEqual(got, tt.want) { + // t.Errorf("EnsureSSHKeypairSecret() = %+v, want %+v", got, tt.want) + // } }) } } From 1f7836a8a0d77818faf1f81f295a6f9a2ebf6f7a Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Fri, 12 Jan 2018 17:50:03 +0100 Subject: [PATCH 09/10] update error message --- pkg/ssh/configmap.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/ssh/configmap.go b/pkg/ssh/configmap.go index 5e6252110..6ad3092a9 100644 --- a/pkg/ssh/configmap.go +++ b/pkg/ssh/configmap.go @@ -16,10 +16,8 @@ import ( const ( privateKeyDataIndex = "id_rsa" - - secretName = "machine-controller-ssh-key" - - rsaPrivateKey = "RSA PRIVATE KEY" + secretName = "machine-controller-ssh-key" + rsaPrivateKey = "RSA PRIVATE KEY" ) // EnsureSSHKeypairSecret From a4da72ec483771ee177d52dd847b7b6cf1dc01f1 Mon Sep 17 00:00:00 2001 From: Guus van Weelden Date: Fri, 12 Jan 2018 17:52:58 +0100 Subject: [PATCH 10/10] remove unnecessary lines --- pkg/ssh/configmap_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/ssh/configmap_test.go b/pkg/ssh/configmap_test.go index ca7085867..399c0f63c 100644 --- a/pkg/ssh/configmap_test.go +++ b/pkg/ssh/configmap_test.go @@ -120,9 +120,6 @@ func TestEnsureSSHKeypairSecret(t *testing.T) { t.Errorf("EnsureSSHKeypairSecret() error = %+v, wantErr %+v", err, tt.wantErr) return } - // if !reflect.DeepEqual(got, tt.want) { - // t.Errorf("EnsureSSHKeypairSecret() = %+v, want %+v", got, tt.want) - // } }) } }