Fixes #1006: Keep track of project status even if plans have been deleted

server/events/command_runner.go

@@ -97,7 +97,7 @@ type DefaultCommandRunner struct {
 	GlobalAutomerge   bool
 	PendingPlanFinder PendingPlanFinder
 	WorkingDir        WorkingDir
-	DB                *db.BoltDB
+	DB                db.BoltDB
 }
 
 // RunAutoplanCommand runs plan when a pull request is opened or updated.

server/events/command_runner_test.go

@@ -24,6 +24,8 @@ import (
 	"github.com/google/go-github/v28/github"
 	. "github.com/petergtz/pegomock"
 	"github.com/runatlantis/atlantis/server/events"
+	dbmocks "github.com/runatlantis/atlantis/server/events/db/mocks"
+	dbmatchers "github.com/runatlantis/atlantis/server/events/db/mocks/matchers"
 	"github.com/runatlantis/atlantis/server/events/mocks"
 	"github.com/runatlantis/atlantis/server/events/mocks/matchers"
 	"github.com/runatlantis/atlantis/server/events/models"
@@ -43,6 +45,7 @@ var ch events.DefaultCommandRunner
 var pullLogger *logging.SimpleLogger
 var workingDir events.WorkingDir
 var pendingPlanFinder *mocks.MockPendingPlanFinder
+var boltDB *dbmocks.MockBoltDB
 
 func setup(t *testing.T) *vcsmocks.MockClient {
 	RegisterMockTestingT(t)
@@ -57,6 +60,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 	projectCommandRunner = mocks.NewMockProjectCommandRunner()
 	workingDir = mocks.NewMockWorkingDir()
 	pendingPlanFinder = mocks.NewMockPendingPlanFinder()
+	boltDB = dbmocks.NewMockBoltDB()
 	When(logger.GetLevel()).ThenReturn(logging.Info)
 	When(logger.NewLogger("runatlantis/atlantis#1", true, logging.Info)).
 		ThenReturn(pullLogger)
@@ -76,6 +80,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		PendingPlanFinder:        pendingPlanFinder,
 		WorkingDir:               workingDir,
 		DisableApplyAll:          false,
+		DB:                       boltDB,
 	}
 	return vcsClient
 }
@@ -234,3 +239,53 @@ func TestRunAutoplanCommand_DeletePlans(t *testing.T) {
 	ch.RunAutoplanCommand(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User)
 	pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
 }
+
+func TestApplyWithAutoMerge_VSCMerge(t *testing.T) {
+	t.Log("if \"atlantis apply\" is run with automerge and at least one project" +
+		" has a discarded plan, automerge should not take place")
+
+	vcsClient := setup(t)
+	pull := &github.PullRequest{
+		State: github.String("open"),
+	}
+	modelPull := models.PullRequest{State: models.OpenPullState}
+	When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(pull, nil)
+	When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
+	ch.GlobalAutomerge = true
+
+	ch.RunCommentCommand(fixtures.GithubRepo, &fixtures.GithubRepo, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.ApplyCommand})
+	vcsClient.VerifyWasCalledOnce().MergePull(modelPull)
+}
+
+func TestApplyWithAutoMerge_DiscardedPlan(t *testing.T) {
+	t.Log("if \"atlantis apply\" is run with automerge and at least one project" +
+		" has a discarded plan, automerge should not take place")
+
+	setup(t)
+	pull := &github.PullRequest{
+		State: github.String("open"),
+	}
+	modelPull := models.PullRequest{State: models.OpenPullState}
+	When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(pull, nil)
+	When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
+	ch.GlobalAutomerge = true
+
+	ch.RunCommentCommand(fixtures.GithubRepo, &fixtures.GithubRepo, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.ApplyCommand})
+	projectStatuses := []models.ProjectStatus{
+		{
+			RepoRelDir:  ".",
+			Workspace:   "default",
+			ProjectName: "automerge-test",
+			Status:      models.DiscardedPlanStatus,
+		},
+	}
+	pullStatus := models.PullStatus{
+		Projects: projectStatuses,
+		Pull:     modelPull,
+	}
+	//When(boltDB.UpdatePullWithResults(modelPull, nil)).ThenReturn(pullStatus, nil)
+	When(boltDB.UpdatePullWithResults(dbmatchers.EqModelsPullRequest(modelPull), dbmatchers.EqSliceOfModelsProjectResult(nil))).ThenReturn(pullStatus, nil)
+	// TODO: stubbing here doesn't seem to work? pullStatus defined here is not actually returned and so I cannot uncomment the next two lines which would verify our scenario here
+	//vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, "not automerging because project at dir %q, workspace %q has status %q")
+	//VerifyWasCalled(Never()).MergePull(modelPull)
+}

