#213 Use python-requests
Merged 7 years ago by mikem. Opened 7 years ago by mikem.
https://github.com/mikem23/koji-playground.git use-requests-2  into  master

unit tests for new_session()
Mike McLean • 7 years ago  
add test for ClientSession.serverPrincipal
Mike McLean • 7 years ago  
more compatrequests unit tests
Mike McLean • 7 years ago  
still more tests
Mike McLean • 7 years ago  
uncovered import exception
Mike McLean • 7 years ago  
more unit tests for compatrequests
Mike McLean • 7 years ago  
some unit tests for koji.compatrequests
Mike McLean • 7 years ago  
more smarts for ssl bug workaround
Mike McLean • 7 years ago  
ssl hack 2
Mike McLean • 7 years ago  
force a new session in ssl_login
Mike McLean • 7 years ago  
allow setting more session options in kojivmd
Mike McLean • 7 years ago  
allow setting more session options in kojid config
Mike McLean • 7 years ago  
allow setting more session options in kojira
Mike McLean • 7 years ago  
let koji-gc config set more session options
Mike McLean • 7 years ago  
use grab_session_options everywhere
Mike McLean • 7 years ago  
fix return
Mike McLean • 7 years ago  
koji-gc: remove --network-hack, use grab_session_options
Mike McLean • 7 years ago  
grab_session_options function
Mike McLean • 7 years ago  
force https when using ssl auth
Mike McLean • 7 years ago  
avoid tickling race condition with common keep alive timeout setting
Mike McLean • 7 years ago  
update connection error retry code
Mike McLean • 7 years ago  
honor verify in compatrequests
Mike McLean • 7 years ago  
avoid with statement for now
Mike McLean • 7 years ago  
suppress warnings in requests
Mike McLean • 7 years ago  
no_ssl_verify session opt
Mike McLean • 7 years ago  
only use default certs if they exist
Mike McLean • 7 years ago  
always pass serverca
Mike McLean • 7 years ago  
move cert date directly into client session opts
Mike McLean • 7 years ago  
fix post handler
Mike McLean • 7 years ago  
use compatrequests if requests no present
Mike McLean • 7 years ago  
add config for use_old_ssl. make new session on errors
Mike McLean • 7 years ago  
use correct port
Mike McLean • 7 years ago  
trying to fix auth
Mike McLean • 7 years ago  
compat mode works anonymously
Mike McLean • 7 years ago  
don't assume content type is given
Mike McLean • 7 years ago  
working?
Mike McLean • 7 years ago  
More work on compatrequests
Mike McLean • 7 years ago  
stub for compatrequests
Mike McLean • 7 years ago  
stop mocking removed function in tests
Mike McLean • 7 years ago  
fix handler url for upload
Mike McLean • 7 years ago  
honor debug_xmlrpc
Mike McLean • 7 years ago  
move is_cert_error(); require python-requests
Mike McLean • 7 years ago  
ssl auth working
Mike McLean • 7 years ago  
parse host from baseurl in _serverPrincipal()
Mike McLean • 7 years ago  
pull requests bits from py3 work
Mike McLean • 7 years ago  
builder/kojid
file modified
+4 -10
@@ -4974,6 +4974,8 @@

                  'offline_retry_interval': 120,

                  'keepalive' : True,

                  'timeout' : None,

+                 'use_old_ssl' : False,

+                 'no_ssl_verify' : False,

                  'use_fast_upload': True,

                  'use_createrepo_c': False,

                  'createrepo_skip_stat': True,
@@ -5001,7 +5003,7 @@

              elif name in ['offline_retry', 'use_createrepo_c',  'createrepo_skip_stat',

                            'createrepo_update', 'keepalive', 'use_fast_upload',

                            'support_rpm_source_layout', 'krb_rdns',

-                           'build_arch_can_fail']:

+                           'build_arch_can_fail', 'use_old_ssl', 'no_ssl_verify']:

                  defaults[name] = config.getboolean('kojid', name)

              elif name in ['plugin', 'plugins']:

                  defaults['plugin'] = value.split()
@@ -5074,16 +5076,8 @@

      if options.admin_emails:

          koji.add_mail_logger("koji", options.admin_emails)

  

-     #build session options

-     session_opts = {}

-     for k in ('user', 'password', 'krbservice', 'krb_rdns', 'debug_xmlrpc', 'debug',

-               'retry_interval', 'max_retries', 'offline_retry', 'offline_retry_interval',

-               'keepalive', 'timeout', 'use_fast_upload',

-              ):

-         v = getattr(options, k, None)

-         if v is not None:

-             session_opts[k] = v

      #start a session and login

+     session_opts = koji.grab_session_options(options)

      session = koji.ClientSession(options.server, session_opts)

      if os.path.isfile(options.cert):

          try:

cli/koji
file modified
+3 -10
@@ -4360,7 +4360,7 @@

          if not options.watch:

              break

          else:

-             time.sleep(5)

+             time.sleep(options.poll_interval)

              # repeat query for later events

              if last_event:

                  kwargs['afterEvent'] = last_event
@@ -7171,15 +7171,8 @@

      else:

          logger.setLevel(logging.WARN)

  

-     session_opts = {}

-     for k in ('user', 'password', 'krbservice', 'debug_xmlrpc', 'debug', 'max_retries',

-               'retry_interval', 'offline_retry', 'offline_retry_interval',

-               'anon_retry', 'keepalive', 'timeout', 'use_fast_upload',

-               'upload_blocksize', 'krb_rdns'):

-         value = getattr(options,k)

-         if value is not None:

-             session_opts[k] = value

-     session = koji.ClientSession(options.server,session_opts)

+     session_opts = koji.grab_session_options(options)

+     session = koji.ClientSession(options.server, session_opts)

      rv = 0

      try:

          rv = locals()[command].__call__(options, session, args)

hub/kojixmlrpc.py
file modified
+1 -1
@@ -750,7 +750,7 @@

              except Exception:

                  return offline_reply(start_response, msg="database outage")

              h = ModXMLRPCRequestHandler(registry)

-             if environ['CONTENT_TYPE'] == 'application/octet-stream':

+             if environ.get('CONTENT_TYPE') == 'application/octet-stream':

                  response = h._wrap_handler(h.handle_upload, environ)

              else:

                  response = h._wrap_handler(h.handle_rpc, environ)

