Ensuring lower case for snapshotDir attribute of volume

cli/cmd/update_volume.go

@@ -47,9 +47,11 @@ var updateVolumeCmd = &cobra.Command{
 		}
 
 		if OperatingMode == ModeTunnel {
+			snapDirBool, _ := strconv.ParseBool(snapDir)
+
 			command := []string{
 				"update", "volume",
-				"--snapshot-dir", snapDir,
+				"--snapshot-dir", strconv.FormatBool(snapDirBool),
 				"--pool-level", poolLevelVal,
 			}
 			out, err := TunnelCommand(append(command, args...))
@@ -94,10 +96,11 @@ func validateCmd(args []string, snapshotDir, poolLevel string) error {
 func updateVolume(volumeName, snapDirValue, poolLevelVal string) error {
 	url := BaseURL() + "/volume/" + volumeName
 
+	snapDirBool, _ := strconv.ParseBool(snapDirValue)
 	poolLevelBool, _ := strconv.ParseBool(poolLevelVal)
 
 	request := utils.VolumeUpdateInfo{
-		SnapshotDirectory: snapDirValue,
+		SnapshotDirectory: strconv.FormatBool(snapDirBool),
 		PoolLevel:         poolLevelBool,
 	}
 

frontend/common/volumes.go

@@ -126,6 +126,17 @@ func GetVolumeConfig(
 	Logc(ctx).Trace(">>>> GetVolumeConfig")
 	defer Logc(ctx).Trace("<<<< GetVolumeConfig")
 
+	// If snapshotDir is provided, ensure it is lower case
+	snapshotDir := utils.GetV(opts, "snapshotDir", "")
+	if snapshotDir != "" {
+		snapDirFormatted, err := utils.GetFormattedBool(snapshotDir)
+		if err != nil {
+			Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir: %v.", snapshotDir)
+			return nil, err
+		}
+		snapshotDir = snapDirFormatted
+	}
+
 	cfg := &storage.VolumeConfig{
 		Name:                name,
 		Size:                fmt.Sprintf("%d", sizeBytes),
@@ -138,7 +149,7 @@ func GetVolumeConfig(
 		SplitOnClone:        utils.GetV(opts, "splitOnClone", ""),
 		SnapshotPolicy:      utils.GetV(opts, "snapshotPolicy", ""),
 		SnapshotReserve:     utils.GetV(opts, "snapshotReserve", ""),
-		SnapshotDir:         utils.GetV(opts, "snapshotDir", ""),
+		SnapshotDir:         snapshotDir,
 		ExportPolicy:        utils.GetV(opts, "exportPolicy", ""),
 		UnixPermissions:     utils.GetV(opts, "unixPermissions", ""),
 		BlockSize:           utils.GetV(opts, "blocksize", ""),

frontend/common/volumes_test.go

@@ -126,12 +126,53 @@ func TestGetVolumeConfig(t *testing.T) {
 		VolumeMode:          config.VolumeMode(dummyString),
 		RequisiteTopologies: dummyMap,
 		PreferredTopologies: dummyMap,
+		SnapshotDir:         "",
 	}
 
+	// Case: volumeConfig created with empty parameters
 	vol, err := GetVolumeConfig(context.Background(), dummyString, dummyString, 100, map[string]string{},
 		config.Protocol(dummyString), config.AccessMode(dummyString), config.VolumeMode(dummyString),
 		dummyMap, dummyMap)
 
 	assert.Nil(t, err, "Error while creating VolumeConfig object")
 	assert.Equal(t, expected, vol, "VolumeConfig object does not match")
+
+	// Case: volumeConfig created with valid snapshotDir value
+	expected = &storage.VolumeConfig{
+		Name:                dummyString,
+		Size:                "100",
+		StorageClass:        dummyString,
+		Protocol:            config.Protocol(dummyString),
+		AccessMode:          config.AccessMode(dummyString),
+		VolumeMode:          config.VolumeMode(dummyString),
+		RequisiteTopologies: dummyMap,
+		PreferredTopologies: dummyMap,
+		SnapshotDir:         "true",
+	}
+
+	vol, err = GetVolumeConfig(context.Background(), dummyString, dummyString, 100, map[string]string{"snapshotDir": "True"},
+		config.Protocol(dummyString), config.AccessMode(dummyString), config.VolumeMode(dummyString),
+		dummyMap, dummyMap)
+
+	assert.Nil(t, err, "Error while creating VolumeConfig object")
+	assert.Equal(t, expected, vol, "VolumeConfig object does not match")
+
+	// Case: volumeConfig created with invalid snapshotDir value
+	expected = &storage.VolumeConfig{
+		Name:                dummyString,
+		Size:                "100",
+		StorageClass:        dummyString,
+		Protocol:            config.Protocol(dummyString),
+		AccessMode:          config.AccessMode(dummyString),
+		VolumeMode:          config.VolumeMode(dummyString),
+		RequisiteTopologies: dummyMap,
+		PreferredTopologies: dummyMap,
+	}
+
+	vol, err = GetVolumeConfig(context.Background(), dummyString, dummyString, 100, map[string]string{"snapshotDir": "TrUe"},
+		config.Protocol(dummyString), config.AccessMode(dummyString), config.VolumeMode(dummyString),
+		dummyMap, dummyMap)
+
+	assert.NotNil(t, err, "No error while creating VolumeConfig object with invalid snapshotDir")
+	assert.Nil(t, vol, "VolumeConfig object is not nil in case of error")
 }

frontend/csi/controller_helpers/kubernetes/helper.go

@@ -556,6 +556,16 @@ func getVolumeConfig(
 		volumeMode = &volumeModeVal
 	}
 
+	// If snapshotDir annotation is provided, ensure it is lower case
+	snapshotDirAnn := getAnnotation(annotations, AnnSnapshotDir)
+	if snapshotDirAnn != "" {
+		snapDirFormatted, err := utils.GetFormattedBool(snapshotDirAnn)
+		if err != nil {
+			Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir annotation: %v.", snapshotDirAnn)
+		}
+		snapshotDirAnn = snapDirFormatted
+	}
+
 	if getAnnotation(annotations, AnnReadOnlyClone) == "" {
 		annotations[AnnReadOnlyClone] = "false"
 	}
@@ -583,7 +593,7 @@ func getVolumeConfig(
 		Protocol:            config.Protocol(getAnnotation(annotations, AnnProtocol)),
 		SnapshotPolicy:      getAnnotation(annotations, AnnSnapshotPolicy),
 		SnapshotReserve:     getAnnotation(annotations, AnnSnapshotReserve),
-		SnapshotDir:         getAnnotation(annotations, AnnSnapshotDir),
+		SnapshotDir:         snapshotDirAnn,
 		ExportPolicy:        getAnnotation(annotations, AnnExportPolicy),
 		UnixPermissions:     getAnnotation(annotations, AnnUnixPermissions),
 		StorageClass:        storageClass.Name,

frontend/csi/controller_helpers/plain/plugin_test.go

@@ -86,25 +86,36 @@ func TestGetVolumeConfig(t *testing.T) {
 	}
 
 	type volumeConfigTest struct {
-		volumeName         string
-		accessMode         []config.AccessMode
-		parameters         map[string]string
-		expected           bool
-		expectedAccessMode config.AccessMode
+		volumeName          string
+		accessMode          []config.AccessMode
+		parameters          map[string]string
+		expected            bool
+		expectedAccessMode  config.AccessMode
+		expectedSnapshotDir string
 	}
 
 	accessMode1 := []config.AccessMode{config.ReadWriteOnce}
 	accessMode2 := []config.AccessMode{config.ModeAny, config.ReadWriteOnce, config.ReadOnlyMany}
 	accessMode3 := []config.AccessMode{config.ModeAny}
 	tests := map[string]volumeConfigTest{
 		"volumeConfigWithOneAccessMode": {
-			"volume1", accessMode1, make(map[string]string), false, config.ReadWriteOnce,
+			"volume1", accessMode1,
+			map[string]string{"snapshotDir": "TRUE"},
+			false, config.ReadWriteOnce, "true",
 		},
 		"volumeConfigWithMultipleAccessMode": {
-			"volume1", accessMode2, make(map[string]string), false, config.ReadWriteMany,
+			"volume1", accessMode2,
+			map[string]string{"snapshotDir": "False"},
+			false, config.ReadWriteMany, "false",
 		},
 		"volumeConfigParameterNil": {
-			"volume1", accessMode3, nil, true, config.ModeAny,
+			"volume1", accessMode3, nil,
+			true, config.ModeAny, "",
+		},
+		"volumeConfigInvalidSnapshotDir": {
+			"volume1", accessMode3,
+			map[string]string{"snapshotDir": "FaLsE"},
+			true, config.ModeAny, "",
 		},
 	}
 
@@ -129,6 +140,7 @@ func TestGetVolumeConfig(t *testing.T) {
 				PreferredTopologies: dummyMap,
 				FileSystem:          "fsType",
 				AccessMode:          test.expectedAccessMode,
+				SnapshotDir:         test.expectedSnapshotDir,
 			}
 			result, err := plugin.GetVolumeConfig(ctx, test.volumeName, 100, test.parameters,
 				config.Protocol(config.File), test.accessMode, config.VolumeMode(config.Filesystem), "fsType",

storage_drivers/azure/azure_anf.go

@@ -272,9 +272,16 @@ func (d *NASStorageDriver) populateConfigurationDefaults(
 		}
 	}
 
-	if config.SnapshotDir == "" {
-		config.SnapshotDir = defaultSnapshotDir
+	// If snapshotDir is provided, ensure it is lower case
+	snapDir := defaultSnapshotDir
+	if config.SnapshotDir != "" {
+		snapDirFormatted, err := utils.GetFormattedBool(config.SnapshotDir)
+		if err != nil {
+			Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir: %v.", config.SnapshotDir)
+		}
+		snapDir = snapDirFormatted
 	}
+	config.SnapshotDir = snapDir
 
 	if config.LimitVolumeSize == "" {
 		config.LimitVolumeSize = defaultLimitVolumeSize
@@ -310,6 +317,15 @@ func (d *NASStorageDriver) populateConfigurationDefaults(
 func (d *NASStorageDriver) initializeStoragePools(ctx context.Context) {
 	d.pools = make(map[string]storage.Pool)
 
+	// If snapshotDir is provided, ensure it is lower case
+	if d.Config.SnapshotDir != "" {
+		snapDirFormatted, err := utils.GetFormattedBool(d.Config.SnapshotDir)
+		if err != nil {
+			Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir: %v.", d.Config.SnapshotDir)
+		}
+		d.Config.SnapshotDir = snapDirFormatted
+	}
+
 	if len(d.Config.Storage) == 0 {
 
 		Logc(ctx).Debug("No vpools defined, reporting single pool.")
@@ -402,7 +418,11 @@ func (d *NASStorageDriver) initializeStoragePools(ctx context.Context) {
 
 			snapshotDir := d.Config.SnapshotDir
 			if vpool.SnapshotDir != "" {
-				snapshotDir = vpool.SnapshotDir
+				snapDirFormatted, err := utils.GetFormattedBool(vpool.SnapshotDir)
+				if err != nil {
+					Logc(ctx).WithError(err).Errorf("Invalid boolean value for vpool's snapshotDir: %v.", vpool.SnapshotDir)
+				}
+				snapshotDir = snapDirFormatted
 			}
 
 			exportRule := d.Config.ExportRule
@@ -631,7 +651,7 @@ func (d *NASStorageDriver) validate(ctx context.Context) error {
 		if pool.InternalAttributes()[SnapshotDir] != "" {
 			_, err := strconv.ParseBool(pool.InternalAttributes()[SnapshotDir])
 			if err != nil {
-				return fmt.Errorf("invalid value for snapshotDir in pool %s; %v", poolName, err)
+				return fmt.Errorf("invalid boolean value for snapshotDir in pool %s; %v", poolName, err)
 			}
 		}
 

storage_drivers/azure/azure_anf_test.go

@@ -758,6 +758,36 @@ func TestPopulateConfigurationDefaults_AllSet(t *testing.T) {
 	assert.Equal(t, "1.1.1.1/32", driver.Config.ExportRule)
 }
 
+func TestPopulateConfigurationDefaults_SnapshotDir(t *testing.T) {
+	config := &drivers.AzureNASStorageDriverConfig{
+		CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{
+			DriverContext:   tridentconfig.ContextCSI,
+			DebugTraceFlags: debugTraceFlags,
+		},
+	}
+
+	_, driver := newMockANFDriver(t)
+	driver.Config = *config
+
+	tests := []struct {
+		name                string
+		inputSnapshotDir    string
+		expectedSnapshotDir string
+	}{
+		{"Default snapshotDir", "", "false"},
+		{"Uppercase snapshotDir", "TRUE", "true"},
+		{"Invalid snapshotDir", "TrUE", "TrUE"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			driver.Config.SnapshotDir = test.inputSnapshotDir
+			driver.populateConfigurationDefaults(ctx, &driver.Config)
+			assert.Equal(t, test.expectedSnapshotDir, driver.Config.SnapshotDir)
+		})
+	}
+}
+
 func TestInitializeStoragePools_NoVirtualPools(t *testing.T) {
 	supportedTopologies := []map[string]string{
 		{"topology.kubernetes.io/region": "europe-west1", "topology.kubernetes.io/zone": "us-east-1c"},
@@ -777,7 +807,7 @@ func TestInitializeStoragePools_NoVirtualPools(t *testing.T) {
 					Size: "1234567890",
 				},
 				UnixPermissions: "0700",
-				SnapshotDir:     "true",
+				SnapshotDir:     "TRUE",
 				ExportRule:      "1.1.1.1/32",
 			},
 			VirtualNetwork:      "VN1",
@@ -883,7 +913,7 @@ func TestInitializeStoragePools_VirtualPools(t *testing.T) {
 			{
 				AzureNASStorageDriverConfigDefaults: drivers.AzureNASStorageDriverConfigDefaults{
 					UnixPermissions: "0770",
-					SnapshotDir:     "false",
+					SnapshotDir:     "FALSE",
 				},
 				VirtualNetwork:      "VN1",
 				Subnet:              "SN2",
@@ -962,6 +992,48 @@ func TestInitializeStoragePools_VirtualPools(t *testing.T) {
 	assert.Equal(t, expectedPools, driver.pools, "pools do not match")
 }
 
+func TestInitializeStoragePools_SnapshotDir(t *testing.T) {
+	// Test with multiple vpools each with different snapshotDir value
+	config := &drivers.AzureNASStorageDriverConfig{
+		CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{
+			BackendName:     "myANFBackend",
+			DriverContext:   tridentconfig.ContextCSI,
+			DebugTraceFlags: debugTraceFlags,
+			LimitVolumeSize: "123456789000",
+		},
+		Storage: []drivers.AzureNASStorageDriverPool{
+			{
+				Region: "us_east_1",
+				Zone:   "us_east_1a",
+			},
+			{
+				AzureNASStorageDriverConfigDefaults: drivers.AzureNASStorageDriverConfigDefaults{SnapshotDir: "False"},
+			},
+			{
+				AzureNASStorageDriverConfigDefaults: drivers.AzureNASStorageDriverConfigDefaults{SnapshotDir: "Invalid"},
+			},
+		},
+	}
+
+	_, driver := newMockANFDriver(t)
+	driver.Config = *config
+	driver.Config.SnapshotDir = "TRUE"
+
+	driver.initializeStoragePools(ctx)
+
+	// Case1: Ensure snapshotDir is picked from driver config and is lower case
+	virtualPool1 := driver.pools["myANFBackend_pool_0"]
+	assert.Equal(t, "true", virtualPool1.InternalAttributes()[SnapshotDir])
+
+	// Case2: Ensure snapshotDir is picked from virtual pool and is lower case
+	virtualPool2 := driver.pools["myANFBackend_pool_1"]
+	assert.Equal(t, "false", virtualPool2.InternalAttributes()[SnapshotDir])
+
+	// Case3: Ensure snapshotDir is picked from virtual pool and is same as passed in case of invalid value
+	virtualPool3 := driver.pools["myANFBackend_pool_2"]
+	assert.Equal(t, "Invalid", virtualPool3.InternalAttributes()[SnapshotDir])
+}
+
 func TestInitialize_KerberosDisabledInPool(t *testing.T) {
 	defer acp.SetAPI(acp.API())
 

storage_drivers/gcp/gcp_cvs.go

@@ -277,9 +277,17 @@ func (d *NFSStorageDriver) populateConfigurationDefaults(
 		config.NfsMountOptions = defaultNfsMountOptions
 	}
 
-	if config.SnapshotDir == "" {
-		config.SnapshotDir = defaultSnapshotDir
+	// If snapshotDir is provided, ensure it is lower case
+	snapDir := defaultSnapshotDir
+	if config.SnapshotDir != "" {
+		snapDirFormatted, err := utils.GetFormattedBool(config.SnapshotDir)
+		if err != nil {
+			Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir: %v.", config.SnapshotDir)
+			return fmt.Errorf("invalid boolean value for snapshotDir: %v", err)
+		}
+		snapDir = snapDirFormatted
 	}
+	config.SnapshotDir = snapDir
 
 	if config.SnapshotReserve == "" {
 		config.SnapshotReserve = defaultSnapshotReserve
@@ -334,6 +342,15 @@ func (d *NFSStorageDriver) populateConfigurationDefaults(
 func (d *NFSStorageDriver) initializeStoragePools(ctx context.Context) {
 	d.pools = make(map[string]storage.Pool)
 
+	// If snapshotDir is provided, ensure it is lower case
+	if d.Config.SnapshotDir != "" {
+		snapDirFormatted, err := utils.GetFormattedBool(d.Config.SnapshotDir)
+		if err != nil {
+			Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir: %v.", d.Config.SnapshotDir)
+		}
+		d.Config.SnapshotDir = snapDirFormatted
+	}
+
 	if len(d.Config.Storage) == 0 {
 
 		Logc(ctx).Debug("No vpools defined, reporting single pool.")
@@ -409,7 +426,11 @@ func (d *NFSStorageDriver) initializeStoragePools(ctx context.Context) {
 
 			snapshotDir := d.Config.SnapshotDir
 			if vpool.SnapshotDir != "" {
-				snapshotDir = vpool.SnapshotDir
+				snapDirFormatted, err := utils.GetFormattedBool(vpool.SnapshotDir)
+				if err != nil {
+					Logc(ctx).WithError(err).Errorf("Invalid boolean value for vpool's snapshotDir: %v.", vpool.SnapshotDir)
+				}
+				snapshotDir = snapDirFormatted
 			}
 
 			snapshotReserve := d.Config.SnapshotReserve