report_root should always use context.user

The RootHistory intends to record by whom and when the root user of the
underlying datastore backend was enabled. So the "user" column should
always be set to the context user, not the actual root user in the
database. But report_root(and report_root_enabled in
taskmanager.models) use the database user instead, this is not correct,
This patch will fix.

Also remove the "user" argument from Root.create() and
HistoryRoot.create() because the context is already passed in as an
argument when these methods are called.

Closes-Bug: #1546372
Change-Id: I3b4c8ee56c7e0876fb384f0c5841d2d391bd555d
Signed-off-by: Zhao Chao <zhaochao1984@gmail.com>

trove/common/strategies/cluster/experimental/galera_common/taskmanager.py

@@ -159,8 +159,7 @@ class GaleraCommonClusterTasks(task_models.ClusterTasks):
         for instance in existing_instances:
             if ext_models.Root.load(context, instance.id):
                 for new_instance in new_instances:
-                    ext_models.RootHistory.create(context, new_instance.id,
+                    ext_models.RootHistory.create(context, new_instance.id)
-                                                  context.user)
                 return
 
     def grow_cluster(self, context, cluster_id, new_instance_ids):

trove/conductor/api.py

@@ -92,14 +92,13 @@ class API(object):
                    sent=sent,
                    **backup_fields)
 
-    def report_root(self, instance_id, user):
+    def report_root(self, instance_id):
         LOG.debug("Making async call to cast report_root for instance: %s",
                   instance_id)
         version = self.API_BASE_VERSION
         cctxt = self.client.prepare(version=version)
         cctxt.cast(self.context, "report_root",
-                   instance_id=instance_id,
+                   instance_id=instance_id)
-                   user=user)
 
     def notify_end(self, **notification_args):
         LOG.debug("Making async call to cast end notification")

trove/conductor/manager.py

@@ -135,8 +135,13 @@ class Manager(periodic_task.PeriodicTasks):
                 setattr(backup, k, v)
         backup.save()
 
