(configd) code style cleanups

(cherry picked from commit 9c84f2b4)

src/opnsense/service/modules/__init__.py

@@ -24,12 +24,17 @@
     POSSIBILITY OF SUCH DAMAGE.
 """
 
+
 def singleton(cls, *args, **kwargs):
     """ singleton pattern, use ad decorator
+    :param cls:
     """
     instances = {}
+
+    # noinspection PyShadowingNames
     def getinstance(*args, **kwargs):
         if cls not in instances:
             instances[cls] = cls(*args, **kwargs)
         return instances[cls]
+
     return getinstance

src/opnsense/service/modules/addons/template_helpers.py

@@ -30,6 +30,8 @@
 
 from operator import itemgetter
 
+
+# noinspection PyPep8Naming
 class Helpers(object):
     def __init__(self, template_in_data):
         """ initialize template helpers
@@ -82,7 +84,7 @@ class Helpers(object):
             return result
         else:
             # resort list by tag
-            return sorted(result,key=itemgetter(sortBy))
+            return sorted(result, key=itemgetter(sortBy))
 
     def getUUIDtag(self, uuid):
         """ retrieve tag name of registered uuid, returns __not_found__ if none available

src/opnsense/service/modules/config.py

@@ -28,14 +28,14 @@
     package : configd
     function: config handler
 """
-__author__ = 'Ad Schellevis'
-
 import os
 import stat
 import collections
 import copy
 import xml.etree.cElementTree as ElementTree
 
+__author__ = 'Ad Schellevis'
+
 
 class Config(object):
     def __init__(self, filename):
@@ -62,14 +62,14 @@ class Config(object):
             self._config_data['__uuid_tags__'] = self.__uuid_tags
             self._file_mod = mod_time
 
-    def _traverse(self, xmlNode):
+    def _traverse(self, xml_node):
         """ traverse xml node and return ordered dictionary structure
-        :param xmlNode: ElementTree node
+        :param xml_node: ElementTree node
         :return: collections.OrderedDict
         """
         this_item = collections.OrderedDict()
-        if len(list(xmlNode)) > 0:
-            for item in list(xmlNode):
+        if len(list(xml_node)) > 0:
+            for item in list(xml_node):
                 item_content = self._traverse(item)
                 if 'uuid' in item.attrib:
                     self.__uuid_data[item.attrib['uuid']] = item_content
@@ -88,7 +88,7 @@ class Config(object):
                     this_item[item.tag] = self._traverse(item)
         else:
             # last node, return text
-            return xmlNode.text
+            return xml_node.text
 
         return this_item
 
@@ -98,14 +98,15 @@ class Config(object):
             @param elem: cElementTree
             @param level: Currentlevel
         """
-        i = "\n" + level*"  "
+        i = "\n" + level * "  "
         if len(elem):
             if not elem.text or not elem.text.strip():
                 elem.text = i + "  "
             for e in elem:
-                self.indent(e, level+1)
+                self.indent(e, level + 1)
                 if not e.tail or not e.tail.strip():
                     e.tail = i + "  "
+            # noinspection PyUnboundLocalVariable
             if not e.tail or not e.tail.strip():
                 e.tail = i
         else:

src/opnsense/service/modules/daemonize.py

@@ -58,6 +58,7 @@ class Daemonize(object):
     def sigterm(self, signum, frame):
         """
         These actions will be done after SIGTERM.
+        :param frame:
         """
         self.logger.warn("Caught signal %s. Stopping daemon." % signum)
         os.remove(self.pid)

src/opnsense/service/modules/ph_inline_actions.py

@@ -48,7 +48,7 @@ def execute(action, parameters):
         # generate template
         tmpl = template.Template(action.root_dir)
         conf = config.Config(action.config)
-        tmpl.setConfig(conf.get())
+        tmpl.set_config(conf.get())
         filenames = tmpl.generate(parameters)
 
         del conf
@@ -78,7 +78,7 @@ def execute(action, parameters):
         # list all available configd actions
         from processhandler import ActionHandler
         act_handler = ActionHandler()
-        actions = act_handler.listActions(['message', 'description'])
+        actions = act_handler.list_actions(['message', 'description'])
 
         if unicode(parameters).lower() == 'json':
             import json

src/opnsense/service/modules/processhandler.py

@@ -28,8 +28,6 @@
     function: unix domain socket process worker process
 """
 
-__author__ = 'Ad Schellevis'
-
 import os
 import subprocess
 import socket
@@ -45,6 +43,8 @@ import tempfile
 import ph_inline_actions
 from modules import singleton
 
+__author__ = 'Ad Schellevis'
+
 
 class Handler(object):
     """ Main handler class, opens unix domain socket and starts listening
@@ -57,7 +57,8 @@ class Handler(object):
                     -> execute ActionHandler command using Action objects
                     <- send back result string
     """
-    def __init__(self, socket_filename, config_path, config_environment={}, simulation_mode=False):
+
+    def __init__(self, socket_filename, config_path, config_environment=None, simulation_mode=False):
         """ Constructor
 
         :param socket_filename: filename of unix domain socket to use
@@ -65,6 +66,8 @@ class Handler(object):
         :param simulation_mode: emulation mode, do not start actual (script) commands
         :return: object
         """
+        if config_environment is None:
+            config_environment = {}
         self.socket_filename = socket_filename
         self.config_path = config_path
         self.simulation_mode = simulation_mode
@@ -77,9 +80,10 @@ class Handler(object):
         :return:
         """
         while True:
+            # noinspection PyBroadException
             try:
                 # open action handler
-                actHandler = ActionHandler(config_path=self.config_path,
+                act_handler = ActionHandler(config_path=self.config_path,
                                             config_environment=self.config_environment)
 
                 # remove previous socket ( if exists )
@@ -99,7 +103,7 @@ class Handler(object):
                     # spawn a client connection
                     cmd_thread = HandlerClient(connection=connection,
                                                client_address=client_address,
-                                               action_handler=actHandler,
+                                               action_handler=act_handler,
                                                simulation_mode=self.simulation_mode)
                     if self.single_threaded:
                         # run single threaded
@@ -120,7 +124,7 @@ class Handler(object):
                     # cleanup on exit, remove socket
                     os.remove(self.socket_filename)
                 return
-            except:
+            except Exception:
                 # something went wrong... send traceback to syslog, restart listener (wait for a short time)
                 print (traceback.format_exc())
                 syslog.syslog(syslog.LOG_ERR, 'Handler died on %s' % traceback.format_exc())
@@ -130,6 +134,7 @@ class Handler(object):
 class HandlerClient(threading.Thread):
     """ Handle commands via specified socket connection
     """
+
     def __init__(self, connection, client_address, action_handler, simulation_mode=False):
         """
         :param connection: socket connection object
@@ -155,6 +160,7 @@ class HandlerClient(threading.Thread):
         exec_action = ''
         exec_params = ''
         exec_in_background = False
+        # noinspection PyBroadException
         try:
             # receive command, maximum data length is 4k... longer messages will be truncated
             data = self.connection.recv(4096)
@@ -186,7 +192,7 @@ class HandlerClient(threading.Thread):
 
                 # execute requested action
                 if self.simulation_mode:
-                    self.action_handler.showAction(exec_command, exec_action, exec_params, self.message_uuid)
+                    self.action_handler.show_action(exec_command, exec_action, exec_params, self.message_uuid)
                     result = 'OK'
                 else:
                     result = self.action_handler.execute(exec_command, exec_action, exec_params, self.message_uuid)
@@ -207,23 +213,28 @@ class HandlerClient(threading.Thread):
         except SystemExit:
             # ignore system exit related errors
             pass
-        except:
+        except Exception:
             print (traceback.format_exc())
-            syslog.syslog(syslog.LOG_ERR,
+            syslog.syslog(
+                    syslog.LOG_ERR,
                     'unable to sendback response [%s] for [%s][%s][%s] {%s}, message was %s' % (result,
                                                                                                 exec_command,
                                                                                                 exec_action,
                                                                                                 exec_params,
                                                                                                 self.message_uuid,
-                                                                                                      traceback.format_exc()))
+                                                                                                traceback.format_exc()
+                                                                                                )
+            )
         finally:
             if not exec_in_background:
                 self.connection.close()
 
+
 @singleton
 class ActionHandler(object):
     """ Start/stop services and functions using configuration data defined in conf/actions_<topic>.conf
     """
+
     def __init__(self, config_path=None, config_environment=None):
         """ Initialize action handler to start system functions
 
@@ -238,6 +249,7 @@ class ActionHandler(object):
 
         # try to load data on initial start
         if not hasattr(self, 'action_map'):
+            self.action_map = {}
             self.load_config()
 
     def load_config(self):
@@ -245,8 +257,6 @@ class ActionHandler(object):
 
         :return: None
         """
-
-        self.action_map = {}
         for config_filename in glob.glob('%s/actions_*.conf' % self.config_path) \
                 + glob.glob('%s/actions.d/actions_*.conf' % self.config_path):
             # this topic's name (service, filter, template, etc)
@@ -260,7 +270,7 @@ class ActionHandler(object):
             cnf.read(config_filename)
             for section in cnf.sections():
                 # map configuration data on object
-                action_obj = Action(config_environment = self.config_environment)
+                action_obj = Action(config_environment=self.config_environment)
                 for act_prop in cnf.items(section):
                     setattr(action_obj, act_prop[0], act_prop[1])
 
@@ -274,10 +284,13 @@ class ActionHandler(object):
                     for alias in section.split('|'):
                         self.action_map[topic_name][alias] = action_obj
 
-    def listActions(self, attributes=[]):
+    def list_actions(self, attributes=None):
         """ list all available actions
+        :param attributes:
         :return: dict
         """
+        if attributes is None:
+            attributes = []
         result = {}
         for command in self.action_map:
             for action in self.action_map[command]:
@@ -285,7 +298,7 @@ class ActionHandler(object):
                     # parse second level actions
                     # TODO: nesting actions may be better to solve recursive in here and in load_config part
                     for subAction in self.action_map[command][action]:
-                        cmd='%s %s %s'%(command,action,subAction)
+                        cmd = '%s %s %s' % (command, action, subAction)
                         result[cmd] = {}
                         for actAttr in attributes:
                             if hasattr(self.action_map[command][action][subAction], actAttr):
@@ -293,7 +306,7 @@ class ActionHandler(object):
                             else:
                                 result[cmd][actAttr] = ''
                 else:
-                    cmd='%s %s'%(command,action)
+                    cmd = '%s %s' % (command, action)
                     result[cmd] = {}
                     for actAttr in attributes:
                         if hasattr(self.action_map[command][action], actAttr):
@@ -303,7 +316,7 @@ class ActionHandler(object):
 
         return result
 
-    def findAction(self, command, action, parameters):
+    def find_action(self, command, action, parameters):
         """ find action object
 
         :param command: command/topic for example interface
@@ -320,7 +333,7 @@ class ActionHandler(object):
                         # 3 level action (  "interface linkup start" for example )
                         if isinstance(self.action_map[command][action][parameters[0]], Action):
                             action_obj = self.action_map[command][action][parameters[0]]
-                            action_obj.setParameterStartPos(1)
+                            action_obj.set_parameter_start_pos(1)
                 elif isinstance(self.action_map[command][action], Action):
                     action_obj = self.action_map[command][action]
 
@@ -336,17 +349,17 @@ class ActionHandler(object):
         :return: OK on success, else error code
         """
         action_params = []
-        action_obj = self.findAction(command, action, parameters)
+        action_obj = self.find_action(command, action, parameters)
 
         if action_obj is not None:
