From 08cff7cc13f260cd0c4ed5df67f1541e845eefda Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 15 Feb 2017 10:53:04 -0500
Subject: [PATCH] have LocalState check Lock ID on Unlock

Have LocalState store and check the lock ID, and strictly enforce
unlocking with the correct ID.

This isn't required for local lock correctness, as we track the file
descriptor to unlock, but it does provide a varification that locking
and unlocking is done correctly throughout terraform.
---
 state/local.go      | 28 ++++++++++++++++++++++++----
 state/local_test.go |  7 ++++---
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/state/local.go b/state/local.go
index c92cc8e48e..43bb6c66f4 100644
--- a/state/local.go
+++ b/state/local.go
@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -24,6 +25,11 @@ type LocalState struct {
 	// the file handle corresponding to PathOut
 	stateFileOut *os.File
 
+	// While the stateFileOut will correspond to the lock directly,
+	// store and check the lock ID to maintain a strict state.Locker
+	// implementation.
+	lockID string
+
 	// created is set to true if stateFileOut didn't exist before we created it.
 	// This is mostly so we can clean up emtpy files during tests, but doesn't
 	// hurt to remove file we never wrote to.
@@ -149,13 +155,26 @@ func (s *LocalState) Lock(info *LockInfo) (string, error) {
 		return "", fmt.Errorf("state file %q locked: %s", s.Path, err)
 	}
 
-	return "", s.writeLockInfo(info)
+	s.lockID = info.ID
+	return s.lockID, s.writeLockInfo(info)
 }
 
 func (s *LocalState) Unlock(id string) error {
-	// we can't be locked if we don't have a file
-	if s.stateFileOut == nil {
-		return nil
+	if s.lockID == "" {
+		return fmt.Errorf("LocalState not locked")
+	}
+
+	if id != s.lockID {
+		idErr := fmt.Errorf("invalid lock id: %q. current id: %q", id, s.lockID)
+		info, err := s.lockInfo()
+		if err == nil {
+			return errwrap.Wrap(idErr, err)
+		}
+
+		return &LockError{
+			Err:  idErr,
+			Info: info,
+		}
 	}
 
 	os.Remove(s.lockInfoPath())
@@ -166,6 +185,7 @@ func (s *LocalState) Unlock(id string) error {
 
 	s.stateFileOut.Close()
 	s.stateFileOut = nil
+	s.lockID = ""
 
 	// clean up the state file if we created it an never wrote to it
 	stat, err := os.Stat(fileName)
diff --git a/state/local_test.go b/state/local_test.go
index b0472aaa6b..27d1763848 100644
--- a/state/local_test.go
+++ b/state/local_test.go
@@ -57,12 +57,13 @@ func TestLocalStateLocks(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Unlock should be repeatable
 	if err := s.Unlock(lockID); err != nil {
 		t.Fatal(err)
 	}
-	if err := s.Unlock(lockID); err != nil {
-		t.Fatal(err)
+
+	// we should not be able to unlock the same lock twice
+	if err := s.Unlock(lockID); err == nil {
+		t.Fatal("unlocking an unlocked state should fail")
 	}
 
 	// make sure lock info is gone