From 780b9d6a1fb9583fd3bc37cfe4e101e9495c2c64 Mon Sep 17 00:00:00 2001 From: slackfan Date: Thu, 4 Dec 2025 09:25:38 +0100 Subject: [PATCH] Support whitespace in tag keys and values --- docs/parameters.md | 3 +- pkg/driver/controller_test.go | 89 ++++++++++++++++++++++++++++++++++- pkg/driver/driver.go | 79 +++++++++++++++++++++---------- 3 files changed, 142 insertions(+), 29 deletions(-) diff --git a/docs/parameters.md b/docs/parameters.md index b1ec21b6d..e730f0e87 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -89,5 +89,4 @@ Enabling the vol-metrics-opt-in parameter activates the gathering of inode and d |-----------------------------|--------|---------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | delete-access-point-root-dir| | false | true | Opt in to delete access point root directory by DeleteVolume. By default, DeleteVolume will delete the access point behind Persistent Volume and deleting access point will not delete the access point root directory or its contents. | | adaptive-retry-mode | | true | true | Opt out to use standard sdk retry mode for EFS API calls. By default, Driver will use adaptive mode for the sdk retry configuration which heavily rate limits EFS API requests to reduce throttling if throttling is observed. | -| tags | | | true | Space separated key:value pairs which will be added as tags for Amazon EFS resources. For example, '--tags=name:efs-tag-test date:Jan24'. To include a ':' in the tag name or value, use \ as an escape character, for example '--tags="tag\:name:tag\:value" | - +| tags | | | true | Space separated key:value pairs which will be added as tags for Amazon EFS resources. For example, '--tags=name:efs-tag-test date:Jan24'. To include a ':' or ' ' in the tag name or value, use \ as an escape character, for example '--tags="tag\:name:tag\:value" | diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 3bd54af17..2affec675 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/rand" + "reflect" "regexp" "strconv" "sync" @@ -802,7 +803,7 @@ func TestCreateVolume(t *testing.T) { } if driver.tags["cluster"] != "efs" || driver.tags["tag2:name2"] != "tag2:val2" { - t.Fatalf("Incorrect tags") + t.Fatalf("Incorrect tags %v", driver.tags) } req := &csi.CreateVolumeRequest{ @@ -862,7 +863,7 @@ func TestCreateVolume(t *testing.T) { cloud: mockCloud, gidAllocator: NewGidAllocator(), lockManager: NewLockManagerMap(), - tags: parseTagsFromStr("cluster-efs"), + tags: parseTagsFromStr("cluster-efs:value1:value2"), } req := &csi.CreateVolumeRequest{ @@ -4844,6 +4845,90 @@ func TestControllerGetCapabilities(t *testing.T) { } } +func TestTaggingCapabilitites(t *testing.T) { + testCases := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: complex split key value colon with backslash", + testFunc: func(t *testing.T) { + given := "aa\\:bb cc\\\\dd:ee\\:ff gg\\\\hh" + expected := []string{"aa:bb cc\\dd", "ee:ff gg\\hh"} + result := splitToList(given, byte(':')) + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + { + name: "Success: complex split key only colon with backslash", + testFunc: func(t *testing.T) { + given := "aa\\:bb cc\\\\dd\\\\" + expected := []string{"aa:bb cc\\dd\\"} + result := splitToList(given, byte(':')) + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + { + name: "Success: simple split whitespace", + testFunc: func(t *testing.T) { + given := "aa:bb cc:dd ee:ff" + expected := []string{"aa:bb", "cc:dd", "ee:ff"} + result := splitToList(given, byte(' ')) + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + { + name: "Success: simple single tag", + testFunc: func(t *testing.T) { + expected := map[string]string{"happy": "case"} + result := parseTagsFromStr("happy:case") + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + { + name: "Success: simple multiple tags", + testFunc: func(t *testing.T) { + expected := map[string]string{"firstkey": "firstvalue", "secondkey": "secondvalue"} + result := parseTagsFromStr("firstkey:firstvalue secondkey:secondvalue") + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + { + name: "Success: complex key escaping", + testFunc: func(t *testing.T) { + expected := map[string]string{"first:key": "first value", "second key": "second:value"} + result := parseTagsFromStr("first\\:key:first\\ value second\\ key:second\\:value") + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + { + name: "Success: complex key escaping maintain backslash", + testFunc: func(t *testing.T) { + expected := map[string]string{"first:key": "first\\value", "second key": "second:value"} + result := parseTagsFromStr("first\\:key:first\\\\value second\\ key:second\\:value") + if !reflect.DeepEqual(result, expected) { + t.Fatalf("Incorrect tags: %v vs. %v", result, expected) + } + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, tc.testFunc) + } +} + // Helper function to create mock client that returns error func createMockClientWithError(errorMsg string) kubernetes.Interface { return fake.NewSimpleClientset() diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 3763f9394..3c1f5567f 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -152,6 +152,50 @@ func (d *Driver) Run() error { return d.srv.Serve(listener) } +func splitToList(tagsStr string, splitter byte) []string { + defer func() { + if r := recover(); r != nil { + klog.Errorf("Failed to parse input string: %v", tagsStr) + } + }() + + l := []string{} + if tagsStr == "" { + klog.Infof("Did not find any input tags.") + return l + } + var tagBuilder strings.Builder + var jumper int = 0 + for index, runeValue := range tagsStr { + if jumper > index { + continue + } + jumper++ + if byte(runeValue) == splitter { + l = append(l, tagBuilder.String()) + tagBuilder.Reset() + continue + } + + // Handle escape character + if runeValue == '\\' && tagsStr[index+1] == byte('\\') { + tagBuilder.WriteRune('\\') + jumper++ + continue + } + + if runeValue == '\\' && tagsStr[index+1] == splitter { + tagBuilder.WriteByte(splitter) + jumper++ + continue + } + + tagBuilder.WriteRune(runeValue) + } + l = append(l, tagBuilder.String()) + return l +} + func parseTagsFromStr(tagStr string) map[string]string { defer func() { if r := recover(); r != nil { @@ -159,37 +203,22 @@ func parseTagsFromStr(tagStr string) map[string]string { } }() - m := make(map[string]string) + m := map[string]string{} if tagStr == "" { klog.Infof("Did not find any input tags.") return m } - tagsSplit := strings.Split(tagStr, " ") + tagsSplit := splitToList(tagStr, byte(' ')) for _, currTag := range tagsSplit { - var nameBuilder strings.Builder - var valBuilder strings.Builder - var currBuilder *strings.Builder = &nameBuilder - - for i := 0; i < len(currTag); i++ { - if currTag[i] == ':' { - if currBuilder == &valBuilder { - break - } else { - currBuilder = &valBuilder - continue - } - } - - // Handle escape character - if currTag[i] == byte('\\') && currTag[i+1] == byte(':') { - currBuilder.WriteRune(':') - i++ // Skip an extra character - continue - } - - currBuilder.WriteByte(currTag[i]) + var tagList = splitToList(currTag, byte(':')) + switch len(tagList) { + case 1: + m[tagList[0]] = "" + case 2: + m[tagList[0]] = tagList[1] + default: + klog.Errorf("Failed to parse input tag: %v", tagList) } - m[nameBuilder.String()] = valBuilder.String() } return m }