server/events/db/boltdb.go

@@ -15,8 +15,17 @@ import (
 	bolt "go.etcd.io/bbolt"
 )
 
-// BoltDB is a database using BoltDB
-type BoltDB struct {
+//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_boltdb.go BoltDB
+
+// BoltDB interface defines the set of methods the DB implements. Use this to allow DB mocking when testing
+type BoltDB interface {
+	UpdatePullWithResults(pull models.PullRequest, newResults []models.ProjectResult) (models.PullStatus, error)
+	DeletePullStatus(pull models.PullRequest) error
+	UpdateProjectStatus(pull models.PullRequest, workspace string, repoRelDir string, targetStatus models.ProjectPlanStatus) error
+}
+
+// DefaultBoltDB is a database using BoltDB
+type DefaultBoltDB struct {
 	db              *bolt.DB
 	locksBucketName []byte
 	pullsBucketName []byte
@@ -30,7 +39,7 @@ const (
 
 // New returns a valid locker. We need to be able to write to dataDir
 // since bolt stores its data as a file
-func New(dataDir string) (*BoltDB, error) {
+func New(dataDir string) (*DefaultBoltDB, error) {
 	if err := os.MkdirAll(dataDir, 0700); err != nil {
 		return nil, errors.Wrap(err, "creating data dir")
 	}
@@ -56,19 +65,19 @@ func New(dataDir string) (*BoltDB, error) {
 		return nil, errors.Wrap(err, "starting BoltDB")
 	}
 	// todo: close BoltDB when server is sigtermed
-	return &BoltDB{db: db, locksBucketName: []byte(locksBucketName), pullsBucketName: []byte(pullsBucketName)}, nil
+	return &DefaultBoltDB{db: db, locksBucketName: []byte(locksBucketName), pullsBucketName: []byte(pullsBucketName)}, nil
 }
 
 // NewWithDB is used for testing.
-func NewWithDB(db *bolt.DB, bucket string) (*BoltDB, error) {
-	return &BoltDB{db: db, locksBucketName: []byte(bucket), pullsBucketName: []byte(pullsBucketName)}, nil
+func NewWithDB(db *bolt.DB, bucket string) (*DefaultBoltDB, error) {
+	return &DefaultBoltDB{db: db, locksBucketName: []byte(bucket), pullsBucketName: []byte(pullsBucketName)}, nil
 }
 
 // TryLock attempts to create a new lock. If the lock is
 // acquired, it will return true and the lock returned will be newLock.
 // If the lock is not acquired, it will return false and the current
 // lock that is preventing this lock from being acquired.
-func (b *BoltDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock, error) {
+func (b *DefaultBoltDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock, error) {
 	var lockAcquired bool
 	var currLock models.ProjectLock
 	key := b.lockKey(newLock.Project, newLock.Workspace)
@@ -105,7 +114,7 @@ func (b *BoltDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock,
 // If there is no lock, then it will return a nil pointer.
 // If there is a lock, then it will delete it, and then return a pointer
 // to the deleted lock.
-func (b *BoltDB) Unlock(p models.Project, workspace string) (*models.ProjectLock, error) {
+func (b *DefaultBoltDB) Unlock(p models.Project, workspace string) (*models.ProjectLock, error) {
 	var lock models.ProjectLock
 	foundLock := false
 	key := b.lockKey(p, workspace)
@@ -128,7 +137,7 @@ func (b *BoltDB) Unlock(p models.Project, workspace string) (*models.ProjectLock
 }
 
 // List lists all current locks.
-func (b *BoltDB) List() ([]models.ProjectLock, error) {
+func (b *DefaultBoltDB) List() ([]models.ProjectLock, error) {
 	var locks []models.ProjectLock
 	var locksBytes [][]byte
 	err := b.db.View(func(tx *bolt.Tx) error {
@@ -156,7 +165,7 @@ func (b *BoltDB) List() ([]models.ProjectLock, error) {
 }
 
 // UnlockByPull deletes all locks associated with that pull request and returns them.
-func (b *BoltDB) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
+func (b *DefaultBoltDB) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
 	var locks []models.ProjectLock
 	err := b.db.View(func(tx *bolt.Tx) error {
 		c := tx.Bucket(b.locksBucketName).Cursor()
@@ -188,7 +197,7 @@ func (b *BoltDB) UnlockByPull(repoFullName string, pullNum int) ([]models.Projec
 
 // GetLock returns a pointer to the lock for that project and workspace.
 // If there is no lock, it returns a nil pointer.
-func (b *BoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLock, error) {
+func (b *DefaultBoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLock, error) {
 	key := b.lockKey(p, workspace)
 	var lockBytes []byte
 	err := b.db.View(func(tx *bolt.Tx) error {
@@ -216,7 +225,7 @@ func (b *BoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLoc
 
 // UpdatePullWithResults updates pull's status with the latest project results.
 // It returns the new PullStatus object.
-func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []models.ProjectResult) (models.PullStatus, error) {
+func (b *DefaultBoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []models.ProjectResult) (models.PullStatus, error) {
 	key, err := b.pullKey(pull)
 	if err != nil {
 		return models.PullStatus{}, err
@@ -281,7 +290,7 @@ func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []mod
 
 // GetPullStatus returns the status for pull.
 // If there is no status, returns a nil pointer.
-func (b *BoltDB) GetPullStatus(pull models.PullRequest) (*models.PullStatus, error) {
+func (b *DefaultBoltDB) GetPullStatus(pull models.PullRequest) (*models.PullStatus, error) {
 	key, err := b.pullKey(pull)
 	if err != nil {
 		return nil, err
@@ -297,7 +306,7 @@ func (b *BoltDB) GetPullStatus(pull models.PullRequest) (*models.PullStatus, err
 }
 
 // DeletePullStatus deletes the status for pull.
-func (b *BoltDB) DeletePullStatus(pull models.PullRequest) error {
+func (b *DefaultBoltDB) DeletePullStatus(pull models.PullRequest) error {
 	key, err := b.pullKey(pull)
 	if err != nil {
 		return err
@@ -309,9 +318,9 @@ func (b *BoltDB) DeletePullStatus(pull models.PullRequest) error {
 	return errors.Wrap(err, "DB transaction failed")
 }
 
-// DeleteProjectStatus deletes all project statuses under pull that match
+// UpdateProjectStatus updates all project statuses under pull that match
 // workspace and repoRelDir.
-func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string, repoRelDir string) error {
+func (b *DefaultBoltDB) UpdateProjectStatus(pull models.PullRequest, workspace string, repoRelDir string, targetStatus models.ProjectPlanStatus) error {
 	key, err := b.pullKey(pull)
 	if err != nil {
 		return err
@@ -332,7 +341,7 @@ func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string,
 		var newProjects []models.ProjectStatus
 		for _, p := range currStatus.Projects {
 			if p.Workspace == workspace && p.RepoRelDir == repoRelDir {
-				continue
+				p.Status = targetStatus
 			}
 			newProjects = append(newProjects, p)
 		}
@@ -344,7 +353,7 @@ func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string,
 	return errors.Wrap(err, "DB transaction failed")
 }
 
-func (b *BoltDB) pullKey(pull models.PullRequest) ([]byte, error) {
+func (b *DefaultBoltDB) pullKey(pull models.PullRequest) ([]byte, error) {
 	hostname := pull.BaseRepo.VCSHost.Hostname
 	if strings.Contains(hostname, pullKeySeparator) {
 		return nil, fmt.Errorf("vcs hostname %q contains illegal string %q", hostname, pullKeySeparator)
@@ -358,11 +367,11 @@ func (b *BoltDB) pullKey(pull models.PullRequest) ([]byte, error) {
 		nil
 }
 
-func (b *BoltDB) lockKey(p models.Project, workspace string) string {
+func (b *DefaultBoltDB) lockKey(p models.Project, workspace string) string {
 	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace)
 }
 
-func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) {
+func (b *DefaultBoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) {
 	serialized := bucket.Get(key)
 	if serialized == nil {
 		return nil, nil
@@ -375,15 +384,15 @@ func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.Pul
 	return &p, nil
 }
 
-func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.PullStatus) error {
+func (b *DefaultBoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.PullStatus) error {
 	serialized, err := json.Marshal(pull)
 	if err != nil {
 		return errors.Wrap(err, "serializing")
 	}
 	return bucket.Put(key, serialized)
 }
 
-func (b *BoltDB) projectResultToProject(p models.ProjectResult) models.ProjectStatus {
+func (b *DefaultBoltDB) projectResultToProject(p models.ProjectResult) models.ProjectStatus {
 	return models.ProjectStatus{
 		Workspace:   p.Workspace,
 		RepoRelDir:  p.RepoRelDir,

server/events/db/boltdb_test.go

@@ -449,10 +449,10 @@ func TestPullStatus_UpdateDeleteGet(t *testing.T) {
 	Assert(t, maybeStatus == nil, "exp nil")
 }
 
-// Test we can create a status, delete a specific project's status within that
+// Test we can create a status, update a specific project's status within that
 // pull status, and when we get all the project statuses, that specific project
-// should not be there.
-func TestPullStatus_UpdateDeleteProject(t *testing.T) {
+// should be updated.
+func TestPullStatus_UpdateProject(t *testing.T) {
 	b, cleanup := newTestDB2(t)
 	defer cleanup()
 
@@ -492,14 +492,20 @@ func TestPullStatus_UpdateDeleteProject(t *testing.T) {
 		})
 	Ok(t, err)
 
-	err = b.DeleteProjectStatus(pull, "default", ".")
+	err = b.UpdateProjectStatus(pull, "default", ".", models.DiscardedPlanStatus)
 	Ok(t, err)
 
 	status, err := b.GetPullStatus(pull)
 	Ok(t, err)
 	Assert(t, status != nil, "exp non-nil")
 	Equals(t, pull, status.Pull) // nolint: staticcheck
 	Equals(t, []models.ProjectStatus{
+		{
+			Workspace:   "default",
+			RepoRelDir:  ".",
+			ProjectName: "",
+			Status:      models.DiscardedPlanStatus,
+		},
 		{
 			Workspace:   "staging",
 			RepoRelDir:  ".",
@@ -686,7 +692,7 @@ func TestPullStatus_UpdateMerge(t *testing.T) {
 }
 
 // newTestDB returns a TestDB using a temporary path.
-func newTestDB() (*bolt.DB, *db.BoltDB) {
+func newTestDB() (*bolt.DB, *db.DefaultBoltDB) {
 	// Retrieve a temporary path.
 	f, err := ioutil.TempFile("", "")
 	if err != nil {
@@ -712,7 +718,7 @@ func newTestDB() (*bolt.DB, *db.BoltDB) {
 	return boltDB, b
 }
 
-func newTestDB2(t *testing.T) (*db.BoltDB, func()) {
+func newTestDB2(t *testing.T) (*db.DefaultBoltDB, func()) {
 	tmp, cleanup := TempDir(t)
 	boltDB, err := db.New(tmp)
 	Ok(t, err)