Skip to content

Goreleaser update#1466

Merged
Steven Gagniere (sgagniere) merged 10 commits intomainfrom
goreleaser-update
Oct 13, 2022
Merged

Goreleaser update#1466
Steven Gagniere (sgagniere) merged 10 commits intomainfrom
goreleaser-update

Conversation

@sgagniere
Copy link
Member

Checklist

  1. [CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?
    N/A

What

Newer versions of goreleaser append a GOAMD64 microarchitecture value to the build directory name in dist for amd64 builds. The default is _v1, which is the same microarchitecture as builds using older versions. This convention change causes Windows and Darwin signing/notarization to fail because the paths to the binaries are hardcoded into the .goreleaser.yml and gon_confluent_amd64.hcl files.

This PR updates the signing lines to account for this.

  • For Windows, the hardcoded relative path in the post hook is replaced with {{ .Path }}, which is a goreleaser variable available to the hook storing the absolute path to the binary.
  • For Darwin, the file path must be provided in an .hcl file. Since modifying the .hcl directly results in a dirty repo, a post hook makes a copy inside the dist folder (which is in .gitignore) and another post hook calls the new gon_filepath_editor.sh script which uses {{ .Path }} to update the copy.
    • I could not get the sed command to work directly in a post hook; that's why I made it into a script
  • I also made this change for the Darwin arm64 signing.

The reason for using {{ .Path }} instead of hardcoding the new convention (and for modifying the arm64 signing even though the folder name hasn't changed) is because the Goreleaser docs note that there is no guarantee that conventions will not change in future versions. So we remove all assumptions on what the naming convention is.

Test & Review

  • Ranrelease-to-stag
  • Downloaded the .exe and .zip (and extracted from the zip) from S3 staging folder & github release and checked that they're all signed correctly with .osslsigncode verify path/to/binary
  • Likewise checked that the amd64 Darwin binaries are signed correctly with codesign -dv --verbose=4 path/to/binary

@sgagniere Steven Gagniere (sgagniere) requested a review from a team as a code owner October 12, 2022 23:38
Copy link
Contributor

@MuweiHe MuweiHe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We shouldve fixed those hardcoded paths long ago... There're also a lot of hardcoded paths in release targets too (like coping stuff from local to s3) that we can follow up as well.

.goreleaser.yml Outdated
post:
- cmd: make download-licenses
- cmd: gon gon_confluent_amd64.hcl
- cmd: ./gon_filepath_editor.sh {{ .Path }} gon_confluent_amd64.hcl

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put all of the gon files in a scripts/ directory? There's a lot of them now and I think that's the best place for them... https://github.com/golang-standards/project-layout#scripts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I can also move the new sh file and the build_linux_glibc file in there too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I just realized... by removing the hardcoded path from the gon files, they're now identical. So I can replace them with one.

echo "BUILDING FOR DARWIN, WINDOWS, AND ALPINE LINUX" && \
GO111MODULE=off go get -u github.com/inconshreveable/mousetrap && \
GOPRIVATE=github.com/confluentinc VERSION=$(VERSION) HOSTNAME="$(HOSTNAME)" GITHUB_TOKEN=$(token) S3FOLDER=$(S3_STAG_FOLDER_NAME)/confluent-cli goreleaser release --rm-dist --timeout 60m -f .goreleaser.yml; \
rm -f CLIEVCodeSigningCertificate2.pfx && \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention: this change is to add redundancy in the case where goreleaser errors out before it can run the post-hook to remove this file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's only remove this if it exists then?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could add the file to .gitignore and the make clean target, just in case it doesn't get cleaned up automatically.

Copy link
Member Author

@sgagniere Steven Gagniere (sgagniere) Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -f flag removes it if it exists and does nothing w/o error if it doesn't.

I think this is enough. It should always be removed now unless the process is stopped with ctrl+c.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, didn't know about -f!

@sgagniere Steven Gagniere (sgagniere) deleted the goreleaser-update branch October 13, 2022 20:22
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