Skip to content

Commit

Permalink
fix: error message for trust policy (#933)
Browse files Browse the repository at this point in the history
This error message improvement is based on the Notation CLI error message improvement guideline that we defined in #834

Resolve part of #934 
Signed-off-by: Junjie Gao <[email protected]>
  • Loading branch information
JeyJeyGao authored May 28, 2024
1 parent 3c218d9 commit d439bbb
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
9 changes: 7 additions & 2 deletions cmd/notation/policy/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ func importCmd() *cobra.Command {
Example - Import trust policy configuration from a file:
notation policy import my_policy.json
`,
Args: cobra.ExactArgs(1),
Args: func(cmd *cobra.Command, args []string) error {
if len(args) != 1 {
return fmt.Errorf("requires 1 argument but received %d.\nUsage: notation policy import <path-to-policy.json>\nPlease specify a trust policy file location as the argument", len(args))
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.filePath = args[0]
return runImport(cmd, opts)
Expand Down Expand Up @@ -71,7 +76,7 @@ func runImport(command *cobra.Command, opts importOpts) error {
// optional confirmation
if !opts.force {
if _, err := trustpolicy.LoadDocument(); err == nil {
confirmed, err := cmdutil.AskForConfirmation(os.Stdin, "Existing trust policy configuration found, do you want to overwrite it?", opts.force)
confirmed, err := cmdutil.AskForConfirmation(os.Stdin, "The trust policy file already exists, do you want to overwrite it?", opts.force)
if err != nil {
return err
}
Expand Down
9 changes: 7 additions & 2 deletions cmd/notation/policy/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ package policy

import (
"encoding/json"
"errors"
"fmt"
"io/fs"
"os"

"github.com/notaryproject/notation-go/dir"
Expand Down Expand Up @@ -53,13 +55,16 @@ func runShow(command *cobra.Command, opts showOpts) error {
// get policy file path
policyPath, err := dir.ConfigFS().SysPath(dir.PathTrustPolicy)
if err != nil {
return fmt.Errorf("failed to obtain path of trust policy configuration file: %w", err)
return fmt.Errorf("failed to obtain path of trust policy file: %w", err)
}

// core process
policyJSON, err := os.ReadFile(policyPath)
if err != nil {
return fmt.Errorf("failed to load trust policy configuration, you may import one via `notation policy import <path-to-policy.json>`: %w", err)
if errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("failed to show trust policy as the trust policy file does not exist.\nYou can import one using `notation policy import <path-to-policy.json>`")
}
return fmt.Errorf("failed to show trust policy: %w", err)
}
var doc trustpolicy.Document
if err = json.Unmarshal(policyJSON, &doc); err == nil {
Expand Down
24 changes: 22 additions & 2 deletions test/e2e/suite/command/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,17 @@ var _ = Describe("trust policy maintainer", func() {
Host(Opts(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
notation.ExpectFailure().
Exec("policy", "show").
MatchErrKeyWords("failed to load trust policy configuration", "notation policy import")
MatchErrKeyWords("failed to show trust policy", "notation policy import")
})
})

It("should show error and hint if policy without read permission", func() {
Host(Opts(AddTrustPolicyOption(TrustPolicyName)), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
trustPolicyPath := vhost.AbsolutePath(NotationDirName, TrustPolicyName)
os.Chmod(trustPolicyPath, 0200)
notation.ExpectFailure().
Exec("policy", "show").
MatchErrKeyWords("failed to show trust policy", "permission denied")
})
})

Expand Down Expand Up @@ -60,7 +70,17 @@ var _ = Describe("trust policy maintainer", func() {
It("should fail if no file path is provided", func() {
Host(opts, func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
notation.ExpectFailure().
Exec("policy", "import")
Exec("policy", "import").
MatchErrKeyWords("requires 1 argument but received 0")

})
})

It("should fail if more than one file path is provided", func() {
Host(opts, func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
notation.ExpectFailure().
Exec("policy", "import", "a", "b").
MatchErrKeyWords("requires 1 argument but received 2")
})
})

Expand Down

0 comments on commit d439bbb

Please sign in to comment.