Use session when not specified token or endpoint

When no token or endpoint, it creates a session and from there
taken the necessary values.
This commit proposes to transfer a session in such cases.
This will avoid unnecessary actions and some of the problems.

Change-Id: Idc874b6c01e915e52904604d59e8e0b460e71621
Partial-bug: #1519546

2 files changed, 94 insertions, 116 deletions

diff --git a/glanceclient/shell.py b/glanceclient/shell.py
index 1b41cac..dec1589 100755
--- a/glanceclient/shell.py
+++ b/glanceclient/shell.py
@@ -349,111 +349,101 @@ class OpenStackImagesShell(object):
         ks_session.auth = auth
         return ks_session
 
-    def _get_endpoint_and_token(self, args):
+    def _get_kwargs_for_create_session(self, args):
+        if not args.os_username:
+            raise exc.CommandError(
+                _("You must provide a username via"
+                  " either --os-username or "
+                  "env[OS_USERNAME]"))
+
+        if not args.os_password:
+            # No password, If we've got a tty, try prompting for it
+            if hasattr(sys.stdin, 'isatty') and sys.stdin.isatty():
+                # Check for Ctl-D
+                try:
+                    args.os_password = getpass.getpass('OS Password: ')
+                except EOFError:
+                    pass
+            # No password because we didn't have a tty or the
+            # user Ctl-D when prompted.
+            if not args.os_password:
+                raise exc.CommandError(
+                    _("You must provide a password via "
+                      "either --os-password, "
+                      "env[OS_PASSWORD], "
+                      "or prompted response"))
+
+        # Validate password flow auth
+        project_info = (
+            args.os_tenant_name or args.os_tenant_id or (
+                args.os_project_name and (
+                    args.os_project_domain_name or
+                    args.os_project_domain_id
+                )
+            ) or args.os_project_id
+        )
+
+        if not project_info:
+            # tenant is deprecated in Keystone v3. Use the latest
+            # terminology instead.
+            raise exc.CommandError(
+                _("You must provide a project_id or project_name ("
+                  "with project_domain_name or project_domain_id) "
+                  "via "
+                  "  --os-project-id (env[OS_PROJECT_ID])"
+                  "  --os-project-name (env[OS_PROJECT_NAME]),"
+                  "  --os-project-domain-id "
+                  "(env[OS_PROJECT_DOMAIN_ID])"
+                  "  --os-project-domain-name "
+                  "(env[OS_PROJECT_DOMAIN_NAME])"))
+
+        if not args.os_auth_url:
+            raise exc.CommandError(
+                _("You must provide an auth url via"
+                  " either --os-auth-url or "
+                  "via env[OS_AUTH_URL]"))
+
+        kwargs = {
+            'auth_url': args.os_auth_url,
+            'username': args.os_username,
+            'user_id': args.os_user_id,
+            'user_domain_id': args.os_user_domain_id,
+            'user_domain_name': args.os_user_domain_name,
+            'password': args.os_password,
+            'tenant_name': args.os_tenant_name,
+            'tenant_id': args.os_tenant_id,
+            'project_name': args.os_project_name,
+            'project_id': args.os_project_id,
+            'project_domain_name': args.os_project_domain_name,
+            'project_domain_id': args.os_project_domain_id,
+            'insecure': args.insecure,
+            'cacert': args.os_cacert,
+            'cert': args.os_cert,
+            'key': args.os_key
+        }
+        return kwargs
+
+    def _get_versioned_client(self, api_version, args):
         endpoint = self._get_image_url(args)
         auth_token = args.os_auth_token
 
         auth_req = (hasattr(args, 'func') and
                     utils.is_authentication_required(args.func))
-
-        if auth_req and not (endpoint and auth_token):
-            if not args.os_username:
-                raise exc.CommandError(
-                    _("You must provide a username via"
-                      " either --os-username or "
-                      "env[OS_USERNAME]"))
-
-            if not args.os_password:
-                # No password, If we've got a tty, try prompting for it
-                if hasattr(sys.stdin, 'isatty') and sys.stdin.isatty():
-                    # Check for Ctl-D
-                    try:
-                        args.os_password = getpass.getpass('OS Password: ')
-                    except EOFError:
-                        pass
-                # No password because we didn't have a tty or the
-                # user Ctl-D when prompted.
-                if not args.os_password:
-                    raise exc.CommandError(
-                        _("You must provide a password via "
-                          "either --os-password, "
-                          "env[OS_PASSWORD], "
-                          "or prompted response"))
-
-            # Validate password flow auth
-            project_info = (
-                args.os_tenant_name or args.os_tenant_id or (
-                    args.os_project_name and (
-                        args.os_project_domain_name or
-                        args.os_project_domain_id
-                    )
-                ) or args.os_project_id
-            )
-
-            if not project_info:
-                # tenant is deprecated in Keystone v3. Use the latest
-                # terminology instead.
-                raise exc.CommandError(
-                    _("You must provide a project_id or project_name ("
-                      "with project_domain_name or project_domain_id) "
-                      "via "
-                      "  --os-project-id (env[OS_PROJECT_ID])"
-                      "  --os-project-name (env[OS_PROJECT_NAME]),"
-                      "  --os-project-domain-id "
-                      "(env[OS_PROJECT_DOMAIN_ID])"
-                      "  --os-project-domain-name "
-                      "(env[OS_PROJECT_DOMAIN_NAME])"))
-
-            if not args.os_auth_url:
-                raise exc.CommandError(
-                    _("You must provide an auth url via"
-                      " either --os-auth-url or "
-                      "via env[OS_AUTH_URL]"))
-
+        if not auth_req or (endpoint and auth_token):
             kwargs = {
-                'auth_url': args.os_auth_url,
-                'username': args.os_username,
-                'user_id': args.os_user_id,
-                'user_domain_id': args.os_user_domain_id,
-                'user_domain_name': args.os_user_domain_name,
-                'password': args.os_password,
-                'tenant_name': args.os_tenant_name,
-                'tenant_id': args.os_tenant_id,
-                'project_name': args.os_project_name,
-                'project_id': args.os_project_id,
-                'project_domain_name': args.os_project_domain_name,
-                'project_domain_id': args.os_project_domain_id,
+                'token': auth_token,
                 'insecure': args.insecure,
+                'timeout': args.timeout,
                 'cacert': args.os_cacert,
                 'cert': args.os_cert,
-                'key': args.os_key
+                'key': args.os_key,
+                'ssl_compression': args.ssl_compression
             }
-            ks_session = self._get_keystone_session(**kwargs)
-            auth_token = args.os_auth_token or ks_session.get_token()
-
-            endpoint_type = args.os_endpoint_type or 'public'
-            service_type = args.os_service_type or 'image'
-            endpoint = args.os_image_url or ks_session.get_endpoint(
-                service_type=service_type,
-                interface=endpoint_type,
-                region_name=args.os_region_name)
-
-        return endpoint, auth_token
-
-    def _get_versioned_client(self, api_version, args):
-        endpoint, token = self._get_endpoint_and_token(args)
+        else:
+            kwargs = self._get_kwargs_for_create_session(args)
+            kwargs = {'session': self._get_keystone_session(**kwargs)}
 
-        kwargs = {
-            'token': token,
-            'insecure': args.insecure,
-            'timeout': args.timeout,
-            'cacert': args.os_cacert,
-            'cert': args.os_cert,
-            'key': args.os_key,
-            'ssl_compression': args.ssl_compression
-        }
-        client = glanceclient.Client(api_version, endpoint, **kwargs)
-        return client
+        return glanceclient.Client(api_version, endpoint, **kwargs)
 
     def _cache_schemas(self, options, client, home_dir='~/.glanceclient'):
         homedir = os.path.expanduser(home_dir)
diff --git a/glanceclient/tests/unit/test_shell.py b/glanceclient/tests/unit/test_shell.py
index 5574ccb..beba3e6 100644
--- a/glanceclient/tests/unit/test_shell.py
+++ b/glanceclient/tests/unit/test_shell.py
@@ -153,14 +153,14 @@ class ShellTest(testutils.TestCase):
     def test_help(self):
         shell = openstack_shell.OpenStackImagesShell()
         argstr = '--os-image-api-version 2 help'
-        with mock.patch.object(shell, '_get_endpoint_and_token') as et_mock:
+        with mock.patch.object(shell, '_get_keystone_session') as et_mock:
             actual = shell.main(argstr.split())
             self.assertEqual(0, actual)
             self.assertFalse(et_mock.called)
 
     def test_blank_call(self):
         shell = openstack_shell.OpenStackImagesShell()
-        with mock.patch.object(shell, '_get_endpoint_and_token') as et_mock:
+        with mock.patch.object(shell, '_get_keystone_session') as et_mock:
             actual = shell.main('')
             self.assertEqual(0, actual)
             self.assertFalse(et_mock.called)
