From 4440be19e052e8d48af8cef2244ff92bf93f2ca6 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 18 Jun 2024 05:48:40 -0600 Subject: [PATCH] chore: Add format type text (#1397) Signed-off-by: Terry Howe Signed-off-by: Billy Zha Co-authored-by: Billy Zha --- cmd/oras/internal/display/handler.go | 16 ++++----- cmd/oras/internal/display/handler_test.go | 44 +++++++++++++++++++++++ cmd/oras/internal/option/format.go | 24 +++++++++---- cmd/oras/root/attach.go | 2 +- cmd/oras/root/discover.go | 5 ++- cmd/oras/root/manifest/fetch.go | 7 ++-- cmd/oras/root/pull.go | 2 +- cmd/oras/root/push.go | 2 +- test/e2e/internal/utils/help.go | 37 +++++++++++++++++++ test/e2e/suite/command/attach.go | 4 +++ test/e2e/suite/command/discover.go | 4 +++ test/e2e/suite/command/manifest.go | 4 +++ test/e2e/suite/command/pull.go | 4 +++ test/e2e/suite/command/push.go | 4 +++ 14 files changed, 135 insertions(+), 24 deletions(-) create mode 100644 cmd/oras/internal/display/handler_test.go create mode 100644 test/e2e/internal/utils/help.go diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index 3b3da398c..367df14ec 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -39,7 +39,7 @@ func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose b var statusHandler status.PushHandler if tty != nil { statusHandler = status.NewTTYPushHandler(tty) - } else if format.Type == "" { + } else if format.Type == option.FormatTypeText.Name { statusHandler = status.NewTextPushHandler(out, verbose) } else { statusHandler = status.NewDiscardHandler() @@ -47,7 +47,7 @@ func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose b var metadataHandler metadata.PushHandler switch format.Type { - case "": + case option.FormatTypeText.Name: metadataHandler = text.NewPushHandler(out) case option.FormatTypeJSON.Name: metadataHandler = json.NewPushHandler(out) @@ -64,7 +64,7 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose var statusHandler status.AttachHandler if tty != nil { statusHandler = status.NewTTYAttachHandler(tty) - } else if format.Type == "" { + } else if format.Type == option.FormatTypeText.Name { statusHandler = status.NewTextAttachHandler(out, verbose) } else { statusHandler = status.NewDiscardHandler() @@ -72,7 +72,7 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose var metadataHandler metadata.AttachHandler switch format.Type { - case "": + case option.FormatTypeText.Name: metadataHandler = text.NewAttachHandler(out) case option.FormatTypeJSON.Name: metadataHandler = json.NewAttachHandler(out) @@ -89,7 +89,7 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi var statusHandler status.PullHandler if tty != nil { statusHandler = status.NewTTYPullHandler(tty) - } else if format.Type == "" { + } else if format.Type == option.FormatTypeText.Name { statusHandler = status.NewTextPullHandler(out, verbose) } else { statusHandler = status.NewDiscardHandler() @@ -97,7 +97,7 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi var metadataHandler metadata.PullHandler switch format.Type { - case "": + case option.FormatTypeText.Name: metadataHandler = text.NewPullHandler(out) case option.FormatTypeJSON.Name: metadataHandler = json.NewPullHandler(out, path) @@ -113,7 +113,7 @@ 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.Name, "": + case option.FormatTypeTree.Name: handler = tree.NewDiscoverHandler(out, path, desc, verbose) case option.FormatTypeTable.Name: handler = table.NewDiscoverHandler(out, rawReference, desc, verbose) @@ -133,7 +133,7 @@ func NewManifestFetchHandler(out io.Writer, format option.Format, outputDescript var contentHandler content.ManifestFetchHandler switch format.Type { - case "": + case option.FormatTypeText.Name: // raw if outputDescriptor { metadataHandler = descriptor.NewManifestFetchHandler(out, pretty) diff --git a/cmd/oras/internal/display/handler_test.go b/cmd/oras/internal/display/handler_test.go new file mode 100644 index 000000000..0373aec4c --- /dev/null +++ b/cmd/oras/internal/display/handler_test.go @@ -0,0 +1,44 @@ +/* +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 display + +import ( + "os" + "testing" + + "oras.land/oras/cmd/oras/internal/option" +) + +func TestNewPushHandler(t *testing.T) { + _, _, err := NewPushHandler(os.Stdout, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, false) + if err != nil { + t.Errorf("NewPushHandler() error = %v, want nil", err) + } +} + +func TestNewAttachHandler(t *testing.T) { + _, _, err := NewAttachHandler(os.Stdout, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, false) + if err != nil { + t.Errorf("NewAttachHandler() error = %v, want nil", err) + } +} + +func TestNewPullHandler(t *testing.T) { + _, _, err := NewPullHandler(os.Stdout, option.Format{Type: option.FormatTypeText.Name}, "", os.Stdout, false) + if err != nil { + t.Errorf("NewPullHandler() error = %v, want nil", err) + } +} diff --git a/cmd/oras/internal/option/format.go b/cmd/oras/internal/option/format.go index 888fa6bf7..76acdc105 100644 --- a/cmd/oras/internal/option/format.go +++ b/cmd/oras/internal/option/format.go @@ -64,6 +64,10 @@ var ( Name: "tree", Usage: "Get referrers recursively and print in tree format", } + FormatTypeText = &FormatType{ + Name: "text", + Usage: "Print in text format", + } ) // Format contains input and parsed options for formatted output flags. @@ -71,17 +75,23 @@ type Format struct { FormatFlag string Type string Template string - AllowedTypes []*FormatType + allowedTypes []*FormatType +} + +// SetTypes sets the default format type and allowed format types. +func (f *Format) SetTypes(defaultType *FormatType, otherTypes ...*FormatType) { + f.FormatFlag = defaultType.Name + f.allowedTypes = append(otherTypes, defaultType) } -// ApplyFlag implements FlagProvider.ApplyFlag. +// ApplyFlags implements FlagProvider.ApplyFlag. func (opts *Format) ApplyFlags(fs *pflag.FlagSet) { buf := bytes.NewBufferString("[Experimental] Format output using a custom template:") w := tabwriter.NewWriter(buf, 0, 0, 2, ' ', 0) - for _, t := range opts.AllowedTypes { + for _, t := range opts.allowedTypes { _, _ = fmt.Fprintf(w, "\n'%s':\t%s", t.Name, t.Usage) } - w.Flush() + _ = 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") @@ -93,7 +103,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { return err } - if opts.Type == "" { + if opts.Type == FormatTypeText.Name { // flag not specified return nil } @@ -106,7 +116,7 @@ func (opts *Format) Parse(_ *cobra.Command) error { } var optionalTypes []string - for _, t := range opts.AllowedTypes { + for _, t := range opts.allowedTypes { if opts.Type == t.Name { // type validation passed return nil @@ -129,7 +139,7 @@ func (opts *Format) parseFlag() error { return nil } - for _, t := range opts.AllowedTypes { + for _, t := range opts.allowedTypes { if !t.HasParams { continue } diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index ddc77aeba..c7d447927 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -102,7 +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} + opts.SetTypes(option.FormatTypeText, option.FormatTypeJSON, option.FormatTypeGoTemplate) option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go index 5cde8cfa4..381b3db4c 100644 --- a/cmd/oras/root/discover.go +++ b/cmd/oras/root/discover.go @@ -99,13 +99,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.Format.FormatFlag, "output", "o", "tree", "[Deprecated] format in which to display referrers (table, json, or tree). tree format will also show indirect referrers") - opts.FormatFlag = option.FormatTypeTree.Name - opts.AllowedTypes = []*option.FormatType{ + opts.SetTypes( 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 c38afe413..7f6e388be 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.FormatFlag != "": + case opts.outputPath == "-" && opts.FormatFlag != option.FormatTypeText.Name: 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,11 @@ 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.AllowedTypes = []*option.FormatType{ + opts.SetTypes( + option.FormatTypeText, 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/pull.go b/cmd/oras/root/pull.go index 061cd1b79..7cc1e0874 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -104,7 +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} + opts.SetTypes(option.FormatTypeText, 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 ad6145373..689c36491 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} + opts.SetTypes(option.FormatTypeText, option.FormatTypeJSON, option.FormatTypeGoTemplate) option.ApplyFlags(&opts, cmd.Flags()) return oerrors.Command(cmd, &opts.Target) } diff --git a/test/e2e/internal/utils/help.go b/test/e2e/internal/utils/help.go new file mode 100644 index 000000000..6dc926273 --- /dev/null +++ b/test/e2e/internal/utils/help.go @@ -0,0 +1,37 @@ +/* +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 utils + +import ( + "fmt" + "strings" + + "github.com/onsi/gomega" +) + +// MatchDefaultFlagValue checks if the flag is found in the output and has the +// default value. +func MatchDefaultFlagValue(flag string, defaultValue string, CommandPath ...string) { + CommandPath = append(CommandPath, "-h") + out := ORAS(CommandPath...).Exec().Out.Contents() + _, help, _ := strings.Cut(string(out), "Flags:") + var found bool + _, help, found = strings.Cut(help, fmt.Sprintf("--%s", flag)) + gomega.Expect(found).Should(gomega.BeTrue(), "%q not found in %q", flag, help) + help, _, _ = strings.Cut(help, "\n -") + help, _, _ = strings.Cut(help, "\n --") + gomega.Expect(help).Should(gomega.ContainSubstring(fmt.Sprintf("(default %q)", defaultValue))) +} diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go index 3f14f915a..026d11575 100644 --- a/test/e2e/suite/command/attach.go +++ b/test/e2e/suite/command/attach.go @@ -45,6 +45,10 @@ var _ = Describe("ORAS beginners:", func() { gomega.Expect(out).Should(gbytes.Say("--distribution-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark))) }) + It("should show text as default format type in help doc", func() { + MatchDefaultFlagValue("format", "text", "attach") + }) + It("should fail when no subject reference provided", func() { ORAS("attach", "--artifact-type", "oras/test").ExpectFailure().MatchErrKeyWords("Error:").Exec() }) diff --git a/test/e2e/suite/command/discover.go b/test/e2e/suite/command/discover.go index 8cdcf5ff1..977c1b03c 100644 --- a/test/e2e/suite/command/discover.go +++ b/test/e2e/suite/command/discover.go @@ -56,6 +56,10 @@ var _ = Describe("ORAS beginners:", func() { gomega.Expect(out).Should(gbytes.Say("--distribution-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark))) }) + It("should show text as default format type in help doc", func() { + MatchDefaultFlagValue("format", "tree", "discover") + }) + It("should fail when no subject reference provided", func() { ORAS("discover").ExpectFailure().MatchErrKeyWords("Error:").Exec() }) diff --git a/test/e2e/suite/command/manifest.go b/test/e2e/suite/command/manifest.go index adbf34ee7..a1f99ea59 100644 --- a/test/e2e/suite/command/manifest.go +++ b/test/e2e/suite/command/manifest.go @@ -93,6 +93,10 @@ var _ = Describe("ORAS beginners:", func() { Exec() }) + It("should show text as default format type in help doc", func() { + MatchDefaultFlagValue("format", "text", "manifest", "fetch") + }) + It("should fail and show detailed error description if no argument provided", func() { err := ORAS("manifest", "fetch").ExpectFailure().Exec().Err gomega.Expect(err).Should(gbytes.Say("Error")) diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index bbeafe43b..cb9e7a666 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -43,6 +43,10 @@ var _ = Describe("ORAS beginners:", func() { gomega.Expect(out).Should(gbytes.Say("--include-subject\\s+%s", regexp.QuoteMeta(feature.Preview.Mark))) }) + It("should show text as default format type in help doc", func() { + MatchDefaultFlagValue("format", "text", "pull") + }) + hintMsg := func(reference string) string { return fmt.Sprintf("Skipped pulling layers without file name in \"org.opencontainers.image.title\"\nUse 'oras copy %s --to-oci-layout ' to pull all layers.\n", reference) } diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index 8ed2c2595..5cb644a66 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -41,6 +41,10 @@ var _ = Describe("ORAS beginners:", func() { gomega.Expect(out).Should(gbytes.Say("--image-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark))) }) + It("should show text as default format type in help doc", func() { + MatchDefaultFlagValue("format", "text", "push") + }) + It("should fail and show detailed error description if no argument provided", func() { err := ORAS("push").ExpectFailure().Exec().Err gomega.Expect(err).Should(gbytes.Say("Error"))