Merge pull request #1580 from pwschuurman/automated-cherry-pick-of-#1485-upstream-release-1.9

[release-1.9] Automated cherry pick of #1485: Update nvme /dev/ device search path to only identify

Author: mattcary

go.mod

@@ -7,6 +7,7 @@ require (
 	cloud.google.com/go/kms v1.12.1
 	github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.0
 	github.com/container-storage-interface/spec v1.6.0
+	github.com/google/go-cmp v0.5.9
 	github.com/google/uuid v1.3.0
 	github.com/kubernetes-csi/csi-proxy/client v1.1.1
 	github.com/kubernetes-csi/csi-test/v4 v4.4.0
@@ -50,7 +51,6 @@ require (
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
-	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/gofuzz v1.2.1-0.20210504230335-f78f29fc09ea // indirect
 	github.com/google/s2a-go v0.1.4 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.2.5 // indirect

pkg/deviceutils/device-utils.go

@@ -38,7 +38,7 @@ const (
 	diskSDPath           = "/dev/sd"
 	diskSDPattern        = "/dev/sd*"
 	diskNvmePath         = "/dev/nvme"
-	diskNvmePattern      = "/dev/nvme*"
+	diskNvmePattern      = "^/dev/nvme[0-9]+n[0-9]+$"
 	// How many times to retry for a consistent read of /proc/mounts.
 	maxListTries = 3
 	// Number of fields per line in /proc/mounts as per the fstab man page.

pkg/deviceutils/device-utils_test.go

@@ -1,7 +1,12 @@
 package deviceutils
 
 import (
+	"fmt"
+	"regexp"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 )
 
 func TestParseNvmeSerial(t *testing.T) {
@@ -45,3 +50,61 @@ func TestParseNvmeSerial(t *testing.T) {
 		}
 	}
 }
+
+func less(a, b fmt.Stringer) bool {
+	return a.String() < b.String()
+}
+
+// Test that the NVMe Regex matches expected paths
+// Note that this only tests the regex, not the actual path finding codepath.
+// The real codepath uses filepath.Glob(), which doesn't have an easy way to mock out local
+// directory paths (eg: using a subdirectory prefix).
+// We could use a recursive child process, setting a root UID, and use Chroot (which requires superuser).
+// This is done in upstream golang tests, but this adds additional complexity
+// and may prevent our tests from running on all platforms. See the following test for an example:
+// https://github.com/golang/go/blob/d33548d178016122726342911f8e15016a691472/src/syscall/exec_linux_test.go#L250
+func TestDiskNvmePattern(t *testing.T) {
+	nvmeDiskRegex := regexp.MustCompile(diskNvmePattern)
+
+	testCases := []struct {
+		paths     []string
+		wantPaths []string
+	}{
+		{
+			paths: []string{
+				"/dev/nvme0n1p15",
+				"/dev/nvme0n1p14",
+				"/dev/nvme0n1p1",
+				"/dev/nvme0n1",
+				"/dev/nvme0n2",
+				"/dev/nvme0",
+			},
+			wantPaths: []string{
+				"/dev/nvme0n1",
+				"/dev/nvme0n2",
+			},
+		},
+		{
+			paths: []string{
+				"/dev/nvme1",
+				"/dev/nvme0n1p15",
+				"/dev/nvme0n1p14",
+				"/dev/nvme0n1p1",
+				"/dev/nvme2",
+			},
+			wantPaths: []string{},
+		},
+	}
+
+	for _, tc := range testCases {
+		gotPaths := []string{}
+		for _, path := range tc.paths {
+			if nvmeDiskRegex.MatchString(path) {
+				gotPaths = append(gotPaths, path)
+			}
+		}
+		if diff := cmp.Diff(gotPaths, tc.wantPaths, cmpopts.SortSlices(less)); diff != "" {
+			t.Errorf("Unexpected NVMe device paths (-got, +want):\n%s", diff)
+		}
+	}
+}