Prevent from pushing to master branch accidentally about community HOT 13 CLOSED

Hi @lifupan - No problem! Mistakes happen and this shouldn't have been allowed anyway.

@gnawux - This is odd as I thought we already had this protection setup. If you click Settings->Branches you can see master as being "protected" (in all the Kata repos). If you then click "Edit" in the "Branch protection rules" section, the "Require pull request reviews before merging" option is checked (set).

However, I think I may have found the issue: near the bottom of that page is a setting called "Include administrators: Enforce all configured restrictions for administrators". That wasn't checked so I've just checked it to hopefully stop this happening again. I'll now go around all the remaining Kata repos and set that option and report back here once done...

from community.

It's my mistake, I apologize for this accident.

from community.

Thanks for letting us know! Thanks @jodh-intel for chasing that, across all repos.

from community.

I've just enabled that extra check so that admins cannot push to the master branch directly for the following repos:

• https://github.com/kata-containers/agent
• https://github.com/kata-containers/ci
• https://github.com/kata-containers/community
• https://github.com/kata-containers/documentation
• https://github.com/kata-containers/kata-containers
• https://github.com/kata-containers/ksm-throttler
• https://github.com/kata-containers/osbuilder
• https://github.com/kata-containers/packaging
• https://github.com/kata-containers/proxy
• https://github.com/kata-containers/qemu
• https://github.com/kata-containers/runtime
• https://github.com/kata-containers/shim
• https://github.com/kata-containers/tests
• https://github.com/kata-containers/www.katacontainers.io

I've tested this indirectly by creating a repo from my personal github account (I am therefore the admin for that repo), enabling the admin check and then attempting to push directly to master. This results in the following error:

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: At least 2 approving reviews are required by reviewers with write access.
To https://github.com/jamesodhunt/bar
 ! [remote rejected] master -> master (protected branch hook declined)

Note: The qemu repo didn't have the standard protections on it, so I've now added them.

from community.

In summary, I think we're now protected against any future accidents 😄

from community.

Thanks @jodh-intel.

@lifupan could you try to repeat the mistake again 😂 ?

from community.

@gnawux Oh, no, I don't want to fall down in the same place twice.

from community.

@gnawux - I've just checked in my test repo and fwics the permission check is done before any checks are performed on the local clone to see if there are actually any new commits to push. So, iff @lifupan's runtime branch is current and matches the HEAD of master on https://github.com/kata-containers/runtime (so that he won't actually be pushing any new commits even if the check doesn't work), it should be safe to attempt to push to master again fwics... 😄

from community.

I've just done a test to https://github.com/kata-containers/www.katacontainers.io since:

• That repo is not actually being used to host the web site any more
• I thought it was low risk.

Attempting to push to master directly gives the expected error and I have confirmed that the remote branch has not been updated:

$ git push -v origin master
Pushing to https://github.com/kata-containers/www.katacontainers.io
Username for 'https://github.com': jodh-intel
Password for 'https://[email protected]': 
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 4 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 311 bytes | 311.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0)
POST git-receive-pack (464 bytes)
remote: Resolving deltas: 100% (2/2), completed with 2 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: 2 of 2 required status checks are expected. At least 2 approving reviews are required by reviewers with write access.
To https://github.com/kata-containers/www.katacontainers.io
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'https://github.com/kata-containers/www.katacontainers.io'

from community.

@jodh-intel Could you check in your personal repo if you can still force merge a PR w/o passing all required checks (reviewer counts, ci etc.)? I thought the Require pull request reviews before merging checkbox is meant to allow it.

from community.

Just re-tested pushing to the www.katacontainers.io repo as I'd removed myself as an admin for that repo. However, I'm now an admin once again and I see the same denial behaviour.

from community.

Hi @bergwolf - my personal repo didn't have any status checks enabled but did have the "Require review from Code Owners " option set. I've just done an "inverse test" by unsetting the "Include administrators" option and with my personal repo I can now push direct to master. So I think that proves the "Include administrators" option is the one we should have had set on all the Kata repos when we created them.

from community.

Of course, if we ever fine a scenario where we need to push direct to a master branch, we can of course undo the change I've made but let's hope we never need to! 😄

@gnawux mentioned 2FA. I believe all Intel admins have 2FA enabled. If you are an admin on any Kata repo and haven't already enabled 2FA, I'd strongly recommend grabbing a Yubikey or similar 2FA device and then enabling 2FA as soon as possible to keep us all as safe as possible.

from community.