Re: [Kimchi-devel] [PATCH 4/5] Return users and groups when fetching VM info

One comment below... On Fri, 2014-04-11 at 17:58 -0300, Aline Manera wrote: > From: Aline Manera <alinefm(a)br.ibm.com&gt; > > 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&gt; > Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; > > show > > show > --- > 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 | 13 ++++++++++--- > tests/test_mockmodel.py | 2 +- > tests/test_model.py | 4 +++- > tests/test_rest.py | 2 ++ > 9 files changed, 52 insertions(+), 7 deletions(-) > > diff --git a/docs/API.md b/docs/API.md > index 143c70c..b726b9c 100644 > --- a/docs/API.md > +++ b/docs/API.md > @@ -94,6 +94,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. Sorry this is a little late, but, from this, it's not obvious what the default empty list means. Does it mean that no users can access the VM, or that there are no restrictions on access? I think a simple patch to clarify will prevent confusion.

> * **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 5ca94e3..6294dc8 100644 > --- a/src/kimchi/API.json > +++ b/src/kimchi/API.json > @@ -201,6 +201,28 @@ > "type": "string", > "minLength": 1, > "error": "KCHVM0011E" > + }, > + "users": { > + "description": "Array of users who have permission to the VM", > + "type": "array", > + "uniqueItems": true, > + "error": "KCHVM0022E", > + "items": { > + "description": "User name", > + "type": "string", > + "error": "KCHVM0023E" > + } > + }, > + "groups": { > + "description": "Array of groups who have permission to the VM", > + "type": "array", > + "uniqueItems": true, > + "error": "KCHVM0024E", > + "items": { > + "description": "Group name", > + "type": "string", > + "error": "KCHVM0025E" > + } > } > } > }, > diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py > index e75b27f..63c1e24 100644 > --- a/src/kimchi/control/vms.py > +++ b/src/kimchi/control/vms.py > @@ -53,7 +53,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 0bde5b9..20f9716 100644 > --- a/src/kimchi/i18n.py > +++ b/src/kimchi/i18n.py > @@ -80,6 +80,10 @@ messages = { > "KCHVM0019E": _("Unable to start virtual machine %(name)s. Details: %(err)s"), > "KCHVM0020E": _("Unable to power off virtual machine %(name)s. Details: %(err)s"), > "KCHVM0021E": _("Unable to delete virtual machine %(name)s. Details: %(err)s"), > + "KCHVM0022E": _("User names list must be an array"), > + "KCHVM0023E": _("User name must be a string"), > + "KCHVM0024E": _("Group names list must be an array"), > + "KCHVM0025E": _("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 bf17975..87ca21c 100644 > --- a/src/kimchi/mockmodel.py > +++ b/src/kimchi/mockmodel.py > @@ -969,7 +969,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 2425a43..ba091e5 100644 > --- a/src/kimchi/model/vms.py > +++ b/src/kimchi/model/vms.py > @@ -34,8 +34,9 @@ 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 > -from kimchi.utils import run_setfacl_set_attr, template_name_from_uri > +from kimchi.utils import kimchi_log, run_setfacl_set_attr > +from kimchi.utils import template_name_from_uri > +from kimchi.xmlutils import xpath_get_text > > > DOM_STATE_MAP = {0: 'nostate', > @@ -304,6 +305,10 @@ 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) > + users = xpath_get_text(xml, "/domain/metadata/kimchi/access/user") > + groups = xpath_get_text(xml, "/domain/metadata/kimchi/access/group") > + > return {'state': state, > 'stats': res, > 'uuid': dom.UUIDString(), > @@ -313,7 +318,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 0798701..9e258ce 100644 > --- a/tests/test_mockmodel.py > +++ b/tests/test_mockmodel.py > @@ -135,7 +135,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 3041196..00c02d3 100644 > --- a/tests/test_model.py > +++ b/tests/test_model.py > @@ -61,7 +61,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')) > @@ -73,6 +73,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 6bce606..9c2f997 100644 > --- a/tests/test_rest.py > +++ b/tests/test_rest.py > @@ -181,6 +181,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'}) _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel