Merge pull request #104593 from nilo19/fix/tag-case

fix: ignore the case when updating tags
This commit is contained in:
Kubernetes Prow Robot 2021-08-31 04:59:37 -07:00 committed by GitHub
commit 9dba11d971
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 103 additions and 29 deletions

View File

@ -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
}

View File

@ -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",
},
},
}

View File

@ -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
}

View File

@ -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
}

View File

@ -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)
})
}
}