diff --git a/apply_executor.go b/apply_executor.go index bbafd3ad5..496956fc1 100644 --- a/apply_executor.go +++ b/apply_executor.go @@ -54,35 +54,35 @@ 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)}} } @@ -90,26 +90,26 @@ func (a *ApplyExecutor) setupAndApply(ctx *ExecutionContext, prCtx *PullRequestC 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) diff --git a/base_executor.go b/base_executor.go index 072ddfb50..c13721f54 100644 --- a/base_executor.go +++ b/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, diff --git a/executor.go b/executor.go index 52e78f66b..57ef3960b 100644 --- a/executor.go +++ b/executor.go @@ -1,5 +1,5 @@ package main type Executor interface { - execute(ctx *ExecutionContext, prCtx *PullRequestContext) ExecutionResult + execute(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult } diff --git a/github_client.go b/github_client.go index 1e06db7d7..ca95faa75 100644 --- a/github_client.go +++ b/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] +} diff --git a/github_comment_renderer.go b/github_comment_renderer.go index 773f29ceb..e9ea7ff36 100644 --- a/github_comment_renderer.go +++ b/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{ diff --git a/help_executor.go b/help_executor.go index f3a54e044..f7cd0ee46 100644 --- a/help_executor.go +++ b/help_executor.go @@ -31,9 +31,8 @@ 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, @@ -41,7 +40,7 @@ func (h *HelpExecutor) execute(ctx *ExecutionContext, github *GithubClient) { terraformApplier: ctx.requesterUsername, terraformApplierEmail: ctx.requesterEmail, } - github.CreateComment(prCtx, helpComment) + github.CreateComment(pullCtx, helpComment) ctx.log.Info("generating help comment....") return diff --git a/locking/boltdb_lock_manager_test.go b/locking/boltdb_lock_manager_test.go index 18ec54037..67862be6c 100644 --- a/locking/boltdb_lock_manager_test.go +++ b/locking/boltdb_lock_manager_test.go @@ -14,11 +14,10 @@ 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(), } @@ -26,11 +25,8 @@ var run = locking.Run{ 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", + description: "lock when existing lock is for different repo name should succeed", sequence: []interface{}{ regularLock, LockCommand{ - keyWithNewField("RepoOwner", "different"), + keyWithNewField("RepoFullName", "repo/different"), locking.TryLockResponse{ LockAcquired: true, - LockingRun: keyWithNewField("RepoOwner", "different"), - }, - }, - }, - }, - { - description: "lock when existing lock is for different repo should succeed", - sequence: []interface{}{ - regularLock, - LockCommand{ - keyWithNewField("RepoName", "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{ diff --git a/locking/lock_manager.go b/locking/lock_manager.go index a78b88b64..da7a15d93 100644 --- a/locking/lock_manager.go +++ b/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) diff --git a/plan_executor.go b/plan_executor.go index 81ed4bff8..1576c5e22 100644 --- a/plan_executor.go +++ b/plan_executor.go @@ -30,7 +30,7 @@ func (p PlanSuccess) Template() *CompiledTemplate { } type RunLockedFailure struct { - LockingPullID int + LockingPullNum int } func (r RunLockedFailure) Template() *CompiledTemplate { @@ -60,18 +60,18 @@ func (e EnvironmentFailure) Template() *CompiledTemplate { return EnvironmentErrorTmpl } -func (p *PlanExecutor) execute(ctx *ExecutionContext, prCtx *PullRequestContext) ExecutionResult { - res := p.setupAndPlan(ctx, prCtx) +func (p *PlanExecutor) execute(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult { + res := p.setupAndPlan(ctx, pullCtx) res.Command = Plan return res } -func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestContext) ExecutionResult { - p.github.UpdateStatus(prCtx, "pending", "Planning...") +func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, pullCtx *PullRequestContext) ExecutionResult { + p.github.UpdateStatus(pullCtx, "pending", "Planning...") // todo: lock when cloning or somehow separate workspaces // clean the directory where we're going to clone - cloneDir := fmt.Sprintf("%s/%s/%s/%d", p.scratchDir, ctx.repoOwner, ctx.repoName, ctx.pullNum) + cloneDir := fmt.Sprintf("%s/%s/%d", p.scratchDir, ctx.repoFullName, ctx.pullNum) 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) @@ -90,7 +90,7 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestCon if err != nil { errMsg := fmt.Sprintf("failed to create git ssh wrapper: %v", err) ctx.log.Err(errMsg) - p.github.UpdateStatus(prCtx, ErrorStatus, "Plan Error") + p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error") return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}} } @@ -105,7 +105,7 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestCon 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(prCtx, ErrorStatus, "Plan Error") + p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error") return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}} } @@ -116,22 +116,22 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestCon 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(prCtx, ErrorStatus, "Plan Error") + p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error") return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}} } ctx.log.Info("listing modified files from pull request") - modifiedFiles, err := p.github.GetModifiedFiles(prCtx) + modifiedFiles, err := p.github.GetModifiedFiles(pullCtx) if err != nil { errMsg := fmt.Sprintf("failed to retrieve list of modified files from GitHub: %v", err) ctx.log.Err(errMsg) - p.github.UpdateStatus(prCtx, ErrorStatus, "Plan Error") + p.github.UpdateStatus(pullCtx, 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(prCtx, FailureStatus, "Plan Failed") + p.github.UpdateStatus(pullCtx, 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) @@ -139,15 +139,15 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestCon execPaths := p.DetermineExecPaths(p.trimSuffix(cloneDir, "/"), modifiedTerraformFiles) if len(execPaths) == 0 { ctx.log.Info("no exec paths found, exiting") - p.github.UpdateStatus(prCtx, FailureStatus, "Plan Failed") + p.github.UpdateStatus(pullCtx, FailureStatus, "Plan Failed") return ExecutionResult{SetupError: GeneralError{errors.New("Plan Failed: there were no paths to run plan in")}} } - planFilesPrefix := fmt.Sprintf("%s_%s_%d", ctx.repoOwner, ctx.repoName, ctx.pullNum) + planFilesPrefix := fmt.Sprintf("%s_%d", strings.Replace(ctx.repoFullName, "/", "_", -1), ctx.pullNum) if err := p.CleanWorkspace(ctx.log, planFilesPrefix, p.scratchDir, execPaths); err != nil { errMsg := fmt.Sprintf("failed to clean workspace, aborting: %v", err) ctx.log.Err(errMsg) - p.github.UpdateStatus(prCtx, ErrorStatus, "Plan Error") + p.github.UpdateStatus(pullCtx, ErrorStatus, "Plan Error") return ExecutionResult{SetupError: GeneralError{errors.New(errMsg)}} } s3Client := NewS3Client(p.awsConfig, p.s3Bucket, "plans") @@ -158,8 +158,8 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestCon for _, path := range execPaths { // 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_%s_%d%s", ctx.repoOwner, ctx.repoName, ctx.pullNum, tfPlanFilename) - s3Key := fmt.Sprintf("%s/%s/%s", ctx.repoOwner, ctx.repoName, tfPlanName) + tfPlanName := fmt.Sprintf("%s_%d%s", strings.Replace(ctx.repoFullName, "/", "_", -1), ctx.pullNum, tfPlanFilename) + s3Key := fmt.Sprintf("%s/%s", ctx.repoFullName, 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) @@ -184,18 +184,18 @@ func (p *PlanExecutor) setupAndPlan(ctx *ExecutionContext, prCtx *PullRequestCon p.terraform.tfExecutableName = "terraform" } } - generatePlanResponse := p.plan(ctx.log, prCtx, cloneDir, p.scratchDir, tfPlanName, s3Client, path, ctx.command.environment, s3Key, p.sshKey, ctx.pullCreator, config.StashPath) + 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 planOutputs = append(planOutputs, generatePlanResponse) } - p.updateGithubStatus(prCtx, planOutputs) + p.updateGithubStatus(pullCtx, 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, - prCtx *PullRequestContext, + pullCtx *PullRequestContext, repoDir string, planOutDir string, tfPlanName string, @@ -208,12 +208,11 @@ func (p *PlanExecutor) plan(log *logging.SimpleLogger, stashPath string) PathResult { log.Info("generating plan for path %q", path) run := locking.Run{ - RepoOwner: prCtx.owner, - RepoName: prCtx.repoName, + RepoFullName: pullCtx.repoFullName, Path: path.Relative, Env: tfEnvName, - PullID: prCtx.number, - User: prCtx.terraformApplier, + PullNum: pullCtx.number, + User: pullCtx.terraformApplier, Timestamp: time.Now(), } @@ -236,10 +235,10 @@ 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.PullID != prCtx.number { + if lockAttempt.LockAcquired == false && lockAttempt.LockingRun.PullNum != pullCtx.number { return PathResult{ Status: "failure", - Result: RunLockedFailure{lockAttempt.LockingRun.PullID}, + Result: RunLockedFailure{lockAttempt.LockingRun.PullNum}, } } diff --git a/request_parser.go b/request_parser.go index 21b610c4a..3538fb778 100644 --- a/request_parser.go +++ b/request_parser.go @@ -68,13 +68,9 @@ func (r *RequestParser) determineCommand(comment *github.IssueCommentEvent) (*Co func (r *RequestParser) extractCommentData(comment *github.IssueCommentEvent, params *ExecutionContext) error { missingField := "" - owner := comment.Repo.Owner.Login - if owner == 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") + repoFullName := comment.Repo.FullName + if repoFullName == nil { + return errors.New("key 'comment.repo.full_name' is null") } pullNum := comment.Issue.Number if pullNum == nil { @@ -102,10 +98,9 @@ func (r *RequestParser) extractCommentData(comment *github.IssueCommentEvent, pa } params.requesterUsername = *commentorUsername params.requesterEmail = *commentorEmail - params.repoOwner = *owner + params.repoFullName = *repoFullName params.pullNum = *pullNum params.pullCreator = *pullCreator - params.repoName = *repoName params.repoSSHUrl = *repoSSHURL params.htmlUrl = *htmlURL return nil diff --git a/server.go b/server.go index 9cea37fd0..b5e8af0dc 100644 --- a/server.go +++ b/server.go @@ -63,8 +63,7 @@ type ServerConfig struct { } type ExecutionContext struct { - repoOwner string - repoName string + repoFullName string pullNum int requesterUsername string requesterEmail string @@ -239,13 +238,13 @@ func (s *Server) postHooks(w http.ResponseWriter, r *http.Request) { } func (s *Server) executeCommand(ctx *ExecutionContext) { - src := fmt.Sprintf("%s/%s/pull/%d", ctx.repoOwner, ctx.repoName, ctx.pullNum) + src := fmt.Sprintf("%s/pull/%d", ctx.repoFullName, ctx.pullNum) // it's safe to reuse the underlying logger s.logger.Log 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.repoOwner, ctx.repoName, ctx.pullNum) + pull, _, err := s.githubClient.GetPullRequest(ctx.repoFullName, ctx.pullNum) if err != nil { ctx.log.Err("pull request data api call failed: %v", err) return diff --git a/web_templates.go b/web_templates.go index a047b1b42..9e0d5d2f8 100644 --- a/web_templates.go +++ b/web_templates.go @@ -35,7 +35,7 @@ var indexTemplate = template.Must(template.New("index.html.tmpl").Parse(` {{ if . }} {{ range . }}
-
{{.RepoOwner}}/{{.RepoName}} - #{{.PullID}}
+
{{.RepoFullName}} - #{{.PullNum}}
Locked
{{.Timestamp}}