Check tag exists on main on release

[haydentherapper]

This mitigates the risk of a tag being generated and pushed off of a branch.

Summary

Release Note

Documentation

[haydentherapper]

Thanks @jku for pointing this out! Inspo from https://stackoverflow.com/questions/63745613/how-to-get-a-branch-name-on-github-action-when-push-on-a-tag, to use git branch --contains.

My bash foo is horrible, so before I copy this to all of the release workflows, wanted to get feedback.

[woodruffw]

Thinking out loud:

1. I think this will be unreliable in a future release scenario where main tracks the latest stable release, but we have "backport" branches that get cherry-picked into. For example, main might track the 2.x series, while the 1.2.3 tag gets created off of the 1.x branch by cherry-picking from main. We may never run into this as a practical matter, but I figured it's worth flagging as something that could break in the future 🙂
2. This might not be a sufficient protection in all cases: in particular, someone with tag push access might still be able to push up a tag that has a modified version of this workflow, removing this check. I'm not 100% sure about that though, someone from GitHub like @steiza might be able to confirm.

(This would be a bigger change, but I think we could solve both of these problems by switching from tag events to release: published events -- that event payload includes the GITHUB_REF so we can differentiate for each language, while also giving us more control over who can actually publish the packages via release signoffs.)

[haydentherapper]

I think this will be unreliable in a future release scenario where main tracks the latest stable release, but we have "backport" branches that get cherry-picked into. For example, main might track the 2.x series, while the 1.2.3 tag gets created off of the 1.x branch by cherry-picking from main. We may never run into this as a practical matter, but I figured it's worth flagging as something that could break in the future 🙂

Yea, to generalize this check, it's really "am i pushing from a protected branch", assuming we create X.Y branches that are protected for previous releases. Sticking with main for now is probably sufficient, unless there's a straightforward way to know if the branch is protected.

This might not be a sufficient protection in all cases: in particular, someone with tag push access might still be able to push up a tag that has a modified version of this workflow, removing this check. I'm not 100% sure about that though, someone from GitHub like @steiza might be able to confirm.

Ugh, good point, I think you're right that the action would run based on what's in the tagged release, invalidating this check.

Would switching to release:published help with preventing pushes from non-protected branches? Couldn't you still publish a release from a tag that is from a branch?

[haydentherapper]

Adding a few others for thoughts: @kommendorkapten @loosebazooka

[haydentherapper]

The other option is to simply trigger releases manually from what's in main.

[woodruffw]

Would switching to release:published help with preventing pushes from non-protected branches? Couldn't you still publish a release from a tag that is from a branch?

I think you could, assuming that user has release creation permissions. However, the release workflow itself could be put in an environment with a configured "required reviewers" rule, to prevent someone from unilaterally publishing a package through a release.

For example, here's what our release environment looks like on pip-audit:

(and in the case of protobuf-specs, I figure we'd want the "Prevent self-review" option as well 🙂)

[kommendorkapten]

Yay to using environments (and prevent self-review) as it's a good way to force manual approval before a workflow is run. We don't release that often, and adding a a manual control is worth it I believe.

Making the switch from tags to release events seems solid. I'm thinking if we should keep workflows for tags for alfa, beta or pre-releases though, as it can be good testing without requiring extra assistance.

[haydentherapper]

#254 to track, closing this out.