Skip to content

Commit d564d2e

Browse files
author
Vladimir Vivien
committed
CSI - Prevents unsupported device mount with CanMountDevice(spec) check
1 parent b13b2bb commit d564d2e

File tree

4 files changed

+549
-1
lines changed

4 files changed

+549
-1
lines changed

pkg/volume/csi/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ go_test(
5050
"csi_drivers_store_test.go",
5151
"csi_mounter_test.go",
5252
"csi_plugin_test.go",
53+
"csi_test.go",
5354
"csi_util_test.go",
5455
"expander_test.go",
5556
],

pkg/volume/csi/csi_plugin.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,18 @@ func (p *csiPlugin) CanAttach(spec *volume.Spec) (bool, error) {
607607
return !skipAttach, nil
608608
}
609609

610-
// TODO (#75352) add proper logic to determine device moutability by inspecting the spec.
610+
// CanDeviceMount returns true if the spec supports device mount
611611
func (p *csiPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
612+
driverMode, err := p.getDriverMode(spec)
613+
if err != nil {
614+
return false, err
615+
}
616+
617+
if driverMode == ephemeralDriverMode {
618+
klog.V(5).Info(log("plugin.CanDeviceMount skipped ephemeral mode detected for spec %v", spec.Name()))
619+
return false, nil
620+
}
621+
612622
return true, nil
613623
}
614624

pkg/volume/csi/csi_plugin_test.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,118 @@ func TestPluginFindAttachablePlugin(t *testing.T) {
958958
}
959959
}
960960

961+
func TestPluginCanDeviceMount(t *testing.T) {
962+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)()
963+
tests := []struct {
964+
name string
965+
driverName string
966+
spec *volume.Spec
967+
canDeviceMount bool
968+
shouldFail bool
969+
}{
970+
{
971+
name: "non device mountable inline",
972+
driverName: "inline-driver",
973+
spec: volume.NewSpecFromVolume(makeTestVol("test-vol", "inline-driver")),
974+
canDeviceMount: false,
975+
},
976+
{
977+
name: "device mountable PV",
978+
driverName: "device-mountable-pv",
979+
spec: volume.NewSpecFromPersistentVolume(makeTestPV("test-vol", 20, "device-mountable-pv", testVol), true),
980+
canDeviceMount: true,
981+
},
982+
{
983+
name: "incomplete spec",
984+
driverName: "device-unmountable",
985+
spec: &volume.Spec{ReadOnly: true},
986+
canDeviceMount: false,
987+
shouldFail: true,
988+
},
989+
{
990+
name: "missing spec",
991+
driverName: "device-unmountable",
992+
canDeviceMount: false,
993+
shouldFail: true,
994+
},
995+
}
996+
997+
for _, test := range tests {
998+
t.Run(test.name, func(t *testing.T) {
999+
plug, tmpDir := newTestPlugin(t, nil)
1000+
defer os.RemoveAll(tmpDir)
1001+
1002+
pluginCanDeviceMount, err := plug.CanDeviceMount(test.spec)
1003+
if err != nil && !test.shouldFail {
1004+
t.Fatalf("unexpected error in plug.CanDeviceMount: %s", err)
1005+
}
1006+
if pluginCanDeviceMount != test.canDeviceMount {
1007+
t.Fatalf("expecting plugin.CanAttach %t got %t", test.canDeviceMount, pluginCanDeviceMount)
1008+
}
1009+
})
1010+
}
1011+
}
1012+
1013+
func TestPluginFindDeviceMountablePluginBySpec(t *testing.T) {
1014+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)()
1015+
tests := []struct {
1016+
name string
1017+
driverName string
1018+
spec *volume.Spec
1019+
canDeviceMount bool
1020+
shouldFail bool
1021+
}{
1022+
{
1023+
name: "non device mountable inline",
1024+
driverName: "inline-driver",
1025+
spec: volume.NewSpecFromVolume(makeTestVol("test-vol", "inline-driver")),
1026+
canDeviceMount: false,
1027+
},
1028+
{
1029+
name: "device mountable PV",
1030+
driverName: "device-mountable-pv",
1031+
spec: volume.NewSpecFromPersistentVolume(makeTestPV("test-vol", 20, "device-mountable-pv", testVol), true),
1032+
canDeviceMount: true,
1033+
},
1034+
{
1035+
name: "incomplete spec",
1036+
driverName: "device-unmountable",
1037+
spec: &volume.Spec{ReadOnly: true},
1038+
canDeviceMount: false,
1039+
shouldFail: true,
1040+
},
1041+
{
1042+
name: "missing spec",
1043+
driverName: "device-unmountable",
1044+
canDeviceMount: false,
1045+
shouldFail: true,
1046+
},
1047+
}
1048+
1049+
for _, test := range tests {
1050+
t.Run(test.name, func(t *testing.T) {
1051+
tmpDir, err := utiltesting.MkTmpdir("csi-test")
1052+
if err != nil {
1053+
t.Fatalf("can't create temp dir: %v", err)
1054+
}
1055+
defer os.RemoveAll(tmpDir)
1056+
1057+
client := fakeclient.NewSimpleClientset()
1058+
host := volumetest.NewFakeVolumeHost(tmpDir, client, nil)
1059+
plugMgr := &volume.VolumePluginMgr{}
1060+
plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, host)
1061+
1062+
plug, err := plugMgr.FindDeviceMountablePluginBySpec(test.spec)
1063+
if err != nil && !test.shouldFail {
1064+
t.Fatalf("unexpected error in plugMgr.FindDeviceMountablePluginBySpec: %s", err)
1065+
}
1066+
if (plug != nil) != test.canDeviceMount {
1067+
t.Fatalf("expecting deviceMountablePlugin, but got nil")
1068+
}
1069+
})
1070+
}
1071+
}
1072+
9611073
func TestPluginNewBlockMapper(t *testing.T) {
9621074
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIBlockVolume, true)()
9631075

0 commit comments

Comments
 (0)