koji.spec
file modified
+1
@@ -28,6 +28,7 @@

  Requires: python-krbV >= 1.0.13

  Requires: rpm-python

  Requires: pyOpenSSL

+ Requires: python-requests

  Requires: python-urlgrabber

  Requires: python-dateutil

  BuildRequires: python

koji/__init__.py
file modified
+228 -129
@@ -38,21 +38,29 @@

  import logging

  import logging.handlers

  from koji.util import md5_constructor

+ SSL_Error = None

+ try:

+     from OpenSSL.SSL import Error as SSL_Error

+ except Exception:  #pragma: no cover

+     # the hub imports koji, and sometimes this import fails there

+     # see: https://cryptography.io/en/latest/faq/#starting-cryptography-using-mod-wsgi-produces-an-internalerror-during-a-call-in-register-osrandom-engine

+     # unfortunately the workaround at the above link does not always work, so

+     # we ignore it here

+     pass

  import optparse

  import os

  import os.path

  import pwd

  import random

  import re

+ try:

+     import requests

+ except ImportError:  #pragma: no cover

+     requests = None

  import rpm

  import shutil

  import signal

  import socket

- import ssl.SSLCommon

- try:

-     from ssl import ssl as pyssl

- except ImportError:  # pragma: no cover

-     pass

  import struct

  import tempfile

  import time
@@ -61,6 +69,7 @@

  import urllib2

  import urlparse

  import util

+ import warnings

  import xmlrpclib

  import xml.sax

  import xml.sax.handler
@@ -1562,16 +1571,17 @@

          'anon_retry' : None,

          'offline_retry' : None,

          'offline_retry_interval' : None,

+         'use_old_ssl' : False,

          'keepalive' : True,

          'timeout' : None,

          'use_fast_upload': False,

          'upload_blocksize': 1048576,

-         'poll_interval': 5,

+         'poll_interval': 6,

          'krbservice': 'host',

          'krb_rdns': True,

-         'cert': '~/.koji/client.crt',

+         'cert': None,

          'ca': '',  # FIXME: remove in next major release

-         'serverca': '~/.koji/serverca.crt',

+         'serverca': None,

          'authtype': None

      }

  
@@ -1623,7 +1633,8 @@

                  #options *can* be set via the config file. Such options should

                  #not have a default value set in the option parser.

                  if result.has_key(name):

-                     if name in ('anon_retry', 'offline_retry', 'keepalive', 'use_fast_upload', 'krb_rdns'):

+                     if name in ('anon_retry', 'offline_retry', 'keepalive',

+                                 'use_fast_upload', 'krb_rdns', 'use_old_ssl'):

                          result[name] = config.getboolean(profile_name, name)

                      elif name in ('max_retries', 'retry_interval',

                                    'offline_retry_interval', 'poll_interval', 'timeout',
@@ -1640,6 +1651,19 @@

          sys.stderr.write("Warning: no configuration for profile name: %s\n" % profile_name)

          sys.stderr.flush()

  

+     # special handling for cert defaults

+     cert_defaults = {

+         'cert': '~/.koji/client.crt',

+         'serverca': '~/.koji/serverca.crt',

+         }

+     for name in cert_defaults:

+         if result.get(name) is None:

+             fn = cert_defaults[name]

+             if os.path.exists(fn):

+                 result[name] = fn

+             else:

+                 result[name] = ''

+ 

      return result

  

  
@@ -1810,6 +1834,76 @@

  

  pathinfo = PathInfo()

  

+ 

+ def is_cert_error(e):

+     """Determine if an OpenSSL error is due to a bad cert"""

+ 

+     if SSL_Error is None:  #pragma: no cover

+         # import failed, so we can't determine

+         raise Exception("OpenSSL library did not load")

+     if not isinstance(e, SSL_Error):

+         return False

+ 

+     # pyOpenSSL doesn't use different exception

+     # subclasses, we have to actually parse the args

+     for arg in e.args:

+         # First, check to see if 'arg' is iterable because

+         # it can be anything..

+         try:

+             iter(arg)

+         except TypeError:

+             continue

+ 

+         # We do all this so that we can detect cert expiry

+         # so we can avoid retrying those over and over.

+         for items in arg:

+             try:

+                 iter(items)

+             except TypeError:

+                 continue

+ 

+             if len(items) != 3:

+                 continue

+ 

+             _, _, ssl_reason = items

+ 

+             if ('certificate revoked' in ssl_reason or

+                     'certificate expired' in ssl_reason):

+                 return True

+ 

+     #otherwise

+     return False

+ 

+ 

+ def is_conn_error(e):

+     """Determine if an error seems to be from a dropped connection"""

+     if isinstance(e, socket.error):

+         if getattr(e, 'errno', None) in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE):

+             return True

+         # else

+         return False

+     if isinstance(e, httplib.BadStatusLine):

+         return True

+     if requests is not None:

+         try:

+             if isinstance(e, requests.exceptions.ConnectionError):

+                 # we see errors like this in keep alive timeout races

+                 # ConnectionError(ProtocolError('Connection aborted.', BadStatusLine("''",)),)

+                 e2 = getattr(e, 'args', [None])[0]

+                 if isinstance(e2, requests.packages.urllib3.exceptions.ProtocolError):

+                     e3 = getattr(e2, 'args', [None, None])[1]

+                     if isinstance(e3, httplib.BadStatusLine):

+                         return True

+                 if isinstance(e2, socket.error):

+                     # same check as unwrapped socket error

+                     if getattr(e, 'errno', None) in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE):

+                         return True

+         except (TypeError, AttributeError):

+             pass

+     # otherwise

+     return False

+ 

+ 

  class VirtualMethod(object):

      # some magic to bind an XML-RPC method to an RPC server.

      # supports "nested" methods (e.g. examples.getStateName)
@@ -1823,6 +1917,39 @@

          return self.__func(self.__name, args, opts)

  

  

+ def grab_session_options(options):

+     '''Convert optparse options to a dict that ClientSession can handle'''

+     s_opts = (

+         'user',

+         'password',

+         'krbservice',

+         'debug_xmlrpc',

+         'debug',

+         'max_retries',

+         'retry_interval',

+         'offline_retry',

+         'offline_retry_interval',

+         'anon_retry',

+         'keepalive',

+         'timeout',

+         'use_fast_upload',

+         'upload_blocksize',

+         'krb_rdns',

+         'use_old_ssl',

+         'no_ssl_verify',

+         'serverca',

+         )

