Merge pull request #63464 from dougm/vsphere-auth

Automatic merge from submit-queue (batch tested with PRs 63364, 63464). 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>.

vsphere: use vim25.Client directly to support token authentication

**What this PR does / why we need it**:

This refactor is in support of SAML token authentication: #63209
Avoid use of govmomi.Client as it only supports username+password authentication via SessionManager.Login().
Using vim25.Client directly will allow VCP to add other authentication methods,
such as SessionManager.LoginByToken().

**Release note**:

```release-note
NONE
```

pkg/cloudprovider/providers/vsphere/BUILD

@@ -39,6 +39,7 @@ go_test(
     deps = [
         "//pkg/cloudprovider:go_default_library",
         "//pkg/cloudprovider/providers/vsphere/vclib:go_default_library",
+        "//vendor/github.com/vmware/govmomi/simulator:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
     ],

pkg/cloudprovider/providers/vsphere/nodemanager.go

@@ -360,7 +360,7 @@ func (nm *NodeManager) renewNodeInfo(nodeInfo *NodeInfo, reconnect bool) (*NodeI
 			return nil, err
 		}
 	}
-	vm := nodeInfo.vm.RenewVM(vsphereInstance.conn.GoVmomiClient)
+	vm := nodeInfo.vm.RenewVM(vsphereInstance.conn.Client)
 	return &NodeInfo{vm: &vm, dataCenter: vm.Datacenter, vcServer: nodeInfo.vcServer}, nil
 }
 

pkg/cloudprovider/providers/vsphere/vclib/BUILD

