From a6dbab8d72cd8f75aac6994a208c0e776a0229c8 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 2 Dec 2022 11:17:06 +0800 Subject: [PATCH] update: updated plugin list command (#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 --- cmd/notation/cert/generateTest.go | 7 ++++--- cmd/notation/cert/list.go | 9 +++++---- cmd/notation/key.go | 2 +- cmd/notation/plugin.go | 27 ++++++++++++++++++++------- cmd/notation/verify.go | 4 ++-- internal/ioutil/print.go | 22 ---------------------- 6 files changed, 32 insertions(+), 39 deletions(-) diff --git a/cmd/notation/cert/generateTest.go b/cmd/notation/cert/generateTest.go index 8f417c982..bea0730a9 100644 --- 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 index 0bc484b97..cb0b89fa6 100644 --- 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 index a54d1c14f..cffef04e5 100644 --- 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 index b2484d303..10622ab60 100644 --- 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 index fbac59d80..56b16fe52 100644 --- 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 index daf0431aa..8d91a9252 100644 --- 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")