From b239570abb17fe4609e7c0adadad14b94916a558 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 16 Jun 2020 12:23:15 -0400 Subject: [PATCH] command: Always validate workspace name The workspace name can be overridden by setting a TF_WORKSPACE environment variable. If this is done, we should still validate the resulting workspace name; otherwise, we could end up with an invalid and unselectable workspace. This change updates the Meta.Workspace function to return an error, and handles that error wherever necessary. --- command/init.go | 7 ++++- command/meta.go | 22 ++++++++++---- command/meta_backend.go | 41 ++++++++++++++++++++------ command/meta_backend_migrate.go | 10 +++++-- command/meta_backend_test.go | 6 +++- command/meta_test.go | 48 +++++++++++++++++++++++++++++-- command/output.go | 6 +++- command/plan.go | 7 ++++- command/providers.go | 6 +++- command/show.go | 6 +++- command/state_list.go | 6 +++- command/state_meta.go | 5 +++- command/state_pull.go | 6 +++- command/state_push.go | 6 +++- command/state_show.go | 6 +++- command/taint.go | 6 +++- command/unlock.go | 6 +++- command/untaint.go | 6 +++- command/validate.go | 6 +++- command/workspace_command_test.go | 10 +++---- command/workspace_delete.go | 7 ++++- command/workspace_show.go | 6 +++- 22 files changed, 194 insertions(+), 41 deletions(-) diff --git a/command/init.go b/command/init.go index 8053d4b12..679055cd7 100644 --- a/command/init.go +++ b/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 diff --git a/command/meta.go b/command/meta.go index a4b00935f..186736cfa 100644 --- a/command/meta.go +++ b/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, diff --git a/command/meta_backend.go b/command/meta_backend.go index ac8371ea1..77706242f 100644 --- a/command/meta_backend.go +++ b/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 diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 2d4fcba20..6283bd99f 100644 --- a/command/meta_backend_migrate.go +++ b/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) } diff --git a/command/meta_backend_test.go b/command/meta_backend_test.go index 7dfbedc82..af213b679 100644 --- a/command/meta_backend_test.go +++ b/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") } } diff --git a/command/meta_test.go b/command/meta_test.go index 5ebae9a8e..7f92da02c 100644 --- a/command/meta_test.go +++ b/command/meta_test.go @@ -170,7 +170,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 +184,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 +193,51 @@ 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_process(t *testing.T) { test = false defer func() { test = true }() diff --git a/command/output.go b/command/output.go index 5848e8340..9d37c09db 100644 --- a/command/output.go +++ b/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) diff --git a/command/plan.go b/command/plan.go index a8a582978..8064cad57 100644 --- a/command/plan.go +++ b/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 diff --git a/command/providers.go b/command/providers.go index 797082dfd..918fd841e 100644 --- a/command/providers.go +++ b/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)) diff --git a/command/show.go b/command/show.go index a359d1c8c..7defadf76 100644 --- a/command/show.go +++ b/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()) diff --git a/command/state_list.go b/command/state_list.go index a1130fa94..3442e1c3c 100644 --- a/command/state_list.go +++ b/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)) diff --git a/command/state_meta.go b/command/state_meta.go index e245dbcfd..884b618a4 100644 --- a/command/state_meta.go +++ b/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 { diff --git a/command/state_pull.go b/command/state_pull.go index 91fd3439b..6ab6328a2 100644 --- a/command/state_pull.go +++ b/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)) diff --git a/command/state_push.go b/command/state_push.go index 8f1fe69b8..5271c477c 100644 --- a/command/state_push.go +++ b/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)) diff --git a/command/state_show.go b/command/state_show.go index 3f3e0d5dd..dd6e292bc 100644 --- a/command/state_show.go +++ b/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)) diff --git a/command/taint.go b/command/taint.go index 50c95e092..97464994c 100644 --- a/command/taint.go +++ b/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)) diff --git a/command/unlock.go b/command/unlock.go index 69189552a..94b2f7d8f 100644 --- a/command/unlock.go +++ b/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)) diff --git a/command/untaint.go b/command/untaint.go index b64eecc14..b1a77c6ae 100644 --- a/command/untaint.go +++ b/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)) diff --git a/command/validate.go b/command/validate.go index bf42f35b9..e3dafceed 100644 --- a/command/validate.go +++ b/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 diff --git a/command/workspace_command_test.go b/command/workspace_command_test.go index 267745423..8a0544737 100644 --- a/command/workspace_command_test.go +++ b/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,7 +330,7 @@ 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) } diff --git a/command/workspace_delete.go b/command/workspace_delete.go index 1413d245f..d22de8e69 100644 --- a/command/workspace_delete.go +++ b/command/workspace_delete.go @@ -91,7 +91,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 } diff --git a/command/workspace_show.go b/command/workspace_show.go index 597ed2c2a..23d438c56 100644 --- a/command/workspace_show.go +++ b/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