HTTP timeouts and proxy

.mockery.yaml

@@ -0,0 +1,12 @@
+with-expecter: true
+dir: "{{.InterfaceDir}}/mocks"
+outpkg: "mocks"
+packages:
+    github.com/rancher/cli/cliclient:
+        # place your package-specific config here
+        config:
+        interfaces:
+            # select the interfaces you want mocked
+            HTTPClienter:
+                # Modify package-level config for this specific interface (if applicable)
+                config:

README.md

@@ -44,6 +44,12 @@ Pass credentials by replacing `<PATH_TO_CONFIG>` with your config file for the s
 
 To build `rancher/cli`, run `make`.  To use a custom Docker repository, do `REPO=custom make`, which produces a `custom/cli` image.
 
+## Contributing
+
+Mocks can ge generated by below command:
+
+`make mock`
+
 ## Contact
 
 For bugs, questions, comments, corrections, suggestions, etc., open an issue in

cliclient/cliclient.go

@@ -1,9 +1,13 @@
 package cliclient
 
 import (
+	"context"
 	"errors"
 	"fmt"
+	"net"
+	"net/http"
 	"strings"
+	"time"
 
 	errorsPkg "github.com/pkg/errors"
 	"github.com/rancher/cli/config"
@@ -25,6 +29,28 @@ type MasterClient struct {
 	CAPIClient       *capiClient.Client
 }
 
+// HTTPClienter is a http.Client factory interface
+type HTTPClienter interface {
+	New() *http.Client
+}
+
+// DefaultHTTPClient stores the default http.Client factory
+var DefaultHTTPClient HTTPClienter = &HTTPClient{}
+
+/*
+TestingReplaceDefaultHTTPClient replaces DefaultHTTPClient for unit tests.
+Call the returned function by defer keyword, for example:
+
+	defer cliclient.TestingReplaceDefaultHTTPClient(mockClient)()
+*/
+func TestingReplaceDefaultHTTPClient(fakeClient HTTPClienter) func() {
+	origHttpClient := DefaultHTTPClient
+	DefaultHTTPClient = fakeClient
+	return func() {
+		DefaultHTTPClient = origHttpClient
+	}
+}
+
 // NewMasterClient returns a new MasterClient with Cluster, Management and Project
 // clients populated
 func NewMasterClient(config *config.ServerConfig) (*MasterClient, error) {
@@ -99,8 +125,30 @@ func NewProjectClient(config *config.ServerConfig) (*MasterClient, error) {
 	return mc, nil
 }
 
+var testingForceClientInsecure = false
+
+/*
+TestingForceClientInsecure sets testForceClientInsecure to true for unit tests.
+It's a workaround to github.com/rancher/norman/clientbase.NewAPIClient,
+which replaces net/http.Client.Transport (including proxy and TLS config),
+so the client TLS config of net/http/httptest.Server will be lost.
+Call the returned function by defer keyword, for example:
+
+	defer cliclient.TestingForceClientInsecure()()
+*/
+func TestingForceClientInsecure() func() {
+	origTestForceClientInsecure := testingForceClientInsecure
+	testingForceClientInsecure = true
+	return func() {
+		testingForceClientInsecure = origTestForceClientInsecure
+	}
+}
+
 func (mc *MasterClient) newManagementClient() error {
 	options := createClientOpts(mc.UserConfig)
+	if testingForceClientInsecure {
+		options.Insecure = true
+	}
 
 	// Setup the management client
 	mClient, err := managementClient.NewClient(options)
@@ -181,10 +229,11 @@ func createClientOpts(config *config.ServerConfig) *clientbase.ClientOpts {
 	}
 
 	options := &clientbase.ClientOpts{
-		URL:       serverURL,
-		AccessKey: config.AccessKey,
-		SecretKey: config.SecretKey,
-		CACerts:   config.CACerts,
+		HTTPClient: DefaultHTTPClient.New(),
+		URL:        serverURL,
+		AccessKey:  config.AccessKey,
+		SecretKey:  config.SecretKey,
+		CACerts:    config.CACerts,
 	}
 	return options
 }
@@ -203,3 +252,27 @@ func CheckProject(s string) []string {
 
 	return clustProj
 }
+
+type HTTPClient struct{}
+
+/*
+HTTPClient.New makes http.Client including http.Transport,
+with default values (for example: proxy) and custom timeouts.
+See: https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/
+*/
+func (c *HTTPClient) New() *http.Client {
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		dialer := &net.Dialer{
+			Timeout:   5 * time.Second,
+			KeepAlive: 30 * time.Second,
+		}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	transport.ResponseHeaderTimeout = 10 * time.Second
+
+	return &http.Client{
+		Transport: transport,
+		Timeout:   time.Minute, // from github.com/rancher/norman/clientbase.NewAPIClient
+	}
+}

cliclient/cliclient_test.go

@@ -0,0 +1,101 @@
+package cliclient
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"reflect"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/rancher/cli/cliclient/mocks"
+	"github.com/rancher/cli/config"
+	"github.com/rancher/norman/types"
+)
+
+func TestCreateClientOpts(t *testing.T) {
+	conf := &config.ServerConfig{
+		URL:       "https://a.b",
+		AccessKey: "AccessKey",
+		SecretKey: "SecretKey",
+		CACerts:   "CACerts",
+	}
+
+	clientOpts := createClientOpts(conf)
+
+	assert.NotNil(t, clientOpts)
+	assert.NotNil(t, clientOpts.HTTPClient)
+	assert.NotNil(t, clientOpts.HTTPClient.Transport)
+	assert.Equal(t, "https://a.b/v3", clientOpts.URL)
+	assert.Equal(t, "AccessKey", clientOpts.AccessKey)
+	assert.Equal(t, "SecretKey", clientOpts.SecretKey)
+	assert.Equal(t, "CACerts", clientOpts.CACerts)
+}
+
+func TestDefaultHTTPClient(t *testing.T) {
+	client := DefaultHTTPClient.New()
+
+	assert.NotNil(t, client)
+	assert.Equal(t, time.Minute, client.Timeout)
+
+	assert.NotNil(t, client.Transport)
+	transport, is := client.Transport.(*http.Transport)
+	assert.True(t, is)
+	assert.NotNil(t, transport)
+	assert.NotNil(t, transport.DialContext)
+	assert.NotNil(t, transport.Proxy)
+	assert.Equal(t, "net/http.ProxyFromEnvironment", runtime.FuncForPC(reflect.ValueOf(transport.Proxy).Pointer()).Name())
+	assert.Equal(t, 10*time.Second, transport.ResponseHeaderTimeout)
+}
+
+func TestMasterClientNewManagementClient(t *testing.T) {
+	server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/v3/settings/cacerts":
+			crt, _ := os.ReadFile("../test/ca-cert.pem")
+			resp := map[string]string{
+				"name":  "cacerts",
+				"value": string(crt),
+			}
+			respBody, _ := json.Marshal(resp)
+			_, _ = w.Write(respBody)
+		case "/v3":
+			schemaURL := url.URL{
+				Scheme: "https",
+				Host:   r.Host,
+				Path:   r.URL.Path,
+			}
+			w.Header().Add("X-API-Schemas", schemaURL.String())
+			schemas := &types.SchemaCollection{
+				Data: []types.Schema{},
+			}
+			respBody, _ := json.Marshal(schemas)
+			_, _ = w.Write(respBody)
+		default:
+			fmt.Println(r.URL.Path)
+		}
+	}))
+	defer server.Close()
+	server.StartTLS()
+
+	fakeClient := server.Client()
+	mockClient := mocks.NewMockHTTPClienter(t)
+	mockClient.EXPECT().New().Return(fakeClient)
+	defer TestingReplaceDefaultHTTPClient(mockClient)()
+	defer TestingForceClientInsecure()()
+
+	conf := &config.ServerConfig{
+		URL: server.URL,
+	}
+
+	mc, err := NewManagementClient(conf)
+
+	assert.Nil(t, err)
+	assert.NotNil(t, mc)
+}

cmd/kubectl_token.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/rancher/cli/cliclient"
 	"github.com/rancher/cli/config"
 	apiv3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
 	managementClient "github.com/rancher/rancher/pkg/client/generated/management/v3"
@@ -639,10 +640,9 @@ func getClient(skipVerify bool, caCerts string) (*http.Client, error) {
 		return nil, err
 	}
 
-	// clone the DefaultTransport to get the default values
-	transport := http.DefaultTransport.(*http.Transport).Clone()
-	transport.TLSClientConfig = tlsConfig
-	return &http.Client{Transport: transport}, nil
+	client := cliclient.DefaultHTTPClient.New()
+	client.Transport.(*http.Transport).TLSClientConfig = tlsConfig
+	return client, nil
 }
 
 func getTLSConfig(skipVerify bool, caCerts string) (*tls.Config, error) {

cmd/kubectl_token_test.go

@@ -114,3 +114,20 @@ var responseOK = `{
         }
     ]
 }`
+
+func Test_getClient(t *testing.T) {
+	/* ca-cert.pem is generated by:
+	openssl genrsa 2048 > ca-key.pem
+	openssl req -new -x509 -nodes -days 365000 -key ca-key.pem -out ca-cert.pem
+	*/
+	client, err := getClient(false, "../test/ca-cert.pem")
+
+	assert.Nil(t, err)
+	assert.NotNil(t, client)
+	assert.NotNil(t, client.Transport)
+	transport, is := client.Transport.(*http.Transport)
+	assert.True(t, is)
+	assert.NotNil(t, transport)
+	assert.NotNil(t, transport.TLSClientConfig)
+	assert.NotNil(t, transport.TLSClientConfig.RootCAs)
+}

cmd/login.go

@@ -260,10 +260,8 @@ func getCertFromServer(ctx *cli.Context, cf *config.ServerConfig) (*cliclient.Ma
 
 	req.SetBasicAuth(cf.AccessKey, cf.SecretKey)
 
-	tr := &http.Transport{
-		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-	}
-	client := &http.Client{Transport: tr}
+	client := cliclient.DefaultHTTPClient.New()
+	client.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 
 	res, err := client.Do(req)
 	if err != nil {