Repository: kubernetes-sigs/cloud-provider-azure
Pull request #2006 (2022-07-11 06:27:30)

[release-1.23] fix: make sure there is no duplicated rules before updating the netwo…
This is an automated cherry-pick of #1998

/assign nilo19

```release-note
fix: make sure there is no duplicated rules before updating the network security group
```


Patch:
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
index ee991f5c1..ceb082fa9 100644
--- a/pkg/provider/azure_loadbalancer.go
+++ b/pkg/provider/azure_loadbalancer.go
@@ -2584,6 +2584,8 @@ func (az *Cloud) reconcileSecurityRules(sg network.SecurityGroup, service *v1.Se
 		}
 	}
 
+	updatedRules = removeDuplicatedSecurityRules(updatedRules)
+
 	for _, r := range updatedRules {
 		klog.V(10).Infof("Updated security rule while processing %s: %s:%s -> %s:%s", service.Name, logSafe(r.SourceAddressPrefix), logSafe(r.SourcePortRange), logSafeCollection(r.DestinationAddressPrefix, r.DestinationAddressPrefixes), logSafe(r.DestinationPortRange))
 	}
diff --git a/pkg/provider/azure_utils.go b/pkg/provider/azure_utils.go
index 8a71d3238..13cf52b38 100644
--- a/pkg/provider/azure_utils.go
+++ b/pkg/provider/azure_utils.go
@@ -316,3 +316,15 @@ func SliceEqual(s1, s2 []string) bool {
 	}
 	return true
 }
+
+func removeDuplicatedSecurityRules(rules []network.SecurityRule) []network.SecurityRule {
+	ruleNames := make(map[string]bool)
+	for i := len(rules) - 1; i >= 0; i-- {
+		if _, ok := ruleNames[to.String(rules[i].Name)]; ok {
+			klog.Warningf("Found duplicated rule %s, will be removed.", to.String(rules[i].Name))
+			rules = append(rules[:i], rules[i+1:]...)
+		}
+		ruleNames[to.String(rules[i].Name)] = true
+	}
+	return rules
+}


Test patch:
diff --git a/pkg/provider/azure_utils_test.go b/pkg/provider/azure_utils_test.go
index 2f5a919f1..48dc6e923 100644
--- a/pkg/provider/azure_utils_test.go
+++ b/pkg/provider/azure_utils_test.go
@@ -21,10 +21,13 @@ import (
 	"testing"
 	"time"
 
+	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/stretchr/testify/assert"
+
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 )
 
@@ -255,3 +258,59 @@ func TestGetServiceAdditionalPublicIPs(t *testing.T) {
 		})
 	}
 }
+
+func TestRemoveDuplicatedSecurityRules(t *testing.T) {
+	for _, testCase := range []struct {
+		description string
+		rules       []network.SecurityRule
+		expected    []network.SecurityRule
+	}{
+		{
+			description: "no duplicated rules",
+			rules: []network.SecurityRule{
+				{
+					Name: to.StringPtr("rule1"),
+				},
+				{
+					Name: to.StringPtr("rule2"),
+				},
+			},
+			expected: []network.SecurityRule{
+				{
+					Name: to.StringPtr("rule1"),
+				},
+				{
+					Name: to.StringPtr("rule2"),
+				},
+			},
+		},
+		{
+			description: "duplicated rules",
+			rules: []network.SecurityRule{
+				{
+					Name: to.StringPtr("rule1"),
+				},
+				{
+					Name: to.StringPtr("rule2"),
+				},
+				{
+					Name: to.StringPtr("rule1"),
+				},
+			},
+			expected: []network.SecurityRule{
+				{
+					Name: to.StringPtr("rule2"),
+				},
+				{
+					Name: to.StringPtr("rule1"),
+				},
+			},
+		},
+	} {
+		t.Run(testCase.description, func(t *testing.T) {
+			rules := testCase.rules
+			rules = removeDuplicatedSecurityRules(rules)
+			assert.Equal(t, testCase.expected, rules)
+		})
+	}
+}