diff --git a/pkg/cli/clivalidation.go b/pkg/cli/clivalidation.go index 8f59d57a92..66c4dc9df2 100644 --- a/pkg/cli/clivalidation.go +++ b/pkg/cli/clivalidation.go @@ -354,6 +354,8 @@ func RequireAzureSubscriptionId(cmd *cobra.Command) (string, error) { // RequireOutput reads the output format flag, normalizes it (trim and lowercase), and validates // it against the supported formats. Returns an error if the format is not supported. +// +// For backwards compatibility, "plain-text" is silently normalized to "table". func RequireOutput(cmd *cobra.Command) (string, error) { format, err := cmd.Flags().GetString("output") if err != nil { @@ -366,14 +368,16 @@ func RequireOutput(cmd *cobra.Command) (string, error) { return output.DefaultFormat, nil } - allFormats := output.AllFormats() - for _, f := range allFormats { + // Backwards compatibility: accept deprecated aliases silently. + format = output.NormalizeFormat(format) + + for _, f := range output.SupportedFormats() { if format == f { return format, nil } } - return "", clierrors.Message("unsupported output format %q, supported formats are: %s", format, strings.Join(allFormats, ", ")) + return "", clierrors.Message("unsupported output format %q, supported formats are: %s", format, strings.Join(output.SupportedFormats(), ", ")) } // RequireWorkspace is used by commands that require an existing workspace either set as the default, diff --git a/pkg/cli/clivalidation_test.go b/pkg/cli/clivalidation_test.go index cb3384bb04..ab264caf7f 100644 --- a/pkg/cli/clivalidation_test.go +++ b/pkg/cli/clivalidation_test.go @@ -21,6 +21,8 @@ import ( "fmt" "testing" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" ) @@ -168,3 +170,71 @@ func Test_ReadResourceTypeNameArgs(t *testing.T) { }) } } + +// newCmdWithOutputFlag creates a cobra command with a string flag named "output" set to the given value. +func newCmdWithOutputFlag(value string) *cobra.Command { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().StringP("output", "o", "", "output format") + err := cmd.Flags().Set("output", value) + if err != nil { + panic(err) + } + return cmd +} + +func Test_RequireOutput(t *testing.T) { + tests := []struct { + name string + format string + want string + wantErr bool + errSubstr string + }{ + { + name: "json is accepted", + format: "json", + want: "json", + }, + { + name: "table is accepted", + format: "table", + want: "table", + }, + { + name: "empty defaults to table", + format: "", + want: output.DefaultFormat, + }, + { + name: "plain-text is normalized to table", + format: "plain-text", + want: "table", + }, + { + name: "text is rejected", + format: "text", + wantErr: true, + errSubstr: `unsupported output format "text", supported formats are: json, table`, + }, + { + name: "unknown format is rejected", + format: "xml", + wantErr: true, + errSubstr: `unsupported output format "xml", supported formats are: json, table`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := newCmdWithOutputFlag(tt.format) + got, err := RequireOutput(cmd) + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errSubstr) + } else { + require.NoError(t, err) + require.Equal(t, tt.want, got) + } + }) + } +} diff --git a/pkg/cli/cmd/commonflags/flags.go b/pkg/cli/cmd/commonflags/flags.go index be46fd1799..0b45c34a69 100644 --- a/pkg/cli/cmd/commonflags/flags.go +++ b/pkg/cli/cmd/commonflags/flags.go @@ -49,10 +49,6 @@ func AddOutputFlag(cmd *cobra.Command) { cmd.Flags().StringP("output", "o", output.DefaultFormat, description) } -func AddOutputFlagWithPlainText(cmd *cobra.Command) { - description := fmt.Sprintf("output format (supported formats are %s)", strings.Join([]string{output.FormatPlainText, output.FormatJson}, ", ")) - cmd.Flags().StringP("output", "o", output.FormatPlainText, description) -} // AddWorkspaceFlag adds a flag to the given command that allows the user to specify a workspace name. func AddWorkspaceFlag(cmd *cobra.Command) { diff --git a/pkg/cli/cmd/recipepack/show/show.go b/pkg/cli/cmd/recipepack/show/show.go index 5c498a26cf..c55c291c8d 100644 --- a/pkg/cli/cmd/recipepack/show/show.go +++ b/pkg/cli/cmd/recipepack/show/show.go @@ -52,7 +52,7 @@ rad recipe-show show my-recipe-pack --group my-group RunE: framework.RunCommand(runner), } - commonflags.AddOutputFlagWithPlainText(cmd) + commonflags.AddOutputFlag(cmd) commonflags.AddResourceGroupFlag(cmd) commonflags.AddWorkspaceFlag(cmd) @@ -130,7 +130,7 @@ func (r *Runner) Run(ctx context.Context) error { return err } - if r.Format != "json" { + if r.Format != output.FormatJson { err = r.Output.WriteFormatted(output.FormatTable, recipePack, objectformats.GetRecipePackTableFormat()) if err != nil { return err diff --git a/pkg/cli/cmd/recipepack/show/show_test.go b/pkg/cli/cmd/recipepack/show/show_test.go index c992be240c..16602e366c 100644 --- a/pkg/cli/cmd/recipepack/show/show_test.go +++ b/pkg/cli/cmd/recipepack/show/show_test.go @@ -83,9 +83,6 @@ func Test_Validate(t *testing.T) { } func Test_Run(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - recipePack := corerpv20250801preview.RecipePackResource{ Name: to.Ptr("sample-pack"), Properties: &corerpv20250801preview.RecipePackProperties{ @@ -101,40 +98,84 @@ func Test_Run(t *testing.T) { }, } - appMgmtClient := clients.NewMockApplicationsManagementClient(ctrl) - appMgmtClient.EXPECT(). - GetRecipePack(gomock.Any(), "sample-pack"). - Return(recipePack, nil). - Times(1) + t.Run("json format", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - workspace := &workspaces.Workspace{ - Connection: map[string]any{ - "kind": "kubernetes", - "context": "kind-kind", - }, - Name: "kind-kind", - Scope: "/planes/radius/local/resourceGroups/test-group", - } + appMgmtClient := clients.NewMockApplicationsManagementClient(ctrl) + appMgmtClient.EXPECT(). + GetRecipePack(gomock.Any(), "sample-pack"). + Return(recipePack, nil). + Times(1) - outputSink := &output.MockOutput{} - runner := &Runner{ - ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: appMgmtClient}, - Workspace: workspace, - Output: outputSink, - RecipePackName: "sample-pack", - Format: "json", - } + workspace := &workspaces.Workspace{ + Connection: map[string]any{ + "kind": "kubernetes", + "context": "kind-kind", + }, + Name: "kind-kind", + Scope: "/planes/radius/local/resourceGroups/test-group", + } + + outputSink := &output.MockOutput{} + runner := &Runner{ + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: appMgmtClient}, + Workspace: workspace, + Output: outputSink, + RecipePackName: "sample-pack", + Format: "json", + } + + err := runner.Run(context.Background()) + require.NoError(t, err) + + expected := []any{ + output.FormattedOutput{ + Format: "json", + Obj: recipePack, + Options: objectformats.GetRecipePackTableFormat(), + }, + } - err := runner.Run(context.Background()) - require.NoError(t, err) + require.Equal(t, expected, outputSink.Writes) + }) - expected := []any{ - output.FormattedOutput{ - Format: "json", - Obj: recipePack, - Options: objectformats.GetRecipePackTableFormat(), - }, - } + t.Run("table format", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - require.Equal(t, expected, outputSink.Writes) + appMgmtClient := clients.NewMockApplicationsManagementClient(ctrl) + appMgmtClient.EXPECT(). + GetRecipePack(gomock.Any(), "sample-pack"). + Return(recipePack, nil). + Times(1) + + workspace := &workspaces.Workspace{ + Connection: map[string]any{ + "kind": "kubernetes", + "context": "kind-kind", + }, + Name: "kind-kind", + Scope: "/planes/radius/local/resourceGroups/test-group", + } + + outputSink := &output.MockOutput{} + runner := &Runner{ + ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: appMgmtClient}, + Workspace: workspace, + Output: outputSink, + RecipePackName: "sample-pack", + Format: "table", + } + + err := runner.Run(context.Background()) + require.NoError(t, err) + + // Table format produces a table write followed by display (LogInfo) output + require.NotEmpty(t, outputSink.Writes) + firstWrite, ok := outputSink.Writes[0].(output.FormattedOutput) + require.True(t, ok) + require.Equal(t, "table", firstWrite.Format) + require.Equal(t, recipePack, firstWrite.Obj) + }) } diff --git a/pkg/cli/cmd/version/version.go b/pkg/cli/cmd/version/version.go index 3e5fe2c8fa..f7b6de0f1c 100644 --- a/pkg/cli/cmd/version/version.go +++ b/pkg/cli/cmd/version/version.go @@ -3,7 +3,9 @@ package version import ( "context" + "github.com/radius-project/radius/pkg/cli" "github.com/radius-project/radius/pkg/cli/bicep" + "github.com/radius-project/radius/pkg/cli/cmd/commonflags" "github.com/radius-project/radius/pkg/cli/framework" "github.com/radius-project/radius/pkg/cli/helm" "github.com/radius-project/radius/pkg/cli/output" @@ -79,6 +81,7 @@ rad version --cli`, RunE: framework.RunCommand(runner), } + commonflags.AddOutputFlag(cmd) cmd.Flags().Bool("cli", false, "Use this flag to only show the rad CLI version") return cmd, runner } @@ -102,13 +105,10 @@ func NewRunner(factory framework.Factory) *Runner { // Validate validates the command arguments func (r *Runner) Validate(cmd *cobra.Command, args []string) error { - format, err := cmd.Flags().GetString("output") + format, err := cli.RequireOutput(cmd) if err != nil { return err } - if format == "" { - format = "table" - } r.Format = format cliOnly, err := cmd.Flags().GetBool("cli") diff --git a/pkg/cli/output/formats.go b/pkg/cli/output/formats.go index 9565c87155..459b91f30c 100644 --- a/pkg/cli/output/formats.go +++ b/pkg/cli/output/formats.go @@ -31,12 +31,12 @@ func SupportedFormats() []string { } } -// AllFormats returns all recognized output format strings, including formats -// that are only available for specific commands (e.g., plain-text). -func AllFormats() []string { - return []string{ - FormatJson, - FormatTable, - FormatPlainText, +// NormalizeFormat maps deprecated format aliases to their canonical form. +// Currently maps "plain-text" → "table" for backwards compatibility. +// Returns the input unchanged if it is not an alias. +func NormalizeFormat(format string) string { + if format == FormatPlainText { + return FormatTable } + return format } diff --git a/pkg/cli/output/formats_test.go b/pkg/cli/output/formats_test.go new file mode 100644 index 0000000000..685bf5e37f --- /dev/null +++ b/pkg/cli/output/formats_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2023 The Radius 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 output + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_NormalizeFormat(t *testing.T) { + tests := []struct { + name string + input string + expect string + }{ + { + name: "plain-text maps to table", + input: "plain-text", + expect: "table", + }, + { + name: "json is unchanged", + input: "json", + expect: "json", + }, + { + name: "table is unchanged", + input: "table", + expect: "table", + }, + { + name: "unknown values pass through unchanged", + input: "xml", + expect: "xml", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NormalizeFormat(tt.input) + require.Equal(t, tt.expect, got) + }) + } +} diff --git a/pkg/cli/output/formatter.go b/pkg/cli/output/formatter.go index 4777b1b9c4..8962144911 100644 --- a/pkg/cli/output/formatter.go +++ b/pkg/cli/output/formatter.go @@ -51,10 +51,10 @@ func NewFormatter(format string) (Formatter, error) { switch normalized { case FormatJson: return &JSONFormatter{}, nil - case FormatTable: + case FormatTable, FormatPlainText: return &TableFormatter{}, nil default: - return nil, fmt.Errorf("unsupported format %s", format) + return nil, fmt.Errorf("unsupported format %q, supported formats are: %s", format, strings.Join(SupportedFormats(), ", ")) } } diff --git a/pkg/cli/output/formatter_test.go b/pkg/cli/output/formatter_test.go new file mode 100644 index 0000000000..e4c8102f9b --- /dev/null +++ b/pkg/cli/output/formatter_test.go @@ -0,0 +1,78 @@ +/* +Copyright 2023 The Radius 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 output + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_NewFormatter(t *testing.T) { + tests := []struct { + name string + format string + expectType string + expectError bool + }{ + { + name: "json returns JSONFormatter", + format: "json", + expectType: "*output.JSONFormatter", + }, + { + name: "table returns TableFormatter", + format: "table", + expectType: "*output.TableFormatter", + }, + { + name: "plain-text returns TableFormatter", + format: "plain-text", + expectType: "*output.TableFormatter", + }, + { + name: "unsupported format returns error", + format: "xml", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + formatter, err := NewFormatter(tt.format) + if tt.expectError { + require.Error(t, err) + require.Nil(t, formatter) + } else { + require.NoError(t, err) + require.IsType(t, getFormatterForType(tt.expectType), formatter) + } + }) + } +} + +// getFormatterForType returns a zero-value instance of the given type name for type assertion. +func getFormatterForType(typeName string) Formatter { + switch typeName { + case "*output.JSONFormatter": + return &JSONFormatter{} + case "*output.TableFormatter": + return &TableFormatter{} + default: + return nil + } +}