From 382f0c48553eb52a20b91e4d937942fb9881f1d5 Mon Sep 17 00:00:00 2001 From: Timon de Groot Date: Tue, 14 Apr 2026 15:56:26 +0200 Subject: [PATCH 1/3] Add and apply pyupgrade Set to python 3.11 + because we have to support Debian 12 --- .pre-commit-config.yaml | 43 ++++++++++++++++------------- nginx_config_reloader/__init__.py | 16 ++++------- nginx_config_reloader/copy_files.py | 2 +- pyproject.toml | 1 + uv.lock | 23 +++++++++++++++ 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 44157eb..13daf5c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,23 +1,28 @@ # See https://pre-commit.com for more information # See https://pre-commit.com/hooks.html for more hooks repos: -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: 'v6.0.0' + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: 'v6.0.0' + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - repo: https://github.com/psf/black + rev: '26.3.1' + hooks: + - id: black + - repo: https://github.com/myint/autoflake + rev: 'v2.3.3' + hooks: + - id: autoflake + args: ["--in-place", "--remove-unused-variables", "--remove-all-unused-imports"] + - repo: https://github.com/pycqa/isort + rev: '8.0.1' + hooks: + - id: isort + name: isort (python) + args: ["--profile", "black"] + - repo: https://github.com/asottile/pyupgrade + rev: v3.21.2 hooks: - - id: trailing-whitespace - - id: end-of-file-fixer -- repo: https://github.com/psf/black - rev: '26.3.1' - hooks: - - id: black -- repo: https://github.com/myint/autoflake - rev: 'v2.3.3' - hooks: - - id: autoflake - args: ["--in-place", "--remove-unused-variables", "--remove-all-unused-imports"] -- repo: https://github.com/pycqa/isort - rev: '8.0.1' - hooks: - - id: isort - name: isort (python) - args: ["--profile", "black"] + - id: pyupgrade + args: [--py311-plus] diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index cd58931..9f82f24 100644 --- a/nginx_config_reloader/__init__.py +++ b/nginx_config_reloader/__init__.py @@ -1,5 +1,4 @@ #!/usr/bin/env python -from __future__ import absolute_import import argparse import fnmatch @@ -12,7 +11,6 @@ import sys import threading import time -from typing import Optional from dasbus.loop import EventLoop from dasbus.signal import Signal @@ -41,7 +39,7 @@ from nginx_config_reloader.utils import directory_is_unmounted logger = logging.getLogger(__name__) -dbus_loop: Optional[EventLoop] = None +dbus_loop: EventLoop | None = None class NginxConfigReloader(FileSystemEventHandler): @@ -173,7 +171,7 @@ def check_no_forbidden_config_directives_are_present(self): ) subprocess.check_output(check_external_resources, shell=True) except subprocess.CalledProcessError: - error = "Unable to load config: {}".format(rules[1]) + error = f"Unable to load config: {rules[1]}" self.logger.error(error) self.write_error_file(error) return True @@ -234,7 +232,7 @@ def _apply(self): extra_output = e.output if isinstance(e.output, bytes): extra_output = extra_output.decode() - error_output += "\n\n{}".format(extra_output) + error_output += f"\n\n{extra_output}" self.logger.error("Installation of custom config failed") self.restore_old_custom_config_dir() self.write_error_file(error_output) @@ -304,9 +302,9 @@ def reload_nginx(self): def get_nginx_pid(self): try: - with open(NGINX_PID_FILE, "r") as f: + with open(NGINX_PID_FILE) as f: return int(f.read()) - except (IOError, ValueError): + except (OSError, ValueError): return None def write_error_file(self, error): @@ -406,9 +404,7 @@ def wait_loop( dbus_thread.start() while not os.path.exists(dir_to_watch): - logger.warning( - "Configuration dir {} not found, waiting...".format(dir_to_watch) - ) + logger.warning(f"Configuration dir {dir_to_watch} not found, waiting...") time.sleep(5) running = True diff --git a/nginx_config_reloader/copy_files.py b/nginx_config_reloader/copy_files.py index 50bb9cb..d7f7a96 100644 --- a/nginx_config_reloader/copy_files.py +++ b/nginx_config_reloader/copy_files.py @@ -22,7 +22,7 @@ def safe_copy_files(src, dest): # Dirs default to 0755 to read. Remove setuid bits. Remove executability for others '--chmod="D755,-s,Fo-wx"', ] - cmd.extend(['--exclude="{}"'.format(pattern) for pattern in SYNC_IGNORE_FILES]) + cmd.extend([f'--exclude="{pattern}"' for pattern in SYNC_IGNORE_FILES]) cmd = " ".join(cmd) # shell=True to ensure globs are not escaped check_output(cmd, shell=True, stderr=STDOUT) diff --git a/pyproject.toml b/pyproject.toml index bae5f51..e084c19 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,6 +27,7 @@ dev = [ "pygobject-stubs>=2.16.0", "pytest>=9.0.2", "pytest-xdist>=3.8.0", + "pyupgrade>=3.21.2", ] [tool.isort] diff --git a/uv.lock b/uv.lock index c0c3fc5..84ddf65 100644 --- a/uv.lock +++ b/uv.lock @@ -149,6 +149,7 @@ dev = [ { name = "pygobject-stubs" }, { name = "pytest" }, { name = "pytest-xdist" }, + { name = "pyupgrade" }, ] [package.metadata] @@ -165,6 +166,7 @@ dev = [ { name = "pygobject-stubs", specifier = ">=2.16.0" }, { name = "pytest", specifier = ">=9.0.2" }, { name = "pytest-xdist", specifier = ">=3.8.0" }, + { name = "pyupgrade", specifier = ">=3.21.2" }, ] [[package]] @@ -353,6 +355,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/c6/78/397db326746f0a342855b81216ae1f0a32965deccfd7c830a2dbc66d2483/pytokens-0.4.1-py3-none-any.whl", hash = "sha256:26cef14744a8385f35d0e095dc8b3a7583f6c953c2e3d269c7f82484bf5ad2de", size = 13729, upload-time = "2026-01-30T01:03:45.029Z" }, ] +[[package]] +name = "pyupgrade" +version = "3.21.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "tokenize-rt" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/7f/a1/dc63caaeed232b1c58eae1b7a75f262d64ab8435882f696ffa9b58c0c415/pyupgrade-3.21.2.tar.gz", hash = "sha256:1a361bea39deda78d1460f65d9dd548d3a36ff8171d2482298539b9dc11c9c06", size = 45455, upload-time = "2025-11-19T00:39:48.012Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/16/8c/433dac11910989a90c40b10149d07ef7224232236971a562d3976790ec53/pyupgrade-3.21.2-py2.py3-none-any.whl", hash = "sha256:2ac7b95cbd176475041e4dfe8ef81298bd4654a244f957167bd68af37d52be9f", size = 62814, upload-time = "2025-11-19T00:39:46.958Z" }, +] + [[package]] name = "pyyaml" version = "6.0.3" @@ -408,6 +422,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f1/12/de94a39c2ef588c7e6455cfbe7343d3b2dc9d6b6b2f40c4c6565744c873d/pyyaml-6.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:ebc55a14a21cb14062aa4162f906cd962b28e2e9ea38f9b4391244cd8de4ae0b", size = 149341, upload-time = "2025-09-25T21:32:56.828Z" }, ] +[[package]] +name = "tokenize-rt" +version = "6.2.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/69/ed/8f07e893132d5051d86a553e749d5c89b2a4776eb3a579b72ed61f8559ca/tokenize_rt-6.2.0.tar.gz", hash = "sha256:8439c042b330c553fdbe1758e4a05c0ed460dbbbb24a606f11f0dee75da4cad6", size = 5476, upload-time = "2025-05-23T23:48:00.035Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/33/f0/3fe8c6e69135a845f4106f2ff8b6805638d4e85c264e70114e8126689587/tokenize_rt-6.2.0-py2.py3-none-any.whl", hash = "sha256:a152bf4f249c847a66497a4a95f63376ed68ac6abf092a2f7cfb29d044ecff44", size = 6004, upload-time = "2025-05-23T23:47:58.812Z" }, +] + [[package]] name = "typing-extensions" version = "4.15.0" From 60486ef1da7f918bd7ec4f4cf2f8ef44c3fceef7 Mon Sep 17 00:00:00 2001 From: Timon de Groot Date: Fri, 17 Apr 2026 10:32:08 +0200 Subject: [PATCH 2/3] pyproject: Set pyright to basic mode --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index e084c19..fab2869 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,5 +30,8 @@ dev = [ "pyupgrade>=3.21.2", ] +[tool.pyright] +typeCheckingMode = "basic" + [tool.isort] profile = "black" # Make isort compatible with black From f94dc9673e4541eb740935f8bf56aabbce0803e1 Mon Sep 17 00:00:00 2001 From: Timon de Groot Date: Fri, 17 Apr 2026 10:32:32 +0200 Subject: [PATCH 3/3] Add support for custom nginx error filename This allows for custom error output filenames to be configured, which is useful when using a shared nginx directory across servers. --- nginx_config_reloader/__init__.py | 86 ++++++++++++------- nginx_config_reloader/copy_files.py | 7 +- ...t_assert_forbidden_statements_in_config.py | 23 ++++- tests/test_main.py | 45 ++++++++++ tests/test_nginx_config_reloader.py | 51 +++++++++++ ...t_parse_nginx_config_reloader_arguments.py | 5 ++ tests/test_wait_loop.py | 4 + 7 files changed, 186 insertions(+), 35 deletions(-) diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index 9f82f24..ee5c1e0 100644 --- a/nginx_config_reloader/__init__.py +++ b/nginx_config_reloader/__init__.py @@ -3,8 +3,8 @@ import argparse import fnmatch import logging -import logging.handlers import os +import re import shutil import signal import subprocess @@ -32,6 +32,7 @@ MAIN_CONFIG_DIR, NGINX, NGINX_PID_FILE, + SYNC_IGNORE_FILES, UNPRIVILEGED_GID, UNPRIVILEGED_UID, WATCH_IGNORE_FILES, @@ -46,11 +47,12 @@ class NginxConfigReloader(FileSystemEventHandler): def __init__( self, logger=None, - no_magento_config=False, - no_custom_config=False, - dir_to_watch=DIR_TO_WATCH, - magento2_flag=None, - use_systemd=False, + no_magento_config: bool = False, + no_custom_config: bool = False, + dir_to_watch: str = DIR_TO_WATCH, + magento2_flag: str | None = None, + use_systemd: bool = False, + error_file: str = ERROR_FILE, ): """Constructor called by ProcessEvent @@ -59,6 +61,7 @@ def __init__( :param bool no_custom_config: True if we should not copy custom configuration :param str dir_to_watch: The directory to watch :param str magento2_flag: Magento 2 flag location + :param str error_file: File name for error output file """ if not logger: self.logger = logging @@ -76,6 +79,7 @@ def __init__( self.dirty = False self.applying = False self._on_config_reload = Signal() + self.error_file = error_file def on_deleted(self, event): """Triggered by inotify on removal of file or removal of dir @@ -113,7 +117,8 @@ def handle_event(self, event): return basename = os.path.basename(event.src_path) - if not any(fnmatch.fnmatch(basename, pat) for pat in WATCH_IGNORE_FILES): + ignore_files = list(WATCH_IGNORE_FILES) + [self.error_file] + if not any(fnmatch.fnmatch(basename, pat) for pat in ignore_files): self.logger.debug( f"{event.event_type.upper()} detected on {event.src_path}" ) @@ -153,37 +158,38 @@ def check_no_forbidden_config_directives_are_present(self): True if forbidden config directives are present False if check couldn't find any forbidden config flags """ - if os.path.isdir(self.dir_to_watch): - for rules in FORBIDDEN_CONFIG_REGEX: - try: - # error file may contain messages that match a forbidden config pattern - # then validation could fail while the actual config is correct. - # we'll exclude the error file from searching for patterns, - # NOTE: exclusion of error_file requires to ensure the - # file is removed before moving it to nginx conf dir - # @TODO: use Python to search for forbidden configs instead - # of spawning external procs. Will have better testing - # and even may consume less system resources - check_external_resources = ( - "[ $(grep -r --exclude={} -P '{}' '{}' | wc -l) -lt 1 ]".format( - ERROR_FILE, rules[0], self.dir_to_watch - ) - ) - subprocess.check_output(check_external_resources, shell=True) - except subprocess.CalledProcessError: - error = f"Unable to load config: {rules[1]}" - self.logger.error(error) - self.write_error_file(error) - return True + if not os.path.isdir(self.dir_to_watch): return False + for pattern, message in FORBIDDEN_CONFIG_REGEX: + try: + # error file may contain messages that match a forbidden config pattern + # then validation could fail while the actual config is correct. + # we'll exclude the error file from searching for patterns, + # NOTE: exclusion of error_file requires to ensure the + # file is removed before moving it to nginx conf dir + # @TODO: use Python to search for forbidden configs instead + # of spawning external procs. Will have better testing + # and even may consume less system resources + check_external_resources = "[ $(grep -r --exclude={} --exclude={} -P '{}' '{}' | wc -l) -lt 1 ]".format( + ERROR_FILE, self.error_file, pattern, self.dir_to_watch + ) + subprocess.check_output(check_external_resources, shell=True) + except subprocess.CalledProcessError: + error = f"Unable to load config: {message}" + self.logger.error(error) + self.write_error_file(error) + return True + + return False + def remove_error_file(self): """Try removing the error file. Return True on success or False on errors :rtype: bool """ removed = False try: - os.unlink(os.path.join(self.dir_to_watch, ERROR_FILE)) + os.unlink(os.path.join(self.dir_to_watch, self.error_file)) removed = True except OSError: pass @@ -282,7 +288,8 @@ def install_new_custom_config_dir(self): if os.path.exists(CUSTOM_CONFIG_DIR): shutil.move(CUSTOM_CONFIG_DIR, BACKUP_CONFIG_DIR) os.mkdir(CUSTOM_CONFIG_DIR) - safe_copy_files(self.dir_to_watch, CUSTOM_CONFIG_DIR) + ignore_files = list(SYNC_IGNORE_FILES) + [self.error_file] + safe_copy_files(self.dir_to_watch, CUSTOM_CONFIG_DIR, ignore_files) def restore_old_custom_config_dir(self): shutil.rmtree(CUSTOM_CONFIG_DIR) @@ -308,7 +315,7 @@ def get_nginx_pid(self): return None def write_error_file(self, error): - with open(os.path.join(self.dir_to_watch, ERROR_FILE), "w") as f: + with open(os.path.join(self.dir_to_watch, self.error_file), "w") as f: f.write(error) @property @@ -366,6 +373,7 @@ def wait_loop( recursive_watch=False, use_systemd=False, no_dbus=False, + error_file: str = ERROR_FILE, ): """Main event loop @@ -382,6 +390,7 @@ def wait_loop( :param bool recursive_watch: True if we should watch the dir recursively :param use_systemd: True if we should reload nginx using systemd instead of process signal :param bool no_dbus: True if we should not use DBus + :param str error_file: Error file to write error output to :return None: """ dir_to_watch = os.path.abspath(dir_to_watch) @@ -392,6 +401,7 @@ def wait_loop( no_custom_config=no_custom_config, dir_to_watch=dir_to_watch, use_systemd=use_systemd, + error_file=error_file, ) if not no_dbus: @@ -474,6 +484,11 @@ def parse_nginx_config_reloader_arguments(): help="Disable DBus interface", default=False, ) + parser.add_argument( + "--error-file", + help="File name for error output", + default=ERROR_FILE, + ) return parser.parse_args() @@ -491,6 +506,11 @@ def main(): args = parse_nginx_config_reloader_arguments() log = get_logger() + error_file_pattern = re.compile(r"[a-zA-Z0-9_\.]*[a-zA-Z0-9_]+") + if not error_file_pattern.fullmatch(args.error_file): + log.error(f"Invalid error file name provided: {args.error_file}") + return 1 + if args.monitor: # Track changed files in the nginx config dir and reload on change wait_loop( @@ -501,6 +521,7 @@ def main(): recursive_watch=args.recursivewatch, use_systemd=args.use_systemd, no_dbus=args.no_dbus, + error_file=args.error_file, ) # should never return return 1 @@ -512,6 +533,7 @@ def main(): no_custom_config=args.nocustomconfig, dir_to_watch=args.watchdir, use_systemd=args.use_systemd, + error_file=args.error_file, ).apply_new_config() return 0 diff --git a/nginx_config_reloader/copy_files.py b/nginx_config_reloader/copy_files.py index d7f7a96..5360e75 100644 --- a/nginx_config_reloader/copy_files.py +++ b/nginx_config_reloader/copy_files.py @@ -7,7 +7,10 @@ logger = logging.getLogger(__name__) -def safe_copy_files(src, dest): +def safe_copy_files(src, dest, ignore_files: list[str] | None = None): + if not ignore_files: + ignore_files = list(SYNC_IGNORE_FILES) + cmd = [ # Adding a / at the end copies contents of the dir and not the dir itself # This is achieved with `os.path.join(x, '')`, which ensures a trailing slash @@ -22,7 +25,7 @@ def safe_copy_files(src, dest): # Dirs default to 0755 to read. Remove setuid bits. Remove executability for others '--chmod="D755,-s,Fo-wx"', ] - cmd.extend([f'--exclude="{pattern}"' for pattern in SYNC_IGNORE_FILES]) + cmd.extend([f'--exclude="{pattern}"' for pattern in ignore_files]) cmd = " ".join(cmd) # shell=True to ensure globs are not escaped check_output(cmd, shell=True, stderr=STDOUT) diff --git a/tests/test_assert_forbidden_statements_in_config.py b/tests/test_assert_forbidden_statements_in_config.py index 50dea3f..23c919e 100644 --- a/tests/test_assert_forbidden_statements_in_config.py +++ b/tests/test_assert_forbidden_statements_in_config.py @@ -4,7 +4,11 @@ import pytest -from nginx_config_reloader import FORBIDDEN_CONFIG_REGEX, NginxConfigReloader +from nginx_config_reloader import ( + ERROR_FILE, + FORBIDDEN_CONFIG_REGEX, + NginxConfigReloader, +) from tests.testcase import TestCase # Skip marker for tests that require grep -P (PCRE support, not available on macOS) @@ -15,6 +19,7 @@ class TestAssertNoForbiddenStatementsInConfig(TestCase): def setUp(self): + self.custom_error_file = "nginx_error_output.hnclusterweb1" self.isdir = self.set_up_patch("nginx_config_reloader.os.path.isdir") self.isdir.return_value = True self.check_output = self.set_up_patch( @@ -32,6 +37,22 @@ def test_assert_no_includes_in_config_does_not_check_config_if_no_dir_to_watch( self.assertFalse(self.check_output.called) + def test_check_no_forbidden_config_excludes_default_and_custom_error_files(self): + reloader = NginxConfigReloader( + dir_to_watch="/tmp/nginx", error_file=self.custom_error_file + ) + + reloader.check_no_forbidden_config_directives_are_present() + + self.assertEqual( + len(self.check_output.call_args_list), len(FORBIDDEN_CONFIG_REGEX) + ) + for call_args in self.check_output.call_args_list: + command = call_args.args[0] + self.assertIn(f"--exclude={ERROR_FILE}", command) + self.assertIn(f"--exclude={self.custom_error_file}", command) + self.assertIn("'/tmp/nginx'", command) + @requires_pcre_grep def test_include_prevention_legal_includes(self): TEST_CASES = [ diff --git a/tests/test_main.py b/tests/test_main.py index 18ed317..5ddccfb 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -2,6 +2,7 @@ from tempfile import mkdtemp from unittest.mock import Mock +import nginx_config_reloader from nginx_config_reloader import main from tests.testcase import TestCase @@ -9,6 +10,7 @@ class TestMain(TestCase): def setUp(self): self.source = mkdtemp() + self.custom_error_file = "nginx_error_output.hnclusterweb1" self.parse_nginx_config_reloader_arguments = self.set_up_patch( "nginx_config_reloader.parse_nginx_config_reloader_arguments" ) @@ -21,6 +23,7 @@ def setUp(self): recursivewatch=False, use_systemd=False, no_dbus=False, + error_file=self.custom_error_file, ) self.get_logger = self.set_up_context_manager_patch( "nginx_config_reloader.get_logger" @@ -54,6 +57,7 @@ def test_main_reloads_config_once_if_monitor_mode_not_specified(self): no_custom_config=self.parse_nginx_config_reloader_arguments.return_value.nocustomconfig, dir_to_watch=self.parse_nginx_config_reloader_arguments.return_value.watchdir, use_systemd=self.parse_nginx_config_reloader_arguments.return_value.use_systemd, + error_file=self.parse_nginx_config_reloader_arguments.return_value.error_file, ) self.reloader.return_value.apply_new_config.assert_called_once_with() @@ -80,6 +84,7 @@ def test_main_watches_the_config_dir_if_monitor_specified(self): recursive_watch=self.parse_nginx_config_reloader_arguments.return_value.recursivewatch, use_systemd=self.parse_nginx_config_reloader_arguments.return_value.use_systemd, no_dbus=self.parse_nginx_config_reloader_arguments.return_value.no_dbus, + error_file=self.parse_nginx_config_reloader_arguments.return_value.error_file, ) def test_main_watches_the_config_dir_if_monitor_mode_is_specified_and_includes_allowed( @@ -98,6 +103,7 @@ def test_main_watches_the_config_dir_if_monitor_mode_is_specified_and_includes_a recursive_watch=self.parse_nginx_config_reloader_arguments.return_value.recursivewatch, use_systemd=self.parse_nginx_config_reloader_arguments.return_value.use_systemd, no_dbus=self.parse_nginx_config_reloader_arguments.return_value.no_dbus, + error_file=self.parse_nginx_config_reloader_arguments.return_value.error_file, ) def test_main_does_not_reload_the_config_once_if_monitor_mode_is_specified(self): @@ -128,4 +134,43 @@ def test_main_passes_no_dbus_to_wait_loop(self): recursive_watch=self.parse_nginx_config_reloader_arguments.return_value.recursivewatch, use_systemd=self.parse_nginx_config_reloader_arguments.return_value.use_systemd, no_dbus=True, + error_file=self.parse_nginx_config_reloader_arguments.return_value.error_file, + ) + + def test_main_rejects_invalid_error_file_name(self): + self.parse_nginx_config_reloader_arguments.return_value.error_file = "bad/name" + + ret = main() + + self.assertEqual(1, ret) + self.get_logger.return_value.error.assert_called_once_with( + "Invalid error file name provided: bad/name" + ) + self.assertFalse(self.wait_loop.called) + self.assertFalse(self.reloader.called) + + def test_main_accepts_default_error_file_name(self): + self.parse_nginx_config_reloader_arguments.return_value.error_file = ( + nginx_config_reloader.ERROR_FILE + ) + + ret = main() + + self.assertEqual(0, ret) + + def test_main_accepts_custom_error_file_name_with_dots(self): + self.parse_nginx_config_reloader_arguments.return_value.error_file = ( + self.custom_error_file + ) + + ret = main() + + self.assertEqual(0, ret) + self.reloader.assert_called_once_with( + logger=self.get_logger.return_value, + no_magento_config=self.parse_nginx_config_reloader_arguments.return_value.nomagentoconfig, + no_custom_config=self.parse_nginx_config_reloader_arguments.return_value.nocustomconfig, + dir_to_watch=self.parse_nginx_config_reloader_arguments.return_value.watchdir, + use_systemd=self.parse_nginx_config_reloader_arguments.return_value.use_systemd, + error_file=self.custom_error_file, ) diff --git a/tests/test_nginx_config_reloader.py b/tests/test_nginx_config_reloader.py index 536916f..34e6bf4 100644 --- a/tests/test_nginx_config_reloader.py +++ b/tests/test_nginx_config_reloader.py @@ -51,6 +51,8 @@ def setUp(self): self.error_file = os.path.join( nginx_config_reloader.DIR_TO_WATCH, nginx_config_reloader.ERROR_FILE ) + self.custom_error_name = "nginx_error_output.hnclusterweb1" + self.custom_error_file = os.path.join(self.source, self.custom_error_name) def tearDown(self): shutil.rmtree(self.source, ignore_errors=True) @@ -471,6 +473,14 @@ def test_that_handle_event_does_not_need_reload_on_change_of_error_file(self): self.assertFalse(tm.dirty) + def test_that_handle_event_does_not_need_reload_on_change_of_custom_error_file( + self, + ): + tm = self._get_nginx_config_reloader_instance(error_file=self.custom_error_name) + tm.handle_event(Event(self.custom_error_name)) + + self.assertFalse(tm.dirty) + def test_that_handle_event_does_not_need_reload_on_change_of_invisible_file(self): tm = self._get_nginx_config_reloader_instance() tm.handle_event(Event(".config.swp")) @@ -497,6 +507,16 @@ def test_remove_error_file_returns_false_on_errors(self): tm = self._get_nginx_config_reloader_instance() self.assertFalse(tm.remove_error_file()) + def test_remove_error_file_unlinks_the_custom_error_file(self): + mock_os = self.set_up_patch("nginx_config_reloader.os") + mock_os.path.join.return_value = self.custom_error_file + + tm = self._get_nginx_config_reloader_instance(error_file=self.custom_error_name) + + self.assertTrue(tm.remove_error_file()) + mock_os.path.join.assert_called_once_with(self.source, self.custom_error_name) + mock_os.unlink.assert_called_once_with(self.custom_error_file) + def test_that_install_new_custom_config_dir_always_removes_the_error_file_before_copying_configs( self, ): @@ -515,6 +535,24 @@ def test_that_install_new_custom_config_dir_always_removes_the_error_file_before self.assertTrue(mock_remove_error_file.called) + def test_that_install_new_custom_config_dir_excludes_custom_error_file_from_sync( + self, + ): + self.set_up_patch("nginx_config_reloader.shutil.rmtree") + self.set_up_patch("nginx_config_reloader.shutil.move") + self.set_up_patch("nginx_config_reloader.os.path.exists", return_value=True) + self.set_up_patch("nginx_config_reloader.os.mkdir") + safe_copy_files = self.set_up_patch("nginx_config_reloader.safe_copy_files") + + tm = self._get_nginx_config_reloader_instance(error_file=self.custom_error_name) + tm.install_new_custom_config_dir() + + safe_copy_files.assert_called_once_with( + self.source, + self.dest, + list(nginx_config_reloader.SYNC_IGNORE_FILES) + [self.custom_error_name], + ) + def test_recursive_symlink_is_not_copied(self): os.mkdir(os.path.join(self.source, "new_dir")) os.symlink(self.source, os.path.join(self.source, "new_dir/recursive_symlink")) @@ -553,6 +591,17 @@ def test_rsync_error_is_placed_in_error_file(self): with open(self.error_file) as fp: self.assertIn("Rsync error", fp.read()) + def test_rsync_error_is_placed_in_custom_error_file(self): + safe_copy_files = self.set_up_patch("nginx_config_reloader.safe_copy_files") + safe_copy_files.side_effect = OSError("Rsync error") + + tm = self._get_nginx_config_reloader_instance(error_file=self.custom_error_name) + tm.apply_new_config() + + self.assertTrue(os.path.exists(self.custom_error_file)) + with open(self.custom_error_file) as fp: + self.assertIn("Rsync error", fp.read()) + def test_reloader_doesnt_crash_if_source_dir_is_empty(self): shutil.rmtree(self.source, ignore_errors=True) os.mkdir(self.source) @@ -692,12 +741,14 @@ def _get_nginx_config_reloader_instance( no_magento_config=False, no_custom_config=False, magento2_flag=None, + error_file=nginx_config_reloader.ERROR_FILE, ): return nginx_config_reloader.NginxConfigReloader( no_magento_config=no_magento_config, no_custom_config=no_custom_config, dir_to_watch=self.source, magento2_flag=magento2_flag, + error_file=error_file, ) def _write_file(self, name, contents): diff --git a/tests/test_parse_nginx_config_reloader_arguments.py b/tests/test_parse_nginx_config_reloader_arguments.py index 076ab9e..2ef7c02 100644 --- a/tests/test_parse_nginx_config_reloader_arguments.py +++ b/tests/test_parse_nginx_config_reloader_arguments.py @@ -60,6 +60,11 @@ def test_parse_nginx_config_reloader_arguments_adds_options(self): help="Disable DBus interface", default=False, ), + call( + "--error-file", + help="File name for error output", + default=nginx_config_reloader.ERROR_FILE, + ), ] self.assertEqual( self.parser.return_value.add_argument.mock_calls, expected_calls diff --git a/tests/test_wait_loop.py b/tests/test_wait_loop.py index b925672..3557191 100644 --- a/tests/test_wait_loop.py +++ b/tests/test_wait_loop.py @@ -11,6 +11,7 @@ class TestWaitLoop(TestCase): def setUp(self): self.source = mkdtemp() + self.custom_error_file = "nginx_error_output.hnclusterweb1" self.mock_logger = Mock(spec_set=logging.Logger) self.nginx_config_reloader = self.set_up_patch( "nginx_config_reloader.NginxConfigReloader" @@ -35,6 +36,7 @@ def test_wait_loop_creates_nginx_config_reloader_handler(self): no_custom_config=False, dir_to_watch=self.source, use_systemd=False, + error_file=nginx_config_reloader.ERROR_FILE, ) def test_wait_loop_creates_handler_with_custom_arguments(self): @@ -42,6 +44,7 @@ def test_wait_loop_creates_handler_with_custom_arguments(self): no_magento_config=True, no_custom_config=True, use_systemd=True, + error_file=self.custom_error_file, ) self.nginx_config_reloader.assert_called_once_with( @@ -50,6 +53,7 @@ def test_wait_loop_creates_handler_with_custom_arguments(self): no_custom_config=True, dir_to_watch=self.source, use_systemd=True, + error_file=self.custom_error_file, ) def test_wait_loop_sets_up_dbus_when_no_dbus_is_false(self):