Refactored script running into its own module.

src/formconfig.py

@@ -7,20 +7,6 @@ configuration being served by this instance of ScriptForm.
 import logging
 import stat
 import os
-import subprocess
-import pwd
-import grp
-
-
-def run_as(uid, gid, groups):
-    """Closure that changes the current running user and groups. Called before
-    executing scripts by Subprocess."""
-    def set_acc():
-        """Change user and groups"""
-        os.setgroups(groups)
-        os.setgid(gid)
-        os.setuid(uid)
-    return set_acc
 
 
 class FormConfigError(Exception):
@@ -80,93 +66,3 @@ class FormConfig(object):
             else:
                 form_list.append(form_def)
         return form_list
-
-    def run_script(self, form_name, form_values, stdout=None, stderr=None):
-        """
-        Perform a callback for the form `form_name`. This calls a script.
-        `form_values` is a dictionary of validated values as returned by
-        FormDefinition.validate(). If form.output is of type 'raw', `stdout`
-        and `stderr` have to be open filehandles where the output of the
-        callback should be written. The output of the script is hooked up to
-        the output, depending on the output type.
-        """
-        # FIXME: This doesn't really belong in FormCOnfig.
-        form = self.get_form_def(form_name)
-
-        # Validate params
-        if form.output == 'raw' and (stdout is None or stderr is None):
-            msg = 'stdout and stderr cannot be none if script output ' \
-                  'is \'raw\''
-            raise ValueError(msg)
-
-        # Pass form values to the script through the environment as strings.
-        env = os.environ.copy()
-        for key, value in form_values.items():
-            env[key] = str(value)
-
-        # Get the user uid, gid and groups we should run as. If the current
-        # user is root, we run as the given user or 'nobody' if no user was
-        # specified. Otherwise, we run as the user we already are.
-        if os.getuid() == 0:
-            if form.run_as is not None:
-                runas_pw = pwd.getpwnam(form.run_as)
-            else:
-                # Run as nobody
-                runas_pw = pwd.getpwnam('nobody')
-            runas_gr = grp.getgrgid(runas_pw.pw_gid)
-            groups = [
-                g.gr_gid
-                for g in grp.getgrall()
-                if runas_pw.pw_name in g.gr_mem
-            ]
-            msg = "Running script as user={0}, gid={1}, groups={2}"
-            run_as_fn = run_as(runas_pw.pw_uid, runas_pw.pw_gid, groups)
-            self.log.info(msg.format(runas_pw.pw_name, runas_gr.gr_name,
-                                     str(groups)))
-        else:
-            run_as_fn = None
-            if form.run_as is not None:
-                self.log.critical("Not running as root, so we can't run the "
-                                  "script as user '{0}'".format(form.run_as))
-
-        # If the form output type is 'raw', we directly stream the output to
-        # the browser. Otherwise we store it for later displaying.
-        if form.output == 'raw':
-            try:
-                proc = subprocess.Popen(form.script, shell=True,
-                                        stdout=stdout,
-                                        stderr=stderr,
-                                        env=env,
-                                        close_fds=True,
-                                        preexec_fn=run_as_fn)
-                stdout, stderr = proc.communicate(input)
-                self.log.info("Exit code: {0}".format(proc.returncode))
-                return proc.returncode
-            except OSError as err:
-                self.log.exception(err)
-                stderr.write(str(err) + '. Please see the log file.')
-                return -1
-        else:
-            try:
-                proc = subprocess.Popen(form.script, shell=True,
-                                        stdin=subprocess.PIPE,
-                                        stdout=subprocess.PIPE,
-                                        stderr=subprocess.PIPE,
-                                        env=env,
-                                        close_fds=True,
-                                        preexec_fn=run_as_fn)
-                stdout, stderr = proc.communicate()
-                self.log.info("Exit code: {0}".format(proc.returncode))
-                return {
-                    'stdout': stdout,
-                    'stderr': stderr,
-                    'exitcode': proc.returncode
-                }
-            except OSError as err:
-                self.log.exception(err)
-                return {
-                    'stdout': '',
-                    'stderr': 'Internal error: {0}. Please see the log '
-                              'file.'.format(str(err)),
-                    'exitcode': -1
-                }

src/runscript.py

