Enforce branch policies on the repository

[toddysm]

What is the version of your ORAS CLI

n/a

What would you like to be added?

To improve the security of the ORAS project we need to enforce the branch policies for this repository. I propose that we enforce the policies as follows:

• Use the following rules for main and release/* branches:
  • Require PR before merging
    • Require 3 approvals
    • Dismiss stale PR approvals when new commits are pushed
    • Require review from Code Owners
    • Require status checks to pass before merging
    • Require conversation resolution before merging
    • Require signed commits
    • Do not allow bypass the above settings

Please add your comments and proposals for additional changes to this issue.

Why is this needed for ORAS?

n/a

Are you willing to submit PRs to contribute to this feature?

• Yes, I am willing to implement it.

[shizhMSFT]

Few comments:

• The pattern release-* is adopted in this repository and thus release/* does not apply. Note that git may be in trouble if there is branch named release when you trying to have release/xxx.
• Requiring 3 approvals is not applicable to this repository since we only have 3 active code owners.
• Additionally, we require branches to be up to date before merging, which is useful but not captured in this issue.

It is worth noting that "require branches to be up to date before merging" somehow conflicts with "dismiss stale PR approvals when new commits are pushed".

[toddysm]

Good points @shizhMSFT. Having in mind the low number of codeowners, we can lower the approvals to 2 but I think this should be the min. The reason behind is that 2 people working on PR (the original submitter and 1 approver) can result of overlooking the PR and lowering the quality. Also, from security point of view it is easier for 2 people to coerce than 3. Hope this makes sense.

Regarding the rest:

• I don't mind how the release branches are named as long as this is documented. Though, from curiosity point of view I would like to understand what you mean with "Note that git may be in trouble if there is branch named release when you trying to have release/xxx"
• Also, what can be the conflict between the two rules? One is for branch while the other if for approvals. Not sure I am following...

[shizhMSFT]

Also, what can be the conflict between the two rules? One is for branch while the other if for approvals. Not sure I am following...

Here comes the scenario:

1. Alice sends out an PR to fix a bug
2. Bob, Charlie, David, as maintainers, review Alice's PR and approve it
3. During the step 2, Eve's PR gets merged
4. Since Alice's PR is not up to date, Alice is required to do a rebase or merge. Since Alice's PR has changed, all approvals get dismissed.
5. Alice has to contact Bob, Charlie, and David for another around of approval, and hope no one merges PR during this process. Otherwise, she has to start over again.

[shizhMSFT]

Again, require branches to be up to date before merging is important for integration. Otherwise, the main branch may be in a failed state by merging an old PR even if the checks pass in that PR.

[toddysm]

So, what is the proposal to avoid somebody sneaking changes that don't go through approval? Couple of comments/questions:

• Shouldn't engineers always sync the branches before submitting PRs?
• If the PR takes too long to approve, there is a good chance that what you are describing happens quite often. Shouldn't we target to be faster with approving and merging PRs?
• Why are we always working in main? Is that the best development practice? I would propose we have development branch that doesn't have those strict requirements and it is used for agility and only main and release follow those where the merging is much more controlled.

[sajayantony]

Should we close this -

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This issue was closed because it has been stalled for 30 days with no activity.