Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-platform cleanup #58

Merged
merged 6 commits into from
May 18, 2023
Merged

Multi-platform cleanup #58

merged 6 commits into from
May 18, 2023

Conversation

onyxraven
Copy link
Member

@onyxraven onyxraven commented May 15, 2023

Background

Build consistency for multiplatform

  • builds docker manifest image using buildx bake
  • builds universal binary for macos
  • builds windows binaries
  • updates installer scripts to be os/platform aware
  • adds apk package
  • adds sboms

Versioning

Minor - we're updating a few file paths and things.

Additional Requests to Reviewers

Tasks

  • Specs written
  • Manual testing

@@ -1,8 +1,14 @@
FROM alpine:latest as build
ARG TARGETARCH
Copy link
Member Author

Choose a reason for hiding this comment

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

gets amd64 etc from the --platform arg

COPY sopsinstall.sh /tmp/sopsinstall.sh
RUN sh /tmp/sopsinstall.sh -b /usr/local/bin
RUN sh /tmp/sopsinstall.sh -b /usr/local/bin -a $TARGETARCH
Copy link
Member Author

Choose a reason for hiding this comment

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

decided to continue using a script to get this since sops puts the version number in their github release artifacts, so it still needs to do the tag lookup.

$DIR/../sopsinstall.sh -o linux -a arm64 "$SOPSDIST"
$DIR/../sopsinstall.sh -o darwin -a amd64 "$SOPSDIST"
$DIR/../sopsinstall.sh -o darwin -a arm64 "$SOPSDIST"
$DIR/../sopsinstall.sh -o windows -a amd64 "$SOPSDIST"
Copy link
Member Author

Choose a reason for hiding this comment

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

uses the same downloader to grab the archives for upload to our s3.

@onyxraven onyxraven force-pushed the multiplatformdocker branch 6 times, most recently from 123bf4e to c5275ee Compare May 15, 2023 20:39
@onyxraven onyxraven force-pushed the multiplatformdocker branch from c5275ee to a10040a Compare May 15, 2023 20:53
@onyxraven onyxraven changed the title Multiplatformdocker Multi-platform cleanup May 15, 2023
@onyxraven onyxraven marked this pull request as ready for review May 15, 2023 20:58
# Whether to remove the previous single-arch binaries from the artifact list.
# If left as false, your end release might have both several macOS archives:
# amd64, arm64 and all.
replace: true
Copy link
Member Author

Choose a reason for hiding this comment

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

locally, it still produces all 3. i'm not sure what the release will look like

# Default is `{{ .ProjectName }}_{{ .Version }}_checksums.txt`.
name_template: "{{ .ProjectName }}_checksums.txt"

sboms:
- artifacts: binary
documents: ["{{ .Binary }}_{{ .Os }}_{{ .Arch }}.sbom"]
Copy link
Member Author

Choose a reason for hiding this comment

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

match format of the binary

