nvmeof: add external client support via allowHostNQNs parameter

Implements dynamic host access control for NVMe-oF volumes to support
external(non-Kubernetes) clients through VolumeAttributesClass.

Changes:
- Add `parseHostsParameters()` to parse YAML host list from VAC
- Implement `modifyNVMeoFHosts()` for runtime host updates
- Add `ListHosts()` and `UpdateHostsForSubsystem()` for host updateing
- Support allowHostNQNs parameter in CreateVolume and ControllerModifyVolume
- Auto-generate subsystem NQN from volumeID if not provided

Users can now specify external hosts in VAC mutable parameters:
  allowHostNQNs: |
    - nqn.2014-08.org.nvmexpress:host1
    - nqn.2014-08.org.nvmexpress:host2

Signed-off-by: gadi-didi <gadi.didi@ibm.com>

Author: gadididi

internal/nvmeof/controller/controllerserver.go

@@ -26,6 +26,7 @@ import (
 	"strconv"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
+	"github.com/ghodss/yaml"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
@@ -167,7 +168,7 @@ func (cs *Server) CreateVolume(
 		}
 	}()
 
-	nvmeofData, err = cs.createNVMeoFResources(ctx, req, rbdPoolName, rbdRadosNameSpace, rbdImageName)
+	nvmeofData, err = cs.createNVMeoFResources(ctx, req, rbdPoolName, rbdRadosNameSpace, rbdImageName, volumeID)
 	if err != nil {
 		log.ErrorLog(ctx, "NVMe-oF resource setup failed for volumeID %s: %v", volumeID, err)
 
@@ -353,11 +354,22 @@ func (cs *Server) ControllerModifyVolume(
 
 		return nil, status.Errorf(codes.InvalidArgument, "failed to parse QoS parameters: %v", err)
 	}
+	hostsList, err := parseHostsParameters(params)
+	if err != nil {
+		log.ErrorLog(ctx, "failed to parse NVMe-oF hosts parameters: %v", err)
+
+		return nil, status.Errorf(codes.InvalidArgument, "failed to parse hosts parameters: %v", err)
+	}
 	if nvmeofQoS != nil {
 		if err := cs.modifyNVMeoFQoS(ctx, req, nvmeofQoS); err != nil {
 			return nil, err
 		}
 	}
+	if hostsList != nil {
+		if err := cs.modifyNVMeoFHosts(ctx, req, hostsList); err != nil {
+			return nil, err
+		}
+	}
 
 	return &csi.ControllerModifyVolumeResponse{}, nil
 }
