update: updated plugin list command (notaryproject#461)

This PR mainly fixed the plugin list command. Previously, if there's an error on getting a plugin, only the error will be printed out. In this PR, on error, the pluginName will also be printed out along with the error. (This PR also includes some of the minor cleanups of notation CLI.) Signed-off-by: Patrick Zheng <[email protected]>
patrickzheng200 · Dec 2, 2022 · a6dbab8 · a6dbab8
1 parent 86287b5
commit a6dbab8
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 39 deletions.
diff --git a/cmd/notation/cert/generateTest.go b/cmd/notation/cert/generateTest.go
@@ -84,11 +84,12 @@ func generateTestCert(opts *certGenerateTestOpts) error {
 
 	// write private key
 	relativeKeyPath, relativeCertPath := dir.LocalKeyPath(name)
-	keyPath, err := dir.ConfigFS().SysPath(relativeKeyPath)
+	configFS := dir.ConfigFS()
+	keyPath, err := configFS.SysPath(relativeKeyPath)
 	if err != nil {
 		return err
 	}
-	certPath, err := dir.ConfigFS().SysPath(relativeCertPath)
+	certPath, err := configFS.SysPath(relativeCertPath)
 	if err != nil {
 		return err
 	}
@@ -164,7 +165,7 @@ func generateSelfSignedCert(privateKey *rsa.PrivateKey, name string) (testhelper
 
 func addKeyToSigningKeys(signingKeys *config.SigningKeys, key config.KeySuite, markDefault bool) error {
 	if slices.Contains(signingKeys.Keys, key.Name) {
-		return errors.New(key.Name + ": already exists")
+		return fmt.Errorf("signing key with name %q already exists", key.Name)
 	}
 	signingKeys.Keys = append(signingKeys.Keys, key)
 	if markDefault {

diff --git a/cmd/notation/cert/list.go b/cmd/notation/cert/list.go
@@ -34,11 +34,12 @@ func certListCommand(opts *certListOpts) *cobra.Command {
 func listCerts(opts *certListOpts) error {
 	namedStore := opts.namedStore
 	storeType := opts.storeType
+	configFS := dir.ConfigFS()
 
 	// List all certificates under truststore/x509, display empty if there's
 	// no certificate yet
 	if namedStore == "" && storeType == "" {
-		path, err := dir.ConfigFS().SysPath(dir.TrustStoreDir, "x509")
+		path, err := configFS.SysPath(dir.TrustStoreDir, "x509")
 		if err := truststore.CheckNonErrNotExistError(err); err != nil {
 			return err
 		}
@@ -52,7 +53,7 @@ func listCerts(opts *certListOpts) error {
 	// List all certificates under truststore/x509/storeType/namedStore,
 	// display empty if there's no such certificate
 	if namedStore != "" && storeType != "" {
-		path, err := dir.ConfigFS().SysPath(dir.TrustStoreDir, "x509", storeType, namedStore)
+		path, err := configFS.SysPath(dir.TrustStoreDir, "x509", storeType, namedStore)
 		if err := truststore.CheckNonErrNotExistError(err); err != nil {
 			return err
 		}
@@ -66,7 +67,7 @@ func listCerts(opts *certListOpts) error {
 	// List all certificates under x509/storeType, display empty if
 	// there's no certificate yet
 	if storeType != "" {
-		path, err := dir.ConfigFS().SysPath(dir.TrustStoreDir, "x509", storeType)
+		path, err := configFS.SysPath(dir.TrustStoreDir, "x509", storeType)
 		if err := truststore.CheckNonErrNotExistError(err); err != nil {
 			return err
 		}
@@ -77,7 +78,7 @@ func listCerts(opts *certListOpts) error {
 		// List all certificates under named store namedStore, display empty if
 		// there's no such certificate
 		for _, t := range notationgoTruststore.Types {
-			path, err := dir.ConfigFS().SysPath(dir.TrustStoreDir, "x509", string(t), namedStore)
+			path, err := configFS.SysPath(dir.TrustStoreDir, "x509", string(t), namedStore)
 			if err := truststore.CheckNonErrNotExistError(err); err != nil {
 				return err
 			}

diff --git a/cmd/notation/key.go b/cmd/notation/key.go
@@ -210,7 +210,7 @@ func addExternalKey(ctx context.Context, opts *keyAddOpts, pluginName, keyName s
 
 func addKeyCore(signingKeys *config.SigningKeys, key config.KeySuite, markDefault bool) error {
 	if slices.Contains(signingKeys.Keys, key.Name) {
-		return errors.New(key.Name + ": already exists")
+		return fmt.Errorf("signing key with name %q already exists", key.Name)
 	}
 	signingKeys.Keys = append(signingKeys.Keys, key)
 	if markDefault {

diff --git a/cmd/notation/plugin.go b/cmd/notation/plugin.go
@@ -1,11 +1,13 @@
 package main
 
 import (
+	"fmt"
 	"os"
+	"text/tabwriter"
 
 	"github.com/notaryproject/notation-go/dir"
 	"github.com/notaryproject/notation-go/plugin"
-	"github.com/notaryproject/notation/internal/ioutil"
+	"github.com/notaryproject/notation-go/plugin/proto"
 	"github.com/spf13/cobra"
 )
 
@@ -35,12 +37,23 @@ func listPlugins(command *cobra.Command) error {
 	if err != nil {
 		return err
 	}
-	var plugins []plugin.Plugin
-	var errors []error
+
+	tw := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
+	fmt.Fprintln(tw, "NAME\tDESCRIPTION\tVERSION\tCAPABILITIES\tERROR\t")
+
+	var pl plugin.Plugin
+	var resp *proto.GetMetadataResponse
 	for _, n := range pluginNames {
-		pl, err := mgr.Get(command.Context(), n)
-		errors = append(errors, err)
-		plugins = append(plugins, pl)
+		pl, err = mgr.Get(command.Context(), n)
+		metaData := &proto.GetMetadataResponse{}
+		if err == nil {
+			resp, err = pl.GetMetadata(command.Context(), &proto.GetMetadataRequest{})
+			if err == nil {
+				metaData = resp
+			}
+		}
+		fmt.Fprintf(tw, "%s\t%s\t%s\t%v\t%v\t\n",
+			n, metaData.Description, metaData.Version, metaData.Capabilities, err)
 	}
-	return ioutil.PrintPlugins(command.Context(), os.Stdout, plugins, errors)
+	return tw.Flush()
 }
diff --git a/cmd/notation/verify.go b/cmd/notation/verify.go
@@ -60,12 +60,12 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 		return err
 	}
 	authClient, plainHTTP, _ := getAuthClient(&opts.SecureFlagOpts, ref)
-	remote_repo := remote.Repository{
+	remoteRepo := remote.Repository{
 		Client:    authClient,
 		Reference: ref,
 		PlainHTTP: plainHTTP,
 	}
-	repo := notationregistry.NewRepository(&remote_repo)
+	repo := notationregistry.NewRepository(&remoteRepo)
 
 	// set up verification plugin config.
 	configs, err := cmd.ParseFlagPluginConfig(opts.pluginConfig)

diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go
@@ -1,40 +1,18 @@
 package ioutil
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"text/tabwriter"
 
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/config"
-	"github.com/notaryproject/notation-go/plugin"
-	"github.com/notaryproject/notation-go/plugin/proto"
 )
 
 func newTabWriter(w io.Writer) *tabwriter.Writer {
 	return tabwriter.NewWriter(w, 0, 0, 3, ' ', 0)
 }
 
-func PrintPlugins(ctx context.Context, w io.Writer, v []plugin.Plugin, errors []error) error {
-	tw := newTabWriter(w)
-	fmt.Fprintln(tw, "NAME\tDESCRIPTION\tVERSION\tCAPABILITIES\tERROR\t")
-	for ind, p := range v {
-		metaData := proto.GetMetadataResponse{}
-		if p != nil {
-			req := &proto.GetMetadataRequest{}
-			metadata, err := p.GetMetadata(ctx, req)
-			if err == nil {
-				metaData = *metadata
-			}
-			errors[ind] = err
-		}
-		fmt.Fprintf(tw, "%s\t%s\t%s\t%v\t%v\t\n",
-			metaData.Name, metaData.Description, metaData.Version, metaData.Capabilities, errors[ind])
-	}
-	return tw.Flush()
-}
-
 func PrintKeyMap(w io.Writer, target string, v []config.KeySuite) error {
 	tw := newTabWriter(w)
 	fmt.Fprintln(tw, "NAME\tKEY PATH\tCERTIFICATE PATH\tID\tPLUGIN NAME\t")