Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protect main branch with .asf.yaml config #3285

Merged
merged 2 commits into from
Nov 10, 2021
Merged

Conversation

glynnbird
Copy link
Contributor

Overview

Added to .asf.yaml to protect the main branch and, by omission, unprotect the master branch.

See https://cwiki.apache.org/confluence/display/INFRA/git+-+.asf.yaml+features#git.asf.yamlfeatures-BranchProtection

Testing recommendations

No code changes. Ensure after merge that the master branch is unprotected and the main branch is protected.

Related Issues or Pull Requests

n/a

Checklist

  • Code is written and works correctly
  • Changes are covered by tests
  • Any new configurable parameters are documented in rel/overlay/etc/default.ini
  • A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation

Copy link
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add 3.x branche(s) here too, or that should be a separate commit on the 3.x branch?

@glynnbird
Copy link
Contributor Author

@nickva my reading of the Apache docs are that although .asy.yaml is branch specific, configuration the pertains to branch protection needs to be in the default (main) branch. So If you want additional branches protected, they need adding to this PR.

@nickva
Copy link
Contributor

nickva commented Dec 11, 2020

@glynnbird ah makes sense, metadata about protected branches would be in the main branch. Let's add 3.x to the list of branches then.

.asf.yaml Outdated
@@ -21,6 +21,8 @@ github:
squash: true
rebase: true
merge: true
protected_branches:
main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The referenced https://cwiki.apache.org/confluence/display/INFRA/git+-+.asf.yaml+features#Git.asf.yamlfeatures-BranchProtection is telling me

All protected branches in the YAML must be dictionary entries. Thus, if you only want to disable force push from a branch, you can construct a fake dictionary like so:

Suggested change
main
main: {}

@kocolosk
Copy link
Member

I agree with @bessbd 's analysis of the documentation. But I'm wondering -- should we be availing ourselves of some of the more specific branch protection features? I honestly thought we had several of those setup in the olden days. Maybe @wohali remembers the requirements we put in place? It looks like the set of options for a branch is as follows:

      required_status_checks:
        # strict means "Require branches to be up to date before merging".
        strict: true
        # contexts are the names of checks that must pass
        contexts:
          - gh-infra/jenkins
          - another/build-that-must-pass
  
      required_pull_request_reviews:
        dismiss_stale_reviews: true
        require_code_owner_reviews: true
        required_approving_review_count: 3
       
      # squash or rebase must be allowed in the repo for this setting to be set to true.
      required_linear_history: false
  
      required_signatures: true

@wohali
Copy link
Member

wohali commented Nov 10, 2021

@kocolosk Yeah, we'd want to block commits to any release-able branch (main, 3.x, etc.) for anything that didn't pass, with some sort of 'emergency bypass' to deal with broken CI - sometimes in the past we had to talk to Infra to help with this, but it should be pretty rare.

Haven't dug into these other (newer?) .asf.yaml features, sorry.

The intent here is to require "branch must be up to date
before merge / squash / rebase", and to further require a clean bill of
health from Jenkins for `main` and `3.x`. I'm not so confident about
the CI health of older branches to enable that protection there, and at
any rate it's unlikely we'd be committing anything to those branches
anyway.
@kocolosk
Copy link
Member

As far as I can tell "emergency bypass" is still an Infra action:

NB (2): If you need to remove a required check in order to push a change to .asf.yaml, you will need to create an Infra Jira ticket with a request to have the check manually removed.

I updated this PR to (I think) require up-to-date branches before merging, and further require Jenkins to be green for main and 3.x. What do you think?

@kocolosk kocolosk merged commit 5020dae into main Nov 10, 2021
@kocolosk kocolosk deleted the protect_main_branch branch November 10, 2021 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants