From d956bcaa911991d250c5c81f97f44950d6757c16 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 7 May 2024 03:31:43 +0000 Subject: [PATCH 01/32] feat: redefine go template experience Signed-off-by: Billy Zha --- cmd/oras/internal/display/handler.go | 77 +++++++++++-------- cmd/oras/internal/option/format.go | 107 ++++++++++++++++++++++++--- cmd/oras/root/attach.go | 5 +- cmd/oras/root/discover.go | 16 ++-- cmd/oras/root/manifest/fetch.go | 12 ++- cmd/oras/root/pull.go | 5 +- cmd/oras/root/push.go | 5 +- 7 files changed, 173 insertions(+), 54 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index 7aeef4dc6..5ef891337 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -30,97 +30,108 @@ import ( "oras.land/oras/cmd/oras/internal/display/metadata/text" "oras.land/oras/cmd/oras/internal/display/metadata/tree" "oras.land/oras/cmd/oras/internal/display/status" + "oras.land/oras/cmd/oras/internal/option" ) // NewPushHandler returns status and metadata handlers for push command. -func NewPushHandler(format string, tty *os.File, out io.Writer, verbose bool) (status.PushHandler, metadata.PushHandler) { +func NewPushHandler(format option.Format, tty *os.File, out io.Writer, verbose bool) (status.PushHandler, metadata.PushHandler, error) { var statusHandler status.PushHandler if tty != nil { statusHandler = status.NewTTYPushHandler(tty) - } else if format == "" { + } else if format.Type == "" { statusHandler = status.NewTextPushHandler(out, verbose) } else { statusHandler = status.NewDiscardHandler() } var metadataHandler metadata.PushHandler - switch format { + switch format.Type { case "": metadataHandler = text.NewPushHandler(out) - case "json": + case option.TypeJSON: metadataHandler = json.NewPushHandler(out) + case option.TypeGoTemplate: + metadataHandler = template.NewPushHandler(out, format.Template) default: - metadataHandler = template.NewPushHandler(out, format) + return nil, nil, format.TypeError() } - return statusHandler, metadataHandler + return statusHandler, metadataHandler, nil } // NewAttachHandler returns status and metadata handlers for attach command. -func NewAttachHandler(format string, tty *os.File, out io.Writer, verbose bool) (status.AttachHandler, metadata.AttachHandler) { +func NewAttachHandler(format option.Format, tty *os.File, out io.Writer, verbose bool) (status.AttachHandler, metadata.AttachHandler, error) { var statusHandler status.AttachHandler if tty != nil { statusHandler = status.NewTTYAttachHandler(tty) - } else if format == "" { + } else if format.Type == "" { statusHandler = status.NewTextAttachHandler(out, verbose) } else { statusHandler = status.NewDiscardHandler() } var metadataHandler metadata.AttachHandler - switch format { + switch format.Type { case "": metadataHandler = text.NewAttachHandler(out) case "json": metadataHandler = json.NewAttachHandler(out) + case "go-template": + metadataHandler = template.NewAttachHandler(out, format.Template) default: - metadataHandler = template.NewAttachHandler(out, format) + return nil, nil, format.TypeError() } - return statusHandler, metadataHandler + return statusHandler, metadataHandler, nil } // NewPullHandler returns status and metadata handlers for pull command. -func NewPullHandler(format string, path string, tty *os.File, out io.Writer, verbose bool) (status.PullHandler, metadata.PullHandler) { +func NewPullHandler(format option.Format, path string, tty *os.File, out io.Writer, verbose bool) (status.PullHandler, metadata.PullHandler, error) { var statusHandler status.PullHandler if tty != nil { statusHandler = status.NewTTYPullHandler(tty) - } else if format == "" { + } else if format.Type == "" { statusHandler = status.NewTextPullHandler(out, verbose) } else { statusHandler = status.NewDiscardHandler() } var metadataHandler metadata.PullHandler - switch format { + switch format.Type { case "": metadataHandler = text.NewPullHandler(out) - case "json": + case option.TypeJSON: metadataHandler = json.NewPullHandler(out, path) + case option.TypeGoTemplate: + metadataHandler = template.NewPullHandler(out, path, format.Template) default: - metadataHandler = template.NewPullHandler(out, path, format) + return nil, nil, format.TypeError() } - return statusHandler, metadataHandler + return statusHandler, metadataHandler, nil } // NewDiscoverHandler returns status and metadata handlers for discover command. -func NewDiscoverHandler(out io.Writer, outputType string, path string, rawReference string, desc ocispec.Descriptor, verbose bool) metadata.DiscoverHandler { - switch outputType { - case "tree", "": - return tree.NewDiscoverHandler(out, path, desc, verbose) - case "table": - return table.NewDiscoverHandler(out, rawReference, desc, verbose) - case "json": - return json.NewDiscoverHandler(out, desc, path) +func NewDiscoverHandler(out io.Writer, format option.Format, path string, rawReference string, desc ocispec.Descriptor, verbose bool) (metadata.DiscoverHandler, error) { + var handler metadata.DiscoverHandler + switch format.Type { + case option.TypeTree, "": + handler = tree.NewDiscoverHandler(out, path, desc, verbose) + case option.TypeTable: + handler = table.NewDiscoverHandler(out, rawReference, desc, verbose) + case option.TypeJSON: + handler = json.NewDiscoverHandler(out, desc, path) + case option.TypeGoTemplate: + handler = template.NewDiscoverHandler(out, desc, path, format.Template) default: - return template.NewDiscoverHandler(out, desc, path, outputType) + return nil, format.TypeError() } + return handler, nil } // NewManifestFetchHandler returns a manifest fetch handler. -func NewManifestFetchHandler(out io.Writer, format string, outputDescriptor, pretty bool, outputPath string) (metadata.ManifestFetchHandler, content.ManifestFetchHandler) { +func NewManifestFetchHandler(out io.Writer, format option.Format, outputDescriptor, pretty bool, outputPath string) (metadata.ManifestFetchHandler, content.ManifestFetchHandler, error) { var metadataHandler metadata.ManifestFetchHandler var contentHandler content.ManifestFetchHandler - switch format { + switch format.Type { case "": // raw if outputDescriptor { @@ -128,22 +139,24 @@ func NewManifestFetchHandler(out io.Writer, format string, outputDescriptor, pre } else { metadataHandler = metadata.NewDiscardHandler() } - case "json": + case option.TypeJSON: // json metadataHandler = json.NewManifestFetchHandler(out) if outputPath == "" { contentHandler = content.NewDiscardHandler() } - default: + case option.TypeGoTemplate: // go template - metadataHandler = template.NewManifestFetchHandler(out, format) + metadataHandler = template.NewManifestFetchHandler(out, format.Template) if outputPath == "" { contentHandler = content.NewDiscardHandler() } + default: + return nil, nil, format.TypeError() } if contentHandler == nil { contentHandler = content.NewManifestFetchHandler(out, pretty, outputPath) } - return metadataHandler, contentHandler + return metadataHandler, contentHandler, nil } diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 07e9489b9..b60df2457 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -15,21 +15,110 @@ limitations under the License. package option -import "github.com/spf13/pflag" +import ( + "fmt" + "strings" + + "github.com/spf13/pflag" + oerrors "oras.land/oras/cmd/oras/internal/errors" +) + +const ( + // format types + TypeJSON = "json" + TypeTree = "tree" + TypeTable = "table" + TypeGoTemplate = "go-template" +) + +type FormatOption struct { + Name string + Usage string +} // Format is a flag to format metadata into output. type Format struct { + Type string Template string + input string + options []FormatOption } // ApplyFlag implements FlagProvider.ApplyFlag. func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { - const name = "format" - if fs.Lookup(name) != nil { - // allow command to overwrite the flag - return - } - fs.StringVar(&opts.Template, name, "", `[Experimental] Format output using a custom template: -'json': Print in JSON format -'$TEMPLATE': Print output using the given Go template.`) + usage := "[Experimental] Format output using a custom template:" + if len(opts.options) == 0 { + opts.options = []FormatOption{ + {Name: TypeJSON, Usage: "Print in JSON format"}, + {Name: TypeGoTemplate, Usage: "Print output using the given Go template"}, + } + } + + // generate usage string + maxLength := 0 + for _, option := range opts.options { + if len(option.Name) > maxLength { + maxLength = len(option.Name) + } + } + for _, option := range opts.options { + usage += fmt.Sprintf("\n'%s':%s%s", option.Name, strings.Repeat(" ", maxLength-len(option.Name)+2), option.Usage) + } + usage += "." + + // apply flags + fs.StringVar(&opts.input, "format", "", usage) + fs.StringVar(&opts.Template, "template", "", `Template string used to format output`) +} + +// Parse parses the input format flag. +func (opts *Format) Parse() error { + if err := opts.parseFlag(); err != nil { + return err + } + + if opts.Template != "" && opts.Type != TypeGoTemplate { + return fmt.Errorf("--template must be used with --format %s", TypeGoTemplate) + } + + var optionalTypes []string + for _, option := range opts.options { + if opts.Type == option.Name { + return nil + } + optionalTypes = append(optionalTypes, option.Name) + } + return &oerrors.Error{ + Err: fmt.Errorf("invalid format type: %s", opts.Type), + Recommendation: fmt.Sprintf("supported types: %s", strings.Join(optionalTypes, ", ")), + } +} + +func (opts *Format) parseFlag() error { + if opts.Template != "" { + // template explicitly set + opts.Type = opts.input + return nil + } + index := strings.Index(opts.input, "=") + if index == -1 { + // no proper template found in the type flag + opts.Type = opts.input + return nil + } else if index == len(opts.Type)-1 || index == 0 { + return fmt.Errorf("invalid format flag: %s", opts.input) + } + opts.Type = opts.Type[:index] + opts.Template = opts.Type[index+1:] + return nil +} + +// SetFormatOptions sets the format options. +func (opts *Format) SetFormatOptions(options []FormatOption) { + opts.options = options +} + +// FormatError generate the error message for an invalid type. +func (opts *Format) TypeError() error { + return fmt.Errorf("unsupported format type: %s", opts.Type) } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 7b9662ce4..7d40175f5 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -119,7 +119,10 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { Recommendation: `To attach to an existing artifact, please provide files via argument or annotations via flag "--annotation". Run "oras attach -h" for more options and examples`, } } - displayStatus, displayMetadata := display.NewAttachHandler(opts.Template, opts.TTY, cmd.OutOrStdout(), opts.Verbose) + displayStatus, displayMetadata, err := display.NewAttachHandler(opts.Format, opts.TTY, cmd.OutOrStdout(), opts.Verbose) + if err != nil { + return err + } // prepare manifest store, err := file.New("") diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index b3e7b695a..1f0972421 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -96,11 +96,12 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") cmd.Flags().StringVarP(&opts.Template, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") - cmd.Flags().StringVarP(&opts.Template, "format", "", "tree", `[Experimental] Format output using a custom template: -'tree': Get referrers recursively and print in tree format (default) -'table': Get direct referrers and output in table format -'json': Get direct referrers and output in JSON format -'$TEMPLATE': Print direct referrers using the given Go template.`) + opts.SetFormatOptions([]option.FormatOption{ + {Name: option.TypeTree, Usage: "Get referrers recursively and print in tree format (default)"}, + {Name: option.TypeTable, Usage: "Get direct referrers and output in table format"}, + {Name: option.TypeJSON, Usage: "Get direct referrers and output in JSON format"}, + {Name: option.TypeGoTemplate, Usage: "Print direct referrers using the given Go template"}, + }) opts.EnableDistributionSpecFlag() option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) @@ -124,7 +125,10 @@ func runDiscover(cmd *cobra.Command, opts *discoverOptions) error { return err } - handler := display.NewDiscoverHandler(cmd.OutOrStdout(), opts.Template, opts.Path, opts.RawReference, desc, opts.Verbose) + handler, err := display.NewDiscoverHandler(cmd.OutOrStdout(), opts.Format, opts.Path, opts.RawReference, desc, opts.Verbose) + if err != nil { + return err + } if handler.MultiLevelSupported() { if err := fetchAllReferrers(ctx, repo, desc, opts.artifactType, handler); err != nil { return err diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index 499c14403..9a9376d60 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -98,9 +98,10 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': cmd.Flags().StringSliceVarP(&opts.mediaTypes, "media-type", "", nil, "accepted media types") cmd.Flags().StringVarP(&opts.outputPath, "output", "o", "", "file `path` to write the fetched manifest to, use - for stdout") - cmd.Flags().StringVar(&opts.Template, "format", "", `[Experimental] Format metadata using a custom template: -'json': Print in prettified JSON format -'$TEMPLATE': Print using the given Go template.`) + opts.SetFormatOptions([]option.FormatOption{ + {Name: option.TypeJSON, Usage: "Print in prettified JSON format"}, + {Name: option.TypeGoTemplate, Usage: "Print using the given Go template"}, + }) option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } @@ -125,7 +126,10 @@ func fetchManifest(cmd *cobra.Command, opts *fetchOptions) (fetchErr error) { if err != nil { return err } - metadataHandler, contentHandler := display.NewManifestFetchHandler(cmd.OutOrStdout(), opts.Template, opts.OutputDescriptor, opts.Pretty.Pretty, opts.outputPath) + metadataHandler, contentHandler, err := display.NewManifestFetchHandler(cmd.OutOrStdout(), opts.Format, opts.OutputDescriptor, opts.Pretty.Pretty, opts.outputPath) + if err != nil { + return err + } var desc ocispec.Descriptor var content []byte diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 9dc62cbb1..4eee51ee8 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -135,7 +135,10 @@ func runPull(cmd *cobra.Command, opts *pullOptions) error { dst.AllowPathTraversalOnWrite = opts.PathTraversal dst.DisableOverwrite = opts.KeepOldFiles - statusHandler, metadataHandler := display.NewPullHandler(opts.Template, opts.Path, opts.TTY, cmd.OutOrStdout(), opts.Verbose) + statusHandler, metadataHandler, err := display.NewPullHandler(opts.Format, opts.Path, opts.TTY, cmd.OutOrStdout(), opts.Verbose) + if err != nil { + return err + } desc, err := doPull(ctx, src, dst, copyOptions, metadataHandler, statusHandler, opts) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index cab1b3c74..e90a32fcd 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -155,7 +155,10 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { if err != nil { return err } - displayStatus, displayMetadata := display.NewPushHandler(opts.Template, opts.TTY, cmd.OutOrStdout(), opts.Verbose) + displayStatus, displayMetadata, err := display.NewPushHandler(opts.Format, opts.TTY, cmd.OutOrStdout(), opts.Verbose) + if err != nil { + return err + } // prepare pack packOpts := oras.PackManifestOptions{ From bb9e9507f41057a2ca4405fee00170da7935191b Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 7 May 2024 15:15:50 +0000 Subject: [PATCH 02/32] add default value Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 27 ++++++++++++++++----------- cmd/oras/root/discover.go | 4 ++-- cmd/oras/root/manifest/fetch.go | 2 +- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index b60df2457..39d880896 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -31,7 +31,7 @@ const ( TypeGoTemplate = "go-template" ) -type FormatOption struct { +type FormatType struct { Name string Usage string } @@ -41,14 +41,14 @@ type Format struct { Type string Template string input string - options []FormatOption + types []FormatType } // ApplyFlag implements FlagProvider.ApplyFlag. func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { usage := "[Experimental] Format output using a custom template:" - if len(opts.options) == 0 { - opts.options = []FormatOption{ + if len(opts.types) == 0 { + opts.types = []FormatType{ {Name: TypeJSON, Usage: "Print in JSON format"}, {Name: TypeGoTemplate, Usage: "Print output using the given Go template"}, } @@ -56,15 +56,14 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { // generate usage string maxLength := 0 - for _, option := range opts.options { + for _, option := range opts.types { if len(option.Name) > maxLength { maxLength = len(option.Name) } } - for _, option := range opts.options { + for _, option := range opts.types { usage += fmt.Sprintf("\n'%s':%s%s", option.Name, strings.Repeat(" ", maxLength-len(option.Name)+2), option.Usage) } - usage += "." // apply flags fs.StringVar(&opts.input, "format", "", usage) @@ -82,7 +81,7 @@ func (opts *Format) Parse() error { } var optionalTypes []string - for _, option := range opts.options { + for _, option := range opts.types { if opts.Type == option.Name { return nil } @@ -113,9 +112,15 @@ func (opts *Format) parseFlag() error { return nil } -// SetFormatOptions sets the format options. -func (opts *Format) SetFormatOptions(options []FormatOption) { - opts.options = options +// SetTypes resets the format options and default value. +func (opts *Format) SetTypes(types []FormatType) { + opts.types = types +} + +// SetTypesAndDefault resets the format options and default value. +func (opts *Format) SetTypesAndDefault(defaultType string, types []FormatType) { + opts.Type = defaultType + opts.types = types } // FormatError generate the error message for an invalid type. diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index 1f0972421..b2b2510a9 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -95,8 +95,8 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout } cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") - cmd.Flags().StringVarP(&opts.Template, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") - opts.SetFormatOptions([]option.FormatOption{ + cmd.Flags().StringVarP(&opts.Template, "output", "o", "", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") + opts.SetTypesAndDefault(option.TypeTree, []option.FormatType{ {Name: option.TypeTree, Usage: "Get referrers recursively and print in tree format (default)"}, {Name: option.TypeTable, Usage: "Get direct referrers and output in table format"}, {Name: option.TypeJSON, Usage: "Get direct referrers and output in JSON format"}, diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index 9a9376d60..591a0c48d 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -98,7 +98,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': cmd.Flags().StringSliceVarP(&opts.mediaTypes, "media-type", "", nil, "accepted media types") cmd.Flags().StringVarP(&opts.outputPath, "output", "o", "", "file `path` to write the fetched manifest to, use - for stdout") - opts.SetFormatOptions([]option.FormatOption{ + opts.SetTypes([]option.FormatType{ {Name: option.TypeJSON, Usage: "Print in prettified JSON format"}, {Name: option.TypeGoTemplate, Usage: "Print using the given Go template"}, }) From 477063e3cb4e9698b1b964cf998772325665cb07 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 7 May 2024 23:59:54 +0000 Subject: [PATCH 03/32] update default Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 3 +++ cmd/oras/root/discover.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 39d880896..d4c58ef8b 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -63,6 +63,9 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { } for _, option := range opts.types { usage += fmt.Sprintf("\n'%s':%s%s", option.Name, strings.Repeat(" ", maxLength-len(option.Name)+2), option.Usage) + if option.Name == opts.Type { + usage += " (default)" + } } // apply flags diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index b2b2510a9..5125b274f 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -97,7 +97,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") cmd.Flags().StringVarP(&opts.Template, "output", "o", "", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") opts.SetTypesAndDefault(option.TypeTree, []option.FormatType{ - {Name: option.TypeTree, Usage: "Get referrers recursively and print in tree format (default)"}, + {Name: option.TypeTree, Usage: "Get referrers recursively and print in tree format"}, {Name: option.TypeTable, Usage: "Get direct referrers and output in table format"}, {Name: option.TypeJSON, Usage: "Get direct referrers and output in JSON format"}, {Name: option.TypeGoTemplate, Usage: "Print direct referrers using the given Go template"}, From 05fc9a8657c960c1719e5bbec6fa710456f164c0 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 8 May 2024 07:38:21 +0000 Subject: [PATCH 04/32] fix: discover init error Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index d4c58ef8b..e3cbc079c 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -19,6 +19,7 @@ import ( "fmt" "strings" + "github.com/spf13/cobra" "github.com/spf13/pflag" oerrors "oras.land/oras/cmd/oras/internal/errors" ) @@ -63,18 +64,15 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { } for _, option := range opts.types { usage += fmt.Sprintf("\n'%s':%s%s", option.Name, strings.Repeat(" ", maxLength-len(option.Name)+2), option.Usage) - if option.Name == opts.Type { - usage += " (default)" - } } // apply flags - fs.StringVar(&opts.input, "format", "", usage) + fs.StringVar(&opts.input, "format", opts.input, usage) fs.StringVar(&opts.Template, "template", "", `Template string used to format output`) } // Parse parses the input format flag. -func (opts *Format) Parse() error { +func (opts *Format) Parse(_ *cobra.Command) error { if err := opts.parseFlag(); err != nil { return err } @@ -110,8 +108,8 @@ func (opts *Format) parseFlag() error { } else if index == len(opts.Type)-1 || index == 0 { return fmt.Errorf("invalid format flag: %s", opts.input) } - opts.Type = opts.Type[:index] - opts.Template = opts.Type[index+1:] + opts.Type = opts.input[:index] + opts.Template = opts.input[index+1:] return nil } @@ -122,7 +120,7 @@ func (opts *Format) SetTypes(types []FormatType) { // SetTypesAndDefault resets the format options and default value. func (opts *Format) SetTypesAndDefault(defaultType string, types []FormatType) { - opts.Type = defaultType + opts.input = defaultType opts.types = types } From 2d86e916b821422261d11fc9c55dcdcbf25c70eb Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 8 May 2024 11:47:46 +0000 Subject: [PATCH 05/32] add tests Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 23 +++++++++------- cmd/oras/root/discover.go | 12 ++++++--- test/e2e/internal/utils/init.go | 3 ++- test/e2e/suite/command/attach.go | 36 ++++++++++++------------- test/e2e/suite/command/cp.go | 10 +++---- test/e2e/suite/command/discover.go | 2 +- test/e2e/suite/command/manifest.go | 10 +++---- test/e2e/suite/command/pull.go | 4 +-- test/e2e/suite/command/push.go | 4 +-- test/e2e/suite/scenario/oci_artifact.go | 4 +-- 10 files changed, 59 insertions(+), 49 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index e3cbc079c..7f44184c0 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -32,6 +32,7 @@ const ( TypeGoTemplate = "go-template" ) +// FormatType represents a custom type for formatting. type FormatType struct { Name string Usage string @@ -41,7 +42,7 @@ type FormatType struct { type Format struct { Type string Template string - input string + Input string types []FormatType } @@ -67,7 +68,7 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { } // apply flags - fs.StringVar(&opts.input, "format", opts.input, usage) + fs.StringVar(&opts.Input, "format", opts.Input, usage) fs.StringVar(&opts.Template, "template", "", `Template string used to format output`) } @@ -80,6 +81,10 @@ func (opts *Format) Parse(_ *cobra.Command) error { if opts.Template != "" && opts.Type != TypeGoTemplate { return fmt.Errorf("--template must be used with --format %s", TypeGoTemplate) } + if opts.Type == "" { + // flag not specified + return nil + } var optionalTypes []string for _, option := range opts.types { @@ -97,19 +102,19 @@ func (opts *Format) Parse(_ *cobra.Command) error { func (opts *Format) parseFlag() error { if opts.Template != "" { // template explicitly set - opts.Type = opts.input + opts.Type = opts.Input return nil } - index := strings.Index(opts.input, "=") + index := strings.Index(opts.Input, "=") if index == -1 { // no proper template found in the type flag - opts.Type = opts.input + opts.Type = opts.Input return nil } else if index == len(opts.Type)-1 || index == 0 { - return fmt.Errorf("invalid format flag: %s", opts.input) + return fmt.Errorf("invalid format flag: %s", opts.Input) } - opts.Type = opts.input[:index] - opts.Template = opts.input[index+1:] + opts.Type = opts.Input[:index] + opts.Template = opts.Input[index+1:] return nil } @@ -120,7 +125,7 @@ func (opts *Format) SetTypes(types []FormatType) { // SetTypesAndDefault resets the format options and default value. func (opts *Format) SetTypesAndDefault(defaultType string, types []FormatType) { - opts.input = defaultType + opts.Input = defaultType opts.types = types } diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index 5125b274f..e7b305131 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -78,16 +78,20 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout if err := oerrors.CheckMutuallyExclusiveFlags(cmd.Flags(), "format", "output"); err != nil { return err } + + opts.RawReference = args[0] + if err := option.Parse(cmd, &opts); err != nil { + return err + } if cmd.Flags().Changed("output") { - switch opts.Template { + switch opts.Format.Type { case "tree", "json", "table": fmt.Fprintf(cmd.ErrOrStderr(), "[DEPRECATED] --output is deprecated, try `--format %s` instead\n", opts.Template) default: return errors.New("output type can only be tree, table or json") } } - opts.RawReference = args[0] - return option.Parse(cmd, &opts) + return nil }, RunE: func(cmd *cobra.Command, args []string) error { return runDiscover(cmd, &opts) @@ -95,7 +99,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout } cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") - cmd.Flags().StringVarP(&opts.Template, "output", "o", "", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") + cmd.Flags().StringVarP(&opts.Format.Input, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") opts.SetTypesAndDefault(option.TypeTree, []option.FormatType{ {Name: option.TypeTree, Usage: "Get referrers recursively and print in tree format"}, {Name: option.TypeTable, Usage: "Get direct referrers and output in table format"}, diff --git a/test/e2e/internal/utils/init.go b/test/e2e/internal/utils/init.go index 5218aa367..4e7f93ab5 100644 --- a/test/e2e/internal/utils/init.go +++ b/test/e2e/internal/utils/init.go @@ -121,7 +121,8 @@ func init() { // Login cmd := exec.Command(ORASPath, "login", Host, "-u", Username, "-p", Password) - gomega.Expect(cmd.Run()).ShouldNot(gomega.HaveOccurred()) + err := cmd.Run() + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) cmd = exec.Command(ORASPath, "login", FallbackHost, "-u", Username, "-p", Password) gomega.Expect(cmd.Run()).ShouldNot(gomega.HaveOccurred()) cmd = exec.Command(ORASPath, "login", ZOTHost, "-u", Username, "-p", Password) diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go index 4d9f4dc11..3f14f915a 100644 --- a/test/e2e/suite/command/attach.go +++ b/test/e2e/suite/command/attach.go @@ -107,7 +107,7 @@ var _ = Describe("1.1 registry users:", func() { ORAS("cp", RegistryRef(ZOTHost, ImageRepo, multi_arch.Tag), subjectRef).Exec() artifactType := "test/attach" // test - out := ORAS("attach", "--artifact-type", artifactType, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.digest}}", "--platform", "linux/amd64"). + out := ORAS("attach", "--artifact-type", artifactType, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.digest}}", "--platform", "linux/amd64"). WithWorkDir(PrepareTempFiles()).Exec().Out.Contents() // validate ORAS("discover", "--artifact-type", artifactType, RegistryRef(ZOTHost, testRepo, multi_arch.LinuxAMD64.Digest.String())).MatchKeyWords(string(out)).Exec() @@ -121,7 +121,7 @@ var _ = Describe("1.1 registry users:", func() { subjectRef := RegistryRef(ZOTHost, testRepo, foobar.Tag) CopyZOTRepo(ImageRepo, testRepo) // test - ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--export-manifest", exportName, "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--export-manifest", exportName, "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() // validate fetched := ORAS("manifest", "fetch", string(ref)).Exec().Out.Contents() @@ -137,7 +137,7 @@ var _ = Describe("1.1 registry users:", func() { CopyZOTRepo(ImageRepo, testRepo) // test delimitter := "---" - output := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--export-manifest", exportName, "--format", fmt.Sprintf("{{.reference}}%s{{.artifactType}}", delimitter)). + output := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--export-manifest", exportName, "--format", fmt.Sprintf("go-template={{.reference}}%s{{.artifactType}}", delimitter)). WithWorkDir(tempDir).Exec().Out.Contents() ref, artifactType, _ := strings.Cut(string(output), delimitter) // validate @@ -167,12 +167,12 @@ var _ = Describe("1.1 registry users:", func() { subjectRef := RegistryRef(ZOTHost, testRepo, foobar.Tag) CopyZOTRepo(ImageRepo, testRepo) // test - ref1 := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.reference}}"). + ref1 := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() - ref2 := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.reference}}"). + ref2 := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() // validate - ORAS("discover", subjectRef, "--format", "{{range .manifests}}{{println .reference}}{{end}}").MatchKeyWords(string(ref1), string(ref2)).Exec() + ORAS("discover", subjectRef, "--format", "go-template={{range .manifests}}{{println .reference}}{{end}}").MatchKeyWords(string(ref1), string(ref2)).Exec() }) It("should attach a file via a OCI Image", func() { @@ -181,10 +181,10 @@ var _ = Describe("1.1 registry users:", func() { subjectRef := RegistryRef(ZOTHost, testRepo, foobar.Tag) CopyZOTRepo(ImageRepo, testRepo) // test - ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() // validate - out := ORAS("discover", subjectRef, "--format", "{{range .manifests}}{{println .reference}}{{end}}").Exec().Out + out := ORAS("discover", subjectRef, "--format", "go-template={{range .manifests}}{{println .reference}}{{end}}").Exec().Out Expect(out).To(gbytes.Say(string(ref))) }) @@ -222,10 +222,10 @@ var _ = Describe("1.0 registry users:", func() { subjectRef := RegistryRef(FallbackHost, testRepo, foobar.Tag) prepare(RegistryRef(FallbackHost, ArtifactRepo, foobar.Tag), subjectRef) // test - ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() // validate - out := ORAS("discover", subjectRef, "--format", "{{range .manifests}}{{println .reference}}{{end}}").Exec().Out + out := ORAS("discover", subjectRef, "--format", "go-template={{range .manifests}}{{println .reference}}{{end}}").Exec().Out Expect(out).To(gbytes.Say(string(ref))) }) @@ -235,11 +235,11 @@ var _ = Describe("1.0 registry users:", func() { subjectRef := RegistryRef(FallbackHost, testRepo, foobar.Tag) prepare(RegistryRef(FallbackHost, ArtifactRepo, foobar.Tag), subjectRef) // test - ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() // validate - out := ORAS("discover", subjectRef, "--format", "{{range .manifests}}{{println .reference}}{{end}}").Exec().Out + out := ORAS("discover", subjectRef, "--format", "go-template={{range .manifests}}{{println .reference}}{{end}}").Exec().Out Expect(out).To(gbytes.Say(string(ref))) }) @@ -249,11 +249,11 @@ var _ = Describe("1.0 registry users:", func() { subjectRef := RegistryRef(FallbackHost, testRepo, foobar.Tag) prepare(RegistryRef(FallbackHost, ArtifactRepo, foobar.Tag), subjectRef) // test - ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--distribution-spec", "v1.1-referrers-tag", "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--distribution-spec", "v1.1-referrers-tag", "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out.Contents() // validate - out := ORAS("discover", subjectRef, "--format", "{{range .manifests}}{{println .reference}}{{end}}").Exec().Out + out := ORAS("discover", subjectRef, "--format", "go-template={{range .manifests}}{{println .reference}}{{end}}").Exec().Out Expect(out).To(gbytes.Say(string(ref))) }) }) @@ -275,7 +275,7 @@ var _ = Describe("OCI image layout users:", func() { root := PrepareTempOCI(ImageRepo) subjectRef := LayoutRef(root, foobar.Tag) // test - ref := ORAS("attach", "--artifact-type", "test/attach", Flags.Layout, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--export-manifest", exportName, "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", Flags.Layout, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--export-manifest", exportName, "--format", "go-template={{.reference}}"). WithWorkDir(root).Exec().Out.Contents() // validate fetched := ORAS("manifest", "fetch", Flags.Layout, string(ref)).Exec().Out.Contents() @@ -287,7 +287,7 @@ var _ = Describe("OCI image layout users:", func() { subjectRef := LayoutRef(root, multi_arch.Tag) artifactType := "test/attach" // test - out := ORAS("attach", Flags.Layout, "--artifact-type", artifactType, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.digest}}", "--platform", "linux/amd64"). + out := ORAS("attach", Flags.Layout, "--artifact-type", artifactType, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.digest}}", "--platform", "linux/amd64"). WithWorkDir(PrepareTempFiles()).Exec().Out.Contents() // validate ORAS("discover", Flags.Layout, "--artifact-type", artifactType, LayoutRef(root, multi_arch.LinuxAMD64.Digest.String())).MatchKeyWords(string(out)).Exec() @@ -297,10 +297,10 @@ var _ = Describe("OCI image layout users:", func() { root := PrepareTempOCI(ImageRepo) subjectRef := LayoutRef(root, foobar.Tag) // test - ref := ORAS("attach", "--artifact-type", "test/attach", Flags.Layout, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "{{.reference}}"). + ref := ORAS("attach", "--artifact-type", "test/attach", Flags.Layout, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.reference}}"). WithWorkDir(root).Exec().Out.Contents() // validate - out := ORAS("discover", Flags.Layout, subjectRef, "--format", "{{range .manifests}}{{println .reference}}{{end}}").Exec().Out + out := ORAS("discover", Flags.Layout, subjectRef, "--format", "go-template={{range .manifests}}{{println .reference}}{{end}}").Exec().Out Expect(out).To(gbytes.Say(string(ref))) }) }) diff --git a/test/e2e/suite/command/cp.go b/test/e2e/suite/command/cp.go index 5b1a16c28..f548dd0be 100644 --- a/test/e2e/suite/command/cp.go +++ b/test/e2e/suite/command/cp.go @@ -188,7 +188,7 @@ var _ = Describe("1.1 registry users:", func() { // validate CompareRef(RegistryRef(ZOTHost, ImageRepo, ma.Digest), dst) - digests := ORAS("discover", dst, "--format", "{{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() + digests := ORAS("discover", dst, "--format", "go-template={{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() for _, digest := range strings.Split(strings.TrimSpace(string(digests)), "\n") { CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), RegistryRef(ZOTHost, dstRepo, digest)) } @@ -206,7 +206,7 @@ var _ = Describe("1.1 registry users:", func() { Exec() // validate CompareRef(RegistryRef(ZOTHost, ImageRepo, ma.Digest), dst) - digests := ORAS("discover", dst, "--format", "{{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() + digests := ORAS("discover", dst, "--format", "go-template={{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() for _, digest := range strings.Split(strings.TrimSpace(string(digests)), "\n") { CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), RegistryRef(ZOTHost, dstRepo, digest)) } @@ -233,7 +233,7 @@ var _ = Describe("1.1 registry users:", func() { Exec() // validate CompareRef(RegistryRef(ZOTHost, ImageRepo, ma.Digest), dst) - digests := ORAS("discover", dst, "--format", "{{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() + digests := ORAS("discover", dst, "--format", "go-template={{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() for _, digest := range strings.Split(strings.TrimSpace(string(digests)), "\n") { CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), RegistryRef(ZOTHost, dstRepo, digest)) } @@ -273,7 +273,7 @@ var _ = Describe("1.1 registry users:", func() { Exec() // validate CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), dst) - digests := ORAS("discover", dst, "--format", "{{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() + digests := ORAS("discover", dst, "--format", "go-template={{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() for _, digest := range strings.Split(strings.TrimSpace(string(digests)), "\n") { CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), RegistryRef(ZOTHost, dstRepo, digest)) } @@ -291,7 +291,7 @@ var _ = Describe("1.1 registry users:", func() { Exec() // validate CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), RegistryRef(ZOTHost, dstRepo, digest)) - digests := ORAS("discover", RegistryRef(ZOTHost, dstRepo, digest), "--format", "{{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() + digests := ORAS("discover", RegistryRef(ZOTHost, dstRepo, digest), "--format", "go-template={{range .manifests}}{{println .digest}}{{end}}").Exec().Out.Contents() for _, digest := range strings.Split(strings.TrimSpace(string(digests)), "\n") { CompareRef(RegistryRef(ZOTHost, ArtifactRepo, digest), RegistryRef(ZOTHost, dstRepo, digest)) } diff --git a/test/e2e/suite/command/discover.go b/test/e2e/suite/command/discover.go index 082ee4eb3..4e0f4dc08 100644 --- a/test/e2e/suite/command/discover.go +++ b/test/e2e/suite/command/discover.go @@ -204,7 +204,7 @@ var _ = Describe("1.1 registry users:", func() { }) When("running discover command with go-template output", func() { It("should show referrers digest of a subject", func() { - ORAS("discover", subjectRef, "--format", "{{(first .manifests).reference}}"). + ORAS("discover", subjectRef, "--format", "go-template={{(first .manifests).reference}}"). MatchContent(RegistryRef(ZOTHost, ArtifactRepo, foobar.SBOMImageReferrer.Digest.String())). Exec() }) diff --git a/test/e2e/suite/command/manifest.go b/test/e2e/suite/command/manifest.go index ca92e4abe..adbf34ee7 100644 --- a/test/e2e/suite/command/manifest.go +++ b/test/e2e/suite/command/manifest.go @@ -106,13 +106,13 @@ var _ = Describe("ORAS beginners:", func() { }) It("should fail if stdout is used inpropriately", func() { - ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--output", "-", "--format", "test"). + ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--output", "-", "--format", "json"). ExpectFailure().Exec() - ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--descriptor", "--format", "test"). + ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--descriptor", "--format", "json"). ExpectFailure().Exec() ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--output", "-", "--descriptor"). ExpectFailure().Exec() - ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--format", "test", "--pretty"). + ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--format", "json", "--pretty"). ExpectFailure().Exec() }) }) @@ -276,13 +276,13 @@ var _ = Describe("1.1 registry users:", func() { }) It("should fetch manifest with platform validation and output content", func() { - out := ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, multi_arch.Tag), "--platform", "linux/amd64", "--format", "{{toJson .content}}"). + out := ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, multi_arch.Tag), "--platform", "linux/amd64", "--format", "go-template={{toJson .content}}"). Exec().Out.Contents() Expect(out).To(MatchJSON(multi_arch.LinuxAMD64Manifest)) }) It("should fetch manifest and format output", func() { - ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, multi_arch.LinuxAMD64.Digest.String()), "--format", "{{(first .content.layers).digest}}"). + ORAS("manifest", "fetch", RegistryRef(ZOTHost, ImageRepo, multi_arch.LinuxAMD64.Digest.String()), "--format", "go-template={{(first .content.layers).digest}}"). MatchContent(multi_arch.LayerDigest). Exec() }) diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index 7d292646d..e71279afb 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -78,7 +78,7 @@ var _ = Describe("ORAS beginners:", func() { It("should not show hint for go template output", func() { tempDir := PrepareTempFiles() ref := RegistryRef(ZOTHost, ArtifactRepo, unnamed.Tag) - out := ORAS("pull", ref, "--format", "{{.}}").WithWorkDir(tempDir).Exec().Out + out := ORAS("pull", ref, "--format", "go-template={{.}}").WithWorkDir(tempDir).Exec().Out gomega.Expect(out).ShouldNot(gbytes.Say(hintMsg(ref))) }) @@ -194,7 +194,7 @@ var _ = Describe("OCI spec 1.1 registry users:", func() { for _, p := range foobar.ImageLayerNames { paths = append(paths, filepath.Join(tempDir, p)) } - ORAS("pull", RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag), "--format", "{{range .files}}{{println .path}}{{end}}"). + ORAS("pull", RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag), "--format", "go-template={{range .files}}{{println .path}}{{end}}"). WithWorkDir(tempDir).MatchKeyWords(paths...).Exec() }) diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index 39beace43..8ed2c2595 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -180,7 +180,7 @@ var _ = Describe("Remote registry users:", func() { tempDir := PrepareTempFiles() extraTag := "2e2" - ORAS("push", fmt.Sprintf("%s,%s", RegistryRef(ZOTHost, repo, tag), extraTag), foobar.FileBarName, "-v", "--format", "{{range .referenceAsTags}}{{println .}}{{end}}"). + ORAS("push", fmt.Sprintf("%s,%s", RegistryRef(ZOTHost, repo, tag), extraTag), foobar.FileBarName, "-v", "--format", "go-template={{range .referenceAsTags}}{{println .}}{{end}}"). MatchContent(fmt.Sprintf("%s\n%s\n", RegistryRef(ZOTHost, repo, extraTag), RegistryRef(ZOTHost, repo, tag))). WithWorkDir(tempDir).Exec() @@ -367,7 +367,7 @@ var _ = Describe("Remote registry users:", func() { annotationValue := "value" // test - out := ORAS("push", RegistryRef(ZOTHost, repo, tag), "-a", fmt.Sprintf("%s=%s", annotationKey, annotationValue), "--format", "{{.reference}}"). + out := ORAS("push", RegistryRef(ZOTHost, repo, tag), "-a", fmt.Sprintf("%s=%s", annotationKey, annotationValue), "--format", "go-template={{.reference}}"). WithWorkDir(tempDir).Exec().Out // validate diff --git a/test/e2e/suite/scenario/oci_artifact.go b/test/e2e/suite/scenario/oci_artifact.go index 41703764e..ee726f8f4 100644 --- a/test/e2e/suite/scenario/oci_artifact.go +++ b/test/e2e/suite/scenario/oci_artifact.go @@ -64,7 +64,7 @@ var _ = Describe("OCI artifact users:", Ordered, func() { WithWorkDir(tempDir). WithDescription("attach with manifest exported").Exec() - ref := string(ORAS("discover", subject, "--format", "{{(first .manifests).reference}}").Exec().Out.Contents()) + ref := string(ORAS("discover", subject, "--format", "go-template={{(first .manifests).reference}}").Exec().Out.Contents()) fetched := ORAS("manifest", "fetch", ref).MatchKeyWords(foobar.AttachFileMedia).Exec() MatchFile(filepath.Join(tempDir, pulledManifest), string(fetched.Out.Contents()), DefaultTimeout) @@ -81,7 +81,7 @@ var _ = Describe("OCI artifact users:", Ordered, func() { WithWorkDir(tempDir). WithDescription("attach again with manifest exported").Exec() - ref = string(ORAS("discover", subject, "--format", "{{(first .manifests).reference}}", "--artifact-type", "test/artifact2").Exec().Out.Contents()) + ref = string(ORAS("discover", subject, "--format", "go-template={{(first .manifests).reference}}", "--artifact-type", "test/artifact2").Exec().Out.Contents()) fetched = ORAS("manifest", "fetch", ref).MatchKeyWords(foobar.AttachFileMedia).Exec() MatchFile(filepath.Join(tempDir, pulledManifest), string(fetched.Out.Contents()), DefaultTimeout) From b65dd71d542d47781dcf966f7f2ae84fde2165f1 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 8 May 2024 11:54:12 +0000 Subject: [PATCH 06/32] fix e2e Signed-off-by: Billy Zha --- cmd/oras/root/manifest/fetch.go | 2 +- test/e2e/suite/command/discover.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index 591a0c48d..c678e69ba 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -73,7 +73,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': Args: oerrors.CheckArgs(argument.Exactly(1), "the manifest to fetch"), PreRunE: func(cmd *cobra.Command, args []string) error { switch { - case opts.outputPath == "-" && opts.Template != "": + case opts.outputPath == "-" && opts.Input != "": return fmt.Errorf("`--output -` cannot be used with `--format %s` at the same time", opts.Template) case opts.outputPath == "-" && opts.OutputDescriptor: return fmt.Errorf("`--descriptor` cannot be used with `--output -` at the same time") diff --git a/test/e2e/suite/command/discover.go b/test/e2e/suite/command/discover.go index 4e0f4dc08..8cdcf5ff1 100644 --- a/test/e2e/suite/command/discover.go +++ b/test/e2e/suite/command/discover.go @@ -77,9 +77,10 @@ var _ = Describe("ORAS beginners:", func() { }) It("should fail if invalid output type is used", func() { - ORAS("discover", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--output", "ukpkmkk"). + invalidType := "ukpkmkk" + ORAS("discover", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "--output", invalidType). ExpectFailure(). - MatchErrKeyWords("Error:", "output type can only be tree, table or json"). + MatchErrKeyWords("Error:", "invalid format type", invalidType, "tree", "table", "json", "go-template"). Exec() }) From 799e313d6f359888800125cb127b270e41482c25 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 03:13:38 +0000 Subject: [PATCH 07/32] add unit test Signed-off-by: Billy Zha --- cmd/oras/root/manifest/fetch_test.go | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 cmd/oras/root/manifest/fetch_test.go diff --git a/cmd/oras/root/manifest/fetch_test.go b/cmd/oras/root/manifest/fetch_test.go new file mode 100644 index 000000000..4875b20f4 --- /dev/null +++ b/cmd/oras/root/manifest/fetch_test.go @@ -0,0 +1,59 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package manifest + +import ( + "context" + "os" + "path/filepath" + "testing" + + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/option" +) + +func Test_fetchManifest_errType(t *testing.T) { + // prpare + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + root := t.TempDir() + err := os.WriteFile(filepath.Join(root, ocispec.ImageLayoutFile), []byte(`{"imageLayoutVersion":"1.0.0"}`), 0644) + if err != nil { + t.Fatalf("failed to write file: %v", err) + } + err = os.WriteFile(filepath.Join(root, ocispec.ImageIndexFile), []byte(`{"manifests": [],"mediaType": "application/vnd.oci.image.index.v1+json","schemaVersion": 2}`), 0644) + if err != nil { + t.Fatalf("failed to write file: %v", err) + } + + // test + opts := &fetchOptions{ + Format: option.Format{ + Type: "unknown", + }, + Target: option.Target{ + Path: root, + Reference: "test", + Type: option.TargetTypeOCILayout, + }, + } + got := fetchManifest(cmd, opts).Error() + want := opts.TypeError().Error() + if got != want { + t.Fatalf("got %v, want %v", got, want) + } +} From 467ab8ed5b1158143d0014f691bd94b37cb35b5f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 03:13:50 +0000 Subject: [PATCH 08/32] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/display/handler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index bc71ec2ca..cd40aa1d5 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -73,9 +73,9 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose switch format.Type { case "": metadataHandler = text.NewAttachHandler(out) - case "json": + case option.TypeJSON: metadataHandler = json.NewAttachHandler(out) - case "go-template": + case option.TypeGoTemplate: metadataHandler = template.NewAttachHandler(out, format.Template) default: return nil, nil, format.TypeError() From 4f0334c8cfc1aed18824b6f2edfaf6b6c2c7f963 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 03:19:37 +0000 Subject: [PATCH 09/32] make ut easier Signed-off-by: Billy Zha --- cmd/oras/root/manifest/fetch.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index c678e69ba..22be86196 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -108,6 +108,10 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': func fetchManifest(cmd *cobra.Command, opts *fetchOptions) (fetchErr error) { ctx, logger := command.GetLogger(cmd, &opts.Common) + metadataHandler, contentHandler, err := display.NewManifestFetchHandler(cmd.OutOrStdout(), opts.Format, opts.OutputDescriptor, opts.Pretty.Pretty, opts.outputPath) + if err != nil { + return err + } target, err := opts.NewReadonlyTarget(ctx, opts.Common, logger) if err != nil { @@ -126,11 +130,6 @@ func fetchManifest(cmd *cobra.Command, opts *fetchOptions) (fetchErr error) { if err != nil { return err } - metadataHandler, contentHandler, err := display.NewManifestFetchHandler(cmd.OutOrStdout(), opts.Format, opts.OutputDescriptor, opts.Pretty.Pretty, opts.outputPath) - if err != nil { - return err - } - var desc ocispec.Descriptor var content []byte if opts.OutputDescriptor && opts.outputPath == "" { From ddb9b0ab579d1d4cbb2cbd7ca64df882ff7f7ccb Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 03:23:07 +0000 Subject: [PATCH 10/32] try make UT easier Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 9 +++++---- cmd/oras/root/pull.go | 9 +++++---- cmd/oras/root/push.go | 5 +++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 388f8ff8a..05df551d3 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -108,6 +108,11 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder func runAttach(cmd *cobra.Command, opts *attachOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) + displayStatus, displayMetadata, err := display.NewAttachHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose) + if err != nil { + return err + } + annotations, err := opts.LoadManifestAnnotations() if err != nil { return err @@ -119,10 +124,6 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { Recommendation: `To attach to an existing artifact, please provide files via argument or annotations via flag "--annotation". Run "oras attach -h" for more options and examples`, } } - displayStatus, displayMetadata, err := display.NewAttachHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose) - if err != nil { - return err - } // prepare manifest store, err := file.New("") diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index a037c5e45..3f5e8cff2 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -110,6 +110,11 @@ Example - Pull artifact files from an OCI layout archive 'layout.tar': func runPull(cmd *cobra.Command, opts *pullOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) + statusHandler, metadataHandler, err := display.NewPullHandler(cmd.OutOrStdout(), opts.Format, opts.Path, opts.TTY, opts.Verbose) + if err != nil { + return err + } + // Copy Options copyOptions := oras.DefaultCopyOptions copyOptions.Concurrency = opts.concurrency @@ -135,10 +140,6 @@ func runPull(cmd *cobra.Command, opts *pullOptions) error { dst.AllowPathTraversalOnWrite = opts.PathTraversal dst.DisableOverwrite = opts.KeepOldFiles - statusHandler, metadataHandler, err := display.NewPullHandler(cmd.OutOrStdout(), opts.Format, opts.Path, opts.TTY, opts.Verbose) - if err != nil { - return err - } desc, err := doPull(ctx, src, dst, copyOptions, metadataHandler, statusHandler, opts) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 6e455fbc6..a06d184cf 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -151,11 +151,12 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t func runPush(cmd *cobra.Command, opts *pushOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - annotations, err := opts.LoadManifestAnnotations() + displayStatus, displayMetadata, err := display.NewPushHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose) if err != nil { return err } - displayStatus, displayMetadata, err := display.NewPushHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose) + + annotations, err := opts.LoadManifestAnnotations() if err != nil { return err } From 97b8cc0ba6f102b772c48f6e72aa0334f5cec8d4 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 03:30:39 +0000 Subject: [PATCH 11/32] add unit tests Signed-off-by: Billy Zha --- cmd/oras/root/attach_test.go | 42 ++++++++++++++++++++++++++++ cmd/oras/root/manifest/fetch_test.go | 17 ----------- cmd/oras/root/pull_test.go | 42 ++++++++++++++++++++++++++++ cmd/oras/root/push_test.go | 42 ++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 cmd/oras/root/attach_test.go create mode 100644 cmd/oras/root/pull_test.go create mode 100644 cmd/oras/root/push_test.go diff --git a/cmd/oras/root/attach_test.go b/cmd/oras/root/attach_test.go new file mode 100644 index 000000000..1cf84e310 --- /dev/null +++ b/cmd/oras/root/attach_test.go @@ -0,0 +1,42 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package root + +import ( + "context" + "testing" + + "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/option" +) + +func Test_runAttach_errType(t *testing.T) { + // prpare + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + // test + opts := &attachOptions{ + Format: option.Format{ + Type: "unknown", + }, + } + got := runAttach(cmd, opts).Error() + want := opts.TypeError().Error() + if got != want { + t.Fatalf("got %v, want %v", got, want) + } +} diff --git a/cmd/oras/root/manifest/fetch_test.go b/cmd/oras/root/manifest/fetch_test.go index 4875b20f4..d13fc9f6e 100644 --- a/cmd/oras/root/manifest/fetch_test.go +++ b/cmd/oras/root/manifest/fetch_test.go @@ -17,11 +17,8 @@ package manifest import ( "context" - "os" - "path/filepath" "testing" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/cobra" "oras.land/oras/cmd/oras/internal/option" ) @@ -30,26 +27,12 @@ func Test_fetchManifest_errType(t *testing.T) { // prpare cmd := &cobra.Command{} cmd.SetContext(context.Background()) - root := t.TempDir() - err := os.WriteFile(filepath.Join(root, ocispec.ImageLayoutFile), []byte(`{"imageLayoutVersion":"1.0.0"}`), 0644) - if err != nil { - t.Fatalf("failed to write file: %v", err) - } - err = os.WriteFile(filepath.Join(root, ocispec.ImageIndexFile), []byte(`{"manifests": [],"mediaType": "application/vnd.oci.image.index.v1+json","schemaVersion": 2}`), 0644) - if err != nil { - t.Fatalf("failed to write file: %v", err) - } // test opts := &fetchOptions{ Format: option.Format{ Type: "unknown", }, - Target: option.Target{ - Path: root, - Reference: "test", - Type: option.TargetTypeOCILayout, - }, } got := fetchManifest(cmd, opts).Error() want := opts.TypeError().Error() diff --git a/cmd/oras/root/pull_test.go b/cmd/oras/root/pull_test.go new file mode 100644 index 000000000..a0f1e5ff7 --- /dev/null +++ b/cmd/oras/root/pull_test.go @@ -0,0 +1,42 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package root + +import ( + "context" + "testing" + + "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/option" +) + +func Test_runPull_errType(t *testing.T) { + // prpare + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + // test + opts := &pullOptions{ + Format: option.Format{ + Type: "unknown", + }, + } + got := runPull(cmd, opts).Error() + want := opts.TypeError().Error() + if got != want { + t.Fatalf("got %v, want %v", got, want) + } +} diff --git a/cmd/oras/root/push_test.go b/cmd/oras/root/push_test.go new file mode 100644 index 000000000..22fbd9b79 --- /dev/null +++ b/cmd/oras/root/push_test.go @@ -0,0 +1,42 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package root + +import ( + "context" + "testing" + + "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/option" +) + +func Test_runPush_errType(t *testing.T) { + // prpare + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + // test + opts := &pushOptions{ + Format: option.Format{ + Type: "unknown", + }, + } + got := runPush(cmd, opts).Error() + want := opts.TypeError().Error() + if got != want { + t.Fatalf("got %v, want %v", got, want) + } +} From ae6b4f231fdedaf928b4482ebbc23a3740db852d Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 06:35:29 +0000 Subject: [PATCH 12/32] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/display/handler.go | 24 ++++++++++++------------ cmd/oras/internal/option/format.go | 18 +++++++++--------- cmd/oras/root/discover.go | 10 +++++----- cmd/oras/root/manifest/fetch.go | 4 ++-- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index cd40aa1d5..75d9db473 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -48,9 +48,9 @@ func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose b switch format.Type { case "": metadataHandler = text.NewPushHandler(out) - case option.TypeJSON: + case option.FormatTypeJSON: metadataHandler = json.NewPushHandler(out) - case option.TypeGoTemplate: + case option.FormatTypeGoTemplate: metadataHandler = template.NewPushHandler(out, format.Template) default: return nil, nil, format.TypeError() @@ -73,9 +73,9 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose switch format.Type { case "": metadataHandler = text.NewAttachHandler(out) - case option.TypeJSON: + case option.FormatTypeJSON: metadataHandler = json.NewAttachHandler(out) - case option.TypeGoTemplate: + case option.FormatTypeGoTemplate: metadataHandler = template.NewAttachHandler(out, format.Template) default: return nil, nil, format.TypeError() @@ -98,9 +98,9 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi switch format.Type { case "": metadataHandler = text.NewPullHandler(out) - case option.TypeJSON: + case option.FormatTypeJSON: metadataHandler = json.NewPullHandler(out, path) - case option.TypeGoTemplate: + case option.FormatTypeGoTemplate: metadataHandler = template.NewPullHandler(out, path, format.Template) default: return nil, nil, format.TypeError() @@ -112,13 +112,13 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi func NewDiscoverHandler(out io.Writer, format option.Format, path string, rawReference string, desc ocispec.Descriptor, verbose bool) (metadata.DiscoverHandler, error) { var handler metadata.DiscoverHandler switch format.Type { - case option.TypeTree, "": + case option.FormatTypeTree, "": handler = tree.NewDiscoverHandler(out, path, desc, verbose) - case option.TypeTable: + case option.FormatTypeTable: handler = table.NewDiscoverHandler(out, rawReference, desc, verbose) - case option.TypeJSON: + case option.FormatTypeJSON: handler = json.NewDiscoverHandler(out, desc, path) - case option.TypeGoTemplate: + case option.FormatTypeGoTemplate: handler = template.NewDiscoverHandler(out, desc, path, format.Template) default: return nil, format.TypeError() @@ -139,13 +139,13 @@ func NewManifestFetchHandler(out io.Writer, format option.Format, outputDescript } else { metadataHandler = metadata.NewDiscardHandler() } - case option.TypeJSON: + case option.FormatTypeJSON: // json metadataHandler = json.NewManifestFetchHandler(out) if outputPath == "" { contentHandler = content.NewDiscardHandler() } - case option.TypeGoTemplate: + case option.FormatTypeGoTemplate: // go template metadataHandler = template.NewManifestFetchHandler(out, format.Template) if outputPath == "" { diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 7f44184c0..e719e7abf 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -26,10 +26,10 @@ import ( const ( // format types - TypeJSON = "json" - TypeTree = "tree" - TypeTable = "table" - TypeGoTemplate = "go-template" + FormatTypeJSON = "json" + FormatTypeTree = "tree" + FormatTypeTable = "table" + FormatTypeGoTemplate = "go-template" ) // FormatType represents a custom type for formatting. @@ -38,7 +38,7 @@ type FormatType struct { Usage string } -// Format is a flag to format metadata into output. +// Format contains input and parsed options to format output. type Format struct { Type string Template string @@ -51,8 +51,8 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { usage := "[Experimental] Format output using a custom template:" if len(opts.types) == 0 { opts.types = []FormatType{ - {Name: TypeJSON, Usage: "Print in JSON format"}, - {Name: TypeGoTemplate, Usage: "Print output using the given Go template"}, + {Name: FormatTypeJSON, Usage: "Print in JSON format"}, + {Name: FormatTypeGoTemplate, Usage: "Print output using the given Go template"}, } } @@ -78,8 +78,8 @@ func (opts *Format) Parse(_ *cobra.Command) error { return err } - if opts.Template != "" && opts.Type != TypeGoTemplate { - return fmt.Errorf("--template must be used with --format %s", TypeGoTemplate) + if opts.Template != "" && opts.Type != FormatTypeGoTemplate { + return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate) } if opts.Type == "" { // flag not specified diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index e7b305131..62640a521 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -100,11 +100,11 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") cmd.Flags().StringVarP(&opts.Format.Input, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") - opts.SetTypesAndDefault(option.TypeTree, []option.FormatType{ - {Name: option.TypeTree, Usage: "Get referrers recursively and print in tree format"}, - {Name: option.TypeTable, Usage: "Get direct referrers and output in table format"}, - {Name: option.TypeJSON, Usage: "Get direct referrers and output in JSON format"}, - {Name: option.TypeGoTemplate, Usage: "Print direct referrers using the given Go template"}, + opts.SetTypesAndDefault(option.FormatTypeTree, []option.FormatType{ + {Name: option.FormatTypeTree, Usage: "Get referrers recursively and print in tree format"}, + {Name: option.FormatTypeTable, Usage: "Get direct referrers and output in table format"}, + {Name: option.FormatTypeJSON, Usage: "Get direct referrers and output in JSON format"}, + {Name: option.FormatTypeGoTemplate, Usage: "Print direct referrers using the given Go template"}, }) opts.EnableDistributionSpecFlag() option.ApplyFlags(&opts, cmd.Flags()) diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index 22be86196..f6c0ece4b 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -99,8 +99,8 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': cmd.Flags().StringSliceVarP(&opts.mediaTypes, "media-type", "", nil, "accepted media types") cmd.Flags().StringVarP(&opts.outputPath, "output", "o", "", "file `path` to write the fetched manifest to, use - for stdout") opts.SetTypes([]option.FormatType{ - {Name: option.TypeJSON, Usage: "Print in prettified JSON format"}, - {Name: option.TypeGoTemplate, Usage: "Print using the given Go template"}, + {Name: option.FormatTypeJSON, Usage: "Print in prettified JSON format"}, + {Name: option.FormatTypeGoTemplate, Usage: "Print using the given Go template"}, }) option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) From 10510b7936161243abe587d371deb17860add294 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 07:24:01 +0000 Subject: [PATCH 13/32] bug fix && add e2e Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- test/e2e/suite/command/pull.go | 45 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index e719e7abf..cd4f2ee6d 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -110,7 +110,7 @@ func (opts *Format) parseFlag() error { // no proper template found in the type flag opts.Type = opts.Input return nil - } else if index == len(opts.Type)-1 || index == 0 { + } else if index == len(opts.Input)-1 || index == 0 { return fmt.Errorf("invalid format flag: %s", opts.Input) } opts.Type = opts.Input[:index] diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index e71279afb..c57320dac 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -82,6 +82,41 @@ var _ = Describe("ORAS beginners:", func() { gomega.Expect(out).ShouldNot(gbytes.Say(hintMsg(ref))) }) + It("should fail if template is specified without go template format", func() { + tempDir := PrepareTempFiles() + ref := RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag) + ORAS("pull", ref, "--format", "json", "--template", "{{.}}"). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords("--template must be used with --format go-template"). + Exec() + ORAS("pull", ref, "--template", "{{.}}"). + WithWorkDir(tempDir). + ExpectFailure(). + Exec() + }) + + It("should fail if template format is invalid", func() { + tempDir := PrepareTempFiles() + invalidPrompt := "invalid format flag" + ref := RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag) + ORAS("pull", ref, "--format", "json", "--format", "="). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords(invalidPrompt). + Exec() + ORAS("pull", ref, "--format", "json", "--format", "={{.}}"). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords(invalidPrompt). + Exec() + ORAS("pull", ref, "--format", "json", "--format", "go-template="). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords(invalidPrompt). + Exec() + }) + It("should fail and show detailed error description if no argument provided", func() { err := ORAS("pull").ExpectFailure().Exec().Err Expect(err).Should(gbytes.Say("Error")) @@ -198,6 +233,16 @@ var _ = Describe("OCI spec 1.1 registry users:", func() { WithWorkDir(tempDir).MatchKeyWords(paths...).Exec() }) + It("should pull and output downloaded file paths using --template", func() { + tempDir := GinkgoT().TempDir() + var paths []string + for _, p := range foobar.ImageLayerNames { + paths = append(paths, filepath.Join(tempDir, p)) + } + ORAS("pull", RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag), "--format", "go-template", "--template", "{{range .files}}{{println .path}}{{end}}"). + WithWorkDir(tempDir).MatchKeyWords(paths...).Exec() + }) + It("should pull and output path in json", func() { tempDir := GinkgoT().TempDir() var paths []string From 69afad6a95451199bf05603e27ae2fb353704ec7 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 08:18:05 +0000 Subject: [PATCH 14/32] bug fix Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 24 ++++++++++++------------ test/e2e/suite/command/pull.go | 10 +++------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index cd4f2ee6d..fa7c9509c 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -78,9 +78,6 @@ func (opts *Format) Parse(_ *cobra.Command) error { return err } - if opts.Template != "" && opts.Type != FormatTypeGoTemplate { - return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate) - } if opts.Type == "" { // flag not specified return nil @@ -89,12 +86,13 @@ func (opts *Format) Parse(_ *cobra.Command) error { var optionalTypes []string for _, option := range opts.types { if opts.Type == option.Name { + // type validation passed return nil } optionalTypes = append(optionalTypes, option.Name) } return &oerrors.Error{ - Err: fmt.Errorf("invalid format type: %s", opts.Type), + Err: fmt.Errorf("invalid format type: %q", opts.Type), Recommendation: fmt.Sprintf("supported types: %s", strings.Join(optionalTypes, ", ")), } } @@ -103,18 +101,20 @@ func (opts *Format) parseFlag() error { if opts.Template != "" { // template explicitly set opts.Type = opts.Input + if opts.Type != FormatTypeGoTemplate { + return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate) + } return nil } - index := strings.Index(opts.Input, "=") - if index == -1 { - // no proper template found in the type flag + + goTemplatePrefix := FormatTypeGoTemplate + "=" + if strings.HasPrefix(opts.Input, goTemplatePrefix) { + // add parameter to template + opts.Type = FormatTypeGoTemplate + opts.Template = opts.Input[len(goTemplatePrefix):] + } else { opts.Type = opts.Input - return nil - } else if index == len(opts.Input)-1 || index == 0 { - return fmt.Errorf("invalid format flag: %s", opts.Input) } - opts.Type = opts.Input[:index] - opts.Template = opts.Input[index+1:] return nil } diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index c57320dac..05ea308c7 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -93,12 +93,13 @@ var _ = Describe("ORAS beginners:", func() { ORAS("pull", ref, "--template", "{{.}}"). WithWorkDir(tempDir). ExpectFailure(). + MatchErrKeyWords("--template must be used with --format go-template"). Exec() }) - It("should fail if template format is invalid", func() { + It("should fail if template format is invalid", Focus, func() { tempDir := PrepareTempFiles() - invalidPrompt := "invalid format flag" + invalidPrompt := "invalid format type" ref := RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag) ORAS("pull", ref, "--format", "json", "--format", "="). WithWorkDir(tempDir). @@ -110,11 +111,6 @@ var _ = Describe("ORAS beginners:", func() { ExpectFailure(). MatchErrKeyWords(invalidPrompt). Exec() - ORAS("pull", ref, "--format", "json", "--format", "go-template="). - WithWorkDir(tempDir). - ExpectFailure(). - MatchErrKeyWords(invalidPrompt). - Exec() }) It("should fail and show detailed error description if no argument provided", func() { From e0eb0fb90cdd9265206bd63285f6d9eb96fac24f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 9 May 2024 08:24:40 +0000 Subject: [PATCH 15/32] remove focus Signed-off-by: Billy Zha --- test/e2e/suite/command/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index 05ea308c7..cc4af84c5 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -97,7 +97,7 @@ var _ = Describe("ORAS beginners:", func() { Exec() }) - It("should fail if template format is invalid", Focus, func() { + It("should fail if template format is invalid", func() { tempDir := PrepareTempFiles() invalidPrompt := "invalid format type" ref := RegistryRef(ZOTHost, ArtifactRepo, foobar.Tag) From 10b70166da73921553b11fa4667489f7906c0065 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 10 May 2024 01:02:51 +0000 Subject: [PATCH 16/32] revert unnecessary change Signed-off-by: Billy Zha --- test/e2e/internal/utils/init.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/internal/utils/init.go b/test/e2e/internal/utils/init.go index 4e7f93ab5..5218aa367 100644 --- a/test/e2e/internal/utils/init.go +++ b/test/e2e/internal/utils/init.go @@ -121,8 +121,7 @@ func init() { // Login cmd := exec.Command(ORASPath, "login", Host, "-u", Username, "-p", Password) - err := cmd.Run() - gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + gomega.Expect(cmd.Run()).ShouldNot(gomega.HaveOccurred()) cmd = exec.Command(ORASPath, "login", FallbackHost, "-u", Username, "-p", Password) gomega.Expect(cmd.Run()).ShouldNot(gomega.HaveOccurred()) cmd = exec.Command(ORASPath, "login", ZOTHost, "-u", Username, "-p", Password) From 53392e26d071768eb2e6e25482e25401c9545521 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 10 May 2024 02:35:34 +0000 Subject: [PATCH 17/32] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 10 +++++----- cmd/oras/root/discover.go | 1 - cmd/oras/root/pull.go | 1 - cmd/oras/root/push.go | 1 - 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index fa7c9509c..2c0405a0d 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -32,13 +32,13 @@ const ( FormatTypeGoTemplate = "go-template" ) -// FormatType represents a custom type for formatting. +// FormatType represents a custom description in help doc. type FormatType struct { Name string Usage string } -// Format contains input and parsed options to format output. +// Format contains input and parsed options for formatted output flags. type Format struct { Type string Template string @@ -69,7 +69,7 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { // apply flags fs.StringVar(&opts.Input, "format", opts.Input, usage) - fs.StringVar(&opts.Template, "template", "", `Template string used to format output`) + fs.StringVar(&opts.Template, "template", "", "Template string used to format output") } // Parse parses the input format flag. @@ -98,6 +98,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { } func (opts *Format) parseFlag() error { + opts.Type = opts.Input if opts.Template != "" { // template explicitly set opts.Type = opts.Input @@ -112,8 +113,6 @@ func (opts *Format) parseFlag() error { // add parameter to template opts.Type = FormatTypeGoTemplate opts.Template = opts.Input[len(goTemplatePrefix):] - } else { - opts.Type = opts.Input } return nil } @@ -124,6 +123,7 @@ func (opts *Format) SetTypes(types []FormatType) { } // SetTypesAndDefault resets the format options and default value. +// Caller should make sure that this function is used before applying flags. func (opts *Format) SetTypesAndDefault(defaultType string, types []FormatType) { opts.Input = defaultType opts.types = types diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index 62640a521..f9abc7436 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -78,7 +78,6 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout if err := oerrors.CheckMutuallyExclusiveFlags(cmd.Flags(), "format", "output"); err != nil { return err } - opts.RawReference = args[0] if err := option.Parse(cmd, &opts); err != nil { return err diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 3f5e8cff2..db12f4bad 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -114,7 +114,6 @@ func runPull(cmd *cobra.Command, opts *pullOptions) error { if err != nil { return err } - // Copy Options copyOptions := oras.DefaultCopyOptions copyOptions.Concurrency = opts.concurrency diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index a06d184cf..5cc78053c 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -155,7 +155,6 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { if err != nil { return err } - annotations, err := opts.LoadManifestAnnotations() if err != nil { return err From 0014f3ada29edaf5fb7f07c658821b0e80cffd1a Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Sat, 11 May 2024 01:38:11 +0000 Subject: [PATCH 18/32] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 25 ++++++++++++++++--------- cmd/oras/root/discover.go | 2 +- cmd/oras/root/manifest/fetch.go | 2 +- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 2c0405a0d..495ccd216 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -42,8 +42,10 @@ type FormatType struct { type Format struct { Type string Template string - Input string - types []FormatType + // FormatFlag can be private once deprecated `--output` is removed from + // `oras discover` + FormatFlag string + types []FormatType } // ApplyFlag implements FlagProvider.ApplyFlag. @@ -68,7 +70,7 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { } // apply flags - fs.StringVar(&opts.Input, "format", opts.Input, usage) + fs.StringVar(&opts.FormatFlag, "format", opts.FormatFlag, usage) fs.StringVar(&opts.Template, "template", "", "Template string used to format output") } @@ -98,10 +100,10 @@ func (opts *Format) Parse(_ *cobra.Command) error { } func (opts *Format) parseFlag() error { - opts.Type = opts.Input + opts.Type = opts.FormatFlag if opts.Template != "" { // template explicitly set - opts.Type = opts.Input + opts.Type = opts.FormatFlag if opts.Type != FormatTypeGoTemplate { return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate) } @@ -109,10 +111,10 @@ func (opts *Format) parseFlag() error { } goTemplatePrefix := FormatTypeGoTemplate + "=" - if strings.HasPrefix(opts.Input, goTemplatePrefix) { + if strings.HasPrefix(opts.FormatFlag, goTemplatePrefix) { // add parameter to template opts.Type = FormatTypeGoTemplate - opts.Template = opts.Input[len(goTemplatePrefix):] + opts.Template = opts.FormatFlag[len(goTemplatePrefix):] } return nil } @@ -125,11 +127,16 @@ func (opts *Format) SetTypes(types []FormatType) { // SetTypesAndDefault resets the format options and default value. // Caller should make sure that this function is used before applying flags. func (opts *Format) SetTypesAndDefault(defaultType string, types []FormatType) { - opts.Input = defaultType + opts.FormatFlag = defaultType opts.types = types } -// FormatError generate the error message for an invalid type. +// FormatError generates the error message for an invalid type. func (opts *Format) TypeError() error { return fmt.Errorf("unsupported format type: %s", opts.Type) } + +// RawFormatFlag returns raw input of --format flag. +func (opts *Format) RawFormatFlag() string { + return opts.FormatFlag +} diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index f9abc7436..f29f75434 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -98,7 +98,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout } cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") - cmd.Flags().StringVarP(&opts.Format.Input, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") + cmd.Flags().StringVarP(&opts.Format.FormatFlag, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") opts.SetTypesAndDefault(option.FormatTypeTree, []option.FormatType{ {Name: option.FormatTypeTree, Usage: "Get referrers recursively and print in tree format"}, {Name: option.FormatTypeTable, Usage: "Get direct referrers and output in table format"}, diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index f6c0ece4b..7ed5cb51f 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -73,7 +73,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': Args: oerrors.CheckArgs(argument.Exactly(1), "the manifest to fetch"), PreRunE: func(cmd *cobra.Command, args []string) error { switch { - case opts.outputPath == "-" && opts.Input != "": + case opts.outputPath == "-" && opts.RawFormatFlag() != "": return fmt.Errorf("`--output -` cannot be used with `--format %s` at the same time", opts.Template) case opts.outputPath == "-" && opts.OutputDescriptor: return fmt.Errorf("`--descriptor` cannot be used with `--output -` at the same time") From 6db10206b1c32e0e4cc62d5c86e4e59cec2bd753 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Sat, 11 May 2024 01:40:14 +0000 Subject: [PATCH 19/32] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 495ccd216..1b7036132 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -103,7 +103,6 @@ func (opts *Format) parseFlag() error { opts.Type = opts.FormatFlag if opts.Template != "" { // template explicitly set - opts.Type = opts.FormatFlag if opts.Type != FormatTypeGoTemplate { return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate) } From 1224dd20cd1aea605c50d87f5d30fdf042a3b80b Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Sat, 11 May 2024 02:29:58 +0000 Subject: [PATCH 20/32] add experimental to template flag Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 1b7036132..fce828898 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -71,7 +71,7 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { // apply flags fs.StringVar(&opts.FormatFlag, "format", opts.FormatFlag, usage) - fs.StringVar(&opts.Template, "template", "", "Template string used to format output") + fs.StringVar(&opts.Template, "template", "", "[Experimental] Template string used to format output") } // Parse parses the input format flag. From 74522e5249044c14c28dbd94c2aa7a8c0f6dcc67 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 13:28:18 +0000 Subject: [PATCH 21/32] resolve comments Signed-off-by: Billy Zha --- cmd/oras/internal/display/handler.go | 35 ++++---- cmd/oras/internal/errors/errors.go | 7 ++ cmd/oras/internal/option/format.go | 117 +++++++++++++-------------- cmd/oras/root/attach_test.go | 3 +- cmd/oras/root/discover.go | 13 +-- cmd/oras/root/manifest/fetch.go | 10 +-- cmd/oras/root/manifest/fetch_test.go | 3 +- cmd/oras/root/pull_test.go | 3 +- cmd/oras/root/push_test.go | 3 +- 9 files changed, 101 insertions(+), 93 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index 75d9db473..3b3da398c 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -30,6 +30,7 @@ import ( "oras.land/oras/cmd/oras/internal/display/metadata/text" "oras.land/oras/cmd/oras/internal/display/metadata/tree" "oras.land/oras/cmd/oras/internal/display/status" + "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" ) @@ -48,12 +49,12 @@ func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose b switch format.Type { case "": metadataHandler = text.NewPushHandler(out) - case option.FormatTypeJSON: + case option.FormatTypeJSON.Name: metadataHandler = json.NewPushHandler(out) - case option.FormatTypeGoTemplate: + case option.FormatTypeGoTemplate.Name: metadataHandler = template.NewPushHandler(out, format.Template) default: - return nil, nil, format.TypeError() + return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } return statusHandler, metadataHandler, nil } @@ -73,12 +74,12 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose switch format.Type { case "": metadataHandler = text.NewAttachHandler(out) - case option.FormatTypeJSON: + case option.FormatTypeJSON.Name: metadataHandler = json.NewAttachHandler(out) - case option.FormatTypeGoTemplate: + case option.FormatTypeGoTemplate.Name: metadataHandler = template.NewAttachHandler(out, format.Template) default: - return nil, nil, format.TypeError() + return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } return statusHandler, metadataHandler, nil } @@ -98,12 +99,12 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi switch format.Type { case "": metadataHandler = text.NewPullHandler(out) - case option.FormatTypeJSON: + case option.FormatTypeJSON.Name: metadataHandler = json.NewPullHandler(out, path) - case option.FormatTypeGoTemplate: + case option.FormatTypeGoTemplate.Name: metadataHandler = template.NewPullHandler(out, path, format.Template) default: - return nil, nil, format.TypeError() + return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } return statusHandler, metadataHandler, nil } @@ -112,16 +113,16 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi func NewDiscoverHandler(out io.Writer, format option.Format, path string, rawReference string, desc ocispec.Descriptor, verbose bool) (metadata.DiscoverHandler, error) { var handler metadata.DiscoverHandler switch format.Type { - case option.FormatTypeTree, "": + case option.FormatTypeTree.Name, "": handler = tree.NewDiscoverHandler(out, path, desc, verbose) - case option.FormatTypeTable: + case option.FormatTypeTable.Name: handler = table.NewDiscoverHandler(out, rawReference, desc, verbose) - case option.FormatTypeJSON: + case option.FormatTypeJSON.Name: handler = json.NewDiscoverHandler(out, desc, path) - case option.FormatTypeGoTemplate: + case option.FormatTypeGoTemplate.Name: handler = template.NewDiscoverHandler(out, desc, path, format.Template) default: - return nil, format.TypeError() + return nil, errors.UnsupportedFormatTypeError(format.Type) } return handler, nil } @@ -139,20 +140,20 @@ func NewManifestFetchHandler(out io.Writer, format option.Format, outputDescript } else { metadataHandler = metadata.NewDiscardHandler() } - case option.FormatTypeJSON: + case option.FormatTypeJSON.Name: // json metadataHandler = json.NewManifestFetchHandler(out) if outputPath == "" { contentHandler = content.NewDiscardHandler() } - case option.FormatTypeGoTemplate: + case option.FormatTypeGoTemplate.Name: // go template metadataHandler = template.NewManifestFetchHandler(out, format.Template) if outputPath == "" { contentHandler = content.NewDiscardHandler() } default: - return nil, nil, format.TypeError() + return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } if contentHandler == nil { diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index bb7784660..86526c252 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -29,6 +29,13 @@ import ( // RegistryErrorPrefix is the commandline prefix for errors from registry. const RegistryErrorPrefix = "Error response from registry:" +// UnsupportedFormatTypeError generates the error message for an invalid type. +type UnsupportedFormatTypeError string + +func (e UnsupportedFormatTypeError) Error() string { + return "unsupported format type: " + string(e) +} + // Error is the error type for CLI error messaging. type Error struct { Err error diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index fce828898..bb6c022c3 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -16,61 +16,71 @@ limitations under the License. package option import ( + "bytes" "fmt" "strings" + "text/tabwriter" "github.com/spf13/cobra" "github.com/spf13/pflag" oerrors "oras.land/oras/cmd/oras/internal/errors" ) -const ( - // format types - FormatTypeJSON = "json" - FormatTypeTree = "tree" - FormatTypeTable = "table" - FormatTypeGoTemplate = "go-template" -) - // FormatType represents a custom description in help doc. type FormatType struct { Name string Usage string } +// WithUsage returns a new format type with provided usage string. +func (ft *FormatType) WithUsage(usage string) *FormatType { + return &FormatType{ + Name: ft.Name, + Usage: usage, + } +} + +// format types +var ( + FormatTypeJSON = &FormatType{ + Name: "json", + Usage: "Print in JSON format", + } + FormatTypeGoTemplate = &FormatType{ + Name: "go-template", + Usage: "Print in JSON format", + } + FormatTypeTable = &FormatType{ + Name: "table", + Usage: "Get direct referrers and output in table format", + } + FormatTypeTree = &FormatType{ + Name: "tree", + Usage: "Get referrers recursively and print in tree format", + } +) + // Format contains input and parsed options for formatted output flags. type Format struct { - Type string - Template string - // FormatFlag can be private once deprecated `--output` is removed from - // `oras discover` - FormatFlag string - types []FormatType + FormatFlag string + Type string + Template string + AllowedTypes []*FormatType } // ApplyFlag implements FlagProvider.ApplyFlag. func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { - usage := "[Experimental] Format output using a custom template:" - if len(opts.types) == 0 { - opts.types = []FormatType{ - {Name: FormatTypeJSON, Usage: "Print in JSON format"}, - {Name: FormatTypeGoTemplate, Usage: "Print output using the given Go template"}, - } + var buf bytes.Buffer + _, _ = buf.WriteString("[Experimental] Format output using a custom template:") + w := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) + if len(opts.AllowedTypes) == 0 { + opts.AllowedTypes = []*FormatType{FormatTypeJSON, FormatTypeGoTemplate} } - - // generate usage string - maxLength := 0 - for _, option := range opts.types { - if len(option.Name) > maxLength { - maxLength = len(option.Name) - } + for _, t := range opts.AllowedTypes { + _, _ = fmt.Fprintf(w, "\n'%s':\t%s", t.Name, t.Usage) } - for _, option := range opts.types { - usage += fmt.Sprintf("\n'%s':%s%s", option.Name, strings.Repeat(" ", maxLength-len(option.Name)+2), option.Usage) - } - // apply flags - fs.StringVar(&opts.FormatFlag, "format", opts.FormatFlag, usage) + fs.StringVar(&opts.FormatFlag, "format", opts.FormatFlag, buf.String()) fs.StringVar(&opts.Template, "template", "", "[Experimental] Template string used to format output") } @@ -85,13 +95,20 @@ func (opts *Format) Parse(_ *cobra.Command) error { return nil } + if opts.Type == FormatTypeGoTemplate.Name && opts.Template == "" { + return &oerrors.Error{ + Err: fmt.Errorf("%q format specified but no template given", opts.Type), + Recommendation: fmt.Sprintf("use `--format %q=TEMPLATE or --template TEMPLATE to specify the template", opts.Type), + } + } + var optionalTypes []string - for _, option := range opts.types { - if opts.Type == option.Name { + for _, t := range opts.AllowedTypes { + if opts.Type == t.Name { // type validation passed return nil } - optionalTypes = append(optionalTypes, option.Name) + optionalTypes = append(optionalTypes, t.Name) } return &oerrors.Error{ Err: fmt.Errorf("invalid format type: %q", opts.Type), @@ -103,39 +120,17 @@ func (opts *Format) parseFlag() error { opts.Type = opts.FormatFlag if opts.Template != "" { // template explicitly set - if opts.Type != FormatTypeGoTemplate { - return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate) + if opts.Type != FormatTypeGoTemplate.Name { + return fmt.Errorf("--template must be used with --format %s", FormatTypeGoTemplate.Name) } return nil } - goTemplatePrefix := FormatTypeGoTemplate + "=" + goTemplatePrefix := FormatTypeGoTemplate.Name + "=" if strings.HasPrefix(opts.FormatFlag, goTemplatePrefix) { // add parameter to template - opts.Type = FormatTypeGoTemplate + opts.Type = FormatTypeGoTemplate.Name opts.Template = opts.FormatFlag[len(goTemplatePrefix):] } return nil } - -// SetTypes resets the format options and default value. -func (opts *Format) SetTypes(types []FormatType) { - opts.types = types -} - -// SetTypesAndDefault resets the format options and default value. -// Caller should make sure that this function is used before applying flags. -func (opts *Format) SetTypesAndDefault(defaultType string, types []FormatType) { - opts.FormatFlag = defaultType - opts.types = types -} - -// FormatError generates the error message for an invalid type. -func (opts *Format) TypeError() error { - return fmt.Errorf("unsupported format type: %s", opts.Type) -} - -// RawFormatFlag returns raw input of --format flag. -func (opts *Format) RawFormatFlag() string { - return opts.FormatFlag -} diff --git a/cmd/oras/root/attach_test.go b/cmd/oras/root/attach_test.go index 1cf84e310..bbd443ee2 100644 --- a/cmd/oras/root/attach_test.go +++ b/cmd/oras/root/attach_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" ) @@ -35,7 +36,7 @@ func Test_runAttach_errType(t *testing.T) { }, } got := runAttach(cmd, opts).Error() - want := opts.TypeError().Error() + want := errors.UnsupportedFormatTypeError(opts.Format.Type).Error() if got != want { t.Fatalf("got %v, want %v", got, want) } diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index f29f75434..5cde8cfa4 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -99,12 +99,13 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") cmd.Flags().StringVarP(&opts.Format.FormatFlag, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") - opts.SetTypesAndDefault(option.FormatTypeTree, []option.FormatType{ - {Name: option.FormatTypeTree, Usage: "Get referrers recursively and print in tree format"}, - {Name: option.FormatTypeTable, Usage: "Get direct referrers and output in table format"}, - {Name: option.FormatTypeJSON, Usage: "Get direct referrers and output in JSON format"}, - {Name: option.FormatTypeGoTemplate, Usage: "Print direct referrers using the given Go template"}, - }) + opts.FormatFlag = option.FormatTypeTree.Name + opts.AllowedTypes = []*option.FormatType{ + option.FormatTypeTree, + option.FormatTypeTable, + option.FormatTypeJSON.WithUsage("Get direct referrers and output in JSON format"), + option.FormatTypeGoTemplate.WithUsage("Print direct referrers using the given Go template"), + } opts.EnableDistributionSpecFlag() option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) diff --git a/cmd/oras/root/manifest/fetch.go b/cmd/oras/root/manifest/fetch.go index 7ed5cb51f..c38afe413 100644 --- a/cmd/oras/root/manifest/fetch.go +++ b/cmd/oras/root/manifest/fetch.go @@ -73,7 +73,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': Args: oerrors.CheckArgs(argument.Exactly(1), "the manifest to fetch"), PreRunE: func(cmd *cobra.Command, args []string) error { switch { - case opts.outputPath == "-" && opts.RawFormatFlag() != "": + case opts.outputPath == "-" && opts.FormatFlag != "": return fmt.Errorf("`--output -` cannot be used with `--format %s` at the same time", opts.Template) case opts.outputPath == "-" && opts.OutputDescriptor: return fmt.Errorf("`--descriptor` cannot be used with `--output -` at the same time") @@ -98,10 +98,10 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar': cmd.Flags().StringSliceVarP(&opts.mediaTypes, "media-type", "", nil, "accepted media types") cmd.Flags().StringVarP(&opts.outputPath, "output", "o", "", "file `path` to write the fetched manifest to, use - for stdout") - opts.SetTypes([]option.FormatType{ - {Name: option.FormatTypeJSON, Usage: "Print in prettified JSON format"}, - {Name: option.FormatTypeGoTemplate, Usage: "Print using the given Go template"}, - }) + opts.AllowedTypes = []*option.FormatType{ + option.FormatTypeJSON.WithUsage("Print in prettified JSON format"), + option.FormatTypeGoTemplate.WithUsage("Print using the given Go template"), + } option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } diff --git a/cmd/oras/root/manifest/fetch_test.go b/cmd/oras/root/manifest/fetch_test.go index d13fc9f6e..7e4d7aa07 100644 --- a/cmd/oras/root/manifest/fetch_test.go +++ b/cmd/oras/root/manifest/fetch_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" ) @@ -35,7 +36,7 @@ func Test_fetchManifest_errType(t *testing.T) { }, } got := fetchManifest(cmd, opts).Error() - want := opts.TypeError().Error() + want := errors.UnsupportedFormatTypeError(opts.Format.Type).Error() if got != want { t.Fatalf("got %v, want %v", got, want) } diff --git a/cmd/oras/root/pull_test.go b/cmd/oras/root/pull_test.go index a0f1e5ff7..8e710c1fc 100644 --- a/cmd/oras/root/pull_test.go +++ b/cmd/oras/root/pull_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" ) @@ -35,7 +36,7 @@ func Test_runPull_errType(t *testing.T) { }, } got := runPull(cmd, opts).Error() - want := opts.TypeError().Error() + want := errors.UnsupportedFormatTypeError(opts.Format.Type).Error() if got != want { t.Fatalf("got %v, want %v", got, want) } diff --git a/cmd/oras/root/push_test.go b/cmd/oras/root/push_test.go index 22fbd9b79..296711979 100644 --- a/cmd/oras/root/push_test.go +++ b/cmd/oras/root/push_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/spf13/cobra" + "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" ) @@ -35,7 +36,7 @@ func Test_runPush_errType(t *testing.T) { }, } got := runPush(cmd, opts).Error() - want := opts.TypeError().Error() + want := errors.UnsupportedFormatTypeError(opts.Format.Type).Error() if got != want { t.Fatalf("got %v, want %v", got, want) } From 91584beee30a5370ed8e600cf30b74ab8714b551 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 13:41:56 +0000 Subject: [PATCH 22/32] add flushing Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index bb6c022c3..c79f52b94 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -79,6 +79,7 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { for _, t := range opts.AllowedTypes { _, _ = fmt.Fprintf(w, "\n'%s':\t%s", t.Name, t.Usage) } + w.Flush() // apply flags fs.StringVar(&opts.FormatFlag, "format", opts.FormatFlag, buf.String()) fs.StringVar(&opts.Template, "template", "", "[Experimental] Template string used to format output") From c1dfafdd1fab7e9b2ac2bd4f4d5eb79911059f46 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:01:54 +0000 Subject: [PATCH 23/32] add e2e Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- test/e2e/suite/command/pull.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index c79f52b94..3c08774a2 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -99,7 +99,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { if opts.Type == FormatTypeGoTemplate.Name && opts.Template == "" { return &oerrors.Error{ Err: fmt.Errorf("%q format specified but no template given", opts.Type), - Recommendation: fmt.Sprintf("use `--format %q=TEMPLATE or --template TEMPLATE to specify the template", opts.Type), + Recommendation: fmt.Sprintf("use `--format %q=TEMPLATE` or `--format %q --template TEMPLATE` to specify the template", opts.Type, opts.Type), } } diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index cc4af84c5..a8df471fb 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -111,6 +111,22 @@ var _ = Describe("ORAS beginners:", func() { ExpectFailure(). MatchErrKeyWords(invalidPrompt). Exec() + noTemplatePrompt := "format specified but no template given" + ORAS("pull", ref, "--format", "json", "--format", "go-template="). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords(noTemplatePrompt). + Exec() + ORAS("pull", ref, "--format", "json", "--format", "go-template=''"). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords(noTemplatePrompt). + Exec() + ORAS("pull", ref, "--format", "json", "--format", "go-template"). + WithWorkDir(tempDir). + ExpectFailure(). + MatchErrKeyWords(noTemplatePrompt). + Exec() }) It("should fail and show detailed error description if no argument provided", func() { From aa601104642db4367f61cfaa70301757f8f6a9a2 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:04:35 +0000 Subject: [PATCH 24/32] refine recommendation Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 3c08774a2..9f0393e98 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -99,7 +99,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { if opts.Type == FormatTypeGoTemplate.Name && opts.Template == "" { return &oerrors.Error{ Err: fmt.Errorf("%q format specified but no template given", opts.Type), - Recommendation: fmt.Sprintf("use `--format %q=TEMPLATE` or `--format %q --template TEMPLATE` to specify the template", opts.Type, opts.Type), + Recommendation: fmt.Sprintf("use `--format %s=TEMPLATE` or `--format %s --template TEMPLATE` to specify the template", opts.Type, opts.Type), } } From 5c53b246ea56edce838a0f16f9e8658cd8e64c29 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:11:30 +0000 Subject: [PATCH 25/32] add default format help doc Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 3 --- cmd/oras/root/attach.go | 1 + cmd/oras/root/pull.go | 1 + cmd/oras/root/push.go | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 9f0393e98..21dd68a98 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -73,9 +73,6 @@ func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { var buf bytes.Buffer _, _ = buf.WriteString("[Experimental] Format output using a custom template:") w := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) - if len(opts.AllowedTypes) == 0 { - opts.AllowedTypes = []*FormatType{FormatTypeJSON, FormatTypeGoTemplate} - } for _, t := range opts.AllowedTypes { _, _ = fmt.Fprintf(w, "\n'%s':\t%s", t.Name, t.Usage) } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 05df551d3..ddc77aeba 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -102,6 +102,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder opts.FlagDescription = "[Preview] attach to an arch-specific subject" _ = cmd.MarkFlagRequired("artifact-type") opts.EnableDistributionSpecFlag() + opts.AllowedTypes = []*option.FormatType{option.FormatTypeJSON, option.FormatTypeGoTemplate} option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index db12f4bad..061cd1b79 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -104,6 +104,7 @@ Example - Pull artifact files from an OCI layout archive 'layout.tar': cmd.Flags().StringVarP(&opts.Output, "output", "o", ".", "output directory") cmd.Flags().StringVarP(&opts.ManifestConfigRef, "config", "", "", "output manifest config file") cmd.Flags().IntVarP(&opts.concurrency, "concurrency", "", 3, "concurrency level") + opts.AllowedTypes = []*option.FormatType{option.FormatTypeJSON, option.FormatTypeGoTemplate} option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 5cc78053c..ad6145373 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -144,7 +144,7 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t cmd.Flags().StringVarP(&opts.manifestConfigRef, "config", "", "", "`path` of image config file") cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type") cmd.Flags().IntVarP(&opts.concurrency, "concurrency", "", 5, "concurrency level") - + opts.AllowedTypes = []*option.FormatType{option.FormatTypeJSON, option.FormatTypeGoTemplate} option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } From cc2c8e39fa11b58230961759564e08b26a908b3d Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:13:03 +0000 Subject: [PATCH 26/32] fix e2e Signed-off-by: Billy Zha --- test/e2e/suite/command/pull.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index a8df471fb..bbeafe43b 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -117,11 +117,6 @@ var _ = Describe("ORAS beginners:", func() { ExpectFailure(). MatchErrKeyWords(noTemplatePrompt). Exec() - ORAS("pull", ref, "--format", "json", "--format", "go-template=''"). - WithWorkDir(tempDir). - ExpectFailure(). - MatchErrKeyWords(noTemplatePrompt). - Exec() ORAS("pull", ref, "--format", "json", "--format", "go-template"). WithWorkDir(tempDir). ExpectFailure(). From 364729f64f2a2cac9de138fe5a1f0f7c38449e65 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:30:08 +0000 Subject: [PATCH 27/32] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/errors/errors.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/oras/internal/errors/errors.go b/cmd/oras/internal/errors/errors.go index 86526c252..6e48556ef 100644 --- a/cmd/oras/internal/errors/errors.go +++ b/cmd/oras/internal/errors/errors.go @@ -32,6 +32,7 @@ const RegistryErrorPrefix = "Error response from registry:" // UnsupportedFormatTypeError generates the error message for an invalid type. type UnsupportedFormatTypeError string +// Error implements the error interface. func (e UnsupportedFormatTypeError) Error() string { return "unsupported format type: " + string(e) } From e571ad20f3e7738476f3506407bc1879b3d9b59b Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:51:59 +0000 Subject: [PATCH 28/32] resolve nit comment Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 21dd68a98..e608cc01c 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -70,9 +70,8 @@ type Format struct { // ApplyFlag implements FlagProvider.ApplyFlag. func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { - var buf bytes.Buffer - _, _ = buf.WriteString("[Experimental] Format output using a custom template:") - w := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) + buf := bytes.NewBufferString("[Experimental] Format output using a custom template:") + w := tabwriter.NewWriter(buf, 0, 0, 2, ' ', 0) for _, t := range opts.AllowedTypes { _, _ = fmt.Fprintf(w, "\n'%s':\t%s", t.Name, t.Usage) } From bc81c38c26bf5ae80ae7f25f7b51755b918d8f9c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 14:52:58 +0000 Subject: [PATCH 29/32] recommend only primary usage Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index e608cc01c..6a4e2454a 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -95,7 +95,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { if opts.Type == FormatTypeGoTemplate.Name && opts.Template == "" { return &oerrors.Error{ Err: fmt.Errorf("%q format specified but no template given", opts.Type), - Recommendation: fmt.Sprintf("use `--format %s=TEMPLATE` or `--format %s --template TEMPLATE` to specify the template", opts.Type, opts.Type), + Recommendation: fmt.Sprintf("use `--format %s=TEMPLATE` to specify the template", opts.Type, opts.Type), } } From 61c5f1f5c56d1583112fad8c0f929c6178ca5631 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 15:04:37 +0000 Subject: [PATCH 30/32] correct description Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 6a4e2454a..a2f9b2de3 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -48,7 +48,7 @@ var ( } FormatTypeGoTemplate = &FormatType{ Name: "go-template", - Usage: "Print in JSON format", + Usage: "Print output using the given Go template", } FormatTypeTable = &FormatType{ Name: "table", From 47c35ecdfeb859c643575a6d68f4f3a11fb04949 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 15:06:36 +0000 Subject: [PATCH 31/32] fix build error Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index a2f9b2de3..41b25e91c 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -95,7 +95,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { if opts.Type == FormatTypeGoTemplate.Name && opts.Template == "" { return &oerrors.Error{ Err: fmt.Errorf("%q format specified but no template given", opts.Type), - Recommendation: fmt.Sprintf("use `--format %s=TEMPLATE` to specify the template", opts.Type, opts.Type), + Recommendation: fmt.Sprintf("use `--format %s=TEMPLATE` to specify the template", opts.Type), } } From 70cd7f068470c1aa5b2c3045dbe4bc5ffc0f4b99 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 13 May 2024 15:17:08 +0000 Subject: [PATCH 32/32] add parameter to format type Signed-off-by: Billy Zha --- cmd/oras/internal/option/format.go | 33 ++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 41b25e91c..888fa6bf7 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -26,17 +26,22 @@ import ( oerrors "oras.land/oras/cmd/oras/internal/errors" ) -// FormatType represents a custom description in help doc. +// FormatType represents a format type. type FormatType struct { - Name string + // Name is the format type name. + Name string + // Usage is the usage string in help doc. Usage string + // HasParams indicates whether the format type has parameters. + HasParams bool } // WithUsage returns a new format type with provided usage string. func (ft *FormatType) WithUsage(usage string) *FormatType { return &FormatType{ - Name: ft.Name, - Usage: usage, + Name: ft.Name, + HasParams: ft.HasParams, + Usage: usage, } } @@ -47,8 +52,9 @@ var ( Usage: "Print in JSON format", } FormatTypeGoTemplate = &FormatType{ - Name: "go-template", - Usage: "Print output using the given Go template", + Name: "go-template", + Usage: "Print output using the given Go template", + HasParams: true, } FormatTypeTable = &FormatType{ Name: "table", @@ -123,11 +129,16 @@ func (opts *Format) parseFlag() error { return nil } - goTemplatePrefix := FormatTypeGoTemplate.Name + "=" - if strings.HasPrefix(opts.FormatFlag, goTemplatePrefix) { - // add parameter to template - opts.Type = FormatTypeGoTemplate.Name - opts.Template = opts.FormatFlag[len(goTemplatePrefix):] + for _, t := range opts.AllowedTypes { + if !t.HasParams { + continue + } + prefix := t.Name + "=" + if strings.HasPrefix(opts.FormatFlag, prefix) { + // parse type and add parameter to template + opts.Type = t.Name + opts.Template = opts.FormatFlag[len(prefix):] + } } return nil }