From e24abdf6ff9a2333e452778c5cd970391c2cd765 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:05:49 +0100 Subject: [PATCH] refactor: Update `workspace select` and `delete` subcommands to use the arguments package for parsing arguments and flags (#38429) * feat: Add `workspace show`-related code to arguments package * refactor: Update `workspace show` to use the arguments package when parsing arguments * refactor: Split code for workspace subcommands into separate files in arguments package * refactor: Move code common to all workspace commands into separate file in arguments package * feat: Add `workspace delete`-related code to arguments package * refactor: Update `workspace delete` to use the arguments package when parsing arguments --- internal/command/arguments/workspace.go | 43 ---- .../command/arguments/workspace_delete.go | 78 ++++++++ .../arguments/workspace_delete_test.go | 184 ++++++++++++++++++ internal/command/arguments/workspace_list.go | 48 +++++ ...rkspace_test.go => workspace_list_test.go} | 0 internal/command/arguments/workspace_show.go | 32 +++ .../command/arguments/workspace_show_test.go | 80 ++++++++ internal/command/workspace_command_test.go | 2 +- internal/command/workspace_delete.go | 48 ++--- internal/command/workspace_show.go | 19 +- 10 files changed, 450 insertions(+), 84 deletions(-) create mode 100644 internal/command/arguments/workspace_delete.go create mode 100644 internal/command/arguments/workspace_delete_test.go create mode 100644 internal/command/arguments/workspace_list.go rename internal/command/arguments/{workspace_test.go => workspace_list_test.go} (100%) create mode 100644 internal/command/arguments/workspace_show.go create mode 100644 internal/command/arguments/workspace_show_test.go diff --git a/internal/command/arguments/workspace.go b/internal/command/arguments/workspace.go index bacb133a39..67c172a1fa 100644 --- a/internal/command/arguments/workspace.go +++ b/internal/command/arguments/workspace.go @@ -3,12 +3,6 @@ package arguments -import ( - "errors" - - "github.com/hashicorp/terraform/internal/tfdiags" -) - // Workspace represents the command-line arguments common between all workspace subcommands. // // Subcommands that accept additional arguments should have a specific struct that embeds this struct. @@ -16,40 +10,3 @@ type Workspace struct { // ViewType specifies which output format to use ViewType ViewType } - -type WorkspaceList struct { - Workspace -} - -// ParseWorkspaceList processes CLI arguments, returning a WorkspaceList value and errors. -// If errors are encountered, an WorkspaceList value is still returned representing -// the best effort interpretation of the arguments. -func ParseWorkspaceList(args []string) (*WorkspaceList, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - - var jsonOutput bool - cmdFlags := defaultFlagSet("workspace list") - cmdFlags.BoolVar(&jsonOutput, "json", false, "produce JSON output") - - if err := cmdFlags.Parse(args); err != nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Failed to parse command-line flags", - err.Error(), - )) - } - - // `workspace list` takes no positional arguments. Historically there was a DIR argument that was replaced with the -chdir flag. - // Here we replicate the old behaviour of suggesting the user to use -chdir if they provide any positional arguments. - args = cmdFlags.Args() - if len(args) != 0 { - diags = diags.Append(errors.New("Too many command line arguments. Did you mean to use -chdir?")) - } - - switch { - case jsonOutput: - return &WorkspaceList{Workspace: Workspace{ViewType: ViewJSON}}, diags - default: - return &WorkspaceList{Workspace: Workspace{ViewType: ViewHuman}}, diags - } -} diff --git a/internal/command/arguments/workspace_delete.go b/internal/command/arguments/workspace_delete.go new file mode 100644 index 0000000000..06b25c4f54 --- /dev/null +++ b/internal/command/arguments/workspace_delete.go @@ -0,0 +1,78 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "errors" + "fmt" + "time" + + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// WorkspaceDelete represent flags and arguments specific to the `terraform workspace delete` command. +type WorkspaceDelete struct { + Workspace + + // Flags + Lock bool + LockTimeout time.Duration + Force bool + + // Positional arguments + Name string +} + +// ParseWorkspaceDelete processes CLI arguments, returning a WorkspaceDelete value and errors. +// If errors are encountered, an WorkspaceDelete value is still returned representing +// the best effort interpretation of the arguments. +func ParseWorkspaceDelete(args []string) (*WorkspaceDelete, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + var force bool + var stateLock bool + var stateLockTimeout time.Duration + cmdFlags := defaultFlagSet("workspace delete") + cmdFlags.BoolVar(&force, "force", false, "force removal of a non-empty workspace") + cmdFlags.BoolVar(&stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&stateLockTimeout, "lock-timeout", 0, "lock timeout") + if err := cmdFlags.Parse(args); err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + err.Error(), + )) + } + + // `workspace delete` takes only one positional argument: workspace name. + args = cmdFlags.Args() + var name string + if len(args) == 0 { + diags = diags.Append(errors.New("Expected a single argument: NAME.")) // Recreating pre-existing error from command package + } else { + + // Obtain and validate name argument + // + // We purposefully don't use ValidWorkspaceName here; if a user + // creates a workspace with an invalid name they should be able to + // delete it easily. + name = args[0] + if name == "" { + diags = diags.Append(fmt.Errorf("Expected a workspace name as an argument, instead got an empty string: %q\n", args[0])) + } + + args = args[1:] + if len(args) != 0 { + diags = diags.Append(errors.New("Expected a single argument: NAME.")) + } + } + + return &WorkspaceDelete{ + Workspace: Workspace{ViewType: ViewHuman}, + Name: name, + Lock: stateLock, + LockTimeout: stateLockTimeout, + Force: force, + }, diags +} diff --git a/internal/command/arguments/workspace_delete_test.go b/internal/command/arguments/workspace_delete_test.go new file mode 100644 index 0000000000..e952cb9723 --- /dev/null +++ b/internal/command/arguments/workspace_delete_test.go @@ -0,0 +1,184 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "testing" + "time" + + "github.com/hashicorp/terraform/internal/tfdiags" +) + +func TestParseWorkspaceDelete_valid(t *testing.T) { + testCases := map[string]struct { + args []string + want *WorkspaceDelete + }{ + "name specified & default flags": { + []string{"my-new-workspace"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "my-new-workspace", + Lock: true, + Force: false, + LockTimeout: 0, + }, + }, + "invalid names are tolerated during delete": { + []string{"§@!invalid-name!@§"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "§@!invalid-name!@§", + Lock: true, + Force: false, + LockTimeout: 0, + }, + }, + "lock flag specified": { + []string{"-lock=false", "my-new-workspace"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "my-new-workspace", + Lock: false, + Force: false, + LockTimeout: 0, + }, + }, + "force flag specified": { + []string{"-force=true", "my-new-workspace"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "my-new-workspace", + Lock: true, + Force: true, + LockTimeout: 0, + }, + }, + "lock-timeout flag specified": { + []string{"-lock-timeout=30s", "my-new-workspace"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "my-new-workspace", + Lock: true, + Force: false, + LockTimeout: 30 * time.Second, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParseWorkspaceDelete(tc.args) + if len(diags) > 0 { + t.Fatalf("unexpected diags: %v", diags) + } + if *got != *tc.want { + t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + }) + } +} + +func TestParseWorkspaceDelete_invalid(t *testing.T) { + testCases := map[string]struct { + args []string + want *WorkspaceDelete + wantDiags tfdiags.Diagnostics + }{ + "unknown flag": { + []string{"-boop", "my-new-workspace"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "my-new-workspace", + Force: false, + Lock: true, + LockTimeout: 0, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "flag provided but not defined: -boop", + ), + }, + }, + "too many arguments": { + []string{"my-new-workspace", "bar"}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Name: "my-new-workspace", // First positional argument is still captured`` + Force: false, + Lock: true, + LockTimeout: 0, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Expected a single argument: NAME.", + "", // No detail + ), + }, + }, + "no arguments": { + []string{}, + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Force: false, + Lock: true, + LockTimeout: 0, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Expected a single argument: NAME.", + "", // No detail + ), + }, + }, + "empty string as workspace name": { + []string{""}, // empty string + &WorkspaceDelete{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + Force: false, + Lock: true, + LockTimeout: 0, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Expected a workspace name as an argument, instead got an empty string: \"\"\n", + "", // No detail + ), + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, gotDiags := ParseWorkspaceDelete(tc.args) + if *got != *tc.want { + t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + tfdiags.AssertDiagnosticsMatch(t, gotDiags, tc.wantDiags) + }) + } +} diff --git a/internal/command/arguments/workspace_list.go b/internal/command/arguments/workspace_list.go new file mode 100644 index 0000000000..6be8981cbb --- /dev/null +++ b/internal/command/arguments/workspace_list.go @@ -0,0 +1,48 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "errors" + + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// WorkspaceList represent arguments specific to the `terraform workspace list` command. +type WorkspaceList struct { + Workspace +} + +// ParseWorkspaceList processes CLI arguments, returning a WorkspaceList value and errors. +// If errors are encountered, an WorkspaceList value is still returned representing +// the best effort interpretation of the arguments. +func ParseWorkspaceList(args []string) (*WorkspaceList, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + var jsonOutput bool + cmdFlags := defaultFlagSet("workspace list") + cmdFlags.BoolVar(&jsonOutput, "json", false, "produce JSON output") + + if err := cmdFlags.Parse(args); err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + err.Error(), + )) + } + + // `workspace list` takes no positional arguments. Historically there was a DIR argument that was replaced with the -chdir flag. + // Here we replicate the old behaviour of suggesting the user to use -chdir if they provide any positional arguments. + args = cmdFlags.Args() + if len(args) != 0 { + diags = diags.Append(errors.New("Too many command line arguments. Did you mean to use -chdir?")) + } + + switch { + case jsonOutput: + return &WorkspaceList{Workspace: Workspace{ViewType: ViewJSON}}, diags + default: + return &WorkspaceList{Workspace: Workspace{ViewType: ViewHuman}}, diags + } +} diff --git a/internal/command/arguments/workspace_test.go b/internal/command/arguments/workspace_list_test.go similarity index 100% rename from internal/command/arguments/workspace_test.go rename to internal/command/arguments/workspace_list_test.go diff --git a/internal/command/arguments/workspace_show.go b/internal/command/arguments/workspace_show.go new file mode 100644 index 0000000000..80769186b1 --- /dev/null +++ b/internal/command/arguments/workspace_show.go @@ -0,0 +1,32 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "github.com/hashicorp/terraform/internal/tfdiags" +) + +type WorkspaceShow struct { + Workspace +} + +func ParseWorkspaceShow(args []string) (*WorkspaceShow, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + cmdFlags := defaultFlagSet("workspace show") + + if err := cmdFlags.Parse(args); err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + err.Error(), + )) + } + + // `workspace show` takes no positional arguments. + // We could add validation here to return an error when unexpected arguments are present, + // but this would be a breaking change as no validation was performed in this case before. + + return &WorkspaceShow{Workspace: Workspace{ViewType: ViewHuman}}, diags +} diff --git a/internal/command/arguments/workspace_show_test.go b/internal/command/arguments/workspace_show_test.go new file mode 100644 index 0000000000..afae0fdf7a --- /dev/null +++ b/internal/command/arguments/workspace_show_test.go @@ -0,0 +1,80 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "testing" + + "github.com/hashicorp/terraform/internal/tfdiags" +) + +func TestParseWorkspaceShow_valid(t *testing.T) { + testCases := map[string]struct { + args []string + want *WorkspaceShow + }{ + "defaults": { + nil, + &WorkspaceShow{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + }, + }, + "currently there is no validation about too many arguments": { + []string{"bar"}, + &WorkspaceShow{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParseWorkspaceShow(tc.args) + if len(diags) > 0 { + t.Fatalf("unexpected diags: %v", diags) + } + if *got != *tc.want { + t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + }) + } +} + +func TestParseWorkspaceShow_invalid(t *testing.T) { + testCases := map[string]struct { + args []string + want *WorkspaceShow + wantDiags tfdiags.Diagnostics + }{ + "unknown flag": { + []string{"-boop"}, + &WorkspaceShow{ + Workspace: Workspace{ + ViewType: ViewHuman, + }, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "flag provided but not defined: -boop", + ), + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, gotDiags := ParseWorkspaceShow(tc.args) + if *got != *tc.want { + t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + tfdiags.AssertDiagnosticsMatch(t, gotDiags, tc.wantDiags) + }) + } +} diff --git a/internal/command/workspace_command_test.go b/internal/command/workspace_command_test.go index ca1fb2ca56..1fd43038ea 100644 --- a/internal/command/workspace_command_test.go +++ b/internal/command/workspace_command_test.go @@ -1182,7 +1182,7 @@ func TestWorkspace_extraArgError(t *testing.T) { if code := deleteCmd.Run(args); code != cli.RunResultHelp { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter) } - expectedError = "Expected a single argument: NAME.\n\n" + expectedError = "\nError: Expected a single argument: NAME.\n\n\n" if ui.ErrorWriter.String() != expectedError { t.Fatalf("expected error to include %s but was missing, got: %s", expectedError, ui.ErrorWriter.String()) } diff --git a/internal/command/workspace_delete.go b/internal/command/workspace_delete.go index a3aab773e4..5d9a654769 100644 --- a/internal/command/workspace_delete.go +++ b/internal/command/workspace_delete.go @@ -6,7 +6,6 @@ package command import ( "fmt" "strings" - "time" "github.com/hashicorp/cli" "github.com/hashicorp/terraform/internal/backend" @@ -23,40 +22,23 @@ type WorkspaceDeleteCommand struct { LegacyName bool } -func (c *WorkspaceDeleteCommand) Run(args []string) int { - args = c.Meta.process(args) - envCommandShowWarning(c.Ui, c.LegacyName) +func (c *WorkspaceDeleteCommand) Run(rawArgs []string) int { + var diags tfdiags.Diagnostics - var force bool - var stateLock bool - var stateLockTimeout time.Duration - cmdFlags := c.Meta.defaultFlagSet("workspace delete") - cmdFlags.BoolVar(&force, "force", false, "force removal of a non-empty workspace") - cmdFlags.BoolVar(&stateLock, "lock", true, "lock state") - cmdFlags.DurationVar(&stateLockTimeout, "lock-timeout", 0, "lock timeout") - cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } - if err := cmdFlags.Parse(args); err != nil { - c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) - return 1 - } + // Process global flags and configure the view/UI. + rawArgs = c.Meta.process(rawArgs) + envCommandShowWarning(c.Ui, c.LegacyName) - args = cmdFlags.Args() - if len(args) != 1 { - c.Ui.Error("Expected a single argument: NAME.\n") - return cli.RunResultHelp - } - if args[0] == "" { - // Disallowing empty string identifiers more explicitly, versus "Workspace "" doesn't exist." - c.Ui.Error(fmt.Sprintf("Expected a workspace name as an argument, instead got an empty string: %q\n", args[0])) + // Process command-specific arguments. + args, diags := arguments.ParseWorkspaceDelete(rawArgs) + if diags.HasErrors() { + c.showDiagnostics(diags) return cli.RunResultHelp } - var diags tfdiags.Diagnostics - // Load the backend - view := arguments.ViewHuman configPath := c.WorkingDir.RootModuleDir() - b, backendDiags := c.backend(configPath, view) + b, backendDiags := c.backend(configPath, args.ViewType) diags = diags.Append(backendDiags) if backendDiags.HasErrors() { c.showDiagnostics(diags) @@ -75,7 +57,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { c.showDiagnostics(diags) // output warnings, if any // Is the user attempting to delete a workspace that doesn't exist? - workspace := args[0] + workspace := args.Name exists := false for _, ws := range workspaces { if workspace == ws { @@ -114,8 +96,8 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { } var stateLocker clistate.Locker - if stateLock { - stateLocker = clistate.NewLocker(c.stateLockTimeout, views.NewStateLocker(arguments.ViewHuman, c.View)) + if args.Lock { + stateLocker = clistate.NewLocker(c.stateLockTimeout, views.NewStateLocker(args.ViewType, c.View)) if diags := stateLocker.Lock(stateMgr, "state-replace-provider"); diags.HasErrors() { c.showDiagnostics(diags) return 1 @@ -133,7 +115,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { hasResources := stateMgr.State().HasManagedResourceInstanceObjects() - if hasResources && !force { + if hasResources && !args.Force { // We'll collect a list of what's being managed here as extra context // for the message. var buf strings.Builder @@ -171,7 +153,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { // be delegated from the Backend to the State itself. stateLocker.Unlock() - dwDiags := b.DeleteWorkspace(workspace, force) + dwDiags := b.DeleteWorkspace(workspace, args.Force) diags = diags.Append(dwDiags) if dwDiags.HasErrors() { c.Ui.Error(dwDiags.Err().Error()) diff --git a/internal/command/workspace_show.go b/internal/command/workspace_show.go index 46cdb6cb19..ffc8a813ad 100644 --- a/internal/command/workspace_show.go +++ b/internal/command/workspace_show.go @@ -7,6 +7,8 @@ import ( "fmt" "strings" + "github.com/hashicorp/cli" + "github.com/hashicorp/terraform/internal/command/arguments" "github.com/posener/complete" ) @@ -14,13 +16,16 @@ type WorkspaceShowCommand struct { Meta } -func (c *WorkspaceShowCommand) Run(args []string) int { - args = c.Meta.process(args) - cmdFlags := c.Meta.extendedFlagSet("workspace show") - cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } - if err := cmdFlags.Parse(args); err != nil { - c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) - return 1 +func (c *WorkspaceShowCommand) Run(rawArgs []string) int { + // Process global flags and configure the view/UI. + rawArgs = c.Meta.process(rawArgs) + + // Process command-specific arguments. + // Currently there are no arguments for this command, so ignore the returned value for now. + _, diags := arguments.ParseWorkspaceShow(rawArgs) + if diags.HasErrors() { + c.showDiagnostics(diags) + return cli.RunResultHelp } workspace, err := c.Workspace()