Delete plan on successful apply (runatlantis#46)

* Return locks when they're deleted

* Implement DeletePlan and DeletePlanByPull

* Clean up data from pull request on close

* Delete plan on successful apply

locking/boltdb/boltdb.go

@@ -81,13 +81,26 @@ func (b *Backend) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock,
 	return lockAcquired, currLock, nil
 }
 
-func (b Backend) Unlock(p models.Project, env string) error {
+func (b Backend) Unlock(p models.Project, env string) (*models.ProjectLock, error) {
+	var lock models.ProjectLock
+	foundLock := false
 	key := b.key(p, env)
 	err := b.db.Update(func(tx *bolt.Tx) error {
-		locks := tx.Bucket(b.bucket)
-		return locks.Delete([]byte(key))
+		bucket := tx.Bucket(b.bucket)
+		serialized := bucket.Get([]byte(key))
+		if serialized != nil {
+			if err := json.Unmarshal(serialized, &lock); err != nil {
+				return errors.Wrap(err, "failed to deserialize lock")
+			}
+			foundLock = true
+		}
+		return bucket.Delete([]byte(key))
 	})
-	return errors.Wrap(err, "DB transaction failed")
+	err = errors.Wrap(err, "DB transaction failed")
+	if foundLock {
+		return &lock, err
+	}
+	return nil, err
 }
 
 func (b Backend) List() ([]models.ProjectLock, error) {
@@ -117,8 +130,7 @@ func (b Backend) List() ([]models.ProjectLock, error) {
 	return locks, nil
 }
 
-func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
-	// get the locks that match that pull request
+func (b Backend) 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.bucket).Cursor()
@@ -138,11 +150,11 @@ func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
 
 	// 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)
+		if _, err = b.Unlock(lock.Project, lock.Env); err != nil {
+			return locks, errors.Wrapf(err, "unlocking repo %s, path %s, env %s", lock.Project.RepoFullName, lock.Project.Path, lock.Env)
 		}
 	}
-	return nil
+	return locks, nil
 }
 
 func (b Backend) key(p models.Project, env string) string {

locking/dynamodb/dynamodb.go

@@ -95,16 +95,27 @@ func (b Backend) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock,
 	return true, newLock, nil
 }
 
