From 04a973be4e5106c0bf001de9054a46a4d7a88463 Mon Sep 17 00:00:00 2001
From: Sripriya <sseetha@brocade.com>
Date: Thu, 26 May 2016 11:28:36 -0700
Subject: [PATCH] Fix keystone error handling in openstack driver

Keystone exceptions are not thrown during client initialization. User
credentials are only validated when keystone client apis such as
regions, services and endpoints are queried using the client object.

Change-Id: I0bda03353e054c0f00a704a4619eceb670e1a7f7
Closes-Bug: #1567123
(cherry picked from commit 92c891b83586fbe40787b02c97a6ba69238d4728)
---
 tacker/nfvo/drivers/vim/openstack_driver.py   | 22 +++++++++----------
 .../nfvo/drivers/vim/test_openstack_driver.py | 14 ++++++++++++
 tacker/vm/keystone.py                         |  9 ++------
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/tacker/nfvo/drivers/vim/openstack_driver.py b/tacker/nfvo/drivers/vim/openstack_driver.py
index f04e25e74..c097a5648 100644
--- a/tacker/nfvo/drivers/vim/openstack_driver.py
+++ b/tacker/nfvo/drivers/vim/openstack_driver.py
@@ -16,6 +16,7 @@
 
 import os
 
+from keystoneclient import exceptions
 from oslo_config import cfg
 
 from tacker.common import log
@@ -95,12 +96,7 @@ class OpenStack_Driver(abstract_vim_driver.VimAbstractDriver):
         return keystone_version
 
     def _initialize_keystone(self, version, auth):
-        try:
-            ks_client = self.keystone.initialize_client(version=version,
-                                                        **auth)
-        except Exception as e:
-            LOG.error(_('VIM authentication failed'))
-            raise nfvo.VimUnauthorizedException(message=e.message)
+        ks_client = self.keystone.initialize_client(version=version, **auth)
         return ks_client
 
     def _find_regions(self, ks_client):
@@ -111,14 +107,12 @@ class OpenStack_Driver(abstract_vim_driver.VimAbstractDriver):
                 if service.type == 'orchestration':
                     heat_service_id = service.id
             endpoints_list = ks_client.endpoints.list()
-            region_list = [endpoint.region for endpoint in endpoints_list if
-                           endpoint.service_id == heat_service_id]
+            region_list = [endpoint.region for endpoint in
+                           endpoints_list if endpoint.service_id ==
+                           heat_service_id]
         else:
             region_info = ks_client.regions.list()
             region_list = [region.id for region in region_info]
-        if not region_list:
-            LOG.info(_('Unable to find VIM regions'))
-            return
         return region_list
 
     def discover_placement_attr(self, vim_obj, ks_client):
@@ -126,7 +120,11 @@ class OpenStack_Driver(abstract_vim_driver.VimAbstractDriver):
 
         Attributes can include regions, AZ.
         """
-        regions_list = self._find_regions(ks_client)
+        try:
+            regions_list = self._find_regions(ks_client)
+        except exceptions.Unauthorized as e:
+            LOG.warn(_("Authorization failed for user"))
+            raise nfvo.VimUnauthorizedException(message=e.message)
         vim_obj['placement_attr'] = {'regions': regions_list}
         return vim_obj
 
diff --git a/tacker/tests/unit/vm/nfvo/drivers/vim/test_openstack_driver.py b/tacker/tests/unit/vm/nfvo/drivers/vim/test_openstack_driver.py
index 48da8a557..815a3d520 100644
--- a/tacker/tests/unit/vm/nfvo/drivers/vim/test_openstack_driver.py
+++ b/tacker/tests/unit/vm/nfvo/drivers/vim/test_openstack_driver.py
@@ -13,9 +13,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from keystoneclient import exceptions
 import mock
 from oslo_config import cfg
 
+from tacker.extensions import nfvo
 from tacker.nfvo.drivers.vim import openstack_driver
 from tacker.tests.unit import base
 from tacker.tests.unit.db import utils
@@ -115,3 +117,15 @@ class TestOpenstack_Driver(base.TestCase):
         mock_os_path.return_value = file_path
         self.openstack_driver.deregister_vim(vim_id)
         mock_os_remove.assert_called_once_with(file_path)
+
+    def test_register_vim_invalid_credentials(self):
+        attrs = {'regions.list.side_effect': exceptions.Unauthorized}
+        keystone_version = 'v3'
+        mock_ks_client = mock.Mock(version=keystone_version, **attrs)
+        self.keystone.get_version.return_value = keystone_version
+        self.keystone.initialize_client.return_value = mock_ks_client
+        self.assertRaises(nfvo.VimUnauthorizedException,
+                          self.openstack_driver.register_vim, self.vim_obj)
+        mock_ks_client.regions.list.assert_called_once_with()
+        self.keystone.initialize_client.assert_called_once_with(
+            version=keystone_version, **self.auth_obj)
diff --git a/tacker/vm/keystone.py b/tacker/vm/keystone.py
index 438b45f98..435a11f19 100644
--- a/tacker/vm/keystone.py
+++ b/tacker/vm/keystone.py
@@ -58,13 +58,8 @@ class Keystone(object):
             from keystoneclient.v3 import client
             auth_plugin = identity.v3.Password(**kwargs)
         ses = self.get_session(auth_plugin=auth_plugin)
-        try:
-            cli = client.Client(session=ses)
-            return cli
-        except (exceptions.AuthorizationFailure,
-                exceptions.Unauthorized):
-            LOG.warn(_("Authorization failed for user"))
-            raise
+        cli = client.Client(session=ses)
+        return cli
 
     @staticmethod
     def create_key_dir(path):