From 8b77b95a162d9467c94dca5ccfbd883f89b4230c Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Tue, 15 Jan 2019 14:19:04 -0500 Subject: [PATCH] Add checkout-strategy flag. This flag can be set to either branch (default) or merge. If set to branch, we will check out the head branch of the pull request (the source). If set to merge, we will check out the base branch of the pull request (the destination) and then attempt to perform a git merge of the pull request branch. This simulates what will happen if the pull request is merged. This allows us to perform terraform commands on what the state of the repo will be *after* the pull request is merged. This is useful if users are often opening up pull requests from branches that aren't up to date with master. With the branch strategy, the terraform plan might be deleting resources that have been created in the master branch but with the merge strategy, we merge the branch's changes into the master branch and so don't have this problem. --- cmd/server.go | 19 +++++++ cmd/server_test.go | 22 ++++++++ server/events/working_dir.go | 80 ++++++++++++++++++++++------ server/events_controller_e2e_test.go | 1 + server/server.go | 3 +- server/user_config.go | 1 + 6 files changed, 109 insertions(+), 17 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 0f4a82feb..3d873c5f9 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -44,6 +44,7 @@ const ( BitbucketUserFlag = "bitbucket-user" BitbucketWebhookSecretFlag = "bitbucket-webhook-secret" ConfigFlag = "config" + CheckoutStrategyFlag = "checkout-strategy" DataDirFlag = "data-dir" GHHostnameFlag = "gh-hostname" GHTokenFlag = "gh-token" @@ -64,6 +65,7 @@ const ( TFETokenFlag = "tfe-token" // Flag defaults. + DefaultCheckoutStrategy = "branch" DefaultBitbucketBaseURL = bitbucketcloud.BaseURL DefaultDataDir = "~/.atlantis" DefaultGHHostname = "github.com" @@ -103,6 +105,16 @@ var stringFlags = []stringFlag{ name: ConfigFlag, description: "Path to config file. All flags can be set in a YAML config file instead.", }, + { + name: CheckoutStrategyFlag, + description: "How to check out pull requests. Accepts either 'branch' (default) or 'merge'." + + " If set to branch, Atlantis will check out the source branch of the pull request." + + " If set to merge, Atlantis will check out the destination branch of the pull request (ex. master)" + + " and then locally perform a git merge of the source branch." + + " This effectively means Atlantis operates on the repo as it will look" + + " after the pull request is merged.", + defaultValue: "branch", + }, { name: DataDirFlag, description: "Path to directory to store Atlantis data.", @@ -372,6 +384,9 @@ func (s *ServerCmd) run() error { } func (s *ServerCmd) setDefaults(c *server.UserConfig) { + if c.CheckoutStrategy == "" { + c.CheckoutStrategy = DefaultCheckoutStrategy + } if c.DataDir == "" { c.DataDir = DefaultDataDir } @@ -397,6 +412,10 @@ func (s *ServerCmd) validate(userConfig server.UserConfig) error { if logLevel != "debug" && logLevel != "info" && logLevel != "warn" && logLevel != "error" { return errors.New("invalid log level: not one of debug, info, warn, error") } + checkoutStrat := userConfig.CheckoutStrategy + if checkoutStrat != "branch" && checkoutStrat != "merge" { + return errors.New("invalid checkout strategy: not one of branch or merge") + } if (userConfig.SSLKeyFile == "") != (userConfig.SSLCertFile == "") { return fmt.Errorf("--%s and --%s are both required for ssl", SSLKeyFileFlag, SSLCertFileFlag) diff --git a/cmd/server_test.go b/cmd/server_test.go index 1f827929a..7d49c9172 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -127,6 +127,14 @@ func TestExecute_ValidateLogLevel(t *testing.T) { Equals(t, "invalid log level: not one of debug, info, warn, error", err.Error()) } +func TestExecute_ValidateCheckoutStrategy(t *testing.T) { + c := setupWithDefaults(map[string]interface{}{ + cmd.CheckoutStrategyFlag: "invalid", + }) + err := c.Execute() + ErrEquals(t, "invalid checkout strategy: not one of branch or merge", err) +} + func TestExecute_ValidateSSLConfig(t *testing.T) { expErr := "--ssl-key-file and --ssl-cert-file are both required for ssl" cases := []struct { @@ -331,6 +339,7 @@ func TestExecute_Defaults(t *testing.T) { Ok(t, err) Equals(t, dataDir, passedConfig.DataDir) + Equals(t, "branch", passedConfig.CheckoutStrategy) Equals(t, "github.com", passedConfig.GithubHostname) Equals(t, "token", passedConfig.GithubToken) Equals(t, "user", passedConfig.GithubUser) @@ -432,6 +441,7 @@ func TestExecute_Flags(t *testing.T) { cmd.BitbucketTokenFlag: "bitbucket-token", cmd.BitbucketUserFlag: "bitbucket-user", cmd.BitbucketWebhookSecretFlag: "bitbucket-secret", + cmd.CheckoutStrategyFlag: "merge", cmd.DataDirFlag: "/path", cmd.GHHostnameFlag: "ghhostname", cmd.GHTokenFlag: "token", @@ -460,6 +470,7 @@ func TestExecute_Flags(t *testing.T) { Equals(t, "bitbucket-token", passedConfig.BitbucketToken) Equals(t, "bitbucket-user", passedConfig.BitbucketUser) Equals(t, "bitbucket-secret", passedConfig.BitbucketWebhookSecret) + Equals(t, "merge", passedConfig.CheckoutStrategy) Equals(t, "/path", passedConfig.DataDir) Equals(t, "ghhostname", passedConfig.GithubHostname) Equals(t, "token", passedConfig.GithubToken) @@ -489,6 +500,7 @@ bitbucket-base-url: "https://mydomain.com" bitbucket-token: "bitbucket-token" bitbucket-user: "bitbucket-user" bitbucket-webhook-secret: "bitbucket-secret" +checkout-strategy: "merge" data-dir: "/path" gh-hostname: "ghhostname" gh-token: "token" @@ -521,6 +533,7 @@ tfe-token: my-token Equals(t, "bitbucket-token", passedConfig.BitbucketToken) Equals(t, "bitbucket-user", passedConfig.BitbucketUser) Equals(t, "bitbucket-secret", passedConfig.BitbucketWebhookSecret) + Equals(t, "merge", passedConfig.CheckoutStrategy) Equals(t, "/path", passedConfig.DataDir) Equals(t, "ghhostname", passedConfig.GithubHostname) Equals(t, "token", passedConfig.GithubToken) @@ -550,6 +563,7 @@ bitbucket-base-url: "https://mydomain.com" bitbucket-token: "bitbucket-token" bitbucket-user: "bitbucket-user" bitbucket-webhook-secret: "bitbucket-secret" +checkout-strategy: "merge" data-dir: "/path" gh-hostname: "ghhostname" gh-token: "token" @@ -578,6 +592,7 @@ ssl-key-file: my-token "BITBUCKET_TOKEN": "override-bitbucket-token", "BITBUCKET_USER": "override-bitbucket-user", "BITBUCKET_WEBHOOK_SECRET": "override-bitbucket-secret", + "CHECKOUT_STRATEGY": "branch", "DATA_DIR": "/override-path", "GH_HOSTNAME": "override-gh-hostname", "GH_TOKEN": "override-gh-token", @@ -610,6 +625,7 @@ ssl-key-file: my-token Equals(t, "override-bitbucket-token", passedConfig.BitbucketToken) Equals(t, "override-bitbucket-user", passedConfig.BitbucketUser) Equals(t, "override-bitbucket-secret", passedConfig.BitbucketWebhookSecret) + Equals(t, "branch", passedConfig.CheckoutStrategy) Equals(t, "/override-path", passedConfig.DataDir) Equals(t, "override-gh-hostname", passedConfig.GithubHostname) Equals(t, "override-gh-token", passedConfig.GithubToken) @@ -639,6 +655,7 @@ bitbucket-base-url: "https://bitbucket-base-url" bitbucket-token: "bitbucket-token" bitbucket-user: "bitbucket-user" bitbucket-webhook-secret: "bitbucket-secret" +checkout-strategy: "merge" data-dir: "/path" gh-hostname: "ghhostname" gh-token: "token" @@ -667,6 +684,7 @@ tfe-token: my-token cmd.BitbucketTokenFlag: "override-bitbucket-token", cmd.BitbucketUserFlag: "override-bitbucket-user", cmd.BitbucketWebhookSecretFlag: "override-bitbucket-secret", + cmd.CheckoutStrategyFlag: "branch", cmd.DataDirFlag: "/override-path", cmd.GHHostnameFlag: "override-gh-hostname", cmd.GHTokenFlag: "override-gh-token", @@ -693,6 +711,7 @@ tfe-token: my-token Equals(t, "override-bitbucket-token", passedConfig.BitbucketToken) Equals(t, "override-bitbucket-user", passedConfig.BitbucketUser) Equals(t, "override-bitbucket-secret", passedConfig.BitbucketWebhookSecret) + Equals(t, "branch", passedConfig.CheckoutStrategy) Equals(t, "/override-path", passedConfig.DataDir) Equals(t, "override-gh-hostname", passedConfig.GithubHostname) Equals(t, "override-gh-token", passedConfig.GithubToken) @@ -723,6 +742,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) { "BITBUCKET_TOKEN": "bitbucket-token", "BITBUCKET_USER": "bitbucket-user", "BITBUCKET_WEBHOOK_SECRET": "bitbucket-secret", + "CHECKOUT_STRATEGY": "merge", "DATA_DIR": "/path", "GH_HOSTNAME": "gh-hostname", "GH_TOKEN": "gh-token", @@ -759,6 +779,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) { cmd.BitbucketTokenFlag: "override-bitbucket-token", cmd.BitbucketUserFlag: "override-bitbucket-user", cmd.BitbucketWebhookSecretFlag: "override-bitbucket-secret", + cmd.CheckoutStrategyFlag: "branch", cmd.DataDirFlag: "/override-path", cmd.GHHostnameFlag: "override-gh-hostname", cmd.GHTokenFlag: "override-gh-token", @@ -787,6 +808,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) { Equals(t, "override-bitbucket-token", passedConfig.BitbucketToken) Equals(t, "override-bitbucket-user", passedConfig.BitbucketUser) Equals(t, "override-bitbucket-secret", passedConfig.BitbucketWebhookSecret) + Equals(t, "branch", passedConfig.CheckoutStrategy) Equals(t, "/override-path", passedConfig.DataDir) Equals(t, "override-gh-hostname", passedConfig.GithubHostname) Equals(t, "override-gh-token", passedConfig.GithubToken) diff --git a/server/events/working_dir.go b/server/events/working_dir.go index b00ab31af..c25e08efb 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -14,6 +14,7 @@ package events import ( + "fmt" "os" "os/exec" "path/filepath" @@ -46,6 +47,11 @@ type WorkingDir interface { // FileWorkspace implements WorkingDir with the file system. type FileWorkspace struct { DataDir string + // CheckoutMerge is true if we should check out the branch that corresponds + // to what the base branch will look like *after* the pull request is merged. + // If this is false, then we will check out the head branch from the pull + // request. + CheckoutMerge bool // TestingOverrideCloneURL can be used during testing to override the URL // that is cloned. If it's empty then we clone normally. TestingOverrideCloneURL string @@ -67,11 +73,21 @@ func (w *FileWorkspace) Clone( // If so, then we do nothing. if _, err := os.Stat(cloneDir); err == nil { log.Debug("clone directory %q already exists, checking if it's at the right commit", cloneDir) - revParseCmd := exec.Command("git", "rev-parse", "HEAD") // #nosec + + // We use git rev-parse to see if our repo is at the right commit. + // If just checking out the pull request branch, we can use HEAD. + // If doing a merge, then HEAD won't be at the pull request's HEAD + // because we'll already have performed a merge. Instead, we'll check + // HEAD^2 since that will be the commit before our merge. + pullHead := "HEAD" + if w.CheckoutMerge { + pullHead = "HEAD^2" + } + revParseCmd := exec.Command("git", "rev-parse", pullHead) // #nosec revParseCmd.Dir = cloneDir output, err := revParseCmd.CombinedOutput() if err != nil { - log.Err("will re-clone repo, could not determine if was at correct commit: git rev-parse HEAD: %s: %s", err, string(output)) + log.Err("will re-clone repo, could not determine if was at correct commit: %s: %s: %s", strings.Join(revParseCmd.Args, " "), err, string(output)) return w.forceClone(log, cloneDir, headRepo, p) } currCommit := strings.Trim(string(output), "\n") @@ -105,22 +121,48 @@ func (w *FileWorkspace) forceClone(log *logging.SimpleLogger, return "", errors.Wrap(err, "creating new workspace") } - log.Info("git cloning %q into %q", headRepo.SanitizedCloneURL, cloneDir) - cloneURL := headRepo.CloneURL - if w.TestingOverrideCloneURL != "" { - cloneURL = w.TestingOverrideCloneURL - } - cloneCmd := exec.Command("git", "clone", cloneURL, cloneDir) // #nosec - if output, err := cloneCmd.CombinedOutput(); err != nil { - return "", errors.Wrapf(err, "cloning %s: %s", headRepo.SanitizedCloneURL, string(output)) + var cmds [][]string + if w.CheckoutMerge { + // NOTE: We can't do a shallow clone when we're merging because we'll + // get merge conflicts if our clone doesn't have the commits that the + // branch we're merging branched off at. + // See https://groups.google.com/forum/#!topic/git-users/v3MkuuiDJ98. + cmds = [][]string{ + { + "git", "clone", "--branch", p.BaseBranch, "--single-branch", p.BaseRepo.CloneURL, cloneDir, + }, + { + "git", "remote", "add", "head", headRepo.CloneURL, + }, + { + "git", "fetch", "head", fmt.Sprintf("+refs/heads/%s:", p.HeadBranch), + }, + { + "git", "merge", "-q", "-m", "atlantis-merge", "FETCH_HEAD", + }, + } + } else { + cloneURL := headRepo.CloneURL + if w.TestingOverrideCloneURL != "" { + cloneURL = w.TestingOverrideCloneURL + } + cmds = [][]string{ + { + "git", "clone", "--branch", p.HeadBranch, "--depth=1", "--single-branch", cloneURL, cloneDir, + }, + } } - // Check out the branch for this PR. - log.Info("checking out branch %q", p.Branch) - checkoutCmd := exec.Command("git", "checkout", p.Branch) // #nosec - checkoutCmd.Dir = cloneDir - if err := checkoutCmd.Run(); err != nil { - return "", errors.Wrapf(err, "checking out branch %s", p.Branch) + for _, args := range cmds { + cmd := exec.Command(args[0], args[1:]...) + cmd.Dir = cloneDir + + cmdStr := w.cmdAsSanitizedStr(cmd, p.BaseRepo, headRepo) + output, err := cmd.CombinedOutput() + if err != nil { + return "", errors.Wrapf(err, "running %s: %s", cmdStr, string(output)) + } + log.Debug("ran: %s. Output: %s", cmdStr, string(output)) } return cloneDir, nil } @@ -161,3 +203,9 @@ func (w *FileWorkspace) repoPullDir(r models.Repo, p models.PullRequest) string func (w *FileWorkspace) cloneDir(r models.Repo, p models.PullRequest, workspace string) string { return filepath.Join(w.repoPullDir(r, p), workspace) } + +func (w *FileWorkspace) cmdAsSanitizedStr(cmd *exec.Cmd, base models.Repo, head models.Repo) string { + cmdAsStr := strings.Join(cmd.Args, " ") + baseReplaced := strings.Replace(cmdAsStr, base.CloneURL, base.SanitizedCloneURL, -1) + return strings.Replace(baseReplaced, head.CloneURL, head.SanitizedCloneURL, -1) +} diff --git a/server/events_controller_e2e_test.go b/server/events_controller_e2e_test.go index 3de8e7b23..e42658151 100644 --- a/server/events_controller_e2e_test.go +++ b/server/events_controller_e2e_test.go @@ -425,6 +425,7 @@ func GitHubPullRequestParsed(headSHA string) *github.PullRequest { FullName: github.String("runatlantis/atlantis-tests"), CloneURL: github.String("/runatlantis/atlantis-tests.git"), }, + Ref: github.String("master"), }, User: &github.User{ Login: github.String("atlantisbot"), diff --git a/server/server.go b/server/server.go index 957bd2ec6..a21837692 100644 --- a/server/server.go +++ b/server/server.go @@ -182,7 +182,8 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { lockingClient := locking.NewClient(boltdb) workingDirLocker := events.NewDefaultWorkingDirLocker() workingDir := &events.FileWorkspace{ - DataDir: userConfig.DataDir, + DataDir: userConfig.DataDir, + CheckoutMerge: userConfig.CheckoutStrategy == "merge", } projectLocker := &events.DefaultProjectLocker{ Locker: lockingClient, diff --git a/server/user_config.go b/server/user_config.go index 6cf174c31..07bf60816 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -13,6 +13,7 @@ type UserConfig struct { BitbucketToken string `mapstructure:"bitbucket-token"` BitbucketUser string `mapstructure:"bitbucket-user"` BitbucketWebhookSecret string `mapstructure:"bitbucket-webhook-secret"` + CheckoutStrategy string `mapstructure:"checkout-strategy"` DataDir string `mapstructure:"data-dir"` GithubHostname string `mapstructure:"gh-hostname"` GithubToken string `mapstructure:"gh-token"`