From b48bd4b2ad34d1c28e612d635d29339cce2564b9 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 8 Apr 2020 16:48:55 +0800 Subject: [PATCH 1/9] Refine VM ID parse functions --- .../compute/linux_virtual_machine_resource.go | 8 +- .../compute/resource_arm_managed_disk.go | 3 +- .../resource_arm_virtual_machine_extension.go | 6 +- .../linux_virtual_machine_resource_test.go | 19 ++-- .../windows_virtual_machine_resource_test.go | 19 ++-- .../compute/validate/virtual_machine.go | 22 +++++ .../services/compute/virtual_machine.go | 42 -------- .../compute/virtual_machine_import.go | 3 +- .../services/compute/virtual_machine_test.go | 98 ------------------- .../windows_virtual_machine_resource.go | 8 +- 10 files changed, 62 insertions(+), 166 deletions(-) create mode 100644 azurerm/internal/services/compute/validate/virtual_machine.go delete mode 100644 azurerm/internal/services/compute/virtual_machine_test.go diff --git a/azurerm/internal/services/compute/linux_virtual_machine_resource.go b/azurerm/internal/services/compute/linux_virtual_machine_resource.go index 68845c87298b..ae46a485f764 100644 --- a/azurerm/internal/services/compute/linux_virtual_machine_resource.go +++ b/azurerm/internal/services/compute/linux_virtual_machine_resource.go @@ -35,7 +35,7 @@ func resourceLinuxVirtualMachine() *schema.Resource { Update: resourceLinuxVirtualMachineUpdate, Delete: resourceLinuxVirtualMachineDelete, Importer: azSchema.ValidateResourceIDPriorToImportThen(func(id string) error { - _, err := ParseVirtualMachineID(id) + _, err := parse.VirtualMachineID(id) return err }, importVirtualMachine(compute.Linux, "azurerm_linux_virtual_machine")), @@ -471,7 +471,7 @@ func resourceLinuxVirtualMachineRead(d *schema.ResourceData, meta interface{}) e ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return err } @@ -626,7 +626,7 @@ func resourceLinuxVirtualMachineUpdate(d *schema.ResourceData, meta interface{}) ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return err } @@ -930,7 +930,7 @@ func resourceLinuxVirtualMachineDelete(d *schema.ResourceData, meta interface{}) ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return err } diff --git a/azurerm/internal/services/compute/resource_arm_managed_disk.go b/azurerm/internal/services/compute/resource_arm_managed_disk.go index aa04b66ec2c1..13f5083f9b81 100644 --- a/azurerm/internal/services/compute/resource_arm_managed_disk.go +++ b/azurerm/internal/services/compute/resource_arm_managed_disk.go @@ -16,6 +16,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/locks" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -383,7 +384,7 @@ func resourceArmManagedDiskUpdate(d *schema.ResourceData, meta interface{}) erro // if we are attached to a VM we bring down the VM as necessary for the operations which are not allowed while it's online if shouldShutDown { - virtualMachine, err := ParseVirtualMachineID(*disk.ManagedBy) + virtualMachine, err := parse.VirtualMachineID(*disk.ManagedBy) if err != nil { return fmt.Errorf("Error parsing VMID %q for disk attachment: %+v", *disk.ManagedBy, err) } diff --git a/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go b/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go index 285a95c17010..16d1a97dfd19 100644 --- a/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go +++ b/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go @@ -11,6 +11,8 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -44,7 +46,7 @@ func resourceArmVirtualMachineExtension() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: ValidateVirtualMachineID, + ValidateFunc: validate.VirtualMachineID, }, "publisher": { @@ -95,7 +97,7 @@ func resourceArmVirtualMachineExtensionsCreateUpdate(d *schema.ResourceData, met defer cancel() name := d.Get("name").(string) - virtualMachineId, err := ParseVirtualMachineID(d.Get("virtual_machine_id").(string)) + virtualMachineId, err := parse.VirtualMachineID(d.Get("virtual_machine_id").(string)) if err != nil { return fmt.Errorf("Error parsing Virtual Machine ID %q: %+v", virtualMachineId, err) } diff --git a/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go b/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go index def8c85afb68..ec2fea371770 100644 --- a/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go +++ b/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -16,12 +17,14 @@ func checkLinuxVirtualMachineIsDestroyed(s *terraform.State) error { continue } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineID(rs.Primary.ID) + if err != nil { + return err + } client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext - resp, err := client.Get(ctx, resourceGroup, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if !utils.ResponseWasNotFound(resp.Response) { return err @@ -41,15 +44,17 @@ func checkLinuxVirtualMachineExists(resourceName string) resource.TestCheckFunc return fmt.Errorf("Not found: %s", resourceName) } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineID(rs.Primary.ID) + if err != nil { + return err + } client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext - resp, err := client.Get(ctx, resourceGroup, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Bad: Linux Virtual Machine %q (Resource Group: %q) does not exist", name, resourceGroup) + return fmt.Errorf("Bad: Linux Virtual Machine %q (Resource Group: %q) does not exist", id.Name, id.ResourceGroup) } return fmt.Errorf("Bad: Get on VMClient: %+v", err) diff --git a/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go b/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go index bb018167d5d7..bdd83c2bb44d 100644 --- a/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go +++ b/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -16,12 +17,14 @@ func checkWindowsVirtualMachineIsDestroyed(s *terraform.State) error { continue } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineID(rs.Primary.ID) + if err != nil { + return err + } client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext - resp, err := client.Get(ctx, resourceGroup, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if !utils.ResponseWasNotFound(resp.Response) { return err @@ -41,15 +44,17 @@ func checkWindowsVirtualMachineExists(resourceName string) resource.TestCheckFun return fmt.Errorf("Not found: %s", resourceName) } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineID(rs.Primary.ID) + if err != nil { + return err + } client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext - resp, err := client.Get(ctx, resourceGroup, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Bad: Windows Virtual Machine %q (Resource Group: %q) does not exist", name, resourceGroup) + return fmt.Errorf("Bad: Windows Virtual Machine %q (Resource Group: %q) does not exist", id.Name, id.ResourceGroup) } return fmt.Errorf("Bad: Get on VMClient: %+v", err) diff --git a/azurerm/internal/services/compute/validate/virtual_machine.go b/azurerm/internal/services/compute/validate/virtual_machine.go new file mode 100644 index 000000000000..d23b7abede9a --- /dev/null +++ b/azurerm/internal/services/compute/validate/virtual_machine.go @@ -0,0 +1,22 @@ +package validate + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" +) + +func VirtualMachineID(i interface{}, k string) (warnings []string, errors []error) { + v, ok := i.(string) + if !ok { + errors = append(errors, fmt.Errorf("expected type of %q to be string", k)) + return + } + + if _, err := parse.VirtualMachineID(v); err != nil { + errors = append(errors, fmt.Errorf("Can not parse %q as a resource id: %v", k, err)) + return + } + + return warnings, errors +} diff --git a/azurerm/internal/services/compute/virtual_machine.go b/azurerm/internal/services/compute/virtual_machine.go index eaaa47aca51f..5e545d1c4a61 100644 --- a/azurerm/internal/services/compute/virtual_machine.go +++ b/azurerm/internal/services/compute/virtual_machine.go @@ -7,54 +7,12 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) -type VirtualMachineID struct { - ResourceGroup string - Name string -} - -func ParseVirtualMachineID(input string) (*VirtualMachineID, error) { - id, err := azure.ParseAzureResourceID(input) - if err != nil { - return nil, fmt.Errorf("[ERROR] Unable to parse Virtual Machine ID %q: %+v", input, err) - } - - virtualMachine := VirtualMachineID{ - ResourceGroup: id.ResourceGroup, - } - - if virtualMachine.Name, err = id.PopSegment("virtualMachines"); err != nil { - return nil, err - } - - if err := id.ValidateNoEmptySegments(input); err != nil { - return nil, err - } - - return &virtualMachine, nil -} - -func ValidateVirtualMachineID(i interface{}, k string) (warnings []string, errors []error) { - v, ok := i.(string) - if !ok { - errors = append(errors, fmt.Errorf("expected type of %q to be string", k)) - return - } - - if _, err := ParseVirtualMachineID(v); err != nil { - errors = append(errors, fmt.Errorf("Can not parse %q as a resource id: %v", k, err)) - return - } - - return warnings, errors -} - func virtualMachineAdditionalCapabilitiesSchema() *schema.Schema { return &schema.Schema{ Type: schema.TypeList, diff --git a/azurerm/internal/services/compute/virtual_machine_import.go b/azurerm/internal/services/compute/virtual_machine_import.go index a2ab6f0ee66e..21987de2aef7 100644 --- a/azurerm/internal/services/compute/virtual_machine_import.go +++ b/azurerm/internal/services/compute/virtual_machine_import.go @@ -6,12 +6,13 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" ) func importVirtualMachine(osType compute.OperatingSystemTypes, resourceType string) func(d *schema.ResourceData, meta interface{}) (data []*schema.ResourceData, err error) { return func(d *schema.ResourceData, meta interface{}) (data []*schema.ResourceData, err error) { - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return []*schema.ResourceData{}, err } diff --git a/azurerm/internal/services/compute/virtual_machine_test.go b/azurerm/internal/services/compute/virtual_machine_test.go deleted file mode 100644 index 4fcf03700d27..000000000000 --- a/azurerm/internal/services/compute/virtual_machine_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package compute - -import "testing" - -func TestParseVirtualMachineID(t *testing.T) { - testData := []struct { - Name string - Input string - Expected *VirtualMachineID - }{ - { - Name: "Empty", - Input: "", - Expected: nil, - }, - { - Name: "No virtual machine segment", - Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/", - Expected: nil, - }, - { - Name: "No virtual machine name", - Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/virtualMachines/", - Expected: nil, - }, - { - Name: "Case incorrect in path element", - Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/VirtualMachines/machine1", - Expected: nil, - }, - { - Name: "Valid", - Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/virtualMachines/machine1", - Expected: &VirtualMachineID{ - ResourceGroup: "myGroup1", - Name: "machine1", - }, - }, - } - - for _, v := range testData { - t.Logf("[DEBUG] Testing %q", v.Name) - - actual, err := ParseVirtualMachineID(v.Input) - if err != nil { - if v.Expected == nil { - continue - } - - t.Fatalf("Expected a value but got an error: %s", err) - } - - if actual.Name != v.Expected.Name { - t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name) - } - - if actual.ResourceGroup != v.Expected.ResourceGroup { - t.Fatalf("Expected %q but got %q for ResourceGroup", v.Expected.ResourceGroup, actual.ResourceGroup) - } - } -} - -func TestValidateVirtualMachineID(t *testing.T) { - cases := []struct { - ID string - Valid bool - }{ - { - ID: "", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/microsoft.compute/virtualMachines/", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/VirtualMachines/machine1", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/virtualMachines/machine1", - Valid: true, - }, - } - for _, tc := range cases { - t.Logf("[DEBUG] Testing Value %s", tc.ID) - _, errors := ValidateVirtualMachineID(tc.ID, "test") - valid := len(errors) == 0 - - if tc.Valid != valid { - t.Fatalf("Expected %t but got %t", tc.Valid, valid) - } - } -} diff --git a/azurerm/internal/services/compute/windows_virtual_machine_resource.go b/azurerm/internal/services/compute/windows_virtual_machine_resource.go index dcc53bb5ba8a..109d195fc63e 100644 --- a/azurerm/internal/services/compute/windows_virtual_machine_resource.go +++ b/azurerm/internal/services/compute/windows_virtual_machine_resource.go @@ -36,7 +36,7 @@ func resourceWindowsVirtualMachine() *schema.Resource { Update: resourceWindowsVirtualMachineUpdate, Delete: resourceWindowsVirtualMachineDelete, Importer: azSchema.ValidateResourceIDPriorToImportThen(func(id string) error { - _, err := ParseVirtualMachineID(id) + _, err := parse.VirtualMachineID(id) return err }, importVirtualMachine(compute.Windows, "azurerm_windows_virtual_machine")), @@ -494,7 +494,7 @@ func resourceWindowsVirtualMachineRead(d *schema.ResourceData, meta interface{}) ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return err } @@ -652,7 +652,7 @@ func resourceWindowsVirtualMachineUpdate(d *schema.ResourceData, meta interface{ ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return err } @@ -925,7 +925,7 @@ func resourceWindowsVirtualMachineDelete(d *schema.ResourceData, meta interface{ ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineID(d.Id()) + id, err := parse.VirtualMachineID(d.Id()) if err != nil { return err } From 4f30719db9aea5a06b61dab858dcd4a68073279f Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 8 Apr 2020 17:19:25 +0800 Subject: [PATCH 2/9] Some follow ups on VM ID refactor --- .../tests/linux_virtual_machine_resource_test.go | 10 ++++++---- .../tests/windows_virtual_machine_resource_test.go | 10 ++++++---- .../mssql/resource_arm_mssql_virtual_machine.go | 4 ++-- .../services/mssql/validate/mssql_virtual_machine.go | 4 ++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go b/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go index ec2fea371770..9c31294c8e7c 100644 --- a/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go +++ b/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_test.go @@ -13,6 +13,9 @@ import ( func checkLinuxVirtualMachineIsDestroyed(s *terraform.State) error { for _, rs := range s.RootModule().Resources { + client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + if rs.Type != "azurerm_linux_virtual_machine" { continue } @@ -22,8 +25,6 @@ func checkLinuxVirtualMachineIsDestroyed(s *terraform.State) error { return err } - client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient - ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if !utils.ResponseWasNotFound(resp.Response) { @@ -39,6 +40,9 @@ func checkLinuxVirtualMachineIsDestroyed(s *terraform.State) error { func checkLinuxVirtualMachineExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { + client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + rs, ok := s.RootModule().Resources[resourceName] if !ok { return fmt.Errorf("Not found: %s", resourceName) @@ -49,8 +53,6 @@ func checkLinuxVirtualMachineExists(resourceName string) resource.TestCheckFunc return err } - client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient - ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { diff --git a/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go b/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go index bdd83c2bb44d..16c84447e343 100644 --- a/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go +++ b/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_test.go @@ -13,6 +13,9 @@ import ( func checkWindowsVirtualMachineIsDestroyed(s *terraform.State) error { for _, rs := range s.RootModule().Resources { + client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + if rs.Type != "azurerm_windows_virtual_machine" { continue } @@ -22,8 +25,6 @@ func checkWindowsVirtualMachineIsDestroyed(s *terraform.State) error { return err } - client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient - ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if !utils.ResponseWasNotFound(resp.Response) { @@ -39,6 +40,9 @@ func checkWindowsVirtualMachineIsDestroyed(s *terraform.State) error { func checkWindowsVirtualMachineExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { + client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + rs, ok := s.RootModule().Resources[resourceName] if !ok { return fmt.Errorf("Not found: %s", resourceName) @@ -49,8 +53,6 @@ func checkWindowsVirtualMachineExists(resourceName string) resource.TestCheckFun return err } - client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.VMClient - ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { diff --git a/azurerm/internal/services/mssql/resource_arm_mssql_virtual_machine.go b/azurerm/internal/services/mssql/resource_arm_mssql_virtual_machine.go index 85771b252bec..fc9a36d79169 100644 --- a/azurerm/internal/services/mssql/resource_arm_mssql_virtual_machine.go +++ b/azurerm/internal/services/mssql/resource_arm_mssql_virtual_machine.go @@ -13,7 +13,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute" + parseCompute "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/mssql/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/mssql/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" @@ -184,7 +184,7 @@ func resourceArmMsSqlVirtualMachineCreateUpdate(d *schema.ResourceData, meta int defer cancel() vmId := d.Get("virtual_machine_id").(string) - id, err := compute.ParseVirtualMachineID(vmId) + id, err := parseCompute.VirtualMachineID(vmId) if err != nil { return err } diff --git a/azurerm/internal/services/mssql/validate/mssql_virtual_machine.go b/azurerm/internal/services/mssql/validate/mssql_virtual_machine.go index 67ac85d9249c..75f8160b7d42 100644 --- a/azurerm/internal/services/mssql/validate/mssql_virtual_machine.go +++ b/azurerm/internal/services/mssql/validate/mssql_virtual_machine.go @@ -4,7 +4,7 @@ import ( "fmt" "regexp" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" ) func VMID(i interface{}, k string) (warnings []string, errors []error) { @@ -14,7 +14,7 @@ func VMID(i interface{}, k string) (warnings []string, errors []error) { return } - if _, err := compute.ParseVirtualMachineID(v); err != nil { + if _, err := parse.VirtualMachineID(v); err != nil { errors = append(errors, fmt.Errorf("Can not parse %q as a virtual machine id: %v", k, err)) } From 5658d03ec87b4856d37205b41531dbd46e637e59 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 8 Apr 2020 17:19:52 +0800 Subject: [PATCH 3/9] VMSS and extension refactor --- .../parse/virtual_machine_scale_set.go | 33 +++++++++++++++++++ .../virtual_machine_scale_set_extension.go | 8 ++--- ...irtual_machine_scale_set_extension_test.go | 8 ++--- .../vritual_machine_scale_set_test.go} | 10 +++--- ...rce_arm_linux_virtual_machine_scale_set.go | 19 ++++++----- ...arm_virtual_machine_scale_set_extension.go | 14 ++++---- ...e_arm_windows_virtual_machine_scale_set.go | 9 ++--- ...rm_linux_virtual_machine_scale_set_test.go | 19 +++++++---- ..._windows_virtual_machine_scale_set_test.go | 19 +++++++---- .../validate/virtual_machine_scale_set.go | 28 ++++++++++++++++ .../internal/services/compute/validation.go | 21 ------------ .../compute/virtual_machine_scale_set.go | 26 --------------- .../virtual_machine_scale_set_import.go | 3 +- .../virtual_machine_scale_set_update.go | 3 +- 14 files changed, 125 insertions(+), 95 deletions(-) create mode 100644 azurerm/internal/services/compute/parse/virtual_machine_scale_set.go rename azurerm/internal/services/compute/{ => parse}/virtual_machine_scale_set_extension.go (74%) rename azurerm/internal/services/compute/{ => parse}/virtual_machine_scale_set_extension_test.go (89%) rename azurerm/internal/services/compute/{virtual_machine_scale_set_test.go => parse/vritual_machine_scale_set_test.go} (84%) create mode 100644 azurerm/internal/services/compute/validate/virtual_machine_scale_set.go diff --git a/azurerm/internal/services/compute/parse/virtual_machine_scale_set.go b/azurerm/internal/services/compute/parse/virtual_machine_scale_set.go new file mode 100644 index 000000000000..afc325d5a537 --- /dev/null +++ b/azurerm/internal/services/compute/parse/virtual_machine_scale_set.go @@ -0,0 +1,33 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type VirtualMachineScaleSetId struct { + ResourceGroup string + Name string +} + +func VirtualMachineScaleSetID(input string) (*VirtualMachineScaleSetId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("[ERROR] Unable to parse Virtual Machine Scale Set ID %q: %+v", input, err) + } + + vmScaleSet := VirtualMachineScaleSetId{ + ResourceGroup: id.ResourceGroup, + } + + if vmScaleSet.Name, err = id.PopSegment("virtualMachineScaleSets"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &vmScaleSet, nil +} diff --git a/azurerm/internal/services/compute/virtual_machine_scale_set_extension.go b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go similarity index 74% rename from azurerm/internal/services/compute/virtual_machine_scale_set_extension.go rename to azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go index ec721364d0ff..346196ae7f84 100644 --- a/azurerm/internal/services/compute/virtual_machine_scale_set_extension.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go @@ -1,4 +1,4 @@ -package compute +package parse import ( "fmt" @@ -6,19 +6,19 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" ) -type VirtualMachineScaleSetExtensionResourceID struct { +type VirtualMachineScaleSetExtensionId struct { ResourceGroup string VirtualMachineName string Name string } -func ParseVirtualMachineScaleSetExtensionID(input string) (*VirtualMachineScaleSetExtensionResourceID, error) { +func VirtualMachineScaleSetExtensionID(input string) (*VirtualMachineScaleSetExtensionId, error) { id, err := azure.ParseAzureResourceID(input) if err != nil { return nil, fmt.Errorf("[ERROR] Unable to parse Virtual Machine Scale Set Extension ID %q: %+v", input, err) } - extension := VirtualMachineScaleSetExtensionResourceID{ + extension := VirtualMachineScaleSetExtensionId{ ResourceGroup: id.ResourceGroup, } diff --git a/azurerm/internal/services/compute/virtual_machine_scale_set_extension_test.go b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go similarity index 89% rename from azurerm/internal/services/compute/virtual_machine_scale_set_extension_test.go rename to azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go index d9a7804229dd..b7c5da52ac57 100644 --- a/azurerm/internal/services/compute/virtual_machine_scale_set_extension_test.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go @@ -1,4 +1,4 @@ -package compute +package parse import "testing" @@ -6,7 +6,7 @@ func TestParseVirtualMachineScaleSetExtensionID(t *testing.T) { testData := []struct { Name string Input string - Expected *VirtualMachineScaleSetExtensionResourceID + Expected *VirtualMachineScaleSetExtensionId }{ { Name: "Empty", @@ -36,7 +36,7 @@ func TestParseVirtualMachineScaleSetExtensionID(t *testing.T) { { Name: "Completed", Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/virtualMachineScaleSets/machine1/extensions/extension1", - Expected: &VirtualMachineScaleSetExtensionResourceID{ + Expected: &VirtualMachineScaleSetExtensionId{ Name: "extension1", VirtualMachineName: "machine1", ResourceGroup: "foo", @@ -47,7 +47,7 @@ func TestParseVirtualMachineScaleSetExtensionID(t *testing.T) { for _, v := range testData { t.Logf("[DEBUG] Testing %q", v.Name) - actual, err := ParseVirtualMachineScaleSetExtensionID(v.Input) + actual, err := VirtualMachineScaleSetExtensionID(v.Input) if err != nil { if v.Expected == nil { continue diff --git a/azurerm/internal/services/compute/virtual_machine_scale_set_test.go b/azurerm/internal/services/compute/parse/vritual_machine_scale_set_test.go similarity index 84% rename from azurerm/internal/services/compute/virtual_machine_scale_set_test.go rename to azurerm/internal/services/compute/parse/vritual_machine_scale_set_test.go index 9c1a87d985ce..e22d1f90a3ff 100644 --- a/azurerm/internal/services/compute/virtual_machine_scale_set_test.go +++ b/azurerm/internal/services/compute/parse/vritual_machine_scale_set_test.go @@ -1,12 +1,12 @@ -package compute +package parse import "testing" -func TestParseVirtualMachineScaleSetID(t *testing.T) { +func TestVirtualMachineScaleSetID(t *testing.T) { testData := []struct { Name string Input string - Expected *VirtualMachineScaleSetResourceID + Expected *VirtualMachineScaleSetId }{ { Name: "Empty", @@ -26,7 +26,7 @@ func TestParseVirtualMachineScaleSetID(t *testing.T) { { Name: "Completed", Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/virtualMachineScaleSets/example", - Expected: &VirtualMachineScaleSetResourceID{ + Expected: &VirtualMachineScaleSetId{ Name: "example", ResourceGroup: "foo", }, @@ -36,7 +36,7 @@ func TestParseVirtualMachineScaleSetID(t *testing.T) { for _, v := range testData { t.Logf("[DEBUG] Testing %q", v.Name) - actual, err := ParseVirtualMachineScaleSetID(v.Input) + actual, err := VirtualMachineScaleSetID(v.Input) if err != nil { if v.Expected == nil { continue diff --git a/azurerm/internal/services/compute/resource_arm_linux_virtual_machine_scale_set.go b/azurerm/internal/services/compute/resource_arm_linux_virtual_machine_scale_set.go index f21f8417509d..a5561fc3e564 100644 --- a/azurerm/internal/services/compute/resource_arm_linux_virtual_machine_scale_set.go +++ b/azurerm/internal/services/compute/resource_arm_linux_virtual_machine_scale_set.go @@ -13,7 +13,8 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" - computeValidate "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/base64" azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" @@ -29,7 +30,7 @@ func resourceArmLinuxVirtualMachineScaleSet() *schema.Resource { Delete: resourceArmLinuxVirtualMachineScaleSetDelete, Importer: azSchema.ValidateResourceIDPriorToImportThen(func(id string) error { - _, err := ParseVirtualMachineScaleSetID(id) + _, err := parse.VirtualMachineScaleSetID(id) return err }, importVirtualMachineScaleSet(compute.Linux, "azurerm_linux_virtual_machine_scale_set")), @@ -180,7 +181,7 @@ func resourceArmLinuxVirtualMachineScaleSet() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, - ValidateFunc: computeValidate.ProximityPlacementGroupID, + ValidateFunc: validate.ProximityPlacementGroupID, // the Compute API is broken and returns the Resource Group name in UPPERCASE :shrug: DiffSuppressFunc: suppress.CaseDifference, }, @@ -199,9 +200,9 @@ func resourceArmLinuxVirtualMachineScaleSet() *schema.Resource { Type: schema.TypeString, Optional: true, ValidateFunc: validation.Any( - computeValidate.ImageID, - computeValidate.SharedImageID, - computeValidate.SharedImageVersionID, + validate.ImageID, + validate.SharedImageID, + validate.SharedImageVersionID, ), }, @@ -501,7 +502,7 @@ func resourceArmLinuxVirtualMachineScaleSetUpdate(d *schema.ResourceData, meta i ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return err } @@ -741,7 +742,7 @@ func resourceArmLinuxVirtualMachineScaleSetRead(d *schema.ResourceData, meta int ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return err } @@ -914,7 +915,7 @@ func resourceArmLinuxVirtualMachineScaleSetDelete(d *schema.ResourceData, meta i ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return err } diff --git a/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go b/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go index 932289a31a71..a57fdb7211fb 100644 --- a/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go +++ b/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go @@ -13,6 +13,8 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate" azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -28,7 +30,7 @@ func resourceArmVirtualMachineScaleSetExtension() *schema.Resource { Delete: resourceArmVirtualMachineScaleSetExtensionDelete, Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error { - _, err := ParseVirtualMachineScaleSetExtensionID(id) + _, err := parse.VirtualMachineScaleSetExtensionID(id) return err }), @@ -51,7 +53,7 @@ func resourceArmVirtualMachineScaleSetExtension() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: ValidateScaleSetResourceID, + ValidateFunc: validate.VirtualMachineScaleSetID, }, "publisher": { @@ -117,7 +119,7 @@ func resourceArmVirtualMachineScaleSetExtensionCreate(d *schema.ResourceData, me defer cancel() name := d.Get("name").(string) - virtualMachineScaleSetId, err := ParseVirtualMachineScaleSetID(d.Get("virtual_machine_scale_set_id").(string)) + virtualMachineScaleSetId, err := parse.VirtualMachineScaleSetID(d.Get("virtual_machine_scale_set_id").(string)) if err != nil { return err } @@ -197,7 +199,7 @@ func resourceArmVirtualMachineScaleSetExtensionUpdate(d *schema.ResourceData, me ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetExtensionID(d.Id()) + id, err := parse.VirtualMachineScaleSetExtensionID(d.Id()) if err != nil { return err } @@ -277,7 +279,7 @@ func resourceArmVirtualMachineScaleSetExtensionRead(d *schema.ResourceData, meta ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetExtensionID(d.Id()) + id, err := parse.VirtualMachineScaleSetExtensionID(d.Id()) if err != nil { return err } @@ -337,7 +339,7 @@ func resourceArmVirtualMachineScaleSetExtensionDelete(d *schema.ResourceData, me ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetExtensionID(d.Id()) + id, err := parse.VirtualMachineScaleSetExtensionID(d.Id()) if err != nil { return err } diff --git a/azurerm/internal/services/compute/resource_arm_windows_virtual_machine_scale_set.go b/azurerm/internal/services/compute/resource_arm_windows_virtual_machine_scale_set.go index 26337fd1f362..ab71a6caeb8d 100644 --- a/azurerm/internal/services/compute/resource_arm_windows_virtual_machine_scale_set.go +++ b/azurerm/internal/services/compute/resource_arm_windows_virtual_machine_scale_set.go @@ -14,6 +14,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/base64" azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" @@ -29,7 +30,7 @@ func resourceArmWindowsVirtualMachineScaleSet() *schema.Resource { Delete: resourceArmWindowsVirtualMachineScaleSetDelete, Importer: azSchema.ValidateResourceIDPriorToImportThen(func(id string) error { - _, err := ParseVirtualMachineScaleSetID(id) + _, err := parse.VirtualMachineScaleSetID(id) return err }, importVirtualMachineScaleSet(compute.Windows, "azurerm_windows_virtual_machine_scale_set")), @@ -527,7 +528,7 @@ func resourceArmWindowsVirtualMachineScaleSetUpdate(d *schema.ResourceData, meta ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return err } @@ -771,7 +772,7 @@ func resourceArmWindowsVirtualMachineScaleSetRead(d *schema.ResourceData, meta i ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return err } @@ -959,7 +960,7 @@ func resourceArmWindowsVirtualMachineScaleSetDelete(d *schema.ResourceData, meta ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return err } diff --git a/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go b/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go index ccc0417d6330..0bd369e59c32 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go @@ -2,6 +2,7 @@ package tests import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" @@ -19,10 +20,12 @@ func testCheckAzureRMLinuxVirtualMachineScaleSetDestroy(s *terraform.State) erro continue } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineScaleSetID(rs.Primary.ID) + if err != nil { + return err + } - resp, err := client.Get(ctx, resourceGroup, name) + resp, err := client.Get(ctx, id.ResourceGroup, id.Name) if err != nil { if !utils.ResponseWasNotFound(resp.Response) { return err @@ -46,13 +49,15 @@ func testCheckAzureRMLinuxVirtualMachineScaleSetExists(resourceName string) reso return fmt.Errorf("Not found: %s", resourceName) } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineScaleSetID(rs.Primary.ID) + if err != nil { + return err + } - resp, err := client.Get(ctx, resourceGroup, name) + resp, err := client.Get(ctx, id.ResourceGroup, id.Name) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Bad: Linux Virtual Machine Scale Set %q (Resource Group: %q) does not exist", name, resourceGroup) + return fmt.Errorf("Bad: Linux Virtual Machine Scale Set %q (Resource Group: %q) does not exist", id.Name, id.ResourceGroup) } return fmt.Errorf("Bad: Get on VMScaleSetClient: %+v", err) diff --git a/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go b/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go index 487d36069853..465016399071 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go @@ -2,6 +2,7 @@ package tests import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" @@ -19,10 +20,12 @@ func testCheckAzureRMWindowsVirtualMachineScaleSetDestroy(s *terraform.State) er continue } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineScaleSetID(rs.Primary.ID) + if err != nil { + return err + } - resp, err := client.Get(ctx, resourceGroup, name) + resp, err := client.Get(ctx, id.ResourceGroup, id.Name) if err != nil { if !utils.ResponseWasNotFound(resp.Response) { return err @@ -46,13 +49,15 @@ func testCheckAzureRMWindowsVirtualMachineScaleSetExists(resourceName string) re return fmt.Errorf("Not found: %s", resourceName) } - resourceGroup := rs.Primary.Attributes["resource_group_name"] - name := rs.Primary.Attributes["name"] + id, err := parse.VirtualMachineScaleSetID(rs.Primary.ID) + if err != nil { + return err + } - resp, err := client.Get(ctx, resourceGroup, name) + resp, err := client.Get(ctx, id.ResourceGroup, id.Name) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Bad: Windows Virtual Machine Scale Set %q (Resource Group: %q) does not exist", name, resourceGroup) + return fmt.Errorf("Bad: Windows Virtual Machine Scale Set %q (Resource Group: %q) does not exist", id.Name, id.ResourceGroup) } return fmt.Errorf("Bad: Get on VMScaleSetClient: %+v", err) diff --git a/azurerm/internal/services/compute/validate/virtual_machine_scale_set.go b/azurerm/internal/services/compute/validate/virtual_machine_scale_set.go new file mode 100644 index 000000000000..12ada49ddebe --- /dev/null +++ b/azurerm/internal/services/compute/validate/virtual_machine_scale_set.go @@ -0,0 +1,28 @@ +package validate + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" +) + +func VirtualMachineScaleSetID(i interface{}, k string) (s []string, es []error) { + v, ok := i.(string) + if !ok { + es = append(es, fmt.Errorf("expected type of %s to be string", k)) + return + } + + id, err := parse.VirtualMachineScaleSetID(v) + if err != nil { + es = append(es, fmt.Errorf("Error parsing %q as a VM Scale Set Resource ID: %s", v, err)) + return + } + + if id.Name == "" { + es = append(es, fmt.Errorf("Error parsing %q as a VM Scale Set Resource ID: `virtualMachineScaleSets` segment was empty", v)) + return + } + + return +} diff --git a/azurerm/internal/services/compute/validation.go b/azurerm/internal/services/compute/validation.go index f1e6cdc88051..a3cdc9f3b334 100644 --- a/azurerm/internal/services/compute/validation.go +++ b/azurerm/internal/services/compute/validation.go @@ -90,27 +90,6 @@ func ValidateWindowsName(i interface{}, k string) (warnings []string, errors []e return warnings, errors } -func ValidateScaleSetResourceID(i interface{}, k string) (s []string, es []error) { - v, ok := i.(string) - if !ok { - es = append(es, fmt.Errorf("expected type of %s to be string", k)) - return - } - - id, err := ParseVirtualMachineScaleSetID(v) - if err != nil { - es = append(es, fmt.Errorf("Error parsing %q as a VM Scale Set Resource ID: %s", v, err)) - return - } - - if id.Name == "" { - es = append(es, fmt.Errorf("Error parsing %q as a VM Scale Set Resource ID: `virtualMachineScaleSets` segment was empty", v)) - return - } - - return -} - func validateDiskEncryptionSetName(i interface{}, k string) (warnings []string, errors []error) { v, ok := i.(string) if !ok { diff --git a/azurerm/internal/services/compute/virtual_machine_scale_set.go b/azurerm/internal/services/compute/virtual_machine_scale_set.go index 205707994d4d..d60b86b12ebd 100644 --- a/azurerm/internal/services/compute/virtual_machine_scale_set.go +++ b/azurerm/internal/services/compute/virtual_machine_scale_set.go @@ -12,32 +12,6 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) -type VirtualMachineScaleSetResourceID struct { - ResourceGroup string - Name string -} - -func ParseVirtualMachineScaleSetID(input string) (*VirtualMachineScaleSetResourceID, error) { - id, err := azure.ParseAzureResourceID(input) - if err != nil { - return nil, fmt.Errorf("[ERROR] Unable to parse Virtual Machine Scale Set ID %q: %+v", input, err) - } - - vmScaleSet := VirtualMachineScaleSetResourceID{ - ResourceGroup: id.ResourceGroup, - } - - if vmScaleSet.Name, err = id.PopSegment("virtualMachineScaleSets"); err != nil { - return nil, err - } - - if err := id.ValidateNoEmptySegments(input); err != nil { - return nil, err - } - - return &vmScaleSet, nil -} - func VirtualMachineScaleSetAdditionalCapabilitiesSchema() *schema.Schema { return &schema.Schema{ Type: schema.TypeList, diff --git a/azurerm/internal/services/compute/virtual_machine_scale_set_import.go b/azurerm/internal/services/compute/virtual_machine_scale_set_import.go index cf2847bcf51a..277ca8f2fb12 100644 --- a/azurerm/internal/services/compute/virtual_machine_scale_set_import.go +++ b/azurerm/internal/services/compute/virtual_machine_scale_set_import.go @@ -6,12 +6,13 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" ) func importVirtualMachineScaleSet(osType compute.OperatingSystemTypes, resourceType string) func(d *schema.ResourceData, meta interface{}) (data []*schema.ResourceData, err error) { return func(d *schema.ResourceData, meta interface{}) (data []*schema.ResourceData, err error) { - id, err := ParseVirtualMachineScaleSetID(d.Id()) + id, err := parse.VirtualMachineScaleSetID(d.Id()) if err != nil { return []*schema.ResourceData{}, err } diff --git a/azurerm/internal/services/compute/virtual_machine_scale_set_update.go b/azurerm/internal/services/compute/virtual_machine_scale_set_update.go index ba053bc95572..e335548d6b09 100644 --- a/azurerm/internal/services/compute/virtual_machine_scale_set_update.go +++ b/azurerm/internal/services/compute/virtual_machine_scale_set_update.go @@ -7,6 +7,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/client" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -22,7 +23,7 @@ type virtualMachineScaleSetUpdateMetaData struct { Client *client.Client Existing compute.VirtualMachineScaleSet - ID *VirtualMachineScaleSetResourceID + ID *parse.VirtualMachineScaleSetId OSType compute.OperatingSystemTypes } From 6facd42c2508db4b76324cba2ca38da4af94b882 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 8 Apr 2020 17:27:55 +0800 Subject: [PATCH 4/9] VM extension refactor --- .../{ => parse}/virtual_machine_extension.go | 24 ++-------- .../virtual_machine_extension_test.go | 45 ++----------------- .../resource_arm_virtual_machine_extension.go | 13 +++--- .../validate/virtual_machine_extension.go | 22 +++++++++ 4 files changed, 38 insertions(+), 66 deletions(-) rename azurerm/internal/services/compute/{ => parse}/virtual_machine_extension.go (50%) rename azurerm/internal/services/compute/{ => parse}/virtual_machine_extension_test.go (56%) create mode 100644 azurerm/internal/services/compute/validate/virtual_machine_extension.go diff --git a/azurerm/internal/services/compute/virtual_machine_extension.go b/azurerm/internal/services/compute/parse/virtual_machine_extension.go similarity index 50% rename from azurerm/internal/services/compute/virtual_machine_extension.go rename to azurerm/internal/services/compute/parse/virtual_machine_extension.go index 40cc136c79c9..ef3b772d9824 100644 --- a/azurerm/internal/services/compute/virtual_machine_extension.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_extension.go @@ -1,24 +1,23 @@ -package compute +package parse import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" ) -type VirtualMachineExtensionID struct { +type VirtualMachineExtensionId struct { ResourceGroup string Name string VirtualMachine string } -func ParseVirtualMachineExtensionID(input string) (*VirtualMachineExtensionID, error) { +func VirtualMachineExtensionID(input string) (*VirtualMachineExtensionId, error) { id, err := azure.ParseAzureResourceID(input) if err != nil { return nil, fmt.Errorf("[ERROR] Unable to parse App Service ID %q: %+v", input, err) } - virtualMachineExtension := VirtualMachineExtensionID{ + virtualMachineExtension := VirtualMachineExtensionId{ ResourceGroup: id.ResourceGroup, } @@ -32,18 +31,3 @@ func ParseVirtualMachineExtensionID(input string) (*VirtualMachineExtensionID, e return &virtualMachineExtension, nil } - -func ValidateVirtualMachineExtensionID(i interface{}, k string) (warnings []string, errors []error) { - v, ok := i.(string) - if !ok { - errors = append(errors, fmt.Errorf("expected type of %q to be string", k)) - return - } - - if _, err := ParseVirtualMachineExtensionID(v); err != nil { - errors = append(errors, fmt.Errorf("Can not parse %q as a resource id: %v", k, err)) - return - } - - return warnings, errors -} diff --git a/azurerm/internal/services/compute/virtual_machine_extension_test.go b/azurerm/internal/services/compute/parse/virtual_machine_extension_test.go similarity index 56% rename from azurerm/internal/services/compute/virtual_machine_extension_test.go rename to azurerm/internal/services/compute/parse/virtual_machine_extension_test.go index 9e4004037277..4cc15751c63e 100644 --- a/azurerm/internal/services/compute/virtual_machine_extension_test.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_extension_test.go @@ -1,4 +1,4 @@ -package compute +package parse import "testing" @@ -6,7 +6,7 @@ func TestParseVirtualMachineExtensionID(t *testing.T) { testData := []struct { Name string Input string - Expected *VirtualMachineExtensionID + Expected *VirtualMachineExtensionId }{ { Name: "Empty", @@ -31,7 +31,7 @@ func TestParseVirtualMachineExtensionID(t *testing.T) { { Name: "Valid", Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/Microsoft.Compute/virtualMachines/machine1/extensions/extName", - Expected: &VirtualMachineExtensionID{ + Expected: &VirtualMachineExtensionId{ ResourceGroup: "myGroup1", Name: "extName", VirtualMachine: "machine1", @@ -41,7 +41,7 @@ func TestParseVirtualMachineExtensionID(t *testing.T) { for _, v := range testData { t.Logf("[DEBUG] Testing %q", v.Name) - actual, err := ParseVirtualMachineExtensionID(v.Input) + actual, err := VirtualMachineExtensionID(v.Input) if err != nil { if v.Expected == nil { continue @@ -59,40 +59,3 @@ func TestParseVirtualMachineExtensionID(t *testing.T) { } } } - -func TestValidateVirtualMachineExtensionID(t *testing.T) { - cases := []struct { - ID string - Valid bool - }{ - { - ID: "", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/virtualMachines/machine1/extension/", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/microsoft.compute/virtualMachines/machine1/Extensions/extName", - Valid: false, - }, - { - ID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myGroup1/providers/Microsoft.Compute/virtualMachines/machine1/extensions/extName", - Valid: true, - }, - } - for _, tc := range cases { - t.Logf("[DEBUG] Testing Value %s", tc.ID) - _, errors := ValidateVirtualMachineExtensionID(tc.ID, "test") - valid := len(errors) == 0 - - if tc.Valid != valid { - t.Fatalf("Expected %t but got %t", tc.Valid, valid) - } - } -} diff --git a/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go b/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go index 16d1a97dfd19..942193dabba6 100644 --- a/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go +++ b/azurerm/internal/services/compute/resource_arm_virtual_machine_extension.go @@ -14,6 +14,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" + azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -24,9 +25,11 @@ func resourceArmVirtualMachineExtension() *schema.Resource { Read: resourceArmVirtualMachineExtensionsRead, Update: resourceArmVirtualMachineExtensionsCreateUpdate, Delete: resourceArmVirtualMachineExtensionsDelete, - Importer: &schema.ResourceImporter{ - State: schema.ImportStatePassthrough, - }, + + Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error { + _, err := parse.VirtualMachineExtensionID(id) + return err + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), @@ -189,7 +192,7 @@ func resourceArmVirtualMachineExtensionsRead(d *schema.ResourceData, meta interf ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineExtensionID(d.Id()) + id, err := parse.VirtualMachineExtensionID(d.Id()) if err != nil { return err } @@ -240,7 +243,7 @@ func resourceArmVirtualMachineExtensionsDelete(d *schema.ResourceData, meta inte ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := ParseVirtualMachineExtensionID(d.Id()) + id, err := parse.VirtualMachineExtensionID(d.Id()) if err != nil { return err } diff --git a/azurerm/internal/services/compute/validate/virtual_machine_extension.go b/azurerm/internal/services/compute/validate/virtual_machine_extension.go new file mode 100644 index 000000000000..9efd43348e76 --- /dev/null +++ b/azurerm/internal/services/compute/validate/virtual_machine_extension.go @@ -0,0 +1,22 @@ +package validate + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" +) + +func VirtualMachineExtensionID(i interface{}, k string) (warnings []string, errors []error) { + v, ok := i.(string) + if !ok { + errors = append(errors, fmt.Errorf("expected type of %q to be string", k)) + return + } + + if _, err := parse.VirtualMachineExtensionID(v); err != nil { + errors = append(errors, fmt.Errorf("Can not parse %q as a resource id: %v", k, err)) + return + } + + return warnings, errors +} From 94236b971104296a6ccbf16da6df7f9585e4c7a2 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 8 Apr 2020 17:34:44 +0800 Subject: [PATCH 5/9] Managed disk ID --- .../compute/resource_arm_managed_disk.go | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/azurerm/internal/services/compute/resource_arm_managed_disk.go b/azurerm/internal/services/compute/resource_arm_managed_disk.go index 13f5083f9b81..7f45932e4c60 100644 --- a/azurerm/internal/services/compute/resource_arm_managed_disk.go +++ b/azurerm/internal/services/compute/resource_arm_managed_disk.go @@ -7,7 +7,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute" - "github.com/hashicorp/go-azure-helpers/response" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" @@ -18,6 +17,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/locks" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" + azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -29,9 +29,10 @@ func resourceArmManagedDisk() *schema.Resource { Update: resourceArmManagedDiskUpdate, Delete: resourceArmManagedDiskDelete, - Importer: &schema.ResourceImporter{ - State: schema.ImportStatePassthrough, - }, + Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error { + _, err := parse.ManagedDiskID(id) + return err + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), @@ -502,25 +503,23 @@ func resourceArmManagedDiskRead(d *schema.ResourceData, meta interface{}) error ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := azure.ParseAzureResourceID(d.Id()) + id, err := parse.ManagedDiskID(d.Id()) if err != nil { return err } - resGroup := id.ResourceGroup - name := id.Path["disks"] - resp, err := client.Get(ctx, resGroup, name) + resp, err := client.Get(ctx, id.ResourceGroup, id.Name) if err != nil { if utils.ResponseWasNotFound(resp.Response) { log.Printf("[INFO] Disk %q does not exist - removing from state", d.Id()) d.SetId("") return nil } - return fmt.Errorf("[ERROR] Error making Read request on Azure Managed Disk %s (resource group %s): %s", name, resGroup, err) + return fmt.Errorf("Error making Read request on Azure Managed Disk %s (resource group %s): %s", id.Name, id.ResourceGroup, err) } d.Set("name", resp.Name) - d.Set("resource_group_name", resGroup) + d.Set("resource_group_name", id.ResourceGroup) d.Set("zones", utils.FlattenStringSlice(resp.Zones)) if location := resp.Location; location != nil { @@ -570,24 +569,18 @@ func resourceArmManagedDiskDelete(d *schema.ResourceData, meta interface{}) erro ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := azure.ParseAzureResourceID(d.Id()) + id, err := parse.ManagedDiskID(d.Id()) if err != nil { return err } - resGroup := id.ResourceGroup - name := id.Path["disks"] - future, err := client.Delete(ctx, resGroup, name) + future, err := client.Delete(ctx, id.ResourceGroup, id.Name) if err != nil { - if !response.WasNotFound(future.Response()) { - return fmt.Errorf("Error deleting Managed Disk %q (Resource Group %q): %+v", name, resGroup, err) - } + return fmt.Errorf("Error deleting Managed Disk %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - if !response.WasNotFound(future.Response()) { - return fmt.Errorf("Error waiting for deletion of Managed Disk %q (Resource Group %q): %+v", name, resGroup, err) - } + return fmt.Errorf("Error waiting for deletion of Managed Disk %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } return nil From 865f82564ab20a243894da17244ca53ab1429b63 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 8 Apr 2020 21:32:14 +0800 Subject: [PATCH 6/9] Fix CI --- .../parse/virtual_machine_extension.go | 1 + .../virtual_machine_scale_set_extension.go | 8 +++---- ...irtual_machine_scale_set_extension_test.go | 6 ++--- ...arm_virtual_machine_scale_set_extension.go | 24 +++++++++---------- ...irtual_machine_scale_set_extension_test.go | 24 ++++++++----------- 5 files changed, 30 insertions(+), 33 deletions(-) diff --git a/azurerm/internal/services/compute/parse/virtual_machine_extension.go b/azurerm/internal/services/compute/parse/virtual_machine_extension.go index ef3b772d9824..06cd9d21ac70 100644 --- a/azurerm/internal/services/compute/parse/virtual_machine_extension.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_extension.go @@ -2,6 +2,7 @@ package parse import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" ) diff --git a/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go index 346196ae7f84..166c0b2ca1c1 100644 --- a/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension.go @@ -7,9 +7,9 @@ import ( ) type VirtualMachineScaleSetExtensionId struct { - ResourceGroup string - VirtualMachineName string - Name string + ResourceGroup string + VirtualMachineScaleSetName string + Name string } func VirtualMachineScaleSetExtensionID(input string) (*VirtualMachineScaleSetExtensionId, error) { @@ -22,7 +22,7 @@ func VirtualMachineScaleSetExtensionID(input string) (*VirtualMachineScaleSetExt ResourceGroup: id.ResourceGroup, } - if extension.VirtualMachineName, err = id.PopSegment("virtualMachineScaleSets"); err != nil { + if extension.VirtualMachineScaleSetName, err = id.PopSegment("virtualMachineScaleSets"); err != nil { return nil, err } diff --git a/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go index b7c5da52ac57..616a7d5e0c10 100644 --- a/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go +++ b/azurerm/internal/services/compute/parse/virtual_machine_scale_set_extension_test.go @@ -37,9 +37,9 @@ func TestParseVirtualMachineScaleSetExtensionID(t *testing.T) { Name: "Completed", Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/virtualMachineScaleSets/machine1/extensions/extension1", Expected: &VirtualMachineScaleSetExtensionId{ - Name: "extension1", - VirtualMachineName: "machine1", - ResourceGroup: "foo", + Name: "extension1", + VirtualMachineScaleSetName: "machine1", + ResourceGroup: "foo", }, }, } diff --git a/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go b/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go index a57fdb7211fb..c377bd9ed864 100644 --- a/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go +++ b/azurerm/internal/services/compute/resource_arm_virtual_machine_scale_set_extension.go @@ -261,13 +261,13 @@ func resourceArmVirtualMachineScaleSetExtensionUpdate(d *schema.ResourceData, me Name: utils.String(id.Name), VirtualMachineScaleSetExtensionProperties: &props, } - future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.VirtualMachineName, id.Name, extension) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name, extension) if err != nil { - return fmt.Errorf("Error updating Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineName, id.ResourceGroup, err) + return fmt.Errorf("Error updating Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup, err) } if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for update of Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineName, id.ResourceGroup, err) + return fmt.Errorf("Error waiting for update of Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup, err) } return resourceArmVirtualMachineScaleSetExtensionRead(d, meta) @@ -284,26 +284,26 @@ func resourceArmVirtualMachineScaleSetExtensionRead(d *schema.ResourceData, meta return err } - vmss, err := vmssClient.Get(ctx, id.ResourceGroup, id.VirtualMachineName) + vmss, err := vmssClient.Get(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName) if err != nil { if utils.ResponseWasNotFound(vmss.Response) { - log.Printf("Virtual Machine Scale Set %q was not found in Resource Group %q - removing Extension from state!", id.VirtualMachineName, id.ResourceGroup) + log.Printf("Virtual Machine Scale Set %q was not found in Resource Group %q - removing Extension from state!", id.VirtualMachineScaleSetName, id.ResourceGroup) d.SetId("") return nil } - return fmt.Errorf("Error retrieving Virtual Machine Scale Set %q (Resource Group %q): %+v", id.VirtualMachineName, id.ResourceGroup, err) + return fmt.Errorf("Error retrieving Virtual Machine Scale Set %q (Resource Group %q): %+v", id.VirtualMachineScaleSetName, id.ResourceGroup, err) } - resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachineName, id.Name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { - log.Printf("Extension %q (Virtual Machine Scale Set %q / Resource Group %q) was not found - removing from state!", id.Name, id.VirtualMachineName, id.ResourceGroup) + log.Printf("Extension %q (Virtual Machine Scale Set %q / Resource Group %q) was not found - removing from state!", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup) d.SetId("") return nil } - return fmt.Errorf("Error retrieving Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineName, id.ResourceGroup, err) + return fmt.Errorf("Error retrieving Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup, err) } d.Set("name", id.Name) @@ -344,17 +344,17 @@ func resourceArmVirtualMachineScaleSetExtensionDelete(d *schema.ResourceData, me return err } - future, err := client.Delete(ctx, id.ResourceGroup, id.VirtualMachineName, id.Name) + future, err := client.Delete(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name) if err != nil { if response.WasNotFound(future.Response()) { return nil } - return fmt.Errorf("Error deleting Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineName, id.ResourceGroup, err) + return fmt.Errorf("Error deleting Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup, err) } if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for deletion of Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineName, id.ResourceGroup, err) + return fmt.Errorf("Error waiting for deletion of Extension %q (Virtual Machine Scale Set %q / Resource Group %q): %+v", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup, err) } return nil diff --git a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go index 9165a35c3998..a5bfbe59d117 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go @@ -2,6 +2,8 @@ package tests import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" "net/http" "testing" @@ -10,7 +12,6 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute" ) func TestAccAzureRMVirtualMachineScaleSetExtension_basicLinux(t *testing.T) { @@ -216,23 +217,20 @@ func testCheckAzureRMVirtualMachineScaleSetExtensionExists(resourceName string) return fmt.Errorf("Not found: %s", resourceName) } - name := rs.Primary.Attributes["name"] - virtualMachineScaleSetIdRaw := rs.Primary.Attributes["virtual_machine_scale_set_id"] - virtualMachineScaleSetId, err := compute.ParseVirtualMachineScaleSetID(virtualMachineScaleSetIdRaw) + id, err := parse.VirtualMachineScaleSetExtensionID(rs.Primary.ID) if err != nil { return err } - resp, err := client.Get(ctx, virtualMachineScaleSetId.ResourceGroup, virtualMachineScaleSetId.Name, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name, "") if err != nil { + if utils.ResponseWasNotFound(resp.Response) { + return fmt.Errorf("Bad: Extension %q (VirtualMachineScaleSet %q / Resource Group: %q) does not exist", id.Name, id.VirtualMachineScaleSetName, id.ResourceGroup) + } return fmt.Errorf("Bad: Get on vmScaleSetClient: %+v", err) } - if resp.StatusCode == http.StatusNotFound { - return fmt.Errorf("Bad: Extension %q (VirtualMachineScaleSet %q / Resource Group: %q) does not exist", name, virtualMachineScaleSetId.Name, virtualMachineScaleSetId.ResourceGroup) - } - - return err + return nil } } @@ -245,14 +243,12 @@ func testCheckAzureRMVirtualMachineScaleSetExtensionDestroy(s *terraform.State) continue } - name := rs.Primary.Attributes["name"] - virtualMachineScaleSetIdRaw := rs.Primary.Attributes["virtual_machine_scale_set_id"] - virtualMachineScaleSetId, err := compute.ParseVirtualMachineScaleSetID(virtualMachineScaleSetIdRaw) + id, err := parse.VirtualMachineScaleSetExtensionID(rs.Primary.ID) if err != nil { return err } - resp, err := client.Get(ctx, virtualMachineScaleSetId.ResourceGroup, virtualMachineScaleSetId.Name, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name, "") if err != nil { return nil From 988a70c859b6f2c42e886114268c6697e4706ba5 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Thu, 9 Apr 2020 00:10:07 +0800 Subject: [PATCH 7/9] Fix another missed one --- ...urce_arm_virtual_machine_extension_test.go | 40 +++++++------------ ...irtual_machine_scale_set_extension_test.go | 13 +++--- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go index 784cb33ec1f1..79410d0d415f 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go @@ -2,7 +2,8 @@ package tests import ( "fmt" - "net/http" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" "regexp" "testing" @@ -11,7 +12,6 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute" ) func TestAccAzureRMVirtualMachineExtension_basic(t *testing.T) { @@ -120,23 +120,18 @@ func testCheckAzureRMVirtualMachineExtensionExists(resourceName string) resource return fmt.Errorf("Not found: %s", resourceName) } - name := rs.Primary.Attributes["name"] - virtualMachineId, err := compute.ParseVirtualMachineID(rs.Primary.Attributes["virtual_machine_id"]) + id, err := parse.VirtualMachineExtensionID(rs.Primary.ID) if err != nil { - return fmt.Errorf("Error parsing Virtual Machine ID %q: %+v", virtualMachineId, err) + return err } - vmName := virtualMachineId.Name - resourceGroup := virtualMachineId.ResourceGroup - resp, err := client.Get(ctx, resourceGroup, vmName, name, "") - if err != nil { + if resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachine, id.Name, ""); err != nil { + if utils.ResponseWasNotFound(resp.Response) { + return fmt.Errorf("Bad: VirtualMachine Extension %q (resource group: %q) does not exist", id.Name, id.ResourceGroup) + } return fmt.Errorf("Bad: Get on vmExtensionClient: %s", err) } - if resp.StatusCode == http.StatusNotFound { - return fmt.Errorf("Bad: VirtualMachine Extension %q (resource group: %q) does not exist", name, resourceGroup) - } - return nil } } @@ -150,23 +145,18 @@ func testCheckAzureRMVirtualMachineExtensionDestroy(s *terraform.State) error { continue } - name := rs.Primary.Attributes["name"] - virtualMachineId, err := compute.ParseVirtualMachineID(rs.Primary.Attributes["virtual_machine_id"]) + id, err := parse.VirtualMachineExtensionID(rs.Primary.ID) if err != nil { - return fmt.Errorf("Error parsing Virtual Machine ID %q: %+v", virtualMachineId, err) + return err } - vmName := virtualMachineId.Name - resourceGroup := virtualMachineId.ResourceGroup - - resp, err := client.Get(ctx, resourceGroup, vmName, name, "") - if err != nil { - return nil + if resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachine, id.Name, ""); err != nil { + if !utils.ResponseWasNotFound(resp.Response) { + return fmt.Errorf("Bad: Get on Compute.VMExtensionClient: %+v", err) + } } - if resp.StatusCode != http.StatusNotFound { - return fmt.Errorf("Virtual Machine Extension still exists:\n%#v", resp.VirtualMachineExtensionProperties) - } + return nil } return nil diff --git a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go index a5bfbe59d117..fc9c02d64e39 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" - "net/http" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" @@ -248,15 +247,13 @@ func testCheckAzureRMVirtualMachineScaleSetExtensionDestroy(s *terraform.State) return err } - resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name, "") - - if err != nil { - return nil + if resp, err := client.Get(ctx, id.ResourceGroup, id.VirtualMachineScaleSetName, id.Name, ""); err != nil { + if !utils.ResponseWasNotFound(resp.Response) { + return fmt.Errorf("Bad: Get on Compute.VMScaleSetExtensionsClient: %+v", err) + } } - if resp.StatusCode != http.StatusNotFound { - return fmt.Errorf("Virtual Machine Scale Set Extension still exists:\n%#v", resp.VirtualMachineScaleSetExtensionProperties) - } + return nil } return nil From 48a2e32564de2060fba57a7cc71cbe7106142321 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Thu, 9 Apr 2020 11:42:17 +0800 Subject: [PATCH 8/9] go imports --- .../resource_arm_linux_virtual_machine_scale_set_test.go | 2 +- .../resource_arm_virtual_machine_scale_set_extension_test.go | 4 ++-- .../resource_arm_windows_virtual_machine_scale_set_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go b/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go index 0bd369e59c32..762e6bb690e2 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_linux_virtual_machine_scale_set_test.go @@ -2,12 +2,12 @@ package tests import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) diff --git a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go index fc9c02d64e39..47aac9d04679 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_scale_set_extension_test.go @@ -2,8 +2,6 @@ package tests import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" @@ -11,6 +9,8 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) func TestAccAzureRMVirtualMachineScaleSetExtension_basicLinux(t *testing.T) { diff --git a/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go b/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go index 465016399071..c6b7daea5f32 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_windows_virtual_machine_scale_set_test.go @@ -2,12 +2,12 @@ package tests import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) From d54f13fb20e2feb8ec520e2c8c4365f5cacfd007 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Thu, 9 Apr 2020 12:48:13 +0800 Subject: [PATCH 9/9] Goimports again... --- .../tests/resource_arm_virtual_machine_extension_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go index 79410d0d415f..d16d3e3aa795 100644 --- a/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go +++ b/azurerm/internal/services/compute/tests/resource_arm_virtual_machine_extension_test.go @@ -2,8 +2,6 @@ package tests import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" "regexp" "testing" @@ -12,6 +10,8 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) func TestAccAzureRMVirtualMachineExtension_basic(t *testing.T) {