+     # cert is omitted for now

+     ret = {}

+     for key in s_opts:

+         if not hasattr(options, key):

+             continue

+         value = getattr(options, key)

+         if value is not None:

+             ret[key] = value

+     return ret

+ 

+ 

  class ClientSession(object):

  

      def __init__(self, baseurl, opts=None, sinfo=None):
@@ -1833,54 +1960,25 @@

              opts = opts.copy()

          self.baseurl = baseurl

          self.opts = opts

-         self._connection = None

-         self._setup_connection()

          self.authtype = None

          self.setSession(sinfo)

          self.multicall = False

          self._calls = []

          self.logger = logging.getLogger('koji')

- 

-     def _setup_connection(self):

-         uri = urlparse.urlsplit(self.baseurl)

-         scheme = uri[0]

-         self._host, _port = urllib.splitport(uri[1])

-         self.explicit_port = bool(_port)

-         self._path = uri[2]

-         default_port = 80

-         if self.opts.get('certs'):

-             ctx = ssl.SSLCommon.CreateSSLContext(self.opts['certs'])

-             cnxOpts = {'ssl_context' : ctx}

-             cnxClass = ssl.SSLCommon.PlgHTTPSConnection

-             default_port = 443

-         elif scheme == 'https':

-             cnxOpts = {}

-             if sys.version_info[:3] >= (2, 7, 9):

-                 #ctx = pyssl.SSLContext(pyssl.PROTOCOL_SSLv23)

-                 ctx = pyssl._create_unverified_context()

-                 # TODO - we should default to verifying where possible

-                 cnxOpts['context'] = ctx

-             cnxClass = httplib.HTTPSConnection

-             default_port = 443

-         elif scheme == 'http':

-             cnxOpts = {}

-             cnxClass = httplib.HTTPConnection

+         self.rsession = None

+         self.new_session()

+         self.opts.setdefault('timeout',  60 * 60 * 12)

+ 

+ 

+     def new_session(self):

+         self.logger.debug("Opening new requests session")

+         if self.rsession:

+             self.rsession.close()

+         if self.opts.get('use_old_ssl', False) or requests is None:

+             import koji.compatrequests

+             self.rsession = koji.compatrequests.Session()

          else:

-             raise IOError, "unsupported XML-RPC protocol"

-         # set a default 12 hour connection timeout.

-         # Some Koji operations can take a long time to return, but after 12

-         # hours we can assume something is seriously wrong.

-         timeout = self.opts.setdefault('timeout', 60 * 60 * 12)

-         self._timeout_compat = False

-         if timeout:

-             if sys.version_info[:3] < (2, 6, 0) and 'ssl_context' not in cnxOpts:

-                 self._timeout_compat = True

-             else:

-                 cnxOpts['timeout'] = timeout

-         self._port = (_port and int(_port) or default_port)

-         self._cnxOpts = cnxOpts

-         self._cnxClass = cnxClass

-         self._close_connection()

