From 0479b37c45aaaada72f8e02f06bbbd6f8014c01b Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 9 Oct 2023 16:54:55 +0800 Subject: [PATCH 1/5] reformed cert list outputs Signed-off-by: Patrick Zheng --- cmd/notation/cert/list.go | 21 +++++++++++-------- .../internal/truststore/truststore.go | 8 ++++--- internal/ioutil/print.go | 15 +++++++++++++ specs/commandline/certificate.md | 8 +++---- 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/cmd/notation/cert/list.go b/cmd/notation/cert/list.go index 3e757f35e..a27581ec4 100644 --- a/cmd/notation/cert/list.go +++ b/cmd/notation/cert/list.go @@ -16,12 +16,14 @@ package cert import ( "context" "fmt" + "os" "github.com/notaryproject/notation-go/dir" "github.com/notaryproject/notation-go/log" notationgoTruststore "github.com/notaryproject/notation-go/verifier/truststore" "github.com/notaryproject/notation/cmd/notation/internal/truststore" "github.com/notaryproject/notation/internal/cmd" + "github.com/notaryproject/notation/internal/ioutil" "github.com/spf13/cobra" ) @@ -72,6 +74,7 @@ func listCerts(ctx context.Context, opts *certListOpts) error { storeType := opts.storeType configFS := dir.ConfigFS() + var certPaths []string // List all certificates under truststore/x509, display empty if there's // no certificate yet if namedStore == "" && storeType == "" { @@ -79,27 +82,27 @@ func listCerts(ctx context.Context, opts *certListOpts) error { if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 2)); err != nil { + if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 2, &certPaths)); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored in the trust store, with error: %s", err.Error()) } - return nil + return ioutil.PrintCertMap(os.Stdout, certPaths) } // List all certificates under truststore/x509/storeType/namedStore, - // display empty if there's no such certificate + // display empty if there's no certificate yet if namedStore != "" && storeType != "" { path, err := configFS.SysPath(dir.TrustStoreDir, "x509", storeType, namedStore) if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 0)); err != nil { + if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 0, &certPaths)); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored in the named store %s of type %s, with error: %s", namedStore, storeType, err.Error()) } - return nil + return ioutil.PrintCertMap(os.Stdout, certPaths) } // List all certificates under x509/storeType, display empty if @@ -109,24 +112,24 @@ func listCerts(ctx context.Context, opts *certListOpts) error { if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 1)); err != nil { + if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 1, &certPaths)); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored of type %s, with error: %s", storeType, err.Error()) } } else { // List all certificates under named store namedStore, display empty if - // there's no such certificate + // there's no certificate yet for _, t := range notationgoTruststore.Types { path, err := configFS.SysPath(dir.TrustStoreDir, "x509", string(t), namedStore) if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 0)); err != nil { + if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 0, &certPaths)); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored in the named store %s, with error: %s", namedStore, err.Error()) } } } - return nil + return ioutil.PrintCertMap(os.Stdout, certPaths) } diff --git a/cmd/notation/internal/truststore/truststore.go b/cmd/notation/internal/truststore/truststore.go index 4125dff4d..54a3bdfa8 100644 --- a/cmd/notation/internal/truststore/truststore.go +++ b/cmd/notation/internal/truststore/truststore.go @@ -87,9 +87,11 @@ func AddCert(path, storeType, namedStore string, display bool) error { // ListCerts walks through root and lists all x509 certificates in it, // sub-dirs are ignored. -func ListCerts(root string, depth int) error { +func ListCerts(root string, depth int, certPaths *[]string) error { maxDepth := strings.Count(root, string(os.PathSeparator)) + depth - + if certPaths == nil { + return errors.New("certPaths cannot be nil") + } return filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if err != nil { return err @@ -107,7 +109,7 @@ func ListCerts(root string, depth int) error { return err } if len(certs) != 0 { - fmt.Println(path) + *certPaths = append(*certPaths, path) } } return nil diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go index 055f0b547..70dc9a871 100644 --- a/internal/ioutil/print.go +++ b/internal/ioutil/print.go @@ -17,6 +17,7 @@ import ( "encoding/json" "fmt" "io" + "path/filepath" "text/tabwriter" "github.com/notaryproject/notation-go/config" @@ -58,6 +59,20 @@ func PrintMetadataMap(w io.Writer, metadata map[string]string) error { return tw.Flush() } +func PrintCertMap(w io.Writer, certPaths []string) error { + tw := newTabWriter(w) + fmt.Fprintln(tw, "STORE TYPE\tNAMED STORE\tCERTIFICATE\t") + for _, cert := range certPaths { + fileName := filepath.Base(cert) + dir := filepath.Dir(cert) + namedStore := filepath.Base(dir) + dir = filepath.Dir(dir) + storeType := filepath.Base(dir) + fmt.Fprintf(tw, "%s\t%s\t%s\t\n", storeType, namedStore, fileName) + } + return tw.Flush() +} + // PrintObjectAsJSON takes an interface and prints it as an indented JSON string func PrintObjectAsJSON(i interface{}) error { jsonBytes, err := json.MarshalIndent(i, "", " ") diff --git a/specs/commandline/certificate.md b/specs/commandline/certificate.md index bbb052625..8f328b98f 100644 --- a/specs/commandline/certificate.md +++ b/specs/commandline/certificate.md @@ -151,7 +151,7 @@ Upon successful adding, the certificate files are added into directory`{NOTATION notation certificate list ``` -Upon successful listing, all the certificate files in the trust store are printed out in a format of absolute filepath. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### List all certificate files of a certain named store @@ -159,7 +159,7 @@ Upon successful listing, all the certificate files in the trust store are printe notation cert list --store ``` -Upon successful listing, all the certificate files in the trust store named `` are printed out in a format of absolute filepath. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store named `` are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### List all certificate files of a certain type of store @@ -167,7 +167,7 @@ Upon successful listing, all the certificate files in the trust store named ` ``` -Upon successful listing, all the certificate files in the trust store of type `` are printed out in a format of absolute filepath. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store of type `` are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### List all certificate files of a certain named store of a certain type @@ -175,7 +175,7 @@ Upon successful listing, all the certificate files in the trust store of type `< notation cert list --type --store ``` -Upon successful listing, all the certificate files in the trust store named `` of type `` are printed out in a format of absolute filepath. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store named `` of type `` are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### Show details of a certain certificate file From 3804f31fe2a3105587cbfda8e4572841e140576d Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 9 Oct 2023 17:03:34 +0800 Subject: [PATCH 2/5] fixed e2e test Signed-off-by: Patrick Zheng --- test/e2e/suite/scenario/quickstart.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/e2e/suite/scenario/quickstart.go b/test/e2e/suite/scenario/quickstart.go index ddf0aa46c..b04f5b22e 100644 --- a/test/e2e/suite/scenario/quickstart.go +++ b/test/e2e/suite/scenario/quickstart.go @@ -62,7 +62,11 @@ var _ = Describe("notation quickstart E2E test", Ordered, func() { ) notation.Exec("cert", "ls"). - MatchKeyWords("notation/truststore/x509/ca/wabbit-networks.io/wabbit-networks.io.crt") + MatchKeyWords( + "ca", + "wabbit-networks.io", + "wabbit-networks.io.crt", + ) }) It("sign the container image with jws format (by default)", func() { From 457a0190294bc9c7a850052ccba991139932f140 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 10 Oct 2023 09:19:14 +0800 Subject: [PATCH 3/5] updated column name Signed-off-by: Patrick Zheng --- internal/ioutil/print.go | 2 +- specs/commandline/certificate.md | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go index 70dc9a871..484c3d26d 100644 --- a/internal/ioutil/print.go +++ b/internal/ioutil/print.go @@ -61,7 +61,7 @@ func PrintMetadataMap(w io.Writer, metadata map[string]string) error { func PrintCertMap(w io.Writer, certPaths []string) error { tw := newTabWriter(w) - fmt.Fprintln(tw, "STORE TYPE\tNAMED STORE\tCERTIFICATE\t") + fmt.Fprintln(tw, "STORE TYPE\tSTORE NAME\tCERTIFICATE\t") for _, cert := range certPaths { fileName := filepath.Base(cert) dir := filepath.Dir(cert) diff --git a/specs/commandline/certificate.md b/specs/commandline/certificate.md index 8f328b98f..3109b782f 100644 --- a/specs/commandline/certificate.md +++ b/specs/commandline/certificate.md @@ -151,7 +151,7 @@ Upon successful adding, the certificate files are added into directory`{NOTATION notation certificate list ``` -Upon successful listing, all the certificate files in the trust store are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store are printed out with information of store type, store name and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### List all certificate files of a certain named store @@ -159,7 +159,7 @@ Upon successful listing, all the certificate files in the trust store are printe notation cert list --store ``` -Upon successful listing, all the certificate files in the trust store named `` are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store named `` are printed out with information of store type, store name and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### List all certificate files of a certain type of store @@ -167,7 +167,7 @@ Upon successful listing, all the certificate files in the trust store named ` ``` -Upon successful listing, all the certificate files in the trust store of type `` are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store of type `` are printed out with information of store type, store name and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### List all certificate files of a certain named store of a certain type @@ -175,7 +175,7 @@ Upon successful listing, all the certificate files in the trust store of type `< notation cert list --type --store ``` -Upon successful listing, all the certificate files in the trust store named `` of type `` are printed out with information of store type, named store and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +Upon successful listing, all the certificate files in the trust store named `` of type `` are printed out with information of store type, store name and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. ### Show details of a certain certificate file From 52213aed570db9924386cc092d59f9cdd71824ee Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 11 Oct 2023 11:30:52 +0800 Subject: [PATCH 4/5] updated per code review Signed-off-by: Patrick Zheng --- cmd/notation/cert/list.go | 42 ++++++++++++------- .../internal/truststore/truststore.go | 18 ++++---- internal/ioutil/print.go | 7 ++++ specs/commandline/certificate.md | 8 ++++ 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/cmd/notation/cert/list.go b/cmd/notation/cert/list.go index a27581ec4..dfa1ab94c 100644 --- a/cmd/notation/cert/list.go +++ b/cmd/notation/cert/list.go @@ -78,41 +78,51 @@ func listCerts(ctx context.Context, opts *certListOpts) error { // List all certificates under truststore/x509, display empty if there's // no certificate yet if namedStore == "" && storeType == "" { - path, err := configFS.SysPath(dir.TrustStoreDir, "x509") - if err := truststore.CheckNonErrNotExistError(err); err != nil { - return err - } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 2, &certPaths)); err != nil { - logger.Debugln("Failed to complete list at path:", path) - return fmt.Errorf("failed to list all certificates stored in the trust store, with error: %s", err.Error()) + for _, t := range notationgoTruststore.Types { + path, err := configFS.SysPath(dir.TrustStoreDir, "x509", string(t)) + if err := truststore.CheckNonErrNotExistError(err); err != nil { + return err + } + certs, err := truststore.ListCerts(path, 1) + if err := truststore.CheckNonErrNotExistError(err); err != nil { + logger.Debugln("Failed to complete list at path:", path) + return fmt.Errorf("failed to list all certificates stored in the trust store, with error: %s", err.Error()) + } + certPaths = append(certPaths, certs...) } - return ioutil.PrintCertMap(os.Stdout, certPaths) } // List all certificates under truststore/x509/storeType/namedStore, - // display empty if there's no certificate yet + // display empty if store type is invalid or there's no certificate yet if namedStore != "" && storeType != "" { + if !truststore.IsValidStoreType(storeType) { + return nil + } path, err := configFS.SysPath(dir.TrustStoreDir, "x509", storeType, namedStore) if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 0, &certPaths)); err != nil { + certPaths, err := truststore.ListCerts(path, 0) + if err := truststore.CheckNonErrNotExistError(err); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored in the named store %s of type %s, with error: %s", namedStore, storeType, err.Error()) } - return ioutil.PrintCertMap(os.Stdout, certPaths) } - // List all certificates under x509/storeType, display empty if - // there's no certificate yet + // List all certificates under x509/storeType, display empty if store type + // is invalid or there's no certificate yet if storeType != "" { + if !truststore.IsValidStoreType(storeType) { + return nil + } path, err := configFS.SysPath(dir.TrustStoreDir, "x509", storeType) if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 1, &certPaths)); err != nil { + certPaths, err = truststore.ListCerts(path, 1) + if err := truststore.CheckNonErrNotExistError(err); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored of type %s, with error: %s", storeType, err.Error()) } @@ -124,10 +134,12 @@ func listCerts(ctx context.Context, opts *certListOpts) error { if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - if err := truststore.CheckNonErrNotExistError(truststore.ListCerts(path, 0, &certPaths)); err != nil { + certs, err := truststore.ListCerts(path, 0) + if err := truststore.CheckNonErrNotExistError(err); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored in the named store %s, with error: %s", namedStore, err.Error()) } + certPaths = append(certPaths, certs...) } } diff --git a/cmd/notation/internal/truststore/truststore.go b/cmd/notation/internal/truststore/truststore.go index 54a3bdfa8..39edb0658 100644 --- a/cmd/notation/internal/truststore/truststore.go +++ b/cmd/notation/internal/truststore/truststore.go @@ -85,14 +85,12 @@ func AddCert(path, storeType, namedStore string, display bool) error { return nil } -// ListCerts walks through root and lists all x509 certificates in it, +// ListCerts walks through root and returns all x509 certificates in it, // sub-dirs are ignored. -func ListCerts(root string, depth int, certPaths *[]string) error { +func ListCerts(root string, depth int) ([]string, error) { maxDepth := strings.Count(root, string(os.PathSeparator)) + depth - if certPaths == nil { - return errors.New("certPaths cannot be nil") - } - return filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + var certPaths []string + if err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } @@ -109,11 +107,15 @@ func ListCerts(root string, depth int, certPaths *[]string) error { return err } if len(certs) != 0 { - *certPaths = append(*certPaths, path) + certPaths = append(certPaths, path) } } return nil - }) + }); err != nil { + return nil, err + } + + return certPaths, nil } // ShowCerts writes out details of certificates diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go index 484c3d26d..a7a3bac6c 100644 --- a/internal/ioutil/print.go +++ b/internal/ioutil/print.go @@ -27,6 +27,7 @@ func newTabWriter(w io.Writer) *tabwriter.Writer { return tabwriter.NewWriter(w, 0, 0, 3, ' ', 0) } +// PrintKeyMap prints out key information given array of KeySuite 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") @@ -48,6 +49,7 @@ func PrintKeyMap(w io.Writer, target *string, v []config.KeySuite) error { return tw.Flush() } +// PrintMetadataMap prints out metadata given the metatdata map func PrintMetadataMap(w io.Writer, metadata map[string]string) error { tw := newTabWriter(w) fmt.Fprintln(tw, "\nKEY\tVALUE\t") @@ -59,7 +61,12 @@ func PrintMetadataMap(w io.Writer, metadata map[string]string) error { return tw.Flush() } +// PrintCertMap lists certificate files in the trust store given array of cert +// paths func PrintCertMap(w io.Writer, certPaths []string) error { + if len(certPaths) == 0 { + return nil + } tw := newTabWriter(w) fmt.Fprintln(tw, "STORE TYPE\tSTORE NAME\tCERTIFICATE\t") for _, cert := range certPaths { diff --git a/specs/commandline/certificate.md b/specs/commandline/certificate.md index 3109b782f..2822d79e0 100644 --- a/specs/commandline/certificate.md +++ b/specs/commandline/certificate.md @@ -153,6 +153,14 @@ notation certificate list Upon successful listing, all the certificate files in the trust store are printed out with information of store type, store name and certificate file name. If the listing fails, an error message is printed out with specific reasons. Nothing is printed out if the trust store is empty. +An example of the output: +``` +STORE TYPE STORE NAME CERTIFICATE +ca myStore1 cert1.pem +ca myStore2 cert2.crt +signingAuthority myStore1 cert3.crt +signingAuthority myStore2 cert4.pem +``` ### List all certificate files of a certain named store ```bash From 928a0f52ada74bc616c05ef808b1714e87b05206 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 11 Oct 2023 15:14:43 +0800 Subject: [PATCH 5/5] code clean up Signed-off-by: Patrick Zheng --- cmd/notation/cert/list.go | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/cmd/notation/cert/list.go b/cmd/notation/cert/list.go index dfa1ab94c..bcaceebc3 100644 --- a/cmd/notation/cert/list.go +++ b/cmd/notation/cert/list.go @@ -74,10 +74,10 @@ func listCerts(ctx context.Context, opts *certListOpts) error { storeType := opts.storeType configFS := dir.ConfigFS() - var certPaths []string // List all certificates under truststore/x509, display empty if there's // no certificate yet if namedStore == "" && storeType == "" { + var certPaths []string for _, t := range notationgoTruststore.Types { path, err := configFS.SysPath(dir.TrustStoreDir, "x509", string(t)) if err := truststore.CheckNonErrNotExistError(err); err != nil { @@ -121,27 +121,28 @@ func listCerts(ctx context.Context, opts *certListOpts) error { if err := truststore.CheckNonErrNotExistError(err); err != nil { return err } - certPaths, err = truststore.ListCerts(path, 1) + certPaths, err := truststore.ListCerts(path, 1) if err := truststore.CheckNonErrNotExistError(err); err != nil { logger.Debugln("Failed to complete list at path:", path) return fmt.Errorf("failed to list all certificates stored of type %s, with error: %s", storeType, err.Error()) } - } else { - // List all certificates under named store namedStore, display empty if - // there's no certificate yet - for _, t := range notationgoTruststore.Types { - path, err := configFS.SysPath(dir.TrustStoreDir, "x509", string(t), namedStore) - if err := truststore.CheckNonErrNotExistError(err); err != nil { - return err - } - certs, err := truststore.ListCerts(path, 0) - if err := truststore.CheckNonErrNotExistError(err); err != nil { - logger.Debugln("Failed to complete list at path:", path) - return fmt.Errorf("failed to list all certificates stored in the named store %s, with error: %s", namedStore, err.Error()) - } - certPaths = append(certPaths, certs...) - } + return ioutil.PrintCertMap(os.Stdout, certPaths) } + // List all certificates under named store namedStore, display empty if + // there's no certificate yet + var certPaths []string + for _, t := range notationgoTruststore.Types { + path, err := configFS.SysPath(dir.TrustStoreDir, "x509", string(t), namedStore) + if err := truststore.CheckNonErrNotExistError(err); err != nil { + return err + } + certs, err := truststore.ListCerts(path, 0) + if err := truststore.CheckNonErrNotExistError(err); err != nil { + logger.Debugln("Failed to complete list at path:", path) + return fmt.Errorf("failed to list all certificates stored in the named store %s, with error: %s", namedStore, err.Error()) + } + certPaths = append(certPaths, certs...) + } return ioutil.PrintCertMap(os.Stdout, certPaths) }