nvmeof: rearrangement create nvmeof resources

internal/nvmeof/controller/controllerserver.go

@@ -431,9 +431,9 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
 		}
 	}
 	// Validate listeners JSON if provided
-	listeners, err := parseListeners(params["listeners"])
+	countOfListeners, err := validateListenersParameter(params["listeners"])
 	if err != nil {
-		return fmt.Errorf("invalid listeners parameter: %w", err)
+		return err
 	}
 	// Validate network mask if provided
 	err = validateNetworkMask(params["networkMask"])
@@ -442,11 +442,11 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
 	}
 	networkMask := params["networkMask"]
 	// Must have EITHER listeners XOR networkMask
-	if len(listeners) == 0 && networkMask == "" {
+	if countOfListeners == 0 && networkMask == "" {
 		return errors.New("must specify either 'listeners' xor 'networkMask', but got neither")
 	}
-	if len(listeners) > 0 && networkMask != "" {
-		return errors.New("must specify either 'listeners' xor 'networkMask',but got both")
+	if countOfListeners > 0 && networkMask != "" {
+		return errors.New("must specify either 'listeners' xor 'networkMask', but got both")
 	}
 	// Validate QoS parameters - cannot mix RBD and NVMe-oF QoS
 	mutableParams := req.GetMutableParameters()
@@ -487,7 +487,7 @@ func validatePublishVolumeRequest(req *csi.ControllerPublishVolumeRequest) error
 	return util.ValidateControllerPublishVolumeRequest(req)
 }
 
-// parseListeners parses the listeners JSON parameter and validates its contents.
+// validateListenersParameter parses the listeners JSON parameter and validates its contents.
 // it possible to have zero listeners if networkMask is provided,
 // because in that case the gateway will automatically create listeners
 // for all interfaces in the specified network mask.
@@ -507,29 +507,13 @@ func validatePublishVolumeRequest(req *csi.ControllerPublishVolumeRequest) error
 //	}
 //
 // ].
-func parseListeners(listenersJSON string) ([]nvmeof.ListenerDetails, error) {
-	if listenersJSON == "" { // No "listeners" entry was provided
-		return []nvmeof.ListenerDetails{}, nil
-	}
-	var listeners []nvmeof.ListenerDetails
-	if err := json.Unmarshal([]byte(listenersJSON), &listeners); err != nil {
-		return nil, fmt.Errorf("failed to parse listeners JSON: %w", err)
-	}
-
-	if len(listeners) == 0 { // At least one listener is required
-		return nil, errors.New("at least one listener must be specified")
-	}
-
-	// Validate each listener
-	// Listener address can be empty. it will set to default 0.0.0.0
-	// Port can be empty (will use default - 4420).
-	for i, listener := range listeners {
-		if listener.Hostname == "" {
-			return nil, fmt.Errorf("listener %d: missing required fields (hostname)", i)
-		}
+func validateListenersParameter(listenersJSON string) (int, error) {
+	listeners, err := nvmeof.SetupListeners(listenersJSON)
+	if err != nil {
+		return -1, err
 	}
 
-	return listeners, nil
+	return len(listeners), nil
 }
 
 // Validate network mask CIDR format.
@@ -764,43 +748,10 @@ func (cs *Server) createNVMeoFResources(
 	params := req.GetParameters()
 
 	networkMask := params["networkMask"]
-	nvmeofData := &nvmeof.NVMeoFVolumeData{
-		SubsystemNQN:  params["subsystemNQN"],
-		NamespaceID:   0,   // will be set after namespace creation,
-		NamespaceUUID: "",  // will be set after namespace creation
-		ListenerInfo:  nil, // will be set after listener creation or retrieval
-		GatewayManagementInfo: nvmeof.GatewayConfig{
-			Address: params["nvmeofGatewayAddress"],
-		},
-		Security: nvmeof.NVMeoFSecurityConfig{
-			DhchapMode:          params["dhchapMode"],
-			AuthenticationKMSID: params["authenticationKMSID"],
-		},
-	}
-	if nvmeofGatewayPortStr := params["nvmeofGatewayPort"]; nvmeofGatewayPortStr != "" {
-		parsed, err := strconv.ParseUint(nvmeofGatewayPortStr, 10, 32)
-		if err != nil {
-			return nil, fmt.Errorf("invalid nvmeofGatewayPort %s: %w", nvmeofGatewayPortStr, err)
-		}
-		nvmeofData.GatewayManagementInfo.Port = uint32(parsed)
-	}
+	nvmeofData := &nvmeof.NVMeoFVolumeData{}
 
-	// setup listeners (if provided, otherwise it will be set by gateway based on network mask)
-	listeners, err := parseListeners(params["listeners"])
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse listeners: %w", err)
-	}
-	nvmeofData.ListenerInfo = listeners
-	// If listener port or address is empty, it will be set to default.
-	nvmeofData.SetListenersWithDefaults()
-
-	// If dhchapMode was explicitly provided and is not "none", and authenticationKMSID is empty,
-	// use a default KMS ID - RBD metadata KMS.
-	// In production, users should always provide a KMS ID when using DH-CHAP.
-	if nvmeofData.Security.DhchapMode != nvmeof.DHCHAPEmpty &&
-		nvmeofData.Security.DhchapMode != nvmeof.DHCHAPModeNone &&
-		nvmeofData.Security.AuthenticationKMSID == "" {
-		nvmeofData.Security.AuthenticationKMSID = "metadata"
+	if err := nvmeofData.SetFromParameters(params); err != nil {
+		return nil, fmt.Errorf("failed to set NVMe-oF volume data: %w", err)
 	}
 
 	// extract Qos parameters if any

internal/nvmeof/volume.go

@@ -16,6 +16,13 @@ limitations under the License.
 
 package nvmeof
 
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"strconv"
+)
+
 // NVMeoFVolumeData holds the data required for an NVMe-oF volume.
 type NVMeoFVolumeData struct {
 	SubsystemNQN          string
@@ -44,3 +51,75 @@ func (v *NVMeoFVolumeData) SetListenersWithDefaults() {
 		}
 	}
 }
+
+// SetupListeners parses the listeners JSON and validates the required fields.
+// It returns a slice of ListenerDetails or an error if the JSON is invalid
+// or if required fields are missing.
+func SetupListeners(listenersJSON string) ([]ListenerDetails, error) {
+	if listenersJSON == "" { // No "listeners" entry was provided
+		return []ListenerDetails{}, nil
+	}
+	var listeners []ListenerDetails
+	if err := json.Unmarshal([]byte(listenersJSON), &listeners); err != nil {
+		return nil, fmt.Errorf("failed to parse listeners JSON: %w", err)
+	}
+
+	if len(listeners) == 0 { // At least one listener is required
+		return nil, errors.New("at least one listener must be specified")
+	}
+
+	// Validate each listener
+	// Listener address can be empty. It will be set later to the default 0.0.0.0.
+	// Port can be empty (it will be set later to the default 4420).
+	for i, listener := range listeners {
+		if listener.Hostname == "" {
+			return nil, fmt.Errorf("listener %d: missing required fields (hostname)", i)
+		}
+	}
+
+	return listeners, nil
+}
+
+// SetFromParameters populates the NVMeoFVolumeData fields based on the provided parameters map.
+// It extracts the subsystem NQN, gateway management info, security config, and
+// listener info from the parameters.
+// It also applies default values to listeners if necessary.
+func (v *NVMeoFVolumeData) SetFromParameters(parameters map[string]string) error {
+	// set subsystem NQN
+	v.SubsystemNQN = parameters["subsystemNQN"]
+
+	// set gw management info
+	if nvmeofGatewayPortStr := parameters["nvmeofGatewayPort"]; nvmeofGatewayPortStr != "" {
+		parsed, err := strconv.ParseUint(nvmeofGatewayPortStr, 10, 32)
+		if err != nil {
+			return fmt.Errorf("invalid nvmeofGatewayPort %s: %w", nvmeofGatewayPortStr, err)
+		}
+		v.GatewayManagementInfo.Port = uint32(parsed)
+	}
+	v.GatewayManagementInfo.Address = parameters["nvmeofGatewayAddress"]
+
+	// set security config
+	v.Security.DhchapMode = parameters["dhchapMode"]
+	v.Security.AuthenticationKMSID = parameters["authenticationKMSID"]
+
+	// If dhchapMode was explicitly provided and is not "none", and authenticationKMSID is empty,
+	// use a default KMS ID - RBD metadata KMS.
+	// In production, users should always provide a KMS ID when using DH-CHAP.
+	if v.Security.DhchapMode != DHCHAPEmpty &&
+		v.Security.DhchapMode != DHCHAPModeNone &&
+		v.Security.AuthenticationKMSID == "" {
+		v.Security.AuthenticationKMSID = "metadata"
+	}
+
+	// set listeners
+	listeners, err := SetupListeners(parameters["listeners"])
+	if err != nil {
+		return fmt.Errorf("failed to set up listeners: %w", err)
+	}
+	v.ListenerInfo = listeners
+
+	// Apply default values to listeners if necessary
+	v.SetListenersWithDefaults()
+
+	return nil
+}

internal/nvmeof/volume_test.go

@@ -50,3 +50,171 @@ func TestSetListenersWithDefaults(t *testing.T) {
 		require.Equal(t, test.out, vol.ListenerInfo)
 	}
 }
+
+func TestSetupListeners(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name        string
+		jsonInput   string
+		expected    []ListenerDetails
+		expectError bool
+	}{
+		{
+			name:      "valid listeners JSON",
+			jsonInput: `[{"hostname": "nvmeof-gw-a", "port": 4420, "address": "10.92.3.12"}]`,
+			expected: []ListenerDetails{
+				{Hostname: "nvmeof-gw-a", GatewayAddress: GatewayAddress{Port: 4420, Address: "10.92.3.12"}},
+			},
+			expectError: false,
+		},
+		{
+			name:        "invalid listeners JSON",
+			jsonInput:   `invalid json`,
+			expected:    nil,
+			expectError: true,
+		},
+		{
+			name:        "missing required fields",
+			jsonInput:   `[{"port": 4420, "address": "10.92.3.12"}]`,
+			expected:    nil,
+			expectError: true,
+		},
+		// An empty listeners array is invalid because at least one listener is required.
+		// the listener label can be omitted, then network mask label must be provided.
+		{
+			name:        "empty listeners array",
+			jsonInput:   `[]`,
+			expected:    nil,
+			expectError: true,
+		},
+		{
+			name:        "no listeners entry",
+			jsonInput:   ``,
+			expected:    []ListenerDetails{},
+			expectError: false,
+		},
+	}
+
+	for _, test := range tests {
+		listeners, err := SetupListeners(test.jsonInput)
+		if test.expectError {
+			require.Error(t, err)
+		} else {
+			require.NoError(t, err)
+			require.Equal(t, test.expected, listeners)
+		}
+	}
+}
+
+func TestSetFromParameters(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name        string
+		params      map[string]string
+		expected    NVMeoFVolumeData
+		expectError bool
+	}{
+		{
+			name: "valid parameters with listeners and gateway info",
+			params: map[string]string{
+				"subsystemNQN":         "nqn.2016-06.io.ceph:subsystem.test-integration",
+				"nvmeofGatewayAddress": "10.241.1.5",
+				"nvmeofGatewayPort":    "5500",
+				"listeners":            `[{"hostname": "nvmeof-gw-a", "port": 4420, "address": "10.92.3.12"}]`,
+			},
+			expected: NVMeoFVolumeData{
+				SubsystemNQN: "nqn.2016-06.io.ceph:subsystem.test-integration",
+				GatewayManagementInfo: GatewayConfig{
+					Address: "10.241.1.5",
+					Port:    5500,
+				},
+				ListenerInfo: []ListenerDetails{
+					{Hostname: "nvmeof-gw-a", GatewayAddress: GatewayAddress{Port: 4420, Address: "10.92.3.12"}},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "valid parameters with DH-CHAP authentication",
+			params: map[string]string{
+				"subsystemNQN":         "nqn.2016-06.io.ceph:subsystem.test-dhchap",
+				"nvmeofGatewayAddress": "10.241.1.6",
+				"nvmeofGatewayPort":    "5500",
+				"dhchapMode":           "bidirectional",
+				"authenticationKMSID":  "vault-kms",
+				"listeners":            `[{"hostname": "nvmeof-gw-b", "port": 4420, "address": "10.92.3.13"}]`,
+			},
+			expected: NVMeoFVolumeData{
+				SubsystemNQN: "nqn.2016-06.io.ceph:subsystem.test-dhchap",
+				GatewayManagementInfo: GatewayConfig{
+					Address: "10.241.1.6",
+					Port:    5500,
+				},
+				Security: NVMeoFSecurityConfig{
+					DhchapMode:          "bidirectional",
+					AuthenticationKMSID: "vault-kms",
+				},
+				ListenerInfo: []ListenerDetails{
+					{Hostname: "nvmeof-gw-b", GatewayAddress: GatewayAddress{Port: 4420, Address: "10.92.3.13"}},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "valid parameters with default KMS type",
+			params: map[string]string{
+				"subsystemNQN":         "nqn.2016-06.io.ceph:subsystem.test-dhchap-default-kms",
+				"nvmeofGatewayAddress": "10.241.1.6",
+				"nvmeofGatewayPort":    "5500",
+				"dhchapMode":           "bidirectional",
+				"listeners":            `[{"hostname": "nvmeof-gw-b", "port": 4420, "address": "10.92.3.13"}]`,
+			},
+			expected: NVMeoFVolumeData{
+				SubsystemNQN: "nqn.2016-06.io.ceph:subsystem.test-dhchap-default-kms",
+				GatewayManagementInfo: GatewayConfig{
+					Address: "10.241.1.6",
+					Port:    5500,
+				},
+				Security: NVMeoFSecurityConfig{
+					DhchapMode:          "bidirectional",
+					AuthenticationKMSID: "metadata",
+				},
+				ListenerInfo: []ListenerDetails{
+					{Hostname: "nvmeof-gw-b", GatewayAddress: GatewayAddress{Port: 4420, Address: "10.92.3.13"}},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "invalid gateway port",
+			params: map[string]string{
+				"subsystemNQN":         "nqn.2016-06.io.ceph:subsystem.test-invalid-port",
+				"nvmeofGatewayAddress": "10.241.1.7",
+				"nvmeofGatewayPort":    "invalid-port",
+			},
+			expected:    NVMeoFVolumeData{},
+			expectError: true,
+		},
+		{
+			name: "invalid listeners JSON",
+			params: map[string]string{
+				"subsystemNQN":         "nqn.2016-06.io.ceph:subsystem.test-invalid-listeners",
+				"nvmeofGatewayAddress": "10.241.1.8",
+				"nvmeofGatewayPort":    "5500",
+				"listeners":            `invalid-json`,
+			},
+			expected:    NVMeoFVolumeData{},
+			expectError: true,
+		},
+	}
+	for _, test := range tests {
+		vol := &NVMeoFVolumeData{}
+		err := vol.SetFromParameters(test.params)
+		if test.expectError {
+			require.Error(t, err)
+		} else {
+			require.NoError(t, err)
+			require.Equal(t, test.expected, *vol)
+		}
+	}
+}