From 54b42cb281ddde60af905b34aadf9c1cd7457f28 Mon Sep 17 00:00:00 2001 From: Pritesh Bandi Date: Wed, 15 Feb 2023 01:00:07 -0800 Subject: [PATCH] Revert "feat: add support for json output for `notation verify` (#527)" (#551) This reverts commit 33c228132389d0b0f7b8c13aa25bf124f0d0b485. We are reverting #527 because we need to write spec first for json output. Signed-off-by: Pritesh Bandi --- cmd/notation/verify.go | 52 ++++++++------------------------ cmd/notation/verify_test.go | 7 +---- internal/cmd/flags.go | 15 +-------- internal/ioutil/print.go | 16 +--------- specs/commandline/verify.md | 25 +-------------- test/e2e/suite/command/verify.go | 41 ------------------------- 6 files changed, 17 insertions(+), 139 deletions(-) diff --git a/cmd/notation/verify.go b/cmd/notation/verify.go index 11bcace99..182bc1cca 100644 --- a/cmd/notation/verify.go +++ b/cmd/notation/verify.go @@ -26,13 +26,6 @@ type verifyOpts struct { reference string pluginConfig []string userMetadata []string - outputFormat string -} - -type verifyOutput struct { - Reference string `json:"reference"` - UserMetadata map[string]string `json:"userMetadata,omitempty"` - Result string `json:"result"` } func verifyCommand(opts *verifyOpts) *cobra.Command { @@ -59,19 +52,14 @@ Example - Verify a signature on an OCI artifact identified by a tag (Notation w opts.reference = args[0] return nil }, - RunE: func(cmnd *cobra.Command, args []string) error { - if opts.outputFormat != cmd.OutputJson && opts.outputFormat != cmd.OutputPlaintext { - return fmt.Errorf("unrecognized output format: %v", opts.outputFormat) - } - - return runVerify(cmnd, opts) + RunE: func(cmd *cobra.Command, args []string) error { + return runVerify(cmd, opts) }, } opts.LoggingFlagOpts.ApplyFlags(command.Flags()) opts.SecureFlagOpts.ApplyFlags(command.Flags()) command.Flags().StringArrayVar(&opts.pluginConfig, "plugin-config", nil, "{key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values") cmd.SetPflagUserMetadata(command.Flags(), &opts.userMetadata, cmd.PflagUserMetadataVerifyUsage) - cmd.SetPflagOutput(command.Flags(), &opts.outputFormat, cmd.PflagOutputUsage) return command } @@ -144,8 +132,13 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error { fmt.Fprintf(os.Stderr, "Warning: %v was set to %q and failed with error: %v\n", result.Type, result.Action, result.Error) } } - - return printResult(opts.outputFormat, ref.String(), outcome) + if reflect.DeepEqual(outcome.VerificationLevel, trustpolicy.LevelSkip) { + fmt.Println("Trust policy is configured to skip signature verification for", ref.String()) + } else { + fmt.Println("Successfully verified signature for", ref.String()) + printMetadataIfPresent(outcome) + } + return nil } func resolveReference(ctx context.Context, opts *SecureFlagOpts, reference string, sigRepo notationregistry.Repository, fn func(registry.Reference, ocispec.Descriptor)) (registry.Reference, error) { @@ -167,33 +160,14 @@ func resolveReference(ctx context.Context, opts *SecureFlagOpts, reference strin return ref, nil } -func printResult(outputFormat, reference string, outcome *notation.VerificationOutcome) error { - if reflect.DeepEqual(outcome.VerificationLevel, trustpolicy.LevelSkip) { - switch outputFormat { - case cmd.OutputJson: - output := verifyOutput{Reference: reference, Result: "SkippedByTrustPolicy", UserMetadata: map[string]string{}} - return ioutil.PrintObjectAsJSON(output) - default: - fmt.Println("Trust policy is configured to skip signature verification for", reference) - return nil - } - } - +func printMetadataIfPresent(outcome *notation.VerificationOutcome) { // the signature envelope is parsed as part of verification. // since user metadata is only printed on successful verification, // this error can be ignored metadata, _ := outcome.UserMetadata() - switch outputFormat { - case cmd.OutputJson: - output := verifyOutput{Reference: reference, Result: "Success", UserMetadata: metadata} - return ioutil.PrintObjectAsJSON(output) - default: - fmt.Println("Successfully verified signature for", reference) - if len(metadata) > 0 { - fmt.Println("\nThe artifact was signed with the following user metadata.") - ioutil.PrintMetadataMap(os.Stdout, metadata) - } - return nil + if len(metadata) > 0 { + fmt.Println("\nThe artifact was signed with the following user metadata.") + ioutil.PrintMetadataMap(os.Stdout, metadata) } } diff --git a/cmd/notation/verify_test.go b/cmd/notation/verify_test.go index 70b2b683e..69d879f25 100644 --- a/cmd/notation/verify_test.go +++ b/cmd/notation/verify_test.go @@ -3,8 +3,6 @@ package main import ( "reflect" "testing" - - "github.com/notaryproject/notation/internal/cmd" ) func TestVerifyCommand_BasicArgs(t *testing.T) { @@ -17,7 +15,6 @@ func TestVerifyCommand_BasicArgs(t *testing.T) { Password: "password", }, pluginConfig: []string{"key1=val1"}, - outputFormat: cmd.OutputPlaintext, } if err := command.ParseFlags([]string{ expected.reference, @@ -43,14 +40,12 @@ func TestVerifyCommand_MoreArgs(t *testing.T) { PlainHTTP: true, }, pluginConfig: []string{"key1=val1", "key2=val2"}, - outputFormat: cmd.OutputJson, } if err := command.ParseFlags([]string{ expected.reference, "--plain-http", "--plugin-config", "key1=val1", - "--plugin-config", "key2=val2", - "--output", "json"}); err != nil { + "--plugin-config", "key2=val2"}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := command.Args(command, command.Flags().Args()); err != nil { diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index f7a62a1b0..c03a9d07d 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -11,11 +11,6 @@ import ( "github.com/spf13/pflag" ) -const ( - OutputPlaintext = "text" - OutputJson = "json" -) - var ( PflagKey = &pflag.Flag{ Name: "key", @@ -80,20 +75,12 @@ var ( Name: "user-metadata", Shorthand: "m", } + PflagUserMetadataSignUsage = "{key}={value} pairs that are added to the signature payload" PflagUserMetadataVerifyUsage = "user defined {key}={value} pairs that must be present in the signature for successful verification if provided" SetPflagUserMetadata = func(fs *pflag.FlagSet, p *[]string, usage string) { fs.StringArrayVarP(p, PflagUserMetadata.Name, PflagUserMetadata.Shorthand, nil, usage) } - - PflagOutput = &pflag.Flag{ - Name: "output", - Shorthand: "o", - } - PflagOutputUsage = fmt.Sprintf("output format, options: '%s', '%s'", OutputJson, OutputPlaintext) - SetPflagOutput = func(fs *pflag.FlagSet, p *string, usage string) { - fs.StringVarP(p, PflagOutput.Name, PflagOutput.Shorthand, OutputPlaintext, usage) - } ) // KeyValueSlice is a flag with type int diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go index 96e452212..f7ae97b4d 100644 --- a/internal/ioutil/print.go +++ b/internal/ioutil/print.go @@ -1,7 +1,6 @@ package ioutil import ( - "encoding/json" "fmt" "io" "text/tabwriter" @@ -34,7 +33,6 @@ func PrintKeyMap(w io.Writer, target *string, v []config.KeySuite) error { return tw.Flush() } -// PrintMetadataMap prints a map to a given Writer as a table func PrintMetadataMap(w io.Writer, metadata map[string]string) error { tw := newTabWriter(w) fmt.Fprintln(tw, "\nKEY\tVALUE\t") @@ -44,16 +42,4 @@ func PrintMetadataMap(w io.Writer, metadata map[string]string) error { } 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, "", " ") - if err != nil { - return err - } - - fmt.Println(string(jsonBytes)) - - return nil -} +} \ No newline at end of file diff --git a/specs/commandline/verify.md b/specs/commandline/verify.md index 5659ce7c2..3741cb966 100644 --- a/specs/commandline/verify.md +++ b/specs/commandline/verify.md @@ -37,12 +37,11 @@ Usage: Flags: -d, --debug debug mode -h, --help help for verify - -o, --output string output format, options: 'json', 'text' (default "text") -p, --password string password for registry operations (default to $NOTATION_PASSWORD if not specified) --plain-http registry access via plain HTTP --plugin-config stringArray {key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values - -m, --user-metadata stringArray user defined {key}={value} pairs that must be present in the signature for successful verification if provided -u, --username string username for registry operations (default to $NOTATION_USERNAME if not specified) + -m, --user-metadata stringArray user defined {key}={value} pairs that must be present in the signature for successful verification if provided -v, --verbose verbose mode ``` @@ -169,25 +168,3 @@ An example of output messages for a successful verification: Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable. Successfully verified signature for localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9 ``` - -### Verify signatures on an OCI artifact with json output - -Use the `--output` flag to format successful verification output in json. - -```shell -notation verify --output json localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9 -``` - -An example of output messages for a successful verification: - -```text -{ - "reference": "localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9", - "userMetadata": { - "io.wabbit-networks.buildId": "123" - }, - "result": "Success" -} -``` - -On unsuccessful verification, nothing is written to `stdout`, and the failure is logged to `stderr`. diff --git a/test/e2e/suite/command/verify.go b/test/e2e/suite/command/verify.go index 9837c1f64..02636be0a 100644 --- a/test/e2e/suite/command/verify.go +++ b/test/e2e/suite/command/verify.go @@ -49,45 +49,4 @@ var _ = Describe("notation verify", func() { MatchKeyWords(VerifySuccessfully) }) }) - - It("with added user metadata", func() { - Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) { - notation.Exec("sign", artifact.ReferenceWithDigest(), "--user-metadata", "io.wabbit-networks.buildId=123"). - MatchKeyWords(SignSuccessfully) - - notation.Exec("verify", artifact.ReferenceWithTag()). - MatchKeyWords( - VerifySuccessfully, - "KEY", - "VALUE", - "io.wabbit-networks.buildId", - "123", - ) - - notation.Exec("verify", artifact.ReferenceWithDigest(), "--user-metadata", "io.wabbit-networks.buildId=123"). - MatchKeyWords( - VerifySuccessfully, - "KEY", - "VALUE", - "io.wabbit-networks.buildId", - "123", - ) - - notation.ExpectFailure().Exec("verify", artifact.ReferenceWithDigest(), "--user-metadata", "io.wabbit-networks.buildId=321"). - MatchErrKeyWords("unable to find specified metadata in the signature") - }) - }) - - It("with json output", func() { - Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) { - notation.Exec("sign", artifact.ReferenceWithDigest(), "--user-metadata", "io.wabbit-networks.buildId=123"). - MatchKeyWords(SignSuccessfully) - - notation.Exec("verify", artifact.ReferenceWithDigest(), "--output", "json"). - MatchContent(fmt.Sprintf("{\n \"reference\": \"%s\",\n \"userMetadata\": {\n \"io.wabbit-networks.buildId\": \"123\"\n },\n \"result\": \"Success\"\n}\n", artifact.ReferenceWithDigest())) - - notation.ExpectFailure().Exec("verify", artifact.ReferenceWithDigest(), "--user-metadata", "io.wabbit-networks.buildId=321"). - MatchErrKeyWords("unable to find specified metadata in the signature") - }) - }) })