Skip to content

Commit

Permalink
network: reverting the locking changes from #3673
Browse files Browse the repository at this point in the history
  • Loading branch information
tombuildsstuff committed Sep 13, 2019
1 parent 1d1d696 commit 2df3b8a
Show file tree
Hide file tree
Showing 7 changed files with 279 additions and 6 deletions.
31 changes: 31 additions & 0 deletions azurerm/internal/services/network/network_security_group.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package network

import (
"fmt"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

type NetworkSecurityGroupResourceID struct {
Base azure.ResourceID

Name string
}

func ParseNetworkSecurityGroupResourceID(input string) (*NetworkSecurityGroupResourceID, error) {
id, err := azure.ParseAzureResourceID(input)
if err != nil {
return nil, fmt.Errorf("[ERROR] Unable to parse Network Security Group ID %q: %+v", input, err)
}

routeTable := NetworkSecurityGroupResourceID{
Base: *id,
Name: id.Path["networkSecurityGroups"],
}

if routeTable.Name == "" {
return nil, fmt.Errorf("ID was missing the `networkSecurityGroups` element")
}

return &routeTable, nil
}
62 changes: 62 additions & 0 deletions azurerm/internal/services/network/network_security_group_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package network

import (
"testing"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

func TestParseNetworkSecurityGroup(t *testing.T) {
testData := []struct {
Name string
Input string
Expected *RouteTableResourceID
}{
{
Name: "Empty",
Input: "",
Expected: nil,
},
{
Name: "No Network Security Groups Segment",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo",
Expected: nil,
},
{
Name: "No Network Security Groups Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/networkSecurityGroups/",
Expected: nil,
},
{
Name: "Completed",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/networkSecurityGroups/example",
Expected: &RouteTableResourceID{
Name: "example",
Base: azure.ResourceID{
ResourceGroup: "foo",
},
},
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

actual, err := ParseNetworkSecurityGroupResourceID(v.Input)
if err != nil {
if v.Expected == nil {
continue
}

t.Fatalf("Expected a value but got an error: %s", err)
}

if actual.Name != v.Expected.Name {
t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name)
}

if actual.Base.ResourceGroup != v.Expected.Base.ResourceGroup {
t.Fatalf("Expected %q but got %q for ResourceGroup", v.Expected.Base.ResourceGroup, actual.Base.ResourceGroup)
}
}
}
34 changes: 34 additions & 0 deletions azurerm/internal/services/network/route_table.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package network

import (
"fmt"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

// NOTE: there's some nice things we can do with this around validation
// since these top level objects exist

type RouteTableResourceID struct {
Base azure.ResourceID

Name string
}

func ParseRouteTableResourceID(input string) (*RouteTableResourceID, error) {
id, err := azure.ParseAzureResourceID(input)
if err != nil {
return nil, fmt.Errorf("[ERROR] Unable to parse Route Table ID %q: %+v", input, err)
}

routeTable := RouteTableResourceID{
Base: *id,
Name: id.Path["routeTables"],
}

if routeTable.Name == "" {
return nil, fmt.Errorf("ID was missing the `routeTables` element")
}

return &routeTable, nil
}
62 changes: 62 additions & 0 deletions azurerm/internal/services/network/route_table_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package network

import (
"testing"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

func TestParseRouteTable(t *testing.T) {
testData := []struct {
Name string
Input string
Expected *RouteTableResourceID
}{
{
Name: "Empty",
Input: "",
Expected: nil,
},
{
Name: "No Route Table Segment",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo",
Expected: nil,
},
{
Name: "No Route Table Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/routeTables/",
Expected: nil,
},
{
Name: "Completed",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/routeTables/example",
Expected: &RouteTableResourceID{
Name: "example",
Base: azure.ResourceID{
ResourceGroup: "foo",
},
},
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

actual, err := ParseRouteTableResourceID(v.Input)
if err != nil {
if v.Expected == nil {
continue
}

t.Fatalf("Expected a value but got an error: %s", err)
}

if actual.Name != v.Expected.Name {
t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name)
}

if actual.Base.ResourceGroup != v.Expected.Base.ResourceGroup {
t.Fatalf("Expected %q but got %q for ResourceGroup", v.Expected.Base.ResourceGroup, actual.Base.ResourceGroup)
}
}
}
46 changes: 44 additions & 2 deletions azurerm/resource_arm_subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/locks"
networksvc "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

Expand Down Expand Up @@ -153,8 +154,8 @@ func resourceArmSubnetCreateUpdate(d *schema.ResourceData, meta interface{}) err

addressPrefix := d.Get("address_prefix").(string)

locks.ByName(name, subnetResourceName)
defer locks.UnlockByName(name, subnetResourceName)
locks.ByName(vnetName, virtualNetworkResourceName)
defer locks.UnlockByName(vnetName, virtualNetworkResourceName)

properties := network.SubnetPropertiesFormat{
AddressPrefix: &addressPrefix,
Expand All @@ -165,6 +166,14 @@ func resourceArmSubnetCreateUpdate(d *schema.ResourceData, meta interface{}) err
properties.NetworkSecurityGroup = &network.SecurityGroup{
ID: &nsgId,
}

parsedNsgId, err := networksvc.ParseNetworkSecurityGroupResourceID(nsgId)
if err != nil {
return err
}

locks.ByName(parsedNsgId.Name, networkSecurityGroupResourceName)
defer locks.UnlockByName(parsedNsgId.Name, networkSecurityGroupResourceName)
} else {
properties.NetworkSecurityGroup = nil
}
Expand All @@ -174,6 +183,14 @@ func resourceArmSubnetCreateUpdate(d *schema.ResourceData, meta interface{}) err
properties.RouteTable = &network.RouteTable{
ID: &rtId,
}

parsedRouteTableId, err := networksvc.ParseRouteTableResourceID(rtId)
if err != nil {
return err
}

locks.ByName(parsedRouteTableId.Name, routeTableResourceName)
defer locks.UnlockByName(parsedRouteTableId.Name, routeTableResourceName)
} else {
properties.RouteTable = nil
}
Expand Down Expand Up @@ -283,6 +300,31 @@ 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)
parsedNetworkSecurityGroupId, err2 := networksvc.ParseNetworkSecurityGroupResourceID(networkSecurityGroupId)
if err2 != nil {
return err2
}

locks.ByName(parsedNetworkSecurityGroupId.Name, networkSecurityGroupResourceName)
defer locks.UnlockByName(parsedNetworkSecurityGroupId.Name, networkSecurityGroupResourceName)
}

if v, ok := d.GetOk("route_table_id"); ok {
rtId := v.(string)
parsedRouteTableId, err2 := networksvc.ParseRouteTableResourceID(rtId)
if err2 != nil {
return err2
}

locks.ByName(parsedRouteTableId.Name, routeTableResourceName)
defer locks.UnlockByName(parsedRouteTableId.Name, routeTableResourceName)
}

locks.ByName(vnetName, virtualNetworkResourceName)
defer locks.UnlockByName(vnetName, virtualNetworkResourceName)

locks.ByName(name, subnetResourceName)
defer locks.UnlockByName(name, subnetResourceName)

Expand Down
24 changes: 24 additions & 0 deletions azurerm/resource_arm_subnet_network_security_group_association.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/locks"
networkSvc "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

Expand Down Expand Up @@ -54,10 +55,21 @@ func resourceArmSubnetNetworkSecurityGroupAssociationCreate(d *schema.ResourceDa
return err
}

parsedNetworkSecurityGroupId, err := networkSvc.ParseNetworkSecurityGroupResourceID(networkSecurityGroupId)
if err != nil {
return err
}

locks.ByName(parsedNetworkSecurityGroupId.Name, networkSecurityGroupResourceName)
defer locks.UnlockByName(parsedNetworkSecurityGroupId.Name, networkSecurityGroupResourceName)

subnetName := parsedSubnetId.Path["subnets"]
virtualNetworkName := parsedSubnetId.Path["virtualNetworks"]
resourceGroup := parsedSubnetId.ResourceGroup

locks.ByName(virtualNetworkName, virtualNetworkResourceName)
defer locks.UnlockByName(virtualNetworkName, virtualNetworkResourceName)

locks.ByName(subnetName, subnetResourceName)
defer locks.UnlockByName(subnetName, subnetResourceName)

Expand Down Expand Up @@ -178,6 +190,18 @@ func resourceArmSubnetNetworkSecurityGroupAssociationDelete(d *schema.ResourceDa
return nil
}

// once we have the network security group id to lock on, lock on that
parsedNetworkSecurityGroupId, err := networkSvc.ParseNetworkSecurityGroupResourceID(*props.NetworkSecurityGroup.ID)
if err != nil {
return err
}

locks.ByName(parsedNetworkSecurityGroupId.Name, networkSecurityGroupResourceName)
defer locks.UnlockByName(parsedNetworkSecurityGroupId.Name, networkSecurityGroupResourceName)

locks.ByName(virtualNetworkName, virtualNetworkResourceName)
defer locks.UnlockByName(virtualNetworkName, virtualNetworkResourceName)

locks.ByName(subnetName, subnetResourceName)
defer locks.UnlockByName(subnetName, subnetResourceName)

Expand Down
26 changes: 22 additions & 4 deletions azurerm/resource_arm_subnet_route_table_association.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/locks"
networkSvc "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

Expand Down Expand Up @@ -54,12 +55,20 @@ func resourceArmSubnetRouteTableAssociationCreate(d *schema.ResourceData, meta i
return err
}

parsedRouteTableId, err := networkSvc.ParseRouteTableResourceID(routeTableId)
if err != nil {
return err
}

locks.ByName(parsedRouteTableId.Name, routeTableResourceName)
defer locks.UnlockByName(parsedRouteTableId.Name, routeTableResourceName)

subnetName := parsedSubnetId.Path["subnets"]
virtualNetworkName := parsedSubnetId.Path["virtualNetworks"]
resourceGroup := parsedSubnetId.ResourceGroup

locks.ByName(subnetName, subnetResourceName)
defer locks.UnlockByName(subnetName, subnetResourceName)
locks.ByName(virtualNetworkName, virtualNetworkResourceName)
defer locks.UnlockByName(virtualNetworkName, virtualNetworkResourceName)

subnet, err := client.Get(ctx, resourceGroup, virtualNetworkName, subnetName, "")
if err != nil {
Expand Down Expand Up @@ -178,8 +187,17 @@ func resourceArmSubnetRouteTableAssociationDelete(d *schema.ResourceData, meta i
return nil
}

locks.ByName(subnetName, subnetResourceName)
defer locks.UnlockByName(subnetName, subnetResourceName)
// once we have the route table id to lock on, lock on that
parsedRouteTableId, err := networkSvc.ParseRouteTableResourceID(*props.RouteTable.ID)
if err != nil {
return err
}

locks.ByName(parsedRouteTableId.Name, routeTableResourceName)
defer locks.UnlockByName(parsedRouteTableId.Name, routeTableResourceName)

locks.ByName(virtualNetworkName, virtualNetworkResourceName)
defer locks.UnlockByName(virtualNetworkName, virtualNetworkResourceName)

// then re-retrieve it to ensure we've got the latest state
read, err = client.Get(ctx, resourceGroup, virtualNetworkName, subnetName, "")
Expand Down

0 comments on commit 2df3b8a

Please sign in to comment.