Merge pull request #67220 from verult/vsphere-panic

Automatic merge from submit-queue (batch tested with PRs 67058, 67083, 67220, 67222, 67209). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fixed unsafe type cast in vSphere volume plugin

**What this PR does / why we need it**: Fixes the controller manager panic caused by vSphere volumes being used on the wrong cloud provider.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #67218

**Release note**:

```release-note
NONE
```

/assign @saad-ali

pkg/volume/vsphere_volume/BUILD

@@ -39,6 +39,9 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
+        "//pkg/cloudprovider:go_default_library",
+        "//pkg/cloudprovider/providers/fake:go_default_library",
+        "//pkg/cloudprovider/providers/vsphere:go_default_library",
         "//pkg/cloudprovider/providers/vsphere/vclib:go_default_library",
         "//pkg/util/mount:go_default_library",
         "//pkg/volume:go_default_library",

pkg/volume/vsphere_volume/vsphere_volume_test.go

@@ -25,6 +25,9 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	utiltesting "k8s.io/client-go/util/testing"
+	"k8s.io/kubernetes/pkg/cloudprovider"
+	"k8s.io/kubernetes/pkg/cloudprovider/providers/fake"
+	"k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
@@ -188,3 +191,49 @@ func TestPlugin(t *testing.T) {
 		t.Errorf("Deleter() failed: %v", err)
 	}
 }
+
+func TestUnsupportedCloudProvider(t *testing.T) {
+	// Initial setup to test volume plugin
+	tmpDir, err := utiltesting.MkTmpdir("vsphereVolumeTest")
+	if err != nil {
+		t.Fatalf("can't make a temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	testcases := []struct {
+		name          string
+		cloudProvider cloudprovider.Interface
+		success       bool
+	}{
+		{name: "nil cloudprovider", cloudProvider: nil},
+		{name: "vSphere", cloudProvider: &vsphere.VSphere{}, success: true},
+		{name: "fake cloudprovider", cloudProvider: &fake.FakeCloud{}},
+	}
+
+	for _, tc := range testcases {
+		t.Logf("test case: %v", tc.name)
+
+		plugMgr := volume.VolumePluginMgr{}
+		plugMgr.InitPlugins(ProbeVolumePlugins(), nil, /* prober */
+			volumetest.NewFakeVolumeHostWithCloudProvider(tmpDir, nil, nil, tc.cloudProvider))
+
+		plug, err := plugMgr.FindAttachablePluginByName("kubernetes.io/vsphere-volume")
+		if err != nil {
+			t.Errorf("Can't find the plugin by name")
+		}
+
+		_, err = plug.NewAttacher()
+		if !tc.success && err == nil {
+			t.Errorf("expected error when creating attacher due to incorrect cloud provider, but got none")
+		} else if tc.success && err != nil {
+			t.Errorf("expected no error when creating attacher, but got error: %v", err)
+		}
+
+		_, err = plug.NewDetacher()
+		if !tc.success && err == nil {
+			t.Errorf("expected error when creating detacher due to incorrect cloud provider, but got none")
+		} else if tc.success && err != nil {
+			t.Errorf("expected no error when creating detacher, but got error: %v", err)
+		}
+	}
+}

pkg/volume/vsphere_volume/vsphere_volume_util.go

@@ -188,8 +188,8 @@ func getCloudProvider(cloud cloudprovider.Interface) (*vsphere.VSphere, error) {
 		return nil, errors.New("Cloud provider not initialized properly")
 	}
 
-	vs := cloud.(*vsphere.VSphere)
-	if vs == nil {
+	vs, ok := cloud.(*vsphere.VSphere)
+	if !ok || vs == nil {
 		return nil, errors.New("Invalid cloud provider: expected vSphere")
 	}
 	return vs, nil