-            if parameters is not None and len(parameters) > action_obj.getParameterStartPos():
-                action_params = parameters[action_obj.getParameterStartPos():]
+            if parameters is not None and len(parameters) > action_obj.get_parameter_start_pos():
+                action_params = parameters[action_obj.get_parameter_start_pos():]
 
             return '%s\n' % action_obj.execute(action_params, message_uuid)
 
         return 'Action not found\n'
 
-    def showAction(self, command, action, parameters, message_uuid):
+    def show_action(self, command, action, parameters, message_uuid):
         """ debug/simulation mode: show action information
         :param command: command/topic for example interface
         :param action: action to run ( for example linkup )
@@ -354,10 +367,10 @@ class ActionHandler(object):
         :param message_uuid: message unique id
         :return: None
         """
-        action_obj = self.findAction(command, action, parameters)
+        action_obj = self.find_action(command, action, parameters)
         print ('---------------------------------------------------------------------')
         print ('execute %s.%s with parameters : %s ' % (command, action, parameters))
-        print ('action object %s (%s)' % (action_obj, action_obj.command))
+        print ('action object %s (%s) %s' % (action_obj, action_obj.command, message_uuid))
         print ('---------------------------------------------------------------------')
 
 
@@ -365,6 +378,7 @@ class Action(object):
     """ Action class,  handles actual (system) calls.
     set command, parameters (template) type and log message
     """
+
     def __init__(self, config_environment):
         """ setup default properties
         :param config_environment: environment to use
@@ -377,7 +391,7 @@ class Action(object):
         self.message = None
         self._parameter_start_pos = 0
 
-    def setParameterStartPos(self, pos):
+    def set_parameter_start_pos(self, pos):
         """
 
         :param pos: start position of parameter list
@@ -385,7 +399,7 @@ class Action(object):
         """
         self._parameter_start_pos = pos
 
-    def getParameterStartPos(self):
+    def get_parameter_start_pos(self):
         """ getter for _parameter_start_pos
         :return: start position of parameter list ( first argument can be part of action to start )
         """
@@ -427,18 +441,18 @@ class Action(object):
                     # use quotes on parameters to prevent code injection
                     if script_command.count('%s') > len(parameters):
                         # script command accepts more parameters then given, fill with empty parameters
-                        for i in range(script_command.count('%s')-len(parameters)):
+                        for i in range(script_command.count('%s') - len(parameters)):
                             parameters.append("")
                     elif len(parameters) > script_command.count('%s'):
                         # parameters then expected, fail execution
                         return 'Parameter mismatch'
 
                     # force escape of shell exploitable characters for all user parameters
-                    for escape_char in ['`','$','!','(',')','|']:
+                    for escape_char in ['`', '$', '!', '(', ')', '|']:
                         for i in range(len(parameters[0:script_command.count('%s')])):
-                            parameters[i] = parameters[i].replace(escape_char,'\\%s'%escape_char)
+                            parameters[i] = parameters[i].replace(escape_char, '\\%s' % escape_char)
 
-                    script_command = script_command % tuple(map(lambda x: '"'+x.replace('"', '\\"')+'"',
+                    script_command = script_command % tuple(map(lambda x: '"' + x.replace('"', '\\"') + '"',
                                                                 parameters[0:script_command.count('%s')]))
             if self.type.lower() == 'script':
                 # execute script type command
@@ -466,8 +480,9 @@ class Action(object):
                             script_output = output_stream.read()
                             script_error_output = error_stream.read()
                             if len(script_error_output) > 0:
-                                syslog.syslog(syslog.LOG_ERR, '[%s] Script action stderr returned "%s"' % (message_uuid,
-                                              script_error_output.strip()[:255])
+                                syslog.syslog(syslog.LOG_ERR,
+                                              '[%s] Script action stderr returned "%s"' %
+                                              (message_uuid, script_error_output.strip()[:255])
                                               )
                             return script_output
                 except Exception as script_exception:

src/opnsense/service/modules/template.py

@@ -28,9 +28,6 @@
     package : configd
     function: template handler, generate configuration files using templates
 """
