update sync secgroup rules

pkg/util/aws/securitygroup.go

@@ -3,8 +3,11 @@ package aws
 import (
 	"fmt"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"sort"
+	"strings"
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
+	"yunion.io/x/onecloud/pkg/httperrors"
 	"yunion.io/x/pkg/util/secrules"
 )
 
@@ -131,6 +134,66 @@ func (self *SRegion) addSecurityGroupRule(secGrpId string, rule *secrules.Securi
 	return nil
 }
 
+func (self *SRegion) delSecurityGroupRule(secGrpId string, rule *secrules.SecurityRule) error {
+	ipPermissions, err := YunionSecRuleToAws(*rule)
+	if err != nil {
+		return err
+	}
+
+	if rule.Direction == secrules.SecurityRuleIngress {
+		params := &ec2.RevokeSecurityGroupIngressInput{}
+		params.SetGroupId(secGrpId)
+		params.SetIpPermissions(ipPermissions)
+		_, err := self.ec2Client.RevokeSecurityGroupIngress(params)
+		if err != nil {
+			return err
+		}
+	}
+
+	if rule.Direction == secrules.SecurityRuleEgress {
+		params := &ec2.RevokeSecurityGroupEgressInput{}
+		params.SetGroupId(secGrpId)
+		params.SetIpPermissions(ipPermissions)
+		_, err := self.ec2Client.RevokeSecurityGroupEgress(params)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (self *SRegion) updateSecurityGroupRuleDescription(secGrpId string, rule *secrules.SecurityRule) error {
+	ipPermissions, err := YunionSecRuleToAws(*rule)
+	if err != nil {
+		return err
+	}
+
+	if rule.Direction == secrules.SecurityRuleIngress {
+		params := &ec2.UpdateSecurityGroupRuleDescriptionsIngressInput{}
+		params.SetGroupId(secGrpId)
+		params.SetIpPermissions(ipPermissions)
+		ret, err := self.ec2Client.UpdateSecurityGroupRuleDescriptionsIngress(params)
+		if err != nil {
+			return err
+		} else if ret.Return != nil && *ret.Return == false {
+			log.Debugf("update security group %s rule description failed: %s", secGrpId, ipPermissions)
+		}
+	}
+
+	if rule.Direction == secrules.SecurityRuleEgress {
+		params := &ec2.UpdateSecurityGroupRuleDescriptionsEgressInput{}
+		params.SetGroupId(secGrpId)
+		params.SetIpPermissions(ipPermissions)
+		ret, err := self.ec2Client.UpdateSecurityGroupRuleDescriptionsEgress(params)
+		if err != nil {
+			return err
+		} else if ret.Return != nil && *ret.Return == false {
+			log.Debugf("update security group %s rule description failed: %s", secGrpId, ipPermissions)
+		}
+	}
+	return nil
+}
+
 func (self *SRegion) createSecurityGroup(vpcId string, name string, desc string) (string, error) {
 	params := &ec2.CreateSecurityGroupInput{}
 	params.SetVpcId(vpcId)
@@ -199,7 +262,16 @@ func (self *SRegion) GetSecurityGroupDetails(secGroupId string) (*SSecurityGroup
 }
 
 func (self *SRegion) getSecurityGroupByTag(vpcId, secgroupId string) (*SSecurityGroup, error) {
-	return nil, nil
+	secgroups, total, err := self.GetSecurityGroups(vpcId, secgroupId, 0, 0)
+	if err != nil {
+		return nil, err
+	}
+
+	if total != 1 {
+		log.Debugf("failed to find  SecurityGroup %s: %d found", secgroupId, total)
+		return nil, httperrors.NewNotFoundError("failed to find SecurityGroup %s", secgroupId)
+	}
+	return &secgroups[0], nil
 }
 
 func (self *SRegion) addTagToSecurityGroup(secgroupId, key, value string, index int) error {
@@ -207,10 +279,73 @@ func (self *SRegion) addTagToSecurityGroup(secgroupId, key, value string, index
 }
 
 func (self *SRegion) modifySecurityGroup(secGrpId string, name string, desc string) error {
+	tagspec := TagSpec{ResourceType: "security-group"}
+	tagspec.SetNameTag(name)
+	tagspec.SetDescTag(desc)
+	ec2Tags, _ := tagspec.GetTagSpecifications()
+	params := &ec2.CreateTagsInput{}
+	params.SetTags(ec2Tags.Tags)
+	params.SetResources([]*string{&secGrpId})
+
+	_, err := self.ec2Client.CreateTags(params)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 
 func (self *SRegion) syncSecgroupRules(secgroupId string, rules []secrules.SecurityRule) error {
+	if secgroup, err := self.GetSecurityGroupDetails(secgroupId); err != nil {
+		return err
+	} else {
+
+		sort.Sort(secrules.SecurityRuleSet(rules))
+		sort.Sort(secrules.SecurityRuleSet(secgroup.Permissions))
+
+		i, j := 0, 0
+		for i < len(rules) || j < len(secgroup.Permissions) {
+			if i < len(rules) && j < len(secgroup.Permissions) {
+				permissionStr := secgroup.Permissions[j].String()
+				ruleStr := rules[i].String()
+				cmp := strings.Compare(permissionStr, ruleStr)
+				if cmp == 0 {
+					if secgroup.Permissions[j].Description != rules[i].Description {
+						if err := self.updateSecurityGroupRuleDescription(secgroupId, &rules[i]); err != nil {
+							log.Errorf("updateSecurityGroupRuleDescription error %v", rules[i])
+							return err
+						}
+					}
+					i += 1
+					j += 1
+				} else if cmp > 0 {
+					if err := self.delSecurityGroupRule(secgroupId, &secgroup.Permissions[j]); err != nil {
+						log.Errorf("delSecurityGroupRule error %v", secgroup.Permissions[j])
+						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) {
+				if err := self.delSecurityGroupRule(secgroupId, &secgroup.Permissions[j]); err != nil {
+					log.Errorf("delSecurityGroupRule error %v", secgroup.Permissions[j])
+					return err
+				}
+				j += 1
+			} else if j >= len(secgroup.Permissions) {
+				if err := self.addSecurityGroupRules(secgroupId, &rules[i]); err != nil {
+					log.Errorf("addSecurityGroupRule error %v", rules[i])
+					return err
+				}
+				i += 1
+			}
+		}
+	}
 	return nil
 }
 
@@ -241,13 +376,17 @@ func (self *SRegion) getSecRules(ingress []*ec2.IpPermission, egress []*ec2.IpPe
 	return rules
 }
 
-func (self *SRegion) GetSecurityGroups(vpcId string, offset int, limit int) ([]SSecurityGroup, int, error) {
+func (self *SRegion) GetSecurityGroups(vpcId string, secgroupId string, offset int, limit int) ([]SSecurityGroup, int, error) {
 	params := &ec2.DescribeSecurityGroupsInput{}
 	filters := make([]*ec2.Filter, 0)
 	if len(vpcId) > 0 {
 		filters = AppendSingleValueFilter(filters, "vpc-id", vpcId)
 	}
 
+	if len(secgroupId) > 0 {
+		filters = AppendSingleValueFilter(filters, "group-id", secgroupId)
+	}
+
 	if len(filters) > 0 {
 		params.SetFilters(filters)
 	}

pkg/util/aws/vpc.go

@@ -143,11 +143,10 @@ func (self *SVpc) SyncSecurityGroup(secgroupId string, name string, rules []secr
 	if secgroup, err := self.region.getSecurityGroupByTag(self.VpcId, secgroupId); err != nil {
 		if secgrpId, err = self.region.createSecurityGroup(self.VpcId, name, ""); err != nil {
 			return "", err
-		} else if err := self.region.addTagToSecurityGroup(secgrpId, "id", secgroupId, 1); err != nil {
-			return "", err
 		}
+
 		//addRules
-		log.Debugf("Add Rules for %s", secgrpId)
+		log.Debugf("Add Rules for %s" , secgrpId)
 		for _, rule := range rules {
 			if err := self.region.addSecurityGroupRule(secgrpId, &rule); err != nil {
 				return "", err