From 03a23be8cc91440625f48a62cece259bfc03d78b Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Thu, 15 Oct 2015 10:05:16 +1100 Subject: [PATCH] Specify default_domain to generic plugin The generic plugin is supposed to work with both the v2 and v3 APIs. This doesn't necessarily work because you either need to provide domain information or not which implies specifying a v2 or v3 preference. By adding default domain we can allow using v2 or v3 authentication interchangeably. This is something that openstackclient does already. Closes-Bug: #1515041 Change-Id: I8d036a080a09b9310ffdd73d3501b6df29a212b9 --- keystoneauth1/discover.py | 11 +++-- keystoneauth1/identity/generic/base.py | 23 ++++++++-- keystoneauth1/identity/generic/password.py | 7 ++- keystoneauth1/loading/identity.py | 10 ++++ .../tests/unit/identity/test_password.py | 18 ++++++++ keystoneauth1/tests/unit/identity/utils.py | 46 +++++++++++++++++++ .../tests/unit/loading/test_generic.py | 10 +++- 7 files changed, 113 insertions(+), 12 deletions(-) diff --git a/keystoneauth1/discover.py b/keystoneauth1/discover.py index 4b05a70e..9f44e679 100644 --- a/keystoneauth1/discover.py +++ b/keystoneauth1/discover.py @@ -179,11 +179,15 @@ class Discover(object): return versions - def version_data(self, **kwargs): + @utils.positional() + def version_data(self, reverse=False, **kwargs): """Get normalized version data. Return version data in a structured way. + :param bool reverse: Reverse the list. reverse=true will mean the + returned list is sorted from newest to oldest + version. :returns: A list of version data dictionaries sorted by version number. Each data element in the returned list is a dictionary consisting of at least: @@ -231,7 +235,7 @@ class Discover(object): 'url': url, 'raw_status': v['status']}) - versions.sort(key=lambda v: v['version']) + versions.sort(key=lambda v: v['version'], reverse=reverse) return versions def data_for(self, version, **kwargs): @@ -247,9 +251,8 @@ class Discover(object): :rtype: dict """ version = normalize_version_number(version) - version_data = self.version_data(**kwargs) - for data in reversed(version_data): + for data in self.version_data(reverse=True, **kwargs): if version_match(version, data['version']): return data diff --git a/keystoneauth1/identity/generic/base.py b/keystoneauth1/identity/generic/base.py index e920cdc8..712f904c 100644 --- a/keystoneauth1/identity/generic/base.py +++ b/keystoneauth1/identity/generic/base.py @@ -41,7 +41,9 @@ class BaseGenericPlugin(base.BaseIdentityPlugin): project_domain_name=None, domain_id=None, domain_name=None, - trust_id=None): + trust_id=None, + default_domain_id=None, + default_domain_name=None): super(BaseGenericPlugin, self).__init__(auth_url=auth_url) self._project_id = project_id or tenant_id @@ -51,6 +53,8 @@ class BaseGenericPlugin(base.BaseIdentityPlugin): self._domain_id = domain_id self._domain_name = domain_name self._trust_id = trust_id + self._default_domain_id = default_domain_id + self._default_domain_name = default_domain_name self._plugin = None @@ -94,11 +98,14 @@ class BaseGenericPlugin(base.BaseIdentityPlugin): @property def _v3_params(self): """Parameters that are common to v3 plugins.""" + pr_domain_id = self._project_domain_id or self._default_domain_id + pr_domain_name = self._project_domain_name or self._default_domain_name + return {'trust_id': self._trust_id, 'project_id': self._project_id, 'project_name': self._project_name, - 'project_domain_id': self._project_domain_id, - 'project_domain_name': self._project_domain_name, + 'project_domain_id': pr_domain_id, + 'project_domain_name': pr_domain_name, 'domain_id': self._domain_id, 'domain_name': self._domain_name} @@ -128,7 +135,15 @@ class BaseGenericPlugin(base.BaseIdentityPlugin): plugin = self.create_plugin(session, (3, 0), self.auth_url) else: - disc_data = disc.version_data() + # NOTE(jamielennox): version_data is always in oldest to newest + # order. This is fine normally because we explicitly skip v2 below + # if there is domain data present. With default_domain params + # though we want a v3 plugin if available and fall back to v2 so we + # have to process in reverse order. FIXME(jamielennox): if we ever + # go for another version we should reverse this logic as we always + # want to favour the newest available version. + reverse = self._default_domain_id or self._default_domain_name + disc_data = disc.version_data(reverse=bool(reverse)) v2_with_domain_scope = False for data in disc_data: diff --git a/keystoneauth1/identity/generic/password.py b/keystoneauth1/identity/generic/password.py index 967fcc3a..f382ec08 100644 --- a/keystoneauth1/identity/generic/password.py +++ b/keystoneauth1/identity/generic/password.py @@ -53,10 +53,13 @@ class Password(base.BaseGenericPlugin): **self._v2_params) elif discover.version_match((3,), version): + u_domain_id = self._user_domain_id or self._default_domain_id + u_domain_name = self._user_domain_name or self._default_domain_name + return v3.Password(auth_url=url, user_id=self._user_id, username=self._username, - user_domain_id=self._user_domain_id, - user_domain_name=self._user_domain_name, + user_domain_id=u_domain_id, + user_domain_name=u_domain_name, password=self._password, **self._v3_params) diff --git a/keystoneauth1/loading/identity.py b/keystoneauth1/loading/identity.py index e500747e..203b1a05 100644 --- a/keystoneauth1/loading/identity.py +++ b/keystoneauth1/loading/identity.py @@ -147,6 +147,16 @@ class BaseGenericLoader(BaseIdentityLoader): opts.Opt('project-domain-name', help='Domain name containing project'), opts.Opt('trust-id', help='Trust ID'), + opts.Opt('default-domain-id', + help='Optional domain ID to use with v3 and v2 ' + 'parameters. It will be used for both the user ' + 'and project domain in v3 and ignored in ' + 'v2 authentication.'), + opts.Opt('default-domain-name', + help='Optional domain name to use with v3 API and v2 ' + 'parameters. It will be used for both the user ' + 'and project domain in v3 and ignored in ' + 'v2 authentication.'), ]) return options diff --git a/keystoneauth1/tests/unit/identity/test_password.py b/keystoneauth1/tests/unit/identity/test_password.py index 6657fe01..04f8be8e 100644 --- a/keystoneauth1/tests/unit/identity/test_password.py +++ b/keystoneauth1/tests/unit/identity/test_password.py @@ -53,3 +53,21 @@ class PasswordTests(utils.GenericPluginTestCase): def test_symbols(self): self.assertIs(v3.Password, v3_password.Password) self.assertIs(v3.PasswordMethod, v3_password.PasswordMethod) + + def test_default_domain_id_with_v3(self): + default_domain_id = uuid.uuid4().hex + + p = super(PasswordTests, self).test_default_domain_id_with_v3( + default_domain_id=default_domain_id) + + self.assertEqual(default_domain_id, + p._plugin.auth_methods[0].user_domain_id) + + def test_default_domain_name_with_v3(self): + default_domain_name = uuid.uuid4().hex + + p = super(PasswordTests, self).test_default_domain_name_with_v3( + default_domain_name=default_domain_name) + + self.assertEqual(default_domain_name, + p._plugin.auth_methods[0].user_domain_name) diff --git a/keystoneauth1/tests/unit/identity/utils.py b/keystoneauth1/tests/unit/identity/utils.py index 4036fa8e..2646eccf 100644 --- a/keystoneauth1/tests/unit/identity/utils.py +++ b/keystoneauth1/tests/unit/identity/utils.py @@ -126,3 +126,49 @@ class GenericPluginTestCase(utils.TestCase): # make a v4 entry that's mostly the same as a v3 self.stub_discovery(v2=False, v3_id='v4.0') self.assertDiscoveryFailure() + + def test_default_domain_id_with_v3(self, **kwargs): + self.stub_discovery() + project_name = uuid.uuid4().hex + default_domain_id = kwargs.setdefault('default_domain_id', + uuid.uuid4().hex) + + p = self.assertCreateV3(project_name=project_name, **kwargs) + + self.assertEqual(default_domain_id, p._plugin.project_domain_id) + self.assertEqual(project_name, p._plugin.project_name) + + return p + + def test_default_domain_id_no_v3(self): + self.stub_discovery(v3=False) + project_name = uuid.uuid4().hex + default_domain_id = uuid.uuid4().hex + + p = self.assertCreateV2(project_name=project_name, + default_domain_id=default_domain_id) + + self.assertEqual(project_name, p._plugin.tenant_name) + + def test_default_domain_name_with_v3(self, **kwargs): + self.stub_discovery() + project_name = uuid.uuid4().hex + default_domain_name = kwargs.setdefault('default_domain_name', + uuid.uuid4().hex) + + p = self.assertCreateV3(project_name=project_name, **kwargs) + + self.assertEqual(default_domain_name, p._plugin.project_domain_name) + self.assertEqual(project_name, p._plugin.project_name) + + return p + + def test_default_domain_name_no_v3(self): + self.stub_discovery(v3=False) + project_name = uuid.uuid4().hex + default_domain_name = uuid.uuid4().hex + + p = self.assertCreateV2(project_name=project_name, + default_domain_name=default_domain_name) + + self.assertEqual(project_name, p._plugin.tenant_name) diff --git a/keystoneauth1/tests/unit/loading/test_generic.py b/keystoneauth1/tests/unit/loading/test_generic.py index cbaca240..c3d85048 100644 --- a/keystoneauth1/tests/unit/loading/test_generic.py +++ b/keystoneauth1/tests/unit/loading/test_generic.py @@ -37,7 +37,10 @@ class PasswordTests(utils.TestCase): 'project-domain-id', 'project-domain-name', 'trust-id', - 'auth-url'] + 'auth-url', + 'default-domain-id', + 'default-domain-name', + ] self.assertEqual(set(allowed_opts), set(opts)) self.assertEqual(len(allowed_opts), len(opts)) @@ -74,7 +77,10 @@ class TokenTests(utils.TestCase): 'project-domain-id', 'project-domain-name', 'trust-id', - 'auth-url'] + 'auth-url', + 'default-domain-id', + 'default-domain-name', + ] self.assertEqual(set(allowed_opts), set(opts)) self.assertEqual(len(allowed_opts), len(opts))