Skip to content

Commit

Permalink
chore(ux): improve error message for oras manifest fetch (#1234)
Browse files Browse the repository at this point in the history
Signed-off-by: Xiaoxuan Wang <[email protected]>
  • Loading branch information
wangxiaoxuan273 authored Jan 16, 2024
1 parent 5bb6c76 commit bbe42fd
Show file tree
Hide file tree
Showing 18 changed files with 94 additions and 75 deletions.
20 changes: 12 additions & 8 deletions cmd/oras/internal/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"strings"

"github.com/spf13/cobra"
"oras.land/oras-go/v2/registry"
"oras.land/oras-go/v2/registry/remote/errcode"
)

Expand Down Expand Up @@ -120,11 +119,16 @@ func reWrap(errA, errB, errC error) error {
}

// NewErrEmptyTagOrDigest creates a new error based on the reference string.
func NewErrEmptyTagOrDigest(ref registry.Reference) error {
return NewErrEmptyTagOrDigestStr(ref.String())
}

// NewErrEmptyTagOrDigestStr creates a new error based on the reference string.
func NewErrEmptyTagOrDigestStr(ref string) error {
return fmt.Errorf("%q: no tag or digest when expecting <name:tag|name@digest>", ref)
func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, needsTag bool) error {
form := `"<name>@<digest>"`
errMsg := `no digest specified`
if needsTag {
form = fmt.Sprintf(`"<name>:<tag>" or %s`, form)
errMsg = "no tag or digest specified"
}
return &Error{
Err: fmt.Errorf(`"%s": %s`, ref, errMsg),
Usage: fmt.Sprintf("%s %s", cmd.Parent().CommandPath(), cmd.Use),
Recommendation: fmt.Sprintf(`Please specify a reference in the form of %s. Run "%s -h" for more options and examples`, form, cmd.CommandPath()),
}
}
14 changes: 11 additions & 3 deletions cmd/oras/internal/option/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,10 @@ func (opts *Target) NewReadonlyTarget(ctx context.Context, common Common, logger
return nil, fmt.Errorf("unknown target type: %q", opts.Type)
}

// EnsureReferenceNotEmpty ensures whether the tag or digest is empty.
func (opts *Target) EnsureReferenceNotEmpty() error {
// EnsureReferenceNotEmpty returns formalized error when the reference is empty.
func (opts *Target) EnsureReferenceNotEmpty(cmd *cobra.Command, needsTag bool) error {
if opts.Reference == "" {
return oerrors.NewErrEmptyTagOrDigestStr(opts.RawReference)
return oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, needsTag)
}
return nil
}
Expand Down Expand Up @@ -298,6 +298,14 @@ type BinaryTarget struct {
resolveFlag []string
}

// EnsureSourceTargetReferenceNotEmpty ensures that the from target reference is not empty.
func (opts *BinaryTarget) EnsureSourceTargetReferenceNotEmpty(cmd *cobra.Command) error {
if opts.From.Reference == "" {
return oerrors.NewErrEmptyTagOrDigest(opts.From.RawReference, cmd, true)
}
return nil
}

// EnableDistributionSpecFlag set distribution specification flag as applicable.
func (opts *BinaryTarget) EnableDistributionSpecFlag() {
opts.From.EnableDistributionSpecFlag()
Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
return runAttach(cmd.Context(), opts)
return runAttach(cmd, opts)
},
}

Expand All @@ -99,8 +99,8 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
return oerrors.Command(cmd, &opts.Target)
}

func runAttach(ctx context.Context, opts attachOptions) error {
ctx, logger := opts.WithContext(ctx)
func runAttach(cmd *cobra.Command, opts attachOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
annotations, err := opts.LoadManifestAnnotations()
if err != nil {
return err
Expand All @@ -120,7 +120,7 @@ func runAttach(ctx context.Context, opts attachOptions) error {
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}
// add both pull and push scope hints for dst repository
Expand Down
9 changes: 4 additions & 5 deletions cmd/oras/root/blob/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.
package blob

import (
"context"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -64,21 +63,21 @@ Example - Delete a blob and print its descriptor:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return deleteBlob(cmd.Context(), opts)
return deleteBlob(cmd, opts)
},
}

option.ApplyFlags(&opts, cmd.Flags())
return oerrors.Command(cmd, &opts.Target)
}

func deleteBlob(ctx context.Context, opts deleteBlobOptions) (err error) {
ctx, logger := opts.WithContext(ctx)
func deleteBlob(cmd *cobra.Command, opts deleteBlobOptions) (err error) {
ctx, logger := opts.WithContext(cmd.Context())
blobs, err := opts.NewBlobDeleter(opts.Common, logger)
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, false); err != nil {
return err
}

Expand Down
12 changes: 5 additions & 7 deletions cmd/oras/root/blob/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ package blob
import (
"context"
"errors"
"fmt"
"io"
"os"

"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/spf13/cobra"
"oras.land/oras-go/v2"
Expand Down Expand Up @@ -83,7 +81,7 @@ Example - Fetch and print a blob from OCI image layout archive file 'layout.tar'
},
Aliases: []string{"get"},
RunE: func(cmd *cobra.Command, args []string) error {
return fetchBlob(cmd.Context(), opts)
return fetchBlob(cmd, opts)
},
}

Expand All @@ -92,16 +90,16 @@ Example - Fetch and print a blob from OCI image layout archive file 'layout.tar'
return oerrors.Command(cmd, &opts.Target)
}

