Consider --os-token when using v2

The `_cache_schemas` call currently forces authentication even when the
`auth_token` and `os_image_url` are passed. Instead of handling forced
authentications, let the client use the passed arguments and
authenticate only once if needed.

This was not caught by the existing tests because the call to
`_cache_schemas` was mocked.

Change-Id: I93cec9a68cafc0992d14dab38114d03e25f1e5da
Closes-bug: #1490462

glanceclient/shell.py

@@ -442,12 +442,13 @@ class OpenStackImagesShell(object):
         ks_session.auth = auth
         return ks_session
 
-    def _get_endpoint_and_token(self, args, force_auth=False):
+    def _get_endpoint_and_token(self, args):
         image_url = self._get_image_url(args)
         auth_token = args.os_auth_token
 
-        auth_reqd = force_auth or (utils.is_authentication_required(args.func)
+        auth_reqd = (not (auth_token and image_url) or
-                                   and not (auth_token and image_url))
+                     (hasattr(args, 'func') and
+                      utils.is_authentication_required(args.func)))
 
         if not auth_reqd:
             endpoint = image_url
@@ -537,9 +538,8 @@ class OpenStackImagesShell(object):
 
         return endpoint, token
 
-    def _get_versioned_client(self, api_version, args, force_auth=False):
+    def _get_versioned_client(self, api_version, args):
-        endpoint, token = self._get_endpoint_and_token(args,
+        endpoint, token = self._get_endpoint_and_token(args)
-                                                       force_auth=force_auth)
 
         kwargs = {
             'token': token,
@@ -621,8 +621,7 @@ class OpenStackImagesShell(object):
 
         if not options.os_image_api_version and api_version == 2:
             switch_version = True
-            client = self._get_versioned_client('2', options,
+            client = self._get_versioned_client('2', options)
-                                                force_auth=True)
 
             resp, body = client.http_client.get('/versions')
 
@@ -690,8 +689,7 @@ class OpenStackImagesShell(object):
         if profile:
             osprofiler_profiler.init(options.profile)
 
-        client = self._get_versioned_client(api_version, args,
+        client = self._get_versioned_client(api_version, args)
-                                            force_auth=False)
 
         try:
             args.func(client, args)

glanceclient/tests/functional/test_readonly_glance.py

@@ -103,7 +103,7 @@ class SimpleReadOnlyGlanceClientTest(base.ClientTestBase):
         self.glance('', flags='--version')
 
     def test_debug_list(self):
-        self.glance('image-list', flags='--debug')
+        self.glance('--os-image-api-version 2 image-list', flags='--debug')
 
     def test_no_ssl_compression(self):
         # Test deprecating this hasn't broken anything

glanceclient/tests/unit/test_shell.py

@@ -219,21 +219,18 @@ class ShellTest(testutils.TestCase):
         self.assertEqual('mytoken', kwargs['token'])
         self.assertEqual('https://image:1234', args[0])
 
-    @mock.patch.object(openstack_shell.OpenStackImagesShell, '_cache_schemas',
+    @mock.patch('glanceclient.v2.client.Client')
-                       return_value=False)
+    def test_no_auth_with_token_and_image_url_with_v2(self, v2_client):
-    def test_no_auth_with_token_and_image_url_with_v2(self,
+        # test no authentication is required if both token and endpoint url
-                                                      cache_schemas):
+        # are specified
-        with mock.patch('glanceclient.v2.client.Client') as v2_client:
+        args = ('--os-image-api-version 2 --os-auth-token mytoken '
-            # test no authentication is required if both token and endpoint url
+                '--os-image-url https://image:1234 image-list')
-            # are specified
+        glance_shell = openstack_shell.OpenStackImagesShell()
-            args = ('--os-auth-token mytoken '
+        glance_shell.main(args.split())
-                    '--os-image-url https://image:1234/v2 '
+        self.assertTrue(v2_client.called)
-                    '--os-image-api-version 2 image-list')
+        (args, kwargs) = v2_client.call_args
-            glance_shell = openstack_shell.OpenStackImagesShell()
+        self.assertEqual('mytoken', kwargs['token'])
-            glance_shell.main(args.split())
+        self.assertEqual('https://image:1234', args[0])
-            ((args), kwargs) = v2_client.call_args
-            self.assertEqual('https://image:1234', args[0])
-            self.assertEqual('mytoken', kwargs['token'])
 
     def _assert_auth_plugin_args(self):
         # make sure our auth plugin is invoked with the correct args
@@ -289,8 +286,14 @@ class ShellTest(testutils.TestCase):
 
     @mock.patch('sys.stdin', side_effect=mock.MagicMock)
     @mock.patch('getpass.getpass', return_value='password')
-    def test_password_prompted_with_v2(self, mock_getpass, mock_stdin):
+    @mock.patch('glanceclient.v2.client.Client')
+    def test_password_prompted_with_v2(self, v2_client,
+                                       mock_getpass, mock_stdin):
         self.requests.post(self.token_url, exc=requests.ConnectionError)
+
+        cli2 = mock.MagicMock()
+        v2_client.return_value = cli2
+        cli2.http_client.get.return_value = (None, {'versions': []})
         glance_shell = openstack_shell.OpenStackImagesShell()
         self.make_env(exclude='OS_PASSWORD')
         self.assertRaises(ks_exc.ConnectionRefused,
@@ -300,7 +303,13 @@ class ShellTest(testutils.TestCase):
 
     @mock.patch('sys.stdin', side_effect=mock.MagicMock)
     @mock.patch('getpass.getpass', side_effect=EOFError)
-    def test_password_prompted_ctrlD_with_v2(self, mock_getpass, mock_stdin):
+    @mock.patch('glanceclient.v2.client.Client')
+    def test_password_prompted_ctrlD_with_v2(self, v2_client,
+                                             mock_getpass, mock_stdin):
+        cli2 = mock.MagicMock()
+        v2_client.return_value = cli2
+        cli2.http_client.get.return_value = (None, {'versions': []})
+
         glance_shell = openstack_shell.OpenStackImagesShell()
         self.make_env(exclude='OS_PASSWORD')
         # We should get Command Error because we mock Ctl-D.
@@ -417,7 +426,7 @@ class ShellTest(testutils.TestCase):
     @mock.patch('glanceclient.v1.client.Client')
     def test_auth_plugin_invocation_without_username_with_v1(self, v1_client):
         self.make_env(exclude='OS_USERNAME')
-        args = 'image-list'
+        args = '--os-image-api-version 2 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         self.assertRaises(exc.CommandError, glance_shell.main, args.split())
 
@@ -431,7 +440,7 @@ class ShellTest(testutils.TestCase):
     @mock.patch('glanceclient.v1.client.Client')
     def test_auth_plugin_invocation_without_auth_url_with_v1(self, v1_client):
         self.make_env(exclude='OS_AUTH_URL')
-        args = 'image-list'
+        args = '--os-image-api-version 1 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         self.assertRaises(exc.CommandError, glance_shell.main, args.split())
 
@@ -448,7 +457,7 @@ class ShellTest(testutils.TestCase):
             self.make_env(exclude='OS_TENANT_NAME')
         if 'OS_PROJECT_ID' in os.environ:
             self.make_env(exclude='OS_PROJECT_ID')
-        args = 'image-list'
+        args = '--os-image-api-version 1 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         self.assertRaises(exc.CommandError, glance_shell.main, args.split())
 
@@ -505,7 +514,8 @@ class ShellTestWithKeystoneV3Auth(ShellTest):
                 side_effect=ks_exc.ClientException())
     def test_api_discovery_failed_with_unversioned_auth_url(self,
                                                             discover):
-        args = '--os-auth-url %s image-list' % DEFAULT_UNVERSIONED_AUTH_URL
+        args = ('--os-image-api-version 2 --os-auth-url %s image-list'
+                % DEFAULT_UNVERSIONED_AUTH_URL)
         glance_shell = openstack_shell.OpenStackImagesShell()
         self.assertRaises(exc.CommandError, glance_shell.main, args.split())