Automatically Backport Security Fixes #8583
Conversation
andremig-bentley
requested review from
a team,
ColinKerr,
DanRod1999,
aruniverse,
calebmshafer and
wgoehrig
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR implements an automated security fix backporting system that allows security PRs to be automatically propagated to supported release branches. The implementation adds mergify configuration to automatically backport PRs labeled "Security" to both the latest minor release branch and the previous major version branch.
- Adds mergify rule for automatic backporting of security-labeled PRs to release branches
- Updates version-bump workflow to maintain current branch references in mergify configuration
- Provides a PR template to standardize security fix submissions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/mergify.yml | Adds branch variables and backport rule for security fixes |
| common/config/azure-pipelines/jobs/version-bump.yaml | Updates mergify branch references during minor releases |
| .github/PULL_REQUEST_TEMPLATE/security_fix.md | Provides template for security fix PRs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.github/mergify.yml
Outdated
| # Configuration file for mergify | ||
|
|
||
| # Branch variables for backport targets | ||
| latestMinor: &latestMinor "release/5.2.x" |
There was a problem hiding this comment.
- plz link to docs explaining the syntax
- how can we automate update this latest minor branch name every time we do a release?
- latest minor is not 4.9, its 4.11
There was a problem hiding this comment.
disregard 2; i see its handled in ver bump pipeline
There was a problem hiding this comment.
re 1 - which syntax do you mean, the branch naming convention?
There was a problem hiding this comment.
no the variable, &latestMinor
There was a problem hiding this comment.
Its using YAML's anchors and aliases:
https://yaml.org/spec/1.2.2/#anchors-and-aliases
https://yaml.org/spec/1.2.2/#alias-nodes
https://support.atlassian.com/bitbucket-cloud/docs/yaml-anchors/
It seemed like a better solution to me than just having the release branches directly named in step, but it is a little more complicated. It can be changed to just:
backport:
branches:
- "release/5.2.x"
- "release/4.11.x"
There was a problem hiding this comment.
why wouldnt hardcoding these instead of adding it to another var that is only referenced once and needs to be updated not be easier?
There was a problem hiding this comment.
it would be easier to just have them hardcoded, my main thinking was just clarity, as in what are these branches, why are we backporting to them specifically, etc. But, that can be taken care of with doc comments. In the latest commit, I've simplified it to hardcode the branch names, a simpler implementation with a doc comment is probably the better way to go.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| - bash: | | ||
| mergifyPath=".github/mergify.yml" | ||
| version=$(echo $(getVersion.version) | sed 's/\([0-9]*\.[0-9]*\)\.[0-9]*/\1.x/') | ||
| releaseBranch="release/$version" |
There was a problem hiding this comment.
if we were to update 4.11 to 12, would this still work? It looks fine, but just double checking updating prev major won't cause unintended problems
There was a problem hiding this comment.
I think there's a chance it would break with this current implementation. Currently it basically just takes the version created from the version bump step and sets the first release branch in the list to that value. If we were to run a version bump on release/4.11.x for a new minor, I think it would set 5.whatever.x to 4.12.x and leave 4.11.x there. There wouldn't be any problem with patches. Is there any precedent for us releasing a new minor on a previous major, though?
There was a problem hiding this comment.
As a fail safe, the most recent commit changed the logic so that the version-bump step looks at the major version number of the new version, and updates the release branch which starts with that same major version number. This means that if we were to release a new minor on a previous major version (4.11 -> 4.12), the bump would update the correct branch.
| # e.g. if this release is `release/5.0.x`, value in `gather-docs.yaml` | ||
| # should be `release/4.<whatever_last_minor_release_version_was>.x` | ||
| # additionally if major version bump, the `mergify.yml` also needs to be edited manually | ||
| # if this release is `release/5.0.x`,change the `latestMinor` variable to `release/5.0.x` |
There was a problem hiding this comment.
updated in most recent commit
4 participants
Resolves #6131.
This PR adds a mergify rule to automatically backport PRs with the "security" label to the latest minor version and the previous major version. It also adds logic to
version-bump.yamlto update the branch names during minor releases, and it adds a PR template to be used for security/cve fix PRs.