@@ -425,14 +437,10 @@ func validateDHCHAPParameter(dhchapMode string) error {
 func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
 	// Validate required parameters
 	params := req.GetParameters()
-	requiredParams := []string{
-		"subsystemNQN", "nvmeofGatewayAddress",
-	}
-	for _, param := range requiredParams {
-		if params[param] == "" {
-			return fmt.Errorf("missing required parameter: %s", param)
-		}
+	if params["nvmeofGatewayAddress"] == "" {
+		return errors.New("missing required parameter nvmeofGatewayAddress")
 	}
+
 	// Validate listeners JSON if provided
 	countOfListeners, err := validateListenersParameter(params["listeners"])
 	if err != nil {
@@ -467,6 +475,11 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
 	if err != nil {
 		return fmt.Errorf("invalid NVMe-oF QoS parameters: %w", err)
 	}
+
+	_, err = parseHostsParameters(mutableParams)
+	if err != nil {
+		return fmt.Errorf("invalid NVMe-oF hosts parameters (for external clients): %w", err)
+	}
 	err = validateDHCHAPParameter(params["dhchapMode"])
 	if err != nil {
 		return err
@@ -533,42 +546,24 @@ func validateNetworkMask(networkMask string) error {
 	return nil
 }
 
-// parseQoSParameters extracts and parses QoS parameters from the given map.
-func parseQoSParameters(params map[string]string) (*nvmeof.NVMeoFQosVolume, error) {
-	qos := &nvmeof.NVMeoFQosVolume{}
-	hasAnyQoS := false
-
-	parseParam := func(key, name string, dest **uint64) error {
-		if val, exists := params[key]; exists && val != "" {
-			parsed, err := strconv.ParseUint(val, 10, 64)
-			if err != nil {
-				return fmt.Errorf("invalid %s: %w", name, err)
-			}
-			*dest = &parsed
-			hasAnyQoS = true
-		}
-
-		return nil
-	}
-
-	if err := parseParam(nvmeof.RwIosPerSecond, nvmeof.RwIosPerSecond, &qos.RwIosPerSecond); err != nil {
-		return nil, err
-	}
-	if err := parseParam(nvmeof.RwMbytesPerSecond, nvmeof.RwMbytesPerSecond, &qos.RwMbytesPerSecond); err != nil {
-		return nil, err
-	}
-	if err := parseParam(nvmeof.RMbytesPerSecond, nvmeof.RMbytesPerSecond, &qos.RMbytesPerSecond); err != nil {
-		return nil, err
+// parseHostsParameters parses the hosts yaml list parameter and validates its contents.
+// It returns a slice of hostNQNs or an error if the YAML is invalid.
+// allowHostNQNs entry can be empty, in that case it means no hosts are allowed to access
+// the subsystem (empty allow list).
+func parseHostsParameters(params map[string]string) ([]string, error) {
+	allowHostNQNs, exists := params[AllowHostNQNs]
+	if !exists {
+		return nil, nil
 	}
-	if err := parseParam(nvmeof.WMbytesPerSecond, nvmeof.WMbytesPerSecond, &qos.WMbytesPerSecond); err != nil {
-		return nil, err
+	var allowHostsList []string
+	if allowHostNQNs == "" {
+		return allowHostsList, nil
 	}
-
-	if !hasAnyQoS {
-		return nil, nil
+	if err := yaml.Unmarshal([]byte(allowHostNQNs), &allowHostsList); err != nil {
+		return nil, fmt.Errorf("invalid %s: must be a YAML list of strings: %w", AllowHostNQNs, err)
 	}
 
-	return qos, nil
+	return allowHostsList, nil
 }
 
 // withGatewayConnection is a helper that manages the common pattern of:
@@ -629,6 +624,69 @@ func (cs *Server) withGatewayConnection(
 	return fn(ctx, gateway, nvmeofData)
 }
 
+// modifyNVMeoFHosts handles adding or removing hosts from the subsystem based on the provided list of host NQNs.
+func (cs *Server) modifyNVMeoFHosts(ctx context.Context, req *csi.ControllerModifyVolumeRequest, hosts []string) error {
+	volumeID := req.GetVolumeId()
+
+	return cs.withGatewayConnection(ctx, req, volumeID, func(
+		ctx context.Context,
+		gateway *nvmeof.GatewayRpcClient,
+		nvmeofData *nvmeof.NVMeoFVolumeData,
+	) error {
+		log.DebugLog(ctx, "Modifying hosts for subsystem=%s, nsid=%d: desired hosts=%v",
+			nvmeofData.SubsystemNQN, nvmeofData.NamespaceID, hosts)
+
+		err := gateway.UpdateHostsForSubsystem(ctx, nvmeofData.SubsystemNQN, hosts)
+		if err != nil {
+			log.ErrorLog(ctx, "Failed to update hosts for subsystem: %v", err)
+
+			return status.Errorf(codes.Internal, "failed to update hosts for subsystem: %v", err)
+		}
+
+		log.DebugLog(ctx, "Successfully modified hosts for volume %s", volumeID)
+
+		return nil
+	})
+}
+
+// parseQoSParameters extracts and parses QoS parameters from the given map.
+func parseQoSParameters(params map[string]string) (*nvmeof.NVMeoFQosVolume, error) {
+	qos := &nvmeof.NVMeoFQosVolume{}
+	hasAnyQoS := false
+
+	parseParam := func(key, name string, dest **uint64) error {
+		if val, exists := params[key]; exists && val != "" {
+			parsed, err := strconv.ParseUint(val, 10, 64)
+			if err != nil {
+				return fmt.Errorf("invalid %s: %w", name, err)
+			}
+			*dest = &parsed
+			hasAnyQoS = true
+		}
+
+		return nil
+	}
+
+	if err := parseParam(nvmeof.RwIosPerSecond, nvmeof.RwIosPerSecond, &qos.RwIosPerSecond); err != nil {
+		return nil, err
+	}
+	if err := parseParam(nvmeof.RwMbytesPerSecond, nvmeof.RwMbytesPerSecond, &qos.RwMbytesPerSecond); err != nil {
+		return nil, err
+	}
+	if err := parseParam(nvmeof.RMbytesPerSecond, nvmeof.RMbytesPerSecond, &qos.RMbytesPerSecond); err != nil {
+		return nil, err
+	}
+	if err := parseParam(nvmeof.WMbytesPerSecond, nvmeof.WMbytesPerSecond, &qos.WMbytesPerSecond); err != nil {
+		return nil, err
+	}
+
+	if !hasAnyQoS {
+		return nil, nil
+	}
+
+	return qos, nil
+}
+
 // modifyNVMeoFQoS handles NVMe-oF gateway QoS modification.
 func (cs *Server) modifyNVMeoFQoS(
 	ctx context.Context,
@@ -760,15 +818,16 @@ func (cs *Server) createNVMeoFResources(
 	req *csi.CreateVolumeRequest,
 	rbdPoolName,
 	rbdRadosNameSpace,
-	rbdImageName string,
+	rbdImageName,
+	volumeID string,
 ) (*nvmeof.NVMeoFVolumeData, error) {
 	// Step 1: Extract parameters (already validated)
 	params := req.GetParameters()
 
 	networkMask := params["networkMask"]
 	nvmeofData := &nvmeof.NVMeoFVolumeData{}
 
-	if err := nvmeofData.SetFromParameters(params); err != nil {
+	if err := nvmeofData.SetFromParameters(params, volumeID); err != nil {
 		return nil, fmt.Errorf("failed to set NVMe-oF volume data: %w", err)
 	}
 
@@ -782,7 +841,15 @@ func (cs *Server) createNVMeoFResources(
 
 		return nil, fmt.Errorf("failed to parse QoS parameters: %w", err)
 	}
+	// If VAC with hosts list is given (for external client)
+	// We need to parse the hosts list and pass it to the gateway for creating host entries
+	// and adding them to the subsystem.
+	hosts, err := parseHostsParameters(mutableParams)
+	if err != nil {
+		log.ErrorLog(ctx, "failed to parse NVMe-oF hosts parameters: %v", err)
 
+		return nil, fmt.Errorf("failed to parse hosts parameters: %w", err)
+	}
 	// Step 2: Connect to gateway
 	config, err := getGatewayConfigFromRequest(params)
 	if err != nil {
@@ -830,7 +897,16 @@ func (cs *Server) createNVMeoFResources(
 			return nvmeofData, fmt.Errorf("setting QoS limits failed: %w", err)
 		}
 	}
-
+	if hosts != nil {
+		log.DebugLog(ctx, "Adding hosts to subsystem: %v", hosts)
+		for _, host := range hosts {
+			// TODO - for now we create host with empty DH-CHAP keys,
+			// in the future we can extend the VAC parameters to allow passing DH-CHAP keys for each host if needed??
+			if err := gateway.AddHost(ctx, nvmeofData.SubsystemNQN, host, nvmeof.DHCHAPKeys{}); err != nil {
+				return nvmeofData, fmt.Errorf("adding host %s to subsystem failed: %w", host, err)
+			}
+		}
+	}
 	// Step 6: If using auto-listeners, query them back for storing in metadata
 	if networkMask != "" {
 		autoListeners, err := gateway.ListListeners(ctx, nvmeofData.SubsystemNQN)
@@ -1030,6 +1106,15 @@ func getHostNQNFromNodeID(nodeID string) (string, error) {
 	return prefix + nodeID, nil
 }
 
+// AllowHostNQNs is the VolumeAttributesClass mutable parameter key for specifying
+// a YAML list of host NQNs to allow access to a volume. Use "*" to allow any host.
+// Example:
+//
+//	allowHostNQNs: |
+//	  - nqn.2014-08.org.nvmexpress:host1
+//	  - nqn.2014-08.org.nvmexpress:host2
+const AllowHostNQNs = "allowHostNQNs"
+
 // VolumeContext metadata keys.
 const (
 	// NVMe-oF resource info.

internal/nvmeof/controller/qos_test.go …nvmeof/controller/mutable_params_test.gointernal/nvmeof/controller/qos_test.go renamed to internal/nvmeof/controller/mutable_params_test.go internal/nvmeof/controller/qos_test.go renamed to internal/nvmeof/controller/mutable_params_test.go

@@ -151,3 +151,109 @@ func TestParseQoSParameters(t *testing.T) {
 		})
 	}
 }
+
+func TestParseHostParameters(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		params      map[string]string
+		expected    []string
+		expectError bool
+	}{
+		{
+			name:     "no allowHostNQNs parameter",
+			params:   map[string]string{},
+			expected: nil,
+		},
+		{
+			name: "empty allowHostNQNs (deny all hosts)",
+			params: map[string]string{
+				AllowHostNQNs: "",
+			},
+			expected: nil, // Empty string returns nil slice, not empty slice
+		},
+		{
+			name: "single host NQN",
+			params: map[string]string{
+				AllowHostNQNs: `- nqn.2014-08.org.nvmexpress:host1`,
+			},
+			expected: []string{"nqn.2014-08.org.nvmexpress:host1"},
+		},
+		{
+			name: "multiple host NQNs",
+			params: map[string]string{
+				AllowHostNQNs: `- nqn.2014-08.org.nvmexpress:host1
+- nqn.2014-08.org.nvmexpress:host2
+- nqn.2014-08.org.nvmexpress:host3`,
+			},
+			expected: []string{
+				"nqn.2014-08.org.nvmexpress:host1",
+				"nqn.2014-08.org.nvmexpress:host2",
+				"nqn.2014-08.org.nvmexpress:host3",
+			},
+		},
+		{
+			name: "wildcard to allow any host",
+			params: map[string]string{
+				AllowHostNQNs: `- "*"`,
+			},
+			expected: []string{"*"},
+		},
+		{
+			name: "YAML list in flow style",
+			params: map[string]string{
+				AllowHostNQNs: `["nqn.2014-08.org.nvmexpress:host1", "nqn.2014-08.org.nvmexpress:host2"]`,
+			},
+			expected: []string{
+				"nqn.2014-08.org.nvmexpress:host1",
+				"nqn.2014-08.org.nvmexpress:host2",
+			},
+		},
+		{
+			name: "invalid YAML - not a list (plain string)",
+			params: map[string]string{
+				AllowHostNQNs: `nqn.2014-08.org.nvmexpress:host1`,
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid YAML - map instead of list",
+			params: map[string]string{
+				AllowHostNQNs: `host1: nqn.2014-08.org.nvmexpress:host1`,
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid YAML - unclosed quote",
+			params: map[string]string{
+				AllowHostNQNs: `- "nqn.2014-08.org.nvmexpress:host1`,
+			},
+			expectError: true,
+		},
+		{
+			name: "other parameters present but no allowHostNQNs",
+			params: map[string]string{
+				"someOtherParam":       "value",
+				"nvmeofRWIOsPerSecond": "10000",
+			},
+			expected: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			result, err := parseHostsParameters(tt.params)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Nil(t, result)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}