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

Pre-create badger directory in Docker image #4910

Closed
wants to merge 1 commit into from

Conversation

freef4ll
Copy link

Which problem is this PR solving?

#4906

Description of the changes

  • Create directory for badger in a non-root environment so that existing setups do not break.

How was this change tested?

  • Tested permissions in a minimal container.

Checklist

@freef4ll freef4ll requested a review from a team as a code owner October 31, 2023 09:15
@freef4ll freef4ll requested a review from joe-elliott October 31, 2023 09:15
@freef4ll freef4ll changed the title pre-create badger directory in Docker image Pre-create badger directory in Docker image Oct 31, 2023
Signed-off-by: freefr4ll <103502659+freef4ll@users.noreply.github.com>
@yurishkuro
Copy link
Member

I don't think this change is going to help. First, there is no requirement that the actual directory is /badger, it's up to user to specify what it is. Second, all this change is doing is creating a dir inside the container file system during image creation, whereas in real deployment such directory would need to be mapped to host file system.

@freef4ll
Copy link
Author

freef4ll commented Nov 1, 2023

Correct, but if it is mapped by a named volume then it works for the default configuration.

@yurishkuro
Copy link
Member

Correct, but if it is mapped by a named volume then it works for the default configuration.

why would it work? if you have files on host file system owned by a different user-id, then the owner of in-container directory still not going to match the permissions on those files. Can you show commands that demonstrate a successful test?

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

Successfully merging this pull request may close these issues.

2 participants