extra_files:
- sopsinstall.sh
- glob: ./*install.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

have the deploy upload the installers instead of the extra script

REGEX="^v([0-9]+)\.([0-9]+)\.([0-9]+)"
export VERSION=$(jq -r '.version' dist/metadata.json)
export TAG=$(jq -r '.tag' dist/metadata.json)
if [[ "${TAG}" =~ ${REGEX} ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

gets the version information from the goreleaser metadata output

Comment on lines +15 to +18
goarm:
- ""
goamd64:
- ""

Choose a reason for hiding this comment

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

Is an empty string meant to clear the defaults? If so, you could set them to:

goarm: []
goamd64: []

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I have to specificaly set them - the default for goamd64 is v1 and it ends up in variant stuff, which is weird. I can try but this is what the docs kinda looked like

Comment on lines +30 to +32
format_overrides:
- goos: windows
format: zip

Choose a reason for hiding this comment

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

Do we use windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ibotta doesnt, but I want to on a windows project.

Comment on lines +10 to +11
# grab appropriate sopstool binary from dist
COPY dist/sopstool_linux_$TARGETARCH/sopstool /usr/local/bin/sopstool

Choose a reason for hiding this comment

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

Did you mean to copy the dist file from the host machine?

Choose a reason for hiding this comment

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

I see now that the Dockerfile is just importing the binary anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes - this is the build of the image from the compiled source outputs of goreleaser.

install.sh Outdated
Comment on lines 64 to 67
http_exec https://raw.githubusercontent.com/Ibotta/sopstool/master/sopsinstall.sh -b "${BINDIR}" "$@" "${SOPS_VERSION}"
fi

http_exec https://raw.githubusercontent.com/Ibotta/sopstool/master/sopstoolinstall.sh -b "$BINDIR" "$SOPSTOOL_VERSION"
http_exec https://raw.githubusercontent.com/Ibotta/sopstool/master/sopstoolinstall.sh -b "${BINDIR}" "$@" "${SOPSTOOL_VERSION}"

Choose a reason for hiding this comment

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

These github links are using master instead of main

Copy link
Member Author

Choose a reason for hiding this comment

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

good call, I thought I found them all but this snuck through

@@ -29,6 +29,9 @@ jobs:
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2

- name: Set up Syft
uses: anchore/sbom-action/download-syft@v0
Copy link
Member Author

Choose a reason for hiding this comment

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

this is so goreleaser can create SBOMs for the binaries (we also have docker buildx, which uses the same tool, create SBOMs for the containers)

@@ -1,3 +1,5 @@
report_sizes: true
Copy link
Member Author

Choose a reason for hiding this comment

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

these get tracked in an output json, this was useful for debugging.

@@ -8,39 +8,69 @@ usage() {

$this: download binaries for sopstool

Usage: $this [bindir]
[bindir] sets bindir or installation directory, Defaults to ./bin
Usage: $this [-b bindir] [-o OS] [-a ARCH] [-s SOPS_VERSION] [-t SOPSTOOL_VERSION] [-d] [bindir]
Copy link
Member Author

Choose a reason for hiding this comment

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

this now uses getopts, but is backward compatible.

if ! is_command sops; then
http_exec https://raw.githubusercontent.com/Ibotta/sopstool/master/sopsinstall.sh -b "$BINDIR" "$SOPS_VERSION"
if [ -n "${TARGET_ARCH}" ]; then
set -- "$@" "-a" "${TARGET_ARCH}"
Copy link
Member Author

Choose a reason for hiding this comment

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

arrays in posix shells lol.

sort -t "." -n -k1,1 -k2,2 -k3,3 -k4,4 | head -n 1)
if [ "${LOWEST_ARCH_VERSION}" != "${MIN_ARCH_VERSION}" ]; then
oldarch="${VERSION}"
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

theres quite a bit of 'friendlyness' here that maybe isn't fully necessary - it'll bork at you if you pick arm64 windows, for example. or arm64 before sops was building for it,

install "${TMPDIR}/${NAME}" "${BINDIR}/${BINARY}"
log_info "installed ${BINDIR}/${BINARY}"
}
archive_binary() {
Copy link
Member Author

Choose a reason for hiding this comment

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

all of this is for our download and re-upload, though its kinda nice to have otherwise too. I might consider contribbing this to sops.

netbsd) return 0 ;;
openbsd) return 0 ;;
plan9) return 0 ;;
solaris) return 0 ;;
Copy link
Member Author

Choose a reason for hiding this comment

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

I went ahead and removed the unsupported os/arch from these lists as well

@@ -257,90 +296,47 @@ github_release() {
test -z "$version" && return 1
echo "$version"
}
hash_sha256() {
Copy link
Member Author

Choose a reason for hiding this comment

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

sops doesn't publish checksums :(

darwin/amd64) BINARIES="sopstool" ;;
darwin/arm64) BINARIES="sopstool" ;;
linux/amd64) BINARIES="sopstool" ;;
linux/arm64) BINARIES="sopstool" ;;
windows/amd64) BINARIES="sopstool.exe" ;;
windows/arm64) BINARIES="sopstool.exe" ;;
Copy link
Member Author

Choose a reason for hiding this comment

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

we can do windows now ;)

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Rishi Sheth <2817944+physik932@users.noreply.github.com>
Copy link
Contributor

@elementalvoid elementalvoid left a comment

Choose a reason for hiding this comment

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

LGTM! These are some much-needed cleanups.

@onyxraven onyxraven merged commit f8ed143 into main May 18, 2023
@onyxraven onyxraven deleted the multiplatformdocker branch May 18, 2023 17:26
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.

5 participants