From: Aline Manera <alinefm(a)br.ibm.com&gt; SimpleCollection is a collection of information that does not have a Resource definition. It can be useful in other situations so move it to a common place. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/control/base.py | 17 +++++++++++++++++ src/kimchi/control/plugins.py | 19 ++----------------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 320239b..f8a5210 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -302,3 +302,20 @@ class AsyncCollection(Collection): task = create(*args) cherrypy.response.status = 202 return kimchi.template.render("Task", task) + + +class SimpleCollection(Collection): + """ + A Collection without Resource definition + """ + def __init__(self, model): + super(SimpleCollection, self).__init__(model) + + def get(self, filter_params): + res_list = [] + try: + get_list = getattr(self.model, model_fn(self, 'get_list')) + res_list = get_list(*self.model_args) + except AttributeError: + pass + return kimchi.template.render(get_class_name(self), res_list) diff --git a/src/kimchi/control/plugins.py b/src/kimchi/control/plugins.py index 12c621e..6399e7c 100644 --- a/src/kimchi/control/plugins.py +++ b/src/kimchi/control/plugins.py @@ -17,26 +17,11 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -import kimchi.template -from kimchi.control.base import Collection -from kimchi.control.utils import get_class_name, model_fn +from kimchi.control.base import SimpleCollection from kimchi.control.utils import UrlSubNode @UrlSubNode("plugins") -class Plugins(Collection): +class Plugins(SimpleCollection): def __init__(self, model): super(Plugins, self).__init__(model) - - @property - def data(self): - return self.info - - def get(self, filter_params): - res_list = [] - try: - get_list = getattr(self.model, model_fn(self, 'get_list')) - res_list = get_list(*self.model_args) - except AttributeError: - pass - return kimchi.template.render(get_class_name(self), res_list)

From: Aline Manera <alinefm(a)br.ibm.com&gt; Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/control/host.py | 14 +++++++++++++- src/kimchi/mockmodel.py | 6 ++++++ src/kimchi/model/host.py | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index a0818c7..003c4b9 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -19,7 +19,7 @@ import cherrypy -from kimchi.control.base import Collection, Resource +from kimchi.control.base import Collection, Resource, SimpleCollection from kimchi.control.utils import UrlSubNode, validate_method from kimchi.exception import OperationFailed from kimchi.template import render @@ -37,6 +37,8 @@ class Host(Resource): self.devices = Devices(self.model) self.packagesupdate = PackagesUpdate(self.model) self.repositories = Repositories(self.model) + self.users = Users(self.model) + self.groups = Groups(self.model) @cherrypy.expose def swupdate(self): @@ -128,3 +130,13 @@ class Repository(Resource): @property def data(self): return self.info + + +class Users(SimpleCollection): + def __init__(self, model): + super(Users, self).__init__(model) + + +class Groups(SimpleCollection): + def __init__(self, model): + super(Groups, self).__init__(model) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 8c7d7bb..b235e1e 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -798,6 +798,12 @@ class MockModel(object): 'net_recv_rate': round(random.uniform(0, 4000), 1), 'net_sent_rate': round(random.uniform(0, 4000), 1)} + def users_get_list(self): + return ["userA", "userB", "userC"] + + def groups_get_list(self): + return ["groupA", "groupB", "groupC", "groupD"] + def vms_get_list_by_state(self, state): ret_list = [] for name in self.vms_get_list(): diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index 7b151f2..e0ac760 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -17,9 +17,11 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import grp import os import time import platform +import pwd from collections import defaultdict import psutil @@ -364,3 +366,19 @@ class RepositoryModel(object): raise InvalidOperation('KCHREPOS0014E') return self._repositories.removeRepository(repo_id) + + +class UsersModel(object): + def __init__(self, **kargs): + pass + + def get_list(self): + return [user.pw_name for user in pwd.getpwall()] + + +class GroupsModel(object): + def __init__(self, **kargs): + pass + + def get_list(self): + return [group.gr_name for group in grp.getgrall()]

