From 98326c72f732481d73f2941827a1dae75c61388b Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Wed, 13 May 2015 16:38:44 +0000 Subject: Prevent attempts to "filter" list() calls by globally unique IDs This use case isn't covered by our current APIs: GET /entities?id={entity_id} Because we have a dedicated API for that: GET /entities/{entity_id} But our list() methods generally support **kwargs, which are passed as query parameters to keystone. When an 'id' is passed to keystone as a query parameter, keystone rightly ignores it and returns an unfiltered collection. This change raises a client-side TypeError (as you'd expect when you try to pass a keyword argument that a function isn't expecting), and includes a helpful suggestion to try calling get() instead. Change-Id: I100b69bbf571ad6de49ccc5ad1099c20b877d13d Closes-Bug: 1452298 --- keystoneclient/base.py | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'keystoneclient/base.py') diff --git a/keystoneclient/base.py b/keystoneclient/base.py index 025362b..eabbdc4 100644 --- a/keystoneclient/base.py +++ b/keystoneclient/base.py @@ -356,6 +356,17 @@ class CrudManager(Manager): @filter_kwargs def list(self, fallback_to_auth=False, **kwargs): + if 'id' in kwargs.keys(): + # Ensure that users are not trying to call things like + # ``domains.list(id='default')`` when they should have used + # ``[domains.get(domain_id='default')]`` instead. Keystone supports + # ``GET /v3/domains/{domain_id}``, not ``GET + # /v3/domains?id={domain_id}``. + raise TypeError( + _("list() got an unexpected keyword argument 'id'. To " + "retrieve a single object using a globally unique " + "identifier, try using get() instead.")) + url = self.build_url(dict_args_in_out=kwargs) try: -- cgit v1.2.1 From c5b03191b6714fed15bd88769c89e897257c337d Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Fri, 24 Jul 2015 14:14:13 -0500 Subject: Proper deprecation for Manager.api base.Manager's api property wasn't properly deprecated since all it had was documentation. Proper deprecation requires use of warnings and documentation. bp deprecations Change-Id: Ic5e218151e9b3f3b66f78729052680691d5ad582 --- keystoneclient/base.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'keystoneclient/base.py') diff --git a/keystoneclient/base.py b/keystoneclient/base.py index eabbdc4..d2c3ea0 100644 --- a/keystoneclient/base.py +++ b/keystoneclient/base.py @@ -21,6 +21,7 @@ Base utilities to build API operation managers and objects on top of. import abc import functools +import warnings import six from six.moves import urllib @@ -91,8 +92,17 @@ class Manager(object): @property def api(self): - """Deprecated. Use `client` instead. + """The client. + + .. warning:: + + This property is deprecated as of the 1.7.0 release in favor of + :meth:`client` and may be removed in the 2.0.0 release. + """ + warnings.warn( + 'api is deprecated as of the 1.7.0 release in favor of client and ' + 'may be removed in the 2.0.0 release', DeprecationWarning) return self.client def _list(self, url, response_key, obj_class=None, body=None, **kwargs): -- cgit v1.2.1 From 16e834dd4597314d79cf4fb0bb98449e6552f804 Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Wed, 5 Aug 2015 11:17:34 -0500 Subject: Move apiclient.base.Resource into keystoneclient keystoneclient is using apiclient.base and in order to properly deprecate and eventually get rid of apiclient we need to move the symbols that keystoneclient uses out of apiclient. This change moves apiclient.base.Resource into keystoneclient.base by merging apiclient.base.Resource into the existing keystoneclient.base.Resource. apiclient.base.Resource is now renaming keystoneclient.base.Resource for backwards-compatibility. Change-Id: Id479711b7c9437aaf171def6976aab8b303ec56d --- keystoneclient/base.py | 93 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 2 deletions(-) (limited to 'keystoneclient/base.py') diff --git a/keystoneclient/base.py b/keystoneclient/base.py index d2c3ea0..f19ed84 100644 --- a/keystoneclient/base.py +++ b/keystoneclient/base.py @@ -20,16 +20,17 @@ Base utilities to build API operation managers and objects on top of. """ import abc +import copy import functools import warnings +from oslo_utils import strutils import six from six.moves import urllib from keystoneclient import auth from keystoneclient import exceptions from keystoneclient.i18n import _ -from keystoneclient.openstack.common.apiclient import base def getid(obj): @@ -439,11 +440,99 @@ class CrudManager(Manager): return rl[0] -class Resource(base.Resource): +class Resource(object): """Base class for OpenStack resources (tenant, user, etc.). This is pretty much just a bag for attributes. """ + HUMAN_ID = False + NAME_ATTR = 'name' + + def __init__(self, manager, info, loaded=False): + """Populate and bind to a manager. + + :param manager: BaseManager object + :param info: dictionary representing resource attributes + :param loaded: prevent lazy-loading if set to True + """ + self.manager = manager + self._info = info + self._add_details(info) + self._loaded = loaded + + def __repr__(self): + reprkeys = sorted(k + for k in self.__dict__.keys() + if k[0] != '_' and k != 'manager') + info = ", ".join("%s=%s" % (k, getattr(self, k)) for k in reprkeys) + return "<%s %s>" % (self.__class__.__name__, info) + + @property + def human_id(self): + """Human-readable ID which can be used for bash completion. + """ + if self.HUMAN_ID: + name = getattr(self, self.NAME_ATTR, None) + if name is not None: + return strutils.to_slug(name) + return None + + def _add_details(self, info): + for (k, v) in six.iteritems(info): + try: + setattr(self, k, v) + self._info[k] = v + except AttributeError: + # In this case we already defined the attribute on the class + pass + + def __getattr__(self, k): + if k not in self.__dict__: + # NOTE(bcwaldon): disallow lazy-loading if already loaded once + if not self.is_loaded(): + self.get() + return self.__getattr__(k) + + raise AttributeError(k) + else: + return self.__dict__[k] + + def get(self): + """Support for lazy loading details. + + Some clients, such as novaclient have the option to lazy load the + details, details which can be loaded with this function. + """ + # set_loaded() first ... so if we have to bail, we know we tried. + self.set_loaded(True) + if not hasattr(self.manager, 'get'): + return + + new = self.manager.get(self.id) + if new: + self._add_details(new._info) + self._add_details( + {'x_request_id': self.manager.client.last_request_id}) + + def __eq__(self, other): + if not isinstance(other, Resource): + return NotImplemented + # two resources of different types are not equal + if not isinstance(other, self.__class__): + return False + if hasattr(self, 'id') and hasattr(other, 'id'): + return self.id == other.id + return self._info == other._info + + def is_loaded(self): + return self._loaded + + def set_loaded(self, val): + self._loaded = val + + def to_dict(self): + return copy.deepcopy(self._info) + def delete(self): return self.manager.delete(self) -- cgit v1.2.1