Make replacing the ACL of a network update the network in place

This prevents having to recreate the whole network and fixes #6630

builtin/providers/cloudstack/resource_cloudstack_network.go

@@ -86,14 +86,12 @@ func resourceCloudStackNetwork() *schema.Resource {
 				Type:          schema.TypeString,
 				Optional:      true,
 				Computed:      true,
-				ForceNew:      true,
 				ConflictsWith: []string{"aclid"},
 			},
 
 			"aclid": &schema.Schema{
 				Type:       schema.TypeString,
 				Optional:   true,
-				ForceNew:   true,
 				Deprecated: "Please use the `acl_id` field instead",
 			},
 
@@ -298,6 +296,25 @@ func resourceCloudStackNetworkUpdate(d *schema.ResourceData, meta interface{}) e
 			"Error updating network %s: %s", name, err)
 	}
 
+	// Replace the ACL if the ID has changed
+	if d.HasChange("acl_id") || d.HasChange("acl") {
+		aclid, ok := d.GetOk("acl_id")
+		if !ok {
+			aclid, ok = d.GetOk("acl")
+		}
+		if !ok {
+			return fmt.Errorf("Replacing the ACL requires a valid ACL ID")
+		}
+
+		p := cs.NetworkACL.NewReplaceNetworkACLListParams(aclid.(string))
+		p.SetNetworkid(d.Id())
+
+		_, err := cs.NetworkACL.ReplaceNetworkACLList(p)
+		if err != nil {
+			return fmt.Errorf("Error replacing ACL: %s", err)
+		}
+	}
+
 	// Update tags if they have changed
 	if d.HasChange("tags") {
 		err = setTags(cs, d, "network")

builtin/providers/cloudstack/resource_cloudstack_network_acl.go

@@ -117,7 +117,9 @@ func resourceCloudStackNetworkACLDelete(d *schema.ResourceData, meta interface{}
 	p := cs.NetworkACL.NewDeleteNetworkACLListParams(d.Id())
 
 	// Delete the network ACL list
-	_, err := cs.NetworkACL.DeleteNetworkACLList(p)
+	_, err := Retry(3, func() (interface{}, error) {
+		return cs.NetworkACL.DeleteNetworkACLList(p)
+	})
 	if err != nil {
 		// This is a very poor way to be told the ID does no longer exist :(
 		if strings.Contains(err.Error(), fmt.Sprintf(

builtin/providers/cloudstack/resource_cloudstack_network_test.go

@@ -50,6 +50,35 @@ func TestAccCloudStackNetwork_vpc(t *testing.T) {
 	})
 }
 
+func TestAccCloudStackNetwork_updateACL(t *testing.T) {
+	var network cloudstack.Network
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckCloudStackNetworkDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccCloudStackNetwork_acl,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudStackNetworkExists(
+						"cloudstack_network.foo", &network),
+					testAccCheckCloudStackNetworkVPCAttributes(&network),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccCloudStackNetwork_updateACL,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudStackNetworkExists(
+						"cloudstack_network.foo", &network),
+					testAccCheckCloudStackNetworkVPCAttributes(&network),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckCloudStackNetworkExists(
 	n string, network *cloudstack.Network) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
@@ -193,3 +222,57 @@ resource "cloudstack_network" "foo" {
 	CLOUDSTACK_ZONE,
 	CLOUDSTACK_VPC_NETWORK_CIDR,
 	CLOUDSTACK_VPC_NETWORK_OFFERING)
+
+var testAccCloudStackNetwork_acl = fmt.Sprintf(`
+resource "cloudstack_vpc" "foobar" {
+	name = "terraform-vpc"
+	cidr = "%s"
+	vpc_offering = "%s"
+	zone = "%s"
+}
+
+resource "cloudstack_network_acl" "foo" {
+	name = "foo"
+	vpc_id = "${cloudstack_vpc.foobar.id}"
+}
+
+resource "cloudstack_network" "foo" {
+	name = "terraform-network"
+	cidr = "%s"
+	network_offering = "%s"
+	vpc_id = "${cloudstack_vpc.foobar.id}"
+	acl_id = "${cloudstack_network_acl.foo.id}"
+	zone = "${cloudstack_vpc.foobar.zone}"
+}`,
+	CLOUDSTACK_VPC_CIDR_1,
+	CLOUDSTACK_VPC_OFFERING,
+	CLOUDSTACK_ZONE,
+	CLOUDSTACK_VPC_NETWORK_CIDR,
+	CLOUDSTACK_VPC_NETWORK_OFFERING)
+
+var testAccCloudStackNetwork_updateACL = fmt.Sprintf(`
+resource "cloudstack_vpc" "foobar" {
+	name = "terraform-vpc"
+	cidr = "%s"
+	vpc_offering = "%s"
+	zone = "%s"
+}
+
+resource "cloudstack_network_acl" "bar" {
+	name = "bar"
+	vpc_id = "${cloudstack_vpc.foobar.id}"
+}
+
+resource "cloudstack_network" "foo" {
+	name = "terraform-network"
+	cidr = "%s"
+	network_offering = "%s"
+	vpc_id = "${cloudstack_vpc.foobar.id}"
+	acl_id = "${cloudstack_network_acl.bar.id}"
+	zone = "${cloudstack_vpc.foobar.zone}"
+}`,
+	CLOUDSTACK_VPC_CIDR_1,
+	CLOUDSTACK_VPC_OFFERING,
+	CLOUDSTACK_ZONE,
+	CLOUDSTACK_VPC_NETWORK_CIDR,
+	CLOUDSTACK_VPC_NETWORK_OFFERING)

website/source/docs/providers/cloudstack/r/network.html.markdown

@@ -57,10 +57,9 @@ The following arguments are supported:
     for. Changing this forces a new resource to be created.
 
 * `acl_id` - (Optional) The network ACL ID that should be attached to the network.
-    Changing this forces a new resource to be created.
 
 * `aclid` - (Optional, Deprecated) The ID of a network ACL that should be attached
-    to the network. Changing this forces a new resource to be created.
+    to the network.
 
 * `project` - (Optional) The name or ID of the project to deploy this
     instance to. Changing this forces a new resource to be created.