Skip to content

Commit e9ea9a0

Browse files
tongyimingmikatong
tongyiming
and
mikatong
authored
Fix/security group rule (#1293)
* fix: delete rule timeout * fix: comparePolicyAndSecurityGroupInfo * fix unit test * fix unit test * update doc Co-authored-by: mikatong <mikatong@tencent.com>
1 parent 808c9be commit e9ea9a0

File tree

4 files changed

+100
-31
lines changed

4 files changed

+100
-31
lines changed

‎tencentcloud/resource_tc_security_group_rule.go‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import (
6262
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
6363
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
6464
"github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common"
65+
sdkErrors "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common/errors"
6566
)
6667

6768
func resourceTencentCloudSecurityGroupRule() *schema.Resource {
@@ -143,7 +144,7 @@ func resourceTencentCloudSecurityGroupRule() *schema.Resource {
143144
"policy_index": {
144145
Type: schema.TypeInt,
145146
Optional: true,
146-
Computed: true,
147+
ForceNew: true,
147148
Description: "The security group rule index number, the value of which dynamically changes as the security group rule changes.",
148149
},
149150
"source_sgid": {
@@ -327,10 +328,14 @@ func resourceTencentCloudSecurityGroupRuleCreate(d *schema.ResourceData, m inter
327328
AddressTemplateGroupId: addressTemplateGroupId,
328329
ProtocolTemplateId: protocolTemplateId,
329330
ProtocolTemplateGroupId: protocolTemplateGroupId,
330-
PolicyIndex: policyIndex,
331+
}
332+
//Forward Compatibility
333+
infoWithPolicyIndex := securityGroupRuleBasicInfoWithPolicyIndex{
334+
info,
335+
policyIndex,
331336
}
332337

333-
ruleId, err := service.CreateSecurityGroupPolicy(ctx, info)
338+
ruleId, err := service.CreateSecurityGroupPolicy(ctx, infoWithPolicyIndex)
334339
if err != nil {
335340
return err
336341
}
@@ -412,9 +417,6 @@ func resourceTencentCloudSecurityGroupRuleRead(d *schema.ResourceData, m interfa
412417
}
413418
_ = d.Set("ip_protocol", inputProtocol)
414419
}
415-
if policy.PolicyIndex != nil {
416-
_ = d.Set("policy_index", *policy.PolicyIndex)
417-
}
418420
if policy.Port != nil && *policy.Port != "" {
419421
_ = d.Set("port_range", *policy.Port)
420422
}
@@ -441,9 +443,19 @@ func resourceTencentCloudSecurityGroupRuleDelete(d *schema.ResourceData, m inter
441443
service := VpcService{client: m.(*TencentCloudClient).apiV3Conn}
442444

443445
ruleId := d.Id()
444-
err := resource.Retry(writeRetryTimeout, func() *resource.RetryError {
445-
e := service.DeleteSecurityGroupPolicy(ctx, ruleId)
446+
sgId, policyType, policy, err := service.DescribeSecurityGroupPolicy(ctx, ruleId)
447+
if err != nil {
448+
log.Printf("[CRITAL]%s security group rule query failed: %s\n ", logId, err.Error())
449+
return err
450+
}
451+
err = resource.Retry(writeRetryTimeout, func() *resource.RetryError {
452+
e := service.DeleteSecurityGroupPolicyByPolicyIndex(ctx, *policy.PolicyIndex, sgId, policyType)
446453
if e != nil {
454+
if ee, ok := e.(*sdkErrors.TencentCloudSDKError); ok {
455+
if ee.GetCode() == "ResourceNotFound" {
456+
return nil
457+
}
458+
}
447459
return resource.RetryableError(fmt.Errorf("security group delete failed: %s", e.Error()))
448460
}
449461
return nil

‎tencentcloud/resource_tc_security_group_rule_test.go‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@ func TestAccTencentCloudSecurityGroupRule_basic(t *testing.T) {
2929
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in", "type", "ingress"),
3030
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in", "policy_index", "0"),
3131
resource.TestCheckNoResourceAttr("tencentcloud_security_group_rule.http-in", "source_sgid"),
32-
33-
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in1", "cidr_ip", "1.1.1.2"),
34-
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in1", "ip_protocol", "tcp"),
35-
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in1", "description", ""),
36-
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in1", "type", "ingress"),
37-
resource.TestCheckResourceAttr("tencentcloud_security_group_rule.http-in1", "policy_index", "1"),
38-
resource.TestCheckNoResourceAttr("tencentcloud_security_group_rule.http-in1", "source_sgid"),
3932
),
4033
},
4134
},
@@ -261,15 +254,6 @@ resource "tencentcloud_security_group_rule" "http-in" {
261254
policy = "accept"
262255
policy_index = 0
263256
}
264-
resource "tencentcloud_security_group_rule" "http-in1" {
265-
security_group_id = tencentcloud_security_group.foo.id
266-
type = "ingress"
267-
cidr_ip = "1.1.1.2"
268-
ip_protocol = "tcp"
269-
port_range = "80,8080"
270-
policy = "accept"
271-
policy_index = 1
272-
}
273257
`
274258

275259
const testAccSecurityGroupRuleConfigSSH = `

‎tencentcloud/service_tencentcloud_vpc.go‎

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,7 @@ func (me *VpcService) DescribeSecurityGroupsAssociate(ctx context.Context, ids [
12951295
return response.Response.SecurityGroupAssociationStatisticsSet, nil
12961296
}
12971297

1298-
func (me *VpcService) CreateSecurityGroupPolicy(ctx context.Context, info securityGroupRuleBasicInfo) (ruleId string, err error) {
1298+
func (me *VpcService) CreateSecurityGroupPolicy(ctx context.Context, info securityGroupRuleBasicInfoWithPolicyIndex) (ruleId string, err error) {
12991299
logId := getLogId(ctx)
13001300

13011301
createRequest := vpc.NewCreateSecurityGroupPoliciesRequest()
@@ -1358,7 +1358,7 @@ func (me *VpcService) CreateSecurityGroupPolicy(ctx context.Context, info securi
13581358
info.SourceSgId = common.StringPtr("")
13591359
}
13601360

1361-
ruleId, err = buildSecurityGroupRuleId(info)
1361+
ruleId, err = buildSecurityGroupRuleId(info.securityGroupRuleBasicInfo)
13621362
if err != nil {
13631363
return "", fmt.Errorf("build rule id error, reason: %v", err)
13641364
}
@@ -1476,6 +1476,10 @@ func (me *VpcService) DeleteSecurityGroupPolicy(ctx context.Context, ruleId stri
14761476
policy.ServiceTemplate.ServiceId = info.ProtocolTemplateId
14771477
}
14781478

1479+
if info.Description != nil && *info.Description != "" {
1480+
policy.PolicyDescription = info.Description
1481+
}
1482+
14791483
switch strings.ToLower(info.PolicyType) {
14801484
case "ingress":
14811485
request.SecurityGroupPolicySet.Ingress = []*vpc.SecurityGroupPolicy{policy}
@@ -1493,6 +1497,32 @@ func (me *VpcService) DeleteSecurityGroupPolicy(ctx context.Context, ruleId stri
14931497
return nil
14941498
}
14951499

1500+
func (me *VpcService) DeleteSecurityGroupPolicyByPolicyIndex(ctx context.Context, policyIndex int64, sgId, policyType string) error {
1501+
logId := getLogId(ctx)
1502+
1503+
request := vpc.NewDeleteSecurityGroupPoliciesRequest()
1504+
request.SecurityGroupId = helper.String(sgId)
1505+
request.SecurityGroupPolicySet = new(vpc.SecurityGroupPolicySet)
1506+
1507+
policy := new(vpc.SecurityGroupPolicy)
1508+
policy.PolicyIndex = helper.Int64(policyIndex)
1509+
switch strings.ToLower(policyType) {
1510+
case "ingress":
1511+
request.SecurityGroupPolicySet.Ingress = []*vpc.SecurityGroupPolicy{policy}
1512+
1513+
case "egress":
1514+
request.SecurityGroupPolicySet.Egress = []*vpc.SecurityGroupPolicy{policy}
1515+
}
1516+
ratelimit.Check(request.GetAction())
1517+
if _, err := me.client.UseVpcClient().DeleteSecurityGroupPolicies(request); err != nil {
1518+
log.Printf("[CRITAL]%s api[%s] fail, request body [%s], reason[%v]",
1519+
logId, request.GetAction(), request.ToJsonString(), err)
1520+
return err
1521+
}
1522+
return nil
1523+
1524+
}
1525+
14961526
func (me *VpcService) ModifySecurityGroupPolicy(ctx context.Context, ruleId string, desc *string) error {
14971527
logId := getLogId(ctx)
14981528

@@ -1792,7 +1822,11 @@ type securityGroupRuleBasicInfo struct {
17921822
AddressTemplateGroupId *string `json:"address_template_group_id,omitempty"`
17931823
ProtocolTemplateId *string `json:"protocol_template_id,omitempty"`
17941824
ProtocolTemplateGroupId *string `json:"protocol_template_group_id,omitempty"`
1795-
PolicyIndex int64 `json:"policy_index"`
1825+
}
1826+
1827+
type securityGroupRuleBasicInfoWithPolicyIndex struct {
1828+
securityGroupRuleBasicInfo
1829+
PolicyIndex int64 `json:"policy_index"`
17961830
}
17971831

17981832
// Build an ID for a Security Group Rule (new version)
@@ -1879,23 +1913,44 @@ func parseSecurityGroupRuleId(ruleId string) (info securityGroupRuleBasicInfo, e
18791913
}
18801914

18811915
func comparePolicyAndSecurityGroupInfo(policy *vpc.SecurityGroupPolicy, info securityGroupRuleBasicInfo) bool {
1916+
if policy.PolicyDescription != nil && *policy.PolicyDescription != "" {
1917+
if info.Description == nil || *policy.PolicyDescription != *info.Description {
1918+
return false
1919+
}
1920+
} else {
1921+
if info.Description != nil && *info.Description != "" {
1922+
return false
1923+
}
1924+
}
18821925
// policy.CidrBlock will be nil if address template is set
18831926
if policy.CidrBlock != nil && *policy.CidrBlock != "" {
1884-
if *policy.CidrBlock != *info.CidrIp {
1927+
if info.CidrIp == nil || *policy.CidrBlock != *info.CidrIp {
1928+
return false
1929+
}
1930+
} else {
1931+
if info.CidrIp != nil && *info.CidrIp != "" {
18851932
return false
18861933
}
18871934
}
18881935

18891936
// policy.Port will be nil if protocol template is set
18901937
if policy.Port != nil && *policy.Port != "" {
1891-
if *policy.Port != *info.PortRange {
1938+
if info.PortRange == nil || *policy.Port != *info.PortRange {
1939+
return false
1940+
}
1941+
} else {
1942+
if info.PortRange != nil && *info.PortRange != "" && *info.PortRange != "ALL" {
18921943
return false
18931944
}
18941945
}
18951946

18961947
// policy.Protocol will be nil if protocol template is set
18971948
if policy.Protocol != nil && *policy.Protocol != "" {
1898-
if !strings.EqualFold(*policy.Protocol, *info.Protocol) {
1949+
if info.Protocol == nil || !strings.EqualFold(*policy.Protocol, *info.Protocol) {
1950+
return false
1951+
}
1952+
} else {
1953+
if info.Protocol != nil && *info.Protocol != "" && *info.Protocol != "ALL" {
18991954
return false
19001955
}
19011956
}
@@ -1915,22 +1970,40 @@ func comparePolicyAndSecurityGroupInfo(policy *vpc.SecurityGroupPolicy, info sec
19151970
log.Printf("%s %v test", *info.ProtocolTemplateId, policy.ServiceTemplate)
19161971
return false
19171972
}
1973+
} else {
1974+
if policy.ServiceTemplate != nil && policy.ServiceTemplate.ServiceId != nil && *policy.ServiceTemplate.ServiceId != "" {
1975+
return false
1976+
}
19181977
}
1978+
19191979
if info.ProtocolTemplateGroupId != nil && *info.ProtocolTemplateGroupId != "" {
19201980
if policy.ServiceTemplate == nil || policy.ServiceTemplate.ServiceGroupId == nil || *info.ProtocolTemplateGroupId != *policy.ServiceTemplate.ServiceGroupId {
19211981
log.Printf("%s %v test", *info.ProtocolTemplateGroupId, policy.ServiceTemplate)
19221982
return false
19231983
}
1984+
} else {
1985+
if policy.ServiceTemplate != nil && policy.ServiceTemplate.ServiceGroupId != nil && *policy.ServiceTemplate.ServiceGroupId != "" {
1986+
return false
1987+
}
19241988
}
1989+
19251990
if info.AddressTemplateGroupId != nil && *info.AddressTemplateGroupId != "" {
19261991
if policy.AddressTemplate == nil || policy.AddressTemplate.AddressGroupId == nil || *info.AddressTemplateGroupId != *policy.AddressTemplate.AddressGroupId {
19271992
return false
19281993
}
1994+
} else {
1995+
if policy.AddressTemplate != nil && policy.AddressTemplate.AddressGroupId != nil && *policy.AddressTemplate.AddressGroupId != "" {
1996+
return false
1997+
}
19291998
}
19301999
if info.AddressTemplateId != nil && *info.AddressTemplateId != "" {
19312000
if policy.AddressTemplate == nil || policy.AddressTemplate.AddressId == nil || *info.AddressTemplateId != *policy.AddressTemplate.AddressId {
19322001
return false
19332002
}
2003+
} else {
2004+
if policy.AddressTemplate != nil && policy.AddressTemplate.AddressId != nil && *policy.AddressTemplate.AddressId != "" {
2005+
return false
2006+
}
19342007
}
19352008

19362009
return true

‎website/docs/r/security_group_rule.html.markdown‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ The following arguments are supported:
7070
* `cidr_ip` - (Optional, String, ForceNew) An IP address network or segment, and conflict with `source_sgid` and `address_template`.
7171
* `description` - (Optional, String, ForceNew) Description of the security group rule.
7272
* `ip_protocol` - (Optional, String, ForceNew) Type of IP protocol. Valid values: `TCP`, `UDP` and `ICMP`. Default to all types protocol, and conflicts with `protocol_template`.
73-
* `policy_index` - (Optional, Int) The security group rule index number, the value of which dynamically changes as the security group rule changes.
73+
* `policy_index` - (Optional, Int, ForceNew) The security group rule index number, the value of which dynamically changes as the security group rule changes.
7474
* `port_range` - (Optional, String, ForceNew) Range of the port. The available value can be one, multiple or one segment. E.g. `80`, `80,90` and `80-90`. Default to all ports, and confilicts with `protocol_template`.
7575
* `protocol_template` - (Optional, List, ForceNew) ID of the address template, and conflict with `ip_protocol`, `port_range`.
7676
* `source_sgid` - (Optional, String, ForceNew) ID of the nested security group, and conflicts with `cidr_ip` and `address_template`.

0 commit comments

Comments
 (0)