Add support for merging to restricted branches

README.md

@@ -195,6 +195,7 @@ Anything that's contained between two `==COMMIT_MSG==` strings will become the
 commit message instead of whole pull request body.
 
 #### What if I don't want to put config files into each repo?
+
 You can add default repository configuration in your bulldozer config file.
 
 It will be used only when your repo config file does not exist.
@@ -214,9 +215,38 @@ which are to be expected, and others that may be caused by mis-configuring Bulld
 
 * Required status checks have not passed
 * Review requirements are not satisfied
-* The merge strategy configured in `.bulldozer.yml` is not allowed by your repository settings
-* Branch protection rules are preventing `bulldozer [bot]` from [pushing to the branch](https://help.github.com/articles/about-branch-restrictions/).
-  Unfortunately GitHub apps cannot be added to the list at this time.
+* The merge strategy configured in `.bulldozer.yml` is not allowed by your
+  repository settings
+* Branch protection rules are preventing `bulldozer[bot]` from [pushing to the
+  branch][push restrictions]. Unfortunately, GitHub apps cannot be added to
+  the list at this time, but there is [a workaround][] if you are running your
+  own Bulldozer server.
+
+[push restrictions]: https://help.github.com/articles/about-branch-restrictions/
+[a workaround]: #can-bulldozer-work-with-push-restrictions-on-branches
+
+#### Can Bulldozer work with push restrictions on branches?
+
+As mentioned above, GitHub Apps cannot be added to the list of users associated
+with [push restrictions][]. To work around this, you can:
+
+1. Use another app like [policy-bot](https://github.com/palantir/policy-bot) to
+   implement _approval_ restrictions as required status checks instead of using
+   push restrictions. This effectively limits who can push to a branch by
+   requiring changes to go through the pull request process and be approved.
+
+2. Configure Bulldozer to use a personal access token for a regular user to
+   perform merges in this case. The token must have the `repo` scope and the
+   user must be allowed to push to the branch. In the server configuration
+   file, set:
+
+   ```yaml
+   options:
+     push_restriction_user_token: <token-value>
+   ```
+
+   The token is _only_ used if the target branch has push restrictions enabled.
+   All other merges are performed as the normal GitHub App user.
 
 ## Deployment
 

bulldozer/merge.go

@@ -29,6 +29,8 @@ import (
 	"github.com/palantir/bulldozer/pull"
 )
 
+const MaxPullRequestPollCount = 5
+
 type Merger interface {
 	// Merge merges the pull request in the context using the commit message
 	// and options. It returns the SHA of the merge commit on success.
@@ -43,6 +45,7 @@ type CommitMessage struct {
 	Message string
 }
 
+// GitHubMerger merges pull requests using a GitHub client.
 type GitHubMerger struct {
 	client *github.Client
 }
@@ -72,8 +75,51 @@ func (m *GitHubMerger) DeleteHead(ctx context.Context, pullCtx pull.Context) err
 	return errors.WithStack(err)
 }
 
-const MaxPullRequestPollCount = 5
+// PushRestrictionMerger delegates merge operations to different Mergers based
+// on whether or not the pull requests targets a branch with push restrictions.
+type PushRestrictionMerger struct {
+	Normal     Merger
+	Restricted Merger
+}
+
+func NewPushRestrictionMerger(normal, restricted Merger) Merger {
+	return &PushRestrictionMerger{
+		Normal:     normal,
+		Restricted: restricted,
+	}
+}
+
+func (m *PushRestrictionMerger) Merge(ctx context.Context, pullCtx pull.Context, method MergeMethod, msg CommitMessage) (string, error) {
+	restricted, err := pullCtx.PushRestrictions(ctx)
+	if err != nil {
+		return "", err
+	}
+
+	if restricted {
+		zerolog.Ctx(ctx).Info().Msg("Target branch has push restrictions, using restricted client for merge")
+		return m.Restricted.Merge(ctx, pullCtx, method, msg)
+	}
+	return m.Normal.Merge(ctx, pullCtx, method, msg)
+}
+
+func (m *PushRestrictionMerger) DeleteHead(ctx context.Context, pullCtx pull.Context) error {
+	restricted, err := pullCtx.PushRestrictions(ctx)
+	if err != nil {
+		return err
+	}
+
+	// this is not necessary: the normal client should have delete permissions,
+	// but having the merge user also delete the branch is a better UX
+	if restricted {
+		zerolog.Ctx(ctx).Info().Msg("Target branch has push restrictions, using restricted client for delete")
+		return m.Restricted.DeleteHead(ctx, pullCtx)
+	}
+	return m.Normal.DeleteHead(ctx, pullCtx)
+}
 
+// MergePR spawns a goroutine that attempts to merge a pull request. It returns
+// an error if an error occurs while preparing for the merge before starting
+// the goroutine.
 func MergePR(ctx context.Context, pullCtx pull.Context, merger Merger, mergeConfig MergeConfig) error {
 	logger := zerolog.Ctx(ctx)
 
@@ -184,7 +230,7 @@ func MergePR(ctx context.Context, pullCtx pull.Context, merger Merger, mergeConf
 						return
 					}
 
-					logger.Debug().Msgf("Attempting to delete ref %s", ref)
+					logger.Info().Msgf("Attempting to delete ref %s", ref)
 					if err := merger.DeleteHead(ctx, pullCtx); err != nil {
 						logger.Error().Err(err).Msgf("Failed to delete ref %s on %q", ref, pullCtx.Locator())
 						return

bulldozer/merge_test.go

@@ -25,6 +25,24 @@ import (
 	"github.com/palantir/bulldozer/pull/pulltest"
 )
 
+type MockMerger struct {
+	MergeCount int
+	MergeError error
+
+	DeleteCount int
+	DeleteError error
+}
+
+func (m *MockMerger) Merge(ctx context.Context, pullCtx pull.Context, method MergeMethod, msg CommitMessage) (string, error) {
+	m.MergeCount++
+	return "deadbeef", m.MergeError
+}
+
+func (m *MockMerger) DeleteHead(ctx context.Context, pullCtx pull.Context) error {
+	m.DeleteCount++
+	return m.DeleteError
+}
+
 func TestCalculateCommitTitle(t *testing.T) {
 	defaultPullContext := &pulltest.MockPullContext{
 		NumberValue: 12,
@@ -78,3 +96,30 @@ func TestCalculateCommitTitle(t *testing.T) {
 		})
 	}
 }
+
+func TestPushRestrictionMerger(t *testing.T) {
+	normal := &MockMerger{}
+	restricted := &MockMerger{}
+	merger := NewPushRestrictionMerger(normal, restricted)
+
+	ctx := context.Background()
+	pullCtx := &pulltest.MockPullContext{}
+
+	_, _ = merger.Merge(ctx, pullCtx, SquashAndMerge, CommitMessage{})
+	assert.Equal(t, 1, normal.MergeCount, "normal merge was not called")
+	assert.Equal(t, 0, restricted.MergeCount, "restricted merge was incorrectly called")
+
+	_ = merger.DeleteHead(ctx, pullCtx)
+	assert.Equal(t, 1, normal.DeleteCount, "normal delete was not called")
+	assert.Equal(t, 0, restricted.DeleteCount, "restricted delete was incorrectly called")
+
+	pullCtx.PushRestrictionsValue = true
+
+	_, _ = merger.Merge(ctx, pullCtx, SquashAndMerge, CommitMessage{})
+	assert.Equal(t, 1, normal.MergeCount, "normal merge was incorrectly called")
+	assert.Equal(t, 1, restricted.MergeCount, "restricted merge was not called")
+
+	_ = merger.DeleteHead(ctx, pullCtx)
+	assert.Equal(t, 1, normal.DeleteCount, "normal delete was incorrectly called")
+	assert.Equal(t, 1, restricted.DeleteCount, "restricted delete was not called")
+}

config/bulldozer.example.yml

@@ -40,6 +40,9 @@ options:
   # The name of the application. This will affect the User-Agent header
   # when making requests to Github.
   app_name: bulldozer
+  # An optional personal access token associated with a normal GitHub user that
+  # is used to merge pull requests into protected branches with push restrictions.
+  push_restriction_user_token: token
   # Default repository config, the same as the config file described in README
   default_repository_config:
     merge:

pull/context.go

@@ -58,6 +58,10 @@ type Context interface {
 	// checks for the pull request.
 	RequiredStatuses(ctx context.Context) ([]string, error)
 
+	// PushRestrictions returns true if the target barnch of the pull request
+	// restricts the users or teams that have push access.
+	PushRestrictions(ctx context.Context) (bool, error)
+
 	// CurrentSuccessStatuses returns the names of all currently
 	// successful status checks for the pull request.
 	CurrentSuccessStatuses(ctx context.Context) ([]string, error)

pull/github_context.go

@@ -36,7 +36,7 @@ type GithubContext struct {
 	// cached fields
 	comments         []string
 	commits          []*Commit
-	requiredStatuses []string
+	branchProtection *github.Protection
 	successStatuses  []string
 }
 
@@ -160,19 +160,40 @@ func (ghc *GithubContext) Commits(ctx context.Context) ([]*Commit, error) {
 }
 
 func (ghc *GithubContext) RequiredStatuses(ctx context.Context) ([]string, error) {
-	if ghc.requiredStatuses == nil {
-		requiredStatuses, _, err := ghc.client.Repositories.GetRequiredStatusChecks(ctx, ghc.owner, ghc.repo, ghc.pr.GetBase().GetRef())
-		if err != nil {
-			if isNotFound(err) {
-				// Github returns 404 when there are no branch protections
-				return nil, nil
-			}
-			return ghc.requiredStatuses, errors.Wrapf(err, "cannot get required status checks for %s", ghc.Locator())
+	if ghc.branchProtection == nil {
+		if err := ghc.loadBranchProtection(ctx); err != nil {
+			return nil, err
 		}
-		ghc.requiredStatuses = requiredStatuses.Contexts
 	}
+	if checks := ghc.branchProtection.GetRequiredStatusChecks(); checks != nil {
+		return checks.Contexts, nil
+	}
+	return nil, nil
+}
 
-	return ghc.requiredStatuses, nil
+func (ghc *GithubContext) PushRestrictions(ctx context.Context) (bool, error) {
+	if ghc.branchProtection == nil {
+		if err := ghc.loadBranchProtection(ctx); err != nil {
+			return false, err
+		}
+	}
+	if r := ghc.branchProtection.GetRestrictions(); r != nil {
+		return len(r.Users) > 0 || len(r.Teams) > 0, nil
+	}
+	return false, nil
+}
+
+func (ghc *GithubContext) loadBranchProtection(ctx context.Context) error {
+	protection, _, err := ghc.client.Repositories.GetBranchProtection(ctx, ghc.owner, ghc.repo, ghc.pr.GetBase().GetRef())
+	if err != nil {
+		if isNotFound(err) {
+			ghc.branchProtection = &github.Protection{}
+			return nil
+		}
+		return errors.Wrapf(err, "cannot get branch protection for %s", ghc.Locator())
+	}
+	ghc.branchProtection = protection
+	return nil
 }
 
 func isNotFound(err error) bool {

pull/pulltest/mock_context.go

@@ -48,6 +48,9 @@ type MockPullContext struct {
 	RequiredStatusesValue    []string
 	RequiredStatusesErrValue error
 
+	PushRestrictionsValue    bool
+	PushRestrictionsErrValue error
+
 	SuccessStatusesValue    []string
 	SuccessStatusesErrValue error
 
@@ -102,6 +105,10 @@ func (c *MockPullContext) RequiredStatuses(ctx context.Context) ([]string, error
 	return c.RequiredStatusesValue, c.RequiredStatusesErrValue
 }
 
+func (c *MockPullContext) PushRestrictions(ctx context.Context) (bool, error) {
+	return c.PushRestrictionsValue, c.PushRestrictionsErrValue
+}
+
 func (c *MockPullContext) CurrentSuccessStatuses(ctx context.Context) ([]string, error) {
 	return c.SuccessStatusesValue, c.SuccessStatusesErrValue
 }

server/config.go

@@ -43,10 +43,12 @@ type LoggingConfig struct {
 }
 
 type Options struct {
-	AppName                 string            `yaml:"app_name"`
-	ConfigurationPath       string            `yaml:"configuration_path"`
-	DefaultRepositoryConfig *bulldozer.Config `yaml:"default_repository_config"`
-	ConfigurationV0Paths    []string          `yaml:"configuration_v0_paths"`
+	AppName                  string            `yaml:"app_name"`
+	ConfigurationPath        string            `yaml:"configuration_path"`
+	DefaultRepositoryConfig  *bulldozer.Config `yaml:"default_repository_config"`
+	PushRestrictionUserToken string            `yaml:"push_restriction_user_token"`
+
+	ConfigurationV0Paths []string `yaml:"configuration_v0_paths"`
 }
 
 func (o *Options) fillDefaults() {

server/handler/base.go

@@ -29,6 +29,8 @@ import (
 type Base struct {
 	githubapp.ClientCreator
 	bulldozer.ConfigFetcher
+
+	PushRestrictionUserToken string
 }
 
 func (b *Base) ProcessPullRequest(ctx context.Context, pullCtx pull.Context, client *github.Client, pr *github.PullRequest) error {
@@ -39,6 +41,15 @@ func (b *Base) ProcessPullRequest(ctx context.Context, pullCtx pull.Context, cli
 		return errors.Wrap(err, "failed to fetch configuration")
 	}
 
+	merger := bulldozer.NewGitHubMerger(client)
+	if b.PushRestrictionUserToken != "" {
+		tokenClient, err := b.NewTokenClient(b.PushRestrictionUserToken)
+		if err != nil {
+			return errors.Wrap(err, "failed to create token client")
+		}
+		merger = bulldozer.NewPushRestrictionMerger(merger, bulldozer.NewGitHubMerger(tokenClient))
+	}
+
 	switch {
 	case bulldozerConfig.Missing():
 		logger.Debug().Msgf("No bulldozer configuration for %q", bulldozerConfig.String())
@@ -53,7 +64,7 @@ func (b *Base) ProcessPullRequest(ctx context.Context, pullCtx pull.Context, cli
 		}
 		if shouldMerge {
 			logger.Debug().Msg("Pull request should be merged")
-			if err := bulldozer.MergePR(ctx, pullCtx, bulldozer.NewGitHubMerger(client), config.Merge); err != nil {
+			if err := bulldozer.MergePR(ctx, pullCtx, merger, config.Merge); err != nil {
 				return errors.Wrap(err, "failed to merge pull request")
 			}
 		}

server/server.go

@@ -66,6 +66,8 @@ func New(c *Config) (*Server, error) {
 	baseHandler := handler.Base{
 		ClientCreator: clientCreator,
 		ConfigFetcher: bulldozer.NewConfigFetcher(c.Options.ConfigurationPath, c.Options.ConfigurationV0Paths, c.Options.DefaultRepositoryConfig),
+
+		PushRestrictionUserToken: c.Options.PushRestrictionUserToken,
 	}
 
 	webhookHandler := githubapp.NewDefaultEventDispatcher(c.Github,