storage: check CSIDriver.Spec.VolumeLifecycleModes

Using a "normal" CSI driver for an inline ephemeral volume may have
unexpected and potentially harmful effects when the driver gets a
NodePublishVolume call that it isn't expecting. To prevent that mistake,
driver deployments for a driver that supports such volumes must:
- deploy a CSIDriver object for the driver
- set CSIDriver.Spec.VolumeLifecycleModes such that it contains "ephemeral"

The default for that field is "persistent", so existing deployments
continue to work and are automatically protected against incorrect
usage.

For the E2E tests we need a way to specify the driver mode. The
existing cluster-driver-registrar doesn't support that and also was
deprecated, so we stop using it altogether and instead deploy and
patch a CSIDriver object.

Author: pohly

pkg/volume/csi/csi_attacher_test.go

@@ -342,9 +342,9 @@ func TestAttacherWithCSIDriver(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			fakeClient := fakeclient.NewSimpleClientset(
-				getTestCSIDriver("not-attachable", nil, &bFalse),
-				getTestCSIDriver("attachable", nil, &bTrue),
-				getTestCSIDriver("nil", nil, nil),
+				getTestCSIDriver("not-attachable", nil, &bFalse, nil),
+				getTestCSIDriver("attachable", nil, &bTrue, nil),
+				getTestCSIDriver("nil", nil, nil, nil),
 			)
 			plug, fakeWatcher, tmpDir, _ := newTestWatchPlugin(t, fakeClient)
 			defer os.RemoveAll(tmpDir)
@@ -430,9 +430,9 @@ func TestAttacherWaitForVolumeAttachmentWithCSIDriver(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			fakeClient := fakeclient.NewSimpleClientset(
-				getTestCSIDriver("not-attachable", nil, &bFalse),
-				getTestCSIDriver("attachable", nil, &bTrue),
-				getTestCSIDriver("nil", nil, nil),
+				getTestCSIDriver("not-attachable", nil, &bFalse, nil),
+				getTestCSIDriver("attachable", nil, &bTrue, nil),
+				getTestCSIDriver("nil", nil, nil, nil),
 			)
 			plug, tmpDir := newTestPlugin(t, fakeClient)
 			defer os.RemoveAll(tmpDir)

pkg/volume/csi/csi_mounter.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/klog"
 
 	api "k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1beta1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -46,32 +47,32 @@ var (
 		driverName,
 		nodeName,
 		attachmentID,
-		csiVolumeMode string
+		volumeLifecycleMode string
 	}{
 		"specVolID",
 		"volumeHandle",
 		"driverName",
 		"nodeName",
 		"attachmentID",
-		"csiVolumeMode",
+		"volumeLifecycleMode",
 	}
 )
 
 type csiMountMgr struct {
 	csiClientGetter
-	k8s            kubernetes.Interface
-	plugin         *csiPlugin
-	driverName     csiDriverName
-	csiVolumeMode  csiVolumeMode
-	volumeID       string
-	specVolumeID   string
-	readOnly       bool
-	spec           *volume.Spec
-	pod            *api.Pod
-	podUID         types.UID
-	options        volume.VolumeOptions
-	publishContext map[string]string
-	kubeVolHost    volume.KubeletVolumeHost
+	k8s                 kubernetes.Interface
+	plugin              *csiPlugin
+	driverName          csiDriverName
+	volumeLifecycleMode storage.VolumeLifecycleMode
+	volumeID            string
+	specVolumeID        string
+	readOnly            bool
+	spec                *volume.Spec
+	pod                 *api.Pod
+	podUID              types.UID
+	options             volume.VolumeOptions
+	publishContext      map[string]string
+	kubeVolHost         volume.KubeletVolumeHost
 	volume.MetricsProvider
 }
 
@@ -145,8 +146,8 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
 		if !utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) {
 			return fmt.Errorf("CSIInlineVolume feature required")
 		}
