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

Automating backports via GitHub Actions #8181

Open
4 tasks
amercader opened this issue Apr 15, 2024 · 0 comments · May be fixed by #8191
Open
4 tasks

Automating backports via GitHub Actions #8181

amercader opened this issue Apr 15, 2024 · 0 comments · May be fixed by #8191
Assignees
Labels

Comments

@amercader
Copy link
Member

As part of a wider effort to reduce the burden of maintenance and increase security (see also #8143) I'd like to propose the integration of an automated process to backport merged pull request to the release branches. I believe this would have a big impact because:

  • Manually cherry-picking PRs marked for backporting is time consuming and error prone
  • Backporting early means less conflicts and that the release branch is a better snapshot of the next release

I've been testing different approaches in my personal fork, and want to give a full summary of my findings

Here is the full workflow definition I'm currently using in my fork:

https://github.com/amercader/ckan/blob/master/.github/workflows/backports.yml

What to use

There are two main approaches to automating backports, bots that listen to GitHub webhooks (like Python's) or GitHub Actions. The former are generally a bit more complex and tailored to specific projects, while actions have a limited set of functionalities but are much easier to integrate.

There are several backport actions available in the Marketplace but I chose korthout/backport-action because it was well documented, well written and actively maintained.

How it works

The setup is easy enough, just a new workflow to start a GitHub action whenever a Pull request is merged. If that PR has a "Backport {branch}" label, the action will cherry-pick the commits from the merged PR:

  • If they merge cleanly into the target branch, it will create a new PR against the target branch
  • If they can't be merged, it will add a comment to the original merged PR with the instructions to fix it manually

Permissions and risks

The workflow is triggered by the pull_request_target event, because the token needs write access to create the PRs and add comments. We need to be mindful of this, and never label for backporting PRs from external forks that could modify the workflows, access the action secrets etc (basically that modify the .github folder).
Of course the action itself could present a risk, and there is no sure way to avoid this other than reviewing its source code. I've reviewed the source, which is is straightforward and well written but others can review it too.

Customizing

I added a couple of modifications to the default behaviour of the action:

  1. By default, the action runs using the GITHUB_TOKEN available to all actions. This token does not start new workflow runs for the events it triggers. This means that PR checks including tests won't be created in backport PRs:

    [Backport dev-v2.10] Avdata99 avoid err 500 for datastore amercader/ckan#4

    This serioulsy limits the usefulness of it, as we need to make sure that tests pass in the release branch as well.The alternative is to use a Personal Access Token (PAT), which does trigger new workflows. Note how in this case the PR was created by my user (as I used one of my own PATs)

    [Backport dev-v2.10] Tkallady patch 1 amercader/ckan#7

    To not impersonate one of our personal accounts, we can create a new ckanbot GH account, limit its access to the main ckan/ckan repo, and create a PAT with further limits the scope of the token to only what the actions needs

  2. By default, if the backport fails, the action will add a comment to the original PR. I think this is easy to miss (we get a lot of Github notifications!):

    Avdata99 fix activity display permissions amercader/ckan#5

    To mitigate this I added another job to the workflow that will add a "Backport failed" label to the original PR. This way it is easier to filter all the pending backports when we are doing a release.

    Mutantsan keep get parameters after redirect amercader/ckan#8

  3. If you merge the PR and forgot to add the "Backport {branch}" label there is no way to retrigger the process. One way to deal with this is to also trigger the backport when you add a comment starting with /backport (or whatever we choose). This is documented in the action (see here) but I think we need a way to limit the allowed users that can trigger it to ones with write access

Of course we can tweak other things like the title or description of the backport PR, whether to copy the assignees etc, but this should just be changing configuration options.

Next steps

First of course, everybody needs to be happy with this approach. If so we then:

  • Create and configure the ckanbot account, giving it the necessary permissions only
  • Create PR with the final backports.yml workflow config
  • Create new labels (eg "Backport dev-v2.10", "Backport failed")
  • Start testing and tweaking it!
@wardi wardi removed the To Discuss label Apr 16, 2024
amercader added a commit that referenced this issue Apr 18, 2024
Fixes #8181

This commit includes the configuration file for an action that:

* When a PR that has a label like "Backport <branch>" is merged,
  it will trigger a backport action
* If the PR commits merge cleanly into the target branch, a new PR will
  be created against it, assigned to the same user as the merged one.
  The usual checks will be run on the new PR
* If the commits don't merge cleanly, a comment will be posted on the
  orginal PR with the manual commands to fix the conflicts, and the PR
  will be labelled with "Backport failed"
* Additionally, Tech Team members can trigger a backport on open or
  already closed PRs adding a comment starting with `/backport` (and
  adding the relevant label)

Some more details:
* The action is run using a token from the @ckanbot account, this needs
  to be stored in the repository secrets (`BACKPORT_ACTION_PAT`)
* There are also two public variables (`TECH_TEAM_USER_IDS` and
  `CKANBOT_USER_ID`) that need to be added to the repository variables.
* The action used source code code can be found at
  https://github.com/korthout/backport-action
@amercader amercader linked a pull request Apr 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants