Revert "Support external http.Client"

This reverts commit 2d7f460.

cmd/main.go

@@ -177,7 +177,7 @@ func main() {
 	}
 
 	// nsxClient is used to interact with NSX API.
-	nsxClient := nsx.GetClient(cf, nil)
+	nsxClient := nsx.GetClient(cf)
 	if nsxClient == nil {
 		log.Error(err, "failed to get nsx client")
 		os.Exit(1)

cmd_clean/main.go

@@ -4,11 +4,8 @@
 package main
 
 import (
-	"crypto/tls"
 	"flag"
-	"net/http"
 	"os"
-	"time"
 
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -21,38 +18,21 @@ import (
 // ./bin/clean -cluster=''  -thumbprint="" -log-level=0 -vc-user="" -vc-passwd="" -vc-endpoint="" -vc-sso-domain="" -vc-https-port=443  -mgr-ip=""
 
 var (
-	log             = logger.Log
-	cf              *config.NSXOperatorConfig
-	mgrIp           string
-	vcEndpoint      string
-	vcUser          string
-	vcPasswd        string
-	nsxUser         string
-	nsxPasswd       string
-	vcSsoDomain     string
-	vcHttpsPort     int
-	thumbprint      string
-	caFile          string
-	cluster         string
-	useExternalHttp bool
+	log         = logger.Log
+	cf          *config.NSXOperatorConfig
+	mgrIp       string
+	vcEndpoint  string
+	vcUser      string
+	vcPasswd    string
+	nsxUser     string
+	nsxPasswd   string
+	vcSsoDomain string
+	vcHttpsPort int
+	thumbprint  string
+	caFile      string
+	cluster     string
 )
 
-type Transport struct {
-	Base http.RoundTripper
-}
-
-func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
-	log.V(1).Info("http request", "method", r.Method, "body", r.Body, "url", r.URL)
-	r.SetBasicAuth(nsxUser, nsxPasswd)
-	return t.base().RoundTrip(r)
-}
-func (t *Transport) base() http.RoundTripper {
-	if t.Base != nil {
-		return t.Base
-	}
-	return http.DefaultTransport
-}
-
 func main() {
 	flag.StringVar(&vcEndpoint, "vc-endpoint", "", "nsx manager ip")
 	flag.StringVar(&vcSsoDomain, "vc-sso-domain", "", "nsx manager ip")
@@ -66,7 +46,6 @@ func main() {
 	flag.StringVar(&caFile, "ca-file", "", "ca file")
 	flag.StringVar(&cluster, "cluster", "", "cluster name")
 	flag.IntVar(&config.LogLevel, "log-level", 0, "Use zap-core log system.")
-	flag.BoolVar(&useExternalHttp, "use-external-http", false, "Use wcp created http client")
 	flag.Parse()
 
 	cf = config.NewNSXOpertorConfig()
@@ -84,33 +63,7 @@ func main() {
 
 	logf.SetLogger(logger.ZapLogger(cf.DefaultConfig.Debug, config.LogLevel))
 
-	// just a demo to show how to use customer http client
-	// customer http client should handle verify and authentication
-	// here using the basic user/password mode for authentication
-	// not handling verify
-	// the error roughly are:
-	// 1. failed to validate config
-	// 2. failed to get nsx client
-	// 3. failed to initialize cleanup service
-	// 4. failed to clean up specific resourc
-	var err error
-	var status clean.Status
-	if useExternalHttp {
-		tr := &http.Transport{
-			IdleConnTimeout: 30 * time.Second,
-			// #nosec G402: ignore insecure options
-			TLSClientConfig: &tls.Config{
-				InsecureSkipVerify: true,
-			},
-		}
-		httpClient := &http.Client{
-			Transport: &Transport{Base: tr},
-			Timeout:   30 * time.Second,
-		}
-		status, err = clean.Clean(cf, httpClient)
-	} else {
-		status, err = clean.Clean(cf, nil)
-	}
+	status, err := clean.Clean(cf)
 	if err != nil {
 		log.Error(err, "failed to clean nsx resources", "status", status)
 		os.Exit(1)

pkg/clean/clean.go

@@ -5,7 +5,6 @@ package clean
 
 import (
 	"fmt"
-	"net/http"
 
 	"k8s.io/client-go/util/retry"
 
@@ -28,18 +27,17 @@ var log = logger.Log
 // including security policy, static route, subnet, subnet port, subnet set, vpc, ip pool, nsx service account
 // it is usually used when nsx-operator is uninstalled and remove all the resources created by nsx-operator
 // return error if any, return nil if no error
-func Clean(cf *config.NSXOperatorConfig, client *http.Client) (Status, error) {
+func Clean(cf *config.NSXOperatorConfig) (Status, error) {
 	log.Info("starting NSX cleanup")
 	if err := cf.ValidateConfigFromCmd(); err != nil {
 		return ValidationFailed, err
 	}
-	nsxClient := nsx.GetClient(cf, client)
+	nsxClient := nsx.GetClient(cf)
 	if nsxClient == nil {
 		return GetNSXClientFailed, fmt.Errorf("failed to get nsx client")
 	}
-	if cleanupService, err := InitializeCleanupService(cf, nsxClient); err != nil {
-		return InitCleanupServiceFailed, err
-
+	if cleanupService, err := InitializeCleanupService(cf); err != nil {
+		return InitCleanupServiceFailed, fmt.Errorf("failed to initialize cleanup service: %w", err)
 	} else if cleanupService.err != nil {
 		return InitCleanupServiceFailed, cleanupService.err
 	} else {
@@ -65,9 +63,14 @@ func Clean(cf *config.NSXOperatorConfig, client *http.Client) (Status, error) {
 }
 
 // InitializeCleanupService initializes all the CR services
-func InitializeCleanupService(cf *config.NSXOperatorConfig, nsxClient *nsx.Client) (*CleanupService, error) {
+func InitializeCleanupService(cf *config.NSXOperatorConfig) (*CleanupService, error) {
 	cleanupService := NewCleanupService()
 
+	nsxClient := nsx.GetClient(cf)
+	if nsxClient == nil {
+		return cleanupService, fmt.Errorf("failed to get nsx client")
+	}
+
 	var commonService = common.Service{
 		NSXClient: nsxClient,
 		NSXConfig: cf,

pkg/config/config.go

@@ -310,7 +310,7 @@ func (nsxConfig *NsxConfig) validateCert() error {
 		return err
 	}
 	if caCount > 0 {
-		configLog.Infof("validate CA file: %d", caCount)
+		configLog.Infof("validate CA file: %s", caCount)
 		if caCount > 1 && caCount != mCount {
 			err := errors.New("ca file count not match manager count")
 			configLog.Error(err, "validate NsxConfig failed", "ca file count", caCount, "manager count", mCount)
@@ -324,7 +324,7 @@ func (nsxConfig *NsxConfig) validateCert() error {
 			}
 		}
 	} else {
-		configLog.Infof("validate thumbprint: %d", tpCount)
+		configLog.Infof("validate thumbprint: %s", tpCount)
 		if tpCount > 1 && tpCount != mCount {
 			err := errors.New("thumbprint count not match manager count")
 			configLog.Error(err, "validate NsxConfig failed", "thumbprint count", tpCount, "manager count", mCount)

pkg/nsx/client.go

@@ -119,7 +119,7 @@ func restConnector(c *Cluster) *client.RestConnector {
 	return connector
 }
 
-func GetClient(cf *config.NSXOperatorConfig, client *http.Client) *Client {
+func GetClient(cf *config.NSXOperatorConfig) *Client {
 	// Set log level for vsphere-automation-sdk-go
 	logger := logrus.New()
 	vspherelog.SetLogger(logger)
@@ -129,7 +129,7 @@ func GetClient(cf *config.NSXOperatorConfig, client *http.Client) *Client {
 	}
 	c := NewConfig(strings.Join(cf.NsxApiManagers, ","), cf.NsxApiUser, cf.NsxApiPassword, cf.CaFile, 10, 3, defaultHttpTimeout, 20, true, true, true,
 		ratelimiter.AIMD, cf.GetTokenProvider(), nil, cf.Thumbprint)
-	cluster, _ := NewCluster(c, client)
+	cluster, _ := NewCluster(c)
 
 	queryClient := search.NewQueryClient(restConnector(cluster))
 	groupClient := domains.NewGroupsClient(restConnector(cluster))

pkg/nsx/client_test.go

@@ -21,7 +21,7 @@ import (
 func TestNSXHealthChecker_CheckNSXHealth(t *testing.T) {
 	host := "1.1.1.1"
 	config := NewConfig(host, "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{})
-	cluster, _ := NewCluster(config, nil)
+	cluster, _ := NewCluster(config)
 	req := &http.Request{}
 
 	res := []ClusterHealth{GREEN, RED, ORANGE}
@@ -64,7 +64,7 @@ func TestNSXHealthChecker_CheckNSXHealth(t *testing.T) {
 func TestGetClient(t *testing.T) {
 	cf := config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{NsxApiUser: "1", NsxApiPassword: "1"}}
 	cf.VCConfig = &config.VCConfig{}
-	client := GetClient(&cf, nil)
+	client := GetClient(&cf)
 	assert.True(t, client != nil)
 
 	cluster := &Cluster{}
@@ -73,7 +73,7 @@ func TestGetClient(t *testing.T) {
 		return nsxVersion, nil
 	})
 
-	client = GetClient(&cf, nil)
+	client = GetClient(&cf)
 	patches.Reset()
 	assert.True(t, client != nil)
 	securityPolicySupported := client.NSXCheckVersion(SecurityPolicy)
@@ -86,7 +86,7 @@ func TestGetClient(t *testing.T) {
 		nsxVersion := &NsxVersion{NodeVersion: "3.2.1"}
 		return nsxVersion, nil
 	})
-	client = GetClient(&cf, nil)
+	client = GetClient(&cf)
 	patches.Reset()
 	assert.True(t, client != nil)
 	securityPolicySupported = client.NSXCheckVersion(SecurityPolicy)
@@ -99,7 +99,7 @@ func TestGetClient(t *testing.T) {
 		nsxVersion := &NsxVersion{NodeVersion: "4.1.0"}
 		return nsxVersion, nil
 	})
-	client = GetClient(&cf, nil)
+	client = GetClient(&cf)
 	patches.Reset()
 	assert.True(t, client != nil)
 	securityPolicySupported = client.NSXCheckVersion(SecurityPolicy)
@@ -112,7 +112,7 @@ func TestGetClient(t *testing.T) {
 		nsxVersion := &NsxVersion{NodeVersion: "4.1.2"}
 		return nsxVersion, nil
 	})
-	client = GetClient(&cf, nil)
+	client = GetClient(&cf)
 	patches.Reset()
 	assert.True(t, client != nil)
 	securityPolicySupported = client.NSXCheckVersion(SecurityPolicy)
@@ -125,7 +125,7 @@ func TestGetClient(t *testing.T) {
 		nsxVersion := &NsxVersion{NodeVersion: "4.1.3"}
 		return nsxVersion, nil
 	})
-	client = GetClient(&cf, nil)
+	client = GetClient(&cf)
 	patches.Reset()
 	assert.True(t, client != nil)
 	securityPolicySupported = client.NSXCheckVersion(SecurityPolicy)
@@ -142,7 +142,7 @@ func IsInstanceOf(objectPtr, typePtr interface{}) bool {
 func TestSRGetClient(t *testing.T) {
 	cf := config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{NsxApiUser: "admin", NsxApiPassword: "Admin!23Admin", NsxApiManagers: []string{"10.173.82.128"}}}
 	cf.VCConfig = &config.VCConfig{}
-	client := GetClient(&cf, nil)
+	client := GetClient(&cf)
 	st, error := client.StaticRouteClient.Get("default", "project-1", "vpc-2", "site1")
 	if error == nil {
 		fmt.Printf("sr %v\n", *st.ResourceType)

pkg/nsx/cluster.go

@@ -58,19 +58,13 @@ var (
 )
 
 // NewCluster creates a cluster based on nsx Config.
-func NewCluster(config *Config, client *http.Client) (*Cluster, error) {
+func NewCluster(config *Config) (*Cluster, error) {
 	log.Info("creating cluster")
 	cluster := &Cluster{}
 	cluster.config = config
 	cluster.transport = cluster.createTransport(time.Duration(config.ConnIdleTimeout))
-	// if client created by third-party, set noBalancerClient to nil to disable keep alive for clean up
-	if client != nil {
-		cluster.client = client
-		cluster.noBalancerClient = client
-	} else {
-		cluster.client = cluster.createHTTPClient(cluster.transport, time.Duration(config.HTTPTimeout))
-		cluster.noBalancerClient = cluster.createNoBalancerClient(time.Duration(config.HTTPTimeout), time.Duration(config.ConnIdleTimeout))
-	}
+	cluster.client = cluster.createHTTPClient(cluster.transport, time.Duration(config.HTTPTimeout))
+	cluster.noBalancerClient = cluster.createNoBalancerClient(time.Duration(config.HTTPTimeout), time.Duration(config.ConnIdleTimeout))
 
 	r := ratelimiter.NewRateLimiter(config.APIRateMode)
 	eps, err := cluster.createEndpoints(config.APIManagers, cluster.client, cluster.noBalancerClient, r, config.TokenProvider)

pkg/nsx/cluster_test.go

@@ -32,7 +32,7 @@ func TestNewCluster(t *testing.T) {
 	a := ts.URL[index+2:]
 	thumbprint := []string{"123"}
 	config := NewConfig(a, "admin", "passw0rd", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, thumbprint)
-	_, err := NewCluster(config, nil)
+	_, err := NewCluster(config)
 	assert.True(t, err == nil, fmt.Sprintf("Created cluster failed %v", err))
 }
 
@@ -90,7 +90,7 @@ func TestCluster_NewRestConnector(t *testing.T) {
 	a := ts.URL[index+2:]
 	thumbprint := []string{"123"}
 	config := NewConfig(a, "admin", "passw0rd", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, thumbprint)
-	c, _ := NewCluster(config, nil)
+	c, _ := NewCluster(config)
 	con, _ := c.NewRestConnector()
 	assert.NotNil(t, con)
 }
@@ -109,7 +109,7 @@ func TestCluster_createTransport(t *testing.T) {
 	a := ts.URL[index+2:]
 	thumbprint := []string{"123"}
 	config := NewConfig(a, "admin", "passw0rd", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, thumbprint)
-	c, _ := NewCluster(config, nil)
+	c, _ := NewCluster(config)
 	assert.NotNil(t, c.createTransport(10))
 }
 
@@ -244,7 +244,7 @@ func TestCluster_getVersion(t *testing.T) {
 	index := strings.Index(ts.URL, "//")
 	a := ts.URL[index+2:]
 	config := NewConfig(a, "admin", "passw0rd", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, thumbprint)
-	cluster, _ := NewCluster(config, nil)
+	cluster, _ := NewCluster(config)
 	nsxVersion, err := cluster.GetVersion()
 	assert.True(t, err == nil)
 	assert.Equal(t, nsxVersion.NodeVersion, "3.1.3.3.0.18844962")

pkg/nsx/endpoint.go

@@ -107,10 +107,6 @@ type epHealthy struct {
 }
 
 func (ep *Endpoint) keepAlive() error {
-	// disable keepAlive for cleanup
-	if ep.noBalancerClient == ep.client {
-		return nil
-	}
 	req, err := http.NewRequest("GET", fmt.Sprintf(healthURL, ep.Scheme(), ep.Host()), nil)
 	if err != nil {
 		log.Error(err, "create keep alive request error")
@@ -259,10 +255,7 @@ func (ep *Endpoint) createAuthSession(certProvider auth.ClientCertProvider, toke
 		log.V(2).Info("Skipping session create with JWT based auth")
 		return nil
 	}
-	// disable createAuthSession for cleanup
-	if ep.noBalancerClient == ep.client {
-		return nil
-	}
+
 	u := &url.URL{Host: ep.Host(), Scheme: ep.Scheme()}
 	postValues := url.Values{}
 	postValues.Add("j_username", username)

pkg/nsx/services/common/store_test.go

@@ -136,7 +136,7 @@ var filterTag = func(v []model.Tag) []string {
 
 func Test_InitializeResourceStore(t *testing.T) {
 	config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{})
-	cluster, _ := nsx.NewCluster(config2, nil)
+	cluster, _ := nsx.NewCluster(config2)
 	rc, _ := cluster.NewRestConnector()
 
 	service := Service{

pkg/nsx/services/ippool/fake_test.go

@@ -53,7 +53,7 @@ func (f fakeRealizedEntitiesClient) List(_ string, _ string, _ string, _ *string
 
 func fakeService() *IPPoolService {
 	c := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{})
-	cluster, _ := nsx.NewCluster(c, nil)
+	cluster, _ := nsx.NewCluster(c)
 	rc, _ := cluster.NewRestConnector()
 	ipPoolStore := &IPPoolStore{ResourceStore: common.ResourceStore{
 		Indexer:     cache.NewIndexer(keyFunc, cache.Indexers{common.TagScopeIPPoolCRUID: indexFunc}),

pkg/nsx/services/securitypolicy/store_test.go

@@ -108,7 +108,7 @@ func Test_KeyFunc(t *testing.T) {
 
 func Test_InitializeRuleStore(t *testing.T) {
 	config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{})
-	cluster, _ := nsx.NewCluster(config2, nil)
+	cluster, _ := nsx.NewCluster(config2)
 	rc, _ := cluster.NewRestConnector()
 
 	service := SecurityPolicyService{
@@ -159,7 +159,7 @@ func Test_InitializeRuleStore(t *testing.T) {
 
 func Test_InitializeGroupStore(t *testing.T) {
 	config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{})
-	cluster, _ := nsx.NewCluster(config2, nil)
+	cluster, _ := nsx.NewCluster(config2)
 	rc, _ := cluster.NewRestConnector()
 
 	service := SecurityPolicyService{
@@ -210,7 +210,7 @@ func Test_InitializeGroupStore(t *testing.T) {
 
 func Test_InitializeSecurityPolicyStore(t *testing.T) {
 	config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{})
-	cluster, _ := nsx.NewCluster(config2, nil)
+	cluster, _ := nsx.NewCluster(config2)
 	rc, _ := cluster.NewRestConnector()
 
 	service := SecurityPolicyService{