diff --git a/keystoneclient/access.py b/keystoneclient/access.py index e54e9868a..e2ba5c0e3 100644 --- a/keystoneclient/access.py +++ b/keystoneclient/access.py @@ -33,21 +33,28 @@ class AccessInfo(dict): """ @classmethod - def factory(cls, resp=None, body=None, **kwargs): + def factory(cls, resp=None, body=None, region_name=None, **kwargs): """Create AccessInfo object given a successful auth response & body or a user-provided dict. """ + # FIXME(jamielennox): Passing region_name is deprecated. Provide an + # appropriate warning. + if body is not None or len(kwargs): if AccessInfoV3.is_valid(body, **kwargs): token = None if resp: token = resp.headers['X-Subject-Token'] if body: + if region_name: + body['token']['region_name'] = region_name return AccessInfoV3(token, **body['token']) else: return AccessInfoV3(token, **kwargs) elif AccessInfoV2.is_valid(body, **kwargs): if body: + if region_name: + body['access']['region_name'] = region_name return AccessInfoV2(**body['access']) else: return AccessInfoV2(**kwargs) @@ -59,7 +66,11 @@ class AccessInfo(dict): def __init__(self, *args, **kwargs): super(AccessInfo, self).__init__(*args, **kwargs) self.service_catalog = service_catalog.ServiceCatalog.factory( - resource_dict=self, region_name=self.get('region_name')) + resource_dict=self, region_name=self._region_name) + + @property + def _region_name(self): + return self.get('region_name') def will_expire_soon(self, stale_duration=None): """Determines if expiration is about to occur. @@ -272,6 +283,9 @@ class AccessInfo(dict): request. If the authentication request wasn't scoped to a tenant (project), this property will return None. + DEPRECATED: this doesn't correctly handle region name. You should fetch + it from the service catalog yourself. + :returns: tuple of urls """ raise NotImplementedError() @@ -282,6 +296,9 @@ class AccessInfo(dict): associated with the authorization request, or None if the authentication request wasn't scoped to a tenant (project). + DEPRECATED: this doesn't correctly handle region name. You should fetch + it from the service catalog yourself. + :returns: tuple of urls """ raise NotImplementedError() @@ -306,7 +323,7 @@ class AccessInfoV2(AccessInfo): self.service_catalog = service_catalog.ServiceCatalog.factory( resource_dict=self, token=self['token']['id'], - region_name=self.get('region_name')) + region_name=self._region_name) @classmethod def is_valid(cls, body, **kwargs): @@ -430,17 +447,23 @@ class AccessInfoV2(AccessInfo): @property def auth_url(self): + # FIXME(jamielennox): this is deprecated in favour of retrieving it + # from the service catalog. Provide a warning. if self.service_catalog: return self.service_catalog.get_urls(service_type='identity', - endpoint_type='publicURL') + endpoint_type='publicURL', + region_name=self._region_name) else: return None @property def management_url(self): + # FIXME(jamielennox): this is deprecated in favour of retrieving it + # from the service catalog. Provide a warning. if self.service_catalog: return self.service_catalog.get_urls(service_type='identity', - endpoint_type='adminURL') + endpoint_type='adminURL', + region_name=self._region_name) else: return None @@ -456,7 +479,7 @@ class AccessInfoV3(AccessInfo): self.service_catalog = service_catalog.ServiceCatalog.factory( resource_dict=self, token=token, - region_name=self.get('region_name')) + region_name=self._region_name) if token: self.update(auth_token=token) @@ -554,16 +577,23 @@ class AccessInfoV3(AccessInfo): @property def auth_url(self): + # FIXME(jamielennox): this is deprecated in favour of retrieving it + # from the service catalog. Provide a warning. if self.service_catalog: return self.service_catalog.get_urls(service_type='identity', - endpoint_type='public') + endpoint_type='public', + region_name=self._region_name) else: return None @property def management_url(self): + # FIXME(jamielennox): this is deprecated in favour of retrieving it + # from the service catalog. Provide a warning. if self.service_catalog: return self.service_catalog.get_urls(service_type='identity', - endpoint_type='admin') + endpoint_type='admin', + region_name=self._region_name) + else: return None diff --git a/keystoneclient/httpclient.py b/keystoneclient/httpclient.py index 9daeeb94d..a0dcf24b1 100644 --- a/keystoneclient/httpclient.py +++ b/keystoneclient/httpclient.py @@ -139,6 +139,7 @@ class HTTPClient(object): self.project_domain_id = None self.project_domain_name = None + self.region_name = None self.auth_url = None self._endpoint = None self._management_url = None @@ -162,6 +163,8 @@ class HTTPClient(object): self._management_url = self.auth_ref.management_url[0] self.auth_token = self.auth_ref.auth_token self.trust_id = self.auth_ref.trust_id + if self.auth_ref.has_service_catalog(): + self.region_name = self.auth_ref.service_catalog.region_name else: self.auth_ref = None @@ -218,7 +221,8 @@ class HTTPClient(object): self.auth_token_from_user = None if endpoint: self._endpoint = endpoint.rstrip('/') - self.region_name = region_name + if region_name: + self.region_name = region_name if not session: verify = cacert or True @@ -304,7 +308,8 @@ class HTTPClient(object): user_id=None, domain_name=None, domain_id=None, project_name=None, project_id=None, user_domain_id=None, user_domain_name=None, project_domain_id=None, - project_domain_name=None, trust_id=None): + project_domain_name=None, trust_id=None, + region_name=None): """Authenticate user. Uses the data provided at instantiation to authenticate against @@ -363,6 +368,7 @@ class HTTPClient(object): project_domain_name = project_domain_name or self.project_domain_name trust_id = trust_id or self.trust_id + region_name = region_name or self.region_name if not token: token = self.auth_token_from_user @@ -391,10 +397,14 @@ class HTTPClient(object): new_token_needed = True kwargs['password'] = password resp, body = self.get_raw_token_from_identity_service(**kwargs) - self.auth_ref = access.AccessInfo.factory(resp, body) + + # TODO(jamielennox): passing region_name here is wrong but required + # for backwards compatibility. Deprecate and provide warning. + self.auth_ref = access.AccessInfo.factory(resp, body, + region_name=region_name) else: self.auth_ref = auth_ref - self.process_token() + self.process_token(region_name=region_name) if new_token_needed: self.store_auth_ref_into_keyring(keyring_key) return True @@ -449,7 +459,7 @@ class HTTPClient(object): except Exception as e: _logger.warning("Failed to store token into keyring %s" % (e)) - def process_token(self): + def process_token(self, region_name=None): """Extract and process information from the new auth_ref. And set the relevant authentication information. @@ -461,8 +471,14 @@ class HTTPClient(object): if not self.auth_ref.tenant_id: raise exceptions.AuthorizationFailure( "Token didn't provide tenant_id") - if self.auth_ref.management_url: - self._management_url = self.auth_ref.management_url[0] + try: + self._management_url = self.auth_ref.service_catalog.url_for( + service_type='identity', + endpoint_type='admin', + region_name=region_name or self.region_name) + except exceptions.EndpointNotFound: + _logger.warning("Failed to retrieve management_url from token") + self.project_name = self.auth_ref.tenant_name self.project_id = self.auth_ref.tenant_id @@ -511,14 +527,6 @@ class HTTPClient(object): """ raise NotImplementedError - def _extract_service_catalog(self, url, body): - """Set the client's service catalog from the response data. - - Not implemented here because data returned may be API - version-specific. - """ - raise NotImplementedError - def serialize(self, entity): return jsonutils.dumps(entity) diff --git a/keystoneclient/service_catalog.py b/keystoneclient/service_catalog.py index b380c5b11..6b89fe4c4 100644 --- a/keystoneclient/service_catalog.py +++ b/keystoneclient/service_catalog.py @@ -37,6 +37,16 @@ class ServiceCatalog(object): else: raise NotImplementedError('Unrecognized auth response') + def __init__(self, region_name=None): + self._region_name = region_name + + @property + def region_name(self): + # FIXME(jamielennox): Having region_name set on the service catalog + # directly is deprecated. It should instead be provided as a parameter + # to calls made to the service_catalog. Provide appropriate warning. + return self._region_name + @abc.abstractmethod def get_token(self): """Fetch token details from service catalog. @@ -72,13 +82,15 @@ class ServiceCatalog(object): service catalog. """ - def get_endpoints(self, service_type=None, endpoint_type=None): + def get_endpoints(self, service_type=None, endpoint_type=None, + region_name=None): """Fetch and filter endpoints for the specified service(s). - Returns endpoints for the specified service (or all) and - that contain the specified type (or all). + Returns endpoints for the specified service (or all) containing + the specified type (or all) and region (or all). """ endpoint_type = self._normalize_endpoint_type(endpoint_type) + region_name = region_name or self._region_name sc = {} @@ -97,20 +109,20 @@ class ServiceCatalog(object): if (endpoint_type and not self._is_endpoint_type_match(endpoint, endpoint_type)): continue - if (self.region_name and - self.region_name != endpoint.get('region')): + if region_name and region_name != endpoint.get('region'): continue sc[st].append(endpoint) return sc def _get_service_endpoints(self, attr, filter_value, service_type, - endpoint_type): + endpoint_type, region_name): """Fetch the endpoints of a particular service_type and handle the filtering. """ sc_endpoints = self.get_endpoints(service_type=service_type, - endpoint_type=endpoint_type) + endpoint_type=endpoint_type, + region_name=region_name) try: endpoints = sc_endpoints[service_type] @@ -129,7 +141,8 @@ class ServiceCatalog(object): @abc.abstractmethod def get_urls(self, attr=None, filter_value=None, - service_type='identity', endpoint_type='publicURL'): + service_type='identity', endpoint_type='publicURL', + region_name=None): """Fetch endpoint urls from the service catalog. Fetch the endpoints from the service catalog for a particular @@ -150,7 +163,8 @@ class ServiceCatalog(object): raise NotImplementedError() def url_for(self, attr=None, filter_value=None, - service_type='identity', endpoint_type='publicURL'): + service_type='identity', endpoint_type='publicURL', + region_name=None): """Fetch an endpoint from the service catalog. Fetch the specified endpoint from the service catalog for @@ -167,16 +181,19 @@ class ServiceCatalog(object): urls = self.get_urls(attr=attr, filter_value=filter_value, service_type=service_type, - endpoint_type=endpoint_type) + endpoint_type=endpoint_type, + region_name=region_name) try: return urls[0] except Exception: pass - msg = '%(endpoint)s endpoint for %(service)s not found' + msg = '%(endpoint)s endpoint for %(service)s%(region)s not found' + region = ' in %s region' % region_name if region_name else '' msg = msg % {'endpoint': endpoint_type, - 'service': service_type} + 'service': service_type, + 'region': region} raise exceptions.EndpointNotFound(msg) @@ -199,7 +216,7 @@ class ServiceCatalogV2(ServiceCatalog): def __init__(self, resource_dict, region_name=None): self.catalog = resource_dict - self.region_name = region_name + super(ServiceCatalogV2, self).__init__(region_name=region_name) @classmethod def is_valid(cls, resource_dict): @@ -232,12 +249,14 @@ class ServiceCatalogV2(ServiceCatalog): return token def get_urls(self, attr=None, filter_value=None, - service_type='identity', endpoint_type='publicURL'): + service_type='identity', endpoint_type='publicURL', + region_name=None): endpoint_type = self._normalize_endpoint_type(endpoint_type) endpoints = self._get_service_endpoints(attr=attr, filter_value=filter_value, service_type=service_type, - endpoint_type=endpoint_type) + endpoint_type=endpoint_type, + region_name=region_name) if endpoints: return tuple([endpoint[endpoint_type] for endpoint in endpoints]) @@ -251,9 +270,9 @@ class ServiceCatalogV3(ServiceCatalog): """ def __init__(self, token, resource_dict, region_name=None): + super(ServiceCatalogV3, self).__init__(region_name=region_name) self._auth_token = token self.catalog = resource_dict - self.region_name = region_name @classmethod def is_valid(cls, resource_dict): @@ -294,11 +313,13 @@ class ServiceCatalogV3(ServiceCatalog): return token def get_urls(self, attr=None, filter_value=None, - service_type='identity', endpoint_type='public'): + service_type='identity', endpoint_type='public', + region_name=None): endpoints = self._get_service_endpoints(attr=attr, filter_value=filter_value, service_type=service_type, - endpoint_type=endpoint_type) + endpoint_type=endpoint_type, + region_name=region_name) if endpoints: return tuple([endpoint['url'] for endpoint in endpoints]) diff --git a/keystoneclient/tests/v2_0/test_client.py b/keystoneclient/tests/v2_0/test_client.py index bf14fdcee..b7f5354df 100644 --- a/keystoneclient/tests/v2_0/test_client.py +++ b/keystoneclient/tests/v2_0/test_client.py @@ -130,3 +130,25 @@ class KeystoneClientTest(utils.TestCase): self.stub_auth(json=second) cl.authenticate() self.assertEqual(cl.management_url, second_url % 35357) + + @httpretty.activate + def test_client_with_region_name_passes_to_service_catalog(self): + # NOTE(jamielennox): this is deprecated behaviour that should be + # removed ASAP, however must remain compatible. + self.stub_auth(json=client_fixtures.AUTH_RESPONSE_BODY) + + cl = client.Client(username='exampleuser', + password='password', + tenant_name='exampleproject', + auth_url=self.TEST_URL, + region_name='North') + self.assertEqual(cl.service_catalog.url_for(service_type='image'), + 'https://image.north.host/v1/') + + cl = client.Client(username='exampleuser', + password='password', + tenant_name='exampleproject', + auth_url=self.TEST_URL, + region_name='South') + self.assertEqual(cl.service_catalog.url_for(service_type='image'), + 'https://image.south.host/v1/') diff --git a/keystoneclient/tests/v2_0/test_service_catalog.py b/keystoneclient/tests/v2_0/test_service_catalog.py index 08212472f..237901626 100644 --- a/keystoneclient/tests/v2_0/test_service_catalog.py +++ b/keystoneclient/tests/v2_0/test_service_catalog.py @@ -77,3 +77,86 @@ class ServiceCatalogTest(utils.TestCase): auth_ref.service_catalog.url_for, service_type='image', endpoint_type='internalURL') + + def test_service_catalog_get_endpoints_region_names(self): + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + endpoints = sc.get_endpoints(service_type='image', region_name='North') + self.assertEqual(len(endpoints), 1) + self.assertEqual(endpoints['image'][0]['publicURL'], + 'https://image.north.host/v1/') + + endpoints = sc.get_endpoints(service_type='image', region_name='South') + self.assertEqual(len(endpoints), 1) + self.assertEqual(endpoints['image'][0]['publicURL'], + 'https://image.south.host/v1/') + + endpoints = sc.get_endpoints(service_type='compute') + self.assertEqual(len(endpoints['compute']), 2) + + endpoints = sc.get_endpoints(service_type='compute', + region_name='North') + self.assertEqual(len(endpoints['compute']), 2) + + endpoints = sc.get_endpoints(service_type='compute', + region_name='West') + self.assertEqual(len(endpoints['compute']), 0) + + def test_service_catalog_url_for_region_names(self): + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + url = sc.url_for(service_type='image', region_name='North') + self.assertEqual(url, 'https://image.north.host/v1/') + + url = sc.url_for(service_type='image', region_name='South') + self.assertEqual(url, 'https://image.south.host/v1/') + + url = sc.url_for(service_type='compute', + region_name='North', + attr='versionId', + filter_value='1.1') + self.assertEqual(url, 'https://compute.north.host/v1.1/3456') + + self.assertRaises(exceptions.EndpointNotFound, sc.url_for, + service_type='image', region_name='West') + + def test_servcie_catalog_get_url_region_names(self): + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + urls = sc.get_urls(service_type='image') + self.assertEqual(len(urls), 2) + + urls = sc.get_urls(service_type='image', region_name='North') + self.assertEqual(len(urls), 1) + self.assertEqual(urls[0], 'https://image.north.host/v1/') + + urls = sc.get_urls(service_type='image', region_name='South') + self.assertEqual(len(urls), 1) + self.assertEqual(urls[0], 'https://image.south.host/v1/') + + urls = sc.get_urls(service_type='image', region_name='West') + self.assertEqual(urls, None) + + def test_service_catalog_param_overrides_body_region(self): + self.AUTH_RESPONSE_BODY['access']['region_name'] = "North" + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + url = sc.url_for(service_type='image') + self.assertEqual(url, 'https://image.north.host/v1/') + + url = sc.url_for(service_type='image', region_name='South') + self.assertEqual(url, 'https://image.south.host/v1/') + + endpoints = sc.get_endpoints(service_type='image') + self.assertEqual(len(endpoints['image']), 1) + self.assertEqual(endpoints['image'][0]['publicURL'], + 'https://image.north.host/v1/') + + endpoints = sc.get_endpoints(service_type='image', region_name='South') + self.assertEqual(len(endpoints['image']), 1) + self.assertEqual(endpoints['image'][0]['publicURL'], + 'https://image.south.host/v1/') diff --git a/keystoneclient/tests/v3/test_client.py b/keystoneclient/tests/v3/test_client.py index fa2c9cb35..c56da4a3b 100644 --- a/keystoneclient/tests/v3/test_client.py +++ b/keystoneclient/tests/v3/test_client.py @@ -169,3 +169,26 @@ class KeystoneClientTest(utils.TestCase): self.stub_auth(json=second) cl.authenticate() self.assertEqual(cl.management_url, second_url % 35357) + + @httpretty.activate + def test_client_with_region_name_passes_to_service_catalog(self): + # NOTE(jamielennox): this is deprecated behaviour that should be + # removed ASAP, however must remain compatible. + + self.stub_auth(json=client_fixtures.AUTH_RESPONSE_BODY) + + cl = client.Client(username='exampleuser', + password='password', + tenant_name='exampleproject', + auth_url=self.TEST_URL, + region_name='North') + self.assertEqual(cl.service_catalog.url_for(service_type='image'), + 'http://glance.north.host/glanceapi/public') + + cl = client.Client(username='exampleuser', + password='password', + tenant_name='exampleproject', + auth_url=self.TEST_URL, + region_name='South') + self.assertEqual(cl.service_catalog.url_for(service_type='image'), + 'http://glance.south.host/glanceapi/public') diff --git a/keystoneclient/tests/v3/test_service_catalog.py b/keystoneclient/tests/v3/test_service_catalog.py index 2752f3afb..a82b9c12f 100644 --- a/keystoneclient/tests/v3/test_service_catalog.py +++ b/keystoneclient/tests/v3/test_service_catalog.py @@ -28,6 +28,20 @@ class ServiceCatalogTest(utils.TestCase): "headers": client_fixtures.AUTH_RESPONSE_HEADERS }) + self.north_endpoints = {'public': + 'http://glance.north.host/glanceapi/public', + 'internal': + 'http://glance.north.host/glanceapi/internal', + 'admin': + 'http://glance.north.host/glanceapi/admin'} + + self.south_endpoints = {'public': + 'http://glance.south.host/glanceapi/public', + 'internal': + 'http://glance.south.host/glanceapi/internal', + 'admin': + 'http://glance.south.host/glanceapi/admin'} + def test_building_a_service_catalog(self): auth_ref = access.AccessInfo.factory(self.RESPONSE, self.AUTH_RESPONSE_BODY) @@ -82,3 +96,84 @@ class ServiceCatalogTest(utils.TestCase): auth_ref.service_catalog.url_for, service_type='image', endpoint_type='internalURL') + + def test_service_catalog_get_endpoints_region_names(self): + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + endpoints = sc.get_endpoints(service_type='image', region_name='North') + self.assertEqual(len(endpoints), 1) + for endpoint in endpoints['image']: + self.assertEqual(endpoint['url'], + self.north_endpoints[endpoint['interface']]) + + endpoints = sc.get_endpoints(service_type='image', region_name='South') + self.assertEqual(len(endpoints), 1) + for endpoint in endpoints['image']: + self.assertEqual(endpoint['url'], + self.south_endpoints[endpoint['interface']]) + + endpoints = sc.get_endpoints(service_type='compute') + self.assertEqual(len(endpoints['compute']), 3) + + endpoints = sc.get_endpoints(service_type='compute', + region_name='North') + self.assertEqual(len(endpoints['compute']), 3) + + endpoints = sc.get_endpoints(service_type='compute', + region_name='West') + self.assertEqual(len(endpoints['compute']), 0) + + def test_service_catalog_url_for_region_names(self): + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + url = sc.url_for(service_type='image', region_name='North') + self.assertEqual(url, self.north_endpoints['public']) + + url = sc.url_for(service_type='image', region_name='South') + self.assertEqual(url, self.south_endpoints['public']) + + self.assertRaises(exceptions.EndpointNotFound, sc.url_for, + service_type='image', region_name='West') + + def test_servcie_catalog_get_url_region_names(self): + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + urls = sc.get_urls(service_type='image') + self.assertEqual(len(urls), 2) + + urls = sc.get_urls(service_type='image', region_name='North') + self.assertEqual(len(urls), 1) + self.assertEqual(urls[0], self.north_endpoints['public']) + + urls = sc.get_urls(service_type='image', region_name='South') + self.assertEqual(len(urls), 1) + self.assertEqual(urls[0], self.south_endpoints['public']) + + urls = sc.get_urls(service_type='image', region_name='West') + self.assertEqual(urls, None) + + def test_service_catalog_param_overrides_body_region(self): + self.AUTH_RESPONSE_BODY['token']['region_name'] = "North" + auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY) + sc = auth_ref.service_catalog + + url = sc.url_for(service_type='image') + self.assertEqual(url, self.north_endpoints['public']) + + url = sc.url_for(service_type='image', region_name='South') + self.assertEqual(url, self.south_endpoints['public']) + + endpoints = sc.get_endpoints(service_type='image') + self.assertEqual(len(endpoints['image']), 3) + for endpoint in endpoints['image']: + self.assertEqual(endpoint['url'], + self.north_endpoints[endpoint['interface']]) + + endpoints = sc.get_endpoints(service_type='image', region_name='South') + self.assertEqual(len(endpoints['image']), 3) + for endpoint in endpoints['image']: + self.assertEqual(endpoint['url'], + self.south_endpoints[endpoint['interface']]) diff --git a/keystoneclient/v3/client.py b/keystoneclient/v3/client.py index 00f3c9cb9..4f30e8373 100644 --- a/keystoneclient/v3/client.py +++ b/keystoneclient/v3/client.py @@ -107,12 +107,12 @@ class Client(httpclient.HTTPClient): def serialize(self, entity): return jsonutils.dumps(entity, sort_keys=True) - def process_token(self): + def process_token(self, **kwargs): """Extract and process information from the new auth_ref. And set the relevant authentication information. """ - super(Client, self).process_token() + super(Client, self).process_token(**kwargs) if self.auth_ref.domain_scoped: if not self.auth_ref.domain_id: raise exceptions.AuthorizationFailure(