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

Switch to a slimmer distroless base image #738

Merged
merged 1 commit into from Aug 9, 2021

Conversation

mattmoyer
Copy link
Contributor

@mattmoyer mattmoyer commented Jul 22, 2021

At a high level, it switches us to a distroless base container image (see #448), but that also includes several related bits:

  • Add a writable /tmp but make the rest of our filesystems read-only at runtime.

  • Condense our main server binaries into a single pinniped-server binary. This saves a bunch of space in the image due to duplicated library code (see Merge all Pinniped servers into a single binary to save image size #449). The correct behavior is dispatched based on os.Args[0], and the pinniped-server binary is symlinked to pinniped-concierge and pinniped-supervisor.

  • Strip debug symbols from our binaries. These aren't really useful in a distroless image anyway and all the normal stuff you'd expect to work, such as stack traces, still does.

  • Add a separate pinniped-concierge-kube-cert-agent binary with "sleep" and "print" functionality instead of using builtin /bin/sleep and /bin/cat for the kube-cert-agent. This is split from the main server binary because the loading/init time of the main server binary was too large for the tiny resource footprint we established in our kube-cert-agent PodSpec. Using a separate binary eliminates this issue and the extra binary adds only around 1.5MiB of image size.

  • Switch the kube-cert-agent code to use a JSON {"tls.crt": "<b64 cert>", "tls.key": "<b64 key>"} format. This is more robust to unexpected input formatting than the old code, which simply concatenated the files with some extra newlines and split on whitespace.

  • Update integration tests that made now-invalid assumptions about the pinniped-server image.

Overall this trims our container image size from 254 MB (in v0.9.2) to roughly 54 MB!

Release note:

Switch to a distroless base image for the Pinniped server components.

@codecov
Copy link

codecov bot commented Jul 22, 2021

Codecov Report

Merging #738 (cd7a4f6) into main (a464c81) will decrease coverage by 0.58%.
The diff coverage is 81.96%.

❗ Current head cd7a4f6 differs from pull request most recent head 58bbffd. Consider uploading reports for the commit 58bbffd to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #738      +/-   ##
==========================================
- Coverage   79.89%   79.30%   -0.59%     
==========================================
  Files         124      127       +3     
  Lines        8369     8636     +267     
==========================================
+ Hits         6686     6849     +163     
- Misses       1464     1563      +99     
- Partials      219      224       +5     
Impacted Files Coverage Δ
internal/concierge/server/server.go 27.96% <0.00%> (-2.59%) ⬇️
...l/localuserauthenticator/localuserauthenticator.go 56.48% <0.00%> (ø)
cmd/pinniped-concierge-kube-cert-agent/main.go 100.00% <100.00%> (ø)
cmd/pinniped-server/main.go 100.00% <100.00%> (ø)
internal/controller/kubecertagent/kubecertagent.go 91.38% <100.00%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a464c81...58bbffd. Read the comment docs.

Copy link
Contributor

@enj enj left a comment

Choose a reason for hiding this comment

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

I will fix these minor issues in a follow-up since Moyer is OOO.

deploy/supervisor/deployment.yaml Show resolved Hide resolved
cmd/pinniped-server/main.go Outdated Show resolved Hide resolved
cmd/pinniped-server/main.go Outdated Show resolved Hide resolved
cmd/pinniped-server/main.go Outdated Show resolved Hide resolved
test/integration/concierge_impersonation_proxy_test.go Outdated Show resolved Hide resolved
At a high level, it switches us to a distroless base container image, but that also includes several related bits:

- Add a writable /tmp but make the rest of our filesystems read-only at runtime.

- Condense our main server binaries into a single pinniped-server binary. This saves a bunch of space in
  the image due to duplicated library code. The correct behavior is dispatched based on `os.Args[0]`, and
  the `pinniped-server` binary is symlinked to `pinniped-concierge` and `pinniped-supervisor`.

- Strip debug symbols from our binaries. These aren't really useful in a distroless image anyway and all the
  normal stuff you'd expect to work, such as stack traces, still does.

- Add a separate `pinniped-concierge-kube-cert-agent` binary with "sleep" and "print" functionality instead of
  using builtin /bin/sleep and /bin/cat for the kube-cert-agent. This is split from the main server binary
  because the loading/init time of the main server binary was too large for the tiny resource footprint we
  established in our kube-cert-agent PodSpec. Using a separate binary eliminates this issue and the extra
  binary adds only around 1.5MiB of image size.

- Switch the kube-cert-agent code to use a JSON `{"tls.crt": "<b64 cert>", "tls.key": "<b64 key>"}` format.
  This is more robust to unexpected input formatting than the old code, which simply concatenated the files
  with some extra newlines and split on whitespace.

- Update integration tests that made now-invalid assumptions about the `pinniped-server` image.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
@enj enj enabled auto-merge August 9, 2021 19:09
@enj enj merged commit 01ddc7a into vmware-tanzu:main Aug 9, 2021
@mattmoyer mattmoyer deleted the distroless branch August 9, 2021 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants