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 possibility to override capabilities (--cap-add) #10655

Closed
oliv3r opened this issue Jun 5, 2023 · 10 comments · Fixed by #10669
Closed

Add possibility to override capabilities (--cap-add) #10655

oliv3r opened this issue Jun 5, 2023 · 10 comments · Fixed by #10669

Comments

@oliv3r
Copy link

oliv3r commented Jun 5, 2023

Description

During normal lifetime of a compose job, the capabilities as defined in the yaml file are fine. However, inital setup might need temporary additional capabilities, especially when not running the container as root. For example, assuming cap-drop: all is set in the yaml file.

A container has a volume mounted, and is not run as root. For example volume: myvolume:/home/user/.config (ignore the poor choice for the example). By default, running this container, will volume-mount things as root. So for bootstrapping purposes, one needs to run:
docker compose run --user root mycontainer chown user:user -R /home/user and maybe docker compose run --user root mycontainer chgrp g+w -R /home/user. Both these operations require additional capabilities. While one can (temporary) add CAP_CHOWN and CAP_FOWNER, it requires manual work on the yaml file.

This could easily be solved by being able to do:

docker compose run --cap-add CAP_CHOWN --user root mycontainer chown user:user -R /home/user
docker compose run  --cap-add CAP_FOWNER --user root mycontainer chgrp g+w -R /home/user

While one can run the (not yet existing) container with plain docker, the container name might not be known yet, the volume name might not be known etc, so using this within the compose context is very important.

@ndeloof
Copy link
Contributor

ndeloof commented Jun 5, 2023

I'm worried if we introduce such command line flag to compose run we will get feature requests to introduce all the others, with valid use cases for all of them.

Fresh new volume being owned by root is a common issue with docker, which can be addressed if you manage volume initial content and ownership in your Dockerfile, maybe that's a better approach?

FROM alpine
RUN mkdir /foo && chown 1001:1001 /foo
VOLUME /foo
USER 1001
$ docker build -t foo .
$ docker run --user 1001 --volume foo:/foo foo ls -al /foo
total 8
drwxr-xr-x    2 1001     1001          4096 Jun  5 12:32 .

@oliv3r
Copy link
Author

oliv3r commented Jun 7, 2023

I'm worried if we introduce such command line flag to compose run we will get feature requests to introduce all the others, with valid use cases for all of them.

Probably all valid too ;) so something worth contemplating.

Fresh new volume being owned by root is a common issue with docker, which can be addressed if you manage volume initial content and ownership in your Dockerfile, maybe that's a better approach?

It was just an example, but:

FROM alpine
RUN mkdir /foo && chown 1001:1001 /foo

How do you know the UID? You'd have to parse it from the passwd/group file (not impossible of course). But you are touching a dockerfile. The case of this example is, there is no dockerfile, right? we are using compose, with an official container from upstream. Lets taket he docker container for arguments sake :p and I want to initialize /var/lib/docker with the right ownership. The container runs as user, so doing chown isn't possible (though overriding as said with --user is an option).

I get that you can do FROM docker instead of FROM alpine, but having a Dockerfile for each/many containers, just to create the volumes is a world of hurt and additional maintainance.

VOLUME /foo
USER 1001


$ docker build -t foo .
$ docker run --user 1001 --volume foo:/foo foo ls -al /foo

so this solution in the end, is no better then doing:

sed -i '/^    cap_drop:$/i\    cap_add:\n      - CAP_CHOWN\n      - CAP_FOWNER' 'compose.yaml'
docker_compose run \
        --user 'root:root' \
        --rm \
        docker chown -R 'docker:docker' '/var/lib/docker'
docker compose run \
        --user 'root:root' \
        --rm \
        docker chmod 'g+w' '/var/lib/docker'
sed -i '/^    cap_add:$/,+2d' 'compose.yaml'
docker compose up --detach docker

It is manageable, but ugly as heck :)

total 8
drwxr-xr-x 2 1001 1001 4096 Jun 5 12:32 .

@ndeloof
Copy link
Contributor

ndeloof commented Jun 7, 2023

The main issue here is that you use your service container to run maintenance commands, which require extra privileges. You should better use a distinct container definition for this need. You could for example declare a general utility container for this scenario:

services:
   set-volume-owner:
     profiles: ["setup"]
     image: alpine
     user: root
     command: chown user:user -R /volume && chgrp g+w -R /volume
     volumes:
        - ${VOLUME}:/volume

so you can run:

VOLUME= myvolume docker compose run set-volume-owner

or as I assume you planned to define this docker compose run --cap-add ... command in a Makefile, you also could just run a plain docker run -v myvolume:/volume alpine chown user:user -R /volume command, as compose is not a replacement for plain docker CLI to manage resources :)

@oliv3r
Copy link
Author

oliv3r commented Jun 7, 2023

The main issue here is that you use your service container to run maintenance commands, which require extra privileges. You should better use a distinct container definition for this need. You could for example declare a general utility container for this scenario:

services:
   set-volume-owner:
     profiles: ["setup"]
     image: alpine
     user: root
     command: chown user:user -R /volume && chgrp g+w -R /volume
     volumes:
        - ${VOLUME}:/volume