+             self.rsession = requests.Session()

  

      def setSession(self, sinfo):

          """Set the session info
@@ -1890,7 +1988,6 @@

              self.logged_in = False

              self.callnum = None

              # do we need to do anything else here?

-             self._setup_connection()

              self.authtype = None

          else:

              self.logged_in = True
@@ -1985,44 +2082,51 @@

      def _serverPrincipal(self, cprinc):

          """Get the Kerberos principal of the server we're connecting

          to, based on baseurl."""

+ 

+         uri = urlparse.urlsplit(self.baseurl)

+         host, port = urllib.splitport(uri[1])

          if self.opts.get('krb_rdns', True):

-             servername = socket.getfqdn(self._host)

+             servername = socket.getfqdn(host)

          else:

-             servername = self._host

-         #portspec = servername.find(':')

-         #if portspec != -1:

-         #    servername = servername[:portspec]

+             servername = host

          realm = cprinc.realm

          service = self.opts.get('krbservice', 'host')

  

          return '%s/%s@%s' % (service, servername, realm)

  

-     def ssl_login(self, cert, ca, serverca, proxyuser=None):

-         certs = {}

-         certs['key_and_cert'] = cert

-         certs['peer_ca_cert'] = serverca

+     def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None):

+         cert = cert or self.opts.get('cert')

+         serverca = serverca or self.opts.get('serverca')

+         if cert is None:

+             raise AuthError('No certification provided')

+         if serverca is None:

+             raise AuthError('No server CA provided')

          # FIXME: ca is not useful here and therefore ignored, can be removed

          # when API is changed

  

-         ctx = ssl.SSLCommon.CreateSSLContext(certs)

-         self._cnxOpts = {'ssl_context' : ctx}

+         # force https

+         uri = urlparse.urlsplit(self.baseurl)

+         if uri[0] != 'https':

+             self.baseurl = 'https://%s%s' % (uri[1], uri[2])

+ 

+         # Force a new session

+         self.new_session()

+ 

          # 60 second timeout during login

-         old_timeout = self._cnxOpts.get('timeout')

-         self._cnxOpts['timeout'] = 60

+         old_opts = self.opts

+         self.opts = old_opts.copy()

+         self.opts['timeout'] = 60

+         self.opts['cert'] = cert

+         self.opts['serverca'] = serverca

          try:

-             self._cnxClass = ssl.SSLCommon.PlgHTTPSConnection

-             if self._port == 80 and not self.explicit_port:

-                 self._port = 443

              sinfo = self.callMethod('sslLogin', proxyuser)

          finally:

-             if old_timeout is None:

-                 del self._cnxOpts['timeout']

-             else:

-                 self._cnxOpts['timeout'] = old_timeout

+             self.opts = old_opts

          if not sinfo:

              raise AuthError, 'unable to obtain a session'

  

-         self.opts['certs'] = certs

+         self.opts['cert'] = cert

+         self.opts['serverca'] = serverca

          self.setSession(sinfo)

  

          self.authtype = AUTHTYPE_SSL
@@ -2076,11 +2180,11 @@

              sinfo = self.sinfo.copy()

              sinfo['callnum'] = self.callnum

              self.callnum += 1

-             handler = "%s?%s" % (self._path, urllib.urlencode(sinfo))

+             handler = "%s?%s" % (self.baseurl, urllib.urlencode(sinfo))

          elif name == 'sslLogin':

-             handler = self._path + '/ssllogin'

+             handler = self.baseurl + '/ssllogin'

          else:

-             handler = self._path

+             handler = self.baseurl

          request = dumps(args, name, allow_none=1)

          headers = [

              # connection class handles Host
@@ -2095,65 +2199,60 @@

          for i in (0, 1):

              try:

                  return self._sendOneCall(handler, headers, request)

-             except socket.error, e:

-                 self._close_connection()

-                 if i or getattr(e, 'errno', None) not in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE):

+             except Exception, e:

+                 if i or not is_conn_error(e):

                      raise

-             except httplib.BadStatusLine:

-                 self._close_connection()

-                 if i:

-                     raise

- 

+                 self.logger.debug("Connection Error: %s", e)

+                 self.new_session()

  

      def _sendOneCall(self, handler, headers, request):

-         cnx = self._get_connection()

+         headers = dict(headers)

+         callopts = {

+             'headers': headers,

+             'data': request,

+             'stream': True,

+         }

+         verify = self.opts.get('serverca')

+         if verify:

+             callopts['verify'] = verify

+         elif self.opts.get('no_ssl_verify'):

+             callopts['verify'] = False

+             # XXX - not great, but this is the previous behavior

+         cert = self.opts.get('cert')

+         if cert:

+             # TODO: we really only need to do this for ssllogin calls

+             callopts['cert'] = cert

+         timeout = self.opts.get('timeout')

+         if timeout:

+             callopts['timeout'] = timeout

          if self.opts.get('debug_xmlrpc', False):

-             cnx.set_debuglevel(1)

-         cnx.putrequest('POST', handler)

-         for n, v in headers:

-             cnx.putheader(n, v)

-         cnx.endheaders()

-         cnx.send(request)

-         response = cnx.getresponse()

+             print "url: %s" % handler

+             for _key in callopts:

+                 _val = callopts[_key]

+                 if _key == 'data' and len(_val) > 1024:

+                     _val = _val[:1024] + '...'

+                 print "%s: %r" % (_key, _val)

+         catcher = None

+         if hasattr(warnings, 'catch_warnings'):

+             # TODO: convert to a with statement when we drop 2.4.3 support

+             catcher = warnings.catch_warnings()

+             catcher.__enter__()

          try:

-             ret = self._read_xmlrpc_response(response, handler)

+             if catcher:

+                 warnings.simplefilter("ignore")

+             r = self.rsession.post(handler, **callopts)

+             try:

+                 ret = self._read_xmlrpc_response(r)

+             finally:

+                 r.close()

          finally:

-             response.close()

+             if catcher:

+                 catcher.__exit__()

          return ret

  

-     def _get_connection(self):

-         key = (self._cnxClass, self._host, self._port)

-         if self._connection and self.opts.get('keepalive'):

-             if key == self._connection[0]:

-                 cnx = self._connection[1]

-                 if getattr(cnx, 'sock', None):

-                     return cnx

-         cnx = self._cnxClass(self._host, self._port, **self._cnxOpts)

-         self._connection = (key, cnx)

-         if self._timeout_compat:

-             # in python < 2.6 httplib does not support the timeout option

-             # but socket supports it since 2.3

-             cnx.connect()

-             cnx.sock.settimeout(self.opts['timeout'])

-         return cnx

- 

-     def _close_connection(self):

-         if self._connection:

-             self._connection[1].close()

-             self._connection = None

- 

-     def _read_xmlrpc_response(self, response, handler=''):

-         #XXX honor debug_xmlrpc

-         if response.status != 200:

-             if (response.getheader("content-length", 0)):

-                 response.read()

-             raise xmlrpclib.ProtocolError(self._host + handler,

-                         response.status, response.reason, response.msg)

+     def _read_xmlrpc_response(self, response):

          p, u = xmlrpclib.getparser()

-         while True:

-             chunk = response.read(8192)

-             if not chunk:

-                 break

+         for chunk in response.iter_content(8192):

              if self.opts.get('debug_xmlrpc', False):

                  print "body: %r" % chunk

              p.feed(chunk)
@@ -2207,9 +2306,9 @@

                      raise

                  except Exception, e:

                      tb_str = ''.join(traceback.format_exception(*sys.exc_info()))

-                     self._close_connection()

+                     self.new_session()

  

-                     if ssl.SSLCommon.is_cert_error(e):

+                     if is_cert_error(e):

                          # There's no point in retrying for this

                          raise

  
@@ -2333,7 +2432,7 @@

              args['overwrite'] = "1"

          size = len(chunk)

          self.callnum += 1

-         handler = "%s?%s" % (self._path, urllib.urlencode(args))

+         handler = "%s?%s" % (self.baseurl, urllib.urlencode(args))

          headers = [

              ('User-Agent', 'koji/1.7'),  #XXX

              ("Content-Type", "application/octet-stream"),

koji/compatrequests.py
file added
+142
@@ -0,0 +1,142 @@

+ """

+ koji.compatrequests

+ ~~~~~~~~~~~~~~~~~~~

+ 

+ This module contains a *very limited* partial implemention of the requests

+ module that is based on the older codepaths in koji. It only provides

+ the bits that koji needs.

