From 2aaa87cbd6475d3d31d35b9a1c64d8f7cd09c94b Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Fri, 22 Mar 2024 08:25:56 +0000 Subject: [PATCH 01/18] bump: bump `oras-go` to 2.5.0 Signed-off-by: Xiaoxuan Wang --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 2a91fd58f..1ca0f0c8c 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( golang.org/x/sync v0.6.0 golang.org/x/term v0.18.0 gopkg.in/yaml.v3 v3.0.1 - oras.land/oras-go/v2 v2.4.0 + oras.land/oras-go/v2 v2.5.0 ) require ( diff --git a/go.sum b/go.sum index b4139fdeb..f55f57036 100644 --- a/go.sum +++ b/go.sum @@ -109,5 +109,5 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -oras.land/oras-go/v2 v2.4.0 h1:i+Wt5oCaMHu99guBD0yuBjdLvX7Lz8ukPbwXdR7uBMs= -oras.land/oras-go/v2 v2.4.0/go.mod h1:osvtg0/ClRq1KkydMAEu/IxFieyjItcsQ4ut4PPF+f8= +oras.land/oras-go/v2 v2.5.0 h1:o8Me9kLY74Vp5uw07QXPiitjsw7qNXi8Twd+19Zf02c= +oras.land/oras-go/v2 v2.5.0/go.mod h1:z4eisnLP530vwIOUOJeBIj0aGI0L1C3d53atvCBqZHg= From 12d4482797cd0a79b7ee17ebea6f139ac5bb4ca4 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Fri, 22 Mar 2024 11:02:58 +0000 Subject: [PATCH 02/18] fix failed e2e test Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 56da7587d..b5853ec16 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -105,13 +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 /[:tag|@digest]", + if _, err := registry.ParseReference(opts.RawReference); err != nil { + if errors.Is(err, errdef.ErrInvalidReference) { + 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 /[:tag|@digest]", + } } + return err } return opts.Remote.Parse() } From 6ab99c4d94a9fec0ec7bd50f258e29a9e0f42dba Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Fri, 22 Mar 2024 11:32:02 +0000 Subject: [PATCH 03/18] fix e2e test case Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 6 ------ test/e2e/suite/command/push.go | 3 +-- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index b5853ec16..59e319ba8 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -106,12 +106,6 @@ func (opts *Target) Parse() error { default: opts.Type = TargetTypeRemote if _, err := registry.ParseReference(opts.RawReference); err != nil { - if errors.Is(err, errdef.ErrInvalidReference) { - 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 /[:tag|@digest]", - } - } return err } return opts.Remote.Parse() diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index 19a89ad39..d41c9be4d 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -51,8 +51,7 @@ var _ = Describe("ORAS beginners:", func() { 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 /[:tag|@digest]"))) + gomega.Expect(err).Should(gbytes.Say(`Error: invalid reference: invalid registry ""`)) }) It("should fail if the to-be-pushed file is not found", func() { From c6c05d429189268c3a9b293c56d866ea960c1397 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Fri, 22 Mar 2024 11:43:38 +0000 Subject: [PATCH 04/18] fix lint Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/spec.go | 4 ++-- cmd/oras/root/attach.go | 2 +- cmd/oras/root/push.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/spec.go b/cmd/oras/internal/option/spec.go index 6f2d7a2b3..7160098f3 100644 --- a/cmd/oras/internal/option/spec.go +++ b/cmd/oras/internal/option/spec.go @@ -45,7 +45,7 @@ func (is *ImageSpec) Set(value string) error { is.flag = value switch value { case ImageSpecV1_1: - is.PackVersion = oras.PackManifestVersion1_1_RC4 + is.PackVersion = oras.PackManifestVersion1_1 case ImageSpecV1_0: is.PackVersion = oras.PackManifestVersion1_0 default: @@ -78,7 +78,7 @@ func (is *ImageSpec) String() string { // ApplyFlags applies flags to a command flag set. func (is *ImageSpec) ApplyFlags(fs *pflag.FlagSet) { // default to v1.1-rc.4 - is.PackVersion = oras.PackManifestVersion1_1_RC4 + is.PackVersion = oras.PackManifestVersion1_1 defaultFlag := ImageSpecV1_1 fs.Var(is, "image-spec", fmt.Sprintf(`[Experimental] specify manifest type for building artifact. Options: %s (default %q)`, is.Options(), defaultFlag)) } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 944810b2f..0e6bbbe75 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -156,7 +156,7 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { Layers: descs, } pack := func() (ocispec.Descriptor, error) { - return oras.PackManifest(ctx, store, oras.PackManifestVersion1_1_RC4, opts.artifactType, packOpts) + return oras.PackManifest(ctx, store, oras.PackManifestVersion1_1, opts.artifactType, packOpts) } copy := func(root ocispec.Descriptor) error { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index fb357e71e..a6541286e 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -113,7 +113,7 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t if opts.manifestConfigRef != "" && opts.artifactType != "" { return errors.New("--artifact-type and --config cannot both be provided for 1.0 OCI image") } - case oras.PackManifestVersion1_1_RC4: + case oras.PackManifestVersion1_1: if opts.manifestConfigRef == "" && opts.artifactType == "" { opts.artifactType = oras.MediaTypeUnknownArtifact } From c7392f16b28d39105d32ba742d158350def0177d Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 05:59:03 +0000 Subject: [PATCH 05/18] revert change Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 9 ++++++++- test/e2e/suite/command/push.go | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 59e319ba8..6aa49abf3 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -94,6 +94,7 @@ func (opts *Target) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description opts.Remote.ApplyFlagsWithPrefix(fs, prefix, description) } +// use parseOCILayoutReference in the 1st branch // Parse gets target options from user input. func (opts *Target) Parse() error { switch { @@ -106,12 +107,17 @@ func (opts *Target) Parse() error { default: opts.Type = TargetTypeRemote if _, err := registry.ParseReference(opts.RawReference); err != nil { - return err + 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 /[:tag|@digest]", + } } return opts.Remote.Parse() } } +// make this function return invalidReference +// make this a receiver function // parseOCILayoutReference parses the raw in format of [:|@] func parseOCILayoutReference(raw string) (path string, ref string, err error) { if idx := strings.LastIndex(raw, "@"); idx != -1 { @@ -120,6 +126,7 @@ func parseOCILayoutReference(raw string) (path string, ref string, err error) { ref = raw[idx+1:] } else { // find `tag` + // use Errors.Join with invalidReference, only when error != nil path, ref, err = fileref.Parse(raw, "") } return diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index d41c9be4d..19a89ad39 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -51,7 +51,8 @@ var _ = Describe("ORAS beginners:", func() { 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: invalid reference: invalid registry ""`)) + 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 /[:tag|@digest]"))) }) It("should fail if the to-be-pushed file is not found", func() { From 582dd58e969aa79d381f2217c0a19918d4c3bd60 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 06:10:12 +0000 Subject: [PATCH 06/18] fix one failed test Signed-off-by: Xiaoxuan Wang --- test/e2e/suite/command/resolve.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/suite/command/resolve.go b/test/e2e/suite/command/resolve.go index adbef52ba..449271830 100644 --- a/test/e2e/suite/command/resolve.go +++ b/test/e2e/suite/command/resolve.go @@ -41,7 +41,7 @@ var _ = Describe("ORAS beginners:", func() { ORAS("resolve").ExpectFailure().MatchErrKeyWords("Error:").Exec() }) It("should fail when repo is invalid", func() { - ORAS("resolve", fmt.Sprintf("%s/%s", ZOTHost, InvalidRepo)).ExpectFailure().MatchErrKeyWords("Error:", fmt.Sprintf("invalid reference: invalid repository %q", InvalidRepo)).Exec() + ORAS("resolve", fmt.Sprintf("%s/%s", ZOTHost, InvalidRepo)).ExpectFailure().MatchErrKeyWords("Error:", "localhost:7000/INVALID", "invalid reference", "/[:tag|@digest]").Exec() }) It("should fail when no tag or digest provided", func() { ORAS("resolve", RegistryRef(ZOTHost, ImageRepo, "")).ExpectFailure().MatchErrKeyWords("Error:", `no tag or digest specified`, "oras resolve [flags] {:|@}", "Please specify a reference").Exec() From 7c3e23c13d0ae58c07b7f88993bbc582613c957e Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 06:38:07 +0000 Subject: [PATCH 07/18] changed tag and target Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 18 ++++++++++-------- cmd/oras/internal/option/target_test.go | 24 +++++++++++++----------- cmd/oras/root/tag.go | 15 ++++++++++----- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 6aa49abf3..be30d7690 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -94,7 +94,6 @@ func (opts *Target) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description opts.Remote.ApplyFlagsWithPrefix(fs, prefix, description) } -// use parseOCILayoutReference in the 1st branch // Parse gets target options from user input. func (opts *Target) Parse() error { switch { @@ -103,7 +102,8 @@ func (opts *Target) Parse() error { if len(opts.headerFlags) != 0 { return errors.New("custom header flags cannot be used on an OCI image layout target") } - return nil + _, _, err := opts.parseOCILayoutReference() + return err default: opts.Type = TargetTypeRemote if _, err := registry.ParseReference(opts.RawReference); err != nil { @@ -116,25 +116,27 @@ func (opts *Target) Parse() error { } } -// make this function return invalidReference -// make this a receiver function // parseOCILayoutReference parses the raw in format of [:|@] -func parseOCILayoutReference(raw string) (path string, ref string, err error) { +func (opts *Target) parseOCILayoutReference() (path string, ref string, err error) { + raw := opts.RawReference if idx := strings.LastIndex(raw, "@"); idx != -1 { // `digest` found path = raw[:idx] ref = raw[idx+1:] } else { // find `tag` - // use Errors.Join with invalidReference, only when error != nil + // // use Errors.Join with invalidReference, only when error != nil path, ref, err = fileref.Parse(raw, "") + if err != nil { + err = errors.Join(err, errdef.ErrInvalidReference) + } } return } func (opts *Target) newOCIStore() (*oci.Store, error) { var err error - opts.Path, opts.Reference, err = parseOCILayoutReference(opts.RawReference) + opts.Path, opts.Reference, err = opts.parseOCILayoutReference() if err != nil { return nil, err } @@ -211,7 +213,7 @@ func (opts *Target) NewReadonlyTarget(ctx context.Context, common Common, logger switch opts.Type { case TargetTypeOCILayout: var err error - opts.Path, opts.Reference, err = parseOCILayoutReference(opts.RawReference) + opts.Path, opts.Reference, err = opts.parseOCILayoutReference() if err != nil { return nil, err } diff --git a/cmd/oras/internal/option/target_test.go b/cmd/oras/internal/option/target_test.go index d3cdf217f..5f9a9b808 100644 --- a/cmd/oras/internal/option/target_test.go +++ b/cmd/oras/internal/option/target_test.go @@ -62,27 +62,29 @@ func TestTarget_Parse_remote_err(t *testing.T) { } func Test_parseOCILayoutReference(t *testing.T) { - type args struct { - raw string + opts := Target{ + RawReference: "/test", + IsOCILayout: false, } tests := []struct { name string - args args + raw string want string want1 string wantErr bool }{ - {"Empty input", args{raw: ""}, "", "", true}, - {"Empty path and tag", args{raw: ":"}, "", "", true}, - {"Empty path and digest", args{raw: "@"}, "", "", false}, - {"Empty digest", args{raw: "path@"}, "path", "", false}, - {"Empty tag", args{raw: "path:"}, "path", "", false}, - {"path and digest", args{raw: "path@digest"}, "path", "digest", false}, - {"path and tag", args{raw: "path:tag"}, "path", "tag", false}, + {"Empty input", "", "", "", true}, + {"Empty path and tag", ":", "", "", true}, + {"Empty path and digest", "@", "", "", false}, + {"Empty digest", "path@", "path", "", false}, + {"Empty tag", "path:", "path", "", false}, + {"path and digest", "path@digest", "path", "digest", false}, + {"path and tag", "path:tag", "path", "tag", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1, err := parseOCILayoutReference(tt.args.raw) + opts.RawReference = tt.raw + got, got1, err := opts.parseOCILayoutReference() if (err != nil) != tt.wantErr { t.Errorf("parseOCILayoutReference() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/cmd/oras/root/tag.go b/cmd/oras/root/tag.go index 42576f7ba..7dc923be8 100644 --- a/cmd/oras/root/tag.go +++ b/cmd/oras/root/tag.go @@ -21,7 +21,7 @@ import ( "github.com/spf13/cobra" "oras.land/oras-go/v2" - "oras.land/oras-go/v2/registry" + "oras.land/oras-go/v2/errdef" "oras.land/oras/cmd/oras/internal/argument" "oras.land/oras/cmd/oras/internal/display/status" oerrors "oras.land/oras/cmd/oras/internal/errors" @@ -74,11 +74,16 @@ Example - Tag the manifest 'v1.0.1' to 'v1.0.2' in an OCI image layout folder 'l }, PreRunE: func(cmd *cobra.Command, args []string) error { opts.RawReference = args[0] - if _, err := registry.ParseReference(opts.RawReference); err != nil { - return fmt.Errorf("unable to add tag for '%s': %w", opts.RawReference, err) - } opts.targetRefs = args[1:] - return option.Parse(&opts) + if err := option.Parse(&opts); err != nil { + if inner, ok := err.(*oerrors.Error); ok { + if errors.Is(inner, errdef.ErrInvalidReference) { + inner.Err = fmt.Errorf("unable to add tag for '%s': %w", opts.RawReference, inner.Err) + } + } + return err + } + return nil }, RunE: func(cmd *cobra.Command, args []string) error { return tagManifest(cmd, &opts) From e2b7722feb4d1adb19309c4ca8e4ff3ad06b4ad5 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 06:44:47 +0000 Subject: [PATCH 08/18] fix failed unit test Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/oras/internal/option/target_test.go b/cmd/oras/internal/option/target_test.go index 5f9a9b808..5f19102ac 100644 --- a/cmd/oras/internal/option/target_test.go +++ b/cmd/oras/internal/option/target_test.go @@ -23,15 +23,16 @@ import ( "testing" "github.com/spf13/cobra" + "oras.land/oras-go/v2/errdef" "oras.land/oras-go/v2/registry/remote/errcode" oerrors "oras.land/oras/cmd/oras/internal/errors" ) func TestTarget_Parse_oci(t *testing.T) { opts := Target{IsOCILayout: true} - - if err := opts.Parse(); err != nil { - t.Errorf("Target.Parse() error = %v", err) + err := opts.Parse() + if !errors.Is(err, errdef.ErrInvalidReference) { + t.Errorf("Target.Parse() error = %v, expect %v", err, errdef.ErrInvalidReference) } if opts.Type != TargetTypeOCILayout { t.Errorf("Target.Parse() failed, got %q, want %q", opts.Type, TargetTypeOCILayout) From 668a75c12b7b3b28ffb6b43241a8d401c0d003a0 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 06:52:01 +0000 Subject: [PATCH 09/18] clean up Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 1 - test/e2e/go.mod | 2 +- test/e2e/go.sum | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index be30d7690..c64d2a972 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -125,7 +125,6 @@ func (opts *Target) parseOCILayoutReference() (path string, ref string, err erro ref = raw[idx+1:] } else { // find `tag` - // // use Errors.Join with invalidReference, only when error != nil path, ref, err = fileref.Parse(raw, "") if err != nil { err = errors.Join(err, errdef.ErrInvalidReference) diff --git a/test/e2e/go.mod b/test/e2e/go.mod index fa5083e6e..8119550d7 100644 --- a/test/e2e/go.mod +++ b/test/e2e/go.mod @@ -8,7 +8,7 @@ require ( github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0 gopkg.in/yaml.v2 v2.4.0 - oras.land/oras-go/v2 v2.4.0 + oras.land/oras-go/v2 v2.5.0 ) require ( diff --git a/test/e2e/go.sum b/test/e2e/go.sum index ce312454a..50510c9ab 100644 --- a/test/e2e/go.sum +++ b/test/e2e/go.sum @@ -47,5 +47,5 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -oras.land/oras-go/v2 v2.4.0 h1:i+Wt5oCaMHu99guBD0yuBjdLvX7Lz8ukPbwXdR7uBMs= -oras.land/oras-go/v2 v2.4.0/go.mod h1:osvtg0/ClRq1KkydMAEu/IxFieyjItcsQ4ut4PPF+f8= +oras.land/oras-go/v2 v2.5.0 h1:o8Me9kLY74Vp5uw07QXPiitjsw7qNXi8Twd+19Zf02c= +oras.land/oras-go/v2 v2.5.0/go.mod h1:z4eisnLP530vwIOUOJeBIj0aGI0L1C3d53atvCBqZHg= From c7fe0823e915f8e8af2831e6307bc4469e09d3b8 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 07:18:14 +0000 Subject: [PATCH 10/18] add error wrap for target Signed-off-by: Xiaoxuan Wang increase coverage resolve comment Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 2 +- test/e2e/suite/command/push.go | 2 +- test/e2e/suite/command/tag.go | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index c64d2a972..9abbec8b6 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -108,7 +108,7 @@ func (opts *Target) Parse() error { opts.Type = TargetTypeRemote if _, err := registry.ParseReference(opts.RawReference); err != nil { return &oerrors.Error{ - Err: fmt.Errorf("%q is an invalid reference", opts.RawReference), + Err: fmt.Errorf("%w: %q", err, opts.RawReference), Recommendation: "Please make sure the provided reference is in the form of /[:tag|@digest]", } } diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index 19a89ad39..4f1f5f058 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -51,7 +51,7 @@ var _ = Describe("ORAS beginners:", func() { 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(`Error: invalid reference: invalid registry "": "/oras" is an invalid reference`)) gomega.Expect(err).Should(gbytes.Say(regexp.QuoteMeta("Please make sure the provided reference is in the form of /[:tag|@digest]"))) }) diff --git a/test/e2e/suite/command/tag.go b/test/e2e/suite/command/tag.go index 4532d7655..5702cb7e4 100644 --- a/test/e2e/suite/command/tag.go +++ b/test/e2e/suite/command/tag.go @@ -36,6 +36,10 @@ var _ = Describe("ORAS beginners:", func() { ORAS("tag", RegistryRef(ZOTHost, ImageRepo, InvalidTag), "tagged").ExpectFailure().MatchErrKeyWords(RegistryErrorPrefix).Exec() }) + It("should fail when the given reference is invalid", func() { + ORAS("tag", fmt.Sprintf("%s/%s:%s", ZOTHost, InvalidRepo, "test"), "latest").ExpectFailure().MatchErrKeyWords("Error:", "unable to add tag", "invalid reference").Exec() + }) + It("should fail and show detailed error description if no argument provided", func() { err := ORAS("tag").ExpectFailure().Exec().Err gomega.Expect(err).Should(gbytes.Say("Error")) From 9dd1d999f5996270fb09a05d55c37259b715ee1a Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 09:19:57 +0000 Subject: [PATCH 11/18] resolved comments Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 13 ++----------- test/e2e/suite/command/push.go | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 9abbec8b6..3d3a2cbdf 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -98,11 +98,12 @@ func (opts *Target) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description func (opts *Target) Parse() error { switch { case opts.IsOCILayout: + var err error opts.Type = TargetTypeOCILayout if len(opts.headerFlags) != 0 { return errors.New("custom header flags cannot be used on an OCI image layout target") } - _, _, err := opts.parseOCILayoutReference() + opts.Path, opts.Reference, err = opts.parseOCILayoutReference() return err default: opts.Type = TargetTypeRemote @@ -134,11 +135,6 @@ func (opts *Target) parseOCILayoutReference() (path string, ref string, err erro } func (opts *Target) newOCIStore() (*oci.Store, error) { - var err error - opts.Path, opts.Reference, err = opts.parseOCILayoutReference() - if err != nil { - return nil, err - } return oci.New(opts.Path) } @@ -211,11 +207,6 @@ type ReadOnlyGraphTagFinderTarget interface { func (opts *Target) NewReadonlyTarget(ctx context.Context, common Common, logger logrus.FieldLogger) (ReadOnlyGraphTagFinderTarget, error) { switch opts.Type { case TargetTypeOCILayout: - var err error - opts.Path, opts.Reference, err = opts.parseOCILayoutReference() - if err != nil { - return nil, err - } info, err := os.Stat(opts.Path) if err != nil { if errors.Is(err, fs.ErrNotExist) { diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index 4f1f5f058..e09efe5a9 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -51,7 +51,7 @@ var _ = Describe("ORAS beginners:", func() { 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: invalid reference: invalid registry "": "/oras" is an invalid reference`)) + gomega.Expect(err).Should(gbytes.Say(`Error: invalid reference: invalid registry "": "/oras"`)) gomega.Expect(err).Should(gbytes.Say(regexp.QuoteMeta("Please make sure the provided reference is in the form of /[:tag|@digest]"))) }) From c7c52e4dab23af7e06d520fa0c837262f0c91d1e Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 09:55:27 +0000 Subject: [PATCH 12/18] test attempt Signed-off-by: Xiaoxuan Wang --- test/e2e/suite/command/tag.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/e2e/suite/command/tag.go b/test/e2e/suite/command/tag.go index 5702cb7e4..be1c16733 100644 --- a/test/e2e/suite/command/tag.go +++ b/test/e2e/suite/command/tag.go @@ -112,5 +112,10 @@ var _ = Describe("OCI image layout users:", func() { It("should add multiple tags to an existent manifest when providing tag reference", func() { tagAndValidate(PrepareTempOCI(ImageRepo), multi_arch.Tag, multi_arch.Digest, "tag1-via-tag", "tag1-via-tag", "tag1-via-tag") }) + It("should be able to retag a manifest at the current directory", func() { + root := PrepareTempOCI(ImageRepo) + ORAS("tag", LayoutRef(root, multi_arch.Tag), Flags.Layout, "latest").WithWorkDir(root).Exec() + ORAS("tag", LayoutRef(root, multi_arch.Tag), Flags.Layout, "tag2").WithWorkDir(root).MatchKeyWords("Pushed").Exec() + }) }) }) From ca95a1166ccdbd01f263ccd9ae6161d999dc7a4f Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 10:07:39 +0000 Subject: [PATCH 13/18] retag test Signed-off-by: Xiaoxuan Wang --- test/e2e/suite/command/tag.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/e2e/suite/command/tag.go b/test/e2e/suite/command/tag.go index be1c16733..3fae41fd3 100644 --- a/test/e2e/suite/command/tag.go +++ b/test/e2e/suite/command/tag.go @@ -17,6 +17,7 @@ package command import ( "fmt" + "path/filepath" "regexp" . "github.com/onsi/ginkgo/v2" @@ -114,8 +115,10 @@ var _ = Describe("OCI image layout users:", func() { }) It("should be able to retag a manifest at the current directory", func() { root := PrepareTempOCI(ImageRepo) - ORAS("tag", LayoutRef(root, multi_arch.Tag), Flags.Layout, "latest").WithWorkDir(root).Exec() - ORAS("tag", LayoutRef(root, multi_arch.Tag), Flags.Layout, "tag2").WithWorkDir(root).MatchKeyWords("Pushed").Exec() + dir := filepath.Dir(root) + ref := filepath.Base(root) + ORAS("tag", LayoutRef(ref, multi_arch.Tag), Flags.Layout, "latest").WithWorkDir(dir).MatchKeyWords("Tagging [oci-layout]", "Tagged latest").Exec() + ORAS("tag", LayoutRef(ref, multi_arch.Tag), Flags.Layout, "tag2").WithWorkDir(dir).MatchKeyWords("Tagging [oci-layout]", "Tagged tag2").Exec() }) }) }) From 40e4e75f5164cbfa64d668fbab6692296eb0220c Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Mar 2024 10:14:05 +0000 Subject: [PATCH 14/18] refined Signed-off-by: Xiaoxuan Wang --- test/e2e/suite/command/tag.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/suite/command/tag.go b/test/e2e/suite/command/tag.go index 3fae41fd3..d1536d1ab 100644 --- a/test/e2e/suite/command/tag.go +++ b/test/e2e/suite/command/tag.go @@ -119,6 +119,7 @@ var _ = Describe("OCI image layout users:", func() { ref := filepath.Base(root) ORAS("tag", LayoutRef(ref, multi_arch.Tag), Flags.Layout, "latest").WithWorkDir(dir).MatchKeyWords("Tagging [oci-layout]", "Tagged latest").Exec() ORAS("tag", LayoutRef(ref, multi_arch.Tag), Flags.Layout, "tag2").WithWorkDir(dir).MatchKeyWords("Tagging [oci-layout]", "Tagged tag2").Exec() + ORAS("repo", "tags", Flags.Layout, LayoutRef(ref, multi_arch.Tag)).WithWorkDir(dir).MatchKeyWords(multi_arch.Tag, "latest", "tag2").Exec() }) }) }) From 87d6805fbe4e2a2dc36b6e3323ff5dbb8af0e024 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Tue, 26 Mar 2024 07:31:33 +0000 Subject: [PATCH 15/18] resolved comments Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 15 ++++++++++----- cmd/oras/internal/option/target_test.go | 10 +++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 3d3a2cbdf..2481c815e 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -98,13 +98,11 @@ func (opts *Target) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description func (opts *Target) Parse() error { switch { case opts.IsOCILayout: - var err error opts.Type = TargetTypeOCILayout if len(opts.headerFlags) != 0 { return errors.New("custom header flags cannot be used on an OCI image layout target") } - opts.Path, opts.Reference, err = opts.parseOCILayoutReference() - return err + return opts.parseOCILayoutReference() default: opts.Type = TargetTypeRemote if _, err := registry.ParseReference(opts.RawReference); err != nil { @@ -118,8 +116,12 @@ func (opts *Target) Parse() error { } // parseOCILayoutReference parses the raw in format of [:|@] -func (opts *Target) parseOCILayoutReference() (path string, ref string, err error) { +func (opts *Target) parseOCILayoutReference() error { raw := opts.RawReference + var path string + var ref string + var err error + if idx := strings.LastIndex(raw, "@"); idx != -1 { // `digest` found path = raw[:idx] @@ -129,9 +131,12 @@ func (opts *Target) parseOCILayoutReference() (path string, ref string, err erro path, ref, err = fileref.Parse(raw, "") if err != nil { err = errors.Join(err, errdef.ErrInvalidReference) + return err } } - return + opts.Path = path + opts.Reference = ref + return nil } func (opts *Target) newOCIStore() (*oci.Store, error) { diff --git a/cmd/oras/internal/option/target_test.go b/cmd/oras/internal/option/target_test.go index 5f19102ac..5dc94249d 100644 --- a/cmd/oras/internal/option/target_test.go +++ b/cmd/oras/internal/option/target_test.go @@ -85,16 +85,16 @@ func Test_parseOCILayoutReference(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { opts.RawReference = tt.raw - got, got1, err := opts.parseOCILayoutReference() + err := opts.parseOCILayoutReference() if (err != nil) != tt.wantErr { t.Errorf("parseOCILayoutReference() error = %v, wantErr %v", err, tt.wantErr) return } - if got != tt.want { - t.Errorf("parseOCILayoutReference() got = %v, want %v", got, tt.want) + if opts.Path != tt.want { + t.Errorf("parseOCILayoutReference() got = %v, want %v", opts.Path, tt.want) } - if got1 != tt.want1 { - t.Errorf("parseOCILayoutReference() got1 = %v, want %v", got1, tt.want1) + if opts.Reference != tt.want1 { + t.Errorf("parseOCILayoutReference() got1 = %v, want %v", opts.Reference, tt.want1) } }) } From 03f7ca58d1b3bec95a51c704cbd7f99ce7855b7b Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 28 Mar 2024 05:56:44 +0000 Subject: [PATCH 16/18] resolve comments Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 2481c815e..c91d60f42 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -107,7 +107,7 @@ func (opts *Target) Parse() error { opts.Type = TargetTypeRemote if _, err := registry.ParseReference(opts.RawReference); err != nil { return &oerrors.Error{ - Err: fmt.Errorf("%w: %q", err, opts.RawReference), + Err: fmt.Errorf("%q: %w", opts.RawReference, err), Recommendation: "Please make sure the provided reference is in the form of /[:tag|@digest]", } } @@ -120,7 +120,6 @@ func (opts *Target) parseOCILayoutReference() error { raw := opts.RawReference var path string var ref string - var err error if idx := strings.LastIndex(raw, "@"); idx != -1 { // `digest` found @@ -128,6 +127,7 @@ func (opts *Target) parseOCILayoutReference() error { ref = raw[idx+1:] } else { // find `tag` + var err error path, ref, err = fileref.Parse(raw, "") if err != nil { err = errors.Join(err, errdef.ErrInvalidReference) From 853756c8aeab10e744cf8265821624914be91ad6 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 28 Mar 2024 06:01:58 +0000 Subject: [PATCH 17/18] fix e2e Signed-off-by: Xiaoxuan Wang --- test/e2e/suite/command/push.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index e09efe5a9..741c70e20 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -51,7 +51,7 @@ var _ = Describe("ORAS beginners:", func() { 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: invalid reference: invalid registry "": "/oras"`)) + gomega.Expect(err).Should(gbytes.Say(`Error: "/oras": invalid reference: invalid registry ""`)) gomega.Expect(err).Should(gbytes.Say(regexp.QuoteMeta("Please make sure the provided reference is in the form of /[:tag|@digest]"))) }) From 39155e836bfa73250e73b5ff071661497b6f66dd Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 28 Mar 2024 07:26:09 +0000 Subject: [PATCH 18/18] resolve comments Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/target.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index c91d60f42..db5ac8357 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -120,7 +120,6 @@ func (opts *Target) parseOCILayoutReference() error { raw := opts.RawReference var path string var ref string - if idx := strings.LastIndex(raw, "@"); idx != -1 { // `digest` found path = raw[:idx] @@ -130,8 +129,7 @@ func (opts *Target) parseOCILayoutReference() error { var err error path, ref, err = fileref.Parse(raw, "") if err != nil { - err = errors.Join(err, errdef.ErrInvalidReference) - return err + return errors.Join(err, errdef.ErrInvalidReference) } } opts.Path = path