-
Notifications
You must be signed in to change notification settings - Fork 1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rename git wrapper to wrappedGit, add a type for static config
Every call to wrappedGit for the same PR uses identical setup for directory, head repo and PR, so passing the
- Loading branch information
Showing
1 changed file
with
45 additions
and
35 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,6 +100,7 @@ func (w *FileWorkspace) Clone( | |
| cloneDir := w.cloneDir(p.BaseRepo, p, workspace) | ||
| defer func() { w.CheckForUpstreamChanges = false }() | ||
|
|
||
| c := wrappedGitContext{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(c) | ||
| } | ||
| 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(c) | ||
| } 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(c) | ||
| } | ||
|
|
||
| // 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(c wrappedGitContext) error { | ||
| value, _ := cloneLocks.LoadOrStore(c.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(c.dir) | ||
|
Check failure Code scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value Error loading related location Loading |
||
| if err != nil { | ||
| return errors.Wrapf(err, "deleting dir %q before cloning", cloneDir) | ||
| return errors.Wrapf(err, "deleting dir %q before cloning", c.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", c.dir) | ||
| if err := os.MkdirAll(c.dir, 0700); err != nil { | ||
|
Check failure Code scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value Error loading related location Loading |
||
| return errors.Wrap(err, "creating new workspace") | ||
| } | ||
|
|
||
| // During testing, we mock some of this out. | ||
| headCloneURL := headRepo.CloneURL | ||
| headCloneURL := c.head.CloneURL | ||
| if w.TestingOverrideHeadCloneURL != "" { | ||
| headCloneURL = w.TestingOverrideHeadCloneURL | ||
| } | ||
| baseCloneURL := p.BaseRepo.CloneURL | ||
| baseCloneURL := c.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(c, "clone", "--depth=1", "--branch", c.pr.HeadBranch, "--single-branch", headCloneURL, c.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(c, "clone", "--branch", c.pr.BaseBranch, "--single-branch", baseCloneURL, c.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(c, "clone", "--depth", fmt.Sprint(w.CheckoutDepth), "--branch", c.pr.BaseBranch, "--single-branch", baseCloneURL, c.dir); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if err := w.runGit(cloneDir, headRepo, p, "remote", "add", "head", headCloneURL); err != nil { | ||
| if err := w.wrappedGit(c, "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(c, "config", "--local", "commit.gpgsign", "false"); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return w.mergeToBaseBranch(cloneDir, headRepo, p) | ||
| return w.mergeToBaseBranch(c) | ||
| } | ||
|
|
||
| // 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(c wrappedGitContext) error { | ||
| value, _ := cloneLocks.LoadOrStore(c.dir, new(sync.Mutex)) | ||
| mutex := value.(*sync.Mutex) | ||
|
|
||
| defer mutex.Unlock() | ||
|
|
@@ -283,58 +284,67 @@ 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(c, "reset", "--hard", fmt.Sprintf("refs/remotes/head/%s", c.pr.BaseBranch)); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return w.mergeToBaseBranch(cloneDir, headRepo, p) | ||
| return w.mergeToBaseBranch(c) | ||
| } | ||
|
|
||
| func (w *FileWorkspace) runGit(cloneDir string, headRepo models.Repo, p models.PullRequest, args ...string) error { | ||
| // wrappedGitContext is the configuration for wrappedGit that is typically unchanged | ||
| // for a series of calls to wrappedGit | ||
| type wrappedGitContext 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(c wrappedGitContext, args ...string) error { | ||
| cmd := exec.Command("git", args...) // nolint: gosec | ||
|
Check failure Code scanning / CodeQL Command built from user-controlled sources Critical
This command depends on a
user-provided value Error loading related location Loading This command depends on a user-provided value Error loading related location Loading |
||
| cmd.Dir = cloneDir | ||
| cmd.Dir = c.dir | ||
| // The git merge command requires these env vars are set. | ||
| cmd.Env = append(os.Environ(), []string{ | ||
| "[email protected]", | ||
| "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, " "), c.pr.BaseRepo, c.head) | ||
| output, err := cmd.CombinedOutput() | ||
| sanitizedOutput := w.sanitizeGitCredentials(string(output), p.BaseRepo, headRepo) | ||
| sanitizedOutput := w.sanitizeGitCredentials(string(output), c.pr.BaseRepo, c.head) | ||
| if err != nil { | ||
| sanitizedErrMsg := w.sanitizeGitCredentials(err.Error(), p.BaseRepo, headRepo) | ||
| sanitizedErrMsg := w.sanitizeGitCredentials(err.Error(), c.pr.BaseRepo, c.head) | ||
| return fmt.Errorf("running %s: %s: %s", cmdStr, sanitizedOutput, sanitizedErrMsg) | ||
| } | ||
| w.Logger.Debug("ran: %s. Output: %s", cmdStr, strings.TrimSuffix(sanitizedOutput, "\n")) | ||
| return nil | ||
| } | ||
|
|
||
| // 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(c wrappedGitContext) error { | ||
| fetchRef := fmt.Sprintf("+refs/heads/%s:", c.pr.HeadBranch) | ||
| fetchRemote := "head" | ||
| if w.GithubAppEnabled { | ||
| fetchRef = fmt.Sprintf("pull/%d/head:", p.Num) | ||
| fetchRef = fmt.Sprintf("pull/%d/head:", c.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(c, "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(c, "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(c, "merge-base", c.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(c, "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(c, "merge", "-q", "--no-ff", "-m", "atlantis-merge", "FETCH_HEAD") | ||
| } | ||
|
|
||
| // GetWorkingDir returns the path to the workspace for this repo and pull. | ||
|
|
||