-
Notifications
You must be signed in to change notification settings - Fork 27
Create logfile at startup #310
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
base: stable
Are you sure you want to change the base?
Conversation
| FRIENDICA_LOGFILE=${FRIENDICA_LOGFILE:-/var/www/html/friendica.log} | ||
| if sudo -u www-data test -w "$(dirname "$FRIENDICA_LOGFILE")"; then | ||
| touch "$FRIENDICA_LOGFILE" | ||
| chown www-data:www-data "$FRIENDICA_LOGFILE" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two suggestions/options:
If it's just supposed to try to create it and not do anything except continue if it fails, one could skip the conditional and needing to install sudo and just try to create it:
touch "$FRIENDICA_LOGFILE" || true
chown --silent www-data:www-data "$FRIENDICA_LOGFILE" || true
(--silent should only be there if we want to suppress error messages)
Alternatively, instead of using sudo I'm guessing su or runuser are available out of the box. I'd suggest the latter if possible since it's deescalating privileges.
Edit: I guess the first approach doesn't really determine if the file is writable by www-data, so I guess the runuser / su approach is the better one of the two.
It would be something like this:
runuser --login www-data --command "test -w \"$(dirname \"$FRIENDICA_LOGFILE\")"; then...or
su --login www-data --command "test -w \"$(dirname \"$FRIENDICA_LOGFILE\")"; thenThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfxa, thank you for the comment.
I will try with the runuser. Not sure if su or runuser are available in the image. But I'll test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfxa I tried. But none of the test worked correctly except for the solution with sudo.
The error in both other tests is: This account is currently not available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Strange that the account is "currently not available".
Maybe runuser will work with --user www-data instead of --login www-data?
From the manpage:
--login:
Start the shell as a login shell with an environment similar to a real login:
--user:
Run command with the effective user ID and group ID of the user name user.
Edit: But I think your Dockerfile idea is better.
|
I believe there is no elegant way to create the logfile with the docker image.
For using Docker usually users would mount the log file and if so, the logfile needs to exist anyway. @mfxa, @nupplaphil: do you agree? |
|
For some reason i can't make a direct comment on your new "comment", so responding here instead:
I think it sounds like a better idea to create it in the Dockerfile, yes. Good idea. |
This PR is opened as draft pull request as it may need some discussion and a bit more testing.
As described in #friendica/friendica#15001 the log file is not creatd by friendica when logging is enabled. This PR is an attempt to fix this.
I only had time to test this with the fpm image (which is debian based).
My concerns are: