Pull cert issuance code up into fitasks.Keypair

nodeup/pkg/model/kube_apiserver_test.go

@@ -18,7 +18,6 @@ package model
 
 import (
 	"bytes"
-	"crypto/x509"
 	"strings"
 	"testing"
 
@@ -74,10 +73,6 @@ func (k fakeKeyStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKe
 	panic("implement me")
 }
 
-func (k fakeKeyStore) CreateKeypair(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) {
-	panic("implement me")
-}
-
 func (k fakeKeyStore) StoreKeypair(id string, cert *pki.Certificate, privateKey *pki.PrivateKey) error {
 	panic("implement me")
 }

pkg/kubeconfig/create_kubecfg_test.go

@@ -20,8 +20,6 @@ import (
 	"reflect"
 	"testing"
 
-	"crypto/x509"
-
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/pki"
@@ -50,8 +48,6 @@ func (f fakeStatusStore) GetApiIngressStatus(cluster *kops.Cluster) ([]kops.ApiI
 type fakeKeyStore struct {
 	FindKeypairFn func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error)
 
-	CreateKeypairFn func(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error)
-
 	// StoreKeypair writes the keypair to the store
 	StoreKeypairFn func(id string, cert *pki.Certificate, privateKey *pki.PrivateKey) error
 
@@ -63,10 +59,6 @@ func (f fakeKeyStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKe
 	return f.FindKeypairFn(name)
 }
 
-func (f fakeKeyStore) CreateKeypair(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) {
-	return f.CreateKeypairFn(signer, name, template, privateKey)
-}
-
 func (f fakeKeyStore) StoreKeypair(id string, cert *pki.Certificate, privateKey *pki.PrivateKey) error {
 	return f.StoreKeypairFn(id, cert, privateKey)
 }

upup/pkg/fi/ca.go

@@ -18,7 +18,6 @@ package fi
 
 import (
 	"bytes"
-	"crypto/x509"
 	"fmt"
 
 	"k8s.io/kops/pkg/apis/kops"
@@ -53,8 +52,6 @@ type Keystore interface {
 	// task to convert a Legacy Keypair to the new Keypair API format.
 	FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, bool, error)
 
-	CreateKeypair(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error)
-
 	// StoreKeypair writes the keypair to the store
 	StoreKeypair(id string, cert *pki.Certificate, privateKey *pki.PrivateKey) error
 

upup/pkg/fi/clientset_castore.go

@@ -19,11 +19,8 @@ package fi
 import (
 	"bytes"
 	"context"
-	"crypto/x509"
 	"fmt"
 	"math/big"
-	"sync"
-	"time"
 
 	"golang.org/x/crypto/ssh"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -40,9 +37,6 @@ type ClientsetCAStore struct {
 	cluster   *kops.Cluster
 	namespace string
 	clientset kopsinternalversion.KopsInterface
-
-	mutex           sync.Mutex
-	cachedCaKeysets map[string]*keyset
 }
 
 var _ CAStore = &ClientsetCAStore{}
@@ -51,10 +45,9 @@ var _ SSHCredentialStore = &ClientsetCAStore{}
 // NewClientsetCAStore is the constructor for ClientsetCAStore
 func NewClientsetCAStore(cluster *kops.Cluster, clientset kopsinternalversion.KopsInterface, namespace string) CAStore {
 	c := &ClientsetCAStore{
-		cluster:         cluster,
+		cluster:   cluster,
-		clientset:       clientset,
+		clientset: clientset,
-		namespace:       namespace,
+		namespace: namespace,
-		cachedCaKeysets: make(map[string]*keyset),
 	}
 
 	return c
@@ -64,38 +57,14 @@ func NewClientsetCAStore(cluster *kops.Cluster, clientset kopsinternalversion.Ko
 func NewClientsetSSHCredentialStore(cluster *kops.Cluster, clientset kopsinternalversion.KopsInterface, namespace string) SSHCredentialStore {
 	// Note: currently identical to NewClientsetCAStore
 	c := &ClientsetCAStore{
-		cluster:         cluster,
+		cluster:   cluster,
-		clientset:       clientset,
+		clientset: clientset,
-		namespace:       namespace,
+		namespace: namespace,
-		cachedCaKeysets: make(map[string]*keyset),
 	}
 
 	return c
 }
 
-// readCAKeypairs retrieves the CA keypair.
-// (No longer generates a keypair if not found.)
-func (c *ClientsetCAStore) readCAKeypairs(ctx context.Context, id string) (*keyset, error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	cached := c.cachedCaKeysets[id]
-	if cached != nil {
-		return cached, nil
-	}
-
-	keyset, err := c.loadKeyset(ctx, id)
-	if err != nil {
-		return nil, err
-	}
-
-	if keyset == nil {
-		return nil, nil
-	}
-	c.cachedCaKeysets[id] = keyset
-	return keyset, nil
-}
-
 // keyset is a parsed Keyset
 type keyset struct {
 	legacyFormat bool
@@ -319,67 +288,6 @@ func (c *ClientsetCAStore) ListSSHCredentials() ([]*kops.SSHCredential, error) {
 	return items, nil
 }
 
-func (c *ClientsetCAStore) issueCert(signer string, name string, serial *big.Int, privateKey *pki.PrivateKey, template *x509.Certificate) (*pki.Certificate, error) {
-	ctx := context.TODO()
-
-	klog.Infof("Issuing new certificate: %q", name)
-
-	template.SerialNumber = serial
-
-	caKeyset, err := c.readCAKeypairs(ctx, signer)
-	if err != nil {
-		return nil, err
-	}
-
-	if caKeyset == nil {
-		return nil, fmt.Errorf("ca keyset was not found; cannot issue certificates")
-	}
-	if caKeyset.primary == nil {
-		return nil, fmt.Errorf("ca keyset did not have any key data; cannot issue certificates")
-	}
-	if caKeyset.primary.certificate == nil {
-		return nil, fmt.Errorf("ca certificate was not found; cannot issue certificates")
-	}
-	if caKeyset.primary.privateKey == nil {
-		return nil, fmt.Errorf("ca privateKey was not found; cannot issue certificates")
-	}
-	cert, err := pki.SignNewCertificate(privateKey, template, caKeyset.primary.certificate.Certificate, caKeyset.primary.privateKey)
-	if err != nil {
-		return nil, err
-	}
-
-	if _, err := c.storeAndVerifyKeypair(ctx, name, cert, privateKey); err != nil {
-		return nil, err
-	}
-
-	return cert, nil
-}
-
-// storeAndVerifyKeypair writes the keypair, also re-reading it to double-check it
-func (c *ClientsetCAStore) storeAndVerifyKeypair(ctx context.Context, name string, cert *pki.Certificate, privateKey *pki.PrivateKey) (*keyset, error) {
-	id := cert.Certificate.SerialNumber.String()
-	if err := c.storeKeypair(ctx, name, id, cert, privateKey); err != nil {
-		return nil, err
-	}
-
-	// Make double-sure it round-trips
-	keyset, err := c.loadKeyset(ctx, name)
-	if err != nil {
-		return nil, fmt.Errorf("error fetching stored certificate: %v", err)
-	}
-
-	if keyset == nil {
-		return nil, fmt.Errorf("stored certificate not found: %v", err)
-	}
-	if keyset.primary == nil {
-		return nil, fmt.Errorf("stored certificate did not have data: %v", err)
-	}
-	if keyset.primary.id != id {
-		return nil, fmt.Errorf("stored certificate changed concurrently (id mismatch)")
-	}
-	return keyset, nil
-}
-
 // StoreKeypair implements CAStore::StoreKeypair
 func (c *ClientsetCAStore) StoreKeypair(name string, cert *pki.Certificate, privateKey *pki.PrivateKey) error {
 	ctx := context.TODO()
@@ -429,18 +337,6 @@ func (c *ClientsetCAStore) FindPrivateKeyset(name string) (*kops.Keyset, error)
 	return o, nil
 }
 
-// CreateKeypair implements CAStore::CreateKeypair
-func (c *ClientsetCAStore) CreateKeypair(signer string, id string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) {
-	serial := c.buildSerial()
-
-	cert, err := c.issueCert(signer, id, serial, privateKey, template)
-	if err != nil {
-		return nil, err
-	}
-
-	return cert, nil
-}
-
 // addKey saves the specified key to the registry
 func (c *ClientsetCAStore) addKey(ctx context.Context, name string, keysetType kops.KeysetType, item *kops.KeysetItem) error {
 	create := false
@@ -573,12 +469,6 @@ func (c *ClientsetCAStore) storeKeypair(ctx context.Context, name string, id str
 	return c.addKey(ctx, name, kops.SecretTypeKeypair, item)
 }
 
-// buildSerial returns a serial for use when issuing certificates
-func (c *ClientsetCAStore) buildSerial() *big.Int {
-	t := time.Now().UnixNano()
-	return pki.BuildPKISerial(t)
-}
-
 // AddSSHPublicKey implements CAStore::AddSSHPublicKey
 func (c *ClientsetCAStore) AddSSHPublicKey(name string, pubkey []byte) error {
 	ctx := context.TODO()

upup/pkg/fi/fitasks/keypair.go

@@ -19,9 +19,11 @@ package fitasks
 import (
 	"crypto/x509"
 	"fmt"
+	"math/big"
 	"net"
 	"sort"
 	"strings"
+	"time"
 
 	"k8s.io/klog"
 	"k8s.io/kops/pkg/pki"
@@ -168,7 +170,7 @@ func (_ *Keypair) Render(c *fi.Context, a, e, changes *Keypair) error {
 		return fi.RequiredField("Name")
 	}
 
-	template, err := e.BuildCertificateTemplate()
+	template, err := e.buildCertificateTemplate()
 	if err != nil {
 		return err
 	}
@@ -221,7 +223,9 @@ func (_ *Keypair) Render(c *fi.Context, a, e, changes *Keypair) error {
 			signer = fi.StringValue(e.Signer.Name)
 		}
 
-		cert, err := c.Keystore.CreateKeypair(signer, name, template, privateKey)
+		serial := pki.BuildPKISerial(time.Now().UnixNano())
+
+		cert, err := e.issueCert(c.Keystore, signer, name, serial, privateKey, template)
 		if err != nil {
 			return err
 		}
@@ -250,7 +254,7 @@ func (_ *Keypair) Render(c *fi.Context, a, e, changes *Keypair) error {
 }
 
 // BuildCertificateTemplate is responsible for constructing a certificate template
-func (e *Keypair) BuildCertificateTemplate() (*x509.Certificate, error) {
+func (e *Keypair) buildCertificateTemplate() (*x509.Certificate, error) {
 	template, err := buildCertificateTemplateForType(e.Type)
 	if err != nil {
 		return nil, err
@@ -344,3 +348,42 @@ func buildTypeDescription(cert *x509.Certificate) string {
 
 	return s
 }
+
+func (e *Keypair) issueCert(keystore fi.Keystore, signer string, id string, serial *big.Int, privateKey *pki.PrivateKey, template *x509.Certificate) (*pki.Certificate, error) {
+	klog.Infof("Issuing new certificate: %q", id)
+
+	template.SerialNumber = serial
+
+	var cert *pki.Certificate
+	if template.IsCA {
+		var err error
+		cert, err = pki.SignNewCertificate(privateKey, template, nil, nil)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		caCertificate, caPrivateKey, _, err := keystore.FindKeypair(signer)
+		if err != nil {
+			return nil, err
+		}
+		if caPrivateKey == nil {
+			return nil, fmt.Errorf("ca key for %q was not found; cannot issue certificates", signer)
+		}
+		if caCertificate == nil {
+			return nil, fmt.Errorf("ca certificate for %q was not found; cannot issue certificates", signer)
+		}
+		cert, err = pki.SignNewCertificate(privateKey, template, caCertificate.Certificate, caPrivateKey)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	err := keystore.StoreKeypair(id, cert, privateKey)
+	if err != nil {
+		return nil, err
+	}
+
+	// Make double-sure it round-trips
+	certificate, _, _, err := keystore.FindKeypair(id)
+	return certificate, err
+}

upup/pkg/fi/vfs_castore.go

@@ -18,13 +18,11 @@ package fi
 
 import (
 	"bytes"
-	"crypto/x509"
 	"fmt"
 	"math/big"
 	"os"
 	"strings"
 	"sync"
-	"time"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog"
@@ -43,10 +41,6 @@ type VFSCAStore struct {
 
 	mutex     sync.Mutex
 	cachedCAs map[string]*cachedEntry
-
-	// SerialGenerator is the function for generating certificate serial numbers
-	// It can be replaced for testing purposes.
-	SerialGenerator func() *big.Int
 }
 
 type cachedEntry struct {
@@ -64,11 +58,6 @@ func NewVFSCAStore(cluster *kops.Cluster, basedir vfs.Path) *VFSCAStore {
 		cachedCAs: make(map[string]*cachedEntry),
 	}
 
-	c.SerialGenerator = func() *big.Int {
-		t := time.Now().UnixNano()
-		return pki.BuildPKISerial(t)
-	}
-
 	return c
 }
 
@@ -586,46 +575,6 @@ func mirrorSSHCredential(cluster *kops.Cluster, basedir vfs.Path, sshCredential
 	return nil
 }
 
-func (c *VFSCAStore) issueCert(signer string, id string, serial *big.Int, privateKey *pki.PrivateKey, template *x509.Certificate) (*pki.Certificate, error) {
-	klog.Infof("Issuing new certificate: %q", id)
-
-	template.SerialNumber = serial
-
-	var cert *pki.Certificate
-	if template.IsCA {
-		var err error
-		cert, err = pki.SignNewCertificate(privateKey, template, nil, nil)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		caCertificates, caPrivateKeys, err := c.readCAKeypairs(signer)
-		if err != nil {
-			return nil, err
-		}
-
-		if caPrivateKeys == nil || caPrivateKeys.primary == nil || caPrivateKeys.primary.privateKey == nil {
-			return nil, fmt.Errorf("ca key for %q was not found; cannot issue certificates", signer)
-		}
-		if caCertificates == nil || caCertificates.primary == nil || caCertificates.primary.certificate == nil {
-			return nil, fmt.Errorf("ca certificate for %q was not found; cannot issue certificates", signer)
-		}
-		cert, err = pki.SignNewCertificate(privateKey, template, caCertificates.primary.certificate.Certificate, caPrivateKeys.primary.privateKey)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	err := c.StoreKeypair(id, cert, privateKey)
-	if err != nil {
-		return nil, err
-	}
-
-	// Make double-sure it round-trips
-	p := c.buildCertificatePath(id, serial.String())
-	return c.loadOneCertificate(p)
-}
-
 func (c *VFSCAStore) StoreKeypair(name string, cert *pki.Certificate, privateKey *pki.PrivateKey) error {
 	serial := cert.Certificate.SerialNumber.String()
 
@@ -729,17 +678,6 @@ func (c *VFSCAStore) FindPrivateKeyset(name string) (*kops.Keyset, error) {
 	return o, nil
 }
 
-func (c *VFSCAStore) CreateKeypair(signer string, id string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) {
-	serial := c.SerialGenerator()
-
-	cert, err := c.issueCert(signer, id, serial, privateKey, template)
-	if err != nil {
-		return nil, err
-	}
-
-	return cert, nil
-}
-
 func (c *VFSCAStore) storePrivateKey(name string, ki *keysetItem) error {
 	if ki.privateKey == nil {
 		return fmt.Errorf("privateKey not provided to storeCertificate")