From 579f5afc6441e2279d3d660b90268c760238202a Mon Sep 17 00:00:00 2001 From: Zhao Chao Date: Sat, 24 Feb 2018 21:59:58 +0800 Subject: [PATCH] Use RootHistory to check if root is ever enabled When disabling root, there is no need to call guestagent to check whether the root user is ever enabled. Root hisotry table should be used for this purpose. As datastore specific root controller of MySQL/Cassandra/PostgreSQL were created only for the '_find_root_user' which were calling guestagent to find root user, these controllers are removed and 'DefaultRootController' is used instead. RedisRootController is also updated as it didn't do this check previously. Unittests directory structure is also slightly changed. It's more clear to use similar directory hierarchies for testing and source code, e.g. trove/extensions/common/service.py trove/tests/unitests/extensions/common/test_service.py Change-Id: I9faac61d9650347b51f23e8fcaf5a92aed5fbf93 Signed-off-by: Zhao Chao --- tools/trove-pylint.config | 12 --- trove/common/cfg.py | 8 +- trove/common/exception.py | 5 ++ trove/common/wsgi.py | 1 + trove/extensions/cassandra/service.py | 28 ------ trove/extensions/common/service.py | 12 ++- trove/extensions/mysql/service.py | 10 --- trove/extensions/postgresql/service.py | 29 ------ trove/extensions/redis/service.py | 4 + trove/tests/unittests/common/test_wsgi.py | 20 +++++ .../unittests/extensions}/__init__.py | 0 .../unittests/extensions/common}/__init__.py | 0 .../common/test_service.py} | 90 +++++++++++++++++++ .../{ => extensions}/redis/__init__.py | 0 .../redis/test_service.py} | 35 +++++++- 15 files changed, 161 insertions(+), 93 deletions(-) delete mode 100644 trove/extensions/cassandra/service.py delete mode 100644 trove/extensions/postgresql/service.py rename trove/{extensions/cassandra => tests/unittests/extensions}/__init__.py (100%) rename trove/{extensions/postgresql => tests/unittests/extensions/common}/__init__.py (100%) rename trove/tests/unittests/{common/test_common_extensions.py => extensions/common/test_service.py} (78%) rename trove/tests/unittests/{ => extensions}/redis/__init__.py (100%) rename trove/tests/unittests/{redis/test_redis_root_controller.py => extensions/redis/test_service.py} (83%) diff --git a/tools/trove-pylint.config b/tools/trove-pylint.config index 970a7ce6a9..cacdadefca 100644 --- a/tools/trove-pylint.config +++ b/tools/trove-pylint.config @@ -843,18 +843,6 @@ "Instance of 'Client' has no 'records' member", "DesignateDriver.delete_entry" ], - [ - "trove/extensions/common/service.py", - "E1101", - "Instance of 'DefaultRootController' has no '_find_root_user' member", - "DefaultRootController.root_delete" - ], - [ - "trove/extensions/common/service.py", - "no-member", - "Instance of 'DefaultRootController' has no '_find_root_user' member", - "DefaultRootController.root_delete" - ], [ "trove/extensions/mgmt/instances/service.py", "E1101", diff --git a/trove/common/cfg.py b/trove/common/cfg.py index 81d6528fe4..c1027eaa15 100644 --- a/trove/common/cfg.py +++ b/trove/common/cfg.py @@ -588,7 +588,7 @@ mysql_opts = [ deprecated_name='backup_incremental_strategy', deprecated_group='DEFAULT'), cfg.StrOpt('root_controller', - default='trove.extensions.mysql.service.MySQLRootController', + default='trove.extensions.common.service.DefaultRootController', help='Root controller implementation for mysql.'), cfg.ListOpt('ignore_users', default=['os_admin', 'root'], help='Users to exclude when listing users.', @@ -921,8 +921,7 @@ cassandra_opts = [ deprecated_name='restore_namespace', deprecated_group='DEFAULT'), cfg.StrOpt('root_controller', - default='trove.extensions.cassandra.service' - '.CassandraRootController', + default='trove.extensions.common.service.DefaultRootController', help='Root controller implementation for Cassandra.'), cfg.ListOpt('ignore_users', default=['os_admin'], help='Users to exclude when listing users.'), @@ -1192,8 +1191,7 @@ postgresql_opts = [ cfg.ListOpt('ignore_users', default=['os_admin', 'postgres', 'root']), cfg.ListOpt('ignore_dbs', default=['os_admin', 'postgres']), cfg.StrOpt('root_controller', - default='trove.extensions.postgresql.service' - '.PostgreSQLRootController', + default='trove.extensions.common.service.DefaultRootController', help='Root controller implementation for postgresql.'), cfg.StrOpt('guest_log_exposed_logs', default='general', help='List of Guest Logs to expose for publishing.'), diff --git a/trove/common/exception.py b/trove/common/exception.py index 9ba5a6a193..094fee41f0 100644 --- a/trove/common/exception.py +++ b/trove/common/exception.py @@ -86,6 +86,11 @@ class UserNotFound(NotFound): message = _("User %(uuid)s cannot be found on the instance.") +class RootHistoryNotFound(NotFound): + + message = _("Root user has never been enabled on the instance.") + + class DatabaseNotFound(NotFound): message = _("Database %(uuid)s cannot be found on the instance.") diff --git a/trove/common/wsgi.py b/trove/common/wsgi.py index cd624d96d2..d28f2bca08 100644 --- a/trove/common/wsgi.py +++ b/trove/common/wsgi.py @@ -347,6 +347,7 @@ class Controller(object): exception.DatastoreNotFound, exception.SwiftNotFound, exception.ModuleTypeNotFound, + exception.RootHistoryNotFound, ], webob.exc.HTTPConflict: [ exception.BackupNotCompleteError, diff --git a/trove/extensions/cassandra/service.py b/trove/extensions/cassandra/service.py deleted file mode 100644 index ffffab07c3..0000000000 --- a/trove/extensions/cassandra/service.py +++ /dev/null @@ -1,28 +0,0 @@ -# Copyright 2015 Tesora Inc. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from trove.common.db.cassandra import models as guest_models -from trove.extensions.common.service import DefaultRootController -from trove.extensions.mysql import models - - -class CassandraRootController(DefaultRootController): - - def _find_root_user(self, context, instance_id): - user = guest_models.CassandraUser.root() - # TODO(pmalik): Using MySQL model until we have datastore specific - # extensions (bug/1498573). - return models.User.load( - context, instance_id, user.name, user.host, root_user=True) diff --git a/trove/extensions/common/service.py b/trove/extensions/common/service.py index 3b24824102..75a659411f 100644 --- a/trove/extensions/common/service.py +++ b/trove/extensions/common/service.py @@ -116,12 +116,9 @@ class DefaultRootController(BaseDatastoreRootController): LOG.info("Disabling root for instance '%s'.", instance_id) LOG.info("req : '%s'\n\n", req) context = req.environ[wsgi.CONTEXT_KEY] - try: - found_user = self._find_root_user(context, instance_id) - except (ValueError, AttributeError) as e: - raise exception.BadRequest(message=str(e)) - if not found_user: - raise exception.UserNotFound(uuid="root") + is_root_enabled = models.Root.load(context, instance_id) + if not is_root_enabled: + raise exception.RootHistoryNotFound() models.Root.delete(context, instance_id) return wsgi.Result(None, 200) @@ -238,7 +235,8 @@ class RootController(ExtensionController): return root_controller.root_delete(req, tenant_id, instance_id, is_cluster) else: - raise NoSuchOptError + opt = 'root_controller' + raise NoSuchOptError(opt, group='datastore_manager') def _get_datastore(self, tenant_id, instance_or_cluster_id): """ diff --git a/trove/extensions/mysql/service.py b/trove/extensions/mysql/service.py index 306ebd6ffe..f68e1c7f88 100644 --- a/trove/extensions/mysql/service.py +++ b/trove/extensions/mysql/service.py @@ -29,7 +29,6 @@ from trove.common.notification import StartNotification from trove.common import pagination from trove.common.utils import correct_id_with_req from trove.common import wsgi -from trove.extensions.common.service import DefaultRootController from trove.extensions.common.service import ExtensionController from trove.extensions.mysql.common import populate_users from trove.extensions.mysql.common import populate_validated_databases @@ -372,12 +371,3 @@ class SchemaController(ExtensionController): self.authorize_target_action( context, 'database:show', instance_id) raise webob.exc.HTTPNotImplemented() - - -class MySQLRootController(DefaultRootController): - - def _find_root_user(self, context, instance_id): - user = guest_models.MySQLUser.root() - return models.User.load(context, instance_id, - user.name, user.host, - root_user=True) diff --git a/trove/extensions/postgresql/service.py b/trove/extensions/postgresql/service.py deleted file mode 100644 index 316641e29d..0000000000 --- a/trove/extensions/postgresql/service.py +++ /dev/null @@ -1,29 +0,0 @@ -# Copyright 2015 Tesora Inc. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from trove.common.db.postgresql import models as guest_models -from trove.extensions.common.service import DefaultRootController -from trove.extensions.mysql import models - - -class PostgreSQLRootController(DefaultRootController): - - def _find_root_user(self, context, instance_id): - user = guest_models.PostgreSQLUser.root() - # This is currently using MySQL model. - # MySQL extension *should* work for now, but may lead to - # future bugs (incompatible input validation, unused field etc). - return models.User.load( - context, instance_id, user.name, user.host, root_user=True) diff --git a/trove/extensions/redis/service.py b/trove/extensions/redis/service.py index f5be1d8d5b..0f5451d143 100644 --- a/trove/extensions/redis/service.py +++ b/trove/extensions/redis/service.py @@ -88,6 +88,10 @@ class RedisRootController(DefaultRootController): LOG.info("req : '%s'\n\n", req) context = req.environ[wsgi.CONTEXT_KEY] + is_root_enabled = RedisRoot.load(context, instance_id) + if not is_root_enabled: + raise exception.RootHistoryNotFound() + original_auth_password = self._get_original_auth_password( context, instance_id) diff --git a/trove/tests/unittests/common/test_wsgi.py b/trove/tests/unittests/common/test_wsgi.py index 7e8c26b0c6..9a07e009bb 100644 --- a/trove/tests/unittests/common/test_wsgi.py +++ b/trove/tests/unittests/common/test_wsgi.py @@ -13,7 +13,12 @@ # License for the specific language governing permissions and limitations # under the License. # +from mock import Mock, patch from testtools.matchers import Equals, Is, Not +import webob.exc + +from trove.common import base_wsgi +from trove.common import exception from trove.common import wsgi from trove.tests.unittests import trove_testtools import webob @@ -40,3 +45,18 @@ class TestWsgi(trove_testtools.TestCase): self.assertThat(ctx.user, Equals(user_id)) self.assertThat(ctx.auth_token, Equals(token)) self.assertEqual(0, len(ctx.service_catalog)) + + +class TestController(trove_testtools.TestCase): + + @patch.object(base_wsgi.Resource, 'execute_action', + side_effect=exception.RootHistoryNotFound()) + @patch.object(wsgi.Controller, 'delete', create=True) + @patch.object(wsgi.Controller, 'validate_request') + def test_exception_root_history_notfound(self, *args): + controller = wsgi.Controller() + resource = controller.create_resource() + req = Mock() + result = resource.execute_action('delete', req) + self.assertIsInstance(result.wrapped_exc, + webob.exc.HTTPNotFound) diff --git a/trove/extensions/cassandra/__init__.py b/trove/tests/unittests/extensions/__init__.py similarity index 100% rename from trove/extensions/cassandra/__init__.py rename to trove/tests/unittests/extensions/__init__.py diff --git a/trove/extensions/postgresql/__init__.py b/trove/tests/unittests/extensions/common/__init__.py similarity index 100% rename from trove/extensions/postgresql/__init__.py rename to trove/tests/unittests/extensions/common/__init__.py diff --git a/trove/tests/unittests/common/test_common_extensions.py b/trove/tests/unittests/extensions/common/test_service.py similarity index 78% rename from trove/tests/unittests/common/test_common_extensions.py rename to trove/tests/unittests/extensions/common/test_service.py index 813c936e47..c934133744 100644 --- a/trove/tests/unittests/common/test_common_extensions.py +++ b/trove/tests/unittests/extensions/common/test_service.py @@ -86,6 +86,49 @@ class TestDefaultRootController(trove_testtools.TestCase): self.controller.root_create, req, body, tenant_id, uuid, is_cluster) + @patch.object(models.Root, "delete") + @patch.object(models.Root, "load") + def test_root_delete(self, root_load, root_delete): + context = Mock() + req = Mock() + req.environ = Mock() + req.environ.__getitem__ = Mock(return_value=context) + tenant_id = Mock() + instance_id = utils.generate_uuid() + is_cluster = False + root_load.return_value = True + self.controller.root_delete(req, tenant_id, instance_id, is_cluster) + root_load.assert_called_with(context, instance_id) + root_delete.assert_called_with(context, instance_id) + + @patch.object(models.Root, "delete") + @patch.object(models.Root, "load") + def test_root_delete_without_root_enabled(self, root_load, root_delete): + context = Mock() + req = Mock() + req.environ = Mock() + req.environ.__getitem__ = Mock(return_value=context) + tenant_id = Mock() + instance_id = utils.generate_uuid() + is_cluster = False + root_load.return_value = False + self.assertRaises( + exception.RootHistoryNotFound, + self.controller.root_delete, + req, tenant_id, instance_id, is_cluster) + root_load.assert_called_with(context, instance_id) + root_delete.assert_not_called() + + def test_root_delete_with_cluster(self): + req = Mock() + tenant_id = Mock() + instance_id = utils.generate_uuid() + is_cluster = True + self.assertRaises( + exception.ClusterOperationNotSupported, + self.controller.root_delete, + req, tenant_id, instance_id, is_cluster) + class TestRootController(trove_testtools.TestCase): @@ -166,6 +209,53 @@ class TestRootController(trove_testtools.TestCase): service_get_datastore.assert_called_with(tenant_id, uuid) service_load_root_controller.assert_called_with(ds_manager) + @patch.object(instance_models.Instance, "load") + @patch.object(RootController, "load_root_controller") + @patch.object(RootController, "_get_datastore") + def test_delete(self, service_get_datastore, service_load_root_controller, + service_load_instance): + req = Mock() + req.environ = {'trove.context': self.context} + tenant_id = Mock() + uuid = utils.generate_uuid() + ds_manager = Mock() + is_cluster = False + service_get_datastore.return_value = (ds_manager, is_cluster) + root_controller = Mock() + ret = Mock() + root_controller.root_delete = Mock(return_value=ret) + service_load_root_controller.return_value = root_controller + + self.assertEqual( + ret, self.controller.delete(req, tenant_id, uuid)) + service_get_datastore.assert_called_with(tenant_id, uuid) + service_load_root_controller.assert_called_with(ds_manager) + root_controller.root_delete.assert_called_with( + req, tenant_id, uuid, is_cluster) + + @patch.object(instance_models.Instance, "load") + @patch.object(RootController, "load_root_controller") + @patch.object(RootController, "_get_datastore") + def test_delete_with_no_root_controller(self, + service_get_datastore, + service_load_root_controller, + service_load_instance): + req = Mock() + req.environ = {'trove.context': self.context} + tenant_id = Mock() + uuid = utils.generate_uuid() + ds_manager = Mock() + is_cluster = False + service_get_datastore.return_value = (ds_manager, is_cluster) + service_load_root_controller.return_value = None + + self.assertRaises( + NoSuchOptError, + self.controller.delete, + req, tenant_id, uuid) + service_get_datastore.assert_called_with(tenant_id, uuid) + service_load_root_controller.assert_called_with(ds_manager) + class TestClusterRootController(trove_testtools.TestCase): diff --git a/trove/tests/unittests/redis/__init__.py b/trove/tests/unittests/extensions/redis/__init__.py similarity index 100% rename from trove/tests/unittests/redis/__init__.py rename to trove/tests/unittests/extensions/redis/__init__.py diff --git a/trove/tests/unittests/redis/test_redis_root_controller.py b/trove/tests/unittests/extensions/redis/test_service.py similarity index 83% rename from trove/tests/unittests/redis/test_redis_root_controller.py rename to trove/tests/unittests/extensions/redis/test_service.py index e528567d82..ad98b30f8c 100644 --- a/trove/tests/unittests/redis/test_redis_root_controller.py +++ b/trove/tests/unittests/extensions/redis/test_service.py @@ -20,6 +20,7 @@ from mock import Mock, patch from trove.common import exception from trove.datastore import models as datastore_models from trove.extensions.common import models +from trove.extensions.redis.models import RedisRoot from trove.extensions.redis.service import RedisRootController from trove.instance import models as instance_models from trove.instance.models import DBInstance @@ -155,8 +156,11 @@ class TestRedisRootController(trove_testtools.TestCase): req, body, tenant_id, instance_id, is_cluster) @patch.object(instance_models.Instance, "load") + @patch.object(RedisRoot, "get_auth_password") @patch.object(models.Root, "delete") - def test_root_delete_on_single_instance(self, root_delete, *args): + @patch.object(models.Root, "load") + def test_root_delete_on_single_instance(self, root_load, + root_delete, *args): context = Mock() req = Mock() req.environ = Mock() @@ -164,12 +168,17 @@ class TestRedisRootController(trove_testtools.TestCase): tenant_id = self.tenant_id instance_id = self.single_db_info.id is_cluster = False + root_load.return_value = True self.controller.root_delete(req, tenant_id, instance_id, is_cluster) + root_load.assert_called_with(context, instance_id) root_delete.assert_called_with(context, instance_id) @patch.object(instance_models.Instance, "load") + @patch.object(RedisRoot, "get_auth_password") @patch.object(models.Root, "delete") - def test_root_delete_on_master_instance(self, root_delete, *args): + @patch.object(models.Root, "load") + def test_root_delete_on_master_instance(self, root_load, + root_delete, *args): context = Mock() req = Mock() req.environ = Mock() @@ -178,7 +187,9 @@ class TestRedisRootController(trove_testtools.TestCase): instance_id = self.master_db_info.id slave_instance_id = self.slave_db_info.id is_cluster = False + root_load.return_value = True self.controller.root_delete(req, tenant_id, instance_id, is_cluster) + root_load.assert_called_with(context, instance_id) root_delete.assert_called_with(context, slave_instance_id) def test_root_delete_on_slave(self): @@ -203,3 +214,23 @@ class TestRedisRootController(trove_testtools.TestCase): exception.ClusterOperationNotSupported, self.controller.root_delete, req, tenant_id, instance_id, is_cluster) + + @patch.object(instance_models.Instance, "load") + @patch.object(models.Root, "delete") + @patch.object(models.Root, "load") + def test_root_delete_without_root_enabled(self, root_load, + root_delete, *args): + context = Mock() + req = Mock() + req.environ = Mock() + req.environ.__getitem__ = Mock(return_value=context) + tenant_id = self.tenant_id + instance_id = self.single_db_info.id + is_cluster = False + root_load.return_value = False + self.assertRaises( + exception.RootHistoryNotFound, + self.controller.root_delete, + req, tenant_id, instance_id, is_cluster) + root_load.assert_called_with(context, instance_id) + root_delete.assert_not_called()