Improve the output of 'kops get keypairs'

cmd/kops/describe_keypairs.go

@@ -28,7 +28,6 @@ import (
 
 	"github.com/spf13/cobra"
 	"k8s.io/kops/pkg/pki"
-	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/templates"
 )
@@ -104,10 +103,9 @@ func (c *DescribeKeypairsCommand) Run(ctx context.Context, args []string) error
 
 	for _, i := range items {
 		fmt.Fprintf(w, "Name:\t%s\n", i.Name)
-		fmt.Fprintf(w, "Type:\t%s\n", i.Type)
+		fmt.Fprintf(w, "Id:\t%s\n", i.Id)
-		fmt.Fprintf(w, "Id:\t%s\n", i.ID)
 
-		err = describeKeypair(keyStore, i, &b)
+		err = describeKeypair(i, &b)
 		if err != nil {
 			return err
 		}
@@ -124,50 +122,31 @@ func (c *DescribeKeypairsCommand) Run(ctx context.Context, args []string) error
 	return w.Flush()
 }
 
-func describeKeypair(keyStore fi.CAStore, item *fi.KeystoreItem, w *bytes.Buffer) error {
+func describeKeypair(item *keypairItem, w *bytes.Buffer) error {
-	name := item.Name
+	cert := item.Certificate
 
-	cert, err := keyStore.FindCert(name)
-	if err != nil {
-		return fmt.Errorf("error retrieving cert %q: %v", name, err)
-	}
-
-	key, err := keyStore.FindPrivateKey(name)
-	if err != nil {
-		return fmt.Errorf("error retrieving private key %q: %v", name, err)
-	}
-
-	var alternateNames []string
 	if cert != nil {
+		var alternateNames []string
 		alternateNames = append(alternateNames, cert.Certificate.DNSNames...)
 		alternateNames = append(alternateNames, cert.Certificate.EmailAddresses...)
 		for _, ip := range cert.Certificate.IPAddresses {
 			alternateNames = append(alternateNames, ip.String())
 		}
 		sort.Strings(alternateNames)
-	}
 
-	if cert != nil {
 		fmt.Fprintf(w, "Subject:\t%s\n", pki.PkixNameToString(&cert.Certificate.Subject))
 		fmt.Fprintf(w, "Issuer:\t%s\n", pki.PkixNameToString(&cert.Certificate.Issuer))
 		fmt.Fprintf(w, "AlternateNames:\t%s\n", strings.Join(alternateNames, ", "))
 		fmt.Fprintf(w, "CA:\t%v\n", cert.IsCA)
 		fmt.Fprintf(w, "NotAfter:\t%s\n", cert.Certificate.NotAfter)
 		fmt.Fprintf(w, "NotBefore:\t%s\n", cert.Certificate.NotBefore)
-
+		if rsaKey, ok := cert.PublicKey.(*rsa.PublicKey); ok {
+			fmt.Fprintf(w, "KeyLength:\t%v\n", rsaKey.N.BitLen())
+		}
 		// PublicKeyAlgorithm doesn't have a String() function.  Also, is this important information?
 		//fmt.Fprintf(w, "PublicKeyAlgorithm:\t%v\n", c.Certificate.PublicKeyAlgorithm)
 		//fmt.Fprintf(w, "SignatureAlgorithm:\t%v\n", c.Certificate.SignatureAlgorithm)
 	}
 
-	if key != nil {
-		if rsaPrivateKey, ok := key.Key.(*rsa.PrivateKey); ok {
-			fmt.Fprintf(w, "PrivateKeyType:\t%v\n", "rsa")
-			fmt.Fprintf(w, "KeyLength:\t%v\n", rsaPrivateKey.N.BitLen())
-		} else {
-			fmt.Fprintf(w, "PrivateKeyType:\tunknown (%T)\n", key.Key)
-		}
-	}
-
 	return nil
 }

cmd/kops/get_keypairs.go

@@ -20,10 +20,10 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os"
 
 	"github.com/spf13/cobra"
 	"k8s.io/kops/cmd/kops/util"
+	"k8s.io/kops/pkg/pki"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/util/pkg/tables"
 	"k8s.io/kubectl/pkg/util/i18n"
@@ -60,7 +60,7 @@ func NewCmdGetKeypairs(f *util.Factory, out io.Writer, getOptions *GetOptions) *
 		Example: getKeypairExample,
 		Run: func(cmd *cobra.Command, args []string) {
 			ctx := context.TODO()
-			err := RunGetKeypairs(ctx, &options, args)
+			err := RunGetKeypairs(ctx, out, &options, args)
 			if err != nil {
 				exitWithError(err)
 			}
@@ -70,46 +70,51 @@ func NewCmdGetKeypairs(f *util.Factory, out io.Writer, getOptions *GetOptions) *
 	return cmd
 }
 
-func listKeypairs(keyStore fi.CAStore, names []string) ([]*fi.KeystoreItem, error) {
+type keypairItem struct {
-	var items []*fi.KeystoreItem
+	Name          string
+	Id            string
+	IsPrimary     bool
+	Certificate   *pki.Certificate
+	HasPrivateKey bool
+}
 
-	{
+func listKeypairs(keyStore fi.CAStore, names []string) ([]*keypairItem, error) {
-		l, err := keyStore.ListKeysets()
+	var items []*keypairItem
-		if err != nil {
-			return nil, fmt.Errorf("error listing Keysets: %v", err)
-		}
 
-		for _, keyset := range l {
+	l, err := keyStore.ListKeysets()
-			for _, key := range keyset.Spec.Keys {
+	if err != nil {
-				item := &fi.KeystoreItem{
+		return nil, fmt.Errorf("error listing Keysets: %v", err)
-					Name: keyset.Name,
-					Type: keyset.Spec.Type,
-					ID:   key.Id,
-				}
-				items = append(items, item)
-			}
-		}
 	}
 
-	if len(names) != 0 {
+	for name, keyset := range l {
-		var matches []*fi.KeystoreItem
+		if len(names) != 0 {
-		for _, arg := range names {
+			found := false
-			var found []*fi.KeystoreItem
+			for _, n := range names {
-			for _, i := range items {
+				if n == name {
-				if i.Name == arg {
+					found = true
-					found = append(found, i)
+					break
 				}
 			}
-
+			if !found {
-			matches = append(matches, found...)
+				continue
+			}
+		}
+
+		for _, item := range keyset.Items {
+			items = append(items, &keypairItem{
+				Name:          name,
+				Id:            item.Id,
+				IsPrimary:     item.Id == keyset.Primary.Id,
+				Certificate:   item.Certificate,
+				HasPrivateKey: item.PrivateKey != nil,
+			})
 		}
-		items = matches
 	}
 
 	return items, nil
 }
 
-func RunGetKeypairs(ctx context.Context, options *GetKeypairsOptions, args []string) error {
+func RunGetKeypairs(ctx context.Context, out io.Writer, options *GetKeypairsOptions, args []string) error {
 	cluster, err := rootCommand.Cluster(ctx)
 	if err != nil {
 		return err
@@ -136,18 +141,26 @@ func RunGetKeypairs(ctx context.Context, options *GetKeypairsOptions, args []str
 	switch options.output {
 
 	case OutputTable:
-
 		t := &tables.Table{}
-		t.AddColumn("NAME", func(i *fi.KeystoreItem) string {
+		t.AddColumn("NAME", func(i *keypairItem) string {
 			return i.Name
 		})
-		t.AddColumn("ID", func(i *fi.KeystoreItem) string {
+		t.AddColumn("ID", func(i *keypairItem) string {
-			return i.ID
+			return i.Id
 		})
-		t.AddColumn("TYPE", func(i *fi.KeystoreItem) string {
+		t.AddColumn("PRIMARY", func(i *keypairItem) string {
-			return string(i.Type)
+			if i.IsPrimary {
+				return "*"
+			}
+			return ""
 		})
-		return t.Render(items, os.Stdout, "TYPE", "NAME", "ID")
+		t.AddColumn("HASPRIVATE", func(i *keypairItem) string {
+			if i.HasPrivateKey {
+				return "*"
+			}
+			return ""
+		})
+		return t.Render(items, out, "NAME", "ID", "PRIMARY", "HASPRIVATE")
 
 	case OutputYaml:
 		return fmt.Errorf("yaml output format is not (currently) supported for keypairs")

nodeup/pkg/model/fakes_test.go

@@ -113,7 +113,7 @@ func (k fakeCAStore) FindCert(name string) (*pki.Certificate, error) {
 	return k.certs[name], nil
 }
 
-func (k fakeCAStore) ListKeysets() ([]*kops.Keyset, error) {
+func (k fakeCAStore) ListKeysets() (map[string]*fi.Keyset, error) {
 	panic("fakeCAStore does not implement ListKeysets")
 }
 

pkg/configserver/keystore.go

@@ -95,7 +95,7 @@ func (s *configserverKeyStore) FindCert(name string) (*pki.Certificate, error) {
 }
 
 // ListKeysets implements fi.CAStore
-func (s *configserverKeyStore) ListKeysets() ([]*kops.Keyset, error) {
+func (s *configserverKeyStore) ListKeysets() (map[string]*fi.Keyset, error) {
 	return nil, fmt.Errorf("ListKeysets not supported by configserverKeyStore")
 }
 

upup/pkg/fi/ca.go

@@ -94,9 +94,8 @@ type CAStore interface {
 	// FindCert returns the specified certificate, if it exists, or nil if not found
 	FindCert(name string) (*pki.Certificate, error)
 
-	// ListKeysets will return all the KeySets
+	// ListKeysets will return all the KeySets.
-	// The key material is not guaranteed to be populated - metadata like the name will be.
+	ListKeysets() (map[string]*Keyset, error)
-	ListKeysets() ([]*kops.Keyset, error)
 
 	// DeleteKeysetItem will delete the specified item from the Keyset
 	DeleteKeysetItem(item *kops.Keyset, id string) error

upup/pkg/fi/clientset_castore.go

@@ -198,9 +198,9 @@ func (c *ClientsetCAStore) FindCertificatePool(name string) (*CertificatePool, e
 }
 
 // ListKeysets implements CAStore::ListKeysets
-func (c *ClientsetCAStore) ListKeysets() ([]*kops.Keyset, error) {
+func (c *ClientsetCAStore) ListKeysets() (map[string]*Keyset, error) {
 	ctx := context.TODO()
-	var items []*kops.Keyset
+	items := map[string]*Keyset{}
 
 	{
 		list, err := c.clientset.Keysets(c.namespace).List(ctx, metav1.ListOptions{})
@@ -212,7 +212,12 @@ func (c *ClientsetCAStore) ListKeysets() ([]*kops.Keyset, error) {
 			keyset := &list.Items[i]
 			switch keyset.Spec.Type {
 			case kops.SecretTypeKeypair:
-				items = append(items, &list.Items[i])
+				item, err := parseKeyset(keyset)
+				if err != nil {
+					return nil, fmt.Errorf("parsing keyset %q: %w", keyset.Name, err)
+				}
+
+				items[keyset.Name] = item
 
 			case kops.SecretTypeSecret:
 				continue // Ignore - this is handled by ClientsetSecretStore
@@ -470,8 +475,8 @@ func (c *ClientsetCAStore) MirrorTo(basedir vfs.Path) error {
 		return err
 	}
 
-	for _, keyset := range keysets {
+	for name, keyset := range keysets {
-		if err := mirrorKeyset(c.cluster, basedir, keyset); err != nil {
+		if err := mirrorKeyset(c.cluster, basedir, name, keyset); err != nil {
 			return err
 		}
 	}

upup/pkg/fi/vfs_castore.go

@@ -179,7 +179,7 @@ func (k *Keyset) ToAPIObject(name string, includePrivateKeyMaterial bool) (*kops
 }
 
 // writeKeysetBundle writes a Keyset bundle to VFS.
-func (c *VFSCAStore) writeKeysetBundle(p vfs.Path, name string, keyset *Keyset, includePrivateKeyMaterial bool) error {
+func writeKeysetBundle(cluster *kops.Cluster, p vfs.Path, name string, keyset *Keyset, includePrivateKeyMaterial bool) error {
 	p = p.Join("keyset.yaml")
 
 	o, err := keyset.ToAPIObject(name, includePrivateKeyMaterial)
@@ -192,7 +192,7 @@ func (c *VFSCAStore) writeKeysetBundle(p vfs.Path, name string, keyset *Keyset,
 		return err
 	}
 
-	acl, err := acls.GetACL(p, c.cluster)
+	acl, err := acls.GetACL(p, cluster)
 	if err != nil {
 		return err
 	}
@@ -215,17 +215,6 @@ func serializeKeysetBundle(o *kops.Keyset) ([]byte, error) {
 	return objectData.Bytes(), nil
 }
 
-// removePrivateKeyMaterial returns a copy of the Keyset with the private key data removed
-func removePrivateKeyMaterial(o *kops.Keyset) *kops.Keyset {
-	c := o.DeepCopy()
-
-	for i := range c.Spec.Keys {
-		c.Spec.Keys[i].PrivateMaterial = nil
-	}
-
-	return c
-}
-
 func (c *VFSCAStore) FindPrimaryKeypair(name string) (*pki.Certificate, *pki.PrivateKey, error) {
 	return FindPrimaryKeypair(c, name)
 }
@@ -321,14 +310,14 @@ func (c *VFSCAStore) FindCertificatePool(name string) (*CertificatePool, error)
 }
 
 // ListKeysets implements CAStore::ListKeysets
-func (c *VFSCAStore) ListKeysets() ([]*kops.Keyset, error) {
+func (c *VFSCAStore) ListKeysets() (map[string]*Keyset, error) {
 	baseDir := c.basedir.Join("private")
 	files, err := baseDir.ReadTree()
 	if err != nil {
 		return nil, fmt.Errorf("error reading directory %q: %v", baseDir, err)
 	}
 
-	var keysets []*kops.Keyset
+	keysets := map[string]*Keyset{}
 
 	for _, f := range files {
 		relativePath, err := vfs.RelativePath(baseDir, f)
@@ -349,12 +338,7 @@ func (c *VFSCAStore) ListKeysets() ([]*kops.Keyset, error) {
 			continue
 		}
 
-		keyset, err := loadedKeyset.ToAPIObject(name, true)
+		keysets[name] = loadedKeyset
-		if err != nil {
-			klog.Warningf("ignoring keyset %q: %w", name, err)
-			continue
-		}
-		keysets = append(keysets, keyset)
 	}
 
 	return keysets, nil
@@ -411,8 +395,8 @@ func (c *VFSCAStore) MirrorTo(basedir vfs.Path) error {
 		return err
 	}
 
-	for _, keyset := range keysets {
+	for name, keyset := range keysets {
-		if err := mirrorKeyset(c.cluster, basedir, keyset); err != nil {
+		if err := mirrorKeyset(c.cluster, basedir, name, keyset); err != nil {
 			return err
 		}
 	}
@@ -432,50 +416,13 @@ func (c *VFSCAStore) MirrorTo(basedir vfs.Path) error {
 }
 
 // mirrorKeyset writes Keyset bundles for the certificates & privatekeys.
-func mirrorKeyset(cluster *kops.Cluster, basedir vfs.Path, keyset *kops.Keyset) error {
+func mirrorKeyset(cluster *kops.Cluster, basedir vfs.Path, name string, keyset *Keyset) error {
-	primary := FindPrimary(keyset)
+	if err := writeKeysetBundle(cluster, basedir.Join("private"), name, keyset, true); err != nil {
-	if primary == nil {
+		return fmt.Errorf("writing private bundle: %v", err)
-		return fmt.Errorf("found keyset with no primary data: %s", keyset.Name)
 	}
 
-	switch keyset.Spec.Type {
+	if err := writeKeysetBundle(cluster, basedir.Join("issued"), name, keyset, false); err != nil {
-	case kops.SecretTypeKeypair:
+		return fmt.Errorf("writing certificate bundle: %v", err)
-		{
-			data, err := serializeKeysetBundle(removePrivateKeyMaterial(keyset))
-			if err != nil {
-				return err
-			}
-			p := basedir.Join("issued", keyset.Name, "keyset.yaml")
-			acl, err := acls.GetACL(p, cluster)
-			if err != nil {
-				return err
-			}
-
-			err = p.WriteFile(bytes.NewReader(data), acl)
-			if err != nil {
-				return fmt.Errorf("error writing %q: %v", p, err)
-			}
-		}
-
-		{
-			data, err := serializeKeysetBundle(keyset)
-			if err != nil {
-				return err
-			}
-			p := basedir.Join("private", keyset.Name, "keyset.yaml")
-			acl, err := acls.GetACL(p, cluster)
-			if err != nil {
-				return err
-			}
-
-			err = p.WriteFile(bytes.NewReader(data), acl)
-			if err != nil {
-				return fmt.Errorf("error writing %q: %v", p, err)
-			}
-		}
-
-	default:
-		return fmt.Errorf("unknown secret type: %q", keyset.Spec.Type)
 	}
 
 	return nil
@@ -516,14 +463,14 @@ func (c *VFSCAStore) StoreKeyset(name string, keyset *Keyset) error {
 
 	{
 		p := c.buildPrivateKeyPoolPath(name)
-		if err := c.writeKeysetBundle(p, name, keyset, true); err != nil {
+		if err := writeKeysetBundle(c.cluster, p, name, keyset, true); err != nil {
 			return fmt.Errorf("writing private bundle: %v", err)
 		}
 	}
 
 	{
 		p := c.buildCertificatePoolPath(name)
-		if err := c.writeKeysetBundle(p, name, keyset, false); err != nil {
+		if err := writeKeysetBundle(c.cluster, p, name, keyset, false); err != nil {
 			return fmt.Errorf("writing certificate bundle: %v", err)
 		}
 	}
@@ -604,7 +551,7 @@ func (c *VFSCAStore) deletePrivateKey(name string, id string) (bool, error) {
 			ks.Primary = nil
 		}
 
-		if err := c.writeKeysetBundle(p, name, ks, true); err != nil {
+		if err := writeKeysetBundle(c.cluster, p, name, ks, true); err != nil {
 			return false, fmt.Errorf("error writing bundle: %v", err)
 		}
 	}
@@ -637,7 +584,7 @@ func (c *VFSCAStore) deleteCertificate(name string, id string) (bool, error) {
 			ks.Primary = nil
 		}
 
-		if err := c.writeKeysetBundle(p, name, ks, false); err != nil {
+		if err := writeKeysetBundle(c.cluster, p, name, ks, false); err != nil {
 			return false, fmt.Errorf("error writing bundle: %v", err)
 		}
 	}