Merge pull request #479 from Sherlock-Holo/lite-rule-optimize

refactor: refine tencentcloud_security_group_lite_rule

Author: likexian

.travis.yml

@@ -29,4 +29,6 @@ matrix:
     - go: tip
 
 env:
-  global: GOFLAGS=-mod=vendor
+  global:
+    - GOFLAGS=-mod=vendor
+    - GO111MODULE=on

CHANGELOG.md

@@ -11,7 +11,7 @@ ENHANCEMENTS:
 * Data Source: `tencentcloud_mongodb_instances` add new argument `charge_type` and `auto_renew_flag` to support prepaid type.
 * Resource: `tencentcloud_mongodb_instance` supports prepaid type, new mongodb SDK version `2019-07-25` and standby instance.
 * Resource: `tencentcloud_mongodb_sharding_instance` supports prepaid type, new mongodb SDK version `2019-07-25` and standby instance.
-
+* Resource: `tencentcloud_security_group_lite_rule` refine update process and doc.
 
 ## 1.39.0 (July 18, 2020)
 

GNUmakefile

@@ -131,6 +131,8 @@ ifeq (,$(wildcard $(GOPATH)/src/$(WEBSITE_REPO)))
 	echo "$(WEBSITE_REPO) not found in your GOPATH (necessary for layouts and assets), getting..."
 	git clone https://$(WEBSITE_REPO) $(GOPATH)/src/$(WEBSITE_REPO)
 endif
+	ln -sf ../../../../ext/providers/tencentcloud/website/docs $(GOPATH)/src/github.com/hashicorp/terraform-website/content/source/docs/providers/tencentcloud
+	ln -sf ../../../ext/providers/tencentcloud/website/tencentcloud.erb $(GOPATH)/src/github.com/hashicorp/terraform-website/content/source/layouts/tencentcloud.erb
 	@$(MAKE) -C $(GOPATH)/src/$(WEBSITE_REPO) website-provider PROVIDER_PATH=$(shell pwd) PROVIDER_NAME=$(PKG_NAME)
 
 website-lint:
@@ -155,6 +157,8 @@ ifeq (,$(wildcard $(GOPATH)/src/$(WEBSITE_REPO)))
 	echo "$(WEBSITE_REPO) not found in your GOPATH (necessary for layouts and assets), getting..."
 	git clone https://$(WEBSITE_REPO) $(GOPATH)/src/$(WEBSITE_REPO)
 endif
+	ln -sf ../../../../ext/providers/tencentcloud/website/docs $(GOPATH)/src/github.com/hashicorp/terraform-website/content/source/docs/providers/tencentcloud
+	ln -sf ../../../ext/providers/tencentcloud/website/tencentcloud.erb $(GOPATH)/src/github.com/hashicorp/terraform-website/content/source/layouts/tencentcloud.erb
 	@$(MAKE) -C $(GOPATH)/src/$(WEBSITE_REPO) website-provider-test PROVIDER_PATH=$(shell pwd) PROVIDER_NAME=$(PKG_NAME)
 
 .PHONY: build sweep test testacc fmt fmtcheck lint tools test-compile doc hooks website website-lint website-test

tencentcloud/resource_tc_security_group_lite_rule.go

