diff --git a/doc/source/users/transport.rst b/doc/source/users/transport.rst index e2e81a59f..3f989a941 100644 --- a/doc/source/users/transport.rst +++ b/doc/source/users/transport.rst @@ -1,6 +1,7 @@ Transport ========= .. automodule:: openstack.transport + :members: Transport Object ---------------- diff --git a/doc/source/users/userguides/usage.rst b/doc/source/users/userguides/usage.rst index c62b4a49b..38d991269 100644 --- a/doc/source/users/userguides/usage.rst +++ b/doc/source/users/userguides/usage.rst @@ -1,4 +1,5 @@ .. TODO(briancurtin): turn this into a full guide on the Connection class +.. TODO(briancurtin): cover user_agent setting ===== Usage diff --git a/openstack/connection.py b/openstack/connection.py index b00111694..97e8e7c4a 100644 --- a/openstack/connection.py +++ b/openstack/connection.py @@ -64,16 +64,13 @@ from openstack import module_loader from openstack import session from openstack import transport as xport - -USER_AGENT = 'OSPythonSDK' -"""Default value for the HTTP User-Agent header""" _logger = logging.getLogger(__name__) class Connection(object): def __init__(self, transport=None, authenticator=None, preference=None, - verify=True, user_agent=USER_AGENT, + verify=True, user_agent=None, auth_plugin=None, **auth_args): """Create a context for a connection to a cloud provider. @@ -106,10 +103,11 @@ class Connection(object): is set to true, which is the default, the SSL cert will be verified. It can also be set to a CA_BUNDLE path. :param str user_agent: If a transport is not provided to the - connection, this parameter will be used to create a transport. - The value of this parameter is used for the ``User-Agent`` HTTP - header. The default value is the module level attribute - ``USER_AGENT`` which is set to ``"OSPythonSDK"``. + connection, this parameter will be used when creating a transport. + The value given here will be prepended to the default, which is + specified in :attr:`~openstack.transport.USER_AGENT`. + The resulting ``user_agent`` value is used for the ``User-Agent`` + HTTP header. :param str auth_plugin: The name of authentication plugin to use. If the authentication plugin name is not provided, the connection will try to guess what plugin to use based on the *auth_url* in the diff --git a/openstack/tests/test_connection.py b/openstack/tests/test_connection.py index c6b36ca75..5fbd62ce3 100644 --- a/openstack/tests/test_connection.py +++ b/openstack/tests/test_connection.py @@ -36,7 +36,7 @@ class TestConnection(base.TestCase): conn = connection.Connection(authenticator='2', verify=True, user_agent='1') self.assertTrue(conn.transport.verify) - self.assertEqual('1', conn.transport._user_agent) + self.assertIn('1', conn.transport._user_agent) def test_create_authenticator_v2(self): auth_args = { @@ -121,6 +121,12 @@ class TestConnection(base.TestCase): self.assertEqual('openstack.telemetry.v2._proxy', conn.telemetry.__class__.__module__) + def test_custom_user_agent(self): + user_agent = "MyProgram/1.0" + conn = connection.Connection(authenticator=self.auth, + user_agent=user_agent) + self.assertTrue(conn.transport._user_agent.startswith(user_agent)) + class TestService(service_filter.ServiceFilter): valid_versions = [service_filter.ValidVersion('v2')] diff --git a/openstack/tests/test_transport.py b/openstack/tests/test_transport.py index a9121ce65..4064c4dad 100644 --- a/openstack/tests/test_transport.py +++ b/openstack/tests/test_transport.py @@ -44,6 +44,17 @@ fake_record2 = { class TestTransport(base.TestTransportBase): + def setUp(self): + super(TestTransport, self).setUp() + + self._orig_user_agent = transport.USER_AGENT + self.test_user_agent = transport.USER_AGENT = "testing/1.0" + + def tearDown(self): + super(TestTransport, self).tearDown() + + transport.USER_AGENT = self._orig_user_agent + @httpretty.activate def test_request(self): self.stub_url(httpretty.GET, body=fake_response) @@ -169,220 +180,76 @@ class TestTransport(base.TestTransportBase): resp = xport.get(self.TEST_URL, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual( - 'User-Agent', - transport.DEFAULT_USER_AGENT, - ) + self.assertRequestHeaderEqual('User-Agent', self.test_user_agent) - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': None}, - accept=None, - ) + resp = xport.get(self.TEST_URL, headers={'User-Agent': None}, + accept=None) self.assertTrue(resp.ok) self.assertRequestHeaderEqual('User-Agent', None) resp = xport.get(self.TEST_URL, user_agent=None, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) + self.assertRequestHeaderEqual('User-Agent', self.test_user_agent) - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': ''}, - accept=None, - ) + resp = xport.get(self.TEST_URL, headers={'User-Agent': ''}, + accept=None) self.assertTrue(resp.ok) self.assertRequestHeaderEqual('User-Agent', '') resp = xport.get(self.TEST_URL, user_agent='', accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', '') + self.assertRequestHeaderEqual('User-Agent', self.test_user_agent) - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - accept=None, - ) + new_agent = 'new-agent' + resp = xport.get(self.TEST_URL, headers={'User-Agent': new_agent}, + accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'new-agent') + self.assertRequestHeaderEqual('User-Agent', new_agent) - resp = xport.get(self.TEST_URL, user_agent='new-agent', accept=None) + resp = xport.get(self.TEST_URL, user_agent=new_agent, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'new-agent') + self.assertRequestHeaderEqual('User-Agent', '%s %s' % ( + new_agent, self.test_user_agent)) - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - user_agent=None, - accept=None, - ) + custom_value = 'new-agent' + resp = xport.get(self.TEST_URL, headers={'User-Agent': custom_value}, + user_agent=None, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) + self.assertRequestHeaderEqual('User-Agent', custom_value) - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': None}, - user_agent='overrides-agent', - accept=None, - ) + override = 'overrides-agent' + resp = xport.get(self.TEST_URL, headers={'User-Agent': None}, + user_agent=override, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'overrides-agent') + self.assertRequestHeaderEqual('User-Agent', '%s %s' % ( + override, self.test_user_agent)) - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - user_agent='overrides-agent', - accept=None, - ) + resp = xport.get(self.TEST_URL, headers={'User-Agent': custom_value}, + user_agent=override, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'overrides-agent') + self.assertRequestHeaderEqual('User-Agent', '%s %s' % ( + override, self.test_user_agent)) @httpretty.activate def test_user_agent_arg_none(self): + # None gets converted to the transport.USER_AGENT by default. self.stub_url(httpretty.GET, body=fake_response) xport = transport.Transport(user_agent=None) resp = xport.get(self.TEST_URL, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual( - 'User-Agent', - transport.DEFAULT_USER_AGENT, - ) - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': None}, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) - - resp = xport.get(self.TEST_URL, user_agent=None, accept=None) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': ''}, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', '') - - resp = xport.get(self.TEST_URL, user_agent='', accept=None) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', '') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'new-agent') - - resp = xport.get(self.TEST_URL, user_agent='new-agent', accept=None) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'new-agent') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - user_agent=None, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': None}, - user_agent='overrides-agent', - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'overrides-agent') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - user_agent='overrides-agent', - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'overrides-agent') + self.assertRequestHeaderEqual('User-Agent', self.test_user_agent) @httpretty.activate def test_user_agent_arg_default(self): self.stub_url(httpretty.GET, body=fake_response) - xport = transport.Transport(user_agent='test-agent') + agent = 'test-agent' + xport = transport.Transport(user_agent=agent) resp = xport.get(self.TEST_URL, accept=None) self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'test-agent') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': None}, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) - - resp = xport.get(self.TEST_URL, user_agent=None, accept=None) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': ''}, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', '') - - resp = xport.get(self.TEST_URL, user_agent='', accept=None) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', '') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'new-agent') - - resp = xport.get(self.TEST_URL, user_agent='new-agent', accept=None) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'new-agent') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - user_agent=None, - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', None) - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': None}, - user_agent='overrides-agent', - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'overrides-agent') - - resp = xport.get( - self.TEST_URL, - headers={'User-Agent': 'new-agent'}, - user_agent='overrides-agent', - accept=None, - ) - self.assertTrue(resp.ok) - self.assertRequestHeaderEqual('User-Agent', 'overrides-agent') + self.assertRequestHeaderEqual('User-Agent', '%s %s' % ( + agent, self.test_user_agent)) def test_verify_no_arg(self): xport = transport.Transport() diff --git a/openstack/transport.py b/openstack/transport.py index d702ced3a..6ec57f3a5 100644 --- a/openstack/transport.py +++ b/openstack/transport.py @@ -92,18 +92,28 @@ for an API. See: https://en.wikipedia.org/wiki/Post/Redirect/Get -User Agent +User-Agent ~~~~~~~~~~ -The ``User-Agent`` header may be set when the Transport object is created in -addition to the existing per-request mode. The determination of how to set -the ``User-Agent`` header is as follows: +User-Agent handling as constructed by this class follows +`RFC 7231 Section 5.5.3 `_. +A well-formed user-agent is constructed on name/version product identifiers, +such that ``MyProgram/1.0`` is a proper user-agent. + +* The default :attr:`~openstack.transport.USER_AGENT` contains + the SDK version as well as RFC-compliant values from + ``requests.utils.default_user_agent``, including versions of ``requests``, + Python, and the operating system. +* Any ``user_agent`` argument passed when creating a + :class:`~openstack.transport.Transport` is prepended to the default. +* Any ``user_agent`` passed in a + :meth:`~openstack.transport.Transport.request` call is prepended + to one used for that ``Transport`` instance. +* Any string passed as the ``User-Agent`` in a dictionary of + headers to :meth:`~openstack.transport.Transport.request` will be + used directly. If at the same time a ``user_agent`` argument has been passed + to ``request()``, it will be used and follows the rules above. -* If the ``user_agent`` argument is included in the ``request()`` call use it -* Else if ``User-Agent`` is set in the headers dict use it -* Else if ``user_agent`` argument is included in the - :class:`~openstack.transport.Transport` construction use it -* Else use ``transport.DEFAULT_USER_AGENT`` """ import json @@ -116,8 +126,11 @@ from six.moves import urllib import openstack from openstack import exceptions - -DEFAULT_USER_AGENT = 'python-OpenStackSDK/' + openstack.__version__ +#: Default value for the HTTP User-Agent header. The default includes the +#: version information of the SDK as well as ``requests``, Python, +#: and the operating system. +USER_AGENT = "python-openstacksdk/%s %s" % ( + openstack.__version__, requests.utils.default_user_agent()) _logger = logging.getLogger(__name__) JSON = 'application/json' @@ -125,8 +138,6 @@ JSON = 'application/json' class Transport(requests.Session): - _user_agent = DEFAULT_USER_AGENT - REDIRECT_STATUSES = (301, 302, 303, 305, 307) DEFAULT_REDIRECT_LIMIT = 30 @@ -142,9 +153,12 @@ class Transport(requests.Session): In addition to those listed below, all arguments available to ``requests.Session`` are available here: - :param string user_agent: Set the default ``User-Agent`` header; - Header is omitted if ``None`` and no value - is supplied in the ``request()`` call. + :param string user_agent: Set the ``User-Agent`` header. When + no value is provided, the default of + :attr:`~openstack.transport.USER_AGENT` + will be used. When a value is provided, + it will be prepended to + :attr:`~openstack.transport.USER_AGENT`. :param boolean/string verify: If ``True``, the SSL cert will be verified. A CA_BUNDLE path can also be provided. @@ -155,18 +169,18 @@ class Transport(requests.Session): if True. (optional) :param string accept: Type of output to accept - User agent handling is as follows: - - * if user_agent arg is included in the request() call, use it - * else if 'User-Agent' is set in the headers dict, use it - * else if user_agent arg is included in the __init__() call, use it - * else use DEFAULT_USER_AGENT - """ super(Transport, self).__init__() - if user_agent: - self._user_agent = user_agent + + # Per RFC 7231 Section 5.5.3, identifiers in a user-agent should + # be ordered by decreasing significance. If a user sets their product, + # we prepend it to the SDK version and then the Python version. + if user_agent is None: + self._user_agent = USER_AGENT + else: + self._user_agent = "%s %s" % (user_agent, USER_AGENT) + self.verify = verify self._redirect = redirect self._accept = accept @@ -192,9 +206,8 @@ class Transport(requests.Session): :param string accept: Set the ``Accept`` header; overwrites any value that may be in the headers dict. Header is omitted if ``None``. - :param string user_agent: Set the ``User-Agent`` header; overwrites - any value that may be in the headers dict. - Header is omitted if ``None``. + :param string user_agent: Prepend an additional value to the existing + ``User-Agent`` header. Remaining kw args from requests.Session.request() supported @@ -209,14 +222,19 @@ class Transport(requests.Session): kwargs['data'] = json.dumps(json_data) headers['Content-Type'] = JSON - # Set User-Agent header if user_agent arg included, or - # fall through the default chain as described above - if 'user_agent' in kwargs: - headers['User-Agent'] = kwargs.pop('user_agent') - elif self._user_agent: - headers.setdefault('User-Agent', self._user_agent) + # Prepend the caller's user_agent to User-Agent header if included, + # or use the default that this transport was created with. + # Note: Only attempt to work with strings and avoid needlessly + # concatenating an empty string. + user_agent = kwargs.pop('user_agent', None) + if isinstance(user_agent, six.string_types) and user_agent != '': + headers['User-Agent'] = '%s %s' % (user_agent, self._user_agent) + elif 'User-Agent' in headers: + # If they've specified their own headers with a User-Agent, + # use that directly. + pass else: - headers.setdefault('User-Agent', DEFAULT_USER_AGENT) + headers.setdefault('User-Agent', self._user_agent) if redirect is None: redirect = self._redirect