Closed haydentherapper closed 2 months ago
preview
on sigstore-github-sync/sigstore/github-prodSo the result seems to be that every Branch Protection gets this addition:
+ restrictPushes : [
+ [0]: {
+ blocksCreations: true
+ pushAllowances : [
+ [0]: "T_kwDOBDzYIc4AYVWd"
+ [1]: "MDQ6VGVhbTQ4OTkzMDk="
+ [2]: "MDQ6VXNlcjg2ODM3MzY5"
blocksCreations
is a new option in GitHub: I believe true as default makes sense but it also changes what used to be the default (as crazy as it sounds, creating new protected branches was not protected).
I will review some specific projects to see if the pushAllowances
list makes sense -- I'm a little surprised something like this was not in there already?
To my best understanding these are correct:
pushAllowances
list does not mean pushes are not allowed: Organization administrators, repository administrators, and users with the Maintain role on the repository can always push when all other requirements have passed (this means we could clean up repositories.yaml quite a bit by removing maintainers from pushRestrictions list)blocksCreations: true
seems completely reasonable to me: If someone wants to override that, they would have to add support to github-sync thoughI'm still baffled by how we only get an added pushAllowances but nothing was removed... What happened to the the PushRestrictions
used in the previous pulumi-github version?
I wish there was a way to deploy this to one or two projects first :)
We could make the change in the conformance org which would only affect one project.
- an empty
pushAllowances
list does not mean pushes are not allowed: Organization administrators, repository administrators, and users with the Maintain role on the repository can always push when all other requirements have passed
I keep re-reading this and I'm not sure if it can be true. What does the "restrict who can push" checkbox even mean at this point?
It seems the checkbox actually means "allow more people to push" instead of "restrict who can push"? the docs on all sides are kind of bad but this would actually be in line with the new argument name: pushAllowances
vs restrictions...
We could make the change in the conformance org which would only affect one project.
I think this would be useful. However, I don't have access to the project settings in GitHub UI so I can't compare what it looks right now and what it looks like after applying... Would maybe have to add admin permissions first.
Before that can we figure out this:
My current assumption based on careful reading of various docs is:
I've got a possible branch in https://github.com/jku/github-sync/tree/tweak-push-restrictions: it tries to avoid setting restrictPushes at all if the list is empty. I assume this is the equivalent of not checking the checkbox at all.
You could modify github-sync-pr-sigstore.yml
in this PR to do uses: jku/github-sync@tweak-push-restrictions
instead of sigstore/github-sync@main if you want to test that one...
when the box is not checked, all collaborators can still push to the branch (if other rules do not prevent the push) If the the box is checked, only maintainers can push to the branch (if other rules do not prevent the push)
That sounds correct to me, and your fix SGTM. From what I understand, the difference is that those with push
permissions can both review and merge if the check box is unselected, but only review if the check box is selected (we leverage this in some repos for example to differentiate between reviewers vs codeowners)
@jku reran based on merged sync PR
So no change from the github-sync PR merge...
restrictPushes.pushAllowances
: it's as if that setting had never been set?Created https://github.com/sigstore/community/pull/434 so that we (we = TSC or anyone who's a maintainer on this repo) can manually run preview
and up
to sync.
This SDK bump seems safe, though we'll try in the conformance org first.
Summary
Release Note
Documentation