Apply branch protection to our repos

[maxime-rainville]

Acceptance criteria

• Branch protections are applied to patch release branch, minor release branches and major release branches.
• Ability to manually tag releases is restricted.
• Ability for CI to auto-merge up and auto-tag patch releases is retained.
• Ability for COW to tag releases is retained and release process is updated accordingly.
• Module standardiser is updated so it can set up branch protection.
• Documentation about repository management is updated to reflect these changes

Notes (already decided)

• Who should have manual tagging access?
  • People doing releases (aka Guy and Steve)
• Which rules are included in "branch protections"?
  • See screenshot below

Related card

• Document branch protection to our repos #196
• Set up permissions/protections for new community roles #143

PRs

• NEW Update rulesets module-standardiser#53
• DOC Update branch protection rules developer-docs#522

[maxime-rainville]

Still need to identify exactly what branch protection we need and to which groups they should apply.

[maxime-rainville]

We concluded those are the branch and tag protection that should be enabled on all our supported repos.

Branch protection

Tag protection

[emteknetnz]

github-actions seems to be pretty easy to handle, it seems like have the "Organisation admin" role meaning we can use that (or the lower "Repository admin", "Maintain", or "Write" roles) as a bypass for any rulesets

I tested this by using the gha-tag-release action on a test repo on my personal account that has a tag ruleset applied to it. Without the bypass I got a 422, though with the "Organisation admin" role as a bypass the github-actions user was able to do its thing

[emteknetnz]

I've created a PR to update module standardiser to update the rulesets, though I haven't run it yet since we probably need to update all the users first

Once we run the new module-standardariser ruleset command, PRs will now requires to 2 approvers to merge unless your role has enough permissions to bypass

I've put this into peer review with the assumption that there are too many ACs on this card, which I've now split into 3 categories, and we should later move this card back into refinement.

[GuySartorelli]

Looks like @maxime-rainville probably needs to take a look and decide whether the core committer stuff should be removed from this card (probably handle it in silverstripeltd/product-issues#842 instead).

We don't want to do part of a card, and then move the card into refinement.

[maxime-rainville]

Can core commiter still approved and merge things with branch protection on? Do they need to a second approver?

I would like to remove ownership from external core committers, but at the same time I want to make sure we don't end up just treating them as regular reviewers.

If that's the case, I'm fine with closing this card and carrying the conversation on the next one.

[GuySartorelli]

Right now core committers are owners, so they can do whatever the heck they want. 😅

There are at least two separate cards talking about changing what core committers can do. I reocmmend we deal with changing their access separately from introducing these branch protection rules.

[maxime-rainville]

Me satisfied.

[maxime-rainville]

Actually lets apply branch protection first.

[emteknetnz]

Having a second look, there's also the "regular branch protection" that's already on all the supported repos - e.g. https://github.com/silverstripe/silverstripe-admin/settings/branches

I think this should remain as is with the the existing pattern match to prevent accidental deletion of existing 2 / 2.1 style branches. If we don't keep it then admins can accidentally delete the branches despite the new branch rulesset that restricts deletion because admins will have bypass permissions.

Even without the checking the tickbox "Do not allow bypassing the above settings - The above settings will apply to administrators and custom roles with the "bypass branch protections" permission.", I was unable to delete a 1 branch on a test repo that I setup on my personal account where I would have been an admin. On that repo I had a new branch ruleset that restricted deletions though it had a bypass for "Repository admin"

[GuySartorelli]

Yup, I think the implication is that this card is just to add additional branch protection on top of whatever is already there.

[emteknetnz]

Annoyingly there isn't a REST api endpoint available to update the old fashioned branch protection using an fnmatch pattern, you can only update it for existing branches.

I've gone and manually checked and updated branch protection for the list of supported modules in repositories.json (all 138 of them) and ensured there is branch protection. I added branch protection for a couple of dozen for the newer or peripheral modules that did not have it

However I was unable to access the repo settings to do this for all o f the non-silverstripe account repos such as symbiote, so presumably we probably also won't be able to add the branch/tag rulesets for them via the API either

[GuySartorelli]

@emteknetnz I'm confused about the status of this now - there's a PR but you can't automate it and have already applied the branch protection manually?

Annoyingly there isn't a REST api endpoint available to update the old fashioned branch protection using an fnmatch pattern, you can only update it for existing branches.

Does this mean we can't automate applying branch protection rules at all?

However I was unable to access the repo settings to do this for all o f the non-silverstripe account repos such as symbiote, so presumably we probably also won't be able to add the branch/tag rulesets for them via the API either

So are all of the branch protection rules applied now?

[emteknetnz]

There are two types of branch protection

1. Regular branch protection, which is what we have now e.g. https://github.com/silverstripe/silverstripe-framework/settings/branches - these are what I just manually updated
2. Branch protection rulesets - which is what module-standardiser will update via the REST API - it's what the PR is for - it will add the rulesets for branches and tags here https://github.com/silverstripe/silverstripe-framework/settings/rules

[GuySartorelli]

PRs merged. Reassigning to Steve to run the new command in standardiser

[emteknetnz]

Have run module-standariser, went smoothly