Required status check from repo settings are not taken into account

[martinm82]

Problem Description

What is actually happening

I have a suborg and repo setting file. The suborg settings file contains settings that the repo setting is supposed to override.

suborg setting:

suborgrepos:
  - my-*

branches:
  - name: default
    protection:
      required_pull_request_reviews:
        required_approving_review_count: 1
        dismiss_stale_reviews: true
        require_code_owner_reviews: true
        dismissal_restrictions: {}
        bypass_pull_request_allowances:
          users: []
          teams: []
          apps:
            - azure-pipelines
        require_last_push_approval: false
      required_status_checks:
        strict: true
        checks:
          - context: validate
      enforce_admins: false
      required_linear_history: true
      allow_force_pushes: false
      allow_deletions: false
      block_creations: true
      required_conversation_resolution: true
      allow_fork_syncing: false
      restrictions: null

repo setting my-repo.yml:

branches:
  - name: default
    protection:
      required_status_checks:
        strict: true
        checks: []

What is the expected behavior

I expect that the status check on my-repo is empty since checks is set to an empty list.

Error output, if available

Unfortunately the status check still contains the validate check in the branch protections.

Context

Are you using the hosted instance of probot/settings or running your own?

Hosted

If running your own instance, are you using it with github.com or GitHub Enterprise?

GitHub.com

Version of probot/settings

latest

Version of GitHub Enterprise

[martinm82]

After further debugging I have the impression that somehow the behaviour changed in how the settings are merged. I remember that some time ago when the org or suborg setting contained for example some status checks and one was trying to override the status checks in a repo one needed to copy&paste the checks from the parents in order to preserve them in for example the repo settings.

Example:

suborg

required_status_checks:
  strict: true
  checks: 
    - context: test

In order to extend the suborg checks in repo one needed to copy manually the ones from the suborg:

required_status_checks:
  strict: true
  checks: 
    - context: test
    - context: specific-test

But now this behaviour actually changed and checks from parent are always merged into the repo settings (and probably the same when defining on org and suborg level).

With this behaviour it is not possible to completely disable status checks in a child config as the ones from the parent will always be added to the child.

[martinm82]

One more issue discovered:

When defining for example bypass allowances on org level:

branches:
  - name: default
    protection:
      required_pull_request_reviews:
        required_approving_review_count: 1
        require_code_owner_reviews: true
        bypass_pull_request_allowances:
          apps:
            - my-app

and having the same defined in a suborg:

branches:
  - name: default
    protection:
      required_pull_request_reviews:
        required_approving_review_count: 2
        dismiss_stale_reviews: true
        bypass_pull_request_allowances:
          users: []
          teams: []
          apps: 
            - my-app
        require_last_push_approval: false

The REST API request is send with two apps apps: ["my-app", "my-app"] and ignored on GitHub side.

[martinm82]

The issue is even worse. If the suborg setting contains an empty checks array, you are not able to provide status checks on repository level.

@decyjphr @anderssonjohan is this something you as well observe? Or am I completely wrong and this behaviour was always like that?

[adrianschmidt]

@martinm82 I work with @anderssonjohan, and I can confirm that we see the same behaviour of required status checks from the suborg not being possible to override from the repo specific config file.

As long as both the suborg and the repo config sets an array with some checks, those arrays seem to be merged, and all the specified checks applied. I haven't tried what happens if the suborg settings uses an empty array or sets checks to null.

In one of our repos, I've worked around the problem by adding a few jobs like this:

  lint:
    name: Lint
    runs-on: ubuntu-latest
    steps:
      - run: echo 'this is a dummy'

Not exactly brilliant… 😞

[adrianschmidt]

Now I have tried with an empty array for the suborg, and then some checks configured for the different repos, and that works just fine. It's just a lot of repetition, since most of the repos have the same checks required, and there's only two repos that deviate 😒