diff --git a/server/events/working_dir.go b/server/events/working_dir.go index 80bca96c9c..23eec74da3 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -100,6 +100,7 @@ func (w *FileWorkspace) Clone( cloneDir := w.cloneDir(p.BaseRepo, p, workspace) defer func() { w.CheckForUpstreamChanges = false }() + config := wrappedGitConfig{cloneDir, headRepo, p} // If the directory already exists, check if it's at the right commit. // If so, then we do nothing. if _, err := os.Stat(cloneDir); err == nil { @@ -119,7 +120,7 @@ func (w *FileWorkspace) Clone( outputRevParseCmd, err := revParseCmd.CombinedOutput() if err != nil { w.Logger.Warn("will re-clone repo, could not determine if was at correct commit: %s: %s: %s", strings.Join(revParseCmd.Args, " "), err, string(outputRevParseCmd)) - return cloneDir, false, w.forceClone(cloneDir, headRepo, p) + return cloneDir, false, w.forceClone(config) } currCommit := strings.Trim(string(outputRevParseCmd), "\n") @@ -128,7 +129,7 @@ func (w *FileWorkspace) Clone( if strings.HasPrefix(currCommit, p.HeadCommit) { if w.CheckForUpstreamChanges && w.CheckoutMerge && w.recheckDiverged(p, headRepo, cloneDir) { w.Logger.Info("base branch has been updated, using merge strategy and will clone again") - return cloneDir, true, w.mergeAgain(cloneDir, headRepo, p) + return cloneDir, true, w.mergeAgain(config) } else { w.Logger.Debug("repo is at correct commit %q so will not re-clone", p.HeadCommit) return cloneDir, false, nil @@ -140,7 +141,7 @@ func (w *FileWorkspace) Clone( } // Otherwise we clone the repo. - return cloneDir, false, w.forceClone(cloneDir, headRepo, p) + return cloneDir, false, w.forceClone(config) } // recheckDiverged returns true if the branch we're merging into has diverged @@ -209,8 +210,8 @@ func (w *FileWorkspace) HasDiverged(cloneDir string) bool { return hasDiverged } -func (w *FileWorkspace) forceClone(cloneDir string, headRepo models.Repo, p models.PullRequest) error { - value, _ := cloneLocks.LoadOrStore(cloneDir, new(sync.Mutex)) +func (w *FileWorkspace) forceClone(config wrappedGitConfig) error { + value, _ := cloneLocks.LoadOrStore(config.dir, new(sync.Mutex)) mutex := value.(*sync.Mutex) defer mutex.Unlock() @@ -219,61 +220,61 @@ func (w *FileWorkspace) forceClone(cloneDir string, headRepo models.Repo, p mode return nil } - err := os.RemoveAll(cloneDir) + err := os.RemoveAll(config.dir) if err != nil { - return errors.Wrapf(err, "deleting dir %q before cloning", cloneDir) + return errors.Wrapf(err, "deleting dir %q before cloning", config.dir) } // Create the directory and parents if necessary. - w.Logger.Info("creating dir %q", cloneDir) - if err := os.MkdirAll(cloneDir, 0700); err != nil { + w.Logger.Info("creating dir %q", config.dir) + if err := os.MkdirAll(config.dir, 0700); err != nil { return errors.Wrap(err, "creating new workspace") } // During testing, we mock some of this out. - headCloneURL := headRepo.CloneURL + headCloneURL := config.head.CloneURL if w.TestingOverrideHeadCloneURL != "" { headCloneURL = w.TestingOverrideHeadCloneURL } - baseCloneURL := p.BaseRepo.CloneURL + baseCloneURL := config.pr.BaseRepo.CloneURL if w.TestingOverrideBaseCloneURL != "" { baseCloneURL = w.TestingOverrideBaseCloneURL } // if branch strategy, use depth=1 if !w.CheckoutMerge { - return w.runGit(cloneDir, headRepo, p, "clone", "--depth=1", "--branch", p.HeadBranch, "--single-branch", headCloneURL, cloneDir) + return w.wrappedGit(config, "clone", "--depth=1", "--branch", config.pr.HeadBranch, "--single-branch", headCloneURL, config.dir) } // if merge strategy... // if no checkout depth, omit depth arg if w.CheckoutDepth == 0 { - if err := w.runGit(cloneDir, headRepo, p, "clone", "--branch", p.BaseBranch, "--single-branch", baseCloneURL, cloneDir); err != nil { + if err := w.wrappedGit(config, "clone", "--branch", config.pr.BaseBranch, "--single-branch", baseCloneURL, config.dir); err != nil { return err } } else { - if err := w.runGit(cloneDir, headRepo, p, "clone", "--depth", fmt.Sprint(w.CheckoutDepth), "--branch", p.BaseBranch, "--single-branch", baseCloneURL, cloneDir); err != nil { + if err := w.wrappedGit(config, "clone", "--depth", fmt.Sprint(w.CheckoutDepth), "--branch", config.pr.BaseBranch, "--single-branch", baseCloneURL, config.dir); err != nil { return err } } - if err := w.runGit(cloneDir, headRepo, p, "remote", "add", "head", headCloneURL); err != nil { + if err := w.wrappedGit(config, "remote", "add", "head", headCloneURL); err != nil { return err } if w.GpgNoSigningEnabled { - if err := w.runGit(cloneDir, headRepo, p, "config", "--local", "commit.gpgsign", "false"); err != nil { + if err := w.wrappedGit(config, "config", "--local", "commit.gpgsign", "false"); err != nil { return err } } - return w.mergeToBaseBranch(cloneDir, headRepo, p) + return w.mergeToBaseBranch(config) } // There is a new upstream update that we need, and we want to update to it // without deleting any existing plans -func (w *FileWorkspace) mergeAgain(cloneDir string, headRepo models.Repo, p models.PullRequest) error { - value, _ := cloneLocks.LoadOrStore(cloneDir, new(sync.Mutex)) +func (w *FileWorkspace) mergeAgain(config wrappedGitConfig) error { + value, _ := cloneLocks.LoadOrStore(config.dir, new(sync.Mutex)) mutex := value.(*sync.Mutex) defer mutex.Unlock() @@ -283,28 +284,37 @@ func (w *FileWorkspace) mergeAgain(cloneDir string, headRepo models.Repo, p mode } // Reset branch as if it was cloned again - if err := w.runGit(cloneDir, headRepo, p, "reset", "--hard", fmt.Sprintf("refs/remotes/head/%s", p.BaseBranch)); err != nil { + if err := w.wrappedGit(config, "reset", "--hard", fmt.Sprintf("refs/remotes/head/%s", config.pr.BaseBranch)); err != nil { return err } - return w.mergeToBaseBranch(cloneDir, headRepo, p) + return w.mergeToBaseBranch(config) } -func (w *FileWorkspace) runGit(cloneDir string, headRepo models.Repo, p models.PullRequest, args ...string) error { +// wrappedGitConfig is the configuration for wrappedGit that is typically unchanged +// for a series of calls to wrappedGit +type wrappedGitConfig struct { + dir string + head models.Repo + pr models.PullRequest +} + +// wrappedGit runs git with additional environment settings required for git merge, +// and with sanitized error logging to avoid leaking git credentials +func (w *FileWorkspace) wrappedGit(config wrappedGitConfig, args ...string) error { cmd := exec.Command("git", args...) // nolint: gosec - cmd.Dir = cloneDir + cmd.Dir = config.dir // The git merge command requires these env vars are set. cmd.Env = append(os.Environ(), []string{ "EMAIL=atlantis@runatlantis.io", "GIT_AUTHOR_NAME=atlantis", "GIT_COMMITTER_NAME=atlantis", }...) - - cmdStr := w.sanitizeGitCredentials(strings.Join(cmd.Args, " "), p.BaseRepo, headRepo) + cmdStr := w.sanitizeGitCredentials(strings.Join(cmd.Args, " "), config.pr.BaseRepo, config.head) output, err := cmd.CombinedOutput() - sanitizedOutput := w.sanitizeGitCredentials(string(output), p.BaseRepo, headRepo) + sanitizedOutput := w.sanitizeGitCredentials(string(output), config.pr.BaseRepo, config.head) if err != nil { - sanitizedErrMsg := w.sanitizeGitCredentials(err.Error(), p.BaseRepo, headRepo) + sanitizedErrMsg := w.sanitizeGitCredentials(err.Error(), config.pr.BaseRepo, config.head) return fmt.Errorf("running %s: %s: %s", cmdStr, sanitizedOutput, sanitizedErrMsg) } w.Logger.Debug("ran: %s. Output: %s", cmdStr, strings.TrimSuffix(sanitizedOutput, "\n")) @@ -312,29 +322,29 @@ func (w *FileWorkspace) runGit(cloneDir string, headRepo models.Repo, p models.P } // Merge the PR into the base branch. -func (w *FileWorkspace) mergeToBaseBranch(cloneDir string, headRepo models.Repo, p models.PullRequest) error { - fetchRef := fmt.Sprintf("+refs/heads/%s:", p.HeadBranch) +func (w *FileWorkspace) mergeToBaseBranch(config wrappedGitConfig) error { + fetchRef := fmt.Sprintf("+refs/heads/%s:", config.pr.HeadBranch) fetchRemote := "head" if w.GithubAppEnabled { - fetchRef = fmt.Sprintf("pull/%d/head:", p.Num) + fetchRef = fmt.Sprintf("pull/%d/head:", config.pr.Num) fetchRemote = "origin" } // if no checkout depth, omit depth arg if w.CheckoutDepth == 0 { - if err := w.runGit(cloneDir, headRepo, p, "fetch", fetchRemote, fetchRef); err != nil { + if err := w.wrappedGit(config, "fetch", fetchRemote, fetchRef); err != nil { return err } } else { - if err := w.runGit(cloneDir, headRepo, p, "fetch", "--depth", fmt.Sprint(w.CheckoutDepth), fetchRemote, fetchRef); err != nil { + if err := w.wrappedGit(config, "fetch", "--depth", fmt.Sprint(w.CheckoutDepth), fetchRemote, fetchRef); err != nil { return err } } - if err := w.runGit(cloneDir, headRepo, p, "merge-base", p.BaseBranch, "FETCH_HEAD"); err != nil { + if err := w.wrappedGit(config, "merge-base", config.pr.BaseBranch, "FETCH_HEAD"); err != nil { // git merge-base returning error means that we did not receive enough commits in shallow clone. // Fall back to retrieving full repo history. - if err := w.runGit(cloneDir, headRepo, p, "fetch", "--unshallow"); err != nil { + if err := w.wrappedGit(config, "fetch", "--unshallow"); err != nil { return err } } @@ -345,7 +355,7 @@ func (w *FileWorkspace) mergeToBaseBranch(cloneDir string, headRepo models.Repo, // git rev-parse HEAD^2 to get the head commit because it will // always succeed whereas without --no-ff, if the merge was fast // forwarded then git rev-parse HEAD^2 would fail. - return w.runGit(cloneDir, headRepo, p, "merge", "-q", "--no-ff", "-m", "atlantis-merge", "FETCH_HEAD") + return w.wrappedGit(config, "merge", "-q", "--no-ff", "-m", "atlantis-merge", "FETCH_HEAD") } // GetWorkingDir returns the path to the workspace for this repo and pull.