From 6f9eeaad4ce2c77166f3348aea6e17a0369906fa Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Tue, 19 Jan 2021 16:17:57 +0000 Subject: [PATCH 1/5] directory config --- supervisor/options.py | 82 ++++++++++++++++++++++++++++++++++++--- supervisor/supervisord.py | 4 +- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index 273d8a5d0..4b6bb3f62 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -312,10 +312,9 @@ def process_config(self, do_usage=True): """Process configuration data structure. This includes reading config file if necessary, setting defaults etc. - """ + """ if self.configfile: - self.process_config_file(do_usage) - + self.process_config_file(do_usage) # Copy config options to attributes of self. This only fills # in options that aren't already set from the command line. for name, confname in self.names_list: @@ -484,7 +483,7 @@ def default_configfile(self): def realize(self, *arg, **kw): Options.realize(self, *arg, **kw) section = self.configroot.supervisord - + # Additional checking of user option; set uid and gid if self.user is not None: try: @@ -520,6 +519,7 @@ def realize(self, *arg, **kw): self.serverurl = None self.server_configs = sconfigs = section.server_configs + self.directories = section.directories # we need to set a fallback serverurl that process.spawn can use @@ -670,6 +670,7 @@ def get(opt, default, **kwargs): env.update(proc.environment) proc.environment = env section.server_configs = self.server_configs_from_parser(parser) + section.directories = self.directories_from_parser(parser) section.profile_options = None return section @@ -1132,6 +1133,29 @@ def server_configs_from_parser(self, parser): return configs + def directories_from_parser(self, parser): + """Read [directory:x] sections from parser.""" + get = parser.saneget + directories = [] + all_sections = parser.sections() + for section in all_sections: + if not section.startswith('directory:'): + continue + name = section.split(':', 1)[1] + path = get(section, "path", None) + if path is None: + raise ValueError('[%s] section requires a value for "path"') + path = normalize_path(path) + create = boolean(get(section, "create", "false")) + mode_str = get(section, "mode", "777") + try: + mode = octal_type(mode_str) + except (TypeError, ValueError): + raise ValueError("Invalid mode value %s" % mode_str) + directory = DirectoryConfig(name, path, create, mode) + directories.append(directory) + return directories + def daemonize(self): self.poller.before_daemonize() self._daemonize() @@ -1496,7 +1520,20 @@ def make_logger(self): self.logger.warn(msg) for msg in self.parse_infos: self.logger.info(msg) - + + def check_directories(self): + # must be called after realize() and after supervisor does setuid() + for directory in self.directories: + self.logger.debug(repr(directory)) + try: + if directory.check(): + self.logger.info( + "created directory (%s) %s" + % (directory.name, directory.path) + ) + except ValueError as error: + self.usage(str(error)) + def make_http_servers(self, supervisord): from supervisor.http import make_http_servers return make_http_servers(self, supervisord) @@ -2060,6 +2097,41 @@ def make_group(self): from supervisor.process import FastCGIProcessGroup return FastCGIProcessGroup(self) + +class DirectoryConfig(object): + """Configuration for a required directory.""" + + def __init__(self, name, path, create, mode): + self.name = name + self.path = path + self.create = create + self.mode = mode + + def __repr__(self): + return "DirectoryConfig({!r}, {!r}, {!r}, {!r})".format( + self.name, self.path, self.create, self.mode + ) + + def check(self): + """Check if the direcory exists, and potentially try to create.""" + if os.path.exists(self.path): + if not os.path.isdir(self.path): + raise ValueError( + "required directory (%s) path %s is not a directory" + % (self.name, self.path) + ) + else: + if self.create: + os.makedirs(self.path, self.mode) + return True + else: + raise ValueError( + "required directory (%s) %s does not exist" + % (self.name, self.path) + ) + return False + + def readFile(filename, offset, length): """ Read length bytes from the file named by filename starting at offset """ diff --git a/supervisor/supervisord.py b/supervisor/supervisord.py index a5281c0bc..70bd14b6c 100755 --- a/supervisor/supervisord.py +++ b/supervisor/supervisord.py @@ -62,7 +62,7 @@ def main(self): # first request self.options.cleanup_fds() - self.options.set_uid_or_exit() + self.options.set_uid_or_exit() if self.options.first: self.options.set_rlimits_or_exit() @@ -71,6 +71,8 @@ def main(self): # delay logger instantiation until after setuid self.options.make_logger() + self.options.check_directories() + if not self.options.nocleanup: # clean up old automatic logs self.options.clear_autochildlogdir() From 95eb2ac56db8a7e08e50fce20257ff245b671066 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 21 Jan 2021 11:46:39 +0000 Subject: [PATCH 2/5] directory tests --- supervisor/options.py | 14 +++- supervisor/tests/base.py | 3 + supervisor/tests/test_options.py | 112 +++++++++++++++++++++++++++++-- 3 files changed, 120 insertions(+), 9 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index 4b6bb3f62..761e4c644 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1523,8 +1523,7 @@ def make_logger(self): def check_directories(self): # must be called after realize() and after supervisor does setuid() - for directory in self.directories: - self.logger.debug(repr(directory)) + for directory in self.directories: try: if directory.check(): self.logger.info( @@ -2108,10 +2107,19 @@ def __init__(self, name, path, create, mode): self.mode = mode def __repr__(self): - return "DirectoryConfig({!r}, {!r}, {!r}, {!r})".format( + return "DirectoryConfig({!r}, {!r}, {!r}, 0o{:o})".format( self.name, self.path, self.create, self.mode ) + def __eq__(self, other): + return ( + isinstance(other, DirectoryConfig) + and self.name == other.name + and self.path == other.path + and self.create == other.create + and self.mode == other.mode + ) + def check(self): """Check if the direcory exists, and potentially try to create.""" if os.path.exists(self.path): diff --git a/supervisor/tests/base.py b/supervisor/tests/base.py index b4ec8149b..b0feea9cb 100644 --- a/supervisor/tests/base.py +++ b/supervisor/tests/base.py @@ -133,6 +133,9 @@ def get_socket_map(self): def make_logger(self): pass + def make_directories(self): + pass + def clear_autochildlogdir(self): self.autochildlogdir_cleared = True diff --git a/supervisor/tests/test_options.py b/supervisor/tests/test_options.py index 70935b952..45e839e84 100644 --- a/supervisor/tests/test_options.py +++ b/supervisor/tests/test_options.py @@ -9,6 +9,7 @@ import shutil import errno import platform +import tempfile from supervisor.compat import StringIO from supervisor.compat import as_bytes @@ -3255,13 +3256,36 @@ def test_drop_privileges_nonroot_different_user(self): msg = instance.drop_privileges(42) self.assertEqual(msg, "Can't drop privilege as nonroot user") - def test_daemonize_notifies_poller_before_and_after_fork(self): + def test_directories_from_parser(self): + from supervisor.options import DirectoryConfig + text = lstrip("""\ + [directory:dir1] + path = /foo/bar + + [directory:dir2] + path = /foo/new + create = True + + [directory:dir3] + path = /foo/new2 + create = True + mode = 755 + """) + from supervisor.options import UnhosedConfigParser + config = UnhosedConfigParser() + config.read_string(text) instance = self._makeOne() - instance._daemonize = lambda: None - instance.poller = Mock() - instance.daemonize() - instance.poller.before_daemonize.assert_called_once_with() - instance.poller.after_daemonize.assert_called_once_with() + directories = instance.directories_from_parser(config) + + self.assertEqual( + directories, + [ + DirectoryConfig('dir1', '/foo/bar', False, 0o777), + DirectoryConfig('dir2', '/foo/new', True, 0o777), + DirectoryConfig('dir3', '/foo/new2', True, 0o755) + ] + ) + class ProcessConfigTests(unittest.TestCase): def _getTargetClass(self): @@ -3732,6 +3756,82 @@ def test_split_namespec(self): self.assertEqual(s('group:'), ('group', None)) self.assertEqual(s('group:*'), ('group', None)) + +class TestDirectories(unittest.TestCase): + + def setUp(self): + self.temp_dir = tempfile.mkdtemp("supervisor", "testdir") + + def tearDown(self): + shutil.rmtree(self.temp_dir) + + def test_repr(self): + from supervisor.options import DirectoryConfig + directory_config = DirectoryConfig( + "foo", + "/foo/bar", + True, + 0o777 + ) + self.assertEqual( + repr(directory_config), + "DirectoryConfig('foo', '/foo/bar', True, 0o777)" + ) + + def check_directory_exists(self): + from supervisor.options import DirectoryConfig + directory_config = DirectoryConfig( + "foo", + self.temp_dir, + False, + 0o777 + ) + # Should return false if directory exists and isn't created in the check + self.assertFalse(directory_config.check()) + + def check_directory_does_not_exist(self): + from supervisor.options import DirectoryConfig + directory = os.path.join(self.temp_dir, "nothing_here") + directory_config = DirectoryConfig( + "foo", + directory, + False, + 0o777 + ) + with self.assertRaises(ValueError): + # Directory doesn't exist, should raise ValueError + directory_config.check() + + def check_directory_not_a_directory(self): + from supervisor.options import DirectoryConfig + directory = os.path.join(self.temp_dir, "foo") + with open(directory, "w") as fh: + fh.write("test") + directory_config = DirectoryConfig( + "foo", + directory, + False, + 0o777 + ) + with self.assertRaises(ValueError): + # Path exists, but not a directory + directory_config.check() + + def check_create(self): + from supervisor.options import DirectoryConfig + directory = os.path.join(self.temp_dir, "foo/bar") + self.assertFalse(os.path.exists(directory)) + directory_config = DirectoryConfig( + "foo", + directory, + True, + 0o777 + ) + # With create = True, the directory will be created + self.assertTrue(directory_config.check()) + self.assertTrue(os.path.isdir(directory)) + + def test_suite(): return unittest.findTestCases(sys.modules[__name__]) From 2c418bec693b5d9c9e5256e91e4a56eca6545803 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 21 Jan 2021 11:54:54 +0000 Subject: [PATCH 3/5] docstring --- supervisor/options.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index 761e4c644..c5497322e 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -2121,11 +2121,12 @@ def __eq__(self, other): ) def check(self): - """Check if the direcory exists, and potentially try to create.""" + """Check if the directory exists, and potentially try to + create it. Return True if directory was created.""" if os.path.exists(self.path): if not os.path.isdir(self.path): raise ValueError( - "required directory (%s) path %s is not a directory" + "required directory (%s) %s is not a directory" % (self.name, self.path) ) else: From 0a46ce9b84b4a157da1824554e1a77ae57b7df07 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 21 Jan 2021 14:28:50 +0000 Subject: [PATCH 4/5] rename check to verify_exists --- supervisor/options.py | 8 ++++---- supervisor/tests/base.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index c5497322e..6c2c7f8a7 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1525,7 +1525,7 @@ def check_directories(self): # must be called after realize() and after supervisor does setuid() for directory in self.directories: try: - if directory.check(): + if directory.verify_exists(): self.logger.info( "created directory (%s) %s" % (directory.name, directory.path) @@ -2104,7 +2104,7 @@ def __init__(self, name, path, create, mode): self.name = name self.path = path self.create = create - self.mode = mode + self.mode = mode def __repr__(self): return "DirectoryConfig({!r}, {!r}, {!r}, 0o{:o})".format( @@ -2120,8 +2120,8 @@ def __eq__(self, other): and self.mode == other.mode ) - def check(self): - """Check if the directory exists, and potentially try to + def verify_exists(self): + """Verify the directory exists, and potentially try to create it. Return True if directory was created.""" if os.path.exists(self.path): if not os.path.isdir(self.path): diff --git a/supervisor/tests/base.py b/supervisor/tests/base.py index b0feea9cb..6cc300f8c 100644 --- a/supervisor/tests/base.py +++ b/supervisor/tests/base.py @@ -133,7 +133,7 @@ def get_socket_map(self): def make_logger(self): pass - def make_directories(self): + def check_directories(self): pass def clear_autochildlogdir(self): From a825104aec8b23adcf5f37ffc0466a7c88a66b30 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 21 Jan 2021 14:30:12 +0000 Subject: [PATCH 5/5] fix method name --- supervisor/tests/test_options.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/supervisor/tests/test_options.py b/supervisor/tests/test_options.py index 45e839e84..e8f9eb027 100644 --- a/supervisor/tests/test_options.py +++ b/supervisor/tests/test_options.py @@ -3787,7 +3787,7 @@ def check_directory_exists(self): 0o777 ) # Should return false if directory exists and isn't created in the check - self.assertFalse(directory_config.check()) + self.assertFalse(directory_config.verify_exists()) def check_directory_does_not_exist(self): from supervisor.options import DirectoryConfig @@ -3800,7 +3800,7 @@ def check_directory_does_not_exist(self): ) with self.assertRaises(ValueError): # Directory doesn't exist, should raise ValueError - directory_config.check() + directory_config.verify_exists() def check_directory_not_a_directory(self): from supervisor.options import DirectoryConfig @@ -3815,7 +3815,7 @@ def check_directory_not_a_directory(self): ) with self.assertRaises(ValueError): # Path exists, but not a directory - directory_config.check() + directory_config.verify_exists() def check_create(self): from supervisor.options import DirectoryConfig @@ -3828,7 +3828,7 @@ def check_create(self): 0o777 ) # With create = True, the directory will be created - self.assertTrue(directory_config.check()) + self.assertTrue(directory_config.verify_exists()) self.assertTrue(os.path.isdir(directory))