From d1a0c1f40574192d0ad5bb3943f7109496a622e5 Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Tue, 19 Nov 2013 16:49:16 +0100 Subject: [PATCH] Remove the dependency on oslo.config Client libraries shouldn't depend on specific configuration libraries, since that will make it difficult for other applications to adopt the library. This patch replaces all oslo.config usages with dictionaries. The client will use those as options holders and all parameters are considered optional. Notice that `options` was preferred instead of `**options` since this dictionary is considered a 'config object' and not a set of optional parameters. This will make it easier for applications using marconiclient to hold a single, mutable instance of options to pass around. Partially-Implements blueprint python-marconiclient-v1 Change-Id: Ib41f2957689545f05dab19338842fd9a953f2f76 --- marconiclient/auth/__init__.py | 23 ++++--- marconiclient/auth/keystone.py | 93 ++++++++++------------------ marconiclient/queues/v1/client.py | 47 +++++++------- marconiclient/queues/v1/queues.py | 5 +- marconiclient/tests/base.py | 12 ++-- marconiclient/tests/queues/queues.py | 4 +- marconiclient/transport/__init__.py | 38 +++--------- marconiclient/transport/base.py | 4 +- marconiclient/transport/http.py | 4 +- marconiclient/transport/request.py | 8 +-- tests/auth/test_base.py | 7 +-- tests/auth/test_keystone.py | 10 +-- 12 files changed, 95 insertions(+), 160 deletions(-) diff --git a/marconiclient/auth/__init__.py b/marconiclient/auth/__init__.py index 7c76d97b..e29ad73b 100644 --- a/marconiclient/auth/__init__.py +++ b/marconiclient/auth/__init__.py @@ -14,7 +14,6 @@ # limitations under the License. -from oslo.config import cfg import six from marconiclient.auth import base @@ -34,18 +33,18 @@ if keystone: _BACKENDS['keystone'] = keystone.KeystoneAuth -def _register_opts(conf): - """Registers auth cli options. +def get_backend(backend='noauth', options=None): + """Loads backend `auth_backend` - This function exists mostly for testing - purposes. + :params backend: The backend name to load. + Default: `noauth` + :type backend: `six.string_types` + :param options: Options to pass to the Auth + backend. Refer to the backend for more info. + :type options: `dict` """ - backend_opt = cfg.StrOpt('auth_backend', default='noauth', - help='Backend plugin to use for authentication') - conf.register_cli_opt(backend_opt) + if options is None: + options = {} - -def get_backend(conf): - _register_opts(conf) - backend = _BACKENDS[conf.auth_backend](conf) + backend = _BACKENDS[backend](options) return backend diff --git a/marconiclient/auth/keystone.py b/marconiclient/auth/keystone.py index 3369b1d1..5b23d41b 100644 --- a/marconiclient/auth/keystone.py +++ b/marconiclient/auth/keystone.py @@ -13,55 +13,31 @@ # See the License for the specific language governing permissions and # limitations under the License. -from oslo.config import cfg - from keystoneclient.v2_0 import client as ksclient from marconiclient.auth import base -from marconiclient.common import utils # NOTE(flaper87): Some of the code below # was brought to you by the very unique # work of ceilometerclient. class KeystoneAuth(base.AuthBackend): + """Keystone Auth backend - _CLI_OPTIONS = [ - cfg.StrOpt("os_username", default=utils.env('OS_USERNAME'), - help='Defaults to env[OS_USERNAME]'), - - cfg.StrOpt("os_password", default=utils.env('OS_PASSWORD'), - help='Defaults to env[OS_PASSWORD]'), - - cfg.StrOpt("os_project_id", default=utils.env('OS_PROJECT_ID'), - help='Defaults to env[OS_PROJECT_ID]'), - - cfg.StrOpt("os_project_name", default=utils.env('OS_PROJECT_NAME'), - help='Defaults to env[OS_PROJECT_NAME]'), - - cfg.StrOpt("os_auth_url", default=utils.env('OS_AUTH_URL'), - help='Defaults to env[OS_AUTH_URL]'), - - cfg.StrOpt("os_auth_token", default=utils.env('OS_AUTH_TOKEN'), - help='Defaults to env[OS_AUTH_TOKEN]'), - - cfg.StrOpt("os_region_name", default=utils.env('OS_REGION_NAME'), - help='Defaults to env[OS_REGION_NAME]'), - - cfg.StrOpt("os_service_type", default=utils.env('OS_SERVICE_TYPE'), - help='Defaults to env[OS_SERVICE_TYPE]'), - - cfg.StrOpt("os_service_type", default=utils.env('OS_SERVICE_TYPE'), - help='Defaults to env[OS_SERVICE_TYPE]'), - - cfg.StrOpt("os_endpoint_type", default=utils.env('OS_ENDPOINT_TYPE'), - help='Defaults to env[OS_ENDPOINT_TYPE]'), - - ] - - def __init__(self, conf): - super(KeystoneAuth, self).__init__(conf) - conf.register_cli_opts(self._CLI_OPTIONS) + :params conf: A dictionary with Keystone's + custom parameters: + - os_username + - os_password + - os_project_id + - os_project_name + - os_auth_url + - os_auth_token + - os_region_name + - os_service_type + - os_service_type + - os_endpoint_type + :type conf: `dict` + """ def _get_ksclient(self, **kwargs): """Get an endpoint and auth token from Keystone. @@ -73,18 +49,11 @@ class KeystoneAuth(base.AuthBackend): * insecure: allow insecure SSL (no cert verification) * project_{name|id}: name or ID of project """ - return ksclient.Client(username=self.conf.os_username, - password=self.conf.os_password, - tenant_id=self.conf.os_project_id, - tenant_name=self.conf.os_project_name, - auth_url=self.conf.os_auth_url, - insecure=self.conf.insecure) + return ksclient.Client(**kwargs) - def _get_endpoint(self, client): + def _get_endpoint(self, client, **extra): """Get an endpoint using the provided keystone client.""" - return client.service_catalog.url_for( - service_type=self.conf.service_type or 'queuing', - endpoint_type=self.conf.endpoint_type or 'publicURL') + return client.service_catalog.url_for(**extra) def authenticate(self, api_version, request): """Get an authtenticated client, based on the credentials @@ -95,21 +64,19 @@ class KeystoneAuth(base.AuthBackend): the auth information. """ - token = self.conf.os_auth_token - if not self.conf.os_auth_token or not request.endpoint: + token = self.conf.get('os_auth_token') + if not token or not request.endpoint: # NOTE(flaper87): Lets assume all the # required information was provided # either through env variables or CLI # params. Let keystoneclient fail otherwise. ks_kwargs = { - 'username': self.conf.os_username, - 'password': self.conf.os_password, - 'tenant_id': self.conf.os_project_id, - 'tenant_name': self.conf.os_project_name, - 'auth_url': self.conf.os_auth_url, - 'service_type': self.conf.os_service_type, - 'endpoint_type': self.conf.os_endpoint_type, - 'insecure': self.conf.insecure, + 'username': self.conf.get('os_username'), + 'password': self.conf.get('os_password'), + 'tenant_id': self.conf.get('os_project_id'), + 'tenant_name': self.conf.get('os_project_name'), + 'auth_url': self.conf.get('os_auth_url'), + 'insecure': self.conf.get('insecure'), } _ksclient = self._get_ksclient(**ks_kwargs) @@ -118,7 +85,13 @@ class KeystoneAuth(base.AuthBackend): token = _ksclient.auth_token if not request.endpoint: - request.endpoint = self._get_endpoint(_ksclient, **ks_kwargs) + extra = { + 'service_type': self.conf.get('os_service_type', + 'queuing'), + 'endpoint_type': self.conf.get('os_endpoint_type', + 'publicURL'), + } + request.endpoint = self._get_endpoint(_ksclient, **extra) # NOTE(flaper87): Update the request spec # with the final token. diff --git a/marconiclient/queues/v1/client.py b/marconiclient/queues/v1/client.py index a31db9d4..f60c934f 100644 --- a/marconiclient/queues/v1/client.py +++ b/marconiclient/queues/v1/client.py @@ -15,40 +15,39 @@ import uuid -from oslo.config import cfg - from marconiclient.queues.v1 import queues from marconiclient import transport -_CLIENT_OPTIONS = [ - cfg.StrOpt('os_queues_url', - help='Queues remote URL'), - - cfg.StrOpt('client_uuid', - default=uuid.uuid4().hex, - help='Client UUID'), -] - - class Client(object): + """Client base class - def __init__(self, conf, url=None, version=1): - self.conf = conf + :param url: Marconi's instance base url. + :type url: `six.text_type` + :param version: API Version pointing to. + :type version: `int` + :param options: Extra options: + - client_uuid: Custom client uuid. A new one + will be generated, if not passed. + - auth_opts: Authentication options: + - backend + - options + :type options: `dict` + """ - # NOTE(flaper87): This won't actually register - # the CLI options until the class is instantiated - # which is dumb. It'll refactored when the CLI API - # work starts. - self.conf.register_cli_opts(_CLIENT_OPTIONS) - self.api_url = self.conf.os_queues_url or url + def __init__(self, url=None, version=1, conf=None): + self.conf = conf or {} + + self.api_url = url self.api_version = version - - self.client_uuid = self.conf.client_uuid + self.auth_opts = self.conf.get('auth_opts', {}) + self.client_uuid = self.conf.get('client_uuid', + uuid.uuid4().hex) def transport(self): - """Gets a transport based on conf.""" - return transport.get_transport_for_conf(self.conf) + """Gets a transport based the api url and version.""" + return transport.get_transport_for(self.url, + self.api_version) def queue(self, ref, **kwargs): """Returns a queue instance diff --git a/marconiclient/queues/v1/queues.py b/marconiclient/queues/v1/queues.py index db7f5256..a65d32b9 100644 --- a/marconiclient/queues/v1/queues.py +++ b/marconiclient/queues/v1/queues.py @@ -43,12 +43,13 @@ class Queue(object): :type request: `transport.request.Request` """ - trans = transport.get_transport_for(self.client.conf, request) + trans = transport.get_transport_for(request, + options=self.client.conf) return (trans or self.client.transport) def _request_and_transport(self): api = 'queues.v' + str(self.client.api_version) - req = request.prepare_request(self.client.conf, + req = request.prepare_request(self.client.auth_opts, endpoint=self.client.api_url, api=api) diff --git a/marconiclient/tests/base.py b/marconiclient/tests/base.py index 7815c13c..9d974eeb 100644 --- a/marconiclient/tests/base.py +++ b/marconiclient/tests/base.py @@ -16,15 +16,13 @@ import fixtures import testtools -from oslo.config import cfg - class TestBase(testtools.TestCase): def setUp(self): super(TestBase, self).setUp() - self.conf = cfg.ConfigOpts() + self.conf = {} self.useFixture(fixtures.FakeLogger('marconi')) # NOTE(kgriffs): Don't monkey-patch stdout since it breaks @@ -40,9 +38,7 @@ class TestBase(testtools.TestCase): If a group argument is supplied, the overrides are applied to the specified configuration option group. - - All overrides are automatically cleared at the end of the current - test by the tearDown() method. """ - for k, v in kw.items(): - self.conf.set_override(k, v, group) + parent = (group and self.conf.setdefault(group, {}) + or self.conf) + parent.update(kw) diff --git a/marconiclient/tests/queues/queues.py b/marconiclient/tests/queues/queues.py index 5c1aa263..dce721d6 100644 --- a/marconiclient/tests/queues/queues.py +++ b/marconiclient/tests/queues/queues.py @@ -42,8 +42,8 @@ class QueuesV1QueueTestBase(base.TestBase): super(QueuesV1QueueTestBase, self).setUp() self.transport = self.transport_cls(self.conf) - self.client = client.Client(self.conf, self.url, - self.version) + self.client = client.Client(self.url, self.version, + self.conf) # NOTE(flaper87): Nasty monkeypatch, lets use # the dummy transport here. diff --git a/marconiclient/transport/__init__.py b/marconiclient/transport/__init__.py index 2e7212ef..9532321c 100644 --- a/marconiclient/transport/__init__.py +++ b/marconiclient/transport/__init__.py @@ -13,29 +13,21 @@ # See the License for the specific language governing permissions and # limitations under the License. -from oslo.config import cfg import six from six.moves.urllib import parse from stevedore import driver from marconiclient import errors as _errors -_TRANSPORT_OPTIONS = [ - cfg.StrOpt('default_transport', default='http', - help='Transport to use as default'), - cfg.IntOpt('default_transport_version', default=1, - help='Transport to use as default'), -] - -def get_transport(conf, transport, version=1): +def get_transport(transport='http', version=1, options=None): """Gets a transport and returns it. - :param conf: the user configuration - :type conf: cfg.ConfigOpts - :param transport: Transport name + :param transport: Transport name. + Default: http :type transport: `six.string_types` :param version: Version of the target transport. + Default: 1 :type version: int :returns: A `Transport` instance. @@ -49,36 +41,20 @@ def get_transport(conf, transport, version=1): mgr = driver.DriverManager(namespace, entry_point, invoke_on_load=True, - invoke_args=[conf]) + invoke_args=[options]) except RuntimeError as ex: raise _errors.DriverLoadFailure(entry_point, ex) return mgr.driver -def get_transport_for_conf(conf): - """Gets a transport based on the config object - - It'll load a transport based on the `default-transport` - and `default-transport-version` params. - - :param conf: the user configuration - :type conf: cfg.ConfigOpts - """ - conf.register_opts(_TRANSPORT_OPTIONS) - return get_transport(conf, conf.default_transport, - conf.default_transport_version) - - -def get_transport_for(conf, url_or_request, version=1): +def get_transport_for(url_or_request, version=1, options=None): """Gets a transport for a given url. An example transport URL might be:: zmq://example.org:8888/v1/ - :param conf: the user configuration - :type conf: cfg.ConfigOpts :param url_or_request: a transport URL :type url_or_request: `six.string_types` or `marconiclient.transport.request.Request` @@ -94,4 +70,4 @@ def get_transport_for(conf, url_or_request, version=1): url = url_or_request.endpoint parsed = parse.urlparse(url) - return get_transport(conf, parsed.scheme, version) + return get_transport(parsed.scheme, version, options) diff --git a/marconiclient/transport/base.py b/marconiclient/transport/base.py index e507958d..0cca86ad 100644 --- a/marconiclient/transport/base.py +++ b/marconiclient/transport/base.py @@ -21,8 +21,8 @@ import six @six.add_metaclass(abc.ABCMeta) class Transport(object): - def __init__(self, conf): - self.conf = conf + def __init__(self, options): + self.options = options @abc.abstractmethod def send(self, request): diff --git a/marconiclient/transport/http.py b/marconiclient/transport/http.py index 80ca8cb4..835e858d 100644 --- a/marconiclient/transport/http.py +++ b/marconiclient/transport/http.py @@ -32,8 +32,8 @@ class HttpTransport(base.Transport): 404: errors.ResourceNotFound, } - def __init__(self, conf): - super(HttpTransport, self).__init__(conf) + def __init__(self, options): + super(HttpTransport, self).__init__(options) self.client = http.Client() def _prepare(self, request): diff --git a/marconiclient/transport/request.py b/marconiclient/transport/request.py index 8aa4c3b4..4edb70e0 100644 --- a/marconiclient/transport/request.py +++ b/marconiclient/transport/request.py @@ -21,7 +21,7 @@ from marconiclient import auth from marconiclient import errors -def prepare_request(conf, data=None, **kwargs): +def prepare_request(auth_opts=None, data=None, **kwargs): """Prepares a request This method takes care of authentication @@ -30,8 +30,8 @@ def prepare_request(conf, data=None, **kwargs): The request returned by this call is ready to be sent to the server. - :param conf: `cfg.ConfigOpts` instance to use. - :type conf: `cfg.ConfigOpts` + :param auth_opts: Auth parameters + :type auth_opts: `dict` :param data: Optional data to send along with the request. If data is not None, it'll be serialized. :type data: Any primitive type that is json-serializable. @@ -42,7 +42,7 @@ def prepare_request(conf, data=None, **kwargs): """ req = Request(**kwargs) - auth_backend = auth.get_backend(conf) + auth_backend = auth.get_backend(**(auth_opts or {})) # TODO(flaper87): Do something smarter # to get the api_version. req = auth_backend.authenticate(1, req) diff --git a/tests/auth/test_base.py b/tests/auth/test_base.py index c50d40b4..c5d40ac4 100644 --- a/tests/auth/test_base.py +++ b/tests/auth/test_base.py @@ -21,16 +21,13 @@ class TestBaseAuth(base.TestBase): def test_get_backend(self): try: - auth.get_backend(self.conf) + auth.get_backend(options=self.conf) except KeyError: self.fail("Test failed") def test_get_non_existing_backend(self): - auth._register_opts(self.conf) - self.config(auth_backend="not_existing") - try: - auth.get_backend(self.conf) + auth.get_backend('not_existing') self.fail("Test failed") except KeyError: pass diff --git a/tests/auth/test_keystone.py b/tests/auth/test_keystone.py index 0484dd17..df02d20b 100644 --- a/tests/auth/test_keystone.py +++ b/tests/auth/test_keystone.py @@ -15,7 +15,6 @@ import mock -from oslo.config import cfg try: from keystoneclient.v2_0 import client as ksclient @@ -42,13 +41,8 @@ class TestKeystoneAuth(base.TestBase): if not ksclient: self.skipTest('Keystone client is not installed') - auth._register_opts(self.conf) - self.config(auth_backend='keystone') - self.auth = auth.get_backend(self.conf) - - # FIXME(flaper87): Remove once insecure is added - # in the right place. - self.conf.register_opt(cfg.BoolOpt('insecure', default=False)) + self.auth = auth.get_backend(backend='keystone', + options=self.conf) def test_no_token(self): test_endpoint = 'http://example.org:8888'