so you can run:

VOLUME= myvolume docker compose run set-volume-owner

or as I assume you planned to define this docker compose run --cap-add ... command in a Makefile, you also could just run a plain docker run -v myvolume:/volume alpine chown user:user -R /volume command, as compose is not a replacement for plain docker CLI to manage resources :)

Ah, but the thing is, we don't know what 'volume' is. compose generates its own naming (how predictable and future proof is it?). Even the container name, is a generated name. So you now have to derive the name, from a not running container, to be able to derive the volume name. Sure, you can use external: true and pre-define things, but it also is, a kludge.

I'm looking at automated bootstrapping, not so much maintanence. Maintanance would be handled differently, rightfully as per your example. Bootstrap is a 'one-off' Might need to do it once (maybe twice) on a service/farm. The additional service is an option, but it's also not pretty imo. The nice thing about docker compose run, is that a) it creates a temporary container (name), and allows you to use your existing compose definition, to get into exactly the properly configured container. Also, we can't do inheritence in services, though a yaml anchor could work. All-in-all, it just makes things harder then it needs to be.

I then also fail to understand the reason why docker compose run exists at all, what are all the flags for, what is run for, if not maintanance/bootstrapping. up is for normal opration is it not?

@ndeloof
Copy link
Contributor

ndeloof commented Jun 7, 2023

ok, then you could define an init-container. There's unfortunately no compact syntax to declare one (yet) but here is an illustration example:

services:
   my-service:
     volumes:
        - my-volume:/volume
     depends_on:
        set-owner:
           condition: service_completed_successfully

    set-owner:
        command: chown user:user -R /volume && chgrp g+w -R /volume
        volumes:
          - my-volume:/volume

volumes:
  my-volume: {}

generally speaking, the issue for volumes to be created with root ownership is already debated on moby/moby#2259

@oliv3r
Copy link
Author

oliv3r commented Jun 8, 2023

ok, then you could define an init-container. There's unfortunately no compact syntax to declare one (yet) but here is an illustration example:

services:
   my-service:
     volumes:
        - my-volume:/volume
     depends_on:
        set-owner:
           condition: service_completed_successfully

    set-owner:
        command: chown user:user -R /volume && chgrp g+w -R /volume
        volumes:
          - my-volume:/volume

volumes:
  my-volume: {}

generally speaking, the issue for volumes to be created with root ownership is already debated on moby/moby#2259
Yeah, not sure if I should hold my breath on that one, it's been open for 10 years :p

Well it was just ONE example :) but 'initialization'/bootstrapping, is always different from maintenance/runtime.

What if you need to run your program with a --firstrun parameter the very first time, to initialize stuff. What if you want your service to initially be brought up in a maintenance mode, run scripts/stuff to configure your container. All these hint very strongly towards a scripted 'run' vs an actual 'service'. Even more so, when we are talking about bootstrapping the actual services.

Everything you do in the compose file, is really just 'trying to polish a turd'. You are defining a 'new service, to pretend it's the same one, to do initial work'.

So in that case, the sed-solution is ugly, but at least its clear what's happening with the rest of the steps.

I suppose the lesser of the evils would be anchors (or really, inheritance/dependencies), to make it clear it's the same service, still ugly though ;)

@ndeloof
Copy link
Contributor

ndeloof commented Jun 8, 2023

What if you need to run your program with a --firstrun parameter the very first time, to initialize stuff. What if you want your service to initially be brought up in a maintenance mode, run scripts/stuff to configure your container. All these hint very strongly towards a scripted 'run' vs an actual 'service'. Even more so, when we are talking about bootstrapping the actual services.

exact, bootstrapping is exactly the reason people use init containers in kubernetes, and we unfortunately don't (yet) have a decent syntax for this purpose in compose - I wish I can get some time to investigate on this topic and we eventually can offer something like this:

   my-service:
     init:
        - command: chown user:user -R /volume && chgrp g+w -R /volume
           user: root
     volumes:
        - my-volume:/volume

On the other hand, I've logged moby/moby#45714 as this is a very common requirement and there's no simple solution to address this.

@ndeloof
Copy link
Contributor

ndeloof commented Jun 8, 2023

I created #10669 as you convinced me --cap-add will unlock many usability constraint for user not running container as root (which they all should!).

@thaJeztah
Copy link
Member

Note that CAP_CHOWN is part of the default capabilities; I'm not sure if adding the capabilities option fixes this (also many capabilities may still require root permissions)

@oliv3r
Copy link
Author

oliv3r commented Jun 8, 2023

Note that CAP_CHOWN is part of the default capabilities; I'm not sure if adding the capabilities option fixes this (also many capabilities may still require root permissions)

well in my example, you create your service with the most restricted permissions possible, cap_drop: all; container ideally sets user to some unpriviledged user etc.

But during bootstrap, you can still do docker compose run --user root:root --cap-add CAP_CHOWN myservice install.sh for example.

@ndeloof thanks for creating #10669 :) I wonder if there's a need for --privileged too ;p But not sure what the difference would be, as I haven't run into the need yet ;)

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

Successfully merging a pull request may close this issue.

4 participants
@ndeloof @oliv3r @thaJeztah and others