@@ -26,7 +26,6 @@ go_library(
     deps = [
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
-        "//vendor/github.com/vmware/govmomi:go_default_library",
         "//vendor/github.com/vmware/govmomi/find:go_default_library",
         "//vendor/github.com/vmware/govmomi/object:go_default_library",
         "//vendor/github.com/vmware/govmomi/pbm:go_default_library",

pkg/cloudprovider/providers/vsphere/vclib/connection.go

@@ -18,19 +18,19 @@ package vclib
 
 import (
 	"context"
-	"fmt"
+	"net"
 	neturl "net/url"
 	"sync"
 
 	"github.com/golang/glog"
-	"github.com/vmware/govmomi"
 	"github.com/vmware/govmomi/session"
 	"github.com/vmware/govmomi/vim25"
+	"github.com/vmware/govmomi/vim25/soap"
 )
 
 // VSphereConnection contains information for connecting to vCenter
 type VSphereConnection struct {
-	GoVmomiClient     *govmomi.Client
+	Client            *vim25.Client
 	Username          string
 	Password          string
 	Hostname          string
@@ -43,23 +43,23 @@ var (
 	clientLock sync.Mutex
 )
 
-// Connect makes connection to vCenter and sets VSphereConnection.GoVmomiClient.
+// Connect makes connection to vCenter and sets VSphereConnection.Client.
-// If connection.GoVmomiClient is already set, it obtains the existing user session.
+// If connection.Client is already set, it obtains the existing user session.
-// if user session is not valid, connection.GoVmomiClient will be set to the new client.
+// if user session is not valid, connection.Client will be set to the new client.
 func (connection *VSphereConnection) Connect(ctx context.Context) error {
 	var err error
 	clientLock.Lock()
 	defer clientLock.Unlock()
 
-	if connection.GoVmomiClient == nil {
+	if connection.Client == nil {
-		connection.GoVmomiClient, err = connection.NewClient(ctx)
+		connection.Client, err = connection.NewClient(ctx)
 		if err != nil {
 			glog.Errorf("Failed to create govmomi client. err: %+v", err)
 			return err
 		}
 		return nil
 	}
-	m := session.NewManager(connection.GoVmomiClient.Client)
+	m := session.NewManager(connection.Client)
 	userSession, err := m.UserSession(ctx)
 	if err != nil {
 		glog.Errorf("Error while obtaining user session. err: %+v", err)
@@ -69,8 +69,8 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error {
 		return nil
 	}
 	glog.Warningf("Creating new client session since the existing session is not valid or not authenticated")
-	connection.GoVmomiClient.Logout(ctx)
+
-	connection.GoVmomiClient, err = connection.NewClient(ctx)
+	connection.Client, err = connection.NewClient(ctx)
 	if err != nil {
 		glog.Errorf("Failed to create govmomi client. err: %+v", err)
 		return err
@@ -78,19 +78,36 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error {
 	return nil
 }
 
+// Logout calls SessionManager.Logout for the given connection.
+func (connection *VSphereConnection) Logout(ctx context.Context) {
+	m := session.NewManager(connection.Client)
+	if err := m.Logout(ctx); err != nil {
+		glog.Errorf("Logout failed: %s", err)
+	}
+}
+
 // NewClient creates a new govmomi client for the VSphereConnection obj
-func (connection *VSphereConnection) NewClient(ctx context.Context) (*govmomi.Client, error) {
+func (connection *VSphereConnection) NewClient(ctx context.Context) (*vim25.Client, error) {
-	url, err := neturl.Parse(fmt.Sprintf("https://%s:%s/sdk", connection.Hostname, connection.Port))
+	url, err := soap.ParseURL(net.JoinHostPort(connection.Hostname, connection.Port))
 	if err != nil {
 		glog.Errorf("Failed to parse URL: %s. err: %+v", url, err)
 		return nil, err
 	}
-	url.User = neturl.UserPassword(connection.Username, connection.Password)
+
-	client, err := govmomi.NewClient(ctx, url, connection.Insecure)
+	sc := soap.NewClient(url, connection.Insecure)
+	client, err := vim25.NewClient(ctx, sc)
 	if err != nil {
 		glog.Errorf("Failed to create new client. err: %+v", err)
 		return nil, err
 	}
+
+	m := session.NewManager(client)
+
+	err = m.Login(ctx, neturl.UserPassword(connection.Username, connection.Password))
+	if err != nil {
+		return nil, err
+	}
+
 	if connection.RoundTripperCount == 0 {
 		connection.RoundTripperCount = RoundTripperDefaultCount
 	}

pkg/cloudprovider/providers/vsphere/vclib/constants.go

@@ -53,7 +53,8 @@ const (
 
 // Test Constants
 const (
-	testDefaultDatacenter = "DC0"
+	TestDefaultDatacenter = "DC0"
-	testDefaultDatastore  = "LocalDS_0"
+	TestDefaultDatastore  = "LocalDS_0"
+	TestDefaultNetwork    = "VM Network"
 	testNameNotFound      = "enoent"
 )

pkg/cloudprovider/providers/vsphere/vclib/datacenter.go

@@ -39,7 +39,7 @@ type Datacenter struct {
 // GetDatacenter returns the DataCenter Object for the given datacenterPath
 // If datacenter is located in a folder, include full path to datacenter else just provide the datacenter name
 func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacenterPath string) (*Datacenter, error) {
-	finder := find.NewFinder(connection.GoVmomiClient.Client, false)
+	finder := find.NewFinder(connection.Client, false)
 	datacenter, err := finder.Datacenter(ctx, datacenterPath)
 	if err != nil {
 		glog.Errorf("Failed to find the datacenter: %s. err: %+v", datacenterPath, err)
@@ -52,7 +52,7 @@ func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacente
 // GetAllDatacenter returns all the DataCenter Objects
 func GetAllDatacenter(ctx context.Context, connection *VSphereConnection) ([]*Datacenter, error) {
 	var dc []*Datacenter
-	finder := find.NewFinder(connection.GoVmomiClient.Client, false)
+	finder := find.NewFinder(connection.Client, false)
 	datacenters, err := finder.DatacenterList(ctx, "*")
 	if err != nil {
 		glog.Errorf("Failed to find the datacenter. err: %+v", err)

pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go

@@ -47,14 +47,14 @@ func TestDatacenter(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	vc := &VSphereConnection{GoVmomiClient: c}
+	vc := &VSphereConnection{Client: c.Client}
 
 	_, err = GetDatacenter(ctx, vc, testNameNotFound)
 	if err == nil {
 		t.Error("expected error")
 	}
 
-	dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter)
+	dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter)
 	if err != nil {
 		t.Error(err)
 	}
@@ -74,7 +74,7 @@ func TestDatacenter(t *testing.T) {
 		t.Error("expected error")
 	}
 
-	vm, err := dc.GetVMByPath(ctx, testDefaultDatacenter+"/vm/"+avm.Name)
+	vm, err := dc.GetVMByPath(ctx, TestDefaultDatacenter+"/vm/"+avm.Name)
 	if err != nil {
 		t.Error(err)
 	}
@@ -103,7 +103,7 @@ func TestDatacenter(t *testing.T) {
 		t.Error("expected error")
 	}
 
-	ds, err := dc.GetDatastoreByName(ctx, testDefaultDatastore)
+	ds, err := dc.GetDatastoreByName(ctx, TestDefaultDatastore)
 	if err != nil {
 		t.Error(err)
 	}
@@ -113,7 +113,7 @@ func TestDatacenter(t *testing.T) {
 		t.Error("expected error")
 	}
 
-	_, err = dc.GetFolderByPath(ctx, testDefaultDatacenter+"/vm")
+	_, err = dc.GetFolderByPath(ctx, TestDefaultDatacenter+"/vm")
 	if err != nil {
 		t.Error(err)
 	}

pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go

@@ -45,9 +45,9 @@ func TestDatastore(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	vc := &VSphereConnection{GoVmomiClient: c}
+	vc := &VSphereConnection{Client: c.Client}
 
-	dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter)
+	dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter)
 	if err != nil {
 		t.Error(err)
 	}

pkg/cloudprovider/providers/vsphere/vclib/folder_test.go

@@ -47,9 +47,9 @@ func TestFolder(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	vc := &VSphereConnection{GoVmomiClient: c}
+	vc := &VSphereConnection{Client: c.Client}
 
-	dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter)
+	dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter)
 	if err != nil {
 		t.Error(err)
 	}

pkg/cloudprovider/providers/vsphere/vclib/utils_test.go

@@ -46,9 +46,9 @@ func TestUtils(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	vc := &VSphereConnection{GoVmomiClient: c}
+	vc := &VSphereConnection{Client: c.Client}
 
-	dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter)
+	dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter)
 	if err != nil {
 		t.Error(err)
 	}

pkg/cloudprovider/providers/vsphere/vclib/virtualmachine.go

@@ -23,9 +23,9 @@ import (
 	"time"
 
 	"github.com/golang/glog"
-	"github.com/vmware/govmomi"
 	"github.com/vmware/govmomi/object"
 	"github.com/vmware/govmomi/property"
+	"github.com/vmware/govmomi/vim25"
 	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/govmomi/vim25/types"
 )
@@ -403,8 +403,8 @@ func (vm *VirtualMachine) deleteController(ctx context.Context, controllerDevice
 }
 
 // RenewVM renews this virtual machine with new client connection.
-func (vm *VirtualMachine) RenewVM(client *govmomi.Client) VirtualMachine {
+func (vm *VirtualMachine) RenewVM(client *vim25.Client) VirtualMachine {
-	dc := Datacenter{Datacenter: object.NewDatacenter(client.Client, vm.Datacenter.Reference())}
+	dc := Datacenter{Datacenter: object.NewDatacenter(client, vm.Datacenter.Reference())}
-	newVM := object.NewVirtualMachine(client.Client, vm.VirtualMachine.Reference())
+	newVM := object.NewVirtualMachine(client, vm.VirtualMachine.Reference())
 	return VirtualMachine{VirtualMachine: newVM, Datacenter: &dc}
 }

pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go

@@ -43,9 +43,9 @@ func TestVirtualMachine(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	vc := &VSphereConnection{GoVmomiClient: c}
+	vc := &VSphereConnection{Client: c.Client}
 
-	dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter)
+	dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter)
 	if err != nil {
 		t.Error(err)
 	}

pkg/cloudprovider/providers/vsphere/vsphere.go

@@ -360,7 +360,10 @@ func populateVsphereInstanceMap(cfg *VSphereConfig) (map[string]*VSphereInstance
 	return vsphereInstanceMap, nil
 }
 
-// Creates new Contreoller node interface and returns
+// getVMUUID allows tests to override GetVMUUID
+var getVMUUID = GetVMUUID
+
+// Creates new Controller node interface and returns
 func newControllerNode(cfg VSphereConfig) (*VSphere, error) {
 	var err error
 
@@ -399,7 +402,7 @@ func newControllerNode(cfg VSphereConfig) (*VSphere, error) {
 		glog.Errorf("Failed to get hostname. err: %+v", err)
 		return nil, err
 	}
-	vs.vmUUID, err = GetVMUUID()
+	vs.vmUUID, err = getVMUUID()
 	if err != nil {
 		glog.Errorf("Failed to get uuid. err: %+v", err)
 		return nil, err
@@ -410,8 +413,8 @@ func newControllerNode(cfg VSphereConfig) (*VSphere, error) {
 
 func logout(vs *VSphere) {
 	for _, vsphereIns := range vs.vsphereInstanceMap {
-		if vsphereIns.conn.GoVmomiClient != nil {
+		if vsphereIns.conn.Client != nil {
-			vsphereIns.conn.GoVmomiClient.Logout(context.TODO())
+			vsphereIns.conn.Logout(context.TODO())
 		}
 	}
 

pkg/cloudprovider/providers/vsphere/vsphere_test.go

@@ -18,12 +18,14 @@ package vsphere
 
 import (
 	"context"
+	"crypto/tls"
 	"log"
 	"os"
 	"strconv"
 	"strings"
 	"testing"
 
+	"github.com/vmware/govmomi/simulator"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/kubernetes/pkg/cloudprovider"
@@ -59,6 +61,50 @@ func configFromEnv() (cfg VSphereConfig, ok bool) {
 	return
 }
 
+// configFromEnvOrSim returns config from configFromEnv if set,
+// otherwise starts a vcsim instance and returns config for use against the vcsim instance.
+func configFromEnvOrSim() (VSphereConfig, func()) {
+	cfg, ok := configFromEnv()
+	if ok {
+		return cfg, func() {}
+	}
+
+	model := simulator.VPX()
+
+	err := model.Create()
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	model.Service.TLS = new(tls.Config)
+	s := model.Service.NewServer()
+
+	cfg.Global.InsecureFlag = true
+	cfg.Global.VCenterIP = s.URL.Hostname()
+	cfg.Global.VCenterPort = s.URL.Port()
+	cfg.Global.User = s.URL.User.Username()
+	cfg.Global.Password, _ = s.URL.User.Password()
+	cfg.Global.Datacenter = vclib.TestDefaultDatacenter
+	cfg.Network.PublicNetwork = vclib.TestDefaultNetwork
+	cfg.Global.DefaultDatastore = vclib.TestDefaultDatastore
+	cfg.Disk.SCSIControllerType = os.Getenv("VSPHERE_SCSICONTROLLER_TYPE")
+	cfg.Global.WorkingDir = os.Getenv("VSPHERE_WORKING_DIR")
+	cfg.Global.VMName = os.Getenv("VSPHERE_VM_NAME")
+
+	if cfg.Global.WorkingDir == "" {
+		cfg.Global.WorkingDir = "vm" // top-level Datacenter.VmFolder
+	}
+
+	uuid := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine).Config.Uuid
+	getVMUUID = func() (string, error) { return uuid, nil }
+
+	return cfg, func() {
+		getVMUUID = GetVMUUID
+		s.Close()
+		model.Remove()
+	}
+}
+
 func TestReadConfig(t *testing.T) {
 	_, err := readConfig(nil)
 	if err == nil {
@@ -110,10 +156,8 @@ func TestNewVSphere(t *testing.T) {
 }
 
 func TestVSphereLogin(t *testing.T) {
-	cfg, ok := configFromEnv()
+	cfg, cleanup := configFromEnvOrSim()
-	if !ok {
+	defer cleanup()
-		t.Skipf("No config found in environment")
-	}
 
 	// Create vSphere configuration object
 	vs, err := newControllerNode(cfg)
@@ -126,8 +170,8 @@ func TestVSphereLogin(t *testing.T) {
 	defer cancel()
 
 	// Create vSphere client
-	var vcInstance *VSphereInstance
+	vcInstance, ok := vs.vsphereInstanceMap[cfg.Global.VCenterIP]
-	if vcInstance, ok = vs.vsphereInstanceMap[cfg.Global.VCenterIP]; !ok {
+	if !ok {
 		t.Fatalf("Couldn't get vSphere instance: %s", cfg.Global.VCenterIP)
 	}
 
@@ -135,7 +179,7 @@ func TestVSphereLogin(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to connect to vSphere: %s", err)
 	}
-	defer vcInstance.conn.GoVmomiClient.Logout(ctx)
+	defer vcInstance.conn.Logout(ctx)
 }
 
 func TestZones(t *testing.T) {