From bda1c13f0516b41a85642295f271e2f25b1157b7 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 11:58:37 +0200 Subject: [PATCH 01/15] loadbalancers: adding an ID parser --- .../internal/services/network/loadbalancer.go | 8 +-- .../services/network/parse/load_balancer.go | 44 ++++++++++++ .../network/parse/load_balancer_test.go | 70 +++++++++++++++++++ 3 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_test.go diff --git a/azurerm/internal/services/network/loadbalancer.go b/azurerm/internal/services/network/loadbalancer.go index cada14151433..20f52e71103b 100644 --- a/azurerm/internal/services/network/loadbalancer.go +++ b/azurerm/internal/services/network/loadbalancer.go @@ -9,21 +9,21 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) // TODO: refactor this +// Deprecated: use `parse.LoadBalancerID` func resourceGroupAndLBNameFromId(loadBalancerId string) (string, string, error) { - id, err := azure.ParseAzureResourceID(loadBalancerId) + id, err := parse.LoadBalancerID(loadBalancerId) if err != nil { return "", "", err } - name := id.Path["loadBalancers"] - resGroup := id.ResourceGroup - return resGroup, name, nil + return id.ResourceGroup, id.Name, nil } func retrieveLoadBalancerById(d *schema.ResourceData, loadBalancerId string, meta interface{}) (*network.LoadBalancer, bool, error) { diff --git a/azurerm/internal/services/network/parse/load_balancer.go b/azurerm/internal/services/network/parse/load_balancer.go new file mode 100644 index 000000000000..930c9b0f9aed --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer.go @@ -0,0 +1,44 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerId struct { + ResourceGroup string + Name string +} + +func NewLoadBalancerID(resourceGroup, name string) LoadBalancerId { + return LoadBalancerId{ + Name: name, + ResourceGroup: resourceGroup, + } +} + +func (id LoadBalancerId) ID(subscriptionId string) string { + return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/loadBalancers/%s", subscriptionId, id.ResourceGroup, id.Name) +} + +func LoadBalancerID(input string) (*LoadBalancerId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer ID %q: %+v", input, err) + } + + loadBalancer := LoadBalancerId{ + ResourceGroup: id.ResourceGroup, + } + + if loadBalancer.Name, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &loadBalancer, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_test.go b/azurerm/internal/services/network/parse/load_balancer_test.go new file mode 100644 index 000000000000..ba2820aab493 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_test.go @@ -0,0 +1,70 @@ +package parse + +import ( + "testing" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/resourceid" +) + +var _ resourceid.Formatter = LoadBalancerId{} + +func TestLoadBalancerIDFormatter(t *testing.T) { + subscriptionId := "12345678-1234-5678-1234-123456789012" + actual := NewLoadBalancerID("group1", "lb1").ID(subscriptionId) + expected := "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1" + if actual != expected { + t.Fatalf("Expected %q but got %q", expected, actual) + } +} + +func TestLoadBalancerIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerId + }{ + { + // lower case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadbalancers/lb1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: &LoadBalancerId{ + ResourceGroup: "group1", + Name: "lb1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} From cfbf7fa8fdc8cf11c81dd14d659df35b4a3bdbb5 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 12:20:34 +0200 Subject: [PATCH 02/15] r/loadbalancer_backend_address_pool: using a consistent ID parser --- .../lb_backend_address_pool_resource.go | 82 +++++++++---------- .../load_balancer_backend_address_pool.go | 38 +++++++++ ...load_balancer_backend_address_pool_test.go | 65 +++++++++++++++ ...ncer_backend_address_pool_resource_test.go | 22 +---- 4 files changed, 146 insertions(+), 61 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go diff --git a/azurerm/internal/services/network/lb_backend_address_pool_resource.go b/azurerm/internal/services/network/lb_backend_address_pool_resource.go index e57228b7a205..d9d274fae454 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_resource.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_resource.go @@ -13,6 +13,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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -78,12 +79,17 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) name := d.Get("name").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + loadBalancerIdRaw := d.Get("loadbalancer_id").(string) + loadBalancerId, err := parse.LoadBalancerID(loadBalancerIdRaw) + if err != nil { + return fmt.Errorf("parsing Load Balancer Name and Group: %+v", err) + } + + locks.ByID(loadBalancerIdRaw) + defer locks.UnlockByID(loadBalancerIdRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIdRaw, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -93,7 +99,9 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met return nil } - backendAddressPools := append(*loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools, expandAzureRmLoadBalancerBackendAddressPools(d)) + backendAddressPools := append(*loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools, network.BackendAddressPool{ + Name: utils.String(name), + }) existingPool, existingPoolIndex, exists := FindLoadBalancerBackEndAddressPoolByName(loadBalancer, name) if exists { if name == *existingPool.Name { @@ -107,26 +115,22 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met } loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools = &backendAddressPools - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error parsing Load Balancer Name and Group: %+v", err) - } - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { - return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("reading ID for Load Balancer %q (Resource Group %q)", loadBalancerId.Name, loadBalancerId.ResourceGroup) } var poolId string @@ -146,26 +150,27 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met } func resourceArmLoadBalancerBackendAddressPoolRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + id, err := parse.LoadBalancerBackendAddressPoolID(d.Id()) if err != nil { return err } - name := id.Path["backendAddressPools"] - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { - return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) + return fmt.Errorf("retrieving Load Balancer by ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.LoadBalancerName) return nil } - config, _, exists := FindLoadBalancerBackEndAddressPoolByName(loadBalancer, name) + config, _, exists := FindLoadBalancerBackEndAddressPoolByName(loadBalancer, id.Name) if !exists { + log.Printf("[INFO] Load Balancer Backend Address Pool %q not found. Removing from state", id.Name) d.SetId("") - log.Printf("[INFO] Load Balancer Backend Address Pool %q not found. Removing from state", name) return nil } @@ -197,14 +202,20 @@ func resourceArmLoadBalancerBackendAddressPoolRead(d *schema.ResourceData, meta func resourceArmLoadBalancerBackendAddressPoolDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + id, err := parse.LoadBalancerBackendAddressPoolID(d.Id()) + if err != nil { + return err + } + + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + locks.ByID(loadBalancerId) + defer locks.UnlockByID(loadBalancerId) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) } @@ -222,12 +233,7 @@ func resourceArmLoadBalancerBackendAddressPoolDelete(d *schema.ResourceData, met newBackEndPools := append(oldBackEndPools[:index], oldBackEndPools[index+1:]...) loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools = &newBackEndPools - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { return fmt.Errorf("Error Creating/Updating LoadBalancer: %+v", err) } @@ -236,19 +242,13 @@ func resourceArmLoadBalancerBackendAddressPoolDelete(d *schema.ResourceData, met return fmt.Errorf("Error waiting for the completion for the LoadBalancer: %+v", err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") if err != nil { - return fmt.Errorf("Error retrieving the Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error retrieving the Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (resource group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %q (resource group %q) ID", id.LoadBalancerName, id.ResourceGroup) } return nil } - -func expandAzureRmLoadBalancerBackendAddressPools(d *schema.ResourceData) network.BackendAddressPool { - return network.BackendAddressPool{ - Name: utils.String(d.Get("name").(string)), - } -} diff --git a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go new file mode 100644 index 000000000000..c8a626dd3690 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go @@ -0,0 +1,38 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerBackendAddressPoolId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func LoadBalancerBackendAddressPoolID(input string) (*LoadBalancerBackendAddressPoolId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Backend Address Pool ID %q: %+v", input, err) + } + + backendAddressPoolId := LoadBalancerBackendAddressPoolId{ + ResourceGroup: id.ResourceGroup, + } + + if backendAddressPoolId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if backendAddressPoolId.Name, err = id.PopSegment("backendAddressPools"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &backendAddressPoolId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go new file mode 100644 index 000000000000..6a88454c4b42 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go @@ -0,0 +1,65 @@ +package parse + +import "testing" + +func TestLoadBalancerBackendAddressPoolIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerBackendAddressPoolId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // lower-case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/backendaddresspools/pool1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool1", + expected: &LoadBalancerBackendAddressPoolId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "pool1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Backendaddresspools/pool1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1/BackendAddressPools/pool1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerBackendAddressPoolID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} diff --git a/azurerm/internal/services/network/tests/loadbalancer_backend_address_pool_resource_test.go b/azurerm/internal/services/network/tests/loadbalancer_backend_address_pool_resource_test.go index a0b66cc85923..9ff6dfce5b1d 100644 --- a/azurerm/internal/services/network/tests/loadbalancer_backend_address_pool_resource_test.go +++ b/azurerm/internal/services/network/tests/loadbalancer_backend_address_pool_resource_test.go @@ -2,7 +2,6 @@ package tests import ( "fmt" - "os" "testing" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-03-01/network" @@ -20,11 +19,6 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_basic(t *testing.T) { var lb network.LoadBalancer addressPoolName := fmt.Sprintf("%d-address-pool", data.RandomInteger) - subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID") - backendAddressPoolId := fmt.Sprintf( - "/subscriptions/%s/resourceGroups/acctestRG-%d/providers/Microsoft.Network/loadBalancers/arm-test-loadbalancer-%d/backendAddressPools/%s", - subscriptionID, data.RandomInteger, data.RandomInteger, addressPoolName) - resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, @@ -35,15 +29,9 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName, &lb), - resource.TestCheckResourceAttr( - "azurerm_lb_backend_address_pool.test", "id", backendAddressPoolId), ), }, - { - ResourceName: "azurerm_lb.test", - ImportState: true, - ImportStateVerify: true, - }, + data.ImportStep(), }, }) } @@ -53,11 +41,6 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_requiresImport(t *testing.T) { var lb network.LoadBalancer addressPoolName := fmt.Sprintf("%d-address-pool", data.RandomInteger) - subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID") - backendAddressPoolId := fmt.Sprintf( - "/subscriptions/%s/resourceGroups/acctestRG-%d/providers/Microsoft.Network/loadBalancers/arm-test-loadbalancer-%d/backendAddressPools/%s", - subscriptionID, data.RandomInteger, data.RandomInteger, addressPoolName) - resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, @@ -68,12 +51,11 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_requiresImport(t *testing.T) { Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName, &lb), - resource.TestCheckResourceAttr("azurerm_lb_backend_address_pool.test", "id", backendAddressPoolId), ), }, { Config: testAccAzureRMLoadBalancerBackEndAddressPool_requiresImport(data, addressPoolName), - ExpectError: acceptance.RequiresImportError("azurerm_lb_backend_address_pool"), + ExpectError: acceptance.RequiresImportError(data.ResourceType), }, }, }) From 6d4da9c496da30c1c9ea87aac6bf94996aca4d5f Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 12:44:25 +0200 Subject: [PATCH 03/15] r/lb_nat_pool: switching to use specific ID parsers --- .../services/network/lb_nat_pool_resource.go | 114 +++++++++++------- ...load_balancer_frontend_ip_configuration.go | 51 ++++++++ ...balancer_frontend_ip_configuration_test.go | 81 +++++++++++++ .../parse/load_balancer_inbound_nat_pool.go | 38 ++++++ .../load_balancer_inbound_nat_pool_test.go | 65 ++++++++++ 5 files changed, 303 insertions(+), 46 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration_test.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool_test.go diff --git a/azurerm/internal/services/network/lb_nat_pool_resource.go b/azurerm/internal/services/network/lb_nat_pool_resource.go index fe4e533c87e3..c6dcf40b3dfd 100644 --- a/azurerm/internal/services/network/lb_nat_pool_resource.go +++ b/azurerm/internal/services/network/lb_nat_pool_resource.go @@ -15,6 +15,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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/state" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -103,24 +104,29 @@ func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta int ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) name := d.Get("name").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + loadBalancerIdRaw := d.Get("loadbalancer_id").(string) + id, err := parse.LoadBalancerID(loadBalancerIdRaw) + if err != nil { + return fmt.Errorf("parsing Load Balancer Name and Group: %+v", err) + } + + locks.ByID(loadBalancerIdRaw) + defer locks.UnlockByID(loadBalancerIdRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIdRaw, meta) if err != nil { - return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) + return fmt.Errorf("retrieving Load Balancer By ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.Name) return nil } newNatPool, err := expandAzureRmLoadBalancerNatPool(d, loadBalancer) if err != nil { - return fmt.Errorf("Error Expanding NAT Pool: %+v", err) + return fmt.Errorf("expanding NAT Pool: %+v", err) } natPools := append(*loadBalancer.LoadBalancerPropertiesFormat.InboundNatPools, *newNatPool) @@ -138,26 +144,22 @@ func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta int } loadBalancer.LoadBalancerPropertiesFormat.InboundNatPools = &natPools - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(loadBalancerID) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for the completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for the completion of Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { - return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", id.Name, id.ResourceGroup) } var natPoolId string @@ -177,26 +179,27 @@ func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta int } func resourceArmLoadBalancerNatPoolRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + id, err := parse.LoadBalancerInboundNATPoolID(d.Id()) if err != nil { return err } - name := id.Path["inboundNatPools"] - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.LoadBalancerName) return nil } - config, _, exists := FindLoadBalancerNatPoolByName(loadBalancer, name) + config, _, exists := FindLoadBalancerNatPoolByName(loadBalancer, id.Name) if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer Nat Pool %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer Nat Pool %q not found. Removing from state", id.Name) return nil } @@ -204,20 +207,38 @@ func resourceArmLoadBalancerNatPoolRead(d *schema.ResourceData, meta interface{} d.Set("resource_group_name", id.ResourceGroup) if props := config.InboundNatPoolPropertiesFormat; props != nil { - d.Set("protocol", props.Protocol) - d.Set("frontend_port_start", props.FrontendPortRangeStart) - d.Set("frontend_port_end", props.FrontendPortRangeEnd) - d.Set("backend_port", props.BackendPort) + backendPort := 0 + if props.BackendPort != nil { + backendPort = int(*props.BackendPort) + } + d.Set("backend_port", backendPort) - if feipConfig := props.FrontendIPConfiguration; feipConfig != nil { - fipID, err := azure.ParseAzureResourceID(*feipConfig.ID) + frontendIPConfigName := "" + frontendIPConfigID := "" + if props.FrontendIPConfiguration != nil && props.FrontendIPConfiguration.ID != nil { + feid, err := parse.LoadBalancerFrontendIPConfigurationID(*props.FrontendIPConfiguration.ID) if err != nil { return err } - d.Set("frontend_ip_configuration_name", fipID.Path["frontendIPConfigurations"]) - d.Set("frontend_ip_configuration_id", feipConfig.ID) + frontendIPConfigName = feid.Name + frontendIPConfigID = feid.ID(subscriptionId) + } + d.Set("frontend_ip_configuration_id", frontendIPConfigID) + d.Set("frontend_ip_configuration_name", frontendIPConfigName) + + frontendPortRangeEnd := 0 + if props.FrontendPortRangeEnd != nil { + frontendPortRangeEnd = int(*props.FrontendPortRangeEnd) } + d.Set("frontend_port_end", frontendPortRangeEnd) + + frontendPortRangeStart := 0 + if props.FrontendPortRangeStart != nil { + frontendPortRangeStart = int(*props.FrontendPortRangeStart) + } + d.Set("frontend_port_start", frontendPortRangeStart) + d.Set("protocol", string(props.Protocol)) } return nil @@ -225,14 +246,20 @@ func resourceArmLoadBalancerNatPoolRead(d *schema.ResourceData, meta interface{} func resourceArmLoadBalancerNatPoolDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + id, err := parse.LoadBalancerInboundNATPoolID(d.Id()) + if err != nil { + return err + } + + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + locks.ByID(loadBalancerId) + defer locks.UnlockByID(loadBalancerId) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) } @@ -241,7 +268,7 @@ func resourceArmLoadBalancerNatPoolDelete(d *schema.ResourceData, meta interface return nil } - _, index, exists := FindLoadBalancerNatPoolByName(loadBalancer, d.Get("name").(string)) + _, index, exists := FindLoadBalancerNatPoolByName(loadBalancer, id.Name) if !exists { return nil } @@ -250,26 +277,21 @@ func resourceArmLoadBalancerNatPoolDelete(d *schema.ResourceData, meta interface newNatPools := append(oldNatPools[:index], oldNatPools[index+1:]...) loadBalancer.LoadBalancerPropertiesFormat.InboundNatPools = &newNatPools - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error creating/updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error creating/updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of the Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for completion of the Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") if err != nil { return fmt.Errorf("Error retrieving Load Balancer: %+v", err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", id.LoadBalancerName, id.ResourceGroup) } return nil diff --git a/azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration.go b/azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration.go new file mode 100644 index 000000000000..7432e17df50f --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration.go @@ -0,0 +1,51 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerFrontendIPConfigurationId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func NewLoadBalancerFrontendIPConfigurationId(loadBalancer LoadBalancerId, name string) LoadBalancerFrontendIPConfigurationId { + return LoadBalancerFrontendIPConfigurationId{ + ResourceGroup: loadBalancer.ResourceGroup, + LoadBalancerName: loadBalancer.Name, + Name: name, + } +} + +func (id LoadBalancerFrontendIPConfigurationId) ID(subscriptionId string) string { + baseId := NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + return fmt.Sprintf("%s/frontendIPConfigurations/%s", baseId, id.Name) +} + +func LoadBalancerFrontendIPConfigurationID(input string) (*LoadBalancerFrontendIPConfigurationId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Frontend IP Configuration ID %q: %+v", input, err) + } + + frontendIPConfigurationId := LoadBalancerFrontendIPConfigurationId{ + ResourceGroup: id.ResourceGroup, + } + + if frontendIPConfigurationId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if frontendIPConfigurationId.Name, err = id.PopSegment("frontendIPConfigurations"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &frontendIPConfigurationId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration_test.go b/azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration_test.go new file mode 100644 index 000000000000..5f9a9ca5fc18 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_frontend_ip_configuration_test.go @@ -0,0 +1,81 @@ +package parse + +import ( + "testing" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/resourceid" +) + +var _ resourceid.Formatter = LoadBalancerFrontendIPConfigurationId{} + +func TestLoadBalancerFrontendIPConfigurationIDFormatter(t *testing.T) { + subscriptionId := "12345678-1234-5678-1234-123456789012" + loadBalancerId := NewLoadBalancerID("group1", "lb1") + actual := NewLoadBalancerFrontendIPConfigurationId(loadBalancerId, "config1").ID(subscriptionId) + expected := "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/frontendIPConfigurations/config1" + if actual != expected { + t.Fatalf("Expected %q but got %q", expected, actual) + } +} + +func TestLoadBalancerFrontendIPConfigurationIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerFrontendIPConfigurationId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // lower-case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/frontendipconfigurations/config1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/frontendIPConfigurations/config1", + expected: &LoadBalancerFrontendIPConfigurationId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "config1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Frontendipconfigurations/config1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1/FrontendIPConfigurations/config1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerFrontendIPConfigurationID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} diff --git a/azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool.go b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool.go new file mode 100644 index 000000000000..993f454fb445 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool.go @@ -0,0 +1,38 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerInboundNATPoolId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func LoadBalancerInboundNATPoolID(input string) (*LoadBalancerInboundNATPoolId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Inbound NAT Pool ID %q: %+v", input, err) + } + + natPoolId := LoadBalancerInboundNATPoolId{ + ResourceGroup: id.ResourceGroup, + } + + if natPoolId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if natPoolId.Name, err = id.PopSegment("inboundNatPools"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &natPoolId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool_test.go b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool_test.go new file mode 100644 index 000000000000..7d10d8ac6a90 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_pool_test.go @@ -0,0 +1,65 @@ +package parse + +import "testing" + +func TestLoadBalancerInboundNATPoolIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerInboundNATPoolId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // lower-case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/inboundnatpools/pool1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/inboundNatPools/pool1", + expected: &LoadBalancerInboundNATPoolId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "pool1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Inboundnatpools/pool1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1/InboundNatPools/pool1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerInboundNATPoolID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} From 9dc7517713c657c41bab97a563e4031b796f0c1a Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 13:16:06 +0200 Subject: [PATCH 04/15] r/lb_nat_rule: switching to use specific ID parsers --- .../services/network/lb_nat_rule_resource.go | 126 +++++++++++------- .../parse/load_balancer_inbound_nat_rule.go | 38 ++++++ .../load_balancer_inbound_nat_rule_test.go | 65 +++++++++ 3 files changed, 178 insertions(+), 51 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule_test.go diff --git a/azurerm/internal/services/network/lb_nat_rule_resource.go b/azurerm/internal/services/network/lb_nat_rule_resource.go index 3fe5fd230203..fba889fbf536 100644 --- a/azurerm/internal/services/network/lb_nat_rule_resource.go +++ b/azurerm/internal/services/network/lb_nat_rule_resource.go @@ -15,6 +15,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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/state" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -118,15 +119,20 @@ func resourceArmLoadBalancerNatRule() *schema.Resource { func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) + if err != nil { + return fmt.Errorf("retrieving Load Balancer Name and Group: %+v", err) + } + loadBalancerIdRaw := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerIdRaw) + defer locks.UnlockByID(loadBalancerIdRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIdRaw, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -136,7 +142,7 @@ func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta int return nil } - newNatRule, err := expandAzureRmLoadBalancerNatRule(d, loadBalancer) + newNatRule, err := expandAzureRmLoadBalancerNatRule(d, loadBalancer, *loadBalancerId, subscriptionId) if err != nil { return fmt.Errorf("Error Expanding NAT Rule: %+v", err) } @@ -156,27 +162,23 @@ func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta int } loadBalancer.LoadBalancerPropertiesFormat.InboundNatRules = &natRules - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(loadBalancerID) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group: %+v", err) - } - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating / Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating / Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { - return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", loadBalancerId.Name, loadBalancerId.ResourceGroup) } var natRuleId string @@ -196,26 +198,27 @@ func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta int } func resourceArmLoadBalancerNatRuleRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + id, err := parse.LoadBalancerInboundNATRuleID(d.Id()) if err != nil { return err } - name := id.Path["inboundNatRules"] - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.LoadBalancerName) return nil } - config, _, exists := FindLoadBalancerNatRuleByName(loadBalancer, name) + config, _, exists := FindLoadBalancerNatRuleByName(loadBalancer, id.Name) if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer Nat Rule %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer Nat Rule %q not found. Removing from state", id.Name) return nil } @@ -223,26 +226,46 @@ func resourceArmLoadBalancerNatRuleRead(d *schema.ResourceData, meta interface{} d.Set("resource_group_name", id.ResourceGroup) if props := config.InboundNatRulePropertiesFormat; props != nil { - d.Set("protocol", props.Protocol) - d.Set("frontend_port", props.FrontendPort) - d.Set("backend_port", props.BackendPort) + backendIPConfigId := "" + if props.BackendIPConfiguration != nil && props.BackendIPConfiguration.ID != nil { + backendIPConfigId = *props.BackendIPConfiguration.ID + } + d.Set("backend_ip_configuration_id", backendIPConfigId) + + backendPort := 0 + if props.BackendPort != nil { + backendPort = int(*props.BackendPort) + } + d.Set("backend_port", backendPort) d.Set("enable_floating_ip", props.EnableFloatingIP) d.Set("enable_tcp_reset", props.EnableTCPReset) - d.Set("idle_timeout_in_minutes", props.IdleTimeoutInMinutes) - if ipconfiguration := props.FrontendIPConfiguration; ipconfiguration != nil { - fipID, err := azure.ParseAzureResourceID(*ipconfiguration.ID) + frontendIPConfigName := "" + frontendIPConfigID := "" + if props.FrontendIPConfiguration != nil && props.FrontendIPConfiguration.ID != nil { + feid, err := parse.LoadBalancerFrontendIPConfigurationID(*props.FrontendIPConfiguration.ID) if err != nil { return err } - d.Set("frontend_ip_configuration_name", fipID.Path["frontendIPConfigurations"]) - d.Set("frontend_ip_configuration_id", ipconfiguration.ID) + frontendIPConfigName = feid.Name + frontendIPConfigID = feid.ID(subscriptionId) } + d.Set("frontend_ip_configuration_name", frontendIPConfigName) + d.Set("frontend_ip_configuration_id", frontendIPConfigID) - if ipconfiguration := props.BackendIPConfiguration; ipconfiguration != nil { - d.Set("backend_ip_configuration_id", ipconfiguration.ID) + frontendPort := 0 + if props.FrontendPort != nil { + frontendPort = int(*props.FrontendPort) } + d.Set("frontend_port", frontendPort) + + idleTimeoutInMinutes := 0 + if props.IdleTimeoutInMinutes != nil { + idleTimeoutInMinutes = int(*props.IdleTimeoutInMinutes) + } + d.Set("idle_timeout_in_minutes", idleTimeoutInMinutes) + d.Set("protocol", string(props.Protocol)) } return nil @@ -250,14 +273,20 @@ func resourceArmLoadBalancerNatRuleRead(d *schema.ResourceData, meta interface{} func resourceArmLoadBalancerNatRuleDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + id, err := parse.LoadBalancerInboundNATRuleID(d.Id()) + if err != nil { + return err + } + + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + locks.ByID(loadBalancerId) + defer locks.UnlockByID(loadBalancerId) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -266,7 +295,7 @@ func resourceArmLoadBalancerNatRuleDelete(d *schema.ResourceData, meta interface return nil } - _, index, exists := FindLoadBalancerNatRuleByName(loadBalancer, d.Get("name").(string)) + _, index, exists := FindLoadBalancerNatRuleByName(loadBalancer, id.Name) if !exists { return nil } @@ -275,32 +304,27 @@ func resourceArmLoadBalancerNatRuleDelete(d *schema.ResourceData, meta interface newNatRules := append(oldNatRules[:index], oldNatRules[index+1:]...) loadBalancer.LoadBalancerPropertiesFormat.InboundNatRules = &newNatRules - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group: %+v", err) - } - - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q) %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q) %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for the completion of Load Balancer updates for %q (Resource Group %q) %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for the completion of Load Balancer updates for %q (Resource Group %q) %+v", id.LoadBalancerName, id.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") if err != nil { - return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (resource group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %q (resource group %q) ID", id.LoadBalancerName, id.ResourceGroup) } return nil } -func expandAzureRmLoadBalancerNatRule(d *schema.ResourceData, lb *network.LoadBalancer) (*network.InboundNatRule, error) { +func expandAzureRmLoadBalancerNatRule(d *schema.ResourceData, lb *network.LoadBalancer, loadBalancerId parse.LoadBalancerId, subscriptionId string) (*network.InboundNatRule, error) { properties := network.InboundNatRulePropertiesFormat{ Protocol: network.TransportProtocol(d.Get("protocol").(string)), FrontendPort: utils.Int32(int32(d.Get("frontend_port").(int))), @@ -317,13 +341,13 @@ func expandAzureRmLoadBalancerNatRule(d *schema.ResourceData, lb *network.LoadBa } if v := d.Get("frontend_ip_configuration_name").(string); v != "" { - rule, exists := FindLoadBalancerFrontEndIpConfigurationByName(lb, v) - if !exists { + if _, exists := FindLoadBalancerFrontEndIpConfigurationByName(lb, v); !exists { return nil, fmt.Errorf("[ERROR] Cannot find FrontEnd IP Configuration with the name %s", v) } + id := parse.NewLoadBalancerFrontendIPConfigurationId(loadBalancerId, v).ID(subscriptionId) properties.FrontendIPConfiguration = &network.SubResource{ - ID: rule.ID, + ID: utils.String(id), } } diff --git a/azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule.go b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule.go new file mode 100644 index 000000000000..c725b76be651 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule.go @@ -0,0 +1,38 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerInboundNATRuleId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func LoadBalancerInboundNATRuleID(input string) (*LoadBalancerInboundNATRuleId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Inbound NAT Rule ID %q: %+v", input, err) + } + + natRuleId := LoadBalancerInboundNATRuleId{ + ResourceGroup: id.ResourceGroup, + } + + if natRuleId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if natRuleId.Name, err = id.PopSegment("inboundNatRules"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &natRuleId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule_test.go b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule_test.go new file mode 100644 index 000000000000..67464a34062c --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_inbound_nat_rule_test.go @@ -0,0 +1,65 @@ +package parse + +import "testing" + +func TestLoadBalancerInboundNATRuleIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerInboundNATRuleId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // lower-case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/inboundnatrules/rule1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/inboundNatRules/rule1", + expected: &LoadBalancerInboundNATRuleId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "rule1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Inboundnatrules/rule1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1/InboundNatRules/rule1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerInboundNATRuleID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} From 37133709b27fffcbafa8f2579b8f9142a92085ee Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 13:34:26 +0200 Subject: [PATCH 05/15] r/loadbalancer_outbound_rule: parsing using a consistent id func --- .../network/lb_outbound_rule_resource.go | 125 ++++++++++-------- .../load_balancer_backend_address_pool.go | 13 ++ ...load_balancer_backend_address_pool_test.go | 18 ++- .../parse/load_balancer_outbound_rule.go | 38 ++++++ .../parse/load_balancer_outbound_rule_test.go | 65 +++++++++ 5 files changed, 201 insertions(+), 58 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer_outbound_rule.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_outbound_rule_test.go diff --git a/azurerm/internal/services/network/lb_outbound_rule_resource.go b/azurerm/internal/services/network/lb_outbound_rule_resource.go index 31954c100db6..8be606b9b634 100644 --- a/azurerm/internal/services/network/lb_outbound_rule_resource.go +++ b/azurerm/internal/services/network/lb_outbound_rule_resource.go @@ -13,6 +13,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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -110,15 +111,20 @@ func resourceArmLoadBalancerOutboundRule() *schema.Resource { func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) + if err != nil { + return err + } + loadBalancerIDRaw := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerIDRaw) + defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIDRaw, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -130,7 +136,7 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met newOutboundRule, err := expandAzureRmLoadBalancerOutboundRule(d, loadBalancer) if err != nil { - return fmt.Errorf("Error Exanding Load Balancer Rule: %+v", err) + return fmt.Errorf("expanding Load Balancer Rule: %+v", err) } outboundRules := make([]network.OutboundRule, 0) @@ -154,12 +160,8 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met outboundRules = append(outboundRules, *newOutboundRule) loadBalancer.LoadBalancerPropertiesFormat.OutboundRules = &outboundRules - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(loadBalancerID) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, *loadBalancer) if err != nil { return fmt.Errorf("Error Creating/Updating LoadBalancer: %+v", err) } @@ -168,13 +170,13 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met return fmt.Errorf("Error waiting for completion for Load Balancer updates: %+v", err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { return fmt.Errorf("Error Getting LoadBalancer: %+v", err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %s (resource group %s) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %s (resource group %s) ID", loadBalancerId.Name, loadBalancerId.ResourceGroup) } var outboundRuleId string @@ -194,67 +196,75 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met } func resourceArmLoadBalancerOutboundRuleRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + id, err := parse.LoadBalancerOutboundRuleID(d.Id()) if err != nil { return err } - name := id.Path["outboundRules"] - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.LoadBalancerName) return nil } - config, _, exists := FindLoadBalancerOutboundRuleByName(loadBalancer, name) + config, _, exists := FindLoadBalancerOutboundRuleByName(loadBalancer, id.Name) if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer Outbound Rule %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer Outbound Rule %q not found. Removing from state", id.Name) return nil } d.Set("name", config.Name) d.Set("resource_group_name", id.ResourceGroup) - if properties := config.OutboundRulePropertiesFormat; properties != nil { - d.Set("protocol", properties.Protocol) - d.Set("backend_address_pool_id", properties.BackendAddressPool.ID) + if props := config.OutboundRulePropertiesFormat; props != nil { + allocatedOutboundPorts := 0 + if props.AllocatedOutboundPorts != nil { + allocatedOutboundPorts = int(*props.AllocatedOutboundPorts) + } + d.Set("allocated_outbound_ports", allocatedOutboundPorts) + + backendAddressPoolId := "" + if props.BackendAddressPool != nil && props.BackendAddressPool.ID != nil { + bapid, err := parse.LoadBalancerBackendAddressPoolID(*props.BackendAddressPool.ID) + if err != nil { + return err + } + + backendAddressPoolId = bapid.ID(subscriptionId) + } + d.Set("backend_address_pool_id", backendAddressPoolId) + d.Set("enable_tcp_reset", props.EnableTCPReset) frontendIpConfigurations := make([]interface{}, 0) - for _, feConfig := range *properties.FrontendIPConfigurations { + for _, feConfig := range *props.FrontendIPConfigurations { if feConfig.ID == nil { continue } - - feConfigId, err := azure.ParseAzureResourceID(*feConfig.ID) + feid, err := parse.LoadBalancerFrontendIPConfigurationID(*feConfig.ID) if err != nil { - return nil + return err } - name := feConfigId.Path["frontendIPConfigurations"] - frontendConfiguration := map[string]interface{}{ - "id": *feConfig.ID, - "name": name, - } - frontendIpConfigurations = append(frontendIpConfigurations, frontendConfiguration) + frontendIpConfigurations = append(frontendIpConfigurations, map[string]interface{}{ + "id": feid.ID(subscriptionId), + "name": feid.Name, + }) } d.Set("frontend_ip_configuration", frontendIpConfigurations) - if properties.EnableTCPReset != nil { - d.Set("enable_tcp_reset", properties.EnableTCPReset) - } - - if properties.IdleTimeoutInMinutes != nil { - d.Set("idle_timeout_in_minutes", properties.IdleTimeoutInMinutes) - } - - if properties.AllocatedOutboundPorts != nil { - d.Set("allocated_outbound_ports", properties.AllocatedOutboundPorts) + idleTimeoutInMinutes := 0 + if props.IdleTimeoutInMinutes != nil { + idleTimeoutInMinutes = int(*props.IdleTimeoutInMinutes) } + d.Set("idle_timeout_in_minutes", idleTimeoutInMinutes) + d.Set("protocol", string(props.Protocol)) } return nil @@ -262,23 +272,29 @@ func resourceArmLoadBalancerOutboundRuleRead(d *schema.ResourceData, meta interf func resourceArmLoadBalancerOutboundRuleDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + id, err := parse.LoadBalancerOutboundRuleID(d.Id()) + if err != nil { + return err + } + + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + locks.ByID(loadBalancerId) + defer locks.UnlockByID(loadBalancerId) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { - return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) + return fmt.Errorf("retrieving Load Balancer By ID: %+v", err) } if !exists { d.SetId("") return nil } - _, index, exists := FindLoadBalancerOutboundRuleByName(loadBalancer, d.Get("name").(string)) + _, index, exists := FindLoadBalancerOutboundRuleByName(loadBalancer, id.Name) if !exists { return nil } @@ -287,26 +303,21 @@ func resourceArmLoadBalancerOutboundRuleDelete(d *schema.ResourceData, meta inte newOutboundRules := append(oldOutboundRules[:index], oldOutboundRules[index+1:]...) loadBalancer.LoadBalancerPropertiesFormat.OutboundRules = &newOutboundRules - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") if err != nil { return fmt.Errorf("Error Getting LoadBalancer: %+v", err) } if read.ID == nil { - return fmt.Errorf("Cannot read ID of Load Balancer %q (resource group %s)", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read ID of Load Balancer %q (resource group %s)", id.LoadBalancerName, id.ResourceGroup) } return nil diff --git a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go index c8a626dd3690..422a97e6eb1b 100644 --- a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go +++ b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go @@ -12,6 +12,19 @@ type LoadBalancerBackendAddressPoolId struct { Name string } +func (id LoadBalancerBackendAddressPoolId) ID(subscriptionId string) string { + baseId := NewLoadBalancerID(id.ResourceGroup, id.Name).ID(subscriptionId) + return fmt.Sprintf("%s/backendAddressPools/%s", baseId, id.Name) +} + +func NewLoadBalancerBackendAddressPoolId(loadBalancerId LoadBalancerId, name string) LoadBalancerBackendAddressPoolId { + return LoadBalancerBackendAddressPoolId{ + ResourceGroup: loadBalancerId.ResourceGroup, + LoadBalancerName: loadBalancerId.Name, + Name: name, + } +} + func LoadBalancerBackendAddressPoolID(input string) (*LoadBalancerBackendAddressPoolId, error) { id, err := azure.ParseAzureResourceID(input) if err != nil { diff --git a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go index 6a88454c4b42..f3a1ec50fd10 100644 --- a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go +++ b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool_test.go @@ -1,6 +1,22 @@ package parse -import "testing" +import ( + "testing" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/resourceid" +) + +var _ resourceid.Formatter = LoadBalancerBackendAddressPoolId{} + +func TestLoadBalancerBackendAddressPoolIDFormatter(t *testing.T) { + subscriptionId := "12345678-1234-5678-1234-123456789012" + loadBalancerId := NewLoadBalancerID("group1", "lb1") + actual := NewLoadBalancerBackendAddressPoolId(loadBalancerId, "pool1").ID(subscriptionId) + expected := "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool1" + if actual != expected { + t.Fatalf("Expected %q but got %q", expected, actual) + } +} func TestLoadBalancerBackendAddressPoolIDParser(t *testing.T) { testData := []struct { diff --git a/azurerm/internal/services/network/parse/load_balancer_outbound_rule.go b/azurerm/internal/services/network/parse/load_balancer_outbound_rule.go new file mode 100644 index 000000000000..945557763c5a --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_outbound_rule.go @@ -0,0 +1,38 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerOutboundRuleId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func LoadBalancerOutboundRuleID(input string) (*LoadBalancerOutboundRuleId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Outbound Rule ID %q: %+v", input, err) + } + + outboundRuleId := LoadBalancerOutboundRuleId{ + ResourceGroup: id.ResourceGroup, + } + + if outboundRuleId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if outboundRuleId.Name, err = id.PopSegment("outboundRules"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &outboundRuleId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_outbound_rule_test.go b/azurerm/internal/services/network/parse/load_balancer_outbound_rule_test.go new file mode 100644 index 000000000000..58baca3a4ab6 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_outbound_rule_test.go @@ -0,0 +1,65 @@ +package parse + +import "testing" + +func TestLoadBalancerOutboundRuleIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerOutboundRuleId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // lower-case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/outboundrules/rule1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/outboundRules/rule1", + expected: &LoadBalancerOutboundRuleId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "rule1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Outboundrules/rule1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1/OutboundRules/rule1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerOutboundRuleID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} From 35c4f6a890c9c3bc90dfd827a7f6688e6f799e06 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 13:46:20 +0200 Subject: [PATCH 06/15] r/loadbalancer_probe: parsing using a consistent method --- .../services/network/lb_probe_resource.go | 105 +++++++++++------- .../load_balancer_backend_address_pool.go | 2 +- .../network/parse/load_balancer_probe.go | 38 +++++++ .../network/parse/load_balancer_probe_test.go | 55 +++++++++ 4 files changed, 156 insertions(+), 44 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer_probe.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_probe_test.go diff --git a/azurerm/internal/services/network/lb_probe_resource.go b/azurerm/internal/services/network/lb_probe_resource.go index d8a8f0970876..80fd6b20a612 100644 --- a/azurerm/internal/services/network/lb_probe_resource.go +++ b/azurerm/internal/services/network/lb_probe_resource.go @@ -15,6 +15,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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -104,15 +105,20 @@ func resourceArmLoadBalancerProbe() *schema.Resource { func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) + if err != nil { + return err + } + loadBalancerIDRaw := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerIDRaw) + defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIDRaw, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -138,26 +144,22 @@ func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta inter } loadBalancer.LoadBalancerPropertiesFormat.Probes = &probes - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(loadBalancerID) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group: %+v", err) - } - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { - return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (resource group %q) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %q (resource group %q) ID", loadBalancerId.Name, loadBalancerId.ResourceGroup) } var createdProbeId string @@ -177,41 +179,57 @@ func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta inter } func resourceArmLoadBalancerProbeRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + id, err := parse.LoadBalancerProbeID(d.Id()) if err != nil { return err } - name := id.Path["probes"] - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.LoadBalancerName) return nil } - config, _, exists := FindLoadBalancerProbeByName(loadBalancer, name) + config, _, exists := FindLoadBalancerProbeByName(loadBalancer, id.Name) if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer Probe %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer Probe %q not found. Removing from state", id.Name) return nil } d.Set("name", config.Name) d.Set("resource_group_name", id.ResourceGroup) - if properties := config.ProbePropertiesFormat; properties != nil { - d.Set("protocol", properties.Protocol) - d.Set("interval_in_seconds", properties.IntervalInSeconds) - d.Set("number_of_probes", properties.NumberOfProbes) - d.Set("port", properties.Port) - d.Set("request_path", properties.RequestPath) + if props := config.ProbePropertiesFormat; props != nil { + intervalInSeconds := 0 + if props.IntervalInSeconds != nil { + intervalInSeconds = int(*props.IntervalInSeconds) + } + d.Set("interval_in_seconds", intervalInSeconds) + + numberOfProbes := 0 + if props.NumberOfProbes != nil { + numberOfProbes = int(*props.NumberOfProbes) + } + d.Set("number_of_probes", numberOfProbes) + + port := 0 + if props.Port != nil { + port = int(*props.Port) + } + d.Set("port", port) + d.Set("protocol", string(props.Protocol)) + d.Set("request_path", props.RequestPath) + // TODO: parse/make these consistent var loadBalancerRules []string - if rules := properties.LoadBalancingRules; rules != nil { + if rules := props.LoadBalancingRules; rules != nil { for _, ruleConfig := range *rules { if id := ruleConfig.ID; id != nil { loadBalancerRules = append(loadBalancerRules, *id) @@ -219,7 +237,7 @@ func resourceArmLoadBalancerProbeRead(d *schema.ResourceData, meta interface{}) } } if err := d.Set("load_balancer_rules", loadBalancerRules); err != nil { - return fmt.Errorf("Error setting `load_balancer_rules` (Load Balancer Probe %q): %+v", name, err) + return fmt.Errorf("Error setting `load_balancer_rules` (Load Balancer Probe %q): %+v", id.Name, err) } } @@ -228,14 +246,20 @@ func resourceArmLoadBalancerProbeRead(d *schema.ResourceData, meta interface{}) func resourceArmLoadBalancerProbeDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + id, err := parse.LoadBalancerProbeID(d.Id()) + if err != nil { + return err + } - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + locks.ByID(loadBalancerId) + defer locks.UnlockByID(loadBalancerId) + + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -244,7 +268,7 @@ func resourceArmLoadBalancerProbeDelete(d *schema.ResourceData, meta interface{} return nil } - _, index, exists := FindLoadBalancerProbeByName(loadBalancer, d.Get("name").(string)) + _, index, exists := FindLoadBalancerProbeByName(loadBalancer, id.Name) if !exists { return nil } @@ -253,26 +277,21 @@ func resourceArmLoadBalancerProbeDelete(d *schema.ResourceData, meta interface{} newProbes := append(oldProbes[:index], oldProbes[index+1:]...) loadBalancer.LoadBalancerPropertiesFormat.Probes = &newProbes - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") if err != nil { return fmt.Errorf("Error Getting LoadBalancer: %+v", err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %s (resource group %s) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %s (resource group %s) ID", id.LoadBalancerName, id.ResourceGroup) } return nil diff --git a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go index 422a97e6eb1b..ea19aa0b8de3 100644 --- a/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go +++ b/azurerm/internal/services/network/parse/load_balancer_backend_address_pool.go @@ -13,7 +13,7 @@ type LoadBalancerBackendAddressPoolId struct { } func (id LoadBalancerBackendAddressPoolId) ID(subscriptionId string) string { - baseId := NewLoadBalancerID(id.ResourceGroup, id.Name).ID(subscriptionId) + baseId := NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) return fmt.Sprintf("%s/backendAddressPools/%s", baseId, id.Name) } diff --git a/azurerm/internal/services/network/parse/load_balancer_probe.go b/azurerm/internal/services/network/parse/load_balancer_probe.go new file mode 100644 index 000000000000..d67c2b1996c2 --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_probe.go @@ -0,0 +1,38 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerProbeId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func LoadBalancerProbeID(input string) (*LoadBalancerProbeId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Probe ID %q: %+v", input, err) + } + + probeId := LoadBalancerProbeId{ + ResourceGroup: id.ResourceGroup, + } + + if probeId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if probeId.Name, err = id.PopSegment("probes"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &probeId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_probe_test.go b/azurerm/internal/services/network/parse/load_balancer_probe_test.go new file mode 100644 index 000000000000..9f162cb2d51d --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_probe_test.go @@ -0,0 +1,55 @@ +package parse + +import "testing" + +func TestLoadBalancerProbeIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerProbeId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/probes/probe1", + expected: &LoadBalancerProbeId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "probe1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Probes/probe1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerProbeID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} From f5d0df0e6f756e4e2f78f8b5df4c06a3bc28c2f5 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 13:58:14 +0200 Subject: [PATCH 07/15] r/loadbalancer_rule: switching to use a consistent id parsing method --- .../services/network/lb_rule_resource.go | 125 +++++++++++------- .../network/parse/load_balancer_rule.go | 38 ++++++ .../network/parse/load_balancer_rule_test.go | 65 +++++++++ 3 files changed, 181 insertions(+), 47 deletions(-) create mode 100644 azurerm/internal/services/network/parse/load_balancer_rule.go create mode 100644 azurerm/internal/services/network/parse/load_balancer_rule_test.go diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index 1276b673f7bf..4ec5a1afba70 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -136,11 +137,17 @@ func resourceArmLoadBalancerRule() *schema.Resource { func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) - loadBalancerID := d.Get("loadbalancer_id").(string) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) + if err != nil { + return err + } + + loadBalancerID := loadBalancerId.ID(subscriptionId) locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) @@ -174,12 +181,8 @@ func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interf } loadBalancer.LoadBalancerPropertiesFormat.LoadBalancingRules = &lbRules - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(loadBalancerID) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, *loadBalancer) if err != nil { return fmt.Errorf("Error Creating/Updating LoadBalancer: %+v", err) } @@ -188,12 +191,12 @@ func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interf return fmt.Errorf("Error waiting for completion for Load Balancer updates: %+v", err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { return fmt.Errorf("Error Getting LoadBalancer: %+v", err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %s (resource group %s) ID", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read Load Balancer %s (resource group %s) ID", loadBalancerId.Name, loadBalancerId.ResourceGroup) } var ruleId string @@ -213,62 +216,88 @@ func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interf } func resourceArmLoadBalancerRuleRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + id, err := parse.LoadBalancerRuleID(d.Id()) if err != nil { return err } - name := id.Path["loadBalancingRules"] - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.LoadBalancerName) return nil } - config, _, exists := FindLoadBalancerRuleByName(loadBalancer, name) + config, _, exists := FindLoadBalancerRuleByName(loadBalancer, id.Name) if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer Rule %q not found. Removing from state", name) + log.Printf("[INFO] Load Balancer Rule %q not found. Removing from state", id.Name) return nil } d.Set("name", config.Name) d.Set("resource_group_name", id.ResourceGroup) - if properties := config.LoadBalancingRulePropertiesFormat; properties != nil { - d.Set("protocol", properties.Protocol) - d.Set("frontend_port", properties.FrontendPort) - d.Set("backend_port", properties.BackendPort) - d.Set("disable_outbound_snat", properties.DisableOutboundSnat) - d.Set("enable_floating_ip", properties.EnableFloatingIP) - d.Set("enable_tcp_reset", properties.EnableTCPReset) - d.Set("idle_timeout_in_minutes", properties.IdleTimeoutInMinutes) - - if properties.FrontendIPConfiguration != nil { - fipID, err := azure.ParseAzureResourceID(*properties.FrontendIPConfiguration.ID) + if props := config.LoadBalancingRulePropertiesFormat; props != nil { + d.Set("disable_outbound_snat", props.DisableOutboundSnat) + d.Set("enable_floating_ip", props.EnableFloatingIP) + d.Set("enable_tcp_reset", props.EnableTCPReset) + d.Set("protocol", string(props.Protocol)) + + backendPort := 0 + if props.BackendPort != nil { + backendPort = int(*props.BackendPort) + } + d.Set("backend_port", backendPort) + + backendAddressPoolId := "" + if props.BackendAddressPool != nil && props.BackendAddressPool.ID != nil { + backendAddressPoolId = *props.BackendAddressPool.ID + } + d.Set("backend_address_pool_id", backendAddressPoolId) + + frontendIPConfigName := "" + frontendIPConfigID := "" + if props.FrontendIPConfiguration != nil && props.FrontendIPConfiguration.ID != nil { + feid, err := parse.LoadBalancerFrontendIPConfigurationID(*props.FrontendIPConfiguration.ID) if err != nil { return err } - d.Set("frontend_ip_configuration_name", fipID.Path["frontendIPConfigurations"]) - d.Set("frontend_ip_configuration_id", properties.FrontendIPConfiguration.ID) + frontendIPConfigName = feid.Name + frontendIPConfigID = feid.ID(subscriptionId) } + d.Set("frontend_ip_configuration_name", frontendIPConfigName) + d.Set("frontend_ip_configuration_id", frontendIPConfigID) - if properties.BackendAddressPool != nil { - d.Set("backend_address_pool_id", properties.BackendAddressPool.ID) + frontendPort := 0 + if props.FrontendPort != nil { + frontendPort = int(*props.FrontendPort) } + d.Set("frontend_port", frontendPort) - if properties.Probe != nil { - d.Set("probe_id", properties.Probe.ID) + idleTimeoutInMinutes := 0 + if props.IdleTimeoutInMinutes != nil { + idleTimeoutInMinutes = int(*props.IdleTimeoutInMinutes) } + d.Set("idle_timeout_in_minutes", idleTimeoutInMinutes) - if properties.LoadDistribution != "" { - d.Set("load_distribution", properties.LoadDistribution) + loadDistribution := "" + if props.LoadDistribution != "" { + loadDistribution = string(props.LoadDistribution) } + d.Set("load_distribution", loadDistribution) + + probeId := "" + if props.Probe != nil && props.Probe.ID != nil { + probeId = *props.Probe.ID + } + d.Set("probe_id", probeId) } return nil @@ -276,14 +305,20 @@ func resourceArmLoadBalancerRuleRead(d *schema.ResourceData, meta interface{}) e func resourceArmLoadBalancerRuleDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - loadBalancerID := d.Get("loadbalancer_id").(string) - locks.ByID(loadBalancerID) - defer locks.UnlockByID(loadBalancerID) + id, err := parse.LoadBalancerRuleID(d.Id()) + if err != nil { + return err + } - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + locks.ByID(loadBalancerId) + defer locks.UnlockByID(loadBalancerId) + + loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -301,26 +336,21 @@ func resourceArmLoadBalancerRuleDelete(d *schema.ResourceData, meta interface{}) newLbRules := append(oldLbRules[:index], oldLbRules[index+1:]...) loadBalancer.LoadBalancerPropertiesFormat.LoadBalancingRules = &newLbRules - resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) - if err != nil { - return fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) - } - - future, err := client.CreateOrUpdate(ctx, resGroup, loadBalancerName, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerName, resGroup, err) + return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } - read, err := client.Get(ctx, resGroup, loadBalancerName, "") + read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") if err != nil { return fmt.Errorf("Error Getting LoadBalancer: %+v", err) } if read.ID == nil { - return fmt.Errorf("Cannot read ID of Load Balancer %q (resource group %s)", loadBalancerName, resGroup) + return fmt.Errorf("Cannot read ID of Load Balancer %q (resource group %s)", id.LoadBalancerName, id.ResourceGroup) } return nil @@ -344,6 +374,7 @@ func expandAzureRmLoadBalancerRule(d *schema.ResourceData, lb *network.LoadBalan properties.LoadDistribution = network.LoadDistribution(v) } + // TODO: ensure these ID's are consistent if v := d.Get("frontend_ip_configuration_name").(string); v != "" { rule, exists := FindLoadBalancerFrontEndIpConfigurationByName(lb, v) if !exists { diff --git a/azurerm/internal/services/network/parse/load_balancer_rule.go b/azurerm/internal/services/network/parse/load_balancer_rule.go new file mode 100644 index 000000000000..187c4870af8c --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_rule.go @@ -0,0 +1,38 @@ +package parse + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type LoadBalancerRuleId struct { + ResourceGroup string + LoadBalancerName string + Name string +} + +func LoadBalancerRuleID(input string) (*LoadBalancerRuleId, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("parsing Load Balancer Rule ID %q: %+v", input, err) + } + + ruleId := LoadBalancerRuleId{ + ResourceGroup: id.ResourceGroup, + } + + if ruleId.LoadBalancerName, err = id.PopSegment("loadBalancers"); err != nil { + return nil, err + } + + if ruleId.Name, err = id.PopSegment("loadBalancingRules"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &ruleId, nil +} diff --git a/azurerm/internal/services/network/parse/load_balancer_rule_test.go b/azurerm/internal/services/network/parse/load_balancer_rule_test.go new file mode 100644 index 000000000000..f435b88c7f4b --- /dev/null +++ b/azurerm/internal/services/network/parse/load_balancer_rule_test.go @@ -0,0 +1,65 @@ +package parse + +import "testing" + +func TestLoadBalancerRuleIDParser(t *testing.T) { + testData := []struct { + input string + expected *LoadBalancerRuleId + }{ + { + // load balancer id + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1", + expected: nil, + }, + { + // lower-case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/loadbalancingrules/rule1", + expected: nil, + }, + { + // camel case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/loadBalancers/lb1/loadBalancingRules/rule1", + expected: &LoadBalancerRuleId{ + ResourceGroup: "group1", + LoadBalancerName: "lb1", + Name: "rule1", + }, + }, + { + // title case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/Loadbalancers/lb1/Loadbalancingrules/rule1", + expected: nil, + }, + { + // pascal case + input: "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/group1/providers/Microsoft.Network/LoadBalancers/lb1/LoadBalancingRules/rule1", + expected: nil, + }, + } + for _, test := range testData { + t.Logf("Testing %q..", test.input) + actual, err := LoadBalancerRuleID(test.input) + if err != nil && test.expected == nil { + continue + } else { + if err == nil && test.expected == nil { + t.Fatalf("Expected an error but didn't get one") + } else if err != nil && test.expected != nil { + t.Fatalf("Expected no error but got: %+v", err) + } + } + + if actual.ResourceGroup != test.expected.ResourceGroup { + t.Fatalf("Expected ResourceGroup to be %q but was %q", test.expected.ResourceGroup, actual.ResourceGroup) + } + + if actual.LoadBalancerName != test.expected.LoadBalancerName { + t.Fatalf("Expected LoadBalancerName to be %q but was %q", test.expected.LoadBalancerName, actual.LoadBalancerName) + } + + if actual.Name != test.expected.Name { + t.Fatalf("Expected name to be %q but was %q", test.expected.Name, actual.Name) + } + } +} From c15702cacf1242e1e07933e4150334b40022450c Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 13:59:37 +0200 Subject: [PATCH 08/15] refactor: removing the legacy parseID method --- .../internal/services/network/loadbalancer.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/azurerm/internal/services/network/loadbalancer.go b/azurerm/internal/services/network/loadbalancer.go index 20f52e71103b..9f18d85199e3 100644 --- a/azurerm/internal/services/network/loadbalancer.go +++ b/azurerm/internal/services/network/loadbalancer.go @@ -16,32 +16,22 @@ import ( // TODO: refactor this -// Deprecated: use `parse.LoadBalancerID` -func resourceGroupAndLBNameFromId(loadBalancerId string) (string, string, error) { - id, err := parse.LoadBalancerID(loadBalancerId) - if err != nil { - return "", "", err - } - - return id.ResourceGroup, id.Name, nil -} - func retrieveLoadBalancerById(d *schema.ResourceData, loadBalancerId string, meta interface{}) (*network.LoadBalancer, bool, error) { client := meta.(*clients.Client).Network.LoadBalancersClient ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - resGroup, name, err := resourceGroupAndLBNameFromId(loadBalancerId) + id, err := parse.LoadBalancerID(loadBalancerId) if err != nil { - return nil, false, fmt.Errorf("Error Getting Load Balancer Name and Group:: %+v", err) + return nil, false, err } - resp, err := client.Get(ctx, resGroup, name, "") + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { return nil, false, nil } - return nil, false, fmt.Errorf("Error making Read request on Azure Load Balancer %s: %s", name, err) + return nil, false, fmt.Errorf("retrieving Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } return &resp, true, nil From 59fe7d676cb7575c701ccf8a858890318dfbdcd8 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 14:13:33 +0200 Subject: [PATCH 09/15] refactor: retrieveLoadBalancerById now takes the ID --- .../lb_backend_address_pool_data_source.go | 14 +++++--- .../lb_backend_address_pool_resource.go | 21 ++++++------ .../services/network/lb_nat_pool_resource.go | 34 ++++++++++--------- .../services/network/lb_nat_rule_resource.go | 11 +++--- .../network/lb_outbound_rule_resource.go | 11 +++--- .../services/network/lb_probe_resource.go | 12 +++---- .../internal/services/network/lb_resource.go | 19 +++++------ .../services/network/lb_rule_resource.go | 11 +++--- .../internal/services/network/loadbalancer.go | 11 ++---- 9 files changed, 74 insertions(+), 70 deletions(-) diff --git a/azurerm/internal/services/network/lb_backend_address_pool_data_source.go b/azurerm/internal/services/network/lb_backend_address_pool_data_source.go index 2d9d741c1d59..30969f8b7885 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_data_source.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_data_source.go @@ -7,6 +7,7 @@ import ( "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/internal/services/network/parse" ) func dataSourceArmLoadBalancerBackendAddressPool() *schema.Resource { @@ -47,20 +48,23 @@ func dataSourceArmLoadBalancerBackendAddressPool() *schema.Resource { } func dataSourceArmLoadBalancerBackendAddressPoolRead(d *schema.ResourceData, meta interface{}) error { - loadBalancerID := d.Get("loadbalancer_id").(string) name := d.Get("name").(string) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) + if err != nil { + return err + } - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Get("loadbalancer_id").(string), meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { - return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) + return fmt.Errorf("retrieving Load Balancer by ID: %+v", err) } if !exists { - return fmt.Errorf("Unable to retrieve Backend Address Pool %q since Load Balancer %q was not found", name, loadBalancerID) + return fmt.Errorf("Load Balancer %q (Resource Group %q) was not found", loadBalancerId.Name, loadBalancerId.ResourceGroup) } bap, _, exists := FindLoadBalancerBackEndAddressPoolByName(loadBalancer, name) if !exists { - return fmt.Errorf("Backend Address Pool %q was not found in Load Balancer %q", name, loadBalancerID) + return fmt.Errorf("Backend Address Pool %q was not found in Load Balancer %q (Resource Group %q)", name, loadBalancerId.Name, loadBalancerId.ResourceGroup) } d.SetId(*bap.ID) diff --git a/azurerm/internal/services/network/lb_backend_address_pool_resource.go b/azurerm/internal/services/network/lb_backend_address_pool_resource.go index d9d274fae454..df9f1455d54a 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_resource.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_resource.go @@ -76,20 +76,21 @@ func resourceArmLoadBalancerBackendAddressPool() *schema.Resource { func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) - loadBalancerIdRaw := d.Get("loadbalancer_id").(string) - loadBalancerId, err := parse.LoadBalancerID(loadBalancerIdRaw) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) if err != nil { return fmt.Errorf("parsing Load Balancer Name and Group: %+v", err) } - locks.ByID(loadBalancerIdRaw) - defer locks.UnlockByID(loadBalancerIdRaw) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIdRaw, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -150,13 +151,12 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met } func resourceArmLoadBalancerBackendAddressPoolRead(d *schema.ResourceData, meta interface{}) error { - subscriptionId := meta.(*clients.Client).Account.SubscriptionId id, err := parse.LoadBalancerBackendAddressPoolID(d.Id()) if err != nil { return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("retrieving Load Balancer by ID: %+v", err) @@ -211,9 +211,10 @@ func resourceArmLoadBalancerBackendAddressPoolDelete(d *schema.ResourceData, met return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) - locks.ByID(loadBalancerId) - defer locks.UnlockByID(loadBalancerId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { diff --git a/azurerm/internal/services/network/lb_nat_pool_resource.go b/azurerm/internal/services/network/lb_nat_pool_resource.go index c6dcf40b3dfd..2f12d1307def 100644 --- a/azurerm/internal/services/network/lb_nat_pool_resource.go +++ b/azurerm/internal/services/network/lb_nat_pool_resource.go @@ -101,26 +101,27 @@ func resourceArmLoadBalancerNatPool() *schema.Resource { func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.LoadBalancersClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) - loadBalancerIdRaw := d.Get("loadbalancer_id").(string) - id, err := parse.LoadBalancerID(loadBalancerIdRaw) + loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) if err != nil { return fmt.Errorf("parsing Load Balancer Name and Group: %+v", err) } - locks.ByID(loadBalancerIdRaw) - defer locks.UnlockByID(loadBalancerIdRaw) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIdRaw, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { return fmt.Errorf("retrieving Load Balancer By ID: %+v", err) } if !exists { d.SetId("") - log.Printf("[INFO] Load Balancer %q not found. Removing from state", id.Name) + log.Printf("[INFO] Load Balancer %q not found. Removing from state", loadBalancerId.Name) return nil } @@ -145,21 +146,21 @@ func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta int loadBalancer.LoadBalancerPropertiesFormat.InboundNatPools = &natPools - future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, *loadBalancer) + future, err := client.CreateOrUpdate(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for the completion of Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + return fmt.Errorf("Error waiting for the completion of Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } - read, err := client.Get(ctx, id.ResourceGroup, id.Name, "") + read, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { - return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + return fmt.Errorf("Error retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } if read.ID == nil { - return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", id.Name, id.ResourceGroup) + return fmt.Errorf("Cannot read Load Balancer %q (Resource Group %q) ID", loadBalancerId.Name, loadBalancerId.ResourceGroup) } var natPoolId string @@ -185,7 +186,7 @@ func resourceArmLoadBalancerNatPoolRead(d *schema.ResourceData, meta interface{} return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) @@ -255,9 +256,10 @@ func resourceArmLoadBalancerNatPoolDelete(d *schema.ResourceData, meta interface return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) - locks.ByID(loadBalancerId) - defer locks.UnlockByID(loadBalancerId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { diff --git a/azurerm/internal/services/network/lb_nat_rule_resource.go b/azurerm/internal/services/network/lb_nat_rule_resource.go index fba889fbf536..166db87a0a26 100644 --- a/azurerm/internal/services/network/lb_nat_rule_resource.go +++ b/azurerm/internal/services/network/lb_nat_rule_resource.go @@ -132,7 +132,7 @@ func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta int locks.ByID(loadBalancerIdRaw) defer locks.UnlockByID(loadBalancerIdRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIdRaw, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -204,7 +204,7 @@ func resourceArmLoadBalancerNatRuleRead(d *schema.ResourceData, meta interface{} return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) @@ -282,9 +282,10 @@ func resourceArmLoadBalancerNatRuleDelete(d *schema.ResourceData, meta interface return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) - locks.ByID(loadBalancerId) - defer locks.UnlockByID(loadBalancerId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { diff --git a/azurerm/internal/services/network/lb_outbound_rule_resource.go b/azurerm/internal/services/network/lb_outbound_rule_resource.go index 8be606b9b634..f8c6b896d311 100644 --- a/azurerm/internal/services/network/lb_outbound_rule_resource.go +++ b/azurerm/internal/services/network/lb_outbound_rule_resource.go @@ -124,7 +124,7 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met locks.ByID(loadBalancerIDRaw) defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIDRaw, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -202,7 +202,7 @@ func resourceArmLoadBalancerOutboundRuleRead(d *schema.ResourceData, meta interf return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) @@ -281,9 +281,10 @@ func resourceArmLoadBalancerOutboundRuleDelete(d *schema.ResourceData, meta inte return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) - locks.ByID(loadBalancerId) - defer locks.UnlockByID(loadBalancerId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { diff --git a/azurerm/internal/services/network/lb_probe_resource.go b/azurerm/internal/services/network/lb_probe_resource.go index 80fd6b20a612..31f9bdc4bb5f 100644 --- a/azurerm/internal/services/network/lb_probe_resource.go +++ b/azurerm/internal/services/network/lb_probe_resource.go @@ -118,7 +118,7 @@ func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta inter locks.ByID(loadBalancerIDRaw) defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerIDRaw, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -179,13 +179,12 @@ func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta inter } func resourceArmLoadBalancerProbeRead(d *schema.ResourceData, meta interface{}) error { - subscriptionId := meta.(*clients.Client).Account.SubscriptionId id, err := parse.LoadBalancerProbeID(d.Id()) if err != nil { return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) @@ -255,9 +254,10 @@ func resourceArmLoadBalancerProbeDelete(d *schema.ResourceData, meta interface{} return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) - locks.ByID(loadBalancerId) - defer locks.UnlockByID(loadBalancerId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + loadBalancerID := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerID) + defer locks.UnlockByID(loadBalancerID) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { diff --git a/azurerm/internal/services/network/lb_resource.go b/azurerm/internal/services/network/lb_resource.go index 44458361e8a0..5f4c60d023b7 100644 --- a/azurerm/internal/services/network/lb_resource.go +++ b/azurerm/internal/services/network/lb_resource.go @@ -13,6 +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/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/state" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" @@ -196,7 +197,7 @@ func resourceArmLoadBalancerCreateUpdate(d *schema.ResourceData, meta interface{ existing, err := client.Get(ctx, resGroup, name, "") if err != nil { if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("Error checking for presence of existing Load Balancer %q (Resource Group %q): %s", name, resGroup, err) + return fmt.Errorf("checking for presence of existing Load Balancer %q (Resource Group %q): %s", name, resGroup, err) } } @@ -249,12 +250,12 @@ func resourceArmLoadBalancerCreateUpdate(d *schema.ResourceData, meta interface{ } func resourceArmLoadBalancerRead(d *schema.ResourceData, meta interface{}) error { - id, err := azure.ParseAzureResourceID(d.Id()) + id, err := parse.LoadBalancerID(d.Id()) if err != nil { return err } - loadBalancer, exists, err := retrieveLoadBalancerById(d, d.Id(), meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *id, meta) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID %q: %+v", d.Id(), err) } @@ -307,20 +308,18 @@ func resourceArmLoadBalancerDelete(d *schema.ResourceData, meta interface{}) err ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := azure.ParseAzureResourceID(d.Id()) + id, err := parse.LoadBalancerID(d.Id()) if err != nil { - return fmt.Errorf("Error Parsing Azure Resource ID: %+v", err) + return err } - resGroup := id.ResourceGroup - name := id.Path["loadBalancers"] - future, err := client.Delete(ctx, resGroup, name) + future, err := client.Delete(ctx, id.ResourceGroup, id.Name) if err != nil { - return fmt.Errorf("Error deleting Load Balancer %q (Resource Group %q): %+v", name, resGroup, err) + return fmt.Errorf("deleting Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for the deleting Load Balancer %q (Resource Group %q): %+v", name, resGroup, err) + return fmt.Errorf("waiting for deletion of Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } return nil diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index 4ec5a1afba70..b45a8e1dfc20 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.go @@ -151,7 +151,7 @@ func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interf locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerID, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -222,7 +222,7 @@ func resourceArmLoadBalancerRuleRead(d *schema.ResourceData, meta interface{}) e return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) @@ -314,9 +314,10 @@ func resourceArmLoadBalancerRuleDelete(d *schema.ResourceData, meta interface{}) return err } - loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName).ID(subscriptionId) - locks.ByID(loadBalancerId) - defer locks.UnlockByID(loadBalancerId) + loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + loadBalancerIDRaw := loadBalancerId.ID(subscriptionId) + locks.ByID(loadBalancerIDRaw) + defer locks.UnlockByID(loadBalancerIDRaw) loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) if err != nil { diff --git a/azurerm/internal/services/network/loadbalancer.go b/azurerm/internal/services/network/loadbalancer.go index 9f18d85199e3..93d1c31b76fd 100644 --- a/azurerm/internal/services/network/loadbalancer.go +++ b/azurerm/internal/services/network/loadbalancer.go @@ -16,22 +16,17 @@ import ( // TODO: refactor this -func retrieveLoadBalancerById(d *schema.ResourceData, loadBalancerId string, meta interface{}) (*network.LoadBalancer, bool, error) { +func retrieveLoadBalancerById(d *schema.ResourceData, loadBalancerId parse.LoadBalancerId, meta interface{}) (*network.LoadBalancer, bool, error) { client := meta.(*clients.Client).Network.LoadBalancersClient ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := parse.LoadBalancerID(loadBalancerId) - if err != nil { - return nil, false, err - } - - resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") + resp, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { return nil, false, nil } - return nil, false, fmt.Errorf("retrieving Load Balancer %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + return nil, false, fmt.Errorf("retrieving Load Balancer %q (Resource Group %q): %+v", loadBalancerId.Name, loadBalancerId.ResourceGroup, err) } return &resp, true, nil From 6ab5e5e75cdb36ab9538b0705d4f9d7f5242bf04 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 14:21:41 +0200 Subject: [PATCH 10/15] refactor: passing ctx/client into the retrieveLoadBalancerById method --- .../network/lb_backend_address_pool_data_source.go | 8 +++++++- .../network/lb_backend_address_pool_resource.go | 10 +++++++--- .../internal/services/network/lb_nat_pool_resource.go | 10 +++++++--- .../internal/services/network/lb_nat_rule_resource.go | 10 +++++++--- .../services/network/lb_outbound_rule_resource.go | 10 +++++++--- azurerm/internal/services/network/lb_probe_resource.go | 10 +++++++--- azurerm/internal/services/network/lb_resource.go | 6 +++++- azurerm/internal/services/network/lb_rule_resource.go | 10 +++++++--- azurerm/internal/services/network/loadbalancer.go | 9 ++------- 9 files changed, 56 insertions(+), 27 deletions(-) diff --git a/azurerm/internal/services/network/lb_backend_address_pool_data_source.go b/azurerm/internal/services/network/lb_backend_address_pool_data_source.go index 30969f8b7885..c56d1cac9a3f 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_data_source.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_data_source.go @@ -7,7 +7,9 @@ import ( "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/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" ) func dataSourceArmLoadBalancerBackendAddressPool() *schema.Resource { @@ -48,13 +50,17 @@ func dataSourceArmLoadBalancerBackendAddressPool() *schema.Resource { } func dataSourceArmLoadBalancerBackendAddressPoolRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + name := d.Get("name").(string) loadBalancerId, err := parse.LoadBalancerID(d.Get("loadbalancer_id").(string)) if err != nil { return err } - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("retrieving Load Balancer by ID: %+v", err) } diff --git a/azurerm/internal/services/network/lb_backend_address_pool_resource.go b/azurerm/internal/services/network/lb_backend_address_pool_resource.go index df9f1455d54a..47df6f8ddf31 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_resource.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_resource.go @@ -90,7 +90,7 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -151,13 +151,17 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met } func resourceArmLoadBalancerBackendAddressPoolRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerBackendAddressPoolID(d.Id()) if err != nil { return err } loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("retrieving Load Balancer by ID: %+v", err) } @@ -216,7 +220,7 @@ func resourceArmLoadBalancerBackendAddressPoolDelete(d *schema.ResourceData, met locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) } diff --git a/azurerm/internal/services/network/lb_nat_pool_resource.go b/azurerm/internal/services/network/lb_nat_pool_resource.go index 2f12d1307def..69ca4951a306 100644 --- a/azurerm/internal/services/network/lb_nat_pool_resource.go +++ b/azurerm/internal/services/network/lb_nat_pool_resource.go @@ -115,7 +115,7 @@ func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta int locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("retrieving Load Balancer By ID: %+v", err) } @@ -180,14 +180,18 @@ func resourceArmLoadBalancerNatPoolCreateUpdate(d *schema.ResourceData, meta int } func resourceArmLoadBalancerNatPoolRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient subscriptionId := meta.(*clients.Client).Account.SubscriptionId + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerInboundNATPoolID(d.Id()) if err != nil { return err } loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) } @@ -261,7 +265,7 @@ func resourceArmLoadBalancerNatPoolDelete(d *schema.ResourceData, meta interface locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID: %+v", err) } diff --git a/azurerm/internal/services/network/lb_nat_rule_resource.go b/azurerm/internal/services/network/lb_nat_rule_resource.go index 166db87a0a26..d1033e607e9d 100644 --- a/azurerm/internal/services/network/lb_nat_rule_resource.go +++ b/azurerm/internal/services/network/lb_nat_rule_resource.go @@ -132,7 +132,7 @@ func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta int locks.ByID(loadBalancerIdRaw) defer locks.UnlockByID(loadBalancerIdRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -198,14 +198,18 @@ func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta int } func resourceArmLoadBalancerNatRuleRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient subscriptionId := meta.(*clients.Client).Account.SubscriptionId + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerInboundNATRuleID(d.Id()) if err != nil { return err } loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -287,7 +291,7 @@ func resourceArmLoadBalancerNatRuleDelete(d *schema.ResourceData, meta interface locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } diff --git a/azurerm/internal/services/network/lb_outbound_rule_resource.go b/azurerm/internal/services/network/lb_outbound_rule_resource.go index f8c6b896d311..b0533ec52d07 100644 --- a/azurerm/internal/services/network/lb_outbound_rule_resource.go +++ b/azurerm/internal/services/network/lb_outbound_rule_resource.go @@ -124,7 +124,7 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met locks.ByID(loadBalancerIDRaw) defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -196,14 +196,18 @@ func resourceArmLoadBalancerOutboundRuleCreateUpdate(d *schema.ResourceData, met } func resourceArmLoadBalancerOutboundRuleRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient subscriptionId := meta.(*clients.Client).Account.SubscriptionId + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerOutboundRuleID(d.Id()) if err != nil { return err } loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -286,7 +290,7 @@ func resourceArmLoadBalancerOutboundRuleDelete(d *schema.ResourceData, meta inte locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("retrieving Load Balancer By ID: %+v", err) } diff --git a/azurerm/internal/services/network/lb_probe_resource.go b/azurerm/internal/services/network/lb_probe_resource.go index 31f9bdc4bb5f..11530b538ee5 100644 --- a/azurerm/internal/services/network/lb_probe_resource.go +++ b/azurerm/internal/services/network/lb_probe_resource.go @@ -118,7 +118,7 @@ func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta inter locks.ByID(loadBalancerIDRaw) defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -179,13 +179,17 @@ func resourceArmLoadBalancerProbeCreateUpdate(d *schema.ResourceData, meta inter } func resourceArmLoadBalancerProbeRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerProbeID(d.Id()) if err != nil { return err } loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -259,7 +263,7 @@ func resourceArmLoadBalancerProbeDelete(d *schema.ResourceData, meta interface{} locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } diff --git a/azurerm/internal/services/network/lb_resource.go b/azurerm/internal/services/network/lb_resource.go index 5f4c60d023b7..96baf5d858f2 100644 --- a/azurerm/internal/services/network/lb_resource.go +++ b/azurerm/internal/services/network/lb_resource.go @@ -250,12 +250,16 @@ func resourceArmLoadBalancerCreateUpdate(d *schema.ResourceData, meta interface{ } func resourceArmLoadBalancerRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerID(d.Id()) if err != nil { return err } - loadBalancer, exists, err := retrieveLoadBalancerById(d, *id, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *id) if err != nil { return fmt.Errorf("Error retrieving Load Balancer by ID %q: %+v", d.Id(), err) } diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index b45a8e1dfc20..2284350cd5a7 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.go @@ -151,7 +151,7 @@ func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interf locks.ByID(loadBalancerID) defer locks.UnlockByID(loadBalancerID) - loadBalancer, exists, err := retrieveLoadBalancerById(d, *loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, *loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -216,14 +216,18 @@ func resourceArmLoadBalancerRuleCreateUpdate(d *schema.ResourceData, meta interf } func resourceArmLoadBalancerRuleRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.LoadBalancersClient subscriptionId := meta.(*clients.Client).Account.SubscriptionId + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + id, err := parse.LoadBalancerRuleID(d.Id()) if err != nil { return err } loadBalancerId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } @@ -319,7 +323,7 @@ func resourceArmLoadBalancerRuleDelete(d *schema.ResourceData, meta interface{}) locks.ByID(loadBalancerIDRaw) defer locks.UnlockByID(loadBalancerIDRaw) - loadBalancer, exists, err := retrieveLoadBalancerById(d, loadBalancerId, meta) + loadBalancer, exists, err := retrieveLoadBalancerById(ctx, client, loadBalancerId) if err != nil { return fmt.Errorf("Error Getting Load Balancer By ID: %+v", err) } diff --git a/azurerm/internal/services/network/loadbalancer.go b/azurerm/internal/services/network/loadbalancer.go index 93d1c31b76fd..230106078249 100644 --- a/azurerm/internal/services/network/loadbalancer.go +++ b/azurerm/internal/services/network/loadbalancer.go @@ -1,6 +1,7 @@ package network import ( + "context" "fmt" "regexp" "strings" @@ -8,19 +9,13 @@ import ( "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-03-01/network" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/parse" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) // TODO: refactor this -func retrieveLoadBalancerById(d *schema.ResourceData, loadBalancerId parse.LoadBalancerId, meta interface{}) (*network.LoadBalancer, bool, error) { - client := meta.(*clients.Client).Network.LoadBalancersClient - ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) - defer cancel() - +func retrieveLoadBalancerById(ctx context.Context, client *network.LoadBalancersClient, loadBalancerId parse.LoadBalancerId) (*network.LoadBalancer, bool, error) { resp, err := client.Get(ctx, loadBalancerId.ResourceGroup, loadBalancerId.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { From 7dcb88afd5f3f9bf8b3a886de91a5e8ef9cedf0b Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 14:25:32 +0200 Subject: [PATCH 11/15] refactor: adding validation it's a Load Balancer ID being passed in --- .../lb_backend_address_pool_data_source.go | 5 ++--- .../lb_backend_address_pool_resource.go | 3 ++- .../services/network/lb_data_source.go | 6 ++--- .../services/network/lb_nat_pool_resource.go | 3 ++- .../services/network/lb_nat_rule_resource.go | 3 ++- .../network/lb_outbound_rule_resource.go | 3 ++- .../services/network/lb_probe_resource.go | 3 ++- .../services/network/lb_rule_resource.go | 3 ++- .../network/validate/load_balancer.go | 22 +++++++++++++++++++ 9 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 azurerm/internal/services/network/validate/load_balancer.go diff --git a/azurerm/internal/services/network/lb_backend_address_pool_data_source.go b/azurerm/internal/services/network/lb_backend_address_pool_data_source.go index c56d1cac9a3f..dea705420ef1 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_data_source.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_data_source.go @@ -6,16 +6,15 @@ import ( "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/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" ) func dataSourceArmLoadBalancerBackendAddressPool() *schema.Resource { return &schema.Resource{ Read: dataSourceArmLoadBalancerBackendAddressPoolRead, - Timeouts: &schema.ResourceTimeout{ Read: schema.DefaultTimeout(5 * time.Minute), }, @@ -30,7 +29,7 @@ func dataSourceArmLoadBalancerBackendAddressPool() *schema.Resource { "loadbalancer_id": { Type: schema.TypeString, Required: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: validate.LoadBalancerID, }, "backend_ip_configurations": { diff --git a/azurerm/internal/services/network/lb_backend_address_pool_resource.go b/azurerm/internal/services/network/lb_backend_address_pool_resource.go index 47df6f8ddf31..fafb57688ab4 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_resource.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_resource.go @@ -14,6 +14,7 @@ import ( "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/network/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -48,7 +49,7 @@ func resourceArmLoadBalancerBackendAddressPool() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: validate.LoadBalancerID, }, "backend_ip_configurations": { diff --git a/azurerm/internal/services/network/lb_data_source.go b/azurerm/internal/services/network/lb_data_source.go index 0d2878f96519..f0ad3c3214a6 100644 --- a/azurerm/internal/services/network/lb_data_source.go +++ b/azurerm/internal/services/network/lb_data_source.go @@ -102,13 +102,13 @@ func dataSourceArmLoadBalancer() *schema.Resource { } func dataSourceArmLoadBalancerRead(d *schema.ResourceData, meta interface{}) error { - name := d.Get("name").(string) - resourceGroup := d.Get("resource_group_name").(string) - client := meta.(*clients.Client).Network.LoadBalancersClient ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() + name := d.Get("name").(string) + resourceGroup := d.Get("resource_group_name").(string) + resp, err := client.Get(ctx, resourceGroup, name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { diff --git a/azurerm/internal/services/network/lb_nat_pool_resource.go b/azurerm/internal/services/network/lb_nat_pool_resource.go index 69ca4951a306..6d95ef9e446f 100644 --- a/azurerm/internal/services/network/lb_nat_pool_resource.go +++ b/azurerm/internal/services/network/lb_nat_pool_resource.go @@ -16,6 +16,7 @@ import ( "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/network/parse" + networkValidate "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/state" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -52,7 +53,7 @@ func resourceArmLoadBalancerNatPool() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: networkValidate.LoadBalancerID, }, "protocol": { diff --git a/azurerm/internal/services/network/lb_nat_rule_resource.go b/azurerm/internal/services/network/lb_nat_rule_resource.go index d1033e607e9d..7601e4c05887 100644 --- a/azurerm/internal/services/network/lb_nat_rule_resource.go +++ b/azurerm/internal/services/network/lb_nat_rule_resource.go @@ -16,6 +16,7 @@ import ( "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/network/parse" + networkValidate "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/state" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -53,7 +54,7 @@ func resourceArmLoadBalancerNatRule() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: networkValidate.LoadBalancerID, }, "protocol": { diff --git a/azurerm/internal/services/network/lb_outbound_rule_resource.go b/azurerm/internal/services/network/lb_outbound_rule_resource.go index b0533ec52d07..a499eca46c72 100644 --- a/azurerm/internal/services/network/lb_outbound_rule_resource.go +++ b/azurerm/internal/services/network/lb_outbound_rule_resource.go @@ -14,6 +14,7 @@ import ( "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/network/parse" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -50,7 +51,7 @@ func resourceArmLoadBalancerOutboundRule() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: validate.LoadBalancerID, }, "frontend_ip_configuration": { diff --git a/azurerm/internal/services/network/lb_probe_resource.go b/azurerm/internal/services/network/lb_probe_resource.go index 11530b538ee5..550fc32d56a3 100644 --- a/azurerm/internal/services/network/lb_probe_resource.go +++ b/azurerm/internal/services/network/lb_probe_resource.go @@ -16,6 +16,7 @@ import ( "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/network/parse" + networkValidate "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -51,7 +52,7 @@ func resourceArmLoadBalancerProbe() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: networkValidate.LoadBalancerID, }, "protocol": { diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index 2284350cd5a7..53cde313c32e 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.go @@ -17,6 +17,7 @@ import ( "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/network/parse" + networkValidate "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -53,7 +54,7 @@ func resourceArmLoadBalancerRule() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: networkValidate.LoadBalancerID, }, "frontend_ip_configuration_name": { diff --git a/azurerm/internal/services/network/validate/load_balancer.go b/azurerm/internal/services/network/validate/load_balancer.go new file mode 100644 index 000000000000..715f8d5885fe --- /dev/null +++ b/azurerm/internal/services/network/validate/load_balancer.go @@ -0,0 +1,22 @@ +package validate + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/parse" +) + +func LoadBalancerID(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.LoadBalancerID(v); err != nil { + errors = append(errors, fmt.Errorf("Can not parse %q as a resource id: %v", k, err)) + return + } + + return warnings, errors +} From bbafb6cf9dcae9b2f7f227559a691bcb8cd767c4 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 19 Aug 2020 14:32:23 +0200 Subject: [PATCH 12/15] refactor: adding import time validation --- .../lb_backend_address_pool_resource.go | 13 +++++++-- .../services/network/lb_nat_pool_resource.go | 13 +++++++-- .../services/network/lb_nat_rule_resource.go | 12 ++++++-- .../network/lb_outbound_rule_resource.go | 12 ++++++-- .../services/network/lb_probe_resource.go | 13 +++++++-- .../internal/services/network/lb_resource.go | 8 +++-- .../services/network/lb_rule_resource.go | 12 ++++++-- .../internal/services/network/loadbalancer.go | 29 ++++++++----------- 8 files changed, 74 insertions(+), 38 deletions(-) diff --git a/azurerm/internal/services/network/lb_backend_address_pool_resource.go b/azurerm/internal/services/network/lb_backend_address_pool_resource.go index fafb57688ab4..96dba2c31748 100644 --- a/azurerm/internal/services/network/lb_backend_address_pool_resource.go +++ b/azurerm/internal/services/network/lb_backend_address_pool_resource.go @@ -24,9 +24,16 @@ func resourceArmLoadBalancerBackendAddressPool() *schema.Resource { Create: resourceArmLoadBalancerBackendAddressPoolCreate, Read: resourceArmLoadBalancerBackendAddressPoolRead, Delete: resourceArmLoadBalancerBackendAddressPoolDelete, - Importer: &schema.ResourceImporter{ - State: loadBalancerSubResourceStateImporter, - }, + + Importer: loadBalancerSubResourceImporter(func(input string) (*parse.LoadBalancerId, error) { + id, err := parse.LoadBalancerBackendAddressPoolID(input) + if err != nil { + return nil, err + } + + lbId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + return &lbId, nil + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/lb_nat_pool_resource.go b/azurerm/internal/services/network/lb_nat_pool_resource.go index 6d95ef9e446f..a2f81b09345f 100644 --- a/azurerm/internal/services/network/lb_nat_pool_resource.go +++ b/azurerm/internal/services/network/lb_nat_pool_resource.go @@ -28,9 +28,16 @@ func resourceArmLoadBalancerNatPool() *schema.Resource { Read: resourceArmLoadBalancerNatPoolRead, Update: resourceArmLoadBalancerNatPoolCreateUpdate, Delete: resourceArmLoadBalancerNatPoolDelete, - Importer: &schema.ResourceImporter{ - State: loadBalancerSubResourceStateImporter, - }, + + Importer: loadBalancerSubResourceImporter(func(input string) (*parse.LoadBalancerId, error) { + id, err := parse.LoadBalancerInboundNATPoolID(input) + if err != nil { + return nil, err + } + + lbId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + return &lbId, nil + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/lb_nat_rule_resource.go b/azurerm/internal/services/network/lb_nat_rule_resource.go index 7601e4c05887..c21006d30969 100644 --- a/azurerm/internal/services/network/lb_nat_rule_resource.go +++ b/azurerm/internal/services/network/lb_nat_rule_resource.go @@ -29,9 +29,15 @@ func resourceArmLoadBalancerNatRule() *schema.Resource { Update: resourceArmLoadBalancerNatRuleCreateUpdate, Delete: resourceArmLoadBalancerNatRuleDelete, - Importer: &schema.ResourceImporter{ - State: loadBalancerSubResourceStateImporter, - }, + Importer: loadBalancerSubResourceImporter(func(input string) (*parse.LoadBalancerId, error) { + id, err := parse.LoadBalancerInboundNATRuleID(input) + if err != nil { + return nil, err + } + + lbId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + return &lbId, nil + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/lb_outbound_rule_resource.go b/azurerm/internal/services/network/lb_outbound_rule_resource.go index a499eca46c72..eea920758b0e 100644 --- a/azurerm/internal/services/network/lb_outbound_rule_resource.go +++ b/azurerm/internal/services/network/lb_outbound_rule_resource.go @@ -26,9 +26,15 @@ func resourceArmLoadBalancerOutboundRule() *schema.Resource { Update: resourceArmLoadBalancerOutboundRuleCreateUpdate, Delete: resourceArmLoadBalancerOutboundRuleDelete, - Importer: &schema.ResourceImporter{ - State: loadBalancerSubResourceStateImporter, - }, + Importer: loadBalancerSubResourceImporter(func(input string) (*parse.LoadBalancerId, error) { + id, err := parse.LoadBalancerOutboundRuleID(input) + if err != nil { + return nil, err + } + + lbId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + return &lbId, nil + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/lb_probe_resource.go b/azurerm/internal/services/network/lb_probe_resource.go index 550fc32d56a3..35a1fc4c6c3d 100644 --- a/azurerm/internal/services/network/lb_probe_resource.go +++ b/azurerm/internal/services/network/lb_probe_resource.go @@ -27,9 +27,16 @@ func resourceArmLoadBalancerProbe() *schema.Resource { Read: resourceArmLoadBalancerProbeRead, Update: resourceArmLoadBalancerProbeCreateUpdate, Delete: resourceArmLoadBalancerProbeDelete, - Importer: &schema.ResourceImporter{ - State: loadBalancerSubResourceStateImporter, - }, + + Importer: loadBalancerSubResourceImporter(func(input string) (*parse.LoadBalancerId, error) { + id, err := parse.LoadBalancerProbeID(input) + if err != nil { + return nil, err + } + + lbId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + return &lbId, nil + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/lb_resource.go b/azurerm/internal/services/network/lb_resource.go index 96baf5d858f2..6d9cd15f601d 100644 --- a/azurerm/internal/services/network/lb_resource.go +++ b/azurerm/internal/services/network/lb_resource.go @@ -15,6 +15,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/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/tf/state" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -27,9 +28,10 @@ func resourceArmLoadBalancer() *schema.Resource { Update: resourceArmLoadBalancerCreateUpdate, Delete: resourceArmLoadBalancerDelete, - Importer: &schema.ResourceImporter{ - State: schema.ImportStatePassthrough, - }, + Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error { + _, err := parse.LoadBalancerID(id) + return err + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index 53cde313c32e..11a657117f1e 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.go @@ -29,9 +29,15 @@ func resourceArmLoadBalancerRule() *schema.Resource { Update: resourceArmLoadBalancerRuleCreateUpdate, Delete: resourceArmLoadBalancerRuleDelete, - Importer: &schema.ResourceImporter{ - State: loadBalancerSubResourceStateImporter, - }, + Importer: loadBalancerSubResourceImporter(func(input string) (*parse.LoadBalancerId, error) { + id, err := parse.LoadBalancerRuleID(input) + if err != nil { + return nil, err + } + + lbId := parse.NewLoadBalancerID(id.ResourceGroup, id.LoadBalancerName) + return &lbId, nil + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), diff --git a/azurerm/internal/services/network/loadbalancer.go b/azurerm/internal/services/network/loadbalancer.go index 230106078249..f3e7c891dcf1 100644 --- a/azurerm/internal/services/network/loadbalancer.go +++ b/azurerm/internal/services/network/loadbalancer.go @@ -3,12 +3,10 @@ package network import ( "context" "fmt" - "regexp" - "strings" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-03-01/network" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) @@ -125,20 +123,17 @@ func FindLoadBalancerProbeByName(lb *network.LoadBalancer, name string) (*networ return nil, -1, false } -// sets the loadbalancer_id in the ResourceData from the sub resources full id -func loadBalancerSubResourceStateImporter(d *schema.ResourceData, _ interface{}) ([]*schema.ResourceData, error) { - r := regexp.MustCompile(`.+/loadBalancers/.+?/`) +func loadBalancerSubResourceImporter(parser func(input string) (*parse.LoadBalancerId, error)) *schema.ResourceImporter { + return &schema.ResourceImporter{ + State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + lbId, err := parser(d.Id()) + if err != nil { + return nil, err + } - lbID := strings.TrimSuffix(r.FindString(d.Id()), "/") - parsed, err := azure.ParseAzureResourceID(lbID) - if err != nil { - return nil, fmt.Errorf("unable to parse loadbalancer id from %s", d.Id()) - } - - if parsed.Path["loadBalancers"] == "" { - return nil, fmt.Errorf("parsed ID is invalid") + d.Set("loadbalancer_id", lbId.ID(subscriptionId)) + return []*schema.ResourceData{d}, nil + }, } - - d.Set("loadbalancer_id", lbID) - return []*schema.ResourceData{d}, nil } From 7051d6120950a0527ad59ab8e147ea3d03fa062d Mon Sep 17 00:00:00 2001 From: Tom Harvey Date: Thu, 20 Aug 2020 15:02:57 +0200 Subject: [PATCH 13/15] Update azurerm/internal/services/network/lb_rule_resource.go Co-authored-by: Steve <11830746+jackofallops@users.noreply.github.com> --- azurerm/internal/services/network/lb_rule_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index 11a657117f1e..1d7ca6caec7a 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.go @@ -354,7 +354,7 @@ func resourceArmLoadBalancerRuleDelete(d *schema.ResourceData, meta interface{}) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for completion of Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) + return fmt.Errorf("waiting for completion of Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } read, err := client.Get(ctx, id.ResourceGroup, id.LoadBalancerName, "") From 7346c00f3223bd730f3541242b841c02c609b627 Mon Sep 17 00:00:00 2001 From: Tom Harvey Date: Thu, 20 Aug 2020 15:03:04 +0200 Subject: [PATCH 14/15] Update azurerm/internal/services/network/lb_rule_resource.go Co-authored-by: Steve <11830746+jackofallops@users.noreply.github.com> --- azurerm/internal/services/network/lb_rule_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/internal/services/network/lb_rule_resource.go b/azurerm/internal/services/network/lb_rule_resource.go index 1d7ca6caec7a..d8942a0a90f1 100644 --- a/azurerm/internal/services/network/lb_rule_resource.go +++ b/azurerm/internal/services/network/lb_rule_resource.go @@ -350,7 +350,7 @@ func resourceArmLoadBalancerRuleDelete(d *schema.ResourceData, meta interface{}) future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) + return fmt.Errorf("Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { From 9abb3bb3e2386c4a1784133d5af8b0d75c5ab0f5 Mon Sep 17 00:00:00 2001 From: Tom Harvey Date: Thu, 20 Aug 2020 15:03:15 +0200 Subject: [PATCH 15/15] Update azurerm/internal/services/network/lb_outbound_rule_resource.go Co-authored-by: Steve <11830746+jackofallops@users.noreply.github.com> --- azurerm/internal/services/network/lb_outbound_rule_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/internal/services/network/lb_outbound_rule_resource.go b/azurerm/internal/services/network/lb_outbound_rule_resource.go index eea920758b0e..e91ed5560e42 100644 --- a/azurerm/internal/services/network/lb_outbound_rule_resource.go +++ b/azurerm/internal/services/network/lb_outbound_rule_resource.go @@ -317,7 +317,7 @@ func resourceArmLoadBalancerOutboundRuleDelete(d *schema.ResourceData, meta inte future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.LoadBalancerName, *loadBalancer) if err != nil { - return fmt.Errorf("Error Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) + return fmt.Errorf("Creating/Updating Load Balancer %q (Resource Group %q): %+v", id.LoadBalancerName, id.ResourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {