difficulty getting branch protection to work about app HOT 41 CLOSED

@travi I'm thinking maybe @bkeepers and his team have not actually deployed the new version of this app yet. The code is in master, but on the app page, you don't see the updated README here: https://github.com/apps/settings

GitHub

Build software better, together

GitHub is where people build software. More than 28 million people use GitHub to discover, fork, and contribute to over 85 million projects.

from app.

I'm also not seeing my test app for Protected Branches working. Something probably changed with the API. I might be able to get some time soon to jump back into it. Feel free to pull this repo down and debug yourself if you'd like, @travi.

from app.

@bkeepers are you able to check this please?

from app.

@jwsloan the newest version was deployed. https://github.com/apps/settings has it's own readme completely independent from this repo, which was not updated. It's most likely an API issue, and we'll gladly review PRs for this, but don't have the bandwith the look into it ourselves.

GitHub

Build software better, together

GitHub is where people build software. More than 28 million people use GitHub to discover, fork, and contribute to over 85 million projects.

from app.

Thank you very much for the reply, @hiimbex. I'll see what I can figure out, and try to get a PR in. If @travi or @davidjdixon have any time, I'd love some help with that, Maybe between the three of us, we can nail it down.

from app.

i'm open to helping out when i can find a bit of time, but i havent done any work with probot yet beyond using it. so, i'm not sure where to start to appropriate feedback about whether updates are on track toward being an appropriate fix. if you have specific things in mind that i could look into, i could probably help out with some guidance.

from app.

@travi I believe I've figured out the issue.

See this link: https://developer.github.com/v3/repos/branches/#get-required-status-checks-of-protected-branch

See the parts about "organization-owned" repos in these two sections:

For my non-org repo I've been testing with, here is what works:

  - name: 'master'
    # https://developer.github.com/v3/repos/branches/#update-branch-protection
    # Leaving this empty will disable branch protection for this branch.
    protection:
      # Required. Require status checks to pass before merging. Set to null to disable
      required_status_checks:
        # Required. Require branches to be up to date before merging.
        strict: true
        # Required. The list of status checks to require in order to merge into this branch
        contexts: []
      # Required. Enforce all configured restrictions for administrators. Set to true to enforce required status checks for repository administrators. Set to null to disable.
      enforce_admins: false
      # Required. Require at least one approving review on a pull request, before merging. Set to null to disable.
      required_pull_request_reviews:
        # Dismiss approved reviews automatically when a new commit is pushed.
        dismiss_stale_reviews: true
        # Blocks merge until code owners have reviewed.
        require_code_owner_reviews: false
      restrictions: null

Branches | GitHub Developer Guide

from app.

FYI, I wasn't able to get the branch protection settings working on an organization either. I know the API works as I manually ran it against my 200 repos when migrating back from GitLab.

I had been hoping to use probot/settings but since it didn't work in my test, I had to do it manually.

from app.

@nesl247 can you share the config you used? What I posted does work for me.

from app.

repository:
  # See https://developer.github.com/v3/repos/#edit for all available settings.

  # The name of the repository. Changing this will rename the repository
  name: test

  # A short description of the repository that will show up on GitHub
  description: description of repo

  # A URL with more information about the repository
  homepage: https://example.github.io/

  # Either `true` to make the repository private, or `false` to make it public.
  private: true

  # Either `true` to enable issues for this repository, `false` to disable them.
  has_issues: true

  # Either `true` to enable the wiki for this repository, `false` to disable it.
  has_wiki: true

  # Either `true` to enable downloads for this repository, `false` to disable them.
  has_downloads: true

  # Updates the default branch for this repository.
  default_branch: master

  # Either `true` to allow squash-merging pull requests, or `false` to prevent
  # squash-merging.
  allow_squash_merge: true

  # Either `true` to allow merging pull requests with a merge commit, or `false`
  # to prevent merging pull requests with merge commits.
  allow_merge_commit: true

  # Either `true` to allow rebase-merging pull requests, or `false` to prevent
  # rebase-merging.
  allow_rebase_merge: true

# Labels: define labels for Issues and Pull Requests
labels:
  - name: bug
    color: CC0000
  - name: feature
    color: 336699
  - name: first-timers-only
    # include the old name to rename and existing label
    oldname: Help Wanted

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
        # Blocks merge until code owners have reviewed.
        require_code_owner_reviews: true
        # Specify which users and teams can dismiss pull request reviews. Pass an empty dismissal_restrictions object to disable. User and team dismissal_restrictions are only available for organization-owned repositories. Omit this parameter for personal repositories.
        dismissal_restrictions:
          users: []
          teams: []
      # Required. Require status checks to pass before merging. Set to null to disable
      required_status_checks:
        # Required. Require branches to be up to date before merging.
        strict: true
        # Required. The list of status checks to require in order to merge into this branch
        contexts: []
      # Required. Enforce all configured restrictions for administrators. Set to true to enforce required status checks for repository administrators. Set to null to disable.
      enforce_admins: false
      # Required. Restrict who can push to this branch. Team and user restrictions are only available for organization-owned repositories. Set to null to disable.
      restrictions:
        users: []
        teams: []

from app.

