palantir / policy-bot

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

[Question] - Add Review Approval Feature #387

Open shresthaujjwal opened 2 years ago

shresthaujjwal commented 2 years ago

I would like to add a new feature where the bot would be able to add a review approval in the PR if all the policy checks is passed. Would you be able open for this contribution ?

derekjobst commented 2 years ago

Hi @shresthaujjwal,

Could you describe the workflow(s) you are hoping to enable with this potential feature? In what way would a review approval enable functionality beyond what can be done with a required status check?

Thanks!

shresthaujjwal commented 2 years ago

@derekjobst basically we have branch protection enabled in all our repository which requires atleast one approval in order for us to be able to auto-merge. For auto-merge, we would be using bulldozer. So we plan on installing policy bot and bulldozer bot to all the repository. Add this feature to policy bot would help bulldozer bot to automerge the pr. That is why i wanted to contribute and add this functionality to policy so that if all policy checks passes it would also approve the pr.

Let me know if this helps.

shresthaujjwal commented 2 years ago

@derekjobst any questions or concerns with this ?

asvoboda commented 2 years ago

Generally we suggest making policy-bot the blocking status check (along with other test statuses as required for your use case). Once a user reviews, policy-bot posts a status rather than a "Github Approval". Bulldozer can then auto-merge without issue.

derekjobst commented 2 years ago

Specifically, you can enforce the "one approval" requirement with a polcy-bot rule rather than the native GitHub branch protection.

shresthaujjwal commented 2 years ago

@derekjobst @asvoboda our use case is little different. Not all prs qualify for automerge. Only few prs that qualify for automerge if all criteria fits. For other prs that don't qualify they need peer review and approval that is our SOD policy (Segregation of Duties). But since I can only have 1 rule in branch protection for that branch the prs that qualify for automerge also need approval that is why i am looking into adding this feature in policy bot. Hope that clarifies my concern

shresthaujjwal commented 2 years ago

@derekjobst @asvoboda any question or concern with this ? Also can you share the policy configuration example for you can enforce the "one approval" requirement with a polcy-bot

asvoboda commented 2 years ago

You can write complex logical AND OR statements in policy bot, combining rules based around authorship, files modified, and other properties of the pull request. Take a look at the examples in

https://github.com/palantir/policy-bot/blob/develop/config/policy-examples/complicated.yml

You can change the required value to 0 to create a scenario where policy bot immediately approves with human intervention.

shresthaujjwal commented 2 years ago

thanks @asvoboda we will try out few configuration and keep you posted

djahandarie commented 11 months ago

@asvoboda Another potential usage for this functionality:

Some external policies (e.g., OSSF Scorecard) or organizational policies (e.g., GitHub rulesets) require repos to have branch protections enabled, with approvals required.

I'd still like to be able to use policy-bot in such cases (since it can be configured to be much more strict and effective than native GitHub Reviews). So, in addition to the passing status check, it'd be nice if it was (configurable) for it to also leave a GitHub Review approval to satisfy such restrictions imposed by external governance bodies.