Skip to content

Commit 6f8aef8

Browse files
gadididigadididi
committed
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>
1 parent 64b7ce9 commit 6f8aef8

4 files changed

Lines changed: 227 additions & 46 deletions

File tree

‎internal/nvmeof/controller/controllerserver.go‎

Lines changed: 133 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"strconv"
2626

2727
"github.com/container-storage-interface/spec/lib/go/csi"
28+
"github.com/ghodss/yaml"
2829
"google.golang.org/grpc/codes"
2930
"google.golang.org/grpc/status"
3031

@@ -166,7 +167,7 @@ func (cs *Server) CreateVolume(
166167
}
167168
}()
168169

169-
nvmeofData, err = cs.createNVMeoFResources(ctx, req, rbdPoolName, rbdRadosNameSpace, rbdImageName)
170+
nvmeofData, err = cs.createNVMeoFResources(ctx, req, rbdPoolName, rbdRadosNameSpace, rbdImageName, volumeID)
170171
if err != nil {
171172
log.ErrorLog(ctx, "NVMe-oF resource setup failed for volumeID %s: %v", volumeID, err)
172173

@@ -352,11 +353,22 @@ func (cs *Server) ControllerModifyVolume(
352353

353354
return nil, status.Errorf(codes.InvalidArgument, "failed to parse QoS parameters: %v", err)
354355
}
356+
hostsList, err := parseHostsParameters(params)
357+
if err != nil {
358+
log.ErrorLog(ctx, "failed to parse NVMe-oF hosts parameters: %v", err)
359+
360+
return nil, status.Errorf(codes.InvalidArgument, "failed to parse hosts parameters: %v", err)
361+
}
355362
if nvmeofQoS != nil {
356363
if err := cs.modifyNVMeoFQoS(ctx, req, nvmeofQoS); err != nil {
357364
return nil, err
358365
}
359366
}
367+
if hostsList != nil {
368+
if err := cs.modifyNVMeoFHosts(ctx, req, hostsList); err != nil {
369+
return nil, err
370+
}
371+
}
360372

361373
return &csi.ControllerModifyVolumeResponse{}, nil
362374
}
@@ -424,14 +436,10 @@ func validateDHCHAPParameter(dhchapMode string) error {
424436
func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
425437
// Validate required parameters
426438
params := req.GetParameters()
427-
requiredParams := []string{
428-
"subsystemNQN", "nvmeofGatewayAddress",
429-
}
430-
for _, param := range requiredParams {
431-
if params[param] == "" {
432-
return fmt.Errorf("missing required parameter: %s", param)
433-
}
439+
if params["nvmeofGatewayAddress"] == "" {
440+
return errors.New("missing required parameter nvmeofGatewayAddress")
434441
}
442+
435443
// Validate listeners JSON if provided
436444
countOfListeners, err := validateListenersParameter(params["listeners"])
437445
if err != nil {
@@ -466,6 +474,11 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
466474
if err != nil {
467475
return fmt.Errorf("invalid NVMe-oF QoS parameters: %w", err)
468476
}
477+
478+
_, err = parseHostsParameters(mutableParams)
479+
if err != nil {
480+
return fmt.Errorf("invalid NVMe-oF hosts parameters (for external clients): %w", err)
481+
}
469482
err = validateDHCHAPParameter(params["dhchapMode"])
470483
if err != nil {
471484
return err
@@ -532,42 +545,24 @@ func validateNetworkMask(networkMask string) error {
532545
return nil
533546
}
534547

535-
// parseQoSParameters extracts and parses QoS parameters from the given map.
536-
func parseQoSParameters(params map[string]string) (*nvmeof.NVMeoFQosVolume, error) {
537-
qos := &nvmeof.NVMeoFQosVolume{}
538-
hasAnyQoS := false
539-
540-
parseParam := func(key, name string, dest **uint64) error {
541-
if val, exists := params[key]; exists && val != "" {
542-
parsed, err := strconv.ParseUint(val, 10, 64)
543-
if err != nil {
544-
return fmt.Errorf("invalid %s: %w", name, err)
545-
}
546-
*dest = &parsed
547-
hasAnyQoS = true
548-
}
549-
550-
return nil
551-
}
552-
553-
if err := parseParam(nvmeof.RwIosPerSecond, nvmeof.RwIosPerSecond, &qos.RwIosPerSecond); err != nil {
554-
return nil, err
555-
}
556-
if err := parseParam(nvmeof.RwMbytesPerSecond, nvmeof.RwMbytesPerSecond, &qos.RwMbytesPerSecond); err != nil {
557-
return nil, err
548+
// parseHostsParameters parses the hosts yaml list parameter and validates its contents.
549+
// It returns a slice of hostNQNs or an error if the YAML is invalid.
550+
// allowHostNQNs entry can be empty, in that case it means no hosts are allowed to access
551+
// the subsystem (empty allow list).
552+
func parseHostsParameters(params map[string]string) ([]string, error) {
553+
allowHostNQNs, exists := params[AllowHostNQNs]
554+
if !exists {
555+
return nil, nil
558556
}
559-
if err := parseParam(nvmeof.RMbytesPerSecond, nvmeof.RMbytesPerSecond, &qos.RMbytesPerSecond); err != nil {
560-
return nil, err
557+
var allowHostsList []string
558+
if allowHostNQNs == "" {
559+
return allowHostsList, nil
561560
}
562-
if err := parseParam(nvmeof.WMbytesPerSecond, nvmeof.WMbytesPerSecond, &qos.WMbytesPerSecond); err != nil {
563-
return nil, err
561+
if err := yaml.Unmarshal([]byte(allowHostNQNs), &allowHostsList); err != nil {
562+
return nil, fmt.Errorf("invalid %s: must be a YAML list of strings: %w", AllowHostNQNs, err)
564563
}
565564

566-
if !hasAnyQoS {
567-
return nil, nil
568-
}
569-
570-
return qos, nil
565+
return allowHostsList, nil
571566
}
572567

573568
// withGatewayConnection is a helper that manages the common pattern of:
@@ -628,6 +623,74 @@ func (cs *Server) withGatewayConnection(
628623
return fn(ctx, gateway, nvmeofData)
629624
}
630625

626+
// modifyNVMeoFHosts handles adding or removing hosts from the subsystem based on the provided list of host NQNs.
627+
func (cs *Server) modifyNVMeoFHosts(ctx context.Context, req *csi.ControllerModifyVolumeRequest, hosts []string) error {
628+
volumeID := req.GetVolumeId()
629+
if len(hosts) == 0 {
630+
log.DebugLog(ctx, "No hosts to add or remove for volume %s", volumeID)
631+
632+
return nil
633+
}
634+
635+
return cs.withGatewayConnection(ctx, req, volumeID, func(
636+
ctx context.Context,
637+
gateway *nvmeof.GatewayRpcClient,
638+
nvmeofData *nvmeof.NVMeoFVolumeData,
639+
) error {
640+
log.DebugLog(ctx, "Modifying hosts for subsystem=%s, nsid=%d: desired hosts=%v",
641+
nvmeofData.SubsystemNQN, nvmeofData.NamespaceID, hosts)
642+
643+
err := gateway.UpdateHostsForSubsystem(ctx, nvmeofData.SubsystemNQN, hosts)
644+
if err != nil {
645+
log.ErrorLog(ctx, "Failed to update hosts for subsystem: %v", err)
646+
647+
return status.Errorf(codes.Internal, "failed to update hosts for subsystem: %v", err)
648+
}
649+
650+
log.DebugLog(ctx, "Successfully modified hosts for volume %s", volumeID)
651+
652+
return nil
653+
})
654+
}
655+
656+
// parseQoSParameters extracts and parses QoS parameters from the given map.
657+
func parseQoSParameters(params map[string]string) (*nvmeof.NVMeoFQosVolume, error) {
658+
qos := &nvmeof.NVMeoFQosVolume{}
659+
hasAnyQoS := false
660+
661+
parseParam := func(key, name string, dest **uint64) error {
662+
if val, exists := params[key]; exists && val != "" {
663+
parsed, err := strconv.ParseUint(val, 10, 64)
664+
if err != nil {
665+
return fmt.Errorf("invalid %s: %w", name, err)
666+
}
667+
*dest = &parsed
668+
hasAnyQoS = true
669+
}
670+
671+
return nil
672+
}
673+
674+
if err := parseParam(nvmeof.RwIosPerSecond, nvmeof.RwIosPerSecond, &qos.RwIosPerSecond); err != nil {
675+
return nil, err
676+
}
677+
if err := parseParam(nvmeof.RwMbytesPerSecond, nvmeof.RwMbytesPerSecond, &qos.RwMbytesPerSecond); err != nil {
678+
return nil, err
679+
}
680+
if err := parseParam(nvmeof.RMbytesPerSecond, nvmeof.RMbytesPerSecond, &qos.RMbytesPerSecond); err != nil {
681+
return nil, err
682+
}
683+
if err := parseParam(nvmeof.WMbytesPerSecond, nvmeof.WMbytesPerSecond, &qos.WMbytesPerSecond); err != nil {
684+
return nil, err
685+
}
686+
687+
if !hasAnyQoS {
688+
return nil, nil
689+
}
690+
691+
return qos, nil
692+
}
693+
631694
// modifyNVMeoFQoS handles NVMe-oF gateway QoS modification.
632695
func (cs *Server) modifyNVMeoFQoS(
633696
ctx context.Context,
@@ -759,15 +822,16 @@ func (cs *Server) createNVMeoFResources(
759822
req *csi.CreateVolumeRequest,
760823
rbdPoolName,
761824
rbdRadosNameSpace,
762-
rbdImageName string,
825+
rbdImageName,
826+
volumeID string,
763827
) (*nvmeof.NVMeoFVolumeData, error) {
764828
// Step 1: Extract parameters (already validated)
765829
params := req.GetParameters()
766830

767831
networkMask := params["networkMask"]
768832
nvmeofData := &nvmeof.NVMeoFVolumeData{}
769833

770-
if err := nvmeofData.SetFromParameters(params); err != nil {
834+
if err := nvmeofData.SetFromParameters(params, volumeID); err != nil {
771835
return nil, fmt.Errorf("failed to set NVMe-oF volume data: %w", err)
772836
}
773837

@@ -781,7 +845,15 @@ func (cs *Server) createNVMeoFResources(
781845

782846
return nil, fmt.Errorf("failed to parse QoS parameters: %w", err)
783847
}
848+
// If VAC with hosts list is given (for external client)
849+
// We need to parse the hosts list and pass it to the gateway for creating host entries
850+
// and adding them to the subsystem.
851+
hosts, err := parseHostsParameters(mutableParams)
852+
if err != nil {
853+
log.ErrorLog(ctx, "failed to parse NVMe-oF hosts parameters: %v", err)
784854

855+
return nil, fmt.Errorf("failed to parse hosts parameters: %w", err)
856+
}
785857
// Step 2: Connect to gateway
786858
config, err := getGatewayConfigFromRequest(params)
787859
if err != nil {
@@ -829,7 +901,16 @@ func (cs *Server) createNVMeoFResources(
829901
return nvmeofData, fmt.Errorf("setting QoS limits failed: %w", err)
830902
}
831903
}
832-
904+
if hosts != nil {
905+
log.DebugLog(ctx, "Adding hosts to subsystem: %v", hosts)
906+
for _, host := range hosts {
907+
// TODO - for now we create host with empty DH-CHAP keys,
908+
// in the future we can extend the VAC parameters to allow passing DH-CHAP keys for each host if needed??
909+
if err := gateway.AddHost(ctx, nvmeofData.SubsystemNQN, host, nvmeof.DHCHAPKeys{}); err != nil {
910+
return nvmeofData, fmt.Errorf("adding host %s to subsystem failed: %w", host, err)
911+
}
912+
}
913+
}
833914
// Step 6: If using auto-listeners, query them back for storing in metadata
834915
if networkMask != "" {
835916
autoListeners, err := gateway.ListListeners(ctx, nvmeofData.SubsystemNQN)
@@ -1029,6 +1110,15 @@ func getHostNQNFromNodeID(nodeID string) (string, error) {
10291110
return prefix + nodeID, nil
10301111
}
10311112

1113+
// AllowHostNQNs is the VolumeAttributesClass mutable parameter key for specifying
1114+
// a YAML list of host NQNs to allow access to a volume. Use "*" to allow any host.
1115+
// Example:
1116+
//
1117+
// allowHostNQNs: |
1118+
// - nqn.2014-08.org.nvmexpress:host1
1119+
// - nqn.2014-08.org.nvmexpress:host2
1120+
const AllowHostNQNs = "allowHostNQNs"
1121+
10321122
// VolumeContext metadata keys.
10331123
const (
10341124
// NVMe-oF resource info.

‎internal/nvmeof/nvmeof.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"crypto/rand"
2222
"fmt"
2323
"math/big"
24+
"slices"
2425
"syscall"
2526

2627
pb "github.com/ceph/ceph-nvmeof/lib/go/nvmeof"
@@ -605,6 +606,74 @@ func (gw *GatewayRpcClient) RemoveHost(ctx context.Context, subsystemNQN, hostNQ
605606
}
606607
}
607608

609+
// ListHosts lists all hosts in a subsystem.
610+
// Returns a slice of host NQNs that have access to the subsystem.
611+
func (gw *GatewayRpcClient) ListHosts(ctx context.Context, subsystemNQN string) ([]string, error) {
612+
log.DebugLog(ctx, "Listing hosts in subsystem %s", subsystemNQN)
613+
614+
req := &pb.ListHostsReq{
615+
Subsystem: subsystemNQN,
616+
}
617+
618+
resp, err := gw.client.ListHosts(ctx, req)
619+
if err != nil {
620+
return nil, fmt.Errorf("failed to list hosts in subsystem %s: %w", subsystemNQN, err)
621+
}
622+
if resp.GetStatus() != 0 {
623+
return nil, fmt.Errorf("gateway ListHosts returned error (status=%d): %s",
624+
resp.GetStatus(), resp.GetErrorMessage())
625+
}
626+
627+
// Extract host NQNs from response
628+
hosts := make([]string, 0, len(resp.GetHosts()))
629+
for _, host := range resp.GetHosts() {
630+
hosts = append(hosts, host.GetNqn())
631+
}
632+
633+
log.DebugLog(ctx, "Listed %d hosts in subsystem %s", len(hosts), subsystemNQN)
634+
635+
return hosts, nil
636+
}
637+
638+
// UpdateHostsForSubsystem reconciles the hosts in a subsystem to match the desired list.
639+
// It lists current hosts, then adds/removes hosts to ensure the subsystem has exactly
640+
// the hosts specified in desiredHosts.
641+
func (gw *GatewayRpcClient) UpdateHostsForSubsystem(
642+
ctx context.Context,
643+
subsystemNQN string,
644+
desiredHosts []string,
645+
) error {
646+
currentHosts, err := gw.ListHosts(ctx, subsystemNQN)
647+
if err != nil {
648+
return fmt.Errorf("failed to list current hosts: %w", err)
649+
}
650+
651+
log.DebugLog(ctx, "Host reconciliation for subsystem %s: current=%v, desired=%v",
652+
subsystemNQN, currentHosts, desiredHosts)
653+
654+
for _, host := range currentHosts {
655+
if !slices.Contains(desiredHosts, host) {
656+
log.DebugLog(ctx, "Removing host %s from subsystem %s", host, subsystemNQN)
657+
if err := gw.RemoveHost(ctx, subsystemNQN, host); err != nil {
658+
return fmt.Errorf("failed to remove host %s: %w", host, err)
659+
}
660+
}
661+
}
662+
663+
for _, host := range desiredHosts {
664+
if !slices.Contains(currentHosts, host) {
665+
log.DebugLog(ctx, "Adding host %s to subsystem %s", host, subsystemNQN)
666+
// Note: AddHost requires DHCHAPKeys, but for now we pass empty keys
667+
// TODO: Support DH-CHAP keys if needed for host updates
668+
if err := gw.AddHost(ctx, subsystemNQN, host, DHCHAPKeys{}); err != nil {
669+
return fmt.Errorf("failed to add host %s: %w", host, err)
670+
}
671+
}
672+
}
673+
674+
return nil
675+
}
676+
608677
// List namespaces in a subsystem.
609678
func (gw *GatewayRpcClient) ListNamespaces(ctx context.Context, subsystemNQN string) (*pb.NamespacesInfo, error) {
610679
log.DebugLog(ctx, "Listing namespaces in subsystem %s", subsystemNQN)

‎internal/nvmeof/volume.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,12 @@ func SetupListeners(listenersJSON string) ([]ListenerDetails, error) {
8484
// It extracts the subsystem NQN, gateway management info, security config, and
8585
// listener info from the parameters.
8686
// It also applies default values to listeners if necessary.
87-
func (v *NVMeoFVolumeData) SetFromParameters(parameters map[string]string) error {
87+
func (v *NVMeoFVolumeData) SetFromParameters(parameters map[string]string, volumeID string) error {
8888
// set subsystem NQN
8989
v.SubsystemNQN = parameters["subsystemNQN"]
90-
90+
if v.SubsystemNQN == "" {
91+
v.SubsystemNQN = "nqn.2016-06.io.ceph:subsystem." + volumeID
92+
}
9193
// set gw management info
9294
if nvmeofGatewayPortStr := parameters["nvmeofGatewayPort"]; nvmeofGatewayPortStr != "" {
9395
parsed, err := strconv.ParseUint(nvmeofGatewayPortStr, 10, 32)

0 commit comments

Comments
 (0)