From 0bdccefe711e0545cc1034f154f0e9da550bd8ff Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 21 Jun 2024 10:33:10 +0000 Subject: [PATCH 01/14] chore(ux): improve error message when attaching without subject artifact Signed-off-by: Billy Zha --- cmd/oras/internal/errors/errors.go | 4 ++-- cmd/oras/internal/option/target.go | 8 +++++--- cmd/oras/root/attach.go | 11 +++++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index 6e48556ef..5b095405d 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -152,10 +152,10 @@ func reWrap(errA, errB, errC error) error { } // NewErrEmptyTagOrDigest creates a new error based on the reference string. -func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, needsTag bool) error { +func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, allowTag bool) error { form := `"@"` errMsg := `no digest specified` - if needsTag { + if allowTag { form = fmt.Sprintf(`":" or %s`, form) errMsg = "no tag or digest specified" } diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index b15613b13..b92b11ff5 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -105,11 +105,13 @@ func (opts *Target) Parse(cmd *cobra.Command) error { return opts.parseOCILayoutReference() default: opts.Type = TargetTypeRemote - if _, err := registry.ParseReference(opts.RawReference); err != nil { + if ref, err := registry.ParseReference(opts.RawReference); err != nil { return &oerrors.Error{ Err: fmt.Errorf("%q: %w", opts.RawReference, err), Recommendation: "Please make sure the provided reference is in the form of /[:tag|@digest]", } + } else { + opts.Reference = ref.Reference } return opts.Remote.Parse(cmd) } @@ -243,9 +245,9 @@ func (opts *Target) NewReadonlyTarget(ctx context.Context, common Common, logger } // EnsureReferenceNotEmpty returns formalized error when the reference is empty. -func (opts *Target) EnsureReferenceNotEmpty(cmd *cobra.Command, needsTag bool) error { +func (opts *Target) EnsureReferenceNotEmpty(cmd *cobra.Command, allowTag bool) error { if opts.Reference == "" { - return oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, needsTag) + return oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, allowTag) } return nil } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index c7d447927..cac81d344 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -90,7 +90,13 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder if err := option.Parse(cmd, &opts); err != nil { return err } - return nil + if opts.Reference == "" && len(opts.FileRefs) == 0 && len(opts.ManifestAnnotations) == 0 { + return &oerrors.Error{ + Err: fmt.Errorf(`"%s" is an invalid artifact reference`, opts.RawReference), + Recommendation: fmt.Sprintf("Did you forget to specify a subject artifact?"), + } + } + return opts.EnsureReferenceNotEmpty(cmd, true) }, RunE: func(cmd *cobra.Command, args []string) error { return runAttach(cmd, &opts) @@ -137,9 +143,6 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { if err != nil { return err } - if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil { - return err - } // add both pull and push scope hints for dst repository // to save potential push-scope token requests during copy ctx = registryutil.WithScopeHint(ctx, dst, auth.ActionPull, auth.ActionPush) From 54d5edf6880ef388c6b72c571ba01453c3b4b7d7 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 21 Jun 2024 10:44:14 +0000 Subject: [PATCH 02/14] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/errors/errors.go | 15 +++++++++++---- cmd/oras/root/attach.go | 5 +++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index 5b095405d..7accfdcae 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -151,14 +151,21 @@ func reWrap(errA, errB, errC error) error { return errC } -// NewErrEmptyTagOrDigest creates a new error based on the reference string. -func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, allowTag bool) error { - form := `"@"` - errMsg := `no digest specified` +// InvalidTagOrDigestMessage returns the form and message for invalid tag or +// digest. +func InvalidTagOrDigestMessage(allowTag bool) (form, errMsg string) { + form = `"@"` + errMsg = `no digest specified` if allowTag { form = fmt.Sprintf(`":" or %s`, form) errMsg = "no tag or digest specified" } + return +} + +// NewErrEmptyTagOrDigest creates a new error based on the reference string. +func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, allowTag bool) error { + form, errMsg := InvalidTagOrDigestMessage(allowTag) return &Error{ Err: fmt.Errorf(`"%s": %s`, ref, errMsg), Usage: fmt.Sprintf("%s %s", cmd.Parent().CommandPath(), cmd.Use), diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index cac81d344..7dbb993c8 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -91,9 +91,10 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder return err } if opts.Reference == "" && len(opts.FileRefs) == 0 && len(opts.ManifestAnnotations) == 0 { + form, errMsg := oerrors.InvalidTagOrDigestMessage(true) return &oerrors.Error{ - Err: fmt.Errorf(`"%s" is an invalid artifact reference`, opts.RawReference), - Recommendation: fmt.Sprintf("Did you forget to specify a subject artifact?"), + Err: fmt.Errorf(`"%s": %s`, opts.RawReference, errMsg), + Recommendation: fmt.Sprintf("Did you forget to specify a subject artifact? Please specify a reference in the form of %s", form), } } return opts.EnsureReferenceNotEmpty(cmd, true) From e7ec3eee528ecc5218d6f23ff26fa974da4a0f00 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 21 Jun 2024 14:09:14 +0000 Subject: [PATCH 03/14] reword Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index f2df3d309..22a4844cb 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -94,8 +94,8 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder if opts.Reference == "" && len(opts.FileRefs) == 0 && len(opts.ManifestAnnotations) == 0 { form, errMsg := oerrors.InvalidTagOrDigestMessage(true) return &oerrors.Error{ - Err: fmt.Errorf(`"%s": %s`, opts.RawReference, errMsg), - Recommendation: fmt.Sprintf("Did you forget to specify a subject artifact? Please specify a reference in the form of %s", form), + Err: fmt.Errorf(`invalid subject artifact "%s": %s`, opts.RawReference, errMsg), + Recommendation: fmt.Sprintf("Did you forget to provide the subject artifact? Please specify a reference in the form of %s", form), } } return opts.EnsureReferenceNotEmpty(cmd, true) From 9a56b57d1e7315e8c503002d98c11be52f6e0a0f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Sat, 22 Jun 2024 06:55:23 +0000 Subject: [PATCH 04/14] refactor: append subject artifact missing recommendation for Signed-off-by: Billy Zha --- cmd/oras/internal/errors/errors.go | 11 +++++++++++ cmd/oras/internal/option/target.go | 1 + cmd/oras/root/attach.go | 18 ++++++++++-------- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index 7accfdcae..0bb329b79 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -26,6 +26,15 @@ import ( "oras.land/oras-go/v2/registry/remote/errcode" ) +// OperationType reperesnts the type of operation. +type OperationType string + +const ( + // ParseArtifactReference is the operation type parsing artifact + // reference. + ParseArtifactReference = OperationType("") +) + // RegistryErrorPrefix is the commandline prefix for errors from registry. const RegistryErrorPrefix = "Error response from registry:" @@ -39,6 +48,7 @@ func (e UnsupportedFormatTypeError) Error() string { // Error is the error type for CLI error messaging. type Error struct { + OperationType Err error Usage string Recommendation string @@ -167,6 +177,7 @@ func InvalidTagOrDigestMessage(allowTag bool) (form, errMsg string) { func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, allowTag bool) error { form, errMsg := InvalidTagOrDigestMessage(allowTag) return &Error{ + OperationType: ParseArtifactReference, 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()), diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index b92b11ff5..16e1a0526 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -107,6 +107,7 @@ func (opts *Target) Parse(cmd *cobra.Command) error { opts.Type = TargetTypeRemote if ref, err := registry.ParseReference(opts.RawReference); err != nil { return &oerrors.Error{ + OperationType: oerrors.ParseArtifactReference, Err: fmt.Errorf("%q: %w", opts.RawReference, err), Recommendation: "Please make sure the provided reference is in the form of /[:tag|@digest]", } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 22a4844cb..02a12de40 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -88,17 +88,19 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder PreRunE: func(cmd *cobra.Command, args []string) error { opts.RawReference = args[0] opts.FileRefs = args[1:] - if err := option.Parse(cmd, &opts); err != nil { - return err + err := option.Parse(cmd, &opts) + if err == nil && opts.Reference == "" { + // ensure reference is not empty + err = oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, true) } - if opts.Reference == "" && len(opts.FileRefs) == 0 && len(opts.ManifestAnnotations) == 0 { - form, errMsg := oerrors.InvalidTagOrDigestMessage(true) - return &oerrors.Error{ - Err: fmt.Errorf(`invalid subject artifact "%s": %s`, opts.RawReference, errMsg), - Recommendation: fmt.Sprintf("Did you forget to provide the subject artifact? Please specify a reference in the form of %s", form), + if len(opts.FileRefs) == 0 && len(opts.ManifestAnnotations) == 0 { + // no file or annotation provided, validate reference + if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.ParseArtifactReference { + // add advice on missing artifact reference + err.Recommendation = fmt.Sprintf("Did you forget to provide the subject artifact? %s", err.Recommendation) } } - return opts.EnsureReferenceNotEmpty(cmd, true) + return err }, RunE: func(cmd *cobra.Command, args []string) error { return runAttach(cmd, &opts) From ce71ca371b36cd4c2b92aa002de6b5e75f9c80de Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 24 Jun 2024 00:25:49 +0000 Subject: [PATCH 05/14] rephrase prompt for missing subject artifact reference Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 02a12de40..2c15b4ceb 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -93,11 +93,11 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder // ensure reference is not empty err = oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, true) } - if len(opts.FileRefs) == 0 && len(opts.ManifestAnnotations) == 0 { - // no file or annotation provided, validate reference + if len(opts.FileRefs) == 0 { + // no file argument provided if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.ParseArtifactReference { - // add advice on missing artifact reference - err.Recommendation = fmt.Sprintf("Did you forget to provide the subject artifact? %s", err.Recommendation) + // invalid reference + err.Recommendation = fmt.Sprintf("Have you specified an artifact reference to attach to? %s", err.Recommendation) } } return err From 0c1c0342ca6b7275af9530fdc8cee5ede7f116c6 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 24 Jun 2024 00:54:45 +0000 Subject: [PATCH 06/14] test(e2e): add tests Signed-off-by: Billy Zha --- test/e2e/suite/command/attach.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go index 026d11575..98c1b7d33 100644 --- a/test/e2e/suite/command/attach.go +++ b/test/e2e/suite/command/attach.go @@ -69,12 +69,25 @@ var _ = Describe("ORAS beginners:", func() { ExpectFailure().MatchErrKeyWords("unknown distribution specification flag").Exec() }) + It("should fail with error suggesting subject missed", func() { + err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, "")).ExpectFailure().Exec().Err + Expect(err).Should(gbytes.Say("Error")) + Expect(err).Should(gbytes.Say("\nHave you specified an artifact reference to attach to?")) + }) + + It("should fail with error suggesting right form", func() { + err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, ""), "test.json").ExpectFailure().Exec().Err + Expect(err).Should(gbytes.Say("Error")) + Expect(err).Should(gbytes.Say("no tag or digest specified")) + Expect(err).ShouldNot(gbytes.Say("\nHave you specified an artifact reference to attach to?")) + }) + It("should fail and show detailed error description if no argument provided", func() { err := ORAS("attach").ExpectFailure().Exec().Err - gomega.Expect(err).Should(gbytes.Say("Error")) - gomega.Expect(err).Should(gbytes.Say("\nUsage: oras attach")) - gomega.Expect(err).Should(gbytes.Say("\n")) - gomega.Expect(err).Should(gbytes.Say(`Run "oras attach -h"`)) + Expect(err).Should(gbytes.Say("Error")) + Expect(err).Should(gbytes.Say("\nUsage: oras attach")) + Expect(err).Should(gbytes.Say("\n")) + Expect(err).Should(gbytes.Say(`Run "oras attach -h"`)) }) It("should fail if distribution spec is not valid", func() { From 117081872c3a3666f742d53685f076e9d05d9164 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 24 Jun 2024 01:06:06 +0000 Subject: [PATCH 07/14] increase coverage Signed-off-by: Billy Zha --- test/e2e/suite/command/attach.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go index 98c1b7d33..395c0fed7 100644 --- a/test/e2e/suite/command/attach.go +++ b/test/e2e/suite/command/attach.go @@ -76,7 +76,7 @@ var _ = Describe("ORAS beginners:", func() { }) It("should fail with error suggesting right form", func() { - err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, ""), "test.json").ExpectFailure().Exec().Err + err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, ""), "./test.json").ExpectFailure().Exec().Err Expect(err).Should(gbytes.Say("Error")) Expect(err).Should(gbytes.Say("no tag or digest specified")) Expect(err).ShouldNot(gbytes.Say("\nHave you specified an artifact reference to attach to?")) From b00fb004d45a53903f9a8f216ef18e3cf46de7cd Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 25 Jun 2024 14:17:07 +0000 Subject: [PATCH 08/14] use iota instead Signed-off-by: Billy Zha --- cmd/oras/internal/errors/errors.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index 0bb329b79..a592ecf6d 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -26,13 +26,12 @@ import ( "oras.land/oras-go/v2/registry/remote/errcode" ) -// OperationType reperesnts the type of operation. -type OperationType string +type OperationType int const ( // ParseArtifactReference is the operation type parsing artifact // reference. - ParseArtifactReference = OperationType("") + ParseArtifactReference = OperationType(iota) ) // RegistryErrorPrefix is the commandline prefix for errors from registry. From 7ef6de4c61f93cdccb2c4e3cf5448ccabe6c1274 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 28 Jun 2024 08:14:09 +0000 Subject: [PATCH 09/14] resolve comments Signed-off-by: Billy Zha --- cmd/oras/internal/errors/errors.go | 28 +++++++++++----------------- cmd/oras/internal/option/target.go | 2 +- cmd/oras/root/attach.go | 2 +- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index a592ecf6d..a19d2459b 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -26,12 +26,13 @@ import ( "oras.land/oras-go/v2/registry/remote/errcode" ) +// OperationType stands for certain type of operations. type OperationType int const ( - // ParseArtifactReference is the operation type parsing artifact - // reference. - ParseArtifactReference = OperationType(iota) + // OperationTypeParseArtifactReference represents parsing artifact + // reference operation. + OperationTypeParseArtifactReference OperationType = iota + 1 ) // RegistryErrorPrefix is the commandline prefix for errors from registry. @@ -47,7 +48,7 @@ func (e UnsupportedFormatTypeError) Error() string { // Error is the error type for CLI error messaging. type Error struct { - OperationType + OperationType OperationType Err error Usage string Recommendation string @@ -160,23 +161,16 @@ func reWrap(errA, errB, errC error) error { return errC } -// InvalidTagOrDigestMessage returns the form and message for invalid tag or -// digest. -func InvalidTagOrDigestMessage(allowTag bool) (form, errMsg string) { - form = `"@"` - errMsg = `no digest specified` - if allowTag { +// NewErrEmptyTagOrDigest creates a new error based on the reference string. +func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, needsTag bool) error { + form := `"@"` + errMsg := `no digest specified` + if needsTag { form = fmt.Sprintf(`":" or %s`, form) errMsg = "no tag or digest specified" } - return -} - -// NewErrEmptyTagOrDigest creates a new error based on the reference string. -func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, allowTag bool) error { - form, errMsg := InvalidTagOrDigestMessage(allowTag) return &Error{ - OperationType: ParseArtifactReference, + OperationType: OperationTypeParseArtifactReference, 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()), diff --git a/cmd/oras/internal/option/target.go b/cmd/oras/internal/option/target.go index 16e1a0526..b81c52b26 100644 --- a/cmd/oras/internal/option/target.go +++ b/cmd/oras/internal/option/target.go @@ -107,7 +107,7 @@ func (opts *Target) Parse(cmd *cobra.Command) error { opts.Type = TargetTypeRemote if ref, err := registry.ParseReference(opts.RawReference); err != nil { return &oerrors.Error{ - OperationType: oerrors.ParseArtifactReference, + OperationType: oerrors.OperationTypeParseArtifactReference, Err: fmt.Errorf("%q: %w", opts.RawReference, err), Recommendation: "Please make sure the provided reference is in the form of /[:tag|@digest]", } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 734840bf5..699565604 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -94,7 +94,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder } if len(opts.FileRefs) == 0 { // no file argument provided - if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.ParseArtifactReference { + if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.OperationTypeParseArtifactReference { // invalid reference err.Recommendation = fmt.Sprintf("Have you specified an artifact reference to attach to? %s", err.Recommendation) } From e2c55fec1bb2edb8dd3b2982d1329bc663025b6b Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 28 Jun 2024 08:23:27 +0000 Subject: [PATCH 10/14] resolve comments Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 699565604..87bf7a775 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -88,7 +88,10 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder opts.RawReference = args[0] opts.FileRefs = args[1:] err := option.Parse(cmd, &opts) - if err == nil && opts.Reference == "" { + if err == nil { + if opts.Reference != "" { + return nil + } // ensure reference is not empty err = oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, true) } From ac1db754f355856bf9c157e51d0a28995d58a396 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 28 Jun 2024 08:34:10 +0000 Subject: [PATCH 11/14] resolve comments Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 87bf7a775..b6658adc4 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -99,7 +99,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder // no file argument provided if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.OperationTypeParseArtifactReference { // invalid reference - err.Recommendation = fmt.Sprintf("Have you specified an artifact reference to attach to? %s", err.Recommendation) + err.Recommendation = fmt.Sprintf("Are you missing an artifact reference to attach to? %s", err.Recommendation) } } return err From 3f7d52c480ad31ee9d16400ad08b0504cbb47308 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 28 Jun 2024 08:38:47 +0000 Subject: [PATCH 12/14] fix e2e Signed-off-by: Billy Zha --- test/e2e/suite/command/attach.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go index 395c0fed7..f0c6ada5b 100644 --- a/test/e2e/suite/command/attach.go +++ b/test/e2e/suite/command/attach.go @@ -72,14 +72,14 @@ var _ = Describe("ORAS beginners:", func() { It("should fail with error suggesting subject missed", func() { err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, "")).ExpectFailure().Exec().Err Expect(err).Should(gbytes.Say("Error")) - Expect(err).Should(gbytes.Say("\nHave you specified an artifact reference to attach to?")) + Expect(err).Should(gbytes.Say("\nAre you missing an artifact reference to attach to?")) }) It("should fail with error suggesting right form", func() { err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, ""), "./test.json").ExpectFailure().Exec().Err Expect(err).Should(gbytes.Say("Error")) Expect(err).Should(gbytes.Say("no tag or digest specified")) - Expect(err).ShouldNot(gbytes.Say("\nHave you specified an artifact reference to attach to?")) + Expect(err).ShouldNot(gbytes.Say("\nAre you missing an artifact reference to attach to?")) }) It("should fail and show detailed error description if no argument provided", func() { From e3e52129829d58cc936ad7b2f5901eff3faf9c02 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 28 Jun 2024 10:34:13 +0000 Subject: [PATCH 13/14] remove unnecessary commment Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index b6658adc4..9e1f19087 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -92,7 +92,6 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder if opts.Reference != "" { return nil } - // ensure reference is not empty err = oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, true) } if len(opts.FileRefs) == 0 { From de8cccac2038f0502231050307b797105592ed4c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 4 Jul 2024 09:26:05 +0000 Subject: [PATCH 14/14] add usage hint Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 9e1f19087..88f829779 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -49,7 +49,7 @@ type attachOptions struct { func attachCmd() *cobra.Command { var opts attachOptions cmd := &cobra.Command{ - Use: "attach [flags] --artifact-type= {:|@} [:] [...]", + Use: "attach [flags] --artifact-type= {:|@} {[:]|--annotation =} [...]", Short: "[Preview] Attach files to an existing artifact", Long: `[Preview] Attach files to an existing artifact @@ -89,10 +89,9 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder opts.FileRefs = args[1:] err := option.Parse(cmd, &opts) if err == nil { - if opts.Reference != "" { + if err = opts.EnsureReferenceNotEmpty(cmd, true); err == nil { return nil } - err = oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, true) } if len(opts.FileRefs) == 0 { // no file argument provided