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

Update Dockerfile #402

Closed
wants to merge 1 commit into from

Conversation

UnaffiliatedCode
Copy link

Added explicit permissions to the tests sub-folder since the folder hasn't been created when the image has been initialized. This can cause permission issues.

Added explicit permissions to the tests sub-folder since the folder hasn't been created when the image has been initialized. This can cause permission issues.
@ppodgorsek
Copy link
Owner

As mentioned in issue #381, the test folder is mounted when running the image. It is the responsibility of the process/thread starting the container to ensure the permissions are correct.

It is a bad practice to run containers as root, and this "fix" cannot work for non-root users.

Closing it as invalid.

@ppodgorsek ppodgorsek closed this Apr 18, 2022
@UnaffiliatedCode
Copy link
Author

This proposal assigns permissions based on the configured environment variable as done with the 1000:1000 user, but also assigns (and creates the folder) for tests.

This has nothing to do with the Root user. This just explicitly extends the permissions to the configured tests sub-folder.

@ppodgorsek
Copy link
Owner

If the test folder belongs to a different user, this will fail, hence why only the root user could execute it in most situations.
It actually highlights a potential bug for the reports folder, where this is done too - this is a mistake. (new issue created to track that defect #403)

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

Successfully merging this pull request may close these issues.

None yet

2 participants