Re: [Kimchi-devel] [PATCH 4/4] Add/remove users and groups to VMs

To update (add/remove) the users and groups of a virtual machine, you should use the REST command: /vms/<vm_name> PUT "{ 'users': [ 'user1', 'user2' ], 'groups': [ 'group1', 'group2' ] }" Users and groups associated with a virtual machine must be valid when they are added to the VM. Such verification is not done when they are removed from the system. This means that virtual machines may contain outdated information about users and groups. Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com&gt; --- docs/API.md | 2 ++ src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 2 ++ src/kimchi/model/vms.py | 29 ++++++++++++++++++++++++++--- tests/test_model.py | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-) diff --git a/docs/API.md b/docs/API.md index e79a38b..2a125bb 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,8 @@ the following general conventions: * **DELETE**: Remove the Virtual Machine * **PUT**: update the parameters of existed VM * name: New name for this VM (only applied for shutoff VM) + * users: New list of system users. + * groups: New list of system groups. * **POST**: *See Virtual Machine Actions* **Actions (POST):** diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 89e9f45..355ca12 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -37,7 +37,7 @@ class VMs(Collection): class VM(Resource): def __init__(self, model, ident): super(VM, self).__init__(model, ident) - self.update_params = ["name"] + self.update_params = ["name", "users", "groups"] self.screenshot = VMScreenShot(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 179b38b..2af3edf 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -81,6 +81,8 @@ messages = { "KCHVM0021E": _("User name must be a string"), "KCHVM0022E": _("Group names list must be an array"), "KCHVM0023E": _("Group name must be a string"), + "KCHVM0024E": _("User %(user)s does not exist"), + "KCHVM0025E": _("Group %(group)s does 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/model/vms.py b/src/kimchi/model/vms.py index dcb352c..e34a547 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -32,6 +32,7 @@ from lxml import etree from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import Group, User from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -262,10 +263,24 @@ class VMModel(object): old_xml = new_xml = dom.XMLDesc(0) + users = _get_vm_metadata(old_xml, "users") + groups = _get_vm_metadata(old_xml, "groups")

+ for key, val in params.items(): - if key in VM_STATIC_UPDATE_PARAMS: - xpath = VM_STATIC_UPDATE_PARAMS[key] - new_xml = xmlutils.xml_item_update(new_xml, xpath, val) + if key == 'users': + for user in val: + if not User(user).exists(): + raise OperationFailed("KCHVM0024E", {'user': user}) + users = val + elif key == 'groups': + for group in val: + if not Group(group).exists(): + raise OperationFailed("KCHVM0025E", {'group': group}) + groups = val + else: + if key in VM_STATIC_UPDATE_PARAMS: + xpath = VM_STATIC_UPDATE_PARAMS[key] + new_xml = xmlutils.xml_item_update(new_xml, xpath, val)

try: if 'name' in params: @@ -276,6 +291,14 @@ class VMModel(object): dom.undefine() conn = self.conn.get() dom = conn.defineXML(new_xml) + metadata_xml = """ + <access> + <users>%s</users> + <groups>%s</groups> + </access> + """ % (str(users), str(groups)) + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, metadata_xml, + KIMCHI_NS_TAG, KIMCHI_NS_URI) except libvirt.libvirtError as e: dom = conn.defineXML(old_xml) raise OperationFailed("KCHVM0008E", {'name': dom.name(), diff --git a/tests/test_model.py b/tests/test_model.py index 474da57..68ea0ea 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -21,9 +21,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 platform import psutil +import pwd import shutil import tempfile import threading @@ -573,6 +575,44 @@ class ModelTests(unittest.TestCase): self.assertEquals(info['uuid'], inst.vm_lookup(u'пeω-∨м')['uuid']) rollback.prependDefer(inst.vm_delete, u'пeω-∨м') + # change only VM users - groups are not changed (default is empty) + users = ['root'] + 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 = ['wheel'] + 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']) + + # change VM users and groups by adding a new element to each one + users.append(pwd.getpwuid(os.getuid()).pw_name) + groups.append(grp.getgrgid(os.getgid()).gr_name) + inst.vm_update(u'пeω-∨м', {'users': users, 'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # 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ω-∨м', + {'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ω-∨м', + {'users': [], 'groups': ['groupdoesnotexist']}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by removing all elements + inst.vm_update(u'пeω-∨м', {'users': [], 'groups': []}) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): inst = model.Model('qemu:///system', self.tmp_store)