Skip to content

Commit

Permalink
fix: improve error messages of notation CLI (#810)
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
  • Loading branch information
Two-Hearts authored and shizhMSFT committed Nov 3, 2023
1 parent 389e720 commit 56b6830
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 52 deletions.
2 changes: 1 addition & 1 deletion cmd/notation/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Example - [Experimental] Inspect signatures on an OCI artifact identified by a d
Long: longMessage,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return errors.New("missing reference")
return errors.New("missing reference to the artifact: use `notation inspect --help` to see what parameters are required")
}
opts.reference = args[0]
return nil
Expand Down
23 changes: 21 additions & 2 deletions cmd/notation/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,32 @@ func listCommand(opts *listOpts) *cobra.Command {
inputType: inputTypeRegistry, // remote registry by default
}
}
longMessage := `List all the signatures associated with signed artifact
Example - List signatures of an OCI artifact:
notation list <registry>/<repository>@<digest>
Example - List signatures of an OCI artifact identified by a tag (Notation will resolve tag to digest)
notation list <registry>/<repository>:<tag>
`
experimentalExamples := `
Example - [Experimental] List signatures of an OCI artifact using the Referrers API. If it's not supported (returns 404), fallback to the Referrers tag schema
notation list --allow-referrers-api <registry>/<repository>@<digest>
Example - [Experimental] List signatures of an OCI artifact referenced in an OCI layout
notation list --oci-layout "<oci_layout_path>@<digest>"
Example - [Experimental] List signatures of an OCI artifact identified by a tag and referenced in an OCI layout
notation list --oci-layout "<oci_layout_path>:<tag>"
`
command := &cobra.Command{
Use: "list [flags] <reference>",
Aliases: []string{"ls"},
Short: "List signatures of the signed artifact",
Long: "List all the signatures associated with signed artifact",
Long: longMessage,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return errors.New("no reference specified")
return errors.New("missing reference to the artifact: use `notation list --help` to see what parameters are required")
}
opts.reference = args[0]
return nil
Expand All @@ -74,6 +92,7 @@ func listCommand(opts *listOpts) *cobra.Command {
command.Flags().BoolVar(&opts.ociLayout, "oci-layout", false, "[Experimental] list signatures stored in OCI image layout")
experimental.HideFlags(command, "", []string{"allow-referrers-api", "oci-layout"})
command.Flags().IntVar(&opts.maxSignatures, "max-signatures", 100, "maximum number of signatures to evaluate or examine")
experimental.HideFlags(command, experimentalExamples, []string{"allow-referrers-api", "oci-layout"})
return command
}

