openstack/trove
Code Issues Proposed changes

Use RootHistory to check if root is ever enabled

Browse Source 
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 <zhaochao1984@gmail.com>
This commit is contained in:
Zhao Chao 2018-02-24 21:59:58 +08:00
parent b59b6ae968
commit 579f5afc64
15 changed files with 161 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
tools/trove-pylint.config
View File

@ -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",

8
trove/common/cfg.py
View File

@ -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.'),

5
trove/common/exception.py
View File

@ -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.")

1
trove/common/wsgi.py
View File

@ -347,6 +347,7 @@ class Controller(object):
exception.DatastoreNotFound,
exception.SwiftNotFound,
exception.ModuleTypeNotFound,
exception.RootHistoryNotFound,
],
webob.exc.HTTPConflict: [
exception.BackupNotCompleteError,

28
trove/extensions/cassandra/service.py
View File

@ -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)

12
trove/extensions/common/service.py
View File

@ -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):
"""

10
trove/extensions/mysql/service.py
View File

@ -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)

29
trove/extensions/postgresql/service.py
View File

@ -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)

4
trove/extensions/redis/service.py
View File

@ -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)

20
trove/tests/unittests/common/test_wsgi.py
View File

@ -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)

0
trove/extensions/cassandra/__init__.py → trove/tests/unittests/extensions/__init__.py
View File

0
trove/extensions/postgresql/__init__.py → trove/tests/unittests/extensions/common/__init__.py
View File

90
trove/tests/unittests/common/test_common_extensions.py → trove/tests/unittests/extensions/common/test_service.py
View File

@ -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):

0
trove/tests/unittests/redis/__init__.py → trove/tests/unittests/extensions/redis/__init__.py
View File

35
trove/tests/unittests/redis/test_redis_root_controller.py → trove/tests/unittests/extensions/redis/test_service.py
View File

@ -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()