Unlock locks when pull request merged (#19)

* Add new FindLocksForPull method to locking backends
* Listen for pull request closed events and delete locks
* DynamoDB handling its own serialization. Implement LocksByPull
* Fix routing to work with encoded lockID
* Move locking backends to individual packages
* Change flag from locking-table to locking-dynamodb-table

cmd/server.go

@@ -29,7 +29,7 @@ const (
 	atlantisURLFlag     = "atlantis-url"
 	dataDirFlag         = "data-dir"
 	lockingBackendFlag  = "locking-backend"
-	lockingTableFlag    = "locking-table"
+	lockingTableFlag    = "locking-dynamodb-table"
 )
 
 var stringFlags = []stringFlag{
@@ -139,6 +139,7 @@ var serverCmd = &cobra.Command{
 
 Flags can also be set in a yaml config file (see --` + configFlag + `).
 Config values are overridden by environment variables which in turn are overridden by flags.`,
+	SilenceUsage: true,
 
 	PreRunE: func(cmd *cobra.Command, args []string) error {
 		// if passed a config file then try and load it
@@ -208,6 +209,9 @@ func validate(config server.ServerConfig) error {
 	if config.GitHubPassword == "" {
 		return fmt.Errorf("%s must be set", ghPasswordFlag)
 	}
+	if config.LockingBackend != server.LockingFileBackend && config.LockingBackend != server.LockingDynamoDBBackend {
+		return fmt.Errorf("unsupported locking backend %q: not one of %q or %q", config.LockingBackend, server.LockingFileBackend, server.LockingDynamoDBBackend)
+	}
 	return nil
 }
 

locking/boltdb/boltdb.go

@@ -0,0 +1,151 @@
+package boltdb
+
+import (
+	"github.com/boltdb/bolt"
+	"fmt"
+	"encoding/json"
+	"github.com/pkg/errors"
+	"bytes"
+	"os"
+	"time"
+	"path"
+	"github.com/hootsuite/atlantis/locking"
+)
+
+type Backend struct {
+	db          *bolt.DB
+	bucket []byte
+}
+
+const bucketName = "runLocks"
+
+func New(dataDir string) (*Backend, error) {
+	if err := os.MkdirAll(dataDir, 0755); err != nil {
+		return nil, errors.Wrap(err,"creating data dir")
+	}
+	db, err := bolt.Open(path.Join(dataDir, "atlantis.db"), 0600, &bolt.Options{Timeout: 1 * time.Second})
+	if err != nil {
+		if err.Error() == "timeout" {
+			return nil, errors.New("starting BoltDB: timeout (a possible cause is another Atlantis instance already running)")
+		}
+		return nil, errors.Wrap(err,"starting BoltDB")
+	}
+	err = db.Update(func(tx *bolt.Tx) error {
+		if _, err := tx.CreateBucketIfNotExists([]byte(bucketName)); err != nil {
+			return errors.Wrapf(err, "creating %q bucketName", bucketName)
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, errors.Wrap(err,"starting BoltDB")
+	}
+	// todo: close BoltDB when server is sigtermed
+	return &Backend{db, []byte(bucketName)}, nil
+}
+
+// NewWithDB is used for testing
+func NewWithDB(db *bolt.DB, bucket string) (*Backend, error) {
+	return &Backend{db, []byte(bucket)}, nil
+}
+
+func (b Backend) TryLock(run locking.Run) (locking.TryLockResponse, error) {
+	var response locking.TryLockResponse
+	newRunSerialized, _ := json.Marshal(run)
+	lockID := run.StateKey()
+	transactionErr := b.db.Update(func(tx *bolt.Tx) error {
+		bucket := tx.Bucket(b.bucket)
+
+		// if there is no run at that key then we're free to create the lock
+		lockingRunSerialized := bucket.Get([]byte(lockID))
+		if lockingRunSerialized == nil {
+			bucket.Put([]byte(lockID), newRunSerialized) // not a readonly bucketName so okay to ignore error
+			response = locking.TryLockResponse{
+				LockAcquired: true,
+				LockingRun:   run,
+				LockID:       lockID,
+			}
+			return nil
+		}
+
+		// otherwise the lock fails, return to caller the run that's holding the lock
+		var lockingRun locking.Run
+		if err := b.deserialize(lockingRunSerialized, &lockingRun); err != nil {
+			return errors.Wrap(err, "failed to deserialize run")
+		}
+		response = locking.TryLockResponse{
+			LockAcquired: false,
+			LockingRun: lockingRun,
+			LockID: lockID,
+		}
+		return nil
+	})
+
+	if transactionErr != nil {
+		return response, errors.Wrap(transactionErr, "DB transaction failed")
+	}
+
+	return response, nil
+}
+
+func (b Backend) Unlock(lockID string) error {
+	err := b.db.Update(func(tx *bolt.Tx) error {
+		locks := tx.Bucket(b.bucket)
+		return locks.Delete([]byte(lockID))
+	})
+	return errors.Wrap(err, "DB transaction failed")
+}
+
+func (b Backend) ListLocks() (map[string]locking.Run, error) {
+	m := make(map[string]locking.Run)
+	bytes := make(map[string][]byte)
+
+	err := b.db.View(func(tx *bolt.Tx) error {
+		bucket := tx.Bucket(b.bucket)
+		c := bucket.Cursor()
+		for k, v := c.First(); k != nil; k, v = c.Next() {
+			bytes[string(k)] = v
+		}
+		return nil
+	})
+	if err != nil {
+		return m, errors.Wrap(err, "DB transaction failed")
+	}
+
+	// deserialize bytes into the proper objects
+	for k, v := range bytes {
+		var run locking.Run
+		if err := b.deserialize(v, &run); err != nil {
+			return m, errors.Wrap(err, fmt.Sprintf("failed to deserialize run at key %q", string(k)))
+		}
+		m[k] = run
+	}
+
+	return m, nil
+}
+
+func (b Backend) FindLocksForPull(repoFullName string, pullNum int) ([]string, error) {
+	var ids []string
+	err := b.db.View(func(tx *bolt.Tx) error {
+		c := tx.Bucket(b.bucket).Cursor()
+
+		// the key for each lock is repoFullName/path/env so we can scan through all entries
+		// and get the locks for that repo. Then we can check if the lock is for the right pull
+		prefix := []byte(repoFullName + "/")
+		for k, v := c.Seek(prefix); k != nil && bytes.HasPrefix(k, prefix); k, v = c.Next() {
+			var run locking.Run
+			if err := b.deserialize(v, &run); err != nil {
+				return errors.Wrapf(err, "failed to deserialize run at key %q", string(k))
+			}
+			if run.PullNum == pullNum {
+				ids = append(ids, string(k))
+			}
+		}
+
+		return nil
+	})
+	return ids, err
+}
+
+func (b Backend) deserialize(bs []byte, run *locking.Run) error {
+	return json.Unmarshal(bs, run)
+}

locking/boltdb/boltdb_test.go

@@ -0,0 +1,325 @@
+package boltdb_test
+
+import (
+	"github.com/hootsuite/atlantis/locking"
+	. "github.com/hootsuite/atlantis/testing_util"
+
+	"github.com/boltdb/bolt"
+	"os"
+	"testing"
+	"io/ioutil"
+	"github.com/pkg/errors"
+
+	"time"
+	"github.com/hootsuite/atlantis/locking/boltdb"
+)
+
+var lockBucket = "bucket"
+var run = locking.Run{
+	RepoFullName: "owner/repo",
+	Path: "parent/child",
+	Env: "default",
+	PullNum: 1,
+	User: "user",
+	Timestamp: time.Now(),
+}
+
+func TestListNoLocks(t *testing.T) {
+	t.Log("listing locks when there are none should return an empty list")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	ls, err := b.ListLocks()
+	Ok(t, err)
+	Equals(t, 0, len(ls))
+}
+
+func TestListOneLock(t *testing.T) {
+	t.Log("listing locks when there is one should return it")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	_, err := b.TryLock(run)
+	Ok(t, err)
+	ls, err := b.ListLocks()
+	Ok(t, err)
+	Equals(t, 1, len(ls))
+	Equals(t, run, ls[run.StateKey()])
+}
+
+func TestListMultipleLocks(t *testing.T) {
+	t.Log("listing locks when there are multiple should return them")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+
+	// add multiple locks
+	_, err := b.TryLock(run)
+	Ok(t, err)
+
+	run2 := run
+	run2.RepoFullName = "different"
+	_, err = b.TryLock(run2)
+	Ok(t, err)
+
+	run3 := run
+	run3.RepoFullName = "different again"
+	_, err = b.TryLock(run3)
+	Ok(t, err)
+
+	run4 := run
+	run4.RepoFullName = "different again again"
+	_, err = b.TryLock(run4)
+	Ok(t, err)
+
+	ls, err := b.ListLocks()
+	Ok(t, err)
+	Equals(t, 4, len(ls))
+	Equals(t, run, ls[run.StateKey()])
+	Equals(t, run2, ls[run2.StateKey()])
+	Equals(t, run3, ls[run3.StateKey()])
+	Equals(t, run4, ls[run4.StateKey()])
+}
+
+func TestListAddRemove(t *testing.T) {
+	t.Log("listing after adding and removing should return none")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	_, err := b.TryLock(run)
+	Ok(t, err)
+	b.Unlock(run.StateKey())
+
+	ls, err := b.ListLocks()
+	Ok(t, err)
+	Equals(t, 0, len(ls))
+}
+
+func TestLockingNoLocks(t *testing.T) {
+	t.Log("with no locks yet, lock should succeed")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	r, err := b.TryLock(run)
+	Ok(t, err)
+	Equals(t, true, r.LockAcquired)
+	Equals(t, run, r.LockingRun)
+	Equals(t, run.StateKey(), r.LockID)
+}
+
+func TestLockingExistingLock(t *testing.T) {
+	t.Log("if there is an existing lock, lock should...")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	_, err := b.TryLock(run)
+	Ok(t, err)
+
+	t.Log("...succeed if the new run has a different path")
+	{
+		new := run
+		new.Path = "different/path"
+		r, err := b.TryLock(new)
+		Ok(t, err)
+		Equals(t, true, r.LockAcquired)
+		Equals(t, new, r.LockingRun)
+		Equals(t, new.StateKey(), r.LockID)
+	}
+
+	t.Log("...succeed if the new run has a different environment")
+	{
+		new := run
+		new.Env = "different-env"
+		r, err := b.TryLock(new)
+		Ok(t, err)
+		Equals(t, true, r.LockAcquired)
+		Equals(t, new, r.LockingRun)
+		Equals(t, new.StateKey(), r.LockID)
+	}
+
+	t.Log("...succeed if the new run has a different repoName")
+	{
+		new := run
+		new.RepoFullName = "new/repo"
+		r, err := b.TryLock(new)
+		Ok(t, err)
+		Equals(t, true, r.LockAcquired)
+		Equals(t, new, r.LockingRun)
+		Equals(t, new.StateKey(), r.LockID)
+	}
+
+	t.Log("...not succeed if the new run only has a different pullNum")
+	{
+		new := run
+		new.PullNum = run.PullNum + 1
+		r, err := b.TryLock(new)
+		Ok(t, err)
+		Equals(t, false, r.LockAcquired)
+		Equals(t, run, r.LockingRun)
+		Equals(t, run.StateKey(), r.LockID)
+	}
+}
+
+func TestUnlockingNoLocks(t *testing.T) {
+	t.Log("unlocking with no locks should succeed")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+
+	Ok(t, b.Unlock("any-lock-id"))
+}
+
+func TestUnlocking(t *testing.T) {
+	t.Log("unlocking with an existing lock should succeed")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+
+	b.TryLock(run)
+	Ok(t, b.Unlock(run.StateKey()))
+
+	// should be no locks listed
+	ls, err := b.ListLocks()
+	Ok(t, err)
+	Equals(t, 0, len(ls))
+
+	// should be able to re-lock that repo with a new pull num
+	new := run
+	new.PullNum = run.PullNum + 1
+	r, err := b.TryLock(new)
+	Ok(t, err)
+	Equals(t, true, r.LockAcquired)
+}
+
+func TestUnlockingMultiple(t *testing.T) {
+	t.Log("unlocking and locking multiple locks should succeed")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+
+	b.TryLock(run)
+
+	new := run
+	new.RepoFullName = "new/repo"
+	b.TryLock(new)
+
+	new2 := run
+	new2.Path = "new/path"
+	b.TryLock(new2)
+
+	new3 := run
+	new3.Env = "new/env"
+	b.TryLock(new3)
+
+	// now try and unlock them
+	Ok(t, b.Unlock(new3.StateKey()))
+	Ok(t, b.Unlock(new2.StateKey()))
+	Ok(t, b.Unlock(new.StateKey()))
+	Ok(t, b.Unlock(run.StateKey()))
+
+	// should be none left
+	ls, err := b.ListLocks()
+	Ok(t, err)
+	Equals(t, 0, len(ls))
+}
+
+func TestFindLocksNone(t *testing.T) {
+	t.Log("find should return no locks when there are none")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+
+	ls, err := b.FindLocksForPull("any/repo", 1)
+	Ok(t, err)
+	Equals(t, 0, len(ls))
+}
+
+func TestFindLocksOne(t *testing.T) {
+	t.Log("with one lock find should...")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	_, err := b.TryLock(run)
+	Ok(t, err)
+
+	t.Log("...return no locks from the same repo but different pull num")
+	{
+		ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum + 1)
+		Ok(t, err)
+		Equals(t, 0, len(ls))
+	}
+	t.Log("...return no locks from a different repo but the same pull num")
+	{
+		ls, err := b.FindLocksForPull(run.RepoFullName + "dif", run.PullNum)
+		Ok(t, err)
+		Equals(t, 0, len(ls))
+	}
+	t.Log("...return the one lock when called with that repo and pull num")
+	{
+		ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum)
+		Ok(t, err)
+		Equals(t, 1, len(ls))
+		Equals(t, run.StateKey(), ls[0])
+	}
+}
+
+func TestFindLocksAfterUnlock(t *testing.T) {
+	t.Log("after locking and unlocking find should return no locks")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	_, err := b.TryLock(run)
+	Ok(t, err)
+	Ok(t, b.Unlock(run.StateKey()))
+
+	ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum)
+	Ok(t, err)
+	Equals(t, 0, len(ls))
+}
+
+func TestFindMultipleMatching(t *testing.T) {
+	t.Log("find should return all matching lock ids")
+	db, b := newTestDB()
+	defer cleanupDB(db)
+	_, err := b.TryLock(run)
+	Ok(t, err)
+
+	// add additional locks with the same repo and pull num but different paths/envs
+	new := run
+	new.Path = "dif/path"
+	_, err = b.TryLock(new)
+	Ok(t, err)
+	new2 := run
+	new2.Env = "new-env"
+	_, err = b.TryLock(new2)
+	Ok(t, err)
+
+	// should get all of them back
+	ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum)
+	Ok(t, err)
+	Equals(t, 3, len(ls))
+	ContainsStr(t, run.StateKey(), ls)
+	ContainsStr(t, new.StateKey(), ls)
+	ContainsStr(t, new2.StateKey(), ls)
+}
+
+// newTestDB returns a TestDB using a temporary path.
+func newTestDB() (*bolt.DB, *boltdb.Backend) {
+	// Retrieve a temporary path.
+	f, err := ioutil.TempFile("", "")
+	if err != nil {
+		panic(errors.Wrap(err, "failed to create temp file"))
+	}
+	path := f.Name()
+	f.Close()
+
+	// Open the database.
+	db, err := bolt.Open(path, 0600, nil)
+	if err != nil {
+		panic(errors.Wrap(err, "could not start bolt DB"))
+	}
+	if err := db.Update(func(tx *bolt.Tx) error {
+		if _, err := tx.CreateBucketIfNotExists([]byte(lockBucket)); err != nil {
+			return errors.Wrap(err, "failed to create bucket")
+		}
+		return nil
+	}); err != nil {
+		panic(errors.Wrap(err, "could not create bucket"))
+	}
+	b, _ := boltdb.NewWithDB(db, lockBucket)
+	return db, b
+}
+
+func cleanupDB(db *bolt.DB) {
+	os.Remove(db.Path())
+	db.Close()
+}
+

locking/boltdb_lock_manager.go

@@ -1,141 +0,0 @@
-package locking
-
-import (
-	"github.com/boltdb/bolt"
-	"fmt"
-	"encoding/json"
-	"github.com/pkg/errors"
-	"encoding/hex"
-	"os"
-	"time"
-	"path"
-)
-
-type BoltDBLockManager struct {
-	db          *bolt.DB
-	locksBucket []byte
-}
-
-func NewBoltDBLockManager(dataDir string, locksBucket string) (*BoltDBLockManager, error) {
-	if err := os.MkdirAll(dataDir, 0755); err != nil {
-		return nil, errors.Wrap(err,"creating data dir")
-	}
-	db, err := bolt.Open(path.Join(dataDir, "atlantis.db"), 0600, &bolt.Options{Timeout: 1 * time.Second})
-	if err != nil {
-		if err.Error() == "timeout" {
-			return nil, errors.New("starting BoltDB: timeout (a possible cause is another Atlantis instance already running)")
-		}
-		return nil, errors.Wrap(err,"starting BoltDB")
-	}
-	err = db.Update(func(tx *bolt.Tx) error {
-		if _, err := tx.CreateBucketIfNotExists([]byte(locksBucket)); err != nil {
-			return errors.Wrapf(err, "creating %q bucket", locksBucket)
-		}
-		return nil
-	})
-	if err != nil {
-		return nil, errors.Wrap(err,"starting BoltDB")
-	}
-	// todo: close BoltDB when server is sigtermed
-	return &BoltDBLockManager{db, []byte(locksBucket)}, nil
-}
-
-// NewBoltDBLockManagerWithDB is used for testing
-func NewBoltDBLockManagerWithDB(db *bolt.DB, locksBucket string) (*BoltDBLockManager, error) {
-	return &BoltDBLockManager{db, []byte(locksBucket)}, nil
-}
-
-func (b BoltDBLockManager) TryLock(run Run) (TryLockResponse, error) {
-	var response TryLockResponse
-	newRunSerialized, err := b.serialize(run)
-	if err != nil {
-		return response, errors.Wrap(err, "failed to serialize run")
-	}
-
-	lockId := run.StateKey()
-	transactionErr := b.db.Update(func(tx *bolt.Tx) error {
-		locksBucket := tx.Bucket(b.locksBucket)
-
-		// if there is no run at that key then we're free to create the lock
-		lockingRunSerialized := locksBucket.Get(lockId)
-		if lockingRunSerialized == nil {
-			locksBucket.Put(lockId, newRunSerialized) // not a readonly bucket so okay to ignore error
-			response = TryLockResponse{
-				LockAcquired: true,
-				LockingRun: run,
-				LockID: b.lockIDToString(lockId),
-			}
-			return nil
-		}
-
-		// otherwise the lock fails, return to caller the run that's holding the lock
-		var lockingRun Run
-		if err := b.deserialize(lockingRunSerialized, &lockingRun); err != nil {
-			return errors.Wrap(err, "failed to deserialize run")
-		}
-		response = TryLockResponse{
-			LockAcquired: false,
-			LockingRun: lockingRun,
-			LockID: b.lockIDToString(lockId),
-		}
-		return nil
-	})
-
-	if transactionErr != nil {
-		return response, errors.Wrap(transactionErr, "DB transaction failed")
-	}
-
-	return response, nil
-}
-
-func (b BoltDBLockManager) Unlock(lockID string) error {
-	idAsHex, err := hex.DecodeString(lockID)
-	if err != nil {
-		return errors.Wrap(err, "id was not in correct format")
-	}
-	err = b.db.Update(func(tx *bolt.Tx) error {
-		locks := tx.Bucket(b.locksBucket)
-		return locks.Delete(idAsHex)
-	})
-	return errors.Wrap(err, "DB transaction failed")
-}
-
-func (b BoltDBLockManager) ListLocks() (map[string]Run, error) {
-	m := make(map[string]Run)
-	bytes := make(map[string][]byte)
-
-	err := b.db.View(func(tx *bolt.Tx) error {
-		bucket := tx.Bucket(b.locksBucket)
-		c := bucket.Cursor()
-		for k, v := c.First(); k != nil; k, v = c.Next() {
-			bytes[b.lockIDToString(k)] = v
-		}
-		return nil
-	})
-	if err != nil {
-		return m, errors.Wrap(err, "DB transaction failed")
-	}
-
-	// deserialize bytes into the proper objects
-	for k, v := range bytes {
-		var run Run
-		if err := b.deserialize(v, &run); err != nil {
-			return m, errors.Wrap(err, fmt.Sprintf("failed to deserialize run at key %q", string(k)))
-		}
-		m[k] = run
-	}
-
-	return m, nil
-}
-
-func (b BoltDBLockManager) lockIDToString(key []byte) string {
-	return string(hex.EncodeToString(key))
-}
-
-func (b BoltDBLockManager) deserialize(bs []byte, run *Run) error {
-	return json.Unmarshal(bs, run)
-}
-
-func (b BoltDBLockManager) serialize(run Run) ([]byte, error) {
-	return json.Marshal(run)
-}

locking/boltdb_lock_manager_test.go

@@ -1,240 +0,0 @@
-package locking_test
-
-import (
-	"testing"
-	"github.com/hootsuite/atlantis/locking"
-	"time"
-	. "github.com/hootsuite/atlantis/testing_util"
-	"os"
-	"github.com/boltdb/bolt"
-	"io/ioutil"
-	"github.com/pkg/errors"
-)
-
-const LockBucket = "locks"
-
-var run = locking.Run{
-	RepoFullName: "owner/repo",
-	Path: "parent/child",
-	Env: "default",
-	PullNum: 1,
-	User: "user",
-	Timestamp: time.Now(),
-}
-
-func keyWithNewField(fieldName string, value string) locking.Run {
-	copy := run
-	switch fieldName {
-	case "RepoFullName":
-		copy.RepoFullName = value
-		return copy
-	case "Path":
-		copy.Path = value
-		return copy
-	case "Env":
-		copy.Env = value
-		return copy
-	default:
-		return copy
-	}
-}
-
-// NewTestDB returns a TestDB using a temporary path.
-func NewTestDB() (*bolt.DB, *locking.BoltDBLockManager) {
-	// Retrieve a temporary path.
-	f, err := ioutil.TempFile("", "")
-	if err != nil {
-		panic(errors.Wrap(err, "failed to create temp file"))
-	}
-	path := f.Name()
-	f.Close()
-
-	// Open the database.
-	db, err := bolt.Open(path, 0600, nil)
-	if err != nil {
-		panic(errors.Wrap(err, "could not start bolt DB"))
-	}
-	if err := db.Update(func(tx *bolt.Tx) error {
-		if _, err := tx.CreateBucketIfNotExists([]byte(LockBucket)); err != nil {
-			return errors.Wrap(err, "failed to create bucket")
-		}
-		return nil
-	}); err != nil {
-		panic(errors.Wrap(err, "could not create bucket"))
-	}
-	b, _ := locking.NewBoltDBLockManagerWithDB(db, LockBucket)
-	return db, b
-}
-
-func cleanupDB(db *bolt.DB) {
-	os.Remove(db.Path())
-	db.Close()
-}
-
-func TestListWhenNoLocks(t *testing.T) {
-	db, b := NewTestDB()
-	defer cleanupDB(db)
-	m, err := b.ListLocks()
-	Ok(t, err)
-	Equals(t, 0, len(m))
-}
-
-func TestListLocks(t *testing.T) {
-	db, b := NewTestDB()
-	defer cleanupDB(db)
-	_, err := b.TryLock(run)
-	Ok(t, err)
-
-	locks, err := b.ListLocks()
-	Assert(t, len(locks) == 1, "should have 1 lock")
-	for _, v := range locks {
-		Equals(t, run, v)
-	}
-}
-
-type LockCommand struct {
-	run      locking.Run
-	expected locking.TryLockResponse
-}
-
-type UnlockCommand struct {
-	runKey   string
-	expected error
-}
-
-var regularLock = LockCommand{
-	run,
-	locking.TryLockResponse{
-		LockAcquired: true,
-		LockingRun: run,
-		LockID: "cae47fa9dfe223b0d8fefd51f2fbbf9849047c9d048d659d1ba906acc2913534",
-	},
-}
-
-var tableTestData = []struct {
-	sequence    []interface{}
-	description string
-}{
-	{
-		description: "regular lock should succeed",
-		sequence: []interface{}{regularLock},
-	},
-	{
-		description: "unlock when there are no locks should succeed",
-		sequence: []interface{}{
-			UnlockCommand{
-				"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-				nil,
-			},
-		},
-	},
-	{
-		description: "lock and unlock should succeed",
-		sequence: []interface{}{
-			regularLock,
-			UnlockCommand{
-				"cae47fa9dfe223b0d8fefd51f2fbbf9849047c9d048d659d1ba906acc2913534",
-				nil,
-			},
-		},
-	},
-	{
-		description: "lock for same run path should fail",
-		sequence: []interface{}{
-			regularLock,
-			LockCommand{
-				run,
-				locking.TryLockResponse{
-					LockAcquired: false,
-					LockingRun: regularLock.run,
-					LockID: regularLock.expected.LockID,
-				},
-			},
-		},
-	},
-	{
-		description: "lock when existing lock is for different repo name should succeed",
-		sequence: []interface{}{
-			regularLock,
-			LockCommand{
-				keyWithNewField("RepoFullName", "repo/different"),
-				locking.TryLockResponse{
-					LockAcquired: true,
-					LockingRun: keyWithNewField("RepoFullName", "repo/different"),
-				},
-			},
-		},
-	},
-	{
-		description: "lock when existing lock is for different path should succeed",
-		sequence: []interface{}{
-			regularLock,
-			LockCommand{
-				keyWithNewField("Path", "different"),
-				locking.TryLockResponse{
-					LockAcquired: true,
-					LockingRun: keyWithNewField("Path", "different"),
-				},
-			},
-		},
-	},
-	{
-		description: "lock when existing lock is for different env should succeed",
-		sequence: []interface{}{
-			regularLock,
-			LockCommand{
-				keyWithNewField("Env", "different"),
-				locking.TryLockResponse{
-					LockAcquired: true,
-					LockingRun: keyWithNewField("Env", "different"),
-				},
-			},
-		},
-	},
-	{
-		description: "lock, unlock, lock should succeed",
-		sequence: []interface{}{
-			regularLock,
-			UnlockCommand{
-				"cae47fa9dfe223b0d8fefd51f2fbbf9849047c9d048d659d1ba906acc2913534",
-				nil,
-			},
-			LockCommand{
-				run,
-				locking.TryLockResponse{
-					LockAcquired: true,
-					LockingRun: run,
-					LockID: regularLock.expected.LockID,
-				},
-			},
-		},
-	},
-}
-
-func TestAllCases(t *testing.T) {
-	for _, testCase := range tableTestData {
-		t.Logf("testing: %q", testCase.description)
-
-		db, b := NewTestDB()
-		for i, cmd := range testCase.sequence {
-			t.Logf("index: %d", i)
-			switch cmd.(type) {
-			case LockCommand:
-				lockCmd := cmd.(LockCommand)
-				r, err := b.TryLock(lockCmd.run)
-				Ok(t, err)
-				Equals(t, lockCmd.expected.LockAcquired, r.LockAcquired)
-				Equals(t, lockCmd.expected.LockingRun, r.LockingRun)
-				if lockCmd.expected.LockID != "" {
-					Equals(t, lockCmd.expected.LockID, r.LockID)
-				}
-			case UnlockCommand:
-				unlockCmd := cmd.(UnlockCommand)
-				err := b.Unlock(unlockCmd.runKey)
-				Equals(t, unlockCmd.expected, err)
-			}
-		}
-		cleanupDB(db)
-	}
-}
-

locking/dynamodb/dynamodb.go

@@ -0,0 +1,215 @@
+package dynamodb
+
+import (
+	"github.com/aws/aws-sdk-go/service/dynamodb"
+	"github.com/aws/aws-sdk-go/aws/client"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/pkg/errors"
+	"encoding/json"
+	"fmt"
+	"encoding/hex"
+	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface"
+	"time"
+	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbattribute"
+	"strconv"
+	"github.com/hootsuite/atlantis/locking"
+)
+
+type Backend struct {
+	DB        dynamodbiface.DynamoDBAPI
+	LockTable string
+}
+
+type dynamoRun struct {
+	LockID       string
+	RepoFullName string
+	Path         string
+	Env          string
+	PullNum      int
+	User         string
+	Timestamp    time.Time
+}
+
+func New(lockTable string, p client.ConfigProvider) *Backend {
+	return &Backend{
+		DB:        dynamodb.New(p),
+		LockTable: lockTable,
+	}
+}
+
+func (d *Backend) TryLock(run locking.Run) (locking.TryLockResponse, error) {
+	var r locking.TryLockResponse
+	newRunSerialized, err := d.toDynamoItem(run)
+	if err != nil {
+		return r, errors.Wrap(err, "serializing")
+	}
+
+	// check if there is an existing lock
+	getItemParams := &dynamodb.GetItemInput{
+		Key: map[string]*dynamodb.AttributeValue{
+			"LockID": {
+				S: aws.String(run.StateKey()),
+			},
+		},
+		TableName: aws.String(d.LockTable),
+		ConsistentRead: aws.Bool(true),
+	}
+	item, err := d.DB.GetItem(getItemParams)
+	if err != nil {
+		return r, errors.Wrap(err, "checking if lock exists")
+	}
+
+
+	// if there is already a lock then we can't acquire a lock. Return the existing lock
+	if len(item.Item) != 0 {
+		var dynamoRun dynamoRun
+		if err := dynamodbattribute.UnmarshalMap(item.Item, &dynamoRun); err != nil {
+			return r, errors.Wrap(err,"found an existing lock at that id but it could not be deserialized. We suggest manually deleting this key from DynamoDB")
+		}
+		lockingRun := d.fromDynamoItem(dynamoRun)
+		return locking.TryLockResponse{
+			LockAcquired: false,
+			LockingRun: lockingRun,
+			LockID: run.StateKey(),
+		}, nil
+	}
+
+	// else we should be able to lock
+	putItem := &dynamodb.PutItemInput{
+		Item:      newRunSerialized,
+		TableName: aws.String(d.LockTable),
+		// this will ensure that we don't insert the new item in a race situation
+		// where someone has written this key just after our read
+		ConditionExpression: aws.String("attribute_not_exists(LockID)"),
+	}
+	if _, err := d.DB.PutItem(putItem); err != nil {
+		return r, errors.Wrap(err, "writing lock")
+	}
+	return locking.TryLockResponse{
+		LockAcquired: true,
+		LockingRun: run,
+		LockID: run.StateKey(),
+	}, nil
+}
+
+func (d *Backend) Unlock(lockID string) error {
+	params := &dynamodb.DeleteItemInput{
+		Key: map[string]*dynamodb.AttributeValue{
+			"LockID": {S: aws.String(lockID)},
+		},
+		TableName: aws.String(d.LockTable),
+	}
+	_, err := d.DB.DeleteItem(params)
+	return errors.Wrap(err, "deleting lock")
+}
+
+func (d *Backend) ListLocks() (map[string]locking.Run, error) {
+	params := &dynamodb.ScanInput{
+		TableName: aws.String(d.LockTable),
+	}
+
+	m := make(map[string]locking.Run)
+	var err, internalErr error
+	err = d.DB.ScanPages(params, func(out *dynamodb.ScanOutput, lastPage bool) bool {
+		var runs []dynamoRun
+		if err := dynamodbattribute.UnmarshalListOfMaps(out.Items, &runs); err != nil {
+			internalErr = errors.Wrap(err,"deserializing locks")
+			return false
+		}
+		for _, run := range runs {
+			m[run.LockID] = d.fromDynamoItem(run)
+		}
+		return lastPage
+	})
+
+	if err == nil && internalErr != nil {
+		err = internalErr
+	}
+	return m, errors.Wrap(err, "scanning dynamodb")
+}
+
+func (d *Backend) FindLocksForPull(repoFullName string, pullNum int) ([]string, error) {
+	params := &dynamodb.ScanInput{
+		ExpressionAttributeValues: map[string]*dynamodb.AttributeValue{
+			":pullNum": {
+				N: aws.String(strconv.Itoa(pullNum)),
+			},
+			":repoFullName": {
+				S: aws.String(repoFullName),
+			},
+		},
+		FilterExpression: aws.String("RepoFullName = :repoFullName and PullNum = :pullNum"),
+		TableName: aws.String(d.LockTable),
+	}
+
+	var ids []string
+	var err, internalErr error
+	err = d.DB.ScanPages(params, func(out *dynamodb.ScanOutput, lastPage bool) bool {
+		var runs []dynamoRun
+		if err := dynamodbattribute.UnmarshalListOfMaps(out.Items, &runs); err != nil {
+			internalErr = errors.Wrap(err,"deserializing locks")
+			return false
+		}
+		for _, run := range runs {
+			ids = append(ids, run.LockID)
+		}
+		return lastPage
+	})
+
+	if err == nil && internalErr != nil {
+		err = internalErr
+	}
+	return ids, errors.Wrap(err,"scanning dynamodb")
+}
+
+func (d *Backend) deserializeItem(item map[string]*dynamodb.AttributeValue) (string, locking.Run, error) {
+	var lockID string
+	var run locking.Run
+
+	lockIDItem, ok := item["LockID"]
+	if !ok || lockIDItem == nil {
+		return lockID, run, fmt.Errorf("lock did not have expected key 'LockID'")
+	}
+	lockID = string(hex.EncodeToString(lockIDItem.B))
+	runItem, ok := item["Run"]
+	if !ok || runItem == nil {
+		return lockID, run, fmt.Errorf("lock did not have expected key 'Run'")
+	}
+
+	if err := d.deserialize(runItem.B, &run); err != nil {
+		return lockID, run, fmt.Errorf("deserializing run at key %q: %s", lockID, err)
+	}
+	return lockID, run, nil
+}
+
+func (d *Backend) deserialize(bs []byte, run *locking.Run) error {
+	return json.Unmarshal(bs, run)
+}
+
+func (d *Backend) serialize(run locking.Run) ([]byte, error) {
+	return json.Marshal(run)
+}
+
+func (d *Backend) toDynamoItem(run locking.Run) (map[string]*dynamodb.AttributeValue, error) {
+	item := dynamoRun{
+		LockID: run.StateKey(),
+		PullNum: run.PullNum,
+		RepoFullName: run.RepoFullName,
+		Env: run.Env,
+		Path: run.Path,
+		Timestamp: run.Timestamp,
+		User: run.User,
+	}
+	return dynamodbattribute.MarshalMap(item)
+}
+
+func (d *Backend) fromDynamoItem(dynamoRun dynamoRun) locking.Run {
+	return locking.Run{
+		User: dynamoRun.User,
+		Timestamp: dynamoRun.Timestamp,
+		Path: dynamoRun.Path,
+		Env: dynamoRun.Env,
+		RepoFullName: dynamoRun.RepoFullName,
+		PullNum: dynamoRun.PullNum,
+	}
+}

locking/dynamodb_lock_manager.go

@@ -1,152 +0,0 @@
-package locking
-
-import (
-	"github.com/aws/aws-sdk-go/service/dynamodb"
-	"github.com/aws/aws-sdk-go/aws/client"
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/pkg/errors"
-	"encoding/json"
-	"fmt"
-	"encoding/hex"
-	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface"
-)
-
-type DynamoDBLockManager struct {
-	DB        dynamodbiface.DynamoDBAPI
-	LockTable string
-}
-
-func NewDynamoDBLockManager(lockTable string, p client.ConfigProvider) *DynamoDBLockManager {
-	return &DynamoDBLockManager{
-		DB:        dynamodb.New(p),
-		LockTable: lockTable,
-	}
-}
-
-func (d *DynamoDBLockManager) TryLock(run Run) (TryLockResponse, error) {
-	var r TryLockResponse
-	newRunSerialized, err := d.serialize(run)
-	if err != nil {
-		return r, errors.Wrap(err, "serializing run data")
-	}
-
-	// check if there is an existing lock
-	getItemParams := &dynamodb.GetItemInput{
-		Key: map[string]*dynamodb.AttributeValue{
-			"LockID": {
-				B: run.StateKey(),
-			},
-		},
-		TableName: aws.String(d.LockTable),
-		ConsistentRead: aws.Bool(true),
-	}
-	item, err := d.DB.GetItem(getItemParams)
-	if err != nil {
-		return r, errors.Wrap(err, "checking if lock exists")
-	}
-
-	// if there is already a lock then we can't acquire a lock. Return the existing lock
-	if len(item.Item) != 0 {
-		runAttr, ok := item.Item["Run"]
-		if !ok || runAttr == nil || len(runAttr.B) == 0 {
-			return r, fmt.Errorf("found an existing lock at that id but it did not contain expected 'Run' key. We suggest manually deleting this key from DynamoDB")
-		}
-		var lockingRun Run
-		if err := d.deserialize(runAttr.B, &lockingRun); err != nil {
-			return r, errors.Wrap(err, "deserializing existing lock")
-		}
-		return TryLockResponse{
-			LockAcquired: false,
-			LockingRun: lockingRun,
-			LockID: string(hex.EncodeToString(run.StateKey())),
-		}, nil
-	}
-
-	// else we should be able to lock
-	putItem := &dynamodb.PutItemInput{
-		Item: map[string]*dynamodb.AttributeValue{
-			"LockID": {B: run.StateKey()},
-			"Run":   {B: newRunSerialized},
-		},
-		TableName:           aws.String(d.LockTable),
-		// this will ensure that we don't insert the new item in a race situation
-		// where someone has written this key just after our read
-		ConditionExpression: aws.String("attribute_not_exists(LockID)"),
-	}
-	if _, err := d.DB.PutItem(putItem); err != nil {
-		return r, errors.Wrap(err, "writing lock")
-	}
-	return TryLockResponse{
-		LockAcquired: true,
-		LockingRun: run,
-		LockID: string(hex.EncodeToString(run.StateKey())),
-	}, nil
-}
-
-func (d *DynamoDBLockManager) Unlock(lockID string) error {
-	idAsBytes, err := hex.DecodeString(lockID)
-	if err != nil {
-		return errors.Wrap(err, "id was not in correct format")
-	}
-
-	params := &dynamodb.DeleteItemInput{
-		Key: map[string]*dynamodb.AttributeValue{
-			"LockID": {B: idAsBytes},
-		},
-		TableName: aws.String(d.LockTable),
-	}
-	_, err = d.DB.DeleteItem(params)
-	return errors.Wrap(err, "deleting lock")
-}
-
-func (d *DynamoDBLockManager) ListLocks() (map[string]Run, error) {
-	m := make(map[string]Run)
-	params := &dynamodb.ScanInput{
-		ProjectionExpression: aws.String("LockID,Run"),
-		TableName: aws.String(d.LockTable),
-	}
-
-	// loop to get all locks since if datasize is over 1MB the client will page.
-	// we're setting a counter here just in case something goes horribly wrong and we loop forever
-	var i int
-	var startKey map[string]*dynamodb.AttributeValue
-	for ; i < 1000; i++ {
-		params.SetExclusiveStartKey(startKey)
-		scanOut, err := d.DB.Scan(params)
-		if err != nil {
-			return m, errors.Wrap(err, "reading dynamodb")
-		}
-		for _, item := range scanOut.Items {
-			lockIDItem, ok := item["LockID"]
-			if !ok || lockIDItem == nil {
-				return m, fmt.Errorf("lock did not have expected key 'LockID'")
-			}
-			lockID := string(hex.EncodeToString(lockIDItem.B))
-			runItem, ok := item["Run"]
-			if !ok || runItem == nil {
-				return m, fmt.Errorf("lock did not have expected key 'Run'")
-			}
-
-			var run Run
-			if err := d.deserialize(runItem.B, &run); err != nil {
-				return m, fmt.Errorf("deserializing run at key %q: %s", lockID, err)
-			}
-			m[lockID] = run
-		}
-		startKey = scanOut.LastEvaluatedKey
-
-		// if there are no more pages then we're done
-		if len(startKey) == 0 {
-			return m, nil
-		}
-	}
-	return m, errors.New("maxed out at 1000 scan iterations on the DynamoDB table. Something must be wrong")
-}
-
-func (d *DynamoDBLockManager) deserialize(bs []byte, run *Run) error {
-	return json.Unmarshal(bs, run)
-}
-
-func (d *DynamoDBLockManager) serialize(run Run) ([]byte, error) {
-	return json.Marshal(run)
-}

locking/dynamodb_lock_manager_test.go

@@ -1,28 +0,0 @@
-package locking_test
-
-import (
-	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface"
-	"github.com/hootsuite/atlantis/locking"
-	"testing"
-	. "github.com/hootsuite/atlantis/testing_util"
-	"github.com/aws/aws-sdk-go/service/dynamodb"
-)
-
-type mockDynamoDB struct {
-	dynamodbiface.DynamoDBAPI
-}
-func (m mockDynamoDB) Scan(s *dynamodb.ScanInput) (*dynamodb.ScanOutput, error) {
-	return &dynamodb.ScanOutput{
-		Items: []map[string]*dynamodb.AttributeValue{},
-	}, nil
-}
-
-func TestListLocksEmpty(t *testing.T) {
-	lock := &locking.DynamoDBLockManager{
-		&mockDynamoDB{},
-		"lockTable",
-	}
-	m, err := lock.ListLocks()
-	Ok(t, err)
-	Equals(t, 0, len(m))
-}

locking/locking.go

@@ -3,7 +3,6 @@ package locking
 import (
 	"time"
 	"fmt"
-	"crypto/sha256"
 )
 
 type Run struct {
@@ -15,21 +14,11 @@ type Run struct {
 	Timestamp  time.Time
 }
 
-const (
-	FileLockingBackend = "file"
-	DynamoDBLockingBackend = "dynamodb"
-	BoltDBRunLocksBucket   = "runLocks"
-)
-
 // StateKey returns the unique key to identify the set of infrastructure being modified by this run.
+// Returns `{fullRepoName}/{tfProjectPath}/{environment}`.
 // Used in locking to determine what part of the infrastructure is locked.
-func (r Run) StateKey() []byte {
-	// we combine the repository, path into the repo where terraform is being run and the environment
-	// to make up the state key and then hash it with sha256
-	key := fmt.Sprintf("%s/%s/%s", r.RepoFullName, r.Path, r.Env)
-	h := sha256.New()
-	h.Write([]byte(key))
-	return h.Sum(nil)
+func (r Run) StateKey() string {
+	return fmt.Sprintf("%s/%s/%s", r.RepoFullName, r.Path, r.Env)
 }
 
 type TryLockResponse struct {
@@ -38,8 +27,9 @@ type TryLockResponse struct {
 	LockID       string
 }
 
-type LockManager interface {
+type Backend interface {
 	TryLock(run Run) (TryLockResponse, error)
 	Unlock(lockID string) error
 	ListLocks() (map[string]Run, error)
+	FindLocksForPull(repoFullName string, pullNum int) ([]string, error)
 }

middleware/middleware.go

@@ -19,6 +19,6 @@ func (l *FailedRequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request,
 	next(rw, r)
 	res := rw.(negroni.ResponseWriter)
 	if res.Status() >= 400 {
-		l.logger.Info("%s %s - Response code %d", r.Method, r.URL.Path, res.Status())
+		l.logger.Info("%s %s - Response code %d", r.Method, r.URL.RequestURI(), res.Status())
 	}
 }

server/apply_executor.go

@@ -175,7 +175,7 @@ func (a *ApplyExecutor) apply(ctx *ExecutionContext, pullCtx *PullRequestContext
 			Timestamp: time.Now(),
 		}
 
-		lockAttempt, err := a.lockManager.TryLock(run)
+		lockAttempt, err := a.lockingBackend.TryLock(run)
 		if err != nil {
 			return PathResult{
 				Status: "error",

server/base_executor.go

@@ -14,7 +14,7 @@ type BaseExecutor struct {
 	ghComments            *GithubCommentRenderer
 	terraform             *TerraformClient
 	githubCommentRenderer *GithubCommentRenderer
-	lockManager           locking.LockManager
+	lockingBackend        locking.Backend
 }
 
 type PullRequestContext struct {

server/plan_executor.go

@@ -16,7 +16,8 @@ import (
 // PlanExecutor handles everything related to running the Terraform plan including integration with S3, Terraform, and Github
 type PlanExecutor struct {
 	BaseExecutor
-	atlantisURL string
+	// DeleteLockURL is a function that given a lock id will return a url for deleting the lock
+	DeleteLockURL func(id string) (url string)
 }
 
 /** Result Types **/
@@ -226,7 +227,7 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 			}
 		}
 	}
-	lockAttempt, err := p.lockManager.TryLock(run)
+	lockAttempt, err := p.lockingBackend.TryLock(run)
 	if err != nil {
 		return PathResult{
 			Status:" failure",
@@ -300,7 +301,7 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 		}
 		log.Err("error running terraform plan: %v", output)
 		log.Info("unlocking state since plan failed")
-		if err := p.lockManager.Unlock(lockAttempt.LockID); err != nil {
+		if err := p.lockingBackend.Unlock(lockAttempt.LockID); err != nil {
 			log.Err("error unlocking state: %v", err)
 		}
 		return PathResult{
@@ -313,7 +314,7 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 	if err := UploadPlanFile(s3Client, s3Key, tfPlanOutputPath); err != nil {
 		err = fmt.Errorf("failed to upload to S3: %v", err)
 		log.Err(err.Error())
-		if err := p.lockManager.Unlock(lockAttempt.LockID); err != nil {
+		if err := p.lockingBackend.Unlock(lockAttempt.LockID); err != nil {
 			log.Err("error unlocking state: %v", err)
 		}
 		return PathResult{
@@ -332,7 +333,7 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 		Status: "success",
 		Result: PlanSuccess{
 			TerraformOutput: output,
-			LockURL: fmt.Sprintf("%s%s/%s?method=DELETE", p.atlantisURL, lockPath, lockAttempt.LockID),
+			LockURL: p.DeleteLockURL(lockAttempt.LockID),
 		},
 	}
 }

server/server.go

@@ -20,14 +20,20 @@ import (
 	"github.com/elazarl/go-bindata-assetfs"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
+	"io/ioutil"
+	"github.com/hootsuite/atlantis/locking/dynamodb"
+	"github.com/hootsuite/atlantis/locking/boltdb"
 )
 
 const (
-	lockPath = "/locks"
+	deleteLockRoute        = "delete-lock"
+	LockingFileBackend     = "file"
+	LockingDynamoDBBackend = "dynamodb"
 )
 
 // WebhookServer listens for Github webhooks and runs the necessary Atlantis command
 type Server struct {
+	router           *mux.Router
 	port             int
 	scratchDir       string
 	awsRegion        string
@@ -40,7 +46,8 @@ type Server struct {
 	logger           *logging.SimpleLogger
 	githubComments   *GithubCommentRenderer
 	requestParser    *RequestParser
-	lockManager      locking.LockManager
+	lockingBackend   locking.Backend
+	atlantisURL      string
 }
 
 // the mapstructure tags correspond to flags in cmd/server.go
@@ -49,17 +56,17 @@ type ServerConfig struct {
 	GitHubUser      string `mapstructure:"gh-user"`
 	GitHubPassword  string `mapstructure:"gh-password"`
 	SSHKey          string `mapstructure:"ssh-key"`
-	AssumeRole      string `mapstructure:"aws-assume-role-arn"`
-	Port            int    `mapstructure:"port"`
-	ScratchDir      string `mapstructure:"scratch-dir"`
-	AWSRegion       string `mapstructure:"aws-region"`
-	S3Bucket        string `mapstructure:"s3-bucket"`
-	LogLevel        string `mapstructure:"log-level"`
-	AtlantisURL     string `mapstructure:"atlantis-url"`
-	RequireApproval bool   `mapstructure:"require-approval"`
-	DataDir         string `mapstructure:"data-dir"`
-	LockingBackend  string `mapstructure:"locking-backend"`
-	LockingTable    string `mapstructure:"locking-table"`
+	AssumeRole           string `mapstructure:"aws-assume-role-arn"`
+	Port                 int    `mapstructure:"port"`
+	ScratchDir           string `mapstructure:"scratch-dir"`
+	AWSRegion            string `mapstructure:"aws-region"`
+	S3Bucket             string `mapstructure:"s3-bucket"`
+	LogLevel             string `mapstructure:"log-level"`
+	AtlantisURL          string `mapstructure:"atlantis-url"`
+	RequireApproval      bool   `mapstructure:"require-approval"`
+	DataDir              string `mapstructure:"data-dir"`
+	LockingBackend       string `mapstructure:"locking-backend"`
+	LockingDynamoDBTable string `mapstructure:"locking-dynamodb-table"`
 }
 
 type ExecutionContext struct {
@@ -101,16 +108,16 @@ func NewServer(config ServerConfig) (*Server, error) {
 		AWSRegion:  config.AWSRegion,
 		AWSRoleArn: config.AssumeRole,
 	}
-	var lockManager locking.LockManager
-	if config.LockingBackend == locking.DynamoDBLockingBackend {
+	var lockingBackend locking.Backend
+	if config.LockingBackend == LockingDynamoDBBackend {
 		session, err := awsConfig.CreateAWSSession()
 		if err != nil {
 			return nil, errors.Wrap(err, "creating aws session for DynamoDB")
 		}
-		lockManager = locking.NewDynamoDBLockManager(config.LockingTable, session)
+		lockingBackend = dynamodb.New(config.LockingDynamoDBTable, session)
 	} else {
 		var err error
-		lockManager, err = locking.NewBoltDBLockManager(config.DataDir, locking.BoltDBRunLocksBucket)
+		lockingBackend, err = boltdb.New(config.DataDir)
 		if err != nil {
 			return nil, err
 		}
@@ -124,13 +131,15 @@ func NewServer(config ServerConfig) (*Server, error) {
 		ghComments:            githubComments,
 		terraform:             terraformClient,
 		githubCommentRenderer: githubComments,
-		lockManager:           lockManager,
+		lockingBackend:        lockingBackend,
 	}
 	applyExecutor := &ApplyExecutor{BaseExecutor: baseExecutor, requireApproval: config.RequireApproval, atlantisGithubUser: config.GitHubUser}
-	planExecutor := &PlanExecutor{BaseExecutor: baseExecutor, atlantisURL: config.AtlantisURL}
+	planExecutor := &PlanExecutor{BaseExecutor: baseExecutor}
 	helpExecutor := &HelpExecutor{BaseExecutor: baseExecutor}
 	logger := logging.NewSimpleLogger("server", log.New(os.Stderr, "", log.LstdFlags), false, logging.ToLogLevel(config.LogLevel))
+	router := mux.NewRouter()
 	return &Server{
+		router:           router,
 		port:             config.Port,
 		scratchDir:       config.ScratchDir,
 		awsRegion:        config.AWSRegion,
@@ -143,36 +152,44 @@ func NewServer(config ServerConfig) (*Server, error) {
 		logger:           logger,
 		githubComments:   githubComments,
 		requestParser:    &RequestParser{},
-		lockManager:      lockManager,
+		lockingBackend:   lockingBackend,
+		atlantisURL:      config.AtlantisURL,
 	}, nil
 }
 
 func (s *Server) Start() error {
-	router := mux.NewRouter()
-	router.HandleFunc("/", s.index).Methods("GET").MatcherFunc(func(r *http.Request, rm *mux.RouteMatch) bool {
+	s.router.HandleFunc("/", s.index).Methods("GET").MatcherFunc(func(r *http.Request, rm *mux.RouteMatch) bool {
 		return r.URL.Path == "/" || r.URL.Path == "/index.html"
 	})
-	router.PathPrefix("/static/").Handler(http.FileServer(&assetfs.AssetFS{Asset: Asset, AssetDir: AssetDir, AssetInfo: AssetInfo}))
-	router.HandleFunc("/hooks", s.postHooks).Methods("POST")
-	router.HandleFunc("/locks/{id}", s.deleteLock).Methods("DELETE")
+	s.router.PathPrefix("/static/").Handler(http.FileServer(&assetfs.AssetFS{Asset: Asset, AssetDir: AssetDir, AssetInfo: AssetInfo}))
+	s.router.HandleFunc("/hooks", s.postHooks).Methods("POST")
+	s.router.HandleFunc("/locks", s.deleteLock).Methods("DELETE").Queries("id", "{id:.*}")
 	// todo: remove this route when there is a detail view
 	// right now we need this route because from the pull request comment in GitHub only a GET request can be made
 	// in the future, the pull discard link will link to the detail view which will have a Delete button which will
 	// make an real DELETE call but we don't have a detail view right now
-	router.HandleFunc("/locks/{id}", s.deleteLock).Queries("method", "DELETE").Methods("GET")
+	deleteLockRoute := s.router.HandleFunc("/locks", s.deleteLock).Queries("id", "{id}", "method", "DELETE").Methods("GET").Name(deleteLockRoute)
+
+	// function that planExecutor can use to construct delete lock urls
+	// injecting this here because this is the earliest routes are created
+	s.planExecutor.DeleteLockURL = func (lockID string) string {
+		// ignoring error since guaranteed to succeed if "id" is specified
+		u, _ := deleteLockRoute.URL("id", url.QueryEscape(lockID))
+		return s.atlantisURL + u.RequestURI()
+	}
 	n := negroni.New(&negroni.Recovery{
 		Logger:     log.New(os.Stdout, "", log.LstdFlags),
 		PrintStack: false,
 		StackAll:   false,
 		StackSize:  1024 * 8,
 	}, middleware.NewNon200Logger(s.logger))
-	n.UseHandler(router)
+	n.UseHandler(s.router)
 	s.logger.Info("Atlantis started - listening on port %v", s.port)
 	return cli.NewExitError(http.ListenAndServe(fmt.Sprintf(":%d", s.port), n), 1)
 }
 
 func (s *Server) index(w http.ResponseWriter, r *http.Request) {
-	locks, err := s.lockManager.ListLocks()
+	locks, err := s.lockingBackend.ListLocks()
 	if err != nil {
 		w.WriteHeader(http.StatusServiceUnavailable)
 		fmt.Fprintf(w, "Could not retrieve locks: %s", err)
@@ -181,13 +198,14 @@ func (s *Server) index(w http.ResponseWriter, r *http.Request) {
 
 	type runLock struct {
 		locking.Run
-		ID string
+		UnlockURL string
 	}
 	var results []runLock
 	for id, v := range locks {
+		u, _ := s.router.Get(deleteLockRoute).URL("id", url.QueryEscape(id))
 		results = append(results, runLock{
 			v,
-			id,
+			u.String(),
 		})
 	}
 	indexTemplate.Execute(w, results)
@@ -199,7 +217,12 @@ func (s *Server) deleteLock(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprint(w, "no lock id in request")
 	}
-	if err := s.lockManager.Unlock(id); err != nil {
+	idUnencoded, err := url.PathUnescape(id)
+	if err != nil {
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprint(w, "invalid lock id")
+	}
+	if err := s.lockingBackend.Unlock(idUnencoded); err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprintf(w, "Failed to unlock: %s", err)
 		return
@@ -207,24 +230,64 @@ func (s *Server) deleteLock(w http.ResponseWriter, r *http.Request) {
 	fmt.Fprint(w, "Unlocked successfully")
 }
 
+// postHooks handles comment and pull request events from GitHub
 func (s *Server) postHooks(w http.ResponseWriter, r *http.Request) {
 	githubReqID := "X-Github-Delivery=" + r.Header.Get("X-Github-Delivery")
 
-	// try to parse webhook data as a comment event
-	decoder := json.NewDecoder(r.Body)
-	var comment github.IssueCommentEvent
-	if err := decoder.Decode(&comment); err != nil {
-		s.logger.Debug("Ignoring non-comment-event request %s", githubReqID)
-		fmt.Fprintln(w, "Ignoring")
+	defer r.Body.Close()
+	bytes, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprintf(w,"could not read body: %s\n", err)
 		return
 	}
 
-	if comment.Action == nil || *comment.Action != "created" {
-		s.logger.Debug("Ignoring request because the action was not 'created' %s", githubReqID)
+	// Try to unmarshal the request into the supported event types
+	var commentEvent github.IssueCommentEvent
+	var pullEvent github.PullRequestEvent
+	if json.Unmarshal(bytes, &commentEvent) == nil && s.isCommentCreatedEvent(commentEvent) {
+		s.logger.Debug("Handling comment event %s", githubReqID)
+		s.handleCommentCreatedEvent(w, commentEvent, githubReqID)
+	} else if json.Unmarshal(bytes, &pullEvent) == nil && s.isPullClosedEvent(pullEvent) {
+		s.logger.Debug("Handling pull request event %s", githubReqID)
+		s.handlePullClosedEvent(w, pullEvent, githubReqID)
+	} else {
+		s.logger.Debug("Ignoring unsupported event %s", githubReqID)
 		fmt.Fprintln(w, "Ignoring")
+	}
+}
+
+// handlePullClosedEvent will delete any locks associated with the pull request
+func (s *Server) handlePullClosedEvent(w http.ResponseWriter, pullEvent github.PullRequestEvent, githubReqID string) {
+	repo := *pullEvent.Repo.FullName
+	pullNum := *pullEvent.PullRequest.Number
+	locks, err := s.lockingBackend.FindLocksForPull(repo, pullNum)
+	if err != nil {
+		s.logger.Err("finding locks for repo %s pull %d: %s", repo, pullNum, err)
+		w.WriteHeader(http.StatusServiceUnavailable)
+		fmt.Fprintf(w, "Error finding locks: %s\n", err)
 		return
 	}
 
+	s.logger.Debug("Unlocking locks %v %s", locks, githubReqID)
+	var errors []error
+	for _, l := range locks {
+		if err := s.lockingBackend.Unlock(l); err != nil {
+			errors = append(errors, err)
+		}
+	}
+
+	if len(errors) != 0 {
+		s.logger.Err("unlocking locks for repo %s pull %d: %v", repo, pullNum, errors)
+		w.WriteHeader(http.StatusServiceUnavailable)
+		fmt.Fprintf(w, "Error unlocking locks: %v\n", errors)
+		return
+	}
+
+	fmt.Fprintln(w,"Locks unlocked")
+}
+
+func (s *Server) handleCommentCreatedEvent(w http.ResponseWriter, comment github.IssueCommentEvent, githubReqID string) {
 	// determine if the comment matches a plan or apply command
 	ctx := &ExecutionContext{}
 	command, err := s.requestParser.determineCommand(&comment)
@@ -274,6 +337,14 @@ func (s *Server) executeCommand(ctx *ExecutionContext) {
 	}
 }
 
+func (s *Server) isCommentCreatedEvent(event github.IssueCommentEvent) bool {
+	return event.Action != nil && *event.Action == "created" && event.Comment != nil
+}
+
+func (s *Server) isPullClosedEvent(event github.PullRequestEvent) bool {
+	return event.Action != nil && *event.Action == "closed" && event.PullRequest != nil
+}
+
 // recover logs and creates a comment on the pull request for panics
 func (s *Server) recover(ctx *ExecutionContext) {
 	if err := recover(); err != nil {

server/web_templates.go

@@ -36,7 +36,7 @@ var indexTemplate = template.Must(template.New("index.html.tmpl").Parse(`
     {{ range . }}
       <div class="twelve columns button content lock-row">
         <div class="list-title">{{.RepoFullName}} - <span class="heading-font-size">#{{.PullNum}}</span></div>
-        <div class="list-unlock"><button class="unlock"><a class="unlock-link" href="/locks/{{.ID}}?method=DELETE">Unlock</a></button></div>
+        <div class="list-unlock"><button class="unlock"><a class="unlock-link" href="{{.UnlockURL}}">Unlock</a></button></div>
         <div class="list-status"><code>Locked</code></div>
         <div class="list-timestamp"><span class="heading-font-size">{{.Timestamp}}</span></div>
       </div>

testing_util/testing_util.go

@@ -36,3 +36,15 @@ func Equals(tb testing.TB, exp, act interface{}) {
 		tb.FailNow()
 	}
 }
+
+// Contains fails the test if the slice doesn't contain the expected element
+func ContainsStr(tb testing.TB, exp string, act []string) {
+	for _, v := range act {
+		if v == exp {
+			return
+		}
+	}
+	_, file, line, _ := runtime.Caller(1)
+	fmt.Printf("\033[31m%s:%d:\n\n\texp: %#v\n\n\tin: %#v\033[39m\n\n", filepath.Base(file), line, exp, act)
+	tb.FailNow()
+}