-    def report_root(self, context, instance_id, user):
+    # NOTE(zhaochao): the 'user' argument is left here to keep
-        mysql_models.RootHistory.create(context, instance_id, user)
+    # compatible with existing instances.
+    def report_root(self, context, instance_id, user=None):
+        if user is not None:
+            LOG.debug("calling report_root with a username: %s, "
+                      "is deprecated now!" % user)
+        mysql_models.RootHistory.create(context, instance_id)
 
     def notify_end(self, context, serialized_notification, notification_args):
         notification = SerializableNotification.deserialize(

trove/extensions/common/models.py

@@ -54,7 +54,7 @@ class Root(object):
         return True
 
     @classmethod
-    def create(cls, context, instance_id, user, root_password,
+    def create(cls, context, instance_id, root_password,
                cluster_instances_list=None):
         load_and_verify(context, instance_id)
         if root_password:
@@ -71,7 +71,7 @@ class Root(object):
         # if cluster_instances_list none, then root create is called for
         # single instance, adding an RootHistory entry for the instance_id
         if cluster_instances_list is None:
-            RootHistory.create(context, instance_id, user)
+            RootHistory.create(context, instance_id)
 
         return root_user
 
@@ -84,15 +84,15 @@ class Root(object):
 class ClusterRoot(Root):
 
     @classmethod
-    def create(cls, context, instance_id, user, root_password,
+    def create(cls, context, instance_id, root_password,
                cluster_instances_list=None):
         root_user = super(ClusterRoot, cls).create(context, instance_id,
-                                                   user, root_password,
+                                                   root_password,
                                                    cluster_instances_list=None)
 
         if cluster_instances_list:
             for instance in cluster_instances_list:
-                RootHistory.create(context, instance, user)
+                RootHistory.create(context, instance)
 
         return root_user
 
@@ -119,9 +119,9 @@ class RootHistory(object):
         return history
 
     @classmethod
-    def create(cls, context, instance_id, user):
+    def create(cls, context, instance_id):
         history = cls.load(context, instance_id)
         if history is not None:
             return history
-        history = RootHistory(instance_id, user)
+        history = RootHistory(instance_id, context.user)
         return history.save()

trove/extensions/common/service.py

@@ -105,10 +105,8 @@ class DefaultRootController(BaseDatastoreRootController):
         LOG.info("Enabling root for instance '%s'.", instance_id)
         LOG.info("req : '%s'\n\n", req)
         context = req.environ[wsgi.CONTEXT_KEY]
-        user_name = context.user
         password = DefaultRootController._get_password_from_body(body)
-        root = models.Root.create(context, instance_id,
+        root = models.Root.create(context, instance_id, password)
-                                  user_name, password)
         return wsgi.Result(views.RootCreatedView(root).data(), 200)
 
     def root_delete(self, req, tenant_id, instance_id, is_cluster):
@@ -175,9 +173,8 @@ class ClusterRootController(DefaultRootController):
         LOG.info("Enabling root for instance '%s'.", instance_id)
         LOG.info("req : '%s'\n\n", req)
         context = req.environ[wsgi.CONTEXT_KEY]
-        user_name = context.user
         password = ClusterRootController._get_password_from_body(body)
-        root = models.ClusterRoot.create(context, instance_id, user_name,
+        root = models.ClusterRoot.create(context, instance_id,
                                          password, cluster_instances)
         return wsgi.Result(views.RootCreatedView(root).data(), 200)
 

trove/extensions/redis/service.py

@@ -54,15 +54,13 @@ class RedisRootController(DefaultRootController):
                  instance_id)
         LOG.info("req : '%s'\n\n", req)
         context = req.environ[wsgi.CONTEXT_KEY]
-        user_name = context.user
 
         original_auth_password = self._get_original_auth_password(
             context, instance_id)
 
         # Do root-enable and roll back once if operation fails.
         try:
-            root = RedisRoot.create(context, instance_id,
+            root = RedisRoot.create(context, instance_id, password)
-                                    user_name, password)
             if not password:
                 password = root.password
         except exception.TroveError:
@@ -77,7 +75,7 @@ class RedisRootController(DefaultRootController):
             try:
                 LOG.info("Enabling authentication for slave instance "
                          "'%s'.", slave_id)
-                RedisRoot.create(context, slave_id, user_name, password)
+                RedisRoot.create(context, slave_id, password)
             except exception.TroveError:
                 failed_slaves.append(slave_id)
 
@@ -125,7 +123,6 @@ class RedisRootController(DefaultRootController):
         LOG.info("Rolling back enable/disable authentication "
                  "for instance '%s'.", instance_id)
         context = req.environ[wsgi.CONTEXT_KEY]
-        user_name = context.user
         try:
             if not original_auth_password:
                 # Instance never did root-enable before.
@@ -133,7 +130,7 @@ class RedisRootController(DefaultRootController):
             else:
                 # Instance has done root-enable successfully before.
                 # So roll back with original password.
-                RedisRoot.create(context, instance_id, user_name,
+                RedisRoot.create(context, instance_id,
                                  original_auth_password)
         except exception.TroveError:
             LOG.exception("Rolling back failed for instance '%s'",

trove/guestagent/datastore/experimental/cassandra/manager.py

@@ -177,7 +177,7 @@ class Manager(manager.Manager):
             self._admin = self.app.build_admin()
 
         if not cluster_config and self.is_root_enabled(context):
-            self.status.report_root(context, self.app.default_superuser_name)
+            self.status.report_root(context)
 
     def change_passwords(self, context, users):
         with EndNotification(context):

trove/guestagent/datastore/experimental/mongodb/manager.py

@@ -91,7 +91,7 @@ class Manager(manager.Manager):
         if not cluster_config and backup_info:
             self._perform_restore(backup_info, context, mount_point, self.app)
             if service.MongoDBAdmin().is_root_enabled():
-                self.app.status.report_root(context, 'root')
+                self.app.status.report_root(context)
 
     def restart(self, context):
         LOG.debug("Restarting MongoDB.")

trove/guestagent/datastore/experimental/postgresql/manager.py

@@ -245,7 +245,7 @@ class Manager(manager.Manager):
         self._admin = PgSqlAdmin(os_admin)
 
         if not cluster_config and self.is_root_enabled(context):
-            self.status.report_root(context, self.app.default_superuser_name)
+            self.status.report_root(context)
 
     def create_backup(self, context, backup_info):
         with EndNotification(context):

trove/guestagent/datastore/mysql_common/manager.py

@@ -234,7 +234,7 @@ class MySqlManager(manager.Manager):
                                   self.mysql_admin().is_root_enabled())
         if enable_root_on_restore:
             app.secure_root(secure_remote_root=False)
-            self.mysql_app_status.get().report_root(context, 'root')
+            self.mysql_app_status.get().report_root(context)
         else:
             app.secure_root(secure_remote_root=True)
 

trove/guestagent/datastore/service.py

@@ -379,8 +379,8 @@ class BaseDbStatus(object):
         """
         LOG.debug("No cleanup action specified for this datastore.")
 
-    def report_root(self, context, user):
+    def report_root(self, context):
         """Use conductor to update the root-enable status."""
         LOG.debug("Casting report_root message to conductor.")
-        conductor_api.API(context).report_root(CONF.guest_id, user)
+        conductor_api.API(context).report_root(CONF.guest_id)
         LOG.debug("Successfully cast report_root.")

trove/taskmanager/models.py

@@ -663,7 +663,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
                                 create_fmt_content, err)
 
     def report_root_enabled(self):
-        mysql_models.RootHistory.create(self.context, self.id, 'root')
+        mysql_models.RootHistory.create(self.context, self.id)
 
     def update_statuses_on_time_out(self):
 

trove/tests/unittests/common/test_common_extensions.py

@@ -72,7 +72,7 @@ class TestDefaultRootController(trove_testtools.TestCase):
         password = Mock()
         body = {'password': password}
         self.controller.root_create(req, body, tenant_id, uuid, is_cluster)
-        root_create.assert_called_with(context, uuid, context.user, password)
+        root_create.assert_called_with(context, uuid, password)
 
     def test_root_create_with_cluster(self):
         req = Mock()
@@ -297,7 +297,7 @@ class TestClusterRootController(trove_testtools.TestCase):
         self.controller.instance_root_create(
             req, body, instance_id, cluster_instances)
         mock_cluster_root_create.assert_called_with(
-            self.context, instance_id, self.context.user, password,
+            self.context, instance_id, password,
             cluster_instances)
 
     @patch.object(models.ClusterRoot, "create")
@@ -314,5 +314,5 @@ class TestClusterRootController(trove_testtools.TestCase):
         self.controller.instance_root_create(
             req, body, instance_id, cluster_instances)
         mock_cluster_root_create.assert_called_with(
-            self.context, instance_id, self.context.user, password,
+            self.context, instance_id, password,
             cluster_instances)

trove/tests/unittests/redis/test_redis_root_controller.py

@@ -103,7 +103,7 @@ class TestRedisRootController(trove_testtools.TestCase):
         self.controller.root_create(req, body, tenant_id,
                                     instance_id, is_cluster)
         root_create.assert_called_with(context, instance_id,
-                                       context.user, password)
+                                       password)
 
     @patch.object(instance_models.Instance, "load")
     @patch.object(models.Root, "create")
@@ -124,7 +124,7 @@ class TestRedisRootController(trove_testtools.TestCase):
         self.controller.root_create(req, body, tenant_id,
                                     instance_id, is_cluster)
         root_create.assert_called_with(context, slave_instance_id,
-                                       context.user, password)
+                                       password)
 
     def test_root_create_on_slave(self):
         user = Mock()

trove/tests/unittests/taskmanager/test_models.py

@@ -1084,17 +1084,20 @@ class RootReportTest(trove_testtools.TestCase):
         super(RootReportTest, self).tearDown()
 
     def test_report_root_first_time(self):
+        context = Mock()
+        context.user = utils.generate_uuid()
         report = mysql_models.RootHistory.create(
-            None, utils.generate_uuid(), 'root')
+            context, utils.generate_uuid())
         self.assertIsNotNone(report)
 
     def test_report_root_double_create(self):
+        context = Mock()
+        context.user = utils.generate_uuid()
         uuid = utils.generate_uuid()
-        history = mysql_models.RootHistory(uuid, 'root').save()
+        history = mysql_models.RootHistory(uuid, context.user).save()
         with patch.object(mysql_models.RootHistory, 'load',
                           Mock(return_value=history)):
-            report = mysql_models.RootHistory.create(
+            report = mysql_models.RootHistory.create(context, uuid)
-                None, uuid, 'root')
             self.assertTrue(mysql_models.RootHistory.load.called)
             self.assertEqual(history.user, report.user)
             self.assertEqual(history.id, report.id)
@@ -1106,17 +1109,17 @@ class ClusterRootTest(trove_testtools.TestCase):
     @patch.object(common_models.Root, "create")
     def test_cluster_root_create(self, root_create, root_history_create):
         context = Mock()
+        context.user = utils.generate_uuid()
         uuid = utils.generate_uuid()
-        user = "root"
         password = "rootpassword"
         cluster_instances = [utils.generate_uuid(), utils.generate_uuid()]
-        common_models.ClusterRoot.create(context, uuid, user, password,
+        common_models.ClusterRoot.create(context, uuid, password,
                                          cluster_instances)
-        root_create.assert_called_with(context, uuid, user, password,
+        root_create.assert_called_with(context, uuid, password,
                                        cluster_instances_list=None)
         self.assertEqual(2, root_history_create.call_count)
         calls = [
-            call(context, cluster_instances[0], user),
+            call(context, cluster_instances[0]),
-            call(context, cluster_instances[1], user)
+            call(context, cluster_instances[1])
         ]
         root_history_create.assert_has_calls(calls)