From 48b03cbaad575be76ec8530d02b0261f9789cdda Mon Sep 17 00:00:00 2001 From: Qu Xuan Date: Fri, 22 Jan 2021 19:52:20 +0800 Subject: [PATCH] fix(region): optimized secgroup rule sync --- pkg/cloudprovider/securitygroup.go | 448 ++++++++++-------- pkg/compute/models/secgroupcache.go | 22 +- pkg/compute/models/secgrouprules.go | 4 +- pkg/compute/models/secgroups.go | 70 +-- pkg/compute/regiondrivers/azure.go | 4 +- pkg/compute/regiondrivers/kvm.go | 21 + pkg/compute/regiondrivers/managedvirtual.go | 15 +- .../regiondrivers/secgroup_aliyun_test.go | 60 ++- .../regiondrivers/secgroup_aws_test.go | 54 +-- .../regiondrivers/secgroup_azure_test.go | 211 ++++----- .../regiondrivers/secgroup_ctyun_test.go | 39 +- .../regiondrivers/secgroup_kvm_test.go | 84 ++++ .../regiondrivers/secgroup_openstack_test.go | 51 +- .../regiondrivers/secgroup_qcloud_test.go | 36 +- pkg/compute/regiondrivers/secgroup_test.go | 115 ++++- 15 files changed, 668 insertions(+), 566 deletions(-) create mode 100644 pkg/compute/regiondrivers/secgroup_kvm_test.go diff --git a/pkg/cloudprovider/securitygroup.go b/pkg/cloudprovider/securitygroup.go index c7c072d42e..71d3541049 100644 --- a/pkg/cloudprovider/securitygroup.go +++ b/pkg/cloudprovider/securitygroup.go @@ -18,12 +18,64 @@ import ( "sort" "strings" + "gopkg.in/fatih/set.v0" + "yunion.io/x/log" "yunion.io/x/pkg/util/secrules" + "yunion.io/x/pkg/utils" ) -const DEFAULT_CLOUD_RULE_ID = "default_cloud_rule_id" -const DEFAULT_LOCAL_RULE_ID = "default_local_rule_id" +type SecDriver interface { + GetDefaultSecurityGroupInRule() SecurityRule + GetDefaultSecurityGroupOutRule() SecurityRule + GetSecurityGroupRuleMaxPriority() int + GetSecurityGroupRuleMinPriority() int + IsOnlySupportAllowRules() bool +} + +func NewSecRuleInfo(driver SecDriver) SecRuleInfo { + return SecRuleInfo{ + InDefaultRule: driver.GetDefaultSecurityGroupInRule(), + OutDefaultRule: driver.GetDefaultSecurityGroupOutRule(), + MinPriority: driver.GetSecurityGroupRuleMinPriority(), + MaxPriority: driver.GetSecurityGroupRuleMaxPriority(), + IsOnlySupportAllowRules: driver.IsOnlySupportAllowRules(), + } +} + +const DEFAULT_DEST_RULE_ID = "default_dest_rule_id" +const DEFAULT_SRC_RULE_ID = "default_src_rule_id" + +type SecRuleInfo struct { + InDefaultRule SecurityRule + OutDefaultRule SecurityRule + Rules SecurityRuleSet + MinPriority int + MaxPriority int + IsOnlySupportAllowRules bool +} + +func (r SecRuleInfo) AddDefaultRule(d SecRuleInfo, inRules, outRules []SecurityRule, isSrc bool) ([]SecurityRule, []SecurityRule) { + min, max := r.MinPriority, r.MaxPriority + r.InDefaultRule.Priority = min + 1 + r.OutDefaultRule.Priority = min + 1 + if max >= min { + r.InDefaultRule.Priority = min - 1 + r.OutDefaultRule.Priority = min - 1 + } + + if isSrc { + r.InDefaultRule.Id = DEFAULT_SRC_RULE_ID + r.OutDefaultRule.Id = DEFAULT_SRC_RULE_ID + } else { + r.InDefaultRule.ExternalId = DEFAULT_DEST_RULE_ID + r.OutDefaultRule.ExternalId = DEFAULT_DEST_RULE_ID + } + + inRules = append(inRules, r.InDefaultRule) + outRules = append(outRules, r.OutDefaultRule) + return inRules, outRules +} type SecurityGroupFilterOptions struct { VpcId string @@ -41,45 +93,10 @@ type SecurityGroupCreateInput struct { type SecurityRule struct { secrules.SecurityRule - Name string - ExternalId string - LocalRulePrority int -} - -type LocalSecurityRule struct { - secrules.SecurityRule + Name string ExternalId string -} - -func (r LocalSecurityRule) String() string { - return r.SecurityRule.String() -} - -type LocalSecurityRuleSet []LocalSecurityRule - -func (srs LocalSecurityRuleSet) Len() int { - return len(srs) -} - -func (srs LocalSecurityRuleSet) Swap(i, j int) { - srs[i], srs[j] = srs[j], srs[i] -} - -func (srs LocalSecurityRuleSet) Less(i, j int) bool { - if srs[i].Priority > srs[j].Priority { - return true - } else if srs[i].Priority == srs[j].Priority { - return srs[i].String() < srs[j].String() - } - return false -} - -func (srs LocalSecurityRuleSet) AllowList() secrules.SecurityRuleSet { - rules := secrules.SecurityRuleSet{} - for _, r := range srs { - rules = append(rules, r.SecurityRule) - } - return rules.AllowList() + Id string + SrcPrority int } func (r SecurityRule) String() string { @@ -88,6 +105,17 @@ func (r SecurityRule) String() string { type SecurityRuleSet []SecurityRule +func (rules SecurityRuleSet) Split() (in, out SecurityRuleSet) { + for i := 0; i < len(rules); i++ { + if rules[i].Direction == secrules.DIR_IN { + in = append(in, rules[i]) + } else { + out = append(out, rules[i]) + } + } + return +} + func (srs SecurityRuleSet) Len() int { return len(srs) } @@ -108,129 +136,119 @@ func (srs SecurityRuleSet) AllowList() secrules.SecurityRuleSet { return rules.AllowList() } -func AddDefaultRule(rules []SecurityRule, defaultRule SecurityRule, localRuleStr string, min, max int, onlyAllowRules bool) []SecurityRule { - if defaultRule.String() == localRuleStr { - return rules +func (srs SecurityRuleSet) Debug() { + for i := 0; i < len(srs); i++ { + log.Debugf("Name: %s id: %s external_id: %s priority: %d %s", srs[i].Name, srs[i].Id, srs[i].ExternalId, srs[i].Priority, srs[i].String()) } - defaultRule.ExternalId = DEFAULT_CLOUD_RULE_ID - if max > min { - defaultRule.Priority = min - 1 - } else { - defaultRule.Priority = max + 1 - } - return append(rules, defaultRule) } -func SortSecurityRule(rules SecurityRuleSet, max, min int, onlyAllowRules bool) { - if onlyAllowRules { - return - } - if max < min { +func SortSecurityRule(rules SecurityRuleSet, max, min int, isAsc, onlyAllowRules bool) { + if (max >= min || onlyAllowRules) && !isAsc { sort.Sort(sort.Reverse(rules)) return } sort.Sort(rules) + return } -func CompareRules( - minPriority, maxPriority int, - localRules LocalSecurityRuleSet, remoteRules []SecurityRule, - defaultInRule, defaultOutRule SecurityRule, - onlyAllowRules bool, debug bool, refreshLocalRule bool, -) (common, inAdds, outAdds, inDels, outDels []SecurityRule) { - localInRules := LocalSecurityRuleSet{} - localOutRules := LocalSecurityRuleSet{} - for i := range localRules { - if localRules[i].Direction == secrules.DIR_IN { - localInRules = append(localInRules, localRules[i]) - } else { - localOutRules = append(localOutRules, localRules[i]) - } +func isAllowListEqual(src, dest secrules.SecurityRuleSet) bool { + if len(src) != len(dest) { + return false } - inRules := SecurityRuleSet{} - outRules := SecurityRuleSet{} - for i := 0; i < len(remoteRules); i++ { - if remoteRules[i].Direction == secrules.DIR_IN { - inRules = append(inRules, remoteRules[i]) - } else { - outRules = append(outRules, remoteRules[i]) - } + s1, s2 := set.New(set.ThreadSafe), set.New(set.ThreadSafe) + for i := 0; i < len(src); i++ { + s1.Add(src[i].String()) + s2.Add(dest[i].String()) } - var inCommon, outCommon = inRules, outRules + return s1.IsEqual(s2) +} - defaultLocalInRule := LocalSecurityRule{ExternalId: DEFAULT_LOCAL_RULE_ID} - defaultLocalInRule.SecurityRule = *secrules.MustParseSecurityRule("in:deny any") - defaultLocalOutRule := LocalSecurityRule{ExternalId: DEFAULT_LOCAL_RULE_ID} - defaultLocalOutRule.SecurityRule = *secrules.MustParseSecurityRule("out:allow any") +func CompareRules(src, dest SecRuleInfo, debug bool) (common, inAdds, outAdds, inDels, outDels SecurityRuleSet) { + srcInRules, srcOutRules := src.Rules.Split() + destInRules, destOutRules := dest.Rules.Split() - inRules = AddDefaultRule(inRules, defaultInRule, defaultLocalInRule.String(), minPriority, maxPriority, onlyAllowRules) - outRules = AddDefaultRule(outRules, defaultOutRule, defaultLocalOutRule.String(), minPriority, maxPriority, onlyAllowRules) + srcInRules, srcOutRules = src.AddDefaultRule(dest, srcInRules, srcOutRules, true) + destInRules, destOutRules = dest.AddDefaultRule(src, destInRules, destOutRules, false) - defaultInEquals, defaultOutEquals := true, true - if defaultLocalInRule.String() != defaultInRule.String() { - localInRules = append(localInRules, defaultLocalInRule) - defaultInEquals = false - } - if defaultLocalOutRule.String() != defaultOutRule.String() { - localOutRules = append(localOutRules, defaultLocalOutRule) - defaultOutEquals = false + if debug { + log.Debugf("src in rules: ") + srcInRules.Debug() } - sort.Sort(localInRules) - sort.Sort(localOutRules) + // AllowList 需要优先级从高到低排序 + SortSecurityRule(srcInRules, src.MaxPriority, src.MinPriority, false, src.IsOnlySupportAllowRules) + SortSecurityRule(srcOutRules, src.MaxPriority, src.MinPriority, false, src.IsOnlySupportAllowRules) - localInAllowList := localInRules.AllowList() - localOutAllowList := localOutRules.AllowList() - _localInRules := LocalSecurityRuleSet{} - for i := range localInAllowList { - rule := LocalSecurityRule{} - rule.SecurityRule = localInAllowList[i] - _localInRules = append(_localInRules, rule) - } - _localOutRules := LocalSecurityRuleSet{} - for i := range localOutAllowList { - rule := LocalSecurityRule{} - rule.SecurityRule = localOutAllowList[i] - _localOutRules = append(_localOutRules, rule) - } - if !refreshLocalRule && onlyAllowRules { - localOutRules, localInRules = _localOutRules, _localInRules - } - if !refreshLocalRule && len(_localInRules) < len(localInRules) { - localInRules = _localInRules - } - if !refreshLocalRule && len(_localOutRules) < len(localOutRules) { - localOutRules = _localOutRules - } + SortSecurityRule(destInRules, dest.MaxPriority, dest.MinPriority, false, dest.IsOnlySupportAllowRules) + SortSecurityRule(destOutRules, dest.MaxPriority, dest.MinPriority, false, dest.IsOnlySupportAllowRules) - SortSecurityRule(inRules, maxPriority, minPriority, onlyAllowRules) - SortSecurityRule(outRules, maxPriority, minPriority, onlyAllowRules) + srcInAllowList := srcInRules.AllowList() + srcOutAllowList := srcOutRules.AllowList() + + destInAllowList := destInRules.AllowList() + destOutAllowList := destOutRules.AllowList() + inEquals, outEquals := isAllowListEqual(srcInAllowList, destInAllowList), isAllowListEqual(srcOutAllowList, destOutAllowList) - inAllowList := inRules.AllowList() - outAllowList := outRules.AllowList() - inEquals, outEquals := inAllowList.Equals(localInAllowList), outAllowList.Equals(localOutAllowList) if inEquals && outEquals { return } - // priority从小到大排列(从默认规则开始对比) - sort.Sort(sort.Reverse(localInRules)) - sort.Sort(sort.Reverse(localOutRules)) - - sort.Sort(sort.Reverse(inRules)) - sort.Sort(sort.Reverse(outRules)) - - startPriority := minPriority - 1 - if maxPriority < minPriority { - startPriority = maxPriority + 1 + if debug { + log.Debugf("In: src: %s dest: %s result: %v", srcInAllowList.String(), destInAllowList.String(), inEquals) + log.Debugf("Out: src: %s dest: %s result: %v", srcOutAllowList.String(), destOutAllowList.String(), outEquals) } - var addPriority = func(priority int, inc int, min, max int, onlyAllowRules bool) int { + var tryUseAllowList = func(defaultRule SecurityRule, allowList secrules.SecurityRuleSet, rules SecurityRuleSet, isOnlyAllowList bool) SecurityRuleSet { + if len(allowList) < len(rules) || isOnlyAllowList { + rules = SecurityRuleSet{} + for i := range allowList { + rule := SecurityRule{} + rule.SecurityRule = allowList[i] + rules = append(rules, rule) + } + + if !utils.IsInStringArray(allowList.String(), []string{ + "", + "in:allow any", + "out:allow any", + "in:deny any", + "out:deny any", + }) && strings.HasSuffix(defaultRule.SecurityRule.String(), "deny any") { + rules = append(rules, defaultRule) + } + } + return rules + } + + srcInRules = tryUseAllowList(src.InDefaultRule, srcInAllowList, srcInRules, dest.IsOnlySupportAllowRules) + srcOutRules = tryUseAllowList(src.OutDefaultRule, srcOutAllowList, srcOutRules, dest.IsOnlySupportAllowRules) + + if inEquals { + srcInRules, destInRules = []SecurityRule{}, []SecurityRule{} + } + if outEquals { + srcOutRules, destOutRules = []SecurityRule{}, []SecurityRule{} + } + + if debug { + log.Debugf("src in rules: ") + srcInRules.Debug() + } + + // 默认从优先级低到高比较 + SortSecurityRule(srcInRules, src.MaxPriority, src.MinPriority, true, src.IsOnlySupportAllowRules) + SortSecurityRule(srcOutRules, src.MaxPriority, src.MinPriority, true, src.IsOnlySupportAllowRules) + + SortSecurityRule(destInRules, dest.MaxPriority, dest.MinPriority, true, dest.IsOnlySupportAllowRules) + SortSecurityRule(destOutRules, dest.MaxPriority, dest.MinPriority, true, dest.IsOnlySupportAllowRules) + + var addPriority = func(priority int, min, max int, onlyAllowRules bool) int { if onlyAllowRules { return priority } + inc := 1 if max < min { - max, min, inc = min, max, inc*-1 + max, min, inc = min, max, -1 } if priority >= max || priority <= min { return priority @@ -238,103 +256,77 @@ func CompareRules( return priority + inc } - var getInitPriority = func(init, min, max int) int { - if init < min || init > max { - return (min + max) / 2 - } - return init - } - - var compare = func(localRules LocalSecurityRuleSet, remoteRules SecurityRuleSet) (common, add, del []SecurityRule) { - i, j, inc, prePriority, localPrority := 0, 0, 1, 0, 50 - for i < len(localRules) || j < len(remoteRules) { - if i < len(localRules) && j < len(remoteRules) { - ruleStr := remoteRules[j].String() - localRuleStr := localRules[i].String() + 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 + for i < len(srcRules) || j < len(destRules) { + if i < len(srcRules) && j < len(destRules) { + destRuleStr := destRules[j].String() + srcRuleStr := srcRules[i].String() if debug { - log.Debugf("compare local priority(%d) %s -> remote name(%s) priority(%d) %s\n", localRules[i].Priority, localRules[i].String(), remoteRules[j].Name, remoteRules[j].Priority, remoteRules[j].String()) + log.Debugf("compare src %s(%s) priority(%d) %s -> dest name(%s) %s(%s) priority(%d) %s\n", + srcRules[i].Id, srcRules[i].ExternalId, srcRules[i].Priority, srcRules[i].String(), + destRules[j].Name, destRules[j].ExternalId, destRules[j].Id, destRules[j].Priority, destRules[j].String()) } - cmp := strings.Compare(ruleStr, localRuleStr) + cmp := strings.Compare(destRuleStr, srcRuleStr) if cmp == 0 { - prePriority = remoteRules[j].Priority - if remoteRules[j].ExternalId == DEFAULT_CLOUD_RULE_ID { - remoteRules[j].Priority = addPriority(remoteRules[j].Priority, 1, minPriority, maxPriority, onlyAllowRules) + 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 localRules[i].ExternalId != DEFAULT_LOCAL_RULE_ID || - (localRules[i].Direction == secrules.DIR_IN && !defaultInEquals) || - (localRules[i].Direction == secrules.DIR_OUT && !defaultOutEquals) { - common = append(common, remoteRules[j]) + if destRules[j].ExternalId != DEFAULT_DEST_RULE_ID { + destPriority = destRules[j].Priority } - localPrority = localRules[i].Priority i++ j++ } else if cmp < 0 { - if remoteRules[j].ExternalId != DEFAULT_CLOUD_RULE_ID { - remoteRules[j].LocalRulePrority = localPrority - localPrority = addPriority(localPrority, 1, 1, 100, false) - del = append(del, remoteRules[j]) - } + destRules[j].SrcPrority = srcPrority + srcPrority = addPriority(srcPrority, src.MinPriority, src.MaxPriority, src.IsOnlySupportAllowRules) + del = append(del, destRules[j]) j++ } else { - initPriority := getInitPriority(prePriority, minPriority, maxPriority) - localRules[i].Priority = addPriority(initPriority, inc, minPriority, maxPriority, onlyAllowRules) - if localRules[i].ExternalId != DEFAULT_LOCAL_RULE_ID || - (localRules[i].Direction == secrules.DIR_IN && !defaultInEquals) || - (localRules[i].Direction == secrules.DIR_OUT && !defaultOutEquals) { - add = append(add, SecurityRule{SecurityRule: localRules[i].SecurityRule, ExternalId: localRules[i].ExternalId}) - } + srcRules[i].Priority = addPriority(destPriority, dest.MinPriority, dest.MaxPriority, dest.IsOnlySupportAllowRules) + add = append(add, srcRules[i]) i++ - inc++ - } - } else if i >= len(localRules) { - if remoteRules[j].ExternalId != DEFAULT_CLOUD_RULE_ID { - remoteRules[j].LocalRulePrority = localPrority - localPrority = addPriority(localPrority, 1, 1, 100, false) - del = append(del, remoteRules[j]) } + } 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(remoteRules) { - initPriority := startPriority - if len(remoteRules) > 0 { - initPriority = remoteRules[len(remoteRules)-1].Priority - } - initPriority = getInitPriority(initPriority, minPriority, maxPriority) // 若是初始添加规则，尽量以中间为节点，避免仅出现天地规则 - localRules[i].Priority = addPriority(initPriority, inc, minPriority, maxPriority, onlyAllowRules) - if localRules[i].ExternalId != DEFAULT_LOCAL_RULE_ID || - (localRules[i].Direction == secrules.DIR_IN && !defaultInEquals) || - (localRules[i].Direction == secrules.DIR_OUT && !defaultOutEquals) { - add = append(add, SecurityRule{SecurityRule: localRules[i].SecurityRule, ExternalId: localRules[i].ExternalId}) - } + } else if j >= len(destRules) { + srcRules[i].Priority = addPriority(destPriority, dest.MinPriority, dest.MaxPriority, dest.IsOnlySupportAllowRules) + add = append(add, srcRules[i]) i++ - inc++ } } return } type rulePair struct { - localRules LocalSecurityRuleSet - remoteRules []SecurityRule - protocol string + srcRules SecurityRuleSet + destRules SecurityRuleSet + protocol string } - var splitRules = func(localRules LocalSecurityRuleSet, remoteRules []SecurityRule) []rulePair { + var splitRules = func(src, dest SecurityRuleSet) []rulePair { rules := map[string]rulePair{} - for _, r := range localRules { + for _, r := range src { pair, ok := rules[r.Protocol] if !ok { - pair = rulePair{localRules: LocalSecurityRuleSet{}, remoteRules: []SecurityRule{}, protocol: r.Protocol} + pair = rulePair{srcRules: SecurityRuleSet{}, destRules: SecurityRuleSet{}, protocol: r.Protocol} } - pair.localRules = append(pair.localRules, r) + pair.srcRules = append(pair.srcRules, r) rules[r.Protocol] = pair } - for _, r := range remoteRules { + for _, r := range dest { pair, ok := rules[r.Protocol] if !ok { - pair = rulePair{localRules: LocalSecurityRuleSet{}, remoteRules: []SecurityRule{}, protocol: r.Protocol} + pair = rulePair{srcRules: SecurityRuleSet{}, destRules: SecurityRuleSet{}, protocol: r.Protocol} } - pair.remoteRules = append(pair.remoteRules, r) + pair.destRules = append(pair.destRules, r) rules[r.Protocol] = pair } @@ -345,24 +337,62 @@ func CompareRules( return ret } - var compareRules = func(localRules LocalSecurityRuleSet, remoteRules []SecurityRule) (common, add, dels []SecurityRule) { - pairs := splitRules(localRules, remoteRules) + var compare = func(src, dest SecurityRuleSet) (common, added, dels SecurityRuleSet) { + pairs := splitRules(src, dest) for _, r := range pairs { - _common, _add, _dels := compare(r.localRules, r.remoteRules) + _common, _add, _dels := _compare(r.srcRules, r.destRules) common = append(common, _common...) - add = append(add, _add...) + added = append(added, _add...) dels = append(dels, _dels...) } return } - if !inEquals { - inCommon, inAdds, inDels = compareRules(localInRules, inRules) + var inCommon, outCommon SecurityRuleSet + inCommon, inAdds, inDels = compare(srcInRules, destInRules) + outCommon, outAdds, outDels = compare(srcOutRules, destOutRules) + + var handleDefaultRules = func(removed, added []SecurityRule, isOnlyAllowList bool) ([]SecurityRule, []SecurityRule) { + ret := []SecurityRule{} + for _, rule := range removed { + if rule.ExternalId == DEFAULT_DEST_RULE_ID { + if debug { + log.Debugf("remove dest default rule: %s external id %s priority: %d", rule.String(), rule.ExternalId, rule.Priority) + } + if rule.Action == secrules.SecurityRuleDeny && isOnlyAllowList { + continue + } + switch rule.Action { + case secrules.SecurityRuleDeny: + rule.Action = secrules.SecurityRuleAllow + case secrules.SecurityRuleAllow: + rule.Action = secrules.SecurityRuleDeny + } + rule.Priority = dest.MinPriority + + find := false + for i := range added { + if added[i].String() == rule.String() { + find = true + break + } + } + if !find { + if debug { + log.Debugf("add new default rule: %s external id %s priority: %d", rule.String(), rule.ExternalId, rule.Priority) + } + added = append(added, rule) + } + } else { + ret = append(ret, rule) + } + } + return ret, added } - if !outEquals { - outCommon, outAdds, outDels = compareRules(localOutRules, outRules) - } - common = append(inCommon, outCommon...) + + inDels, inAdds = handleDefaultRules(inDels, inAdds, dest.IsOnlySupportAllowRules) + outDels, outAdds = handleDefaultRules(outDels, outAdds, dest.IsOnlySupportAllowRules) + common, _ = handleDefaultRules(append(inCommon, outCommon...), []SecurityRule{}, dest.IsOnlySupportAllowRules) return } diff --git a/pkg/compute/models/secgroupcache.go b/pkg/compute/models/secgroupcache.go index ec35a29fdc..9393c44d47 100644 --- a/pkg/compute/models/secgroupcache.go +++ b/pkg/compute/models/secgroupcache.go @@ -404,11 +404,12 @@ func (self *SSecurityGroupCache) syncWithCloudSecurityGroup(ctx context.Context, if cacheCount > 1 { return nil } - info, err := SecurityGroupManager.getRuleInfo(provider, ext) + dest := cloudprovider.NewSecRuleInfo(GetRegionDriver(provider.Provider)) + dest.Rules, err = ext.GetRules() if err != nil { - return errors.Wrapf(err, "getRuleInfo") + return errors.Wrapf(err, "GetRules") } - secgroup.SyncSecurityGroupRules(ctx, userCred, info) + secgroup.SyncSecurityGroupRules(ctx, userCred, dest) return nil } @@ -647,19 +648,18 @@ func (self *SSecurityGroupCache) SyncRules() error { return errors.Wrapf(err, "iSecgroup.GetRules") } - maxPriority := region.GetDriver().GetSecurityGroupRuleMaxPriority() - minPriority := region.GetDriver().GetSecurityGroupRuleMinPriority() - - defaultInRule := region.GetDriver().GetDefaultSecurityGroupInRule() - defaultOutRule := region.GetDriver().GetDefaultSecurityGroupOutRule() - onlyAllowRules := region.GetDriver().IsOnlySupportAllowRules() - localRules, err := secgroup.GetSecuritRuleSet() if err != nil { return errors.Wrapf(err, "GetSecuritRuleSet") } - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, localRules, rules, defaultInRule, defaultOutRule, onlyAllowRules, false, false) + src := cloudprovider.NewSecRuleInfo(GetRegionDriver(api.CLOUD_PROVIDER_ONECLOUD)) + src.Rules = localRules + + dest := cloudprovider.NewSecRuleInfo(GetRegionDriver(region.Provider)) + dest.Rules = rules + + common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(src, dest, false) if len(inAdds) == 0 && len(inDels) == 0 && len(outAdds) == 0 && len(outDels) == 0 { return nil diff --git a/pkg/compute/models/secgrouprules.go b/pkg/compute/models/secgrouprules.go index e3497a54f5..3cacc91e85 100644 --- a/pkg/compute/models/secgrouprules.go +++ b/pkg/compute/models/secgrouprules.go @@ -469,7 +469,7 @@ func (self *SSecurityGroup) newFromCloudSecurityGroupRule(ctx context.Context, u cidr = rule.IPNet.String() } - rule.Priority = rule.LocalRulePrority + rule.Priority = rule.SrcPrority err := rule.ValidateRule() if err != nil { @@ -477,7 +477,7 @@ func (self *SSecurityGroup) newFromCloudSecurityGroupRule(ctx context.Context, u } secrule := &SSecurityGroupRule{ - Priority: int64(rule.LocalRulePrority), + Priority: int64(rule.Priority), Protocol: protocol, Ports: rule.GetPortsString(), Direction: string(rule.Direction), diff --git a/pkg/compute/models/secgroups.go b/pkg/compute/models/secgroups.go index 6bdf82a553..20f7358be1 100644 --- a/pkg/compute/models/secgroups.go +++ b/pkg/compute/models/secgroups.go @@ -560,8 +560,8 @@ func (self *SSecurityGroup) getSecurityRules() ([]SSecurityGroupRule, error) { return rules, nil } -func (self *SSecurityGroup) GetSecuritRuleSet() (cloudprovider.LocalSecurityRuleSet, error) { - ruleSet := cloudprovider.LocalSecurityRuleSet{} +func (self *SSecurityGroup) GetSecuritRuleSet() (cloudprovider.SecurityRuleSet, error) { + ruleSet := cloudprovider.SecurityRuleSet{} rules, err := self.getSecurityRules() if err != nil { return ruleSet, errors.Wrapf(err, "getSecurityRules") @@ -572,7 +572,7 @@ func (self *SSecurityGroup) GetSecuritRuleSet() (cloudprovider.LocalSecurityRule if err != nil { return nil, errors.Wrapf(err, "toRule") } - ruleSet = append(ruleSet, cloudprovider.LocalSecurityRule{SecurityRule: *rule, ExternalId: rules[i].Id}) + ruleSet = append(ruleSet, cloudprovider.SecurityRule{SecurityRule: *rule, ExternalId: rules[i].Id}) } return ruleSet, nil } @@ -944,14 +944,18 @@ func (self *SSecurityGroup) removeRules(ruleIds []string, result *compare.SyncRe } } -func (self *SSecurityGroup) SyncSecurityGroupRules(ctx context.Context, userCred mcclient.TokenCredential, info *sRuleInfo) compare.SyncResult { +func (self *SSecurityGroup) SyncSecurityGroupRules(ctx context.Context, userCred mcclient.TokenCredential, src cloudprovider.SecRuleInfo) compare.SyncResult { result := compare.SyncResult{} localRules, err := self.GetSecuritRuleSet() if err != nil { result.Error(errors.Wrapf(err, "GetSecuritRuleSet")) return result } - _, inDels, outDels, inAdds, outAdds := cloudprovider.CompareRules(info.minPriority, info.maxPriority, localRules, info.rules, info.defaultInRule, info.defaultOutRule, info.onlyAllowRules, false, true) + + dest := cloudprovider.NewSecRuleInfo(GetRegionDriver(api.CLOUD_PROVIDER_ONECLOUD)) + dest.Rules = localRules + + _, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(src, dest, false) if len(inAdds)+len(inDels)+len(outAdds)+len(outDels) == 0 { return result } @@ -982,54 +986,14 @@ func (self *SSecurityGroup) SyncSecurityGroupRules(ctx context.Context, userCred return result } -type sRuleInfo struct { - rules []cloudprovider.SecurityRule - inRules []cloudprovider.SecurityRule - outRules []cloudprovider.SecurityRule - defaultInRule cloudprovider.SecurityRule - defaultOutRule cloudprovider.SecurityRule - onlyAllowRules bool - maxPriority int - minPriority int -} - -func (manager *SSecurityGroupManager) getRuleInfo(provider *SCloudprovider, extSec cloudprovider.ICloudSecurityGroup) (*sRuleInfo, error) { - regionDriver, err := provider.GetRegionDriver() - if err != nil { - return nil, errors.Wrap(err, "provider.GetRegionDriver") - } - - rules, err := extSec.GetRules() - if err != nil { - return nil, errors.Wrap(err, "extSec.GetRules") - } - - info := &sRuleInfo{ - rules: rules, - inRules: []cloudprovider.SecurityRule{}, - outRules: []cloudprovider.SecurityRule{}, - defaultInRule: regionDriver.GetDefaultSecurityGroupInRule(), - defaultOutRule: regionDriver.GetDefaultSecurityGroupOutRule(), - onlyAllowRules: regionDriver.IsOnlySupportAllowRules(), - maxPriority: regionDriver.GetSecurityGroupRuleMaxPriority(), - minPriority: regionDriver.GetSecurityGroupRuleMinPriority(), - } - - for i := range rules { - if rules[i].Direction == secrules.DIR_IN { - info.inRules = append(info.inRules, rules[i]) - } else { - info.outRules = append(info.outRules, rules[i]) - } - } - return info, nil -} - func (manager *SSecurityGroupManager) newFromCloudSecgroup(ctx context.Context, userCred mcclient.TokenCredential, provider *SCloudprovider, extSec cloudprovider.ICloudSecurityGroup) (*SSecurityGroup, error) { - info, err := manager.getRuleInfo(provider, extSec) + dest := cloudprovider.NewSecRuleInfo(GetRegionDriver(provider.Provider)) + var err error + dest.Rules, err = extSec.GetRules() if err != nil { - return nil, errors.Wrapf(err, "getRuleInfo") + return nil, errors.Wrapf(err, "extSec.GetRules") } + src := cloudprovider.NewSecRuleInfo(GetRegionDriver(api.CLOUD_PROVIDER_ONECLOUD)) if options.Options.EnableAutoMergeSecurityGroup { // 查询与provider在同域的安全组，比对寻找一个与云上安全组规则相同的安全组 @@ -1040,12 +1004,12 @@ func (manager *SSecurityGroupManager) newFromCloudSecgroup(ctx context.Context, return nil, errors.Wrap(err, "db.FetchModelObjects") } for i := range secgroups { - localRules, err := secgroups[i].GetSecuritRuleSet() + src.Rules, err = secgroups[i].GetSecuritRuleSet() if err != nil { log.Warningf("GetSecuritRuleSet %s(%s) error: %v", secgroups[i].Name, secgroups[i].Id, err) continue } - _, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(info.minPriority, info.maxPriority, localRules, info.rules, info.defaultInRule, info.defaultOutRule, info.onlyAllowRules, false, false) + _, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(src, dest, false) if len(inAdds) == 0 && len(outAdds) == 0 && len(inDels) == 0 && len(outDels) == 0 { return &secgroups[i], nil } @@ -1072,7 +1036,7 @@ func (manager *SSecurityGroupManager) newFromCloudSecgroup(ctx context.Context, return nil, errors.Wrapf(err, "Insert") } - secgroup.SyncSecurityGroupRules(ctx, userCred, info) + secgroup.SyncSecurityGroupRules(ctx, userCred, dest) db.OpsLog.LogEvent(&secgroup, db.ACT_CREATE, secgroup.GetShortDesc(ctx), userCred) return &secgroup, nil diff --git a/pkg/compute/regiondrivers/azure.go b/pkg/compute/regiondrivers/azure.go index 1f6af21d48..003783299d 100644 --- a/pkg/compute/regiondrivers/azure.go +++ b/pkg/compute/regiondrivers/azure.go @@ -66,11 +66,11 @@ func (self *SAzureRegionDriver) GetDefaultSecurityGroupOutRule() cloudprovider.S } func (self *SAzureRegionDriver) GetSecurityGroupRuleMaxPriority() int { - return 4096 + return 100 } func (self *SAzureRegionDriver) GetSecurityGroupRuleMinPriority() int { - return 100 + return 4096 } func (self *SAzureRegionDriver) ValidateCreateVpcData(ctx context.Context, userCred mcclient.TokenCredential, input api.VpcCreateInput) (api.VpcCreateInput, error) { diff --git a/pkg/compute/regiondrivers/kvm.go b/pkg/compute/regiondrivers/kvm.go index 7d5df9bf53..d1eb291515 100644 --- a/pkg/compute/regiondrivers/kvm.go +++ b/pkg/compute/regiondrivers/kvm.go @@ -24,6 +24,7 @@ import ( "yunion.io/x/jsonutils" "yunion.io/x/log" "yunion.io/x/pkg/errors" + "yunion.io/x/pkg/util/secrules" "yunion.io/x/pkg/utils" api "yunion.io/x/onecloud/pkg/apis/compute" @@ -67,6 +68,26 @@ func (self *SKVMRegionDriver) GetProvider() string { return api.CLOUD_PROVIDER_ONECLOUD } +func GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { + return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} +} + +func GetDefaultSecurityGroupOutRule() cloudprovider.SecurityRule { + return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("out:allow any")} +} + +func GetSecurityGroupRuleMaxPriority() int { + return 100 +} + +func GetSecurityGroupRuleMinPriority() int { + return 1 +} + +func IsOnlySupportAllowRules() bool { + return false +} + func (self *SKVMRegionDriver) ValidateCreateLoadbalancerData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) { networkV := validators.NewModelIdOrNameValidator("network", "network", ownerId) addressV := validators.NewIPv4AddrValidator("address") diff --git a/pkg/compute/regiondrivers/managedvirtual.go b/pkg/compute/regiondrivers/managedvirtual.go index 6515488e0b..15136f06f3 100644 --- a/pkg/compute/regiondrivers/managedvirtual.go +++ b/pkg/compute/regiondrivers/managedvirtual.go @@ -1682,19 +1682,18 @@ func (self *SManagedVirtualizationRegionDriver) RequestSyncSecurityGroup(ctx con return errors.Wrapf(err, "iSecgroup.GetRules") } - maxPriority := region.GetDriver().GetSecurityGroupRuleMaxPriority() - minPriority := region.GetDriver().GetSecurityGroupRuleMinPriority() - - defaultInRule := region.GetDriver().GetDefaultSecurityGroupInRule() - defaultOutRule := region.GetDriver().GetDefaultSecurityGroupOutRule() - onlyAllowRules := region.GetDriver().IsOnlySupportAllowRules() - localRules, err := secgroup.GetSecuritRuleSet() if err != nil { return errors.Wrapf(err, "GetSecuritRuleSet") } - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, localRules, rules, defaultInRule, defaultOutRule, onlyAllowRules, false, false) + src := cloudprovider.NewSecRuleInfo(&SKVMRegionDriver{}) + src.Rules = localRules + + dest := cloudprovider.NewSecRuleInfo(region.GetDriver()) + dest.Rules = rules + + common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(src, dest, false) if len(inAdds) == 0 && len(inDels) == 0 && len(outAdds) == 0 && len(outDels) == 0 { return nil diff --git a/pkg/compute/regiondrivers/secgroup_aliyun_test.go b/pkg/compute/regiondrivers/secgroup_aliyun_test.go index aac5c9d55a..9ced085fc7 100644 --- a/pkg/compute/regiondrivers/secgroup_aliyun_test.go +++ b/pkg/compute/regiondrivers/secgroup_aliyun_test.go @@ -15,37 +15,28 @@ package regiondrivers import ( - "sort" "testing" "yunion.io/x/onecloud/pkg/cloudprovider" ) func TestAliyunRuleSync(t *testing.T) { - driver := SAliyunRegionDriver{} - maxPriority := driver.GetSecurityGroupRuleMaxPriority() - minPriority := driver.GetSecurityGroupRuleMinPriority() - - defaultInRule := driver.GetDefaultSecurityGroupInRule() - defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - isOnlyAllowRules := driver.IsOnlySupportAllowRules() - data := []TestData{ { Name: "Test out rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:allow tcp 1212", 52), - localRuleWithPriority("in:allow tcp 22", 51), - localRuleWithPriority("in:allow tcp 3389", 50), - localRuleWithPriority("in:allow udp 1231", 49), - localRuleWithPriority("in:deny tcp 443", 48), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:allow tcp 1212", 52), + ruleWithPriority("in:allow tcp 22", 51), + ruleWithPriority("in:allow tcp 3389", 50), + ruleWithPriority("in:allow udp 1231", 49), + ruleWithPriority("in:deny tcp 443", 48), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:deny tcp 443", 1), - remoteRuleWithName("", "in:allow udp 1231", 1), - remoteRuleWithName("", "in:allow tcp 3389", 100), - remoteRuleWithName("", "in:allow tcp 22", 100), - remoteRuleWithName("", "in:allow tcp 1212", 100), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("", "in:deny tcp 443", 1), + ruleWithName("", "in:allow udp 1231", 1), + ruleWithName("", "in:allow tcp 3389", 100), + ruleWithName("", "in:allow tcp 22", 100), + ruleWithName("", "in:allow tcp 1212", 100), }, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{}, @@ -53,20 +44,23 @@ func TestAliyunRuleSync(t *testing.T) { InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, }, + { + Name: "Test tcp rules", + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("out:deny tcp 443", 48), + }, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, + OutAdds: []cloudprovider.SecurityRule{ + ruleWithName("", "out:deny tcp 443", 49), + }, + InDels: []cloudprovider.SecurityRule{}, + OutDels: []cloudprovider.SecurityRule{}, + }, } for _, d := range data { - t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true, true) - sort.Sort(cloudprovider.SecurityRuleSet(common)) - sort.Sort(cloudprovider.SecurityRuleSet(inAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(outAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(inDels)) - sort.Sort(cloudprovider.SecurityRuleSet(outDels)) - check(t, "common", common, d.Common) - check(t, "inAdds", inAdds, d.InAdds) - check(t, "outAdds", outAdds, d.OutAdds) - check(t, "inDels", inDels, d.InDels) - check(t, "outDels", outDels, d.OutDels) + d.Test(t, &SKVMRegionDriver{}, &SAliyunRegionDriver{}) } } diff --git a/pkg/compute/regiondrivers/secgroup_aws_test.go b/pkg/compute/regiondrivers/secgroup_aws_test.go index d1952afe55..44a7e04318 100644 --- a/pkg/compute/regiondrivers/secgroup_aws_test.go +++ b/pkg/compute/regiondrivers/secgroup_aws_test.go @@ -21,51 +21,43 @@ import ( ) func TestAwsRuleSync(t *testing.T) { - driver := SAwsRegionDriver{} - maxPriority := driver.GetSecurityGroupRuleMaxPriority() - minPriority := driver.GetSecurityGroupRuleMinPriority() - - defaultInRule := driver.GetDefaultSecurityGroupInRule() - defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - isOnlyAllowRules := driver.IsOnlySupportAllowRules() - data := []TestData{ { Name: "Test remove out allow rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 1), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("test-allow any", "out:allow any", 1), }, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 1), + ruleWithName("test-allow any", "out:allow any", 1), }, }, { Name: "Test out deny rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{}, - Common: []cloudprovider.SecurityRule{}, - InAdds: []cloudprovider.SecurityRule{}, - OutAdds: []cloudprovider.SecurityRule{}, - InDels: []cloudprovider.SecurityRule{}, - OutDels: []cloudprovider.SecurityRule{}, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, + OutAdds: []cloudprovider.SecurityRule{}, + InDels: []cloudprovider.SecurityRule{}, + OutDels: []cloudprovider.SecurityRule{}, }, { - Name: "Test out allow rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{}, - RemoteRules: []cloudprovider.SecurityRule{}, - Common: []cloudprovider.SecurityRule{}, - InAdds: []cloudprovider.SecurityRule{}, + Name: "Test out allow rules", + SrcRules: cloudprovider.SecurityRuleSet{}, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 0), + ruleWithName("", "out:allow any", 0), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, @@ -73,12 +65,6 @@ func TestAwsRuleSync(t *testing.T) { } for _, d := range data { - t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true, false) - check(t, "common", common, d.Common) - check(t, "inAdds", inAdds, d.InAdds) - check(t, "outAdds", outAdds, d.OutAdds) - check(t, "inDels", inDels, d.InDels) - check(t, "outDels", outDels, d.OutDels) + d.Test(t, &SKVMRegionDriver{}, &SAwsRegionDriver{}) } } diff --git a/pkg/compute/regiondrivers/secgroup_azure_test.go b/pkg/compute/regiondrivers/secgroup_azure_test.go index 444edbdb4b..331b7f4042 100644 --- a/pkg/compute/regiondrivers/secgroup_azure_test.go +++ b/pkg/compute/regiondrivers/secgroup_azure_test.go @@ -15,94 +15,85 @@ package regiondrivers import ( - "sort" "testing" "yunion.io/x/onecloud/pkg/cloudprovider" ) func TestAzureRuleSync(t *testing.T) { - driver := SAzureRegionDriver{} - maxPriority := driver.GetSecurityGroupRuleMaxPriority() - minPriority := driver.GetSecurityGroupRuleMinPriority() - - defaultInRule := driver.GetDefaultSecurityGroupInRule() - defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - isOnlyAllowRules := driver.IsOnlySupportAllowRules() - data := []TestData{ { - Name: "Test empty rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{}, - RemoteRules: []cloudprovider.SecurityRule{}, - Common: []cloudprovider.SecurityRule{}, - InAdds: []cloudprovider.SecurityRule{}, + Name: "Test empty rules", + SrcRules: cloudprovider.SecurityRuleSet{}, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 2099), + ruleWithName("", "out:allow any", 2096), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, }, { Name: "Test diff rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("out:allow tcp 100-200", 99), - localRuleWithPriority("out:allow udp 200-300", 98), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("out:allow tcp 100-200", 99), + ruleWithPriority("out:allow udp 200-300", 98), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("test-tcp", "out:allow tcp 100-200", 1000), - remoteRuleWithName("test-udp", "out:allow udp 200-300", 1002), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("test-tcp", "out:allow tcp 100-200", 1000), + ruleWithName("test-udp", "out:allow udp 200-300", 1002), }, Common: []cloudprovider.SecurityRule{ - remoteRuleWithName("test-tcp", "out:allow tcp 100-200", 1000), - remoteRuleWithName("test-udp", "out:allow udp 200-300", 1002), + ruleWithName("test-tcp", "out:allow tcp 100-200", 1000), + ruleWithName("test-udp", "out:allow udp 200-300", 1002), }, InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 2099), + ruleWithName("", "out:allow any", 2096), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, }, { Name: "Test add rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:allow tcp", 100), - localRuleWithPriority("in:allow udp", 99), - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:allow tcp", 100), + ruleWithPriority("in:allow udp", 99), + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("allow-ssh", "in:allow tcp 22", 300), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("allow-ssh", "in:allow tcp 22", 300), }, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow tcp", 2099), - remoteRuleWithName("", "in:allow udp", 2099), + ruleWithName("", "in:allow tcp", 2098), + ruleWithName("", "in:allow udp", 2098), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ - remoteRuleWithName("allow-ssh", "in:allow tcp 22", 300), + ruleWithName("allow-ssh", "in:allow tcp 22", 300), }, OutDels: []cloudprovider.SecurityRule{}, }, { Name: "Test insert rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:allow tcp", 100), - localRuleWithPriority("in:allow udp", 99), - localRuleWithPriority("in:allow icmp", 98), - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:allow tcp", 100), + ruleWithPriority("in:allow udp", 99), + ruleWithPriority("in:allow icmp", 98), + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("allow-tcp", "in:allow tcp", 300), - remoteRuleWithName("allow-icmp", "in:allow icmp", 400), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("allow-tcp", "in:allow tcp", 300), + ruleWithName("allow-icmp", "in:allow icmp", 400), }, Common: []cloudprovider.SecurityRule{ - remoteRuleWithName("allow-tcp", "in:allow tcp", 300), - remoteRuleWithName("allow-icmp", "in:allow icmp", 400), + ruleWithName("allow-tcp", "in:allow tcp", 300), + ruleWithName("allow-icmp", "in:allow icmp", 400), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow udp", 2099), + ruleWithName("", "in:allow udp", 2098), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{}, @@ -110,19 +101,19 @@ func TestAzureRuleSync(t *testing.T) { }, { Name: "Test icmp rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:allow tcp 33", 10), - localRuleWithPriority("in:allow tcp 22", 1), - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:allow tcp 33", 10), + ruleWithPriority("in:allow tcp 22", 1), + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("allow-tcp-22", "in:allow tcp 22", 300), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("allow-tcp-22", "in:allow tcp 22", 300), }, Common: []cloudprovider.SecurityRule{ - remoteRuleWithName("allow-tcp-22", "in:allow tcp 22", 300), + ruleWithName("allow-tcp-22", "in:allow tcp 22", 300), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow tcp 33", 301), + ruleWithName("", "in:allow tcp 33", 301), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{}, @@ -130,88 +121,82 @@ func TestAzureRuleSync(t *testing.T) { }, { Name: "Test a rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:allow tcp 1050", 5), - localRuleWithPriority("in:allow tcp 1011", 4), - localRuleWithPriority("in:allow tcp 1002", 3), - localRuleWithPriority("in:allow tcp 22", 2), - localRuleWithPriority("in:allow udp 55", 1), - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:allow tcp 1050", 5), + ruleWithPriority("in:allow tcp 1011", 4), + ruleWithPriority("in:allow tcp 1002", 3), + ruleWithPriority("in:allow tcp 22", 2), + ruleWithPriority("in:allow udp 55", 1), + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), - remoteRuleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), - remoteRuleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), - remoteRuleWithName("in_allow_tcp_1010_4011", "in:allow tcp 1010", 4011), - remoteRuleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), + ruleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), + ruleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), + ruleWithName("in_allow_tcp_1010_4011", "in:allow tcp 1010", 4011), + ruleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), }, Common: []cloudprovider.SecurityRule{ - remoteRuleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), - remoteRuleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), - remoteRuleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), - remoteRuleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), + ruleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), + ruleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow tcp 1011", 4013), + ruleWithName("", "in:allow tcp 1011", 4014), + ruleWithName("", "in:allow tcp 1050", 4014), + ruleWithName("", "in:allow tcp 1002", 4014), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ - remoteRuleWithName("in_allow_tcp_1010_4011", "in:allow tcp 1010", 4011), + ruleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), + ruleWithName("in_allow_tcp_1010_4011", "in:allow tcp 1010", 4011), + ruleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), }, OutDels: []cloudprovider.SecurityRule{}, }, { Name: "Test b rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:allow udp 1055", 20), - localRuleWithPriority("in:allow icmp", 15), - localRuleWithPriority("in:allow tcp 1050", 5), - localRuleWithPriority("in:allow tcp 1012", 4), - localRuleWithPriority("in:allow tcp 1002", 3), - localRuleWithPriority("in:allow tcp 22", 2), - localRuleWithPriority("in:allow udp 55", 1), - localRuleWithPriority("out:deny any", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:allow udp 1055", 20), + ruleWithPriority("in:allow icmp", 15), + ruleWithPriority("in:allow tcp 1050", 5), + ruleWithPriority("in:allow tcp 1012", 4), + ruleWithPriority("in:allow tcp 1002", 3), + ruleWithPriority("in:allow tcp 22", 2), + ruleWithPriority("in:allow udp 55", 1), + ruleWithPriority("out:deny any", 1), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), - remoteRuleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), - remoteRuleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), - remoteRuleWithName("in_allow_tcp_1012_4011", "in:allow tcp 1012", 4011), - remoteRuleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), - remoteRuleWithName("in_allow_tcp_1055_4009", "in:allow tcp 1055", 4009), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), + ruleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), + ruleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), + ruleWithName("in_allow_tcp_1012_4011", "in:allow tcp 1012", 4011), + ruleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), + ruleWithName("in_allow_tcp_1055_4009", "in:allow tcp 1055", 4009), }, Common: []cloudprovider.SecurityRule{ - remoteRuleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), - remoteRuleWithName("in_allow_tcp_1012_4011", "in:allow tcp 1012", 4011), - remoteRuleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), - remoteRuleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), - remoteRuleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), + ruleWithName("in_allow_tcp_22_4013", "in:allow tcp 22", 4013), + ruleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow icmp", 2099), - remoteRuleWithName("", "in:allow udp 1055", 4015), + 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), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ - remoteRuleWithName("in_allow_tcp_1055_4009", "in:allow tcp 1055", 4009), + ruleWithName("in_allow_tcp_1055_4009", "in:allow tcp 1055", 4009), + ruleWithName("in_allow_tcp_1050_4010", "in:allow tcp 1050", 4010), + ruleWithName("in_allow_tcp_1012_4011", "in:allow tcp 1012", 4011), + ruleWithName("in_allow_tcp_1002_4012", "in:allow tcp 1002", 4012), }, OutDels: []cloudprovider.SecurityRule{}, }, } for _, d := range data { - t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true, false) - sort.Sort(cloudprovider.SecurityRuleSet(common)) - sort.Sort(cloudprovider.SecurityRuleSet(inAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(outAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(inDels)) - sort.Sort(cloudprovider.SecurityRuleSet(outDels)) - check(t, "common", common, d.Common) - check(t, "inAdds", inAdds, d.InAdds) - check(t, "outAdds", outAdds, d.OutAdds) - check(t, "inDels", inDels, d.InDels) - check(t, "outDels", outDels, d.OutDels) + d.Test(t, &SKVMRegionDriver{}, &SAzureRegionDriver{}) } } @@ -222,14 +207,14 @@ func TestAzureUniqPriorityRuleSync(t *testing.T) { }{ { in: cloudprovider.SecurityRuleSet{ - remoteRuleWithName("", "out:allow icmp", 99), - remoteRuleWithName("", "out:allow tcp 100-200", 99), - remoteRuleWithName("", "out:allow udp 200-300", 99), + ruleWithName("", "out:allow icmp", 99), + ruleWithName("", "out:allow tcp 100-200", 99), + ruleWithName("", "out:allow udp 200-300", 99), }, out: cloudprovider.SecurityRuleSet{ - remoteRuleWithName("", "out:allow icmp", 99), - remoteRuleWithName("", "out:allow tcp 100-200", 98), - remoteRuleWithName("", "out:allow udp 200-300", 97), + ruleWithName("", "out:allow icmp", 99), + ruleWithName("", "out:allow tcp 100-200", 98), + ruleWithName("", "out:allow udp 200-300", 97), }, }, } diff --git a/pkg/compute/regiondrivers/secgroup_ctyun_test.go b/pkg/compute/regiondrivers/secgroup_ctyun_test.go index 7dd7c29b30..06227fb086 100644 --- a/pkg/compute/regiondrivers/secgroup_ctyun_test.go +++ b/pkg/compute/regiondrivers/secgroup_ctyun_test.go @@ -15,35 +15,27 @@ package regiondrivers import ( - "sort" "testing" "yunion.io/x/onecloud/pkg/cloudprovider" ) func TestCtyunRuleSync(t *testing.T) { - driver := SCtyunRegionDriver{} - maxPriority := driver.GetSecurityGroupRuleMaxPriority() - minPriority := driver.GetSecurityGroupRuleMinPriority() - - defaultInRule := driver.GetDefaultSecurityGroupInRule() - defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - isOnlyAllowRules := driver.IsOnlySupportAllowRules() data := []TestData{ { Name: "Test out deny rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("out:deny tcp 200", 1), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("out:deny tcp 200", 1), }, - RemoteRules: []cloudprovider.SecurityRule{}, - Common: []cloudprovider.SecurityRule{}, - InAdds: []cloudprovider.SecurityRule{}, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow icmp", 0), - remoteRuleWithName("", "out:allow tcp 1-199", 0), - remoteRuleWithName("", "out:allow tcp 201-65535", 0), - remoteRuleWithName("", "out:allow udp", 0), + ruleWithName("", "out:allow icmp", 0), + ruleWithName("", "out:allow tcp 1-199", 0), + ruleWithName("", "out:allow tcp 201-65535", 0), + ruleWithName("", "out:allow udp", 0), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, @@ -51,17 +43,6 @@ func TestCtyunRuleSync(t *testing.T) { } for _, d := range data { - t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true, false) - sort.Sort(cloudprovider.SecurityRuleSet(common)) - sort.Sort(cloudprovider.SecurityRuleSet(inAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(outAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(inDels)) - sort.Sort(cloudprovider.SecurityRuleSet(outDels)) - check(t, "common", common, d.Common) - check(t, "inAdds", inAdds, d.InAdds) - check(t, "outAdds", outAdds, d.OutAdds) - check(t, "inDels", inDels, d.InDels) - check(t, "outDels", outDels, d.OutDels) + d.Test(t, &SKVMRegionDriver{}, &SCtyunRegionDriver{}) } } diff --git a/pkg/compute/regiondrivers/secgroup_kvm_test.go b/pkg/compute/regiondrivers/secgroup_kvm_test.go new file mode 100644 index 0000000000..3663f15944 --- /dev/null +++ b/pkg/compute/regiondrivers/secgroup_kvm_test.go @@ -0,0 +1,84 @@ +// Copyright 2019 Yunion +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package regiondrivers + +import ( + "testing" + + "yunion.io/x/onecloud/pkg/cloudprovider" +) + +func TestKvmRuleSync(t *testing.T) { + data := []TestData{ + { + Name: "Test kvm deny rules", + SrcRules: cloudprovider.SecurityRuleSet{}, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, + OutAdds: []cloudprovider.SecurityRule{ + ruleWithName("", "out:deny any", 1), + }, + InDels: []cloudprovider.SecurityRule{}, + OutDels: []cloudprovider.SecurityRule{}, + }, + } + + for _, d := range data { + d.Test(t, &SAzureRegionDriver{}, &SKVMRegionDriver{}) + } + + aliyun := []TestData{ + { + Name: "Test aliyun rules", + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithName("allow tcp 443", "in:allow tcp 443", 1), + ruleWithName("allow tcp 6379", "in:allow tcp 6379", 1), + ruleWithName("allow tcp 3389", "in:allow tcp 3389", 1), + ruleWithName("allow tcp 1521", "in:allow tcp 1521", 1), + ruleWithName("allow tcp 80", "in:allow tcp 80", 1), + ruleWithName("deny tcp 1521", "in:deny tcp 1521", 12), + }, + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("allow tcp", "in:allow tcp", 51), + ruleWithName("allow tcp 1521", "in:allow tcp 1521", 50), + ruleWithName("allow tcp 3389", "in:allow tcp 3389", 50), + ruleWithName("allow tcp 443", "in:allow tcp 443", 50), + ruleWithName("allow tcp 6379", "in:allow tcp 6379", 50), + ruleWithName("allow tcp 80", "in:allow tcp 80", 50), + ruleWithName("allow tcp", "in:allow tcp", 1), + }, + Common: []cloudprovider.SecurityRule{ + ruleWithName("allow tcp 1521", "in:allow tcp 1521", 50), + ruleWithName("allow tcp 3389", "in:allow tcp 3389", 50), + ruleWithName("allow tcp 443", "in:allow tcp 443", 50), + ruleWithName("allow tcp 6379", "in:allow tcp 6379", 50), + ruleWithName("allow tcp 80", "in:allow tcp 80", 50), + }, + InAdds: []cloudprovider.SecurityRule{}, + OutAdds: []cloudprovider.SecurityRule{}, + InDels: []cloudprovider.SecurityRule{ + ruleWithName("allow tcp", "in:allow tcp", 51), + ruleWithName("allow tcp", "in:allow tcp", 1), + }, + OutDels: []cloudprovider.SecurityRule{}, + }, + } + + for _, d := range aliyun { + d.Test(t, &SAliyunRegionDriver{}, &SKVMRegionDriver{}) + } + +} diff --git a/pkg/compute/regiondrivers/secgroup_openstack_test.go b/pkg/compute/regiondrivers/secgroup_openstack_test.go index cea9f44ee8..a42b331e57 100644 --- a/pkg/compute/regiondrivers/secgroup_openstack_test.go +++ b/pkg/compute/regiondrivers/secgroup_openstack_test.go @@ -21,44 +21,49 @@ import ( ) func TestOpenStackRuleSync(t *testing.T) { - driver := SOpenStackRegionDriver{} - maxPriority := driver.GetSecurityGroupRuleMaxPriority() - minPriority := driver.GetSecurityGroupRuleMinPriority() - - defaultInRule := driver.GetDefaultSecurityGroupInRule() - defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - isOnlyAllowRules := driver.IsOnlySupportAllowRules() - data := []TestData{ { Name: "Test deny rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("in:deny any", 100), - localRuleWithPriority("in:allow any", 99), - localRuleWithPriority("out:allow any", 100), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:deny any", 100), + ruleWithPriority("in:allow any", 99), + ruleWithPriority("out:allow any", 100), }, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow any", 1), + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("", "in:allow any", 1), }, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 0), + ruleWithName("", "out:allow any", 0), }, InDels: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow any", 1), + ruleWithName("", "in:allow any", 1), + }, + OutDels: []cloudprovider.SecurityRule{}, + }, + { + Name: "Test deny rules", + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("in:deny any", 100), + ruleWithPriority("in:allow any", 99), + ruleWithPriority("out:allow any", 100), + }, + DestRules: []cloudprovider.SecurityRule{ + ruleWithName("", "in:allow any", 0), + ruleWithName("", "out:allow any", 0), + }, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, + OutAdds: []cloudprovider.SecurityRule{}, + InDels: []cloudprovider.SecurityRule{ + ruleWithName("", "in:allow any", 0), }, OutDels: []cloudprovider.SecurityRule{}, }, } for _, d := range data { - t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true, false) - check(t, "common", common, d.Common) - check(t, "inAdds", inAdds, d.InAdds) - check(t, "outAdds", outAdds, d.OutAdds) - check(t, "inDels", inDels, d.InDels) - check(t, "outDels", outDels, d.OutDels) + d.Test(t, &SKVMRegionDriver{}, &SOpenStackRegionDriver{}) } } diff --git a/pkg/compute/regiondrivers/secgroup_qcloud_test.go b/pkg/compute/regiondrivers/secgroup_qcloud_test.go index c28b07fc93..42c69180b2 100644 --- a/pkg/compute/regiondrivers/secgroup_qcloud_test.go +++ b/pkg/compute/regiondrivers/secgroup_qcloud_test.go @@ -15,33 +15,24 @@ package regiondrivers import ( - "sort" "testing" "yunion.io/x/onecloud/pkg/cloudprovider" ) func TestQcloudRuleSync(t *testing.T) { - driver := SQcloudRegionDriver{} - maxPriority := driver.GetSecurityGroupRuleMaxPriority() - minPriority := driver.GetSecurityGroupRuleMinPriority() - - defaultInRule := driver.GetDefaultSecurityGroupInRule() - defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - isOnlyAllowRules := driver.IsOnlySupportAllowRules() - data := []TestData{ { Name: "Test out rules", - LocalRules: cloudprovider.LocalSecurityRuleSet{ - localRuleWithPriority("out:allow any", 11), - localRuleWithPriority("out:deny any", 10), + SrcRules: cloudprovider.SecurityRuleSet{ + ruleWithPriority("out:allow any", 11), + ruleWithPriority("out:deny any", 10), }, - RemoteRules: []cloudprovider.SecurityRule{}, - Common: []cloudprovider.SecurityRule{}, - InAdds: []cloudprovider.SecurityRule{}, + DestRules: []cloudprovider.SecurityRule{}, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 49), + ruleWithName("", "out:allow any", 48), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, @@ -49,17 +40,6 @@ func TestQcloudRuleSync(t *testing.T) { } for _, d := range data { - t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true, false) - sort.Sort(cloudprovider.SecurityRuleSet(common)) - sort.Sort(cloudprovider.SecurityRuleSet(inAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(outAdds)) - sort.Sort(cloudprovider.SecurityRuleSet(inDels)) - sort.Sort(cloudprovider.SecurityRuleSet(outDels)) - check(t, "common", common, d.Common) - check(t, "inAdds", inAdds, d.InAdds) - check(t, "outAdds", outAdds, d.OutAdds) - check(t, "inDels", inDels, d.InDels) - check(t, "outDels", outDels, d.OutDels) + d.Test(t, &SKVMRegionDriver{}, &SQcloudRegionDriver{}) } } diff --git a/pkg/compute/regiondrivers/secgroup_test.go b/pkg/compute/regiondrivers/secgroup_test.go index 1edc340f1c..14b3898e74 100644 --- a/pkg/compute/regiondrivers/secgroup_test.go +++ b/pkg/compute/regiondrivers/secgroup_test.go @@ -16,47 +16,110 @@ package regiondrivers import ( "fmt" + "sort" "testing" - "yunion.io/x/log" "yunion.io/x/pkg/util/secrules" + "yunion.io/x/pkg/util/stringutils" + "yunion.io/x/pkg/utils" "yunion.io/x/onecloud/pkg/cloudprovider" ) type TestData struct { - Name string - LocalRules cloudprovider.LocalSecurityRuleSet - RemoteRules cloudprovider.SecurityRuleSet - Common cloudprovider.SecurityRuleSet - InAdds cloudprovider.SecurityRuleSet - OutAdds cloudprovider.SecurityRuleSet - InDels cloudprovider.SecurityRuleSet - OutDels cloudprovider.SecurityRuleSet + Name string + SrcRules cloudprovider.SecurityRuleSet + DestRules cloudprovider.SecurityRuleSet + Common cloudprovider.SecurityRuleSet + InAdds cloudprovider.SecurityRuleSet + OutAdds cloudprovider.SecurityRuleSet + InDels cloudprovider.SecurityRuleSet + OutDels cloudprovider.SecurityRuleSet } -var localRuleWithPriority = func(ruleStr string, priority int) cloudprovider.LocalSecurityRule { +func (d TestData) Clone() TestData { + return TestData{ + Name: d.Name, + SrcRules: d.SrcRules, + DestRules: d.DestRules, + Common: cloudprovider.SecurityRuleSet{}, + InAdds: cloudprovider.SecurityRuleSet{}, + OutAdds: cloudprovider.SecurityRuleSet{}, + InDels: cloudprovider.SecurityRuleSet{}, + OutDels: cloudprovider.SecurityRuleSet{}, + } +} + +func (d TestData) Test(t *testing.T, srcD, destD cloudprovider.SecDriver) { + t.Logf("check %s", d.Name) + src, dest := cloudprovider.NewSecRuleInfo(srcD), cloudprovider.NewSecRuleInfo(destD) + src.Rules, dest.Rules = d.SrcRules, d.DestRules + common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(src, dest, true) + check(t, "common", common, d.Common, 0, 0) + check(t, "inAdds", inAdds, d.InAdds, dest.MinPriority, dest.MaxPriority) + check(t, "outAdds", outAdds, d.OutAdds, dest.MinPriority, dest.MaxPriority) + check(t, "inDels", inDels, d.InDels, 0, 0) + check(t, "outDels", outDels, d.OutDels, 0, 0) + + t.Logf("check %s reverse", d.Name) + + rd := d.Clone() + dest.Rules = append(dest.Rules, inAdds...) + dest.Rules = append(dest.Rules, outAdds...) + externalIds := []string{} + for i := range inDels { + if len(inDels[i].ExternalId) > 0 { + externalIds = append(externalIds, inDels[i].ExternalId) + } else { + externalIds = append(externalIds, fmt.Sprintf("%s-%d", inDels[i].String(), inDels[i].Priority)) + } + } + for i := range outDels { + if len(outDels[i].ExternalId) > 0 { + externalIds = append(externalIds, outDels[i].ExternalId) + } else { + externalIds = append(externalIds, fmt.Sprintf("%s-%d", outDels[i].String(), outDels[i].Priority)) + } + } + destRules := cloudprovider.SecurityRuleSet{} + for i := range dest.Rules { + if utils.IsInStringArray(dest.Rules[i].ExternalId, externalIds) || utils.IsInStringArray(fmt.Sprintf("%s-%d", dest.Rules[i].String(), dest.Rules[i].Priority), externalIds) { + continue + } + destRules = append(destRules, dest.Rules[i]) + } + dest.Rules = destRules + _, inAdds, outAdds, inDels, outDels = cloudprovider.CompareRules(src, dest, true) + //check(t, "common", common, rd.Common) + check(t, "inAdds", inAdds, rd.InAdds, dest.MinPriority, dest.MaxPriority) + check(t, "outAdds", outAdds, rd.OutAdds, dest.MinPriority, dest.MaxPriority) + check(t, "inDels", inDels, rd.InDels, 0, 0) + check(t, "outDels", outDels, rd.OutDels, 0, 0) +} + +var ruleWithPriority = func(ruleStr string, priority int) cloudprovider.SecurityRule { rule := secrules.MustParseSecurityRule(ruleStr) if rule == nil { - log.Errorf("invalid rule str %s", ruleStr) - return cloudprovider.LocalSecurityRule{} + panic(fmt.Sprintf("invalid rule str %s", ruleStr)) + return cloudprovider.SecurityRule{} } rule.Priority = priority - return cloudprovider.LocalSecurityRule{SecurityRule: *rule} + return cloudprovider.SecurityRule{SecurityRule: *rule, Id: stringutils.UUID4()} } -var remoteRuleWithName = func(name, ruleStr string, priority int) cloudprovider.SecurityRule { +var ruleWithName = func(name, ruleStr string, priority int) cloudprovider.SecurityRule { return cloudprovider.SecurityRule{ Name: name, - SecurityRule: localRuleWithPriority(ruleStr, priority).SecurityRule, + ExternalId: name, + SecurityRule: ruleWithPriority(ruleStr, priority).SecurityRule, } } -var check = func(t *testing.T, name string, ret, expect []cloudprovider.SecurityRule) { +var check = func(t *testing.T, name string, ret, expect []cloudprovider.SecurityRule, min, max int) { var show = func(info string, rules []cloudprovider.SecurityRule) { t.Logf("%s: %d\n", info, len(rules)) for _, r := range rules { - t.Logf("Name: %s priority: %d %s\n", r.Name, r.Priority, r.String()) + t.Logf("Name: %s id: %s external id: %s priority: %d %s\n", r.Name, r.Id, r.ExternalId, r.Priority, r.String()) } } if len(ret) != len(expect) { @@ -64,21 +127,31 @@ var check = func(t *testing.T, name string, ret, expect []cloudprovider.Security show(fmt.Sprintf("%s expect", name), expect) t.Fatalf("invalid rules for %s current is %d expect %d", name, len(ret), len(expect)) } + sort.Sort(cloudprovider.SecurityRuleSet(ret)) + sort.Sort(cloudprovider.SecurityRuleSet(expect)) + if max < min { + max, min = min, max + } for i := range ret { if ret[i].Name != expect[i].Name { show(fmt.Sprintf("%s rule", name), ret) show(fmt.Sprintf("%s expect", name), expect) t.Fatalf("invalid index(%d) %s rule name %s expect %s", i, name, ret[i].Name, expect[i].Name) } - if ret[i].Priority != expect[i].Priority { - show(fmt.Sprintf("%s rule", name), ret) - show(fmt.Sprintf("%s expect", name), expect) - t.Fatalf("invalid index(%d) %s rule priority %d expect %d", i, name, ret[i].Priority, expect[i].Priority) + // if ret[i].Priority != expect[i].Priority { + // show(fmt.Sprintf("%s rule", name), ret) + // show(fmt.Sprintf("%s expect", name), expect) + // t.Fatalf("invalid index(%d) %s rule priority %d expect %d", i, name, ret[i].Priority, expect[i].Priority) + // } + if max != min && (ret[i].Priority < min || ret[i].Priority > max) { + t.Fatalf("invalid index(%d) %s rules %s priority should be in [%d, %d] current is %d", i, name, ret[i].String(), min, max, ret[i].Priority) } + if ret[i].String() != expect[i].String() { show(fmt.Sprintf("%s rule", name), ret) show(fmt.Sprintf("%s expect", name), expect) t.Fatalf("invalid index(%d) %s rules %s expect %s", i, name, ret[i].String(), expect[i].String()) } + } }