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

sudo fails to run in k8s #14

Open
telmich opened this issue Dec 19, 2021 · 10 comments
Open

sudo fails to run in k8s #14

telmich opened this issue Dec 19, 2021 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@telmich
Copy link

telmich commented Dec 19, 2021

When trying to run thecodingmachine/workadventure-front:v1.6.4 it fails with:

[10:58] nb3:generic% kubectl logs -f front-5f7d75f9cb-9n6xc 
sudo: unable to send audit message
sudo: pam_open_session: System error
sudo: policy plugin failed session initialization

Changing the cmd to shell and testing manually:

docker@front-758dbc949-jm2v2:/var/www/html$ sudo -i
sudo: unable to send audit message
sudo: pam_open_session: System error
sudo: policy plugin failed session initialization

Seeing that the initial script switches directly to execute sudo tini, I wonder if it wouldn't make sense to...

  • Remove sudo
  • Execute the container with uid 0 by default?
@mistraloz mistraloz added the bug Something isn't working label Dec 20, 2021
@mistraloz mistraloz self-assigned this Dec 20, 2021
@mistraloz
Copy link
Contributor

Hi @telmich, i'm agree about the UID 0 but if we do that, it's will generate a breaking change if any dependent image have a Dockerfile ended by USER docker (the recommanded good pratice... even if it's has no sense with sudo allowed).
I will look for a solution but tell me if you have any more informations/suggests.

@telmich
Copy link
Author

telmich commented Dec 20, 2021

Good morning @mistraloz! I think the USER docker makes sense in docker environments, not so much though in k8s environments, TBH.

I have seen that your back/front containers are separated, but they are derived from the same base image. If you want to avoid breaking existing users, I'd suggest the following:

  • Create a fresh image w/o sudo usage
  • Create dependent images with -nosudo or -root suffix
  • This can even live in the same namespace (like thecodingmachine/workadventure-back) as before - other images do this with -alpine or similar

If you happen to have a chat (slack, matrix, etc.) I can also join there, as I would be open to some improvements, maybe even supporting an -alpine version.

@mistraloz
Copy link
Contributor

1/ If you know how to create one without required of sudo usage, do it, it's the best way :).
2/ Otherwise instead of -nosudo / -root suffix, we may have an environment to choice (or just test the current user, if it's root, with don't need sudo).
3/ An alpine version may be very nice but it's another point. If you would like to create one, we can have both (alpine and debian) but please create another issue for that, I will follow this PR with you. I'm agree for a tag "-alpine" (or "-slim").
4/ I think this issue it's provide by a miss configuration in sudoers config. So we can fix it too.

I'm not available for 3hours but after i may discuss with you by chat if you are available.

@moufmouf
Copy link
Member

In fact, in Kubernetes, by default, you cannot use "sudo" in the containers unless you explicitly allow privilege escalation (which is documented here: https://github.com/thecodingmachine/docker-images-nodejs#usage-in-kubernetes )

Indeed, we do this for Docker environments, so that the running user in Docker is the same as the current user of the OS (which is helpful if your process is writing files).
We inherited this behavior from the TheCodingmachine Docker PHP image (https://github.com/thecodingmachine/docker-images-php) where this is a must have (PHP processes usually write their cache in files and Docker containers often have write issues because the users of Docker images do not match with the users of the host).

I'm not 100% sure making a variant of the image is the way to go. Because if you build images on top of those images, you will have to make several variants.

The best possible solution would be to be able to detect (somehow, I don't really know how), that we are running in Kubernetes and to avoid using "sudo" in this case. If we can detect that the "/usr/src/app" directory is NOT mounted (but part of the image itself), then there is no point in changing the user ID dynamically as we do, and therefore, no point in running sudo ?

@telmich
Copy link
Author

telmich commented Dec 20, 2021

I prefer staying slim/keeping the number of variants down, @moufmouf, but due to the way how the image is built, by default the userid is set to docker at the moment.

We did patch our way around this by using

securityContext:
    runAsUser: 0

(compare
https://code.ungleich.ch/ungleich-public/ungleich-k8s/src/branch/master/apps/workadventure/v3/templates/back-deployment.yaml)

So on entrance point, the user is already set to docker by default and if the image is not changed, you cannot easily revert this.

In other words: to be k8s native, the image will need to not switch away user in the first place, which is currently part of the Dockerfile.

Does it make sense?

@telmich
Copy link
Author

telmich commented Dec 20, 2021

Comparing with https://github.com/thecodingmachine/docker-images-nodejs/blob/master/Dockerfile.14-apache:

RUN useradd -ms /bin/bash docker && adduser docker sudo
 # Users in the sudoers group can sudo as root without password.
 RUN echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
...
USER docker
...
RUN mkdir -p /var/www/html && chown docker:docker /var/www/html
...
ENV APACHE_RUN_USER=docker \
    APACHE_RUN_GROUP=docker

could be removed and additionally the entrypoint scripts could either be modified or a new one that is made for running without sudo could be added.

It might be possible to create the image like this and use docker-compose with the user: attribute to force another user, if necessary.

@mistraloz
Copy link
Contributor

I'm not sure but this part may not require any changes. I think the mandatory changes are here
This file is used to :

  • Test if the user who run the entrypoint have uid 1000
  • If not, set the uid of user docker as the required
  • Run a2enmod command (root is required but can be changed to allow it to be run by another user : it's just a symbolic link to be created)
  • Run supercronic (cron binary) but may not required to be run as root
  • Run startup commands as docker user (so may not required root privileges too)
  • Expose vars to apache (for -apache variant) and run entrypoint (not require root privilges).

So to make a PR who accept to be run without sudoers, I think we have to :

  • Allow to bypass tests of UID (just an environment var like SUDO_DISABLED=true or we can test too comething like env | grep -qE "^KUBERNETES_" but it's less customisable)
  • Find a solution to run a2enmod as docker user
  • Verify that supercronic run well as docker user (may not required any change, but test is required)
  • Verify compatibility with regular variant and -apache variant

@telmich
Copy link
Author

telmich commented Dec 20, 2021

The item "Find a solution to run a2enmod as docker user" can be easily solved by

chown docker /etc/apache2/mods-enabled/

in the Dockerfile.

In regards to autodetection vs. SUDO_DISABLED=true, I would have a strong preference on the latter, as it is explicit and might be used in other scenarios as well.

@mistraloz
Copy link
Contributor

I'm suspicious of solutions that seem too easy :p ... but you have right, it's seem to be the only need. Can you make a PR for that ? I'm will be on holidays and make changes during the christmas sales may be not a good idea but i can merge that early january.

@ericzolf
Copy link

ericzolf commented Mar 2, 2022

I don't know how far it is related but I was able to reproduce the same sudo issue with debian:stretch-slim, on which this image is based, and fixing it by using debian:bullseye-slim instead. Could it be that upgrading the base for the image would fix the issue?
see workadventure/workadventure#1925

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants