Respect proxy settings and set http timeout in login and ssh commands (…

…#406) Ref: rancher/rancher#48321
rancher · Dec 9, 2024 · 80099f6 · 80099f6
1 parent f0fe6d3
commit 80099f6
Show file tree

Hide file tree

Showing 6 changed files with 162 additions and 27 deletions.
diff --git a/cliclient/cliclient.go b/cliclient/cliclient.go
@@ -180,13 +180,14 @@ func createClientOpts(config *config.ServerConfig) *clientbase.ClientOpts {
 		serverURL = config.URL + "/v3"
 	}
 
-	options := &clientbase.ClientOpts{
+	return &clientbase.ClientOpts{
 		URL:       serverURL,
 		AccessKey: config.AccessKey,
 		SecretKey: config.SecretKey,
 		CACerts:   config.CACerts,
+		ProxyURL:  config.ProxyURL,
+		Timeout:   config.GetHTTPTimeout(),
 	}
-	return options
 }
 
 func SplitOnColon(s string) []string {

diff --git a/cmd/common.go b/cmd/common.go
@@ -3,11 +3,13 @@ package cmd
 import (
 	"bufio"
 	"bytes"
+	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
 	"io"
 	"math/rand"
+	"net/http"
 	"net/url"
 	"os"
 	"os/exec"
@@ -39,6 +41,7 @@ const (
 	letters             = "abcdefghijklmnopqrstuvwxyz0123456789"
 	cfgFile             = "cli2.json"
 	kubeConfigKeyFormat = "%s-%s"
+	defaultHTTPTimeout  = time.Minute // Matches the default timeout of the Norman Api Client.
 )
 
 var (
@@ -624,3 +627,33 @@ func ConfigDir() (string, error) {
 	}
 	return filepath.Join(homeDir, ".rancher"), nil
 }
+
+func newHTTPClient(serverConfig *config.ServerConfig, tlsConfig *tls.Config) (*http.Client, error) {
+	var proxy func(*http.Request) (*url.URL, error)
+	if serverConfig.ProxyURL != "" {
+		proxyURL, err := url.Parse(serverConfig.ProxyURL)
+		if err != nil {
+			return nil, fmt.Errorf("invalid proxy address %s: %w", serverConfig.ProxyURL, err)
+		}
+		proxy = http.ProxyURL(proxyURL)
+	} else {
+		proxy = http.ProxyFromEnvironment
+	}
+
+	tr := &http.Transport{
+		Proxy: proxy,
+	}
+	if tlsConfig != nil {
+		tr.TLSClientConfig = tlsConfig
+	}
+
+	timeout := serverConfig.GetHTTPTimeout()
+	if timeout == 0 {
+		timeout = defaultHTTPTimeout
+	}
+
+	return &http.Client{
+		Transport: tr,
+		Timeout:   timeout,
+	}, nil
+}
diff --git a/cmd/common_test.go b/cmd/common_test.go
@@ -1,11 +1,15 @@
 package cmd
 
 import (
+	"crypto/tls"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strconv"
 	"testing"
+	"time"
 
+	"github.com/rancher/cli/config"
 	managementClient "github.com/rancher/rancher/pkg/client/generated/management/v3"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -200,3 +204,91 @@ func TestGetMemberNameFromPrincipal(t *testing.T) {
 		})
 	}
 }
+
+func TestNewHTTPClient(t *testing.T) {
+	t.Run("default timeout and no proxy", func(t *testing.T) {
+		serverConfig := &config.ServerConfig{}
+		tlsConfig := &tls.Config{InsecureSkipVerify: true}
+
+		client, err := newHTTPClient(serverConfig, tlsConfig)
+		require.NoError(t, err)
+
+		assert.Equal(t, defaultHTTPTimeout, client.Timeout)
+
+		transport, ok := client.Transport.(*http.Transport)
+		require.True(t, ok)
+
+		assert.True(t, transport.TLSClientConfig.InsecureSkipVerify)
+	})
+
+	t.Run("set timeout", func(t *testing.T) {
+		serverConfig := &config.ServerConfig{
+			HTTPTimeoutSeconds: 30,
+		}
+
+		client, err := newHTTPClient(serverConfig, nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, 30*time.Second, client.Timeout)
+	})
+
+	t.Run("explicitly set proxy URL", func(t *testing.T) {
+		httpProxy := "http://corp.example.com:8080"
+		serverConfig := &config.ServerConfig{
+			ProxyURL: httpProxy,
+		}
+
+		client, err := newHTTPClient(serverConfig, nil)
+		require.NoError(t, err)
+
+		transport, ok := client.Transport.(*http.Transport)
+		require.True(t, ok)
+
+		req, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+		require.NoError(t, err)
+
+		proxyURL, err := transport.Proxy(req)
+		require.NoError(t, err)
+		require.NotNil(t, proxyURL)
+		assert.Equal(t, httpProxy, proxyURL.String())
+	})
+
+	t.Run("invalid proxy URL", func(t *testing.T) {
+		invalidURL := "http://corp .example.com:8080"
+		serverConfig := &config.ServerConfig{
+			ProxyURL: invalidURL,
+		}
+
+		_, err := newHTTPClient(serverConfig, nil)
+		require.Error(t, err)
+	})
+
+	t.Run("set proxy via env vars", func(t *testing.T) {
+		httpProxy := "http://corp.example.com:8080"
+		t.Setenv("HTTP_PROXY", httpProxy)
+		t.Setenv("NO_PROXY", "foo.com")
+
+		serverConfig := &config.ServerConfig{}
+
+		client, err := newHTTPClient(serverConfig, nil)
+		require.NoError(t, err)
+
+		transport, ok := client.Transport.(*http.Transport)
+		require.True(t, ok)
+
+		req, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+		require.NoError(t, err)
+
+		proxyURL, err := transport.Proxy(req)
+		require.NoError(t, err)
+		require.NotNil(t, proxyURL)
+		assert.Equal(t, httpProxy, proxyURL.String())
+
+		req, err = http.NewRequest(http.MethodGet, "http://foo.com", nil)
+		require.NoError(t, err)
+
+		proxyURL, err = transport.Proxy(req)
+		require.NoError(t, err)
+		require.Nil(t, proxyURL)
+	})
+}
diff --git a/cmd/login.go b/cmd/login.go
@@ -252,18 +252,19 @@ func getProjectContext(ctx *cli.Context, c *cliclient.MasterClient) (string, err
 	return projectCollection.Data[selection].ID, nil
 }
 
-func getCertFromServer(ctx *cli.Context, cf *config.ServerConfig) (*cliclient.MasterClient, error) {
-	req, err := http.NewRequest("GET", cf.URL+"/v3/settings/cacerts", nil)
+func getCertFromServer(ctx *cli.Context, serverConfig *config.ServerConfig) (*cliclient.MasterClient, error) {
+	req, err := http.NewRequest("GET", serverConfig.URL+"/v3/settings/cacerts", nil)
 	if err != nil {
 		return nil, err
 	}
 
-	req.SetBasicAuth(cf.AccessKey, cf.SecretKey)
+	req.SetBasicAuth(serverConfig.AccessKey, serverConfig.SecretKey)
 
-	tr := &http.Transport{
-		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+	tlsConfig := &tls.Config{InsecureSkipVerify: true}
+	client, err := newHTTPClient(serverConfig, tlsConfig)
+	if err != nil {
+		return nil, err
 	}
-	client := &http.Client{Transport: tr}
 
 	res, err := client.Do(req)
 	if err != nil {
@@ -280,7 +281,7 @@ func getCertFromServer(ctx *cli.Context, cf *config.ServerConfig) (*cliclient.Ma
 	var certReponse *CACertResponse
 	err = json.Unmarshal(content, &certReponse)
 	if err != nil {
-		return nil, fmt.Errorf("Unable to parse response from %s/v3/settings/cacerts\nError: %s\nResponse:\n%s", cf.URL, err, content)
+		return nil, fmt.Errorf("Unable to parse response from %s/v3/settings/cacerts\nError: %s\nResponse:\n%s", serverConfig.URL, err, content)
 	}
 
 	cert, err := verifyCert([]byte(certReponse.Value))
@@ -295,14 +296,14 @@ func getCertFromServer(ctx *cli.Context, cf *config.ServerConfig) (*cliclient.Ma
 	}
 
 	if !ctx.Bool("skip-verify") {
-		if ok := verifyUserAcceptsCert(serverCerts, cf.URL); !ok {
+		if ok := verifyUserAcceptsCert(serverCerts, serverConfig.URL); !ok {
 			return nil, errors.New("CACert of server was not accepted, unable to login")
 		}
 	}
 
-	cf.CACerts = cert
+	serverConfig.CACerts = cert
 
-	return cliclient.NewManagementClient(cf)
+	return cliclient.NewManagementClient(serverConfig)
 }
 
 func verifyUserAcceptsCert(certs []string, url string) bool {

diff --git a/cmd/ssh.go b/cmd/ssh.go
@@ -170,20 +170,21 @@ func getSSHKey(c *cliclient.MasterClient, link, nodeName string) ([]byte, string
 	req.SetBasicAuth(c.UserConfig.AccessKey, c.UserConfig.SecretKey)
 	req.Header.Add("Accept-Encoding", "zip")
 
-	client := &http.Client{}
-
+	var tlsConfig *tls.Config
 	if c.UserConfig.CACerts != "" {
 		roots := x509.NewCertPool()
 		ok := roots.AppendCertsFromPEM([]byte(c.UserConfig.CACerts))
 		if !ok {
 			return []byte{}, "", err
 		}
-		tr := &http.Transport{
-			TLSClientConfig: &tls.Config{
-				RootCAs: roots,
-			},
+		tlsConfig = &tls.Config{
+			RootCAs: roots,
 		}
-		client.Transport = tr
+	}
+
+	client, err := newHTTPClient(c.UserConfig, tlsConfig)
+	if err != nil {
+		return nil, "", err
 	}
 
 	resp, err := client.Do(req)

diff --git a/config/config.go b/config/config.go
@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"time"
 
 	"github.com/sirupsen/logrus"
 	"k8s.io/client-go/tools/clientcmd/api"
@@ -27,14 +28,20 @@ type Config struct {
 
 // ServerConfig holds the config for each server the user has setup
 type ServerConfig struct {
-	AccessKey       string                     `json:"accessKey"`
-	SecretKey       string                     `json:"secretKey"`
-	TokenKey        string                     `json:"tokenKey"`
-	URL             string                     `json:"url"`
-	Project         string                     `json:"project"`
-	CACerts         string                     `json:"cacert"`
-	KubeCredentials map[string]*ExecCredential `json:"kubeCredentials"`
-	KubeConfigs     map[string]*api.Config     `json:"kubeConfigs"`
+	AccessKey          string                     `json:"accessKey"`
+	SecretKey          string                     `json:"secretKey"`
+	TokenKey           string                     `json:"tokenKey"`
+	URL                string                     `json:"url"`
+	Project            string                     `json:"project"`
+	CACerts            string                     `json:"cacert"`
+	KubeCredentials    map[string]*ExecCredential `json:"kubeCredentials"`
+	KubeConfigs        map[string]*api.Config     `json:"kubeConfigs"`
+	ProxyURL           string                     `json:"proxyUrl"`
+	HTTPTimeoutSeconds int                        `json:"httpTimeoutSeconds"`
+}
+
+func (c *ServerConfig) GetHTTPTimeout() time.Duration {
+	return time.Duration(c.HTTPTimeoutSeconds) * time.Second
 }
 
 // LoadFromPath attempts to load a config from the given file path. If the file