Skip to content

Commit

Permalink
reloader: Do not apply config if target dir is unmounted
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tdgroot committed May 16, 2024
1 parent 065782a commit b31728e
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 0 deletions.
7 changes: 7 additions & 0 deletions nginx_config_reloader/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
14 changes: 14 additions & 0 deletions nginx_config_reloader/utils.py
Original file line number Diff line number Diff line change
@@ -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
99 changes: 99 additions & 0 deletions tests/test_directory_is_unmounted.py
Original file line number Diff line number Diff line change
@@ -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"))
60 changes: 60 additions & 0 deletions tests/test_nginx_config_reloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down

0 comments on commit b31728e

Please sign in to comment.