[Enhancement] Block features from getting merged in to release branch after a release

[bbarani]

Is your feature request related to a problem? Please describe

OpenSearch project follows semVar and as per semVar guidelines, only backward compatible bug fixes are acceptable in patch release.

Hving said that, we recently identified multiple features and enhancements backported in to the 2.11 branch post the 2.11 release as listed here delaying the ability to release of 2.11.1 patch version in timely manner.

Describe the solution you'd like

Implement a process, mechanism and add necessary guard rails to enforce SemVar standards across all repos in OpenSearch organization.

Some of the possible approaches (I can think of!) :

• Standardize code commit messages to track the PR type for all commits
• Block PR's that are bigger than certain threshold in to release branch after a release
• Bock the PR's based on commit message in to release branch after a release

Adding documentation and relying on maintainers would not work all the time so we need to add automated mechanisms and guard rails to surface these anomalies in proactive manner.

Describe alternatives you've considered

No response

Additional context

No response

[bbarani]

@smortex @CEHENKLE @Divyaasm @dblock @peternied @reta @joshuarrrr @macohen @scrawfor99 @krisfreedain @davidlago Please provide your inputs on this issue.

[smortex]

If I am correct, backports are triggered by adding labels to pull-requests.

While automating the detection of mislabeling seems required in order to avoid the wrong merges we have seen, the detection criteria based on commit messages or commit size seems fragile: a mis-written commit message should prevent a PR from being merged (which is unfriendly for contributors), and the commit size might be an indicator, but changing a default value from true to false will probably be a breaking change that is accepted because it is under the limit; and fixing a typo that has been copied over and over in many file can be rejected because it is above this limit.

In order to avoid these pitfalls, I would like to propose another kind of automation based on (more) labels: maintainers can add and remove them quickly (so it is not one more burden on the contributors), and we can rely on automation with GitHub actions to detect inconsistencies and request fixes (and even automate more if required).

The bug, enhancement, backwards-incompatible labels

While some of these labels do exist in the OpenSearch repository and are used from time to time, this is currently not systematic.

In some organizations I contribute to (e.g. Voxpupuli), we use some tooling that generate changelog from GitHub merged pull-requests, and it rely on these labels to determine in which section each change should appear:

• PR labelled bug are added to the "Fixed bugs" section;
• PR labelled enhancement are added to the "Implemented enhancements" section;
• PR labelled backwards-incompatible are added to the "Breaking changes" section.

This help the voxpupuli team to determine what is the version number of the next release very quickly (we only maintain the latest version).

My proposal is to require such labels for backports: if I want to backport a change to the next PATCH release, it MUST be labelled bug. If I want to backport a change to the next MINOR release, it MUST be labelled bug or enhancement.

The following automation could be setup:

• When a backport-2.11 label is added to a PR without label, or with an enhancement or backwards-incompatible label, a message is added to say that such backport is not possible and the backport-2.11 label is removed;
• When a backport-2.11 label is added to a PR with a bug label, this is valid and automation of the backport can be triggered as it is currently done;
• When a backport-2.x label is added to a PR without label, or with a backwards-incompatible label, a message is added to say that such backport is not possible and the backport-2.x label is removed;
• When a backport-2.x label is added to a PR with a bug or an enhancement label, this is valid and automation of the backport can be triggered as it is currently done;
• When a bug, enhancement or backwards-incompatible label is added to a PR that already had one of these labels, a message is added to remind the purpose of these labels and the fact that they are mutually exclusive.

TL;DR

• Adding a backport-2.11 label to a PR would only work if the PR is already labelled bug;
• Adding a backport-2.x label to a PR would only work if the PR is already labelled bug or enhancement.

[peternied]

Proposal: Review changes during while creating patch release

I recommend that we repeat inspection of changes in minor release branches, as what was done for the 2.11.1 release. IMO the failing in 2.11.1 was a human understanding of how those branches were used and what expectation where to be followed. With the drastic shift to revert changes in the 2.11.1 I have a feeling these expectations have been reset and communicated.

Benefit of this process is that maintainers continue to have autonomy to merge checkstyle or spotless fixes in to a X.Y branches. If owners of the OpenSearch distribution want to put a change into question IMO that is there prerogative, I would recommend approaching this on a case by case basis its easier to understand and educate around particular changes.

The biggest drawback is that a some repositories could delay the release during a critical release moment - which aligns with existing issues with repos merging patch version bumps. In the event there is critical release that is held up - there are many engineers across the OpenSearch-Project that could be engaged to address the different repositories.

[dblock]

Is there an automated, easy way to include the set of changes since release in the build for easy examination? Maybe as part of an automated CHANGELOG generation process.

[bbarani]

May be we can add an automation to allow only the PR's tagged as "Bug-fix" or "security-fix" to be merged in to the release branch after a release (Once the tag is cut)? CC: @smortex @dblock.

[smortex]

@bbarani I never tried it myself, basically the gh(1) command can do all sort of things with labels from a workflow as documented here: https://docs.github.com/en/actions/managing-issues-and-pull-requests/adding-labels-to-issues

More globally, a lot of gh pr ... command are available from actions in order to check which branch is targeted (gh pr view --json baseRefName) and which labels are set (gh pr view --json labels) before adding labels (gh pr edit --add-label ...), removing them (gh pr edit --remove-label ...) or closing the PR (gh pr close) after posting a comment (gh pr comment). This feels like we can do almost anything we want with some shell scripting.

What would be the algorithm you have in mind?

if targeting a release branch
  if labels include "Bug-fix" | "security-fix"
    remove_label("invalid")
  else
    add_label("invalid")
    fail action
  end
end