Commit a5eba783 authored by Michael Hanselmann's avatar Michael Hanselmann
Browse files

RAPI client: Don't re-use PycURL object



With this patch, a new PycURL object will be created for each request.
This should make the RAPI client safe for simultaneous calls from
multiple threads. Unittests are adjusted accordingly.

An unnecessary variable assignment is also removed from the unittest
script.

This patch survived a small QA and unittests.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
parent 94e9e1e9
......@@ -243,7 +243,7 @@ class GanetiRapiClient(object):
def __init__(self, host, port=GANETI_RAPI_PORT,
username=None, password=None, logger=logging,
curl_config_fn=None, curl=None):
curl_config_fn=None, curl_factory=None):
"""Initializes this class.
@type host: string
......@@ -259,14 +259,28 @@ class GanetiRapiClient(object):
@param logger: Logging object
"""
self._host = host
self._port = port
self._username = username
self._password = password
self._logger = logger
self._curl_config_fn = curl_config_fn
self._curl_factory = curl_factory
self._base_url = "https://%s:%s" % (host, port)
# Create pycURL object if not supplied
if not curl:
if username is not None:
if password is None:
raise Error("Password not specified")
elif password:
raise Error("Specified password without username")
def _CreateCurl(self):
"""Creates a cURL object.
"""
# Create pycURL object if no factory is provided
if self._curl_factory:
curl = self._curl_factory()
else:
curl = pycurl.Curl()
# Default cURL settings
......@@ -282,20 +296,20 @@ class GanetiRapiClient(object):
"Content-type: %s" % HTTP_APP_JSON,
])
# Setup authentication
if username is not None:
if password is None:
raise Error("Password not specified")
assert ((self._username is None and self._password is None) ^
(self._username is not None and self._password is not None))
if self._username:
# Setup authentication
curl.setopt(pycurl.HTTPAUTH, pycurl.HTTPAUTH_BASIC)
curl.setopt(pycurl.USERPWD, str("%s:%s" % (username, password)))
elif password:
raise Error("Specified password without username")
curl.setopt(pycurl.USERPWD,
str("%s:%s" % (self._username, self._password)))
# Call external configuration function
if curl_config_fn:
curl_config_fn(curl, logger)
if self._curl_config_fn:
self._curl_config_fn(curl, self._logger)
self._curl = curl
return curl
@staticmethod
def _EncodeQuery(query):
......@@ -349,7 +363,7 @@ class GanetiRapiClient(object):
"""
assert path.startswith("/")
curl = self._curl
curl = self._CreateCurl()
if content is not None:
encoded_content = self._json_encoder.encode(content)
......@@ -364,8 +378,8 @@ class GanetiRapiClient(object):
url = "".join(urlparts)
self._logger.debug("Sending request %s %s to %s:%s (content=%r)",
method, url, self._host, self._port, encoded_content)
self._logger.debug("Sending request %s %s (content=%r)",
method, url, encoded_content)
# Buffer for response
encoded_resp_body = StringIO()
......
......@@ -170,11 +170,11 @@ def _FakeGnuTlsPycurlVersion():
class TestExtendedConfig(unittest.TestCase):
def testAuth(self):
curl = FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com",
username="user", password="pw",
curl=curl)
curl_factory=lambda: FakeCurl(RapiMock()))
curl = cl._CreateCurl()
self.assertEqual(curl.getopt(pycurl.HTTPAUTH), pycurl.HTTPAUTH_BASIC)
self.assertEqual(curl.getopt(pycurl.USERPWD), "user:pw")
......@@ -209,10 +209,12 @@ class TestExtendedConfig(unittest.TestCase):
verify_hostname=verify_hostname,
_pycurl_version_fn=pcverfn)
curl = FakeCurl(RapiMock())
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com",
curl_config_fn=cfgfn, curl=curl)
curl_config_fn=cfgfn,
curl_factory=curl_factory)
curl = cl._CreateCurl()
self.assertEqual(curl.getopt(pycurl.PROXY), proxy)
self.assertEqual(curl.getopt(pycurl.NOSIGNAL), not use_signal)
......@@ -224,10 +226,11 @@ class TestExtendedConfig(unittest.TestCase):
def testNoCertVerify(self):
cfgfn = client.GenericCurlConfig()
curl = FakeCurl(RapiMock())
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl=curl)
curl_factory=curl_factory)
curl = cl._CreateCurl()
self.assertFalse(curl.getopt(pycurl.SSL_VERIFYPEER))
self.assertFalse(curl.getopt(pycurl.CAINFO))
self.assertFalse(curl.getopt(pycurl.CAPATH))
......@@ -235,10 +238,11 @@ class TestExtendedConfig(unittest.TestCase):
def testCertVerifyCurlBundle(self):
cfgfn = client.GenericCurlConfig(use_curl_cabundle=True)
curl = FakeCurl(RapiMock())
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl=curl)
curl_factory=curl_factory)
curl = cl._CreateCurl()
self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER))
self.assertFalse(curl.getopt(pycurl.CAINFO))
self.assertFalse(curl.getopt(pycurl.CAPATH))
......@@ -247,10 +251,11 @@ class TestExtendedConfig(unittest.TestCase):
mycert = "/tmp/some/UNUSED/cert/file.pem"
cfgfn = client.GenericCurlConfig(cafile=mycert)
curl = FakeCurl(RapiMock())
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl=curl)
curl_factory=curl_factory)
curl = cl._CreateCurl()
self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER))
self.assertEqual(curl.getopt(pycurl.CAINFO), mycert)
self.assertFalse(curl.getopt(pycurl.CAPATH))
......@@ -261,10 +266,11 @@ class TestExtendedConfig(unittest.TestCase):
cfgfn = client.GenericCurlConfig(capath=certdir,
_pycurl_version_fn=pcverfn)
curl = FakeCurl(RapiMock())
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl=curl)
curl_factory=curl_factory)
curl = cl._CreateCurl()
self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER))
self.assertEqual(curl.getopt(pycurl.CAPATH), certdir)
self.assertFalse(curl.getopt(pycurl.CAINFO))
......@@ -275,9 +281,11 @@ class TestExtendedConfig(unittest.TestCase):
cfgfn = client.GenericCurlConfig(capath=certdir,
_pycurl_version_fn=pcverfn)
curl = FakeCurl(RapiMock())
self.assertRaises(client.Error, client.GanetiRapiClient,
"master.example.com", curl_config_fn=cfgfn, curl=curl)
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl_factory=curl_factory)
self.assertRaises(client.Error, cl._CreateCurl)
def testCertVerifyNoSsl(self):
certdir = "/tmp/some/UNUSED/cert/directory"
......@@ -285,9 +293,11 @@ class TestExtendedConfig(unittest.TestCase):
cfgfn = client.GenericCurlConfig(capath=certdir,
_pycurl_version_fn=pcverfn)
curl = FakeCurl(RapiMock())
self.assertRaises(client.Error, client.GanetiRapiClient,
"master.example.com", curl_config_fn=cfgfn, curl=curl)
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl_factory=curl_factory)
self.assertRaises(client.Error, cl._CreateCurl)
def testCertVerifyFancySsl(self):
certdir = "/tmp/some/UNUSED/cert/directory"
......@@ -295,9 +305,11 @@ class TestExtendedConfig(unittest.TestCase):
cfgfn = client.GenericCurlConfig(capath=certdir,
_pycurl_version_fn=pcverfn)
curl = FakeCurl(RapiMock())
self.assertRaises(NotImplementedError, client.GanetiRapiClient,
"master.example.com", curl_config_fn=cfgfn, curl=curl)
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl_factory=curl_factory)
self.assertRaises(NotImplementedError, cl._CreateCurl)
def testCertVerifyCapath(self):
for connect_timeout in [None, 1, 5, 10, 30, 60, 300]:
......@@ -305,10 +317,11 @@ class TestExtendedConfig(unittest.TestCase):
cfgfn = client.GenericCurlConfig(connect_timeout=connect_timeout,
timeout=timeout)
curl = FakeCurl(RapiMock())
curl_factory = lambda: FakeCurl(RapiMock())
cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn,
curl=curl)
curl_factory=curl_factory)
curl = cl._CreateCurl()
self.assertEqual(curl.getopt(pycurl.CONNECTTIMEOUT), connect_timeout)
self.assertEqual(curl.getopt(pycurl.TIMEOUT), timeout)
......@@ -320,18 +333,7 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
self.rapi = RapiMock()
self.curl = FakeCurl(self.rapi)
self.client = client.GanetiRapiClient("master.example.com",
curl=self.curl)
# Signals should be disabled by default
self.assert_(self.curl.getopt(pycurl.NOSIGNAL))
# No auth and no proxy
self.assertFalse(self.curl.getopt(pycurl.USERPWD))
self.assert_(self.curl.getopt(pycurl.PROXY) is None)
# Content-type is required for requests
headers = self.curl.getopt(pycurl.HTTPHEADER)
self.assert_("Content-type: application/json" in headers)
curl_factory=lambda: self.curl)
def assertHandler(self, handler_cls):
self.failUnless(isinstance(self.rapi.GetLastHandler(), handler_cls))
......@@ -372,6 +374,22 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
for i in [[1, 2, 3], {"moo": "boo"}, (1, 2, 3)]:
self.assertRaises(ValueError, self.client._EncodeQuery, [("x", i)])
def testCurlSettings(self):
self.rapi.AddResponse("2")
self.assertEqual(2, self.client.GetVersion())
self.assertHandler(rlib2.R_version)
# Signals should be disabled by default
self.assert_(self.curl.getopt(pycurl.NOSIGNAL))
# No auth and no proxy
self.assertFalse(self.curl.getopt(pycurl.USERPWD))
self.assert_(self.curl.getopt(pycurl.PROXY) is None)
# Content-type is required for requests
headers = self.curl.getopt(pycurl.HTTPHEADER)
self.assert_("Content-type: application/json" in headers)
def testHttpError(self):
self.rapi.AddResponse(None, code=404)
try:
......@@ -382,7 +400,6 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
self.fail("Didn't raise exception")
def testGetVersion(self):
self.client._version = None
self.rapi.AddResponse("2")
self.assertEqual(2, self.client.GetVersion())
self.assertHandler(rlib2.R_version)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment