Merge pull request #1463 from k8s-infra-cherrypick-robot/cherry-pick-1461-to-release-1.24

[release-1.24] feat: add tag value delimiter

Author: andyzhangx

pkg/blob/blob.go

@@ -153,6 +153,8 @@ const (
 	defaultStorageEndPointSuffix = "core.windows.net"
 
 	FSGroupChangeNone = "None"
+	// define tag value delimiter and default is comma
+	tagValueDelimiterField = "tagValueDelimiter"
 )
 
 var (

pkg/blob/controllerserver.go

@@ -96,7 +96,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	if parameters == nil {
 		parameters = make(map[string]string)
 	}
-	var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace string
+	var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace, tagValueDelimiter string
 	var isHnsEnabled, requireInfraEncryption, enableBlobVersioning, createPrivateEndpoint, enableNfsV3 *bool
 	var vnetResourceGroup, vnetName, subnetName, accessTier, networkEndpointType, storageEndpointSuffix, fsGroupChangePolicy string
 	var matchTags, useDataPlaneAPI, getLatestAccountKey bool
@@ -213,6 +213,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			useDataPlaneAPI = strings.EqualFold(v, trueValue)
 		case fsGroupChangePolicyField:
 			fsGroupChangePolicy = v
+		case tagValueDelimiterField:
+			tagValueDelimiter = v
 		default:
 			return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", k))
 		}
@@ -299,7 +301,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		}
 	}
 
-	tags, err := util.ConvertTagsToMap(customTags)
+	tags, err := util.ConvertTagsToMap(customTags, tagValueDelimiter)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, err.Error())
 	}

pkg/util/util.go

@@ -39,7 +39,6 @@ import (
 const (
 	GiB                  = 1024 * 1024 * 1024
 	TiB                  = 1024 * GiB
-	tagsDelimiter        = ","
 	tagKeyValueDelimiter = "="
 )
 
@@ -157,20 +156,23 @@ func (lm *LockMap) unlockEntry(entry string) {
 	lm.mutexMap[entry].Unlock()
 }
 
-func ConvertTagsToMap(tags string) (map[string]string, error) {
+func ConvertTagsToMap(tags string, tagsDelimiter string) (map[string]string, error) {
 	m := make(map[string]string)
 	if tags == "" {
 		return m, nil
 	}
+	if tagsDelimiter == "" {
+		tagsDelimiter = ","
+	}
 	s := strings.Split(tags, tagsDelimiter)
 	for _, tag := range s {
-		kv := strings.Split(tag, tagKeyValueDelimiter)
+		kv := strings.SplitN(tag, tagKeyValueDelimiter, 2)
 		if len(kv) != 2 {
-			return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", tags)
+			return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1%skey2=value2'", tags, tagsDelimiter)
 		}
 		key := strings.TrimSpace(kv[0])
 		if key == "" {
-			return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", tags)
+			return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1%skey2=value2'", tags, tagsDelimiter)
 		}
 		value := strings.TrimSpace(kv[1])
 		m[key] = value

pkg/util/util_test.go

@@ -176,33 +176,57 @@ func TestMakeDir(t *testing.T) {
 
 func TestConvertTagsToMap(t *testing.T) {
 	tests := []struct {
-		desc        string
-		tags        string
-		expectedOut map[string]string
-		expectedErr error
+		desc          string
+		tags          string
+		tagsDelimiter string
+		expectedOut   map[string]string
+		expectedErr   error
 	}{
 		{
-			desc:        "Improper KeyValuePair",
-			tags:        "foo=bar=gar,lorem=ipsum",
-			expectedOut: nil,
-			expectedErr: fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", "foo=bar=gar,lorem=ipsum"),
+			desc:          "Improper KeyValuePair",
+			tags:          "foo,lorem=ipsum",
+			tagsDelimiter: ",",
+			expectedOut:   nil,
+			expectedErr:   fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", "foo,lorem=ipsum"),
 		},
 		{
-			desc:        "Missing Key",
-			tags:        "=bar,lorem=ipsum",
-			expectedOut: nil,
-			expectedErr: fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", "=bar,lorem=ipsum"),
+			desc:          "Missing Key",
+			tags:          "=bar,lorem=ipsum",
+			tagsDelimiter: ",",
+			expectedOut:   nil,
+			expectedErr:   fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", "=bar,lorem=ipsum"),
 		},
 		{
-			desc:        "Successful Input/Output",
-			tags:        "foo=bar,lorem=ipsum",
-			expectedOut: map[string]string{"foo": "bar", "lorem": "ipsum"},
+			desc:          "Successful Input/Output",
+			tags:          "foo=bar,lorem=ipsum",
+			tagsDelimiter: ",",
+			expectedOut:   map[string]string{"foo": "bar", "lorem": "ipsum"},
+			expectedErr:   nil,
+		},
+		{
+			desc:          "should return success for empty tagsDelimiter",
+			tags:          "key1=value1,key2=value2",
+			tagsDelimiter: "",
+			expectedOut: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expectedErr: nil,
+		},
+		{
+			desc:          "should return success for special tagsDelimiter and tag values containing commas and equal sign",
+			tags:          "key1=aGVsbG8=;key2=value-2, value-3",
+			tagsDelimiter: ";",
+			expectedOut: map[string]string{
+				"key1": "aGVsbG8=",
+				"key2": "value-2, value-3",
+			},
 			expectedErr: nil,
 		},
 	}
 
 	for _, test := range tests {
-		output, err := ConvertTagsToMap(test.tags)
+		output, err := ConvertTagsToMap(test.tags, test.tagsDelimiter)
 		assert.Equal(t, test.expectedOut, output, test.desc)
 		assert.Equal(t, test.expectedErr, err, test.desc)
 	}
@@ -241,7 +265,7 @@ func TestConvertTagsToMap2(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		result, err := ConvertTagsToMap(test.tags)
+		result, err := ConvertTagsToMap(test.tags, "")
 		if test.err {
 			assert.NotNil(t, err)
 		} else {