Bypass kernel DNS upcall in NFS mount path (CRUSOE-70481)

The CSI driver previously passed remoteports=dns to the kernel NFS client
when the hostname was nfs.crusoecloudcompute.com, which triggered the
dns_resolver keyring upcall through OVN's DNS intercept. That intercept
produced three distinct production failure modes: ENOKEY (required key
not available), EPROTONOSUPPORT (requested NFS version or transport
protocol is not supported), and the musl REFUSED bug from INC-450.

Resolve the hostname in-process at NodePublishVolume time, filter out
unspecified (::, 0.0.0.0) and non-IPv4 results, and pass an explicit IPv4
list in remoteports=ip,ip,ip so the kernel never invokes the keyring
upcall. The first IP also becomes the host portion of the mount source
("<ip>:/volumes/<id>"), so busybox-mount in the Alpine CSI pod no longer
needs to do its own getaddrinfo — closing the INC-450 musl surface too.

Also invert ResolveNFSTarget's preference so disk.Vips is tried before
disk.DnsName: the VIP list is the authoritative VIP set from the storage
API and doesn't round-trip through OVN at all.

On resolver failure the helper logs a warning and falls back to the
legacy "dns" pair so a bug in the new path can never make the situation
worse than before this change.

Author: npujari-crusoe

internal/crusoe/disk.go

@@ -158,30 +158,50 @@ func CheckDiskMatchesRequest(disk *crusoeapi.DiskV1Alpha5,
 // ResolveNFSTarget returns the NFS host and remoteports value to use when
 // mounting the disk based on the data path connectivity fields populated by
 // the storage API (added in CRUSOE-60428). It returns ok=false when the disk
-// carries neither dns_name nor vips, signalling that the caller should fall
+// carries neither vips nor dns_name, signalling that the caller should fall
 // back to a static configuration.
 //
-// vips is contracted to be a 2-element [startIP, endIP] range; we tolerate
+// Vips is preferred over DnsName (CRUSOE-70481): the VIP list is the
+// authoritative VIP set from the storage API and bypasses OVN's DNS intercept,
+// which has produced ENOKEY, EPROTONOSUPPORT and the musl REFUSED failure
+// modes from INC-450 in production. When vips is populated we return the
+// full set joined by "," so the kernel's NFS client receives an explicit IP
+// list in remoteports= and never invokes the dns_resolver keyring upcall.
+// Only when vips is empty do we fall back to dns_name (which the caller will
+// resolve in-process before passing to mount).
+//
+// Vips is contracted to be a 2-element [startIP, endIP] range; we tolerate
 // other lengths defensively but warn so the discrepancy is visible.
 func ResolveNFSTarget(disk *crusoeapi.DiskV1Alpha5) (host, remotePorts string, ok bool) {
+	if len(disk.Vips) > 0 {
+		return resolveFromVIPs(disk)
+	}
 	if disk.DnsName != "" {
 		return disk.DnsName, "dns", true
 	}
+
+	return "", "", false
+}
+
+// resolveFromVIPs builds an explicit IP-list remoteports string from a disk's
+// vips field. The first IP doubles as the mount-source host so busybox-mount
+// in the Alpine CSI pod never has to do its own getaddrinfo either.
+func resolveFromVIPs(disk *crusoeapi.DiskV1Alpha5) (host, remotePorts string, ok bool) {
 	switch len(disk.Vips) {
 	case 0:
 		return "", "", false
 	case ExpectedVIPRangeLen:
-		return disk.Vips[0], fmt.Sprintf("%s-%s", disk.Vips[0], disk.Vips[1]), true
+		return disk.Vips[0], strings.Join(disk.Vips, ","), true
 	case 1:
 		klog.Warningf("disk %s returned %d vip(s), expected %d ([startIP, endIP]); using single vip %q",
 			disk.Id, len(disk.Vips), ExpectedVIPRangeLen, disk.Vips[0])
 
 		return disk.Vips[0], disk.Vips[0], true
 	default:
-		klog.Warningf("disk %s returned %d vips, expected %d ([startIP, endIP]); using first and last %q-%q",
-			disk.Id, len(disk.Vips), ExpectedVIPRangeLen, disk.Vips[0], disk.Vips[len(disk.Vips)-1])
+		klog.Warningf("disk %s returned %d vips, expected %d ([startIP, endIP]); joining all into remoteports",
+			disk.Id, len(disk.Vips), ExpectedVIPRangeLen)
 
-		return disk.Vips[0], fmt.Sprintf("%s-%s", disk.Vips[0], disk.Vips[len(disk.Vips)-1]), true
+		return disk.Vips[0], strings.Join(disk.Vips, ","), true
 	}
 }
 

internal/crusoe/disk_test.go

@@ -32,20 +32,20 @@ func TestResolveNFSTarget(t *testing.T) {
 			wantOK:          true,
 		},
 		{
-			name: "dns name preferred over vips",
+			name: "vips preferred over dns name",
 			disk: crusoeapi.DiskV1Alpha5{
 				DnsName: "nfs.crusoecloudcompute.com",
 				Vips:    []string{"1.2.3.4", "1.2.3.8"},
 			},
-			wantHost:        "nfs.crusoecloudcompute.com",
-			wantRemotePorts: "dns",
+			wantHost:        "1.2.3.4",
+			wantRemotePorts: "1.2.3.4,1.2.3.8",
 			wantOK:          true,
 		},
 		{
-			name:            "vip range produces hyphenated remoteports",
+			name:            "vip range produces comma-joined remoteports",
 			disk:            crusoeapi.DiskV1Alpha5{Vips: []string{"1.2.3.4", "1.2.3.8"}},
 			wantHost:        "1.2.3.4",
-			wantRemotePorts: "1.2.3.4-1.2.3.8",
+			wantRemotePorts: "1.2.3.4,1.2.3.8",
 			wantOK:          true,
 		},
 		{
@@ -56,10 +56,10 @@ func TestResolveNFSTarget(t *testing.T) {
 			wantOK:          true,
 		},
 		{
-			name:            "more than two vips uses first and last",
+			name:            "more than two vips joins the entire list",
 			disk:            crusoeapi.DiskV1Alpha5{Vips: []string{"1.2.3.4", "1.2.3.5", "1.2.3.8"}},
 			wantHost:        "1.2.3.4",
-			wantRemotePorts: "1.2.3.4-1.2.3.8",
+			wantRemotePorts: "1.2.3.4,1.2.3.5,1.2.3.8",
 			wantOK:          true,
 		},
 	}

internal/node/fs/export_test.go

@@ -0,0 +1,29 @@
+// Package fs export hooks for in-package tests.
+//
+// This file uses the conventional _test.go suffix so it is only compiled into
+// the test binary. It exposes package-private symbols (materializeNFSTarget,
+// lookupIP) under exported names so resolve_test.go can live in the fs_test
+// (black-box) package alongside the other tests in this directory.
+package fs
+
+import "net"
+
+// MaterializeNFSTarget is a test-only exported handle for materializeNFSTarget.
+//
+//nolint:gochecknoglobals // deliberate test seam
+var MaterializeNFSTarget = materializeNFSTarget
+
+// SetLookupIP swaps the package-private DNS lookup function for the duration
+// of a test and returns the previous value so callers can restore it via
+// t.Cleanup or defer.
+func SetLookupIP(fn func(host string) ([]net.IP, error)) func(host string) ([]net.IP, error) {
+	prev := lookupIP
+	lookupIP = fn
+
+	return prev
+}
+
+// ErrNoUsableNFSAddressForTest re-exports the sentinel error from resolve.go
+// so tests can match it with errors.Is without taking a package-private
+// dependency.
+var ErrNoUsableNFSAddressForTest = ErrNoUsableNFSAddress

internal/node/fs/fs_node.go

@@ -94,9 +94,18 @@ func (d *Node) NodePublishVolume(ctx context.Context, request *csi.NodePublishVo
 
 // resolveNFSTarget determines the NFS host and remoteports value to use when
 // publishing a volume. It prefers per-disk data path connectivity returned by
-// the storage API (dns_name / vips, CRUSOE-60428), falling back to legacy
+// the storage API (vips / dns_name, CRUSOE-60428), falling back to legacy
 // configuration (the ICAT secondary-cluster DNS escape hatch and finally the
 // CLI-flag defaults) when the API does not yet populate those fields.
+//
+// Whichever branch produces the (host, remoteports) pair, the result is run
+// through materializeNFSTarget so that the kernel never receives the literal
+// "dns" remoteports value (CRUSOE-70481): doing DNS resolution in-process
+// avoids the kernel dns_resolver keyring upcall (which has produced ENOKEY,
+// EPROTONOSUPPORT and the musl REFUSED bug from INC-450 in production). On
+// resolution failure we log a warning and fall through with the original
+// inputs so behaviour degrades to the prior code path rather than failing
+// the mount outright.
 func (d *Node) resolveNFSTarget(
 	ctx context.Context, volumeID string, nfsEnabled bool,
 ) (nfsHost, nfsRemotePorts string) {
@@ -109,7 +118,7 @@ func (d *Node) resolveNFSTarget(
 			klog.Infof("Resolved NFS target from disk API for %s: host=%s remoteports=%s",
 				volumeID, host, remotePorts)
 
-			return host, remotePorts
+			return materializeOrPassthrough(host, remotePorts)
 		} else {
 			klog.Warningf("disk %s did not return data path connectivity fields; falling back to defaults",
 				volumeID)
@@ -128,7 +137,26 @@ func (d *Node) resolveNFSTarget(
 			nfsHost, nfsRemotePorts)
 	}
 
-	return nfsHost, nfsRemotePorts
+	return materializeOrPassthrough(nfsHost, nfsRemotePorts)
+}
+
+// materializeOrPassthrough resolves a "dns" remoteports target to an explicit
+// IPv4 list. On any resolver error it logs a warning and returns the original
+// inputs so the mount still attempts the legacy DNS-via-keyring path.
+func materializeOrPassthrough(host, remotePorts string) (resolvedHost, resolvedRemotePorts string) {
+	newHost, newRemotePorts, err := materializeNFSTarget(host, remotePorts)
+	if err != nil {
+		klog.Warningf("failed to materialize NFS target host=%s remoteports=%s, passing through: %s",
+			host, remotePorts, err.Error())
+
+		return host, remotePorts
+	}
+	if newHost != host || newRemotePorts != remotePorts {
+		klog.Infof("Materialized NFS target: host=%s remoteports=%s (was host=%s remoteports=%s)",
+			newHost, newRemotePorts, host, remotePorts)
+	}
+
+	return newHost, newRemotePorts
 }
 
 func (d *Node) useDNSForMount(ctx context.Context) bool {

internal/node/fs/resolve.go

@@ -0,0 +1,71 @@
+package fs
+
+import (
+	"errors"
+	"fmt"
+	"net"
+	"strings"
+)
+
+// ErrNoUsableNFSAddress is returned when DNS resolution yields no usable IPv4
+// address for an NFS mount target. Callers can use this to distinguish a
+// resolver failure from a programmer error.
+var ErrNoUsableNFSAddress = errors.New("no usable IPv4 address for NFS target")
+
+// lookupIP is a package-level indirection over net.LookupIP so tests can stub
+// out DNS resolution without making real network calls. Production callers
+// must not reassign this outside of tests.
+//
+//nolint:gochecknoglobals // deliberate package-private test seam
+var lookupIP = net.LookupIP
+
+// materializeNFSTarget converts a (host, remotePorts) pair into an explicit
+// IPv4-only mount target, bypassing the kernel dns_resolver keyring upcall
+// that the NFS client would otherwise trigger when remoteports=dns is passed.
+//
+// If remotePorts is anything other than the literal "dns" (i.e. it is already
+// a comma-separated or hyphenated list of IPs), the inputs are returned
+// unchanged and no DNS lookup is performed.
+//
+// If remotePorts == "dns", the hostname is resolved via lookupIP, IPv4-mapped
+// IPv6 addresses are normalized to IPv4, unspecified addresses (::, 0.0.0.0)
+// and non-IPv4 results are filtered out, and the result is returned as:
+//   - newHost: the first valid IPv4, suitable as the host portion of the
+//     mount source string ("<ip>:/volumes/<id>"), so that busybox-mount in
+//     the Alpine CSI pod never needs to do its own getaddrinfo (closes the
+//     INC-450 musl REFUSED surface).
+//   - newRemotePorts: comma-separated list of all valid IPv4s, suitable for
+//     the kernel's remoteports= option.
+//
+// Returns ErrNoUsableNFSAddress (wrapped) when the hostname has no usable
+// IPv4 records after filtering. Callers should log the error and fall back
+// to legacy "dns" behaviour rather than failing the mount outright so that
+// a helper bug never makes the situation worse than before this change.
+func materializeNFSTarget(host, remotePorts string) (newHost, newRemotePorts string, err error) {
+	if remotePorts != dnsRemotePorts {
+		return host, remotePorts, nil
+	}
+
+	addrs, err := lookupIP(host)
+	if err != nil {
+		return host, remotePorts, fmt.Errorf("dns lookup for %q failed: %w", host, err)
+	}
+
+	ipv4s := make([]string, 0, len(addrs))
+	for _, ip := range addrs {
+		if ip.IsUnspecified() {
+			continue
+		}
+		v4 := ip.To4()
+		if v4 == nil {
+			continue
+		}
+		ipv4s = append(ipv4s, v4.String())
+	}
+
+	if len(ipv4s) == 0 {
+		return host, remotePorts, fmt.Errorf("%w: %s", ErrNoUsableNFSAddress, host)
+	}
+
+	return ipv4s[0], strings.Join(ipv4s, ","), nil
+}