Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve error messages of notation CLI #810

Merged
merged 22 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
Two-Hearts marked this conversation as resolved.
Show resolved Hide resolved
} 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
priteshbandi marked this conversation as resolved.
Show resolved Hide resolved
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
Loading