From b6866f2187a9b391cc5102f4f81b1fc73004ae9a Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Thu, 13 Apr 2017 15:26:51 +0100 Subject: [PATCH 1/5] Locking the NSG to only operate on one resource at a time in the create --- .../azurerm/resource_arm_virtual_network.go | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/builtin/providers/azurerm/resource_arm_virtual_network.go b/builtin/providers/azurerm/resource_arm_virtual_network.go index 5d4ba9a297cc..9f61720365c6 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_network.go +++ b/builtin/providers/azurerm/resource_arm_virtual_network.go @@ -97,6 +97,22 @@ func resourceArmVirtualNetworkCreate(d *schema.ResourceData, meta interface{}) e Tags: expandTags(tags), } + networkSecurityGroupNames := make([]string, 0) + for _, subnet := range *vnet.VirtualNetworkPropertiesFormat.Subnets { + if subnet.NetworkSecurityGroup != nil { + subnetId := *subnet.NetworkSecurityGroup.ID + id, err := parseAzureResourceID(subnetId) + if err != nil { + return fmt.Errorf("[ERROR] Unable to Parse Network Security Group ID '%s': %+v", subnetId, err) + } + nsgName := id.Path["networkSecurityGroups"] + networkSecurityGroupNames = append(networkSecurityGroupNames, nsgName) + } + } + + azureRMVirtualNetworkLockNetworkSecurityGroups(&networkSecurityGroupNames) + defer azureRMVirtualNetworkUnlockNetworkSecurityGroups(&networkSecurityGroupNames) + _, err := vnetClient.CreateOrUpdate(resGroup, name, vnet, make(chan struct{})) if err != nil { return err @@ -182,6 +198,8 @@ func resourceArmVirtualNetworkDelete(d *schema.ResourceData, meta interface{}) e resGroup := id.ResourceGroup name := id.Path["virtualNetworks"] + // TODO: lock any associated NSG's + _, err = vnetClient.Delete(resGroup, name, make(chan struct{})) return err @@ -245,3 +263,14 @@ func resourceAzureSubnetHash(v interface{}) int { } return hashcode.String(subnet) } + +func azureRMVirtualNetworkUnlockNetworkSecurityGroups(networkSecurityGroupNames *[]string) { + for _, networkSecurityGroupName := range *networkSecurityGroupNames { + armMutexKV.Unlock(networkSecurityGroupName) + } +} +func azureRMVirtualNetworkLockNetworkSecurityGroups(networkSecurityGroupNames *[]string) { + for _, networkSecurityGroupName := range *networkSecurityGroupNames { + armMutexKV.Lock(networkSecurityGroupName) + } +} \ No newline at end of file From b93e6e3af75d8a4e70ed091b44faa9e7a589b514 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Thu, 13 Apr 2017 15:28:13 +0100 Subject: [PATCH 2/5] Locking on the delete too --- .../azurerm/resource_arm_virtual_network.go | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/builtin/providers/azurerm/resource_arm_virtual_network.go b/builtin/providers/azurerm/resource_arm_virtual_network.go index 9f61720365c6..e8ad55c97083 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_network.go +++ b/builtin/providers/azurerm/resource_arm_virtual_network.go @@ -100,12 +100,11 @@ func resourceArmVirtualNetworkCreate(d *schema.ResourceData, meta interface{}) e networkSecurityGroupNames := make([]string, 0) for _, subnet := range *vnet.VirtualNetworkPropertiesFormat.Subnets { if subnet.NetworkSecurityGroup != nil { - subnetId := *subnet.NetworkSecurityGroup.ID - id, err := parseAzureResourceID(subnetId) + nsgName, err := parseNetworkSecurityGroupName(*subnet.NetworkSecurityGroup.ID) if err != nil { - return fmt.Errorf("[ERROR] Unable to Parse Network Security Group ID '%s': %+v", subnetId, err) + return err } - nsgName := id.Path["networkSecurityGroups"] + networkSecurityGroupNames = append(networkSecurityGroupNames, nsgName) } } @@ -198,7 +197,13 @@ func resourceArmVirtualNetworkDelete(d *schema.ResourceData, meta interface{}) e resGroup := id.ResourceGroup name := id.Path["virtualNetworks"] - // TODO: lock any associated NSG's + nsgNames, err := expandAzureRmVirtualNetworkVirtualNetworkSecurityGroupNames(d) + if err != nil { + return fmt.Errorf("[ERROR] Error parsing Network Security Group ID's: %+v", err) + } + + azureRMVirtualNetworkLockNetworkSecurityGroups(&nsgNames) + defer azureRMVirtualNetworkUnlockNetworkSecurityGroups(&nsgNames) _, err = vnetClient.Delete(resGroup, name, make(chan struct{})) @@ -264,6 +269,38 @@ func resourceAzureSubnetHash(v interface{}) int { return hashcode.String(subnet) } +func parseNetworkSecurityGroupName(networkSecurityGroupId string) (string, error) { + id, err := parseAzureResourceID(networkSecurityGroupId) + if err != nil { + return "", fmt.Errorf("[ERROR] Unable to Parse Network Security Group ID '%s': %+v", networkSecurityGroupId, err) + } + + return id.Path["networkSecurityGroups"], nil +} + +func expandAzureRmVirtualNetworkVirtualNetworkSecurityGroupNames(d *schema.ResourceData) ([]string, error) { + nsgNames := make([]string, 0) + + if v, ok := d.GetOk("subnet"); ok { + subnets := v.(*schema.Set).List() + for _, subnet := range subnets { + subnet := subnet.(map[string]interface{}) + + networkSecurityGroupId := subnet["security_group"].(string) + if networkSecurityGroupId != "" { + nsgName, err := parseNetworkSecurityGroupName(networkSecurityGroupId) + if err != nil { + return nil, err + } + + nsgNames = append(nsgNames, nsgName) + } + } + } + + return nsgNames, nil +} + func azureRMVirtualNetworkUnlockNetworkSecurityGroups(networkSecurityGroupNames *[]string) { for _, networkSecurityGroupName := range *networkSecurityGroupNames { armMutexKV.Unlock(networkSecurityGroupName) From 3ecb0f4fc4e6d4a553e7f5441b9bbf0e1b6ba49e Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Thu, 13 Apr 2017 16:28:48 +0100 Subject: [PATCH 3/5] Locking on the NSG ID --- .../resource_arm_network_interface_card.go | 19 +++++++++++++++++++ .../providers/azurerm/resource_arm_subnet.go | 19 +++++++++++++++++++ .../azurerm/resource_arm_virtual_network.go | 9 --------- builtin/providers/azurerm/resourceid.go | 9 +++++++++ 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/builtin/providers/azurerm/resource_arm_network_interface_card.go b/builtin/providers/azurerm/resource_arm_network_interface_card.go index 7450519b1e85..3c74bc3e0d0e 100644 --- a/builtin/providers/azurerm/resource_arm_network_interface_card.go +++ b/builtin/providers/azurerm/resource_arm_network_interface_card.go @@ -172,6 +172,14 @@ func resourceArmNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) properties.NetworkSecurityGroup = &network.SecurityGroup{ ID: &nsgId, } + + networkSecurityGroupName, err := parseNetworkSecurityGroupName(nsgId) + if err != nil { + return err + } + + armMutexKV.Lock(networkSecurityGroupName) + defer armMutexKV.Unlock(networkSecurityGroupName) } dns, hasDns := d.GetOk("dns_servers") @@ -308,6 +316,17 @@ func resourceArmNetworkInterfaceDelete(d *schema.ResourceData, meta interface{}) resGroup := id.ResourceGroup name := id.Path["networkInterfaces"] + if v, ok := d.GetOk("network_security_group_id"); ok { + networkSecurityGroupId := v.(string) + networkSecurityGroupName, err := parseNetworkSecurityGroupName(networkSecurityGroupId) + if err != nil { + return err + } + + armMutexKV.Lock(networkSecurityGroupName) + defer armMutexKV.Unlock(networkSecurityGroupName) + } + _, err = ifaceClient.Delete(resGroup, name, make(chan struct{})) return err diff --git a/builtin/providers/azurerm/resource_arm_subnet.go b/builtin/providers/azurerm/resource_arm_subnet.go index c5329b9f8663..65df4f447cb4 100644 --- a/builtin/providers/azurerm/resource_arm_subnet.go +++ b/builtin/providers/azurerm/resource_arm_subnet.go @@ -89,6 +89,14 @@ func resourceArmSubnetCreate(d *schema.ResourceData, meta interface{}) error { properties.NetworkSecurityGroup = &network.SecurityGroup{ ID: &nsgId, } + + networkSecurityGroupName, err := parseNetworkSecurityGroupName(nsgId) + if err != nil { + return err + } + + armMutexKV.Lock(networkSecurityGroupName) + defer armMutexKV.Unlock(networkSecurityGroupName) } if v, ok := d.GetOk("route_table_id"); ok { @@ -182,6 +190,17 @@ func resourceArmSubnetDelete(d *schema.ResourceData, meta interface{}) error { name := id.Path["subnets"] vnetName := id.Path["virtualNetworks"] + if v, ok := d.GetOk("network_security_group_id"); ok { + networkSecurityGroupId := v.(string) + networkSecurityGroupName, err := parseNetworkSecurityGroupName(networkSecurityGroupId) + if err != nil { + return err + } + + armMutexKV.Lock(networkSecurityGroupName) + defer armMutexKV.Unlock(networkSecurityGroupName) + } + armMutexKV.Lock(vnetName) defer armMutexKV.Unlock(vnetName) diff --git a/builtin/providers/azurerm/resource_arm_virtual_network.go b/builtin/providers/azurerm/resource_arm_virtual_network.go index e8ad55c97083..6be08d9f1988 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_network.go +++ b/builtin/providers/azurerm/resource_arm_virtual_network.go @@ -269,15 +269,6 @@ func resourceAzureSubnetHash(v interface{}) int { return hashcode.String(subnet) } -func parseNetworkSecurityGroupName(networkSecurityGroupId string) (string, error) { - id, err := parseAzureResourceID(networkSecurityGroupId) - if err != nil { - return "", fmt.Errorf("[ERROR] Unable to Parse Network Security Group ID '%s': %+v", networkSecurityGroupId, err) - } - - return id.Path["networkSecurityGroups"], nil -} - func expandAzureRmVirtualNetworkVirtualNetworkSecurityGroupNames(d *schema.ResourceData) ([]string, error) { nsgNames := make([]string, 0) diff --git a/builtin/providers/azurerm/resourceid.go b/builtin/providers/azurerm/resourceid.go index b05f4d75f77b..5a0a19a8a40b 100644 --- a/builtin/providers/azurerm/resourceid.go +++ b/builtin/providers/azurerm/resourceid.go @@ -95,3 +95,12 @@ func parseAzureResourceID(id string) (*ResourceID, error) { return idObj, nil } + +func parseNetworkSecurityGroupName(networkSecurityGroupId string) (string, error) { + id, err := parseAzureResourceID(networkSecurityGroupId) + if err != nil { + return "", fmt.Errorf("[ERROR] Unable to Parse Network Security Group ID '%s': %+v", networkSecurityGroupId, err) + } + + return id.Path["networkSecurityGroups"], nil +} \ No newline at end of file From fd03d0310e8f69a72b35694e170a4282bfaf27bd Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Thu, 13 Apr 2017 17:13:41 +0100 Subject: [PATCH 4/5] Formatting --- builtin/providers/azurerm/resource_arm_virtual_network.go | 2 +- builtin/providers/azurerm/resourceid.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/providers/azurerm/resource_arm_virtual_network.go b/builtin/providers/azurerm/resource_arm_virtual_network.go index 6be08d9f1988..e4f38e09d756 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_network.go +++ b/builtin/providers/azurerm/resource_arm_virtual_network.go @@ -301,4 +301,4 @@ func azureRMVirtualNetworkLockNetworkSecurityGroups(networkSecurityGroupNames *[ for _, networkSecurityGroupName := range *networkSecurityGroupNames { armMutexKV.Lock(networkSecurityGroupName) } -} \ No newline at end of file +} diff --git a/builtin/providers/azurerm/resourceid.go b/builtin/providers/azurerm/resourceid.go index 5a0a19a8a40b..f981b410b8fd 100644 --- a/builtin/providers/azurerm/resourceid.go +++ b/builtin/providers/azurerm/resourceid.go @@ -103,4 +103,4 @@ func parseNetworkSecurityGroupName(networkSecurityGroupId string) (string, error } return id.Path["networkSecurityGroups"], nil -} \ No newline at end of file +} From 079043d5f83f1b253215af0c013c9157c1fa9e76 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 18 Apr 2017 16:20:29 +0100 Subject: [PATCH 5/5] Checking the type of the subnet before using it --- builtin/providers/azurerm/resource_arm_virtual_network.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/builtin/providers/azurerm/resource_arm_virtual_network.go b/builtin/providers/azurerm/resource_arm_virtual_network.go index e4f38e09d756..217323a6f7b8 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_network.go +++ b/builtin/providers/azurerm/resource_arm_virtual_network.go @@ -275,7 +275,10 @@ func expandAzureRmVirtualNetworkVirtualNetworkSecurityGroupNames(d *schema.Resou if v, ok := d.GetOk("subnet"); ok { subnets := v.(*schema.Set).List() for _, subnet := range subnets { - subnet := subnet.(map[string]interface{}) + subnet, ok := subnet.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("[ERROR] Subnet should be a Hash - was '%+v'", subnet) + } networkSecurityGroupId := subnet["security_group"].(string) if networkSecurityGroupId != "" {