Merge pull request #25262 from hashicorp/alisdair/always-validate-workspace-name

command: Always validate workspace name

command/init.go

@@ -264,7 +264,12 @@ func (c *InitCommand) Run(args []string) int {
 	// on a previous run) we'll use the current state as a potential source
 	// of provider dependencies.
 	if back != nil {
-		sMgr, err := back.StateMgr(c.Workspace())
+		workspace, err := c.Workspace()
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+			return 1
+		}
+		sMgr, err := back.StateMgr(workspace)
 		if err != nil {
 			c.Ui.Error(fmt.Sprintf("Error loading state: %s", err))
 			return 1

command/meta.go

@@ -341,7 +341,12 @@ const (
 
 // contextOpts returns the options to use to initialize a Terraform
 // context with the settings from this Meta.
-func (m *Meta) contextOpts() *terraform.ContextOpts {
+func (m *Meta) contextOpts() (*terraform.ContextOpts, error) {
+	workspace, err := m.Workspace()
+	if err != nil {
+		return nil, err
+	}
+
 	var opts terraform.ContextOpts
 	opts.Hooks = []terraform.Hook{m.uiHook()}
 	opts.Hooks = append(opts.Hooks, m.ExtraHooks...)
@@ -379,10 +384,10 @@ func (m *Meta) contextOpts() *terraform.ContextOpts {
 	}
 
 	opts.Meta = &terraform.ContextMeta{
-		Env: m.Workspace(),
+		Env: workspace,
 	}
 
-	return &opts
+	return &opts, nil
 }
 
 // defaultFlagSet creates a default flag set for commands.
@@ -599,11 +604,16 @@ func (m *Meta) outputShadowError(err error, output bool) bool {
 // and `terraform workspace delete`.
 const WorkspaceNameEnvVar = "TF_WORKSPACE"
 
+var invalidWorkspaceNameEnvVar = fmt.Errorf("Invalid workspace name set using %s", WorkspaceNameEnvVar)
+
 // Workspace returns the name of the currently configured workspace, corresponding
 // to the desired named state.
-func (m *Meta) Workspace() string {
-	current, _ := m.WorkspaceOverridden()
-	return current
+func (m *Meta) Workspace() (string, error) {
+	current, overridden := m.WorkspaceOverridden()
+	if overridden && !validWorkspaceName(current) {
+		return "", invalidWorkspaceNameEnvVar
+	}
+	return current, nil
 }
 
 // WorkspaceOverridden returns the name of the currently configured workspace,

command/meta_backend.go

@@ -101,7 +101,11 @@ func (m *Meta) Backend(opts *BackendOpts) (backend.Enhanced, tfdiags.Diagnostics
 	}
 
 	// Setup the CLI opts we pass into backends that support it.
-	cliOpts := m.backendCLIOpts()
+	cliOpts, err := m.backendCLIOpts()
+	if err != nil {
+		diags = diags.Append(err)
+		return nil, diags
+	}
 	cliOpts.Validation = true
 
 	// If the backend supports CLI initialization, do it.
@@ -180,7 +184,10 @@ func (m *Meta) selectWorkspace(b backend.Backend) error {
 	}
 
 	// Get the currently selected workspace.
-	workspace := m.Workspace()
+	workspace, err := m.Workspace()
+	if err != nil {
+		return err
+	}
 
 	// Check if any of the existing workspaces matches the selected
 	// workspace and create a numbered list of existing workspaces.
@@ -249,7 +256,11 @@ func (m *Meta) BackendForPlan(settings plans.Backend) (backend.Enhanced, tfdiags
 
 	// If the backend supports CLI initialization, do it.
 	if cli, ok := b.(backend.CLI); ok {
-		cliOpts := m.backendCLIOpts()
+		cliOpts, err := m.backendCLIOpts()
+		if err != nil {
+			diags = diags.Append(err)
+			return nil, diags
+		}
 		if err := cli.CLIInit(cliOpts); err != nil {
 			diags = diags.Append(fmt.Errorf(
 				"Error initializing backend %T: %s\n\n"+
@@ -270,7 +281,11 @@ func (m *Meta) BackendForPlan(settings plans.Backend) (backend.Enhanced, tfdiags
 	// Otherwise, we'll wrap our state-only remote backend in the local backend
 	// to cause any operations to be run locally.
 	log.Printf("[TRACE] Meta.Backend: backend %T does not support operations, so wrapping it in a local backend", b)
-	cliOpts := m.backendCLIOpts()
+	cliOpts, err := m.backendCLIOpts()
+	if err != nil {
+		diags = diags.Append(err)
+		return nil, diags
+	}
 	cliOpts.Validation = false // don't validate here in case config contains file(...) calls where the file doesn't exist
 	local := backendLocal.NewWithBackend(b)
 	if err := local.CLIInit(cliOpts); err != nil {
@@ -283,7 +298,11 @@ func (m *Meta) BackendForPlan(settings plans.Backend) (backend.Enhanced, tfdiags
 
 // backendCLIOpts returns a backend.CLIOpts object that should be passed to
 // a backend that supports local CLI operations.
-func (m *Meta) backendCLIOpts() *backend.CLIOpts {
+func (m *Meta) backendCLIOpts() (*backend.CLIOpts, error) {
+	contextOpts, err := m.contextOpts()
+	if err != nil {
+		return nil, err
+	}
 	return &backend.CLIOpts{
 		CLI:                 m.Ui,
 		CLIColor:            m.Colorize(),
@@ -291,10 +310,10 @@ func (m *Meta) backendCLIOpts() *backend.CLIOpts {
 		StatePath:           m.statePath,
 		StateOutPath:        m.stateOutPath,
 		StateBackupPath:     m.backupPath,
-		ContextOpts:         m.contextOpts(),
+		ContextOpts:         contextOpts,
 		Input:               m.Input(),
 		RunningInAutomation: m.RunningInAutomation,
-	}
+	}, nil
 }
 
 // IsLocalBackend returns true if the backend is a local backend. We use this
@@ -318,7 +337,13 @@ func (m *Meta) IsLocalBackend(b backend.Backend) bool {
 // be called.
 func (m *Meta) Operation(b backend.Backend) *backend.Operation {
 	schema := b.ConfigSchema()
-	workspace := m.Workspace()
+	workspace, err := m.Workspace()
+	if err != nil {
+		// An invalid workspace error would have been raised when creating the
+		// backend, and the caller should have already exited. Seeing the error
+		// here first is a bug, so panic.
+		panic(fmt.Sprintf("invalid workspace: %s", err))
+	}
 	planOutBackend, err := m.backendState.ForPlan(schema, workspace)
 	if err != nil {
 		// Always indicates an implementation error in practice, because

command/meta_backend_migrate.go

@@ -180,7 +180,10 @@ func (m *Meta) backendMigrateState_S_S(opts *backendMigrateOpts) error {
 func (m *Meta) backendMigrateState_S_s(opts *backendMigrateOpts) error {
 	log.Printf("[TRACE] backendMigrateState: target backend type %q does not support named workspaces", opts.TwoType)
 
-	currentEnv := m.Workspace()
+	currentEnv, err := m.Workspace()
+	if err != nil {
+		return err
+	}
 
 	migrate := opts.force
 	if !migrate {
@@ -261,9 +264,12 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error {
 				return nil, err
 			}
 
+			// Ignore invalid workspace name as it is irrelevant in this context.
+			workspace, _ := m.Workspace()
+
 			// If the currently selected workspace is the default workspace, then set
 			// the named workspace as the new selected workspace.
-			if m.Workspace() == backend.DefaultStateName {
+			if workspace == backend.DefaultStateName {
 				if err := m.SetWorkspace(opts.twoEnv); err != nil {
 					return nil, fmt.Errorf("Failed to set new workspace: %s", err)
 				}

command/meta_backend_test.go

@@ -1002,7 +1002,11 @@ func TestMetaBackend_configuredChangeCopy_multiToSingle(t *testing.T) {
 	}
 
 	// Verify we are now in the default env, or we may not be able to access the new backend
-	if env := m.Workspace(); env != backend.DefaultStateName {
+	env, err := m.Workspace()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if env != backend.DefaultStateName {
 		t.Fatal("using non-default env with single-env backend")
 	}
 }

command/meta_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 
 	"github.com/hashicorp/terraform/backend"
+	"github.com/hashicorp/terraform/backend/local"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -170,7 +171,10 @@ func TestMeta_Env(t *testing.T) {
 
 	m := new(Meta)
 
-	env := m.Workspace()
+	env, err := m.Workspace()
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if env != backend.DefaultStateName {
 		t.Fatalf("expected env %q, got env %q", backend.DefaultStateName, env)
@@ -181,7 +185,7 @@ func TestMeta_Env(t *testing.T) {
 		t.Fatal("error setting env:", err)
 	}
 
-	env = m.Workspace()
+	env, _ = m.Workspace()
 	if env != testEnv {
 		t.Fatalf("expected env %q, got env %q", testEnv, env)
 	}
@@ -190,12 +194,84 @@ func TestMeta_Env(t *testing.T) {
 		t.Fatal("error setting env:", err)
 	}
 
-	env = m.Workspace()
+	env, _ = m.Workspace()
 	if env != backend.DefaultStateName {
 		t.Fatalf("expected env %q, got env %q", backend.DefaultStateName, env)
 	}
 }
 
+func TestMeta_Workspace_override(t *testing.T) {
+	defer func(value string) {
+		os.Setenv(WorkspaceNameEnvVar, value)
+	}(os.Getenv(WorkspaceNameEnvVar))
+
+	m := new(Meta)
+
+	testCases := map[string]struct {
+		workspace string
+		err       error
+	}{
+		"": {
+			"default",
+			nil,
+		},
+		"development": {
+			"development",
+			nil,
+		},
+		"invalid name": {
+			"",
+			invalidWorkspaceNameEnvVar,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			os.Setenv(WorkspaceNameEnvVar, name)
+			workspace, err := m.Workspace()
+			if workspace != tc.workspace {
+				t.Errorf("Unexpected workspace\n got: %s\nwant: %s\n", workspace, tc.workspace)
+			}
+			if err != tc.err {
+				t.Errorf("Unexpected error\n got: %s\nwant: %s\n", err, tc.err)
+			}
+		})
+	}
+}
+
+func TestMeta_Workspace_invalidSelected(t *testing.T) {
+	td := tempDir(t)
+	os.MkdirAll(td, 0755)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	// this is an invalid workspace name
+	workspace := "test workspace"
+
+	// create the workspace directories
+	if err := os.MkdirAll(filepath.Join(local.DefaultWorkspaceDir, workspace), 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	// create the workspace file to select it
+	if err := os.MkdirAll(DefaultDataDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(filepath.Join(DefaultDataDir, local.DefaultWorkspaceFile), []byte(workspace), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	m := new(Meta)
+
+	ws, err := m.Workspace()
+	if ws != workspace {
+		t.Errorf("Unexpected workspace\n got: %s\nwant: %s\n", ws, workspace)
+	}
+	if err != nil {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
+
 func TestMeta_process(t *testing.T) {
 	test = false
 	defer func() { test = true }()

command/output.go

@@ -64,7 +64,11 @@ func (c *OutputCommand) Run(args []string) int {
 		return 1
 	}
 
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 
 	// Get the state
 	stateStore, err := b.StateMgr(env)

command/plan.go

@@ -135,7 +135,12 @@ func (c *PlanCommand) Run(args []string) int {
 		}
 		var backendForPlan plans.Backend
 		backendForPlan.Type = backendPseudoState.Type
-		backendForPlan.Workspace = c.Workspace()
+		workspace, err := c.Workspace()
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+			return 1
+		}
+		backendForPlan.Workspace = workspace
 
 		// Configuration is a little more awkward to handle here because it's
 		// stored in state as raw JSON but we need it as a plans.DynamicValue

command/providers.go

@@ -83,7 +83,11 @@ func (c *ProvidersCommand) Run(args []string) int {
 	}
 
 	// Get the state
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	s, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))

command/show.go

@@ -130,7 +130,11 @@ func (c *ShowCommand) Run(args []string) int {
 			}
 		}
 	} else {
-		env := c.Workspace()
+		env, err := c.Workspace()
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+			return 1
+		}
 		stateFile, stateErr = getStateFromEnv(b, env)
 		if stateErr != nil {
 			c.Ui.Error(stateErr.Error())

command/state_list.go

@@ -41,7 +41,11 @@ func (c *StateListCommand) Run(args []string) int {
 	}
 
 	// Get the state
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateLoadingState, err))

command/state_meta.go

@@ -37,7 +37,10 @@ func (c *StateMeta) State() (statemgr.Full, error) {
 			return nil, backendDiags.Err()
 		}
 
-		workspace := c.Workspace()
+		workspace, err := c.Workspace()
+		if err != nil {
+			return nil, err
+		}
 		// Get the state
 		s, err := b.StateMgr(workspace)
 		if err != nil {

command/state_pull.go

@@ -32,7 +32,11 @@ func (c *StatePullCommand) Run(args []string) int {
 	}
 
 	// Get the state manager for the current workspace
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateLoadingState, err))

command/state_push.go

@@ -72,7 +72,11 @@ func (c *StatePushCommand) Run(args []string) int {
 	}
 
 	// Get the state manager for the currently-selected workspace
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load destination state: %s", err))

command/state_show.go

@@ -89,7 +89,11 @@ func (c *StateShowCommand) Run(args []string) int {
 	schemas := ctx.Schemas()
 
 	// Get the state
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateLoadingState, err))

command/taint.go

@@ -71,7 +71,11 @@ func (c *TaintCommand) Run(args []string) int {
 	}
 
 	// Get the state
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))

command/unlock.go

@@ -65,7 +65,11 @@ func (c *UnlockCommand) Run(args []string) int {
 		return 1
 	}
 
-	env := c.Workspace()
+	env, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))

command/untaint.go

@@ -66,7 +66,11 @@ func (c *UntaintCommand) Run(args []string) int {
 	}
 
 	// Get the state
-	workspace := c.Workspace()
+	workspace, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	stateMgr, err := b.StateMgr(workspace)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))

command/validate.go

@@ -110,7 +110,11 @@ func (c *ValidateCommand) validate(dir string) tfdiags.Diagnostics {
 		}
 	}
 
-	opts := c.contextOpts()
+	opts, err := c.contextOpts()
+	if err != nil {
+		diags = diags.Append(err)
+		return diags
+	}
 	opts.Config = cfg
 	opts.Variables = varValues
 

command/workspace_command_test.go

@@ -27,7 +27,7 @@ func TestWorkspace_createAndChange(t *testing.T) {
 
 	newCmd := &WorkspaceNewCommand{}
 
-	current := newCmd.Workspace()
+	current, _ := newCmd.Workspace()
 	if current != backend.DefaultStateName {
 		t.Fatal("current workspace should be 'default'")
 	}
@@ -39,7 +39,7 @@ func TestWorkspace_createAndChange(t *testing.T) {
 		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter)
 	}
 
-	current = newCmd.Workspace()
+	current, _ = newCmd.Workspace()
 	if current != "test" {
 		t.Fatalf("current workspace should be 'test', got %q", current)
 	}
@@ -52,7 +52,7 @@ func TestWorkspace_createAndChange(t *testing.T) {
 		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter)
 	}
 
-	current = newCmd.Workspace()
+	current, _ = newCmd.Workspace()
 	if current != backend.DefaultStateName {
 		t.Fatal("current workspace should be 'default'")
 	}
@@ -307,7 +307,7 @@ func TestWorkspace_delete(t *testing.T) {
 		Meta: Meta{Ui: ui},
 	}
 
-	current := delCmd.Workspace()
+	current, _ := delCmd.Workspace()
 	if current != "test" {
 		t.Fatal("wrong workspace:", current)
 	}
@@ -330,11 +330,44 @@ func TestWorkspace_delete(t *testing.T) {
 		t.Fatalf("error deleting workspace: %s", ui.ErrorWriter)
 	}
 
-	current = delCmd.Workspace()
+	current, _ = delCmd.Workspace()
 	if current != backend.DefaultStateName {
 		t.Fatalf("wrong workspace: %q", current)
 	}
 }
+
+func TestWorkspace_deleteInvalid(t *testing.T) {
+	td := tempDir(t)
+	os.MkdirAll(td, 0755)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	// choose an invalid workspace name
+	workspace := "test workspace"
+	path := filepath.Join(local.DefaultWorkspaceDir, workspace)
+
+	// create the workspace directories
+	if err := os.MkdirAll(path, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	ui := new(cli.MockUi)
+	delCmd := &WorkspaceDeleteCommand{
+		Meta: Meta{Ui: ui},
+	}
+
+	// delete the workspace
+	if code := delCmd.Run([]string{workspace}); code != 0 {
+		t.Fatalf("error deleting workspace: %s", ui.ErrorWriter)
+	}
+
+	if _, err := os.Stat(path); err == nil {
+		t.Fatalf("should have deleted workspace, but %s still exists", path)
+	} else if !os.IsNotExist(err) {
+		t.Fatalf("unexpected error for workspace path: %s", err)
+	}
+}
+
 func TestWorkspace_deleteWithState(t *testing.T) {
 	td := tempDir(t)
 	os.MkdirAll(td, 0755)

command/workspace_delete.go

@@ -40,13 +40,6 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int {
 		return cli.RunResultHelp
 	}
 
-	workspace := args[0]
-
-	if !validWorkspaceName(workspace) {
-		c.Ui.Error(fmt.Sprintf(envInvalidName, workspace))
-		return 1
-	}
-
 	configPath, err := ModulePath(args[1:])
 	if err != nil {
 		c.Ui.Error(err.Error())
@@ -78,6 +71,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int {
 		return 1
 	}
 
+	workspace := args[0]
 	exists := false
 	for _, ws := range workspaces {
 		if workspace == ws {
@@ -91,7 +85,12 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int {
 		return 1
 	}
 
-	if workspace == c.Workspace() {
+	currentWorkspace, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
+	if workspace == currentWorkspace {
 		c.Ui.Error(fmt.Sprintf(strings.TrimSpace(envDelCurrent), workspace))
 		return 1
 	}

command/workspace_show.go

@@ -20,7 +20,11 @@ func (c *WorkspaceShowCommand) Run(args []string) int {
 		return 1
 	}
 
-	workspace := c.Workspace()
+	workspace, err := c.Workspace()
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err))
+		return 1
+	}
 	c.Ui.Output(workspace)
 
 	return 0