-
-__author__ = 'Ad Schellevis'
-
 import os
 import os.path
 import syslog
@@ -40,9 +37,10 @@ import codecs
 import jinja2
 import addons.template_helpers
 
+__author__ = 'Ad Schellevis'
 
-class Template(object):
 
+class Template(object):
     def __init__(self, target_root_directory="/"):
         """ constructor
         :return:
@@ -54,11 +52,12 @@ class Template(object):
         self._target_root_directory = target_root_directory
 
         # setup jinja2 environment
-        self._template_dir = os.path.dirname(os.path.abspath(__file__))+'/../templates/'
+        self._template_dir = os.path.dirname(os.path.abspath(__file__)) + '/../templates/'
         self._j2_env = jinja2.Environment(loader=jinja2.FileSystemLoader(self._template_dir), trim_blocks=True,
-                                          extensions=["jinja2.ext.do",])
+                                          extensions=["jinja2.ext.do", ])
 
-    def _read_manifest(self, filename):
+    @staticmethod
+    def _read_manifest(filename):
         """
 
         :param filename: manifest filename (path/+MANIFEST)
@@ -72,7 +71,8 @@ class Template(object):
 
         return result
 
-    def _read_targets(self, filename):
+    @staticmethod
+    def _read_targets(filename):
         """ read raw target filename masks
 
         :param filename: targets filename (path/+TARGETS)
@@ -118,18 +118,19 @@ class Template(object):
 
         return result
 
-    def setConfig(self, config_data):
+    def set_config(self, config_data):
         """ set config data
         :param config_data: config data as dictionary/list structure
         :return: None
         """
-        if type(config_data) in(dict, collections.OrderedDict):
+        if type(config_data) in (dict, collections.OrderedDict):
             self._config = config_data
         else:
             # no data given, reset
             self._config = {}
 
-    def __find_string_tags(self, instr):
+    @staticmethod
+    def __find_string_tags(instr):
         """
         :param instr: string with optional tags [field.$$]
         :return:
@@ -195,7 +196,8 @@ class Template(object):
 
         return result
 
-    def _create_directory(self, filename):
+    @staticmethod
+    def _create_directory(filename):
         """ create directory
         :param filename: create path for filename ( if not existing )
         :return: None
@@ -232,7 +234,7 @@ class Template(object):
                             result_filenames[new_filename] = copy.deepcopy(result_filenames[filename])
                             result_filenames[new_filename][key] = target_filters[target_filter][key]
 
-            template_filename = '%s/%s'%(module_name.replace('.', '/'), src_template)
+            template_filename = '%s/%s' % (module_name.replace('.', '/'), src_template)
             # parse template, make sure issues can be traced back to their origin
             try:
                 j2_page = self._j2_env.get_template(template_filename)
@@ -269,8 +271,8 @@ class Template(object):
                         # it was in the original template.
                         # It looks like Jinja sometimes isn't consistent on placing this last end-of-line in.
                         if len(content) > 1 and content[-1] != '\n':
-                            src_file = '%s%s'%(self._template_dir,template_filename)
-                            src_file_handle = open(src_file,'r')
+                            src_file = '%s%s' % (self._template_dir, template_filename)
+                            src_file_handle = open(src_file, 'r')
                             src_file_handle.seek(-1, os.SEEK_END)
                             last_bytes_template = src_file_handle.read()
                             src_file_handle.close()
@@ -299,7 +301,7 @@ class Template(object):
                 # direct match
                 do_generate = True
             elif wildcard_pos == -1 and len(module_name) < len(template_name) \
-                    and '%s.'%module_name == template_name[0:len(module_name)+1]:
+                    and '%s.' % module_name == template_name[0:len(module_name) + 1]:
                 # match child item
                 do_generate = True