Refactoring

* set version in one place
* rename ExecutionContext to CommandContext
* delete unneeded PullRequestContext since it just duplicated what is in CommandContext
* split fields inside CommandContext into Repo, PullRequest, and User models
* clean up unused fields
* remove base_executor stuff which was trying to do inheritance in go and didn't get us anywhere
* clean up unused code and assets
* created a locking.Client that creates the Keys that are used on the front-end. This allows the backends to store the locks any way they like as long as they support the interface

cmd/version.go

@@ -3,15 +3,14 @@ package cmd
 import (
 	"fmt"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 )
 
-const version = "0.1.0"
-
 var versionCmd = &cobra.Command{
 	Use:   "version",
 	Short: "Print the current atlantis version",
 	Run: func(cmd *cobra.Command, args []string) {
-		fmt.Printf("atlantis %s\n", version)
+		fmt.Printf("atlantis %s\n", viper.Get("version"))
 	},
 }
 

locking/boltdb/boltdb.go

@@ -1,15 +1,15 @@
 package boltdb
 
 import (
-	"github.com/boltdb/bolt"
-	"fmt"
-	"encoding/json"
-	"github.com/pkg/errors"
 	"bytes"
+	"encoding/json"
+	"fmt"
+	"github.com/boltdb/bolt"
+	"github.com/hootsuite/atlantis/models"
+	"github.com/pkg/errors"
 	"os"
-	"time"
 	"path"
-	"github.com/hootsuite/atlantis/locking"
+	"time"
 )
 
 type Backend struct {
@@ -48,104 +48,111 @@ 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()
+func (b *Backend) TryLock(project models.Project, env string, pullNum int) (bool, int, error) {
+	// return variables
+	var lockAcquired bool
+	var lockingPullNum int
+	key := b.key(project, env)
+	newLock := models.ProjectLock{
+		PullNum: pullNum,
+		Project: project,
+		Time:    time.Now(),
+		Env:     env,
+	}
+	newLockSerialized, _ := json.Marshal(newLock)
 	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,
-			}
+		currLockSerialized := bucket.Get([]byte(key))
+		if currLockSerialized == nil {
+			bucket.Put([]byte(key), newLockSerialized) // not a readonly bucketName so okay to ignore error
+			lockAcquired = true
+			lockingPullNum = pullNum
 			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,
+		var currLock models.ProjectLock
+		if err := json.Unmarshal(currLockSerialized, &currLock); err != nil {
+			return errors.Wrap(err, "failed to deserialize current lock")
 		}
+		lockAcquired = false
+		lockingPullNum = currLock.PullNum
 		return nil
 	})
 
 	if transactionErr != nil {
-		return response, errors.Wrap(transactionErr, "DB transaction failed")
+		return false, lockingPullNum, errors.Wrap(transactionErr, "DB transaction failed")
 	}
 
-	return response, nil
+	return lockAcquired, lockingPullNum, nil
 }
 
-func (b Backend) Unlock(lockID string) error {
+func (b Backend) Unlock(p models.Project, env string) error {
+	key := b.key(p, env)
 	err := b.db.Update(func(tx *bolt.Tx) error {
 		locks := tx.Bucket(b.bucket)
-		return locks.Delete([]byte(lockID))
+		return locks.Delete([]byte(key))
 	})
 	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)
-
+func (b Backend) List() ([]models.ProjectLock, error) {
+	var locks []models.ProjectLock
+	var locksBytes [][]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
+			locksBytes = append(locksBytes, v)
 		}
 		return nil
 	})
 	if err != nil {
-		return m, errors.Wrap(err, "DB transaction failed")
+		return locks, 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)))
+	for k, v := range locksBytes {
+		var lock models.ProjectLock
+		if err := json.Unmarshal(v, &lock); err != nil {
+			return locks, errors.Wrap(err, fmt.Sprintf("failed to deserialize lock at key %q", string(k)))
 		}
-		m[k] = run
+		locks = append(locks, lock)
 	}
 
-	return m, nil
+	return locks, nil
 }
 
-func (b Backend) FindLocksForPull(repoFullName string, pullNum int) ([]string, error) {
-	var ids []string
+func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
+	// get the locks that match that pull request
+	var locks []models.ProjectLock
 	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))
+		// we can use the repoFullName as a prefix search since that's the first part of the key
+		for k, v := c.Seek([]byte(repoFullName)); k != nil && bytes.HasPrefix(k, []byte(repoFullName)); k, v = c.Next() {
+			var lock models.ProjectLock
+			if err := json.Unmarshal(v, &lock); err != nil {
+				return errors.Wrapf(err, "failed to deserialize lock at key %q", string(k))
 			}
-			if run.PullNum == pullNum {
-				ids = append(ids, string(k))
+			if lock.PullNum == pullNum {
+				locks = append(locks, lock)
 			}
 		}
-
 		return nil
 	})
-	return ids, err
+
+	// delete the locks
+	for _, lock := range locks {
+		if err = b.Unlock(lock.Project, lock.Env); err != nil {
+			return errors.Wrapf(err, "unlocking repo %s, path %s, env %s", lock.Project.RepoFullName, lock.Project.Path, lock.Env)
+		}
+	}
+	return nil
 }
 
-func (b Backend) deserialize(bs []byte, run *locking.Run) error {
-	return json.Unmarshal(bs, run)
+func (b Backend) key(p models.Project, env string) string {
+	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, env)
 }

locking/boltdb/boltdb_test.go

@@ -1,34 +1,28 @@
 package boltdb_test
 
 import (
-	"github.com/hootsuite/atlantis/locking"
 	. "github.com/hootsuite/atlantis/testing_util"
 
 	"github.com/boltdb/bolt"
+	"github.com/pkg/errors"
+	"io/ioutil"
 	"os"
 	"testing"
-	"io/ioutil"
-	"github.com/pkg/errors"
 
-	"time"
 	"github.com/hootsuite/atlantis/locking/boltdb"
+	"github.com/hootsuite/atlantis/models"
 )
 
 var lockBucket = "bucket"
-var run = locking.Run{
-	RepoFullName: "owner/repo",
-	Path: "parent/child",
-	Env: "default",
-	PullNum: 1,
-	User: "user",
-	Timestamp: time.Now(),
-}
+var project = models.NewProject("owner/repo", "parent/child")
+var env = "default"
+var pullNum = 1
 
 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()
+	ls, err := b.List()
 	Ok(t, err)
 	Equals(t, 0, len(ls))
 }
@@ -37,12 +31,11 @@ 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)
+	_, _, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
-	ls, err := b.ListLocks()
+	ls, err := b.List()
 	Ok(t, err)
 	Equals(t, 1, len(ls))
-	Equals(t, run, ls[run.StateKey()])
 }
 
 func TestListMultipleLocks(t *testing.T) {
@@ -51,42 +44,40 @@ func TestListMultipleLocks(t *testing.T) {
 	defer cleanupDB(db)
 
 	// add multiple locks
-	_, err := b.TryLock(run)
-	Ok(t, err)
+	repos := []string{
+		"owner/repo1",
+		"owner/repo2",
+		"owner/repo3",
+		"owner/repo4",
+	}
 
-	run2 := run
-	run2.RepoFullName = "different"
-	_, err = b.TryLock(run2)
+	for _, r := range repos {
+		_, _, err := b.TryLock(models.NewProject(r, "path"), env, pullNum)
 		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()
+	}
+	ls, err := b.List()
 	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()])
+	for _, r := range repos {
+		found := false
+		for _, l := range ls {
+			if l.Project.RepoFullName == r {
+				found = true
+			}
+		}
+		Assert(t, found == true, "expected %s in %v", r, ls)
+	}
 }
 
 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)
+	_, _, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
-	b.Unlock(run.StateKey())
+	b.Unlock(project, env)
 
-	ls, err := b.ListLocks()
+	ls, err := b.List()
 	Ok(t, err)
 	Equals(t, 0, len(ls))
 }
@@ -95,62 +86,50 @@ 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)
+	acquired, lockingPullNum, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
-	Equals(t, true, r.LockAcquired)
-	Equals(t, run, r.LockingRun)
-	Equals(t, run.StateKey(), r.LockID)
+	Equals(t, true, acquired)
+	Equals(t, pullNum, lockingPullNum)
 }
 
 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)
+	_, _, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
 
-	t.Log("...succeed if the new run has a different path")
+	t.Log("...succeed if the new project has a different path")
 	{
-		new := run
-		new.Path = "different/path"
-		r, err := b.TryLock(new)
+		acquired, lockingPull, err := b.TryLock(models.NewProject(project.RepoFullName, "different/path"), env, pullNum)
 		Ok(t, err)
-		Equals(t, true, r.LockAcquired)
-		Equals(t, new, r.LockingRun)
-		Equals(t, new.StateKey(), r.LockID)
+		Equals(t, true, acquired)
+		Equals(t, pullNum, lockingPull)
 	}
 
-	t.Log("...succeed if the new run has a different environment")
+	t.Log("...succeed if the new project has a different environment")
 	{
-		new := run
-		new.Env = "different-env"
-		r, err := b.TryLock(new)
+		acquired, lockingPull, err := b.TryLock(project, "different-env", pullNum)
 		Ok(t, err)
-		Equals(t, true, r.LockAcquired)
-		Equals(t, new, r.LockingRun)
-		Equals(t, new.StateKey(), r.LockID)
+		Equals(t, true, acquired)
+		Equals(t, pullNum, lockingPull)
 	}
 
-	t.Log("...succeed if the new run has a different repoName")
+	t.Log("...succeed if the new project has a different repoName")
 	{
-		new := run
-		new.RepoFullName = "new/repo"
-		r, err := b.TryLock(new)
+		acquired, lockingPull, err := b.TryLock(models.NewProject("different/repo", project.Path), env, pullNum)
 		Ok(t, err)
-		Equals(t, true, r.LockAcquired)
-		Equals(t, new, r.LockingRun)
-		Equals(t, new.StateKey(), r.LockID)
+		Equals(t, true, acquired)
+		Equals(t, pullNum, lockingPull)
 	}
 
-	t.Log("...not succeed if the new run only has a different pullNum")
+	t.Log("...not succeed if the new project only has a different pullNum")
 	{
-		new := run
-		new.PullNum = run.PullNum + 1
-		r, err := b.TryLock(new)
+		newPull := pullNum + 1
+		acquired, lockingPull, err := b.TryLock(project, env, newPull)
 		Ok(t, err)
-		Equals(t, false, r.LockAcquired)
-		Equals(t, run, r.LockingRun)
-		Equals(t, run.StateKey(), r.LockID)
+		Equals(t, false, acquired)
+		Equals(t, lockingPull, pullNum)
 	}
 }
 
