Skip to content

Commit

Permalink
VM and VMSS validation fixes (#6639)
Browse files Browse the repository at this point in the history 
Separate validation for VM (resource) name, and linux/windows computer names

- Adjust naming rules to meet current API behavior
- Length validation for VMSS computer name prefixes
- Test cases for new validation functions
  • Loading branch information
@manicminer
manicminer committed Apr 30, 2020
1 parent ae12fa6 commit 15aac6a
Show file tree
Hide file tree
Showing 17 changed files with 570 additions and 105 deletions.
11 changes: 7 additions & 4 deletions azurerm/internal/services/compute/linux_virtual_machine_resource.go
View file
Expand Up @@ -51,7 +51,7 @@ func resourceLinuxVirtualMachine() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: ValidateLinuxName,
ValidateFunc: ValidateVmName,
},

"resource_group_name": azure.SchemaResourceGroupName(),
Expand Down Expand Up @@ -128,9 +128,8 @@ func resourceLinuxVirtualMachine() *schema.Resource {
// Computed since we reuse the VM name if one's not specified
Computed: true,
ForceNew: true,
// note: whilst the portal says 1-15 characters it seems to mirror the rules for the vm name
// (e.g. 1-15 for Windows, 1-63 for Linux)
ValidateFunc: ValidateLinuxName,

ValidateFunc: ValidateLinuxComputerNameFull,
},

"custom_data": base64.OptionalSchema(true),
Expand Down Expand Up @@ -295,6 +294,10 @@ func resourceLinuxVirtualMachineCreate(d *schema.ResourceData, meta interface{})
if v, ok := d.GetOk("computer_name"); ok && len(v.(string)) > 0 {
computerName = v.(string)
} else {
_, errs := ValidateLinuxComputerNameFull(d.Get("name"), "computer_name")
if len(errs) > 0 {
return fmt.Errorf("unable to assume default computer name %s Please adjust the %q, or specify an explicit %q", errs[0], "name", "computer_name")
}
computerName = name
}
disablePasswordAuthentication := d.Get("disable_password_authentication").(bool)
Expand Down
11 changes: 7 additions & 4 deletions azurerm/internal/services/compute/resource_arm_linux_virtual_machine_scale_set.go
View file
Expand Up @@ -49,7 +49,7 @@ func resourceArmLinuxVirtualMachineScaleSet() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: ValidateLinuxName,
ValidateFunc: ValidateVmName,
},

"resource_group_name": azure.SchemaResourceGroupName(),
Expand Down Expand Up @@ -106,9 +106,8 @@ func resourceArmLinuxVirtualMachineScaleSet() *schema.Resource {
// Computed since we reuse the VM name if one's not specified
Computed: true,
ForceNew: true,
// note: whilst the portal says 1-15 characters it seems to mirror the rules for the vm name
// (e.g. 1-15 for Windows, 1-63 for Linux)
ValidateFunc: ValidateLinuxName,

ValidateFunc: ValidateLinuxComputerNamePrefix,
},

"custom_data": base64.OptionalSchema(false),
Expand Down Expand Up @@ -352,6 +351,10 @@ func resourceArmLinuxVirtualMachineScaleSetCreate(d *schema.ResourceData, meta i
if v, ok := d.GetOk("computer_name_prefix"); ok && len(v.(string)) > 0 {
computerNamePrefix = v.(string)
} else {
_, errs := ValidateLinuxComputerNamePrefix(d.Get("name"), "computer_name_prefix")
if len(errs) > 0 {
return fmt.Errorf("unable to assume default computer name prefix %s. Please adjust the %q, or specify an explicit %q", errs[0], "name", "computer_name_prefix")
}
computerNamePrefix = name
}

Expand Down
11 changes: 7 additions & 4 deletions azurerm/internal/services/compute/resource_arm_windows_virtual_machine_scale_set.go
View file
Expand Up @@ -49,7 +49,7 @@ func resourceArmWindowsVirtualMachineScaleSet() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: ValidateWindowsName,
ValidateFunc: ValidateVmName,
},

"resource_group_name": azure.SchemaResourceGroupName(),
Expand Down Expand Up @@ -107,9 +107,8 @@ func resourceArmWindowsVirtualMachineScaleSet() *schema.Resource {
// Computed since we reuse the VM name if one's not specified
Computed: true,
ForceNew: true,
// note: whilst the portal says 1-15 characters it seems to mirror the rules for the vm name
// (e.g. 1-15 for Windows, 1-63 for Windows)
ValidateFunc: ValidateWindowsName,

ValidateFunc: ValidateWindowsComputerNamePrefix,
},

"custom_data": base64.OptionalSchema(false),
Expand Down Expand Up @@ -371,6 +370,10 @@ func resourceArmWindowsVirtualMachineScaleSetCreate(d *schema.ResourceData, meta
if v, ok := d.GetOk("computer_name_prefix"); ok && len(v.(string)) > 0 {
computerNamePrefix = v.(string)
} else {
_, errs := ValidateWindowsComputerNamePrefix(d.Get("name"), "computer_name_prefix")
if len(errs) > 0 {
return fmt.Errorf("unable to assume default computer name prefix %s. Please adjust the %q, or specify an explicit %q", errs[0], "name", "computer_name_prefix")
}
computerNamePrefix = name
}

Expand Down
56 changes: 54 additions & 2 deletions azurerm/internal/services/compute/tests/linux_virtual_machine_resource_other_test.go
View file
Expand Up @@ -2,6 +2,7 @@ package tests

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
Expand Down Expand Up @@ -104,6 +105,22 @@ func TestAccLinuxVirtualMachine_otherComputerNameDefault(t *testing.T) {
})
}

func TestAccLinuxVirtualMachine_otherComputerNameDefaultInvalid(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine", "test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acceptance.PreCheck(t) },
Providers: acceptance.SupportedProviders,
CheckDestroy: checkLinuxVirtualMachineIsDestroyed,
Steps: []resource.TestStep{
{
Config: testLinuxVirtualMachine_otherComputerNameDefaultInvalid(data),
ExpectError: regexp.MustCompile("unable to assume default computer name"),
},
},
})
}

func TestAccLinuxVirtualMachine_otherComputerNameCustom(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine", "test")

Expand All @@ -116,7 +133,7 @@ func TestAccLinuxVirtualMachine_otherComputerNameCustom(t *testing.T) {
Config: testLinuxVirtualMachine_otherComputerNameCustom(data),
Check: resource.ComposeTestCheckFunc(
checkLinuxVirtualMachineExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "computer_name", "custom123"),
resource.TestCheckResourceAttr(data.ResourceName, "computer_name", "custom-linux-hostname-123"),
),
},
data.ImportStep(),
Expand Down Expand Up @@ -504,6 +521,41 @@ resource "azurerm_linux_virtual_machine" "test" {
`, template, data.RandomInteger)
}

func testLinuxVirtualMachine_otherComputerNameDefaultInvalid(data acceptance.TestData) string {
template := testLinuxVirtualMachine_template(data)
return fmt.Sprintf(`
%s
resource "azurerm_linux_virtual_machine" "test" {
name = "acctestVM-this-name-too-long-to-be-a-linux-vm-computer-name-1234567890"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
size = "Standard_F2"
admin_username = "adminuser"
network_interface_ids = [
azurerm_network_interface.test.id,
]
admin_ssh_key {
username = "adminuser"
public_key = local.first_public_key
}
os_disk {
caching = "ReadWrite"
storage_account_type = "Standard_LRS"
}
source_image_reference {
publisher = "Canonical"
offer = "UbuntuServer"
sku = "16.04-LTS"
version = "latest"
}
}
`, template)
}

func testLinuxVirtualMachine_otherComputerNameCustom(data acceptance.TestData) string {
template := testLinuxVirtualMachine_template(data)
return fmt.Sprintf(`
Expand All @@ -515,7 +567,7 @@ resource "azurerm_linux_virtual_machine" "test" {
location = azurerm_resource_group.test.location
size = "Standard_F2"
admin_username = "adminuser"
computer_name = "custom123"
computer_name = "custom-linux-hostname-123"
network_interface_ids = [
azurerm_network_interface.test.id,
]
Expand Down
61 changes: 60 additions & 1 deletion ...nternal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_other_test.go
View file
Expand Up @@ -2,6 +2,7 @@ package tests

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
Expand Down Expand Up @@ -71,6 +72,22 @@ func TestAccAzureRMLinuxVirtualMachineScaleSet_otherComputerNamePrefix(t *testin
})
}

func TestAccAzureRMLinuxVirtualMachineScaleSet_otherComputerNamePrefixInvalid(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine_scale_set", "test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acceptance.PreCheck(t) },
Providers: acceptance.SupportedProviders,
CheckDestroy: testCheckAzureRMLinuxVirtualMachineScaleSetDestroy,
Steps: []resource.TestStep{
{
Config: testAccAzureRMLinuxVirtualMachineScaleSet_otherComputerNamePrefixInvalid(data),
ExpectError: regexp.MustCompile("unable to assume default computer name prefix"),
},
},
})
}

func TestAccAzureRMLinuxVirtualMachineScaleSet_otherCustomData(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine_scale_set", "test")

Expand Down Expand Up @@ -629,7 +646,49 @@ resource "azurerm_linux_virtual_machine_scale_set" "test" {
instances = 1
admin_username = "adminuser"
admin_password = "P@ssword1234!"
computer_name_prefix = "morty"
computer_name_prefix = "my-linux-computer-name-prefix"
disable_password_authentication = false
source_image_reference {
publisher = "Canonical"
offer = "UbuntuServer"
sku = "16.04-LTS"
version = "latest"
}
os_disk {
storage_account_type = "Standard_LRS"
caching = "ReadWrite"
}
network_interface {
name = "example"
primary = true
ip_configuration {
name = "internal"
primary = true
subnet_id = azurerm_subnet.test.id
}
}
}
`, template, data.RandomInteger)
}

func testAccAzureRMLinuxVirtualMachineScaleSet_otherComputerNamePrefixInvalid(data acceptance.TestData) string {
template := testAccAzureRMLinuxVirtualMachineScaleSet_template(data)
return fmt.Sprintf(`
%s
resource "azurerm_linux_virtual_machine_scale_set" "test" {
name = "acctestvmss-%d-too-long-to-be-a-computer-name-but-not-vmss-name"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "Standard_F2"
instances = 1
admin_username = "adminuser"
admin_password = "P@ssword1234!"
disable_password_authentication = false
Expand Down
5 changes: 2 additions & 3 deletions ...l/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_disk_data_test.go
View file
Expand Up @@ -479,10 +479,9 @@ func testAccAzureRMWindowsVirtualMachineScaleSet_disksDataDisk_diskEncryptionSet
// TODO: switch back to default location
location := "westus2"

name := testAccAzureRMWindowsVirtualMachineScaleSet_vmName(data)
return fmt.Sprintf(`
locals {
vm_name = "%s"
vm_name = "acctestVM-%d"
}
data "azurerm_client_config" "current" {}
Expand Down Expand Up @@ -550,7 +549,7 @@ resource "azurerm_subnet" "test" {
virtual_network_name = azurerm_virtual_network.test.name
address_prefix = "10.0.2.0/24"
}
`, name, data.RandomInteger, location, data.RandomString, data.RandomInteger)
`, data.RandomInteger, data.RandomInteger, location, data.RandomString, data.RandomInteger)
}

func testAccAzureRMWindowsVirtualMachineScaleSet_disksDataDisk_diskEncryptionSetResource(data acceptance.TestData) string {
Expand Down
5 changes: 2 additions & 3 deletions ...nal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_disk_os_test.go
View file
Expand Up @@ -302,10 +302,9 @@ func testAccAzureRMWindowsVirtualMachineScaleSet_disksOSDisk_diskEncryptionSetDe
// TODO: switch back to default location
location := "westus2"

name := testAccAzureRMWindowsVirtualMachineScaleSet_vmName(data)
return fmt.Sprintf(`
locals {
vm_name = "%s"
vm_name = "acctestVM-%d"
}
data "azurerm_client_config" "current" {}
Expand Down Expand Up @@ -373,7 +372,7 @@ resource "azurerm_subnet" "test" {
virtual_network_name = azurerm_virtual_network.test.name
address_prefix = "10.0.2.0/24"
}
`, name, data.RandomInteger, location, data.RandomString, data.RandomInteger)
`, data.RandomInteger, data.RandomInteger, location, data.RandomString, data.RandomInteger)
}

func testAccAzureRMWindowsVirtualMachineScaleSet_disksOSDisk_diskEncryptionSetResource(data acceptance.TestData) string {
Expand Down
5 changes: 2 additions & 3 deletions ...nal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_network_test.go
View file
Expand Up @@ -621,7 +621,6 @@ resource "azurerm_windows_virtual_machine_scale_set" "test" {
}

func testAccAzureRMWindowsVirtualMachineScaleSet_networkApplicationGateway(data acceptance.TestData) string {
name := testAccAzureRMWindowsVirtualMachineScaleSet_vmName(data)
return fmt.Sprintf(`
provider "azurerm" {
features {}
Expand Down Expand Up @@ -718,7 +717,7 @@ resource "azurerm_application_gateway" "test" {
}
locals {
vm_name = "%s"
vm_name = "acctestVM-%d"
}
resource "azurerm_subnet" "other" {
Expand Down Expand Up @@ -761,7 +760,7 @@ resource "azurerm_windows_virtual_machine_scale_set" "test" {
}
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, name)
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger)
}

func testAccAzureRMWindowsVirtualMachineScaleSet_networkApplicationSecurityGroup(data acceptance.TestData) string {
Expand Down

0 comments on commit 15aac6a

Please sign in to comment.