-func (b Backend) Unlock(project models.Project, env string) error {
+func (b Backend) Unlock(project models.Project, env string) (*models.ProjectLock, error) {
 	key := b.key(project, env)
 	params := &dynamodb.DeleteItemInput{
 		Key: map[string]*dynamodb.AttributeValue{
 			"LockKey": {S: aws.String(key)},
 		},
 		TableName: aws.String(b.LockTable),
+		ReturnValues: aws.String("ALL_OLD"),
 	}
-	_, err := b.DB.DeleteItem(params)
-	return errors.Wrap(err, "deleting lock")
+	output, err := b.DB.DeleteItem(params)
+	if err != nil {
+		return nil, errors.Wrap(err, "deleting lock")
+	}
+
+	// deserialize the lock so we can return it
+	var dLock dynamoLock
+	if err := dynamodbattribute.UnmarshalMap(output.Attributes, &dLock); err != nil {
+		return nil, errors.Wrap(err, "found an existing lock at that key but it could not be deserialized. We suggest manually deleting this key from DynamoDB")
+	}
+	lock := b.fromDynamo(dLock)
+	return &lock, nil
 }
 
 func (b Backend) List() ([]models.ProjectLock, error) {
@@ -131,7 +142,7 @@ func (b Backend) List() ([]models.ProjectLock, error) {
 	return locks, errors.Wrap(err, "scanning dynamodb")
 }
 
-func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
+func (b Backend) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
 	params := &dynamodb.ScanInput{
 		ExpressionAttributeValues: map[string]*dynamodb.AttributeValue{
 			":pullNum": {
@@ -146,10 +157,11 @@ func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
 	}
 
 	// scan DynamoDB for locks that match the pull request
-	var locks []dynamoLock
+	var dLocks []dynamoLock
+	var locks []models.ProjectLock
 	var err, internalErr error
 	err = b.DB.ScanPages(params, func(out *dynamodb.ScanOutput, lastPage bool) bool {
-		if err := dynamodbattribute.UnmarshalListOfMaps(out.Items, &locks); err != nil {
+		if err := dynamodbattribute.UnmarshalListOfMaps(out.Items, &dLocks); err != nil {
 			internalErr = errors.Wrap(err, "deserializing locks")
 			return false
 		}
@@ -159,16 +171,17 @@ func (b Backend) UnlockByPull(repoFullName string, pullNum int) error {
 		err = internalErr
 	}
 	if err != nil {
-		return errors.Wrap(err, "scanning dynamodb")
+		return locks, errors.Wrap(err, "scanning dynamodb")
 	}
 
 	// 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)
+	for _, lock := range dLocks {
+		if _, err := b.Unlock(models.NewProject(lock.RepoFullName, lock.Path), lock.Env); err != nil {
+			return locks, errors.Wrapf(err, "unlocking repo %s, path %s, env %s", lock.RepoFullName, lock.Path, lock.Env)
 		}
+		locks = append(locks, b.fromDynamo(lock))
 	}
-	return nil
+	return locks, nil
 }
 
 func (b Backend) toDynamo(key string, l models.ProjectLock) dynamoLock {

locking/locking.go

@@ -10,9 +10,9 @@ import (
 
 type Backend interface {
 	TryLock(lock models.ProjectLock) (bool, models.ProjectLock, error)
-	Unlock(project models.Project, env string) error
+	Unlock(project models.Project, env string) (*models.ProjectLock, error)
 	List() ([]models.ProjectLock, error)
-	UnlockByPull(repoFullName string, pullNum int) error
+	UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error)
 }
 
 type TryLockResponse struct {
@@ -49,10 +49,10 @@ func (c *Client) TryLock(p models.Project, env string, pull models.PullRequest,
 	return TryLockResponse{lockAcquired, currLock, c.key(p, env)}, nil
 }
 
-func (c *Client) Unlock(key string) error {
+func (c *Client) Unlock(key string) (*models.ProjectLock, error) {
 	matches := keyRegex.FindStringSubmatch(key)
 	if len(matches) != 4 {
-		return errors.New("invalid key format")
+		return nil, errors.New("invalid key format")
 	}
 	return c.backend.Unlock(models.Project{matches[1], matches[2]}, matches[3])
 }
@@ -69,7 +69,7 @@ func (c *Client) List() (map[string]models.ProjectLock, error) {
 	return m, nil
 }
 
-func (c *Client) UnlockByPull(repoFullName string, pullNum int) error {
+func (c *Client) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
 	return c.backend.UnlockByPull(repoFullName, pullNum)
 }
 

plan/backend.go

@@ -7,6 +7,8 @@ import (
 type Backend interface {
 	SavePlan(path string, project models.Project, env string, pullNum int) error
 	CopyPlans(dstRepoPath string, repoFullName string, env string, pullNum int) ([]Plan, error)
+	DeletePlan(project models.Project, env string, pullNum int) error
+	DeletePlansByPull(repoFullName string, pullNum int) error
 }
 
 type Plan struct {

plan/file/file.go

@@ -15,8 +15,10 @@ type Backend struct {
 	baseDir string
 }
 
+const planPath = "plans"
+
 func New(baseDir string) (*Backend, error) {
-	baseDir = filepath.Clean(baseDir)
+	baseDir = filepath.Join(filepath.Clean(baseDir), planPath)
 	if err := os.MkdirAll(baseDir, 0755); err != nil {
 		return nil, err
 	}
@@ -25,11 +27,11 @@ func New(baseDir string) (*Backend, error) {
 
 // save plans to baseDir/owner/repo/pullNum/path/env.tfplan
 func (b *Backend) SavePlan(path string, project models.Project, env string, pullNum int) error {
-	savePath := b.path(project, pullNum)
-	if err := os.MkdirAll(savePath, 0755); err != nil {
+	file := b.path(project, env, pullNum)
+	if err := os.MkdirAll(filepath.Dir(file), 0755); err != nil {
 		return errors.Wrap(err, "creating save directory")
 	}
-	if err := b.copy(path, filepath.Join(savePath, env+".tfplan")); err != nil {
+	if err := b.copy(path, file); err != nil {
 		return errors.Wrap(err, "saving plan")
 	}
 	return nil
@@ -76,6 +78,14 @@ func (b *Backend) CopyPlans(dstRepo string, repoFullName string, env string, pul
 	return plans, nil
 }
 
+func (b *Backend) DeletePlan(project models.Project, env string, pullNum int) error {
+	return os.Remove(b.path(project, env, pullNum))
+}
+
+func (b *Backend) DeletePlansByPull(repoFullName string, pullNum int) error {
+	return os.RemoveAll(filepath.Join(b.baseDir, repoFullName, strconv.Itoa(pullNum)))
+}
+
 func (b *Backend) copy(src string, dst string) error {
 	data, err := ioutil.ReadFile(src)
 	if err != nil {
@@ -88,6 +98,6 @@ func (b *Backend) copy(src string, dst string) error {
 	return nil
 }
 
-func (b *Backend) path(p models.Project, pullNum int) string {
-	return filepath.Join(b.baseDir, p.RepoFullName, strconv.Itoa(pullNum), p.Path)
+func (b *Backend) path(p models.Project, env string, pullNum int) string {
+	return filepath.Join(b.baseDir, p.RepoFullName, strconv.Itoa(pullNum), p.Path, env+".tfplan")
 }

plan/s3/s3.go

@@ -87,7 +87,7 @@ func (b *Backend) SavePlan(path string, project models.Project, env string, pull
 		return errors.Wrapf(err, "opening plan at %s", path)
 	}
 
-	key := pathutil.Join(b.keyPrefix, project.RepoFullName, strconv.Itoa(pullNum), project.Path, env+".tfplan")
+	key := b.path(project, env, pullNum)
 	_, err = b.uploader.Upload(&s3manager.UploadInput{
 		Bucket: aws.String(b.bucket),
 		Key:    &key,
@@ -104,3 +104,35 @@ func (b *Backend) SavePlan(path string, project models.Project, env string, pull
 	}
 	return nil
 }
+
+func (b *Backend) DeletePlan(project models.Project, env string, pullNum int) error {
+	_, err := b.s3.DeleteObject(&s3.DeleteObjectInput{
+		Bucket: aws.String(b.bucket),
+		Key: aws.String(b.path(project, env, pullNum)),
+	})
+	return err
+}
+
+func (b *Backend) DeletePlansByPull(repoFullName string, pullNum int) error {
+	// first list the plans with the correct prefix
+	prefix := pathutil.Join(b.keyPrefix, repoFullName, strconv.Itoa(pullNum))
+	list, err := b.s3.ListObjects(&s3.ListObjectsInput{Bucket: aws.String(b.bucket), Prefix: &prefix})
+	if err != nil {
+		return errors.Wrap(err, "listing plans")
+	}
+
+	var deleteList []*s3.ObjectIdentifier
+	for _, obj := range list.Contents {
+		deleteList = append(deleteList, &s3.ObjectIdentifier{Key: obj.Key})
+	}
+
+	_, err = b.s3.DeleteObjects(&s3.DeleteObjectsInput{
+		Bucket: aws.String(b.bucket),
+		Delete: &s3.Delete{Objects: deleteList},
+	})
+	return err
+}
+
+func (b *Backend) path(project models.Project, env string, pullNum int) string {
+	return pathutil.Join(b.keyPrefix, project.RepoFullName, strconv.Itoa(pullNum), project.Path, env+".tfplan")
+}

server/apply_executor.go

@@ -24,7 +24,7 @@ type ApplyExecutor struct {
 	githubCommentRenderer *GithubCommentRenderer
 	lockingClient         *locking.Client
 	requireApproval       bool
-	planStorage           plan.Backend
+	planBackend           plan.Backend
 }
 
 /** Result Types **/
@@ -63,7 +63,7 @@ func (a *ApplyExecutor) execute(ctx *CommandContext, github *GithubClient) {
 	res := a.setupAndApply(ctx)
 	res.Command = Apply
 	comment := a.githubCommentRenderer.render(res, ctx.Log.History.String(), ctx.Command.verbose)
-	github.CreateComment(ctx, comment)
+	github.CreateComment(ctx.Repo, ctx.Pull, comment)
 }
 
 func (a *ApplyExecutor) setupAndApply(ctx *CommandContext) ExecutionResult {
@@ -73,7 +73,7 @@ func (a *ApplyExecutor) setupAndApply(ctx *CommandContext) ExecutionResult {
 
 	// todo: reclone repo and switch branch, don't assume it's already there
 	repoDir := filepath.Join(a.scratchDir, ctx.Repo.FullName, strconv.Itoa(ctx.Pull.Num))
-	plans, err := a.planStorage.CopyPlans(repoDir, ctx.Repo.FullName, ctx.Command.environment, ctx.Pull.Num)
+	plans, err := a.planBackend.CopyPlans(repoDir, ctx.Repo.FullName, ctx.Command.environment, ctx.Pull.Num)
 	if err != nil {
 		errMsg := fmt.Sprintf("failed to get plans: %s", err)
 		ctx.Log.Err(errMsg)
@@ -209,6 +209,9 @@ func (a *ApplyExecutor) apply(ctx *CommandContext, repoDir string, plan plan.Pla
 
 	// clean up, delete local plan file
 	os.Remove(plan.LocalPath) // swallow errors, okay if we failed to delete
+	if err := a.planBackend.DeletePlan(plan.Project, ctx.Command.environment, ctx.Pull.Num); err != nil {
+		ctx.Log.Err("deleting plan for repo %s, path %s, env %s: %s", plan.Project.RepoFullName, plan.Project.Path, ctx.Command.environment, err)
+	}
 	return PathResult{
 		Status: Success,
 		Result: ApplySuccess{output},

server/command_handler.go

@@ -81,7 +81,7 @@ func (s *CommandHandler) SetDeleteLockURL(f func(id string) (url string)) {
 func (s *CommandHandler) recover(ctx *CommandContext) {
 	if err := recover(); err != nil {
 		stack := recovery.Stack(3)
-		s.githubClient.CreateComment(ctx, fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack))
+		s.githubClient.CreateComment(ctx.Repo, ctx.Pull, 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/event_parser.go

@@ -49,21 +49,9 @@ func (e *EventParser) DetermineCommand(comment *github.IssueCommentEvent) (*Comm
 }
 
 func (e *EventParser) ExtractCommentData(comment *github.IssueCommentEvent, ctx *CommandContext) error {
-	repoFullName := comment.Repo.GetFullName()
-	if repoFullName == "" {
-		return errors.New("repository.full_name is null")
-	}
-	repoOwner := comment.Repo.Owner.GetLogin()
-	if repoOwner == "" {
-		return errors.New("repository.owner.login is null")
-	}
-	repoName := comment.Repo.GetName()
-	if repoName == "" {
-		return errors.New("repository.name is null")
-	}
-	repoSSHURL := comment.Repo.GetSSHURL()
-	if repoSSHURL == "" {
-		return errors.New("comment.repository.ssh_url is null")
+	repo, err := e.ExtractRepoData(comment.Repo)
+	if err != nil {
+		return err
 	}
 	pullNum := comment.Issue.GetNumber()
 	if pullNum == 0 {
@@ -81,12 +69,7 @@ func (e *EventParser) ExtractCommentData(comment *github.IssueCommentEvent, ctx
 	if htmlURL == "" {
 		return errors.New("comment.issue.html_url is null")
 	}
-	ctx.Repo = models.Repo{
-		FullName: repoFullName,
-		Owner:    repoOwner,
-		Name:     repoName,
-		SSHURL:   repoSSHURL,
-	}
+	ctx.Repo = repo
 	ctx.User = models.User{
 		Username: commentorUsername,
 	}
@@ -131,3 +114,29 @@ func (e *EventParser) ExtractPullData(pull *github.PullRequest) (models.PullRequ
 		Num:        num,
 	}, nil
 }
+
+func (e *EventParser) ExtractRepoData(ghRepo *github.Repository) (models.Repo, error) {
+	var repo models.Repo
+	repoFullName := ghRepo.GetFullName()
+	if repoFullName == "" {
+		return repo, errors.New("repository.full_name is null")
+	}
+	repoOwner := ghRepo.Owner.GetLogin()
+	if repoOwner == "" {
+		return repo, errors.New("repository.owner.login is null")
+	}
+	repoName := ghRepo.GetName()
+	if repoName == "" {
+		return repo, errors.New("repository.name is null")
+	}
+	repoSSHURL := ghRepo.GetSSHURL()
+	if repoSSHURL == "" {
+		return repo, errors.New("repository.ssh_url is null")
+	}
+	return models.Repo{
+		Owner: repoOwner,
+		FullName: repoFullName,
+		SSHURL: repoSSHURL,
+		Name: repoName,
+	}, nil
+}

server/github_client.go

@@ -27,8 +27,8 @@ func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullReques
 	return files, nil
 }
 
-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})
+func (g *GithubClient) CreateComment(repo models.Repo, pull models.PullRequest, comment string) error {
+	_, _, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pull.Num, &github.IssueComment{Body: &comment})
 	return err
 }