update: refactored notation list command (notaryproject#481)

This PR refactors the `notation list` command reducing the number of calls to getSignatureRepository() and repo.Resolve(). Signed-off-by: Patrick Zheng <[email protected]>
patrickzheng200 · Dec 19, 2022 · b8ce43a · b8ce43a
1 parent 7a947d5
commit b8ce43a
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 43 deletions.
diff --git a/cmd/notation/list.go b/cmd/notation/list.go
@@ -6,13 +6,15 @@ import (
 	"fmt"
 
 	notationregistry "github.com/notaryproject/notation-go/registry"
+	"github.com/notaryproject/notation/internal/cmd"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/spf13/cobra"
 	"oras.land/oras-go/v2/registry"
 )
 
 type listOpts struct {
+	cmd.LoggingFlagOpts
 	SecureFlagOpts
 	reference string
 }
@@ -34,40 +36,37 @@ func listCommand(opts *listOpts) *cobra.Command {
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runList(cmd, opts)
+			return runList(cmd.Context(), opts)
 		},
 	}
-	opts.ApplyFlags(cmd.Flags())
+	opts.LoggingFlagOpts.ApplyFlags(cmd.Flags())
+	opts.SecureFlagOpts.ApplyFlags(cmd.Flags())
 	return cmd
 }
 
