Skip to content

fix(cmdutil): standardize -o output-format dispatch across list commands #57

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
shreemaan-abhishek merged 1 commit into from
May 27, 2026
+105 −13
Merged

fix(cmdutil): standardize -o output-format dispatch across list commands #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/cmd/consumer/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
Comment on lines 42 to 47
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Handle flag parsing errors instead of discarding them.

Line 42, Line 46, and Line 47 ignore GetString errors. If flag registration changes, this silently falls back to zero values and masks the root cause.

Proposed fix 
-			opts.Output, _ = c.Flags().GetString("output")
+			var err error
+			opts.Output, err = c.Flags().GetString("output")
+			if err != nil {
+				return err
+			}
 			if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
 				return err
 			}
-			opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
-			opts.Label, _ = c.Flags().GetString("label")
+			opts.GatewayGroup, err = c.Flags().GetString("gateway-group")
+			if err != nil {
+				return err
+			}
+			opts.Label, err = c.Flags().GetString("label")
+			if err != nil {
+				return err
+			}
 			return actionRun(opts)

As per coding guidelines, **/*.go: Never suppress errors. Always handle and propagate errors explicitly; and **/*.{js,ts,tsx,go}: every function return value must be checked for errors when applicable.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/cmd/consumer/list/list.go` around lines 42 - 47, The calls to
c.Flags().GetString for opts.Output, opts.GatewayGroup and opts.Label currently
ignore errors; update the code in the function that sets these fields so you
capture and handle the returned error from each GetString call (e.g., err :=
c.Flags().GetString("output"); if err != nil { return err }) before calling
cmdutil.ValidateOutputFormat(opts.Output), and likewise for "gateway-group" and
"label" — return or propagate any flag parsing error instead of discarding it to
avoid silently using zero values.

return actionRun(opts)
Expand Down Expand Up @@ -95,7 +98,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
return cmdutil.NewExporter(opts.Output, opts.IO.Out).Write(resp.List)
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/context/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
return listRun(opts, f)
},
}
Expand All @@ -49,7 +52,7 @@ func listRun(opts *Options, f *cmd.Factory) error {
return nil
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
type contextJSON struct {
Name string `json:"name"`
Server string `json:"server"`
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/credential/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Consumer, _ = c.Flags().GetString("consumer")
opts.Label, _ = c.Flags().GetString("label")
Expand Down Expand Up @@ -98,7 +101,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
return cmdutil.NewExporter(opts.Output, opts.IO.Out).Write(resp.List)
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/gateway-group/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.Label, _ = c.Flags().GetString("label")
return listRun(opts)
},
Expand Down Expand Up @@ -66,7 +69,7 @@ func listRun(opts *Options) error {
return fmt.Errorf("failed to list gateway groups: %s", cmdutil.FormatAPIError(err))
}

if opts.Output == "json" || opts.Output == "yaml" {
if cmdutil.IsStructuredOutput(opts.Output) {
exp := cmdutil.NewExporter(opts.Output, opts.IO.Out)
var result api.ListResponse[api.GatewayGroup]
if err := json.Unmarshal(body, &result); err != nil {
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/global-rule/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
return listRun(opts)
Expand Down Expand Up @@ -88,7 +91,7 @@ func listRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
return exporter.Write(resp.List)
}
Expand Down
7 changes: 5 additions & 2 deletions pkg/cmd/plugin/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
return actionRun(opts)
},
Expand Down Expand Up @@ -74,8 +77,8 @@ func actionRun(opts *Options) error {
return fmt.Errorf("failed to parse plugin list response: %w", err)
}

if opts.Output == "json" {
return cmdutil.NewExporter("json", opts.IO.Out).Write(plugins)
if cmdutil.IsStructuredOutput(opts.Output) {
return cmdutil.NewExporter(opts.Output, opts.IO.Out).Write(plugins)
}

for _, name := range plugins {
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/proto/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
return actionRun(opts)
Expand Down Expand Up @@ -88,7 +91,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
return exporter.Write(resp.List)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/route/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
opts.ServiceID, _ = c.Flags().GetString("service-id")
Expand Down Expand Up @@ -85,7 +88,7 @@ func actionRun(opts *Options) error {
routes = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
return exporter.Write(routes)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/secret/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
return actionRun(opts)
Expand Down Expand Up @@ -88,7 +91,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
return exporter.Write(api.RedactSecrets(resp.List))
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/service/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
return actionRun(opts)
Expand Down Expand Up @@ -87,7 +90,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
return exporter.Write(resp.List)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/ssl/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
return actionRun(opts)
Expand Down Expand Up @@ -95,7 +98,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
return cmdutil.NewExporter(opts.Output, opts.IO.Out).Write(api.RedactSSLs(resp.List))
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/stream-route/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
Args: cobra.NoArgs,
RunE: func(c *cobra.Command, args []string) error {
opts.Output, _ = c.Flags().GetString("output")
if err := cmdutil.ValidateOutputFormat(opts.Output); err != nil {
return err
}
opts.GatewayGroup, _ = c.Flags().GetString("gateway-group")
opts.Label, _ = c.Flags().GetString("label")
opts.ServiceID, _ = c.Flags().GetString("service-id")
Expand Down Expand Up @@ -95,7 +98,7 @@ func actionRun(opts *Options) error {
resp.List = filtered
}

if opts.Output != "" {
if cmdutil.IsStructuredOutput(opts.Output) {
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
return exporter.Write(resp.List)
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/cmdutil/exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,30 @@ import (
"gopkg.in/yaml.v3"
)

// ValidateOutputFormat returns nil when the given --output value is one of
// the four shapes the CLI claims to support: empty (default table), "table",
// "json", "yaml". Anything else (typos, abbreviations, unknown formats) is
// rejected with a clear message listing the valid set.
//
// Every command that accepts `-o`/`--output` should call this before
// dispatching, so a typo like `-o jzon` fails fast with a helpful error
// instead of silently falling back to table rendering.
func ValidateOutputFormat(format string) error {
switch format {
case "", "table", "json", "yaml":
return nil
default:
return fmt.Errorf("unsupported output format: %q (valid: table, json, yaml)", format)
}
}

// IsStructuredOutput returns true when the format should be rendered through
// the JSON/YAML exporter, false when the command should fall through to its
// own table renderer. Pairs with ValidateOutputFormat.
func IsStructuredOutput(format string) bool {
return format == "json" || format == "yaml"
}

// Exporter formats and writes data to the given writer.
type Exporter struct {
format string
Expand Down
32 changes: 32 additions & 0 deletions pkg/cmdutil/exporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,35 @@ func TestExporter_WriteAPIResponse_InvalidJSON_ReturnsError(t *testing.T) {
t.Fatalf("error did not mention decoding: %v", err)
}
}

func TestValidateOutputFormat(t *testing.T) {
for _, ok := range []string{"", "table", "json", "yaml"} {
if err := ValidateOutputFormat(ok); err != nil {
t.Errorf("expected %q to be accepted, got %v", ok, err)
}
}
for _, bad := range []string{"jzon", "yml", "TABLE", "csv", "totally-not-a-format"} {
err := ValidateOutputFormat(bad)
if err == nil {
t.Errorf("expected %q to be rejected", bad)
continue
}
if !strings.Contains(err.Error(), "valid: table, json, yaml") {
t.Errorf("error for %q should list the valid set, got: %v", bad, err)
}
}
}

func TestIsStructuredOutput(t *testing.T) {
cases := map[string]bool{
"": false,
"table": false,
"json": true,
"yaml": true,
}
for format, want := range cases {
if got := IsStructuredOutput(format); got != want {
t.Errorf("IsStructuredOutput(%q) = %v, want %v", format, got, want)
}
}
}
Comment on lines +65 to +95
Loading