@@ -0,0 +1,112 @@
+"""
+The runscript module is responsible for running external scripts and processing
+their output.
+"""
+
+import logging
+import os
+import pwd
+import grp
+import subprocess
+
+
+log = logging.getLogger('RUNSCRIPT')
+
+def run_as(uid, gid, groups):
+    """Closure that changes the current running user and groups. Called before
+    executing scripts by Subprocess."""
+    def set_acc():
+        """Change user and groups"""
+        os.setgroups(groups)
+        os.setgid(gid)
+        os.setuid(uid)
+    return set_acc
+
+def run_script(form_def, form_values, stdout=None, stderr=None):
+    """
+    Perform a callback for the form `form_def`. This calls a script.
+    `form_values` is a dictionary of validated values as returned by
+    FormDefinition.validate(). If form_def.output is of type 'raw', `stdout`
+    and `stderr` have to be open filehandles where the output of the
+    callback should be written. The output of the script is hooked up to
+    the output, depending on the output type.
+    """
+    # Validate params
+    if form_def.output == 'raw' and (stdout is None or stderr is None):
+        msg = 'stdout and stderr cannot be none if script output ' \
+              'is \'raw\''
+        raise ValueError(msg)
+
+    # Pass form values to the script through the environment as strings.
+    env = os.environ.copy()
+    for key, value in form_values.items():
+        env[key] = str(value)
+
+    # Get the user uid, gid and groups we should run as. If the current
+    # user is root, we run as the given user or 'nobody' if no user was
+    # specified. Otherwise, we run as the user we already are.
+    if os.getuid() == 0:
+        if form_def.run_as is not None:
+            runas_pw = pwd.getpwnam(form_def.run_as)
+        else:
+            # Run as nobody
+            runas_pw = pwd.getpwnam('nobody')
+        runas_gr = grp.getgrgid(runas_pw.pw_gid)
+        groups = [
+            g.gr_gid
+            for g in grp.getgrall()
+            if runas_pw.pw_name in g.gr_mem
+        ]
+        msg = "Running script as user={0}, gid={1}, groups={2}"
+        run_as_fn = run_as(runas_pw.pw_uid, runas_pw.pw_gid, groups)
+        log.info(msg.format(runas_pw.pw_name, runas_gr.gr_name,
+                                 str(groups)))
+    else:
+        run_as_fn = None
+        if form_def.run_as is not None:
+            log.critical("Not running as root, so we can't run the "
+                              "script as user '{0}'".format(form_def.run_as))
+
+    # If the form output type is 'raw', we directly stream the output to
+    # the browser. Otherwise we store it for later displaying.
+    if form_def.output == 'raw':
+        try:
+            proc = subprocess.Popen(form_def.script,
+                                    shell=True,
+                                    stdout=stdout,
+                                    stderr=stderr,
+                                    env=env,
+                                    close_fds=True,
+                                    preexec_fn=run_as_fn)
+            stdout, stderr = proc.communicate(input)
+            log.info("Exit code: {0}".format(proc.returncode))
+            return proc.returncode
+        except OSError as err:
+            log.exception(err)
+            stderr.write(str(err) + '. Please see the log file.')
+            return -1
+    else:
+        try:
+            proc = subprocess.Popen(form_def.script,
+                                    shell=True,
+                                    stdin=subprocess.PIPE,
+                                    stdout=subprocess.PIPE,
+                                    stderr=subprocess.PIPE,
+                                    env=env,
+                                    close_fds=True,
+                                    preexec_fn=run_as_fn)
+            stdout, stderr = proc.communicate()
+            log.info("Exit code: {0}".format(proc.returncode))
+            return {
+                'stdout': stdout,
+                'stderr': stderr,
+                'exitcode': proc.returncode
+            }
+        except OSError as err:
+            log.exception(err)
+            return {
+                'stdout': '',
+                'stderr': 'Internal error: {0}. Please see the log '
+                          'file.'.format(str(err)),
+                'exitcode': -1
+            }

src/webapp.py

@@ -12,6 +12,7 @@ import hashlib
 
 from formrender import FormRender
 from webserver import HTTPError, RequestHandler
+import runscript
 
 
 HTML_HEADER = u'''<html>
@@ -417,7 +418,8 @@ class ScriptFormWebApp(RequestHandler):
             log.info("User: {0}".format(username))
             log.info("Variables: {0}".format(dict(form_values.items())))
 
-            result = form_config.run_script(form_name, form_values, self.wfile,
+            form_def = form_config.get_form_def(form_name)
+            result = runscript.run_script(form_def, form_values, self.wfile,
                                             self.wfile)
             if form_def.output != 'raw':
                 # Ignore everything if we're doing raw output, since it's the

test/test.py

@@ -49,7 +49,8 @@ class FormConfigTestCase(unittest.TestCase):
         """Test a callback that returns output in strings"""
         sf = scriptform.ScriptForm('test_formconfig_callback.json')
         fc = sf.get_form_config()
-        res = fc.run_script('test_store', {})
+        fd = fc.get_form_def('test_store')
+        res = runscript.run_script(fd, {})
         self.assertEquals(res['exitcode'], 33)
         self.assertTrue('stdout' in res['stdout'])
         self.assertTrue('stderr' in res['stderr'])
@@ -58,9 +59,10 @@ class FormConfigTestCase(unittest.TestCase):
         """Test a callback that returns raw output"""
         sf = scriptform.ScriptForm('test_formconfig_callback.json')
         fc = sf.get_form_config()
+        fd = fc.get_form_def('test_raw')
         stdout = file('tmp_stdout', 'w+') # can't use StringIO
         stderr = file('tmp_stderr', 'w+')
-        exitcode = fc.run_script('test_raw', {}, stdout, stderr)
+        exitcode = runscript.run_script(fd, {}, stdout, stderr)
         stdout.seek(0)
         stderr.seek(0)
         self.assertTrue(exitcode == 33)
@@ -71,7 +73,8 @@ class FormConfigTestCase(unittest.TestCase):
         """
         sf = scriptform.ScriptForm('test_formconfig_callback.json')
         fc = sf.get_form_config()
-        self.assertRaises(ValueError, fc.run_script, 'test_raw', {})
+        fd = fc.get_form_def('test_raw')
+        self.assertRaises(ValueError, runscript.run_script, fd, {})
 
 
 class FormDefinitionTest(unittest.TestCase):
@@ -566,6 +569,7 @@ if __name__ == '__main__':
 
     sys.path.insert(0, '../src')
     import scriptform
+    import runscript
     unittest.main(exit=False)
 
     cov.stop()