Merge pull request #1969 from umagnus/tag_delimiter

feat: add tagValueDelimiter parameter

pkg/azurefile/azurefile.go

@@ -185,6 +185,8 @@ const (
 	SnapshotID       = "snapshot_id"
 
 	FSGroupChangeNone = "None"
+	// define tag value delimiter and default is comma
+	tagValueDelimiterField = "tagValueDelimiter"
 )
 
 var (

pkg/azurefile/controllerserver.go

@@ -131,7 +131,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		parameters = make(map[string]string)
 	}
 	var sku, subsID, resourceGroup, location, account, fileShareName, diskName, fsType, secretName string
-	var secretNamespace, pvcNamespace, protocol, customTags, storageEndpointSuffix, networkEndpointType, shareAccessTier, accountAccessTier, rootSquashType string
+	var secretNamespace, pvcNamespace, protocol, customTags, storageEndpointSuffix, networkEndpointType, shareAccessTier, accountAccessTier, rootSquashType, tagValueDelimiter string
 	var createAccount, useDataPlaneAPI, useSeretCache, matchTags, selectRandomMatchingAccount, getLatestAccountKey bool
 	var vnetResourceGroup, vnetName, subnetName, shareNamePrefix, fsGroupChangePolicy string
 	var requireInfraEncryption, disableDeleteRetentionPolicy, enableLFS, isMultichannelEnabled *bool
@@ -271,6 +271,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 				return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid accountQuota %s in storage class, minimum quota: %d", v, minimumAccountQuota))
 			}
 			accountQuota = int32(value)
+		case tagValueDelimiterField:
+			tagValueDelimiter = v
 		default:
 			return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", k))
 		}
@@ -423,7 +425,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		validFileShareName = getValidFileShareName(name)
 	}
 
-	tags, err := ConvertTagsToMap(customTags)
+	tags, err := ConvertTagsToMap(customTags, tagValueDelimiter)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, err.Error())
 	}

pkg/azurefile/utils.go

@@ -31,7 +31,6 @@ import (
 )
 
 const (
-	tagsDelimiter        = ","
 	tagKeyValueDelimiter = "="
 )
 
@@ -185,20 +184,23 @@ func createStorageAccountSecret(account, key string) map[string]string {
 	return secret
 }
 
-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/azurefile/utils_test.go

@@ -388,27 +388,43 @@ func TestConvertTagsToMap(t *testing.T) {
 	tests := []struct {
 		desc          string
 		tags          string
+		tagsDelimiter string
 		expectedError error
 	}{
 		{
 			desc:          "Invalid tag",
-			tags:          "invalid=test=tag",
-			expectedError: errors.New("Tags 'invalid=test=tag' are invalid, the format should like: 'key1=value1,key2=value2'"),
+			tags:          "invalid,test,tag",
+			tagsDelimiter: ",",
+			expectedError: errors.New("Tags 'invalid,test,tag' are invalid, the format should like: 'key1=value1,key2=value2'"),
 		},
 		{
 			desc:          "Invalid key",
 			tags:          "=test",
+			tagsDelimiter: ",",
 			expectedError: errors.New("Tags '=test' are invalid, the format should like: 'key1=value1,key2=value2'"),
 		},
 		{
 			desc:          "Valid tags",
 			tags:          "testTag=testValue",
+			tagsDelimiter: ",",
+			expectedError: nil,
+		},
+		{
+			desc:          "should return success for empty tagsDelimiter",
+			tags:          "key1=value1,key2=value2",
+			tagsDelimiter: "",
+			expectedError: 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: ";",
 			expectedError: nil,
 		},
 	}
 
 	for _, test := range tests {
-		_, err := ConvertTagsToMap(test.tags)
+		_, err := ConvertTagsToMap(test.tags, test.tagsDelimiter)
 		if !reflect.DeepEqual(err, test.expectedError) {
 			t.Errorf("test[%s]: unexpected error: %v, expected error: %v", test.desc, err, test.expectedError)
 		}

test/e2e/testsuites/dynamically_provisioned_tags_tester.go

@@ -68,7 +68,7 @@ func (t *DynamicallyProvisionedAccountWithTags) Run(ctx context.Context, client
 
 		resultTags := account.Tags
 
-		specifiedTags, err := azurefile.ConvertTagsToMap(t.Tags)
+		specifiedTags, err := azurefile.ConvertTagsToMap(t.Tags, "")
 		framework.ExpectNoError(err, fmt.Sprintf("failed to convert tags(%s) %v", t.Tags, err))
 		specifiedTags["k8s-azure-created-by"] = "azure"