Re: [PR REVIEW] New package: nagios-4.4.6

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 154 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452324119

Comment:
not changed

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r453250236

Comment:
I looked through the configuration parameters and the CLI args to `nagios` and cannot see any way for it to create the directories it needs.

It would be possible to move this directory creation logic into the runit `run` script.  It would be a couple of `mkdir` commands followed by `chown` commands.  Would this be preferred? I'm personally ok with any methodology for creating these directories.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r453250094

Comment:
I did some testing into this, and `chpst -u _nagios ...` is possible with modifications to change 2 things:.

1. files in `/etc/nagios` are `660` owned by `root:root`.
2. `nagios` executable is `770` owned by `root:root`.

Both of these things make it so running `chpst -u _nagios nagios ...` fails because 1. it cannot execute the nagios executable.  If i manually `chmod 755` the executable, then it fails again because it cannot read the configuration files.  If I run a find command to change all of the config files to have `644` permissions *then* nagios is able to run under `chpst -u _nagios`.

`nagios` the core executable does not bind to any network port - it simply ships the CGI and HTML files and requires an external web server to host them.  `nagios` instead creates a named pipe at `/var/nagios/rw/nagios.cmd` for communication with the daemon.

I like the idea of running with `chpst` but that'll require the permissions of the executables and the configuration files be opened up to be readable by all, or to modify them to be in the `_nagios` group.  I personally like the idea of the configs and executables being *readable* (not writable) by all, but I understand that that goes against the spirit of how nagios is compiled.  Having it start as root is probably *ok* because it does drop privileges after it starts up.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r453249626

Comment:
These files are owned by `root:root` by default, so only the root user or anyone in the root group will be able to execute `nagios` or `nagiostats`... this has issues using `chpst` to run the `nagios` executable (i'll detail more in the `chpst` comment).

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r453117176

Comment:
I meant can nagios create those directories upon startup.
The current `make_dir` trigger doesn't delete the directories (upon removal) in reverse orders so ...

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r453116242

Comment:
I think a bit more, and think it requires root priviledges for network binding.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452965796

Comment:
I will update the permissions.

Nagios creates some of these directories during the installation process (though not all of them), however the void package hooks delete the directories since they are empty.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452965280

Comment:
As of right now, the config that gets shipped after compiling will have the `nagios_user` and `nagios_parameter` set to `_nagios` and will `setgid`/`setuid` after starting to drop its privileges. 

However, I don't know if there is anything that requires root in the startup process - i'll test it using `chpst` and report back.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 155 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452963951

Comment:
gotcha

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 198 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452963819

Comment:
Understood. will update to remove these commands.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452558684

Comment:
Can `nagios` `mkdir` its subdir itself?
I think the permission should be 770 as the same with nagios default.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452556923

Comment:
Does it need to start as root?
If yes, does it drop permission after starting?
If no, `chpst` to approriate user.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 190 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452556712

Comment:
`/etc/nagios/...` is shipped, don't `mkdir` it.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452556262

Comment:
If they set it to `770`, they should have a reason. Allow your user by adding to `_nagios` group is better.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452554495

Comment:
Those flags are set by `gnu-configure`:

https://github.com/void-linux/void-packages/blob/ab8ff2e758d8d8d9314c296596ad86dca4e417a7/common/environment/configure/gnu-configure-args.sh#L7-L17
https://github.com/void-linux/void-packages/blob/ab8ff2e758d8d8d9314c296596ad86dca4e417a7/common/environment/configure/gnu-configure-args.sh#L37

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452506091

Comment:
I tested with `--prefix` removed from `configure_args` and it appears to work as expected!

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452500156

Comment:
Tested - the installation portion of the install fails.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 223 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452499979

Comment:
This is outdated and has been removed in my latest commit from 10 days ago

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452404106

Comment:
I don't know.  I can test it and report back.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452403959

Comment:
This is an outdated commit - it has been replaced with `make_dirs` to specifically affect certain directories.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452403745

Comment:
Which flags specifically? I can test omitting `--prefix` and seeing what happens.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452403464

Comment:
I based this on the Arch Linux AUR (https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=nagios)

I'll look into this - now that you've pointed it out it doesn't look proper.

Re: [PR REVIEW] New package: nagios-4.4.6

[bahamas10]

[-- Attachment #1: Type: text/plain, Size: 232 bytes --]

New review comment by bahamas10 on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452401512

Comment:
It is changed.

It's in a separate commit pushed 10 days ago to this same branch.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 177 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452327052

Comment:
What happens if we don't set this?

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452326757

Comment:
I don't like `chown -R` in run file.
On a big directory and slow filesystem, it will take a lot of time.

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452326189

Comment:
Doesn't those flags set by `gnu-configure`?

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 176 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452325271

Comment:
Why `prefix` to `/usr/share/....`

Re: [PR REVIEW] New package: nagios-4.4.6

[sgn]

[-- Attachment #1: Type: text/plain, Size: 223 bytes --]

New review comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/23249#discussion_r452324119

Comment:
not changed

---
I meant please not press resolve until you changed the line.