palantir / policy-bot

A GitHub App that enforces approval policies on pull requests
Apache License 2.0
773 stars 106 forks source link

Request for Advice on Using Policy Bot in Open Source Projects for Testing, Approving, Merging of PRs #705

Open osterman opened 9 months ago

osterman commented 9 months ago

We came across policy-bot and eagerly deployed it without realizing we couldn't do what we had hoped to do.

We run a large Open Source GitHub organization with hundreds of repositories. We are a small business. Adding trusted contributors to our enterprise organization as outside collaborators or team members is cost-prohibitive (this is what we currently do).

We would like to instead follow in the footsteps of kubernetes/test-infra to implement a ChatOps style mechanism to approve PRs for testing and separately approve PRs for merging. Under the hood, this project uses prow.

We would like this solution to play well with GitHub Action triggers

Ideal Workflow

  1. Untrusted open-source PR is opened against one of our repos from a fork (never a branch in the repo)
  2. A vetted contributor inspects the PR. If it looks good, they comment /ok-to-test
  3. policy-bot receives the payload, and applies its advanced policy controls. Instead of approving the PR, it can be configured to comment on the PR with something like /terratest (or add a label, such as ok-to-test)
  4. GitHub Actions configured on the PR trigger on comments from the policy-bot or labels. Note, non-org members cannot add labels.
  5. Tests pass
  6. policy-bot awakens and approves the PR (as a CODEOWNER), so branch-protection rules now pass
  7. A vetted contributor now decides if this PR is mergeable, and then comments /ok-to-merge
  8. Something like bulldozer then auto-merges the PR.

Limitations

Alternatives considered

  1. Implement a native GHA solution. GHA suck for ChatOps. It's like talking to the Mars Lander. Commands take minutes to receive an acknowledgment.
  2. Trigger GHA on policy-bot status checks. GHAs can only trigger on status checks in the default branch, precluding it's use on PRs
  3. Write a custom bot using one of the plethora of GH bot frameworks. We would like not to have to maintain a custom solution.
  4. Deploy prow used by test-infra, however, the project feels overwhelming in complexity and doesn't seem to support GHA (only Jenkins), a requirement for us.
osterman commented 9 months ago

Another alternative we considered was Mergify, which is free for open-source, but limited on features. I believe the features we needed are not available in the open-source offering.

https://docs.mergify.com/

bluekeyes commented 9 months ago

As you discovered, Policy Bot was designed for overall/merge approval workflows and doesn't support the phased approval (approved for testing vs approved for merge) that your workflow requires. Given the limitations of GitHub Actions and Policy Bot, I don't think you can achieve what you want using only features that are available today.

That said, here's an idea that might work with one new Policy Bot feature:

  1. Implement the feature requested in #387, which is a way for Policy Bot to leave GitHub reviews instead of only posting status checks. Since this is a feature we don't have a use for internally, I'm not sure my team will have time to implement it, but I think it should be relatively straightforward to add and I'm happy to review a contribution or discuss the implementation.
  2. For your organization, run two instances of Policy Bot, the "Test" instance and the "Merge" instance
  3. The "Test" instance looks at a test-specific policy file and leaves an approving review when it is satisfied. You configure the GitHub branch protection and the "Merge" policy to ignore approvals from this app instance. Instead, this approval is the trigger condition for the GitHub Actions workflow.
  4. The "Merge" instance looks at a merge-specific policy file and approves the PR (either by leaving a review or via status check) when code review is complete and the PR is ready to merge

Running two instances of Policy Bot is awkward, but they can be configured to not conflict and it avoids adding more complicated features.

To achieve what you want with only a single instance, I think we'd have to add significant new features. We don't really have a concept of sub-policies or policies for different actions (the disapproval policy is an explicit special case) and I don't immediately see a place to hook in logic to leave comments or add labels when certain conditions are met.

osterman commented 9 months ago

Thanks @bluekeyes for the phenomenally prompt response. That's very helpful! Will discuss with the team what we should do, but the suggestion makes sense. Also, thanks for confirming our suspicion that we would need to deploy two bots for the reasons you mentioned.