@@ -172,7 +172,7 @@ class ShellTest(testutils.TestCase):
     def test_help_v2_no_schema(self):
         shell = openstack_shell.OpenStackImagesShell()
         argstr = '--os-image-api-version 2 help image-create'
-        with mock.patch.object(shell, '_get_endpoint_and_token') as et_mock:
+        with mock.patch.object(shell, '_get_keystone_session') as et_mock:
             actual = shell.main(argstr.split())
             self.assertEqual(0, actual)
             self.assertNotIn('<unavailable>', actual)
@@ -275,15 +275,13 @@ class ShellTest(testutils.TestCase):
         # authenticate to get the verison list *and* to excuted the command.
         # This is not the ideal behavior and it should be fixed in a follow
         # up patch.
-        self._assert_auth_plugin_args()
 
     @mock.patch('glanceclient.v1.client.Client')
     def test_auth_plugin_invocation_with_v1(self, v1_client):
         args = '--os-image-api-version 1 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         glance_shell.main(args.split())
-        self.assertEqual(1, self.v2_auth.call_count)
-        self._assert_auth_plugin_args()
+        self.assertEqual(0, self.v2_auth.call_count)
 
     @mock.patch('glanceclient.v2.client.Client')
     def test_auth_plugin_invocation_with_v2(self,
@@ -291,8 +289,7 @@ class ShellTest(testutils.TestCase):
         args = '--os-image-api-version 2 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         glance_shell.main(args.split())
-        self.assertEqual(1, self.v2_auth.call_count)
-        self._assert_auth_plugin_args()
+        self.assertEqual(0, self.v2_auth.call_count)
 
     @mock.patch('glanceclient.v1.client.Client')
     def test_auth_plugin_invocation_with_unversioned_auth_url_with_v1(
@@ -301,7 +298,6 @@ class ShellTest(testutils.TestCase):
                 DEFAULT_UNVERSIONED_AUTH_URL)
         glance_shell = openstack_shell.OpenStackImagesShell()
         glance_shell.main(args.split())
-        self._assert_auth_plugin_args()
 
     @mock.patch('glanceclient.v2.client.Client')
     @mock.patch.object(openstack_shell.OpenStackImagesShell, '_cache_schemas',
@@ -312,7 +308,6 @@ class ShellTest(testutils.TestCase):
                 'image-list') % DEFAULT_UNVERSIONED_AUTH_URL
         glance_shell = openstack_shell.OpenStackImagesShell()
         glance_shell.main(args.split())
-        self._assert_auth_plugin_args()
 
     @mock.patch('glanceclient.Client')
     def test_endpoint_token_no_auth_req(self, mock_client):
@@ -333,22 +328,17 @@ class ShellTest(testutils.TestCase):
         glance_shell.main(args)
         self.assertEqual(1, mock_client.call_count)
 
-    @mock.patch('sys.stdin', side_effect=mock.MagicMock)
-    @mock.patch('getpass.getpass', return_value='password')
     @mock.patch('glanceclient.v2.client.Client')
-    def test_password_prompted_with_v2(self, v2_client,
-                                       mock_getpass, mock_stdin):
+    def test_password_prompted_with_v2(self, v2_client):
         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,
+        os.environ['OS_PASSWORD'] = 'password'
+        self.assertRaises(exc.CommunicationError,
                           glance_shell.main, ['image-list'])
-        # Make sure we are actually prompted.
-        mock_getpass.assert_called_once_with('OS Password: ')
 
     @mock.patch('sys.stdin', side_effect=mock.MagicMock)
     @mock.patch('getpass.getpass', side_effect=EOFError)
@@ -561,16 +551,14 @@ class ShellTestWithKeystoneV3Auth(ShellTest):
         args = '--os-image-api-version 1 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         glance_shell.main(args.split())
-        self.assertEqual(1, self.v3_auth.call_count)
-        self._assert_auth_plugin_args()
+        self.assertEqual(0, self.v3_auth.call_count)
 
     @mock.patch('glanceclient.v2.client.Client')
     def test_auth_plugin_invocation_with_v2(self, v2_client):
         args = '--os-image-api-version 2 image-list'
         glance_shell = openstack_shell.OpenStackImagesShell()
         glance_shell.main(args.split())
-        self.assertEqual(1, self.v3_auth.call_count)
-        self._assert_auth_plugin_args()
+        self.assertEqual(0, self.v3_auth.call_count)
 
     @mock.patch('keystoneclient.discover.Discover',
                 side_effect=ks_exc.ClientException())