2PR manual approval workflow

[gaiksaya]

Overview:

With 1-click release process any component within opensearch organization is free to release anytime they want. All they need to do is push a tag. This process has helped reduce the manual overhead with each release cycle and avoid human error as much as possible. However, recently we faced few scenarios which made us realize that even though the process is automated, the step to approve that process needs to be a 2 person rule.
This is mainly to get following things checked:

• The release is security reviewed (especially major releases and first release of any new product)
• Other maintainers are aware and approve of this release
• There is no on-going maintenance anywhere that would block the release in someway, etc
• More?

Approaches:

Each of the below action needs to be added as a part of the GHA release workflow that is triggered via a tag push.

Not choosen: Using GitHub provided environments

1. Using GitHub provided environments

GitHub recently introduced environments for deployments related workflows. These include approver list, secrets, variables, etc.

Overview of what approver section looks like:

The GHA workflow then only need to add just one line which would apply all the above rules to that workflow:

environment: production

More details in the above linked page.

Pros:

• A direct GitHub feature hence has better support and good integration with everything else on GitHub
• Easy to integrate with any workflow once the environments are in place

Cons:

• Only 1 approval is required. Not an ideal solution if we need 2 people to approve the workflow
• If a maintainer is also the actor of the workflow, they have access to self-approve the workflow

Use required reviewers to require a specific person or team to approve workflow jobs that reference the environment. You can list up to six users or teams as reviewers. The reviewers must have at least read access to the repository. Only one of the required reviewers needs to approve the job for it to proceed.

2. Using manual-approver action (Recommended)

Use trstringer/manual-approval in the release workflow. The mentioned action from the marketplace allows just to have 2 or more approvers requiring approval. Also has an option to exclude the actor of the workflow run to be excluded from the approval process.

Sample workflow: https://github.com/gaiksaya/opensearch-build/blob/test-env/.github/workflows/groovy-tests.yml
Sample GitHub issue created by the action: gaiksaya#4

Pros:

• Number of approvers can be customized
• Tracks the approval via GitHub issue assigned to the approver
• Workflow actor can be excluded
• Even though it is a third party GHA, it is well maintained for now.

Cons:

• Not a direct GitHub feature
• The integration needs to be at workflow level and not at repo level.

[gaiksaya]

Hi @bbarani @dblock @wbeckler,

Please review issue description regarding 2PR approval process for 1-click release process and let me know what you think would be the best approach.
Accordingly we can run a campaign to implement this for existing and then future 1-click release processes.

Thanks!

cc: @opensearch-project/engineering-effectiveness

[dblock]

I like the recommended approach. I would experiment with a subset of projects first, e.g. a couple of clients.

[bbarani]

@gaiksaya Can we start with the clients that gets published to one-way door distribution channels (i.e. Maven etc..)? CC: @dblock @CEHENKLE @wbeckler

[gaiksaya]

Sure! We can get started with opensearch-java if that's okay. Let me know who should be in the approval list @dblock @wbeckler

[wbeckler]

I propose that we use the maintainers list to keep it in line with our principles, and I'm assuming here that a maintainer would use extra caution at this stage. Thoughts?

[gaiksaya]

Sounds good to me. Should the code-owners (example) be also one of the approvers? I believe that will be added as a team.

[wbeckler]

Because you have to be a member of the org to be on a team, they are of limited use and won't work as we add non-Amazonians to projects. It would be a temporary concept.

…

On Fri, Jan 20, 2023 at 7:13 PM Sayali Gaikawad ***@***.***> wrote: Sounds good to me. Should the code-owners (example <https://github.com/opensearch-project/opensearch-build/blob/main/.github/CODEOWNERS>) be also one of the approvers? I believe that will be added as a team. — Reply to this email directly, view it on GitHub <#3111 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5PRLULY2MQ534KJFIXMDLWTMSZHANCNFSM6AAAAAAT6PNHVM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[gaiksaya]

I was suggesting both the maintainers as well as code-owners so all the affiliations will be considered. Sometimes it happens that you belong to the team but not the maintainer of a specific repo. Or we just missed adding them as maintainer. There might be overlap which is fine. (I think)

[wbeckler]

I think we should encourage the repos to follow our publicly stated requirements about maintainers. We don't have policies around codeowners.

…

On Fri, Jan 20, 2023 at 8:29 PM Sayali Gaikawad ***@***.***> wrote: I was suggesting both the maintainers as well as code-owners so all the affiliations will be considered. Sometimes it happens that you belong to the team but not the maintainer of a specific repo. Or we just missed adding them as maintainer. — Reply to this email directly, view it on GitHub <#3111 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5PRLS6FNZY5OQ4YBAIW33WTM3ZDANCNFSM6AAAAAAT6PNHVM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[gaiksaya]

Makes sense. Let me know if you have any preference on the repository or has upcoming release to get started with this.
Thanks!

[dblock]

For most repos I think codeowners should start by matching the list of maintainers and all groups should be removed. I wrote this up in opensearch-project/.github#125 and would like to add it to the docs. Please comment there?

[gaiksaya]

In order to grab the maintainers' list dynamically via MAINTAINERS.md we need to add some enhancements to https://github.com/opensearch-project/project-tools thats can list maintainers along with specific repo.
Or find solution to do so. Something that can be easily run in GHA.
Update: Below shell works too:

cat MAINTAINERS.md | grep \| | grep 'github.com' | cut -d '|' -f3 | cut -d '/' -f4 | tr -d ')'

[gaiksaya]

PRs have been opened for the repositories that had updated codeowner's file. For remaining ones, comments are added to the baseline issue. (See above linked issues).
This process is documented in the on-boarding guide: https://github.com/opensearch-project/opensearch-build/blob/main/ONBOARDING.md#onboarding-to-universal--1-click-release-process

No action item remaining from our end. Let me know if I need to create separate issues for components already on-boarded to 1-click release process and are missing the feature.

Thanks!

[gaiksaya]

Workflow used in actual release: opensearch-project/opensearch-benchmark#231 🎉

[shyim]

The idea is good, but the implementation is totally wrong for me. Running a CI job synchron for max 72 hours to get issue approvals feels for me unbelievable irrational. I really think nobody would implement that in this way if they would need to pay the runner for each execution. It's definitely for me an abuse of the CI.

You can build the same thing within 1-2 hours with an Lambda in async at the end you trigger from external the workflow: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#repository_dispatch to make it 100% secure you could set a secret in Actions (repo or global org) and assume that the payload secret is the same (request is from the lambda and not from the maintainer manually)

[dblock]

I think @peternied built a cron-based solution to this in https://github.com/peternied/bake-time, wdyt?