Merge ~guoqiao/charm-nrpe:LP1906991-chown-nagios-dirs into charm-nrpe:master

[Joe Guo]

nrpe_utils.py: ensure permission for /var/lib/nagios

This nrpe charm will install nagios-nrpe-server deb package.
In its preinst script[0], it will add nagios user and create `/var/lib/nagios` as home dir.

When other charm like hw-health relates to this charm, they will:

1) setup cronjob to run script
2) generate output file into /var/lib/nagios
3) read output file from /var/lib/nagios

Before, these are all done via root user, so no permission issue.
But recently, the cronjob user is switched from root to nagios, which caused following issues:

1) original output file was created by root, cronjob script by nagios user can not write to it.
2) In some situation, owner of `/var/lib/nagios/` is changed to root, cronjob script can not write file into this dir.

related bugs:

LP: #1906991
LP: #1904045
LP: #1866382

In this patch, we:

1) ensure `/var/lib/nagios` is owned by nagios user
2) setgid on dir group, to ensure any new created file in `/var/lib/nagios` has group `nagios`.

NOTE: this patch avoids to chown recursively, which implies, if there is a legacy output file
owned by root, we have to fix it manually, or fix it from the related charm side.

[0]: https://git.launchpad.net/ubuntu/+source/nagios-nrpe/tree/debian/nagios-nrpe-server.preinst#n28

[David O Neill (dmzoneill)]

Looks fine, added the pythonic alternative. Not sure of our preference to use sub process.

get the additonal +1

[Joe Guo (guoqiao)]

Hi David, see my inline reply.

[Andrea Ieri (aieri)]

+1 to use the more pythonic os.chown
-1 to chown recursively: if permissions on files written by other charms are wrong, it's up to them to fix them

Also note Xav's comment[0]. I think using setgid is a good idea.

[0] https://bugs.launchpad.net/charm-hw-health/+bug/1906991/comments/2

review: Needs Fixing

5db37c5... by Joe Guo on 2021-02-05

nrpe_utils.py: ensure permission for /var/lib/nagios

This nrpe charm will install nagios-nrpe-server deb package.
In its preinst script[0], it will add nagios user and create `/var/lib/nagios` as home dir.

When other charm like hw-health relates to this charm, they will:

1) setup cronjob to run script
2) generate output file into /var/lib/nagios
3) read output file from /var/lib/nagios

Before, these are all done via root user, so no permission issue.
But recently, the cronjob user is switched from root to nagios, which caused following issues:

1) original output file was created by root, cronjob script by nagios user can not write to it.
2) In some situation, owner of `/var/lib/nagios/` is changed to root, cronjob script can not write file into this dir.

related bugs:

LP: #1906991
LP: #1904045
LP: #1866382

In this patch, we:

1) ensure `/var/lib/nagios` is owned by nagios user
2) setgid on dir group, to ensure any new created file in `/var/lib/nagios` has group `nagios`.

NOTE: this patch avoids to chown recursively, which implies, if there is a legacy output file
owned by root, we have to fix it manually, or fix it from the related charm side.

[0]: https://git.launchpad.net/ubuntu/+source/nagios-nrpe/tree/debian/nagios-nrpe-server.preinst#n28

Signed-off-by: Joe Guo <email address hidden>

[Andrea Ieri (aieri)]

lgtm

review: Approve

[Xav Paice (xavpaice)]

LGTM

review: Approve

[Joe Guo (guoqiao)]

Merged into master as 4379294097433aef3a190b7a15376ba7e04ce60b.
Thanks!

diff --git a/hooks/nrpe_utils.py b/hooks/nrpe_utils.py
index d59276e..d6b46eb 100644
--- a/hooks/nrpe_utils.py
+++ b/hooks/nrpe_utils.py
@@ -64,6 +64,20 @@ def remove_host_export_fragments(service_name):
 
 def install_charm_files(service_name):
     """Install files shipped with charm."""
+    # The preinst script of nagios-nrpe-server deb package will add nagios user
+    # and create this dir as home
+    # ref: https://git.launchpad.net/ubuntu/+source/nagios-nrpe/tree/debian/nagios-nrpe-server.preinst#n28  # NOQA: E501
+    nagios_home = "/var/lib/nagios"
+
+    # it's possible dir owner be changed to root by other process, e.g.: LP1866382
+    # here we ensure owner is nagios, but didn't apply it resursively intentionally.
+    shutil.chown(nagios_home, user="nagios", group="nagios")
+
+    # the `2` in mode will setgid for group, set dir permission to `drwxr-sr-x`.
+    # the `s` (setgid) will ensure any file created in this dir inherits parent dir
+    # group `nagios`, regardless of the effective user, such as root.
+    os.chmod(nagios_home, 0o2755)  # 2 will set the s flag for group
+
     nag_dirs = [
         "/etc/nagios/nrpe.d/",
         "/usr/local/lib/nagios/plugins",