@@ -159,7 +138,7 @@ func TestUnlockingNoLocks(t *testing.T) {
 	db, b := newTestDB()
 	defer cleanupDB(db)
 
-	Ok(t, b.Unlock("any-lock-id"))
+	Ok(t, b.Unlock(project, env))
 }
 
 func TestUnlocking(t *testing.T) {
@@ -167,20 +146,20 @@ func TestUnlocking(t *testing.T) {
 	db, b := newTestDB()
 	defer cleanupDB(db)
 
-	b.TryLock(run)
-	Ok(t, b.Unlock(run.StateKey()))
+	b.TryLock(project, env, pullNum)
+	Ok(t, b.Unlock(project, env))
 
 	// should be no locks listed
-	ls, err := b.ListLocks()
+	ls, err := b.List()
 	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)
+	newPull := pullNum + 1
+	acquired, lockingPull, err := b.TryLock(project, env, newPull)
 	Ok(t, err)
-	Equals(t, true, r.LockAcquired)
+	Equals(t, true, acquired)
+	Equals(t, newPull, lockingPull)
 }
 
 func TestUnlockingMultiple(t *testing.T) {
@@ -188,107 +167,114 @@ func TestUnlockingMultiple(t *testing.T) {
 	db, b := newTestDB()
 	defer cleanupDB(db)
 
-	b.TryLock(run)
+	b.TryLock(project, env, pullNum)
 
-	new := run
+	new := project
 	new.RepoFullName = "new/repo"
-	b.TryLock(new)
+	b.TryLock(project, env, pullNum)
 
-	new2 := run
+	new2 := project
 	new2.Path = "new/path"
-	b.TryLock(new2)
+	b.TryLock(project, env, pullNum)
 
-	new3 := run
-	new3.Env = "new/env"
-	b.TryLock(new3)
+	new3Env := "new-env"
+	b.TryLock(project, new3Env, pullNum)
 
 	// 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()))
+	Ok(t, b.Unlock(project, new3Env))
+	Ok(t, b.Unlock(new2, env))
+	Ok(t, b.Unlock(new, env))
+	Ok(t, b.Unlock(project, env))
 
 	// should be none left
-	ls, err := b.ListLocks()
+	ls, err := b.List()
 	Ok(t, err)
 	Equals(t, 0, len(ls))
 }
 
-func TestFindLocksNone(t *testing.T) {
-	t.Log("find should return no locks when there are none")
+func TestUnlockByPullNone(t *testing.T) {
+	t.Log("UnlockByPull should be successful when there are no locks")
 	db, b := newTestDB()
 	defer cleanupDB(db)
 
-	ls, err := b.FindLocksForPull("any/repo", 1)
+	err := b.UnlockByPull("any/repo", 1)
 	Ok(t, err)
-	Equals(t, 0, len(ls))
 }
 
-func TestFindLocksOne(t *testing.T) {
-	t.Log("with one lock find should...")
+func TestUnlockByPullOne(t *testing.T) {
+	t.Log("with one lock, UnlockByPull should...")
 	db, b := newTestDB()
 	defer cleanupDB(db)
-	_, err := b.TryLock(run)
+	_, _, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
 
-	t.Log("...return no locks from the same repo but different pull num")
+	t.Log("...delete nothing when its the same repo but a different pull")
 	{
-		ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum + 1)
+		err := b.UnlockByPull(project.RepoFullName, 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)
+		ls, err := b.List()
 		Ok(t, err)
 		Equals(t, 1, len(ls))
-		Equals(t, run.StateKey(), ls[0])
+	}
+	t.Log("...delete nothing when its the same pull but a different repo")
+	{
+		err := b.UnlockByPull("different/repo", pullNum)
+		Ok(t, err)
+		ls, err := b.List()
+		Ok(t, err)
+		Equals(t, 1, len(ls))
+	}
+	t.Log("...delete the lock when its the same repo and pull")
+	{
+		err := b.UnlockByPull(project.RepoFullName, pullNum)
+		Ok(t, err)
+		ls, err := b.List()
+		Ok(t, err)
+		Equals(t, 0, len(ls))
 	}
 }
 
-func TestFindLocksAfterUnlock(t *testing.T) {
-	t.Log("after locking and unlocking find should return no locks")
+func TestUnlockByPullAfterUnlock(t *testing.T) {
+	t.Log("after locking and unlocking, UnlockByPull should be successful")
 	db, b := newTestDB()
 	defer cleanupDB(db)
-	_, err := b.TryLock(run)
+	_, _, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
-	Ok(t, b.Unlock(run.StateKey()))
+	Ok(t, b.Unlock(project, env))
 
-	ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum)
+	err = b.UnlockByPull(project.RepoFullName, pullNum)
+	Ok(t, err)
+	ls, err := b.List()
 	Ok(t, err)
 	Equals(t, 0, len(ls))
 }
 
-func TestFindMultipleMatching(t *testing.T) {
-	t.Log("find should return all matching lock ids")
+func TestUnlockByPullMatching(t *testing.T) {
+	t.Log("UnlockByPull should delete all locks in that repo and pull num")
 	db, b := newTestDB()
 	defer cleanupDB(db)
-	_, err := b.TryLock(run)
+	_, _, err := b.TryLock(project, env, pullNum)
 	Ok(t, err)
 
 	// add additional locks with the same repo and pull num but different paths/envs
-	new := run
+	new := project
 	new.Path = "dif/path"
-	_, err = b.TryLock(new)
+	_, _, err = b.TryLock(new, env, pullNum)
 	Ok(t, err)
-	new2 := run
-	new2.Env = "new-env"
-	_, err = b.TryLock(new2)
+	_, _, err = b.TryLock(project, "new-env", pullNum)
 	Ok(t, err)
 
-	// should get all of them back
-	ls, err := b.FindLocksForPull(run.RepoFullName, run.PullNum)
+	// there should now be 3
+	ls, err := b.List()
 	Ok(t, err)
 	Equals(t, 3, len(ls))
-	ContainsStr(t, run.StateKey(), ls)
-	ContainsStr(t, new.StateKey(), ls)
-	ContainsStr(t, new2.StateKey(), ls)
+
+	// should all be unlocked
+	err = b.UnlockByPull(project.RepoFullName, pullNum)
+	Ok(t, err)
+	ls, err = b.List()
+	Ok(t, err)
+	Equals(t, 0, len(ls))
 }
 
 // newTestDB returns a TestDB using a temporary path.
@@ -322,4 +308,3 @@ func cleanupDB(db *bolt.DB) {
 	os.Remove(db.Path())
 	db.Close()
 }
-

locking/dynamodb/dynamodb.go

@@ -1,18 +1,16 @@
 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/aws"
+	"github.com/aws/aws-sdk-go/aws/client"
+	"github.com/aws/aws-sdk-go/service/dynamodb"
 	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbattribute"
+	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface"
+	"github.com/hootsuite/atlantis/models"
+	"github.com/pkg/errors"
 	"strconv"
-	"github.com/hootsuite/atlantis/locking"
+	"time"
 )
 
 type Backend struct {
@@ -20,104 +18,114 @@ type Backend struct {
 	LockTable string
 }
 
-type dynamoRun struct {
-	LockID       string
+// dynamoLock duplicates the fields of models.ProjectLock and adds LocksKey
+// so everything is a top-level field for serialization and then querying
+// in DynamodB and also so any changes to models.ProjectLock won't affect
+// how we're storing our data or will at least cause a compile error
+type dynamoLock struct {
+	LockKey      string
 	RepoFullName string
 	Path         string
-	Env          string
 	PullNum      int
-	User         string
-	Timestamp    time.Time
+	Env          string
+	Time         time.Time
 }
 
-func New(lockTable string, p client.ConfigProvider) *Backend {
-	return &Backend{
+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)
+func (b Backend) key(project models.Project, env string) string {
+	return fmt.Sprintf("%s/%s/%s", project.RepoFullName, project.Path, env)
+}
+
+func (b Backend) TryLock(project models.Project, env string, pullNum int) (bool, int, error) {
+	key := b.key(project, env)
+	newDynamoLock := dynamoLock{
+		LockKey:      key,
+		RepoFullName: project.RepoFullName,
+		Path:         project.Path,
+		PullNum:      pullNum,
+		Env:          env,
+		Time:         time.Now(),
+	}
+	newLockSerialized, err := dynamodbattribute.MarshalMap(newDynamoLock)
 	if err != nil {
-		return r, errors.Wrap(err, "serializing")
+		return false, 0, 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()),
+			"LockKey": {
+				S: aws.String(key),
 			},
 		},
-		TableName: aws.String(d.LockTable),
+		TableName:      aws.String(b.LockTable),
 		ConsistentRead: aws.Bool(true),
 	}
-	item, err := d.DB.GetItem(getItemParams)
+	item, err := b.DB.GetItem(getItemParams)
 	if err != nil {
-		return r, errors.Wrap(err, "checking if lock exists")
+		return false, 0, errors.Wrap(err, "checking if lock exists")
 	}
 
-
 	// if there is already a lock then we can't acquire a lock. Return the existing lock
+	var currLock dynamoLock
 	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")
+		if err := dynamodbattribute.UnmarshalMap(item.Item, &currLock); err != nil {
+			return false, 0, errors.Wrap(err, "found an existing lock at that key 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
+		return false, currLock.PullNum, nil
 	}
 
 	// else we should be able to lock
 	putItem := &dynamodb.PutItemInput{
-		Item:      newRunSerialized,
-		TableName: aws.String(d.LockTable),
+		Item:      newLockSerialized,
+		TableName: aws.String(b.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)"),
+		ConditionExpression: aws.String("attribute_not_exists(LockKey)"),
 	}
-	if _, err := d.DB.PutItem(putItem); err != nil {
-		return r, errors.Wrap(err, "writing lock")
+	if _, err := b.DB.PutItem(putItem); err != nil {
+		return false, 0, errors.Wrap(err, "writing lock")
 	}
-	return locking.TryLockResponse{
-		LockAcquired: true,
-		LockingRun: run,
-		LockID: run.StateKey(),
-	}, nil
+	return true, pullNum, nil
 }
 
-func (d *Backend) Unlock(lockID string) error {
+func (b Backend) Unlock(project models.Project, env string) error {
+	key := b.key(project, env)
 	params := &dynamodb.DeleteItemInput{
 		Key: map[string]*dynamodb.AttributeValue{
-			"LockID": {S: aws.String(lockID)},
+			"LockKey": {S: aws.String(key)},
 		},
-		TableName: aws.String(d.LockTable),
+		TableName: aws.String(b.LockTable),
 	}
-	_, err := d.DB.DeleteItem(params)
+	_, err := b.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)
+func (b Backend) List() ([]models.ProjectLock, error) {
+	var locks []models.ProjectLock
 	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 {
+	params := &dynamodb.ScanInput{
+		TableName: aws.String(b.LockTable),
+	}
+	err = b.DB.ScanPages(params, func(out *dynamodb.ScanOutput, lastPage bool) bool {
+		var dynamoLocks []dynamoLock
+		if err := dynamodbattribute.UnmarshalListOfMaps(out.Items, &dynamoLocks); err != nil {
 			internalErr = errors.Wrap(err, "deserializing locks")
 			return false
 		}
-		for _, run := range runs {
-			m[run.LockID] = d.fromDynamoItem(run)
+		for _, lock := range dynamoLocks {
+			locks = append(locks, models.ProjectLock{
+				PullNum: lock.PullNum,
+				Project: models.NewProject(lock.RepoFullName, lock.Path),
+				Env:     lock.Env,
+				Time:    lock.Time,
+			})
 		}
 		return lastPage
 	})
@@ -125,10 +133,10 @@ func (d *Backend) ListLocks() (map[string]locking.Run, error) {
 	if err == nil && internalErr != nil {
 		err = internalErr
 	}
-	return m, errors.Wrap(err, "scanning dynamodb")
+	return locks, errors.Wrap(err, "scanning dynamodb")
 }
 
-func (d *Backend) FindLocksForPull(repoFullName string, pullNum int) ([]string, error) {
+func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
 	params := &dynamodb.ScanInput{
 		ExpressionAttributeValues: map[string]*dynamodb.AttributeValue{
 			":pullNum": {
@@ -139,77 +147,31 @@ func (d *Backend) FindLocksForPull(repoFullName string, pullNum int) ([]string,
 			},
 		},
 		FilterExpression: aws.String("RepoFullName = :repoFullName and PullNum = :pullNum"),
-		TableName: aws.String(d.LockTable),
+		TableName:        aws.String(b.LockTable),
 	}
 
-	var ids []string
+	// scan DynamoDB for locks that match the pull request
+	var locks []dynamoLock
 	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 {
+	err = b.DB.ScanPages(params, func(out *dynamodb.ScanOutput, lastPage bool) bool {
+		if err := dynamodbattribute.UnmarshalListOfMaps(out.Items, &locks); 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 {
+	if err == nil {
 		err = internalErr
 	}
-	return ids, errors.Wrap(err,"scanning dynamodb")
+	if err != nil {
+		return 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,
+	// now we can unlock all of them
+	for _, lock := range locks {
+		if err := b.Unlock(models.NewProject(lock.RepoFullName, lock.Path), lock.Env); err != nil {
+			return errors.Wrapf(err, "unlocking repo %s, path %s, env %s", lock.RepoFullName, lock.Path, lock.Env)
 		}
 	}
+	return nil
+}

locking/locking.go

@@ -1,35 +1,70 @@
 package locking
 
 import (
-	"time"
+	"errors"
 	"fmt"
+	"github.com/hootsuite/atlantis/models"
+	"regexp"
 )
 
-type Run struct {
-	RepoFullName string
-	Path       string
-	Env        string
-	PullNum     int
-	User       string
-	Timestamp  time.Time
-}
-
-// 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() string {
-	return fmt.Sprintf("%s/%s/%s", r.RepoFullName, r.Path, r.Env)
+type Backend interface {
+	TryLock(project models.Project, env string, pullNum int) (bool, int, error)
+	Unlock(project models.Project, env string) error
+	List() ([]models.ProjectLock, error)
+	UnlockByPull(repoFullName string, pullNum int) error
 }
 
 type TryLockResponse struct {
 	LockAcquired   bool
-	LockingRun   Run // what is currently holding the lock
-	LockID       string
+	LockingPullNum int
+	LockKey        string
 }
 
-type Backend interface {
-	TryLock(run Run) (TryLockResponse, error)
-	Unlock(lockID string) error
-	ListLocks() (map[string]Run, error)
-	FindLocksForPull(repoFullName string, pullNum int) ([]string, error)
+type Client struct {
+	backend Backend
+}
+
+func NewClient(backend Backend) *Client {
+	return &Client{
+		backend: backend,
+	}
+}
+
+// keyRegex matches and captures {repoFullName}/{path}/{env} where path can have multiple /'s in it
+var keyRegex = regexp.MustCompile(`^(.*?\/.*?)\/(.*)\/(.*)$`)
+
+func (c *Client) TryLock(p models.Project, env string, pullNum int) (TryLockResponse, error) {
+	lockAcquired, lockingPullNum, err := c.backend.TryLock(p, env, pullNum)
+	if err != nil {
+		return TryLockResponse{}, err
+	}
+	return TryLockResponse{lockAcquired, lockingPullNum, c.key(p, env)}, nil
+}
+
+func (c *Client) Unlock(key string) error {
+	matches := keyRegex.FindStringSubmatch(key)
+	if len(matches) != 4 {
+		return errors.New("invalid key format")
+	}
+	return c.backend.Unlock(models.Project{matches[1], matches[2]}, matches[3])
+}
+
+func (c *Client) List() (map[string]models.ProjectLock, error) {
+	m := make(map[string]models.ProjectLock)
+	locks, err := c.backend.List()
+	if err != nil {
+		return m, err
+	}
+	for _, lock := range locks {
+		m[c.key(lock.Project, lock.Env)] = lock
+	}
+	return m, nil
+}
+
+func (c *Client) UnlockByPull(repoFullName string, pullNum int) error {
+	return c.backend.UnlockByPull(repoFullName, pullNum)
+}
+
+func (c *Client) key(p models.Project, env string) string {
+	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, env)
 }

middleware/middleware.go

@@ -1,9 +1,9 @@
 package middleware
 
 import (
-	"net/http"
-	"github.com/urfave/negroni"
 	"github.com/hootsuite/atlantis/logging"
+	"github.com/urfave/negroni"
+	"net/http"
 )
 
 func NewNon200Logger(logger *logging.SimpleLogger) *FailedRequestLogger {

models/models.go

@@ -0,0 +1,54 @@
+package models
+
+import (
+	paths "path"
+	"time"
+)
+
+type Repo struct {
+	FullName string
+	Owner    string
+	Name     string
+	SSHURL   string
+}
+
+type PullRequest struct {
+	Num        int
+	HeadCommit string
+	BaseCommit string
+	Link       string
+	Branch     string
+	Author     string
+}
+
+type User struct {
+	Username string
+}
+
+type ProjectLock struct {
+	Project Project
+	PullNum int
+	Env     string
+	Time    time.Time
+}
+
+// Project represents a Terraform project.
+// Since there may be multiple Terraform projects in a single repo we also include Path
+type Project struct {
+	RepoFullName string
+	// Path to project root in the repo.
+	// If "." then project is at root.
+	// Never ends in "/".
+	Path string
+}
+
+func NewProject(repoFullName string, path string) Project {
+	path = paths.Clean(path)
+	if path == "/" {
+		path = "."
+	}
+	return Project{
+		RepoFullName: repoFullName,
+		Path:         path,
+	}
+}

server/apply_executor.go

@@ -14,13 +14,20 @@ import (
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/aws/aws-sdk-go/service/s3/s3manager"
 	"github.com/hootsuite/atlantis/locking"
-	"time"
+	"github.com/hootsuite/atlantis/models"
+	"strconv"
 )
 
 type ApplyExecutor struct {
-	BaseExecutor
+	github                *GithubClient
+	awsConfig             *AWSConfig
+	scratchDir            string
+	s3Bucket              string
+	sshKey                string
+	terraform             *TerraformClient
+	githubCommentRenderer *GithubCommentRenderer
+	lockingClient         *locking.Client
 	requireApproval       bool
-	atlantisGithubUser string
 }
 
 /** Result Types **/
@@ -54,82 +61,85 @@ func (n NoPlansFailure) Template() *CompiledTemplate {
 	return NoPlansFailureTmpl
 }
 
-func (a *ApplyExecutor) execute(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult {
-	res := a.setupAndApply(ctx, pullCtx)
+func (a *ApplyExecutor) execute(ctx *CommandContext, github *GithubClient) {
+	res := a.setupAndApply(ctx)
 	res.Command = Apply
-	return res
+	comment := a.githubCommentRenderer.render(res, ctx.Log.History.String(), ctx.Command.verbose)
+	github.CreateComment(ctx, comment)
 }
 
-func (a *ApplyExecutor) setupAndApply(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult {
-	a.github.UpdateStatus(pullCtx, PendingStatus, "Applying...")
+func (a *ApplyExecutor) setupAndApply(ctx *CommandContext) ExecutionResult {
+	a.github.UpdateStatus(ctx.Repo, ctx.Pull, PendingStatus, "Applying...")
 
 	if a.requireApproval {
-		ok, err := a.github.PullIsApproved(pullCtx)
+		ok, err := a.github.PullIsApproved(ctx.Repo, ctx.Pull)
 		if err != nil {
 			msg := fmt.Sprintf("failed to determine if pull request was approved: %v", err)
-			ctx.log.Err(msg)
-			a.github.UpdateStatus(pullCtx, ErrorStatus, "Apply Error")
+			ctx.Log.Err(msg)
+			a.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Apply Error")
 			return ExecutionResult{SetupError: GeneralError{errors.New(msg)}}
 		}
 		if !ok {
-			ctx.log.Info("pull request was not approved")
-			a.github.UpdateStatus(pullCtx, FailureStatus, "Apply Failed")
+			ctx.Log.Info("pull request was not approved")
+			a.github.UpdateStatus(ctx.Repo, ctx.Pull, FailureStatus, "Apply Failed")
 			return ExecutionResult{SetupFailure: PullNotApprovedFailure{}}
 		}
 	}
 
-	planPaths, err := a.downloadPlans(ctx.repoFullName, ctx.pullNum, ctx.command.environment, a.scratchDir, a.awsConfig, a.s3Bucket)
+	planPaths, err := a.downloadPlans(ctx.Repo.FullName, ctx.Pull.Num, ctx.Command.environment, a.scratchDir, a.awsConfig, a.s3Bucket)
 	if err != nil {
 		errMsg := fmt.Sprintf("failed to download plans: %v", err)
-		ctx.log.Err(errMsg)
-		a.github.UpdateStatus(pullCtx, ErrorStatus, "Apply Error")
+		ctx.Log.Err(errMsg)
+		a.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Apply Error")
 		return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 	}
 
 	// If there are no plans found for the pull request
 	if len(planPaths) == 0 {
 		failure := "found 0 plans for this pull request"
-		ctx.log.Warn(failure)
-		a.github.UpdateStatus(pullCtx, FailureStatus, "Apply Failure")
+		ctx.Log.Warn(failure)
+		a.github.UpdateStatus(ctx.Repo, ctx.Pull, FailureStatus, "Apply Failure")
 		return ExecutionResult{SetupFailure: NoPlansFailure{}}
 	}
 
 	//runLog = append(runLog, fmt.Sprintf("-> Downloaded plans: %v", planPaths))
 	applyOutputs := []PathResult{}
 	for _, planPath := range planPaths {
-		output := a.apply(ctx, pullCtx, planPath)
+		output := a.apply(ctx, planPath)
 		output.Path = planPath
 		applyOutputs = append(applyOutputs, output)
 	}
-	a.updateGithubStatus(pullCtx, applyOutputs)
+	a.updateGithubStatus(ctx, applyOutputs)
 	return ExecutionResult{PathResults: applyOutputs}
 }
 
-func (a *ApplyExecutor) apply(ctx *ExecutionContext, pullCtx *PullRequestContext, planPath string) PathResult {
-	//runLog = append(runLog, fmt.Sprintf("-> Running apply %s", planPath))
+func (a *ApplyExecutor) apply(ctx *CommandContext, planPath string) PathResult {
 	planName := path.Base(planPath)
-	planSubDir := a.determinePlanSubDir(planName, ctx.pullNum)
-	planDir := filepath.Join(a.scratchDir, ctx.repoFullName, fmt.Sprintf("%v", ctx.pullNum), planSubDir)
+	planSubDir := a.determinePlanSubDir(planName, ctx.Pull.Num)
+	// todo: don't assume repo is cloned here
+	repoDir := filepath.Join(a.scratchDir, ctx.Repo.FullName, strconv.Itoa(ctx.Pull.Num))
+	planDir := filepath.Join(repoDir, planSubDir)
+	project := models.NewProject(ctx.Repo.FullName, planSubDir)
 	execPath := NewExecutionPath(planDir, planSubDir)
 	var config Config
 	var remoteStatePath string
 	// check if config file is found, if not we continue the run
 	if config.Exists(execPath.Absolute) {
-		ctx.log.Info("Config file found in %s", execPath.Absolute)
+		ctx.Log.Info("Config file found in %s", execPath.Absolute)
 		err := config.Read(execPath.Absolute)
 		if err != nil {
 			msg := fmt.Sprintf("Error reading config file: %v", err)
-			ctx.log.Err(msg)
+			ctx.Log.Err(msg)
 			return PathResult{
 				Status: "error",
 				Result: GeneralError{errors.New(msg)},
 			}
 		}
 		// need to use the remote state path and backend to do remote configure
-		err = PreRun(&config, ctx.log, execPath.Absolute, ctx.command)
+		err = PreRun(&config, ctx.Log, execPath.Absolute, ctx.Command)
 		if err != nil {
 			msg := fmt.Sprintf("pre run failed: %v", err)
-			ctx.log.Err(msg)
+			ctx.Log.Err(msg)
 			return PathResult{
 				Status: "error",
 				Result: GeneralError{errors.New(msg)},
@@ -146,10 +156,10 @@ func (a *ApplyExecutor) apply(ctx *ExecutionContext, pullCtx *PullRequestContext
 	// NOTE: THIS CODE IS TO SUPPORT TERRAFORM PROJECTS THAT AREN'T USING ATLANTIS CONFIG FILE.
 	if config.StashPath == "" {
 		// configure remote state
-		statePath, err := a.terraform.ConfigureRemoteState(ctx.log, execPath, ctx.command.environment, a.sshKey)
+		statePath, err := a.terraform.ConfigureRemoteState(ctx.Log, repoDir, project, ctx.Command.environment, a.sshKey)
 		if err != nil {
 			msg := fmt.Sprintf("failed to set up remote state: %v", err)
-			ctx.log.Err(msg)
+			ctx.Log.Err(msg)
 			return PathResult{
 				Status: "error",
 				Result: GeneralError{errors.New(msg)},
@@ -158,72 +168,62 @@ func (a *ApplyExecutor) apply(ctx *ExecutionContext, pullCtx *PullRequestContext
 		remoteStatePath = statePath
 	} else {
 		// use state path from config file
-		remoteStatePath = generateStatePath(config.StashPath, ctx.command.environment)
+		remoteStatePath = generateStatePath(config.StashPath, ctx.Command.environment)
 	}
 
 	if remoteStatePath != "" {
-		tfEnv := ctx.command.environment
+		tfEnv := ctx.Command.environment
 		if tfEnv == "" {
 			tfEnv = "default"
 		}
-		run := locking.Run{
-			RepoFullName: pullCtx.repoFullName,
-			Path: execPath.Relative,
-			Env: tfEnv,
-			PullNum: pullCtx.number,
-			User: pullCtx.terraformApplier,
-			Timestamp: time.Now(),
-		}
 
-		lockAttempt, err := a.lockingBackend.TryLock(run)
+		lockAttempt, err := a.lockingClient.TryLock(project, tfEnv, ctx.Pull.Num)
 		if err != nil {
 			return PathResult{
 				Status: "error",
 				Result: GeneralError{fmt.Errorf("failed to acquire lock: %s", err)},
 			}
 		}
-		if lockAttempt.LockAcquired != true && lockAttempt.LockingRun.PullNum != pullCtx.number {
+		if lockAttempt.LockAcquired != true && lockAttempt.LockingPullNum != ctx.Pull.Num {
 			return PathResult{
 				Status: "error",
-				Result: GeneralError{fmt.Errorf("failed to acquire lock: lock held by pull request #%d", lockAttempt.LockingRun.PullNum)},
+				Result: GeneralError{fmt.Errorf("failed to acquire lock: lock held by pull request #%d", lockAttempt.LockingPullNum)},
 			}
 		}
 	}
 
 	// need to get auth data from assumed role
 	// todo: de-duplicate calls to assumeRole
-	//runLog = append(runLog, "-> Assuming role prior to running apply")
-	a.awsConfig.AWSSessionName = ctx.pullCreator
+	a.awsConfig.AWSSessionName = ctx.User.Username
 	awsSession, err := a.awsConfig.CreateAWSSession()
 	if err != nil {
-		ctx.log.Err(err.Error())
+		ctx.Log.Err(err.Error())
 		return PathResult{
 			Status: "error",
 			Result: GeneralError{err},
 		}
 	}
-	//runLog = append(runLog, "-> Assumed AWS role successfully")
 
 	credVals, err := awsSession.Config.Credentials.Get()
 	if err != nil {
 		msg := fmt.Sprintf("failed to get assumed role credentials: %v", err)
-		ctx.log.Err(msg)
+		ctx.Log.Err(msg)
 		return PathResult{
 			Status: "error",
 			Result: GeneralError{errors.New(msg)},
 		}
 	}
 
-	ctx.log.Info("running apply from %q", execPath.Relative)
+	ctx.Log.Info("running apply from %q", execPath.Relative)
 
-	terraformApplyCmdArgs, output, err := a.terraform.RunTerraformCommand(execPath, []string{"apply", "-no-color", planPath}, []string{
+	terraformApplyCmdArgs, output, err := a.terraform.RunTerraformCommand(execPath.Absolute, []string{"apply", "-no-color", planPath}, []string{
 		fmt.Sprintf("AWS_ACCESS_KEY_ID=%s", credVals.AccessKeyID),
 		fmt.Sprintf("AWS_SECRET_ACCESS_KEY=%s", credVals.SecretAccessKey),
 		fmt.Sprintf("AWS_SESSION_TOKEN=%s", credVals.SessionToken),
 	})
 	//runLog = append(runLog, "```\n Apply output:\n", fmt.Sprintf("```bash\n%s\n", string(out[:])))
 	if err != nil {
-		ctx.log.Err("failed to apply: %v %s", err, output)
+		ctx.Log.Err("failed to apply: %v %s", err, output)
 		return PathResult{
 			Status: "failure",
 			Result: ApplyFailure{Command: strings.Join(terraformApplyCmdArgs, " "), Output: output, ErrorMessage: err.Error()},
@@ -292,3 +292,27 @@ func (a *ApplyExecutor) determinePlanSubDir(planName string, pullNum int) string
 	dirsStr := match[1] // in form dir_subdir_subsubdir
 	return filepath.Clean(strings.Replace(dirsStr, "_", "/", -1))
 }
+
+func (a *ApplyExecutor) updateGithubStatus(ctx *CommandContext, pathResults []PathResult) {
+	// the status will be the worst result
+	worstResult := a.worstResult(pathResults)
+	if worstResult == "success" {
+		a.github.UpdateStatus(ctx.Repo, ctx.Pull, SuccessStatus, "Apply Succeeded")
+	} else if worstResult == "failure" {
+		a.github.UpdateStatus(ctx.Repo, ctx.Pull, FailureStatus, "Apply Failed")
+	} else {
+		a.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Apply Error")
+	}
+}
+
+func (a *ApplyExecutor) worstResult(results []PathResult) string {
+	var worst string = "success"
+	for _, result := range results {
+		if result.Status == "error" {
+			return result.Status
+		} else if result.Status == "failure" {
+			worst = result.Status
+		}
+	}
+	return worst
+}

server/atlantis_config_file_test.go

@@ -1,10 +1,10 @@
 package server
 
 import (
+	. "github.com/hootsuite/atlantis/testing_util"
 	"io/ioutil"
 	"os"
 	"testing"
-	. "github.com/hootsuite/atlantis/testing_util"
 )
 
 var tempConfigFile = "/tmp/" + AtlantisConfigFile

server/base_executor.go

@@ -1,109 +0,0 @@
-package server
-
-import (
-	"path/filepath"
-	"github.com/hootsuite/atlantis/locking"
-)
-
-type BaseExecutor struct {
-	github                *GithubClient
-	awsConfig             *AWSConfig
-	scratchDir            string
-	s3Bucket              string
-	sshKey                string
-	ghComments            *GithubCommentRenderer
-	terraform             *TerraformClient
-	githubCommentRenderer *GithubCommentRenderer
-	lockingBackend        locking.Backend
-}
-
-type PullRequestContext struct {
-	repoFullName          string
-	head                  string
-	base                  string
-	number                int
-	pullRequestLink       string
-	terraformApplier      string
-	terraformApplierEmail string
-}
-
-type ExecutionResult struct {
-	SetupError   Templater
-	SetupFailure Templater
-	PathResults  []PathResult
-	Command      CommandType
-}
-
-type PathResult struct {
-	Path   string
-	Status string // todo: this should be an enum for success/error/failure
-	Result Templater
-}
-
-type ExecutionPath struct {
-	// Absolute is the full path on the OS where we will execute.
-	// Will never end with a '/'.
-	Absolute string
-	// Relative is the path relative to the repo root.
-	// Will never end with a '/'.
-	Relative string
-}
-
-func NewExecutionPath(absolutePath string, relativePath string) ExecutionPath {
-	return ExecutionPath{filepath.Clean(absolutePath), filepath.Clean(relativePath)}
-}
-
-func (b *BaseExecutor) updateGithubStatus(pullCtx *PullRequestContext, pathResults []PathResult) {
-	// the status will be the worst result
-	worstResult := b.worstResult(pathResults)
-	if worstResult == "success" {
-		b.github.UpdateStatus(pullCtx, SuccessStatus, "Plan Succeeded")
-	} else if worstResult == "failure" {
-		b.github.UpdateStatus(pullCtx, FailureStatus, "Plan Failed")
-	} else {
-		b.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
-	}
-}
-
-func (b *BaseExecutor) worstResult(results []PathResult) string {
-	var worst string = "success"
-	for _, result := range results {
-		if result.Status == "error" {
-			return result.Status
-		} else if result.Status == "failure" {
-			worst = result.Status
-		}
-	}
-	return worst
-}
-
-func (b *BaseExecutor) Exec(f func(*ExecutionContext, *PullRequestContext) ExecutionResult, ctx *ExecutionContext, github *GithubClient) {
-	pullCtx := b.githubContext(ctx)
-	result := f(ctx, pullCtx)
-	comment := b.githubCommentRenderer.render(result, ctx.log.History.String(), ctx.command.verbose)
-	github.CreateComment(pullCtx, comment)
-}
-
-func (b *BaseExecutor) githubContext(ctx *ExecutionContext) *PullRequestContext {
-	return &PullRequestContext{
-		repoFullName:          ctx.repoFullName,
-		head:                  ctx.head,
-		base:                  ctx.base,
-		number:                ctx.pullNum,
-		pullRequestLink:       ctx.pullLink,
-		terraformApplier:      ctx.requesterUsername,
-		terraformApplierEmail: ctx.requesterEmail,
-	}
-}
-
-type Templater interface {
-	Template() *CompiledTemplate
-}
-
-type GeneralError struct {
-	Error error
-}
-
-func (g GeneralError) Template() *CompiledTemplate {
-	return GeneralErrorTmpl
-}

server/bindata_assetfs.go

@@ -12,10 +12,10 @@
 package server
 
 import (
-	"github.com/elazarl/go-bindata-assetfs"
 	"bytes"
 	"compress/gzip"
 	"fmt"
+	"github.com/elazarl/go-bindata-assetfs"
 	"io"
 	"io/ioutil"
 	"os"
@@ -315,18 +315,19 @@ type bintree struct {
 	Func     func() (*asset, error)
 	Children map[string]*bintree
 }
+
 var _bintree = &bintree{nil, map[string]*bintree{
-	"static": &bintree{nil, map[string]*bintree{
-		"atlantis-icon.png": &bintree{staticAtlantisIconPng, map[string]*bintree{}},
-		"atlantis-icon_512.png": &bintree{staticAtlantisIcon_512Png, map[string]*bintree{}},
-		"css": &bintree{nil, map[string]*bintree{
-			"custom.css": &bintree{staticCssCustomCss, map[string]*bintree{}},
-			"normalize.css": &bintree{staticCssNormalizeCss, map[string]*bintree{}},
-			"skeleton.css": &bintree{staticCssSkeletonCss, map[string]*bintree{}},
+	"static": {nil, map[string]*bintree{
+		"atlantis-icon.png":     {staticAtlantisIconPng, map[string]*bintree{}},
+		"atlantis-icon_512.png": {staticAtlantisIcon_512Png, map[string]*bintree{}},
+		"css": {nil, map[string]*bintree{
+			"custom.css":    {staticCssCustomCss, map[string]*bintree{}},
+			"normalize.css": {staticCssNormalizeCss, map[string]*bintree{}},
+			"skeleton.css":  {staticCssSkeletonCss, map[string]*bintree{}},
 		}},
-		"images": &bintree{nil, map[string]*bintree{
-			"atlantis-icon.png": &bintree{staticImagesAtlantisIconPng, map[string]*bintree{}},
-			"atlantis-icon_512.png": &bintree{staticImagesAtlantisIcon_512Png, map[string]*bintree{}},
+		"images": {nil, map[string]*bintree{
+			"atlantis-icon.png":     {staticImagesAtlantisIconPng, map[string]*bintree{}},
+			"atlantis-icon_512.png": {staticImagesAtlantisIcon_512Png, map[string]*bintree{}},
 		}},
 	}},
 }}
@@ -378,7 +379,6 @@ func _filePath(dir, name string) string {
 	return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...)
 }
 
-
 func assetFS() *assetfs.AssetFS {
 	for k := range _bintree.Children {
 		return &assetfs.AssetFS{Asset: Asset, AssetDir: AssetDir, AssetInfo: AssetInfo, Prefix: k}

server/executor.go

@@ -1,5 +0,0 @@
-package server
-
-type Executor interface {
-	execute(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult
-}

server/github_client.go

@@ -1,10 +1,10 @@
 package server
 
 import (
+	"context"
 	"fmt"
 	"github.com/google/go-github/github"
-	"context"
-	"strings"
+	"github.com/hootsuite/atlantis/models"
 )
 
 type GithubClient struct {
@@ -20,17 +20,17 @@ const (
 	FailureStatus = "failure"
 )
 
-func (g *GithubClient) UpdateStatus(ctx *PullRequestContext, status string, description string) {
+func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, status string, description string) {
 	repoStatus := github.RepoStatus{State: github.String(status), Description: github.String(description), Context: github.String(statusContext)}
-	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
-	g.client.Repositories.CreateStatus(g.ctx, owner, repo, ctx.head, &repoStatus)
+	g.client.Repositories.CreateStatus(g.ctx, repo.Owner, repo.Name, pull.HeadCommit, &repoStatus)
 	// todo: deal with error updating status
 }
 
-func (g *GithubClient) GetModifiedFiles(ctx *PullRequestContext) ([]string, error) {
-	var files = []string{}
-	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
-	comparison, _, err := g.client.Repositories.CompareCommits(g.ctx, owner, repo, ctx.base, ctx.head)
+// GetModifiedFiles returns the names of files that were modified in the pull request.
+// The names include the path to the file from the repo root, ex. parent/child/file.txt
+func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) {
+	var files []string
+	comparison, _, err := g.client.Repositories.CompareCommits(g.ctx, repo.Owner, repo.Name, pull.BaseCommit, pull.HeadCommit)
 	if err != nil {
 		return files, err
 	}
@@ -40,40 +40,15 @@ func (g *GithubClient) GetModifiedFiles(ctx *PullRequestContext) ([]string, erro
 	return files, nil
 }
 
-func (g *GithubClient) CreateComment(ctx *PullRequestContext, comment string) error {
-	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
-	_, _, err := g.client.Issues.CreateComment(g.ctx, owner, repo, ctx.number, &github.IssueComment{Body: &comment})
+func (g *GithubClient) CreateComment(ctx *CommandContext, comment string) error {
+	_, _, err := g.client.Issues.CreateComment(g.ctx, ctx.Repo.Owner, ctx.Repo.Name, ctx.Pull.Num, &github.IssueComment{Body: &comment})
 	return err
 }
 
-// CommentExists searches through comments on a pull request and returns true if one matches matcher
-func (g *GithubClient) CommentExists(ctx *PullRequestContext, matcher func(*github.IssueComment) bool) (bool, error) {
-	opt := &github.IssueListCommentsOptions{}
-	// need to loop since there may be multiple pages of comments
-	for {
-		owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
-		comments, resp, err := g.client.Issues.ListComments(g.ctx, owner, repo, ctx.number, opt)
-		if err != nil {
-			return false, fmt.Errorf("failed to retrieve comments: %v", err)
-		}
-		for _, comment := range comments {
-			if matcher(comment) {
-				return true, nil
-			}
-		}
-		if resp.NextPage == 0 {
-			break
-		}
-		opt.ListOptions.Page = resp.NextPage
-	}
-	return false, nil
-}
-
-func (g *GithubClient) PullIsApproved(ctx *PullRequestContext) (bool, error) {
+func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) {
 	// todo: move back to using g.client.PullRequests.ListReviews when we update our GitHub enterprise version
 	// to where we don't need to include the custom accept header
-	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
-	u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", owner, repo, ctx.number)
+	u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", repo.Owner, repo.Name, pull.Num)
 	req, err := g.client.NewRequest("GET", u, nil)
 	if err != nil {
 		return false, err
@@ -93,17 +68,6 @@ func (g *GithubClient) PullIsApproved(ctx *PullRequestContext) (bool, error) {
 	return false, nil
 }
 
-func (g *GithubClient) GetPullRequest(repoFullName string, number int) (*github.PullRequest, *github.Response, error) {
-	owner, repo := g.repoFullNameToOwnerAndRepo(repoFullName)
-	return g.client.PullRequests.Get(g.ctx, owner, repo, number)
-}
-
-// repoFullNameToOwnerAndRepo splits up a repository full name which contains the organization and repo name separated by /
-// into its two parts: organization and repo name. ex baxterthehacker/public-repo => (baxterthehacker, public-repo)
-func (g *GithubClient) repoFullNameToOwnerAndRepo(fullName string) (string, string) {
-	split := strings.SplitN(fullName, "/", 2)
-	if len(split) != 2 {
-		return fmt.Sprintf("repo name %s could not be split into organization and name", fullName), ""
-	}
-	return split[0], split[1]
+func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRequest, *github.Response, error) {
+	return g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num)
 }

server/help_executor.go

@@ -2,9 +2,7 @@ package server
 
 import "github.com/spf13/viper"
 
-type HelpExecutor struct {
-	BaseExecutor
-}
+type HelpExecutor struct{}
 
 var helpComment = "```cmake\n" +
 	`atlantis - Terraform collaboration tool that enables you to collaborate on infrastructure
@@ -32,18 +30,8 @@ atlantis apply staging
 atlantis apply
 `
 
-func (h *HelpExecutor) execute(ctx *ExecutionContext, github *GithubClient) {
-	pullCtx := &PullRequestContext{
-		repoFullName:          ctx.repoFullName,
-		head:                  ctx.head,
-		base:                  ctx.base,
-		number:                ctx.pullNum,
-		pullRequestLink:       ctx.pullLink,
-		terraformApplier:      ctx.requesterUsername,
-		terraformApplierEmail: ctx.requesterEmail,
-	}
-	github.CreateComment(pullCtx, helpComment)
-
-	ctx.log.Info("generating help comment....")
+func (h *HelpExecutor) execute(ctx *CommandContext, github *GithubClient) {
+	ctx.Log.Info("generating help comment....")
+	github.CreateComment(ctx, helpComment)
 	return
 }

server/plan_executor.go

@@ -3,19 +3,27 @@ package server
 import (
 	"errors"
 	"fmt"
+	"github.com/hootsuite/atlantis/locking"
+	"github.com/hootsuite/atlantis/logging"
+	"github.com/hootsuite/atlantis/models"
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"strings"
-	"github.com/hootsuite/atlantis/locking"
-	"time"
-	"github.com/hootsuite/atlantis/logging"
 )
 
 // PlanExecutor handles everything related to running the Terraform plan including integration with S3, Terraform, and Github
 type PlanExecutor struct {
-	BaseExecutor
+	github                *GithubClient
+	awsConfig             *AWSConfig
+	scratchDir            string
+	s3Bucket              string
+	sshKey                string
+	terraform             *TerraformClient
+	githubCommentRenderer *GithubCommentRenderer
+	lockingClient         *locking.Client
 	// DeleteLockURL is a function that given a lock id will return a url for deleting the lock
 	DeleteLockURL func(id string) (url string)
 }
@@ -61,37 +69,38 @@ func (e EnvironmentFailure) Template() *CompiledTemplate {
 	return EnvironmentErrorTmpl
 }
 
-func (p *PlanExecutor) execute(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult {
-	res := p.setupAndPlan(ctx, pullCtx)
+func (p *PlanExecutor) execute(ctx *CommandContext, github *GithubClient) {
+	res := p.setupAndPlan(ctx)
 	res.Command = Plan
-	return res
+	comment := p.githubCommentRenderer.render(res, ctx.Log.History.String(), ctx.Command.verbose)
+	github.CreateComment(ctx, comment)
 }
 
-func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult {
-	p.github.UpdateStatus(pullCtx, "pending", "Planning...")
+func (p *PlanExecutor) setupAndPlan(ctx *CommandContext) ExecutionResult {
+	p.github.UpdateStatus(ctx.Repo, ctx.Pull, "pending", "Planning...")
 
 	// todo: lock when cloning or somehow separate workspaces
 	// clean the directory where we're going to clone
-	cloneDir := fmt.Sprintf("%s/%s/%d", p.scratchDir, ctx.repoFullName, ctx.pullNum)
-	ctx.log.Info("cleaning clone directory %q", cloneDir)
+	cloneDir := fmt.Sprintf("%s/%s/%d", p.scratchDir, ctx.Repo.FullName, ctx.Pull.Num)
+	ctx.Log.Info("cleaning clone directory %q", cloneDir)
 	if err := os.RemoveAll(cloneDir); err != nil {
-		ctx.log.Warn("failed to clean dir %q before cloning, attempting to continue: %v", cloneDir, err)
+		ctx.Log.Warn("failed to clean dir %q before cloning, attempting to continue: %v", cloneDir, err)
 	}
 
 	// create the directory and parents if necessary
-	ctx.log.Info("creating dir %q", cloneDir)
+	ctx.Log.Info("creating dir %q", cloneDir)
 	if err := os.MkdirAll(cloneDir, 0755); err != nil {
-		ctx.log.Warn("failed to create dir %q prior to cloning, attempting to continue: %v", cloneDir, err)
+		ctx.Log.Warn("failed to create dir %q prior to cloning, attempting to continue: %v", cloneDir, err)
 	}
 
 	// Check if ssh key is set and create git ssh wrapper
-	cloneCmd := exec.Command("git", "clone", ctx.repoSSHUrl, cloneDir)
+	cloneCmd := exec.Command("git", "clone", ctx.Repo.SSHURL, cloneDir)
 	if p.sshKey != "" {
 		err := GenerateSSHWrapper()
 		if err != nil {
 			errMsg := fmt.Sprintf("failed to create git ssh wrapper: %v", err)
-			ctx.log.Err(errMsg)
-			p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
+			ctx.Log.Err(errMsg)
+			p.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Plan Error")
 			return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 		}
 
@@ -102,53 +111,53 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, pullCtx *PullRequestC
 	}
 
 	// git clone the repo
-	ctx.log.Info("git cloning %q into %q", ctx.repoSSHUrl, cloneDir)
+	ctx.Log.Info("git cloning %q into %q", ctx.Repo.SSHURL, cloneDir)
 	if output, err := cloneCmd.CombinedOutput(); err != nil {
-		errMsg := fmt.Sprintf("failed to clone repository %q: %v: %s", ctx.repoSSHUrl, err, string(output))
-		ctx.log.Err(errMsg)
-		p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
+		errMsg := fmt.Sprintf("failed to clone repository %q: %v: %s", ctx.Repo.SSHURL, err, string(output))
+		ctx.Log.Err(errMsg)
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Plan Error")
 		return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 	}
 
 	// check out the branch for this PR
-	ctx.log.Info("checking out branch %q", ctx.branch)
-	checkoutCmd := exec.Command("git", "checkout", ctx.branch)
+	ctx.Log.Info("checking out branch %q", ctx.Pull.Branch)
+	checkoutCmd := exec.Command("git", "checkout", ctx.Pull.Branch)
 	checkoutCmd.Dir = cloneDir
 	if err := checkoutCmd.Run(); err != nil {
-		errMsg := fmt.Sprintf("failed to git checkout branch %q: %v", ctx.branch, err)
-		ctx.log.Err(errMsg)
-		p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
+		errMsg := fmt.Sprintf("failed to git checkout branch %q: %v", ctx.Pull.Branch, err)
+		ctx.Log.Err(errMsg)
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Plan Error")
 		return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 	}
 
-	ctx.log.Info("listing modified files from pull request")
-	modifiedFiles, err := p.github.GetModifiedFiles(pullCtx)
+	ctx.Log.Info("listing modified files from pull request")
+	modifiedFiles, err := p.github.GetModifiedFiles(ctx.Repo, ctx.Pull)
 	if err != nil {
 		errMsg := fmt.Sprintf("failed to retrieve list of modified files from GitHub: %v", err)
-		ctx.log.Err(errMsg)
-		p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
+		ctx.Log.Err(errMsg)
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Plan Error")
 		return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 	}
 	modifiedTerraformFiles := p.filterToTerraform(modifiedFiles)
 	if len(modifiedTerraformFiles) == 0 {
-		ctx.log.Info("no modified terraform files found, exiting")
-		p.github.UpdateStatus(pullCtx, FailureStatus, "Plan Failed")
+		ctx.Log.Info("no modified terraform files found, exiting")
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, FailureStatus, "Plan Failed")
 		return ExecutionResult{SetupError: GeneralError{errors.New("Plan Failed: no modified terraform files found")}}
 	}
-	ctx.log.Debug("Found %d modified terraform files: %v", len(modifiedTerraformFiles), modifiedTerraformFiles)
+	ctx.Log.Debug("Found %d modified terraform files: %v", len(modifiedTerraformFiles), modifiedTerraformFiles)
 
-	execPaths := p.DetermineExecPaths(p.trimSuffix(cloneDir, "/"), modifiedTerraformFiles)
-	if len(execPaths) == 0 {
-		ctx.log.Info("no exec paths found, exiting")
-		p.github.UpdateStatus(pullCtx, FailureStatus, "Plan Failed")
-		return ExecutionResult{SetupError: GeneralError{errors.New("Plan Failed: there were no paths to run plan in")}}
+	projects := p.ModifiedProjects(ctx.Repo.FullName, modifiedTerraformFiles)
+	if len(projects) == 0 {
+		ctx.Log.Info("no Terraform projects were modified")
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, FailureStatus, "Plan Failed")
+		return ExecutionResult{SetupError: GeneralError{errors.New("Plan Failed: we determined that no terraform projects were modified")}}
 	}
 
-	planFilesPrefix := fmt.Sprintf("%s_%d", strings.Replace(ctx.repoFullName, "/", "_", -1), ctx.pullNum)
-	if err := p.CleanWorkspace(ctx.log, planFilesPrefix, p.scratchDir, execPaths); err != nil {
+	planFilesPrefix := fmt.Sprintf("%s_%d", strings.Replace(ctx.Repo.FullName, "/", "_", -1), ctx.Pull.Num)
+	if err := p.CleanWorkspace(ctx.Log, planFilesPrefix, p.scratchDir, cloneDir, projects); err != nil {
 		errMsg := fmt.Sprintf("failed to clean workspace, aborting: %v", err)
-		ctx.log.Err(errMsg)
-		p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
+		ctx.Log.Err(errMsg)
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Plan Error")
 		return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 	}
 	s3Client := NewS3Client(p.awsConfig, p.s3Bucket, "plans")
@@ -156,25 +165,26 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, pullCtx *PullRequestC
 	var config Config
 	// run `terraform plan` in each plan path and collect the results
 	planOutputs := []PathResult{}
-	for _, path := range execPaths {
+	for _, project := range projects {
 		// todo: not sure it makes sense to be generating the output filename and plan name here
-		tfPlanFilename := p.GenerateOutputFilename(cloneDir, path, ctx.command.environment)
-		tfPlanName := fmt.Sprintf("%s_%d%s", strings.Replace(ctx.repoFullName, "/", "_", -1), ctx.pullNum, tfPlanFilename)
-		s3Key := fmt.Sprintf("%s/%s", ctx.repoFullName, tfPlanName)
+		tfPlanFilename := p.GenerateOutputFilename(project, ctx.Command.environment)
+		tfPlanName := fmt.Sprintf("%s_%d%s", strings.Replace(ctx.Repo.FullName, "/", "_", -1), ctx.Pull.Num, tfPlanFilename)
+		s3Key := fmt.Sprintf("%s/%s", ctx.Repo.FullName, tfPlanName)
 		// check if config file is found, if not we continue the run
-		if config.Exists(path.Absolute) {
-			ctx.log.Info("Config file found in %s", path.Absolute)
-			err := config.Read(path.Absolute)
+		absolutePath := filepath.Join(cloneDir, project.Path)
+		if config.Exists(absolutePath) {
+			ctx.Log.Info("Config file found in %s", absolutePath)
+			err := config.Read(absolutePath)
 			if err != nil {
 				errMsg := fmt.Sprintf("Error reading config file: %v", err)
-				ctx.log.Err(errMsg)
+				ctx.Log.Err(errMsg)
 				return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 			}
 			// need to use the remote state path and backend to do remote configure
-			err = PreRun(&config, ctx.log, path.Absolute, ctx.command)
+			err = PreRun(&config, ctx.Log, absolutePath, ctx.Command)
 			if err != nil {
 				errMsg := fmt.Sprintf("pre run failed: %v", err)
-				ctx.log.Err(errMsg)
+				ctx.Log.Err(errMsg)
 				return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}}
 			}
 
@@ -185,41 +195,31 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, pullCtx *PullRequestC
 				p.terraform.tfExecutableName = "terraform"
 			}
 		}
-		generatePlanResponse := p.plan(ctx.log, pullCtx, cloneDir, p.scratchDir, tfPlanName, s3Client, path, ctx.command.environment, s3Key, p.sshKey, ctx.pullCreator, config.StashPath)
-		generatePlanResponse.Path = path.Relative
+		generatePlanResponse := p.plan(ctx, cloneDir, p.scratchDir, tfPlanName, s3Client, project, s3Key, p.sshKey, config.StashPath)
+		generatePlanResponse.Path = project.Path
 		planOutputs = append(planOutputs, generatePlanResponse)
 	}
-	p.updateGithubStatus(pullCtx, planOutputs)
+	p.updateGithubStatus(ctx, planOutputs)
 	return ExecutionResult{PathResults: planOutputs}
 }
 
 // plan runs the steps necessary to run `terraform plan`. If there is an error, the error message will be encapsulated in error
 // and the GeneratePlanResponse struct will also contain the full log including the error
-func (p *PlanExecutor) plan(log *logging.SimpleLogger,
-	pullCtx *PullRequestContext,
+func (p *PlanExecutor) plan(
+	ctx *CommandContext,
 	repoDir string,
 	planOutDir string,
 	tfPlanName string,
 	s3Client S3Client,
-	path ExecutionPath,
-	tfEnvName string,
+	project models.Project,
 	s3Key string,
 	sshKey string,
-	pullRequestCreator string,
 	stashPath string) PathResult {
-	log.Info("generating plan for path %q", path)
-	run := locking.Run{
-		RepoFullName:  pullCtx.repoFullName,
-		Path:      path.Relative,
-		Env:       tfEnvName,
-		PullNum:    pullCtx.number,
-		User:      pullCtx.terraformApplier,
-		Timestamp: time.Now(),
-	}
+	ctx.Log.Info("generating plan for path %q", project.Path)
 
 	// NOTE: THIS CODE IS TO SUPPORT TERRAFORM PROJECTS THAT AREN'T USING ATLANTIS CONFIG FILE.
 	if stashPath == "" {
-		_, err := p.terraform.ConfigureRemoteState(log, path, tfEnvName, sshKey)
+		_, err := p.terraform.ConfigureRemoteState(ctx.Log, repoDir, project, ctx.Command.environment, sshKey)
 		if err != nil {
 			return PathResult{
 				Status: "error",
@@ -227,7 +227,12 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 			}
 		}
 	}
-	lockAttempt, err := p.lockingBackend.TryLock(run)
+	// todo: setting environment to default should be done elsewhere
+	tfEnv := ctx.Command.environment
+	if tfEnv == "" {
+		tfEnv = "default"
+	}
+	lockAttempt, err := p.lockingClient.TryLock(project, ctx.Command.environment, ctx.Pull.Num)
 	if err != nil {
 		return PathResult{
 			Status: " failure",
@@ -236,25 +241,25 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 	}
 
 	// the run is locked unless the locking run is the same pull id as this run
-	if lockAttempt.LockAcquired == false && lockAttempt.LockingRun.PullNum != pullCtx.number {
+	if lockAttempt.LockAcquired == false && lockAttempt.LockingPullNum != ctx.Pull.Num {
 		return PathResult{
 			Status: "failure",
-			Result: RunLockedFailure{lockAttempt.LockingRun.PullNum},
+			Result: RunLockedFailure{lockAttempt.LockingPullNum},
 		}
 	}
 
 	// Run terraform plan
-	log.Info("running terraform plan in directory %q", path.Relative)
+	ctx.Log.Info("running terraform plan in directory %q", project.Path)
 	tfPlanCmd := []string{"plan", "-refresh", "-no-color"}
 	// Generate terraform plan filename
 	tfPlanOutputPath := filepath.Join(planOutDir, tfPlanName)
 	// Generate terraform plan arguments
-	if tfEnvName != "" {
-		tfEnvFileName := filepath.Join("env", tfEnvName+".tfvars")
-		if _, err := os.Stat(filepath.Join(path.Absolute, tfEnvFileName)); err == nil {
+	if ctx.Command.environment != "" {
+		tfEnvFileName := filepath.Join("env", ctx.Command.environment+".tfvars")
+		if _, err := os.Stat(filepath.Join(repoDir, project.Path, tfEnvFileName)); err == nil {
 			tfPlanCmd = append(tfPlanCmd, "-var-file", tfEnvFileName, "-out", tfPlanOutputPath)
 		} else {
-			log.Err("environment file %q not found", tfEnvFileName)
+			ctx.Log.Err("environment file %q not found", tfEnvFileName)
 			return PathResult{
 				Status: "failure",
 				Result: EnvironmentFileNotFoundFailure{tfEnvFileName},
@@ -265,10 +270,10 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 	}
 
 	// set pull request creator as the session name
-	p.awsConfig.AWSSessionName = pullRequestCreator
+	p.awsConfig.AWSSessionName = ctx.Pull.Author
 	awsSession, err := p.awsConfig.CreateAWSSession()
 	if err != nil {
-		log.Err(err.Error())
+		ctx.Log.Err(err.Error())
 		return PathResult{
 			Status: "error",
 			Result: GeneralError{err},
@@ -278,14 +283,14 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 	credVals, err := awsSession.Config.Credentials.Get()
 	if err != nil {
 		err = fmt.Errorf("failed to get assumed role credentials: %v", err)
-		log.Err(err.Error())
+		ctx.Log.Err(err.Error())
 		return PathResult{
 			Status: "error",
 			Result: GeneralError{err},
 		}
 	}
 
-	terraformPlanCmdArgs, output, err := p.terraform.RunTerraformCommand(path, tfPlanCmd, []string{
+	terraformPlanCmdArgs, output, err := p.terraform.RunTerraformCommand(filepath.Join(repoDir, project.Path), tfPlanCmd, []string{
 		fmt.Sprintf("AWS_ACCESS_KEY_ID=%s", credVals.AccessKeyID),
 		fmt.Sprintf("AWS_SECRET_ACCESS_KEY=%s", credVals.SecretAccessKey),
 		fmt.Sprintf("AWS_SESSION_TOKEN=%s", credVals.SessionToken),
@@ -299,10 +304,10 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 			Command: strings.Join(terraformPlanCmdArgs, " "),
 			Output:  output,
 		}
-		log.Err("error running terraform plan: %v", output)
-		log.Info("unlocking state since plan failed")
-		if err := p.lockingBackend.Unlock(lockAttempt.LockID); err != nil {
-			log.Err("error unlocking state: %v", err)
+		ctx.Log.Err("error running terraform plan: %v", output)
+		ctx.Log.Info("unlocking state since plan failed")
+		if err := p.lockingClient.Unlock(lockAttempt.LockKey); err != nil {
+			ctx.Log.Err("error unlocking state: %v", err)
 		}
 		return PathResult{
 			Status: "failure",
@@ -310,12 +315,12 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 		}
 	}
 	// Upload plan to S3
-	log.Info("uploading plan to S3 with key %q", s3Key)
+	ctx.Log.Info("uploading plan to S3 with key %q", s3Key)
 	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.lockingBackend.Unlock(lockAttempt.LockID); err != nil {
-			log.Err("error unlocking state: %v", err)
+		ctx.Log.Err(err.Error())
+		if err := p.lockingClient.Unlock(lockAttempt.LockKey); err != nil {
+			ctx.Log.Err("error unlocking state: %v", err)
 		}
 		return PathResult{
 			Status: "error",
@@ -324,22 +329,22 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger,
 	}
 	// Delete local plan file
 	planFilePath := fmt.Sprintf("%s/%s", planOutDir, tfPlanName)
-	log.Info("deleting local plan file %q", planFilePath)
+	ctx.Log.Info("deleting local plan file %q", planFilePath)
 	if err := os.Remove(planFilePath); err != nil {
-		log.Err("failed to delete local plan file %q", planFilePath, err)
+		ctx.Log.Err("failed to delete local plan file %q", planFilePath, err)
 		// todo: return an error
 	}
 	return PathResult{
 		Status: "success",
 		Result: PlanSuccess{
 			TerraformOutput: output,
-			LockURL: p.DeleteLockURL(lockAttempt.LockID),
+			LockURL:         p.DeleteLockURL(lockAttempt.LockKey),
 		},
 	}
 }
 
 func (p *PlanExecutor) filterToTerraform(files []string) []string {
-	var out = []string{}
+	var out []string
 	for _, fileName := range files {
 		if !p.isInExcludeList(fileName) && strings.Contains(fileName, ".tf") {
 			out = append(out, fileName)
@@ -359,46 +364,40 @@ func (p *PlanExecutor) trimSuffix(s, suffix string) string {
 	return s
 }
 
-func (p *PlanExecutor) removeDuplicates(paths []ExecutionPath) []ExecutionPath {
-	deDuped := []ExecutionPath{}
-	seen := map[ExecutionPath]bool{}
-	for _, path := range paths {
-		if _, ok := seen[path]; !ok {
-			deDuped = append(deDuped, path)
-			seen[path] = true
-		}
-	}
-	return deDuped
-}
-
-// DetermineExecPaths returns the list of directories in which we'll need to run `terraform plan`
-func (p *PlanExecutor) DetermineExecPaths(repoPath string, modifiedFiles []string) []ExecutionPath {
-	var paths []ExecutionPath
+// ModifiedProjects returns the list of Terraform projects that have been changed due to the
+// modified files
+func (p *PlanExecutor) ModifiedProjects(repoFullName string, modifiedFiles []string) []models.Project {
+	var projects []models.Project
+	seenPaths := make(map[string]bool)
 	for _, modifiedFile := range modifiedFiles {
-		relative := p.getRelativePlanPath(modifiedFile)
-		absolute := filepath.Join(repoPath, relative) + "/"
-		paths = append(paths, NewExecutionPath(absolute, relative))
+		path := p.getProjectPath(modifiedFile)
+		if _, ok := seenPaths[path]; !ok {
+			projects = append(projects, models.NewProject(repoFullName, path))
+			seenPaths[path] = true
 		}
-	return p.removeDuplicates(paths)
+	}
+	return projects
 }
 
-func (p *PlanExecutor) getRelativePlanPath(modifiedFilePath string) string {
-	parentDir := filepath.Dir(modifiedFilePath)
-	if filepath.Base(parentDir) == "env" {
+// getProjectPath returns the path to the project relative to the repo root
+// if the project is at the root returns "."
+func (p *PlanExecutor) getProjectPath(modifiedFilePath string) string {
+	dir := path.Dir(modifiedFilePath)
+	if path.Base(dir) == "env" {
 		// if the modified file was inside an env/ directory, we treat this specially and
 		// run plan one level up
-		return filepath.Dir(parentDir)
+		return path.Dir(dir)
 	}
-	return parentDir
+	return dir
 }
 
-// CleanWorkspace deletes all .terraform/ folders from the plan paths and cleans up any plans in the output directory
-func (p *PlanExecutor) CleanWorkspace(log *logging.SimpleLogger, deleteFilesPrefix string, planOutDir string, execPaths []ExecutionPath) error {
-	log.Info("cleaning workspace directory %q and plan paths %v", planOutDir, execPaths)
+// CleanWorkspace deletes all .terraform/ folders from the project folders and cleans up any plans in the output directory
+func (p *PlanExecutor) CleanWorkspace(log *logging.SimpleLogger, deleteFilesPrefix string, planOutDir string, repoDir string, projects []models.Project) error {
+	log.Info("cleaning workspace directory %q", planOutDir)
 
 	// delete .terraform directories
-	for _, path := range execPaths {
-		os.RemoveAll(filepath.Join(path.Absolute, ".terraform"))
+	for _, project := range projects {
+		os.RemoveAll(filepath.Join(repoDir, project.Path, ".terraform"))
 	}
 	// delete old plan files
 	files, err := ioutil.ReadDir(planOutDir)
@@ -423,14 +422,14 @@ func (p *PlanExecutor) DeleteLocalPlanFile(path string) error {
 
 // GenerateOutputFilename determines the name of the plan that will be stored in s3
 // if we're executing inside a sub directory, there will be a leading underscore
-func (p *PlanExecutor) GenerateOutputFilename(repoDir string, execPath ExecutionPath, tfEnvName string) string {
+func (p *PlanExecutor) GenerateOutputFilename(project models.Project, tfEnvName string) string {
 	prefix := ""
-	if execPath.Relative != "." {
+	if project.Path != "." {
 		// If not executing at repo root, need to encode the sub dir in the name of the output file.
 		// We do this by substituting / for _
 		// We also add an _ because this gets appended to a larger path
 		// todo: refactor the path handling so it's all in one place
-		prefix = "_" + strings.Replace(execPath.Relative, "/", "_", -1)
+		prefix = "_" + strings.Replace(project.Path, "/", "_", -1)
 	}
 	suffix := ""
 	if tfEnvName != "" {
@@ -443,3 +442,27 @@ func (p *PlanExecutor) GenerateOutputFilename(repoDir string, execPath Execution
 func generateStatePath(path string, tfEnvName string) string {
 	return strings.Replace(path, "$ENVIRONMENT", tfEnvName, -1)
 }
+
+func (p *PlanExecutor) updateGithubStatus(ctx *CommandContext, pathResults []PathResult) {
+	// the status will be the worst result
+	worstResult := p.worstResult(pathResults)
+	if worstResult == "success" {
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, SuccessStatus, "Plan Succeeded")
+	} else if worstResult == "failure" {
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, FailureStatus, "Plan Failed")
+	} else {
+		p.github.UpdateStatus(ctx.Repo, ctx.Pull, ErrorStatus, "Plan Error")
+	}
+}
+
+func (p *PlanExecutor) worstResult(results []PathResult) string {
+	var worst string = "success"
+	for _, result := range results {
+		if result.Status == "error" {
+			return result.Status
+		} else if result.Status == "failure" {
+			worst = result.Status
+		}
+	}
+	return worst
+}

server/plan_executor_test.go

@@ -1,40 +1,41 @@
 package server
 
 import (
-	"reflect"
+	"github.com/hootsuite/atlantis/models"
+	. "github.com/hootsuite/atlantis/testing_util"
 	"testing"
 )
 
-func TestDetermineExecPaths(t *testing.T) {
-	runTest(t, "should handle empty plan paths", "/repoPath", []string{}, []ExecutionPath{})
-	runTest(t, "should extract path and append to repoPath", "/repoPath", []string{"a/b/c.ext"}, []ExecutionPath{{"/repoPath/a/b", "a/b"}})
-	runTest(t, "should return repoPath if at root", "/repoPath", []string{"a.ext"}, []ExecutionPath{{"/repoPath", "."}})
-	runTest(t, "should handle repoPath with trailing slash", "/repoPath/", []string{"a.ext"}, []ExecutionPath{{"/repoPath", "."}})
-	runTest(t, "should set plan dir one level up from env/ directories", "/repoPath/", []string{"env/a.ext"}, []ExecutionPath{{"/repoPath", "."}})
-	runTest(t, "should set plan dir one level up from env/ directories and deduplicate plans.", "/repoPath/", []string{"env/a.ext", "b.ext"}, []ExecutionPath{{"/repoPath", "."}})
-	runTest(t, "should de-depluciate", "/repoPath/", []string{"a/b/c.ext", "a/b/d.ext"}, []ExecutionPath{{"/repoPath/a/b", "a/b"}})
+var p PlanExecutor
+
+func TestModifiedProjects(t *testing.T) {
+	runTest(t, "should handle no files modified", []string{}, []string{})
+	runTest(t, "should handle files at root", []string{"root.tf"}, []string{"."})
+	runTest(t, "should de-duplicate files at root", []string{"root1.tf", "root2.tf"}, []string{"."})
+	runTest(t, "should handle sub directories", []string{"sub/dir/file.tf"}, []string{"sub/dir"})
+	runTest(t, "should de-duplicate files in sub directories", []string{"sub/dir/file.tf", "sub/dir/file2.tf"}, []string{"sub/dir"})
+	runTest(t, "should handle nested sub directories", []string{"root.tf", "sub/child.tf", "sub/sub/child.tf"}, []string{".", "sub", "sub/sub"})
+	runTest(t, "should use parent of env/ dirs", []string{"env/env.tf"}, []string{"."})
+	runTest(t, "should use parent of env/ dirs in sub dirs", []string{"sub/env/env.tf"}, []string{"sub"})
 }
 
-func runTest(t *testing.T, testDescrip string, repoPath string, filesChanged []string, expected []ExecutionPath) {
-	p := PlanExecutor{}
-	plans := p.DetermineExecPaths(repoPath, filesChanged)
-	if !reflect.DeepEqual(expected, plans) {
-		t.Errorf("%s: expected %v, got %v", testDescrip, expected, plans)
+func runTest(t *testing.T, testDescrip string, filesChanged []string, expectedPaths []string) {
+	projects := p.ModifiedProjects("owner/repo", filesChanged)
+	for i, p := range projects {
+		t.Log(testDescrip)
+		Equals(t, expectedPaths[i], p.Path)
 	}
 }
 
 func TestGenerateOutputFilename(t *testing.T) {
-	runTestGetOutputFilename(t, "should handle empty plan path", "/repoPath", NewExecutionPath("", ""), "env", ".tfplan.env")
-	runTestGetOutputFilename(t, "should handle empty environment", "/repoPath", NewExecutionPath("", ""), "", ".tfplan")
-	runTestGetOutputFilename(t, "should prepend underscore on relative paths", "/repoPath", NewExecutionPath("", "a/b"), "", "_a_b.tfplan")
-	runTestGetOutputFilename(t, "should work with relative path and environment", "/repoPath", NewExecutionPath("", "a/b"), "env", "_a_b.tfplan.env")
-	runTestGetOutputFilename(t, "should exec path at root", "/a/b", NewExecutionPath("", "."), "env", ".tfplan.env")
+	runTestGetOutputFilename(t, "should handle root", ".", "env", ".tfplan.env")
+	runTestGetOutputFilename(t, "should handle empty environment", ".", "", ".tfplan")
+	runTestGetOutputFilename(t, "should prepend underscore on relative paths", "a/b", "", "_a_b.tfplan")
+	runTestGetOutputFilename(t, "should prepend underscore on relative paths and env", "a/b", "env", "_a_b.tfplan.env")
 }
 
-func runTestGetOutputFilename(t *testing.T, testDescrip string, repoPath string, tfPlanPath ExecutionPath, tfEnvName string, expected string) {
-	p := PlanExecutor{}
-	outputFileName := p.GenerateOutputFilename(repoPath, tfPlanPath, tfEnvName)
-	if !reflect.DeepEqual(expected, outputFileName) {
-		t.Errorf("%s: expected %v, got %v", testDescrip, expected, outputFileName)
-	}
+func runTestGetOutputFilename(t *testing.T, testDescrip string, path string, env string, expected string) {
+	t.Log(testDescrip)
+	outputFileName := p.GenerateOutputFilename(models.NewProject("owner/repo", path), env)
+	Equals(t, expected, outputFileName)
 }

server/pre_run.go

@@ -3,17 +3,18 @@ package server
 import (
 	"bufio"
 	"fmt"
+	"github.com/hootsuite/atlantis/logging"
 	"io/ioutil"
 	"os"
 	"os/exec"
-	"github.com/hootsuite/atlantis/logging"
 )
 
 const InlineShebang = "/bin/sh -e"
 
+// todo: make OO
 // PreRun is a function that will determine whether
-func PreRun(c *Config, log *logging.SimpleLogger, execPath string, command *Command) error {
-	log.Info("Staring pre run in %s", execPath)
+func PreRun(c *Config, log *logging.SimpleLogger, path string, command *Command) error {
+	log.Info("Staring pre run in %s", path)
 	var execScript string
 
 	if command.commandType == Plan {
@@ -44,7 +45,7 @@ func PreRun(c *Config, log *logging.SimpleLogger, execPath string, command *Comm
 		if c.TerraformVersion != "" {
 			os.Setenv("ATLANTIS_TERRAFORM_VERSION", c.TerraformVersion)
 		}
-		os.Setenv("WORKSPACE", execPath)
+		os.Setenv("WORKSPACE", path)
 		output, err := execute(execScript)
 		if err != nil {
 			return err

server/pre_run_test.go

@@ -1,11 +1,11 @@
 package server
 
 import (
+	"github.com/hootsuite/atlantis/logging"
+	. "github.com/hootsuite/atlantis/testing_util"
 	"log"
 	"os"
 	"testing"
-	. "github.com/hootsuite/atlantis/testing_util"
-	"github.com/hootsuite/atlantis/logging"
 )
 
 var level logging.LogLevel = logging.Info

server/request_parser.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/google/go-github/github"
+	"github.com/hootsuite/atlantis/models"
 	"regexp"
 )
 
@@ -24,6 +25,7 @@ type Command struct {
 }
 
 func (r *RequestParser) determineCommand(comment *github.IssueCommentEvent) (*Command, error) {
+	// for legacy, also support "run" instead of atlantis
 	atlantisCommentRegex := `^(?:run|atlantis) (plan|apply|help)([[:blank:]])?([a-zA-Z0-9_-]+)?\s*(--verbose)?$`
 	runPlanMatcher := regexp.MustCompile(atlantisCommentRegex)
 
@@ -37,7 +39,7 @@ func (r *RequestParser) determineCommand(comment *github.IssueCommentEvent) (*Co
 		return nil, errors.New("key 'comment.comment.body' is null")
 	}
 
-	// extract the command and environment. ex. for "run plan staging", the command is "plan", and the environment is "staging"
+	// extract the command and environment. ex. for "atlantis plan staging", the command is "plan", and the environment is "staging"
 	match := runPlanMatcher.FindStringSubmatch(*commentBody)
 	if len(match) < 5 {
 		var truncated = *commentBody
@@ -65,13 +67,19 @@ func (r *RequestParser) determineCommand(comment *github.IssueCommentEvent) (*Co
 	return command, nil
 }
 
-func (r *RequestParser) extractCommentData(comment *github.IssueCommentEvent, params *ExecutionContext) error {
-	missingField := "<nil>"
-
+func (r *RequestParser) extractCommentData(comment *github.IssueCommentEvent, ctx *CommandContext) error {
 	repoFullName := comment.Repo.FullName
 	if repoFullName == nil {
 		return errors.New("key 'comment.repo.full_name' is null")
 	}
+	repoOwner := comment.Repo.Owner.Login
+	if repoOwner == nil {
+		return errors.New("key 'comment.repo.owner.login' is null")
+	}
+	repoName := comment.Repo.Name
+	if repoName == nil {
+		return errors.New("key 'comment.repo.name' is null")
+	}
 	pullNum := comment.Issue.Number
 	if pullNum == nil {
 		return errors.New("key 'comment.issue.number' is null")
@@ -84,10 +92,6 @@ func (r *RequestParser) extractCommentData(comment *github.IssueCommentEvent, pa
 	if commentorUsername == nil {
 		return errors.New("key 'comment.comment.user.login' is null")
 	}
-	commentorEmail := comment.Comment.User.Email
-	if commentorEmail == nil {
-		commentorEmail = &missingField
-	}
 	repoSSHURL := comment.Repo.SSHURL
 	if repoSSHURL == nil {
 		return errors.New("key 'comment.repo.sshurl' is null")
@@ -96,17 +100,23 @@ func (r *RequestParser) extractCommentData(comment *github.IssueCommentEvent, pa
 	if htmlURL == nil {
 		return errors.New("key 'comment.issue.htmlUrl' is null")
 	}
-	params.requesterUsername = *commentorUsername
-	params.requesterEmail = *commentorEmail
-	params.repoFullName = *repoFullName
-	params.pullNum = *pullNum
-	params.pullCreator = *pullCreator
-	params.repoSSHUrl = *repoSSHURL
-	params.htmlUrl = *htmlURL
+	ctx.Repo = models.Repo{
+		FullName: *repoFullName,
+		Owner:    *repoOwner,
+		Name:     *repoName,
+		SSHURL:   *repoSSHURL,
+	}
+	ctx.User = models.User{
+		Username: *commentorUsername,
+	}
+	ctx.Pull = models.PullRequest{
+		Num: *pullNum,
+	}
+
 	return nil
 }
 
-func (r *RequestParser) extractPullData(pull *github.PullRequest, params *ExecutionContext) error {
+func (r *RequestParser) extractPullData(pull *github.PullRequest, params *CommandContext) error {
 	commit := pull.Head.SHA
 	if commit == nil {
 		return errors.New("key 'pull.head.sha' is null")
@@ -123,9 +133,21 @@ func (r *RequestParser) extractPullData(pull *github.PullRequest, params *Execut
 	if branch == nil {
 		return errors.New("key 'pull.head.ref' is null")
 	}
-	params.branch = *branch
-	params.head = *commit
-	params.base = *base
-	params.pullLink = *pullLink
+	authorUsername := pull.User.Login
+	if authorUsername == nil {
+		return errors.New("key 'pull.user.login' is null")
+	}
+	num := pull.Number
+	if num == nil {
+		return errors.New("key 'pull.num' is null")
+	}
+	params.Pull = models.PullRequest{
+		BaseCommit: *base,
+		Author:     *authorUsername,
+		Branch:     *branch,
+		HeadCommit: *commit,
+		Link:       *pullLink,
+		Num:        *num,
+	}
 	return nil
 }

server/s3_client.go

@@ -48,6 +48,7 @@ func NewS3Client(awsConfig *AWSConfig, bucketName string, prefix string) S3Clien
 	return _s3.GetS3Info()
 }
 
+// todo: make OO
 func UploadPlanFile(s S3Client, key string, outputFilePath string) error {
 	file, err := os.Open(outputFilePath)
 	if err != nil {
@@ -75,7 +76,7 @@ func UploadPlanFile(s S3Client, key string, outputFilePath string) error {
 		ContentLength: aws.Int64(size),
 		ContentType:   aws.String(fileType),
 		Metadata: map[string]*string{
-			"Key": aws.String("MetadataValue"), //required
+			"Key": aws.String("MetadataValue"), //required //todo: I don't think this is required
 		},
 	}
 

server/server.go

@@ -2,27 +2,30 @@ package server
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"log"
 	"net/http"
 	"net/url"
 	"os"
 	"strings"
-	"encoding/json"
 
-	"github.com/google/go-github/github"
-	"github.com/urfave/cli"
-	"github.com/hootsuite/atlantis/recovery"
-	"github.com/hootsuite/atlantis/locking"
-	"github.com/urfave/negroni"
-	"github.com/hootsuite/atlantis/middleware"
-	"github.com/hootsuite/atlantis/logging"
 	"github.com/elazarl/go-bindata-assetfs"
+	"github.com/google/go-github/github"
 	"github.com/gorilla/mux"
-	"github.com/pkg/errors"
-	"io/ioutil"
-	"github.com/hootsuite/atlantis/locking/dynamodb"
+	"github.com/hootsuite/atlantis/locking"
 	"github.com/hootsuite/atlantis/locking/boltdb"
+	"github.com/hootsuite/atlantis/locking/dynamodb"
+	"github.com/hootsuite/atlantis/logging"
+	"github.com/hootsuite/atlantis/middleware"
+	"github.com/hootsuite/atlantis/models"
+	"github.com/hootsuite/atlantis/recovery"
+	"github.com/pkg/errors"
+	"github.com/urfave/cli"
+	"github.com/urfave/negroni"
+	"io/ioutil"
+	"path/filepath"
+	"time"
 )
 
 const (
@@ -31,7 +34,7 @@ const (
 	LockingDynamoDBBackend = "dynamodb"
 )
 
-// WebhookServer listens for Github webhooks and runs the necessary Atlantis command
+// Server listens for Github webhooks and runs the necessary Atlantis command
 type Server struct {
 	router           *mux.Router
 	port             int
@@ -46,7 +49,7 @@ type Server struct {
 	logger           *logging.SimpleLogger
 	githubComments   *GithubCommentRenderer
 	requestParser    *RequestParser
-	lockingBackend   locking.Backend
+	lockingClient    *locking.Client
 	atlantisURL      string
 }
 
@@ -69,22 +72,51 @@ type ServerConfig struct {
 	LockingDynamoDBTable string `mapstructure:"locking-dynamodb-table"`
 }
 
-type ExecutionContext struct {
-	repoFullName      string
-	pullNum           int
-	requesterUsername string
-	requesterEmail    string
-	comment           string
-	repoSSHUrl        string
-	head              string
-	// commit base sha
-	base              string
-	pullLink          string
-	branch            string
-	htmlUrl           string
-	pullCreator       string
-	command           *Command
-	log         *logging.SimpleLogger
+// todo: rename to Command
+type CommandContext struct {
+	Repo    models.Repo
+	Pull    models.PullRequest
+	User    models.User
+	Command *Command
+	Log     *logging.SimpleLogger
+}
+
+type ExecutionResult struct {
+	SetupError   Templater
+	SetupFailure Templater
+	PathResults  []PathResult
+	Command      CommandType
+}
+
+type PathResult struct {
+	Path   string
+	Status string // todo: this should be an enum for success/error/failure
+	Result Templater
+}
+
+type ExecutionPath struct {
+	// Absolute is the full path on the OS where we will execute.
+	// Will never end with a '/'.
+	Absolute string
+	// Relative is the path relative to the repo root.
+	// Will never end with a '/'.
+	Relative string
+}
+
+func NewExecutionPath(absolutePath string, relativePath string) ExecutionPath {
+	return ExecutionPath{filepath.Clean(absolutePath), filepath.Clean(relativePath)}
+}
+
+type Templater interface {
+	Template() *CompiledTemplate
+}
+
+type GeneralError struct {
+	Error error
+}
+
+func (g GeneralError) Template() *CompiledTemplate {
+	return GeneralErrorTmpl
 }
 
 func NewServer(config ServerConfig) (*Server, error) {
@@ -108,34 +140,42 @@ func NewServer(config ServerConfig) (*Server, error) {
 		AWSRegion:  config.AWSRegion,
 		AWSRoleArn: config.AssumeRole,
 	}
-	var lockingBackend locking.Backend
+	var lockingClient *locking.Client
 	if config.LockingBackend == LockingDynamoDBBackend {
 		session, err := awsConfig.CreateAWSSession()
 		if err != nil {
 			return nil, errors.Wrap(err, "creating aws session for DynamoDB")
 		}
-		lockingBackend = dynamodb.New(config.LockingDynamoDBTable, session)
+		lockingClient = locking.NewClient(dynamodb.New(config.LockingDynamoDBTable, session))
 	} else {
-		var err error
-		lockingBackend, err = boltdb.New(config.DataDir)
+		backend, err := boltdb.New(config.DataDir)
 		if err != nil {
 			return nil, err
 		}
+		lockingClient = locking.NewClient(backend)
 	}
-	baseExecutor := BaseExecutor{
+	applyExecutor := &ApplyExecutor{
 		github:                githubClient,
 		awsConfig:             awsConfig,
 		scratchDir:            config.ScratchDir,
 		s3Bucket:              config.S3Bucket,
 		sshKey:                config.SSHKey,
-		ghComments:            githubComments,
 		terraform:             terraformClient,
 		githubCommentRenderer: githubComments,
-		lockingBackend:        lockingBackend,
+		lockingClient:         lockingClient,
+		requireApproval:       config.RequireApproval,
 	}
-	applyExecutor := &ApplyExecutor{BaseExecutor: baseExecutor, requireApproval: config.RequireApproval, atlantisGithubUser: config.GitHubUser}
-	planExecutor := &PlanExecutor{BaseExecutor: baseExecutor}
-	helpExecutor := &HelpExecutor{BaseExecutor: baseExecutor}
+	planExecutor := &PlanExecutor{
+		github:                githubClient,
+		awsConfig:             awsConfig,
+		scratchDir:            config.ScratchDir,
+		s3Bucket:              config.S3Bucket,
+		sshKey:                config.SSHKey,
+		terraform:             terraformClient,
+		githubCommentRenderer: githubComments,
+		lockingClient:         lockingClient,
+	}
+	helpExecutor := &HelpExecutor{}
 	logger := logging.NewSimpleLogger("server", log.New(os.Stderr, "", log.LstdFlags), false, logging.ToLogLevel(config.LogLevel))
 	router := mux.NewRouter()
 	return &Server{
@@ -152,7 +192,7 @@ func NewServer(config ServerConfig) (*Server, error) {
 		logger:           logger,
 		githubComments:   githubComments,
 		requestParser:    &RequestParser{},
-		lockingBackend:   lockingBackend,
+		lockingClient:    lockingClient,
 		atlantisURL:      config.AtlantisURL,
 	}, nil
 }
@@ -189,23 +229,27 @@ func (s *Server) Start() error {
 }
 
 func (s *Server) index(w http.ResponseWriter, r *http.Request) {
-	locks, err := s.lockingBackend.ListLocks()
+	locks, err := s.lockingClient.List()
 	if err != nil {
 		w.WriteHeader(http.StatusServiceUnavailable)
 		fmt.Fprintf(w, "Could not retrieve locks: %s", err)
 		return
 	}
 
-	type runLock struct {
-		locking.Run
+	type lock struct {
 		UnlockURL    string
+		RepoFullName string
+		PullNum      int
+		Time         time.Time
 	}
-	var results []runLock
+	var results []lock
 	for id, v := range locks {
 		u, _ := s.router.Get(deleteLockRoute).URL("id", url.QueryEscape(id))
-		results = append(results, runLock{
-			v,
-			u.String(),
+		results = append(results, lock{
+			UnlockURL:    u.String(),
+			RepoFullName: v.Project.RepoFullName,
+			PullNum:      v.PullNum,
+			Time:         v.Time,
 		})
 	}
 	indexTemplate.Execute(w, results)
@@ -222,7 +266,7 @@ func (s *Server) deleteLock(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprint(w, "invalid lock id")
 	}
-	if err := s.lockingBackend.Unlock(idUnencoded); err != nil {
+	if err := s.lockingClient.Unlock(idUnencoded); err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		fmt.Fprintf(w, "Failed to unlock: %s", err)
 		return
@@ -261,42 +305,27 @@ func (s *Server) postHooks(w http.ResponseWriter, r *http.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)
+	s.logger.Debug("Unlocking locks for repo %s and pull %d %s", repo, pullNum, githubReqID)
+	err := s.lockingClient.UnlockByPull(repo, pullNum)
 	if err != nil {
-		s.logger.Err("finding locks for repo %s pull %d: %s", repo, pullNum, err)
+		s.logger.Err("unlocking locks for repo %s pull %d: %v", repo, pullNum, err)
 		w.WriteHeader(http.StatusServiceUnavailable)
-		fmt.Fprintf(w, "Error finding locks: %s\n", err)
+		fmt.Fprintf(w, "Error unlocking locks: %v\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{}
+	ctx := &CommandContext{}
 	command, err := s.requestParser.determineCommand(&comment)
 	if err != nil {
 		s.logger.Debug("Ignoring request: %v %s", err, githubReqID)
 		fmt.Fprintln(w, "Ignoring")
 		return
 	}
-	ctx.command = command
+	ctx.Command = command
 
 	if err = s.requestParser.extractCommentData(&comment, ctx); err != nil {
 		s.logger.Err("Failed parsing event: %v %s", err, githubReqID)
@@ -308,32 +337,32 @@ func (s *Server) handleCommentCreatedEvent(w http.ResponseWriter, comment github
 	go s.executeCommand(ctx)
 }
 
-func (s *Server) executeCommand(ctx *ExecutionContext) {
-	src := fmt.Sprintf("%s/pull/%d", ctx.repoFullName, ctx.pullNum)
+func (s *Server) executeCommand(ctx *CommandContext) {
+	src := fmt.Sprintf("%s/pull/%d", ctx.Repo.FullName, ctx.Pull.Num)
 	// it's safe to reuse the underlying logger s.logger.Log
-	ctx.log = logging.NewSimpleLogger(src, s.logger.Log, true, s.logger.Level)
+	ctx.Log = logging.NewSimpleLogger(src, s.logger.Log, true, s.logger.Level)
 	defer s.recover(ctx)
 
 	// we've got data from the comment, now we need to get data from the actual PR
-	pull, _, err := s.githubClient.GetPullRequest(ctx.repoFullName, ctx.pullNum)
+	pull, _, err := s.githubClient.GetPullRequest(ctx.Repo, ctx.Pull.Num)
 	if err != nil {
-		ctx.log.Err("pull request data api call failed: %v", err)
+		ctx.Log.Err("pull request data api call failed: %v", err)
 		return
 	}
 	if err := s.requestParser.extractPullData(pull, ctx); err != nil {
-		ctx.log.Err("failed to extract required fields from comment data: %v", err)
+		ctx.Log.Err("failed to extract required fields from comment data: %v", err)
 		return
 	}
 
-	switch ctx.command.commandType {
+	switch ctx.Command.commandType {
 	case Plan:
-		s.planExecutor.Exec(s.planExecutor.execute, ctx, s.githubClient)
+		s.planExecutor.execute(ctx, s.githubClient)
 	case Apply:
-		s.applyExecutor.Exec(s.applyExecutor.execute, ctx, s.githubClient)
+		s.applyExecutor.execute(ctx, s.githubClient)
 	case Help:
 		s.helpExecutor.execute(ctx, s.githubClient)
 	default:
-		ctx.log.Err("failed to determine desired command, neither plan nor apply")
+		ctx.Log.Err("failed to determine desired command, neither plan nor apply")
 	}
 }
 
@@ -346,11 +375,10 @@ func (s *Server) isPullClosedEvent(event github.PullRequestEvent) bool {
 }
 
 // recover logs and creates a comment on the pull request for panics
-func (s *Server) recover(ctx *ExecutionContext) {
+func (s *Server) recover(ctx *CommandContext) {
 	if err := recover(); err != nil {
-		ghCtx := s.planExecutor.githubContext(ctx) // this won't have every field but it has the ones needed by CreateComment
 		stack := recovery.Stack(3)
-		s.githubClient.CreateComment(ghCtx, fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack))
-		ctx.log.Err("PANIC: %s\n%s", err, stack)
+		s.githubClient.CreateComment(ctx, fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack))
+		ctx.Log.Err("PANIC: %s\n%s", err, stack)
 	}
 }

server/terraform_client.go

@@ -2,52 +2,55 @@ package server
 
 import (
 	"fmt"
+	"github.com/hootsuite/atlantis/logging"
+	"github.com/hootsuite/atlantis/models"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"regexp"
-	"github.com/hootsuite/atlantis/logging"
 )
 
 type TerraformClient struct {
 	tfExecutableName string
 }
 
-func (t *TerraformClient) ConfigureRemoteState(log *logging.SimpleLogger, execPath ExecutionPath, tfEnvName string, sshKey string) (string, error) {
-	log.Info("setting up remote state in directory %q", execPath.Absolute)
+func (t *TerraformClient) ConfigureRemoteState(log *logging.SimpleLogger, repoDir string, project models.Project, env string, sshKey string) (string, error) {
+	absolutePath := filepath.Join(repoDir, project.Path)
+	log.Info("setting up remote state in directory %q", absolutePath)
 	var remoteSetupCmdArgs []string
 
 	// Check if setup file exists
-	setupFileInfo, err := os.Stat(filepath.Join(execPath.Absolute, "setup.sh"))
+	setupFileInfo, err := os.Stat(filepath.Join(absolutePath, "setup.sh"))
 	if err != nil {
-		return "", fmt.Errorf("setup.sh file doesn't exist in terraform plan path %q: %v", execPath.Relative, err)
+		return "", fmt.Errorf("setup.sh file doesn't exist in terraform plan path %q: %v", project.Path, err)
 	}
 	// Check if setup file is executed
 	if setupFileInfo.Mode() != os.FileMode(0755) {
 		return "", fmt.Errorf("setup file isn't executable, required permissions are 0755")
 	}
 	// Check if environment is specified
-	if tfEnvName == "" {
+	if env == "" {
 		// Check if env/ folder exist and environment isn't specified
-		if _, err := os.Stat(filepath.Join(execPath.Absolute, "env")); err == nil {
+		// todo: make env a constant (environmentDirName)
+		if _, err := os.Stat(filepath.Join(absolutePath, "env")); err == nil {
 			log.Info("environment directory exists but no environment was supplied")
 			return "", nil
 		}
 	} else {
 		// Check if the environment file exists
-		envFile := tfEnvName + ".tfvars"
-		envPath := filepath.Join(execPath.Absolute, "env")
+		envFile := env + ".tfvars"
+		envPath := filepath.Join(absolutePath, "env")
 		if _, err := os.Stat(filepath.Join(envPath, envFile)); err != nil {
 			return "", fmt.Errorf("environment file %q not found in %q", envFile, envPath)
 		}
 	}
 
 	// Set environment file parameter for ./setup.sh
-	if tfEnvName != "" {
-		remoteSetupCmdArgs = append(remoteSetupCmdArgs, "-e", tfEnvName)
+	if env != "" {
+		remoteSetupCmdArgs = append(remoteSetupCmdArgs, "-e", env)
 	}
 	remoteSetupCmd := exec.Command("./setup.sh", remoteSetupCmdArgs...)
-	remoteSetupCmd.Dir = execPath.Absolute
+	remoteSetupCmd.Dir = absolutePath
 	// Check if ssh key is set
 	if sshKey != "" {
 		// Fixing a bug when git isn't found in path when environment variables are set for command
@@ -71,9 +74,9 @@ func (t *TerraformClient) ConfigureRemoteState(log *logging.SimpleLogger, execPa
 	return remoteStatePath, nil
 }
 
-func (t *TerraformClient) RunTerraformCommand(path ExecutionPath, tfPlanCmd []string, tfEnvVars []string) ([]string, string, error) {
+func (t *TerraformClient) RunTerraformCommand(path string, tfPlanCmd []string, tfEnvVars []string) ([]string, string, error) {
 	terraformCmd := exec.Command(t.tfExecutableName, tfPlanCmd...)
-	terraformCmd.Dir = path.Absolute
+	terraformCmd.Dir = path
 	terraformCmd.Env = tfEnvVars
 	out, err := terraformCmd.CombinedOutput()
 	output := string(out)

server/testing_util.go

@@ -1,38 +0,0 @@
-package server
-
-// taken from https://github.com/benbjohnson/testing
-
-import (
-	"fmt"
-	"path/filepath"
-	"reflect"
-	"runtime"
-	"testing"
-)
-
-// assert fails the test if the condition is false.
-func assert(tb testing.TB, condition bool, msg string, v ...interface{}) {
-	if !condition {
-		_, file, line, _ := runtime.Caller(1)
-		fmt.Printf("\033[31m%s:%d: "+msg+"\033[39m\n\n", append([]interface{}{filepath.Base(file), line}, v...)...)
-		tb.FailNow()
-	}
-}
-
-// ok fails the test if an err is not nil.
-func ok(tb testing.TB, err error) {
-	if err != nil {
-		_, file, line, _ := runtime.Caller(1)
-		fmt.Printf("\033[31m%s:%d: unexpected error: %s\033[39m\n\n", filepath.Base(file), line, err.Error())
-		tb.FailNow()
-	}
-}
-
-// equals fails the test if exp is not equal to act.
-func equals(tb testing.TB, exp, act interface{}) {
-	if !reflect.DeepEqual(exp, act) {
-		_, file, line, _ := runtime.Caller(1)
-		fmt.Printf("\033[31m%s:%d:\n\n\texp: %#v\n\n\tgot: %#v\033[39m\n\n", filepath.Base(file), line, exp, act)
-		tb.FailNow()
-	}
-}

server/util.go

@@ -1,11 +0,0 @@
-package server
-
-// ContainsStr returns true if slice has a value equal to search and false otherwise
-func ContainsStr(slice []string, search string) bool {
-	for _, v := range slice {
-		if v == search {
-			return true
-		}
-	}
-	return false
-}

server/web_templates.go

@@ -16,12 +16,12 @@ var indexTemplate = template.Must(template.New("index.html.tmpl").Parse(`
   <link rel="stylesheet" href="/static/css/normalize.css">
   <link rel="stylesheet" href="/static/css/skeleton.css">
   <link rel="stylesheet" href="/static/css/custom.css">
-  <link rel="icon" type="image/png" href="/static/atlantis-icon.png">
+  <link rel="icon" type="image/png" href="/static/images/atlantis-icon.png">
 </head>
 <body>
 <div class="container">
   <section class="header">
-    <a title="atlantis" href="/"><img src="/static/atlantis-icon.png"/></a>
+    <a title="atlantis" href="/"><img src="/static/images/atlantis-icon.png"/></a>
     <p style="font-family: monospace, monospace; font-size: 1.1em; text-align: center;">atlantis</p>
   </section>
   <nav class="navbar">
@@ -38,7 +38,7 @@ var indexTemplate = template.Must(template.New("index.html.tmpl").Parse(`
         <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="{{.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 class="list-timestamp"><span class="heading-font-size">{{.Time}}</span></div>
       </div>
     {{ end }}
     {{ else }}

testing_util/testing_util.go

@@ -38,13 +38,13 @@ func Equals(tb testing.TB, exp, act interface{}) {
 }
 
 // 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 {
+func Contains(tb testing.TB, exp interface{}, slice []interface{}) {
+	for _, v := range slice {
+		if reflect.DeepEqual(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)
+	fmt.Printf("\033[31m%s:%d:\n\n\texp: %#v\n\n\twas not in: %#v\033[39m\n\n", filepath.Base(file), line, exp, slice)
 	tb.FailNow()
 }