[Kimchi-devel] [PATCH 1/2] Centralize graphics information

Recently the ticket information was added to the vm to report the graphics password and its expiration time. That way, graphics information was found in 2 different data structuce: some information under "graphics" and other ones under "ticket". { "graphics":{ "type":"vnc", "port":null, "listen":"127.0.0.1" }, "ticket":{ "passwd":null, "expire":null }, "name":"Ubuntu14.04", "uuid":"8522d82d-d51b-436f-88ff-aff1fed27613", "access":"full", "state":"shutoff", "memory":1024 ... } As "ticket" data structure only contains graphics information it makes more sense to centralize it under "graphics" and also use the same terminology used by libvirt (passwd and passwdValidTo) "graphics":{ "type":"vnc", "port":null, "listen":"127.0.0.1" "passwd":null, "passwdValidTo":null }, Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; --- docs/API.md | 14 +++--- src/kimchi/API.json | 12 ++--- src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 4 +- src/kimchi/mockmodel.py | 38 +++++++++------- src/kimchi/model/vms.py | 112 ++++++++++++++++++++++------------------------ tests/test_mockmodel.py | 2 +- tests/test_model.py | 19 ++++---- tests/test_rest.py | 13 +++--- 9 files changed, 111 insertions(+), 105 deletions(-) diff --git a/docs/API.md b/docs/API.md index ebb6e61..20f5f23 100644 --- a/docs/API.md +++ b/docs/API.md @@ -96,9 +96,8 @@ 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. - * ticket: A ticket represents credentials to access VM. - * password: the password of a ticket. - * expire: lifetime of a ticket. + * passwd: console password + * passwdValidTo: lifetime for the console password. * users: A list of system users who have permission to access the VM. Default is: empty (i.e. only root-users may access). * groups: A list of system groups whose users have permission to access @@ -112,9 +111,12 @@ the following general conventions: will take effect in next reboot) * memory: New amount of memory (MB) for this VM (if VM is running, new value will take effect in next reboot) - * ticket: A ticket represents credentials to access VM. - * password *(optional)*: the password of a ticket. - * expire *(optional)*: lifetime of a ticket. + * graphics: A dict to show detail of VM graphics. + * passwd *(optional)*: console password. When omitted a random password + willbe generated. + * passwdValidTo *(optional)*: lifetime for the console password. When + omitted the password will be valid just + for 30 seconds. * **POST**: *See Virtual Machine Actions* diff --git a/src/kimchi/API.json b/src/kimchi/API.json index c3fc5e3..67612f2 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -235,17 +235,17 @@ "error": "KCHVM0026E" } }, - "ticket": { - "description": "A ticket represents credentials to access VM", + "graphics": { + "description": "Graphics information from guest", "type": "object", "properties": { - "password": { - "description": "the password of a ticket.", + "passwd": { + "description": "New graphics password.", "type": "string", "error": "KCHVM0031E" }, - "size": { - "description": "lifetime of a ticket.", + "passwdValidTo": { + "description": "Life time for the graphics password.", "type": "number", "error": "KCHVM0032E" } diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 6632e52..88d8a81 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -36,7 +36,7 @@ class VM(Resource): super(VM, self).__init__(model, ident) self.role_key = 'guests' self.update_params = ["name", "users", "groups", "cpus", "memory", - "ticket"] + "graphics"] 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 2eae7e8..c276b38 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -94,8 +94,8 @@ messages = { "KCHVM0028E": _("Group(s) '%(groups)s' do not exist"), "KCHVM0029E": _("Unable to shutdown virtual machine %(name)s. Details: %(err)s"), "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"), - "KCHVM0031E": _("password of a vm ticket must be a string."), - "KCHVM0032E": _("expire of a vm ticket must be a number."), + "KCHVM0031E": _("The guest console password must be a string."), + "KCHVM0032E": _("The life time for the guest console password must be a number."), "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 fc474de..1322def 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -91,15 +91,6 @@ class MockModel(object): def _static_vm_update(self, dom, params): state = dom.info['state'] - if 'ticket' in params: - ticket = params.pop('ticket') - passwd = ticket.get('passwd') - dom.ticket["passwd"] = passwd if passwd is not None else "".join( - random.sample(string.ascii_letters + string.digits, 8)) - expire = ticket.get('expire') - dom.ticket["ValidTo"] = expire if expire is None else round( - time.time()) + expire - for key, val in params.items(): if key == 'name': if state == 'running' or params['name'] in self.vms_get_list(): @@ -126,7 +117,21 @@ class MockModel(object): dom.info[key] = val def _live_vm_update(self, dom, params): - pass + if 'graphics' not in params: + return + + graphics = params.pop('graphics') + passwd = graphics.get('passwd') + if passwd is None: + passwd = "".join(random.sample(string.ascii_letters + + string.digits, 8)) + + expire = graphics.get('passwdValidTo') + if expire is not None: + expire = round(time.time()) + expire + + dom.info['graphics']["passwd"] = passwd + dom.info['graphics']["passwdValidTo"] = expire def vm_update(self, name, params): dom = self._get_vm(name) @@ -141,10 +146,11 @@ class MockModel(object): vm.info['screenshot'] = self.vmscreenshot_lookup(name) else: vm.info['screenshot'] = None - vm.info['ticket'] = {"passwd": vm.ticket["passwd"]} - validTo = vm.ticket["ValidTo"] - vm.info['ticket']["expire"] = (validTo - round(time.time()) - if validTo is not None else None) + + validTo = vm.info['graphics']['passwdValidTo'] + validTo = (validTo - round(time.time()) if validTo is not None + else None) + vm.info['graphics']['passwdValidTo'] = validTo return vm.info def vm_delete(self, name): @@ -1050,7 +1056,6 @@ class MockVM(object): self.networks = template_info['networks'] ifaces = [MockVMIface(net) for net in self.networks] self.storagedevices = {} - self.ticket = {"passwd": "123456", "ValidTo": None} self.ifaces = dict([(iface.info['mac'], iface) for iface in ifaces]) stats = {'cpu_utilization': 20, @@ -1066,7 +1071,8 @@ class MockVM(object): 'cpus': self.cpus, 'icon': None, 'graphics': {'type': 'vnc', 'listen': '127.0.0.1', - 'port': None}, + 'port': None, 'passwd': '123456', + 'passwdValidTo': None}, 'users': ['user1', 'user2', 'root'], 'groups': ['group1', 'group2', 'admin'], 'access': 'full' diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 5721b48..c14d126 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -251,12 +251,6 @@ class VMModel(object): self.groups = import_class('kimchi.model.host.GroupsModel')(**kargs) def update(self, name, params): - if 'ticket' in params: - password = params['ticket'].get("passwd") - password = password if password is not None else "".join( - random.sample(string.ascii_letters + string.digits, 8)) - params['ticket']['passwd'] = password - dom = self.get_vm(name, self.conn) dom = self._static_vm_update(dom, params) self._live_vm_update(dom, params) @@ -315,14 +309,15 @@ class VMModel(object): os_elem = E.os({"distro": distro, "version": version}) set_metadata_node(dom, os_elem) - def _set_ticket(self, xml, params, flag=0): + def _set_graphics_passwd(self, xml, params, flag=0): DEFAULT_VALID_TO = 30 password = params.get("passwd") - expire = params.get("expire") + expire = params.get("passwdValidTo") root = objectify.fromstring(xml) graphic = root.devices.find("graphics") if graphic is None: return None + graphic.attrib['passwd'] = password to = graphic.attrib.get('passwdValidTo') if to is not None: @@ -337,27 +332,27 @@ class VMModel(object): return root if flag == 0 else graphic - def _set_persistent_ticket(self, dom, xml, params): - if 'ticket' not in params: - # store the password for copy - ticket = self._get_ticket(dom) - if ticket['passwd'] is None: - return xml - else: - params['ticket'] = ticket - node = self._set_ticket(xml, params['ticket']) - return xml if node is None else ET.tostring(node, encoding="utf-8") + def _update_graphics(self, dom, xml, params): + if 'graphics' not in params: + return xml - def _set_live_ticket(self, dom, params): - if 'ticket' not in params: - return - if dom.isActive(): - xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE) - flag = libvirt.VIR_DOMAIN_XML_SECURE - node = self._set_ticket(xml, params['ticket'], flag) - if node is not None: - dom.updateDeviceFlags(etree.tostring(node), - libvirt.VIR_DOMAIN_AFFECT_LIVE) + password = params['graphics'].get("passwd") + if password is None: + password = "".join(random.sample(string.ascii_letters + + string.digits, 8)) + params['graphics']['passwd'] = password + + if not dom.isActive(): + node = self._set_graphics_passwd(xml, params['graphics']) + return xml if node is None else ET.tostring(node, encoding="utf-8") + + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE) + flag = libvirt.VIR_DOMAIN_XML_SECURE + node = self._set_graphics_passwd(xml, params['graphics'], flag) + if node is not None: + dom.updateDeviceFlags(etree.tostring(node), + libvirt.VIR_DOMAIN_AFFECT_LIVE) + return xml def _static_vm_update(self, dom, params): state = DOM_STATE_MAP[dom.info()[0]] @@ -374,7 +369,7 @@ class VMModel(object): xpath = VM_STATIC_UPDATE_PARAMS[key] new_xml = xmlutils.xml_item_update(new_xml, xpath, val) - new_xml = self._set_persistent_ticket(dom, new_xml, params) + new_xml = self._update_graphics(dom, new_xml, params) conn = self.conn.get() try: @@ -397,35 +392,19 @@ class VMModel(object): def _live_vm_update(self, dom, params): self._vm_update_access_metadata(dom, params) - self._set_live_ticket(dom, params) def _has_video(self, dom): dom = ElementTree.fromstring(dom.XMLDesc(0)) return dom.find('devices/video') is not None - def _get_ticket(self, dom): - xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE) - root = objectify.fromstring(xml) - graphic = root.devices.find("graphics") - if graphic is None: - return {"passwd": None, "expire": None} - passwd = graphic.attrib.get('passwd') - ticket = {"passwd": passwd} - valid_to = graphic.attrib.get('passwdValidTo') - ticket['expire'] = None - if valid_to is not None: - to = time.mktime(time.strptime(valid_to, '%Y-%m-%dT%H:%M:%S')) - ticket['expire'] = to - time.mktime(time.gmtime()) - - return ticket - def lookup(self, name): dom = self.get_vm(name, self.conn) info = dom.info() state = DOM_STATE_MAP[info[0]] screenshot = None + # (type, listen, port, passwd, passwdValidTo) graphics = self._vm_get_graphics(name) - graphics_type, graphics_listen, graphics_port = graphics + graphics_port = graphics[2] graphics_port = graphics_port if state == 'running' else None try: if state == 'running' and self._has_video(dom): @@ -465,10 +444,12 @@ class VMModel(object): 'cpus': info[3], 'screenshot': screenshot, 'icon': icon, - 'graphics': {"type": graphics_type, - "listen": graphics_listen, - "port": graphics_port}, - 'ticket': self._get_ticket(dom), + # (type, listen, port, passwd, passwdValidTo) + 'graphics': {"type": graphics[0], + "listen": graphics[1], + "port": graphics_port, + "passwd": graphics[3], + "passwdValidTo": graphics[4]}, 'users': users, 'groups': groups, 'access': 'full', @@ -568,23 +549,38 @@ class VMModel(object): def _vm_get_graphics(self, name): dom = self.get_vm(name, self.conn) - xml = dom.XMLDesc(0) + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE) + expr = "/domain/devices/graphics/@type" res = xmlutils.xpath_get_text(xml, expr) graphics_type = res[0] if res else None + expr = "/domain/devices/graphics/@listen" res = xmlutils.xpath_get_text(xml, expr) graphics_listen = res[0] if res else None - graphics_port = None + + graphics_port = graphics_passwd = graphics_passwdValidTo = None if graphics_type: - expr = "/domain/devices/graphics[@type='%s']/@port" % graphics_type - res = xmlutils.xpath_get_text(xml, expr) + expr = "/domain/devices/graphics[@type='%s']/@port" + res = xmlutils.xpath_get_text(xml, expr % graphics_type) graphics_port = int(res[0]) if res else None - return graphics_type, graphics_listen, graphics_port + + expr = "/domain/devices/graphics[@type='%s']/@passwd" + res = xmlutils.xpath_get_text(xml, expr % graphics_type) + graphics_passwd = res[0] if res else None + + expr = "/domain/devices/graphics[@type='%s']/@passwdValidTo" + res = xmlutils.xpath_get_text(xml, expr % graphics_type) + if res: + to = time.mktime(time.strptime(res[0], '%Y-%m-%dT%H:%M:%S')) + graphics_passwdValidTo = to - time.mktime(time.gmtime()) + + return (graphics_type, graphics_listen, graphics_port, + graphics_passwd, graphics_passwdValidTo) def connect(self, name): - graphics = self._vm_get_graphics(name) - graphics_type, graphics_listen, graphics_port = graphics + # (type, listen, port, passwd, passwdValidTo) + graphics_port = self._vm_get_graphics(name)[2] if graphics_port is not None: vnc.add_proxy_token(name, graphics_port) else: diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index b7e6a48..8de7ce9 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -143,7 +143,7 @@ class MockModelTests(unittest.TestCase): keys = set(('name', 'state', 'stats', 'uuid', 'memory', 'cpus', 'screenshot', 'icon', 'graphics', 'users', 'groups', - 'access', 'ticket')) + 'access')) stats_keys = set(('cpu_utilization', 'net_throughput', 'net_throughput_peak', diff --git a/tests/test_model.py b/tests/test_model.py index 59f416c..4eed8b3 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -65,7 +65,7 @@ class ModelTests(unittest.TestCase): keys = set(('name', 'state', 'stats', 'uuid', 'memory', 'cpus', 'screenshot', 'icon', 'graphics', 'users', 'groups', - 'access', 'ticket', 'persistent')) + 'access', 'persistent')) stats_keys = set(('cpu_utilization', 'net_throughput', 'net_throughput_peak', @@ -765,24 +765,25 @@ class ModelTests(unittest.TestCase): vms = inst.vms_get_list() self.assertTrue('kimchi-vm1' in vms) - # update vm ticket when vm is not running + # update vm graphics when vm is not running inst.vm_update(u'kimchi-vm1', - {"ticket": {"passwd": "123456"}}) + {"graphics": {"passwd": "123456"}}) inst.vm_start('kimchi-vm1') rollback.prependDefer(self._rollback_wrapper, inst.vm_poweroff, 'kimchi-vm1') vm_info = inst.vm_lookup(u'kimchi-vm1') - self.assertEquals('123456', vm_info['ticket']["passwd"]) - self.assertEquals(None, vm_info['ticket']["expire"]) + self.assertEquals('123456', vm_info['graphics']["passwd"]) + self.assertEquals(None, vm_info['graphics']["passwdValidTo"]) - # update vm ticket when vm is running + # update vm graphics when vm is running inst.vm_update(u'kimchi-vm1', - {"ticket": {"passwd": "abcdef", "expire": 20}}) + {"graphics": {"passwd": "abcdef", + "passwdValidTo": 20}}) vm_info = inst.vm_lookup(u'kimchi-vm1') - self.assertEquals('abcdef', vm_info['ticket']["passwd"]) - self.assertGreaterEqual(20, vm_info['ticket']['expire']) + self.assertEquals('abcdef', vm_info['graphics']["passwd"]) + self.assertGreaterEqual(20, vm_info['graphics']['passwdValidTo']) info = inst.vm_lookup('kimchi-vm1') self.assertEquals('running', info['state']) diff --git a/tests/test_rest.py b/tests/test_rest.py index 2117399..0bf5997 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -221,20 +221,21 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(200, resp.status) - req = json.dumps({"ticket": {'passwd': "abcdef"}}) + req = json.dumps({"graphics": {'passwd': "abcdef"}}) resp = self.request('/vms/vm-1', req, 'PUT') info = json.loads(resp.read()) - self.assertEquals('abcdef', info["ticket"]["passwd"]) - self.assertEquals(None, info["ticket"]["expire"]) + self.assertEquals('abcdef', info["graphics"]["passwd"]) + self.assertEquals(None, info["graphics"]["passwdValidTo"]) resp = self.request('/vms/vm-1/poweroff', '{}', 'POST') self.assertEquals(200, resp.status) - req = json.dumps({"ticket": {'passwd': "123456", "expire": 20}}) + req = json.dumps({"graphics": {'passwd': "123456", + 'passwdValidTo': 20}}) resp = self.request('/vms/vm-1', req, 'PUT') info = json.loads(resp.read()) - self.assertEquals('123456', info["ticket"]["passwd"]) - self.assertGreaterEqual(20, info["ticket"]["expire"]) + self.assertEquals('123456', info["graphics"]["passwd"]) + self.assertGreaterEqual(20, info["graphics"]["passwdValidTo"]) req = json.dumps({'name': 12}) resp = self.request('/vms/vm-1', req, 'PUT') -- 1.9.3