diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go index 0674952ec60..42209d45805 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go @@ -2132,7 +2132,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 { @@ -2155,12 +2154,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 } @@ -2721,7 +2718,6 @@ 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 } @@ -2729,18 +2725,15 @@ func (az *Cloud) ensureLoadBalancerTagged(lb *network.LoadBalancer) bool { 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 } @@ -2748,11 +2741,9 @@ func (az *Cloud) ensureSecurityGroupTagged(sg *network.SecurityGroup) bool { 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 } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go index 1406179a8b2..2e67909ddf1 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go @@ -3815,7 +3815,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", }, }, } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go index a438d9864b7..a03083f8aca 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go @@ -542,16 +542,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 } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go index 94be01a147f..777c73877cb 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go @@ -139,3 +139,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 +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go index 3010ff1f907..ca9e29f12d8 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/Azure/go-autorest/autorest/to" "github.com/stretchr/testify/assert" ) @@ -153,3 +154,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) + }) + } +}