From ed8ef0df9ff3a095d53be90ce535393078bf267b Mon Sep 17 00:00:00 2001 From: Dean Troyer Date: Tue, 25 Apr 2017 07:27:14 -0500 Subject: [PATCH] Tell ClientManager when auth is required Commands that do not reuire authentication still call things that want to check the service catalog; no auth means no service catalog. (I'm looking at you OSC's is_networking_enabled()) Get more aggressive at not doing auth when it is not required. The cost of this is commands with auth_required=False need to be more selective about what they do with ClientManager.auth_ref as it will now happily return None when auth is not required _and_ no credentials are present, rather than throw up all over the place. Change-Id: I72ae6154268bdf26be6054c0fef6a4c67c71119c --- osc_lib/clientmanager.py | 8 ++++++++ osc_lib/shell.py | 7 ++++++- osc_lib/tests/test_clientmanager.py | 12 ++++++++++-- osc_lib/tests/utils.py | 2 ++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/osc_lib/clientmanager.py b/osc_lib/clientmanager.py index 75bd0a7..6017115 100644 --- a/osc_lib/clientmanager.py +++ b/osc_lib/clientmanager.py @@ -56,6 +56,11 @@ class ClientCache(object): class ClientManager(object): """Manages access to API clients, including authentication.""" + # NOTE(dtroyer): Keep around the auth required state of the _current_ + # command since ClientManager has no visibility to the + # command itself; assume auth is not required. + _auth_required = False + def __init__( self, cli_options=None, @@ -225,6 +230,9 @@ class ClientManager(object): @property def auth_ref(self): """Dereference will trigger an auth if it hasn't already""" + if not self._auth_required: + # Forcibly skip auth if we know we do not need it + return None if not self._auth_ref: self.setup_auth() LOG.debug("Get auth_ref") diff --git a/osc_lib/shell.py b/osc_lib/shell.py index adfdbc2..6882770 100644 --- a/osc_lib/shell.py +++ b/osc_lib/shell.py @@ -424,16 +424,21 @@ class OpenStackShell(app.App): def prepare_to_run_command(self, cmd): """Set up auth and API versions""" self.log.info( - 'command: %s -> %s.%s', + 'command: %s -> %s.%s (auth=%s)', getattr(cmd, 'cmd_name', ''), cmd.__class__.__module__, cmd.__class__.__name__, + cmd.auth_required, ) # NOTE(dtroyer): If auth is not required for a command, skip # get_one_Cloud()'s validation to avoid loading plugins validate = cmd.auth_required + # NOTE(dtroyer): Save the auth required state of the _current_ command + # in the ClientManager + self.client_manager._auth_required = cmd.auth_required + # Validate auth options self.cloud = self.cloud_config.get_one_cloud( cloud=self.options.cloud, diff --git a/osc_lib/tests/test_clientmanager.py b/osc_lib/tests/test_clientmanager.py index 22789fe..5f20bf0 100644 --- a/osc_lib/tests/test_clientmanager.py +++ b/osc_lib/tests/test_clientmanager.py @@ -102,7 +102,9 @@ class TestClientManager(utils.TestClientManager): ) def test_client_manager_password(self): - client_manager = self._make_clientmanager() + client_manager = self._make_clientmanager( + auth_required=True, + ) self.assertEqual( fakes.AUTH_URL, @@ -139,7 +141,9 @@ class TestClientManager(utils.TestClientManager): self.assertTrue(client_manager.is_service_available('network')) def test_client_manager_password_verify(self): - client_manager = self._make_clientmanager() + client_manager = self._make_clientmanager( + auth_required=True, + ) self.assertTrue(client_manager.verify) self.assertIsNone(client_manager.cacert) @@ -151,6 +155,7 @@ class TestClientManager(utils.TestClientManager): } client_manager = self._make_clientmanager( config_args=config_args, + auth_required=True, ) # Test that client_manager.verify is Requests-compatible, @@ -166,6 +171,7 @@ class TestClientManager(utils.TestClientManager): } client_manager = self._make_clientmanager( config_args=config_args, + auth_required=True, ) self.assertFalse(client_manager.verify) @@ -178,6 +184,7 @@ class TestClientManager(utils.TestClientManager): } client_manager = self._make_clientmanager( config_args=config_args, + auth_required=True, ) self.assertFalse(client_manager.verify) @@ -191,6 +198,7 @@ class TestClientManager(utils.TestClientManager): } client_manager = self._make_clientmanager( config_args=config_args, + auth_required=True, ) # insecure overrides cacert diff --git a/osc_lib/tests/utils.py b/osc_lib/tests/utils.py index b55e9ef..d137b2b 100644 --- a/osc_lib/tests/utils.py +++ b/osc_lib/tests/utils.py @@ -199,6 +199,7 @@ class TestClientManager(TestCase): config_args=None, identity_api_version=None, auth_plugin_name=None, + auth_required=None, ): if identity_api_version is None: @@ -238,6 +239,7 @@ class TestClientManager(TestCase): 'identity': identity_api_version, }, ) + client_manager._auth_required = auth_required is True client_manager.setup_auth() client_manager.auth_ref