Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
osterman opened this issue Feb 7, 2024 · 3 comments

Comments

@osterman
Copy link

osterman commented Feb 7, 2024

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

  • The major limitation seems that policy-bot cannot interact with PRs beyond approving them, and we cannot approve a PR without first running tests.
  • We cannot run tests automatically on untrusted Pull Requests from open-source forks due to the massive attack surface and the requirement that tests have access to secrets required to validate terraform code.

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
Copy link
Author

osterman commented Feb 7, 2024

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
Copy link
Member

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 [Question] - Add Review Approval Feature #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
Copy link
Author

osterman commented Feb 7, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants