Comments (13)
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.
Related Issues (20)
- contributors: Add Aurรฉlien Bombo (@sprt) to the list of contributors HOT 6
- Add Wedson Almeida Filho (@wedsonaf) as part of the committers / maintainers HOT 4
- elections: 2023-10
- election: 2023-10: Add Fabiano Fidรชncio candidacy
- election: 2023-10: Add Zvonko Kaiser candidacy
- Add Dan Mihai (@danmihai1) as a maintainer / contributor to the project HOT 6
- Remove maintainers who are not active anymore HOT 5
- election 2023-10: Add Jeremi Piotrowski candidacy
- Add my candidacy for the October 2023 cycle.
- election 2023-10 : Add Greg Kurz's candidacy
- election 2023-10 : Add Gerry Liu's candidacy
- tools: Update election tools to ignore two more repos
- Adding @justxuewei to the Kata Containers reviewers team HOT 8
- Add @Apokleos to the Kata Containers reviewers team HOT 8
- election: 202-10 - update dates
- election: 2023-10 - Report results
- Add stratovirt maintainers to the team HOT 3
- Problem with Kata Container installation
- Add Ryan Savino as part of the committers team HOT 9
- Add Beraldo Leal as part of committers team HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. ๐๐๐
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google โค๏ธ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from community.