When using protected branches (1 peer review) auto fails

[jrschumacher]

Describe the bug

We have protected branches branches turned on to ensure
To Reproduce

Add "Require pull request reviews before merging"

Expected behavior

Auto should deploy, bypassing the branch protections.

Screenshots

https://github.com/virtru/virtuoso-design-system/runs/1023167159?check_suite_focus=true#step:6:181

Error: Running command 'git' with args [push, --follow-tags, --set-upstream, ***github.com/virtru/virtuoso-design-system, master] failed

remote: error: GH006: Protected branch update failed for refs/heads/master.        
remote: error: At least 1 approving review is required by reviewers with write access.        
To https://github.com/virtru/virtuoso-design-system
 * [new tag]         v1.6.2 -> v1.6.2
 ! [remote rejected] master -> master (protected branch hook declined)

Environment information:

https://github.com/virtru/virtuoso-design-system/blob/master/package.json#L78

Additional context

Settings:

[jrschumacher]

Seems like the only way around this is to use the Github API to do the merge:
https://github.jparrowsec.cnmunity/t/allowing-github-actions-bot-to-push-to-protected-branch/16536

[hipstersmoothie]

Another option is to use a PAT instead of the gh action's token.

[jrschumacher]

We are using a PAT. We were using the wrong token 😅

[hipstersmoothie]

I 🙏 it works lol

[jrschumacher]

Nope 😢 https://github.com/virtru/virtuoso-design-system/runs/1024339602?check_suite_focus=true#step:6:189

[vincentbriglia]

This isn't really an Auto issue but rather a thought-out limitation of Github Actions:

• https://github.jparrowsec.cnmunity/t/allowing-github-actions-bot-to-push-to-protected-branch/16536/2

workarounds:

• https://github.com/benjefferies/branch-protection-bot
• https://github.com/ridedott/merge-me-action

[jrschumacher]

Yes I agree that it isn't autos issue, but given auto have a deep relationship with Github it should be considered.

I like the automated merge PR. Seems like it's something that an auto plugin could support? If protected branches is enabled then create a release PR rather than pushing to master. Additionally, it could utilize the API to use the associated admin permissions granted through the PAT to merge the release PR.

[hipstersmoothie]

If protected branches is enabled then create a release PR rather than pushing to master.

Currently this would be a little more work than that. Each plugin is responsible for pushing the changlog/version commits. To make this pluggable we would need:

1. to move that behavior into core
2. make a new hook for pushing the release
3. make a plugin that uses that hook to do what you described

---

I kind of like what https://github.com/benjefferies/branch-protection-bot does. this would be easier to do via a plugin:

1. Turn off branch protection during the afterVersion hook
2. Turn on branch protection during the afterPublish hook

Just gotta find the right octokit API

[jrschumacher]

It surely will work, but it seems precarious and potentially problematic for orgs that have strict controls around branch protection.

I scanned through the plugins and I see what you mean. Instead of moving the changelog / version to core what if there were different types of plugins. For instance if the conventional-commit plugin was broken into two plugin types: conventional-commit-changelog and conventional-commit-version. If I want to use conventional-commit changelogs, but use github labels for my versioning I can.

Possibly, a much larger conversation, but I'd be interested in assisting in developing a more robust solution which would satisfy various org requirements.

[hipstersmoothie]

It surely will work, but it seems precarious and potentially problematic for orgs that have strict controls around branch protection

And if it the publish were to fail the branch protection rules would be lost 😢

---

The work to move the tag pushing into core looks like:

1. Move all git push --tags calls from publish hooks to after this line

   Examples of git push --tags calls:

   https://github.com/intuit/auto/blob/master/plugins/npm/src/index.ts#L1172-L1179
   https://github.com/intuit/auto/blob/master/plugins/cocoapods/src/index.ts#L163-L169
   https://github.com/intuit/auto/blob/master/plugins/gradle/src/index.ts#L221-L227

   Looking at this more it might be tricky finding all the places where we would push a commit to the baseBranch to switch to the PR flow you described.

2. Create a new hook called something like push that's a bail hook

3. Set up the default behavior to just do the push

4. Create a plugin (as a package or internally) that overrides the default behavior and makes the PR

---

So I'm gonna close the PR I made to do this in a plugin as it seems to brittle. I'd be willing to accept a PR for this but it also seems like a pretty big lift. Although I had planned on moving the tag pushing into core at some point to facilitate #917

Possibly, a much larger conversation, but I'd be interested in assisting in developing a more robust solution which would satisfy various org requirements.

