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

Add su directive as option for logroate #511

Merged
merged 4 commits into from Sep 5, 2018

Conversation

Josue-T
Copy link
Contributor

@Josue-T Josue-T commented Jul 31, 2018

The problem

If the owner of a log file is not root we could receive this message from logrotate :

/etc/cron.daily/logrotate:
error: skipping "/var/www/horde/horde/log.log" because parent directory has insecure permissions (It's world writable or writable by group which is not "root") Set "su" directive in config file to tell logrotate which user/group should be used for rotation.
error: skipping "/var/www/horde/horde/services/log.log" because parent directory has insecure permissions (It's world writable or writable by group which is not "root") Set "su" directive in config file to tell logrotate which user/group should be used for rotation.
error: skipping "/var/www/horde/horde/services/portal/log.log" because parent directory has insecure permissions (It's world writable or writable by group which is not "root") Set "su" directive in config file to tell logrotate which user/group should be used for rotation.
run-parts: /etc/cron.daily/logrotate exited with return code 1

The helper ynh_use_logrotate don't support the su directive witch could be really usefull for some apps.

Solution

As the su directive optionally.

PR Status

Tested with horde with some different case of parameters.

In this case the management of the argument is not the best way. It will be better when we will implement ynh_handle_getopts_args in the core.

How to test

Test it on any apps.

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 : Maniack C
  • Deep review 0/1 : Maniack C

Copy link
Member

@alexAubin alexAubin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I really don't have any experience with this helper. Code-wise this looks small and legit so why not.

Anybody from @YunoHost/apps has some experience/opinion about this ?

Copy link
Member

@Psycojoker Psycojoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

I nearly never review helpers (well, still look at the diff) because that's way too much out of my field of usage and knowledge so I don't feel like I can have a good opinion on that.

It's part of the syntax
@Psycojoker Psycojoker merged commit c3d8da7 into YunoHost:stretch-unstable Sep 5, 2018
@alexAubin alexAubin added this to the 3.2.x milestone Oct 24, 2018
@Josue-T Josue-T deleted the patch-10 branch September 3, 2019 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants