From ccebf0c166c4204ec32e922871ec39bd62225ab0 Mon Sep 17 00:00:00 2001
From: Shashank Hegde <shashank@arista.com>
Date: Mon, 8 Sep 2014 15:05:30 -0700
Subject: [PATCH] Passing admin tenant name to EOS

The Arista ML2 plugin was not passing the admin tenant name to EOS without which
it is not possible to authenticate with keystone using just the admin name
and password. This patch passes the admin tenant name along with the admin
credentials.

Change-Id: I6c8b872087d17da2c3de43186d1916fc368dd786
Closes-Bug: 1359417
---
 .../ml2/drivers/arista/mechanism_arista.py    | 41 +++++++++++++------
 .../arista/test_arista_mechanism_driver.py    | 23 +++++++++++
 2 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/neutron/plugins/ml2/drivers/arista/mechanism_arista.py b/neutron/plugins/ml2/drivers/arista/mechanism_arista.py
index b52c98bce..990cca983 100644
--- a/neutron/plugins/ml2/drivers/arista/mechanism_arista.py
+++ b/neutron/plugins/ml2/drivers/arista/mechanism_arista.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import itertools
 import threading
 
 import jsonrpclib
@@ -377,15 +378,18 @@ class AristaRPCWrapper(object):
         This the initial handshake between Neutron and EOS.
         critical end-point information is registered with EOS.
         """
-        cmds = ['auth url %s user "%s" password "%s"' %
-                (self._keystone_url(),
-                self.keystone_conf.admin_user,
-                self.keystone_conf.admin_password)]
-
-        log_cmds = ['auth url %s user %s password ******' %
-                    (self._keystone_url(),
-                    self.keystone_conf.admin_user)]
 
+        cmds = ['auth url %s user %s password %s tenant %s' % (
+                self._keystone_url(),
+                self.keystone_conf.admin_user,
+                self.keystone_conf.admin_password,
+                self.keystone_conf.admin_tenant_name)]
+
+        log_cmds = ['auth url %s user %s password %s tenant %s' % (
+                    self._keystone_url(),
+                    self.keystone_conf.admin_user,
+                    '******',
+                    self.keystone_conf.admin_tenant_name)]
         self._run_openstack_cmds(cmds, commands_to_log=log_cmds)
 
     def clear_region_updated_time(self):
@@ -424,11 +428,11 @@ class AristaRPCWrapper(object):
                                  param is logged.
         """
 
-        log_cmd = commands
+        log_cmds = commands
         if commands_to_log:
-            log_cmd = commands_to_log
+            log_cmds = commands_to_log
 
-        LOG.info(_('Executing command on Arista EOS: %s'), log_cmd)
+        LOG.info(_('Executing command on Arista EOS: %s'), log_cmds)
 
         try:
             # this returns array of return values for every command in
@@ -436,10 +440,21 @@ class AristaRPCWrapper(object):
             ret = self._server.runCmds(version=1, cmds=commands)
         except Exception as error:
             host = cfg.CONF.ml2_arista.eapi_host
+            error_msg_str = unicode(error)
+            if commands_to_log:
+                # The command might contain sensitive information. If the
+                # command to log is different from the actual command, use
+                # that in the error message.
+                for cmd, log_cmd in itertools.izip(commands, log_cmds):
+                    error_msg_str = error_msg_str.replace(cmd, log_cmd)
             msg = (_('Error %(err)s while trying to execute '
                      'commands %(cmd)s on EOS %(host)s') %
-                   {'err': error, 'cmd': commands_to_log, 'host': host})
-            LOG.exception(msg)
+                  {'err': error_msg_str,
+                   'cmd': commands_to_log,
+                   'host': host})
+            # Logging exception here can reveal passwords as the exception
+            # contains the CLI command which contains the credentials.
+            LOG.error(msg)
             raise arista_exc.AristaRpcError(msg=msg)
 
         return ret
diff --git a/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py b/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py
index 56b8a42af..7eeb97cac 100644
--- a/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py
+++ b/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py
@@ -503,6 +503,28 @@ class PositiveRPCWrapperValidConfigTestCase(base.BaseTestCase):
         cmds = ['show openstack config region RegionOne timestamp']
         self.drv._server.runCmds.assert_called_once_with(version=1, cmds=cmds)
 
+    def test_register_with_eos(self):
+        self.drv.register_with_eos()
+        auth = fake_keystone_info_class()
+        keystone_url = '%s://%s:%s/v2.0/' % (auth.auth_protocol,
+                                             auth.auth_host,
+                                             auth.auth_port)
+        auth_cmd = 'auth url %s user %s password %s tenant %s' % (keystone_url,
+                    auth.admin_user,
+                    auth.admin_password,
+                    auth.admin_tenant_name)
+        cmds = ['enable',
+                'configure',
+                'cvx',
+                'service openstack',
+                'region %s' % self.region,
+                auth_cmd,
+                'exit',
+                'exit',
+                'exit',
+                ]
+        self.drv._server.runCmds.assert_called_once_with(version=1, cmds=cmds)
+
 
 class AristaRPCWrapperInvalidConfigTestCase(base.BaseTestCase):
     """Negative test cases to test the Arista Driver configuration."""
@@ -676,6 +698,7 @@ class fake_keystone_info_class(object):
     auth_port = 5000
     admin_user = 'neutron'
     admin_password = 'fun'
+    admin_tenant_name = 'tenant_name'
 
 
 class FakeNetworkContext(object):
-- 
2.45.2