[Discussion] Maintenance Policy with unsupported branches

[peternied]

During the Jan 9th triage meeting there was discussion about how to handle PRs on branches that are no longer maintained when CI / tests are failing [1]. There are a couple of approaches we can take after interpreting the maintenance policy from the project website

The software maintainers will not back-port fixes or features to versions outside of the maintenance window. That said, PRs with said back-ports are welcome and will follow the project’s review process. No new releases will result from these changes, but interested parties can create their own distribution from the updated source after the PRs are merged.
From https://opensearch.org/releases.html#maintenance-policy

• Status quo: individual maintainers apply their own quality criteria and end up retrying / bypassing branch protection to merge changes. This is hardly a process as its highly discretionary depending on the who and the circumstances of the backport. Setting poor expectations with members of the community that are actively in pulling changes into alternative distributions.
• Absolutist stance: Require backport contributors to full fix broken CI / legacy system, if they still desire the change to be merged without those contributors tell them to fork. This is a uncharitable to our contributors of this community and tells them to effective start a new community.
• Reflect unsupported nature: Spend some time to update the CI checks to only represent minimal requirements in the no longer supported branches.

PRs failing due to build issues [1]

• [Backport 1.0] Username validation for special characters (#2277) #2330
• [Backport 1.1] Username validation for special characters (#2277)  #2331

[peternied]

Note; There was discussion about open an issue in the wider OpenSearch-project repo. I think this is a good area for repo specific discretion instead of being prescriptive top-down. We can certainly broaden this discussion if that seems appropriate.

@opensearch-project/security

[peternied]

I am voting for Reflect unsupported nature, as I think this would be easier to do and communicate. It would set the clearest expectations for the community's contributors.

If there is enough pushback around concerns of quality dips, those interested in maintaining the quality of these branches can always augment support. IMO this is a great path for maintainership for those that are interested.

[davidlago]

Thanks for getting the discussion going, @peternied. @dblock thoughts on moving this discussion to the overall project vs nested in the security repo? As the guidance on backports is project-wide, any updates to it should probably be inclusive of other repos.

My two cents:

I am hypothesizing here that parties interested in backporting fixes to no longer supported versions of this project is because even though we are not supporting those versions, they are. I would also go on and assume that as the current CI does not give us confidence on the artifacts produced from these versions, these parties are getting that confidence from somewhere else (running their own test suites, manual testing...)

The problematic thing I see is whether there could be any assumptions or confusion around the quality of no-longer supported versions from other users. At a minimum, we should make it abundantly clear (if it is not already) which versions we are no longer considering supported.

At that point... one could argue: if we are no longer supporting those versions, or expecting CI to pass, or keeping them current with security fixes and other critical changes, I would rather take a stronger stance and archive/prevent future commits on them to avoid any sliver of confusion as to the level of support on those branches. Anyone still interested in maintaining them might as well fork the repo and cherry-pick/backport to their hearts' content.

[davidlago]

@CEHENKLE FYI, as I know you've given this topic some thought as well.

[DarshitChanpura]

I agree with updating CI checks as this will give some sense of stability before merging. This "sense of stability" (steps involved in CI) will be discussed upon by maintainers and can be documented if needed. This will set the clearest expectations/guidelines for community to make contributions to unsupported branches.
I vote Reflect unsupported nature

[cwperks]

[Triage] @peternied Can you provide a conclusion for this issue?

[peternied]

The pull requests this discussion topic were created from have been closed out as there hasn't been any activity on them. We've got three votes on this topic out of 6 maintainers and no external feedback - I don't feel like there is any conclusive action for us to take.

I am closing out this issue and we can always reopen if interest in topic comes back up.

[davidlago]

Re-assessing this today, let's see if we can get closure on it.

[peternied]

Recently permissions in this and other repositories was changed so maintainers only have 'maintain' access - not admin. This has prevented some pull requests from being merged.

See these from the security-dashboards-plugin repo

• [Backport 1.2] Replace legacy template with index template security-dashboards-plugin#1363
• [Backport opendistro-1.11]Replace legacy template with index template security-dashboards-plugin#1373
  Many more see https://github.com/opensearch-project/security-dashboards-plugin/pulls

Also these in the security repo

• [Backport 1.1] Username validation for special characters (#2277)  #2331
• [Backport 1.0] Username validation for special characters (#2277) #2330

FYI - @cliu123

[cliu123]

Good discussion! Given that the old branches have been officially no longer supported, I do not see necessity to prevent backports with CI. It is fine to me that the author of PRs takes responsibility of quality as not being supported means that maintainers are not responsible for quality of those versions.
I see back and forth discussion going on for quite a while, could we have a quick decision? I vote for updating the branch protection policies to not requiring passing CI for those old branches. Thanks!
@davidlago @peternied

[davidlago]

Closing this back again after further discussion. We will not be merging PRs without passing CI. Unsupported branches (branches that are not under active maintenance and not seeing new releases) can still accept backport PRs, but only if they have a passing CI.