refactor: verify display handler (notaryproject#1167)

Refactor:
- move the verify command output related logic to be `VerifyHandler`

Example:
```sh
notation verify notationreg.azurecr.io/hello-world:v1
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 notationreg.azurecr.io/hello-world@sha256:d37ada95d47ad12224c205a938129df7a3e52345828b4fa27b03a98825d1e2e7

The artifact was signed with the following user metadata.

KEY   VALUE
foo   bar
```

Resolve part of notaryproject#1151

---------

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

Author: JeyJeyGao

cmd/notation/internal/display/handler.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/notaryproject/notation/cmd/notation/internal/display/metadata"
 	"github.com/notaryproject/notation/cmd/notation/internal/display/metadata/json"
+	"github.com/notaryproject/notation/cmd/notation/internal/display/metadata/text"
 	"github.com/notaryproject/notation/cmd/notation/internal/display/metadata/tree"
 	"github.com/notaryproject/notation/cmd/notation/internal/display/output"
 	"github.com/notaryproject/notation/cmd/notation/internal/option"
@@ -40,3 +41,9 @@ func NewInpsectHandler(printer *output.Printer, format option.Format) (metadata.
 	}
 	return nil, fmt.Errorf("unrecognized output format %s", format.CurrentType)
 }
+
+// NewVerifyHandler creates a new metadata VerifyHandler for printing
+// veriifcation result and warnings.
+func NewVerifyHandler(printer *output.Printer) metadata.VerifyHandler {
+	return text.NewVerifyHandler(printer)
+}

cmd/notation/internal/display/metadata/interface.go

@@ -18,6 +18,7 @@ package metadata
 
 import (
 	"github.com/notaryproject/notation-core-go/signature"
+	"github.com/notaryproject/notation-go"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
@@ -37,3 +38,19 @@ type InspectHandler interface {
 	// InspectSignature inspects a signature to get it ready to be rendered.
 	InspectSignature(manifestDesc ocispec.Descriptor, envelope signature.Envelope) error
 }
+
+// VerifyHandler is a handler for rendering metadata information of
+// verification outcome.
+//
+// It only supports text format for now.
+type VerifyHandler interface {
+	Renderer
+
+	// OnResolvingTagReference outputs the tag reference warning.
+	OnResolvingTagReference(reference string)
+
+	// OnVerifySucceeded sets the successful verification result for the handler.
+	//
+	// outcomes must not be nil or empty.
+	OnVerifySucceeded(outcomes []*notation.VerificationOutcome, digestReference string)
+}

cmd/notation/internal/display/metadata/text/verify.go

@@ -0,0 +1,109 @@
+// Copyright The Notary Project Authors.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package text provides the text output in human-readable format for metadata
+// information.
+package text
+
+import (
+	"fmt"
+	"reflect"
+	"text/tabwriter"
+
+	"github.com/notaryproject/notation-go"
+	"github.com/notaryproject/notation-go/verifier/trustpolicy"
+	"github.com/notaryproject/notation/cmd/notation/internal/display/output"
+)
+
+// VerifyHandler is a handler for rendering output for verify command in
+// human-readable format.
+type VerifyHandler struct {
+	printer *output.Printer
+
+	outcome         *notation.VerificationOutcome
+	digestReference string
+	hasWarning      bool
+}
+
+// NewVerifyHandler creates a VerifyHandler to render verification results in
+// human-readable format.
+func NewVerifyHandler(printer *output.Printer) *VerifyHandler {
+	return &VerifyHandler{
+		printer: printer,
+	}
+}
+
+// OnResolvingTagReference outputs the tag reference warning.
+func (h *VerifyHandler) OnResolvingTagReference(reference string) {
+	h.printer.PrintErrorf("Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:%s) because resolved digest may not point to the same signed artifact, as tags are mutable.\n", reference)
+	h.hasWarning = true
+}
+
+// OnVerifySucceeded sets the successful verification result for the handler.
+//
+// outcomes must not be nil or empty.
+func (h *VerifyHandler) OnVerifySucceeded(outcomes []*notation.VerificationOutcome, digestReference string) {
+	h.outcome = outcomes[0]
+	h.digestReference = digestReference
+}
+
+// Render prints out the verification results in human-readable format.
+func (h *VerifyHandler) Render() error {
+	// write out on success
+	// print out warning for any failed result with logged verification action
+	for _, result := range h.outcome.VerificationResults {
+		if result.Error != nil {
+			// at this point, the verification action has to be logged and
+			// it's failed
+			h.printer.PrintErrorf("Warning: %v was set to %q and failed with error: %v\n", result.Type, result.Action, result.Error)
+			h.hasWarning = true
+		}
+	}
+	if h.hasWarning {
+		// print a newline to separate the warning from the final message
+		h.printer.Println()
+	}
+	if reflect.DeepEqual(h.outcome.VerificationLevel, trustpolicy.LevelSkip) {
+		h.printer.Println("Trust policy is configured to skip signature verification for", h.digestReference)
+	} else {
+		h.printer.Println("Successfully verified signature for", h.digestReference)
+		h.printMetadataIfPresent(h.outcome)
+	}
+	return nil
+}
+
+func (h *VerifyHandler) 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()
+
+	if len(metadata) > 0 {
+		h.printer.Println("\nThe artifact was signed with the following user metadata.")
+		h.printMetadataMap(metadata)
+	}
+}
+
+// printMetadataMap prints out metadata given the metatdata map
+//
+// The metadata is additional information of text output.
+func (h *VerifyHandler) printMetadataMap(metadata map[string]string) error {
+	tw := tabwriter.NewWriter(h.printer, 0, 0, 3, ' ', 0)
+	fmt.Fprintln(tw, "\nKEY\tVALUE\t")
+
+	for k, v := range metadata {
+		fmt.Fprintf(tw, "%v\t%v\t\n", k, v)
+	}
+
+	return tw.Flush()
+}

