From b80ae5e13edc34ac55a4f33f775ea8a2f68c4ac9 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 6 Feb 2017 13:26:03 -0500 Subject: [PATCH 1/3] Add source path argument to testLockState The new test pattern is to chdir into a temp location for the test, but the prevents us from locating the testdata directory in the source. Add a source path to testLockState so we can find the statelocker.go source. --- command/apply_destroy_test.go | 2 +- command/apply_test.go | 2 +- command/command_test.go | 8 ++++++-- command/plan_test.go | 2 +- command/refresh_test.go | 2 +- command/taint_test.go | 2 +- command/untaint_test.go | 2 +- 7 files changed, 12 insertions(+), 8 deletions(-) diff --git a/command/apply_destroy_test.go b/command/apply_destroy_test.go index 6e1277cca..f8e37f8cb 100644 --- a/command/apply_destroy_test.go +++ b/command/apply_destroy_test.go @@ -111,7 +111,7 @@ func TestApply_destroyLockedState(t *testing.T) { statePath := testStateFile(t, originalState) - unlock, err := testLockState(statePath) + unlock, err := testLockState("./testdata", statePath) if err != nil { t.Fatal(err) } diff --git a/command/apply_test.go b/command/apply_test.go index 3981b01ad..86028d52f 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -63,7 +63,7 @@ func TestApply(t *testing.T) { func TestApply_lockedState(t *testing.T) { statePath := testTempFile(t) - unlock, err := testLockState(statePath) + unlock, err := testLockState("./testdata", statePath) if err != nil { t.Fatal(err) } diff --git a/command/command_test.go b/command/command_test.go index d3bded69a..abf134331 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -535,7 +535,9 @@ func testRemoteState(t *testing.T, s *terraform.State, c int) (*terraform.Remote // testlockState calls a separate process to the lock the state file at path. // deferFunc should be called in the caller to properly unlock the file. -func testLockState(path string) (func(), error) { +// Since many tests change the working durectory, the sourcedir argument must be +// supplied to locate the statelocker.go source. +func testLockState(sourceDir, path string) (func(), error) { // build and run the binary ourselves so we can quickly terminate it for cleanup buildDir, err := ioutil.TempDir("", "locker") if err != nil { @@ -545,8 +547,10 @@ func testLockState(path string) (func(), error) { os.RemoveAll(buildDir) } + source := filepath.Join(sourceDir, "statelocker.go") lockBin := filepath.Join(buildDir, "statelocker") - out, err := exec.Command("go", "build", "-o", lockBin, "testdata/statelocker.go").CombinedOutput() + + out, err := exec.Command("go", "build", "-o", lockBin, source).CombinedOutput() if err != nil { cleanFunc() return nil, fmt.Errorf("%s %s", err, out) diff --git a/command/plan_test.go b/command/plan_test.go index b9e25bf22..b42b07710 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -46,7 +46,7 @@ func TestPlan_lockedState(t *testing.T) { } testPath := testFixturePath("plan") - unlock, err := testLockState(filepath.Join(testPath, DefaultStateFilename)) + unlock, err := testLockState("./testdata", filepath.Join(testPath, DefaultStateFilename)) if err != nil { t.Fatal(err) } diff --git a/command/refresh_test.go b/command/refresh_test.go index 4c79aec68..52aa33112 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -63,7 +63,7 @@ func TestRefresh_lockedState(t *testing.T) { state := testState() statePath := testStateFile(t, state) - unlock, err := testLockState(statePath) + unlock, err := testLockState("./testdata", statePath) if err != nil { t.Fatal(err) } diff --git a/command/taint_test.go b/command/taint_test.go index 26c42b6a5..9aa9b2664 100644 --- a/command/taint_test.go +++ b/command/taint_test.go @@ -63,7 +63,7 @@ func TestTaint_lockedState(t *testing.T) { } statePath := testStateFile(t, state) - unlock, err := testLockState(statePath) + unlock, err := testLockState("./testdata", statePath) if err != nil { t.Fatal(err) } diff --git a/command/untaint_test.go b/command/untaint_test.go index ed63de78d..80bf7bd72 100644 --- a/command/untaint_test.go +++ b/command/untaint_test.go @@ -68,7 +68,7 @@ func TestUntaint_lockedState(t *testing.T) { }, } statePath := testStateFile(t, state) - unlock, err := testLockState(statePath) + unlock, err := testLockState("./testdata", statePath) if err != nil { t.Fatal(err) } From 015198ca111f534532e56eba9099696e8f843a83 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 6 Feb 2017 13:45:30 -0500 Subject: [PATCH 2/3] Add lock/unlock commands --- command/lock.go | 78 +++++++++++++++++++++++++++++++++ command/lock_test.go | 97 ++++++++++++++++++++++++++++++++++++++++++ command/unlock.go | 78 +++++++++++++++++++++++++++++++++ command/unlock_test.go | 46 ++++++++++++++++++++ 4 files changed, 299 insertions(+) create mode 100644 command/lock.go create mode 100644 command/lock_test.go create mode 100644 command/unlock.go create mode 100644 command/unlock_test.go diff --git a/command/lock.go b/command/lock.go new file mode 100644 index 000000000..e6b2d211b --- /dev/null +++ b/command/lock.go @@ -0,0 +1,78 @@ +package command + +import ( + "fmt" + "strings" + + "github.com/hashicorp/terraform/state" +) + +// LockCommand is a cli.Command implementation that manually locks +// the state. +type LockCommand struct { + Meta +} + +func (c *LockCommand) Run(args []string) int { + args = c.Meta.process(args, false) + + cmdFlags := c.Meta.flagSet("lock") + cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } + if err := cmdFlags.Parse(args); err != nil { + return 1 + } + + // assume everything is initialized. The user can manually init if this is + // required. + configPath, err := ModulePath(cmdFlags.Args()) + if err != nil { + c.Ui.Error(err.Error()) + return 1 + } + + // Load the backend + b, err := c.Backend(&BackendOpts{ + ConfigPath: configPath, + }) + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to load backend: %s", err)) + return 1 + } + + st, err := b.State() + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err)) + return 1 + } + + s, ok := st.(state.Locker) + if !ok { + c.Ui.Error("Current state does not support locking") + return 1 + } + + if err := s.Lock("lock"); err != nil { + c.Ui.Error(fmt.Sprintf("Failed to lock state: %s", err)) + return 1 + } + + return 0 +} + +func (c *LockCommand) Help() string { + helpText := ` +Usage: terraform lock [DIR] + + Manually lock the state for the defined configuration. + + This will not modify your infrastructure. This command obtains a lock on the + state for the current configuration. The behavior of this lock is dependent + on the backend being used. A lock on a local state file only lasts for the + duration of the calling process. +` + return strings.TrimSpace(helpText) +} + +func (c *LockCommand) Synopsis() string { + return "Manually lock the terraform state" +} diff --git a/command/lock_test.go b/command/lock_test.go new file mode 100644 index 000000000..0e5331133 --- /dev/null +++ b/command/lock_test.go @@ -0,0 +1,97 @@ +package command + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/hashicorp/terraform/terraform" + "github.com/mitchellh/cli" +) + +func TestLock(t *testing.T) { + testData, _ := filepath.Abs("./testdata") + + td := tempDir(t) + os.MkdirAll(td, 0755) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + // Write the legacy state + statePath := DefaultStateFilename + { + f, err := os.Create(statePath) + if err != nil { + t.Fatalf("err: %s", err) + } + err = terraform.WriteState(testState(), f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + } + + p := testProvider() + ui := new(cli.MockUi) + c := &LockCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + if code := c.Run(nil); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + unlock, err := testLockState(testData, statePath) + if err == nil { + unlock() + t.Fatal("expected error locking state") + } else if !strings.Contains(err.Error(), "locked") { + t.Fatal("does not appear to be a lock error:", err) + } +} + +func TestLock_lockedState(t *testing.T) { + testData, _ := filepath.Abs("./testdata") + + td := tempDir(t) + os.MkdirAll(td, 0755) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + // Write the legacy state + statePath := DefaultStateFilename + { + f, err := os.Create(statePath) + if err != nil { + t.Fatalf("err: %s", err) + } + err = terraform.WriteState(testState(), f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + } + + p := testProvider() + ui := new(cli.MockUi) + c := &LockCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + unlock, err := testLockState(testData, statePath) + if err != nil { + t.Fatal(err) + } + defer unlock() + + if code := c.Run(nil); code == 0 { + t.Fatal("expected error when locking a locked state") + } +} diff --git a/command/unlock.go b/command/unlock.go new file mode 100644 index 000000000..c31e82ad9 --- /dev/null +++ b/command/unlock.go @@ -0,0 +1,78 @@ +package command + +import ( + "fmt" + "strings" + + "github.com/hashicorp/terraform/state" +) + +// UnlockCommand is a cli.Command implementation that manually unlocks +// the state. +type UnlockCommand struct { + Meta +} + +func (c *UnlockCommand) Run(args []string) int { + args = c.Meta.process(args, false) + + cmdFlags := c.Meta.flagSet("unlock") + cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } + if err := cmdFlags.Parse(args); err != nil { + return 1 + } + + // assume everything is initialized. The user can manually init if this is + // required. + configPath, err := ModulePath(cmdFlags.Args()) + if err != nil { + c.Ui.Error(err.Error()) + return 1 + } + + // Load the backend + b, err := c.Backend(&BackendOpts{ + ConfigPath: configPath, + }) + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to load backend: %s", err)) + return 1 + } + + st, err := b.State() + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err)) + return 1 + } + + s, ok := st.(state.Locker) + if !ok { + c.Ui.Error("Current state does not support locking") + return 1 + } + + if err := s.Unlock(); err != nil { + c.Ui.Error(fmt.Sprintf("Failed to unlock state: %s", err)) + return 1 + } + + return 0 +} + +func (c *UnlockCommand) Help() string { + helpText := ` +Usage: terraform unlock [DIR] + + Manually unlock the state for the defined configuration. + + This will not modify your infrastructure. This command removes the lock on the + state for the current configuration. The behavior of this lock is dependent + on the backend being used. Local state files cannot be unlocked by another + process. +` + return strings.TrimSpace(helpText) +} + +func (c *UnlockCommand) Synopsis() string { + return "Manually unlock the terraform state" +} diff --git a/command/unlock_test.go b/command/unlock_test.go new file mode 100644 index 000000000..b133dcb18 --- /dev/null +++ b/command/unlock_test.go @@ -0,0 +1,46 @@ +package command + +import ( + "os" + "testing" + + "github.com/hashicorp/terraform/terraform" + "github.com/mitchellh/cli" +) + +// Since we can't unlock a local state file, just test that calling unlock +// doesn't fail. +// TODO: mock remote state for UI testing +func TestUnlock(t *testing.T) { + td := tempDir(t) + os.MkdirAll(td, 0755) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + // Write the legacy state + statePath := DefaultStateFilename + { + f, err := os.Create(statePath) + if err != nil { + t.Fatalf("err: %s", err) + } + err = terraform.WriteState(testState(), f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + } + + p := testProvider() + ui := new(cli.MockUi) + c := &UnlockCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + if code := c.Run(nil); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } +} From 65abe9804727e1fbc63a78c526d19e0b9bbecf45 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 7 Feb 2017 18:00:09 -0500 Subject: [PATCH 3/3] Remove lock command and rename lock/force-unlock Remove the lock command for now to avoid confusion about the behavior of locks. Rename lock to force-unlock to make it more aparent what it does. Add a success message, and chose red because it can be a dangerous operation. Add confirmation akin to `destroy`, and a `-force` option for automation and testing. --- command/lock.go | 78 --------------------------------- command/lock_test.go | 97 ------------------------------------------ command/unlock.go | 59 +++++++++++++++++++++++-- command/unlock_test.go | 2 +- commands.go | 6 +++ 5 files changed, 63 insertions(+), 179 deletions(-) delete mode 100644 command/lock.go delete mode 100644 command/lock_test.go diff --git a/command/lock.go b/command/lock.go deleted file mode 100644 index e6b2d211b..000000000 --- a/command/lock.go +++ /dev/null @@ -1,78 +0,0 @@ -package command - -import ( - "fmt" - "strings" - - "github.com/hashicorp/terraform/state" -) - -// LockCommand is a cli.Command implementation that manually locks -// the state. -type LockCommand struct { - Meta -} - -func (c *LockCommand) Run(args []string) int { - args = c.Meta.process(args, false) - - cmdFlags := c.Meta.flagSet("lock") - cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } - if err := cmdFlags.Parse(args); err != nil { - return 1 - } - - // assume everything is initialized. The user can manually init if this is - // required. - configPath, err := ModulePath(cmdFlags.Args()) - if err != nil { - c.Ui.Error(err.Error()) - return 1 - } - - // Load the backend - b, err := c.Backend(&BackendOpts{ - ConfigPath: configPath, - }) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load backend: %s", err)) - return 1 - } - - st, err := b.State() - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err)) - return 1 - } - - s, ok := st.(state.Locker) - if !ok { - c.Ui.Error("Current state does not support locking") - return 1 - } - - if err := s.Lock("lock"); err != nil { - c.Ui.Error(fmt.Sprintf("Failed to lock state: %s", err)) - return 1 - } - - return 0 -} - -func (c *LockCommand) Help() string { - helpText := ` -Usage: terraform lock [DIR] - - Manually lock the state for the defined configuration. - - This will not modify your infrastructure. This command obtains a lock on the - state for the current configuration. The behavior of this lock is dependent - on the backend being used. A lock on a local state file only lasts for the - duration of the calling process. -` - return strings.TrimSpace(helpText) -} - -func (c *LockCommand) Synopsis() string { - return "Manually lock the terraform state" -} diff --git a/command/lock_test.go b/command/lock_test.go deleted file mode 100644 index 0e5331133..000000000 --- a/command/lock_test.go +++ /dev/null @@ -1,97 +0,0 @@ -package command - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "github.com/hashicorp/terraform/terraform" - "github.com/mitchellh/cli" -) - -func TestLock(t *testing.T) { - testData, _ := filepath.Abs("./testdata") - - td := tempDir(t) - os.MkdirAll(td, 0755) - defer os.RemoveAll(td) - defer testChdir(t, td)() - - // Write the legacy state - statePath := DefaultStateFilename - { - f, err := os.Create(statePath) - if err != nil { - t.Fatalf("err: %s", err) - } - err = terraform.WriteState(testState(), f) - f.Close() - if err != nil { - t.Fatalf("err: %s", err) - } - } - - p := testProvider() - ui := new(cli.MockUi) - c := &LockCommand{ - Meta: Meta{ - ContextOpts: testCtxConfig(p), - Ui: ui, - }, - } - - if code := c.Run(nil); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) - } - - unlock, err := testLockState(testData, statePath) - if err == nil { - unlock() - t.Fatal("expected error locking state") - } else if !strings.Contains(err.Error(), "locked") { - t.Fatal("does not appear to be a lock error:", err) - } -} - -func TestLock_lockedState(t *testing.T) { - testData, _ := filepath.Abs("./testdata") - - td := tempDir(t) - os.MkdirAll(td, 0755) - defer os.RemoveAll(td) - defer testChdir(t, td)() - - // Write the legacy state - statePath := DefaultStateFilename - { - f, err := os.Create(statePath) - if err != nil { - t.Fatalf("err: %s", err) - } - err = terraform.WriteState(testState(), f) - f.Close() - if err != nil { - t.Fatalf("err: %s", err) - } - } - - p := testProvider() - ui := new(cli.MockUi) - c := &LockCommand{ - Meta: Meta{ - ContextOpts: testCtxConfig(p), - Ui: ui, - }, - } - - unlock, err := testLockState(testData, statePath) - if err != nil { - t.Fatal(err) - } - defer unlock() - - if code := c.Run(nil); code == 0 { - t.Fatal("expected error when locking a locked state") - } -} diff --git a/command/unlock.go b/command/unlock.go index c31e82ad9..f277df693 100644 --- a/command/unlock.go +++ b/command/unlock.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/hashicorp/terraform/state" + "github.com/hashicorp/terraform/terraform" ) // UnlockCommand is a cli.Command implementation that manually unlocks @@ -16,7 +17,9 @@ type UnlockCommand struct { func (c *UnlockCommand) Run(args []string) int { args = c.Meta.process(args, false) - cmdFlags := c.Meta.flagSet("unlock") + force := false + cmdFlags := c.Meta.flagSet("force-unlock") + cmdFlags.BoolVar(&force, "force", false, "force") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -47,21 +50,60 @@ func (c *UnlockCommand) Run(args []string) int { s, ok := st.(state.Locker) if !ok { - c.Ui.Error("Current state does not support locking") + c.Ui.Error("The remote state backend in use does not support locking, and therefor\n" + + "cannot be unlocked.") return 1 } + isLocal := false + switch s := st.(type) { + case *state.BackupState: + if _, ok := s.Real.(*state.LocalState); ok { + isLocal = true + } + case *state.LocalState: + isLocal = true + } + + if !force { + // Forcing this doesn't do anything, but doesn't break anything either, + // and allows us to run the basic command test too. + if isLocal { + c.Ui.Error("Local state cannot be unlocked by another process") + return 1 + } + + desc := "Terraform will remove the lock on the remote state.\n" + + "This will allow local Terraform commands to modify this state, even though it\n" + + "may be still be in use. Only 'yes' will be accepted to confirm." + + v, err := c.UIInput().Input(&terraform.InputOpts{ + Id: "force-unlock", + Query: "Do you really want to force-unlock?", + Description: desc, + }) + if err != nil { + c.Ui.Error(fmt.Sprintf("Error asking for confirmation: %s", err)) + return 1 + } + if v != "yes" { + c.Ui.Output("force-unlock cancelled.") + return 1 + } + } + if err := s.Unlock(); err != nil { c.Ui.Error(fmt.Sprintf("Failed to unlock state: %s", err)) return 1 } + c.Ui.Output(c.Colorize().Color(strings.TrimSpace(outputUnlockSuccess))) return 0 } func (c *UnlockCommand) Help() string { helpText := ` -Usage: terraform unlock [DIR] +Usage: terraform force-unlock [DIR] Manually unlock the state for the defined configuration. @@ -69,6 +111,10 @@ Usage: terraform unlock [DIR] state for the current configuration. The behavior of this lock is dependent on the backend being used. Local state files cannot be unlocked by another process. + +Options: + + -force Don't ask for input for unlock confirmation. ` return strings.TrimSpace(helpText) } @@ -76,3 +122,10 @@ Usage: terraform unlock [DIR] func (c *UnlockCommand) Synopsis() string { return "Manually unlock the terraform state" } + +const outputUnlockSuccess = ` +[reset][bold][red]Terraform state has been successfully unlocked![reset][red] + +The state has been unlocked, and Terraform commands should now be able to +obtain a new lock on the remote state. +` diff --git a/command/unlock_test.go b/command/unlock_test.go index b133dcb18..2496f3fe6 100644 --- a/command/unlock_test.go +++ b/command/unlock_test.go @@ -40,7 +40,7 @@ func TestUnlock(t *testing.T) { }, } - if code := c.Run(nil); code != 0 { + if code := c.Run([]string{"-force"}); code != 0 { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } } diff --git a/commands.go b/commands.go index ec4d03793..20e2ff892 100644 --- a/commands.go +++ b/commands.go @@ -75,6 +75,12 @@ func init() { }, nil }, + "force-unlock": func() (cli.Command, error) { + return &command.UnlockCommand{ + Meta: meta, + }, nil + }, + "get": func() (cli.Command, error) { return &command.GetCommand{ Meta: meta,