fix: ignore the case when updating tags

Author: nilo19

staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go

@@ -2139,7 +2139,6 @@ func shouldReleaseExistingOwnedPublicIP(existingPip *network.PublicIPAddress, lb
 
 //  ensurePIPTagged ensures the public IP of the service is tagged as configured
 func (az *Cloud) ensurePIPTagged(service *v1.Service, pip *network.PublicIPAddress) bool {
-	changed := false
 	configTags := parseTags(az.Tags)
 	annotationTags := make(map[string]*string)
 	if _, ok := service.Annotations[ServiceAnnotationAzurePIPTags]; ok {
@@ -2162,12 +2161,10 @@ func (az *Cloud) ensurePIPTagged(service *v1.Service, pip *network.PublicIPAddre
 	if serviceNames != nil {
 		configTags[serviceTagKey] = serviceNames
 	}
-	for k, v := range configTags {
-		if vv, ok := pip.Tags[k]; !ok || !strings.EqualFold(to.String(v), to.String(vv)) {
-			pip.Tags[k] = v
-			changed = true
-		}
-	}
+
+	tags, changed := reconcileTags(pip.Tags, configTags)
+	pip.Tags = tags
+
 	return changed
 }
 
@@ -2728,38 +2725,32 @@ func unbindServiceFromPIP(pip *network.PublicIPAddress, serviceName string) erro
 
 // ensureLoadBalancerTagged ensures every load balancer in the resource group is tagged as configured
 func (az *Cloud) ensureLoadBalancerTagged(lb *network.LoadBalancer) bool {
-	changed := false
 	if az.Tags == "" {
 		return false
 	}
 	tags := parseTags(az.Tags)
 	if lb.Tags == nil {
 		lb.Tags = make(map[string]*string)
 	}
-	for k, v := range tags {
-		if vv, ok := lb.Tags[k]; !ok || !strings.EqualFold(to.String(v), to.String(vv)) {
-			lb.Tags[k] = v
-			changed = true
-		}
-	}
+
+	tags, changed := reconcileTags(lb.Tags, tags)
+	lb.Tags = tags
+
 	return changed
 }
 
 // ensureSecurityGroupTagged ensures the security group is tagged as configured
 func (az *Cloud) ensureSecurityGroupTagged(sg *network.SecurityGroup) bool {
-	changed := false
 	if az.Tags == "" {
 		return false
 	}
 	tags := parseTags(az.Tags)
 	if sg.Tags == nil {
 		sg.Tags = make(map[string]*string)
 	}
-	for k, v := range tags {
-		if vv, ok := sg.Tags[k]; !ok || !strings.EqualFold(to.String(v), to.String(vv)) {
-			sg.Tags[k] = v
-			changed = true
-		}
-	}
+
+	tags, changed := reconcileTags(sg.Tags, tags)
+	sg.Tags = tags
+
 	return changed
 }

staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go

@@ -3784,7 +3784,7 @@ func TestEnsurePIPTagged(t *testing.T) {
 		service := v1.Service{
 			ObjectMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
-					ServiceAnnotationAzurePIPTags: "a=b,c=d,e=,=f,ghi",
+					ServiceAnnotationAzurePIPTags: "A=b,c=d,e=,=f,ghi",
 				},
 			},
 		}

staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go

@@ -541,16 +541,13 @@ func (az *Cloud) ensureRouteTableTagged(rt *network.RouteTable) (map[string]*str
 	if az.Tags == "" {
 		return nil, false
 	}
-	changed := false
 	tags := parseTags(az.Tags)
 	if rt.Tags == nil {
 		rt.Tags = make(map[string]*string)
 	}
-	for k, v := range tags {
-		if vv, ok := rt.Tags[k]; !ok || !strings.EqualFold(to.String(v), to.String(vv)) {
-			rt.Tags[k] = v
-			changed = true
-		}
-	}
+
+	tags, changed := reconcileTags(rt.Tags, tags)
+	rt.Tags = tags
+
 	return rt.Tags, changed
 }

staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go

@@ -138,3 +138,28 @@ func parseTags(tags string) map[string]*string {
 	}
 	return formatted
 }
+
+func findKeyInMapCaseInsensitive(targetMap map[string]*string, key string) (bool, string) {
+	for k := range targetMap {
+		if strings.EqualFold(k, key) {
+			return true, k
+		}
+	}
+
+	return false, ""
+}
+
+func reconcileTags(currentTagsOnResource, newTags map[string]*string) (reconciledTags map[string]*string, changed bool) {
+	for k, v := range newTags {
+		found, key := findKeyInMapCaseInsensitive(currentTagsOnResource, k)
+		if !found {
+			currentTagsOnResource[k] = v
+			changed = true
+		} else if !strings.EqualFold(to.String(v), to.String(currentTagsOnResource[key])) {
+			currentTagsOnResource[key] = v
+			changed = true
+		}
+	}
+
+	return currentTagsOnResource, changed
+}

staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -152,3 +153,63 @@ func TestConvertTagsToMap(t *testing.T) {
 		}
 	}
 }
+
+func TestReconcileTags(t *testing.T) {
+	for _, testCase := range []struct {
+		description                                  string
+		currentTagsOnResource, newTags, expectedTags map[string]*string
+		expectedChanged                              bool
+	}{
+		{
+			description: "reconcileTags should add missing tags and update existing tags",
+			currentTagsOnResource: map[string]*string{
+				"a": to.StringPtr("b"),
+			},
+			newTags: map[string]*string{
+				"a": to.StringPtr("c"),
+				"b": to.StringPtr("d"),
+			},
+			expectedTags: map[string]*string{
+				"a": to.StringPtr("c"),
+				"b": to.StringPtr("d"),
+			},
+			expectedChanged: true,
+		},
+		{
+			description: "reconcileTags should ignore the case of keys when comparing",
+			currentTagsOnResource: map[string]*string{
+				"A": to.StringPtr("b"),
+				"c": to.StringPtr("d"),
+			},
+			newTags: map[string]*string{
+				"a": to.StringPtr("b"),
+				"C": to.StringPtr("d"),
+			},
+			expectedTags: map[string]*string{
+				"A": to.StringPtr("b"),
+				"c": to.StringPtr("d"),
+			},
+		},
+		{
+			description: "reconcileTags should ignore the case of values when comparing",
+			currentTagsOnResource: map[string]*string{
+				"A": to.StringPtr("b"),
+				"c": to.StringPtr("d"),
+			},
+			newTags: map[string]*string{
+				"a": to.StringPtr("B"),
+				"C": to.StringPtr("D"),
+			},
+			expectedTags: map[string]*string{
+				"A": to.StringPtr("b"),
+				"c": to.StringPtr("d"),
+			},
+		},
+	} {
+		t.Run(testCase.description, func(t *testing.T) {
+			tags, changed := reconcileTags(testCase.currentTagsOnResource, testCase.newTags)
+			assert.Equal(t, testCase.expectedChanged, changed)
+			assert.Equal(t, testCase.expectedTags, tags)
+		})
+	}
+}