palantir / policy-bot

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

Bug: Bulldozer update merges invalidate approval #336

Open anindyameister opened 3 years ago

anindyameister commented 3 years ago

I'm using policy-bot v1.23.2 alongside bulldozer v1.13.2, working against github enterprise v3.0. Here's my .policy.yml

- name: Allowed paths
    if:
      only_changed_files:
        paths:
          - 'dev/.*'
      only_has_contributors_in:
        teams: ["myorg/myteam"]
    requires:
      count: 0
    options:
      ignore_update_merges: true
      invalidate_on_push: true
      ignore_commits_by:
        users: ["bulldozer[bot]"]

In a scenario where an update merge is done by bulldozer, this approval gets rejected, even though the commit before was in green. policy-bot detail ui shows below reason.

Contributor "bulldozer[bot]" does not meet the required membership conditions

Shouldn't ignore_commits_by directive prevent this ? Does only_has_contributors_in directive require that bulldozer be a member of myteam ?

anindyameister commented 3 years ago

Upon enabling debug log, these messages were seen in policy-bot log output.

"skipping rule, predicate of type *predicate.OnlyHasContributorsIn was not satisfied"
"rule evaluation resulted in skipped:\"Contributor \"bulldozer[bot]\" does not meet the required membership conditions\""
anindyameister commented 3 years ago

This may be the reason why ignore_commits_by was skipped

  "commit": {
    "sha": "XXXX",
    "node_id": "XXXX",
    "commit": {
      "author": {
        "name": "bulldozer[bot]",
        "email": "1018+bulldozer[bot]@users.noreply.example.com",
        "date": "2021-10-09T01:16:14Z"
      },
      "committer": {
        "name": "GitHub Enterprise",
        "email": "noreply@example.com",
        "date": "2021-10-09T01:16:14Z"
      },
......

However even after adding GitHub Enterprise into the users list, seeing the same behavior.

    options:
      invalidate_on_push: true
      ignore_update_merges: true
      ignore_commits_by:
        users: ["bulldozer[bot]", "GitHub Enterprise"]
anindyameister commented 2 years ago

Any update on this ?