From 8952495c9df2d32dc05bee73462714e3bac43ba3 Mon Sep 17 00:00:00 2001 From: Kamil Monticolo Date: Tue, 28 Oct 2025 09:03:44 +0100 Subject: [PATCH 1/3] Add validation to require serial device when vga type is set to serial --- proxmox/resource_vm_qemu.go | 42 +++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/proxmox/resource_vm_qemu.go b/proxmox/resource_vm_qemu.go index 1d7fde38..b1fc2c82 100755 --- a/proxmox/resource_vm_qemu.go +++ b/proxmox/resource_vm_qemu.go @@ -612,6 +612,10 @@ func resourceVmQemuCreate(ctx context.Context, d *schema.ResourceData, meta inte config.EFIDisk = qemuEfiDisks[0] } + if err := validateVGAAndSerial(d); err != nil { + return diag.FromErr(err) + } + var vmr *pveSDK.VmRef if guestID := vmID.Get(d); guestID != 0 { // Manually set vmID log.Print("[DEBUG][QemuVmCreate] checking if vmId: " + guestID.String() + " already exists") @@ -871,6 +875,10 @@ func resourceVmQemuUpdate(ctx context.Context, d *schema.ResourceData, meta inte return diags } + if err := validateVGAAndSerial(d); err != nil { + return diag.FromErr(err) + } + logger.Debug().Int(vmID.Root, int(resourceID.ID)).Msgf("Updating VM with the following configuration: %+v", config) var rebootRequired bool @@ -1521,3 +1529,37 @@ func mapToSDK_QemuGuestAgent(d *schema.ResourceData) *pveSDK.QemuGuestAgent { Enable: &tmpEnable, } } + +func validateVGAAndSerial(d *schema.ResourceData) error { + // Get VGA configuration + vgaList := d.Get("vga").([]interface{}) + + if len(vgaList) > 0 { + vga := vgaList[0].(map[string]interface{}) + vgaType := vga["type"].(string) + + // Check if vga type is serial0, serial1, serial2, or serial3 + if strings.HasPrefix(vgaType, "serial") { + // Extract serial number (0-3) + serialNum := strings.TrimPrefix(vgaType, "serial") + + // Check if corresponding serial device is defined + serialList := d.Get("serial").([]interface{}) + serialFound := false + + for _, s := range serialList { + serial := s.(map[string]interface{}) + if fmt.Sprintf("%d", serial["id"].(int)) == serialNum { + serialFound = true + break + } + } + + if !serialFound { + return fmt.Errorf("vga type '%s' requires serial device with id=%s to be defined", vgaType, serialNum) + } + } + } + + return nil +} From 43122037814939886e6b02e7212204b511282246 Mon Sep 17 00:00:00 2001 From: Kamil Monticolo Date: Wed, 29 Oct 2025 19:01:55 +0100 Subject: [PATCH 2/3] Add tests --- proxmox/resource_vm_qemu_test.go | 232 +++++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+) diff --git a/proxmox/resource_vm_qemu_test.go b/proxmox/resource_vm_qemu_test.go index f57fc230..2724604a 100644 --- a/proxmox/resource_vm_qemu_test.go +++ b/proxmox/resource_vm_qemu_test.go @@ -398,3 +398,235 @@ func TestAccProxmoxVmQemu_UpdateRebootRequired(t *testing.T) { }, }) } + +func TestValidateVGAAndSerial(t *testing.T) { + tests := []struct { + name string + vgaConfig []interface{} + serialConfig []interface{} + expectError bool + errorMsg string + }{ + { + name: "valid serial0 with matching serial device", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial0", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 0, + "type": "socket", + }, + }, + expectError: false, + }, + { + name: "serial0 without matching serial device - should fail", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial0", + }, + }, + serialConfig: []interface{}{}, + expectError: true, + errorMsg: "vga type 'serial0' requires serial device with id=0 to be defined", + }, + { + name: "serial1 with matching serial device", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial1", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 1, + "type": "socket", + }, + }, + expectError: false, + }, + { + name: "serial1 without matching serial device - should fail", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial1", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 0, + "type": "socket", + }, + }, + expectError: true, + errorMsg: "vga type 'serial1' requires serial device with id=1 to be defined", + }, + { + name: "serial2 with matching serial device", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial2", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 2, + "type": "socket", + }, + }, + expectError: false, + }, + { + name: "serial3 with matching serial device", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial3", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 3, + "type": "socket", + }, + }, + expectError: false, + }, + { + name: "non-serial vga type - should pass", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "std", + }, + }, + serialConfig: []interface{}{}, + expectError: false, + }, + { + name: "cirrus vga type - should pass", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "cirrus", + }, + }, + serialConfig: []interface{}{}, + expectError: false, + }, + { + name: "vmware vga type - should pass", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "vmware", + }, + }, + serialConfig: []interface{}{}, + expectError: false, + }, + { + name: "no vga config - should pass", + vgaConfig: []interface{}{}, + serialConfig: []interface{}{}, + expectError: false, + }, + { + name: "serial0 with multiple serial devices including correct one", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial0", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 1, + "type": "socket", + }, + map[string]interface{}{ + "id": 0, + "type": "socket", + }, + map[string]interface{}{ + "id": 2, + "type": "socket", + }, + }, + expectError: false, + }, + { + name: "serial0 with multiple serial devices but no matching id", + vgaConfig: []interface{}{ + map[string]interface{}{ + "type": "serial0", + }, + }, + serialConfig: []interface{}{ + map[string]interface{}{ + "id": 1, + "type": "socket", + }, + map[string]interface{}{ + "id": 2, + "type": "socket", + }, + }, + expectError: true, + errorMsg: "vga type 'serial0' requires serial device with id=0 to be defined", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock ResourceData + d := schema.TestResourceDataRaw(t, map[string]*schema.Schema{ + "vga": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "type": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + "serial": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "id": { + Type: schema.TypeInt, + Required: true, + }, + "type": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, map[string]interface{}{ + "vga": tt.vgaConfig, + "serial": tt.serialConfig, + }) + + // Call the validation function + err := validateVGAAndSerial(d) + + // Check the result + if tt.expectError { + if err == nil { + t.Errorf("expected error but got none") + } else if tt.errorMsg != "" && err.Error() != tt.errorMsg { + t.Errorf("expected error message '%s' but got '%s'", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + } + }) + } +} From 7c95a7a7d76b56c20d27742b8b57ce55e78d8f1c Mon Sep 17 00:00:00 2001 From: Kamil Monticolo Date: Sat, 7 Feb 2026 13:48:22 +0100 Subject: [PATCH 3/3] Fix validateVGAAndSerial panic and validation logic --- proxmox/resource_vm_qemu.go | 92 +++++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 40 deletions(-) mode change 100755 => 100644 proxmox/resource_vm_qemu.go diff --git a/proxmox/resource_vm_qemu.go b/proxmox/resource_vm_qemu.go old mode 100755 new mode 100644 index b1fc2c82..cb6bb13b --- a/proxmox/resource_vm_qemu.go +++ b/proxmox/resource_vm_qemu.go @@ -89,6 +89,7 @@ func resourceVmQemu() *schema.Resource { }, ), reboot.CustomizeDiff(), + validateVGAAndSerialDiff, ), Schema: map[string]*schema.Schema{ @@ -612,10 +613,6 @@ func resourceVmQemuCreate(ctx context.Context, d *schema.ResourceData, meta inte config.EFIDisk = qemuEfiDisks[0] } - if err := validateVGAAndSerial(d); err != nil { - return diag.FromErr(err) - } - var vmr *pveSDK.VmRef if guestID := vmID.Get(d); guestID != 0 { // Manually set vmID log.Print("[DEBUG][QemuVmCreate] checking if vmId: " + guestID.String() + " already exists") @@ -875,10 +872,6 @@ func resourceVmQemuUpdate(ctx context.Context, d *schema.ResourceData, meta inte return diags } - if err := validateVGAAndSerial(d); err != nil { - return diag.FromErr(err) - } - logger.Debug().Int(vmID.Root, int(resourceID.ID)).Msgf("Updating VM with the following configuration: %+v", config) var rebootRequired bool @@ -1530,36 +1523,55 @@ func mapToSDK_QemuGuestAgent(d *schema.ResourceData) *pveSDK.QemuGuestAgent { } } -func validateVGAAndSerial(d *schema.ResourceData) error { - // Get VGA configuration - vgaList := d.Get("vga").([]interface{}) - - if len(vgaList) > 0 { - vga := vgaList[0].(map[string]interface{}) - vgaType := vga["type"].(string) - - // Check if vga type is serial0, serial1, serial2, or serial3 - if strings.HasPrefix(vgaType, "serial") { - // Extract serial number (0-3) - serialNum := strings.TrimPrefix(vgaType, "serial") - - // Check if corresponding serial device is defined - serialList := d.Get("serial").([]interface{}) - serialFound := false - - for _, s := range serialList { - serial := s.(map[string]interface{}) - if fmt.Sprintf("%d", serial["id"].(int)) == serialNum { - serialFound = true - break - } - } - - if !serialFound { - return fmt.Errorf("vga type '%s' requires serial device with id=%s to be defined", vgaType, serialNum) - } - } - } - - return nil +// validateVGAAndSerialDiff is a CustomizeDiff function that validates VGA and Serial +// configuration during terraform plan phase. +func validateVGAAndSerialDiff(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + // Safely get VGA configuration + vgaRaw, vgaExists := d.GetOk("vga") + if !vgaExists { + return nil // No VGA configured, validation not needed + } + + // VGA is stored as a Set in the schema + vgaSet, ok := vgaRaw.(*schema.Set) + if !ok { + return nil // Invalid type, skip validation + } + + vgaList := vgaSet.List() + if len(vgaList) == 0 { + return nil // Empty VGA list, validation not needed + } + + // Extract VGA configuration + vgaMap, ok := vgaList[0].(map[string]interface{}) + if !ok { + return nil // Invalid format, skip validation + } + + // Check if VGA type is "serial" + vgaType, ok := vgaMap["type"].(string) + if !ok || vgaType != "serial" { + return nil // Not serial type, validation not needed + } + + // VGA type is "serial", now validate that serial devices exist + serialRaw, serialExists := d.GetOk("serial") + if !serialExists { + return fmt.Errorf("when vga.type is set to 'serial', at least one serial device must be configured in the 'serial' block") + } + + // Serial is stored as a Set in the schema + serialSet, ok := serialRaw.(*schema.Set) + if !ok || serialSet.Len() == 0 { + return fmt.Errorf("when vga.type is set to 'serial', at least one serial device must be configured in the 'serial' block") + } + + // Verify at least one serial device is properly configured + serialList := serialSet.List() + if len(serialList) == 0 { + return fmt.Errorf("when vga.type is set to 'serial', at least one serial device must be configured in the 'serial' block") + } + + return nil }