From 15012344b1df21c84baa6777c3f9cccbfa46e10b Mon Sep 17 00:00:00 2001 From: "M. Rehan" Date: Tue, 14 Jan 2025 23:25:29 +0500 Subject: [PATCH] Allow specifying VNC passwords for incus VMs --- .../middlewared/api/v25_04_0/virt_instance.py | 9 +++- .../middlewared/plugins/virt/instance.py | 43 ++++++++++++++++--- .../middlewared/plugins/virt/utils.py | 15 ++----- tests/api2/test_virt_vm.py | 26 ++++++++--- 4 files changed, 68 insertions(+), 25 deletions(-) diff --git a/src/middlewared/middlewared/api/v25_04_0/virt_instance.py b/src/middlewared/middlewared/api/v25_04_0/virt_instance.py index 74fb138963341..15c22c9a7816b 100644 --- a/src/middlewared/middlewared/api/v25_04_0/virt_instance.py +++ b/src/middlewared/middlewared/api/v25_04_0/virt_instance.py @@ -1,6 +1,6 @@ from typing import Annotated, Literal, TypeAlias -from pydantic import Field, model_validator, StringConstraints +from pydantic import Field, model_validator, Secret, StringConstraints from middlewared.api.base import BaseModel, ForUpdateMetaclass, NonEmptyString, single_argument_args @@ -63,6 +63,7 @@ class VirtInstanceEntry(BaseModel): raw: dict | None vnc_enabled: bool vnc_port: int | None + vnc_password: Secret[NonEmptyString | None] # Lets require at least 32MiB of reserved memory @@ -87,6 +88,7 @@ class VirtInstanceCreateArgs(BaseModel): memory: MemoryType | None = None enable_vnc: bool = False vnc_port: int | None = Field(ge=5900, le=65535, default=None) + vnc_password: Secret[NonEmptyString | None] = None @model_validator(mode='after') def validate_attrs(self): @@ -99,6 +101,9 @@ def validate_attrs(self): if self.enable_vnc and self.vnc_port is None: raise ValueError('VNC port must be set when VNC is enabled') + if self.vnc_password is not None and not self.enable_vnc: + raise ValueError('VNC password can only be set when VNC is enabled') + if self.source_type == 'ISO' and self.iso_volume is None: raise ValueError('ISO volume must be set when source type is "ISO"') @@ -119,6 +124,8 @@ class VirtInstanceUpdate(BaseModel, metaclass=ForUpdateMetaclass): memory: MemoryType | None = None vnc_port: int | None = Field(ge=5900, le=65535) enable_vnc: bool + vnc_password: Secret[NonEmptyString | None] + '''Setting vnc_password to null will unset VNC password''' class VirtInstanceUpdateArgs(BaseModel): diff --git a/src/middlewared/middlewared/plugins/virt/instance.py b/src/middlewared/middlewared/plugins/virt/instance.py index 5c988b9ce7e5c..2a7fbaf64b928 100644 --- a/src/middlewared/middlewared/plugins/virt/instance.py +++ b/src/middlewared/middlewared/plugins/virt/instance.py @@ -158,15 +158,35 @@ async def validate(self, new, schema_name, verrors, old=None): enable_vnc = new.get('enable_vnc') if enable_vnc is False: # User explicitly disabled VNC support, let's remove vnc port - new['vnc_port'] = None + new.update({ + 'vnc_port': None, + 'vnc_password': None, + }) + elif enable_vnc is True: + if not old['vnc_port'] and not new.get('vnc_port'): + verrors.add(f'{schema_name}.vnc_port', 'VNC port is required when VNC is enabled') + elif not new.get('vnc_port'): + new['vnc_port'] = old['vnc_port'] + + if 'vnc_password' not in new: + new['vnc_password'] = old['vnc_password'] elif enable_vnc is None: - if new.get('vnc_port'): - verrors.add(f'{schema_name}.enable_vnc', 'Should be set when vnc_port is specified') - elif old['vnc_enabled'] and old['vnc_port']: + for k in ('vnc_port', 'vnc_password'): + if new.get(k): + verrors.add(f'{schema_name}.enable_vnc', f'Should be set when {k!r} is specified') + + if old['vnc_enabled'] and old['vnc_port']: # We want to handle the case where nothing has been changed on vnc attrs new.update({ 'enable_vnc': True, 'vnc_port': old['vnc_port'], + 'vnc_password': old['vnc_password'], + }) + else: + new.update({ + 'enable_vnc': False, + 'vnc_port': None, + 'vnc_password': None, }) else: # Creation case @@ -217,11 +237,20 @@ def __data_to_config(self, data: dict, raw: dict = None, instance_type=None): config['boot.autostart'] = str(data['autostart']).lower() if instance_type == 'VM': + config['user.ix_old_raw_qemu_config'] = raw.get('raw.qemu', '') if raw else '' + config['user.ix_vnc_config'] = json.dumps({ + 'vnc_enabled': data['enable_vnc'], + 'vnc_port': data['vnc_port'], + 'vnc_password': data['vnc_password'], + }) + if data.get('enable_vnc') and data.get('vnc_port'): - config['user.ix_old_raw_qemu_config'] = raw.get('raw.qemu', '') if raw else '' - config['raw.qemu'] = f'-vnc :{data["vnc_port"] - VNC_BASE_PORT}' + vnc_config = f'-vnc :{data["vnc_port"] - VNC_BASE_PORT}' + if data.get('vnc_password'): + vnc_config = f'-object secret,id=vnc0,data={data["vnc_password"]} {vnc_config},password-secret=vnc0' + + config['raw.qemu'] = vnc_config if data.get('enable_vnc') is False: - config['user.ix_old_raw_qemu_config'] = raw.get('raw.qemu', '') if raw else '' config['raw.qemu'] = '' return config diff --git a/src/middlewared/middlewared/plugins/virt/utils.py b/src/middlewared/middlewared/plugins/virt/utils.py index aad7accc8c5cf..23ecc8df16f90 100644 --- a/src/middlewared/middlewared/plugins/virt/utils.py +++ b/src/middlewared/middlewared/plugins/virt/utils.py @@ -2,7 +2,7 @@ import aiohttp import enum import httpx -import re +import json from collections.abc import Callable from .websocket import IncusWS @@ -12,7 +12,6 @@ SOCKET = '/var/lib/incus/unix.socket' HTTP_URI = 'http://unix.socket' -RE_VNC_PORT = re.compile(r'vnc.*?:(\d+)\s*') VNC_BASE_PORT = 5900 @@ -98,15 +97,9 @@ def get_vnc_info_from_config(config: dict): vnc_config = { 'vnc_enabled': False, 'vnc_port': None, + 'vnc_password': None, } - if not (raw_qemu_config := config.get('raw.qemu')) or 'vnc' not in raw_qemu_config: + if not (vnc_raw_config := config.get('user.ix_vnc_config')): return vnc_config - for flag in raw_qemu_config.split('-'): - if port := RE_VNC_PORT.findall(flag): - return { - 'vnc_enabled': True, - 'vnc_port': int(port[0]) + VNC_BASE_PORT, - } - - return vnc_config + return json.loads(vnc_raw_config) diff --git a/tests/api2/test_virt_vm.py b/tests/api2/test_virt_vm.py index 86f504a39d8df..4ddf6763c6a91 100644 --- a/tests/api2/test_virt_vm.py +++ b/tests/api2/test_virt_vm.py @@ -35,6 +35,7 @@ def vm(virt_pool): 'vnc_port': VNC_PORT, 'enable_vnc': True, 'instance_type': 'VM', + 'vnc_password': 'test123' }, job=True) call('virt.instance.stop', VM_NAME, {'force': True, 'timeout': 1}, job=True) try: @@ -119,11 +120,13 @@ def test_vm_props(vm): # Testing VNC specific bits assert instance['vnc_enabled'] is True, instance assert instance['vnc_port'] == VNC_PORT, instance + assert instance['vnc_password'] == 'test123', instance # Going to unset VNC call('virt.instance.update', VM_NAME, {'enable_vnc': False}, job=True) instance = call('virt.instance.get_instance', VM_NAME, {'extra': {'raw': True}}) - assert instance['raw']['config']['user.ix_old_raw_qemu_config'] == f'-vnc :{VNC_PORT - 5900}' + assert instance['raw']['config']['user.ix_old_raw_qemu_config'] == f'-object secret,id=vnc0,data=test123 ' \ + f'-vnc :{VNC_PORT - 5900},password-secret=vnc0' assert instance['vnc_enabled'] is False, instance assert instance['vnc_port'] is None, instance @@ -134,9 +137,18 @@ def test_vm_props(vm): assert instance['raw']['config']['raw.qemu'] == f'-vnc :{1001}' assert instance['vnc_port'] == 6901, instance + # Going to update password + call('virt.instance.update', VM_NAME, {'vnc_password': 'update_test123', 'enable_vnc': True}, job=True) + instance = call('virt.instance.get_instance', VM_NAME, {'extra': {'raw': True}}) + assert instance['raw']['config'].get('user.ix_old_raw_qemu_config') == f'-vnc :{1001}' + assert instance['raw']['config']['raw.qemu'] == f'-object secret,id=vnc0,data=update_test123' \ + f' -vnc :{1001},password-secret=vnc0' + assert instance['vnc_port'] == 6901, instance + # Changing nothing instance = call('virt.instance.update', VM_NAME, {}, job=True) assert instance['vnc_port'] == 6901, instance + assert instance['vnc_password'] == 'update_test123', instance def test_vm_iso_volume(vm, iso_volume): @@ -184,18 +196,20 @@ def test_iso_param_validation_on_vm_create(virt_pool, iso_volume, error_msg): assert ve.value.errors[0].errmsg == error_msg -@pytest.mark.parametrize('enable_vnc,vnc_port,error_msg', [ - (True, None, 'Value error, VNC port must be set when VNC is enabled'), - (True, 6901, 'VNC port is already in use by another virt instance'), - (True, 23, 'Input should be greater than or equal to 5900'), +@pytest.mark.parametrize('enable_vnc,vnc_password,vnc_port,error_msg', [ + (True, None, None, 'Value error, VNC port must be set when VNC is enabled'), + (True, None, 6901, 'VNC port is already in use by another virt instance'), + (True, None, 23, 'Input should be greater than or equal to 5900'), + (False, 'test_123', None, 'Value error, VNC password can only be set when VNC is enabled'), ]) -def test_vnc_validation_on_vm_create(virt_pool, enable_vnc, vnc_port, error_msg): +def test_vnc_validation_on_vm_create(virt_pool, enable_vnc, vnc_password, vnc_port, error_msg): with pytest.raises(ValidationErrors) as ve: call('virt.instance.create', { 'name': 'test-vnc-vm', 'instance_type': 'VM', 'source_type': None, 'vnc_port': vnc_port, + 'vnc_password': vnc_password, 'enable_vnc': enable_vnc, }, job=True)