@@ -1,7 +1,7 @@
 /*
 Provide a resource to create security group some lite rules quickly.
 
--> **NOTE:** It can't be used with tencentcloud_security_group_rule.
+-> **NOTE:** It can't be used with tencentcloud_security_group_rule, and don't create multiple tencentcloud_security_group_rule resources, otherwise it may cause problems.
 
 Example Usage
 
@@ -170,36 +170,103 @@ func resourceTencentCloudSecurityGroupLiteRuleUpdate(d *schema.ResourceData, m i
 	service := VpcService{client: m.(*TencentCloudClient).apiV3Conn}
 
 	var (
-		ingress []VpcSecurityGroupLiteRule
-		egress  []VpcSecurityGroupLiteRule
+		ingress       []VpcSecurityGroupLiteRule
+		egress        []VpcSecurityGroupLiteRule
+		deleteIngress bool
+		deleteEgress  bool
 	)
 
-	if raw, ok := d.GetOk("ingress"); ok {
-		ingressStrs := helper.InterfacesStrings(raw.([]interface{}))
-		for _, ingressStr := range ingressStrs {
-			liteRule, err := parseRule(ingressStr)
-			if err != nil {
-				return err
+	if d.HasChange("ingress") {
+		if raw, ok := d.GetOk("ingress"); ok {
+			ingressStrs := helper.InterfacesStrings(raw.([]interface{}))
+			for _, ingressStr := range ingressStrs {
+				liteRule, err := parseRule(ingressStr)
+				if err != nil {
+					return err
+				}
+				ingress = append(ingress, liteRule)
 			}
-			ingress = append(ingress, liteRule)
+		} else {
+			old, _ := d.GetChange("ingress")
+			ingressStrs := helper.InterfacesStrings(old.([]interface{}))
+			for _, ingressStr := range ingressStrs {
+				liteRule, err := parseRule(ingressStr)
+				if err != nil {
+					return err
+				}
+				ingress = append(ingress, liteRule)
+			}
+
+			deleteIngress = true
 		}
 	}
 
-	if raw, ok := d.GetOk("egress"); ok {
-		egressStrs := helper.InterfacesStrings(raw.([]interface{}))
-		for _, egressStr := range egressStrs {
-			liteRule, err := parseRule(egressStr)
-			if err != nil {
-				return err
+	if d.HasChange("egress") {
+		if raw, ok := d.GetOk("egress"); ok {
+			egressStrs := helper.InterfacesStrings(raw.([]interface{}))
+			for _, egressStr := range egressStrs {
+				liteRule, err := parseRule(egressStr)
+				if err != nil {
+					return err
+				}
+				egress = append(egress, liteRule)
 			}
-			egress = append(egress, liteRule)
+		} else {
+			old, _ := d.GetChange("egress")
+			egressStrs := helper.InterfacesStrings(old.([]interface{}))
+			for _, egressStr := range egressStrs {
+				liteRule, err := parseRule(egressStr)
+				if err != nil {
+					return err
+				}
+				egress = append(egress, liteRule)
+			}
+
+			deleteEgress = true
 		}
 	}
 
-	if err := service.AttachLiteRulesToSecurityGroup(ctx, id, ingress, egress); err != nil {
-		return err
+	d.Partial(true)
+
+	if deleteIngress && deleteEgress {
+		if err := service.DetachAllLiteRulesFromSecurityGroup(ctx, id); err != nil {
+			return err
+		}
+
+		d.Partial(false)
+
+		return resourceTencentCloudSecurityGroupLiteRuleRead(d, m)
+	}
+
+	if deleteIngress {
+		if err := service.DeleteLiteRules(ctx, id, ingress, true); err != nil {
+			return err
+		}
+
+		d.SetPartial("ingress")
+
+		ingress = nil
 	}
 
+	if deleteEgress {
+		if err := service.DeleteLiteRules(ctx, id, egress, false); err != nil {
+			return err
+		}
+
+		d.SetPartial("egress")
+
+		egress = nil
+	}
+
+	// if both len == 0, means both rules are deleted
+	if len(ingress) > 0 || len(egress) > 0 {
+		if err := service.modifyLiteRulesInSecurityGroup(ctx, id, ingress, egress); err != nil {
+			return err
+		}
+	}
+
+	d.Partial(false)
+
 	return resourceTencentCloudSecurityGroupLiteRuleRead(d, m)
 }
 

tencentcloud/resource_tc_security_group_lite_rule_test.go

@@ -11,6 +11,8 @@ import (
 )
 
 func TestAccTencentCloudSecurityGroupLiteRule_basic(t *testing.T) {
+	t.Parallel()
+
 	var liteRuleId string
 
 	resource.Test(t, resource.TestCase{
@@ -41,6 +43,8 @@ func TestAccTencentCloudSecurityGroupLiteRule_basic(t *testing.T) {
 }
 
 func TestAccTencentCloudSecurityGroupLiteRule_update(t *testing.T) {
+	t.Parallel()
+
 	var liteRuleId string
 
 	resource.Test(t, resource.TestCase{

tencentcloud/service_tencentcloud_vpc.go

@@ -1404,40 +1404,32 @@ func (me *VpcService) modifyLiteRulesInSecurityGroup(ctx context.Context, sgId s
 	request.SecurityGroupId = &sgId
 	request.SecurityGroupPolicySet = new(vpc.SecurityGroupPolicySet)
 
-	for _, in := range ingress {
+	for i := range egress {
 		policy := &vpc.SecurityGroupPolicy{
-			Protocol:  helper.String(in.protocol),
-			CidrBlock: helper.String(in.cidrIp),
-			Action:    helper.String(in.action),
+			Protocol:  &egress[i].protocol,
+			CidrBlock: &egress[i].cidrIp,
+			Action:    &egress[i].action,
 		}
 
-		if in.port != "" {
-			policy.Port = helper.String(in.port)
+		if egress[i].port != "" {
+			policy.Port = &egress[i].port
 		}
 
-		request.SecurityGroupPolicySet.Ingress = append(request.SecurityGroupPolicySet.Ingress, policy)
+		request.SecurityGroupPolicySet.Egress = append(request.SecurityGroupPolicySet.Egress, policy)
 	}
 
-	for _, eg := range egress {
+	for i := range ingress {
 		policy := &vpc.SecurityGroupPolicy{
-			Protocol:  helper.String(eg.protocol),
-			CidrBlock: helper.String(eg.cidrIp),
-			Action:    helper.String(eg.action),
+			Protocol:  &ingress[i].protocol,
+			CidrBlock: &ingress[i].cidrIp,
+			Action:    &ingress[i].action,
 		}
 
-		if eg.port != "" {
-			policy.Port = helper.String(eg.port)
+		if ingress[i].port != "" {
+			policy.Port = &ingress[i].port
 		}
 
-		request.SecurityGroupPolicySet.Egress = append(request.SecurityGroupPolicySet.Egress, policy)
-	}
-
-	// delete all rules
-	if len(request.SecurityGroupPolicySet.Ingress) == 0 && len(request.SecurityGroupPolicySet.Egress) == 0 {
-		request.SecurityGroupPolicySet.Ingress = nil
-		request.SecurityGroupPolicySet.Egress = nil
-		// 0 means delete all rules
-		request.SecurityGroupPolicySet.Version = helper.String("0")
+		request.SecurityGroupPolicySet.Ingress = append(request.SecurityGroupPolicySet.Ingress, policy)
 	}
 
 	return resource.Retry(writeRetryTimeout, func() *resource.RetryError {
@@ -1453,19 +1445,55 @@ func (me *VpcService) modifyLiteRulesInSecurityGroup(ctx context.Context, sgId s
 	})
 }
 
-func (me *VpcService) AttachLiteRulesToSecurityGroup(ctx context.Context, sgId string, ingress, egress []VpcSecurityGroupLiteRule) error {
+func (me *VpcService) DeleteLiteRules(ctx context.Context, sgId string, rules []VpcSecurityGroupLiteRule, isIngress bool) error {
 	logId := getLogId(ctx)
 
-	// if we want to delete a direction rules, we must delete all and then attach we want rules again
-	if len(ingress) == 0 || len(egress) == 0 {
-		if err := me.DetachAllLiteRulesFromSecurityGroup(ctx, sgId); err != nil {
-			log.Printf("[CRITAL]%s attach lite rules to security group failed, reason: %v", logId, err)
-			return err
+	request := vpc.NewDeleteSecurityGroupPoliciesRequest()
+	request.SecurityGroupId = &sgId
+	request.SecurityGroupPolicySet = new(vpc.SecurityGroupPolicySet)
+
+	polices := make([]*vpc.SecurityGroupPolicy, 0, len(rules))
+
+	for i := range rules {
+		policy := &vpc.SecurityGroupPolicy{
+			Protocol:  &rules[i].protocol,
+			CidrBlock: &rules[i].cidrIp,
+			Action:    &rules[i].action,
+		}
+
+		if rules[i].port != "" {
+			policy.Port = &rules[i].port
 		}
+
+		polices = append(polices, policy)
 	}
 
+	if isIngress {
+		request.SecurityGroupPolicySet.Ingress = polices
+	} else {
+		request.SecurityGroupPolicySet.Egress = polices
+	}
+
+	return resource.Retry(writeRetryTimeout, func() *resource.RetryError {
+		ratelimit.Check(request.GetAction())
+
+		if _, err := me.client.UseVpcClient().DeleteSecurityGroupPolicies(request); err != nil {
+			log.Printf("[CRITAL]%s api[%s] fail, request body [%s], reason[%v]",
+				logId, request.GetAction(), request.ToJsonString(), err)
+
+			return retryError(err)
+		}
+
+		return nil
+	})
+}
+
+func (me *VpcService) AttachLiteRulesToSecurityGroup(ctx context.Context, sgId string, ingress, egress []VpcSecurityGroupLiteRule) error {
+	logId := getLogId(ctx)
+
 	if err := me.modifyLiteRulesInSecurityGroup(ctx, sgId, ingress, egress); err != nil {
 		log.Printf("[CRITAL]%s attach lite rules to security group failed, reason: %v", logId, err)
+
 		return err
 	}
 
@@ -1552,12 +1580,23 @@ func (me *VpcService) DescribeSecurityGroupPolices(ctx context.Context, sgId str
 func (me *VpcService) DetachAllLiteRulesFromSecurityGroup(ctx context.Context, sgId string) error {
 	logId := getLogId(ctx)
 
-	if err := me.modifyLiteRulesInSecurityGroup(ctx, sgId, nil, nil); err != nil {
-		log.Printf("[CRITAL]%s detach all lite rules from security group failed, reason: %v", logId, err)
-		return err
+	request := vpc.NewModifySecurityGroupPoliciesRequest()
+	request.SecurityGroupId = &sgId
+	request.SecurityGroupPolicySet = &vpc.SecurityGroupPolicySet{
+		Version: helper.String("0"),
 	}
 
-	return nil
+	return resource.Retry(writeRetryTimeout, func() *resource.RetryError {
+		ratelimit.Check(request.GetAction())
+
+		if _, err := me.client.UseVpcClient().ModifySecurityGroupPolicies(request); err != nil {
+			log.Printf("[CRITAL]%s api[%s] fail, request body [%s], reason[%v]",
+				logId, request.GetAction(), request.ToJsonString(), err)
+			return retryError(err)
+		}
+
+		return nil
+	})
 }
 
 type securityGroupRuleBasicInfo struct {

website/docs/r/security_group_lite_rule.html.markdown

@@ -10,7 +10,7 @@ description: |-
 
 Provide a resource to create security group some lite rules quickly.
 
--> **NOTE:** It can't be used with tencentcloud_security_group_rule.
+-> **NOTE:** It can't be used with tencentcloud_security_group_rule, and don't create multiple tencentcloud_security_group_rule resources, otherwise it may cause problems.
 
 ## Example Usage