i was just able to get a personal (non-org) public project as well as a private project under an org to work using the following config: https://github.com/travi/scaffolder-sub-command/blob/9b91d50159bfcc7495c5abe8ec286f54b9f1f9f8/.github/settings.yml#L30-L36, which results in the following:

i did confirm that no protected branches were configured before making the change to the .github/settings.yml file, so it was the probot plugin that made the change. it does appear that something changed between my initial attempt with these same settings and now because what just worked did not work at that point.

either way, this satisfies my common need, so thank you everyone for getting this in place!

from app.

I just figured out what was breaking in the config I posted. The required review count is in preview, and does not work in this bot yet. It may have been copied into the README by accident.

from app.

Good catch, @nesl247! Any chance you'd be willing to put in a PR to fix the README?

from app.

Sure

from app.

Done in #92. I don't know if there is any other issues with branch protection right now, so I didn't mark it as fixing this issue.

from app.

It turns out I'm having trouble getting this working as well. The I cannot get dismissal_restrictions or restrictions working. Have you been able to, @nesl247 ?

from app.

@jwsloan Can you give an example as to what is not working for you? I don't currently use the bot, only started investigating being able to use it.

While testing I wasn't putting anyone in dismissal_restrictions or restrictions, just leaving it as empty arrays.

I will say the rest of the schema looks good to me, so I don't see why it wouldn't work unless you may have an issue in your config.

from app.

@nesl247 try putting anything in the teams array for either type of restriction.

from app.

@jwsloan I will give it a shot. While I am, can you confirm you have all the permissions granted for proobot/settings? If you did it prior to any permission changes, it wouldn't work.

It specifically would require the read members permission.

from app.

waiting on #92 or #98 to get merged, but for now, the example provided in the README is not going to work for users, maybe update the README until one of those two are merged?

chased my tail this morning trying to figure this out, until I found this issue thread.

from app.

Looks like #98 has been merged & deployed.

from app.

@TheKevJames that is correct! Adding Teams to a repository and setting their permissions does not work. However, you can use teams for restrictions within Branch Protection. They just have to already exist on the repo.

from app.

Unfortunately, #98 did not quite fix this issue. I still had a hard time getting it to work in our org repo...

And it is because dismissal_restrictions and restrictions behave differently.

See here:

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

One requires an empty object to disable, and the other requires a null field. The comments in the README actually say that, but the app should probably not require people to read that closely.

from app.

+1 I can't get branch protection to work either, I've enabled it on all repositories in my organization but only added settings file to one repo. Any help would be appreciated.

EDIT: It's not just branch protection, it doesn't seem like any of the settings (for example squash/merge/rebase settings) aren't being applied either 😕

from app.

@hassankhan, I would suggest pulling the app down locally, and running it yourself. Temporarily remove the hosted version from your app, and install your local one. Then you can watch the logs. There are instructions on how to do that in the repo.

Otherwise, you could create a test repo, and start removing portions of that settings yml until it works. That would help you figure out which piece is broken.

from app.

@jwsloan thanks for the advice, will give it a shot 👍

EDIT: All working now, thanks again @jwsloan 😄

from app.

I'm using a private repo which is an organization owned repo ( not using the Settings app ).
I've checked all the boxes in the "Branch protection rule" added myself and other teams

Still, we can commit to master. Its not working - is this available only trough the Settings app? Any idea why just using the UI it doesn't work? I'm puzzled.

Thanks for your help

from app.

@antoniogomezalvarado It might be helpful to check the default visibility of your organization's users. If the repo settings allow administrators to override branch protection, and all your organization's users are admins, then branch protection would never apply.

from app.

Thanks @hassankhan. I've checked, some are indeed Admins, however, the majority are members - yet they can commit directly to the master branch, even after the rule is applied. Funny fact - this happens only committing via the UI, if they use the CLI - it will fail because the branch is protected.

from app.

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

from app.

Still an issue

from app.

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

from app.

from app.

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

from app.

from app.

@travi, it looks like you can close this one.

from app.

I'm still not getting branch protection with an astrix working

something like stable/* which should protect all branches starting with stable/, e.g. stable/0.20.0, stable/1.0.0.

This is working fine when I'm entering it directly into GitHub UI. Config: https://github.com/bincrafters/templates/blob/f261e3094f86fc209d1f77c9243e565718d1d06b/.github/settings.yml#L14

GitHub

bincrafters/templates

Collection of current templates used in Conan recipes - bincrafters/templates

from app.

When you say "directly into the GitHub API", do you mean the web interface? Or you're actually able to hit the API with something like Postman and have that work?

from app.

Sorry, I got confused at that point 🙄

I meant, I can successfully create and use such a branch protection rule when entered via user interface.

I haven't tested a rule like that directly via API, but Probot doesn't seem to be able to handle it. No branch protection rule is being created via Probot by the config linked above. The other settings, like disabling issues and wikis do work however. So Probot works for us in general.

from app.

No worries. If you'd be willing to prove out whether or not the API can handle that request, I'd be willing to make the updates necessary here for it to work.

from app.

@Croydon would you mind opening a new issue for the trouble that you have been seeing?

i do think that the original problem is resolved, since it has been working well for my projects for a while now. i'm going to close this issue in favor of more focused issues for anything that might remain.

from app.