#866 consolidate ConfigParser usage
Closed 4 years ago by tkopecek. Opened 6 years ago by tkopecek.
tkopecek/koji issue865  into  master

file modified
+2 -2
@@ -56,7 +56,7 @@ 

  import zipfile

  import copy

  import Cheetah.Template

- from ConfigParser import ConfigParser

+ from ConfigParser import SafeConfigParser

  from fnmatch import fnmatch

  from gzip import GzipFile

  from optparse import OptionParser, SUPPRESS_HELP
@@ -5610,7 +5610,7 @@ 

          assert False  # pragma: no cover

  

      # load local config

-     config = ConfigParser()

+     config = SafeConfigParser()

      config.read(options.configFile)

      for x in config.sections():

          if x != 'kojid':

file modified
+2 -4
@@ -5645,10 +5645,8 @@ 

          if not os.path.exists(task_options.config):

              parser.error(_("%s not found!" % task_options.config))

          section = 'image-build'

-         config = six.moves.configparser.ConfigParser()

-         conf_fd = open(task_options.config)

-         config.readfp(conf_fd)

-         conf_fd.close()

+         config = six.moves.configparser.SafeConfigParser()

+         config.read(task_options.config)

          if not config.has_section(section):

              parser.error(_("single section called [%s] is required" % section))

          # pluck out the positional arguments first

file modified
+5 -10
@@ -18,7 +18,7 @@ 

  # Authors:

  #       Mike McLean <mikem@redhat.com>

  

- from ConfigParser import RawConfigParser

+ from ConfigParser import SafeConfigParser

  import datetime

  import inspect

  import logging
@@ -381,16 +381,11 @@ 

      """

      logger = logging.getLogger("koji")

      #get our config file(s)

-     cf = environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')

-     cfdir = environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')

-     if cfdir:

-         configs = koji.config_directory_contents(cfdir)

-     else:

-         configs = []

-     if cf and os.path.isfile(cf):

-         configs.append(cf)

+     cfs = [environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')] or []

+     cfdirs = [environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')] or []

+     configs = koji.get_config_files(cfdirs, cfs)

      if configs:

-         config = RawConfigParser()

+         config = SafeConfigParser()

          config.read(configs)

      else:

          config = None

file modified
+37 -26
@@ -1630,6 +1630,34 @@ 

      return configs

  

  

+ def get_config_files(dirs=None, files=None, strict=False):

+     # get list of config files in given directories and files, order is

+     # preserved

+     configs = []

+     if dirs is not None:

+         for d in dirs:

+             cfgs = config_directory_contents(d)

+             if cfgs:

+                 configs.extend(cfgs)

+             else:

+                 logging.debug("No config files found in directory: %s" % d)

+     if files is not None:

+         configs += files

+ 

+     # check access

+     result = []

+     for f in configs:

+         if os.path.isfile(f) and os.access(f, os.F_OK):

+             result.append(f)

+         else:

+             if strict:

+                 raise ConfigurationError("Config file %s can't be opened." % f)

+             else:

+                 logging.warn("Config file %s can't be opened." % f)

+ 

+     return result

+ 

+ 

  def read_config(profile_name, user_config=None):

      config_defaults = {

          'server' : 'http://localhost/kojihub',
@@ -1667,42 +1695,25 @@ 

      #note: later config files override earlier ones

  

      # /etc/koji.conf.d

-     configs = config_directory_contents('/etc/koji.conf.d')

- 

-     # /etc/koji.conf

-     if os.access('/etc/koji.conf', os.F_OK):

-         configs.append('/etc/koji.conf')

+     config_dirs = ['/etc/koji.conf.d']

+     config_files = ['/etc/koji.conf']

  

      # User specific configuration

      if user_config:

          # Config file specified on command line

-         fn = os.path.expanduser(user_config)

-         if os.path.isdir(fn):

-             # Specified config is a directory

-             contents = config_directory_contents(fn)

-             if not contents:

-                 raise ConfigurationError("No config files found in directory: %s" % fn)

-             configs.extend(contents)

-         else:

-             # Specified config is a file

-             if not os.access(fn, os.F_OK):

-                 raise ConfigurationError("No such file: %s" % fn)

-             configs.append(fn)

+         config_dirs.append(os.path.expanduser(user_config))

      else:

          # User config

-         user_config_dir = os.path.expanduser("~/.koji/config.d")

-         configs.extend(config_directory_contents(user_config_dir))

-         fn = os.path.expanduser("~/.koji/config")

-         if os.access(fn, os.F_OK):

-             configs.append(fn)

+         config_dirs.append(os.path.expanduser("~/.koji/config.d"))

+         config_files.append(os.path.expanduser("~/.koji/config"))

+ 

+     configs = get_config_files(dirs=config_dirs, files=config_files)

  

      # Load the configs in a particular order

      got_conf = False

      for configFile in configs:

-         f = open(configFile)

-         config = six.moves.configparser.ConfigParser()

-         config.readfp(f)

-         f.close()

+         config = six.moves.configparser.SafeConfigParser()

+         config.read(configFile)

          if config.has_section(profile_name):

              got_conf = True

              for name, value in config.items(profile_name):

file modified
+3 -5
@@ -667,11 +667,9 @@ 

      """

      if not isinstance(confs, (list, tuple)):

          confs = [confs]

-     config = six.moves.configparser.ConfigParser()

-     for conf in confs:

-         conf_fd = open(conf)

-         config.readfp(conf_fd)

-         conf_fd.close()

+     configs = koji.get_config_files(files=confs)

+     config = six.moves.configparser.SafeConfigParser()

+     config.read(configs)

      builds = {}

      for package in config.sections():

          buildtype = 'maven'

file modified
+1 -2
@@ -270,8 +270,7 @@ 

      global CONFIG

      if not CONFIG:

          conf = ConfigParser.SafeConfigParser()

-         with open(CONFIG_FILE) as conffile:

-             conf.readfp(conffile)

+         conf.read(CONFIG_FILE)

          CONFIG = conf

      urls = CONFIG.get('broker', 'urls').split()

      test_mode = False

@@ -12,7 +12,6 @@ 

  import ConfigParser

  import fnmatch

  import os

- import shutil

  import subprocess

  

  CONFIG_FILE = '/etc/koji-hub/plugins/rpm2maven.conf'

@@ -213,10 +213,10 @@ 

              return self.custom_os_path_exists[filepath]

          return self.os_path_exists(filepath)

  

-     def mock_builtin_open(self, filepath, *args):

+     def mock_builtin_open(self, filepath, *args, **kwargs):

          if filepath in self.custom_open:

              return self.custom_open[filepath]

-         return self.builtin_open(filepath, *args)

+         return self.builtin_open(filepath, *args, **kwargs)

  

      def setUp(self):

          self.os_path_exists = os.path.exists
@@ -307,12 +307,14 @@ 

  """

          self.custom_open[config_file] = six.StringIO(fake_config)

  

-         with mock.patch('os.path.exists', new=self.mock_os_path_exists), \

-                 mock.patch('koji_cli.commands.open', new=self.mock_builtin_open):

-             handle_image_build(

-                 self.options,

-                 self.session,

-                 ['--config', config_file])

+         if six.PY2:

+             with mock.patch('os.path.exists', new=self.mock_os_path_exists), \

+                     mock.patch('__builtin__.open', new=self.mock_builtin_open):

+                 handle_image_build(self.options, self.session, ['--config', config_file])

+         else:

+             with mock.patch('os.path.exists', new=self.mock_os_path_exists), \

+                     mock.patch('builtins.open', new=self.mock_builtin_open):

+                 handle_image_build(self.options, self.session, ['--config', config_file])

  

          args, kwargs = build_image_oz_mock.call_args

          self.assertDictEqual(TASK_OPTIONS, args[1].__dict__)

@@ -40,6 +40,3 @@ 

          return

      else:

          errors[n] = None

- 

- 

- 

file modified
+1 -1
@@ -487,7 +487,7 @@ 

          self.assertEqual(cm.exception.args[0], 'total ordering not possible')

  

      def _read_conf(self, cfile):

-         config = six.moves.configparser.ConfigParser()

+         config = six.moves.configparser.SafeConfigParser()

          path = os.path.dirname(__file__)

          with open(path + cfile, 'r') as conf_file:

              config.readfp(conf_file)

file modified
+5 -12
@@ -114,21 +114,14 @@ 

  

      defaults = parser.get_default_values()

  

-     config = ConfigParser.ConfigParser()

-     cf = getattr(options, 'config_file', None)

-     if cf:

-         if not os.access(cf, os.F_OK):

-             parser.error(_("No such file: %s") % cf)

-             assert False  # pragma: no cover

-     else:

-         cf = '/etc/koji-gc/koji-gc.conf'

-         if not os.access(cf, os.F_OK):

-             cf = None

-     if not cf:

+     config = ConfigParser.SafeConfigParser()

+     cf = getattr(options, 'config_file', '/etc/koji-gc/koji-gc.conf')

+     configs = koji.get_config_files(files=[cf])

+     if not configs:

          print("no config file")

          config = None

      else:

-         config.read(cf)

+         config.read(configs)

          # List of values read from config file to update default parser values

          cfgmap = [

              # name, alias, type

file modified
+5 -12
@@ -160,21 +160,14 @@ 

      (options, args) = parser.parse_args()

  

      defaults = parser.get_default_values()

-     config = ConfigParser.ConfigParser()

-     cf = getattr(options, 'config_file', None)

-     if cf:

-         if not os.access(cf, os.F_OK):

-             parser.error(_("No such file: %s") % cf)

-             assert False  # pragma: no cover

-     else:

-         cf = '/etc/koji-shadow/koji-shadow.conf'

-         if not os.access(cf, os.F_OK):

-             cf = None

-     if not cf:

+     config = ConfigParser.SafeConfigParser()

+     cf = getattr(options, 'config_file', '/etc/koji-shadow/koji-shadow.conf')

+     configs = koji.get_config_files(files=[cf])

+     if not configs:

          log("no config file")

          config = None

      else:

-         config.read(cf)

+         config.read(configs)

          #allow config file to update defaults

          for opt in parser.option_list:

              if not opt.dest:

file modified
+2 -2
@@ -25,7 +25,7 @@ 

  import koji

  from koji.util import rmtree, parseStatus

  from optparse import OptionParser

- from ConfigParser import ConfigParser

+ from ConfigParser import SafeConfigParser

  import errno

  import logging

  import logging.handlers
@@ -811,7 +811,7 @@ 

      parser.add_option("--logfile", help="Specify logfile")

      (options, args) = parser.parse_args()

  

-     config = ConfigParser()

+     config = SafeConfigParser()

      config.read(options.configFile)

      section = 'kojira'

      for x in config.sections():

file modified
+2 -2
@@ -27,7 +27,7 @@ 

  # in a cygwin shell.

  

  from optparse import OptionParser

- from ConfigParser import ConfigParser

+ from ConfigParser import SafeConfigParser

  import os

  import subprocess

  import sys
@@ -201,7 +201,7 @@ 

          elif len(specfiles) > 1:

              raise BuildError('Multiple .ini files found')

  

-         conf = ConfigParser()

+         conf = SafeConfigParser()

          conf.read(os.path.join(self.spec_dir, specfiles[0]))

  

          # [naming] section

file modified
+2 -2
@@ -42,7 +42,7 @@ 

  import pwd

  import requests

  import fnmatch

- from ConfigParser import ConfigParser

+ from ConfigParser import SafeConfigParser

  from contextlib import closing

  from optparse import OptionParser

  try:
@@ -98,7 +98,7 @@ 

          assert False  # pragma: no cover

  

      # load local config

-     config = ConfigParser()

+     config = SafeConfigParser()

      config.read(options.configFile)

      for x in config.sections():

          if x != 'kojivmd':

file modified
+5 -10
@@ -29,7 +29,7 @@ 

  import sys

  import traceback

  

- from ConfigParser import RawConfigParser

+ from ConfigParser import SafeConfigParser

  from koji.server import ServerError, ServerRedirect

  from koji.util import dslice

  
@@ -123,16 +123,11 @@ 

              - all PythonOptions (except koji.web.ConfigFile) are now deprecated and

                support for them will disappear in a future version of Koji

          """

-         cf = environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')

-         cfdir = environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')

-         if cfdir:

-             configs = koji.config_directory_contents(cfdir)

-         else:

-             configs = []

-         if cf and os.path.isfile(cf):

-             configs.append(cf)

+         cfs = [environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')] or []

+         cfdirs = [environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')] or []]

+         configs = koji.get_config_files(dirs=cfdirs, files=cfs)

          if configs:

-             config = RawConfigParser()

+             config = SafeConfigParser()

              config.read(configs)

          else:

              raise koji.GenericError("Configuration missing")

rebased onto 6c953a4

6 years ago

Is there a specific reason, why RawConfigParser was used in kojixmlrpc.py? Other places seems to be ok with SafeConfigParser, but not sure if there was reason here to be more strict.

The other config parsers support a substitution syntax that we do not use, and that can break certain values in the config (notably LogFormat settings)

So, do you think it is ok to leave RawconfigParser in kojixmlrpc and all other change to SafeConfigParser?

Pull-Request has been closed by tkopecek

4 years ago