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

Add rootless (alpine) images #4617

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Add rootless (alpine) images #4617

wants to merge 7 commits into from

Conversation

pat-s
Copy link
Contributor

@pat-s pat-s commented Dec 24, 2024

Background

Currently, two image variants exist:

  • scratch-based images
  • alpine-based

The former is a few MB smaller than the alpine one but lacks the ability to exec into it as it has no shell.
The latter has a shell but as both run as root user, using the alpine image is somewhat risky.

Possible solutions

There are two ways to approach this:

  1. Add a new rootless variant for the alpine variant (-> allows to exec into the pod but without being root)
  2. Replace the scratch variant by a rootless alpine variant and make it the default

With (1), there would be three image variants that need to be build and maintained. Having this many, possible confusion among users exists. Also sticking with the scratch image as the default is not super convenient as adminds always have to switch to the alpine one first for interactive actions

With (2), there would be only two image variants (alpine-rootless and alpine-rootful). Making alpine-rootless the default (i.e. tagging with only semver versions) provides the same security as before because running rootless = same as using scratch, one cannot do anything destructive within the container.)

It would additionally provide the ability to exec into the container/pod. A possible downside would be that the image is a few MB bigger (e.g. 17mb instead of 13mb for the server image).

I'd like to get a maintainers vote here how to proceed. Based on that, I would like to a add a new docs page which explains the existing tags and their indented usage, similar to what has been added recently to the repository descriptions on DockerHub.

@woodpecker-ci/maintainers
Vote with 🚀 for keeping scratch and having a new alpine-rootless variant.
Vote with 👀 for dropping scratch and replacing it with the new alpine-rootless variant.

PS: I tested the server and agent images in a k8s instance. The CLI image was tested locally.

@pat-s pat-s added the feature add new functionality label Dec 24, 2024
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Dec 24, 2024

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-4617.surge.sh

Copy link

codecov bot commented Dec 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.25%. Comparing base (8a98b3f) to head (ae694ae).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4617      +/-   ##
==========================================
+ Coverage   28.23%   28.25%   +0.01%     
==========================================
  Files         399      399              
  Lines       28238    28238              
==========================================
+ Hits         7973     7978       +5     
+ Misses      19560    19555       -5     
  Partials      705      705              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xoxys
Copy link
Member

xoxys commented Dec 25, 2024

The latter has a shell but as both run as root user, using the alpine image is somewhat risky.

Im a bit confused. The scratch image is not running as root. So in production just use the scratch one and for debugging if you need a shell use the alpine one? For everyday operation I dont see any reason to have a shell.

Im fine to make the alpine one rootless by default as this can be reverted by the user easily if needed.

@pat-s
Copy link
Contributor Author

pat-s commented Dec 25, 2024

Im a bit confused. The scratch image is not running as root.

It is? Unless a different USER is set, root is being used. Also the directories are owned by root. It is only "secure" because there is no shell, yet all operations are done as root.

So in production just use the scratch one and for debugging if you need a shell use the alpine one? For everyday operation I dont see any reason to have a shell.

Exactly this is the problem. Switching the image just for this is tedious. Scratch images are fine for plugins and other "read-only" approaches IMO but the agent and server images should have a shell.

Hence, a better default would be rootless image (i.e. running as an unprivileged non-root user) but having the ability to exec in. This way, one can inspect logs and other things in a limited way.

For everyday operation I don't see any reason to have a shell.

I'd argue that there should always be a shell paired with an unprivileged user by default for applications like WP.


If the scratch image would be replaced by the rootless-alpine one, the alpine one per se would have to be renamed as well then. So then

  • vx.y.z -> alpine-rootless with shell
  • vx.y.z-rootful -> alpine-rootful with shell

@xoxys
Copy link
Member

xoxys commented Dec 25, 2024

Could you explain why you need a shell for the server container? I use multiple distroless/scratch images in production and there is IMO no need to have a shell for pure binary apps. It is a very common practice to switch images for debugging.

Im against adding more flavours due to the not necessary maintenance. As I said Im fine to add a rootless alpine image but this should replace the default alpine one and not the scratch one.

@pat-s
Copy link
Contributor Author

pat-s commented Dec 25, 2024

For all sorts of debugging activities? Inspecting connection issues to the agent, checking ports, grpc, DB connections.
The runtime env (k8s, docker) doesn't matter here. A shell in a container is always helpful.
Yet it should be non-privileged by default.

It is a very common practice to switch images for debugging.

To me, using scratch images is a predated approach to securing containers before the rootless idea became usable/accepted. One should always have the option to undertake the above mentioned checks in a limited and non-destructive environment without having to switch images upfront.

@xoxys
Copy link
Member

xoxys commented Dec 25, 2024

How often do you need to debug your apps? So yes I do this every time I have to debug something. Tracing network issues can be done outside of the container itself, logs can be written to a volume or even better stdout etc.

I disagree that rootless images fix every problem that could be fixed by distroless base containers. So I would like to keep scratch images. No strong opinion on adding other flavours even if I still think rootless alpine should just replace the existing alpine variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants