Support whitespace in tag keys and values

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" |

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()

pkg/driver/driver.go

@@ -152,44 +152,73 @@ 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 {
 			klog.Errorf("Failed to parse input tag string: %v", tagStr)
 		}
 	}()
 
-	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
 }