From 6aeea0f9bde281fbc3dbcba0782fd057a15c16c2 Mon Sep 17 00:00:00 2001
From: Qu Xuan <quxuan@yunionyun.com>
Date: Sat, 19 Dec 2020 14:47:03 +0800
Subject: [PATCH] fix(region): azure secgroup rule not allow with same priority
 and direction

---
 pkg/cloudprovider/securitygroup.go            | 16 +++++++++
 .../regiondrivers/secgroup_azure_test.go      | 36 +++++++++++++++++++
 pkg/multicloud/azure/securitygroup.go         | 15 +++++++-
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/pkg/cloudprovider/securitygroup.go b/pkg/cloudprovider/securitygroup.go
index ca6c402c0d..73f4ca77cf 100644
--- a/pkg/cloudprovider/securitygroup.go
+++ b/pkg/cloudprovider/securitygroup.go
@@ -372,3 +372,19 @@ func CompareRules(
 	common = append(inCommon, outCommon...)
 	return
 }
+
+func SortUniqPriority(rules SecurityRuleSet) []SecurityRule {
+	sort.Sort(rules)
+	priMap := map[int]bool{}
+	for i := range rules {
+		for {
+			_, ok := priMap[rules[i].Priority]
+			if !ok {
+				priMap[rules[i].Priority] = true
+				break
+			}
+			rules[i].Priority = rules[i].Priority - 1
+		}
+	}
+	return rules
+}
diff --git a/pkg/compute/regiondrivers/secgroup_azure_test.go b/pkg/compute/regiondrivers/secgroup_azure_test.go
index da9c459e0f..500a64a9ff 100644
--- a/pkg/compute/regiondrivers/secgroup_azure_test.go
+++ b/pkg/compute/regiondrivers/secgroup_azure_test.go
@@ -233,3 +233,39 @@ func TestAzureRuleSync(t *testing.T) {
 		check(t, "outDels", outDels, d.OutDels)
 	}
 }
+
+func TestAzureUniqPriorityRuleSync(t *testing.T) {
+	cases := []struct {
+		in  cloudprovider.SecurityRuleSet
+		out cloudprovider.SecurityRuleSet
+	}{
+		{
+			in: cloudprovider.SecurityRuleSet{
+				remoteRuleWithName("", "out:allow icmp", 99),
+				remoteRuleWithName("", "out:allow tcp 100-200", 99),
+				remoteRuleWithName("", "out:allow udp 200-300", 99),
+			},
+			out: cloudprovider.SecurityRuleSet{
+				remoteRuleWithName("", "out:allow icmp", 99),
+				remoteRuleWithName("", "out:allow tcp 100-200", 98),
+				remoteRuleWithName("", "out:allow udp 200-300", 97),
+			},
+		},
+	}
+
+	for _, c := range cases {
+		rules := cloudprovider.SortUniqPriority(c.in)
+		if len(c.out) != len(rules) {
+			t.Fatalf("invalid output")
+		}
+		for i := range rules {
+			if rules[i].String() != c.out[i].String() {
+				t.Fatalf("rule shoud be %s not %s", c.out[i].String(), rules[i].String())
+			}
+			if rules[i].Priority != c.out[i].Priority {
+				t.Fatalf("rule %s priority should be %d not %d", rules[i].String(), c.out[i].Priority, rules[i].Priority)
+			}
+		}
+	}
+
+}
diff --git a/pkg/multicloud/azure/securitygroup.go b/pkg/multicloud/azure/securitygroup.go
index a288a97556..14e457be32 100644
--- a/pkg/multicloud/azure/securitygroup.go
+++ b/pkg/multicloud/azure/securitygroup.go
@@ -479,6 +479,19 @@ func (self *SSecurityGroup) SetRules(rules []cloudprovider.SecurityRule) error {
 }
 
 func (self *SSecurityGroup) SyncRules(common, inAdds, outAdds, inDels, outDels []cloudprovider.SecurityRule) error {
-	rules := append(common, append(inAdds, outAdds...)...)
+	for i := range common {
+		switch common[i].Direction {
+		case secrules.DIR_IN:
+			inAdds = append(inAdds, common[i])
+		case secrules.DIR_OUT:
+			outAdds = append(outAdds, common[i])
+		default:
+			return fmt.Errorf("invalid rule %s direction %s", common[i].String(), common[i].Direction)
+		}
+	}
+	// Azure 不允许同方向的规则优先级相同
+	inRules := cloudprovider.SortUniqPriority(inAdds)
+	outRules := cloudprovider.SortUniqPriority(outAdds)
+	rules := append(inRules, outRules...)
 	return self.SetRules(rules)
 }