-		if c.csiVolumeMode != ephemeralVolumeMode {
-			return fmt.Errorf("unexpected volume mode: %s", c.csiVolumeMode)
+		if c.volumeLifecycleMode != storage.VolumeLifecycleEphemeral {
+			return fmt.Errorf("unexpected volume mode: %s", c.volumeLifecycleMode)
 		}
 		if volSrc.FSType != nil {
 			fsType = *volSrc.FSType
@@ -160,8 +161,8 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
 			secretRef = &api.SecretReference{Name: secretName, Namespace: ns}
 		}
 	case pvSrc != nil:
-		if c.csiVolumeMode != persistentVolumeMode {
-			return fmt.Errorf("unexpected driver mode: %s", c.csiVolumeMode)
+		if c.volumeLifecycleMode != storage.VolumeLifecyclePersistent {
+			return fmt.Errorf("unexpected driver mode: %s", c.volumeLifecycleMode)
 		}
 
 		fsType = pvSrc.FSType
@@ -319,7 +320,7 @@ func (c *csiMountMgr) podAttributes() (map[string]string, error) {
 		"csi.storage.k8s.io/serviceAccount.name": c.pod.Spec.ServiceAccountName,
 	}
 	if utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) {
-		attrs["csi.storage.k8s.io/ephemeral"] = strconv.FormatBool(c.csiVolumeMode == ephemeralVolumeMode)
+		attrs["csi.storage.k8s.io/ephemeral"] = strconv.FormatBool(c.volumeLifecycleMode == storage.VolumeLifecycleEphemeral)
 	}
 
 	klog.V(4).Infof(log("CSIDriver %q requires pod information", c.driverName))

pkg/volume/csi/csi_mounter_test.go

@@ -28,6 +28,7 @@ import (
 
 	api "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1"
+	storagev1beta1 "k8s.io/api/storage/v1beta1"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -151,13 +152,16 @@ func MounterSetUpTests(t *testing.T, podInfoEnabled bool) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			klog.Infof("Starting test %s", test.name)
+			// Modes must be set if (and only if) CSIInlineVolume is enabled.
+			var modes []storagev1beta1.VolumeLifecycleMode
 			if test.csiInlineVolume {
 				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)()
+				modes = append(modes, storagev1beta1.VolumeLifecyclePersistent)
 			}
 			fakeClient := fakeclient.NewSimpleClientset(
-				getTestCSIDriver("no-info", &noPodMountInfo, nil),
-				getTestCSIDriver("info", &currentPodInfoMount, nil),
-				getTestCSIDriver("nil", nil, nil),
+				getTestCSIDriver("no-info", &noPodMountInfo, nil, modes),
+				getTestCSIDriver("info", &currentPodInfoMount, nil, modes),
+				getTestCSIDriver("nil", nil, nil, modes),
 			)
 			plug, tmpDir := newTestPlugin(t, fakeClient)
 			defer os.RemoveAll(tmpDir)
@@ -278,16 +282,16 @@ func TestMounterSetUpSimple(t *testing.T) {
 	testCases := []struct {
 		name       string
 		podUID     types.UID
-		mode       csiVolumeMode
+		mode       storagev1beta1.VolumeLifecycleMode
 		fsType     string
 		options    []string
 		spec       func(string, []string) *volume.Spec
 		shouldFail bool
 	}{
 		{
-			name:       "setup with vol source",
+			name:       "setup with ephemeral source",
 			podUID:     types.UID(fmt.Sprintf("%08X", rand.Uint64())),
-			mode:       ephemeralVolumeMode,
+			mode:       storagev1beta1.VolumeLifecycleEphemeral,
 			fsType:     "ext4",
 			shouldFail: true,
 			spec: func(fsType string, options []string) *volume.Spec {
@@ -299,7 +303,7 @@ func TestMounterSetUpSimple(t *testing.T) {
 		{
 			name:   "setup with persistent source",
 			podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
-			mode:   persistentVolumeMode,
+			mode:   storagev1beta1.VolumeLifecyclePersistent,
 			fsType: "zfs",
 			spec: func(fsType string, options []string) *volume.Spec {
 				pvSrc := makeTestPV("pv1", 20, testDriver, "vol1")
@@ -311,7 +315,7 @@ func TestMounterSetUpSimple(t *testing.T) {
 		{
 			name:   "setup with persistent source without unspecified fstype and options",
 			podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
-			mode:   persistentVolumeMode,
+			mode:   storagev1beta1.VolumeLifecyclePersistent,
 			spec: func(fsType string, options []string) *volume.Spec {
 				return volume.NewSpecFromPersistentVolume(makeTestPV("pv1", 20, testDriver, "vol2"), false)
 			},
@@ -345,8 +349,8 @@ func TestMounterSetUpSimple(t *testing.T) {
 			csiMounter := mounter.(*csiMountMgr)
 			csiMounter.csiClient = setupClient(t, true)
 
-			if csiMounter.csiVolumeMode != persistentVolumeMode {
-				t.Fatal("unexpected volume mode: ", csiMounter.csiVolumeMode)
+			if csiMounter.volumeLifecycleMode != storagev1beta1.VolumeLifecyclePersistent {
+				t.Fatal("unexpected volume mode: ", csiMounter.volumeLifecycleMode)
 			}
 
 			attachID := getAttachmentName(csiMounter.volumeID, string(csiMounter.driverName), string(plug.host.GetNodeName()))
@@ -397,14 +401,10 @@ func TestMounterSetUpSimple(t *testing.T) {
 func TestMounterSetUpWithInline(t *testing.T) {
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)()
 
-	fakeClient := fakeclient.NewSimpleClientset()
-	plug, tmpDir := newTestPlugin(t, fakeClient)
-	defer os.RemoveAll(tmpDir)
-
 	testCases := []struct {
 		name       string
 		podUID     types.UID
-		mode       csiVolumeMode
+		mode       storagev1beta1.VolumeLifecycleMode
 		fsType     string
 		options    []string
 		spec       func(string, []string) *volume.Spec
@@ -413,7 +413,7 @@ func TestMounterSetUpWithInline(t *testing.T) {
 		{
 			name:   "setup with vol source",
 			podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
-			mode:   ephemeralVolumeMode,
+			mode:   storagev1beta1.VolumeLifecycleEphemeral,
 			fsType: "ext4",
 			spec: func(fsType string, options []string) *volume.Spec {
 				volSrc := makeTestVol("pv1", testDriver)
@@ -424,7 +424,7 @@ func TestMounterSetUpWithInline(t *testing.T) {
 		{
 			name:   "setup with persistent source",
 			podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
-			mode:   persistentVolumeMode,
+			mode:   storagev1beta1.VolumeLifecyclePersistent,
 			fsType: "zfs",
 			spec: func(fsType string, options []string) *volume.Spec {
 				pvSrc := makeTestPV("pv1", 20, testDriver, "vol1")
@@ -436,7 +436,7 @@ func TestMounterSetUpWithInline(t *testing.T) {
 		{
 			name:   "setup with persistent source without unspecified fstype and options",
 			podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
-			mode:   persistentVolumeMode,
+			mode:   storagev1beta1.VolumeLifecyclePersistent,
 			spec: func(fsType string, options []string) *volume.Spec {
 				return volume.NewSpecFromPersistentVolume(makeTestPV("pv1", 20, testDriver, "vol2"), false)
 			},
@@ -449,6 +449,15 @@ func TestMounterSetUpWithInline(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
+		// The fake driver currently supports all modes.
+		volumeLifecycleModes := []storagev1beta1.VolumeLifecycleMode{
+			storagev1beta1.VolumeLifecycleEphemeral,
+			storagev1beta1.VolumeLifecyclePersistent,
+		}
+		driver := getTestCSIDriver(testDriver, nil, nil, volumeLifecycleModes)
+		fakeClient := fakeclient.NewSimpleClientset(driver)
+		plug, tmpDir := newTestPlugin(t, fakeClient)
+		defer os.RemoveAll(tmpDir)
 		registerFakePlugin(testDriver, "endpoint", []string{"1.0.0"}, t)
 		t.Run(tc.name, func(t *testing.T) {
 			mounter, err := plug.NewMounter(
@@ -470,15 +479,15 @@ func TestMounterSetUpWithInline(t *testing.T) {
 			csiMounter := mounter.(*csiMountMgr)
 			csiMounter.csiClient = setupClient(t, true)
 
-			if csiMounter.csiVolumeMode != tc.mode {
-				t.Fatal("unexpected volume mode: ", csiMounter.csiVolumeMode)
+			if csiMounter.volumeLifecycleMode != tc.mode {
+				t.Fatal("unexpected volume mode: ", csiMounter.volumeLifecycleMode)
 			}
 
-			if csiMounter.csiVolumeMode == ephemeralVolumeMode && csiMounter.volumeID != makeVolumeHandle(string(tc.podUID), csiMounter.specVolumeID) {
+			if csiMounter.volumeLifecycleMode == storagev1beta1.VolumeLifecycleEphemeral && csiMounter.volumeID != makeVolumeHandle(string(tc.podUID), csiMounter.specVolumeID) {
 				t.Fatal("unexpected generated volumeHandle:", csiMounter.volumeID)
 			}
 
-			if csiMounter.csiVolumeMode == persistentVolumeMode {
+			if csiMounter.volumeLifecycleMode == storagev1beta1.VolumeLifecyclePersistent {
 				attachID := getAttachmentName(csiMounter.volumeID, string(csiMounter.driverName), string(plug.host.GetNodeName()))
 				attachment := makeTestAttachment(attachID, "test-node", csiMounter.spec.Name())
 				_, err = csiMounter.k8s.StorageV1().VolumeAttachments().Create(attachment)
@@ -503,10 +512,10 @@ func TestMounterSetUpWithInline(t *testing.T) {
 			}
 
 			// validate stagingTargetPath
-			if tc.mode == ephemeralVolumeMode && vol.DeviceMountPath != "" {
+			if tc.mode == storagev1beta1.VolumeLifecycleEphemeral && vol.DeviceMountPath != "" {
 				t.Errorf("unexpected devicePathTarget sent to driver: %s", vol.DeviceMountPath)
 			}
-			if tc.mode == persistentVolumeMode {
+			if tc.mode == storagev1beta1.VolumeLifecyclePersistent {
 				devicePath, err := makeDeviceMountPath(plug, csiMounter.spec)
 				if err != nil {
 					t.Fatal(err)