diff --git a/cli/cmd/update_volume.go b/cli/cmd/update_volume.go index 7c2798a60..152661158 100644 --- a/cli/cmd/update_volume.go +++ b/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, } diff --git a/frontend/common/volumes.go b/frontend/common/volumes.go index 23c41bfa9..bd0648d2a 100644 --- a/frontend/common/volumes.go +++ b/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", ""), diff --git a/frontend/common/volumes_test.go b/frontend/common/volumes_test.go index f6221eb6a..09e30b1c7 100644 --- a/frontend/common/volumes_test.go +++ b/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") } diff --git a/frontend/csi/controller_helpers/kubernetes/helper.go b/frontend/csi/controller_helpers/kubernetes/helper.go index b2278b03f..6d4806e1a 100644 --- a/frontend/csi/controller_helpers/kubernetes/helper.go +++ b/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, diff --git a/frontend/csi/controller_helpers/plain/plugin_test.go b/frontend/csi/controller_helpers/plain/plugin_test.go index 16aea7455..870550495 100644 --- a/frontend/csi/controller_helpers/plain/plugin_test.go +++ b/frontend/csi/controller_helpers/plain/plugin_test.go @@ -86,11 +86,12 @@ 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} @@ -98,13 +99,23 @@ func TestGetVolumeConfig(t *testing.T) { 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", diff --git a/storage_drivers/azure/azure_anf.go b/storage_drivers/azure/azure_anf.go index 0b93f521a..e20e5f1bd 100644 --- a/storage_drivers/azure/azure_anf.go +++ b/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) } } diff --git a/storage_drivers/azure/azure_anf_test.go b/storage_drivers/azure/azure_anf_test.go index b5cd328eb..03328923b 100644 --- a/storage_drivers/azure/azure_anf_test.go +++ b/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()) diff --git a/storage_drivers/gcp/gcp_cvs.go b/storage_drivers/gcp/gcp_cvs.go index e9d32ff15..8242869b4 100644 --- a/storage_drivers/gcp/gcp_cvs.go +++ b/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 diff --git a/storage_drivers/gcp/gcp_cvs_test.go b/storage_drivers/gcp/gcp_cvs_test.go index f734dc2bc..a0a513af4 100644 --- a/storage_drivers/gcp/gcp_cvs_test.go +++ b/storage_drivers/gcp/gcp_cvs_test.go @@ -484,6 +484,71 @@ func TestInitializeStoragePoolsUnixPermissions(t *testing.T) { assert.Equal(t, "0777", virtualPool.InternalAttributes()[UnixPermissions]) } +func TestPopulateConfigurationDefaultsSnapshotDir(t *testing.T) { + ctx := context.Background() + d := newTestGCPDriver(nil) + + tests := []struct { + name string + inputSnapshotDir string + expectedSnapshotDir string + expectErr bool + }{ + {"Default snapshotDir", "", "false", false}, + {"Uppercase snapshotDir", "TRUE", "true", false}, + {"Invalid snapshotDir", "TrUE", "", true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + d.Config.SnapshotDir = test.inputSnapshotDir + response := d.populateConfigurationDefaults(ctx, &d.Config) + if test.expectErr { + assert.Error(t, response) + } else { + assert.Equal(t, test.expectedSnapshotDir, d.Config.SnapshotDir) + } + }) + } +} + +func TestInitializeStoragePoolsSnapshotDir(t *testing.T) { + // Test with multiple vpools defined, one with snapshotDir set + ctx := context.Background() + d := newTestGCPDriver(nil) + + d.Config.SnapshotDir = "TRUE" + d.Config.Storage = []drivers.GCPNFSStorageDriverPool{ + { + Region: "us_east_1", + Zone: "us_east_1a", + }, + { + GCPNFSStorageDriverConfigDefaults: drivers.GCPNFSStorageDriverConfigDefaults{SnapshotDir: "False"}, + Region: "us_east_1", + Zone: "us_east_1a", + }, + { + GCPNFSStorageDriverConfigDefaults: drivers.GCPNFSStorageDriverConfigDefaults{SnapshotDir: "Invalid"}, + Region: "us_east_1", + Zone: "us_east_1a", + }, + } + d.initializeStoragePools(ctx) + + // Case1: Ensure snapshotDir is picked from driver config and is lower case if valid + virtualPool1 := d.pools["gcpcvs_12345_pool_0"] + assert.Equal(t, "true", virtualPool1.InternalAttributes()[SnapshotDir]) + + // Case2: Ensure snapshotDir is picked from virtual pool and is lower case if valid + virtualPool2 := d.pools["gcpcvs_12345_pool_1"] + assert.Equal(t, "false", virtualPool2.InternalAttributes()[SnapshotDir]) + + // Case3: Ensure snapshotDir is picked from virtual pool and is same as passed if invalid bool + virtualPool3 := d.pools["gcpcvs_12345_pool_2"] + assert.Equal(t, "Invalid", virtualPool3.InternalAttributes()[SnapshotDir]) +} + func TestEnsureTopologyRegionAndZone(t *testing.T) { tests := []struct { Name string diff --git a/storage_drivers/ontap/ontap_common.go b/storage_drivers/ontap/ontap_common.go index 27b758c8c..9efffbf7b 100644 --- a/storage_drivers/ontap/ontap_common.go +++ b/storage_drivers/ontap/ontap_common.go @@ -1273,9 +1273,17 @@ func PopulateConfigurationDefaults(ctx context.Context, config *drivers.OntapSto } } - 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.DriverContext != tridentconfig.ContextCSI { config.AutoExportPolicy = false @@ -1852,6 +1860,14 @@ func InitializeStoragePoolsCommon( pool.Attributes()[sa.Zone] = sa.NewStringOffer(config.Zone) } + if config.SnapshotDir != "" { + config.SnapshotDir, err = utils.GetFormattedBool(config.SnapshotDir) + if err != nil { + Logc(ctx).WithError(err).Errorf("Invalid boolean value for snapshotDir: %v.", config.SnapshotDir) + return nil, nil, fmt.Errorf("invalid boolean value for snapshotDir: %v", err) + } + } + pool.Attributes()[sa.Labels] = sa.NewLabelOffer(config.Labels) pool.Attributes()[sa.NASType] = sa.NewStringOffer(config.NASType) pool.Attributes()[sa.SANType] = sa.NewStringOffer(config.SANType) @@ -1937,7 +1953,11 @@ func InitializeStoragePoolsCommon( snapshotDir := config.SnapshotDir if vpool.SnapshotDir != "" { - snapshotDir = vpool.SnapshotDir + snapshotDir, err = utils.GetFormattedBool(vpool.SnapshotDir) + if err != nil { + Logc(ctx).WithError(err).Errorf("Invalid boolean value for vpool's snapshotDir: %v.", vpool.SnapshotDir) + return nil, nil, fmt.Errorf("invalid boolean value for snapshotDir: %v", err) + } } exportPolicy := config.ExportPolicy @@ -2320,9 +2340,17 @@ func getVolumeOptsCommon( if volConfig.UnixPermissions != "" { opts["unixPermissions"] = volConfig.UnixPermissions } + + // If snapshotDir is provided, ensure it is lower case if volConfig.SnapshotDir != "" { - opts["snapshotDir"] = volConfig.SnapshotDir + snapshotDirFormatted, err := utils.GetFormattedBool(volConfig.SnapshotDir) + if err != nil { + Logc(ctx).WithError(err).Errorf( + "Invalid boolean value for volume '%v' snapshotDir: %v.", volConfig.Name, volConfig.SnapshotDir) + } + opts["snapshotDir"] = snapshotDirFormatted } + if volConfig.ExportPolicy != "" { opts["exportPolicy"] = volConfig.ExportPolicy } diff --git a/storage_drivers/ontap/ontap_common_test.go b/storage_drivers/ontap/ontap_common_test.go index 89dc2a444..8eec2235e 100644 --- a/storage_drivers/ontap/ontap_common_test.go +++ b/storage_drivers/ontap/ontap_common_test.go @@ -3435,6 +3435,54 @@ func TestGetVolumeOptsCommon_InvalidEncryptionType(t *testing.T) { assert.NotEqual(t, 0, len(opts)) } +func TestGetVolumeOptsCommon_DifferentSnapshotDir(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + inputSnapshotDir string + expectedSnapshotDir string + }{ + {"Default snapshotDir", "", ""}, + {"Valid uppercase snapshotDir", "TRUE", "true"}, + {"Valid lowercase snapshotDir", "false", "false"}, + {"Invalid snapshotDir", "fakeSnapDir", "fakeSnapDir"}, + } + + volConfig := &storage.VolumeConfig{ + Name: "fakeVolName", + InternalName: "fakeInternalName", + SnapshotPolicy: "fakeSnapPoliy", + SnapshotReserve: "fakeSnapReserve", + UnixPermissions: "fakePermissions", + ExportPolicy: "fakeExportPolicy", + SpaceReserve: "fakeSpaceReserve", + SecurityStyle: "fakeSecurityStyle", + SplitOnClone: "fakeSplitOnClone", + FileSystem: "fakeFilesystem", + Encryption: "fakeEncryption", + QosPolicy: "fakeQoSPolicy", + AdaptiveQosPolicy: "fakeAdaptiveQosPolicy", + } + + // "Encryption: "false" + requests := map[string]sa.Request{ + sa.IOPS: sa.NewIntRequest(40), + sa.Snapshots: sa.NewBoolRequest(true), + sa.Encryption: sa.NewBoolRequest(false), + sa.ProvisioningType: sa.NewStringRequest("thin"), + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + volConfig.SnapshotDir = test.inputSnapshotDir + opts := getVolumeOptsCommon(ctx, volConfig, requests) + assert.NotEqual(t, 0, len(opts)) + assert.Equal(t, test.expectedSnapshotDir, opts["snapshotDir"]) + }) + } +} + func TestGetVolumeOptsCommon_NoVolumeOptsReturned(t *testing.T) { ctx := context.Background() volConfig := &storage.VolumeConfig{ @@ -4805,7 +4853,7 @@ func TestInitializeStoragePoolsCommon(t *testing.T) { SnapshotReserve: "fakeSnapshotReserve", SplitOnClone: "false", UnixPermissions: "777", - SnapshotDir: "fakeSnapshotDir", + SnapshotDir: "TRUE", ExportPolicy: "fakeExportPolicy", SecurityStyle: "fakeSecurityStyle", FileSystemType: "fakeFileSystem", @@ -4839,6 +4887,8 @@ func TestInitializeStoragePoolsCommon(t *testing.T) { assert.NotNil(t, physicalPool, "Physical Pool not found when expected") assert.NotNil(t, virtualPool, "Virtual Pool not found when expected") + // Ensure snapshotDir is a lower case value + assert.Equal(t, "true", virtualPool["dummyBackend_pool_0"].InternalAttributes()[SnapshotDir]) assert.NoError(t, err) // Test2 - Invalid value of encryption @@ -4881,6 +4931,47 @@ func TestInitializeStoragePoolsCommon(t *testing.T) { assert.Nil(t, physicalPool, "Physical Pool not expected but found") assert.Nil(t, virtualPool, "Virtual pool not exepcted but found") assert.Error(t, err) + + // Test3 - Invalid value of snapshotDir + defaults = &drivers.OntapStorageDriverConfigDefaults{ + SpaceAllocation: "fake", + SpaceReserve: "fakeSpaceReserve", + SnapshotPolicy: "fakeSnapshotPolicy", + SnapshotReserve: "fakeSnapshotReserve", + SplitOnClone: "false", + UnixPermissions: "777", + SnapshotDir: "FaLsE", + ExportPolicy: "fakeExportPolicy", + SecurityStyle: "fakeSecurityStyle", + FileSystemType: "fakeFileSystem", + Encryption: "true", + LUKSEncryption: "false", + TieringPolicy: "fakeTieringPolicy", + QosPolicy: "fakeQosPolicy", + AdaptiveQosPolicy: "fakeAdaptiveQosPolicy", + CommonStorageDriverConfigDefaults: *CommonConfigDefault, + } + storageDriver.Config.Storage = []drivers.OntapStorageDriverPool{ + { + Region: "fakeRegion", + Zone: "fakeZone", + SupportedTopologies: []map[string]string{ + { + "topology.kubernetes.io/region": "us_east_1", + "topology.kubernetes.io/zone": "us_east_1a", + }, + }, + OntapStorageDriverConfigDefaults: *defaults, + }, + } + mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return([]string{"dummyPool1"}, nil) + mockAPI.EXPECT().GetSVMAggregateAttributes(ctx).Return(map[string]string{"dummyPool1": "hdd"}, nil) + + physicalPool, virtualPool, err = InitializeStoragePoolsCommon(ctx, storageDriver, pool1.Attributes(), backendName) + + assert.Nil(t, physicalPool, "Physical Pool not expected but found") + assert.Nil(t, virtualPool, "Virtual pool not exepcted but found") + assert.Error(t, err) } func TestValidateDataLIF(t *testing.T) { diff --git a/utils/utils.go b/utils/utils.go index 2db01c968..91168167b 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1078,3 +1078,13 @@ func DecodeBase64StringToObject(encodedObject string, destination any) error { } return nil } + +// GetFormattedBool Returns lowercase string value for a valid boolean value, else returns same value with error. +func GetFormattedBool(value string) (string, error) { + valBool, err := strconv.ParseBool(value) + if err != nil { + return value, err + } + + return strconv.FormatBool(valBool), nil +} diff --git a/utils/utils_test.go b/utils/utils_test.go index 6e6425c03..1bb4bff5b 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -1642,3 +1642,29 @@ func TestEncodeAndDecodeToAndFromBase64(t *testing.T) { assert.Equal(t, originalObject.Bar, actualObject.Bar) assert.Equal(t, originalObject.Baz, actualObject.Baz) } + +func TestGetFormattedValidBool(t *testing.T) { + tests := []struct { + name string + input string + expected string + expectErr bool + }{ + {"Valid uppercase bool value", "TRUE", "true", false}, + {"Valid Camelcase bool value", "True", "true", false}, + {"Invalid bool value", "TrUe", "TrUe", true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + response, responseErr := GetFormattedBool(test.input) + + assert.Equal(t, test.expected, response) + if test.expectErr { + assert.Error(t, responseErr) + } else { + assert.Nil(t, responseErr) + } + }) + } +}