Branch protection not being applied

[octonato]

None of the repos where I have branch protection configured gets the settings applied.

I did two different tests without success.

1. new repo without any rule, rules are not created
2. existing repo with rule for master, settings are not applied

Initially, I thought that this could be related to #136. That's why I did test num 2.

Test were done on this repo:
https://github.com/renatocaval/oh-my-git/
https://github.com/renatocaval/oh-my-git/blob/master/.github/settings.yml#L13-L23

[travi]

we should probably hide these details if true, but it looks like i had to set some properties that i wasn't trying to configure to null in order to get my config to apply.

could you try some variations of that to see if gets you working? if that does end up working, i would be interested in the combination that worked as well as those that didnt so that we can try to improve configuring these.

[octonato]

You are right. It works if all top-level settings are included.

Thanks for the tip.

That's my current one:

branches:
  - name: master
    # https://developer.github.com/v3/repos/branches/#update-branch-protection
    # Branch Protection settings. Set to null to disable
    protection:
      # Required. Require at least one approving review on a pull request, before merging. Set to null to disable.
      required_pull_request_reviews:
        # The number of approvals required. (1-6)
        required_approving_review_count: 1
        # Dismiss approved reviews automatically when a new commit is pushed.
        dismiss_stale_reviews: true
      required_status_checks: null
      restrictions: null
      enforce_admins: null

I won't work if I set then to false like in:

branches:
  - name: master
    # https://developer.github.com/v3/repos/branches/#update-branch-protection
    # Branch Protection settings. Set to null to disable
    protection:
      # Required. Require at least one approving review on a pull request, before merging. Set to null to disable.
      required_pull_request_reviews:
        # The number of approvals required. (1-6)
        required_approving_review_count: 1
        # Dismiss approved reviews automatically when a new commit is pushed.
        dismiss_stale_reviews: true
      required_status_checks: false
      restrictions: false
      enforce_admins: false

[travi]

glad it got you unblocked. thanks for following up with the extra info. that'll be a big help when we get a chance to improve that situation.

if you'd be interested in sending a PR to improve this, thats always appreciated and i'd be happy to help where you might need it.

[octonato]

We are now one huge release cycle involving many projects so I don't expect any bandwith before at least end November. Otherwise, I would be willing to help.

On the other hand, with a workaround, it makes it less urgent as well.

[travi]

not a problem. i'll leave the issue open until someone can clean that up and so others can hopefully find the workaround in the mean time.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[johnmartel]

Sorry for the noisy reference, I was trying to make this work, but was not actually able to, even after trying all possible combinations. I ended up with the following, but still not working:

branches:
  - name: master
    protection:
      required_pull_request_reviews:
        required_approving_review_count: 1
        dismiss_stale_reviews: true
        require_code_owner_reviews: true
        dismissal_restrictions: {}
      required_status_checks:
        strict: true
        contexts: ['YAML lint']
      enforce_admins: null
      restrictions: null

The status check context I got from the API itself at https://api.github.com/repos/john-badass-test-org/.github/branches/master/protection/required_status_checks. The response is simple enough:

{
    "url": "https://api.github.com/repos/john-badass-test-org/.github/branches/master/protection/required_status_checks",
    "strict": true,
    "contexts": [
        "YAML lint"
    ],
    "contexts_url": "https://api.github.com/repos/john-badass-test-org/.github/branches/master/protection/required_status_checks/contexts"
}

Any idea would be appreciated while I investigate a little further to see if I can get to the root of this.

[johnmartel]

To my previous comment, this does not seem to be working for my .github repository, but does work for other repositories in the same org, even through extensible configuration used in a template repository.

[travi]

is branch protection working in your .github repo for the other settings. i notice that the one difference in the config for that repo vs your _opensource.yml config is the addition of the YAML lint context. I'm curious if it would work with that removed.

[johnmartel]

I force-pushed a few attempts to my .github repo, as you can see here and I’m pretty sure I tried without the context (along other combinations), but honestly, I may have missed one (that’s the bad part about force-pushes: you don’t get to see what you overwrote). Let me try that before I get to work tomorrow morning on a new test org and confirm here if it works. I will also try updateBranchProtection direct API call for a .github repo and report back.

[travi]

in case it is helpful, here is a config that works on one of my .github repositories: https://github.com/form8ion/.github/blob/b443bce9aa99a29d2b292043b963c8232158836c/.github/settings.yml#L9-L14

it has been a while since i configured that one and the config is simpler than you are attempting, but that one did work. there shouldn't be anything unique about configuring the .github repo vs others.

we still have some steps to get to a point where troubleshooting issues like this are simpler, but i'm hoping we can identify what is preventing your config from working.

GitHub

form8ion/.github

organization level configuration repository. Contribute to form8ion/.github development by creating an account on GitHub.

[johnmartel]

@travi My original config works on a new org, both with/without the required checks context.
See this example without the context and this one with the context. Both configurations are now applied correctly.

I wonder if the previous behavior I observed was due to #221 ?

EDIT Actually, this is not quite my original config. I removed:

...
collaborators:
...
  - username:
    permission: pull

which was taken from an example I can't find back. Might be the culprit...

[travi]

I wonder if the previous behavior I observed was due to #221 ?

seems unlikely since a failure there would still be a failure no matter how that handling was happening

Might be the culprit...

the missing username would likely result in an error, so that would make sense to me

regardless, glad to hear that it is working for you now

[ahasna]

I might be coming late to this party. But after few hours of against-the-wall-head-banging debugging I realised that branch protection rules only work if you have an actual matching pattern in your repo.

[mattwynne]

I can't get this to work either. I'd love some visibility of what's happening when it tries to apply my yaml!

[Shahard2]

Anyone have solution for this ?

[szatanjl]

Hello. I am trying to make settings.yml branch protection work too. No luck unfortunately. I have even tried the absolute minimum configuration. Still nothing happens. I don't know why. Other settings and labels work.

branches:
  - name: main
    protection:
      required_pull_request_reviews: null
      required_status_checks: null
      enforce_admins: null
      restrictions: null

Doesn't work ^

curl -L \                
  -X PUT \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer ..."\
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/szatanjl/.../branches/main/protection \
  -d '{"required_pull_request_reviews": null, "required_status_checks": null, "enforce_admins": null, "restrictions": null}'

Works ^

Edit: Today it just worked (TM). I don't know why, but it sometimes works, sometimes doesn't.

[travi]

as i've mentioned in other threads, configuring branch protection does still work as long as there are no errors in the config and no other errors occurred when the app attempted to apply earlier changes through the api.

#518 needs to be implemented to provide feedback about failures to give a better chance to resolve config problems, but i can't provide an ETA for getting that implemented at this point