Add minimal Dockerfile and a goreleaser docker config

[BenPhegan]

I was looking to introduce the creation of a minimal docker image to help with using this in automated CI flows, and developer workstations.

Not sure of the approach you use to release, so I am more than happy to discuss and modify to ensure that this suits your workflow. Also, it has been a while since I worked with GoLang tooling, so please let me know if you would prefer this to be implemented another way.

This also includes a few fixes to the Makefile to ensure that goreleaser is available for targets that require it, as it did not function on my machine out of the box.

[BenPhegan]

@wolfeidau - let me know if you need anything else on this? Happy to discuss the approach further. I noticed that there are about 6 images on Docker Hub already that are providing saml2aws (all old to some degree). Would prefer we could get an official one published as part of the release process so it can stay updated.

[BenPhegan]

@wolfeidau - any updates on this one?

[BenPhegan]

Bump

[mapkon]

@BenPhegan Thank you for this. Can you rebase from master and fix the conflicts? I am happy to merge after that

[BenPhegan]

@mapkon - great, will have to take a look at a few of the changes that have crept in since I put the PR in. Looks like some of the changes will mean a few tweaks.

[gliptak]

relatedly what is the major difference between go and release GHA? goreleaser/goreleaser-action usage seems to be similar

https://github.com/Versent/saml2aws/tree/master/.github/workflows

[BenPhegan]

@mapkon - this is a little more complicated now. With the inclusion of this commit: https://github.com/Versent/saml2aws/commit/08a0a70fb1afc96211317560245c783fbe7d24aa it creates an issue with lightweight docker images, as it becomes difficult to statically compile and distribute a single file binary. This seems to be due to taking on a dependency for hidraw, which brings in libudev, which according to this is not built statically.

I could:

1. Change the FROM to a more complete Docker image, however that is going to make this a much larger download.
2. Look at pulling in and compiling a static version of libudev (which may be a bit of work).
3. Frankencontainer with modified rpath/LD_LIBRARY_PATH and a bunch of copied dependencies?

Or there may be another simpler way to solve this.

@smlx - do you have any ideas on this one? Is libudev strictly required if using hidraw?

[smlx]

Yes I believe so. You'll probably need to switch to an alpine image for Linux.

[mapkon]

Yes I believe so. You'll probably need to switch to an alpine image for Linux.

I wouldn't worry about switching to alpine images. We need something light-weight, not prohibitively light weight. @gliptak - what do you think?

[gliptak]

I recommend separate PR for the goreleaser bump

Submitted #1023 which can be used to build on

[smlx]

Well in case it helps I got an alpine based image working on Linux with this Dockerfile:

FROM golang:1-alpine3.17 AS builder

RUN cd / && \
      wget https://github.com/goreleaser/goreleaser/releases/download/v1.16.2/goreleaser_Linux_x86_64.tar.gz \
      && tar -xzf goreleaser_Linux_x86_64.tar.gz

RUN apk add --no-cache \
      alpine-sdk \
      linux-headers \
      eudev-dev

COPY . /saml2aws

RUN cd /saml2aws \
      && ln -fs /goreleaser ./bin/goreleaser \
      && make build

FROM alpine:3.17

RUN apk add --no-cache \
      eudev

COPY --from=builder \
      /saml2aws/dist/saml2aws_linux_amd64_v1/saml2aws \
      /saml2aws

ENTRYPOINT ["/saml2aws"]

Run it like this:

docker run --rm -it --device /dev/hidraw7 -v $HOME/.saml2aws:/root/.saml2aws -v $HOME/.aws:/root/.aws saml2aws login --disable-keychain -a exampleAccount

This image weighs 24.7MB vs the distroless image which is 41.1MB (when built on my machine).

[BenPhegan]

Ok, a few thoughts after looking at this:

1. Neither suggested Dockerfile will work in their current form. Both expect the source code to be available as part of the Docker build context. However, with goreleaser you only get a binary from the builds: that matches your goarch/goos. There is the option of adding extra_files to the docker build, however this does not accept globs. So, using default goreleaser config it does not seem a simple process to build the executable within docker.
2. goreleaser does not allow an arbitrary build process to be run as a build. If we could, we could just build and drop out the MUSL/Alpine build and then just pick it up later.
3. We could build the Dockerfile (any arbitrary one really) from within the Makefile, and drop out a build target file as part of the Makefile target. However this would create a fragile contract between goreleaser and the Makefile, based on a location in the dist directory and a bunch of arbitrary goos/goarch values.

Suggested ways forward:

1. We add an additional set of build definitions to the existing goreleaser definition that build a static version of an amd64 binary in addition, and just forego the added capabilities (also clearly stating this in documentation). This would mean that there would be two builds for linux/amd64, one static and one dynamic. We then use the static binary for the Docker builds. I think this is probably the easiest way?
2. Package in a minimal ubuntu image. That way, nothing changes on the build or the application functionality, but you get a big container download.
3. Add in a bunch of Makefile special sauce and goreleaser tweaks to try and fool the Docker build process into packaging a binary build externally to goreleaser via the extra_files functionality.
4. Completely decouple the Docker build from goreleaser and just use a Makefile target.

I will look to implement 1. and see how I go. I have most of this working locally at the moment, with a single goreleaser definition building both static and dynamic, so it seems that it should slot in nicely and also provide people with a statically compiled binary option as well for those that do not need the newly added functionality that requires CGO.

Let me know if you guys have strong opinions either way though.

[mapkon]

Ok, a few thoughts after looking at this:

1. Neither suggested Dockerfile will work in their current form. Both expect the source code to be available as part of the Docker build context. However, with goreleaser you only get a binary from the builds: that matches your goarch/goos. There is the option of adding extra_files to the docker build, however this does not accept globs. So, using default goreleaser config it does not seem a simple process to build the executable within docker.
2. goreleaser does not allow an arbitrary build process to be run as a build. If we could, we could just build and drop out the MUSL/Alpine build and then just pick it up later.
3. We could build the Dockerfile (any arbitrary one really) from within the Makefile, and drop out a build target file as part of the Makefile target. However this would create a fragile contract between goreleaser and the Makefile, based on a location in the dist directory and a bunch of arbitrary goos/goarch values.

Suggested ways forward:

1. We add an additional set of build definitions to the existing goreleaser definition that build a static version of an amd64 binary in addition, and just forego the added capabilities (also clearly stating this in documentation). This would mean that there would be two builds for linux/amd64, one static and one dynamic. We then use the static binary for the Docker builds. I think this is probably the easiest way?
2. Package in a minimal ubuntu image. That way, nothing changes on the build or the application functionality, but you get a big container download.
3. Add in a bunch of Makefile special sauce and goreleaser tweaks to try and fool the Docker build process into packaging a binary build externally to goreleaser via the extra_files functionality.
4. Completely decouple the Docker build from goreleaser and just use a Makefile target.

I will look to implement 1. and see how I go. I have most of this working locally at the moment, with a single goreleaser definition building both static and dynamic, so it seems that it should slot in nicely and also provide people with a statically compiled binary option as well for those that do not need the newly added functionality that requires CGO.

Let me know if you guys have strong opinions either way though.

@BenPhegan

I am not sure what the appetite for this is, but I don't suppose that having a big container download is a big let off. I would seriously consider option 2 above, since it is the least intrusive from a maintenance perspective.

[BenPhegan]

I actually went with static as a first pass, as I noticed there was some appetite for this outside of the use of Docker. Let me know if this PR makes sense now, happy to discuss further and adjust as required.

[BenPhegan]

Ok, rebased due to conflicts in master. It seems that reproducibility is drifting as the Github Actions are defining versions that are not reproducible locally, including Golang and Goreleaser versions. Is there an intent to bring this back in line and share across build mechanisms?

[codecov-commenter]

Codecov Report

Merging #890 (f2ad410) into master (68d09f2) will not change coverage. The diff coverage is n/a.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master     #890   +/-   ##
=======================================
  Coverage   36.40%   36.40%           
=======================================
  Files          52       52           
  Lines        7546     7546           
=======================================
  Hits         2747     2747           
  Misses       4415     4415           
  Partials      384      384           

Flag	Coverage Δ
unittests	36.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[BenPhegan]

@mapkon - put a comment on the other PR, but they take different approaches. I would suggest closing one or the other, and merging the efforts. My preference is an approach that includes goreleaser compatibility, but that is only based on the fact that it appears to be the primary mechanism the project is using to build/deploy currently.

[BenPhegan]

@mapkon - outstanding issues with this PR:

1. Acceptance that this change creates both dynamic and static binaries for Linux, where the static linux binaries have reduced capabilities.
2. Documentation should be updated to reflect this, if we accept 1. above. Happy to update as necessary if you are ok with the static binary approach.
3. goreleaser Docker manifest creation and publishing have not been tested, as they require a Docker Hub account registered to Versent and a full deployment to be run. There may be tweaks associated with this that I just cannot test.
4. Optimisation would be to remove the .goreleaser.macos-latest.yml definition completely and just use a single definition file/build. I can do this as a seperate PR as well.

As this is being used in preference to a single Dockerfile as per the linked PR, it would be good to get feedback from @gliptak to make sure he is comfortable with this approach as well?

[gliptak]

maybe merge #1010 and start working Dockerfile/etc. after?

@BenPhegan we might also consider publishing to ghcr.io

https://github.com/goreleaser/goreleaser/blob/main/.goreleaser.yaml#L83

[mapkon]

@mapkon - outstanding issues with this PR:

1. Acceptance that this change creates both dynamic and static binaries for Linux, where the static linux binaries have reduced capabilities.
2. Documentation should be updated to reflect this, if we accept 1. above. Happy to update as necessary if you are ok with the static binary approach.
3. goreleaser Docker manifest creation and publishing have not been tested, as they require a Docker Hub account registered to Versent and a full deployment to be run. There may be tweaks associated with this that I just cannot test.
4. Optimisation would be to remove the .goreleaser.macos-latest.yml definition completely and just use a single definition file/build. I can do this as a seperate PR as well.

As this is being used in preference to a single Dockerfile as per the linked PR, it would be good to get feedback from @gliptak to make sure he is comfortable with this approach as well?

The management of docker credentials really worries me. Is there a way we can avoid that?

[mapkon]

maybe merge #1010 and start working Dockerfile/etc. after?

@BenPhegan we might also consider publishing to ghcr.io

https://github.com/goreleaser/goreleaser/blob/main/.goreleaser.yaml#L83

There is no PR on #1010 - I realize you meant this PR: https://github.com/Versent/saml2aws/pull/1029, which I will merge.

[gliptak]

@mapkon ghcr.io requires no additional credentials

[BenPhegan]

@mapkon - ok, I will hold off as I will need to redo the PR based on top of #1029 (which actually ends up in a similar place as this one I believe). There will be conflicts due to the duplication. I will pick this up again when #1029 is complete. Also, yes it makes complete sense to use ghcr.io, I will update PR.

[mapkon]

@BenPhegan #1029 is merged, and I have closed #1023 so we can focus on this.

[BenPhegan]

Ok, great, I will try and get some time to tidy this (rebase etc) today.

[BenPhegan]

Took me a bit longer than expected to get to this, but I think this is looking reasonable now, with the caveats that the Docker manifest publishing has not been tested...

[mapkon]

Took me a bit longer than expected to get to this, but I think this is looking reasonable now, with the caveats that the Docker manifest publishing has not been tested...

Thank you. What is remaining here for us to be ready to merge?

[BenPhegan]

@mapkon the release process remains to be tested, which I cannot do as I do not have the ability to generate a token for the project to push to ghcr.io. There are probably some tweaks to the documentation depending on that release process as well. So two approaches we could take:

1. Someone with the ability to generate a release environment similar to the Github Actions environment for the branch pulls this locally, sets up the environment and tests the release process from their machine, and adjusts anything that does not work accordingly.
2. We merge it, and I follow up with a PR fixing any release based issues if they arise.

I think 2. is probably easier, if you are happy to give that a shot. If you let me know when it is going to be merged I can try to be on line to ensure I can respond quickly to any issues with the docker manifest deployment process.

[gliptak]

ghcr.io relies on secrets.GITHUB_TOKEN

@BenPhegan consider running (ghcr.io) release on your fork

[mapkon]

ghcr.io relies on secrets.GITHUB_TOKEN

@BenPhegan consider running (ghcr.io) release on your fork

@BenPhegan Please see @gliptak reply. We should test on your branch/fork before merging to master. That way, we can limit the blast radius.