From e7407d3687e4c9f0081c2bcedeafcc4447a7970f Mon Sep 17 00:00:00 2001 From: Qu Xuan Date: Thu, 28 Jan 2021 11:03:44 +0800 Subject: [PATCH] fix(region): secgroup priority fix --- pkg/cloudprovider/securitygroup.go | 19 ++++++------------- pkg/compute/models/secgrouprules.go | 2 -- .../regiondrivers/secgroup_azure_test.go | 16 ++++++++-------- pkg/compute/regiondrivers/secgroup_test.go | 1 - 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/pkg/cloudprovider/securitygroup.go b/pkg/cloudprovider/securitygroup.go index 71d3541049..8bba34b335 100644 --- a/pkg/cloudprovider/securitygroup.go +++ b/pkg/cloudprovider/securitygroup.go @@ -96,7 +96,6 @@ type SecurityRule struct { Name string ExternalId string Id string - SrcPrority int } func (r SecurityRule) String() string { @@ -257,7 +256,7 @@ func CompareRules(src, dest SecRuleInfo, debug bool) (common, inAdds, outAdds, i } var _compare = func(srcRules SecurityRuleSet, destRules SecurityRuleSet) (common, add, del SecurityRuleSet) { - i, j, destPriority, srcPrority := 0, 0, (dest.MinPriority-1+dest.MaxPriority)/2, (src.MinPriority-1+src.MaxPriority)/2 + i, j, priority := 0, 0, (dest.MinPriority-1+dest.MaxPriority)/2 for i < len(srcRules) || j < len(destRules) { if i < len(srcRules) && j < len(destRules) { destRuleStr := destRules[j].String() @@ -269,34 +268,28 @@ func CompareRules(src, dest SecRuleInfo, debug bool) (common, inAdds, outAdds, i } cmp := strings.Compare(destRuleStr, srcRuleStr) if cmp == 0 { - destRules[j].SrcPrority = srcRules[i].Priority destRules[j].Id = srcRules[i].Id common = append(common, destRules[j]) - if srcRules[i].Id != DEFAULT_SRC_RULE_ID { - srcPrority = srcRules[i].Priority - } if destRules[j].ExternalId != DEFAULT_DEST_RULE_ID { - destPriority = destRules[j].Priority + priority = destRules[j].Priority } i++ j++ } else if cmp < 0 { - destRules[j].SrcPrority = srcPrority - srcPrority = addPriority(srcPrority, src.MinPriority, src.MaxPriority, src.IsOnlySupportAllowRules) del = append(del, destRules[j]) j++ } else { - srcRules[i].Priority = addPriority(destPriority, dest.MinPriority, dest.MaxPriority, dest.IsOnlySupportAllowRules) + priority = addPriority(priority, dest.MinPriority, dest.MaxPriority, dest.IsOnlySupportAllowRules) + srcRules[i].Priority = priority add = append(add, srcRules[i]) i++ } } else if i >= len(srcRules) { - destRules[j].SrcPrority = srcPrority - srcPrority = addPriority(srcPrority, src.MinPriority, src.MaxPriority, false) del = append(del, destRules[j]) j++ } else if j >= len(destRules) { - srcRules[i].Priority = addPriority(destPriority, dest.MinPriority, dest.MaxPriority, dest.IsOnlySupportAllowRules) + priority = addPriority(priority, dest.MinPriority, dest.MaxPriority, dest.IsOnlySupportAllowRules) + srcRules[i].Priority = priority add = append(add, srcRules[i]) i++ } diff --git a/pkg/compute/models/secgrouprules.go b/pkg/compute/models/secgrouprules.go index 3cacc91e85..b2e5dc0b74 100644 --- a/pkg/compute/models/secgrouprules.go +++ b/pkg/compute/models/secgrouprules.go @@ -469,8 +469,6 @@ func (self *SSecurityGroup) newFromCloudSecurityGroupRule(ctx context.Context, u cidr = rule.IPNet.String() } - rule.Priority = rule.SrcPrority - err := rule.ValidateRule() if err != nil { return nil, errors.Wrapf(err, "ValidateRule") diff --git a/pkg/compute/regiondrivers/secgroup_azure_test.go b/pkg/compute/regiondrivers/secgroup_azure_test.go index 331b7f4042..09acd02b1d 100644 --- a/pkg/compute/regiondrivers/secgroup_azure_test.go +++ b/pkg/compute/regiondrivers/secgroup_azure_test.go @@ -141,9 +141,9 @@ func TestAzureRuleSync(t *testing.T) { ruleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), }, InAdds: []cloudprovider.SecurityRule{ - ruleWithName("", "in:allow tcp 1011", 4014), - ruleWithName("", "in:allow tcp 1050", 4014), - ruleWithName("", "in:allow tcp 1002", 4014), + ruleWithName("", "in:allow tcp 1050", 4010), + ruleWithName("", "in:allow tcp 1011", 4011), + ruleWithName("", "in:allow tcp 1002", 4012), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ @@ -178,11 +178,11 @@ func TestAzureRuleSync(t *testing.T) { ruleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), }, InAdds: []cloudprovider.SecurityRule{ - ruleWithName("", "in:allow icmp", 2098), - ruleWithName("", "in:allow udp 1055", 4015), - ruleWithName("", "in:allow tcp 1050", 4014), - ruleWithName("", "in:allow tcp 1012", 4014), - ruleWithName("", "in:allow tcp 1002", 4014), + ruleWithName("", "in:allow icmp", 2096), + ruleWithName("", "in:allow tcp 1050", 4010), + ruleWithName("", "in:allow tcp 1012", 4011), + ruleWithName("", "in:allow tcp 1002", 4012), + ruleWithName("", "in:allow udp 1055", 4013), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ diff --git a/pkg/compute/regiondrivers/secgroup_test.go b/pkg/compute/regiondrivers/secgroup_test.go index 14b3898e74..84ed022df7 100644 --- a/pkg/compute/regiondrivers/secgroup_test.go +++ b/pkg/compute/regiondrivers/secgroup_test.go @@ -101,7 +101,6 @@ var ruleWithPriority = func(ruleStr string, priority int) cloudprovider.Security rule := secrules.MustParseSecurityRule(ruleStr) if rule == nil { panic(fmt.Sprintf("invalid rule str %s", ruleStr)) - return cloudprovider.SecurityRule{} } rule.Priority = priority return cloudprovider.SecurityRule{SecurityRule: *rule, Id: stringutils.UUID4()}