11:09 a.m.
Crístian Viana (4):
Override only the updated "User" methods in "patch_auth"
Add functions to check if a user/group exists
Return users and groups when fetching VM info
Add/remove users and groups to VMs
docs/API.md | 6 +++++
src/kimchi/API.json | 22 ++++++++++++++++
src/kimchi/auth.py | 22 ++++++++++++++++
src/kimchi/control/vms.py | 6 +++--
src/kimchi/i18n.py | 6 +++++
src/kimchi/mockmodel.py | 4 ++-
src/kimchi/model/vms.py | 61 ++++++++++++++++++++++++++++++++++++++++++---
tests/test_authorization.py | 19 ++++++++++++++
tests/test_mockmodel.py | 2 +-
tests/test_model.py | 44 +++++++++++++++++++++++++++++++-
tests/test_rest.py | 2 ++
tests/utils.py | 24 +++++-------------
12 files changed, 191 insertions(+), 27 deletions(-)
--
1.8.5.3
11:09 a.m.
New subject: [PATCH 1/4] Override only the updated "User" methods in "patch_auth"
The function "patch_auth" creates a fake class to override the existing
kimchi.auth.User with a few updated methods. That attribution overrides
the entire User class along with all its methods.
In order to avoid side effects, override only the methods we need to
change in the "User" class. Other methods not related to "patch_auth"
will not be affected now.
---
tests/utils.py | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/tests/utils.py b/tests/utils.py
index 18b707c..faf6730 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -152,25 +152,12 @@ def patch_auth(sudo=True):
Override the authenticate function with a simple test against an
internal dict of users and passwords.
"""
- USER_ID = 'userid'
- USER_GROUPS = 'groups'
- USER_SUDO = 'sudo'
- class _User(object):
- def __init__(self, userid):
- self.user = {}
- self.user[USER_ID] = userid
- self.user[USER_GROUPS] = None
- self.user[USER_SUDO] = sudo
+ def _get_groups(self):
+ return None
- def get_groups(self):
- return self.user[USER_GROUPS]
-
- def has_sudo(self):
- return self.user[USER_SUDO]
-
- def get_user(self):
- return self.user
+ def _has_sudo(self):
+ return sudo
def _authenticate(username, password, service="passwd"):
try:
@@ -181,7 +168,8 @@ def patch_auth(sudo=True):
import kimchi.auth
kimchi.auth.authenticate = _authenticate
- kimchi.auth.User = _User
+ kimchi.auth.User.get_groups = _get_groups
+ kimchi.auth.User.has_sudo = _has_sudo
def normalize_xml(xml_str):
--
1.8.5.3
11:09 a.m.
New subject: [PATCH 2/4] Add functions to check if a user/group exists
The user/group validation is done on the current system.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/auth.py | 22 ++++++++++++++++++++++
tests/test_authorization.py | 19 +++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
index 6f34772..d7f5845 100644
--- a/src/kimchi/auth.py
+++ b/src/kimchi/auth.py
@@ -26,6 +26,7 @@ import base64
import cherrypy
import grp
import PAM
+import pwd
import re
@@ -83,6 +84,27 @@ class User(object):
def get_user(self):
return self.user
+ def exists(self):
+ try:
+ pwd.getpwnam(self.user[USER_ID])
+ except KeyError:
+ return False
+ else:
+ return True
+
+
+class Group(object):
+ def __init__(self, groupid):
+ self.groupid = groupid
+
+ def exists(self):
+ try:
+ grp.getgrnam(self.groupid)
+ 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 24ce4bd..a93dad2 100644
--- a/tests/test_authorization.py
+++ b/tests/test_authorization.py
@@ -20,14 +20,17 @@
# 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,3 +125,19 @@ 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())
--
1.8.5.3
11:09 a.m.
New subject: [PATCH 3/4] Return users and groups when fetching VM info
In order to have more control over what users can use a virtual machine
under Kimchi, a VM should have users and groups associated with it.
Kimchi uses metadata information
(http://libvirt.org/formatdomain.html#elementsMetadata) to store users
and groups to the virtual machines. The data is stored as string
representations of Python lists.
Keep in mind that these security restrictions will not apply when using
other virtual machine managers (e.g. virsh, virt-manager, etc).
Users and groups stored in the VM are now returned when the user queries the
VM data. Currently, there is no way to associate users and groups to a VM.
The REST command below now returns the users and groups of every
virtual machine:
/vms/:name GET: It now lists the users and groups associated with that VM.
{
"users":[
"user1",
"user2"
],
"groups":[
"group1",
"group2"
],
...
}
If there is no metadata information about users and groups, empty lists
will be displayed
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
docs/API.md | 4 ++++
src/kimchi/API.json | 22 ++++++++++++++++++++++
src/kimchi/control/vms.py | 4 +++-
src/kimchi/i18n.py | 4 ++++
src/kimchi/mockmodel.py | 4 +++-
src/kimchi/model/vms.py | 32 +++++++++++++++++++++++++++++++-
tests/test_mockmodel.py | 2 +-
tests/test_model.py | 4 +++-
tests/test_rest.py | 2 ++
9 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index e97eace..e79a38b 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -96,6 +96,10 @@ the following general conventions:
* port: The real port number of the graphics, vnc or spice. Users
can use this port to connect to the vm with general vnc/spice
clients.
+ * users: A list of system users who have permission to access the VM.
+ Default is: empty.
+ * groups: A list of system groups whose users have permission to access
+ the VM. Default is: empty.
* **DELETE**: Remove the Virtual Machine
* **PUT**: update the parameters of existed VM
* name: New name for this VM (only applied for shutoff VM)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json
index f595bbf..9d19351 100644
--- a/src/kimchi/API.json
+++ b/src/kimchi/API.json
@@ -183,6 +183,28 @@
"type": "string",
"minLength": 1,
"error": "KCHVM0011E"
+ },
+ "users": {
+ "description": "Array of users who have permission to
the VM",
+ "type": "array",
+ "uniqueItems": true,
+ "error": "KCHVM0020E",
+ "items": {
+ "description": "User name",
+ "type": "string",
+ "error": "KCHVM0021E"
+ }
+ },
+ "groups": {
+ "description": "Array of groups who have permission to
the VM",
+ "type": "array",
+ "uniqueItems": true,
+ "error": "KCHVM0022E",
+ "items": {
+ "description": "Group name",
+ "type": "string",
+ "error": "KCHVM0023E"
+ }
}
}
},
diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
index a74ce27..89e9f45 100644
--- a/src/kimchi/control/vms.py
+++ b/src/kimchi/control/vms.py
@@ -58,7 +58,9 @@ class VM(Resource):
'icon': self.info['icon'],
'graphics': {'type':
self.info['graphics']['type'],
'listen':
self.info['graphics']['listen'],
- 'port':
self.info['graphics']['port']}
+ 'port':
self.info['graphics']['port']},
+ 'users': self.info['users'],
+ 'groups': self.info['groups']
}
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index ca72a80..179b38b 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -77,6 +77,10 @@ messages = {
"KCHVM0017E": _("Volume list (LUNs names) not given."),
"KCHVM0018E": _("Virtual machine volumes must be a list of strings
with distinct LUNs names."),
"KCHVM0019E": _("Unable to start virtual machine %(name)s. Details:
%(err)s"),
+ "KCHVM0020E": _("User names list must be an array"),
+ "KCHVM0021E": _("User name must be a string"),
+ "KCHVM0022E": _("Group names list must be an array"),
+ "KCHVM0023E": _("Group name must be a string"),
"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 b23a024..5cdbc01 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -937,7 +937,9 @@ class MockVM(object):
'cpus': template_info['cpus'],
'icon': None,
'graphics': {'type': 'vnc',
'listen': '0.0.0.0',
- 'port': None}
+ 'port': None},
+ 'users': ['user1', 'user2',
'root'],
+ 'groups': ['group1', 'group2',
'admin']
}
self.info['graphics'].update(template_info['graphics'])
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index b6a42e6..dcb352c 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -20,6 +20,7 @@
# 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 ast
import os
import time
import uuid
@@ -27,6 +28,7 @@ from xml.etree import ElementTree
import libvirt
from cherrypy.process.plugins import BackgroundTask
+from lxml import etree
from kimchi import vnc
from kimchi import xmlutils
@@ -53,6 +55,17 @@ VM_LIVE_UPDATE_PARAMS = {}
stats = {}
+KIMCHI_NS_TAG = 'kimchi'
+KIMCHI_NS_URI = 'http://github.com/kimchi-project/kimchi'
+
+
+def _get_vm_metadata(xml, tag):
+ et = etree.fromstring(xml)
+ val = et.xpath("/domain/metadata/%s:access/%s:%s"
+ % (KIMCHI_NS_TAG, KIMCHI_NS_TAG, tag),
+ namespaces={KIMCHI_NS_TAG: KIMCHI_NS_URI})
+ return ast.literal_eval(val[0].text) if val else []
+
class VMsModel(object):
def __init__(self, **kargs):
@@ -305,6 +318,21 @@ class VMModel(object):
res['io_throughput'] = vm_stats.get('disk_io', 0)
res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100)
+ xml = dom.XMLDesc(0)
+ try:
+ users = _get_vm_metadata(xml, "users")
+ except:
+ # Wrong XML tree structure, wrong array syntax, or empty value:
+ # ignore error and build an empty list
+ users = []
+
+ try:
+ groups = _get_vm_metadata(xml, "groups")
+ except:
+ # Wrong XML tree structure, wrong array syntax, or empty value:
+ # ignore error and build an empty list
+ groups = []
+
return {'state': state,
'stats': res,
'uuid': dom.UUIDString(),
@@ -314,7 +342,9 @@ class VMModel(object):
'icon': icon,
'graphics': {"type": graphics_type,
"listen": graphics_listen,
- "port": graphics_port}
+ "port": graphics_port},
+ 'users': users,
+ 'groups': groups
}
def _vm_get_disk_paths(self, dom):
diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py
index ab5eb59..863c740 100644
--- a/tests/test_mockmodel.py
+++ b/tests/test_mockmodel.py
@@ -138,7 +138,7 @@ class MockModelTests(unittest.TestCase):
self.assertEquals(u'test', vms[0])
keys = set(('state', 'stats', 'uuid', 'memory',
'cpus', 'screenshot',
- 'icon', 'graphics'))
+ 'icon', 'graphics', 'users',
'groups'))
stats_keys = set(('cpu_utilization',
'net_throughput', 'net_throughput_peak',
'io_throughput', 'io_throughput_peak'))
diff --git a/tests/test_model.py b/tests/test_model.py
index 74e2424..474da57 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -58,7 +58,7 @@ class ModelTests(unittest.TestCase):
self.assertEquals('test', vms[0])
keys = set(('state', 'stats', 'uuid', 'memory',
'cpus', 'screenshot',
- 'icon', 'graphics'))
+ 'icon', 'graphics', 'users',
'groups'))
stats_keys = set(('cpu_utilization',
'net_throughput', 'net_throughput_peak',
'io_throughput', 'io_throughput_peak'))
@@ -70,6 +70,8 @@ class ModelTests(unittest.TestCase):
self.assertEquals(None, info['icon'])
self.assertEquals(stats_keys, set(info['stats'].keys()))
self.assertRaises(NotFoundError, inst.vm_lookup, 'nosuchvm')
+ self.assertEquals([], info['users'])
+ self.assertEquals([], info['groups'])
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_vm_lifecycle(self):
diff --git a/tests/test_rest.py b/tests/test_rest.py
index ca96dc0..f8a87e6 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -182,6 +182,8 @@ class RestTests(unittest.TestCase):
vm = json.loads(self.request('/vms/vm-1').read())
self.assertEquals('vm-1', vm['name'])
self.assertEquals('shutoff', vm['state'])
+ self.assertEquals(['user1', 'user2', 'root'],
vm['users'])
+ self.assertEquals(['group1', 'group2', 'admin'],
vm['groups'])
def test_edit_vm(self):
req = json.dumps({'name': 'test', 'cdrom':
'/nonexistent.iso'})
--
1.8.5.3
11:09 a.m.
New subject: [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>
---
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)
--
1.8.5.3
7:20 a.m.
New subject: [PATCH 2/4] Add functions to check if a user/group exists
On 02/26/2014 03:09 PM, Crístian Viana wrote:
The user/group validation is done on the current system.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/auth.py | 22 ++++++++++++++++++++++
tests/test_authorization.py | 19 +++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
index 6f34772..d7f5845 100644
--- a/src/kimchi/auth.py
+++ b/src/kimchi/auth.py
@@ -26,6 +26,7 @@ import base64
import cherrypy
import grp
import PAM
+import pwd
import re
@@ -83,6 +84,27 @@ class User(object):
def get_user(self):
return self.user
Maybe the naming is wrong below. You are passing the User ID and
getpwnam expects the User name
+ def exists(self):
+ try:
+ pwd.getpwnam(self.user[USER_ID])
+ except KeyError:
+ return False
+ else:
+ return True
+
+
+class Group(object):
+ def __init__(self, groupid):
+ self.groupid = groupid
+
+ def exists(self):
+ try:
Same here
+ grp.getgrnam(self.groupid)
+ 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 24ce4bd..a93dad2 100644
--- a/tests/test_authorization.py
+++ b/tests/test_authorization.py
@@ -20,14 +20,17 @@
# 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,3 +125,19 @@ class AuthorizationTests(unittest.TestCase):
self.assertEquals(403, resp.status)
resp = self.request('/vms', '{}', 'DELETE')
self.assertEquals(403, resp.status)
+
+
I did not test the patch manually, not sure if it will work. Same
problem that before, because you are
passing the User name to User Class , which expects the User ID ... If
this work, then the name of
parameters should change
+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())
+
Same here
+ 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())
7:37 a.m.
New subject: [PATCH 3/4] Return users and groups when fetching VM info
I was wondering whether would be better to store the user id in vm xml
instead of the user name (login).
Although it is possible to change both by the system admin, the login
seems to be more likely to change.
More comments below.
On 02/26/2014 03:09 PM, Crístian Viana wrote:
In order to have more control over what users can use a virtual
machine
under Kimchi, a VM should have users and groups associated with it.
Kimchi uses metadata information
(http://libvirt.org/formatdomain.html#elementsMetadata) to store users
and groups to the virtual machines. The data is stored as string
representations of Python lists.
Keep in mind that these security restrictions will not apply when using
other virtual machine managers (e.g. virsh, virt-manager, etc).
Users and groups stored in the VM are now returned when the user queries the
VM data. Currently, there is no way to associate users and groups to a VM.
The REST command below now returns the users and groups of every
virtual machine:
/vms/:name GET: It now lists the users and groups associated with that VM.
{
"users":[
"user1",
"user2"
],
"groups":[
"group1",
"group2"
],
...
}
If there is no metadata information about users and groups, empty lists
will be displayed
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
docs/API.md | 4 ++++
src/kimchi/API.json | 22 ++++++++++++++++++++++
src/kimchi/control/vms.py | 4 +++-
src/kimchi/i18n.py | 4 ++++
src/kimchi/mockmodel.py | 4 +++-
src/kimchi/model/vms.py | 32 +++++++++++++++++++++++++++++++-
tests/test_mockmodel.py | 2 +-
tests/test_model.py | 4 +++-
tests/test_rest.py | 2 ++
9 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index e97eace..e79a38b 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -96,6 +96,10 @@ the following general conventions:
* port: The real port number of the graphics, vnc or spice. Users
can use this port to connect to the vm with general vnc/spice
clients.
+ * users: A list of system users who have permission to access the VM.
+ Default is: empty.
+ * groups: A list of system groups whose users have permission to access
+ the VM. Default is: empty.
* **DELETE**: Remove the Virtual Machine
* **PUT**: update the parameters of existed VM
* name: New name for this VM (only applied for shutoff VM)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json
index f595bbf..9d19351 100644
--- a/src/kimchi/API.json
+++ b/src/kimchi/API.json
@@ -183,6 +183,28 @@
"type": "string",
"minLength": 1,
"error": "KCHVM0011E"
+ },
+ "users": {
+ "description": "Array of users who have permission to
the VM",
+ "type": "array",
+ "uniqueItems": true,
+ "error": "KCHVM0020E",
+ "items": {
+ "description": "User name",
+ "type": "string",
+ "error": "KCHVM0021E"
+ }
+ },
+ "groups": {
+ "description": "Array of groups who have permission
to the VM",
+ "type": "array",
+ "uniqueItems": true,
+ "error": "KCHVM0022E",
+ "items": {
+ "description": "Group name",
+ "type": "string",
+ "error": "KCHVM0023E"
+ }
}
}
},
diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
index a74ce27..89e9f45 100644
--- a/src/kimchi/control/vms.py
+++ b/src/kimchi/control/vms.py
@@ -58,7 +58,9 @@ class VM(Resource):
'icon': self.info['icon'],
'graphics': {'type':
self.info['graphics']['type'],
'listen':
self.info['graphics']['listen'],
- 'port':
self.info['graphics']['port']}
+ 'port':
self.info['graphics']['port']},
+ 'users': self.info['users'],
+ 'groups': self.info['groups']
}
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index ca72a80..179b38b 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -77,6 +77,10 @@ messages = {
"KCHVM0017E": _("Volume list (LUNs names) not given."),
"KCHVM0018E": _("Virtual machine volumes must be a list of strings
with distinct LUNs names."),
"KCHVM0019E": _("Unable to start virtual machine %(name)s. Details:
%(err)s"),
+ "KCHVM0020E": _("User names list must be an array"),
+ "KCHVM0021E": _("User name must be a string"),
+ "KCHVM0022E": _("Group names list must be an array"),
+ "KCHVM0023E": _("Group name must be a string"),
"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 b23a024..5cdbc01 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -937,7 +937,9 @@ class MockVM(object):
'cpus': template_info['cpus'],
'icon': None,
'graphics': {'type': 'vnc',
'listen': '0.0.0.0',
- 'port': None}
+ 'port': None},
+ 'users': ['user1', 'user2',
'root'],
+ 'groups': ['group1', 'group2',
'admin']
}
self.info['graphics'].update(template_info['graphics'])
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index b6a42e6..dcb352c 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -20,6 +20,7 @@
# 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 ast
import os
import time
import uuid
@@ -27,6 +28,7 @@ from xml.etree import ElementTree
import libvirt
from cherrypy.process.plugins import BackgroundTask
+from lxml import etree
from kimchi import vnc
from kimchi import xmlutils
@@ -53,6 +55,17 @@ VM_LIVE_UPDATE_PARAMS = {}
stats = {}
+KIMCHI_NS_TAG = 'kimchi'
+KIMCHI_NS_URI = 'http://github.com/kimchi-project/kimchi'
+
+
+def _get_vm_metadata(xml, tag):
+ et = etree.fromstring(xml)
+ val = et.xpath("/domain/metadata/%s:access/%s:%s"
+ % (KIMCHI_NS_TAG, KIMCHI_NS_TAG, tag),
+ namespaces={KIMCHI_NS_TAG: KIMCHI_NS_URI})
+ return ast.literal_eval(val[0].text) if val else []
+
class VMsModel(object):
def __init__(self, **kargs):
@@ -305,6 +318,21 @@ class VMModel(object):
res['io_throughput'] = vm_stats.get('disk_io', 0)
res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100)
+ xml = dom.XMLDesc(0)
+ try:
+ users = _get_vm_metadata(xml, "users")
+ except:
You could log the error here. I think its good for debugging and
general
information.
+ # Wrong XML tree structure, wrong array syntax, or empty
value:
+ # ignore error and build an empty list
+ users = []
+
+ try:
+ groups = _get_vm_metadata(xml, "groups")
+ except:
Same here
+ # Wrong XML tree structure, wrong array syntax, or empty
value:
+ # ignore error and build an empty list
+ groups = []
+
return {'state': state,
'stats': res,
'uuid': dom.UUIDString(),
@@ -314,7 +342,9 @@ class VMModel(object):
'icon': icon,
'graphics': {"type": graphics_type,
"listen": graphics_listen,
- "port": graphics_port}
+ "port": graphics_port},
+ 'users': users,
+ 'groups': groups
}
def _vm_get_disk_paths(self, dom):
diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py
index ab5eb59..863c740 100644
--- a/tests/test_mockmodel.py
+++ b/tests/test_mockmodel.py
@@ -138,7 +138,7 @@ class MockModelTests(unittest.TestCase):
self.assertEquals(u'test', vms[0])
keys = set(('state', 'stats', 'uuid', 'memory',
'cpus', 'screenshot',
- 'icon', 'graphics'))
+ 'icon', 'graphics', 'users',
'groups'))
stats_keys = set(('cpu_utilization',
'net_throughput', 'net_throughput_peak',
'io_throughput', 'io_throughput_peak'))
diff --git a/tests/test_model.py b/tests/test_model.py
index 74e2424..474da57 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -58,7 +58,7 @@ class ModelTests(unittest.TestCase):
self.assertEquals('test', vms[0])
keys = set(('state', 'stats', 'uuid', 'memory',
'cpus', 'screenshot',
- 'icon', 'graphics'))
+ 'icon', 'graphics', 'users',
'groups'))
stats_keys = set(('cpu_utilization',
'net_throughput', 'net_throughput_peak',
'io_throughput', 'io_throughput_peak'))
@@ -70,6 +70,8 @@ class ModelTests(unittest.TestCase):
self.assertEquals(None, info['icon'])
self.assertEquals(stats_keys, set(info['stats'].keys()))
self.assertRaises(NotFoundError, inst.vm_lookup, 'nosuchvm')
+ self.assertEquals([], info['users'])
+ self.assertEquals([], info['groups'])
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_vm_lifecycle(self):
diff --git a/tests/test_rest.py b/tests/test_rest.py
index ca96dc0..f8a87e6 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -182,6 +182,8 @@ class RestTests(unittest.TestCase):
vm = json.loads(self.request('/vms/vm-1').read())
self.assertEquals('vm-1', vm['name'])
self.assertEquals('shutoff', vm['state'])
+ self.assertEquals(['user1', 'user2', 'root'],
vm['users'])
+ self.assertEquals(['group1', 'group2', 'admin'],
vm['groups'])
def test_edit_vm(self):
req = json.dumps({'name': 'test', 'cdrom':
'/nonexistent.iso'})
8:43 a.m.
New subject: [PATCH 2/4] Add functions to check if a user/group exists
Am 27-02-2014 11:20, schrieb Rodrigo Trujillo:
Maybe the naming is wrong below.
The map key USER_ID actually
contains the user name, not the ID. That is
how the class User is already implemented. We can send another patch
later to rename it but that is not the scope of this patch.
You are passing the User ID and getpwnam expects the User name
No, I am passing the user name because, as you said, getpwnam expects a
user name.
I did not test the patch manually,
I did.
not sure if it will work.
It does :)
Same problem that before, because you are
passing the User name to User Class , which expects the User ID
No, it expects a
user name.
... If this work, then the name of
parameters should change
I agree. But in another patch. I am working on an existing
code which
has a variable named "userid" containing a user name.
9:09 a.m.
New subject: [PATCH 4/4] Add/remove users and groups to VMs
2014/2/27 2:09, Crístian Viana :
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>
---
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")
How about the result when two request come into Kimchi at the same time
to update the users and groups to the same VM?
+
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)
The same question for current requests.
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)
9:15 a.m.
New subject: [PATCH 3/4] Return users and groups when fetching VM info
Am 27-02-2014 11:37, schrieb Rodrigo Trujillo:
I was wondering whether would be better to store the user id in vm
xml
instead of the user name (login).
Yes, that is also an option. But as you said
below, the user ID can also
be changed, so we can never make sure that what we store in the VM
metadata will hold true forever. In any case, the user can always be
deleted and we will have invalid information. While none of the
approaches are perfect, I chose to store user names as they are more
easily readable.
But we can store user IDs as well, if there is any advantage.
+ try:
> + users = _get_vm_metadata(xml, "users")
> + except:
You could log the error here. I think its good for debugging and
general information.
> + # Wrong XML tree structure, wrong array syntax, or empty
> value:
> + # ignore error and build an empty list
> + users = []
I decided not to log an error message because, in most
of the times (at
least for now), there will no VM metadata, so we would see two errors
for each VM fetched. E.g.: if you have 10 VMs and you click on the tab
"Guests", you will see a flood of at least 20 error messages. Not having
user and groups metadata is not an error; if the user just creates a VM
and do not assign users or groups, I do not think their VM info is wrong.
I can rewrite that snippet to log only actual error messages (e.g. if
someone edits the XML manually and puts wrong metadata info), but not
empty metadata info.
9:23 a.m.
New subject: [PATCH 4/4] Add/remove users and groups to VMs
Another useful API comes to my mind, the administrator will most likely
just add one user from the existing users. How can he do that? He is
lazy and would like to input one new user instead of all the existing
users plus the new user. I think we can have something like these:
/vms/vm1 PUT { 'IsAdd': True, 'users': "user4'} Add user4 to
existing
['user1', 'user2', user3'] for vm1
/vms/<vm1 PUT { 'IsAdd': False, 'users': "user1'} Delete
user1 from
existing ['user1', 'user2', user3'] vm1
2014/2/27 2:09, Crístian Viana:
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>
---
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)
1:59 p.m.
New subject: [PATCH 4/4] Add/remove users and groups to VMs
Am 27-02-2014 13:23, schrieb Shu Ming:
Another useful API comes to my mind, the administrator will most
likely just add one user from the existing users. How can he do
that? He is lazy and would like to input one new user instead of all
the existing users plus the new user. I think we can have something
like these:
/vms/vm1 PUT { 'IsAdd': True, 'users': "user4'} Add user4 to
existing
['user1', 'user2', user3'] for vm1
/vms/<vm1 PUT { 'IsAdd': False, 'users': "user1'} Delete
user1 from
existing ['user1', 'user2', user3'] vm1
It would be useful but it is not needed. If the administrator is lazy,
he should not use the REST API directly. There is (well there will be) a
UI in which they can click, and add, and remove users very easily. The
UI handles the REST commands and generates the new array with the
resulting users/groups.
9:32 a.m.
New subject: [PATCH 4/4] Add/remove users and groups to VMs
Am 27-02-2014 13:09, schrieb Shu Ming:
How about the result when two request come into Kimchi at the same
time to update the users and groups to the same VM?
Yes, there is a race condition
here. If multiple users update that
information at the same time, one update may overwrite the other one.
Race conditions are all around the Kimchi code: if one user creates a VM
using some template and, at the same time, another user removes that
template, the VM may be created with an invalid template. That is just
one of the several scenarios in which we may hit a race condition today.
But we do not have an easy solution for this right now. In order to make
sure our code is safe from race conditions, we need to use
synchronization features like locks and mutexes. It will take a lot of
effort for that to be implemented and tested across the multiple Kimchi
layers. We also need to make sure that libvirt has some locking commands
in its API, otherwise, even if we get this right in Kimchi, users from
other VM managers can invalidate our operations. It would be useless to
secure our own code from parallel access if someone is able to edit the
VM XML using, let's say, virsh.
So in summary, I am aware of this problem (well, I wasn't before you
brought it up here :) ) but I cannot fix it right away. We need to have
a long discussion on how we will tackle race conditions in Kimchi and
then we can fix this. In the meantime, we can only hope that
simultaneous access for the same operations do not happen.
3720
days inactive
3722
days old
12 comments
3 participants
participants (3)
-
Crístian Viana -
Rodrigo Trujillo -
Shu Ming