Skip to content

Commit 4a328c4

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1716 from slackfan/tags_with_whitespace
Support whitespace in tag keys and values
2 parents b7ecd93 + 780b9d6 commit 4a328c4

File tree

3 files changed

+142
-29
lines changed

3 files changed

+142
-29
lines changed

‎docs/parameters.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,5 +89,4 @@ Enabling the vol-metrics-opt-in parameter activates the gathering of inode and d
8989
|-----------------------------|--------|---------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
9090
| 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. |
9191
| 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. |
92-
| 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" |
93-
92+
| 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" |

‎pkg/driver/controller_test.go‎

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"math/rand"
8+
"reflect"
89
"regexp"
910
"strconv"
1011
"sync"
@@ -802,7 +803,7 @@ func TestCreateVolume(t *testing.T) {
802803
}
803804

804805
if driver.tags["cluster"] != "efs" || driver.tags["tag2:name2"] != "tag2:val2" {
805-
t.Fatalf("Incorrect tags")
806+
t.Fatalf("Incorrect tags %v", driver.tags)
806807
}
807808

808809
req := &csi.CreateVolumeRequest{
@@ -862,7 +863,7 @@ func TestCreateVolume(t *testing.T) {
862863
cloud: mockCloud,
863864
gidAllocator: NewGidAllocator(),
864865
lockManager: NewLockManagerMap(),
865-
tags: parseTagsFromStr("cluster-efs"),
866+
tags: parseTagsFromStr("cluster-efs:value1:value2"),
866867
}
867868

868869
req := &csi.CreateVolumeRequest{
@@ -4844,6 +4845,90 @@ func TestControllerGetCapabilities(t *testing.T) {
48444845
}
48454846
}
48464847

4848+
func TestTaggingCapabilitites(t *testing.T) {
4849+
testCases := []struct {
4850+
name string
4851+
testFunc func(t *testing.T)
4852+
}{
4853+
{
4854+
name: "Success: complex split key value colon with backslash",
4855+
testFunc: func(t *testing.T) {
4856+
given := "aa\\:bb cc\\\\dd:ee\\:ff gg\\\\hh"
4857+
expected := []string{"aa:bb cc\\dd", "ee:ff gg\\hh"}
4858+
result := splitToList(given, byte(':'))
4859+
if !reflect.DeepEqual(result, expected) {
4860+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4861+
}
4862+
},
4863+
},
4864+
{
4865+
name: "Success: complex split key only colon with backslash",
4866+
testFunc: func(t *testing.T) {
4867+
given := "aa\\:bb cc\\\\dd\\\\"
4868+
expected := []string{"aa:bb cc\\dd\\"}
4869+
result := splitToList(given, byte(':'))
4870+
if !reflect.DeepEqual(result, expected) {
4871+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4872+
}
4873+
},
4874+
},
4875+
{
4876+
name: "Success: simple split whitespace",
4877+
testFunc: func(t *testing.T) {
4878+
given := "aa:bb cc:dd ee:ff"
4879+
expected := []string{"aa:bb", "cc:dd", "ee:ff"}
4880+
result := splitToList(given, byte(' '))
4881+
if !reflect.DeepEqual(result, expected) {
4882+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4883+
}
4884+
},
4885+
},
4886+
{
4887+
name: "Success: simple single tag",
4888+
testFunc: func(t *testing.T) {
4889+
expected := map[string]string{"happy": "case"}
4890+
result := parseTagsFromStr("happy:case")
4891+
if !reflect.DeepEqual(result, expected) {
4892+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4893+
}
4894+
},
4895+
},
4896+
{
4897+
name: "Success: simple multiple tags",
4898+
testFunc: func(t *testing.T) {
4899+
expected := map[string]string{"firstkey": "firstvalue", "secondkey": "secondvalue"}
4900+
result := parseTagsFromStr("firstkey:firstvalue secondkey:secondvalue")
4901+
if !reflect.DeepEqual(result, expected) {
4902+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4903+
}
4904+
},
4905+
},
4906+
{
4907+
name: "Success: complex key escaping",
4908+
testFunc: func(t *testing.T) {
4909+
expected := map[string]string{"first:key": "first value", "second key": "second:value"}
4910+
result := parseTagsFromStr("first\\:key:first\\ value second\\ key:second\\:value")
4911+
if !reflect.DeepEqual(result, expected) {
4912+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4913+
}
4914+
},
4915+
},
4916+
{
4917+
name: "Success: complex key escaping maintain backslash",
4918+
testFunc: func(t *testing.T) {
4919+
expected := map[string]string{"first:key": "first\\value", "second key": "second:value"}
4920+
result := parseTagsFromStr("first\\:key:first\\\\value second\\ key:second\\:value")
4921+
if !reflect.DeepEqual(result, expected) {
4922+
t.Fatalf("Incorrect tags: %v vs. %v", result, expected)
4923+
}
4924+
},
4925+
},
4926+
}
4927+
for _, tc := range testCases {
4928+
t.Run(tc.name, tc.testFunc)
4929+
}
4930+
}
4931+
48474932
// Helper function to create mock client that returns error
48484933
func createMockClientWithError(errorMsg string) kubernetes.Interface {
48494934
return fake.NewSimpleClientset()

‎pkg/driver/driver.go‎

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -152,44 +152,73 @@ func (d *Driver) Run() error {
152152
return d.srv.Serve(listener)
153153
}
154154

155+
func splitToList(tagsStr string, splitter byte) []string {
156+
defer func() {
157+
if r := recover(); r != nil {
158+
klog.Errorf("Failed to parse input string: %v", tagsStr)
159+
}
160+
}()
161+
162+
l := []string{}
163+
if tagsStr == "" {
164+
klog.Infof("Did not find any input tags.")
165+
return l
166+
}
167+
var tagBuilder strings.Builder
168+
var jumper int = 0
169+
for index, runeValue := range tagsStr {
170+
if jumper > index {
171+
continue
172+
}
173+
jumper++
174+
if byte(runeValue) == splitter {
175+
l = append(l, tagBuilder.String())
176+
tagBuilder.Reset()
177+
continue
178+
}
179+
180+
// Handle escape character
181+
if runeValue == '\\' && tagsStr[index+1] == byte('\\') {
182+
tagBuilder.WriteRune('\\')
183+
jumper++
184+
continue
185+
}
186+
187+
if runeValue == '\\' && tagsStr[index+1] == splitter {
188+
tagBuilder.WriteByte(splitter)
189+
jumper++
190+
continue
191+
}
192+
193+
tagBuilder.WriteRune(runeValue)
194+
}
195+
l = append(l, tagBuilder.String())
196+
return l
197+
}
198+
155199
func parseTagsFromStr(tagStr string) map[string]string {
156200
defer func() {
157201
if r := recover(); r != nil {
158202
klog.Errorf("Failed to parse input tag string: %v", tagStr)
159203
}
160204
}()
161205

162-
m := make(map[string]string)
206+
m := map[string]string{}
163207
if tagStr == "" {
164208
klog.Infof("Did not find any input tags.")
165209
return m
166210
}
167-
tagsSplit := strings.Split(tagStr, " ")
211+
tagsSplit := splitToList(tagStr, byte(' '))
168212
for _, currTag := range tagsSplit {
169-
var nameBuilder strings.Builder
170-
var valBuilder strings.Builder
171-
var currBuilder *strings.Builder = &nameBuilder
172-
173-
for i := 0; i < len(currTag); i++ {
174-
if currTag[i] == ':' {
175-
if currBuilder == &valBuilder {
176-
break
177-
} else {
178-
currBuilder = &valBuilder
179-
continue
180-
}
181-
}
182-
183-
// Handle escape character
184-
if currTag[i] == byte('\\') && currTag[i+1] == byte(':') {
185-
currBuilder.WriteRune(':')
186-
i++ // Skip an extra character
187-
continue
188-
}
189-
190-
currBuilder.WriteByte(currTag[i])
213+
var tagList = splitToList(currTag, byte(':'))
214+
switch len(tagList) {
215+
case 1:
216+
m[tagList[0]] = ""
217+
case 2:
218+
m[tagList[0]] = tagList[1]
219+
default:
220+
klog.Errorf("Failed to parse input tag: %v", tagList)
191221
}
192-
m[nameBuilder.String()] = valBuilder.String()
193222
}
194223
return m
195224
}

0 commit comments

Comments
 (0)