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

Merged
merged 24 commits into from
Jan 8, 2025
Merged

Add rootless (alpine) images #4617

merged 24 commits into from
Jan 8, 2025

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
Contributor

woodpecker-bot commented Dec 24, 2024

Deployment of preview was torn down

Copy link

codecov bot commented Dec 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.23%. Comparing base (8a98b3f) to head (34ee024).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4617   +/-   ##
=======================================
  Coverage   28.23%   28.23%           
=======================================
  Files         399      399           
  Lines       28238    28238           
=======================================
  Hits         7973     7973           
  Misses      19560    19560           
  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.

@pat-s
Copy link
Contributor Author

pat-s commented Dec 25, 2024

How often do you need to debug your apps?

I am managing close to 10 WP instances, so it sums up. Especially in highly-restricted environments, it is a quite regular tasks, as changes to the networking layer often happens without prior information.

rootless alpine should just replace the existing alpine variant.

Alright, as it seems this is the only agreement we have here, I'll then proceed this way. This at least enables the option to use the alpine variants by default without running a shell container as root.

@pat-s

This comment was marked as off-topic.

@pat-s pat-s force-pushed the feat/rootless-images branch from c455d89 to 34ee024 Compare December 27, 2024 10:11
@xoxys

This comment was marked as off-topic.

@pat-s

This comment was marked as off-topic.

@pat-s pat-s marked this pull request as ready for review December 29, 2024 14:22
@pat-s pat-s force-pushed the feat/rootless-images branch from b5d909a to 3f806e2 Compare December 31, 2024 13:59
@lafriks
Copy link
Contributor

lafriks commented Dec 31, 2024

Rootless image can be also easily implemented for scratch images

@xoxys
Copy link
Member

xoxys commented Jan 3, 2025

@pat-s do you plan to finish this one?

@lafriks
Copy link
Contributor

lafriks commented Jan 5, 2025

If I understand correctly that alpine version will be rootless only?

Imho best solution if we do not provide both rootfull and rootless images would be to switch both (scratch and alpine) images as rootless so that there are no permission problems when switching between them etc and 3.0 would be probably best time to do that

@pat-s
Copy link
Contributor Author

pat-s commented Jan 5, 2025

Yes, switching between both variants is then not easily possible anymore as the directory permissions are different.

To make this change smoother, there should ideally be an init-container for the helm chart which adjust permissions, if needed.

But yes, this change will break all existing instances and manual permission adjustments are required.

@lafriks
Copy link
Contributor

lafriks commented Jan 5, 2025

Ideally we should probably switch both types (alpine&scratch) then to the same user id and release notes should warn about permission changes required when upgrading to 3.0

@lafriks lafriks added the breaking will break existing installations if no manual action happens label Jan 5, 2025
docker/Dockerfile.agent.multiarch.rootless Outdated Show resolved Hide resolved
docker/Dockerfile.agent.multiarch.rootless Outdated Show resolved Hide resolved
docs/docs/30-administration/04-image-variants.md Outdated Show resolved Hide resolved
docs/docs/30-administration/04-image-variants.md Outdated Show resolved Hide resolved
docs/src/pages/migrations.md Outdated Show resolved Hide resolved
@xoxys
Copy link
Member

xoxys commented Jan 6, 2025

The created group and user should be created with a fixed uid/gid. A common choice is 1000.

@pat-s pat-s added this to the 3.0.0 milestone Jan 6, 2025
@pat-s pat-s requested a review from xoxys January 7, 2025 07:53
@xoxys
Copy link
Member

xoxys commented Jan 7, 2025

Have you tested the new images?

@pat-s
Copy link
Contributor Author

pat-s commented Jan 8, 2025

Yes, built locally and applied them in my dev instance without issues (I even didn't had to any permissions changes for the persistent volumes, I guess because content is only consumed and reading is permitted anyway).

Instances using the docker backend and storing logs in files in the respective dirs might encounter permissions issues though, I guess.

@pat-s pat-s merged commit 20323a8 into main Jan 8, 2025
7 checks passed
@pat-s pat-s deleted the feat/rootless-images branch January 8, 2025 09:05
@woodpecker-bot woodpecker-bot mentioned this pull request Jan 8, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking will break existing installations if no manual action happens feature add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants