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

feat(Dockerfile): switch to scratch base image #544

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

paulopontesm
Copy link
Contributor

@paulopontesm paulopontesm commented Oct 19, 2023

📝 Summary

This MR switches the base Docker image from Alpine to scratch, reducing the image size and eliminating unnecessary dependencies.

Again, not changing logic so I didn't open an issue. Let me know if I should. I am making an assumption (explained below) that we don't need this because of the way that we are compiling the binary. I did a simple test of running locally and it works, but I might be missing something.

⛱ Motivation and Context

Why Use Scratch?

  1. Reduced Size: A scratch image is an empty layer, meaning it doesn't carry any extra libraries, making the image extremely small in size. Decrease from 32MB -> 22MB
  2. Reduced Dependencies: No need for extra packages or libraries, making the image lighter and less complex.
  3. Security Advantages:
    • Reduced Surface for Attacks: Eliminates potential risks tied to any libraries or packages that Alpine might bring in.
      • Example: The absence of a shell in scratch means shell-based attacks are not applicable.
    • No Outdated Libraries: No risk of having vulnerable or outdated libraries as we are building from scratch.
    • Dependabot handles updates, ensuring that all package versions are maintained in the go.mod file.

Why Don't We Need Alpine and Libraries?

Assumption: During the go build ..., we use -linkmode external -extldflags '-static'. These flags instruct the compiler to statically link all dependencies into the binary. As a result, the mev-boost-relay binary should be fully self-contained and not dependent on any shared libraries 🤞. This makes the transition to scratch seamless while making the image more secure and less prone to vulnerabilities.

📚 References


✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy
  • I have seen and agree to CONTRIBUTING.md

@codecov-commenter
Copy link

Codecov Report

Merging #544 (3d42492) into main (c52858a) will not change coverage.
Report is 4 commits behind head on main.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff           @@
##             main     #544   +/-   ##
=======================================
  Coverage   33.26%   33.26%           
=======================================
  Files          24       24           
  Lines        5090     5090           
=======================================
  Hits         1693     1693           
  Misses       3178     3178           
  Partials      219      219           
Flag Coverage Δ
unittests 33.26% <ø> (ø)

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

@metachris
Copy link
Collaborator

Thanks for proposing this!

I recall trying scratch a while ago and not going with it, but don't remember if it was just due to lack of time or any issues I spotted. Open to changing the base image if we can make sure there's no side-effects.

@paulopontesm
Copy link
Contributor Author

Open to changing the base image if we can make sure there's no side-effects

No problem. I might be missing something, so your input is welcome. What tests could we run to make sure the changes are safe? Got an idea for quick tests?

I recall trying scratch a while ago and not going with it, but don't remember if it was just due to lack of time or any issues I spotted

Got it 😄. I dug into the Git history to try to get more context 🕵️. Let me know if I'm looking wrong at something...

  1. Looking at this diff
    • Initially, the project used scratch as the base image.
    • The flags needed to include libraries in the final binary during the go build... weren't set.
    • Because of this, the base was changed to alpine + apk add lib{}
  2. A recent change linked the binary statically. This was to adapt to Go version 1.20 (looking at this diff).
    • I'm not sure what caused the initial issue. It could be a mismatch between the libs in golang:1.20 builder and the alpine image 🤔

Also, for reference, here are the differences between dynamically vs. statically linked binaries:

  1. Dynamically Linked

    $ file mev-boost-relay
    mev-boost-relay: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=1ada77a40e9dfd9ef74bef38c2f0df7bf747e4d0, for GNU/Linux 3.7.0, stripped
  2. Statically Linked

    $ file mev-boost-relay
    mev-boost-relay: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, BuildID[sha1]=5d31b9f3a24200632cf57d027f2169b7c5bdceda, for GNU/Linux 3.7.0, stripped

Note: We're copying the CA certificates from the builder stage into our image, ensuring that mev-boost-relay can establish secure SSL connections without issues. I believe that this step wasn't necessary with the alpine base image but is required when using scratch.

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