From b31728e36194483174723968c3c16869727ff5bf Mon Sep 17 00:00:00 2001 From: Timon de Groot Date: Thu, 16 May 2024 16:45:35 +0200 Subject: [PATCH] reloader: Do not apply config if target dir is unmounted If the target dir is unmounted but is a very specific mount, the config should not be applied, because either the target directory contains incorrect files or it is empty. --- nginx_config_reloader/__init__.py | 7 ++ nginx_config_reloader/utils.py | 14 ++++ tests/test_directory_is_unmounted.py | 99 ++++++++++++++++++++++++++++ tests/test_nginx_config_reloader.py | 60 +++++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 nginx_config_reloader/utils.py create mode 100644 tests/test_directory_is_unmounted.py diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index 80498a6..fe93697 100644 --- a/nginx_config_reloader/__init__.py +++ b/nginx_config_reloader/__init__.py @@ -36,6 +36,7 @@ UNPRIVILEGED_UID, WATCH_IGNORE_FILES, ) +from nginx_config_reloader.utils import directory_is_unmounted logger = logging.getLogger(__name__) dbus_loop: Optional[EventLoop] = None @@ -298,6 +299,12 @@ def reloaded(self): return self._on_config_reload def reload(self, send_signal=True): + if directory_is_unmounted(self.dir_to_watch): + self.logger.warning( + f"Directory {self.dir_to_watch} is unmounted, not reloading!" + ) + return + self.apply_new_config() if send_signal: self._on_config_reload.emit() diff --git a/nginx_config_reloader/utils.py b/nginx_config_reloader/utils.py new file mode 100644 index 0000000..42dcbea --- /dev/null +++ b/nginx_config_reloader/utils.py @@ -0,0 +1,14 @@ +import json +import subprocess + + +def directory_is_unmounted(path): + output = subprocess.check_output( + ["systemctl", "list-units", "-t", "mount", "--all", "-o", "json"], + encoding="utf-8", + ) + units = json.loads(output) + for unit in units: + if unit["description"] == path: + return unit["active"] != "active" or unit["sub"] != "mounted" + return False diff --git a/tests/test_directory_is_unmounted.py b/tests/test_directory_is_unmounted.py new file mode 100644 index 0000000..ece1149 --- /dev/null +++ b/tests/test_directory_is_unmounted.py @@ -0,0 +1,99 @@ +import json + +from nginx_config_reloader import directory_is_unmounted +from tests.testcase import TestCase + + +class TestDirectoryIsUnmounted(TestCase): + def setUp(self): + self.check_output = self.set_up_patch( + "nginx_config_reloader.utils.subprocess.check_output", + return_value=json.dumps( + [ + { + "unit": "-.mount", + "load": "loaded", + "active": "active", + "sub": "mounted", + "description": "Root Mount", + }, + { + "unit": "data-web-nginx.mount", + "load": "loaded", + "active": "active", + "sub": "mounted", + "description": "/data/web/nginx", + }, + ] + ), + ) + + def test_it_calls_systemctl_list_units(self): + directory_is_unmounted("/data/web/nginx") + + self.check_output.assert_called_once_with( + ["systemctl", "list-units", "-t", "mount", "--all", "-o", "json"], + encoding="utf-8", + ) + + def test_it_returns_false_if_no_mount_found(self): + self.check_output.return_value = json.dumps( + [ + { + "unit": "-.mount", + "load": "loaded", + "active": "active", + "sub": "mounted", + "description": "Root Mount", + }, + ] + ) + + self.assertFalse(directory_is_unmounted("/data/web/nginx")) + + def test_it_returns_false_if_mount_exists_active_mounted(self): + self.assertFalse(directory_is_unmounted("/data/web/nginx")) + + def test_it_returns_true_if_mount_exists_not_active(self): + self.check_output.return_value = json.dumps( + [ + { + "unit": "-.mount", + "load": "loaded", + "active": "active", + "sub": "mounted", + "description": "Root Mount", + }, + { + "unit": "data-web-nginx.mount", + "load": "loaded", + "active": "inactive", + "sub": "dead", + "description": "/data/web/nginx", + }, + ] + ) + + self.assertTrue(directory_is_unmounted("/data/web/nginx")) + + def test_it_returns_true_if_mount_exists_active_not_mounted(self): + self.check_output.return_value = json.dumps( + [ + { + "unit": "-.mount", + "load": "loaded", + "active": "active", + "sub": "mounted", + "description": "Root Mount", + }, + { + "unit": "data-web-nginx.mount", + "load": "loaded", + "active": "active", + "sub": "dead", + "description": "/data/web/nginx", + }, + ] + ) + + self.assertTrue(directory_is_unmounted("/data/web/nginx")) diff --git a/tests/test_nginx_config_reloader.py b/tests/test_nginx_config_reloader.py index 653072d..2076f8b 100644 --- a/tests/test_nginx_config_reloader.py +++ b/tests/test_nginx_config_reloader.py @@ -350,6 +350,66 @@ def test_that_apply_new_config_does_not_fix_custom_config_dir_permissions_if_spe self.assertFalse(self.fix_custom_config_dir_permissions.called) + def test_that_reload_calls_apply_new_config(self): + directory_is_unmounted = self.set_up_patch( + "nginx_config_reloader.directory_is_unmounted", + return_value=False, + ) + apply_new_config = self.set_up_patch( + "nginx_config_reloader.NginxConfigReloader.apply_new_config" + ) + + tm = self._get_nginx_config_reloader_instance() + tm.reload() + + directory_is_unmounted.assert_called_once_with( + nginx_config_reloader.DIR_TO_WATCH + ) + apply_new_config.assert_called_once_with() + + def test_that_reload_does_not_apply_new_config_if_directory_is_unmounted(self): + directory_is_unmounted = self.set_up_patch( + "nginx_config_reloader.directory_is_unmounted", + return_value=True, + ) + signal = Mock() + self.set_up_patch( + "nginx_config_reloader.Signal", + return_value=signal, + ) + apply_new_config = self.set_up_patch( + "nginx_config_reloader.NginxConfigReloader.apply_new_config" + ) + + tm = self._get_nginx_config_reloader_instance() + tm.reload() + + directory_is_unmounted.assert_called_once_with( + nginx_config_reloader.DIR_TO_WATCH + ) + apply_new_config.assert_not_called() + signal.emit.assert_not_called() + + def test_that_reload_does_not_send_signal_if_specified(self): + self.set_up_patch( + "nginx_config_reloader.directory_is_unmounted", + return_value=False, + ) + signal = Mock() + self.set_up_patch( + "nginx_config_reloader.Signal", + return_value=signal, + ) + apply_new_config = self.set_up_patch( + "nginx_config_reloader.NginxConfigReloader.apply_new_config" + ) + + tm = self._get_nginx_config_reloader_instance() + tm.reload(send_signal=False) + + apply_new_config.assert_called_once_with() + signal.emit.assert_not_called() + def test_that_error_file_is_not_moved_to_dest_dir(self): self._write_file(self._source(nginx_config_reloader.ERROR_FILE), "some error")