From eccd7c7c1768730b932565504a0a55b540814924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B1=88=E8=BD=A9?= Date: Fri, 10 Aug 2018 21:47:11 +0800 Subject: [PATCH 1/2] =?UTF-8?q?=E5=90=8C=E6=AD=A5=E9=98=BF=E9=87=8C?= =?UTF-8?q?=E4=BA=91=E5=AE=89=E5=85=A8=E7=BB=84=E8=A7=84=E5=88=99=E5=88=B0?= =?UTF-8?q?=E6=9C=AC=E5=9C=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/cloudprovider/resources.go | 3 + pkg/compute/models/secgrouprules.go | 94 +++++++++++ pkg/compute/models/secgroups.go | 78 ++++++++++ .../tasks/cloud_provider_sync_info_task.go | 37 +++++ pkg/util/aliyun/host.go | 4 +- pkg/util/aliyun/securitygroup.go | 146 ++++++++++++++---- pkg/util/aliyun/vpc.go | 20 ++- .../yunionio/pkg/util/secrules/secrules.go | 17 +- 8 files changed, 357 insertions(+), 42 deletions(-) diff --git a/pkg/cloudprovider/resources.go b/pkg/cloudprovider/resources.go index c75cba6a55..f7bb43bb9d 100644 --- a/pkg/cloudprovider/resources.go +++ b/pkg/cloudprovider/resources.go @@ -171,6 +171,8 @@ type ICloudEIP interface { type ICloudSecurityGroup interface { ICloudResource + GetDescription() string + GetRules() ([]secrules.SecurityRule, error) } type ICloudDisk interface { @@ -201,6 +203,7 @@ type ICloudVpc interface { GetCidrBlock() string // GetStatus() string GetIWires() ([]ICloudWire, error) + GetISecurityGroups() ([]ICloudSecurityGroup, error) GetManagerId() string diff --git a/pkg/compute/models/secgrouprules.go b/pkg/compute/models/secgrouprules.go index 27391766b6..af2bd87e84 100644 --- a/pkg/compute/models/secgrouprules.go +++ b/pkg/compute/models/secgrouprules.go @@ -11,7 +11,9 @@ import ( "github.com/yunionio/onecloud/pkg/cloudcommon/db" "github.com/yunionio/onecloud/pkg/httperrors" "github.com/yunionio/onecloud/pkg/mcclient" + "github.com/yunionio/pkg/util/compare" "github.com/yunionio/pkg/util/secrules" + "github.com/yunionio/pkg/util/sets" "github.com/yunionio/pkg/util/stringutils" "github.com/yunionio/sqlchemy" ) @@ -232,3 +234,95 @@ func (self *SSecurityGroupRule) PostUpdate(ctx context.Context, userCred mcclien secgroup.DoSync(ctx, userCred) } } + +func (manager *SSecurityGroupRuleManager) getRulesBySecurityGroup(secgroup *SSecurityGroup) ([]SSecurityGroupRule, error) { + rules := make([]SSecurityGroupRule, 0) + q := manager.Query().Equals("secgroup_id", secgroup.Id) + if err := db.FetchModelObjects(manager, q, &rules); err != nil { + return nil, err + } + return rules, nil +} + +func (manager *SSecurityGroupRuleManager) SyncRules(ctx context.Context, userCred mcclient.TokenCredential, secgroup *SSecurityGroup, rules []secrules.SecurityRule) ([]SSecurityGroupRule, []SSecurityGroupRule, compare.SyncResult) { + syncResult := compare.SyncResult{} + + if _dbRules, err := manager.getRulesBySecurityGroup(secgroup); err != nil { + return nil, nil, syncResult + } else { + dbRules := make([]secrules.SecurityRule, len(_dbRules)) + for _, _rule := range _dbRules { + rule, _ := secrules.ParseSecurityRule(_rule.GetRule()) + rule.Description = _rule.Description + dbRules = append(dbRules, *rule) + } + + oldRules, oldStrs := make(map[string]secrules.SecurityRule, len(dbRules)), sets.NewString() + for _, rule := range dbRules { + if str := jsonutils.Marshal(rule).String(); !oldStrs.Has(str) { + oldStrs.Insert(str) + log.Errorf("old: %s", str) + oldRules[str] = rule + } + } + newRules, newStrs := make(map[string]secrules.SecurityRule, len(rules)), sets.NewString() + for _, rule := range rules { + if str := jsonutils.Marshal(rule).String(); !newStrs.Has(str) { + newStrs.Insert(str) + log.Errorf("new: %s", str) + newRules[str] = rule + } + } + for _, rule := range newStrs.Difference(oldStrs).List() { + log.Errorf("need add : %s", rule) + if _, err := manager.newFromCloudSecurityGroup(newRules[rule], secgroup); err != nil { + syncResult.AddError(err) + } else { + syncResult.Add() + } + } + for _, rule := range oldStrs.Difference(newStrs).List() { + log.Errorf("need remove : %s", rule) + // if _, err := manager.newFromCloudSecurityGroup(newRules[rule], secgroup); err != nil { + // syncResult.AddError(err) + // } else { + // syncResult.Add() + // } + } + } + return nil, nil, syncResult +} + +func (manager *SSecurityGroupRuleManager) newFromCloudSecurityGroup(rule secrules.SecurityRule, secgroup *SSecurityGroup) (*SSecurityGroupRule, error) { + protocol := rule.Protocol + if rule.Protocol == "any" { + protocol = "" + } + ports, _ports := "", make([]string, len(rule.Ports)) + if len(rule.Ports) > 0 { + for _, port := range rule.Ports { + _ports = append(_ports, fmt.Sprintf("%d", port)) + } + ports = strings.Join(_ports, ",") + } else if rule.PortStart != 0 || rule.PortEnd != 0 { + if rule.PortStart == rule.PortEnd { + ports = fmt.Sprintf("%d", rule.PortStart) + } else { + ports = fmt.Sprintf("%d-%d", rule.PortStart, rule.PortEnd) + } + } + secrule := &SSecurityGroupRule{ + Priority: int64(rule.Priority), + Protocol: protocol, + Ports: ports, + Direction: string(rule.Direction), + CIDR: rule.IPNet.String(), + Action: string(rule.Action), + Description: rule.Description, + SecgroupID: secgroup.Id, + } + if err := manager.TableSpec().Insert(secrule); err != nil { + return nil, err + } + return secrule, nil +} diff --git a/pkg/compute/models/secgroups.go b/pkg/compute/models/secgroups.go index a6303144fa..34fbf15578 100644 --- a/pkg/compute/models/secgroups.go +++ b/pkg/compute/models/secgroups.go @@ -7,8 +7,10 @@ import ( "github.com/yunionio/jsonutils" "github.com/yunionio/log" "github.com/yunionio/onecloud/pkg/cloudcommon/db" + "github.com/yunionio/onecloud/pkg/cloudprovider" "github.com/yunionio/onecloud/pkg/httperrors" "github.com/yunionio/onecloud/pkg/mcclient" + "github.com/yunionio/pkg/util/compare" "github.com/yunionio/pkg/util/secrules" "github.com/yunionio/sqlchemy" ) @@ -169,6 +171,82 @@ func (self *SSecurityGroup) PerformClone(ctx context.Context, userCred mcclient. return nil, nil } +func (manager *SSecurityGroupManager) getSecurityGroups() ([]SSecurityGroup, error) { + secgroups := make([]SSecurityGroup, 0) + q := manager.Query() + if err := db.FetchModelObjects(manager, q, &secgroups); err != nil { + return nil, err + } else { + return secgroups, nil + } +} + +func (manager *SSecurityGroupManager) SyncSecgroups(ctx context.Context, userCred mcclient.TokenCredential, secgroups []cloudprovider.ICloudSecurityGroup) ([]SSecurityGroup, []cloudprovider.ICloudSecurityGroup, compare.SyncResult) { + localSecgroups := make([]SSecurityGroup, 0) + remoteSecgroups := make([]cloudprovider.ICloudSecurityGroup, 0) + syncResult := compare.SyncResult{} + + if dbSecgroups, err := manager.getSecurityGroups(); err != nil { + syncResult.Error(err) + return nil, nil, syncResult + } else { + removed := make([]SSecurityGroup, 0) + commondb := make([]SSecurityGroup, 0) + commonext := make([]cloudprovider.ICloudSecurityGroup, 0) + added := make([]cloudprovider.ICloudSecurityGroup, 0) + if err := compare.CompareSets(dbSecgroups, secgroups, &removed, &commondb, &commonext, &added); err != nil { + syncResult.Error(err) + return nil, nil, syncResult + } + + for i := 0; i < len(commondb); i += 1 { + if err = commondb[i].SyncWithCloudSecurityGroup(userCred, commonext[i]); err != nil { + syncResult.UpdateError(err) + } else { + localSecgroups = append(localSecgroups, commondb[i]) + remoteSecgroups = append(remoteSecgroups, commonext[i]) + syncResult.Update() + } + } + + for i := 0; i < len(added); i += 1 { + if new, err := manager.newFromCloudVpc(added[i]); err != nil { + syncResult.AddError(err) + } else { + localSecgroups = append(localSecgroups, *new) + remoteSecgroups = append(remoteSecgroups, added[i]) + syncResult.Add() + } + } + } + return localSecgroups, remoteSecgroups, syncResult +} + +func (self *SSecurityGroup) SyncWithCloudSecurityGroup(userCred mcclient.TokenCredential, extSec cloudprovider.ICloudSecurityGroup) error { + if _, err := self.GetModelManager().TableSpec().Update(self, func() error { + self.Name = extSec.GetName() + self.Description = extSec.GetDescription() + return nil + }); err != nil { + log.Errorf("syncWithCloudSecurityGroup error %s", err) + return err + } + return nil +} + +func (manager *SSecurityGroupManager) newFromCloudVpc(extSecgroup cloudprovider.ICloudSecurityGroup) (*SSecurityGroup, error) { + secgroup := SSecurityGroup{} + secgroup.SetModelManager(manager) + secgroup.Name = extSecgroup.GetName() + secgroup.ExternalId = extSecgroup.GetGlobalId() + secgroup.Description = extSecgroup.GetDescription() + + if err := manager.TableSpec().Insert(&secgroup); err != nil { + return nil, err + } + return &secgroup, nil +} + func (self *SSecurityGroup) DoSync(ctx context.Context, userCred mcclient.TokenCredential) { if _, err := self.GetModelManager().TableSpec().Update(self, func() error { self.IsDirty = true diff --git a/pkg/compute/tasks/cloud_provider_sync_info_task.go b/pkg/compute/tasks/cloud_provider_sync_info_task.go index ba0b27e77b..6f3b9af24f 100644 --- a/pkg/compute/tasks/cloud_provider_sync_info_task.go +++ b/pkg/compute/tasks/cloud_provider_sync_info_task.go @@ -140,6 +140,43 @@ func syncRegionVPCs(ctx context.Context, provider *models.SCloudprovider, task * for j := 0; j < len(localVpcs); j += 1 { syncVpcWires(ctx, provider, task, &localVpcs[j], remoteVpcs[j]) + syncVpcSecGroup(ctx, provider, task, &localVpcs[j], remoteVpcs[j]) + } +} + +func syncVpcSecGroup(ctx context.Context, provider *models.SCloudprovider, task *CloudProviderSyncInfoTask, localVpc *models.SVpc, remoteVpc cloudprovider.ICloudVpc) { + if secgroups, err := remoteVpc.GetISecurityGroups(); err != nil { + msg := fmt.Sprintf("GetIWires for vps %s failed %s", remoteVpc.GetId(), err) + log.Errorf(msg) + logSyncFailed(provider, task, msg) + return + } else { + localSecgroups, removeSecgroups, result := models.SecurityGroupManager.SyncSecgroups(ctx, task.UserCred, secgroups) + msg := result.Result() + log.Infof("SyncSecurityGroup for VPC %s result: %s", localVpc.Name, msg) + if result.IsError() { + logSyncFailed(provider, task, msg) + return + } + //db.OpsLog.LogEvent(provider, db.ACT_SYNC_HOST_COMPLETE, msg, task.GetUserCred()) + for i := 0; i < len(localSecgroups); i += 1 { + syncSecgroupRules(ctx, provider, task, &localSecgroups[i], removeSecgroups[i]) + } + } +} + +func syncSecgroupRules(ctx context.Context, provider *models.SCloudprovider, task *CloudProviderSyncInfoTask, localSecgroup *models.SSecurityGroup, remoteSecgroup cloudprovider.ICloudSecurityGroup) { + rules, err := remoteSecgroup.GetRules() + if err != nil { + logSyncFailed(provider, task, err.Error()) + return + } + _, _, result := models.SecurityGroupRuleManager.SyncRules(ctx, task.UserCred, localSecgroup, rules) + msg := result.Result() + log.Infof("SyncRules for secgroups %s result: %s", localSecgroup.Name, msg) + if result.IsError() { + logSyncFailed(provider, task, msg) + return } } diff --git a/pkg/util/aliyun/host.go b/pkg/util/aliyun/host.go index 680b99b388..2c8bf97a86 100644 --- a/pkg/util/aliyun/host.go +++ b/pkg/util/aliyun/host.go @@ -222,7 +222,7 @@ func (self *SHost) _createVM(name string, imgId string, sysDiskSize int, cpu int return "", fmt.Errorf("vsiwtch's wire's vpc is empty") } - secgroups, err := net.wire.vpc.GetSecurityGroups() + secgroups, err := net.wire.vpc.GetISecurityGroups() if err != nil { return "", fmt.Errorf("get security group error %s", err) } @@ -236,7 +236,7 @@ func (self *SHost) _createVM(name string, imgId string, sysDiskSize int, cpu int secgroupId = secId } } else { - secgroupId = secgroups[0].SecurityGroupId + //secgroupId = secgroups[0].SecurityGroupId } keypair := "" diff --git a/pkg/util/aliyun/securitygroup.go b/pkg/util/aliyun/securitygroup.go index a93896436c..855c9fc91d 100644 --- a/pkg/util/aliyun/securitygroup.go +++ b/pkg/util/aliyun/securitygroup.go @@ -5,11 +5,12 @@ import ( "strings" "time" - "github.com/deckarep/golang-set" "github.com/yunionio/jsonutils" "github.com/yunionio/log" + "github.com/yunionio/onecloud/pkg/compute/models" "github.com/yunionio/onecloud/pkg/httperrors" "github.com/yunionio/pkg/util/secrules" + "github.com/yunionio/pkg/util/sets" "github.com/yunionio/pkg/utils" ) @@ -47,6 +48,7 @@ type SPermissions struct { } type SSecurityGroup struct { + vpc *SVpc CreationTime time.Time Description string SecurityGroupId string @@ -57,6 +59,59 @@ type SSecurityGroup struct { RegionId string } +func (self *SSecurityGroup) GetId() string { + return self.SecurityGroupId +} + +func (self *SSecurityGroup) GetGlobalId() string { + return self.SecurityGroupId +} + +func (self *SSecurityGroup) GetDescription() string { + return self.Description +} + +func (self *SSecurityGroup) GetRules() ([]secrules.SecurityRule, error) { + rules := make([]secrules.SecurityRule, 0) + if secgrp, err := self.vpc.region.GetSecurityGroupDetails(self.SecurityGroupId); err != nil { + return rules, err + } else { + for _, permission := range secgrp.Permissions.Permission { + if rule, err := secrules.ParseSecurityRule(permission.toString()); err != nil { + return rules, err + } else { + rule.Priority = 101 - rule.Priority + rule.Description = permission.Description + rules = append(rules, *rule) + } + } + } + return rules, nil +} + +func (self *SSecurityGroup) GetName() string { + if len(self.SecurityGroupName) > 0 { + return self.SecurityGroupName + } + return self.SecurityGroupId +} + +func (self *SSecurityGroup) GetStatus() string { + return "" +} + +func (self *SSecurityGroup) IsEmulated() bool { + return false +} + +func (self *SSecurityGroup) Refresh() error { + if new, err := self.vpc.region.GetSecurityGroupDetails(self.SecurityGroupId); err != nil { + return err + } else { + return jsonutils.Update(self, new) + } +} + func (self *SRegion) GetSecurityGroups(vpcId string, offset int, limit int) ([]SSecurityGroup, int, error) { if limit > 50 || limit <= 0 { limit = 50 @@ -256,6 +311,46 @@ func (self *SRegion) getSecurityGroupByTag(vpcId, secgroupId string) (*SSecurity return &secgrps[0], nil } +func (self *SPermission) String() string { + return self.toString() +} + +func (self *SPermission) ConvertSecurityGroupRule() models.SSecurityGroupRule { + protocol := strings.ToLower(self.IpProtocol) + if protocol == "all" { + protocol = "" + } + direction := self.Direction + cidr := self.SourceCidrIp + if direction == "ingress" { + direction = "in" + } else { + direction = "out" + cidr = self.DestCidrIp + } + ports := "" + if _ports := strings.Split(self.PortRange, "/"); len(_ports) == 2 { + if _ports[0] == _ports[1] && _ports[0] != "-1" { + ports = _ports[0] + } else { + ports = strings.Replace(self.PortRange, "/", "-", -1) + } + } + action := "allow" + if strings.ToLower(self.Policy) == "drop" { + action = "deny" + } + return models.SSecurityGroupRule{ + Priority: int64(101 - self.Priority), + Protocol: protocol, + Ports: ports, + Direction: direction, + CIDR: cidr, + Action: action, + Description: self.Description, + } +} + func (self *SPermission) toString() string { action := secrules.SecurityRuleDeny if strings.ToLower(self.Policy) == "accept" { @@ -327,55 +422,48 @@ func (self *SRegion) syncSecgroupRules(secgroupId string, rules []*secrules.Secu if secgroup, err := self.GetSecurityGroupDetails(secgroupId); err != nil { return err } else { - newRules, newStr := make(map[string]*secrules.SecurityRule), mapset.NewSet() + newRules, newStr := make(map[string]*secrules.SecurityRule), sets.NewString() for _, rule := range rules { rule.Priority = 101 - rule.Priority if len(rule.Ports) > 0 { for _, port := range rule.Ports { rule.PortStart, rule.PortEnd = port, port - if jsonStr := jsonutils.Marshal(rule).String(); newStr.Add(jsonStr) { + if jsonStr := jsonutils.Marshal(rule).String(); !newStr.Has(jsonStr) { + newStr.Insert(jsonStr) + log.Errorf("new: %s", jsonStr) newRules[jsonStr] = rule } } - continue - } else if rule.PortStart == 0 || rule.PortEnd == 0 { - rule.PortStart, rule.PortEnd = -1, -1 - } - if jsonStr := jsonutils.Marshal(rule).String(); newStr.Add(jsonStr) { + } else if jsonStr := jsonutils.Marshal(rule).String(); !newStr.Has(jsonStr) { + newStr.Insert(jsonStr) + log.Errorf("new: %s", jsonStr) newRules[jsonStr] = rule } } - oldRules, oldStr := make(map[string]*secrules.SecurityRule), mapset.NewSet() + oldRules, oldStr := make(map[string]*secrules.SecurityRule), sets.NewString() for _, r := range secgroup.Permissions.Permission { if rule, err := secrules.ParseSecurityRule(r.toString()); err != nil { return err } else { rule.Priority = r.Priority - if jsonStr := jsonutils.Marshal(rule).String(); oldStr.Add(jsonStr) { + if jsonStr := jsonutils.Marshal(rule).String(); !oldStr.Has(jsonStr) { + oldStr.Insert(jsonStr) + log.Errorf("old: %s", jsonStr) oldRules[jsonStr] = rule } } } - - newStr.Each(func(r interface{}) bool { - rStr := r.(string) - if !oldStr.Contains(r) { - rule := newRules[rStr] - log.Debugf("add secgroup rule: %v Priority: %d", rule, rule.Priority) - self.addSecurityGroupRule(secgroupId, rule) - } - return false - }) - oldStr.Each(func(r interface{}) bool { - rStr := r.(string) - if !newStr.Contains(r) { - rule := oldRules[rStr] - log.Debugf("del secgroup rule: %v Priority: %d", rule, rule.Priority) - self.delSecurityGroupRule(secgroupId, rule) - } - return false - }) + for _, jsonStr := range newStr.Difference(oldStr).List() { + rule := newRules[jsonStr] + log.Debugf("add secgroup rule: %v Priority: %d", rule, rule.Priority) + self.addSecurityGroupRule(secgroupId, rule) + } + for _, jsonStr := range oldStr.Difference(newStr).List() { + rule := oldRules[jsonStr] + log.Debugf("del secgroup rule: %v Priority: %d", rule, rule.Priority) + self.delSecurityGroupRule(secgroupId, rule) + } } return nil } diff --git a/pkg/util/aliyun/vpc.go b/pkg/util/aliyun/vpc.go index 2260bd1cac..e10fe59f57 100644 --- a/pkg/util/aliyun/vpc.go +++ b/pkg/util/aliyun/vpc.go @@ -30,7 +30,7 @@ type SVpc struct { iwires []cloudprovider.ICloudWire - secgroups []SSecurityGroup + secgroups []cloudprovider.ICloudSecurityGroup CidrBlock string CreationTime time.Time @@ -161,11 +161,15 @@ func (self *SVpc) fetchSecurityGroups() error { break } } - self.secgroups = secgroups + self.secgroups = make([]cloudprovider.ICloudSecurityGroup, len(secgroups)) + for index, secgroup := range secgroups { + secgroup.vpc = self + self.secgroups[index] = &secgroup + } return nil } -func (self *SVpc) GetSecurityGroups() ([]SSecurityGroup, error) { +func (self *SVpc) GetISecurityGroups() ([]cloudprovider.ICloudSecurityGroup, error) { if self.secgroups == nil { err := self.fetchSecurityGroups() if err != nil { @@ -175,6 +179,16 @@ func (self *SVpc) GetSecurityGroups() ([]SSecurityGroup, error) { return self.secgroups, nil } +// func (self *SVpc) GetSecurityGroups() ([]SSecurityGroup, error) { +// if self.secgroups == nil { +// err := self.fetchSecurityGroups() +// if err != nil { +// return nil, err +// } +// } +// return self.secgroups, nil +// } + func (self *SVpc) GetManagerId() string { return self.region.client.providerId } diff --git a/vendor/github.com/yunionio/pkg/util/secrules/secrules.go b/vendor/github.com/yunionio/pkg/util/secrules/secrules.go index 7336b51751..56b6c17493 100644 --- a/vendor/github.com/yunionio/pkg/util/secrules/secrules.go +++ b/vendor/github.com/yunionio/pkg/util/secrules/secrules.go @@ -25,14 +25,15 @@ const ( ) type SecurityRule struct { - Priority int // [1, 100] - Action TSecurityRuleAction - IPNet *net.IPNet - Protocol string - Direction TSecurityRuleDirection - PortStart int - PortEnd int - Ports []int + Priority int // [1, 100] + Action TSecurityRuleAction + IPNet *net.IPNet + Protocol string + Direction TSecurityRuleDirection + PortStart int + PortEnd int + Ports []int + Description string } const ( From bc73e77e3594992110e74f05a9774ef49952f5ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B1=88=E8=BD=A9?= Date: Tue, 14 Aug 2018 14:54:11 +0800 Subject: [PATCH 2/2] =?UTF-8?q?=E6=9B=B4=E6=94=B9sync=E7=AE=97=E6=B3=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/climc/shell/secgrouprules.go | 2 - pkg/cloudprovider/resources.go | 2 +- pkg/compute/models/guests.go | 12 +- pkg/compute/models/secgrouprules.go | 151 ++++++++---- pkg/compute/models/secgroups.go | 50 ++-- .../tasks/cloud_provider_sync_info_task.go | 21 +- pkg/util/aliyun/instance.go | 2 +- pkg/util/aliyun/securitygroup.go | 230 +++++++++++++----- pkg/util/aliyun/vpc.go | 4 +- pkg/util/esxi/virtualmachine.go | 2 +- .../yunion.io/x/pkg/util/secrules/secrules.go | 19 ++ 11 files changed, 346 insertions(+), 149 deletions(-) diff --git a/cmd/climc/shell/secgrouprules.go b/cmd/climc/shell/secgrouprules.go index eb9fd4b621..13fdd49730 100644 --- a/cmd/climc/shell/secgrouprules.go +++ b/cmd/climc/shell/secgrouprules.go @@ -59,7 +59,6 @@ func init() { }) type SecGroupRulesCreateOptions struct { - NAME string `help:"Name of security group rule to create"` SECGROUP string `help:"Secgroup ID or Name" metavar:"Secgroup"` Direction string `help:"Direction of rule" choices:"in|out"` Action string `help:"Action of rule" choices:"allow|deny"` @@ -72,7 +71,6 @@ func init() { R(&SecGroupRulesCreateOptions{}, "secgroup-rule-create", "Create all security group rule", func(s *mcclient.ClientSession, args *SecGroupRulesCreateOptions) error { params := jsonutils.NewDict() - params.Add(jsonutils.NewString(args.NAME), "name") if len(args.Desc) > 0 { params.Add(jsonutils.NewString(args.Desc), "description") } diff --git a/pkg/cloudprovider/resources.go b/pkg/cloudprovider/resources.go index c948c3c78a..b7f6029711 100644 --- a/pkg/cloudprovider/resources.go +++ b/pkg/cloudprovider/resources.go @@ -144,7 +144,7 @@ type ICloudVM interface { GetBios() string GetMachine() string - SyncSecurityGroup(secgroupId string, name string, rules []*secrules.SecurityRule) error + SyncSecurityGroup(secgroupId string, name string, rules []secrules.SecurityRule) error GetHypervisor() string // GetSecurityGroup() ICloudSecurityGroup diff --git a/pkg/compute/models/guests.go b/pkg/compute/models/guests.go index 24f376fd64..6f345efec3 100644 --- a/pkg/compute/models/guests.go +++ b/pkg/compute/models/guests.go @@ -1146,20 +1146,20 @@ func (self *SGuest) getAdminSecgroupName() string { return "" } -func (self *SGuest) GetSecRules() []*secrules.SecurityRule { +func (self *SGuest) GetSecRules() []secrules.SecurityRule { return self.getSecRules() } -func (self *SGuest) getSecRules() []*secrules.SecurityRule { +func (self *SGuest) getSecRules() []secrules.SecurityRule { if secgrp := self.getSecgroup(); secgrp != nil { return secgrp.getSecRules() } if rule, err := secrules.ParseSecurityRule(options.Options.DefaultSecurityRules); err == nil { - return []*secrules.SecurityRule{rule} + return []secrules.SecurityRule{*rule} } else { log.Errorf("Default SecurityRules error: %v", err) } - return []*secrules.SecurityRule{} + return []secrules.SecurityRule{} } func (self *SGuest) getSecurityRules() string { @@ -3193,8 +3193,8 @@ func (manager *SGuestManager) getIpsByExit(ips []string, isExitOnly bool) []stri return extRet } -func (manager *SGuestManager) getExpiredPendingDeleteGuests() ([]SGuest) { - deadline := time.Now().Add(time.Duration(options.Options.PendingDeleteExpireSeconds)*time.Second) +func (manager *SGuestManager) getExpiredPendingDeleteGuests() []SGuest { + deadline := time.Now().Add(time.Duration(options.Options.PendingDeleteExpireSeconds) * time.Second) q := manager.Query() q = q.IsTrue("pending_deleted").LT("pending_deleted_at", deadline).In("hypervisor", []string{"aliyun"}).Limit(options.Options.PendingDeleteMaxCleanBatchSize) diff --git a/pkg/compute/models/secgrouprules.go b/pkg/compute/models/secgrouprules.go index 8cfa24f451..4347ea48c9 100644 --- a/pkg/compute/models/secgrouprules.go +++ b/pkg/compute/models/secgrouprules.go @@ -3,6 +3,7 @@ package models import ( "context" "fmt" + "sort" "strconv" "strings" @@ -13,7 +14,6 @@ import ( "yunion.io/x/onecloud/pkg/mcclient" "yunion.io/x/pkg/util/compare" "yunion.io/x/pkg/util/secrules" - "yunion.io/x/pkg/util/sets" "yunion.io/x/pkg/util/stringutils" "yunion.io/x/sqlchemy" ) @@ -41,6 +41,25 @@ type SSecurityGroupRule struct { SecgroupID string `width:"128" charset:"ascii" create:"required"` } +type SecurityGroupRuleSet []SSecurityGroupRule + +func (v SecurityGroupRuleSet) Len() int { + return len(v) +} + +func (v SecurityGroupRuleSet) Swap(i, j int) { + v[i], v[j] = v[j], v[i] +} + +func (v SecurityGroupRuleSet) Less(i, j int) bool { + if v[i].Priority < v[j].Priority { + return true + } else if v[i].Priority == v[j].Priority { + return strings.Compare(v[i].String(), v[j].String()) <= 0 + } + return false +} + func (manager *SSecurityGroupRuleManager) AllowCreateItem(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) bool { return true } @@ -133,6 +152,8 @@ func (manager *SSecurityGroupRuleManager) ValidateCreateData( key += ":" } fields = append(fields, key) + } else if field == "cidr" { + data.Add(jsonutils.NewString("0.0.0.0/0"), "cidr") } } if _, err := secrules.ParseSecurityRule(strings.Join(fields, " ")); err != nil { @@ -187,7 +208,7 @@ func (self *SSecurityGroupRule) ValidateUpdateData(ctx context.Context, userCred return self.SResourceBase.ValidateUpdateData(ctx, userCred, query, data) } -func (self *SSecurityGroupRule) GetRule() string { +func (self *SSecurityGroupRule) String() string { var fields []string for _, field := range []string{"direction", "action", "cidr", "protocol", "ports"} { switch field { @@ -199,7 +220,7 @@ func (self *SSecurityGroupRule) GetRule() string { case "action": fields = append(fields, self.Action) case "cidr": - if len(self.CIDR) > 0 { + if len(self.CIDR) > 0 && self.CIDR != "0.0.0.0/0" { fields = append(fields, self.CIDR) } case "protocol": @@ -217,6 +238,43 @@ func (self *SSecurityGroupRule) GetRule() string { return fields[0] + strings.Join(fields[1:], " ") } +func (self *SSecurityGroupRule) SingleRules() ([]secrules.SecurityRule, error) { + rules := make([]secrules.SecurityRule, 0) + ruleStr := self.String() + if rule, err := secrules.ParseSecurityRule(ruleStr); err != nil { + return nil, err + } else if len(rule.Ports) > 0 { + for _, port := range rule.Ports { + _rule := secrules.SecurityRule{ + Priority: int(self.Priority), + Action: rule.Action, + IPNet: rule.IPNet, + Protocol: rule.Protocol, + Direction: rule.Direction, + PortStart: -1, + PortEnd: -1, + Ports: []int{port}, + Description: self.Description, + } + rules = append(rules, _rule) + } + } else { + _rule := secrules.SecurityRule{ + Priority: int(self.Priority), + Action: rule.Action, + IPNet: rule.IPNet, + Protocol: rule.Protocol, + Direction: rule.Direction, + PortStart: rule.PortStart, + PortEnd: rule.PortEnd, + Ports: []int{}, + Description: self.Description, + } + rules = append(rules, _rule) + } + return rules, nil +} + func (self *SSecurityGroupRule) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerProjId string, query jsonutils.JSONObject, data jsonutils.JSONObject) { self.SResourceBase.PostCreate(ctx, userCred, ownerProjId, query, data) @@ -255,50 +313,59 @@ func (manager *SSecurityGroupRuleManager) getRulesBySecurityGroup(secgroup *SSec func (manager *SSecurityGroupRuleManager) SyncRules(ctx context.Context, userCred mcclient.TokenCredential, secgroup *SSecurityGroup, rules []secrules.SecurityRule) ([]SSecurityGroupRule, []SSecurityGroupRule, compare.SyncResult) { syncResult := compare.SyncResult{} - if _dbRules, err := manager.getRulesBySecurityGroup(secgroup); err != nil { + if dbRules, err := manager.getRulesBySecurityGroup(secgroup); err != nil { return nil, nil, syncResult } else { - dbRules := make([]secrules.SecurityRule, len(_dbRules)) - originRules := make(map[string]*SSecurityGroupRule, len(dbRules)) - oldRules, oldStrs := make(map[string]secrules.SecurityRule, len(dbRules)), sets.NewString() + sort.Sort(SecurityGroupRuleSet(dbRules)) + sort.Sort(secrules.SecurityRuleSet(rules)) - for i := 0; i < len(_dbRules); i += 1 { - _rule := _dbRules[i] - if rule, err := secrules.ParseSecurityRule(_rule.GetRule()); err != nil { - syncResult.AddError(err) - } else { - rule.Priority = int(_rule.Priority) - rule.Description = _rule.Description - if str := jsonutils.Marshal(rule).String(); !oldStrs.Has(str) { - oldStrs.Insert(str) - oldRules[str] = *rule - originRules[str] = &_rule - } else if err := _rule.Delete(ctx, userCred); err != nil { - syncResult.AddError(err) + i, j := 0, 0 + for i < len(rules) || j < len(dbRules) { + if i < len(rules) && j < len(dbRules) { + dbStr := dbRules[j].String() + ruleStr := rules[i].String() + cmp := strings.Compare(dbStr, ruleStr) + if cmp == 0 { + if dbRules[j].Description != rules[i].Description { + if _, err := manager.TableSpec().Update(dbRules[j], func() error { + dbRules[j].Description = rules[i].Description + return nil + }); err != nil { + log.Errorf("Update SecurityGroupRule failed: %v", err) + } + } + i += 1 + j += 1 + } else if cmp > 0 { + if err := dbRules[j].Delete(ctx, userCred); err != nil { + syncResult.AddError(err) + } else { + syncResult.Delete() + } + j += 1 + } else { + if _, err := manager.newFromCloudSecurityGroup(rules[i], secgroup); err != nil { + syncResult.AddError(err) + } else { + syncResult.Add() + } + i += 1 } - } - } - - newRules, newStrs := make(map[string]secrules.SecurityRule, len(rules)), sets.NewString() - for _, rule := range rules { - if str := jsonutils.Marshal(rule).String(); !newStrs.Has(str) { - newStrs.Insert(str) - newRules[str] = rule - } - } - for _, _rule := range newStrs.Difference(oldStrs).List() { - rule := newRules[_rule] - if _, err := manager.newFromCloudSecurityGroup(rule, secgroup); err != nil { - syncResult.AddError(err) - } else { - syncResult.Add() - } - } - for _, _rule := range oldStrs.Difference(newStrs).List() { - syncResult.Delete() - if err := originRules[_rule].Delete(ctx, userCred); err != nil { - syncResult.AddError(err) + } else if i >= len(rules) { + if err := dbRules[j].Delete(ctx, userCred); err != nil { + syncResult.AddError(err) + } else { + syncResult.Delete() + } + j += 1 + } else if j >= len(dbRules) { + if _, err := manager.newFromCloudSecurityGroup(rules[i], secgroup); err != nil { + syncResult.AddError(err) + } else { + syncResult.Add() + } + i += 1 } } } diff --git a/pkg/compute/models/secgroups.go b/pkg/compute/models/secgroups.go index 34beb2e020..8f13b65782 100644 --- a/pkg/compute/models/secgroups.go +++ b/pkg/compute/models/secgroups.go @@ -99,17 +99,19 @@ func (self *SSecurityGroup) getSecurityRules() (rules []SSecurityGroupRule) { sql := secgrouprules.Query().Filter(sqlchemy.Equals(secgrouprules.Field("secgroup_id"), self.Id)) if err := db.FetchModelObjects(SecurityGroupRuleManager, sql, &rules); err != nil { log.Errorf("GetGuests fail %s", err) - return nil + return } return } -func (self *SSecurityGroup) getSecRules() []*secrules.SecurityRule { - rules := make([]*secrules.SecurityRule, 0) - for _, rule := range self.getSecurityRules() { - r, _ := secrules.ParseSecurityRule(rule.GetRule()) - r.Priority = int(rule.Priority) - rules = append(rules, r) +func (self *SSecurityGroup) getSecRules() []secrules.SecurityRule { + rules := make([]secrules.SecurityRule, 0) + for _, _rule := range self.getSecurityRules() { + singleRules, err := _rule.SingleRules() + if err != nil { + log.Errorf(err.Error()) + } + rules = append(rules, singleRules...) } return rules } @@ -118,7 +120,7 @@ func (self *SSecurityGroup) getSecurityRuleString() string { secgrouprules := self.getSecurityRules() var rules []string for _, rule := range secgrouprules { - rules = append(rules, rule.GetRule()) + rules = append(rules, rule.String()) } return strings.Join(rules, SECURITY_GROUP_SEPARATOR) } @@ -202,22 +204,32 @@ func (manager *SSecurityGroupManager) SyncSecgroups(ctx context.Context, userCre } for i := 0; i < len(commondb); i += 1 { - if err = commondb[i].SyncWithCloudSecurityGroup(userCred, commonext[i]); err != nil { - syncResult.UpdateError(err) - } else { - localSecgroups = append(localSecgroups, commondb[i]) - remoteSecgroups = append(remoteSecgroups, commonext[i]) - syncResult.Update() + if rules, err := commonext[i].GetRules(); err != nil { + syncResult.Error(err) + } else if len(rules) > 0 { + if err = commondb[i].SyncWithCloudSecurityGroup(userCred, commonext[i]); err != nil { + syncResult.UpdateError(err) + } else { + localSecgroups = append(localSecgroups, commondb[i]) + remoteSecgroups = append(remoteSecgroups, commonext[i]) + SecurityGroupRuleManager.SyncRules(ctx, userCred, &commondb[i], rules) + syncResult.Update() + } } } for i := 0; i < len(added); i += 1 { - if new, err := manager.newFromCloudVpc(added[i]); err != nil { + if rules, err := added[i].GetRules(); err != nil { syncResult.AddError(err) - } else { - localSecgroups = append(localSecgroups, *new) - remoteSecgroups = append(remoteSecgroups, added[i]) - syncResult.Add() + } else if len(rules) > 0 { + if new, err := manager.newFromCloudVpc(added[i]); err != nil { + syncResult.AddError(err) + } else { + localSecgroups = append(localSecgroups, *new) + remoteSecgroups = append(remoteSecgroups, added[i]) + SecurityGroupRuleManager.SyncRules(ctx, userCred, new, rules) + syncResult.Add() + } } } } diff --git a/pkg/compute/tasks/cloud_provider_sync_info_task.go b/pkg/compute/tasks/cloud_provider_sync_info_task.go index cdb5c6bf46..b06d05b483 100644 --- a/pkg/compute/tasks/cloud_provider_sync_info_task.go +++ b/pkg/compute/tasks/cloud_provider_sync_info_task.go @@ -152,32 +152,13 @@ func syncVpcSecGroup(ctx context.Context, provider *models.SCloudprovider, task logSyncFailed(provider, task, msg) return } else { - localSecgroups, removeSecgroups, result := models.SecurityGroupManager.SyncSecgroups(ctx, task.UserCred, secgroups) + _, _, result := models.SecurityGroupManager.SyncSecgroups(ctx, task.UserCred, secgroups) msg := result.Result() log.Infof("SyncSecurityGroup for VPC %s result: %s", localVpc.Name, msg) if result.IsError() { logSyncFailed(provider, task, msg) return } - //db.OpsLog.LogEvent(provider, db.ACT_SYNC_HOST_COMPLETE, msg, task.GetUserCred()) - for i := 0; i < len(localSecgroups); i += 1 { - syncSecgroupRules(ctx, provider, task, &localSecgroups[i], removeSecgroups[i]) - } - } -} - -func syncSecgroupRules(ctx context.Context, provider *models.SCloudprovider, task *CloudProviderSyncInfoTask, localSecgroup *models.SSecurityGroup, remoteSecgroup cloudprovider.ICloudSecurityGroup) { - rules, err := remoteSecgroup.GetRules() - if err != nil { - logSyncFailed(provider, task, err.Error()) - return - } - _, _, result := models.SecurityGroupRuleManager.SyncRules(ctx, task.UserCred, localSecgroup, rules) - msg := result.Result() - log.Infof("SyncRules for secgroups %s result: %s", localSecgroup.Name, msg) - if result.IsError() { - logSyncFailed(provider, task, msg) - return } } diff --git a/pkg/util/aliyun/instance.go b/pkg/util/aliyun/instance.go index 973236d825..80a2e9c267 100644 --- a/pkg/util/aliyun/instance.go +++ b/pkg/util/aliyun/instance.go @@ -514,7 +514,7 @@ func (self *SInstance) GetVNCInfo() (jsonutils.JSONObject, error) { return ret, nil } -func (self *SInstance) SyncSecurityGroup(secgroupId string, name string, rules []*secrules.SecurityRule) error { +func (self *SInstance) SyncSecurityGroup(secgroupId string, name string, rules []secrules.SecurityRule) error { if vpc, err := self.getVpc(); err != nil { return err } else if len(secgroupId) == 0 { diff --git a/pkg/util/aliyun/securitygroup.go b/pkg/util/aliyun/securitygroup.go index dce8d329df..69515ae79d 100644 --- a/pkg/util/aliyun/securitygroup.go +++ b/pkg/util/aliyun/securitygroup.go @@ -2,6 +2,7 @@ package aliyun import ( "fmt" + "sort" "strings" "time" @@ -9,7 +10,6 @@ import ( "yunion.io/x/log" "yunion.io/x/onecloud/pkg/httperrors" "yunion.io/x/pkg/util/secrules" - "yunion.io/x/pkg/util/sets" "yunion.io/x/pkg/utils" ) @@ -58,6 +58,25 @@ type SSecurityGroup struct { RegionId string } +type PermissionSet []SPermission + +func (v PermissionSet) Len() int { + return len(v) +} + +func (v PermissionSet) Swap(i, j int) { + v[i], v[j] = v[j], v[i] +} + +func (v PermissionSet) Less(i, j int) bool { + if v[i].Priority < v[j].Priority { + return true + } else if v[i].Priority == v[j].Priority { + return strings.Compare(v[i].String(), v[j].String()) <= 0 + } + return false +} + func (self *SSecurityGroup) GetId() string { return self.SecurityGroupId } @@ -76,7 +95,7 @@ func (self *SSecurityGroup) GetRules() ([]secrules.SecurityRule, error) { return rules, err } else { for _, permission := range secgrp.Permissions.Permission { - if rule, err := secrules.ParseSecurityRule(permission.toString()); err != nil { + if rule, err := secrules.ParseSecurityRule(permission.String()); err != nil { return rules, err } else { priority := permission.Priority @@ -184,6 +203,52 @@ func (self *SRegion) createSecurityGroup(vpcId string, name string, desc string) return body.GetString("SecurityGroupId") } +func (self *SRegion) modifySecurityGroupRule(secGrpId string, rule *secrules.SecurityRule) error { + params := make(map[string]string) + params["RegionId"] = self.RegionId + params["SecurityGroupId"] = secGrpId + params["NicType"] = string(IntranetNicType) + params["Description"] = rule.Description + params["PortRange"] = fmt.Sprintf("%d/%d", rule.PortStart, rule.PortEnd) + protocol := rule.Protocol + if len(rule.Protocol) == 0 || rule.Protocol == secrules.PROTO_ANY { + protocol = "all" + } + params["IpProtocol"] = protocol + if rule.PortStart == 0 && rule.PortEnd == 0 { + if protocol == "udp" || protocol == "tcp" { + params["PortRange"] = "1/65535" + } else { + params["PortRange"] = "-1/-1" + } + } + if rule.Action == secrules.SecurityRuleAllow { + params["Policy"] = "accept" + } else { + params["Policy"] = "drop" + } + params["Priority"] = fmt.Sprintf("%d", rule.Priority) + if rule.Direction == secrules.SecurityRuleIngress { + if rule.IPNet != nil { + params["SourceCidrIp"] = rule.IPNet.String() + } else { + params["SourceCidrIp"] = "0.0.0.0/0" + } + _, err := self.ecsRequest("ModifySecurityGroupRule", params) + return err + } else { // rule.Direction == secrules.SecurityRuleEgress { + //阿里云不支持出方向API接口调用 + return nil + // if rule.IPNet != nil { + // params["DestCidrIp"] = rule.IPNet.String() + // } else { + // params["DestCidrIp"] = "0.0.0.0/0" + // } + // _, err := self.ecsRequest("ModifySecurityGroupRule", params) + // return err + } +} + func (self *SRegion) modifySecurityGroup(secGrpId string, name string, desc string) error { params := make(map[string]string) params["RegionId"] = self.RegionId @@ -196,25 +261,45 @@ func (self *SRegion) modifySecurityGroup(secGrpId string, name string, desc stri return err } +func (self *SRegion) addSecurityGroupRules(secGrpId string, rule *secrules.SecurityRule) error { + if len(rule.Ports) != 0 { + for _, port := range rule.Ports { + rule.PortStart, rule.PortEnd = port, port + if err := self.addSecurityGroupRule(secGrpId, rule); err != nil { + return err + } + } + } else { + return self.addSecurityGroupRule(secGrpId, rule) + } + return nil +} + func (self *SRegion) addSecurityGroupRule(secGrpId string, rule *secrules.SecurityRule) error { params := make(map[string]string) params["RegionId"] = self.RegionId params["SecurityGroupId"] = secGrpId params["NicType"] = string(IntranetNicType) - params["PortRange"] = fmt.Sprintf("%d/%d", rule.PortStart, rule.PortEnd) params["Description"] = rule.Description + params["PortRange"] = fmt.Sprintf("%d/%d", rule.PortStart, rule.PortEnd) protocol := rule.Protocol if len(rule.Protocol) == 0 || rule.Protocol == secrules.PROTO_ANY { protocol = "all" - params["PortRange"] = "-1/-1" } params["IpProtocol"] = protocol + if rule.PortStart == 0 && rule.PortEnd == 0 { + if protocol == "udp" || protocol == "tcp" { + params["PortRange"] = "1/65535" + } else { + params["PortRange"] = "-1/-1" + } + } if rule.Action == secrules.SecurityRuleAllow { params["Policy"] = "accept" } else { params["Policy"] = "drop" } - params["Priority"] = fmt.Sprintf("%d", rule.Priority) + params["Priority"] = fmt.Sprintf("%d", 101-rule.Priority) if rule.Direction == secrules.SecurityRuleIngress { if rule.IPNet != nil { params["SourceCidrIp"] = rule.IPNet.String() @@ -243,9 +328,15 @@ func (self *SRegion) delSecurityGroupRule(secGrpId string, rule *secrules.Securi protocol := rule.Protocol if len(rule.Protocol) == 0 || rule.Protocol == secrules.PROTO_ANY { protocol = "all" - params["PortRange"] = "-1/-1" } params["IpProtocol"] = protocol + if rule.PortStart == 0 && rule.PortEnd == 0 { + if protocol == "udp" || protocol == "tcp" { + params["PortRange"] = "1/65535" + } else { + params["PortRange"] = "-1/-1" + } + } if rule.Action == secrules.SecurityRuleAllow { params["Policy"] = "accept" } else { @@ -284,7 +375,7 @@ func (self *SRegion) createDefaultSecurityGroup(vpcId string) (string, error) { PortStart: -1, PortEnd: -1, } - err = self.addSecurityGroupRule(secId, &inRule) + err = self.addSecurityGroupRules(secId, &inRule) if err != nil { return "", err } @@ -296,7 +387,7 @@ func (self *SRegion) createDefaultSecurityGroup(vpcId string) (string, error) { PortStart: -1, PortEnd: -1, } - err = self.addSecurityGroupRule(secId, &outRule) + err = self.addSecurityGroupRules(secId, &outRule) if err != nil { return "", err } @@ -324,10 +415,6 @@ func (self *SRegion) getSecurityGroupByTag(vpcId, secgroupId string) (*SSecurity } func (self *SPermission) String() string { - return self.toString() -} - -func (self *SPermission) toString() string { action := secrules.SecurityRuleDeny if strings.ToLower(self.Policy) == "accept" { action = secrules.SecurityRuleAllow @@ -340,17 +427,30 @@ func (self *SPermission) toString() string { if direction == "out" { cidr = self.DestCidrIp } + if cidr == "0.0.0.0/0" { + cidr = "" + } protocol := strings.ToLower(self.IpProtocol) if protocol == "all" { protocol = "any" } port, ports := "", strings.Split(self.PortRange, "/") - if ports[0] == ports[1] && (ports[0] != "-1") { - port = ports[0] - } else { + if ports[0] == ports[1] { + if ports[0] != "-1" { + port = ports[0] + } + } else if ports[0] != "1" && ports[1] != "65535" { port = fmt.Sprintf("%s-%s", ports[0], ports[1]) } - return fmt.Sprintf("%s:%s %s %s %s", direction, string(action), cidr, protocol, port) + result := fmt.Sprintf("%s:%s", direction, string(action)) + if len(cidr) > 0 { + result += fmt.Sprintf(" %s", cidr) + } + result += fmt.Sprintf(" %s", protocol) + if len(port) > 0 { + result += fmt.Sprintf(" %s", port) + } + return result } func (self *SRegion) addTagToSecurityGroup(secgroupId, key, value string, index int) error { @@ -371,11 +471,11 @@ func (self *SRegion) revokeSecurityGroup(secgroupId, instanceId string, keep boo if secgroup, err := self.GetSecurityGroupDetails(secgroupId); err != nil { return err } else { - for _, r := range secgroup.Permissions.Permission { - if rule, err := secrules.ParseSecurityRule(r.toString()); err != nil { + for _, permission := range secgroup.Permissions.Permission { + if rule, err := secrules.ParseSecurityRule(permission.String()); err != nil { return err } else { - rule.Priority = r.Priority + rule.Priority = permission.Priority if err := self.delSecurityGroupRule(secgroup.SecurityGroupId, rule); err != nil { return err } @@ -383,13 +483,13 @@ func (self *SRegion) revokeSecurityGroup(secgroupId, instanceId string, keep boo } if rule, err := secrules.ParseSecurityRule("in:allow any"); err != nil { rule.Priority = 100 - if err := self.addSecurityGroupRule(secgroup.SecurityGroupId, rule); err != nil { + if err := self.addSecurityGroupRules(secgroup.SecurityGroupId, rule); err != nil { return err } } if rule, err := secrules.ParseSecurityRule("out:allow any"); err != nil { rule.Priority = 100 - if err := self.addSecurityGroupRule(secgroup.SecurityGroupId, rule); err != nil { + if err := self.addSecurityGroupRules(secgroup.SecurityGroupId, rule); err != nil { return err } } @@ -397,48 +497,68 @@ func (self *SRegion) revokeSecurityGroup(secgroupId, instanceId string, keep boo return nil } -func (self *SRegion) syncSecgroupRules(secgroupId string, rules []*secrules.SecurityRule) error { +func (self *SRegion) syncSecgroupRules(secgroupId string, rules []secrules.SecurityRule) error { if secgroup, err := self.GetSecurityGroupDetails(secgroupId); err != nil { return err } else { - newRules, newStr := make(map[string]*secrules.SecurityRule), sets.NewString() - for _, rule := range rules { - rule.Priority = 101 - rule.Priority - if len(rule.Ports) > 0 { - for _, port := range rule.Ports { - rule.PortStart, rule.PortEnd = port, port - if jsonStr := jsonutils.Marshal(rule).String(); !newStr.Has(jsonStr) { - newStr.Insert(jsonStr) - newRules[jsonStr] = rule + + sort.Sort(secrules.SecurityRuleSet(rules)) + sort.Sort(PermissionSet(secgroup.Permissions.Permission)) + + i, j := 0, 0 + for i < len(rules) || j < len(secgroup.Permissions.Permission) { + if i < len(rules) && j < len(secgroup.Permissions.Permission) { + permissionStr := secgroup.Permissions.Permission[j].String() + ruleStr := rules[i].String() + cmp := strings.Compare(permissionStr, ruleStr) + if cmp == 0 { + if secgroup.Permissions.Permission[j].Description != rules[i].Description { + rules[i].Priority = secgroup.Permissions.Permission[j].Priority + if err := self.modifySecurityGroupRule(secgroupId, &rules[i]); err != nil { + log.Errorf("modifySecurityGroupRule error %v", rules[i]) + return err + } + } + i += 1 + j += 1 + } else if cmp > 0 { + if rule, err := secrules.ParseSecurityRule(permissionStr); err != nil { + return err + } else { + rule.Priority = secgroup.Permissions.Permission[j].Priority + if err := self.delSecurityGroupRule(secgroupId, rule); err != nil { + log.Errorf("delSecurityGroupRule error %v", rule) + return err + } + } + j += 1 + } else { + if err := self.addSecurityGroupRules(secgroupId, &rules[i]); err != nil { + log.Errorf("addSecurityGroupRule error %v", rules[i]) + return err + } + i += 1 + } + } else if i >= len(rules) { + permissionStr := secgroup.Permissions.Permission[j].String() + if rule, err := secrules.ParseSecurityRule(permissionStr); err != nil { + return err + } else { + rule.Priority = secgroup.Permissions.Permission[j].Priority + if err := self.delSecurityGroupRule(secgroupId, rule); err != nil { + log.Errorf("delSecurityGroupRule error %v", rule) + return err } } - } else if jsonStr := jsonutils.Marshal(rule).String(); !newStr.Has(jsonStr) { - newStr.Insert(jsonStr) - newRules[jsonStr] = rule - } - } - - oldRules, oldStr := make(map[string]*secrules.SecurityRule), sets.NewString() - for _, r := range secgroup.Permissions.Permission { - if rule, err := secrules.ParseSecurityRule(r.toString()); err != nil { - return err - } else { - rule.Priority = r.Priority - rule.Description = r.Description - if jsonStr := jsonutils.Marshal(rule).String(); !oldStr.Has(jsonStr) { - oldStr.Insert(jsonStr) - oldRules[jsonStr] = rule + j += 1 + } else if j >= len(secgroup.Permissions.Permission) { + if err := self.addSecurityGroupRules(secgroupId, &rules[i]); err != nil { + log.Errorf("addSecurityGroupRule error %v", rules[i]) + return err } + i += 1 } } - for _, jsonStr := range newStr.Difference(oldStr).List() { - rule := newRules[jsonStr] - self.addSecurityGroupRule(secgroupId, rule) - } - for _, jsonStr := range oldStr.Difference(newStr).List() { - rule := oldRules[jsonStr] - self.delSecurityGroupRule(secgroupId, rule) - } } return nil } diff --git a/pkg/util/aliyun/vpc.go b/pkg/util/aliyun/vpc.go index 04a02da4be..9751039c39 100644 --- a/pkg/util/aliyun/vpc.go +++ b/pkg/util/aliyun/vpc.go @@ -187,7 +187,7 @@ func (self *SVpc) Delete() error { return self.region.DeleteVpc(self.VpcId) } -func (self *SVpc) syncSecurityGroup(secgroupId string, name string, rules []*secrules.SecurityRule) (string, error) { +func (self *SVpc) syncSecurityGroup(secgroupId string, name string, rules []secrules.SecurityRule) (string, error) { secgrpId := "" if secgroup, err := self.region.getSecurityGroupByTag(self.VpcId, secgroupId); err != nil { if secgrpId, err = self.region.createSecurityGroup(self.VpcId, name, ""); err != nil { @@ -198,7 +198,7 @@ func (self *SVpc) syncSecurityGroup(secgroupId string, name string, rules []*sec //addRules log.Debugf("Add Rules for %s", secgrpId) for _, rule := range rules { - if err := self.region.addSecurityGroupRule(secgrpId, rule); err != nil { + if err := self.region.addSecurityGroupRule(secgrpId, &rule); err != nil { return "", err } } diff --git a/pkg/util/esxi/virtualmachine.go b/pkg/util/esxi/virtualmachine.go index 52ae4ddbb4..2005bbc9bc 100644 --- a/pkg/util/esxi/virtualmachine.go +++ b/pkg/util/esxi/virtualmachine.go @@ -35,7 +35,7 @@ func (self *SVirtualMachine) GetGlobalId() string { return self.getUuid() } -func (self *SVirtualMachine) SyncSecurityGroup(secgroupId, name string, rules []*secrules.SecurityRule) error { +func (self *SVirtualMachine) SyncSecurityGroup(secgroupId, name string, rules []secrules.SecurityRule) error { return nil } diff --git a/vendor/yunion.io/x/pkg/util/secrules/secrules.go b/vendor/yunion.io/x/pkg/util/secrules/secrules.go index c9d9b410ba..4f249fdcd6 100644 --- a/vendor/yunion.io/x/pkg/util/secrules/secrules.go +++ b/vendor/yunion.io/x/pkg/util/secrules/secrules.go @@ -63,6 +63,25 @@ var ( ErrInvalidPort = errors.New("invalid port") ) +type SecurityRuleSet []SecurityRule + +func (v SecurityRuleSet) Len() int { + return len(v) +} + +func (v SecurityRuleSet) Swap(i, j int) { + v[i], v[j] = v[j], v[i] +} + +func (v SecurityRuleSet) Less(i, j int) bool { + if v[i].Priority > v[j].Priority { + return true + } else if v[i].Priority == v[j].Priority { + return strings.Compare(v[i].String(), v[j].String()) <= 0 + } + return false +} + func parsePortString(ps string) (int, error) { p, err := strconv.ParseUint(ps, 10, 16) if err != nil || p == 0 {