-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: run container as dfly
user
#1775
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romange
previously approved these changes
Aug 31, 2023
Thanks for solving this issue!
Can you please update all other Dockerfiles in the folder with this change?
…On Thu, Aug 31, 2023 at 12:09 AM Søren Hansen ***@***.***> wrote:
- Beautified the Dockerfile a little bit
- Running the container as the dfly user resolves write permission
issues on volumes provisioned on the Container-as-a-service provider,
fly.io. If the container runs as root (which is the default if no user
is specified), I'm unable to write files to an external /data volume.
Additionally - it's also considered as best practice to execute a container
with a user account that has lower privileges than root
------------------------------
You can view, comment on, or merge this pull request online at:
#1775
Commit Summary
- c3ee115
<c3ee115>
fix: run container as `dfly` user
File Changes
(1 file <https://github.com/dragonflydb/dragonfly/pull/1775/files>)
- *M* tools/docker/Dockerfile.ubuntu-prod
<https://github.com/dragonflydb/dragonfly/pull/1775/files#diff-2a449c3fff2749483b17b95e2b9ac00dd1aa9d2d0495fa75c256667b70501e8d>
(8)
Patch Links:
- https://github.com/dragonflydb/dragonfly/pull/1775.patch
- https://github.com/dragonflydb/dragonfly/pull/1775.diff
—
Reply to this email directly, view it on GitHub
<#1775>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4BFCGS4LTWBHVL4275UA3XX6TZ3ANCNFSM6AAAAAA4FDHU5E>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
--
Roman Gershman
CTO
---
*www.dragonflydb.io <http://www.dragonflydb.io>*
|
Sure! I have force-pushed a fix to all 4 files 👍 |
romange
approved these changes
Aug 31, 2023
Gonna be released in 1.9.0 . Thanks again for catching this! |
romange
added a commit
that referenced
this pull request
Aug 15, 2024
Fixes #2917 The problem is described in this "working as intended" issue moby/moby#3124 So the advised approach of using "USER dfly" directive does not really work because it requires that the host will also define 'dfly' user with the same id. It's unrealistic expectation. Therefore, we revert the fix done in #1775 and follow valkey approach: https://github.com/valkey-io/valkey-container/blob/mainline/docker-entrypoint.sh#L12 1. we run the entrypoint in the container as root which later spawns the dragonfly process 2. if we run as root: a. we chmod files under /data to dfly. b. use su-exec to run exec ourselves as dfly. 3. if we do not run as root we execute the docker command. So even though the process starts as root, the server runs as dfly and only the bootstrap part has elevated permissions is used to fix the volume access. While we are at it, we also switched to setpriv following the change of https://github.com/valkey-io/valkey-container/pull/24/files Signed-off-by: Roman Gershman <[email protected]>
romange
added a commit
that referenced
this pull request
Aug 15, 2024
Fixes #2917 The problem is described in this "working as intended" issue moby/moby#3124 So the advised approach of using "USER dfly" directive does not really work because it requires that the host will also define 'dfly' user with the same id. It's unrealistic expectation. Therefore, we revert the fix done in #1775 and follow valkey approach: https://github.com/valkey-io/valkey-container/blob/mainline/docker-entrypoint.sh#L12 1. we run the entrypoint in the container as root which later spawns the dragonfly process 2. if we run as root: a. we chmod files under /data to dfly. b. use su-exec to run exec ourselves as dfly. 3. if we do not run as root we execute the docker command. So even though the process starts as root, the server runs as dfly and only the bootstrap part has elevated permissions is used to fix the volume access. While we are at it, we also switched to setpriv following the change of https://github.com/valkey-io/valkey-container/pull/24/files Signed-off-by: Roman Gershman <[email protected]>
romange
added a commit
that referenced
this pull request
Aug 15, 2024
Fixes #2917 The problem is described in this "working as intended" issue moby/moby#3124 So the advised approach of using "USER dfly" directive does not really work because it requires that the host will also define 'dfly' user with the same id. It's unrealistic expectation. Therefore, we revert the fix done in #1775 and follow valkey approach: https://github.com/valkey-io/valkey-container/blob/mainline/docker-entrypoint.sh#L12 1. we run the entrypoint in the container as root which later spawns the dragonfly process 2. if we run as root: a. we chmod files under /data to dfly. b. use setpriv to exec ourselves as dfly. 3. if we do not run as root we execute the docker command. So even though the process starts as root, the server runs as dfly and only the bootstrap part has elevated permissions is used to fix the volume access. While we are at it, we also switched to setpriv following the change of https://github.com/valkey-io/valkey-container/pull/24/files Signed-off-by: Roman Gershman <[email protected]>
romange
added a commit
that referenced
this pull request
Aug 22, 2024
Fixes #2917 The problem is described in this "working as intended" issue moby/moby#3124 So the advised approach of using "USER dfly" directive does not really work because it requires that the host will also define 'dfly' user with the same id. It's unrealistic expectation. Therefore, we revert the fix done in #1775 and follow valkey approach: https://github.com/valkey-io/valkey-container/blob/mainline/docker-entrypoint.sh#L12 1. we run the entrypoint in the container as root which later spawns the dragonfly process 2. if we run as root: a. we chmod files under /data to dfly. b. use setpriv to exec ourselves as dfly. 3. if we do not run as root we execute the docker command. So even though the process starts as root, the server runs as dfly and only the bootstrap part has elevated permissions is used to fix the volume access. While we are at it, we also switched to setpriv following the change of https://github.com/valkey-io/valkey-container/pull/24/files Signed-off-by: Roman Gershman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dfly
user resolves write permission issues on volumes provisioned on the Container-as-a-service provider, fly.io. If the container runs as root (which is the default if no user is specified), I'm unable to write files to an external/data
volume. Additionally - it's also considered as best practice to execute a container with a user account that has lower privileges than rootI think it can solve this discussion: #1527