Skip to content

Create backport PR automatically#148

Open
r0mant wants to merge 1 commit intomainfrom
roman/backportpr
Open

Create backport PR automatically#148
r0mant wants to merge 1 commit intomainfrom
roman/backportpr

Conversation

@r0mant
Copy link
Contributor

@r0mant r0mant commented Aug 3, 2023

2 changes to the backport bot:

  • Attempt to create the backport PR automatically. Fallback to the current "quick PR" link if it fails.
  • Transfer all labels (except for "backport/" ones) to the opened backport PR.

@r0mant r0mant requested review from a team August 3, 2023 22:49
@r0mant
Copy link
Contributor Author

r0mant commented Aug 7, 2023

@jentfoo @reedloden A side effect of this change would be that the original PR creator could technically approve their own backport PR because it will be opened by the bot account. We would still require the 2nd approval.

Just want to make sure it's fine from the compliance perspective like SOC2, etc.

@jentfoo
Copy link
Contributor

jentfoo commented Aug 7, 2023

@r0mant, brainstorming, what if we protected the automatic backport branches so that custom changes can't be pushed to them? If that's reasonable then I don't think there is any concerns, since we have control over what content can be in the PR.

@r0mant
Copy link
Contributor Author

r0mant commented Aug 7, 2023

@jentfoo Yeah, we can create a branch protection rule that will require a pull request to merge to backport/xxx branches. I'll send a PR to github-terraform.

@r0mant
Copy link
Contributor Author

r0mant commented Aug 7, 2023

@jentfoo Before I do it though: do we have to do it? I.e. my question was, will we actually violate any compliance provisions if we don't protect backport branches? Just trying to not introduce any extra barriers for the team unless really necessary.

@reedloden
Copy link
Contributor

reedloden commented Aug 7, 2023

From a purely compliance standpoint, as long as we have at least one peer review (as in, review from an authorized non-bot approver who is not the code author), we're fine. The two separate reviews is a goal, but not a hard requirement.

From a security standpoint, adding branch protections is the right call. Lack of branch protections has bitten us in the past, and I'm concerned what would happen if a non-bot user pushed branches that matched the same pattern. How does the bot handle it if a branch already exists? What if somebody changes the underlying branch contents (at any time during the process)?

As such, I'd prefer we add branch protections, as these are special branches and should be treated as such.

@r0mant
Copy link
Contributor Author

r0mant commented Aug 9, 2023

@reedloden If non-bot user pushes backport/ branch, nothing would change compared to now. Since it's a user that pushes it, it will require 2 approvals from others to merge to master/release branches just like now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants