From b8ce43a072204614571c41d19fa570fffd6671aa Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 19 Dec 2022 18:08:04 +0800 Subject: [PATCH] update: refactored notation list command (#481) This PR refactors the `notation list` command reducing the number of calls to getSignatureRepository() and repo.Resolve(). Signed-off-by: Patrick Zheng --- cmd/notation/list.go | 36 +++++++++++++++--------------------- cmd/notation/manifest.go | 14 ++++++++------ cmd/notation/sign.go | 13 +++++++------ cmd/notation/verify.go | 24 ++++++++++++++---------- 4 files changed, 44 insertions(+), 43 deletions(-) diff --git a/cmd/notation/list.go b/cmd/notation/list.go index 8923ffec8..92c5c8880 100644 --- a/cmd/notation/list.go +++ b/cmd/notation/list.go @@ -6,6 +6,7 @@ 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" @@ -13,6 +14,7 @@ import ( ) 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 index e5e859b4b..84ac72742 100644 --- 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 index 33440f06a..74ce1d8dd 100644 --- 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 index 36ffcd4e1..d523aac92 100644 --- 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 }