fix(vm,lxc): add name validation (#2631)

Signed-off-by: Stanislav Shamilov <shamilovstas@protonmail.com>
Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
Co-authored-by: Pavel Boldyrev <pavel@bpg.sh>

Author: shamilovstas

docs/resources/virtual_environment_container.md

@@ -165,7 +165,7 @@ output "ubuntu_container_public_key" {
             The `server` attribute is deprecated and will be removed in a future release. Please use
             the `servers` attribute instead.
         - `servers` - (Optional) The list of DNS servers.
-    - `hostname` - (Optional) The hostname.
+    - `hostname` - (Optional) The hostname. Must be a valid DNS name.
     - `ip_config` - (Optional) The IP configuration (one block per network
         device).
         - `ipv4` - (Optional) The IPv4 configuration.

docs/resources/virtual_environment_vm.md

@@ -483,7 +483,7 @@ output "ubuntu_vm_public_key" {
 
 - `migrate` - (Optional) Migrate the VM on node change instead of re-creating
     it (defaults to `false`).
-- `name` - (Optional) The virtual machine name.
+- `name` - (Optional) The virtual machine name. Must be a valid DNS name.
 - `network_device` - (Optional) A network device (multiple blocks supported).
     - `bridge` - (Optional) The name of the network bridge (defaults to `vmbr0`).
     - `disconnected` - (Optional) Whether to disconnect the network device from the network (defaults to `false`).

fwprovider/nodes/vm/resource_schema.go

@@ -60,7 +60,7 @@ func (r *Resource) Schema(
 				Optional:            true,
 				Validators: []validator.String{
 					stringvalidator.RegexMatches(
-						regexp.MustCompile(`^([a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9])$`),
+						regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`),
 						"must be a valid DNS name",
 					),
 				},

fwprovider/nodes/vm/resource_test.go

@@ -59,6 +59,16 @@ func TestAccResourceVM(t *testing.T) {
 			}`),
 			ExpectError: regexp.MustCompile(`name must be a valid DNS name`),
 		}}},
+		{"set a FQDN VM name", []resource.TestStep{{
+			Config: te.RenderConfig(`
+			resource "proxmox_virtual_environment_vm2" "test_vm" {
+				node_name = "{{.NodeName}}"
+				name = "vm.example.com"
+			}`),
+			Check: test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{
+				"name": "vm.example.com",
+			}),
+		}}},
 		{"set, update, import with primitive fields", []resource.TestStep{
 			{
 				Config: te.RenderConfig(`

fwprovider/test/resource_container_test.go

@@ -12,6 +12,7 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -883,6 +884,18 @@ func TestAccResourceContainerHostname(t *testing.T) {
 	resource.ParallelTest(t, resource.TestCase{
 		ProtoV6ProviderFactories: te.AccProviders,
 		Steps: []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+					resource "proxmox_virtual_environment_container" "test_container" {
+					    node_name = "pve"
+					    initialization {
+					      hostname = ".world"
+					    }
+					}
+				`),
+				ExpectError: regexp.MustCompile(`invalid value for hostname \(must be a valid DNS name\)`),
+				PlanOnly:    true,
+			},
 			{
 				Config: te.RenderConfig(`
 				resource "proxmox_virtual_environment_container" "test_container" {

fwprovider/test/resource_vm_test.go

@@ -88,6 +88,15 @@ func TestAccResourceVM(t *testing.T) {
 				}`),
 			ExpectError: regexp.MustCompile(`expected "node_name" to not be an empty string, got `),
 		}}},
+		{"name", []resource.TestStep{{
+			Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm" "vm" {
+				    node_name = "pve"
+				    name = ".hello"
+				}`),
+			ExpectError: regexp.MustCompile(`invalid value for name \(must be a valid DNS name\)`),
+			PlanOnly:    true,
+		}}},
 		{"protection", []resource.TestStep{
 			{
 				Config: te.RenderConfig(`

proxmoxtf/resource/container/container.go

@@ -519,10 +519,11 @@ func Container() *schema.Resource {
 							DiffSuppressFunc: skipDnsDiffIfEmpty,
 						},
 						mkInitializationHostname: {
-							Type:        schema.TypeString,
-							Description: "The hostname",
-							Optional:    true,
-							Default:     dvInitializationHostname,
+							Type:             schema.TypeString,
+							Description:      "The hostname. Must be a valid DNS name.",
+							Optional:         true,
+							Default:          dvInitializationHostname,
+							ValidateDiagFunc: resource.HostnameValidator(),
 						},
 						mkInitializationIPConfig: {
 							Type:        schema.TypeList,

proxmoxtf/resource/vm/validators.go

@@ -403,3 +403,18 @@ func RangeSemicolonValidator() schema.SchemaValidateDiagFunc {
 		),
 	)
 }
+
+// HostnameValidator is a scheme validation function for virtual machine name. According to Proxmox documentation,
+// it must be a valid DNS name.
+// Regexp is based on Proxmox validation at https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm
+func HostnameValidator() schema.SchemaValidateDiagFunc {
+	return validation.ToDiagFunc(
+		validation.Any(
+			validation.StringIsEmpty,
+			validation.StringMatch(
+				regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`),
+				"must be a valid DNS name",
+			),
+		),
+	)
+}

proxmoxtf/resource/vm/validators_test.go

@@ -9,6 +9,7 @@ package resource
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -77,3 +78,34 @@ func TestMachineType(t *testing.T) {
 		})
 	}
 }
+
+func TestVmHostname(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		value string
+		valid bool
+	}{
+		{"empty", "", true},
+		{"underscores", "my_name", false},
+		{"trailing dot", "my-name.com.", false},
+		{"starts with alphanumeric", "-my-name.com", false},
+		{"ends with alphanumeric", "my-name.com!", false},
+		{"single letter", "a", true},
+		{"domain name", "my-name.com", true},
+		{"multi domain", "my-name.com.edu.net.xyz.dev", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			f := HostnameValidator()
+			res := f(tt.value, nil)
+
+			valid := !res.HasError()
+			assert.Equal(t, tt.valid, valid)
+		})
+	}
+}

proxmoxtf/resource/vm/vm.go

@@ -1210,10 +1210,11 @@ func VM() *schema.Resource {
 			MinItems: 0,
 		},
 		mkName: {
-			Type:        schema.TypeString,
-			Description: "The name",
-			Optional:    true,
-			Default:     dvName,
+			Type:             schema.TypeString,
+			Description:      "The name of the VM. Must be a valid DNS name.",
+			Optional:         true,
+			Default:          dvName,
+			ValidateDiagFunc: HostnameValidator(),
 		},
 		mkNodeName: {
 			Type:         schema.TypeString,