Use adapter from keystoneclient

The adapter in keystoneclient is there to abstract the common client
parameters that may be used when sending a request. We should use that
rather than keeping our own methods.

Closes-Bug: #1403726
Change-Id: I727ac76babb6f6aef1a0f619c011ad67a6e2fccf

neutronclient/client.py

@@ -14,7 +14,6 @@
 #    under the License.
 #
 
-import abc
 try:
     import json
 except ImportError:
@@ -23,9 +22,8 @@ import logging
 import os
 
 from keystoneclient import access
-from keystoneclient.auth.identity.base import BaseIdentityPlugin
+from keystoneclient import adapter
 import requests
-import six
 
 from neutronclient.common import exceptions
 from neutronclient.common import utils
@@ -44,35 +42,14 @@ else:
 logging.getLogger("requests").setLevel(_requests_log_level)
 
 
-@six.add_metaclass(abc.ABCMeta)
+class HTTPClient(object):
-class AbstractHTTPClient(object):
+    """Handles the REST calls and responses, include authn."""
 
     USER_AGENT = 'python-neutronclient'
     CONTENT_TYPE = 'application/json'
 
-    def request(self, url, method, body=None, content_type=None, headers=None,
+    # 8192 Is the default max URI len for eventlet.wsgi.server
-                **kwargs):
+    MAX_URI_LEN = 8192
-        """Request without authentication."""
-
-        headers = headers or {}
-        content_type = content_type or self.CONTENT_TYPE
-        headers.setdefault('Accept', content_type)
-        if body:
-            headers.setdefault('Content-Type', content_type)
-
-        return self._request(url, method, body=body, headers=headers, **kwargs)
-
-    @abc.abstractmethod
-    def do_request(self, url, method, **kwargs):
-        """Request with authentication."""
-
-    @abc.abstractmethod
-    def _request(self, url, method, body=None, headers=None, **kwargs):
-        """Request without authentication nor headers population."""
-
-
-class HTTPClient(AbstractHTTPClient):
-    """Handles the REST calls and responses, include authentication."""
 
     def __init__(self, username=None, user_id=None,
                  tenant_name=None, tenant_id=None,
@@ -149,8 +126,16 @@ class HTTPClient(AbstractHTTPClient):
         elif not self.endpoint_url:
             self.endpoint_url = self._get_endpoint_url()
 
-    def _request(self, url, method, body=None, headers=None, **kwargs):
+    def request(self, url, method, body=None, headers=None, **kwargs):
+        """Request without authentication."""
+
+        content_type = kwargs.pop('content_type', None) or 'application/json'
         headers = headers or {}
+        headers.setdefault('Accept', content_type)
+
+        if body:
+            headers.setdefault('Content-Type', content_type)
+
         headers['User-Agent'] = self.USER_AGENT
 
         resp = requests.request(
@@ -164,8 +149,17 @@ class HTTPClient(AbstractHTTPClient):
 
         return resp, resp.text
 
+    def _check_uri_length(self, action):
+        uri_len = len(self.endpoint_url) + len(action)
+        if uri_len > self.MAX_URI_LEN:
+            raise exceptions.RequestURITooLong(
+                excess=uri_len - self.MAX_URI_LEN)
+
     def do_request(self, url, method, **kwargs):
+        # Ensure client always has correct uri - do not guesstimate anything
         self.authenticate_and_fetch_endpoint_url()
+        self._check_uri_length(url)
+
         # Perform the request once. If we get a 401 back then it
         # might be because the auth token expired, so try to
         # re-authenticate and try again. If it still fails, bail.
@@ -280,71 +274,67 @@ class HTTPClient(AbstractHTTPClient):
                 'endpoint_url': self.endpoint_url}
 
 
-class SessionClient(AbstractHTTPClient):
+class SessionClient(adapter.Adapter):
 
-    def __init__(self,
+    def request(self, *args, **kwargs):
-                 session,
-                 auth,
-                 interface=None,
-                 service_type=None,
-                 region_name=None):
-
-        self.session = session
-        self.auth = auth
-        self.interface = interface
-        self.service_type = service_type
-        self.region_name = region_name
-        self.auth_token = None
-        self.endpoint_url = None
-
-    def _request(self, url, method, body=None, headers=None, **kwargs):
-        kwargs.setdefault('user_agent', self.USER_AGENT)
-        kwargs.setdefault('auth', self.auth)
         kwargs.setdefault('authenticated', False)
         kwargs.setdefault('raise_exc', False)
 
-        endpoint_filter = kwargs.setdefault('endpoint_filter', {})
+        content_type = kwargs.pop('content_type', None) or 'application/json'
-        endpoint_filter.setdefault('interface', self.interface)
-        endpoint_filter.setdefault('service_type', self.service_type)
-        endpoint_filter.setdefault('region_name', self.region_name)
 
-        resp = self.session.request(url, method, data=body, headers=headers,
+        headers = kwargs.setdefault('headers', {})
-                                    **kwargs)
+        headers.setdefault('Accept', content_type)
+
+        try:
+            kwargs.setdefault('data', kwargs.pop('body'))
+        except KeyError:
+            pass
+
+        if kwargs.get('data'):
+            headers.setdefault('Content-Type', content_type)
+
+        resp = super(SessionClient, self).request(*args, **kwargs)
         return resp, resp.text
 
     def do_request(self, url, method, **kwargs):
         kwargs.setdefault('authenticated', True)
         return self.request(url, method, **kwargs)
 
-    def authenticate(self):
+    @property
-        # This method is provided for backward compatibility only.
+    def endpoint_url(self):
-        # We only care about setting the service endpoint.
+        # NOTE(jamielennox): This is used purely by the CLI and should be
-        self.endpoint_url = self.session.get_endpoint(
+        # removed when the CLI gets smarter.
-            self.auth,
+        return self.get_endpoint()
-            service_type=self.service_type,
-            region_name=self.region_name,
-            interface=self.interface)
 
-    def authenticate_and_fetch_endpoint_url(self):
+    @property
-        # This method is provided for backward compatibility only.
+    def auth_token(self):
-        # We only care about setting the service endpoint.
+        # NOTE(jamielennox): This is used purely by the CLI and should be
-        self.authenticate()
+        # removed when the CLI gets smarter.
+        return self.get_token()
+
+    def authenticate(self):
+        # NOTE(jamielennox): This is used purely by the CLI and should be
+        # removed when the CLI gets smarter.
+        self.get_token()
 
     def get_auth_info(self):
-        # This method is provided for backward compatibility only.
+        auth_info = {'auth_token': self.auth_token,
-        if not isinstance(self.auth, BaseIdentityPlugin):
+                     'endpoint_url': self.endpoint_url}
-            msg = ('Auth info not available. Auth plugin is not an identity '
+
-                   'auth plugin.')
+        # NOTE(jamielennox): This is the best we can do here. It will work
-            raise exceptions.NeutronClientException(message=msg)
+        # with identity plugins which is the primary case but we should
-        access_info = self.auth.get_access(self.session)
+        # deprecate it's usage as much as possible.
-        endpoint_url = self.auth.get_endpoint(self.session,
+        try:
-                                              service_type=self.service_type,
+            get_access = (self.auth or self.session.auth).get_access
-                                              region_name=self.region_name,
+        except AttributeError:
-                                              interface=self.interface)
+            pass
-        return {'auth_token': access_info.auth_token,
+        else:
-                'auth_tenant_id': access_info.tenant_id,
+            auth_ref = get_access(self.session)
-                'auth_user_id': access_info.user_id,
+
-                'endpoint_url': endpoint_url}
+            auth_info['auth_tenant_id'] = auth_ref.project_id
+            auth_info['auth_user_id'] = auth_ref.user_id
+
+        return auth_info
 
 
 # FIXME(bklei): Should refactor this to use kwargs and only
@@ -366,14 +356,15 @@ def construct_http_client(username=None,
                           ca_cert=None,
                           service_type='network',
                           session=None,
-                          auth=None):
+                          **kwargs):
 
     if session:
+        kwargs.setdefault('user_agent', 'python-neutronclient')
+        kwargs.setdefault('interface', endpoint_type)
         return SessionClient(session=session,
-                             auth=auth,
-                             interface=endpoint_type,
                              service_type=service_type,
-                             region_name=region_name)
+                             region_name=region_name,
+                             **kwargs)
     else:
         # FIXME(bklei): username and password are now optional. Need
         # to test that they were provided in this mode.  Should also

neutronclient/tests/unit/test_auth.py

@@ -316,7 +316,6 @@ class CLITestAuthKeystone(testtools.TestCase):
             auth_url=AUTH_URL, region_name=REGION,
             session=auth_session, auth=auth_plugin)
 
-        self.client.authenticate()
         self.assertEqual(self.client.endpoint_url, PUBLIC_ENDPOINT_URL)
 
         # Test admin url
@@ -325,7 +324,6 @@ class CLITestAuthKeystone(testtools.TestCase):
             auth_url=AUTH_URL, region_name=REGION, endpoint_type='adminURL',
             session=auth_session, auth=auth_plugin)
 
-        self.client.authenticate()
         self.assertEqual(self.client.endpoint_url, ADMIN_ENDPOINT_URL)
 
         # Test public url
@@ -334,7 +332,6 @@ class CLITestAuthKeystone(testtools.TestCase):
             auth_url=AUTH_URL, region_name=REGION, endpoint_type='publicURL',
             session=auth_session, auth=auth_plugin)
 
-        self.client.authenticate()
         self.assertEqual(self.client.endpoint_url, PUBLIC_ENDPOINT_URL)
 
         # Test internal url
@@ -343,7 +340,6 @@ class CLITestAuthKeystone(testtools.TestCase):
             auth_url=AUTH_URL, region_name=REGION, endpoint_type='internalURL',
             session=auth_session, auth=auth_plugin)
 
-        self.client.authenticate()
         self.assertEqual(self.client.endpoint_url, INTERNAL_ENDPOINT_URL)
 
         # Test url that isn't found in the service catalog
@@ -354,7 +350,7 @@ class CLITestAuthKeystone(testtools.TestCase):
 
         self.assertRaises(
             ks_exceptions.EndpointNotFound,
-            self.client.authenticate)
+            getattr, self.client, 'endpoint_url')
 
     def test_strip_credentials_from_log(self):
         m = self.requests.post(AUTH_URL + '/tokens', json=KS_TOKEN_RESULT)

neutronclient/tests/unit/test_cli20_network.py

@@ -519,13 +519,15 @@ class CLITestV20NetworkJSON(test_cli20.CLITestV20Base):
             filters, response = self._build_test_data(data)
 
             # 1 char of extra URI len will cause a split in 2 requests
-            self.mox.StubOutWithMock(self.client, "_check_uri_length")
+            self.mox.StubOutWithMock(self.client.httpclient,
-            self.client._check_uri_length(mox.IgnoreArg()).AndRaise(
+                                     "_check_uri_length")
+            self.client.httpclient._check_uri_length(mox.IgnoreArg()).AndRaise(
                 exceptions.RequestURITooLong(excess=1))
 
             for data in sub_data_lists:
                 filters, response = self._build_test_data(data)
-                self.client._check_uri_length(mox.IgnoreArg()).AndReturn(None)
+                self.client.httpclient._check_uri_length(
+                    mox.IgnoreArg()).AndReturn(None)
                 self.client.httpclient.request(
                     test_cli20.MyUrlComparator(
                         test_cli20.end_url(

neutronclient/tests/unit/test_cli20_securitygroup.py

@@ -265,13 +265,14 @@ class CLITestV20SecurityGroupsJSON(test_cli20.CLITestV20Base):
     def test_extend_list_exceed_max_uri_len(self):
         def mox_calls(path, data):
             # 1 char of extra URI len will cause a split in 2 requests
-            self.mox.StubOutWithMock(self.client, '_check_uri_length')
+            self.mox.StubOutWithMock(self.client.httpclient,
-            self.client._check_uri_length(mox.IgnoreArg()).AndRaise(
+                                     '_check_uri_length')
+            self.client.httpclient._check_uri_length(mox.IgnoreArg()).AndRaise(
                 exceptions.RequestURITooLong(excess=1))
             responses = self._build_test_data(data, excess=1)
 
             for item in responses:
-                self.client._check_uri_length(
+                self.client.httpclient._check_uri_length(
                     mox.IgnoreArg()).AndReturn(None)
                 self.client.httpclient.request(
                     test_cli20.end_url(path, item['filter']),

neutronclient/v2_0/client.py

@@ -256,8 +256,6 @@ class Client(object):
                      'net_partitions': 'net_partition',
                      'packet_filters': 'packet_filter',
                      }
-    # 8192 Is the default max URI len for eventlet.wsgi.server
-    MAX_URI_LEN = 8192
 
     def get_attr_metadata(self):
         if self.format == 'json':
@@ -1219,12 +1217,6 @@ class Client(object):
         # Raise the appropriate exception
         exception_handler_v20(status_code, des_error_body)
 
-    def _check_uri_length(self, action):
-        uri_len = len(self.httpclient.endpoint_url) + len(action)
-        if uri_len > self.MAX_URI_LEN:
-            raise exceptions.RequestURITooLong(
-                excess=uri_len - self.MAX_URI_LEN)
-
     def do_request(self, method, action, body=None, headers=None, params=None):
         # Add format and tenant_id
         action += ".%s" % self.format
@@ -1232,9 +1224,6 @@ class Client(object):
         if type(params) is dict and params:
             params = utils.safe_encode_dict(params)
             action += '?' + urlparse.urlencode(params, doseq=1)
-        # Ensure client always has correct uri - do not guesstimate anything
-        self.httpclient.authenticate_and_fetch_endpoint_url()
-        self._check_uri_length(action)
 
         if body:
             body = self.serialize(body)