Onboard Vault to CRT version bump automation
PR Description
This PR introduces several of the changes detailed in ENGSRV-087: CRT standardization for release branches and versions.
- The new CRT version bump automation including a change to where the release build version is set (new
version/VERSIONfile formajor.minor.patch<-prerelease>) and the new actions-set-product-version that is responsible for reading from the new file and setting version information for the entire build workflow. See here and here for more info. - actions-go-build, an action that builds/packages/uploads go binaries and assesses their reproducibility. *NOTE: assert is disabled for now.
Additionally:
- The new prepare workflow to replace the several workflows that were previously involved in processing release artifacts. See here for more info.
There has been some refactoring to the enos bits when it comes to reading version information as part of CI - so a review from the Quality team would be good here.
Note - this PR will not be back-ported to the release branches as it relies on the recent changes to main that are targeted for the next minor release (moving version out of the sdk folder) cc @ncabatoff @mladlow
Note - PR for vault-enterprise will be ready for review tomorrow.
TODO Before Merging
- [ ] Remove
testing-set-product-versionbranch frombuild.ymlworkflow dispatch list - [ ] Remove
testing-set-product-versionbranch fromci.hclallowlist
Adding myself as a reviewer so updates on this PR go to my inbox :)
Any reason why we're using version/VERSION as the location, specifically the directory? I kind of liked that .go-version is in the root, and it feels like those files belong together. I'm also not fond of having it in a directory where we have .go files, though I guess we could move those. (Yes I know main.go lives in the root, but it's special.)
Also https://hashicorp.atlassian.net/wiki/spaces/RELENG/pages/2484306717/How+To+Onboard+Version+Bump+Automation says the directory should be .release, can we make the wiki and the PR consistent?
@ncabatoff - yes we should definately update the wiki. We had initially thought to use .release or place the file in the root of the repo as you mentioned. But in an effort to find one location that suited all repos - having it in the version folder beside version.go, allowed for the case where some teams were using go::embed to set the version from an existing static file. There seem to be limitations on where the static files can live in that instance. We did discuss allowing the location of the VERSION file to be overridden in both the actions-set-product-version action and in the version bump workflow, which we could implement to allow flexibility here.
@ncabatoff - yes we should definately update the wiki. We had initially thought to use
.releaseor place the file in the root of the repo as you mentioned. But in an effort to find one location that suited all repos - having it in theversionfolder besideversion.go, allowed for the case where some teams were usinggo::embedto set the version from an existing static file. There seem to be limitations on where the static files can live in that instance. We did discuss allowing the location of theVERSIONfile to be overridden in both theactions-set-product-versionaction and in the version bump workflow, which we could implement to allow flexibility here.
Makes sense. No, I don't feel strongly enough about that I want us to diverge from a standard. There are still some comment(s) in this PR that refer to .release, FYI (at least one that I saw anyway.)
I'm very excited to see all of the packaging steps handled automatically! Super cool.
My primary concerns with this are two:
- We seem to be splitting up build and test helpers and I'm not entirely sure why.
- We're going to make changes to main that will differ significantly from those on release branches. This is going to make modifications really painful after we just did loads of work to make them identical.
Thanks @ryancragun! We aren't so much splitting build and test helpers, but splitting those functions needed by local Enos runs when it comes to verifying the binary version. If we were ok with the local Enos runs building Vault in the same way as in CI (parsing the values from the new VERSION file and then passing them in as ldflags) then I think we could remove that new bash script entirely and simplify the build function. I'm not totally familiar with how local Enos builds work but when I get back from holidays I'll ping you to work through this. I totally agree that we definitely don't want to undo all of the great work that's gone into this so far!
An aside here, @ncabatoff and I chatted briefly about not backporting these changes to release branches due to the SDK version move work being targeted for the next minor version only. But if having significant changes between main and the release branches is going to be a massive headache, maybe we could rethink this
@mladlow @ryancragun @ncabatoff - apologies for the delay on this, it has been a bit hectic since January. I have tidied this up Ryan to hopefully make it clearer that for local enos runs, the version for verification purposes needs to be generated differently (read directly from version-base.go).
I realized that even if we do this the way I suggested previously in the PR (get the local enos build to use the values from version/VERSION.go like we will now do in CI), there are going to be differences between main and the release branches anyway - as in CI our new action set-product-version grabs the version from version/VERSION and actions-go-build does more than just run the make target (bundles dir etc..). So we would have to have a script/make targets to do these bits for local runs regardless.
❓ Since we built our changes on top of the changes that moved version.go out of the SDK folder, I think the bigger question here is, are we happy with not back-porting those original changes to the release branches and having release branches that will differ from main for a good while? The changes in this PR could easily be added to all release branches if the original changes were back ported first.
The changes in this PR could easily be added to all release branches if the original changes were back ported first.
Backporting the sdk version changes will be painful, because there were many plugins that needed to be updated to remove a dependency on the sdk. I'm not saying that we therefore shouldn't do it, just wanted to make sure that was clear.
@ncabatoff Should we backport this change to the 1.13 branch? I think that one has the SDK changes? I have no strong feelings about the other release branches.
@mladlow Yes, definitely to 1.13 at least.
@mladlow @ncabatoff @ryancragun - this PR is now back up to date with main.
There are no big changes from what y'all previously reviewed.
The main bulk of these changes are to do with how we set the version both in the binary and in the artifact names - and then how we verify that the version has been set correctly, in tests.
My assumptions around local enos runs could be wrong, so please let me know @ryancragun if you'd rather go a different way with this (i.e using the default version from version.go instead of passing in a version at build time).
I've added the backport labels for 1.13.x and 1.14.x only due to the convo above about the sdk changes.
I am going to work this afternoon on truing up the vault-enterprise PR but would appreciate your 👀 on this asap so we can get this squared away! 🙏
CI Results: All Go tests succeeded! :white_check_mark:
Build Results: All builds succeeded! :white_check_mark: