-
Notifications
You must be signed in to change notification settings - Fork 6
Description
There are a couple of patterns for this job in our workflow templates. Some run only im-open/is-tag-reachable-from-default-branch and others run im-open/is-tag-reachable-from-default-branch followed by im-open/is-release-production-ready.
Neither of these actions ensures that the requested tag refers to a commit that was actually merged to the default branch. Even when used together, there are still gaps.
Problem with is-tag-reachable-from-default-branch
im-open/is-tag-reachable-from-default-branch permits tags on commits that were placed on a branch prior to that branch being merged into the default branch. Based on the name of the action, this behavior is reasonable.
Given this example of a history "reachable from default branch"
* 8d1b01e87 HEAD -> main, tag: 3.169.79, origin/main, origin/HEAD
|\
| * 20cd91656 tag: 3.169.79-branch-b.3
| * 7bc411250
| * beb397d8c tag: 3.169.79-branch-b.2
| * 78fe99c29
| * e4a0c177e tag: 3.169.79-branch-b.1
* | 36ccfee6f tag: 3.169.78
|\ \
| |/
|/|
| * 193c149f2 tag: 3.169.78-branch-a.2
| * 36b9f9633 tag: 3.169.78-branch-a.1
| |\
| |/
|/|
* | ddc7a2714
/
* 567f0bdb1
All of the tags in that log would be accepted by the action because they're in the ancestry of main—they're reachable, however, only the commits tagged 3.169.78 and 3.169.79 were the results of approved PRs merged to the main branch.
If our goal is to deploy only builds that came from (approved and) merged PRs, that action is not sufficient.
It would be better to check the --first-parent history of the default branch to ensure the tag is found on one of the commits that actually modified the default branch. (However, that won't work as long as we continue to have the problem where the build of a merged PR places the release tag on the head of the incoming branch instead of on the resulting merge commit. In that case, the release tags must on an immediate parent of the merge commit.)
Problem with is-release-production-ready
This action looks only at the attributes of the GitHub release (draft, pre-release), not at the semver format of the release's associated tag or where it lies in the Git history.
It would be better to also require that the tag associated is a release semver version (without a pre-release label).
Putting it together
A GitHub release object is production ready if and only if
- Its assets were built from code on the default branch, (the protected branch), after an approved, merged PR— that's the point of our branch protection rules.
- The GitHub release needs to be associated to a tag on the commit holding the code that was built.
- This should be signified by a non-prerelease semver tag placed on a commit to the default branch. It could be a merge commit, or a squashed commit depending on how the team manages their PR workflow. However, since we can't prevent arbitrary tags from being placed on arbitrary commits in the repository, we need to examine the direct history of the default branch and use other heuristics, (maybe even inspect GitHub Pull Request objects), to ensure that a release's source code and build outputs came from code in approved PRs.
- It has already been deployed to a test environment, tested and cleared through the regular QA process.
- Teams might rely on the QA attestation step in the deployment workflow, or they might rely on a non-automated process outside of GitHub
- It might be worth considering ways to integrate some of that tracking in Git or GitHub. With some improved deployment tracking, we could allow teams to ensure that a release was at least deployed to a certain environment before going to prod, like was done with lifecycles in Octopus. We also might find a way to securely record a QA approval in the Git repository itself, prior to the deployment workflow.
I suggest we update the is-release-production-ready action to check the semver of the tag and to have it ensure the tag is identifies only the approved merges to the protected branch. Then we can deprecate the is-tag-reachable-from-default-branch action. Then we can simplify our deployment workflows a little by putting this action into the first job. We also probably wouldn't need verify-git-ref anymore.