Skip to content

Commit

Permalink
Fix : Enable system tag support prefix match (#8151)
Browse files Browse the repository at this point in the history
  • Loading branch information
nilo19 authored Feb 5, 2025
1 parent 3489717 commit 37d6c85
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 1 deletion.
1 change: 1 addition & 0 deletions pkg/provider/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ type Config struct {
// SystemTags determines the tag keys managed by cloud provider. If it is not set, no tags would be deleted if
// the `Tags` is changed. However, the old tags would be deleted if they are neither included in `Tags` nor
// in `SystemTags` after the update of `Tags`.
// SystemTags now support prefix match, which means that if a key in `SystemTags` is a prefix of a key in `Tags`, that tag will not be deleted
SystemTags string `json:"systemTags,omitempty" yaml:"systemTags,omitempty"`
// Sku of Load Balancer and Public IP. Candidate values are: basic and standard.
// If not set, it will be default to basic.
Expand Down
19 changes: 18 additions & 1 deletion pkg/provider/azure_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,23 @@ func findKeyInMapCaseInsensitive(targetMap map[string]*string, key string) (bool
return false, ""
}

// This function extends the functionality of findKeyInMapCaseInsensitive by supporting both
// exact case-insensitive key matching and prefix-based key matching in the given map.
// 1. If the key is found in the map (case-insensitively), the function returns true and the matching key in the map.
// 2. If the key's prefix is found in the map (case-insensitively), the function also returns true and the matching key in the map.
// This function is designed to enable systemTags to support prefix-based tag keys,
// allowing more flexible and efficient tag key matching.
func findKeyInMapWithPrefix(targetMap map[string]*string, key string) (bool, string) {
for k := range targetMap {
// use prefix-based key matching
// use case-insensitive comparison
if strings.HasPrefix(strings.ToLower(key), strings.ToLower(k)) {
return true, k
}
}
return false, ""
}

func (az *Cloud) reconcileTags(currentTagsOnResource, newTags map[string]*string) (reconciledTags map[string]*string, changed bool) {
var systemTags []string
systemTagsMap := make(map[string]*string)
Expand Down Expand Up @@ -189,7 +206,7 @@ func (az *Cloud) reconcileTags(currentTagsOnResource, newTags map[string]*string
if len(systemTagsMap) > 0 {
for k := range currentTagsOnResource {
if _, ok := newTags[k]; !ok {
if found, _ := findKeyInMapCaseInsensitive(systemTagsMap, k); !found {
if found, _ := findKeyInMapWithPrefix(systemTagsMap, k); !found {
klog.V(2).Infof("reconcileTags: delete tag %s: %s", k, pointer.StringDeref(currentTagsOnResource[k], ""))
delete(currentTagsOnResource, k)
changed = true
Expand Down
63 changes: 63 additions & 0 deletions pkg/provider/azure_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,69 @@ func TestReconcileTags(t *testing.T) {
},
expectedChanged: true,
},
{
description: "reconcileTags should support prefix matching in systemTags",
currentTagsOnResource: map[string]*string{
"prefix-a": pointer.String("b"),
"c": pointer.String("d"),
},
systemTags: "prefix",
expectedTags: map[string]*string{
"prefix-a": pointer.String("b"),
},
expectedChanged: true,
},
{
description: "reconcileTags should support prefix matching in systemTags case insensitive",
currentTagsOnResource: map[string]*string{
"prefix-a": pointer.String("b"),
"c": pointer.String("d"),
},
systemTags: "PrEFiX",
expectedTags: map[string]*string{
"prefix-a": pointer.String("b"),
},
expectedChanged: true,
},
{
description: "reconcileTags should support prefix matching in systemTags with multiple prefixes",
currentTagsOnResource: map[string]*string{
"prefix-a": pointer.String("b"),
"sys-b": pointer.String("c"),
},
systemTags: "prefix, sys",
expectedTags: map[string]*string{
"prefix-a": pointer.String("b"),
"sys-b": pointer.String("c"),
},
expectedChanged: false,
},
{
description: "reconcileTags should work with full length aks managed cluster tags",
currentTagsOnResource: map[string]*string{
"aks-managed-cluster-name": pointer.String("test-name"),
"aks-managed-cluster-rg": pointer.String("test-rg"),
},
systemTags: "aks-managed-cluster-name, aks-managed-cluster-rg",
expectedTags: map[string]*string{
"aks-managed-cluster-name": pointer.String("test-name"),
"aks-managed-cluster-rg": pointer.String("test-rg"),
},
expectedChanged: false,
},
{
description: "real case test for systemTags",
currentTagsOnResource: map[string]*string{
"aks-managed-cluster-name": pointer.String("test-name"),
"aks-managed-cluster-rg": pointer.String("test-rg"),
},
systemTags: "aks-managed",
expectedTags: map[string]*string{
"aks-managed-cluster-name": pointer.String("test-name"),
"aks-managed-cluster-rg": pointer.String("test-rg"),
},
expectedChanged: false,
},
} {
t.Run(testCase.description, func(t *testing.T) {
cloud := &Cloud{}
Expand Down

0 comments on commit 37d6c85

Please sign in to comment.