feel free to discuss this here!

[hipstersmoothie]

I've made changes to my PR that will re-enable the Peer Reviews even when something in auto throws an error. So this should be a little less brittle now.

So the only way this wouldn't work now is if:

• you don't have permission to change this setting
• you hit the "merge quickly" window perfectly (low chance and i'm pretty sure the first one would abort before any weirdness)

---

As for the PR method you are describing. It goes pretty far out of the way of auto's normal workflow. The biggest pain point I can imagine is that since you would be pushing tags to branches there would be a possibility of multiple PR merges creating branches with conflicting tags. We could write code that makes sure we only have 1 of those PRs up at a time but I feel like there would be some edge case weirdness to deal with.

---

What's weird is that this works fine for me on GitHub enterprise...

[hipstersmoothie]

On enterprise we use a dummy "bot" account with admin permissions to do this.

Locally on my test project i've made a key with all permissons and it still doesn't work at all.

[jrschumacher]

That is strange. We don't use Enterprise at my org, but I have experienced the same issue on our (teams?) account despite being an Admin or using our dummy account which is also an admin.

I need to look at how our Buildkite jobs do this. Maybe they found a trick that works.

[jrschumacher]

I'm going to formalize my thoughts and post about it tomorrow. Thanks for your attentiveness and support.

[hipstersmoothie]

All sources point to a token from an admin with the right permissions should fix this. I'm trying this on this repo to no avail. what's weird is everything works fine locally using the same exact token. I can push directly to master but the same token doesn't work in CI

[hipstersmoothie]

I'm in contact with someone who works at github and there might be a fix to this coming out at some point

[bahrmichael]

I've just stumbled upon this problem and am wondering if there has been any progress. If not, I'd be happy to extend the troubleshooting section.

[airtonix]

@hipstersmoothie you were half way there:

• have to be on enterprise org, because there you can now...
• describe which groups/users can skip branch protection rules
• use a seat license to create a service account
• on that service account create a PAT that has all required scopes to make changes repo:write, packages, workflows (if your bot is rewriting workflows), etc etc.

for us peasants on anything less than enterprise we have to forage for clams and fabricobble our paper towels into the eternal diaper.

[throttlehead-dev]

To anyone who ends up here searching this problem, I thought I'd share our workflow:

name: auto-release

on:
  push:
    branches:
      - main

jobs:
  release:
    runs-on: ubuntu-latest

    if: "!contains(github.event.head_commit.message, 'skip ci') && !contains(github.event.head_commit.message, 'skip ci')"

    steps:
      - name: Checkout
        uses: actions/checkout@v2
        with:
          token: ${{ secrets.GH_PAT }}

      - name: Fetch tags
        run: git fetch --unshallow --tags

      - name: Setup node
        uses: actions/setup-node@v3
        with:
          node-version: '16.x'
          token: ${{ secrets.NPM_TOKEN_PUBLISH }}

      - name: Npm install
        run: |
          npm i

      - name: Build
        run: |
          npm run build

      - name: Temporarily allow force pushes
        uses: benjefferies/branch-protection-bot@master
        if: always()
        with:
          access_token: ${{ secrets.GH_PAT }}
          branch: ${{ github.event.repository.default_branch }}

      - name: Release
        env:
          GITHUB_TOKEN: ${{ secrets.GH_PAT }}
          NPM_TOKEN: ${{ secrets.NPM_TOKEN_PUBLISH }}
          SPARKHIRE_NPM_TOKEN: ${{ secrets.NPM_TOKEN_PUBLISH }}
          SLACK_WEBHOOK_URL: ${{ secrets.SLACK_RELEASE_CHANNEL }}
        run: |
          npx auto shipit

      - name: Disable allow force pushes
        uses: benjefferies/branch-protection-bot@master
        if: always()
        with:
          access_token: ${{ secrets.GH_PAT }}
          branch: ${{ github.event.repository.default_branch }}

Our machine user needed to be made an admin (not great but we accepted the risk). This workflow works for us on the "Team" GitHub plan. Our protection rule requires 1 approval, allows the machines group to bypass, requires other workflow checks to pass (build test, release label test), and disallows admin force pushes. The action here temporarily disables that last bit in order for this to work.

[hipstersmoothie]

We have a plugin that handles this. Doesn't seem to be in the docs though I'll fix that https://www.npmjs.com/package/@auto-it/protected-branch

I've been using it on jimp and it works well. if any of you have issues with it feel free to open another issue