Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(docker): add support for pgid and puid env variables #1759

Merged
merged 10 commits into from
Dec 31, 2024

Conversation

Meierschlumpf
Copy link
Member


Homarr

Thank you for your contribution. Please ensure that your pull request meets the following pull request:

  • Builds without warnings or errors (pnpm buid, autofix with pnpm format:fix)
  • Pull request targets dev branch
  • Commits follow the conventional commits guideline
  • No shorthand variable names are used (eg. x, y, i or any abbrevation)

@Meierschlumpf Meierschlumpf self-assigned this Dec 23, 2024
@Meierschlumpf Meierschlumpf requested a review from a team as a code owner December 23, 2024 13:25
@Meierschlumpf Meierschlumpf added the bug Something isn't working label Dec 23, 2024
Copy link

deepsource-io bot commented Dec 23, 2024

Here's the code health analysis summary for commits 56b57ad..0bb52fa. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

github-actions bot commented Dec 23, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 21.53% 7908 / 36720
🔵 Statements 21.53% 7908 / 36720
🔵 Functions 24.63% 285 / 1157
🔵 Branches 61.76% 861 / 1394
File CoverageNo changed files found.
Generated in workflow #4426 for commit 0bb52fa by the Vitest Coverage Report Action

@Meierschlumpf Meierschlumpf changed the title fix: issue with docker socket permissions fix(docker): issue with docker socket permissions Dec 23, 2024
@darkcloud784
Copy link

Won't chowning the socket also impact other things using said socket? IE portainer. If so, might want to think about setting a user field that someone can run the container as that will have access to said socket.

@Meierschlumpf
Copy link
Member Author

Won't chowning the socket also impact other things using said socket? IE portainer. If so, might want to think about setting a user field that someone can run the container as that will have access to said socket.

You are right, I didn't know that. I guess then we'll need to add PUID and PGID
Reference for me: https://docs.linuxserver.io/general/understanding-puid-and-pgid/#why-use-these

@Meierschlumpf
Copy link
Member Author

What do you think of that solution? We allow the user to change PUID and PGID to whatever they need and chown the volumes the same way and internally still use the internal_user to run the processes as we don't want to change the owner of all files in the /app directory when starting up the container. But that user is member of the homarr group which is owner of the volumes

@Meierschlumpf Meierschlumpf changed the title fix(docker): issue with docker socket permissions feat(docker): add support for pgid and puid env variables Dec 30, 2024
@Meierschlumpf Meierschlumpf added the enhancement New feature or request label Dec 30, 2024
manuel-rw
manuel-rw previously approved these changes Dec 30, 2024
@Meierschlumpf Meierschlumpf added this to the 1.0.0 milestone Dec 31, 2024
@Meierschlumpf Meierschlumpf merged commit aeb681a into dev Dec 31, 2024
13 checks passed
@Meierschlumpf Meierschlumpf deleted the docker-socket-permissions branch December 31, 2024 10:36
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants