Add test for existing Etcd cert

Signed-off-by: Chuck Ha <[email protected]>

Author: chuckha

cloudinit/cloudinit_test.go

@@ -22,6 +22,7 @@ import (
 
 	infrav1 "sigs.k8s.io/cluster-api-bootstrap-provider-kubeadm/api/v1alpha2"
 	"sigs.k8s.io/cluster-api-bootstrap-provider-kubeadm/internal"
+	"sigs.k8s.io/cluster-api/util/certs"
 )
 
 func TestNewInitControlPlaneAdditionalFileEncodings(t *testing.T) {
@@ -51,8 +52,10 @@ func TestNewInitControlPlaneAdditionalFileEncodings(t *testing.T) {
 	}
 
 	for _, certificate := range cpinput.Certificates {
-		certificate.KeyPair.Cert = []byte("some certificate")
-		certificate.KeyPair.Key = []byte("some key")
+		certificate.KeyPair = &certs.KeyPair{
+			Cert: []byte("some certificate"),
+			Key:  []byte("some key"),
+		}
 	}
 
 	out, err := NewInitControlPlane(cpinput)

controllers/kubeadmconfig_controller_test.go

@@ -966,6 +966,38 @@ func TestKubeadmConfigReconciler_ClusterToKubeadmConfigs(t *testing.T) {
 	}
 }
 
+// Reconcile should not fail if the Etcd CA Secret already exists
+func TestKubeadmConfigReconciler_Reconcile_DoesNotFailIfCASecretsAlreadyExist(t *testing.T) {
+	cluster := newCluster("my-cluster")
+	cluster.Status.InfrastructureReady = true
+	cluster.Status.ControlPlaneInitialized = false
+	m := newControlPlaneMachine(cluster)
+	configName := "my-config"
+	c := newControlPlaneInitKubeadmConfig(m, configName)
+	scrt := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      fmt.Sprintf("%s-%s", cluster.Name, internal.EtcdCAName),
+			Namespace: "default",
+		},
+		Data: map[string][]byte{
+			"tls.crt": []byte("hello world"),
+			"tls.key": []byte("hello world"),
+		},
+	}
+	fakec := fake.NewFakeClientWithScheme(setupScheme(), []runtime.Object{cluster, m, c, scrt}...)
+	reconciler := &KubeadmConfigReconciler{
+		Log:             log.Log,
+		Client:          fakec,
+		KubeadmInitLock: &myInitLocker{},
+	}
+	req := ctrl.Request{
+		NamespacedName: types.NamespacedName{Namespace: "default", Name: configName},
+	}
+	if _, err := reconciler.Reconcile(req); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // test utils
 
 // newCluster return a CAPI cluster object