+ """

+ 

+ import httplib

+ import urlparse

+ import urllib

+ import sys

+ import ssl.SSLCommon

+ try:

+     from ssl import ssl as pyssl

+ except ImportError:  # pragma: no cover

+     pass

+ 

+ 

+ class Session(object):

+ 

+     def __init__(self):

+         self.connection = None

+ 

+     def post(self, url, data=None, headers=None, stream=None, verify=None,

+                 cert=None, timeout=None):

+         uri = urlparse.urlsplit(url)

+         if uri[3]:

+             handler = "%s?%s" % (uri[2], uri[3])

+         else:

+             handler = uri[2]

+         cnx = self.get_connection(uri, cert, verify, timeout)

+         #cnx.set_debuglevel(1)

+         cnx.putrequest('POST', handler)

+         if headers:

+             for k in headers:

+                 cnx.putheader(k, headers[k])

+         cnx.endheaders()

+         if data is not None:

+             cnx.send(data)

+         response = cnx.getresponse()

+         return Response(self, response)

+ 

+     def get_connection(self, uri, cert, verify, timeout):

+         scheme = uri[0]

+         host, port = urllib.splitport(uri[1])

+         key = (scheme, host, cert, verify, timeout)

+         #if self.connection and self.opts.get('keepalive'):

+         if self.connection:   # XXX honor keepalive

+             if key == self.connection[0]:

+                 cnx = self.connection[1]

+                 if getattr(cnx, 'sock', None):

+                     return cnx

+         # Otherwise we make a new one

+         default_port = 80

+         certs = {}

+         if isinstance(verify, basestring):

+             certs['peer_ca_cert'] = verify

+         if cert:

+             certs['key_and_cert'] = cert

+             ctx = ssl.SSLCommon.CreateSSLContext(certs)

+             cnxOpts = {'ssl_context' : ctx}

+             cnxClass = ssl.SSLCommon.PlgHTTPSConnection

+             default_port = 443

+         elif scheme == 'https':

+             cnxOpts = {}

+             if verify:

+                 if sys.version_info[:3] >= (2, 7, 9):

+                     try:

+                         proto = pyssl.PROTOCOL_TLS

+                     except AttributeError:

+                         proto = pyssl.PROTOCOL_SSLv23

+                     ctx = pyssl.SSLContext(proto)

+                     ctx.load_verify_locations(cafile=verify)

+                     ctx.verify_mode = pyssl.CERT_REQUIRED

+                     cnxOpts['context'] = ctx

+                 else:

+                     cnxOpts['cert_file'] = verify

+             elif verify is None:

+                 # not specified, leave as default

+                 pass

+             elif sys.version_info[:3] >= (2, 7, 9):

+                 # no verify

+                 ctx = pyssl._create_unverified_context()

+                 cnxOpts['context'] = ctx

+             cnxClass = httplib.HTTPSConnection

+             default_port = 443

+         elif scheme == 'http':

+             cnxOpts = {}

+             cnxClass = httplib.HTTPConnection

+         else:

+             raise IOError, "unsupported protocol: %s" % scheme

+ 

+         timeout_compat = False

+         if timeout:

+             if sys.version_info[:3] < (2, 6, 0) and 'ssl_context' not in cnxOpts:

+                 timeout_compat = True

+             else:

+                 cnxOpts['timeout'] = timeout

+         # no need to close connection

+         port = (port and int(port) or default_port)

+         cnx = cnxClass(host, port, **cnxOpts)

+         self.connection = (key, cnx)

+         if timeout_compat:

+             # in python < 2.6 httplib does not support the timeout option

+             # but socket supports it since 2.3

+             cnx.connect()

+             cnx.sock.settimeout(timeout)

+         return cnx

+ 

+     def close(self):

+         if self.connection:

+             self.connection[1].close()

+             self.connection = None

+ 

+ 

+ class Response(object):

+ 

+     def __init__(self, session, response):

+         self.session = session

+         self.response = response

+ 

+     def iter_content(self, blocksize=8192):

+         # should we check this in Session.post()?

+         # should we even check this here?

+         if self.response.status != 200:

+             if (self.response.getheader("content-length", 0)):

+                 self.response.read()

+             # XXX wrong exception

+             raise Exception("Server status: %s" % self.response.status)

+         while True:

+             chunk = self.response.read(blocksize)

+             if not chunk:

+                 break

+             yield chunk

+ 

+     def close(self):

+         self.response.close()

+ 

+ 

+ 

koji/ssl/SSLCommon.py
file modified
-37
@@ -28,43 +28,6 @@

      return preverifyOK

  

  

- def is_cert_error(e):

-     """Determine if an OpenSSL error is due to a bad cert"""

- 

-     if not isinstance(e, SSL.Error):

-         return False

- 

-     # pyOpenSSL doesn't use different exception

-     # subclasses, we have to actually parse the args

-     for arg in e.args:

-         # First, check to see if 'arg' is iterable because

-         # it can be anything..

-         try:

-             iter(arg)

-         except TypeError:

-             continue

- 

-         # We do all this so that we can detect cert expiry

-         # so we can avoid retrying those over and over.

-         for items in arg:

-             try:

-                 iter(items)

-             except TypeError:

-                 continue

- 

-             if len(items) != 3:

-                 continue

- 

-             _, _, ssl_reason = items

- 

-             if ('certificate revoked' in ssl_reason or

-                     'certificate expired' in ssl_reason):

-                 return True

- 

-     #otherwise

-     return False

- 

- 

  def CreateSSLContext(certs):

      key_and_cert = certs['key_and_cert']

      peer_ca_cert = certs['peer_ca_cert']

tests/test_client_session.py
file added
+65
@@ -0,0 +1,65 @@

+ import mock

+ import unittest

+ 

+ import koji

+ 

+ 

+ class TestClientSession(unittest.TestCase):

+ 

+     @mock.patch('socket.getfqdn')

+     def test_server_principal_rdns(self, getfqdn):

+         opts = {'krb_rdns': True}

+         session = koji.ClientSession('http://koji.example.com/kojihub', opts)

+         cprinc = mock.MagicMock()

+         cprinc.realm = "REALM"

+         getfqdn.return_value = 'koji02.example.com'

+ 

+         princ = session._serverPrincipal(cprinc)

+         self.assertEqual(princ, 'host/koji02.example.com@REALM')

+         getfqdn.assert_called_with('koji.example.com')

+ 

+     @mock.patch('socket.getfqdn')

+     def test_server_principal_no_rdns(self, getfqdn):

+         opts = {'krb_rdns': False}

+         session = koji.ClientSession('http://koji.example.com/kojihub', opts)

+         cprinc = mock.MagicMock()

+         cprinc.realm = "REALM"

+         getfqdn.return_value = 'koji02.example.com'

+ 

+         princ = session._serverPrincipal(cprinc)

+         self.assertEqual(princ, 'host/koji.example.com@REALM')

+         getfqdn.assert_not_called()

+ 

+     @mock.patch('koji.compatrequests.Session')

+     @mock.patch('requests.Session')

+     def test_new_session(self, rsession, compat_session):

+         opts = {'use_old_ssl': False}

+         ksession = koji.ClientSession('http://koji.example.com/kojihub', opts)

+ 

+         # init should have called new_session for us

+ 

+         rsession.assert_called_once()

+         compat_session.assert_not_called()

+ 

+     @mock.patch('koji.compatrequests.Session')

+     @mock.patch('requests.Session')

+     def test_new_session_old(self, rsession, compat_session):

+         opts = {'use_old_ssl': True}

+         ksession = koji.ClientSession('http://koji.example.com/kojihub', opts)

+ 

+         # init should have called new_session for us

+ 

+         rsession.assert_not_called()

+         compat_session.assert_called_once()

+ 

+     @mock.patch('koji.compatrequests.Session')

+     @mock.patch('requests.Session')

+     def test_new_session_close(self, rsession, compat_session):

+         opts = {'use_old_ssl': True}

+         ksession = koji.ClientSession('http://koji.example.com/kojihub', opts)

+         my_rsession = mock.MagicMock()

+         ksession.rsession = my_rsession

+ 

+         ksession.new_session()

+         my_rsession.close.assert_called()

+         self.assertNotEqual(ksession.rsession, my_rsession)

tests/test_compatrequests.py
file added
+259
@@ -0,0 +1,259 @@

+ import mock

+ import unittest

+ import urlparse

+ 

+ import koji.compatrequests

+ 

+ 

+ class TestResponse(unittest.TestCase):

+ 

+     def setUp(self):

+         session = mock.MagicMock()

+         response = mock.MagicMock()

+         self.response = koji.compatrequests.Response(session, response)

+ 

+     def tearDown(self):

+         del self.response

+ 

+     def test_read(self):

+         self.response.response.status = 200

+         data = [

+             "Here's some data",

+             "Here's some mooore data",

+             "And look!",

+             "Here's a nice block of lorem text",

+             "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do "

+             "eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut "

+             "enim ad minim veniam, quis nostrud exercitation ullamco laboris "

+             "nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor "

+             "in reprehenderit in voluptate velit esse cillum dolore eu fugiat "

+             "nulla pariatur. Excepteur sint occaecat cupidatat non proident, "

+             "sunt in culpa qui officia deserunt mollit anim id est laborum.",

+             "",  #eof

+         ]

+         self.response.response.read.side_effect = data

+ 

+         result = list(self.response.iter_content(blocksize=10240))

+ 

+         self.assertEqual(result, data[:-1])

+         rcalls = [mock.call(10240) for s in data]

+         self.response.response.read.assert_has_calls(rcalls)

+ 

+         self.response.close()

+         self.response.response.close.assert_called_once()

+ 

+     def test_error(self):

+         self.response.response.status = 404

+         self.response.response.getheader.return_value = 0

+         with self.assertRaises(Exception):

+             list(self.response.iter_content())

+         self.response.response.read.assert_not_called()

+ 

+         self.response.response.status = 404

+         self.response.response.getheader.return_value = 42

+         with self.assertRaises(Exception):

+             list(self.response.iter_content())

+         self.response.response.read.assert_called_once()

+ 

+ 

+ class TestSessionPost(unittest.TestCase):

+ 

+     def test_simple(self):

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain.org/KOJIHUB'

+         cnx = mock.MagicMock()

+         session.get_connection = mock.MagicMock()

+         session.get_connection.return_value = cnx

+         response = mock.MagicMock()

+         cnx.getresponse.return_value = response

+ 

+         ret  = session.post(url, data="data", headers={"foo": "bar"})

+         cnx.putrequest.assert_called_once_with('POST', '/KOJIHUB')

+         cnx.putheader.assert_called_once_with('foo', 'bar')

+         cnx.send.assert_called_once_with("data")

+         self.assertEqual(ret.response, response)

+ 

+     def test_less_simple(self):

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain.org/KOJIHUB?a=1&b=2'

+         cnx = mock.MagicMock()

+         session.get_connection = mock.MagicMock()

+         session.get_connection.return_value = cnx

+         response = mock.MagicMock()

+         cnx.getresponse.return_value = response

+ 

+         ret  = session.post(url, data="data", headers={"foo": "bar"},

+                 cert="cert", verify="verify", stream=True, timeout=1701)

+         cnx.putrequest.assert_called_once_with('POST', '/KOJIHUB?a=1&b=2')

+         cnx.putheader.assert_called_once_with('foo', 'bar')

+         cnx.send.assert_called_once_with("data")

+         self.assertEqual(ret.response, response)

+ 

+ 

+ class TestSessionConnection(unittest.TestCase):

+ 

+     @mock.patch('httplib.HTTPConnection')

+     def test_http(self, HTTPConnection):

+         # no cert, no verify, no timeout

+         session = koji.compatrequests.Session()

+         url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+ 

+         cnx = session.get_connection(uri, None, None, None)

+         HTTPConnection.assert_called_once_with('www.fakedomain234234.org', 80)

+         key = ('http', 'www.fakedomain234234.org', None, None, None)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+         # and close it

+         session.close()

+         self.assertEqual(session.connection, None)

+         cnx.close.assert_called_with()

+ 

+         # double close should not error

+         session.close()

+ 

+     def test_cached(self):

+         session = koji.compatrequests.Session()

+         url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         key = ('http', 'www.fakedomain234234.org', None, None, None)

+         cnx = mock.MagicMock()

+         session.connection = (key, cnx)

+ 

+         ret = session.get_connection(uri, None, None, None)

+         self.assertEqual(ret, cnx)

+ 

+     def test_badproto(self):

+         session = koji.compatrequests.Session()

+         url = 'nosuchproto://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+ 

+         with self.assertRaises(IOError):

+             ret = session.get_connection(uri, None, None, None)

+ 

+     @mock.patch('httplib.HTTPConnection')

+     @mock.patch('sys.version_info', new=(2,7,12,'final', 0))

+     def test_timeout(self, HTTPConnection):

+         # no cert, no verify

+         session = koji.compatrequests.Session()

+         url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         timeout = 1701

+ 

+         cnx = session.get_connection(uri, None, None, 1701)

+         HTTPConnection.assert_called_once_with('www.fakedomain234234.org', 80, timeout=1701)

+         key = ('http', 'www.fakedomain234234.org', None, None, 1701)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+     @mock.patch('httplib.HTTPConnection')

+     @mock.patch('sys.version_info', new=(2, 4, 3, 'final', 0))

+     def test_timeout_compat(self, HTTPConnection):

+         # no cert, no verify

+         session = koji.compatrequests.Session()

+         url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         timeout = 1701

+ 

+         cnx = session.get_connection(uri, None, None, 1701)

+         HTTPConnection.assert_called_once_with('www.fakedomain234234.org', 80)

+         key = ('http', 'www.fakedomain234234.org', None, None, 1701)

+         self.assertEqual(session.connection, (key, cnx))

+         cnx.connect.assert_called_once()

+         cnx.sock.settimeout.assert_called_with(1701)

+ 

+     @mock.patch('httplib.HTTPSConnection')

+     def test_https(self, HTTPSConnection):

+         # no cert, no verify, no timeout

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+ 

+         cnx = session.get_connection(uri, None, None, None)

+         HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443)

+         key = ('https', 'www.fakedomain234234.org', None, None, None)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+     @mock.patch('koji.ssl.SSLCommon.CreateSSLContext')

+     @mock.patch('koji.ssl.SSLCommon.PlgHTTPSConnection')

+     def test_cert(self, PlgHTTPSConnection, CreateSSLContext):

+         # no verify, no timeout

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         cert = '/path/to/cert/file'

+         context = mock.MagicMock()

+         CreateSSLContext.return_value = context

+ 

+         cnx = session.get_connection(uri, cert, None, None)

+         PlgHTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, ssl_context=context)

+         key = ('https', 'www.fakedomain234234.org', cert, None, None)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+     @mock.patch('ssl._create_unverified_context')

+     @mock.patch('httplib.HTTPSConnection')

+     @mock.patch('sys.version_info', new=(2,7,12,'final', 0))

+     def test_unverified(self, HTTPSConnection, create_unverified_context):

+         # no cert, verify=False, no timeout

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         context = mock.MagicMock()

+         create_unverified_context.return_value = context

+ 

+         cnx = session.get_connection(uri, None, False, None)

+         create_unverified_context.assert_called_once()

+         HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, context=context)

+         key = ('https', 'www.fakedomain234234.org', None, False, None)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+     @mock.patch('httplib.HTTPSConnection')

+     @mock.patch('sys.version_info', new=(2, 4, 3, 'final', 0))

+     def test_unverified_compat(self, HTTPSConnection):

+         # no cert, verify=False, no timeout

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+ 

+         cnx = session.get_connection(uri, None, False, None)

+         HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443)

+         key = ('https', 'www.fakedomain234234.org', None, False, None)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+     @mock.patch('ssl._create_unverified_context')

+     @mock.patch('ssl.SSLContext')

+     @mock.patch('httplib.HTTPSConnection')

+     @mock.patch('sys.version_info', new=(2,7,12,'final', 0))

+     def test_verify(self, HTTPSConnection, SSLContext, create_unverified_context):

+         # no cert, no timeout

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         context = mock.MagicMock()

+         SSLContext.return_value = context

+         verify = '/path/to/verify/cert'

+ 

+         cnx = session.get_connection(uri, None, verify, None)

+         create_unverified_context.assert_not_called()

+         SSLContext.assert_called_once()

+         context.load_verify_locations.called_once_with(cafile=verify)

+         HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, context=context)

+         key = ('https', 'www.fakedomain234234.org', None, verify, None)

+         self.assertEqual(session.connection, (key, cnx))

+ 

+     @mock.patch('ssl._create_unverified_context')

+     @mock.patch('ssl.SSLContext')

+     @mock.patch('httplib.HTTPSConnection')

+     @mock.patch('sys.version_info', new=(2, 4, 3, 'final', 0))

+     def test_verify_compat(self, HTTPSConnection, SSLContext, create_unverified_context):

+         # no cert, no timeout

+         session = koji.compatrequests.Session()

+         url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2'

+         uri = urlparse.urlsplit(url)

+         verify = '/path/to/verify/cert'

+ 

+         cnx = session.get_connection(uri, None, verify, None)

+         create_unverified_context.assert_not_called()

+         SSLContext.assert_not_called()

+         HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, cert_file=verify)

+         key = ('https', 'www.fakedomain234234.org', None, verify, None)

+         self.assertEqual(session.connection, (key, cnx))

tests/test_krbv.py
file modified
+1 -2
@@ -9,8 +9,7 @@

  class KrbVTestCase(unittest.TestCase):

  

      @mock.patch('koji.krbV', new=None)

-     @mock.patch('koji.ClientSession._setup_connection')

-     def test_krbv_disabled(self, krbV):

+     def test_krbv_disabled(self):

          """ Test that when krbV is absent, we behave rationally. """

          self.assertEquals(koji.krbV, None)

          session = koji.ClientSession('whatever')

util/koji-gc
file modified
+6 -39
@@ -36,35 +36,6 @@

          sys.exit(2)

      OptionParser.error = _op_error

  

- class MySession(koji.ClientSession):

-     """This is a hack to work around server timeouts"""

- 

-     def _callMethod(self, name, args, kwargs):

-         retries = 10

-         i = 0

-         while True:

-             i += 1

-             try:

-                 return super(MySession, self)._callMethod(name, args, kwargs)

-             except (socket.timeout, socket.error, xmlrpclib.ProtocolError), e:

-                 if i > retries:

-                     raise

-                 else:

-                     print "Socket Error: %s [%i], retrying..." % (e, i)

-                     time.sleep(60)

- 

-     #an even worse hack

-     def multiCall(self):

-         if not self.multicall:

-             raise GenericError, 'ClientSession.multicall must be set to True before calling multiCall()'

-         if len(self._calls) == 0:

-             return []

- 

-         self.multicall = False

-         calls = self._calls

-         self._calls = []

-         return self._callMethod('multiCall', (calls,), {})

- 

  

  def _(args):

      """Stub function for translation"""
@@ -88,7 +59,7 @@

      parser.add_option("--noauth", action="store_true", default=False,

                        help=_("do not authenticate"))

      parser.add_option("--network-hack", action="store_true", default=False,

-                       help=_("enable hackish workaround for broken networks"))

+                       help=optparse.SUPPRESS_HELP)  # no longer used

      parser.add_option("--cert", default='/etc/koji-gc/client.crt',

                        help=_("Client SSL certificate file for authentication"))

      parser.add_option("--ca", default='',
@@ -178,6 +149,9 @@

              ['unprotected_keys', None, 'string'],

              ['grace_period', None, 'string'],

              ['trashcan_tag', None, 'string'],

+             ['use_old_ssl', None, 'boolean'],

+             ['no_ssl_verify', None, 'boolean'],

+             ['timeout', None, 'integer'],

              ]

          for name, alias, type in cfgmap:

              if alias is None:
@@ -926,15 +900,8 @@

  

      options, args = get_options()

  

-     session_opts = {}

-     for k in ('user', 'password', 'krbservice', 'krb_rdns', 'email_domain',

-                 'debug_xmlrpc', 'debug'):

-         session_opts[k] = getattr(options,k)

-     if options.network_hack:

-         socket.setdefaulttimeout(180)

-         session = MySession(options.server, session_opts)

-     else:

-         session = koji.ClientSession(options.server, session_opts)

+     session_opts = koji.grab_session_options(options)

+     session = koji.ClientSession(options.server, session_opts)

      rv = 0

      try:

          if not options.skip_main:

util/koji-shadow
file modified
+1 -3
@@ -1326,9 +1326,7 @@

  

      options, args = get_options()

  

-     session_opts = {}

-     for k in ('user', 'password', 'krbservice', 'debug_xmlrpc', 'debug'):

-         session_opts[k] = getattr(options, k)

+     session_opts = koji.grab_session_options(options)

      session = koji.ClientSession(options.server, session_opts)

      if not options.noauth:

          activate_session(session)

util/kojira
file modified
+6 -5
@@ -719,6 +719,8 @@

                  'max_retries': 120,

                  'offline_retry': True,

                  'offline_retry_interval': 120,

+                 'use_old_ssl': False,

+                 'no_ssl_verify': False,

                  'max_delete_processes': 4,

                  'max_repo_tasks' : 4,

                  'max_repo_tasks_maven' : 2,
@@ -737,7 +739,8 @@

                      'max_delete_processes', 'max_repo_tasks_maven', 'delete_batch_size', )

          str_opts = ('topdir', 'server', 'user', 'password', 'logfile', 'principal', 'keytab', 'krbservice',

                      'cert', 'ca', 'serverca', 'debuginfo_tags', 'source_tags')  # FIXME: remove ca here

-         bool_opts = ('with_src','verbose','debug','ignore_stray_repos', 'offline_retry', 'krb_rdns')

+         bool_opts = ('with_src','verbose','debug','ignore_stray_repos', 'offline_retry',

+                      'krb_rdns', 'use_old_ssl', 'no_ssl_verify')

          for name in config.options(section):

              if name in int_opts:

                  defaults[name] = config.getint(section, name)
@@ -791,10 +794,8 @@

          logger.setLevel(logging.ERROR)

      else:

          logger.setLevel(logging.WARNING)

-     session_opts = {}

-     for k in ('user', 'password', 'krbservice', 'krb_rdns', 'debug_xmlrpc', 'debug',

-               'retry_interval', 'max_retries', 'offline_retry', 'offline_retry_interval'):

-         session_opts[k] = getattr(options,k)

+ 

+     session_opts = koji.grab_session_options(options)

      session = koji.ClientSession(options.server,session_opts)

      if os.path.isfile(options.cert):

          # authenticate using SSL client certificates

vm/kojivmd
file modified
+5 -8
@@ -125,6 +125,8 @@

                  'server': None,

                  'user': None,

                  'password': None,

+                 'use_old_ssl': False,

+                 'no_ssl_verify': False,

                  'retry_interval': 60,

                  'max_retries': 120,

                  'offline_retry': True,
@@ -142,7 +144,8 @@

                      defaults[name] = int(value)

                  except ValueError:

                      quit("value for %s option must be a valid integer" % name)

-             elif name in ['offline_retry', 'krb_rdns']:

+             elif name in ['offline_retry', 'krb_rdns', 'use_old_ssl',

+                           'no_ssl_verify']:

                  defaults[name] = config.getboolean('kojivmd', name)

              elif name in ['plugin', 'plugins']:

                  defaults['plugin'] = value.split()
@@ -1055,14 +1058,8 @@

      if options.admin_emails:

          koji.add_mail_logger("koji", options.admin_emails)

  

-     #build session options

-     session_opts = {}

-     for k in ('user', 'password', 'krbservice', 'krb_rdns', 'debug_xmlrpc', 'debug',

-               'retry_interval', 'max_retries', 'offline_retry', 'offline_retry_interval'):

-         v = getattr(options, k, None)

-         if v is not None:

-             session_opts[k] = v

      #start a session and login

+     session_opts = koji.grab_session_options(options)

      session = koji.ClientSession(options.server, session_opts)

      if os.path.isfile(options.cert):

          try:

no initial comment

This change reworks ClientSession to use python-requests by default. For systems where requests is unavailable (cough rhel5), we have a thin "compatrequests" layer that is based on the old ssl code. This path can also be accessed by setting 'use_old_ssl' to true in the config options of the various commands.

The default behavior of python-requests is to verify the cert of the http server. This is unarguably better and safer, but it is also a behavior change for Koji. This can be disabled by setting no_ssl_verify to True. The serverca option is now valid even if not using ssl auth. If it is set, then this ca is used to verify the hub. If it is not set, the normal system ca bundle is used.

In the midst of adding several new session options and exposing them in several command configurations, I opted to add a single grab_session_options function in the library. All the tools now use this instead of the old boilerplate code.

@mikem The changes look good to me for merging. However, I think it'd be a good idea to also include the GSSAPI work that @puiterwijk did in this branch.

He had previously sent a PR to your playground repository to this branch on GitHub, but it looks like it doesn't cleanly merge now.

I think it'd be a good idea to pull in the patch into your branch as part of this PR, especially in light of the move to Kerberos auth for Fedora's Koji instance.

@ngompa the gssapi change is a single patch and it still cherry picks fine. It's kind of a separate feature, so I think I'll leave it out of the PR, but I definitely intend to merge it promptly

@mikem Then I don't see a problem with merging this now as long as that is pulled in right after it.

rebased

7 years ago

Pull-Request has been merged by mikem

7 years ago