Feature Request: Option to count skipped jobs in has_successful_status

[stevoland]

GitHub branch protection rules and rulesets count skipped jobs as "success". Given GitHub's underpowered required checks config, this "feature" is commonly used with path filtering to avoid running unnecessary jobs in monorepos etc.

We also use policy-bot to ping reviewers directly when a PR is in a reviewable state (including minimum checks pass).

As has_successful_status only counts "success" state we have had to consider these workarounds:

1. Only policy-bot as the required check

The path filtering is moved to the policies - expensive jobs run unnecessarily

2. Duplicate the conditions in policy-bot

High risk of the filtering logic getting out-of-sync

3. Additional job (chosen)

Add an additional job that is dependent on the conditional jobs and fails if any of the conditional job results is not "success" or "skipped". This job is referenced in has_successful_status.

Small extra expense, some additional complexity and confusing PR check summary.

Thanks for your time!

[bluekeyes]

Thanks for the suggestion. I can think of several ways this could be added - are any of these preferable (or objectionable) for your usage?

• As an option on the rule (in the options block)
• As an option on the has_successful_status precondition
• As a server-level configuration (i.e. "skipped" is considered success for all policies evaluated by the server)

[stevoland]

From my perspective, the first 2 are equal. Server-level configuration is less good - it's possible that there are teams in our org replying on the current behaviour although I'm not aware of a specific use-case for that.

Let me know if you would rather we contribute the change once you have a suggested approach. Thanks!

[iainlane]

We need this too at our organisation, so I had a go at implementing the second option in #789. Hope it helps!

[iainlane]

Just writing down our requirements since I discovered that what I thought this was was not quite correct 😁.

We've got a monorepo with a whole bunch of GitHub Actions workflows that run conditionally. Imagine quite a lot of workflows like this.

on:
  pull_request:
    branches:
      - main
    paths:
      - foo/**
      - bar/baz/*

jobs:
  a:
  b:
  c:

What I want us to be able to say is that if the workflow is run, then all jobs in it must pass.

What I implemented in #789 allows that to happen at the job level. So if you have a workflow like this

on:
  pull_request:
    branches:
      - main

job:
   a:
     if: something
     # ...

You could require that a must be successful or skipped. This works because GitHub will "enter" this kind of workflow for every pull request, and generate a "skipped" status for the jobs that don't run. We can find that in the API and then decide what to do with it (allow it).

But in the case of the first example that's not true. GitHub will entirely skip over the workflow and there is nothing that I've been able to find in any API which you can use to find this out directly.

The approach I've been exploring, which seems to work when I test it with a fork of Policy Bot with all these PRs merged in, is a combination of things:

1. Add a new predicate (I've called it has_workflow_result). This gets the conclusion of the entire workflow if it ran. It's a rollup of the status of each job. That's PR #794 (draft).
2. Use a script to generate a Policy Bot config for all of the required workflows, including path filters if there are any. That's making use of PR #796 (also draft) to be able to use the upstream types to generate the intermediate structure and then render it to YAML.

The config it generates looks like this:

# This file is generated by generate-policy-bot.yml.
# Do not edit directly. Run "make .policy-bot.yml" to update
policy:
  approval:
    - and:
        - .github/workflows/foo.yml succeeded
        - .github/workflows/bar.yml succeeded or skipped
        # This is so that the group succeeds if all the workflows have path filters. In that case we get "All rules were skipped. At least one rule must match."
        - default to approval
    - policy bot config is valid when modified
approval_rules:
  # A workflow which doesn't have any path filter
  - name: .github/workflows/foo.yml succeeded
    requires:
      conditions:
        has_workflow_result:
          workflows:
            - .github/workflows/foo.yml
  # A workflow which has path filters
  - name: .github/workflows/bar.yml succeeded or skipped
    if:
      changed_files:
        paths:
          # This is `foo/**` as a regex!
          - ^foo\/(?:(?:[^/]*(?:/|$))*)$
    requires:
      conditions:
        has_workflow_result:
          workflows:
            - .github/workflows/bar.yml
  - name: default to approval
  # Merged from a local config so we can have generated & non-generated policies and rules
  - name: policy bot config is valid when modified
    if:
      changed_files:
        paths:
          - ^\.policy\.yml
    requires:
      conditions:
        has_successful_status:
          statuses:
            - Validate Policy Bot Config

(feedback on this structure is appreciated; I'm still new at writing configs)

[bluekeyes]

@iainlane sorry, I missed this explanation of your goal when you posted it originally. Looking at it now, I wonder if something like this could work:

policy:
  approval:
    - policy bot config is valid when modified
    - or:
      - workflow bypass authorized
      - and:
        - workflow foo passed
        - workflow bar passed

approval_rules:
  - name: workflow foo passed
    if:
      has_status:
        conclusions: [action_required, cancelled, failure, neutral, success, skipped, stale, timed_out]
        statuses:
          - Workflow Foo
    requires:
      conditions:
        has_successful_status:
          statuses:
            - Workflow Foo

  - name: workflow bar passed
    if:
      has_status:
        conclusions: [action_required, cancelled, failure, neutral, success, skipped, stale, timed_out]
        statuses:
          - Workflow Bar
    requires:
      conditions:
        has_successful_status:
          statuses:
            - Workflow Bar

  - name: workflow bypass authorized
    requires:
      count: 2
      permissions: [admin]

  - name: policy bot config is valid when modified
    if:
      changed_files:
        paths:
          - ^\.policy\.yml
    requires:
      conditions:
        has_successful_status:
          statuses:
            - Validate Policy Bot Config

Assuming you trust your workflows to have the correct paths (and based on your original goal for not duplicating paths, I think you do), the basic idea here is:

• For each workflow status, create a rule that applies if that workflow results in any conclusion. I used a list of all conclusions, but we could support a special any value to simplify the config.
• If any status is present for a workflow, require that it is success. This leaves the rule pending if the workflow fails or has any other conclusion (but skips it if there's no workflow status at all.)
• Because workflow statuses only appear some time after opening the PR, the default rule requires approval instead of approving automatically. I chose to set this up so that two admins can override the workflow statuses, but you could do whatever made sense to you, including creating a rule that can never be satisfied. But I think having an emergency mechanism to override failing statuses if something goes wrong could be useful.

I haven't tested this, but I think it would work and save you from having to duplicate the file paths (and maybe generate the config at all, depending on the number of workflows you have.) The most important thing to keep in mind is the third point above: the fallback rule must not pass automatically, or it is very likely that the first policy-bot status will be a success before reverting to pending once GitHub Actions starts running workflows.