From 85c3de40e55baae7c3e7e62afe9ace3148e4c049 Mon Sep 17 00:00:00 2001 From: Cheng Xing Date: Thu, 9 Aug 2018 15:49:23 -0700 Subject: [PATCH] Fixed vsphere volume plugin unsafe type cast; added unit tests --- pkg/volume/vsphere_volume/BUILD | 3 ++ .../vsphere_volume/vsphere_volume_test.go | 49 +++++++++++++++++++ .../vsphere_volume/vsphere_volume_util.go | 4 +- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/pkg/volume/vsphere_volume/BUILD b/pkg/volume/vsphere_volume/BUILD index 2865dfacdfa..364553f4f72 100644 --- a/pkg/volume/vsphere_volume/BUILD +++ b/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", diff --git a/pkg/volume/vsphere_volume/vsphere_volume_test.go b/pkg/volume/vsphere_volume/vsphere_volume_test.go index 848d6848d3a..92c4b4d0b95 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_test.go +++ b/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) + } + } +} diff --git a/pkg/volume/vsphere_volume/vsphere_volume_util.go b/pkg/volume/vsphere_volume/vsphere_volume_util.go index 42053ad45f9..3babd361405 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_util.go +++ b/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