vault icon indicating copy to clipboard operation
vault copied to clipboard

Onboard Vault to CRT version bump automation

Open sarahethompson opened this issue 3 years ago • 4 comments

PR Description

This PR introduces several of the changes detailed in ENGSRV-087: CRT standardization for release branches and versions.

  1. The new CRT version bump automation including a change to where the release build version is set (new version/VERSION file for major.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.
  2. actions-go-build, an action that builds/packages/uploads go binaries and assesses their reproducibility. *NOTE: assert is disabled for now.

Additionally:

  1. 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-version branch from build.yml workflow dispatch list
  • [ ] Remove testing-set-product-version branch from ci.hcl allowlist

sarahethompson avatar Dec 12 '22 16:12 sarahethompson

Adding myself as a reviewer so updates on this PR go to my inbox :)

mladlow avatar Dec 14 '22 15:12 mladlow

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 avatar Dec 16 '22 15:12 ncabatoff

@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.

sarahethompson avatar Dec 16 '22 16:12 sarahethompson

@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.

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.)

ncabatoff avatar Dec 16 '22 16:12 ncabatoff

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

sarahethompson avatar Dec 21 '22 19:12 sarahethompson

@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.

sarahethompson avatar Feb 15 '23 13:02 sarahethompson

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 avatar Feb 15 '23 13:02 ncabatoff

@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 avatar Feb 15 '23 14:02 mladlow

@mladlow Yes, definitely to 1.13 at least.

ncabatoff avatar Feb 15 '23 17:02 ncabatoff

@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! 🙏

sarahethompson avatar Jun 16 '23 12:06 sarahethompson

CI Results: All Go tests succeeded! :white_check_mark:

github-actions[bot] avatar Jul 21 '23 12:07 github-actions[bot]

Build Results: All builds succeeded! :white_check_mark:

github-actions[bot] avatar Aug 18 '23 11:08 github-actions[bot]