From bf80f2b9e0ad86a7795932fa07d7ebeb834fd88b Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Thu, 29 Apr 2021 10:58:19 -0600 Subject: [PATCH 1/5] New feature to allow the program environment to be loaded from an external file or program. - This allows supervisord to be used in conjunction with any secrets pattern using a root-only file or a program that can provide environment variables that a program should have. It can be set globally in the supervisord section, or per program in a program section. The new options are environment_file or environment_loader. They are optional, and errors in one of them will prevent startup. They can be set in the [supervisord] section and then will be passed down to the programs, or in the program definitions. The file/loader is checked right before the calls to spawn() in order to avoid problems with calling a subprocess after the child fork, and to allow restarts to reload those environment values. - Updated the docs for these new options - Updated the tests to add a new test to check these new options. --- docs/configuration.rst | 67 +++++++++++++++ supervisor/options.py | 66 ++++++++++++++- supervisor/process.py | 11 ++- supervisor/tests/base.py | 9 +- supervisor/tests/test_options.py | 140 +++++++++++++++++++++++++++++++ 5 files changed, 289 insertions(+), 4 deletions(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index 8d23c261e..1db4eaeed 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -464,6 +464,42 @@ follows. *Introduced*: 3.0 +``environment_file`` + + An absolute path to a file that contains a ``KEY=VAL`` entry on each line. + Lines that begin with a '#' character are ignored. Leading and trailing + whitespace are stripped off. Each valid ``KEY=VAL`` line will be placed + in the environment of all child processes. The VAL entries must not be quoted, + and interpolation is not supported for these values. The file must be readable + by supervisord, and may be only readable by the user supervisord runs as since + these values are loaded before any privileges are dropped for child processes. + All other behaviors of the ``environment`` values are followed. When this is + set in the supervisord section, it will be applied to all program sections unless + they explicitly set either ``environment_file`` or ``environment_loader``. Only one of + the program setting or the supervisord setting for environment_file is processed. + + *Default*: no value + + *Required*: No. + + *Introduced*: 4.2.3 + +``environment_loader`` + + A shell command or an absolute path to a program that will be run by supervisord before launching + the child processes, and the stdout will be captured and parsed according to the rules for + ``environment_file``. Only one of ``environment_file`` or ``environment_loader`` should be set, and + ``environment_file`` takes precedence. When this is set in the supervisord section, + it will be applied to all program sections unless they explicitly set either + ``environment_file`` or ``environment_loader``. Only one of the program setting or the + supervisord setting for environment_loader is processed. + + *Default*: no value + + *Required*: No. + + *Introduced*: 4.2.3 + ``identifier`` The identifier string for this supervisor process, used by the RPC @@ -1099,6 +1135,37 @@ where specified. *Introduced*: 3.0 +``environment_file`` + + An absolute path to a file that contains a ``KEY=VAL`` entry on each line. + Lines that begin with a '#' character are ignored. Leading and trailing + whitespace between the values are stripped off. Each valid ``KEY=VAL`` line will be placed + in the environment of all child processes. The VAL entries must not be quoted, + and interpolation is not supported for these values. The file must be readable + by supervisord, and may be only readable by the user supervisord runs as since + these values are loaded before any privileges are dropped for child processes. + All other behaviors of the ``environment`` values are followed. + + *Default*: no value + + *Required*: No. + + *Introduced*: 4.2.3 + +``environment_loader`` + + A shell command or an absolute path to a program that will be by supervisord before launching + a child process, and the stdout will be captured and parsed according to the rules for + ``environment_file``. The program must be executable by supervisord. Only one of + ``environment_file`` or ``environment_loader`` should be set, and ``environment_file`` takes precedence. + + *Default*: no values + + *Required*: No. + + *Introduced*: 4.2.3 + + ``directory`` A file path representing a directory to which :program:`supervisord` diff --git a/supervisor/options.py b/supervisor/options.py index 7b53cc760..e05266429 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -656,6 +656,8 @@ def get(opt, default, **kwargs): environ_str = get('environment', '') environ_str = expand(environ_str, expansions, 'environment') section.environment = dict_of_key_value_pairs(environ_str) + section.environment_file = get('environment_file', None) + section.environment_loader = get('environment_loader', None) # extend expansions for global from [supervisord] environment definition for k, v in section.environment.items(): @@ -674,6 +676,13 @@ def get(opt, default, **kwargs): env = section.environment.copy() env.update(proc.environment) proc.environment = env + + # set the environment file/loader on the process configs but let them override it + if not proc.environment_file and not proc.environment_loader: + if section.environment_file: + proc.environment_file = section.environment_file + elif section.environment_loader: + proc.environment_loader = section.environment_loader section.server_configs = self.server_configs_from_parser(parser) section.profile_options = None return section @@ -925,6 +934,8 @@ def get(section, opt, *args, **kwargs): numprocs = integer(get(section, 'numprocs', 1)) numprocs_start = integer(get(section, 'numprocs_start', 0)) environment_str = get(section, 'environment', '', do_expand=False) + environment_file = get(section, 'environment_file', '', do_expand=False) + environment_loader = get(section, 'environment_loader', '', do_expand=False) stdout_cmaxbytes = byte_size(get(section,'stdout_capture_maxbytes','0')) stdout_events = boolean(get(section, 'stdout_events_enabled','false')) stderr_cmaxbytes = byte_size(get(section,'stderr_capture_maxbytes','0')) @@ -1057,6 +1068,8 @@ def get(section, opt, *args, **kwargs): exitcodes=exitcodes, redirect_stderr=redirect_stderr, environment=environment, + environment_file=environment_file, + environment_loader=environment_loader, serverurl=serverurl) programs.append(pconfig) @@ -1875,7 +1888,7 @@ class ProcessConfig(Config): 'stderr_events_enabled', 'stderr_syslog', 'stopsignal', 'stopwaitsecs', 'stopasgroup', 'killasgroup', 'exitcodes', 'redirect_stderr' ] - optional_param_names = [ 'environment', 'serverurl' ] + optional_param_names = [ 'environment', 'environment_file', 'environment_loader', 'serverurl' ] def __init__(self, options, **params): self.options = options @@ -1939,6 +1952,57 @@ def make_dispatchers(self, proc): dispatchers[stdin_fd] = PInputDispatcher(proc, 'stdin', stdin_fd) return dispatchers, p + def load_external_environment_definition(self): + return self.load_external_environment_definition_for_config(self) + + # this is separated out in order to make it easier to test + @classmethod + def load_external_environment_definition_for_config(cls, config): + # lazily load extra env vars before we drop privileges so that this can be used to load a secrets file + # or execute a program to get more env configuration. It doesn't have to be secrets, just config that + # needs to be separate from the supervisor config for whatever reason. The supervisor config interpolation + # is not supported here. The data format is just plain text, with one k=v value per line. Lines starting + # with '#' are ignored. + env = {} + envdata = None + if config.environment_file: + if os.path.exists(config.environment_file): + try: + with open(config.environment_file, 'r') as f: + envdata = f.read() + + except Exception as e: + raise ProcessException("environment_file read failure on %s: %s" % (config.environment_file, e)) + + elif config.environment_loader: + try: + from subprocess import check_output, CalledProcessError + kwargs = dict(shell=True) + if not PY2: + kwargs['text'] = True + + envdata = check_output(config.environment_loader, **kwargs) + + except CalledProcessError as e: + raise ProcessException("environment_loader failure with %s: %d, %s" % (config.environment_loader, e.returncode, e.output)) + + if envdata: + extra_env = {} + + for line in envdata.splitlines(): + line = line.strip() + if line.startswith('#'): # ignore comments + continue + + key, val = [s.strip() for s in line.split('=', 1)] + if key: + extra_env[key.upper()] = val + + if extra_env: + env.update(extra_env) + + return env + class EventListenerConfig(ProcessConfig): def make_dispatchers(self, proc): # always use_stderr=True for eventlisteners because mixing stderr diff --git a/supervisor/process.py b/supervisor/process.py index d6f60f3e2..b047886e5 100644 --- a/supervisor/process.py +++ b/supervisor/process.py @@ -217,6 +217,10 @@ def spawn(self): try: filename, argv = self.get_execv_args() + + # check the environment_file/environment_loader options before we fork to simplify child process management + extra_env = self.config.load_external_environment_definition() + except ProcessException as what: self.record_spawnerr(what.args[0]) self._assertInState(ProcessStates.STARTING) @@ -260,7 +264,7 @@ def spawn(self): return self._spawn_as_parent(pid) else: - return self._spawn_as_child(filename, argv) + return self._spawn_as_child(filename, argv, extra_env=extra_env) def _spawn_as_parent(self, pid): # Parent @@ -284,7 +288,7 @@ def _prepare_child_fds(self): for i in range(3, options.minfds): options.close_fd(i) - def _spawn_as_child(self, filename, argv): + def _spawn_as_child(self, filename, argv, extra_env=None): options = self.config.options try: # prevent child from receiving signals sent to the @@ -322,6 +326,9 @@ def _spawn_as_child(self, filename, argv): if self.config.environment is not None: env.update(self.config.environment) + if extra_env: + env.update(extra_env) + # change directory cwd = self.config.directory try: diff --git a/supervisor/tests/base.py b/supervisor/tests/base.py index bef82e964..f849ea7aa 100644 --- a/supervisor/tests/base.py +++ b/supervisor/tests/base.py @@ -520,7 +520,8 @@ def __init__(self, options, name, command, directory=None, umask=None, stderr_syslog=False, redirect_stderr=False, stopsignal=None, stopwaitsecs=10, stopasgroup=False, killasgroup=False, - exitcodes=(0,), environment=None, serverurl=None): + exitcodes=(0,), environment=None, environment_file=None, environment_loader=None, + serverurl=None): self.options = options self.name = name self.command = command @@ -552,6 +553,8 @@ def __init__(self, options, name, command, directory=None, umask=None, self.killasgroup = killasgroup self.exitcodes = exitcodes self.environment = environment + self.environment_file = environment_file + self.environment_loader = environment_loader self.directory = directory self.umask = umask self.autochildlogs_created = False @@ -582,6 +585,10 @@ def make_dispatchers(self, proc): dispatchers[stdin_fd] = DummyDispatcher(writable=True) return dispatchers, pipes + def load_external_environment_definition(self): + from supervisor.options import ProcessConfig + return ProcessConfig.load_external_environment_definition_for_config(self) + def makeExecutable(file, substitutions=None): import os import sys diff --git a/supervisor/tests/test_options.py b/supervisor/tests/test_options.py index 5c9a35f6d..f669894a0 100644 --- a/supervisor/tests/test_options.py +++ b/supervisor/tests/test_options.py @@ -3316,6 +3316,146 @@ def test_daemonize_notifies_poller_before_and_after_fork(self): instance.poller.before_daemonize.assert_called_once_with() instance.poller.after_daemonize.assert_called_once_with() + def test_options_with_environment_options(self): + f1 = f2 = f3 = None + + try: + f1 = tempfile.NamedTemporaryFile(mode="w+", delete=False) + f1.write("""# skip comment + TEST_SECRET1 = asdf +""") + f1.flush() + + f2 = tempfile.NamedTemporaryFile(mode="w+", delete=False) + f2.write("TEST_SECRET2=qwerty\n") + f2.flush() + + f3 = tempfile.NamedTemporaryFile(mode="w+", delete=False) + f3.write("""#!/bin/bash +echo "TEST_SECRET3=zxcv" +""") + f3.flush() + + f4 = tempfile.NamedTemporaryFile(mode="w+", delete=False) + f4.write("""#!/bin/bash +echo "TEST_SECRET4=yuio" +exit 64 +""") + f4.flush() + + f5 = tempfile.NamedTemporaryFile(mode="w+", delete=False) + f5.write("TEST_SECRET2=qwerty\n") + f5.flush() + os.chmod(f5.name, 0o000) + + s = lstrip(""" + [supervisord] + environment_file=%s + + [supervisorctl] + serverurl=http://localhost:9001 + + [program:test1] + command=/bin/sh + + [program:test2] + command=/bin/bash + environment_file=%s + + [program:test3] + command=/bin/echo + environment_loader=/bin/sh %s + + [program:test4] + command=/bin/bash + environment_loader=/bin/sh %s + + [program:test5] + command=/bin/ls + environment_file=%s + """ % (f1.name, f2.name, f3.name, f4.name, f5.name)) + + fp = StringIO(s) + instance = self._makeOne() + instance.configfile = fp + instance.realize(args=[]) + + options = instance.configroot.supervisord + self.assertEqual(options.environment_file, f1.name) + self.assertEqual(options.environment_loader, None) + + test1, test2, test3, test4, test5 = options.process_group_configs + proc1, proc2, proc3, proc4, proc5 = \ + test1.process_configs[0], test2.process_configs[0], test3.process_configs[0], \ + test4.process_configs[0], test5.process_configs[0] + + self.assertEqual(test1.name, 'test1') + self.assertEqual(len(test1.process_configs), 1) + self.assertEqual(test2.name, 'test2') + self.assertEqual(len(test2.process_configs), 1) + self.assertEqual(test3.name, 'test3') + self.assertEqual(len(test3.process_configs), 1) + self.assertEqual(test4.name, 'test4') + self.assertEqual(len(test4.process_configs), 1) + self.assertEqual(test5.name, 'test5') + self.assertEqual(len(test5.process_configs), 1) + + self.assertEqual(proc1.name, 'test1') + self.assertEqual(proc1.command, '/bin/sh') + self.assertEqual(proc1.environment_file, f1.name) + self.assertEqual(proc1.environment_loader, '') + + env = proc1.load_external_environment_definition() + self.assertEqual(env, {'TEST_SECRET1': 'asdf'}) + + self.assertEqual(proc2.name, 'test2') + self.assertEqual(proc2.command, '/bin/bash') + self.assertEqual(proc2.environment_file, f2.name) + self.assertEqual(proc2.environment_loader, '') + + env = proc2.load_external_environment_definition() + self.assertEqual(env, {'TEST_SECRET2': 'qwerty'}) + + self.assertEqual(proc3.name, 'test3') + self.assertEqual(proc3.command, '/bin/echo') + self.assertEqual(proc3.environment_file, '') + self.assertEqual(proc3.environment_loader, '/bin/sh %s' % f3.name) + + env = proc3.load_external_environment_definition() + self.assertEqual(env, {'TEST_SECRET3': 'zxcv'}) + + # validate that an error in the loader gets raised + self.assertEqual(proc4.name, 'test4') + self.assertEqual(proc4.command, '/bin/bash') + self.assertEqual(proc4.environment_file, '') + self.assertEqual(proc4.environment_loader, '/bin/sh %s' % f4.name) + + from supervisor.options import ProcessException + with self.assertRaises(ProcessException): + proc4.load_external_environment_definition() + + # validate that an error in the file reader gets raised + self.assertEqual(proc5.name, 'test5') + self.assertEqual(proc5.command, '/bin/ls') + self.assertEqual(proc5.environment_file, f5.name) + self.assertEqual(proc5.environment_loader, '') + + with self.assertRaises(ProcessException): + proc5.load_external_environment_definition() + + finally: + if f1: + os.unlink(f1.name) + if f2: + os.unlink(f2.name) + if f3: + os.unlink(f3.name) + if f4: + os.unlink(f4.name) + if f5: + os.unlink(f5.name) + + class ProcessConfigTests(unittest.TestCase): def _getTargetClass(self): from supervisor.options import ProcessConfig From 8f1675ad4ae7d314890780643317c1fc73a3c68c Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Fri, 30 Apr 2021 14:06:27 -0600 Subject: [PATCH 2/5] Update to the environment_{file|loader} feature to fix compatibility with python 3.{4,5,6} --- supervisor/options.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index e05266429..3dc84afd9 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1978,11 +1978,17 @@ def load_external_environment_definition_for_config(cls, config): try: from subprocess import check_output, CalledProcessError kwargs = dict(shell=True) - if not PY2: - kwargs['text'] = True + if sys.version_info.major >= 3: + if sys.version_info.minor >= 7: + kwargs['text'] = True + else: + pass # we will decode the bytes returned after reading it for these versions of python envdata = check_output(config.environment_loader, **kwargs) + if sys.version_info.major >= 3 and sys.version_info.minor < 7: + envdata = envdata.decode('utf8') + except CalledProcessError as e: raise ProcessException("environment_loader failure with %s: %d, %s" % (config.environment_loader, e.returncode, e.output)) From a95213f54908687e9a27c4e1a4ef107352f0d729 Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Fri, 30 Apr 2021 15:50:07 -0600 Subject: [PATCH 3/5] Tweaks to the initial environment_{file|loader} implementation - Remove the blocking call to subprocess.check_output in the main supervisord process. Instead make that call in the child process immediately after the initial fork, before doing any changes to the process config and state. This keeps the blocking call out of the danger zone, while still keeping the simple design. --- supervisor/options.py | 21 ++++++++------------- supervisor/process.py | 15 ++++++++++----- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index 3dc84afd9..86a5a6f4b 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1955,7 +1955,8 @@ def make_dispatchers(self, proc): def load_external_environment_definition(self): return self.load_external_environment_definition_for_config(self) - # this is separated out in order to make it easier to test + # NOTE - THIS IS BLOCKING CODE AND MUST ONLY BE CALLED IN TESTS OR IN CHILD PROCESSES, NOT THE + # MAIN SUPERVISORD THREAD OF EXECUTION @classmethod def load_external_environment_definition_for_config(cls, config): # lazily load extra env vars before we drop privileges so that this can be used to load a secrets file @@ -1976,21 +1977,15 @@ def load_external_environment_definition_for_config(cls, config): elif config.environment_loader: try: - from subprocess import check_output, CalledProcessError - kwargs = dict(shell=True) - if sys.version_info.major >= 3: - if sys.version_info.minor >= 7: - kwargs['text'] = True - else: - pass # we will decode the bytes returned after reading it for these versions of python - - envdata = check_output(config.environment_loader, **kwargs) + from subprocess import check_output, CalledProcessError, STDOUT as subprocess_STDOUT - if sys.version_info.major >= 3 and sys.version_info.minor < 7: - envdata = envdata.decode('utf8') + envdata = check_output(config.environment_loader, shell=True, stderr=subprocess_STDOUT) + envdata = as_string(envdata) except CalledProcessError as e: - raise ProcessException("environment_loader failure with %s: %d, %s" % (config.environment_loader, e.returncode, e.output)) + raise ProcessException("environment_loader failure with %s: %d, %s" % \ + (config.environment_loader, e.returncode, as_string(e.output)) + ) if envdata: extra_env = {} diff --git a/supervisor/process.py b/supervisor/process.py index b047886e5..e303a6b2d 100644 --- a/supervisor/process.py +++ b/supervisor/process.py @@ -218,9 +218,6 @@ def spawn(self): try: filename, argv = self.get_execv_args() - # check the environment_file/environment_loader options before we fork to simplify child process management - extra_env = self.config.load_external_environment_definition() - except ProcessException as what: self.record_spawnerr(what.args[0]) self._assertInState(ProcessStates.STARTING) @@ -264,7 +261,7 @@ def spawn(self): return self._spawn_as_parent(pid) else: - return self._spawn_as_child(filename, argv, extra_env=extra_env) + return self._spawn_as_child(filename, argv) def _spawn_as_parent(self, pid): # Parent @@ -288,9 +285,17 @@ def _prepare_child_fds(self): for i in range(3, options.minfds): options.close_fd(i) - def _spawn_as_child(self, filename, argv, extra_env=None): + def _spawn_as_child(self, filename, argv): options = self.config.options try: + # check the environment_file/environment_loader options after forking in order to avoid blocking the + # main supervisord thread, but do it before we start to mix up the process signals/state + try: + extra_env = self.config.load_external_environment_definition() + except ProcessException as e: + self.record_spawnerr(e.args[0]) + raise + # prevent child from receiving signals sent to the # parent by calling os.setpgrp to create a new process # group for the child; this prevents, for instance, From 40aeb028f042121554a2e67ba7451b12b999bbcb Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Wed, 23 Aug 2023 08:35:37 -0600 Subject: [PATCH 4/5] Update to the environment loading functionality to provide dotenv compatibility - Handle env files formatted like dotenv files where values can be quoted. We'll unquote those, preserving embedded whitespace, and also expand newline literals into actual newlines for double quoted strings. This allows supervisors to reuse the dotenv files created nodejs projects. --- supervisor/options.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/supervisor/options.py b/supervisor/options.py index 86a5a6f4b..668cd977f 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1995,8 +1995,22 @@ def load_external_environment_definition_for_config(cls, config): if line.startswith('#'): # ignore comments continue - key, val = [s.strip() for s in line.split('=', 1)] + key, val = [s.strip() for s in line.split("=", 1)] if key: + # for compatibility with .env files and the npm dotenv library, if the value is quoted let's + # unquote it. Also, double quoted strings get embedded '\\n' chars expanded to real newlines. + # Don't strip whitespace inside quoted values. + if val.startswith("\""): + val = val.strip("\"") + + # expand embedded '\n' chars into actual newlines + val.replace("\\n", "\n") + + elif val.startswith("'"): + val = val.strip("'") + elif val.startswith("`"): + val = val.strip("`") + extra_env[key.upper()] = val if extra_env: From f10b1798352491da332523bb27638d1917cdccd0 Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Wed, 23 Aug 2023 11:00:47 -0600 Subject: [PATCH 5/5] More fixes for dotenv compatibility - Handle env files with empty and malformed lines. Just skip those and ignore them. --- supervisor/options.py | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/supervisor/options.py b/supervisor/options.py index 668cd977f..9d83037e0 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1992,30 +1992,38 @@ def load_external_environment_definition_for_config(cls, config): for line in envdata.splitlines(): line = line.strip() - if line.startswith('#'): # ignore comments + if not line or line.startswith('#'): # ignore comments and empty lines continue - key, val = [s.strip() for s in line.split("=", 1)] - if key: - # for compatibility with .env files and the npm dotenv library, if the value is quoted let's - # unquote it. Also, double quoted strings get embedded '\\n' chars expanded to real newlines. - # Don't strip whitespace inside quoted values. - if val.startswith("\""): - val = val.strip("\"") + line_parts = line.split("=", 1) # ignore malformed lines + if len(line_parts) != 2: + continue + + key, val = [s.strip() for s in line_parts] + if not key or not val: + continue - # expand embedded '\n' chars into actual newlines - val.replace("\\n", "\n") + # for compatibility with .env files and the npm dotenv library, if the value is quoted let's + # unquote it. Also, double quoted strings get embedded '\\n' chars expanded to real newlines. + # Don't strip whitespace inside quoted values. + if val.startswith("\""): + val = val.strip("\"") - elif val.startswith("'"): - val = val.strip("'") - elif val.startswith("`"): - val = val.strip("`") + # expand embedded '\n' chars into actual newlines + val.replace("\\n", "\n") - extra_env[key.upper()] = val + elif val.startswith("'"): + val = val.strip("'") + elif val.startswith("`"): + val = val.strip("`") + + extra_env[key.upper()] = val if extra_env: + print(f"updating process env with: {extra_env}") env.update(extra_env) + return env class EventListenerConfig(ProcessConfig):