Clean up "kops get secrets"

cmd/kops/delete_secret.go

@@ -134,13 +134,13 @@ func completeSecretNames(cmd *cobra.Command, args []string, complete string) ([]
 
 	alreadySelected := sets.NewString(args...)
 	var secrets []string
-	items, err := listSecrets(secretStore, "secret", nil)
+	items, err := listSecrets(secretStore, nil)
 	if err != nil {
 		return commandutils.CompletionError("listing secrets", err)
 	}
 	for _, secret := range items {
-		if !alreadySelected.Has(secret.Name) {
-			secrets = append(secrets, secret.Name)
+		if !alreadySelected.Has(secret) {
+			secrets = append(secrets, secret)
 		}
 	}
 

cmd/kops/get_secrets.go

@@ -20,12 +20,11 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os"
 	"strings"
 
 	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/kops/cmd/kops/util"
-	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/util/pkg/tables"
 	"k8s.io/kubectl/pkg/util/i18n"
@@ -33,14 +32,11 @@ import (
 )
 
 var (
-	getSecretLong = templates.LongDesc(i18n.T(`
-	Display one or many secrets.`))
-
 	getSecretExample = templates.Examples(i18n.T(`
-	# Get a secret
-	kops get secrets kube -oplaintext
+	# List the secrets
+	kops get secrets
 
-	# Get the admin password for a cluster
+	# Get the admin static token for a cluster
 	kops get secrets admin -oplaintext`))
 
 	getSecretShort = i18n.T(`Get one or many secrets.`)
@@ -48,7 +44,8 @@ var (
 
 type GetSecretsOptions struct {
 	*GetOptions
-	Type string
+	Type        string
+	SecretNames []string
 }
 
 func NewCmdGetSecrets(f *util.Factory, out io.Writer, getOptions *GetOptions) *cobra.Command {
@@ -56,74 +53,44 @@ func NewCmdGetSecrets(f *util.Factory, out io.Writer, getOptions *GetOptions) *c
 		GetOptions: getOptions,
 	}
 	cmd := &cobra.Command{
-		Use:     "secrets",
+		Use:     "secrets [SECRET_NAME]...",
 		Aliases: []string{"secret"},
 		Short:   getSecretShort,
-		Long:    getSecretLong,
 		Example: getSecretExample,
-		Run: func(cmd *cobra.Command, args []string) {
-			ctx := context.TODO()
-			err := RunGetSecrets(ctx, &options, args)
-			if err != nil {
-				exitWithError(err)
+		Args: func(cmd *cobra.Command, args []string) error {
+			options.SecretNames = args
+			options.ClusterName = rootCommand.ClusterName(true)
+
+			if options.ClusterName == "" {
+				return fmt.Errorf("--name is required")
 			}
+
+			return nil
+		},
+		ValidArgsFunction: completeSecretNames,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return RunGetSecrets(context.TODO(), f, out, &options)
 		},
 	}
 
 	cmd.Flags().StringVarP(&options.Type, "type", "", "", "Filter by secret type")
+	cmd.Flags().MarkHidden("type")
 	return cmd
 }
 
-func listSecrets(secretStore fi.SecretStore, secretType string, names []string) ([]*fi.KeystoreItem, error) {
-	var items []*fi.KeystoreItem
-
-	findType := strings.ToLower(secretType)
-	switch findType {
-	case "", "secret":
-	// OK
-	case "sshpublickey":
-		return nil, fmt.Errorf("use 'kops get sshpublickey' instead")
-	case "keypair":
-		return nil, fmt.Errorf("use 'kops get keypairs' instead")
-	default:
-		return nil, fmt.Errorf("unknown secret type %q", secretType)
-	}
-
-	if findType == "" || findType == strings.ToLower(string(kops.SecretTypeSecret)) {
-		names, err := secretStore.ListSecrets()
-		if err != nil {
-			return nil, fmt.Errorf("error listing secrets %v", err)
-		}
-
-		for _, name := range names {
-			i := &fi.KeystoreItem{
-				Name: name,
-				Type: kops.SecretTypeSecret,
-			}
-			if findType != "" && findType != strings.ToLower(string(i.Type)) {
-				continue
-			}
-
-			items = append(items, i)
-		}
+func listSecrets(secretStore fi.SecretStore, names []string) ([]string, error) {
+	items, err := secretStore.ListSecrets()
+	if err != nil {
+		return nil, fmt.Errorf("listing secrets %v", err)
 	}
 
 	if len(names) != 0 {
-		var matches []*fi.KeystoreItem
-		for _, arg := range names {
-			var found []*fi.KeystoreItem
-			for _, i := range items {
-				// There may be multiple secrets with the same name (of different type)
-				if i.Name == arg {
-					found = append(found, i)
-				}
+		nameSet := sets.NewString(names...)
+		var matches []string
+		for _, item := range items {
+			if nameSet.Has(item) {
+				matches = append(matches, item)
 			}
-
-			if len(found) == 0 {
-				return nil, fmt.Errorf("Secret not found: %q", arg)
-			}
-
-			matches = append(matches, found...)
 		}
 		items = matches
 	}
@@ -131,23 +98,33 @@ func listSecrets(secretStore fi.SecretStore, secretType string, names []string)
 	return items, nil
 }
 
-func RunGetSecrets(ctx context.Context, options *GetSecretsOptions, args []string) error {
-	cluster, err := rootCommand.Cluster(ctx)
+func RunGetSecrets(ctx context.Context, f *util.Factory, out io.Writer, options *GetSecretsOptions) error {
+	switch strings.ToLower(options.Type) {
+	case "", "secret":
+	// OK
+	case "sshpublickey":
+		return fmt.Errorf("use 'kops get sshpublickey' instead")
+	case "keypair":
+		return fmt.Errorf("use 'kops get keypairs' instead")
+	default:
+		return fmt.Errorf("unknown secret type %q", options.Type)
+	}
+
+	clientset, err := f.Clientset()
 	if err != nil {
 		return err
 	}
 
-	clientset, err := rootCommand.Clientset()
+	cluster, err := GetCluster(ctx, f, options.ClusterName)
 	if err != nil {
 		return err
 	}
-
 	secretStore, err := clientset.SecretStore(cluster)
 	if err != nil {
 		return err
 	}
 
-	items, err := listSecrets(secretStore, options.Type, args)
+	items, err := listSecrets(secretStore, options.SecretNames)
 	if err != nil {
 		return err
 	}
@@ -158,49 +135,36 @@ func RunGetSecrets(ctx context.Context, options *GetSecretsOptions, args []strin
 	switch options.Output {
 
 	case OutputTable:
-
 		t := &tables.Table{}
-		t.AddColumn("NAME", func(i *fi.KeystoreItem) string {
-			return i.Name
+		t.AddColumn("NAME", func(i string) string {
+			return i
 		})
-		t.AddColumn("ID", func(i *fi.KeystoreItem) string {
-			return i.ID
-		})
-		t.AddColumn("TYPE", func(i *fi.KeystoreItem) string {
-			return string(i.Type)
-		})
-		return t.Render(items, os.Stdout, "TYPE", "NAME", "ID")
+		return t.Render(items, out, "NAME")
 
 	case OutputYaml:
 		return fmt.Errorf("yaml output format is not (currently) supported for secrets")
 	case OutputJSON:
 		return fmt.Errorf("json output format is not (currently) supported for secrets")
 	case "plaintext":
-		for _, i := range items {
+		for _, item := range items {
 			var data string
-			switch i.Type {
-			case kops.SecretTypeSecret:
-				secret, err := secretStore.FindSecret(i.Name)
-				if err != nil {
-					return fmt.Errorf("error getting secret %q: %v", i.Name, err)
-				}
-				if secret == nil {
-					return fmt.Errorf("cannot find secret %q", i.Name)
-				}
-				data = string(secret.Data)
-
-			default:
-				return fmt.Errorf("secret type %v cannot (currently) be exported as plaintext", i.Type)
-			}
-
-			_, err := fmt.Fprintf(os.Stdout, "%s\n", data)
+			secret, err := secretStore.FindSecret(item)
 			if err != nil {
-				return fmt.Errorf("error writing output: %v", err)
+				return fmt.Errorf("getting secret %q: %v", item, err)
+			}
+			if secret == nil {
+				return fmt.Errorf("cannot find secret %q", item)
+			}
+			data = string(secret.Data)
+
+			_, err = fmt.Fprintf(out, "%s\n", data)
+			if err != nil {
+				return fmt.Errorf("writing output: %v", err)
 			}
 		}
 		return nil
 
 	default:
-		return fmt.Errorf("Unknown output format: %q", options.Output)
+		return fmt.Errorf("unknown output format: %q", options.Output)
 	}
 }

docs/cli/kops_get_secrets.md

@@ -5,29 +5,24 @@
 
 Get one or many secrets.
 
-### Synopsis
-
-Display one or many secrets.
-
 ```
-kops get secrets [flags]
+kops get secrets [SECRET_NAME]... [flags]
 ```
 
 ### Examples
 
 ```
-  # Get a secret
-  kops get secrets kube -oplaintext
+  # List the secrets
+  kops get secrets
   
-  # Get the admin password for a cluster
+  # Get the admin static token for a cluster
   kops get secrets admin -oplaintext
 ```
 
 ### Options
 
 ```
-  -h, --help          help for secrets
-      --type string   Filter by secret type
+  -h, --help   help for secrets
 ```
 
 ### Options inherited from parent commands

upup/pkg/fi/ca.go

@@ -42,13 +42,6 @@ const (
 	keysetFormatLatest = "v1alpha2"
 )
 
-type KeystoreItem struct {
-	Type kops.KeysetType
-	Name string
-	ID   string
-	Data []byte
-}
-
 // Keyset is a parsed api.Keyset.
 type Keyset struct {
 	// LegacyFormat instructs a keypair task to convert a Legacy Keyset to the new Keyset API format.