anatol / pacoloco

Caching proxy server for Arch Linux pacman
MIT License
199 stars 30 forks source link

Docker image writes files as root, ignores PUID, GUID #99

Closed Gibbz closed 5 months ago

Gibbz commented 5 months ago

Ive noticed the docker image downloads and stores the files as root user on my system. I have environment set as bellow, which is the normal user on the system. Any chance this will be fixed soon? Its causing me issues with updates as I have to chown the dir when I want to update using some other tools...

    environment:
      - PUID=1000
      - PGID=1000
anatol commented 5 months ago

cc @dezeroku @ilya-zlobintsev @HarHarLinks

HarHarLinks commented 5 months ago

While I agree that running the container rootless would be better, I'm not sure you're using pacoloco in the intended way. It is a caching proxy for arch package repos. You should not require to touch the cached files on any way?

Can you explain more what your usecase is?

ilya-zlobintsev commented 5 months ago

Instead of specifying the user id as an env var, you can try putting user: 1000 in the compose so the entire container runs with that uid.

dezeroku commented 5 months ago

I've played with making the container rootless and there's surprisingly many points where it can go wrong, some pointers:

  1. Simply specifying user: UID or user: UID:GID in docker-compose.yml won't work as user with such an id doesn't exist in the container, it actually makes sense
  2. If a host path is defined in volumes in docker-compose.yml, and it doesn't exist on host yet, it'll be created with the root:root ownership, completely ignoring the UID set in container or docker-compose.yml. As such pacoloco inside container will fail with wrong permissions

Ad 1. Honestly I don't think it makes sense to try to read UID and GID from the host and dynamically create the user, but we can definitely go for the 1000:1000 as a default. It's a rather standard approach for the web-stuff containers

Ad 2. I can imagine the confusion this will cause regarding permissions. It works fine as long as the required directories already exist on host, they are mounted with the same ownership then. We'd have to extend the README with creating the required directory tree. This would be also breaking for everyone updating from an older version, but it should be enough to chown -R 1000:1000 the required directories.

All in all I'd be willing to whip up a PR for that, if we're all on the same page regarding the UID:GID "hardcode".

Oh, and this whole comment is a bit offtop in the context of this issue :D It'll "solve" the issue when the host user is 1000:1000 (it should be true for most desktops, not so much for servers), but fail in the same manner if host IDs are different

HarHarLinks commented 5 months ago

While @dezeroku is correct, I can't agree with the proposed course of action. The 1000 user can be any unprivileged user and we would not want them to be able to mess with the cached packages, root is much safer. If at all, it should be the user explicitly stated by the one running the container (i.e. docker-compose) as explained, which also as explained isn't as trivial to do.

My standpoint remains tat this issue is invalid, as having other tools access (specifically write) the cached files is simply not the intended use case for pacoloco, sorry.

dezeroku commented 5 months ago

If at all, it should be the user explicitly stated by the one running the container (i.e. docker-compose) as explained, which also as explained isn't as trivial to do.

I tried some more and it might not be so hard in the end. The failing code was related to sanity checks about the config directory, but if we ditch the username part of this check and print just the userid it works fine. With this in place we still default to root and can opt-in for a different user: UID:GID pair if desired. Please take a look at the linked PR.

Gibbz commented 5 months ago

Thanks. I've been writing a tool that downloads updated packages on my phone. Then when I'm home I transfer them to the pacoloco cache so they're ready for updating the local computers.

I don't have much data at home so this saves a big chunk for me.

Gibbz commented 5 months ago

Ive updated to this new version. Im getting the error bellow. Ive checked the uid and done a chown on the folder to ensure its all good, but its still giving this error...

pacoloco | config.go:85: directory /var/cache/pacoloco does not exist or isn't writable for userid 1000

This is the docker file im using:

version: '3'

services:
  pacoloco:
    container_name: pacoloco
    image: ghcr.io/anatol/pacoloco:latest
    user: 1000:1000
    ports:
      - "9129:9129"
    volumes:
      - /mnt/backup/Docker/pacoloco:/var/cache/pacoloco/pkgs
      - ${HOME}/Docker/pacoloco/config/pacoloco.yaml:/etc/pacoloco.yaml
      - ${HOME}/Docker/pacoloco/config/manjaro_mirrorlist:/etc/pacman.d/manjaro_mirrorlist
      - ${HOME}/Docker/pacoloco/config/arch_mirrorlist:/etc/pacman.d/arch_mirrorlist
      - /etc/localtime:/etc/localtime:ro
    restart: unless-stopped
dezeroku commented 5 months ago

Problem is most likely with this mount:

      - /mnt/backup/Docker/pacoloco:/var/cache/pacoloco/pkgs

pacoloco checks the permissions on cache_dir, which defaults to /var/cache/pacoloco (and it doesn't seem like you've changed it), so when you mount your directory under /var/cache/pacoloco/pkgs what happens is:

You'd have to change the mount to something like

- /mnt/backup/Docker/pacoloco:/var/cache/pacoloco

and one-time action move the content from /mnt/backup/Docker/pacoloco to a pkgs subdirectory. Then it should work fine