cmd/notation/internal/display/metadata/text/verify_test.go

@@ -0,0 +1,57 @@
+// Copyright The Notary Project Authors.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package text
+
+import (
+	"bytes"
+	"encoding/json"
+	"testing"
+
+	"github.com/notaryproject/notation-core-go/signature"
+	"github.com/notaryproject/notation-go"
+	"github.com/notaryproject/notation/cmd/notation/internal/display/output"
+	"github.com/notaryproject/notation/internal/envelope"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
+func TestPrintMetadataIfPresent(t *testing.T) {
+	payload := &envelope.Payload{
+		TargetArtifact: ocispec.Descriptor{
+			Annotations: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}
+	payloadBytes, _ := json.Marshal(payload)
+
+	outcome := &notation.VerificationOutcome{
+		EnvelopeContent: &signature.EnvelopeContent{
+			Payload: signature.Payload{
+				Content: payloadBytes,
+			},
+		},
+	}
+
+	t.Run("with metadata", func(t *testing.T) {
+		buf := bytes.Buffer{}
+		printer := output.NewPrinter(&buf, &buf)
+		h := NewVerifyHandler(printer)
+		h.printMetadataIfPresent(outcome)
+		got := buf.String()
+		expected := "\nThe artifact was signed with the following user metadata.\n\nKEY   VALUE   \nfoo   bar     \n"
+		if got != expected {
+			t.Errorf("unexpected output: %q", got)
+		}
+	})
+}

cmd/notation/internal/display/output/print.go

@@ -71,7 +71,7 @@ func (p *Printer) Println(a ...any) error {
 	return nil
 }
 
-// Printf prints objects concurrent-safely with newline.
+// Printf prints objects concurrent-safely.
 func (p *Printer) Printf(format string, a ...any) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -85,6 +85,15 @@ func (p *Printer) Printf(format string, a ...any) error {
 	return nil
 }
 
+// PrintErrorf prints objects to error output concurrent-safely.
+func (p *Printer) PrintErrorf(format string, a ...any) error {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
+	_, err := fmt.Fprintf(p.err, format, a...)
+	return err
+}
+
 // PrintPrettyJSON prints object to out in JSON format.
 func PrintPrettyJSON(out io.Writer, object any) error {
 	encoder := json.NewEncoder(out)

cmd/notation/verify.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"reflect"
 
 	"github.com/notaryproject/notation-core-go/revocation/purpose"
 	"github.com/notaryproject/notation-go"
@@ -28,9 +27,11 @@ import (
 	"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/display"
 	"github.com/notaryproject/notation/cmd/notation/internal/experimental"
+	"github.com/notaryproject/notation/cmd/notation/internal/option"
 	"github.com/notaryproject/notation/internal/cmd"
-	"github.com/notaryproject/notation/internal/ioutil"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/spf13/cobra"
 
 	clirev "github.com/notaryproject/notation/internal/revocation"
@@ -39,6 +40,7 @@ import (
 type verifyOpts struct {
 	cmd.LoggingFlagOpts
 	SecureFlagOpts
+	option.Common
 	reference            string
 	pluginConfig         []string
 	userMetadata         []string
@@ -87,6 +89,7 @@ Example - [Experimental] Verify a signature on an OCI artifact identified by a t
 			if opts.ociLayout {
 				opts.inputType = inputTypeOCILayout
 			}
+			opts.Common.Parse(cmd)
 			return experimental.CheckFlagsAndWarn(cmd, "allow-referrers-api", "oci-layout", "scope")
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -116,6 +119,8 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 	// set log level
 	ctx := opts.LoggingFlagOpts.InitializeLogger(command.Context())
 
+	displayHandler := display.NewVerifyHandler(opts.Printer)
+
 	// initialize
 	sigVerifier, err := getVerifier(ctx)
 	if err != nil {
@@ -142,8 +147,9 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 	if err != nil {
 		return err
 	}
-	// resolve the given reference and set the digest
-	_, resolvedRef, err := resolveReferenceWithWarning(ctx, opts.inputType, reference, sigRepo, "verify")
+	_, resolvedRef, err := resolveReference(ctx, opts.inputType, reference, sigRepo, func(ref string, manifestDesc ocispec.Descriptor) {
+		displayHandler.OnResolvingTagReference(ref)
+	})
 	if err != nil {
 		return err
 	}
@@ -159,8 +165,8 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 	if err != nil {
 		return err
 	}
-	reportVerificationSuccess(outcomes, resolvedRef)
-	return nil
+	displayHandler.OnVerifySucceeded(outcomes, resolvedRef)
+	return displayHandler.Render()
 }
 
 func checkVerificationFailure(outcomes []*notation.VerificationOutcome, printOut string, err error) error {
@@ -195,37 +201,6 @@ func checkVerificationFailure(outcomes []*notation.VerificationOutcome, printOut
 	return nil
 }
 
-func reportVerificationSuccess(outcomes []*notation.VerificationOutcome, printout string) {
-	// write out on success
-	outcome := outcomes[0]
-	// print out warning for any failed result with logged verification action
-	for _, result := range outcome.VerificationResults {
-		if result.Error != nil {
-			// at this point, the verification action has to be logged and
-			// it's failed
-			fmt.Fprintf(os.Stderr, "Warning: %v was set to %q and failed with error: %v\n", result.Type, result.Action, result.Error)
-		}
-	}
-	if reflect.DeepEqual(outcome.VerificationLevel, trustpolicy.LevelSkip) {
-		fmt.Println("Trust policy is configured to skip signature verification for", printout)
-	} else {
-		fmt.Println("Successfully verified signature for", printout)
-		printMetadataIfPresent(outcome)
-	}
-}
-
-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()
-
-	if len(metadata) > 0 {
-		fmt.Println("\nThe artifact was signed with the following user metadata.")
-		ioutil.PrintMetadataMap(os.Stdout, metadata)
-	}
-}
-
 func getVerifier(ctx context.Context) (notation.Verifier, error) {
 	// revocation check
 	revocationCodeSigningValidator, err := clirev.NewRevocationValidator(ctx, purpose.CodeSigning)

internal/ioutil/print.go

@@ -48,18 +48,6 @@ func PrintKeyMap(w io.Writer, target *string, v []config.KeySuite) error {
 	return tw.Flush()
 }
 
-// PrintMetadataMap prints out metadata given the metatdata map
-func PrintMetadataMap(w io.Writer, metadata map[string]string) error {
-	tw := newTabWriter(w)
-	fmt.Fprintln(tw, "\nKEY\tVALUE\t")
-
-	for k, v := range metadata {
-		fmt.Fprintf(tw, "%v\t%v\t\n", k, v)
-	}
-
-	return tw.Flush()
-}
-
 // PrintCertMap lists certificate files in the trust store given array of cert
 // paths
 func PrintCertMap(w io.Writer, certPaths []string) error {