Expand Down
11 changes: 7 additions & 4 deletions cmd/notation/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ func resolveReference(ctx context.Context, inputType inputType, reference string
case inputTypeRegistry:
ref, err := registry.ParseReference(reference)
if err != nil {
return ocispec.Descriptor{}, "", fmt.Errorf("failed to resolve user input reference: %w", err)
return ocispec.Descriptor{}, "", fmt.Errorf("%q: %w. Expecting <registry>/<repository>:<tag> or <registry>/<repository>@<digest>", reference, err)
}
if ref.Reference == "" {
return ocispec.Descriptor{}, "", fmt.Errorf("%q: invalid reference: no tag or digest. Expecting <registry>/<repo>:<tag> or <registry>/<repo>@<digest>", reference)
}
tagOrDigestRef = ref.Reference
resolvedRef = ref.Registry + "/" + ref.Repository
Expand Down Expand Up @@ -113,16 +116,16 @@ func parseOCILayoutReference(raw string) (string, string, error) {
// find `tag`
idx := strings.LastIndex(raw, ":")
if idx == -1 || (idx == 1 && len(raw) > 2 && unicode.IsLetter(rune(raw[0])) && raw[2] == '\\') {
return "", "", notationerrors.ErrorOCILayoutMissingReference{}
return "", "", notationerrors.ErrorOCILayoutMissingReference{Msg: fmt.Sprintf("%q: invalid reference: missing tag or digest. Expecting <file_path>:<tag> or <file_path>@<digest>", raw)}
} else {
path, ref = raw[:idx], raw[idx+1:]
}
}
if path == "" {
return "", "", fmt.Errorf("found empty file path in %q", raw)
return "", "", fmt.Errorf("%q: invalid reference: missing oci-layout file path. Expecting <file_path>:<tag> or <file_path>@<digest>", raw)
}
if ref == "" {
return "", "", fmt.Errorf("found empty reference in %q", raw)
return "", "", notationerrors.ErrorOCILayoutMissingReference{Msg: fmt.Sprintf("%q: invalid reference: missing tag or digest. Expecting <file_path>:<tag> or <file_path>@<digest>", raw)}
}
return path, ref, nil
}
Expand Down
6 changes: 4 additions & 2 deletions cmd/notation/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,11 @@ func getRemoteRepository(ctx context.Context, opts *SecureFlagOpts, reference st
logger := log.GetLogger(ctx)
ref, err := registry.ParseReference(reference)
if err != nil {
return nil, err
return nil, fmt.Errorf("%q: %w. Expecting <registry>/<repository>:<tag> or <registry>/<repository>@<digest>", reference, err)
}
if ref.Reference == "" {
return nil, fmt.Errorf("%q: invalid reference: no tag or digest. Expecting <registry>/<repository>:<tag> or <registry>/<repository>@<digest>", reference)
}

// generate notation repository
remoteRepo, err := getRepositoryClient(ctx, opts, ref)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions cmd/notation/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestRegistry_getRemoteRepositoryWithReferrersAPISupported(t *testing.T) {
t.Fatal("failed to enable experimental")
}
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest {
if r.Method == http.MethodGet && r.URL.Path == "/v2/test/v1/referrers/"+zeroDigest {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{ "test": "TEST" }`))
return
Expand All @@ -49,7 +49,7 @@ func TestRegistry_getRemoteRepositoryWithReferrersAPISupported(t *testing.T) {
secureOpts := SecureFlagOpts{
InsecureRegistry: true,
}
_, err = getRemoteRepository(context.Background(), &secureOpts, uri.Host+"/test", true)
_, err = getRemoteRepository(context.Background(), &secureOpts, uri.Host+"/test:v1", true)
if err != nil {
t.Errorf("getRemoteRepository() expected nil error, but got error: %v", err)
}
Expand All @@ -61,7 +61,7 @@ func TestRegistry_getRemoteRepositoryWithReferrersAPINotSupported(t *testing.T)
t.Fatal("failed to enable experimental")
}
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest {
if r.Method == http.MethodGet && r.URL.Path == "/v2/test/v1/referrers/"+zeroDigest {
w.WriteHeader(http.StatusNotFound)
return
}
Expand All @@ -76,15 +76,15 @@ func TestRegistry_getRemoteRepositoryWithReferrersAPINotSupported(t *testing.T)
secureOpts := SecureFlagOpts{
InsecureRegistry: true,
}
_, err = getRemoteRepository(context.Background(), &secureOpts, uri.Host+"/test", true)
_, err = getRemoteRepository(context.Background(), &secureOpts, uri.Host+"/test:v1", true)
if err != nil {
t.Errorf("getRemoteRepository() expected nil error, but got error: %v", err)
}
}

func TestRegistry_getRemoteRepositoryWithReferrersTagSchema(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest {
if r.Method == http.MethodGet && r.URL.Path == "/v2/test/v1/referrers/"+zeroDigest {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{ "test": "TEST" }`))
return
Expand All @@ -100,7 +100,7 @@ func TestRegistry_getRemoteRepositoryWithReferrersTagSchema(t *testing.T) {
secureOpts := SecureFlagOpts{
InsecureRegistry: true,
}
_, err = getRemoteRepository(context.Background(), &secureOpts, uri.Host+"/test", false)
_, err = getRemoteRepository(context.Background(), &secureOpts, uri.Host+"/test:v1", false)
if err != nil {
t.Errorf("getRemoteRepository() expected nil error, but got error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/notation/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Example - [Experimental] Sign an OCI artifact identified by a tag and referenced
Long: longMessage,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return errors.New("missing reference")
return errors.New("missing reference to the artifact: use `notation sign --help` to see what parameters are required")
}
opts.reference = args[0]
return nil
Expand Down
22 changes: 21 additions & 1 deletion cmd/notation/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ package main
import (
"errors"
"fmt"
"io/fs"
"os"
"reflect"

"github.com/notaryproject/notation-go"
"github.com/notaryproject/notation-go/verifier"
"github.com/notaryproject/notation-go/verifier/trustpolicy"
"github.com/notaryproject/notation-go/verifier/truststore"
"github.com/notaryproject/notation/cmd/notation/internal/experimental"
"github.com/notaryproject/notation/internal/cmd"
"github.com/notaryproject/notation/internal/ioutil"
Expand Down Expand Up @@ -73,7 +75,7 @@ Example - [Experimental] Verify a signature on an OCI artifact identified by a t
Long: longMessage,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return errors.New("missing reference")
return errors.New("missing reference to the artifact: use `notation verify --help` to see what parameters are required")
}
opts.reference = args[0]
return nil
Expand Down Expand Up @@ -157,6 +159,24 @@ func checkVerificationFailure(outcomes []*notation.VerificationOutcome, printOut
// write out on failure
if err != nil || len(outcomes) == 0 {
if err != nil {
var errTrustStore truststore.TrustStoreError
if errors.As(err, &errTrustStore) {
if errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("%w. Use command 'notation cert add' to create and add trusted certificates to the trust store", errTrustStore)
} else {
return fmt.Errorf("%w. %w", errTrustStore, errTrustStore.InnerError)
}
}

var errCertificate truststore.CertificateError
if errors.As(err, &errCertificate) {
if errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("%w. Use command 'notation cert add' to create and add trusted certificates to the trust store", errCertificate)
} else {
return fmt.Errorf("%w. %w", errCertificate, errCertificate.InnerError)
}
}

var errorVerificationFailed notation.ErrorVerificationFailed
if !errors.As(err, &errorVerificationFailed) {
return fmt.Errorf("signature verification failed: %w", err)
Expand Down
19 changes: 10 additions & 9 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,30 @@ module github.com/notaryproject/notation
go 1.20

require (
github.com/notaryproject/notation-core-go v1.0.0
github.com/notaryproject/notation-go v1.0.0
github.com/notaryproject/notation-core-go v1.0.1
github.com/notaryproject/notation-go v1.0.1-0.20231028005734-765d02b5beed
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.1.0-rc5
github.com/oras-project/oras-credentials-go v0.3.1
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.7.0
github.com/spf13/pflag v1.0.5
golang.org/x/term v0.13.0
oras.land/oras-go/v2 v2.3.0
oras.land/oras-go/v2 v2.3.1
)

require (
github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect
github.com/fxamacker/cbor/v2 v2.4.0 // indirect
github.com/go-asn1-ber/asn1-ber v1.5.4 // indirect
github.com/go-ldap/ldap/v3 v3.4.5 // indirect
github.com/fxamacker/cbor/v2 v2.5.0 // indirect
github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect
github.com/go-ldap/ldap/v3 v3.4.6 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/google/uuid v1.3.1 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/veraison/go-cose v1.1.0 // indirect
github.com/x448/float16 v0.8.4 // indirect
golang.org/x/crypto v0.11.0 // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/sync v0.3.0 // indirect
golang.org/x/crypto v0.14.0 // indirect
golang.org/x/mod v0.13.0 // indirect
golang.org/x/sync v0.4.0 // indirect
golang.org/x/sys v0.13.0 // indirect
)
Loading

0 comments on commit 56b6830

Please sign in to comment.