Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ux): improve error message for invalid registry and file reference #1258

Merged
merged 12 commits into from
Jan 31, 2024
8 changes: 8 additions & 0 deletions cmd/oras/internal/option/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ func (opts *Target) Parse() error {
return nil
default:
opts.Type = TargetTypeRemote
if ref, err := registry.ParseReference(opts.RawReference); err != nil {
return err
} else if ref.Registry == "" || ref.Repository == "" {
return &oerrors.Error{
Err: fmt.Errorf("%q is an invalid reference", opts.RawReference),
Recommendation: "Please make sure the provided reference is in the form of <registry>/<repo>[:tag|@digest]",
}
}
return opts.Remote.Parse()
}
}
Expand Down
15 changes: 14 additions & 1 deletion cmd/oras/internal/option/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ func TestTarget_Parse_oci(t *testing.T) {
}

func TestTarget_Parse_remote(t *testing.T) {
opts := Target{IsOCILayout: false}
opts := Target{
RawReference: "mocked/test",
IsOCILayout: false,
}
if err := opts.Parse(); err != nil {
t.Errorf("Target.Parse() error = %v", err)
}
Expand All @@ -48,6 +51,16 @@ func TestTarget_Parse_remote(t *testing.T) {
}
}

func TestTarget_Parse_remote_err(t *testing.T) {
opts := Target{
RawReference: "/test",
IsOCILayout: false,
}
if err := opts.Parse(); err == nil {
t.Errorf("expect Target.Parse() to fail but not")
}
}

func Test_parseOCILayoutReference(t *testing.T) {
type args struct {
raw string
Expand Down
16 changes: 15 additions & 1 deletion cmd/oras/root/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ package root

import (
"context"
"errors"
"fmt"
"io/fs"
"path/filepath"

ocispec "github.com/opencontainers/image-spec/specs-go/v1"
Expand All @@ -42,7 +44,7 @@ func loadFiles(ctx context.Context, store *file.Store, annotations map[string]ma
if verbose {
fmt.Println("Preparing", name)
}
file, err := store.Add(ctx, name, mediaType, filename)
file, err := addFile(ctx, store, name, mediaType, filename)
if err != nil {
return nil, err
}
Expand All @@ -62,3 +64,15 @@ func loadFiles(ctx context.Context, store *file.Store, annotations map[string]ma
}
return files, nil
}

func addFile(ctx context.Context, store *file.Store, name string, mediaType string, filename string) (ocispec.Descriptor, error) {
file, err := store.Add(ctx, name, mediaType, filename)
if err != nil {
var pathErr *fs.PathError
if errors.As(err, &pathErr) {
err = fmt.Errorf("%s %s: %w", pathErr.Op, pathErr.Path, pathErr.Err)
qweeah marked this conversation as resolved.
Show resolved Hide resolved
}
return ocispec.Descriptor{}, err
}
return file, nil
}
2 changes: 1 addition & 1 deletion cmd/oras/root/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func runPush(ctx context.Context, opts *pushOptions) error {
if err != nil {
return err
}
desc, err := store.Add(ctx, option.AnnotationConfig, cfgMediaType, path)
desc, err := addFile(ctx, store, option.AnnotationConfig, cfgMediaType, path)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/suite/command/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ var _ = Describe("ORAS beginners:", func() {

It("should fail if no file reference or manifest annotation provided for OCI layout", func() {
root := GinkgoT().TempDir()
ORAS("attach", "--artifact-type", "oras/test", LayoutRef(root, foobar.Tag)).
ORAS("attach", "--artifact-type", "oras/test", LayoutRef(root, foobar.Tag), Flags.Layout).
ExpectFailure().MatchErrKeyWords("Error: neither file nor annotation", "Usage:").Exec()
})

Expand Down
16 changes: 16 additions & 0 deletions test/e2e/suite/command/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@ var _ = Describe("ORAS beginners:", func() {
gomega.Expect(err).Should(gbytes.Say(`Run "oras push -h"`))
})

It("should fail if the provided reference is not valid", func() {
err := ORAS("push", "/oras").ExpectFailure().Exec().Err
gomega.Expect(err).Should(gbytes.Say(`Error: "/oras" is an invalid reference`))
gomega.Expect(err).Should(gbytes.Say(regexp.QuoteMeta("Please make sure the provided reference is in the form of <registry>/<repo>[:tag|@digest]")))
})

It("should fail if the to-be-pushed file is not found", func() {
tempDir := GinkgoT().TempDir()
notFoundFilePath := "file/not/found"
err := ORAS("push", RegistryRef(ZOTHost, pushTestRepo("file-not-found"), ""), notFoundFilePath).
WithWorkDir(tempDir).
ExpectFailure().Exec().Err
gomega.Expect(err).Should(gbytes.Say(filepath.Join(tempDir, notFoundFilePath)))
gomega.Expect(err).Should(gbytes.Say("no such file or directory"))
})

It("should fail to use --config and --artifact-type at the same time for OCI spec v1.0 registry", func() {
tempDir := PrepareTempFiles()
repo := pushTestRepo("no-mediatype")
Expand Down
Loading