internal/certificates.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package internal
 
 import (
@@ -8,10 +24,8 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/hex"
-	"encoding/pem"
 	"fmt"
 	"math/big"
-	"net"
 	"strings"
 	"time"
 
@@ -22,14 +36,17 @@ import (
 	"k8s.io/client-go/util/cert"
 	bootstrapv1 "sigs.k8s.io/cluster-api-bootstrap-provider-kubeadm/api/v1alpha2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha2"
+	"sigs.k8s.io/cluster-api/util/certs"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
-	rsaKeySize     = 2048
 	rootOwnerValue = "root:root"
 
+	// TLSKeyDataName is the name of the secret key within the certificate secret of the key
 	TLSKeyDataName = "tls.key"
+
+	// TLSCrtDataName is the name of the secret key within the certificate secret of the certificate
 	TLSCrtDataName = "tls.crt"
 
 	// ClusterCAName is the secret name suffix for apiserver CA
@@ -45,8 +62,10 @@ const (
 	FrontProxyCAName = "proxy"
 )
 
+// Certificates are the certificates necessary to bootstrap a cluster.
 type Certificates []*Certificate
 
+// NewCertificates return an initialized but empty set of CA certificates needed to bootstrap a cluster.
 func NewCertificates() Certificates {
 	return Certificates{
 		&Certificate{Name: ClusterCAName},
@@ -56,6 +75,8 @@ func NewCertificates() Certificates {
 	}
 }
 
+// GetCertificateByName returns a certificate by the given name.
+// This could be removed if we use a map instead of a slice to hold certificates, however other code becomes more complex.
 func (c Certificates) GetCertificateByName(name string) *Certificate {
 	for _, certificate := range c {
 		if certificate.Name == name {
@@ -65,6 +86,7 @@ func (c Certificates) GetCertificateByName(name string) *Certificate {
 	return nil
 }
 
+// GetCertificates looks up each certificate from secrets and populates the certificate with the secret data.
 func (c Certificates) GetCertificates(ctx context.Context, ctrlclient client.Client, cluster *clusterv1.Cluster) error {
 	// Look up each certificate as a secret and populate the certificate/key
 	for _, certificate := range c {
@@ -95,6 +117,7 @@ func (c Certificates) GetCertificates(ctx context.Context, ctrlclient client.Cli
 	return nil
 }
 
+// GenerateCertificates will generate any certificates that do not have KeyPair data.
 func (c Certificates) GenerateCertificates() error {
 	for _, certificate := range c {
 		if certificate.KeyPair == nil {
@@ -109,6 +132,7 @@ func (c Certificates) GenerateCertificates() error {
 	return nil
 }
 
+// SaveGeneratedCertificates will save any certificates that have been generated as Kubernetes secrets.
 func (c Certificates) SaveGeneratedCertificates(ctx context.Context, ctrlclient client.Client, cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig) error {
 	for _, certificate := range c {
 		if !certificate.Generated {
@@ -122,6 +146,7 @@ func (c Certificates) SaveGeneratedCertificates(ctx context.Context, ctrlclient
 	return nil
 }
 
+// GetOrCreateCertificates is a convenience function that wraps cluster bootstrap certificate behavior.
 func (c Certificates) GetOrCreateCertificates(ctx context.Context, ctrlclient client.Client, cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig) error {
 	// Get the certificates that exist
 	if err := c.GetCertificates(ctx, ctrlclient, cluster); err != nil {
@@ -141,17 +166,19 @@ func (c Certificates) GetOrCreateCertificates(ctx context.Context, ctrlclient cl
 	return nil
 }
 
+// Certificate represents a single certificate CA.
 type Certificate struct {
 	Generated bool
 	Name      string
-	*KeyPair
+	*certs.KeyPair
 }
 
+// SecretName is the expected name of the Kubernetes secret the certificate is saved in.
 func (c *Certificate) SecretName(clustername string) string {
 	return fmt.Sprintf("%s-%s", clustername, c.Name)
 }
 
-// CertificateHashes hash all the certificates stored in a CA certificate.
+// Hashes hash all the certificates stored in a CA certificate.
 func (c *Certificate) Hashes() ([]string, error) {
 	certificates, err := cert.ParseCertsPEM(c.Cert)
 	if err != nil {
@@ -164,17 +191,13 @@ func (c *Certificate) Hashes() ([]string, error) {
 	return out, nil
 }
 
-// HashCert will calculate the sha256 of the incoming certificate.
+// hashCert will calculate the sha256 of the incoming certificate.
 func hashCert(certificate *x509.Certificate) string {
 	spkiHash := sha256.Sum256(certificate.RawSubjectPublicKeyInfo)
 	return "sha256:" + strings.ToLower(hex.EncodeToString(spkiHash[:]))
 }
 
-// KeyPair holds the raw bytes for a certificate and key
-type KeyPair struct {
-	Cert, Key []byte
-}
-
+// AsSecret will convert a single certificate into a Kubernetes secret.
 func (c *Certificate) AsSecret(cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig) *corev1.Secret {
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -203,6 +226,7 @@ func (c *Certificate) AsSecret(cluster *clusterv1.Cluster, config *bootstrapv1.K
 	return secret
 }
 
+// AsFiles converts a slice of certificates into bootstrap files.
 func (c Certificates) AsFiles() []bootstrapv1.File {
 	clusterCA := c.GetCertificateByName(ClusterCAName)
 	etcdCA := c.GetCertificateByName(EtcdCAName)
@@ -264,19 +288,15 @@ func (c Certificates) AsFiles() []bootstrapv1.File {
 // Validate checks that all KeyPairs are valid
 func (c Certificates) Validate() error {
 	for _, certificate := range c {
-		if !certificate.KeyPair.isValid() {
+		if !certificate.KeyPair.IsValid() {
 			return errors.Errorf("CA cert material is missing cert/key for %s", certificate.Name)
 		}
 	}
 	return nil
 }
 
-func (kp *KeyPair) isValid() bool {
-	return kp.Cert != nil && kp.Key != nil
-}
-
-func secretToKeyPair(s *corev1.Secret) (*KeyPair, error) {
-	cert, exists := s.Data[TLSCrtDataName]
+func secretToKeyPair(s *corev1.Secret) (*certs.KeyPair, error) {
+	c, exists := s.Data[TLSCrtDataName]
 	if !exists {
 		return nil, errors.Errorf("missing data for key %s", TLSCrtDataName)
 	}
@@ -286,47 +306,41 @@ func secretToKeyPair(s *corev1.Secret) (*KeyPair, error) {
 		return nil, errors.Errorf("missing data for key %s", TLSKeyDataName)
 	}
 
-	return &KeyPair{
-		Cert: cert,
+	return &certs.KeyPair{
+		Cert: c,
 		Key:  key,
 	}, nil
 }
 
-func generateCACert() (*KeyPair, error) {
+func generateCACert() (*certs.KeyPair, error) {
 	x509Cert, privKey, err := newCertificateAuthority()
 	if err != nil {
 		return nil, err
 	}
-	return &KeyPair{
-		Cert: encodeCertPEM(x509Cert),
-		Key:  encodePrivateKeyPEM(privKey),
+	return &certs.KeyPair{
+		Cert: certs.EncodeCertPEM(x509Cert),
+		Key:  certs.EncodePrivateKeyPEM(privKey),
 	}, nil
 }
 
 // newCertificateAuthority creates new certificate and private key for the certificate authority
 func newCertificateAuthority() (*x509.Certificate, *rsa.PrivateKey, error) {
-	key, err := newPrivateKey()
+	key, err := certs.NewPrivateKey()
 	if err != nil {
 		return nil, nil, err
 	}
 
-	cert, err := newSelfSignedCACert(key)
+	c, err := newSelfSignedCACert(key)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	return cert, key, nil
-}
-
-// newPrivateKey creates an RSA private key
-func newPrivateKey() (*rsa.PrivateKey, error) {
-	pk, err := rsa.GenerateKey(rand.Reader, rsaKeySize)
-	return pk, errors.WithStack(err)
+	return c, key, nil
 }
 
 // newSelfSignedCACert creates a CA certificate.
 func newSelfSignedCACert(key *rsa.PrivateKey) (*x509.Certificate, error) {
-	cfg := config{
+	cfg := certs.Config{
 		CommonName: "kubernetes",
 	}
 
@@ -352,41 +366,6 @@ func newSelfSignedCACert(key *rsa.PrivateKey) (*x509.Certificate, error) {
 		return nil, errors.Wrapf(err, "failed to create self signed CA certificate: %+v", tmpl)
 	}
 
-	cert, err := x509.ParseCertificate(b)
-	return cert, errors.WithStack(err)
-}
-
-// encodeCertPEM returns PEM-endcoded certificate data.
-func encodeCertPEM(cert *x509.Certificate) []byte {
-	block := pem.Block{
-		Type:  "CERTIFICATE",
-		Bytes: cert.Raw,
-	}
-	return pem.EncodeToMemory(&block)
-}
-
-// encodePrivateKeyPEM returns PEM-encoded private key data.
-func encodePrivateKeyPEM(key *rsa.PrivateKey) []byte {
-	block := pem.Block{
-		Type:  "RSA PRIVATE KEY",
-		Bytes: x509.MarshalPKCS1PrivateKey(key),
-	}
-
-	return pem.EncodeToMemory(&block)
-}
-
-// config contains the basic fields required for creating a certificate
-type config struct {
-	CommonName   string
-	Organization []string
-	AltNames     altNames
-	Usages       []x509.ExtKeyUsage
-}
-
-// AltNames contains the domain names and IP addresses that will be added
-// to the API Server's x509 certificate SubAltNames field. The values will
-// be passed directly to the x509.Certificate object.
-type altNames struct {
-	DNSNames []string
-	IPs      []net.IP
+	c, err := x509.ParseCertificate(b)
+	return c, errors.WithStack(err)
 }