cleanup: generalize the parseBool function

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

internal/rbd/nodeserver.go

@@ -67,6 +67,9 @@ const (
 	xfsReflinkUnset int = iota
 	xfsReflinkNoSupport
 	xfsReflinkSupport
+
+	staticVol    = "staticVolume"
+	volHealerCtx = "volumeHealerContext"
 )
 
 var (
@@ -98,34 +101,20 @@ var (
 	xfsHasReflink = xfsReflinkUnset
 )
 
-// isHealerContext checks if the request is been made from volumeHealer.
+// parseBoolOption checks if parameters contain option and parse it. If it is
-func isHealerContext(parameters map[string]string) bool {
+// empty or not set return default.
-	var err error
+// nolint:unparam // currently defValue is always false, this can change in the future
-	healerContext := false
+func parseBoolOption(ctx context.Context, parameters map[string]string, optionName string, defValue bool) bool {
+	boolVal := defValue
 
-	val, ok := parameters["volumeHealerContext"]
+	if val, ok := parameters[optionName]; ok {
-	if ok {
+		var err error
-		if healerContext, err = strconv.ParseBool(val); err != nil {
+		if boolVal, err = strconv.ParseBool(val); err != nil {
-			return false
+			log.ErrorLog(ctx, "failed to parse value of %q: %q", optionName, val)
 		}
 	}
 
-	return healerContext
+	return boolVal
-}
-
-// isStaticVolume checks if the volume is static.
-func isStaticVolume(parameters map[string]string) bool {
-	var err error
-	staticVol := false
-
-	val, ok := parameters["staticVolume"]
-	if ok {
-		if staticVol, err = strconv.ParseBool(val); err != nil {
-			return false
-		}
-	}
-
-	return staticVol
 }
 
 // healerStageTransaction attempts to attach the rbd Image with previously
@@ -189,7 +178,7 @@ func populateRbdVol(
 	}
 
 	rv.ThickProvision = isThickProvisionRequest(req.GetVolumeContext())
-	isStaticVol := isStaticVolume(req.GetVolumeContext())
+	isStaticVol := parseBoolOption(ctx, req.GetVolumeContext(), staticVol, false)
 	// get rbd image name from the volume journal
 	// for static volumes, the image name is actually the volume ID itself
 	if isStaticVol {
@@ -299,7 +288,7 @@ func (ns *NodeServer) NodeStageVolume(
 	stagingParentPath := req.GetStagingTargetPath()
 	stagingTargetPath := stagingParentPath + "/" + volID
 
-	isHealer := isHealerContext(req.GetVolumeContext())
+	isHealer := parseBoolOption(ctx, req.GetVolumeContext(), volHealerCtx, false)
 	if !isHealer {
 		var isNotMnt bool
 		// check if stagingPath is already mounted
@@ -313,7 +302,7 @@ func (ns *NodeServer) NodeStageVolume(
 		}
 	}
 
-	isStaticVol := isStaticVolume(req.GetVolumeContext())
+	isStaticVol := parseBoolOption(ctx, req.GetVolumeContext(), staticVol, false)
 
 	// throw error when imageFeatures parameter is missing or empty
 	// for backward compatibility, ignore error for non-static volumes from older cephcsi version

internal/rbd/nodeserver_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package rbd
 
 import (
+	"context"
 	"testing"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -56,3 +57,51 @@ func TestGetStagingPath(t *testing.T) {
 		t.Errorf("getStagingTargetPath() = %s, got %s", stagingPath, expect)
 	}
 }
+
+func TestParseBoolOption(t *testing.T) {
+	t.Parallel()
+	ctx := context.TODO()
+	optionName := "myOption"
+	defaultValue := false
+
+	tests := []struct {
+		name         string
+		scParameters map[string]string
+		expect       bool
+	}{
+		{
+			name:         "myOption => true",
+			scParameters: map[string]string{optionName: "true"},
+			expect:       true,
+		},
+		{
+			name:         "myOption => false",
+			scParameters: map[string]string{optionName: "false"},
+			expect:       false,
+		},
+		{
+			name:         "myOption => empty",
+			scParameters: map[string]string{optionName: ""},
+			expect:       defaultValue,
+		},
+		{
+			name:         "myOption => not-parsable",
+			scParameters: map[string]string{optionName: "non-boolean"},
+			expect:       defaultValue,
+		},
+		{
+			name:         "myOption => not-set",
+			scParameters: map[string]string{},
+			expect:       defaultValue,
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		val := parseBoolOption(ctx, tc.scParameters, optionName, defaultValue)
+		if val != tc.expect {
+			t.Errorf("parseBoolOption(%v) returned: %t, expected: %t",
+				tc.scParameters, val, tc.expect)
+		}
+	}
+}