From 38c2156180e3f04ad907f2935dba69ac8bb7d8e5 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Wed, 24 Apr 2024 13:51:55 +0800 Subject: [PATCH 1/5] fix: error message for trust policy Signed-off-by: Junjie Gao --- cmd/notation/policy/import.go | 9 +++++++-- cmd/notation/policy/show.go | 5 ++++- test/e2e/suite/command/policy.go | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/cmd/notation/policy/import.go b/cmd/notation/policy/import.go index d4d360514..8d4fd6782 100644 --- a/cmd/notation/policy/import.go +++ b/cmd/notation/policy/import.go @@ -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 \nPlease specify a trust policy file as the argument", len(args)) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { opts.filePath = args[0] return runImport(cmd, opts) @@ -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 configuration already exists, do you want to overwrite it?", opts.force) if err != nil { return err } diff --git a/cmd/notation/policy/show.go b/cmd/notation/policy/show.go index 2c60206a0..746a6b01c 100644 --- a/cmd/notation/policy/show.go +++ b/cmd/notation/policy/show.go @@ -59,7 +59,10 @@ func runShow(command *cobra.Command, opts showOpts) error { // 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 `: %w", err) + if os.IsNotExist(err) { + return fmt.Errorf("failed to show trust policy configuration because the trust policy file does not exist.\nYou may import one via `notation policy import `") + } + return fmt.Errorf("failed to load trust policy configuration: %w", err) } var doc trustpolicy.Document if err = json.Unmarshal(policyJSON, &doc); err == nil { diff --git a/test/e2e/suite/command/policy.go b/test/e2e/suite/command/policy.go index 5aeada38e..61d05fb41 100644 --- a/test/e2e/suite/command/policy.go +++ b/test/e2e/suite/command/policy.go @@ -30,7 +30,7 @@ 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 configuration", "notation policy import") }) }) From c9a229b7cc533bb209bcc5b9108a1e292ebca245 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 29 Apr 2024 13:55:32 +0800 Subject: [PATCH 2/5] fix: resolve comments Signed-off-by: Junjie Gao --- cmd/notation/policy/show.go | 6 +++--- test/e2e/suite/command/policy.go | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/notation/policy/show.go b/cmd/notation/policy/show.go index 746a6b01c..06e8e94df 100644 --- a/cmd/notation/policy/show.go +++ b/cmd/notation/policy/show.go @@ -53,16 +53,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 { if os.IsNotExist(err) { - return fmt.Errorf("failed to show trust policy configuration because the trust policy file does not exist.\nYou may import one via `notation policy import `") + return fmt.Errorf("failed to show trust policy because the trust policy file does not exist.\nYou may import one via `notation policy import `") } - return fmt.Errorf("failed to load trust policy configuration: %w", err) + return fmt.Errorf("failed to show trust policy: %w", err) } var doc trustpolicy.Document if err = json.Unmarshal(policyJSON, &doc); err == nil { diff --git a/test/e2e/suite/command/policy.go b/test/e2e/suite/command/policy.go index 61d05fb41..da164abbf 100644 --- a/test/e2e/suite/command/policy.go +++ b/test/e2e/suite/command/policy.go @@ -30,7 +30,7 @@ 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 show trust policy configuration", "notation policy import") + MatchErrKeyWords("failed to show trust policy", "notation policy import") }) }) @@ -60,7 +60,9 @@ 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") + }) }) From fcdba42b719cf56c0a00bbfcf59f766f0a61b80a Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 30 Apr 2024 09:45:14 +0800 Subject: [PATCH 3/5] fix: resolve comments Signed-off-by: Junjie Gao --- cmd/notation/policy/show.go | 4 +++- test/e2e/suite/command/policy.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cmd/notation/policy/show.go b/cmd/notation/policy/show.go index 06e8e94df..602b43711 100644 --- a/cmd/notation/policy/show.go +++ b/cmd/notation/policy/show.go @@ -15,7 +15,9 @@ package policy import ( "encoding/json" + "errors" "fmt" + "io/fs" "os" "github.com/notaryproject/notation-go/dir" @@ -59,7 +61,7 @@ func runShow(command *cobra.Command, opts showOpts) error { // core process policyJSON, err := os.ReadFile(policyPath) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("failed to show trust policy because the trust policy file does not exist.\nYou may import one via `notation policy import `") } return fmt.Errorf("failed to show trust policy: %w", err) diff --git a/test/e2e/suite/command/policy.go b/test/e2e/suite/command/policy.go index da164abbf..8d9ff2cb7 100644 --- a/test/e2e/suite/command/policy.go +++ b/test/e2e/suite/command/policy.go @@ -34,6 +34,16 @@ var _ = Describe("trust policy maintainer", func() { }) }) + 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") + }) + }) + It("should show exist policy", func() { content, err := os.ReadFile(filepath.Join(NotationE2ETrustPolicyDir, TrustPolicyName)) Expect(err).NotTo(HaveOccurred()) From ebe64d933eeb35c09eb26be551e5fb2d70b80b2f Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 6 May 2024 13:09:54 +0800 Subject: [PATCH 4/5] test: added e2e test case Signed-off-by: Junjie Gao --- test/e2e/suite/command/policy.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/suite/command/policy.go b/test/e2e/suite/command/policy.go index 8d9ff2cb7..118939c58 100644 --- a/test/e2e/suite/command/policy.go +++ b/test/e2e/suite/command/policy.go @@ -76,6 +76,14 @@ var _ = Describe("trust policy maintainer", func() { }) }) + 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") + }) + }) + It("should fail if provided file doesn't exist", func() { Host(opts, func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) { notation.ExpectFailure(). From 967a0cf0a73c7f5057b6721e1fb5e81df2a7c694 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 21 May 2024 09:34:16 +0800 Subject: [PATCH 5/5] fix: resolve comments Signed-off-by: Junjie Gao --- cmd/notation/policy/import.go | 4 ++-- cmd/notation/policy/show.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/notation/policy/import.go b/cmd/notation/policy/import.go index 8d4fd6782..ec3d3e854 100644 --- a/cmd/notation/policy/import.go +++ b/cmd/notation/policy/import.go @@ -44,7 +44,7 @@ Example - Import trust policy configuration from a file: `, 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 \nPlease specify a trust policy file as the argument", len(args)) + return fmt.Errorf("requires 1 argument but received %d.\nUsage: notation policy import \nPlease specify a trust policy file location as the argument", len(args)) } return nil }, @@ -76,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, "The trust policy configuration already exists, 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 } diff --git a/cmd/notation/policy/show.go b/cmd/notation/policy/show.go index 602b43711..dca73c942 100644 --- a/cmd/notation/policy/show.go +++ b/cmd/notation/policy/show.go @@ -62,7 +62,7 @@ func runShow(command *cobra.Command, opts showOpts) error { policyJSON, err := os.ReadFile(policyPath) if err != nil { if errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("failed to show trust policy because the trust policy file does not exist.\nYou may import one via `notation policy import `") + return fmt.Errorf("failed to show trust policy as the trust policy file does not exist.\nYou can import one using `notation policy import `") } return fmt.Errorf("failed to show trust policy: %w", err) }