func fetchBlob(ctx context.Context, opts fetchBlobOptions) (fetchErr error) {
ctx, logger := opts.WithContext(ctx)
func fetchBlob(cmd *cobra.Command, opts fetchBlobOptions) (fetchErr error) {
ctx, logger := opts.WithContext(cmd.Context())
var target oras.ReadOnlyTarget
target, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
return err
}

if _, err = digest.Parse(opts.Reference); err != nil {
return fmt.Errorf("%s: blob reference must be of the form <name@digest>", opts.RawReference)
if err := opts.EnsureReferenceNotEmpty(cmd, false); err != nil {
return err
}

if repo, ok := target.(*remote.Repository); ok {
Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/root/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Example - Copy an artifact with multiple tags with concurrency tuned:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return runCopy(cmd.Context(), opts)
return runCopy(cmd, opts)
},
}
cmd.Flags().BoolVarP(&opts.recursive, "recursive", "r", false, "[Preview] recursively copy the artifact and its referrer artifacts")
Expand All @@ -105,15 +105,15 @@ Example - Copy an artifact with multiple tags with concurrency tuned:
return oerrors.Command(cmd, &opts.BinaryTarget)
}

func runCopy(ctx context.Context, opts copyOptions) error {
ctx, logger := opts.WithContext(ctx)
func runCopy(cmd *cobra.Command, opts copyOptions) error {
ctx, logger := opts.WithContext(cmd.Context())

// Prepare source
src, err := opts.From.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
return err
}
if err := opts.From.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureSourceTargetReferenceNotEmpty(cmd); err != nil {
return err
}

Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/root/discover.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return runDiscover(cmd.Context(), opts)
return runDiscover(cmd, opts)
},
}

Expand All @@ -92,13 +92,13 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout
return oerrors.Command(cmd, &opts.Target)
}

func runDiscover(ctx context.Context, opts discoverOptions) error {
ctx, logger := opts.WithContext(ctx)
func runDiscover(cmd *cobra.Command, opts discoverOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
repo, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}

Expand Down
9 changes: 4 additions & 5 deletions cmd/oras/root/manifest/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.
package manifest

import (
"context"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -67,7 +66,7 @@ Example - Delete a manifest by digest 'sha256:99e4703fbf30916f549cd6bfa9cdbab614
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return deleteManifest(cmd.Context(), opts)
return deleteManifest(cmd, opts)
},
}

Expand All @@ -76,13 +75,13 @@ Example - Delete a manifest by digest 'sha256:99e4703fbf30916f549cd6bfa9cdbab614
return oerrors.Command(cmd, &opts.Target)
}

func deleteManifest(ctx context.Context, opts deleteOptions) error {
ctx, logger := opts.WithContext(ctx)
func deleteManifest(cmd *cobra.Command, opts deleteOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
manifests, err := opts.NewManifestDeleter(opts.Common, logger)
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}

Expand Down
9 changes: 4 additions & 5 deletions cmd/oras/root/manifest/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.
package manifest

import (
"context"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -81,7 +80,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar':
},
Aliases: []string{"get"},
RunE: func(cmd *cobra.Command, args []string) error {
return fetchManifest(cmd.Context(), opts)
return fetchManifest(cmd, opts)
},
}

Expand All @@ -91,14 +90,14 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar':
return oerrors.Command(cmd, &opts.Target)
}

func fetchManifest(ctx context.Context, opts fetchOptions) (fetchErr error) {
ctx, logger := opts.WithContext(ctx)
func fetchManifest(cmd *cobra.Command, opts fetchOptions) (fetchErr error) {
ctx, logger := opts.WithContext(cmd.Context())

target, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}
if repo, ok := target.(*remote.Repository); ok {
Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/root/manifest/fetch_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Example - Fetch and print the prettified descriptor of the config:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return fetchConfig(cmd.Context(), opts)
return fetchConfig(cmd, opts)
},
}

Expand All @@ -87,14 +87,14 @@ Example - Fetch and print the prettified descriptor of the config:
return oerrors.Command(cmd, &opts.Target)
}

func fetchConfig(ctx context.Context, opts fetchConfigOptions) (fetchErr error) {
ctx, logger := opts.WithContext(ctx)
func fetchConfig(cmd *cobra.Command, opts fetchConfigOptions) (fetchErr error) {
ctx, logger := opts.WithContext(cmd.Context())

repo, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}
src, err := opts.CachedTarget(repo)
Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/root/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Example - Pull artifact files from an OCI layout archive 'layout.tar':
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return runPull(cmd.Context(), &opts)
return runPull(cmd, &opts)
},
}

Expand All @@ -106,8 +106,8 @@ Example - Pull artifact files from an OCI layout archive 'layout.tar':
return oerrors.Command(cmd, &opts.Target)
}

func runPull(ctx context.Context, opts *pullOptions) error {
ctx, logger := opts.WithContext(ctx)
func runPull(cmd *cobra.Command, opts *pullOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
// Copy Options
copyOptions := oras.DefaultCopyOptions
copyOptions.Concurrency = opts.concurrency
Expand All @@ -118,7 +118,7 @@ func runPull(ctx context.Context, opts *pullOptions) error {
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}
src, err := opts.CachedTarget(target)
Expand Down
9 changes: 4 additions & 5 deletions cmd/oras/root/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.
package root

import (
"context"
"fmt"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -52,7 +51,7 @@ Example - Resolve digest of the target artifact:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return runResolve(cmd.Context(), opts)
return runResolve(cmd, opts)
},
}

Expand All @@ -61,13 +60,13 @@ Example - Resolve digest of the target artifact:
return oerrors.Command(cmd, &opts.Target)
}

func runResolve(ctx context.Context, opts resolveOptions) error {
ctx, logger := opts.WithContext(ctx)
func runResolve(cmd *cobra.Command, opts resolveOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
repo, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(); err != nil {
if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
return err
}
resolveOpts := oras.DefaultResolveOptions
Expand Down
Loading

0 comments on commit bbe42fd

Please sign in to comment.