fix vpc acl inconsistent when port is ALL (#2135)

* fix vpc acl inconsistent when port is ALL * add changelog
tencentcloudstack · Sep 20, 2023 · d7b83df · d7b83df
1 parent 06730df
commit d7b83df
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 12 deletions.
diff --git a/.changelog/2135.txt b/.changelog/2135.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/tencentcloud_vpc_acl: Fix vpc acl entry inconsistent problem while port is `ALL`.
+```
diff --git a/tencentcloud/resource_tc_vpc_acl.go b/tencentcloud/resource_tc_vpc_acl.go
@@ -39,6 +39,8 @@ import (
 	"log"
 	"strings"
 
+	vpc "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/vpc/v20170312"
+
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/tencentcloudstack/terraform-provider-tencentcloud/tencentcloud/internal/helper"
 )
@@ -190,28 +192,77 @@ func resourceTencentCloudVpcACLRead(d *schema.ResourceData, meta interface{}) er
 	_ = d.Set("name", info.NetworkAclName)
 	egressList := make([]string, 0, len(info.EgressEntries))
 	for i := range info.EgressEntries {
-		if info.EgressEntries[i].Port == nil || *info.EgressEntries[i].Port == "" {
+		// remove default rule
+		if CheckIfDefaultRule(info.EgressEntries[i]) {
 			continue
 		}
+
+		var (
+			action    string
+			cidrBlock string
+			port      string
+			protocol  string
+		)
+
+		if info.EgressEntries[i].Action != nil {
+			action = *info.EgressEntries[i].Action
+		}
+		if info.EgressEntries[i].CidrBlock != nil {
+			cidrBlock = *info.EgressEntries[i].CidrBlock
+		}
+		if info.EgressEntries[i].Port == nil || *info.EgressEntries[i].Port == "" {
+			port = "ALL"
+		} else {
+			port = *info.EgressEntries[i].Port
+		}
+		if info.EgressEntries[i].Protocol != nil {
+			protocol = *info.EgressEntries[i].Protocol
+		}
+
 		result := strings.Join([]string{
-			*info.EgressEntries[i].Action,
-			*info.EgressEntries[i].CidrBlock,
-			*info.EgressEntries[i].Port,
-			*info.EgressEntries[i].Protocol,
+			action,
+			cidrBlock,
+			port,
+			protocol,
 		}, FILED_SP)
+
 		egressList = append(egressList, strings.ToUpper(result))
 	}
 
 	ingressList := make([]string, 0, len(info.IngressEntries))
 	for i := range info.IngressEntries {
-		if info.IngressEntries[i].Port == nil || *info.IngressEntries[i].Port == "" {
+		// remove default rule
+		if CheckIfDefaultRule(info.IngressEntries[i]) {
 			continue
 		}
+
+		var (
+			action    string
+			cidrBlock string
+			port      string
+			protocol  string
+		)
+
+		if info.IngressEntries[i].Action != nil {
+			action = *info.IngressEntries[i].Action
+		}
+		if info.IngressEntries[i].CidrBlock != nil {
+			cidrBlock = *info.IngressEntries[i].CidrBlock
+		}
+		if info.IngressEntries[i].Port == nil || *info.IngressEntries[i].Port == "" {
+			port = "ALL"
+		} else {
+			port = *info.IngressEntries[i].Port
+		}
+		if info.IngressEntries[i].Protocol != nil {
+			protocol = *info.IngressEntries[i].Protocol
+		}
+
 		result := strings.Join([]string{
-			*info.IngressEntries[i].Action,
-			*info.IngressEntries[i].CidrBlock,
-			*info.IngressEntries[i].Port,
-			*info.IngressEntries[i].Protocol,
+			action,
+			cidrBlock,
+			port,
+			protocol,
 		}, FILED_SP)
 		ingressList = append(ingressList, strings.ToUpper(result))
 	}
@@ -350,3 +401,19 @@ func resourceTencentCloudVpcACLDelete(d *schema.ResourceData, meta interface{})
 	}
 	return nil
 }
+
+func CheckIfDefaultRule(aclEntry *vpc.NetworkAclEntry) bool {
+	// remove default ipv6 rule
+	if aclEntry.Protocol != nil && *aclEntry.Protocol == "all" &&
+		aclEntry.Ipv6CidrBlock != nil && *aclEntry.Ipv6CidrBlock == "::/0" &&
+		aclEntry.Action != nil && *aclEntry.Action == "Accept" {
+		return true
+	}
+	// remove default cidr rule
+	if aclEntry.Protocol != nil && *aclEntry.Protocol == "all" &&
+		aclEntry.CidrBlock != nil && *aclEntry.CidrBlock == "0.0.0.0/0" &&
+		aclEntry.Action != nil && *aclEntry.Action == "Drop" {
+		return true
+	}
+	return false
+}
diff --git a/tencentcloud/resource_tc_vpc_acl_test.go b/tencentcloud/resource_tc_vpc_acl_test.go
@@ -9,7 +9,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 )
 
-func TestAccTencentCloudVpcAcl_basic(t *testing.T) {
+func TestAccTencentCloudVpcAclResource_basic(t *testing.T) {
 	t.Parallel()
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -33,7 +33,7 @@ func TestAccTencentCloudVpcAcl_basic(t *testing.T) {
 		},
 	})
 }
-func TestAccTencentCloudVpcAclRulesUpdate(t *testing.T) {
+func TestAccTencentCloudVpcAclRulesResource_Update(t *testing.T) {
 	t.Parallel()
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -89,6 +89,15 @@ func TestAccTencentCloudVpcAclRulesUpdate(t *testing.T) {
 					resource.TestCheckResourceAttr("tencentcloud_vpc_acl.foo", "egress.1", "ACCEPT#192.168.1.0/24#800-900#TCP"),
 				),
 			},
+			{
+				Config: testAccVpcACLConfigAllRules,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckVpcACLExists("tencentcloud_vpc_acl.foo"),
+					resource.TestCheckResourceAttr("tencentcloud_vpc_acl.foo", "name", "test_acl_update"),
+					resource.TestCheckResourceAttr("tencentcloud_vpc_acl.foo", "ingress.0", "ACCEPT#0.0.0.0/0#ALL#ALL"),
+					resource.TestCheckResourceAttr("tencentcloud_vpc_acl.foo", "egress.0", "ACCEPT#0.0.0.0/0#ALL#ALL"),
+				),
+			},
 		},
 	})
 }
@@ -222,3 +231,19 @@ resource "tencentcloud_vpc_acl" "foo" {
 	]
 } 
 `
+const testAccVpcACLConfigAllRules = `
+data "tencentcloud_vpc_instances" "default" {
+  is_default = true
+}
+
+resource "tencentcloud_vpc_acl" "foo" {
+  vpc_id  = data.tencentcloud_vpc_instances.default.instance_list.0.vpc_id
+  name    = "test_acl_update"
+  ingress = [
+    "ACCEPT#0.0.0.0/0#ALL#ALL"
+  ]
+  egress  = [
+    "ACCEPT#0.0.0.0/0#ALL#ALL"
+  ]
+}
+`