Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

network: reverting the locking changes from #3673 #4320

Merged
merged 1 commit into from
Sep 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
}

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

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

return &networkSecurityGroup, 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