From: Aline Manera <alinefm(a)br.ibm.com&gt; Use the APIs /host/users and /host/groups to check user input That way we can check the input data at once instead of checking item by item. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/i18n.py | 4 ++-- src/kimchi/mockmodel.py | 24 ++++++++++++++++++------ src/kimchi/model/vms.py | 20 ++++++++++++-------- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 3fc3013..89bcd02 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -84,8 +84,8 @@ messages = { "KCHVM0024E": _("User name must be a string"), "KCHVM0025E": _("Group names list must be an array"), "KCHVM0026E": _("Group name must be a string"), - "KCHVM0027E": _("User %(user)s does not exist"), - "KCHVM0028E": _("Group %(group)s does not exist"), + "KCHVM0027E": _("User(s) '%(users)s' do not exist"), + "KCHVM0028E": _("Group(s) '%(groups)s' do not exist"), "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index b235e1e..866ad2c 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -91,14 +91,26 @@ class MockModel(object): if state == 'running' or params['name'] in self.vms_get_list(): msg_args = {'name': dom.name, 'new_name': params['name']} raise InvalidParameter("KCHVM0003E", msg_args) - else: - del self._mock_vms[dom.name] - dom.name = params['name'] - self._mock_vms[dom.name] = dom + + del self._mock_vms[dom.name] + dom.name = params['name'] + self._mock_vms[dom.name] = dom for key, val in params.items(): - if key in dom.info: - dom.info[key] = val + if key == 'users': + invalid_users = set(val) - set(self.users_get_list()) + if len(invalid_users) != 0: + raise InvalidParameter("KCHVM0027E", + {'users': ", ".join(invalid_users)}) + + elif key == 'groups': + invalid_groups = set(val) - set(self.groups_get_list()) + if len(invalid_groups) != 0: + raise InvalidParameter("KCHVM0028E", + {'groups': + ", ".join(invalid_groups)}) + + dom.info[key] = val def _live_vm_update(self, dom, params): pass diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 90e9537..ede54a3 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -29,7 +29,6 @@ from lxml.builder import E from kimchi import vnc from kimchi import xmlutils -from kimchi.auth import Group, User from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed @@ -37,7 +36,7 @@ from kimchi.model.config import CapabilitiesModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot -from kimchi.utils import kimchi_log, run_setfacl_set_attr +from kimchi.utils import import_class, kimchi_log, run_setfacl_set_attr from kimchi.utils import template_name_from_uri from kimchi.xmlutils import xpath_get_text @@ -236,6 +235,8 @@ class VMModel(object): self.conn = kargs['conn'] self.objstore = kargs['objstore'] self.vmscreenshot = VMScreenshotModel(**kargs) + self.users = import_class('kimchi.model.host.UsersModel')(**kargs) + self.groups = import_class('kimchi.model.host.GroupsModel')(**kargs) def update(self, name, params): dom = self.get_vm(name, self.conn) @@ -264,14 +265,17 @@ class VMModel(object): for key, val in params.items(): if key == 'users': - for user in val: - if not User(user).exists(): - raise OperationFailed("KCHVM0027E", {'user': user}) + invalid_users = set(val) - set(self.users.get_list()) + if len(invalid_users) != 0: + raise InvalidParameter("KCHVM0027E", + {'users': ", ".join(invalid_users)}) users = val elif key == 'groups': - for group in val: - if not Group(group).exists(): - raise OperationFailed("KCHVM0028E", {'group': group}) + invalid_groups = set(val) - set(self.groups.get_list()) + if len(invalid_groups) != 0: + raise InvalidParameter("KCHVM0028E", + {'groups': + ", ".join(invalid_groups)}) groups = val else: if key in VM_STATIC_UPDATE_PARAMS:

From: Aline Manera <alinefm(a)br.ibm.com&gt; With the new APIs /host/users and /host/groups we don't need the User.exists() and Group() class so remove them. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/auth.py | 22 ---------------------- tests/test_authorization.py | 19 ------------------- 2 files changed, 41 deletions(-) diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index 2186987..dc78ded 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -25,7 +25,6 @@ import multiprocessing import os import PAM import pty -import pwd import re import termios import time @@ -97,27 +96,6 @@ class User(object): def get_user(self): return self.user - def exists(self): - try: - pwd.getpwnam(self.user[USER_NAME]) - except KeyError: - return False - else: - return True - - -class Group(object): - def __init__(self, groupname): - self.groupname = groupname - - def exists(self): - try: - grp.getgrnam(self.groupname) - except KeyError: - return False - else: - return True - def authenticate(username, password, service="passwd"): '''Returns True if authenticate is OK via PAM.''' diff --git a/tests/test_authorization.py b/tests/test_authorization.py index ab98987..b211e06 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -17,17 +17,14 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -import grp import json import os -import pwd import unittest from functools import partial -import kimchi.auth import kimchi.mockmodel from utils import get_free_port, patch_auth, request from utils import run_server @@ -122,19 +119,3 @@ class AuthorizationTests(unittest.TestCase): self.assertEquals(403, resp.status) resp = self.request('/vms', '{}', 'DELETE') self.assertEquals(403, resp.status) - - -class CurrentUserGroupTests(unittest.TestCase): - def test_current_user(self): - current_user = pwd.getpwuid(os.getuid()).pw_name - self.assertTrue(kimchi.auth.User(current_user).exists()) - - invalid_user = "userdoesnotexist" - self.assertFalse(kimchi.auth.User(invalid_user).exists()) - - def test_current_group(self): - current_group = grp.getgrgid(os.getgid()).gr_name - self.assertTrue(kimchi.auth.Group(current_group).exists()) - - invalid_group = "groupdoesnotexist" - self.assertFalse(kimchi.auth.Group(invalid_group).exists())

From: Aline Manera <alinefm(a)br.ibm.com&gt; Properly test new APIs /host/users and /host/groups Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- tests/test_model.py | 8 ++++---- tests/test_rest.py | 24 ++++++++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index 357d969..2fb4446 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -611,13 +611,13 @@ class ModelTests(unittest.TestCase): u'пeω-∨м') # change only VM users - groups are not changed (default is empty) - users = ['root'] + users = inst.users_get_list()[:3] inst.vm_update(u'пeω-∨м', {'users': users}) self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) # change only VM groups - users are not changed (default is empty) - groups = ['root'] + groups = inst.groups_get_list()[:2] inst.vm_update(u'пeω-∨м', {'groups': groups}) self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) @@ -631,14 +631,14 @@ class ModelTests(unittest.TestCase): # change VM users (wrong value) and groups # when an error occurs, everything fails and nothing is changed - self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + self.assertRaises(InvalidParameter, inst.vm_update, u'пeω-∨м', {'users': ['userdoesnotexist'], 'groups': []}) self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) # change VM users and groups (wrong value) # when an error occurs, everything fails and nothing is changed - self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + self.assertRaises(InvalidParameter, inst.vm_update, u'пeω-∨м', {'users': [], 'groups': ['groupdoesnotexist']}) self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) diff --git a/tests/test_rest.py b/tests/test_rest.py index a40ba93..06396db 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -225,18 +225,34 @@ class RestTests(unittest.TestCase): self.assertEquals(u'∨м-црdαtеd', vm['name']) # change only VM users - groups are not changed (default is empty) - req = json.dumps({'users': ['root']}) + resp = self.request('/host/users', '{}', 'GET') + users = json.loads(resp.read()) + req = json.dumps({'users': users}) resp = self.request('/vms/∨м-црdαtеd', req, 'PUT') self.assertEquals(200, resp.status) info = json.loads(self.request('/vms/∨м-црdαtеd', '{}').read()) - self.assertEquals(['root'], info['users']) + self.assertEquals(users, info['users']) # change only VM groups - users are not changed (default is empty) - req = json.dumps({'groups': ['kimchi']}) + resp = self.request('/host/groups', '{}', 'GET') + groups = json.loads(resp.read()) + req = json.dumps({'groups': groups}) resp = self.request('/vms/∨м-црdαtеd', req, 'PUT') self.assertEquals(200, resp.status) info = json.loads(self.request('/vms/∨м-црdαtеd', '{}').read()) - self.assertEquals(['kimchi'], info['groups']) + self.assertEquals(groups, info['groups']) + + # change VM users (wrong value) and groups + # when an error occurs, everything fails and nothing is changed + req = json.dumps({'users': ['userdoesnotexist'], 'groups': []}) + resp = self.request('/vms/∨м-црdαtеd', req, 'PUT') + self.assertEquals(400, resp.status) + + # change VM users and groups (wrong value) + # when an error occurs, everything fails and nothing is changed + req = json.dumps({'users': [], 'groups': ['groupdoesnotexist']}) + resp = self.request('/vms/∨м-црdαtеd', req, 'PUT') + self.assertEquals(400, resp.status) def test_vm_lifecycle(self): # Create a Template