Lift some k/website branch protections #30394
Conversation
- Allow website admins to merge PRs even with failing checks - Don't protect dev-* branches from deletion
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sftim The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Prompted by kubernetes/website#41545 - I can't delete dev-1.17-ru.1 |
|
Thanks for this @sftim ! |
|
I'm not very experienced configuring Prow - extra views welcome; please don't assume I got this change right. |
There was a problem hiding this comment.
Just putting it out there that I think this PR should probably be accompanied with a change to https://github.com/kubernetes/sig-release.
| contexts: | ||
| - EasyCLA | ||
| exclude: | ||
| - "^dev-" # don't protect dev branches |
There was a problem hiding this comment.
The docs release team handbook used to mention explicitly to protect the dev- branch: https://github.com/kubernetes/sig-release/blob/e57c4d957e371dd9a96dc7ba0ae8b5cadd5928bd/release-team/role-handbooks/docs/Release-Timeline.md#enable-branch-protection . This was only recently removed by me assuming that all branches are protected by default, however the new docs role handbook still says the branch should be protected (https://github.com/kubernetes/sig-release/blob/master/release-team/role-handbooks/docs/Release-Timeline.md#modify-prow-config-file):
e.g. Create a #21727 against k/test-infra to configure prow to automatically apply milestones to future release branch and to remove the configuration for the last release e.g. Add branch protection and milestone applier for k/website.
This might seem misleading to readers of the handbook after this change (since the handbook mentions that you still have branch protection for dev-).
There was a problem hiding this comment.
Prow experts,
Can we explicitly add dev- branches to have branch protection?
There was a problem hiding this comment.
By default, all branches would be protected. Is there something special about this branch that it shouldn't have that protection?
There was a problem hiding this comment.
I want to be able to delete a localization branch - see #30394 (comment)
I don't know how we delete stale localization branches nor how we delete the dev branches for previous releases. Is there a process that can work without making this change?
There was a problem hiding this comment.
Another example for deleting specific branches in k/website
At the end of the 1.27 release cycle, a release-1.27 branch was incorrectly made so I deleted it
The same thing happened last week for the 1.28 release cycle, a release-1.28 branch was incorrectly made and now we have to delete it before the end of the 1.29 release cycle (which is months away at this point)
| @@ -466,6 +466,12 @@ branch-protection: | |||
| - stage-bots | |||
| website: | |||
| protect: true | |||
| enforce_admins: false | |||
There was a problem hiding this comment.
This one is concerning because it would allow bypass of CLA.
What kinds of checks are failing that we would need admins to have a green merge button and not go through tide?
There was a problem hiding this comment.
We had this happen for v1.28: a contributor whose CLA checks had previously passed changed his details within GitHub, which meant that the commits we'd merged into dev-1.28 weren't seen (by the CLA bot) as valid to merge from dev-1.28 into main.
We always bypass Tide for the actual docs change on release day, but we still need a green button to click.
There was a problem hiding this comment.
Should we allow admins to bypass this rule? I can't decide, but I do trust repository admins to use this privilege only with care.
|
/retitle Lift some k/website branch protections |
k8s-ci-robot
changed the title
|
What needs to happen to get this PR merged? |
|
cc: @kubernetes/owners for review. |
|
Hi folks! I am one of the SIG-Docs Localization Subproject Leads.
While some localization teams are actively creating and using dev branches in k/website, they are unable to remove stale ones due to the We are looking for a way to easily remove dev branches. |
|
A repo admin for the k/website repo can remove the branches, they just have to delete the auto-created branch protection rule in the settings first. This can be done without having to change any configs here in k/test-infra. |
|
What needs to happen to get this PR merged? I cannot work out which feedback is ”I don't understand this, please clarify” and which feedback is ”we have the context, and we won't make that change”. |
|
@sftim I'm not sure if it's actually needed. A repo admin can delete branches or force a merge. The only thing is it'd require updating or removing the branch protection rules through the UI when an incident occurs. It does mean there would be an extra step, but would also prevent any accidental type things from happening. |
|
We might want to make sure that a repo admin is available for each release day (and document that). |
|
or change these rules so they are in place most of the time but not on release day |
|
+1 to @mrbobbytables's comments
+1 |
|
OK. This isn't needed; instead, we (SIG Release / SIG Docs especially) need to plan releases around the availability of a repository admin. /close |
|
@sftim: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@kubernetes/sig-docs-leads FYI
This PR partially reverts #28789