From 06f8d249671fd3c8f322eded2ff98555fb70f2c8 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:42:50 +0000 Subject: Merge #213 `Use python-requests` --- diff --git a/builder/kojid b/builder/kojid index b5ff393..2858f05 100755 --- a/builder/kojid +++ b/builder/kojid @@ -4974,6 +4974,8 @@ def get_options(): '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 @@ def get_options(): 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 __name__ == "__main__": 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: diff --git a/cli/koji b/cli/koji index 67900b7..f093c50 100755 --- a/cli/koji +++ b/cli/koji @@ -4360,7 +4360,7 @@ def anon_handle_list_history(options, session, args): 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 @@ if __name__ == "__main__": 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) diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index f3057a1..295a197 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -750,7 +750,7 @@ def application(environ, start_response): 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) diff --git a/koji.spec b/koji.spec index 5d12d27..3beff7e 100644 --- a/koji.spec +++ b/koji.spec @@ -28,6 +28,7 @@ BuildArch: noarch Requires: python-krbV >= 1.0.13 Requires: rpm-python Requires: pyOpenSSL +Requires: python-requests Requires: python-urlgrabber Requires: python-dateutil BuildRequires: python diff --git a/koji/__init__.py b/koji/__init__.py index 54d7356..90b770c 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -38,21 +38,29 @@ import imp 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 urllib import urllib2 import urlparse import util +import warnings import xmlrpclib import xml.sax import xml.sax.handler @@ -1562,16 +1571,17 @@ def read_config(profile_name, user_config=None): '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 @@ def read_config(profile_name, user_config=None): #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 @@ def read_config(profile_name, user_config=None): 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 @@ class PathInfo(object): 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 @@ class VirtualMethod(object): 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 @@ class ClientSession(object): 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 @@ class ClientSession(object): 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 @@ class ClientSession(object): 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 @@ class ClientSession(object): 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 @@ class ClientSession(object): 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 @@ class ClientSession(object): 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 @@ class ClientSession(object): 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"), diff --git a/koji/compatrequests.py b/koji/compatrequests.py new file mode 100644 index 0000000..2855da0 --- /dev/null +++ b/koji/compatrequests.py @@ -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() + + + diff --git a/koji/ssl/SSLCommon.py b/koji/ssl/SSLCommon.py index fa5e64a..ffcd5e9 100644 --- a/koji/ssl/SSLCommon.py +++ b/koji/ssl/SSLCommon.py @@ -28,43 +28,6 @@ def our_verify(connection, x509, errNum, errDepth, preverifyOK): 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'] diff --git a/tests/test_client_session.py b/tests/test_client_session.py new file mode 100644 index 0000000..3931891 --- /dev/null +++ b/tests/test_client_session.py @@ -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) diff --git a/tests/test_compatrequests.py b/tests/test_compatrequests.py new file mode 100644 index 0000000..f835d79 --- /dev/null +++ b/tests/test_compatrequests.py @@ -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)) diff --git a/tests/test_krbv.py b/tests/test_krbv.py index b8a88e1..0fea679 100644 --- a/tests/test_krbv.py +++ b/tests/test_krbv.py @@ -9,8 +9,7 @@ import koji 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') diff --git a/util/koji-gc b/util/koji-gc index 97232d1..b43f8aa 100755 --- a/util/koji-gc +++ b/util/koji-gc @@ -36,35 +36,6 @@ if optparse.__version__ == "1.4.1+": 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 @@ def get_options(): 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 @@ def get_options(): ['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 @@ if __name__ == "__main__": 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: diff --git a/util/koji-shadow b/util/koji-shadow index a0f7cf4..bd5f967 100755 --- a/util/koji-shadow +++ b/util/koji-shadow @@ -1326,9 +1326,7 @@ if __name__ == "__main__": 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) diff --git a/util/kojira b/util/kojira index 9f776fb..905fec4 100755 --- a/util/kojira +++ b/util/kojira @@ -719,6 +719,8 @@ def get_options(): '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 @@ def get_options(): '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 @@ if __name__ == "__main__": 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 diff --git a/vm/kojivmd b/vm/kojivmd index 3cce429..50d48a3 100755 --- a/vm/kojivmd +++ b/vm/kojivmd @@ -125,6 +125,8 @@ def get_options(): '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 @@ def get_options(): 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 __name__ == "__main__": 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: