Fixes runatlantis#10. Renaming for consistency (runatlantis#12)

Repo owner, name => repoFullName
PullID => PullNum to match GitHub API
PR => Pull to match GitHub API and avoid writing PRID anywhere

apply_executor.go

@@ -54,62 +54,62 @@ func (n NoPlansFailure) Template() *CompiledTemplate {
 	return NoPlansFailureTmpl
 }
 
-func (a *ApplyExecutor) execute(ctx *ExecutionContext, prCtx *PullRequestContext) ExecutionResult {
-	res := a.setupAndApply(ctx, prCtx)
+func (a *ApplyExecutor) execute(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult {
+	res := a.setupAndApply(ctx, pullCtx)
 	res.Command = Apply
 	return res
 }
 
-func (a *ApplyExecutor) setupAndApply(ctx *ExecutionContext, prCtx *PullRequestContext) ExecutionResult {
-	a.github.UpdateStatus(prCtx, PendingStatus, "Applying...")
+func (a *ApplyExecutor) setupAndApply(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult {
+	a.github.UpdateStatus(pullCtx, PendingStatus, "Applying...")
 
 	if a.requireApproval {
-		ok, err := a.github.PullIsApproved(prCtx)
+		ok, err := a.github.PullIsApproved(pullCtx)
 		if err != nil {
 			msg := fmt.Sprintf("failed to determine if pull request was approved: %v", err)
 			ctx.log.Err(msg)
-			a.github.UpdateStatus(prCtx, ErrorStatus, "Apply Error")
+			a.github.UpdateStatus(pullCtx, ErrorStatus, "Apply Error")
 			return ExecutionResult{SetupError: GeneralError{errors.New(msg)}}
 		}
 		if !ok {
 			ctx.log.Info("pull request was not approved")
-			a.github.UpdateStatus(prCtx, FailureStatus, "Apply Failed")
+			a.github.UpdateStatus(pullCtx, FailureStatus, "Apply Failed")
 			return ExecutionResult{SetupFailure: PullNotApprovedFailure{}}
 		}
 	}
 
-	planPaths, err := a.downloadPlans(ctx.repoOwner, ctx.repoName, ctx.pullNum, ctx.command.environment, a.scratchDir, a.awsConfig, a.s3Bucket)
+	planPaths, err := a.downloadPlans(ctx.repoFullName, ctx.pullNum, 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(prCtx, ErrorStatus, "Apply Error")
+		a.github.UpdateStatus(pullCtx, 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(prCtx, FailureStatus, "Apply Failure")
+		a.github.UpdateStatus(pullCtx, 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, prCtx, planPath)
+		output := a.apply(ctx, pullCtx, planPath)
 		output.Path = planPath
 		applyOutputs = append(applyOutputs, output)
 	}
-	a.updateGithubStatus(prCtx, applyOutputs)
+	a.updateGithubStatus(pullCtx, applyOutputs)
 	return ExecutionResult{PathResults: applyOutputs}
 }
 
-func (a *ApplyExecutor) apply(ctx *ExecutionContext, prCtx *PullRequestContext, planPath string) PathResult {
+func (a *ApplyExecutor) apply(ctx *ExecutionContext, pullCtx *PullRequestContext, planPath string) PathResult {
 	//runLog = append(runLog, fmt.Sprintf("-> Running apply %s", planPath))
 	planName := path.Base(planPath)
 	planSubDir := a.determinePlanSubDir(planName, ctx.pullNum)
-	planDir := filepath.Join(a.scratchDir, ctx.repoOwner, ctx.repoName, fmt.Sprintf("%v", ctx.pullNum), planSubDir)
+	planDir := filepath.Join(a.scratchDir, ctx.repoFullName, fmt.Sprintf("%v", ctx.pullNum), planSubDir)
 	execPath := NewExecutionPath(planDir, planSubDir)
 	var config Config
 	var remoteStatePath string
@@ -167,12 +167,11 @@ func (a *ApplyExecutor) apply(ctx *ExecutionContext, prCtx *PullRequestContext,
 			tfEnv = "default"
 		}
 		run := locking.Run{
-			RepoOwner: prCtx.owner,
-			RepoName: prCtx.repoName,
+			RepoFullName: pullCtx.repoFullName,
 			Path: execPath.Relative,
 			Env: tfEnv,
-			PullID: prCtx.number,
-			User: prCtx.terraformApplier,
+			PullNum: pullCtx.number,
+			User: pullCtx.terraformApplier,
 			Timestamp: time.Now(),
 		}
 
@@ -183,10 +182,10 @@ func (a *ApplyExecutor) apply(ctx *ExecutionContext, prCtx *PullRequestContext,
 				Result: GeneralError{fmt.Errorf("failed to acquire lock: %s", err)},
 			}
 		}
-		if lockAttempt.LockAcquired != true && lockAttempt.LockingRun.PullID != prCtx.number {
+		if lockAttempt.LockAcquired != true && lockAttempt.LockingRun.PullNum != pullCtx.number {
 			return PathResult{
 				Status: "error",
-				Result: GeneralError{fmt.Errorf("failed to acquire lock: lock held by pull request #%d", lockAttempt.LockingRun.PullID)},
+				Result: GeneralError{fmt.Errorf("failed to acquire lock: lock held by pull request #%d", lockAttempt.LockingRun.PullNum)},
 			}
 		}
 	}
@@ -239,7 +238,7 @@ func (a *ApplyExecutor) apply(ctx *ExecutionContext, prCtx *PullRequestContext,
 	}
 }
 
-func (a *ApplyExecutor) downloadPlans(repoOwner string, repoName string, pullNum int, env string, outputDir string, awsConfig *AWSConfig, s3Bucket string) (planPaths []string, err error) {
+func (a *ApplyExecutor) downloadPlans(repoFullName string, pullNum int, env string, outputDir string, awsConfig *AWSConfig, s3Bucket string) (planPaths []string, err error) {
 	awsSession, err := awsConfig.CreateAWSSession()
 	if err != nil {
 		return nil, fmt.Errorf("failed to assume role: %v", err)
@@ -249,7 +248,7 @@ func (a *ApplyExecutor) downloadPlans(repoOwner string, repoName string, pullNum
 	s3Client := s3.New(awsSession)
 
 	// this will be plans/owner/repo/owner_repo_1, may be more than one if there are subdirs or multiple envs
-	plansPath := fmt.Sprintf("plans/%s/%s/%s_%s_%d", repoOwner, repoName, repoOwner, repoName, pullNum)
+	plansPath := fmt.Sprintf("plans/%s/%s_%d", repoFullName, strings.Replace(repoFullName, "/", "_", -1), pullNum)
 	list, err := s3Client.ListObjects(&s3.ListObjectsInput{Bucket: aws.String(s3Bucket), Prefix: &plansPath})
 	if err != nil {
 		return nil, fmt.Errorf("failed to list plans in path: %v", err)

base_executor.go

@@ -18,8 +18,7 @@ type BaseExecutor struct {
 }
 
 type PullRequestContext struct {
-	owner                 string
-	repoName              string
+	repoFullName          string
 	head                  string
 	base                  string
 	number                int
@@ -54,15 +53,15 @@ func NewExecutionPath(absolutePath string, relativePath string) ExecutionPath {
 	return ExecutionPath{filepath.Clean(absolutePath), filepath.Clean(relativePath)}
 }
 
-func (b *BaseExecutor) updateGithubStatus(prCtx *PullRequestContext, pathResults []PathResult) {
+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(prCtx, SuccessStatus, "Plan Succeeded")
+		b.github.UpdateStatus(pullCtx, SuccessStatus, "Plan Succeeded")
 	} else if worstResult == "failure" {
-		b.github.UpdateStatus(prCtx, FailureStatus, "Plan Failed")
+		b.github.UpdateStatus(pullCtx, FailureStatus, "Plan Failed")
 	} else {
-		b.github.UpdateStatus(prCtx, ErrorStatus, "Plan Error")
+		b.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error")
 	}
 }
 
@@ -79,16 +78,15 @@ func (b *BaseExecutor) worstResult(results []PathResult) string {
 }
 
 func (b *BaseExecutor) Exec(f func(*ExecutionContext, *PullRequestContext) ExecutionResult, ctx *ExecutionContext, github *GithubClient) {
-	prCtx := b.githubContext(ctx)
-	result := f(ctx, prCtx)
+	pullCtx := b.githubContext(ctx)
+	result := f(ctx, pullCtx)
 	comment := b.githubCommentRenderer.render(result, ctx.log.History.String(), ctx.command.verbose)
-	github.CreateComment(prCtx, comment)
+	github.CreateComment(pullCtx, comment)
 }
 
 func (b *BaseExecutor) githubContext(ctx *ExecutionContext) *PullRequestContext {
 	return &PullRequestContext{
-		owner:                 ctx.repoOwner,
-		repoName:              ctx.repoName,
+		repoFullName:          ctx.repoFullName,
 		head:                  ctx.head,
 		base:                  ctx.base,
 		number:                ctx.pullNum,

executor.go

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

github_client.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/google/go-github/github"
 	"context"
+	"strings"
 )
 
 type GithubClient struct {
@@ -21,13 +22,15 @@ const (
 
 func (g *GithubClient) UpdateStatus(ctx *PullRequestContext, status string, description string) {
 	repoStatus := github.RepoStatus{State: github.String(status), Description: github.String(description), Context: github.String(statusContext)}
-	g.client.Repositories.CreateStatus(g.ctx, ctx.owner, ctx.repoName, ctx.head, &repoStatus)
+	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
+	g.client.Repositories.CreateStatus(g.ctx, owner, repo, ctx.head, &repoStatus)
 	// todo: deal with error updating status
 }
 
 func (g *GithubClient) GetModifiedFiles(ctx *PullRequestContext) ([]string, error) {
 	var files = []string{}
-	comparison, _, err := g.client.Repositories.CompareCommits(g.ctx, ctx.owner, ctx.repoName, ctx.base, ctx.head)
+	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
+	comparison, _, err := g.client.Repositories.CompareCommits(g.ctx, owner, repo, ctx.base, ctx.head)
 	if err != nil {
 		return files, err
 	}
@@ -38,7 +41,8 @@ func (g *GithubClient) GetModifiedFiles(ctx *PullRequestContext) ([]string, erro
 }
 
 func (g *GithubClient) CreateComment(ctx *PullRequestContext, comment string) error {
-	_, _, err := g.client.Issues.CreateComment(g.ctx, ctx.owner, ctx.repoName, ctx.number, &github.IssueComment{Body: &comment})
+	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
+	_, _, err := g.client.Issues.CreateComment(g.ctx, owner, repo, ctx.number, &github.IssueComment{Body: &comment})
 	return err
 }
 
@@ -47,7 +51,8 @@ func (g *GithubClient) CommentExists(ctx *PullRequestContext, matcher func(*gith
 	opt := &github.IssueListCommentsOptions{}
 	// need to loop since there may be multiple pages of comments
 	for {
-		comments, resp, err := g.client.Issues.ListComments(g.ctx, ctx.owner, ctx.repoName, ctx.number, opt)
+		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)
 		}
@@ -67,7 +72,8 @@ func (g *GithubClient) CommentExists(ctx *PullRequestContext, matcher func(*gith
 func (g *GithubClient) PullIsApproved(ctx *PullRequestContext) (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
-	u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", ctx.owner, ctx.repoName, ctx.number)
+	owner, repo := g.repoFullNameToOwnerAndRepo(ctx.repoFullName)
+	u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", owner, repo, ctx.number)
 	req, err := g.client.NewRequest("GET", u, nil)
 	if err != nil {
 		return false, err
@@ -87,6 +93,17 @@ func (g *GithubClient) PullIsApproved(ctx *PullRequestContext) (bool, error) {
 	return false, nil
 }
 
-func (g *GithubClient) GetPullRequest(owner string, repo string, number int) (*github.PullRequest, *github.Response, error) {
+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]
+}

github_comment_renderer.go

@@ -50,7 +50,7 @@ var (
 			"* To **discard** this plan click [here]({{.LockURL}}).",
 	}
 	RunLockedFailureTmpl *CompiledTemplate = &CompiledTemplate{
-		text: "This plan is currently locked by #{{.LockingPullID}}\n" +
+		text: "This plan is currently locked by #{{.LockingPullNum}}\n" +
 			"The locking plan must be applied or discarded before future plans can execute.",
 	}
 	TerraformFailureTmpl *CompiledTemplate = &CompiledTemplate{

help_executor.go

@@ -31,17 +31,16 @@ atlantis apply
 `
 
 func (h *HelpExecutor) execute(ctx *ExecutionContext, github *GithubClient) {
-	prCtx := &PullRequestContext{
-		owner:                 ctx.repoOwner,
-		repoName:              ctx.repoName,
+	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(prCtx, helpComment)
+	github.CreateComment(pullCtx, helpComment)
 
 	ctx.log.Info("generating help comment....")
 	return

locking/boltdb_lock_manager_test.go

@@ -14,23 +14,19 @@ import (
 const LockBucket = "locks"
 
 var run = locking.Run{
-	RepoOwner: "repoOwner",
-	RepoName: "repo",
+	RepoFullName: "owner/repo",
 	Path: "parent/child",
 	Env: "default",
-	PullID: 1,
+	PullNum: 1,
 	User: "user",
 	Timestamp: time.Now(),
 }
 
 func keyWithNewField(fieldName string, value string) locking.Run {
 	copy := run
 	switch fieldName {
-	case "RepoOwner":
-		copy.RepoOwner = value
-		return copy
-	case "RepoName":
-		copy.RepoName = value
+	case "RepoFullName":
+		copy.RepoFullName = value
 		return copy
 	case "Path":
 		copy.Path = value
@@ -112,7 +108,7 @@ var regularLock = LockCommand{
 	locking.TryLockResponse{
 		LockAcquired: true,
 		LockingRun: run,
-		LockID: "f13c9d87dc7156638cc075f0164d628f1338d23b17296dd391d922ea6fe6e9f8",
+		LockID: "cae47fa9dfe223b0d8fefd51f2fbbf9849047c9d048d659d1ba906acc2913534",
 	},
 }
 
@@ -138,7 +134,7 @@ var tableTestData = []struct {
 		sequence: []interface{}{
 			regularLock,
 			UnlockCommand{
-				"f13c9d87dc7156638cc075f0164d628f1338d23b17296dd391d922ea6fe6e9f8",
+				"cae47fa9dfe223b0d8fefd51f2fbbf9849047c9d048d659d1ba906acc2913534",
 				nil,
 			},
 		},
@@ -158,27 +154,14 @@ var tableTestData = []struct {
 		},
 	},
 	{
-		description: "lock when existing lock is for different repo owner should succeed",
-		sequence: []interface{}{
-			regularLock,
-			LockCommand{
-				keyWithNewField("RepoOwner", "different"),
-				locking.TryLockResponse{
-					LockAcquired: true,
-					LockingRun: keyWithNewField("RepoOwner", "different"),
-				},
-			},
-		},
-	},
-	{
-		description: "lock when existing lock is for different repo should succeed",
+		description: "lock when existing lock is for different repo name should succeed",
 		sequence: []interface{}{
 			regularLock,
 			LockCommand{
-				keyWithNewField("RepoName", "different"),
+				keyWithNewField("RepoFullName", "repo/different"),
 				locking.TryLockResponse{
 					LockAcquired: true,
-					LockingRun: keyWithNewField("RepoName", "different"),
+					LockingRun: keyWithNewField("RepoFullName", "repo/different"),
 				},
 			},
 		},
@@ -214,7 +197,7 @@ var tableTestData = []struct {
 		sequence: []interface{}{
 			regularLock,
 			UnlockCommand{
-				"f13c9d87dc7156638cc075f0164d628f1338d23b17296dd391d922ea6fe6e9f8",
+				"cae47fa9dfe223b0d8fefd51f2fbbf9849047c9d048d659d1ba906acc2913534",
 				nil,
 			},
 			LockCommand{

locking/lock_manager.go

@@ -7,11 +7,10 @@ import (
 )
 
 type Run struct {
-	RepoOwner  string
-	RepoName   string
+	RepoFullName string
 	Path       string
 	Env        string
-	PullID     int
+	PullNum     int
 	User       string
 	Timestamp  time.Time
 }
@@ -21,7 +20,7 @@ type Run struct {
 func (r Run) StateKey() []byte {
 	// we combine the repository, path into the repo where terraform is being run and the environment
 	// to make up the state key and then hash it with sha256
-	key := fmt.Sprintf("%s/%s/%s/%s", r.RepoOwner, r.RepoName, r.Path, r.Env)
+	key := fmt.Sprintf("%s/%s/%s", r.RepoFullName, r.Path, r.Env)
 	h := sha256.New()
 	h.Write([]byte(key))
 	return h.Sum(nil)