From 8d46288e9cfb10745adfb06a510ed93433a7a9c5 Mon Sep 17 00:00:00 2001 From: Stuart McLaren Date: Tue, 21 Feb 2012 15:07:51 +0000 Subject: [PATCH] Allow region selection when using V2 keystone Fix for bug 936798. Allow a client to select which region endpoint they want from the service catalogue. The region can be set via the environment variable OS_REGION_NAME or via the '-R/--region' command line option. If a single image endpoint is returned from keystone, the client will use that even if no region was specified (the default). Where multiple endpoints exist a region must be provided. Change-Id: I6797f8ddf88d5875caf194913082c3fe24c86585 --- bin/glance | 11 +- doc/source/glance.rst | 6 + doc/source/man/glance.rst | 7 + glance/common/auth.py | 94 ++++-- glance/common/client.py | 3 +- glance/common/exception.py | 11 + .../tests/functional/test_private_images.py | 2 + glance/tests/unit/test_auth.py | 286 ++++++++++++++---- 8 files changed, 329 insertions(+), 91 deletions(-) diff --git a/bin/glance b/bin/glance index a692fd9a..51d62a5f 100755 --- a/bin/glance +++ b/bin/glance @@ -798,7 +798,9 @@ def get_client(options): os.getenv('OS_TENANT_NAME')), auth_url=options.auth_url or os.getenv('OS_AUTH_URL'), strategy=force_strategy or options.auth_strategy or \ - os.getenv('OS_AUTH_STRATEGY', 'noauth')) + os.getenv('OS_AUTH_STRATEGY', 'noauth'), + region=options.region or os.getenv('OS_REGION_NAME'), + ) if creds['strategy'] == 'keystone' and not creds['auth_url']: msg = ("--auth_url option or OS_AUTH_URL environment variable " @@ -865,6 +867,13 @@ def create_options(parser): parser.add_option('-K', '--password', dest="password", metavar="PASSWORD", default=None, help="Password used to acquire an authentication token") + parser.add_option('-R', '--region', dest="region", + metavar="REGION", default=None, + help="Region name. When using keystone authentication " + "version 2.0 or later this identifies the region " + "name to use when selecting the service endpoint. A " + "region name must be provided if more than one " + "region endpoint is available") parser.add_option('-T', '--tenant', dest="tenant", metavar="TENANT", default=None, help="Tenant name") diff --git a/doc/source/glance.rst b/doc/source/glance.rst index cba64e57..49ee9f41 100644 --- a/doc/source/glance.rst +++ b/doc/source/glance.rst @@ -104,6 +104,12 @@ a brief help message, like so:: requests. The server certificate will not be verified against any certificate authorities. This option should be used with caution. + -R REGION, --region=REGION + When using keystone authentication version 2.0 + or later this identifies the region name to + use when selecting the service endpoint. Where + more than one region endpoint is available a + region must be provided. --limit=LIMIT Page size to use while requesting image metadata --marker=MARKER Image index after which to begin pagination --sort_key=KEY Sort results by this image attribute. diff --git a/doc/source/man/glance.rst b/doc/source/man/glance.rst index ae174f9a..13f6982c 100644 --- a/doc/source/man/glance.rst +++ b/doc/source/man/glance.rst @@ -99,6 +99,13 @@ OPTIONS The server certificate will not be verified against any certificate authorities. This option should be used with caution. + **-R REGION, --region=REGION** + When using keystone authentication version 2.0 or later this + identifies the region name to use when selecting the service + endpoint. If no region is specified and only one region is + available that region will be used by default. Where more than + one region endpoint is available a region must be provided. + **-A TOKEN, --auth_token=TOKEN** Authentication token to use to identify the client to the glance server diff --git a/glance/common/auth.py b/glance/common/auth.py index ea2ab0b5..4d726fbf 100644 --- a/glance/common/auth.py +++ b/glance/common/auth.py @@ -38,13 +38,9 @@ from glance.common import exception class BaseStrategy(object): - def __init__(self, creds): - self.creds = creds + def __init__(self): self.auth_token = None - - # TODO(sirp): For now we're just dealing with one endpoint, eventually - # this should expose the entire service catalog so that the client can - # choose which service/region/(public/private net) combo they want. + # TODO(sirp): Should expose selecting public/internal/admin URL. self.management_url = None def authenticate(self): @@ -54,6 +50,10 @@ class BaseStrategy(object): def is_authenticated(self): raise NotImplementedError + @property + def strategy(self): + raise NotImplementedError + class NoAuthStrategy(BaseStrategy): def authenticate(self): @@ -63,10 +63,32 @@ class NoAuthStrategy(BaseStrategy): def is_authenticated(self): return True + @property + def strategy(self): + return 'noauth' + class KeystoneStrategy(BaseStrategy): MAX_REDIRECTS = 10 + def __init__(self, creds): + self.creds = creds + super(KeystoneStrategy, self).__init__() + + def check_auth_params(self): + # Ensure that supplied credential parameters are as required + for required in ('username', 'password', 'auth_url', + 'strategy'): + if required not in self.creds: + raise exception.MissingCredentialError(required=required) + if self.creds['strategy'] != 'keystone': + raise exception.BadAuthStrategy(expected='keystone', + received=self.creds['strategy']) + # For v2.0 also check tenant is present + if self.creds['auth_url'].rstrip('/').endswith('v2.0'): + if 'tenant' not in self.creds: + raise exception.MissingCredentialError(required='tenant') + def authenticate(self): """Authenticate with the Keystone service. @@ -82,16 +104,6 @@ class KeystoneStrategy(BaseStrategy): case, we rewrite the url to contain /v2.0/ and retry using the v2 protocol. """ - def check_auth_params(): - # Ensure that supplied credential parameters are as required - for required in ('username', 'password', 'auth_url'): - if required not in self.creds: - raise exception.MissingCredentialError(required=required) - # For v2.0 also check tenant is present - if self.creds['auth_url'].rstrip('/').endswith('v2.0'): - if 'tenant' not in self.creds: - raise exception.MissingCredentialError(required='tenant') - def _authenticate(auth_url): # If OS_AUTH_URL is missing a trailing slash add one if not auth_url.endswith('/'): @@ -104,7 +116,7 @@ class KeystoneStrategy(BaseStrategy): else: self._v1_auth(token_url) - check_auth_params() + self.check_auth_params() auth_url = self.creds['auth_url'] for _ in range(self.MAX_REDIRECTS): try: @@ -168,6 +180,32 @@ class KeystoneStrategy(BaseStrategy): raise Exception(_('Unexpected response: %s' % resp.status)) def _v2_auth(self, token_url): + def get_endpoint(service_catalog): + """ + Select an endpoint from the service catalog + + We search the full service catalog for services + matching both type and region. If the client + supplied no region then any 'image' endpoint + is considered a match. There must be one -- and + only one -- successful match in the catalog, + otherwise we will raise an exception. + """ + # FIXME(sirp): for now just use the public url. + endpoint = None + region = self.creds.get('region') + for service in service_catalog: + if service['type'] == 'image': + for ep in service['endpoints']: + if region is None or region == ep['region']: + if endpoint is not None: + # This is a second match, abort + raise exception.RegionAmbiguity(region=region) + endpoint = ep + if endpoint is None: + raise exception.NoServiceEndpoint() + return endpoint['publicURL'] + creds = self.creds creds = { @@ -189,17 +227,7 @@ class KeystoneStrategy(BaseStrategy): if resp.status == 200: resp_auth = json.loads(resp_body)['access'] - - # FIXME(sirp): for now just using the first endpoint we get back - # from the service catalog for glance, and using the public url. - for service in resp_auth['serviceCatalog']: - if service['type'] == 'image': - glance_endpoint = service['endpoints'][0]['publicURL'] - break - else: - raise exception.NoServiceEndpoint() - - self.management_url = glance_endpoint + self.management_url = get_endpoint(resp_auth['serviceCatalog']) self.auth_token = resp_auth['token']['id'] elif resp.status == 305: raise exception.RedirectException(resp['location']) @@ -216,6 +244,10 @@ class KeystoneStrategy(BaseStrategy): def is_authenticated(self): return self.auth_token is not None + @property + def strategy(self): + return 'keystone' + @staticmethod def _do_request(url, method, headers=None, body=None): headers = headers or {} @@ -226,10 +258,10 @@ class KeystoneStrategy(BaseStrategy): return resp, resp_body -def get_plugin_from_strategy(strategy): +def get_plugin_from_strategy(strategy, creds=None): if strategy == 'noauth': - return NoAuthStrategy + return NoAuthStrategy() elif strategy == 'keystone': - return KeystoneStrategy + return KeystoneStrategy(creds) else: raise Exception(_("Unknown auth strategy '%s'") % strategy) diff --git a/glance/common/client.py b/glance/common/client.py index b1409d3c..b9ab076c 100644 --- a/glance/common/client.py +++ b/glance/common/client.py @@ -339,8 +339,7 @@ class BaseClient(object): Returns an instantiated authentication plugin. """ strategy = creds.get('strategy', 'noauth') - plugin_class = auth.get_plugin_from_strategy(strategy) - plugin = plugin_class(creds) + plugin = auth.get_plugin_from_strategy(strategy, creds) return plugin def get_connection_type(self): diff --git a/glance/common/exception.py b/glance/common/exception.py index 6a69637d..3ddf38fa 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -63,6 +63,11 @@ class MissingCredentialError(GlanceException): message = _("Missing required credential: %(required)s") +class BadAuthStrategy(GlanceException): + message = _("Incorrect auth strategy, expected \"%(expected)s\" but " + "received \"%(received)s\"") + + class NotFound(GlanceException): message = _("An object with the specified identifier was not found.") @@ -180,3 +185,9 @@ class InvalidRedirect(GlanceException): class NoServiceEndpoint(GlanceException): message = _("Response from Keystone does not contain a Glance endpoint.") + + +class RegionAmbiguity(GlanceException): + message = _("Multiple 'image' service matches for region %(region)s. This " + "generally means that a region is required and you have not " + "supplied one.") diff --git a/glance/tests/functional/test_private_images.py b/glance/tests/functional/test_private_images.py index 4e145761..aa9ecde6 100644 --- a/glance/tests/functional/test_private_images.py +++ b/glance/tests/functional/test_private_images.py @@ -844,6 +844,7 @@ class TestPrivateImagesCli(keystone_utils.KeystoneTests): os.environ.pop('OS_AUTH_STRATEGY', None) os.environ.pop('OS_AUTH_USER', None) os.environ.pop('OS_AUTH_KEY', None) + os.environ.pop('OS_REGION_NAME', None) @skip_if_disabled def test_glance_cli_noauth_strategy(self): @@ -878,6 +879,7 @@ class TestPrivateImagesCli(keystone_utils.KeystoneTests): os.environ['OS_AUTH_STRATEGY'] = 'keystone' os.environ['OS_AUTH_USER'] = 'pattieblack' os.environ['OS_AUTH_KEY'] = 'secrete' + os.environ['OS_REGION_NAME'] = 'RegionOne' cmd = minimal_add_command(self.api_port, 'MyImage', public=False) self._do_test_glance_cli(cmd) diff --git a/glance/tests/unit/test_auth.py b/glance/tests/unit/test_auth.py index ed6631e5..6e6ac325 100644 --- a/glance/tests/unit/test_auth.py +++ b/glance/tests/unit/test_auth.py @@ -40,6 +40,63 @@ class FakeResponse(object): return self.resp.status_int +class V2Token(object): + def __init__(self): + self.tok = self.base_token + + def add_service(self, s_type, region_list=[]): + catalog = self.tok['access']['serviceCatalog'] + service_type = {"type": s_type, "name": "glance"} + catalog.append(service_type) + service = catalog[-1] + endpoint_list = [] + + if region_list == []: + endpoint_list.append(self.base_endpoint) + else: + for region in region_list: + endpoint = self.base_endpoint + endpoint['region'] = region + endpoint_list.append(endpoint) + + service['endpoints'] = endpoint_list + + @property + def token(self): + return self.tok + + @property + def base_endpoint(self): + return { + "adminURL": "http://localhost:9292", + "internalURL": "http://localhost:9292", + "publicURL": "http://localhost:9292" + } + + @property + def base_token(self): + return { + "access": { + "token": { + "expires": "2010-11-23T16:40:53.321584", + "id": "5c7f8799-2e54-43e4-851b-31f81871b6c", + "tenant": {"id": "1", "name": "tenant-ok"} + }, + "serviceCatalog": [ + ], + "user": { + "id": "2", + "roles": [{ + "tenantId": "1", + "id": "1", + "name": "Admin" + }], + "name": "joeadmin" + } + } + } + + class TestKeystoneAuthPlugin(unittest.TestCase): """Test that the Keystone auth plugin works properly""" @@ -78,10 +135,10 @@ class TestKeystoneAuthPlugin(unittest.TestCase): try: plugin = auth.KeystoneStrategy(creds) plugin.authenticate() + self.fail("Failed to raise correct exception when supplying " + "bad credentials: %r" % creds) except exception.MissingCredentialError: continue # Expected - self.fail("Failed to raise correct exception when supplying bad " - "credentials: %r" % creds) def test_invalid_auth_url(self): """ @@ -94,13 +151,17 @@ class TestKeystoneAuthPlugin(unittest.TestCase): { 'username': 'user1', 'auth_url': 'http://localhost/badauthurl/', - 'password': 'pass' + 'password': 'pass', + 'strategy': 'keystone', + 'region': 'RegionOne' }, # v1 Keystone { 'username': 'user1', 'auth_url': 'http://localhost/badauthurl/v2.0/', 'password': 'pass', - 'tenant': 'tenant1' + 'tenant': 'tenant1', + 'strategy': 'keystone', + 'region': 'RegionOne' } # v2 Keystone ] @@ -138,11 +199,15 @@ class TestKeystoneAuthPlugin(unittest.TestCase): { 'username': 'wronguser', 'auth_url': 'http://localhost/badauthurl/', + 'strategy': 'keystone', + 'region': 'RegionOne', 'password': 'pass' }, # wrong username { 'username': 'user1', 'auth_url': 'http://localhost/badauthurl/', + 'strategy': 'keystone', + 'region': 'RegionOne', 'password': 'badpass' }, # bad password... ] @@ -151,16 +216,33 @@ class TestKeystoneAuthPlugin(unittest.TestCase): try: plugin = auth.KeystoneStrategy(creds) plugin.authenticate() + self.fail("Failed to raise NotAuthorized when supplying bad " + "credentials: %r" % creds) except exception.NotAuthorized: continue # Expected - self.fail("Failed to raise NotAuthorized when supplying bad " - "credentials: %r" % creds) + + no_strategy_creds = { + 'username': 'user1', + 'auth_url': 'http://localhost/redirect/', + 'password': 'pass', + 'region': 'RegionOne' + } + + try: + plugin = auth.KeystoneStrategy(no_strategy_creds) + plugin.authenticate() + self.fail("Failed to raise MissingCredentialError when " + "supplying no strategy: %r" % no_strategy_creds) + except exception.MissingCredentialError: + pass # Expected good_creds = [ { 'username': 'user1', 'auth_url': 'http://localhost/redirect/', - 'password': 'pass' + 'password': 'pass', + 'strategy': 'keystone', + 'region': 'RegionOne' } ] @@ -170,39 +252,7 @@ class TestKeystoneAuthPlugin(unittest.TestCase): def test_v2_auth(self): """Test v2 auth code paths""" - service_type = "image" - - def v2_token(service_type="image"): - # Mock up a token to satisfy v2 auth - token = { - "access": { - "token": { - "expires": "2010-11-23T16:40:53.321584", - "id": "5c7f8799-2e54-43e4-851b-31f81871b6c", - "tenant": {"id": "1", "name": "tenant-ok"} - }, - "serviceCatalog": [{ - "endpoints": [{ - "region": "RegionOne", - "adminURL": "http://localhost:9292", - "internalURL": "http://localhost:9292", - "publicURL": "http://localhost:9292" - }], - "type": service_type, - "name": "glance" - }], - "user": { - "id": "2", - "roles": [{ - "tenantId": "1", - "id": "1", - "name": "Admin" - }], - "name": "joeadmin" - } - } - } - return token + mock_token = None def fake_do_request(cls, url, method, headers=None, body=None): if (not url.rstrip('/').endswith('v2.0/tokens') or @@ -220,10 +270,12 @@ class TestKeystoneAuthPlugin(unittest.TestCase): resp.status = 401 else: resp.status = 200 - body = v2_token(service_type) + body = mock_token.token return FakeResponse(resp), json.dumps(body) + mock_token = V2Token() + mock_token.add_service('image', ['RegionOne']) self.stubs.Set(auth.KeystoneStrategy, '_do_request', fake_do_request) unauthorized_creds = [ @@ -231,19 +283,25 @@ class TestKeystoneAuthPlugin(unittest.TestCase): 'username': 'wronguser', 'auth_url': 'http://localhost/v2.0', 'password': 'pass', - 'tenant': 'tenant-ok' + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionOne' }, # wrong username { 'username': 'user1', 'auth_url': 'http://localhost/v2.0', 'password': 'badpass', - 'tenant': 'tenant-ok' + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionOne' }, # bad password... { 'username': 'user1', 'auth_url': 'http://localhost/v2.0', 'password': 'pass', - 'tenant': 'carterhayes' + 'tenant': 'carterhayes', + 'strategy': 'keystone', + 'region': 'RegionOne' }, # bad tenant... ] @@ -251,43 +309,157 @@ class TestKeystoneAuthPlugin(unittest.TestCase): try: plugin = auth.KeystoneStrategy(creds) plugin.authenticate() + self.fail("Failed to raise NotAuthorized when supplying bad " + "credentials: %r" % creds) except exception.NotAuthorized: continue # Expected - self.fail("Failed to raise NotAuthorized when supplying bad " - "credentials: %r" % creds) + + no_region_creds = { + 'username': 'user1', + 'tenant': 'tenant-ok', + 'auth_url': 'http://localhost/redirect/v2.0/', + 'password': 'pass', + 'strategy': 'keystone' + } + + plugin = auth.KeystoneStrategy(no_region_creds) + self.assertTrue(plugin.authenticate() is None) + self.assertEquals(plugin.management_url, 'http://localhost:9292') + + # Add another image service, with a different region + mock_token.add_service('image', ['RegionTwo']) + + try: + plugin = auth.KeystoneStrategy(no_region_creds) + plugin.authenticate() + self.fail("Failed to raise RegionAmbiguity when no region present " + "and multiple regions exist: %r" % no_region_creds) + except exception.RegionAmbiguity: + pass # Expected + + wrong_region_creds = { + 'username': 'user1', + 'tenant': 'tenant-ok', + 'auth_url': 'http://localhost/redirect/v2.0/', + 'password': 'pass', + 'strategy': 'keystone', + 'region': 'NonExistantRegion' + } + + try: + plugin = auth.KeystoneStrategy(wrong_region_creds) + plugin.authenticate() + self.fail("Failed to raise NoServiceEndpoint when supplying " + "wrong region: %r" % wrong_region_creds) + except exception.NoServiceEndpoint: + pass # Expected + + no_strategy_creds = { + 'username': 'user1', + 'tenant': 'tenant-ok', + 'auth_url': 'http://localhost/redirect/v2.0/', + 'password': 'pass', + 'region': 'RegionOne' + } + + try: + plugin = auth.KeystoneStrategy(no_strategy_creds) + plugin.authenticate() + self.fail("Failed to raise MissingCredentialError when " + "supplying no strategy: %r" % no_strategy_creds) + except exception.MissingCredentialError: + pass # Expected + + bad_strategy_creds = { + 'username': 'user1', + 'tenant': 'tenant-ok', + 'auth_url': 'http://localhost/redirect/v2.0/', + 'password': 'pass', + 'region': 'RegionOne', + 'strategy': 'keypebble' + } + + try: + plugin = auth.KeystoneStrategy(bad_strategy_creds) + plugin.authenticate() + self.fail("Failed to raise BadAuthStrategy when supplying " + "bad auth strategy: %r" % bad_strategy_creds) + except exception.BadAuthStrategy: + pass # Expected + + mock_token = V2Token() + mock_token.add_service('image', ['RegionOne', 'RegionTwo']) good_creds = [ { 'username': 'user1', 'auth_url': 'http://localhost/v2.0/', 'password': 'pass', - 'tenant': 'tenant-ok' + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionOne' }, # auth_url with trailing '/' { 'username': 'user1', 'auth_url': 'http://localhost/v2.0', 'password': 'pass', - 'tenant': 'tenant-ok' - } # auth_url without trailing '/' + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionOne' + }, # auth_url without trailing '/' + { + 'username': 'user1', + 'auth_url': 'http://localhost/v2.0', + 'password': 'pass', + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionTwo' + } # Second region ] for creds in good_creds: plugin = auth.KeystoneStrategy(creds) self.assertTrue(plugin.authenticate() is None) + self.assertEquals(plugin.management_url, 'http://localhost:9292') - creds = { - 'username': 'user1', - 'auth_url': 'http://localhost/v2.0/', - 'password': 'pass', - 'tenant': 'tenant-ok' + ambiguous_region_creds = { + 'username': 'user1', + 'auth_url': 'http://localhost/v2.0/', + 'password': 'pass', + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionOne' } - service_type = "bad-image" + mock_token = V2Token() + # Add two identical services + mock_token.add_service('image', ['RegionOne']) + mock_token.add_service('image', ['RegionOne']) try: - plugin = auth.KeystoneStrategy(creds) + plugin = auth.KeystoneStrategy(ambiguous_region_creds) + plugin.authenticate() + self.fail("Failed to raise RegionAmbiguity when " + "non-unique regions exist: %r" % ambiguous_region_creds) + except exception.RegionAmbiguity: + pass + + mock_token = V2Token() + mock_token.add_service('bad-image', ['RegionOne']) + + good_creds = { + 'username': 'user1', + 'auth_url': 'http://localhost/v2.0/', + 'password': 'pass', + 'tenant': 'tenant-ok', + 'strategy': 'keystone', + 'region': 'RegionOne' + } + + try: + plugin = auth.KeystoneStrategy(good_creds) plugin.authenticate() self.fail("Failed to raise NoServiceEndpoint when bad service " - "type encountered: %r" % service_type) + "type encountered") except exception.NoServiceEndpoint: pass