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

Docker: set permissions for config directory #94

Merged
merged 1 commit into from
Sep 3, 2023
Merged

Docker: set permissions for config directory #94

merged 1 commit into from
Sep 3, 2023

Conversation

mkuf
Copy link
Contributor

@mkuf mkuf commented Sep 1, 2023

Hi there,

while adding spoolman to prind, I noticed that when using a named volume to store the spoolman-db, permissions for /home/app/.local/share/spoolman are set to root:root which causes the server to fail as the app user lacks permissions to write to it.

Example:

services:
  spoolman:
    image: ghcr.io/donkie/spoolman:edge
    restart: unless-stopped
    volumes:
      - spoolman-db:/home/app/.local/share/spoolman

volumes:
  spoolman-db:

This PR creates the database directory and sets app:app as owner at image build time.
Furthermore, to follow Best practices adds the VOLUME directive to the dockerfile for the database directory.

Looking forward to your response.
-Markus

@Donkie
Copy link
Owner

Donkie commented Sep 3, 2023

Seems to work well, thank you!

@Donkie Donkie merged commit 2db9823 into Donkie:master Sep 3, 2023
9 checks passed
@Donkie
Copy link
Owner

Donkie commented Nov 7, 2023

How important is the VOLUME directive for you @mkuf ?

I've been looking into the issue people are having with data going missing etc. One issue is that they forget to mount the data dir, or they mount it incorrectly. If I remove the VOLUME directive from the Dockerfile, I will be able to inside the container detect if the directory is mounted or not (that info is available from the unix mount command). However if the VOLUME directive is there then I can't distinguish if the data dir has been mounted by the user or just by the default anonymous volume that docker creates.

If I then can detect that it's not mounted, I can warn people that data will go poof in the server logs

@mkuf
Copy link
Contributor Author

mkuf commented Nov 7, 2023

There is no requirement for the volume directive, its just best practice.
If it makes life easier for you and users of spoolman, you can remove it from the image without breaking anything.

The important thing about this PR is that the directory is created and permissions for it are set at build time, so named docker volumes can be used.

-Markus

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

Successfully merging this pull request may close these issues.

2 participants