-func runList(command *cobra.Command, opts *listOpts) error {
+func runList(ctx context.Context, opts *listOpts) error {
+	// set log level
+	ctx = opts.LoggingFlagOpts.SetLoggerLevel(ctx)
+
 	// initialize
 	reference := opts.reference
-	sigRepo, err := getSignatureRepository(command.Context(), &opts.SecureFlagOpts, reference)
+	sigRepo, err := getSignatureRepository(ctx, &opts.SecureFlagOpts, reference)
 	if err != nil {
 		return err
 	}
-
-	// core process
-	manifestDesc, _, err := getManifestDescriptor(command.Context(), &opts.SecureFlagOpts, reference)
+	manifestDesc, ref, err := getManifestDescriptor(ctx, &opts.SecureFlagOpts, reference, sigRepo)
 	if err != nil {
 		return err
 	}
 
 	// print all signature manifest digests
-	return printSignatureManifestDigests(command.Context(), manifestDesc.Digest, sigRepo, reference)
+	return printSignatureManifestDigests(ctx, manifestDesc, sigRepo, ref)
 }
 
 // printSignatureManifestDigests returns the signature manifest digests of
 // the subject manifest.
-func printSignatureManifestDigests(ctx context.Context, manifestDigest digest.Digest, sigRepo notationregistry.Repository, reference string) error {
-	// prepare title
-	ref, err := registry.ParseReference(reference)
-	if err != nil {
-		return err
-	}
-	ref.Reference = manifestDigest.String()
+func printSignatureManifestDigests(ctx context.Context, manifestDesc ocispec.Descriptor, sigRepo notationregistry.Repository, ref registry.Reference) error {
+	ref.Reference = manifestDesc.Digest.String()
 	titlePrinted := false
 	printTitle := func() {
 		if !titlePrinted {
@@ -77,13 +76,8 @@ func printSignatureManifestDigests(ctx context.Context, manifestDigest digest.Di
 		}
 	}
 
-	// traverse referrers
-	artifactDescriptor, err := sigRepo.Resolve(ctx, reference)
-	if err != nil {
-		return err
-	}
 	var prevDigest digest.Digest
-	err = sigRepo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error {
+	err := sigRepo.ListSignatures(ctx, manifestDesc, func(signatureManifests []ocispec.Descriptor) error {
 		for _, sigManifestDesc := range signatureManifests {
 			if prevDigest != "" {
 				// check and print title

diff --git a/cmd/notation/manifest.go b/cmd/notation/manifest.go
@@ -4,13 +4,17 @@ import (
 	"context"
 	"errors"
 
+	"github.com/notaryproject/notation-go/log"
+	notationregistry "github.com/notaryproject/notation-go/registry"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2/registry"
 )
 
 // getManifestDescriptor returns target artifact manifest descriptor and
 // registry.Reference given user input reference.
-func getManifestDescriptor(ctx context.Context, opts *SecureFlagOpts, reference string) (ocispec.Descriptor, registry.Reference, error) {
+func getManifestDescriptor(ctx context.Context, opts *SecureFlagOpts, reference string, sigRepo notationregistry.Repository) (ocispec.Descriptor, registry.Reference, error) {
+	logger := log.GetLogger(ctx)
+
 	if reference == "" {
 		return ocispec.Descriptor{}, registry.Reference{}, errors.New("missing reference")
 	}
@@ -21,14 +25,12 @@ func getManifestDescriptor(ctx context.Context, opts *SecureFlagOpts, reference
 	if ref.Reference == "" {
 		return ocispec.Descriptor{}, registry.Reference{}, errors.New("reference is missing digest or tag")
 	}
-	repo, err := getRepositoryClient(ctx, opts, ref)
-	if err != nil {
-		return ocispec.Descriptor{}, registry.Reference{}, err
-	}
 
-	manifestDesc, err := repo.Resolve(ctx, ref.Reference)
+	manifestDesc, err := sigRepo.Resolve(ctx, ref.Reference)
 	if err != nil {
 		return ocispec.Descriptor{}, registry.Reference{}, err
 	}
+
+	logger.Infof("Reference %s resolved to manifest descriptor: %+v", ref.Reference, manifestDesc)
 	return manifestDesc, ref, nil
 }
diff --git a/cmd/notation/sign.go b/cmd/notation/sign.go
@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/notaryproject/notation-go"
+	notationregistry "github.com/notaryproject/notation-go/registry"
 	"github.com/notaryproject/notation/internal/cmd"
 	"github.com/notaryproject/notation/internal/envelope"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -77,16 +78,16 @@ func runSign(command *cobra.Command, cmdOpts *signOpts) error {
 	if err != nil {
 		return err
 	}
-
-	// core process
-	opts, ref, err := prepareSigningContent(ctx, cmdOpts)
+	sigRepo, err := getSignatureRepository(ctx, &cmdOpts.SecureFlagOpts, cmdOpts.reference)
 	if err != nil {
 		return err
 	}
-	sigRepo, err := getSignatureRepository(ctx, &cmdOpts.SecureFlagOpts, cmdOpts.reference)
+	opts, ref, err := prepareSigningContent(ctx, cmdOpts, sigRepo)
 	if err != nil {
 		return err
 	}
+
+	// core process
 	_, err = notation.Sign(ctx, signer, sigRepo, opts)
 	if err != nil {
 		return err
@@ -97,8 +98,8 @@ func runSign(command *cobra.Command, cmdOpts *signOpts) error {
 	return nil
 }
 
-func prepareSigningContent(ctx context.Context, opts *signOpts) (notation.SignOptions, registry.Reference, error) {
-	ref, err := resolveReference(ctx, &opts.SecureFlagOpts, opts.reference, func(ref registry.Reference, manifestDesc ocispec.Descriptor) {
+func prepareSigningContent(ctx context.Context, opts *signOpts, sigRepo notationregistry.Repository) (notation.SignOptions, registry.Reference, error) {
+	ref, err := resolveReference(ctx, &opts.SecureFlagOpts, opts.reference, sigRepo, func(ref registry.Reference, manifestDesc ocispec.Descriptor) {
 		fmt.Printf("Warning: Always sign the artifact using digest(`@sha256:...`) rather than a tag(`:%s`) because tags are mutable and a tag reference can point to a different artifact than the one signed.\n", ref.Reference)
 		fmt.Printf("Resolved artifact tag `%s` to digest `%s` before signing.\n", ref.Reference, manifestDesc.Digest.String())
 	})

diff --git a/cmd/notation/verify.go b/cmd/notation/verify.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/log"
+	notationregistry "github.com/notaryproject/notation-go/registry"
 	"github.com/notaryproject/notation-go/verifier"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
 	"github.com/notaryproject/notation/internal/cmd"
@@ -64,8 +65,15 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 	ctx := opts.LoggingFlagOpts.SetLoggerLevel(command.Context())
 	logger := log.GetLogger(ctx)
 
+	// initialize
+	reference := opts.reference
+	sigRepo, err := getSignatureRepository(ctx, &opts.SecureFlagOpts, reference)
+	if err != nil {
+		return err
+	}
+
 	// resolve the given reference and set the digest
-	ref, err := resolveReference(command.Context(), &opts.SecureFlagOpts, opts.reference, func(ref registry.Reference, manifestDesc ocispec.Descriptor) {
+	ref, err := resolveReference(command.Context(), &opts.SecureFlagOpts, reference, sigRepo, func(ref registry.Reference, manifestDesc ocispec.Descriptor) {
 		fmt.Printf("Resolved artifact tag `%s` to digest `%s` before verification.\n", ref.Reference, manifestDesc.Digest.String())
 		fmt.Println("Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.")
 	})
@@ -79,10 +87,6 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 		return err
 	}
 
-	repo, err := getRepositoryClient(ctx, &opts.SecureFlagOpts, ref)
-	if err != nil {
-		return err
-	}
 	// set up verification plugin config.
 	configs, err := cmd.ParseFlagPluginConfig(opts.pluginConfig)
 	if err != nil {
@@ -97,8 +101,8 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 		MaxSignatureAttempts: math.MaxInt64,
 	}
 
-	// core verify process.
-	_, outcomes, err := notation.Verify(ctx, verifier, repo, verifyOpts)
+	// core process
+	_, outcomes, err := notation.Verify(ctx, verifier, sigRepo, verifyOpts)
 	if err != nil {
 		logger.Error(err)
 	}
@@ -107,7 +111,7 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 		return fmt.Errorf("signature verification failed for all the signatures associated with %s", ref.String())
 	}
 
-	// on success
+	// write out on success
 	outcome := outcomes[0]
 	// print out warning for any failed result with logged verification action
 	for _, result := range outcome.VerificationResults {
@@ -125,8 +129,8 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error {
 	return nil
 }
 
-func resolveReference(ctx context.Context, opts *SecureFlagOpts, reference string, fn func(registry.Reference, ocispec.Descriptor)) (registry.Reference, error) {
-	manifestDesc, ref, err := getManifestDescriptor(ctx, opts, reference)
+func resolveReference(ctx context.Context, opts *SecureFlagOpts, reference string, sigRepo notationregistry.Repository, fn func(registry.Reference, ocispec.Descriptor)) (registry.Reference, error) {
+	manifestDesc, ref, err := getManifestDescriptor(ctx, opts, reference, sigRepo)
 	if err != nil {
 		return registry.Reference{}, err
 	}