diff --git a/pkg/cloudprovider/securitygroup.go b/pkg/cloudprovider/securitygroup.go index 73f4ca77cf..c7c072d42e 100644 --- a/pkg/cloudprovider/securitygroup.go +++ b/pkg/cloudprovider/securitygroup.go @@ -41,8 +41,9 @@ type SecurityGroupCreateInput struct { type SecurityRule struct { secrules.SecurityRule - Name string - ExternalId string + Name string + ExternalId string + LocalRulePrority int } type LocalSecurityRule struct { @@ -81,13 +82,6 @@ func (srs LocalSecurityRuleSet) AllowList() secrules.SecurityRuleSet { return rules.AllowList() } -type TPriorityOrder int - -var ( - PriorityOrderByDesc = TPriorityOrder(1) - PriorityOrderByAsc = TPriorityOrder(-1) -) - func (r SecurityRule) String() string { return r.SecurityRule.String() } @@ -114,26 +108,24 @@ func (srs SecurityRuleSet) AllowList() secrules.SecurityRuleSet { return rules.AllowList() } -func AddDefaultRule(rules []SecurityRule, defaultRule SecurityRule, localRuleStr string, order TPriorityOrder, min, max int, onlyAllowRules bool) []SecurityRule { +func AddDefaultRule(rules []SecurityRule, defaultRule SecurityRule, localRuleStr string, min, max int, onlyAllowRules bool) []SecurityRule { if defaultRule.String() == localRuleStr { return rules } defaultRule.ExternalId = DEFAULT_CLOUD_RULE_ID - if order == PriorityOrderByDesc { - defaultRule.Priority = min + if max > min { + defaultRule.Priority = min - 1 } else { - defaultRule.Priority = max + defaultRule.Priority = max + 1 } - defaultRule.Priority -= int(order) return append(rules, defaultRule) } -func SortSecurityRule(rules SecurityRuleSet, order TPriorityOrder, onlyAllowRules bool) { +func SortSecurityRule(rules SecurityRuleSet, max, min int, onlyAllowRules bool) { if onlyAllowRules { - sort.Sort(rules) return } - if order == PriorityOrderByAsc { + if max < min { sort.Sort(sort.Reverse(rules)) return } @@ -141,20 +133,18 @@ func SortSecurityRule(rules SecurityRuleSet, order TPriorityOrder, onlyAllowRule } func CompareRules( - minPriority, maxPriority int, order TPriorityOrder, - localRules secrules.SecurityRuleSet, remoteRules []SecurityRule, + minPriority, maxPriority int, + localRules LocalSecurityRuleSet, remoteRules []SecurityRule, defaultInRule, defaultOutRule SecurityRule, - onlyAllowRules bool, debug bool, + onlyAllowRules bool, debug bool, refreshLocalRule bool, ) (common, inAdds, outAdds, inDels, outDels []SecurityRule) { localInRules := LocalSecurityRuleSet{} localOutRules := LocalSecurityRuleSet{} for i := range localRules { - localRule := LocalSecurityRule{} - localRule.SecurityRule = localRules[i] if localRules[i].Direction == secrules.DIR_IN { - localInRules = append(localInRules, localRule) + localInRules = append(localInRules, localRules[i]) } else { - localOutRules = append(localOutRules, localRule) + localOutRules = append(localOutRules, localRules[i]) } } inRules := SecurityRuleSet{} @@ -173,8 +163,8 @@ func CompareRules( defaultLocalOutRule := LocalSecurityRule{ExternalId: DEFAULT_LOCAL_RULE_ID} defaultLocalOutRule.SecurityRule = *secrules.MustParseSecurityRule("out:allow any") - inRules = AddDefaultRule(inRules, defaultInRule, defaultLocalInRule.String(), order, minPriority, maxPriority, onlyAllowRules) - outRules = AddDefaultRule(outRules, defaultOutRule, defaultLocalOutRule.String(), order, minPriority, maxPriority, onlyAllowRules) + inRules = AddDefaultRule(inRules, defaultInRule, defaultLocalInRule.String(), minPriority, maxPriority, onlyAllowRules) + outRules = AddDefaultRule(outRules, defaultOutRule, defaultLocalOutRule.String(), minPriority, maxPriority, onlyAllowRules) defaultInEquals, defaultOutEquals := true, true if defaultLocalInRule.String() != defaultInRule.String() { @@ -203,18 +193,18 @@ func CompareRules( rule.SecurityRule = localOutAllowList[i] _localOutRules = append(_localOutRules, rule) } - if onlyAllowRules { + if !refreshLocalRule && onlyAllowRules { localOutRules, localInRules = _localOutRules, _localInRules } - if len(_localInRules) < len(localInRules) { + if !refreshLocalRule && len(_localInRules) < len(localInRules) { localInRules = _localInRules } - if len(_localOutRules) < len(localOutRules) { + if !refreshLocalRule && len(_localOutRules) < len(localOutRules) { localOutRules = _localOutRules } - SortSecurityRule(inRules, order, onlyAllowRules) - SortSecurityRule(outRules, order, onlyAllowRules) + SortSecurityRule(inRules, maxPriority, minPriority, onlyAllowRules) + SortSecurityRule(outRules, maxPriority, minPriority, onlyAllowRules) inAllowList := inRules.AllowList() outAllowList := outRules.AllowList() @@ -231,23 +221,21 @@ func CompareRules( sort.Sort(sort.Reverse(outRules)) startPriority := minPriority - 1 - if order == PriorityOrderByAsc { + if maxPriority < minPriority { startPriority = maxPriority + 1 } - var addPriority = func(priority int, order TPriorityOrder, inc int, min, max int, onlyAllowRules bool) int { + var addPriority = func(priority int, inc int, min, max int, onlyAllowRules bool) int { if onlyAllowRules { - return 0 + return priority } - inc = inc * int(order) //+ int(order) - priority += inc - if priority < min { - return min + if max < min { + max, min, inc = min, max, inc*-1 } - if priority > max { - return max + if priority >= max || priority <= min { + return priority } - return priority + return priority + inc } var getInitPriority = func(init, min, max int) int { @@ -258,7 +246,7 @@ func CompareRules( } var compare = func(localRules LocalSecurityRuleSet, remoteRules SecurityRuleSet) (common, add, del []SecurityRule) { - i, j, inc, prePriority := 0, 0, 1, 0 + 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() @@ -270,33 +258,38 @@ func CompareRules( if cmp == 0 { prePriority = remoteRules[j].Priority if remoteRules[j].ExternalId == DEFAULT_CLOUD_RULE_ID { - remoteRules[j].Priority = addPriority(remoteRules[j].Priority, order, 1, minPriority, maxPriority, onlyAllowRules) + remoteRules[j].Priority = addPriority(remoteRules[j].Priority, 1, 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) { common = append(common, remoteRules[j]) } + 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]) } j++ } else { initPriority := getInitPriority(prePriority, minPriority, maxPriority) - localRules[i].Priority = addPriority(initPriority, order, inc, minPriority, maxPriority, onlyAllowRules) + 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}) + add = append(add, SecurityRule{SecurityRule: localRules[i].SecurityRule, ExternalId: localRules[i].ExternalId}) } 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]) } j++ @@ -306,11 +299,11 @@ func CompareRules( initPriority = remoteRules[len(remoteRules)-1].Priority } initPriority = getInitPriority(initPriority, minPriority, maxPriority) // 若是初始添加规则，尽量以中间为节点，避免仅出现天地规则 - localRules[i].Priority = addPriority(initPriority, order, inc, minPriority, maxPriority, onlyAllowRules) + 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}) + add = append(add, SecurityRule{SecurityRule: localRules[i].SecurityRule, ExternalId: localRules[i].ExternalId}) } i++ inc++ diff --git a/pkg/compute/models/guests.go b/pkg/compute/models/guests.go index 581d303597..f80479d0c1 100644 --- a/pkg/compute/models/guests.go +++ b/pkg/compute/models/guests.go @@ -2353,7 +2353,8 @@ func (self *SGuest) getSecurityGroupsRules() string { func (self *SGuest) getAdminSecurityRules() string { secgrp := self.getAdminSecgroup() if secgrp != nil { - return secgrp.getSecurityRuleString("") + ret, _ := secgrp.getSecurityRuleString() + return ret } else { return options.Options.DefaultAdminSecurityRules } diff --git a/pkg/compute/models/regiondrivers.go b/pkg/compute/models/regiondrivers.go index 1cccfd3415..080655e7aa 100644 --- a/pkg/compute/models/regiondrivers.go +++ b/pkg/compute/models/regiondrivers.go @@ -129,7 +129,6 @@ type IRegionDriver interface { RequestCacheSecurityGroup(ctx context.Context, userCred mcclient.TokenCredential, region *SCloudregion, vpc *SVpc, secgroup *SSecurityGroup, classic bool, removeProjectId string, task taskman.ITask) error RequestSyncSecurityGroup(ctx context.Context, userCred mcclient.TokenCredential, vpcId string, vpc *SVpc, secgroup *SSecurityGroup, removeProjectId, service string) (string, error) - GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder // Desc(priority值越大,优先级越高) Asc(priority值越小,优先级越高) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule GetDefaultSecurityGroupOutRule() cloudprovider.SecurityRule GetSecurityGroupRuleMaxPriority() int diff --git a/pkg/compute/models/secgroupcache.go b/pkg/compute/models/secgroupcache.go index fc8f884e2f..ec35a29fdc 100644 --- a/pkg/compute/models/secgroupcache.go +++ b/pkg/compute/models/secgroupcache.go @@ -23,7 +23,6 @@ import ( "yunion.io/x/log" "yunion.io/x/pkg/errors" "yunion.io/x/pkg/util/compare" - "yunion.io/x/pkg/util/secrules" "yunion.io/x/sqlchemy" "yunion.io/x/onecloud/pkg/apis" @@ -409,10 +408,7 @@ func (self *SSecurityGroupCache) syncWithCloudSecurityGroup(ctx context.Context, if err != nil { return errors.Wrapf(err, "getRuleInfo") } - err = secgroup.SyncSecurityGroupRules(ctx, userCred, info) - if err != nil { - return errors.Wrapf(err, "SyncSecurityGroupRules") - } + secgroup.SyncSecurityGroupRules(ctx, userCred, info) return nil } @@ -656,12 +652,14 @@ func (self *SSecurityGroupCache) SyncRules() error { defaultInRule := region.GetDriver().GetDefaultSecurityGroupInRule() defaultOutRule := region.GetDriver().GetDefaultSecurityGroupOutRule() - order := region.GetDriver().GetSecurityGroupRuleOrder() onlyAllowRules := region.GetDriver().IsOnlySupportAllowRules() - localRules := secrules.SecurityRuleSet(secgroup.GetSecRules("")) + localRules, err := secgroup.GetSecuritRuleSet() + if err != nil { + return errors.Wrapf(err, "GetSecuritRuleSet") + } - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, localRules, rules, defaultInRule, defaultOutRule, onlyAllowRules, false) + common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, localRules, rules, defaultInRule, defaultOutRule, onlyAllowRules, false, 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 20d0ee3684..e3497a54f5 100644 --- a/pkg/compute/models/secgrouprules.go +++ b/pkg/compute/models/secgrouprules.go @@ -150,13 +150,6 @@ func (self *SSecurityGroupRule) AllowDeleteItem(ctx context.Context, userCred mc return false } -/*func (self *SSecurityGroupRule) GetSecGroup() *SSecurityGroup { - if secgroup, _ := SecurityGroupManager.FetchById(self.SecgroupI); secgroup != nil { - return secgroup.(*SSecurityGroup) - } - return nil -}*/ - func (manager *SSecurityGroupRuleManager) FilterById(q *sqlchemy.SQuery, idStr string) *sqlchemy.SQuery { return q.Equals("id", idStr) } @@ -462,23 +455,6 @@ func (manager *SSecurityGroupRuleManager) getRulesBySecurityGroup(secgroup *SSec return rules, nil } -func (self *SSecurityGroup) SyncRules(ctx context.Context, userCred mcclient.TokenCredential, rules cloudprovider.SecurityRuleSet) error { - priority, prePriority := 10, 0 - for i := 0; i < len(rules); i++ { - // 这里避免了Rule规则优先级在 1-100之外的问题,ext.GetRules()不需要进行优先级转换 - if prePriority != 0 && rules[i].Priority != prePriority && priority < 100 { - priority++ - } - prePriority = rules[i].Priority - rules[i].Priority = priority - _, err := self.newFromCloudSecurityGroupRule(ctx, userCred, rules[i]) - if err != nil { - return errors.Wrapf(err, "newFromCloudSecurityGroupRule") - } - } - return nil -} - func (self *SSecurityGroup) newFromCloudSecurityGroupRule(ctx context.Context, userCred mcclient.TokenCredential, rule cloudprovider.SecurityRule) (*SSecurityGroupRule, error) { lockman.LockObject(ctx, self) defer lockman.ReleaseObject(ctx, self) @@ -493,8 +469,15 @@ func (self *SSecurityGroup) newFromCloudSecurityGroupRule(ctx context.Context, u cidr = rule.IPNet.String() } + rule.Priority = rule.LocalRulePrority + + err := rule.ValidateRule() + if err != nil { + return nil, errors.Wrapf(err, "ValidateRule") + } + secrule := &SSecurityGroupRule{ - Priority: int64(rule.Priority), + Priority: int64(rule.LocalRulePrority), Protocol: protocol, Ports: rule.GetPortsString(), Direction: string(rule.Direction), @@ -505,7 +488,7 @@ func (self *SSecurityGroup) newFromCloudSecurityGroupRule(ctx context.Context, u secrule.SetModelManager(SecurityGroupRuleManager, secrule) secrule.SecgroupId = self.Id - err := SecurityGroupRuleManager.TableSpec().Insert(ctx, secrule) + err = SecurityGroupRuleManager.TableSpec().Insert(ctx, secrule) if err != nil { return nil, errors.Wrapf(err, "SecurityGroupRuleManager.Insert") } diff --git a/pkg/compute/models/secgroups.go b/pkg/compute/models/secgroups.go index 89728c1791..6bdf82a553 100644 --- a/pkg/compute/models/secgroups.go +++ b/pkg/compute/models/secgroups.go @@ -23,6 +23,7 @@ import ( "yunion.io/x/jsonutils" "yunion.io/x/log" "yunion.io/x/pkg/errors" + "yunion.io/x/pkg/util/compare" "yunion.io/x/pkg/util/regutils" "yunion.io/x/pkg/util/secrules" "yunion.io/x/pkg/utils" @@ -95,8 +96,10 @@ func (manager *SSecurityGroupManager) ListItemFilter( return nil, httperrors.NewInputParameterError("Failed fetching secgroup %s", input.Equals) } secgroup := _secgroup.(*SSecurityGroup) - inAllowList := secgroup.GetInAllowList() - outAllowList := secgroup.GetOutAllowList() + inAllowList, outAllowList, err := secgroup.GetAllowList() + if err != nil { + return q, httperrors.NewGeneralError(errors.Wrapf(err, "GetAllowList")) + } sq := manager.Query().NotEquals("id", secgroup.Id) secgroups := []SSecurityGroup{} err = db.FetchModelObjects(manager, sq, &secgroups) @@ -105,12 +108,11 @@ func (manager *SSecurityGroupManager) ListItemFilter( } secgroupIds := []string{} for i := 0; i < len(secgroups); i++ { - _inAllowList := secgroups[i].GetInAllowList() - if !inAllowList.Equals(_inAllowList) { - continue + _inAllowList, _outAllowList, err := secgroups[i].GetAllowList() + if err != nil { + return nil, httperrors.NewGeneralError(errors.Wrapf(err, "GetAllowList")) } - _outAllowList := secgroups[i].GetOutAllowList() - if !outAllowList.Equals(_outAllowList) { + if !inAllowList.Equals(_inAllowList) || !outAllowList.Equals(_outAllowList) { continue } secgroupIds = append(secgroupIds, secgroups[i].Id) @@ -547,40 +549,61 @@ func (manager *SSecurityGroupManager) FetchSecgroupById(secId string) (*SSecurit return secgrp.(*SSecurityGroup), nil } -func (self *SSecurityGroup) getSecurityRules(direction string) (rules []SSecurityGroupRule) { +func (self *SSecurityGroup) getSecurityRules() ([]SSecurityGroupRule, error) { secgrouprules := SecurityGroupRuleManager.Query().SubQuery() sql := secgrouprules.Query().Filter(sqlchemy.Equals(secgrouprules.Field("secgroup_id"), self.Id)).Desc("priority") - if len(direction) > 0 && utils.IsInStringArray(direction, []string{"in", "out"}) { - sql = sql.Equals("direction", direction) + rules := []SSecurityGroupRule{} + err := db.FetchModelObjects(SecurityGroupRuleManager, sql, &rules) + if err != nil { + return nil, errors.Wrapf(err, "db.FetchModelObjects") } - if err := db.FetchModelObjects(SecurityGroupRuleManager, sql, &rules); err != nil { - log.Errorf("GetGuests fail %s", err) - return - } - return + return rules, nil } -func (self *SSecurityGroup) GetSecRules(direction string) []secrules.SecurityRule { +func (self *SSecurityGroup) GetSecuritRuleSet() (cloudprovider.LocalSecurityRuleSet, error) { + ruleSet := cloudprovider.LocalSecurityRuleSet{} + rules, err := self.getSecurityRules() + if err != nil { + return ruleSet, errors.Wrapf(err, "getSecurityRules") + } + for i := range rules { + //这里没必要拆分为单个单个的端口,到公有云那边适配 + rule, err := rules[i].toRule() + if err != nil { + return nil, errors.Wrapf(err, "toRule") + } + ruleSet = append(ruleSet, cloudprovider.LocalSecurityRule{SecurityRule: *rule, ExternalId: rules[i].Id}) + } + return ruleSet, nil +} + +func (self *SSecurityGroup) GetSecRules() ([]secrules.SecurityRule, error) { rules := make([]secrules.SecurityRule, 0) - for _, _rule := range self.getSecurityRules(direction) { + _rules, err := self.getSecurityRules() + if err != nil { + return nil, errors.Wrapf(err, "getSecurityRules()") + } + for _, _rule := range _rules { //这里没必要拆分为单个单个的端口,到公有云那边适配 rule, err := _rule.toRule() if err != nil { - log.Errorln(err) - continue + return nil, errors.Wrapf(err, "toRule") } rules = append(rules, *rule) } - return rules + return rules, nil } -func (self *SSecurityGroup) getSecurityRuleString(direction string) string { - secgrouprules := self.getSecurityRules(direction) +func (self *SSecurityGroup) getSecurityRuleString() (string, error) { + secgrouprules, err := self.getSecurityRules() + if err != nil { + return "", errors.Wrapf(err, "getSecurityRules()") + } var rules []string for _, rule := range secgrouprules { rules = append(rules, rule.String()) } - return strings.Join(rules, SECURITY_GROUP_SEPARATOR) + return strings.Join(rules, SECURITY_GROUP_SEPARATOR), nil } func totalSecurityGroupCount(scope rbacutils.TRbacScope, ownerId mcclient.IIdentityProvider) (int, error) { @@ -743,7 +766,10 @@ func (self *SSecurityGroup) PerformClone(ctx context.Context, userCred mcclient. return input, httperrors.NewGeneralError(errors.Wrapf(err, "Insert")) } - secgrouprules := self.getSecurityRules("") + secgrouprules, err := self.getSecurityRules() + if err != nil { + return input, httperrors.NewGeneralError(errors.Wrapf(err, "getSecurityRules")) + } for _, rule := range secgrouprules { secgrouprule := &SSecurityGroupRule{} secgrouprule.SetModelManager(SecurityGroupRuleManager, secgrouprule) @@ -780,8 +806,10 @@ func (self *SSecurityGroup) PerformMerge(ctx context.Context, userCred mcclient. if len(input.SecgroupIds) == 0 { return nil, httperrors.NewMissingParameterError("secgroup_ids") } - inAllowList := self.GetInAllowList() - outAllowList := self.GetOutAllowList() + inAllowList, outAllowList, err := self.GetAllowList() + if err != nil { + return nil, httperrors.NewGeneralError(errors.Wrapf(err, "GetAllowList")) + } secgroups := []*SSecurityGroup{} for _, secgroupId := range input.SecgroupIds { _secgroup, err := SecurityGroupManager.FetchByIdOrName(userCred, secgroupId) @@ -793,11 +821,13 @@ func (self *SSecurityGroup) PerformMerge(ctx context.Context, userCred mcclient. } secgroup := _secgroup.(*SSecurityGroup) secgroup.SetModelManager(SecurityGroupManager, secgroup) - _inAllowList := secgroup.GetInAllowList() + _inAllowList, _outAllowList, err := secgroup.GetAllowList() + if err != nil { + return nil, httperrors.NewGeneralError(errors.Wrapf(err, "GetAllowList")) + } if !inAllowList.Equals(_inAllowList) { return nil, httperrors.NewUnsupportOperationError("secgroup %s rules not equals %s rules", secgroup.Name, self.Name) } - _outAllowList := secgroup.GetOutAllowList() if !outAllowList.Equals(_outAllowList) { return nil, httperrors.NewUnsupportOperationError("secgroup %s rules not equals %s rules", secgroup.Name, self.Name) } @@ -821,27 +851,20 @@ func (self *SSecurityGroup) PerformMerge(ctx context.Context, userCred mcclient. return nil, nil } -func (self *SSecurityGroup) GetOutAllowList() secrules.SecurityRuleSet { - rules := self.GetSecRules("out") - ruleSet := secrules.SecurityRuleSet(rules) - rules = append(rules, *secrules.MustParseSecurityRule("out:allow any")) - return ruleSet.AllowList() -} - -func (self *SSecurityGroup) GetInAllowList() secrules.SecurityRuleSet { - rules := self.GetSecRules("in") - rules = append(rules, *secrules.MustParseSecurityRule("in:deny any")) - ruleSet := secrules.SecurityRuleSet(rules) - return ruleSet.AllowList() -} - -func (self *SSecurityGroup) getSecurityGroupRuleSet() secrules.SecurityGroupRuleSet { - rules := self.GetSecRules("") - srs := secrules.SecurityGroupRuleSet{} - for i := 0; i < len(rules); i++ { - srs.AddRule(rules[i]) +func (self *SSecurityGroup) GetAllowList() (secrules.SecurityRuleSet, secrules.SecurityRuleSet, error) { + in, out := secrules.SecurityRuleSet{*secrules.MustParseSecurityRule("in:deny any")}, secrules.SecurityRuleSet{*secrules.MustParseSecurityRule("out:allow any")} + rules, err := self.GetSecRules() + if err != nil { + return in, out, errors.Wrapf(err, "GetSecRules") } - return srs + for i := range rules { + if rules[i].Direction == secrules.DIR_IN { + in = append(in, rules[i]) + } else { + in = append(in, rules[i]) + } + } + return in.AllowList(), out.AllowList(), nil } func (self *SSecurityGroup) mergeSecurityGroupCache(secgroup *SSecurityGroup) error { @@ -900,42 +923,63 @@ func (manager *SSecurityGroupManager) getSecurityGroups() ([]SSecurityGroup, err } } -func (self *SSecurityGroup) cleanRules(ctx context.Context, userCred mcclient.TokenCredential) error { +func (self *SSecurityGroup) removeRules(ruleIds []string, result *compare.SyncResult) { + if len(ruleIds) == 0 { + return + } rules := []SSecurityGroupRule{} - q := SecurityGroupRuleManager.Query().Equals("secgroup_id", self.Id) + q := SecurityGroupRuleManager.Query().In("id", ruleIds) err := db.FetchModelObjects(SecurityGroupRuleManager, q, &rules) if err != nil { - return errors.Wrapf(err, "db.FetchModelObjects") + result.DeleteError(errors.Wrapf(err, "db.FetchModelObjects")) + return } for i := range rules { - err = rules[i].Delete(ctx, userCred) + err = rules[i].Delete(context.TODO(), nil) if err != nil { - return errors.Wrapf(err, "DeleteRule(%s)", rules[i].Id) + result.DeleteError(errors.Wrapf(err, "delte rule %s", rules[i].Id)) + continue } + result.Delete() } - return nil } -func (self *SSecurityGroup) SyncSecurityGroupRules(ctx context.Context, userCred mcclient.TokenCredential, info *sRuleInfo) error { - inRules := cloudprovider.AddDefaultRule(info.inRules, info.defaultInRule, "in:deny any", info.order, info.minPriority, info.maxPriority, info.onlyAllowRules) - cloudprovider.SortSecurityRule(inRules, info.order, info.onlyAllowRules) - outRules := cloudprovider.AddDefaultRule(info.outRules, info.defaultOutRule, "out:allow any", info.order, info.minPriority, info.maxPriority, info.onlyAllowRules) - cloudprovider.SortSecurityRule(outRules, info.order, info.onlyAllowRules) - - err := self.cleanRules(ctx, userCred) +func (self *SSecurityGroup) SyncSecurityGroupRules(ctx context.Context, userCred mcclient.TokenCredential, info *sRuleInfo) compare.SyncResult { + result := compare.SyncResult{} + localRules, err := self.GetSecuritRuleSet() if err != nil { - return errors.Wrapf(err, "cleanRules") + 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) + if len(inAdds)+len(inDels)+len(outAdds)+len(outDels) == 0 { + return result } - err = self.SyncRules(ctx, userCred, inRules) - if err != nil { - return errors.Wrapf(err, "SyncInRules") + ruleIds := []string{} + for _, dels := range [][]cloudprovider.SecurityRule{inDels, outDels} { + for i := range dels { + if len(dels[i].ExternalId) > 0 { + ruleIds = append(ruleIds, dels[i].ExternalId) + } + } } - err = self.SyncRules(ctx, userCred, outRules) - if err != nil { - return errors.Wrapf(err, "SyncOutRules") + + self.removeRules(ruleIds, &result) + + for _, adds := range [][]cloudprovider.SecurityRule{inAdds, outAdds} { + for i := range adds { + _, err := self.newFromCloudSecurityGroupRule(ctx, userCred, adds[i]) + if err != nil { + result.AddError(errors.Wrapf(err, "newFromCloudSecurityGroupRule")) + continue + } + result.Add() + } } - return nil + + log.Infof("Sync Rules for Secgroup %s(%s) result: %s", self.Name, self.Id, result.Result()) + return result } type sRuleInfo struct { @@ -944,7 +988,6 @@ type sRuleInfo struct { outRules []cloudprovider.SecurityRule defaultInRule cloudprovider.SecurityRule defaultOutRule cloudprovider.SecurityRule - order cloudprovider.TPriorityOrder onlyAllowRules bool maxPriority int minPriority int @@ -967,7 +1010,6 @@ func (manager *SSecurityGroupManager) getRuleInfo(provider *SCloudprovider, extS outRules: []cloudprovider.SecurityRule{}, defaultInRule: regionDriver.GetDefaultSecurityGroupInRule(), defaultOutRule: regionDriver.GetDefaultSecurityGroupOutRule(), - order: regionDriver.GetSecurityGroupRuleOrder(), onlyAllowRules: regionDriver.IsOnlySupportAllowRules(), maxPriority: regionDriver.GetSecurityGroupRuleMaxPriority(), minPriority: regionDriver.GetSecurityGroupRuleMinPriority(), @@ -998,8 +1040,12 @@ func (manager *SSecurityGroupManager) newFromCloudSecgroup(ctx context.Context, return nil, errors.Wrap(err, "db.FetchModelObjects") } for i := range secgroups { - localRules := secrules.SecurityRuleSet(secgroups[i].GetSecRules("")) - _, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(info.minPriority, info.maxPriority, info.order, localRules, info.rules, info.defaultInRule, info.defaultOutRule, info.onlyAllowRules, false) + localRules, 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) if len(inAdds) == 0 && len(outAdds) == 0 && len(inDels) == 0 && len(outDels) == 0 { return &secgroups[i], nil } @@ -1026,10 +1072,7 @@ func (manager *SSecurityGroupManager) newFromCloudSecgroup(ctx context.Context, return nil, errors.Wrapf(err, "Insert") } - err = secgroup.SyncSecurityGroupRules(ctx, userCred, info) - if err != nil { - return nil, errors.Wrapf(err, "SyncSecurityGroupRules") - } + secgroup.SyncSecurityGroupRules(ctx, userCred, info) db.OpsLog.LogEvent(&secgroup, db.ACT_CREATE, secgroup.GetShortDesc(ctx), userCred) return &secgroup, nil diff --git a/pkg/compute/regiondrivers/aliyun.go b/pkg/compute/regiondrivers/aliyun.go index 4d9c48d884..07377d63a9 100644 --- a/pkg/compute/regiondrivers/aliyun.go +++ b/pkg/compute/regiondrivers/aliyun.go @@ -53,10 +53,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SAliyunRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SAliyunRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/aws.go b/pkg/compute/regiondrivers/aws.go index 17be20a665..2438e71fa8 100644 --- a/pkg/compute/regiondrivers/aws.go +++ b/pkg/compute/regiondrivers/aws.go @@ -52,10 +52,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SAwsRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SAwsRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/azure.go b/pkg/compute/regiondrivers/azure.go index d66039af6c..1f6af21d48 100644 --- a/pkg/compute/regiondrivers/azure.go +++ b/pkg/compute/regiondrivers/azure.go @@ -57,10 +57,6 @@ func (self *SAzureRegionDriver) IsSupportClassicSecurityGroup() bool { return true } -func (self *SAzureRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SAzureRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/base.go b/pkg/compute/regiondrivers/base.go index 20ea561585..89a1ef223b 100644 --- a/pkg/compute/regiondrivers/base.go +++ b/pkg/compute/regiondrivers/base.go @@ -263,10 +263,6 @@ func (self *SBaseRegionDriver) RequestSyncSecurityGroup(ctx context.Context, use return "", fmt.Errorf("Not Implemented RequestSyncSecurityGroup") } -func (self *SBaseRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByDesc -} - func (self *SBaseRegionDriver) IsOnlySupportAllowRules() bool { return false } diff --git a/pkg/compute/regiondrivers/ctyun.go b/pkg/compute/regiondrivers/ctyun.go index 463f3be14a..c746b0eea3 100644 --- a/pkg/compute/regiondrivers/ctyun.go +++ b/pkg/compute/regiondrivers/ctyun.go @@ -37,10 +37,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SCtyunRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SCtyunRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/google.go b/pkg/compute/regiondrivers/google.go index 8c5c0349c4..81d27e0747 100644 --- a/pkg/compute/regiondrivers/google.go +++ b/pkg/compute/regiondrivers/google.go @@ -43,10 +43,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SGoogleRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SGoogleRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/huawei.go b/pkg/compute/regiondrivers/huawei.go index f88e1d8a4c..0e79a959ff 100644 --- a/pkg/compute/regiondrivers/huawei.go +++ b/pkg/compute/regiondrivers/huawei.go @@ -54,10 +54,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SHuaWeiRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SHuaWeiRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/managedvirtual.go b/pkg/compute/regiondrivers/managedvirtual.go index 7f39750fea..6515488e0b 100644 --- a/pkg/compute/regiondrivers/managedvirtual.go +++ b/pkg/compute/regiondrivers/managedvirtual.go @@ -25,7 +25,6 @@ import ( "yunion.io/x/log" "yunion.io/x/pkg/errors" "yunion.io/x/pkg/util/netutils" - "yunion.io/x/pkg/util/secrules" "yunion.io/x/pkg/utils" billing_api "yunion.io/x/onecloud/pkg/apis/billing" @@ -1656,7 +1655,10 @@ func (self *SManagedVirtualizationRegionDriver) RequestSyncSecurityGroup(ctx con Desc: secgroup.Description, VpcId: vpcId, ProjectId: remoteProjectId, - Rules: secgroup.GetSecRules(""), + } + conf.Rules, err = secgroup.GetSecRules() + if err != nil { + return errors.Wrapf(err, "GetSecRules") } iSecgroup, err = iRegion.CreateISecurityGroup(conf) if err != nil { @@ -1685,12 +1687,14 @@ func (self *SManagedVirtualizationRegionDriver) RequestSyncSecurityGroup(ctx con defaultInRule := region.GetDriver().GetDefaultSecurityGroupInRule() defaultOutRule := region.GetDriver().GetDefaultSecurityGroupOutRule() - order := region.GetDriver().GetSecurityGroupRuleOrder() onlyAllowRules := region.GetDriver().IsOnlySupportAllowRules() - localRules := secrules.SecurityRuleSet(secgroup.GetSecRules("")) + localRules, err := secgroup.GetSecuritRuleSet() + if err != nil { + return errors.Wrapf(err, "GetSecuritRuleSet") + } - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, localRules, rules, defaultInRule, defaultOutRule, onlyAllowRules, false) + common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, localRules, rules, defaultInRule, defaultOutRule, onlyAllowRules, false, false) if len(inAdds) == 0 && len(inDels) == 0 && len(outAdds) == 0 && len(outDels) == 0 { return nil diff --git a/pkg/compute/regiondrivers/openstack.go b/pkg/compute/regiondrivers/openstack.go index 87a7cff05f..984299250d 100644 --- a/pkg/compute/regiondrivers/openstack.go +++ b/pkg/compute/regiondrivers/openstack.go @@ -48,10 +48,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SOpenStackRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByDesc -} - func (self *SOpenStackRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/qcloud.go b/pkg/compute/regiondrivers/qcloud.go index b8ac335c69..d91e706174 100644 --- a/pkg/compute/regiondrivers/qcloud.go +++ b/pkg/compute/regiondrivers/qcloud.go @@ -53,10 +53,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SQcloudRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SQcloudRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/secgroup_aliyun_test.go b/pkg/compute/regiondrivers/secgroup_aliyun_test.go new file mode 100644 index 0000000000..aac5c9d55a --- /dev/null +++ b/pkg/compute/regiondrivers/secgroup_aliyun_test.go @@ -0,0 +1,72 @@ +// 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 ( + "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), + }, + 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), + }, + Common: []cloudprovider.SecurityRule{}, + InAdds: []cloudprovider.SecurityRule{}, + OutAdds: []cloudprovider.SecurityRule{}, + 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) + } +} diff --git a/pkg/compute/regiondrivers/secgroup_aws_test.go b/pkg/compute/regiondrivers/secgroup_aws_test.go index c4d5bd6b27..d1952afe55 100644 --- a/pkg/compute/regiondrivers/secgroup_aws_test.go +++ b/pkg/compute/regiondrivers/secgroup_aws_test.go @@ -17,8 +17,6 @@ package regiondrivers import ( "testing" - "yunion.io/x/pkg/util/secrules" - "yunion.io/x/onecloud/pkg/cloudprovider" ) @@ -29,13 +27,12 @@ func TestAwsRuleSync(t *testing.T) { defaultInRule := driver.GetDefaultSecurityGroupInRule() defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - order := driver.GetSecurityGroupRuleOrder() isOnlyAllowRules := driver.IsOnlySupportAllowRules() data := []TestData{ { Name: "Test remove out allow rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("out:deny any", 1), }, RemoteRules: []cloudprovider.SecurityRule{ @@ -51,7 +48,7 @@ func TestAwsRuleSync(t *testing.T) { }, { Name: "Test out deny rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("out:deny any", 1), }, RemoteRules: []cloudprovider.SecurityRule{}, @@ -63,7 +60,7 @@ func TestAwsRuleSync(t *testing.T) { }, { Name: "Test out allow rules", - LocalRules: secrules.SecurityRuleSet{}, + LocalRules: cloudprovider.LocalSecurityRuleSet{}, RemoteRules: []cloudprovider.SecurityRule{}, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{}, @@ -77,7 +74,7 @@ func TestAwsRuleSync(t *testing.T) { for _, d := range data { t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true) + 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) diff --git a/pkg/compute/regiondrivers/secgroup_azure_test.go b/pkg/compute/regiondrivers/secgroup_azure_test.go index 500a64a9ff..444edbdb4b 100644 --- a/pkg/compute/regiondrivers/secgroup_azure_test.go +++ b/pkg/compute/regiondrivers/secgroup_azure_test.go @@ -18,8 +18,6 @@ import ( "sort" "testing" - "yunion.io/x/pkg/util/secrules" - "yunion.io/x/onecloud/pkg/cloudprovider" ) @@ -30,41 +28,24 @@ func TestAzureRuleSync(t *testing.T) { defaultInRule := driver.GetDefaultSecurityGroupInRule() defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - order := driver.GetSecurityGroupRuleOrder() isOnlyAllowRules := driver.IsOnlySupportAllowRules() data := []TestData{ { Name: "Test empty rules", - LocalRules: secrules.SecurityRuleSet{}, + LocalRules: cloudprovider.LocalSecurityRuleSet{}, RemoteRules: []cloudprovider.SecurityRule{}, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 2097), + remoteRuleWithName("", "out:allow any", 2099), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, }, - { - Name: "Test remove rules", - LocalRules: secrules.SecurityRuleSet{}, - RemoteRules: []cloudprovider.SecurityRule{ - remoteRuleWithName("test-name", "out:allow any", 1000), - }, - Common: []cloudprovider.SecurityRule{}, - InAdds: []cloudprovider.SecurityRule{}, - OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 2097), - }, - InDels: []cloudprovider.SecurityRule{}, - OutDels: []cloudprovider.SecurityRule{ - remoteRuleWithName("test-name", "out:allow any", 1000), - }, - }, { Name: "Test diff rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("out:allow tcp 100-200", 99), localRuleWithPriority("out:allow udp 200-300", 98), }, @@ -78,14 +59,14 @@ func TestAzureRuleSync(t *testing.T) { }, InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 2097), + remoteRuleWithName("", "out:allow any", 2099), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, }, { Name: "Test add rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("in:allow tcp", 100), localRuleWithPriority("in:allow udp", 99), localRuleWithPriority("out:deny any", 1), @@ -95,8 +76,8 @@ func TestAzureRuleSync(t *testing.T) { }, Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow tcp", 2097), - remoteRuleWithName("", "in:allow udp", 2097), + remoteRuleWithName("", "in:allow tcp", 2099), + remoteRuleWithName("", "in:allow udp", 2099), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ @@ -106,7 +87,7 @@ func TestAzureRuleSync(t *testing.T) { }, { Name: "Test insert rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("in:allow tcp", 100), localRuleWithPriority("in:allow udp", 99), localRuleWithPriority("in:allow icmp", 98), @@ -121,7 +102,7 @@ func TestAzureRuleSync(t *testing.T) { remoteRuleWithName("allow-icmp", "in:allow icmp", 400), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow udp", 2097), + remoteRuleWithName("", "in:allow udp", 2099), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{}, @@ -129,7 +110,7 @@ func TestAzureRuleSync(t *testing.T) { }, { Name: "Test icmp rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("in:allow tcp 33", 10), localRuleWithPriority("in:allow tcp 22", 1), localRuleWithPriority("out:deny any", 1), @@ -141,7 +122,7 @@ func TestAzureRuleSync(t *testing.T) { remoteRuleWithName("allow-tcp-22", "in:allow tcp 22", 300), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow tcp 33", 299), + remoteRuleWithName("", "in:allow tcp 33", 301), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{}, @@ -149,7 +130,7 @@ func TestAzureRuleSync(t *testing.T) { }, { Name: "Test a rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("in:allow tcp 1050", 5), localRuleWithPriority("in:allow tcp 1011", 4), localRuleWithPriority("in:allow tcp 1002", 3), @@ -171,7 +152,7 @@ func TestAzureRuleSync(t *testing.T) { remoteRuleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow tcp 1011", 4011), + remoteRuleWithName("", "in:allow tcp 1011", 4013), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ @@ -181,7 +162,7 @@ func TestAzureRuleSync(t *testing.T) { }, { Name: "Test b rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("in:allow udp 1055", 20), localRuleWithPriority("in:allow icmp", 15), localRuleWithPriority("in:allow tcp 1050", 5), @@ -207,8 +188,8 @@ func TestAzureRuleSync(t *testing.T) { remoteRuleWithName("in_allow_udp_55_4014", "in:allow udp 55", 4014), }, InAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "in:allow icmp", 2097), - remoteRuleWithName("", "in:allow udp 1055", 4013), + remoteRuleWithName("", "in:allow icmp", 2099), + remoteRuleWithName("", "in:allow udp 1055", 4015), }, OutAdds: []cloudprovider.SecurityRule{}, InDels: []cloudprovider.SecurityRule{ @@ -220,7 +201,7 @@ func TestAzureRuleSync(t *testing.T) { for _, d := range data { t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true) + 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)) diff --git a/pkg/compute/regiondrivers/secgroup_ctyun_test.go b/pkg/compute/regiondrivers/secgroup_ctyun_test.go index d04b026e21..7dd7c29b30 100644 --- a/pkg/compute/regiondrivers/secgroup_ctyun_test.go +++ b/pkg/compute/regiondrivers/secgroup_ctyun_test.go @@ -18,8 +18,6 @@ import ( "sort" "testing" - "yunion.io/x/pkg/util/secrules" - "yunion.io/x/onecloud/pkg/cloudprovider" ) @@ -30,13 +28,12 @@ func TestCtyunRuleSync(t *testing.T) { defaultInRule := driver.GetDefaultSecurityGroupInRule() defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - order := driver.GetSecurityGroupRuleOrder() isOnlyAllowRules := driver.IsOnlySupportAllowRules() data := []TestData{ { Name: "Test out deny rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("out:deny tcp 200", 1), }, RemoteRules: []cloudprovider.SecurityRule{}, @@ -55,7 +52,7 @@ func TestCtyunRuleSync(t *testing.T) { for _, d := range data { t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true) + 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)) diff --git a/pkg/compute/regiondrivers/secgroup_openstack_test.go b/pkg/compute/regiondrivers/secgroup_openstack_test.go index a5a57d110d..cea9f44ee8 100644 --- a/pkg/compute/regiondrivers/secgroup_openstack_test.go +++ b/pkg/compute/regiondrivers/secgroup_openstack_test.go @@ -17,8 +17,6 @@ package regiondrivers import ( "testing" - "yunion.io/x/pkg/util/secrules" - "yunion.io/x/onecloud/pkg/cloudprovider" ) @@ -29,13 +27,12 @@ func TestOpenStackRuleSync(t *testing.T) { defaultInRule := driver.GetDefaultSecurityGroupInRule() defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - order := driver.GetSecurityGroupRuleOrder() isOnlyAllowRules := driver.IsOnlySupportAllowRules() data := []TestData{ { Name: "Test deny rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("in:deny any", 100), localRuleWithPriority("in:allow any", 99), localRuleWithPriority("out:allow any", 100), @@ -57,7 +54,7 @@ func TestOpenStackRuleSync(t *testing.T) { for _, d := range data { t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true) + 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) diff --git a/pkg/compute/regiondrivers/secgroup_qcloud_test.go b/pkg/compute/regiondrivers/secgroup_qcloud_test.go index a5702dc38e..c28b07fc93 100644 --- a/pkg/compute/regiondrivers/secgroup_qcloud_test.go +++ b/pkg/compute/regiondrivers/secgroup_qcloud_test.go @@ -18,8 +18,6 @@ import ( "sort" "testing" - "yunion.io/x/pkg/util/secrules" - "yunion.io/x/onecloud/pkg/cloudprovider" ) @@ -30,13 +28,12 @@ func TestQcloudRuleSync(t *testing.T) { defaultInRule := driver.GetDefaultSecurityGroupInRule() defaultOutRule := driver.GetDefaultSecurityGroupOutRule() - order := driver.GetSecurityGroupRuleOrder() isOnlyAllowRules := driver.IsOnlySupportAllowRules() data := []TestData{ { Name: "Test out rules", - LocalRules: secrules.SecurityRuleSet{ + LocalRules: cloudprovider.LocalSecurityRuleSet{ localRuleWithPriority("out:allow any", 11), localRuleWithPriority("out:deny any", 10), }, @@ -44,7 +41,7 @@ func TestQcloudRuleSync(t *testing.T) { Common: []cloudprovider.SecurityRule{}, InAdds: []cloudprovider.SecurityRule{}, OutAdds: []cloudprovider.SecurityRule{ - remoteRuleWithName("", "out:allow any", 100), + remoteRuleWithName("", "out:allow any", 49), }, InDels: []cloudprovider.SecurityRule{}, OutDels: []cloudprovider.SecurityRule{}, @@ -53,7 +50,7 @@ func TestQcloudRuleSync(t *testing.T) { for _, d := range data { t.Logf("check %s", d.Name) - common, inAdds, outAdds, inDels, outDels := cloudprovider.CompareRules(minPriority, maxPriority, order, d.LocalRules, d.RemoteRules, defaultInRule, defaultOutRule, isOnlyAllowRules, true) + 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)) diff --git a/pkg/compute/regiondrivers/secgroup_test.go b/pkg/compute/regiondrivers/secgroup_test.go index 694c71bd6a..1edc340f1c 100644 --- a/pkg/compute/regiondrivers/secgroup_test.go +++ b/pkg/compute/regiondrivers/secgroup_test.go @@ -26,7 +26,7 @@ import ( type TestData struct { Name string - LocalRules secrules.SecurityRuleSet + LocalRules cloudprovider.LocalSecurityRuleSet RemoteRules cloudprovider.SecurityRuleSet Common cloudprovider.SecurityRuleSet InAdds cloudprovider.SecurityRuleSet @@ -35,20 +35,20 @@ type TestData struct { OutDels cloudprovider.SecurityRuleSet } -var localRuleWithPriority = func(ruleStr string, priority int) secrules.SecurityRule { +var localRuleWithPriority = func(ruleStr string, priority int) cloudprovider.LocalSecurityRule { rule := secrules.MustParseSecurityRule(ruleStr) if rule == nil { log.Errorf("invalid rule str %s", ruleStr) - return secrules.SecurityRule{} + return cloudprovider.LocalSecurityRule{} } rule.Priority = priority - return *rule + return cloudprovider.LocalSecurityRule{SecurityRule: *rule} } var remoteRuleWithName = func(name, ruleStr string, priority int) cloudprovider.SecurityRule { return cloudprovider.SecurityRule{ Name: name, - SecurityRule: localRuleWithPriority(ruleStr, priority), + SecurityRule: localRuleWithPriority(ruleStr, priority).SecurityRule, } } diff --git a/pkg/compute/regiondrivers/ucloud.go b/pkg/compute/regiondrivers/ucloud.go index bcf8e28f6c..bb6420a0d2 100644 --- a/pkg/compute/regiondrivers/ucloud.go +++ b/pkg/compute/regiondrivers/ucloud.go @@ -37,10 +37,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SUcloudRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByDesc -} - func (self *SUcloudRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} } diff --git a/pkg/compute/regiondrivers/zstack.go b/pkg/compute/regiondrivers/zstack.go index 87e56408a3..bde71d311d 100644 --- a/pkg/compute/regiondrivers/zstack.go +++ b/pkg/compute/regiondrivers/zstack.go @@ -37,10 +37,6 @@ func init() { models.RegisterRegionDriver(&driver) } -func (self *SZStackRegionDriver) GetSecurityGroupRuleOrder() cloudprovider.TPriorityOrder { - return cloudprovider.PriorityOrderByAsc -} - func (self *SZStackRegionDriver) GetDefaultSecurityGroupInRule() cloudprovider.SecurityRule { return cloudprovider.SecurityRule{SecurityRule: *secrules.MustParseSecurityRule("in:deny any")} }