utils,cluster/spec: honor NO_PROXY in HTTP clients

NewHTTPClient and makeTransport set the proxy via http.ProxyURL, which ignores
NO_PROXY and forces internal PD/TiKV requests through HTTP_PROXY, breaking
`tiup cluster reload` behind a proxy. Resolve it via x/net/http/httpproxy so
NO_PROXY is honored; the TIUP_INNER_HTTP_PROXY override is kept. Add unit tests.

Signed-off-by: Jaehui-Lee <tkdoqr1234@gmail.com>

Author: Jaehui-Lee

go.mod

@@ -56,6 +56,7 @@ require (
 	go.uber.org/zap v1.24.0
 	golang.org/x/crypto v0.48.0
 	golang.org/x/mod v0.33.0
+	golang.org/x/net v0.50.0
 	golang.org/x/sync v0.19.0
 	golang.org/x/sys v0.41.0
 	golang.org/x/term v0.40.0
@@ -145,7 +146,6 @@ require (
 	golang.org/x/exp v0.0.0-20260218203240-3dfff04db8fa // indirect
 	golang.org/x/exp/typeparams v0.0.0-20230321023759-10a507213a29 // indirect
 	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect
-	golang.org/x/net v0.50.0 // indirect
 	golang.org/x/telemetry v0.0.0-20260213145524-e0ab670178e1 // indirect
 	golang.org/x/tools v0.42.0 // indirect
 	golang.org/x/tools/go/expect v0.1.1-deprecated // indirect

pkg/cluster/spec/tikv.go

@@ -37,6 +37,7 @@ import (
 	dto "github.com/prometheus/client_model/go"
 	"github.com/prometheus/prom2json"
 	"go.uber.org/zap"
+	"golang.org/x/net/http/httpproxy"
 )
 
 const (
@@ -514,15 +515,18 @@ func makeTransport(tlsCfg *tls.Config) *http.Transport {
 		transport.TLSClientConfig = tlsCfg.Clone()
 	}
 
-	// prefer to use the inner http proxy
-	httpProxy := os.Getenv("TIUP_INNER_HTTP_PROXY")
-	if len(httpProxy) == 0 {
-		httpProxy = os.Getenv("HTTP_PROXY")
+	// Resolve the proxy from the environment so that both HTTP(S)_PROXY and
+	// NO_PROXY are honored. TIUP_INNER_HTTP_PROXY, if set, overrides the proxy
+	// URL but NO_PROXY is still respected, so internal endpoints (e.g. the TiKV
+	// status port) are scraped directly instead of through the proxy.
+	proxyCfg := httpproxy.FromEnvironment()
+	if inner := os.Getenv("TIUP_INNER_HTTP_PROXY"); len(inner) > 0 {
+		proxyCfg.HTTPProxy = inner
+		proxyCfg.HTTPSProxy = inner
 	}
-	if len(httpProxy) > 0 {
-		if proxyURL, err := url.Parse(httpProxy); err == nil {
-			transport.Proxy = http.ProxyURL(proxyURL)
-		}
+	proxyFunc := proxyCfg.ProxyFunc()
+	transport.Proxy = func(req *http.Request) (*url.URL, error) {
+		return proxyFunc(req.URL)
 	}
 	return transport
 }

pkg/cluster/spec/tikv_proxy_test.go

@@ -0,0 +1,74 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package spec
+
+import (
+	"net/http"
+	"testing"
+)
+
+// transportProxyFor returns the proxy URL string that makeTransport would use
+// for rawurl ("" means a direct connection / no proxy).
+func transportProxyFor(t *testing.T, rawurl string) string {
+	t.Helper()
+	tr := makeTransport(nil)
+	if tr.Proxy == nil {
+		t.Fatal("transport proxy func is not set")
+	}
+	req, err := http.NewRequest(http.MethodGet, rawurl, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	u, err := tr.Proxy(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u == nil {
+		return ""
+	}
+	return u.String()
+}
+
+// TestMakeTransportHonorsNoProxy verifies that a host listed in NO_PROXY (e.g.
+// an internal TiKV status endpoint scraped for the leader count) is reached
+// directly even when HTTP(S)_PROXY is set, while other hosts use the proxy.
+func TestMakeTransportHonorsNoProxy(t *testing.T) {
+	t.Setenv("TIUP_INNER_HTTP_PROXY", "")
+	t.Setenv("HTTP_PROXY", "http://proxy.example.com:3128")
+	t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128")
+	t.Setenv("NO_PROXY", "10.0.0.0/8,.internal.example.com")
+
+	if got := transportProxyFor(t, "http://tikv-1.internal.example.com:20180/metrics"); got != "" {
+		t.Errorf("host in NO_PROXY must bypass the proxy, got %q", got)
+	}
+	if got := transportProxyFor(t, "https://tiup-mirrors.pingcap.com/timestamp.json"); got != "http://proxy.example.com:3128" {
+		t.Errorf("external host must use the proxy, got %q", got)
+	}
+}
+
+// TestMakeTransportInnerProxyRespectsNoProxy verifies the TIUP_INNER_HTTP_PROXY
+// override still applies to external hosts but also respects NO_PROXY.
+func TestMakeTransportInnerProxyRespectsNoProxy(t *testing.T) {
+	t.Setenv("HTTP_PROXY", "")
+	t.Setenv("HTTPS_PROXY", "")
+	t.Setenv("NO_PROXY", ".internal.example.com")
+	t.Setenv("TIUP_INNER_HTTP_PROXY", "http://inner.example.com:3128")
+
+	if got := transportProxyFor(t, "https://tiup-mirrors.pingcap.com/timestamp.json"); got != "http://inner.example.com:3128" {
+		t.Errorf("external host must use the inner proxy, got %q", got)
+	}
+	if got := transportProxyFor(t, "http://tikv-1.internal.example.com:20180/metrics"); got != "" {
+		t.Errorf("host in NO_PROXY must bypass the inner proxy, got %q", got)
+	}
+}

pkg/utils/http_client.go

@@ -24,6 +24,8 @@ import (
 	"os"
 	"path/filepath"
 	"time"
+
+	"golang.org/x/net/http/httpproxy"
 )
 
 // HTTPClient is a wrap of http.Client
@@ -37,19 +39,22 @@ func NewHTTPClient(timeout time.Duration, tlsConfig *tls.Config) *HTTPClient {
 	if timeout < time.Second {
 		timeout = 10 * time.Second // default timeout is 10s
 	}
+	// Resolve the proxy from the environment so that both HTTP(S)_PROXY and
+	// NO_PROXY are honored. TIUP_INNER_HTTP_PROXY, if set, overrides the proxy
+	// URL but NO_PROXY is still respected, so internal endpoints (e.g. PD)
+	// listed in NO_PROXY are reached directly instead of through the proxy.
+	proxyCfg := httpproxy.FromEnvironment()
+	if inner := os.Getenv("TIUP_INNER_HTTP_PROXY"); len(inner) > 0 {
+		proxyCfg.HTTPProxy = inner
+		proxyCfg.HTTPSProxy = inner
+	}
+	proxyFunc := proxyCfg.ProxyFunc()
 	tr := &http.Transport{
 		TLSClientConfig: tlsConfig,
 		Dial:            (&net.Dialer{Timeout: 3 * time.Second}).Dial,
-	}
-	// prefer to use the inner http proxy
-	httpProxy := os.Getenv("TIUP_INNER_HTTP_PROXY")
-	if len(httpProxy) == 0 {
-		httpProxy = os.Getenv("HTTP_PROXY")
-	}
-	if len(httpProxy) > 0 {
-		if proxyURL, err := url.Parse(httpProxy); err == nil {
-			tr.Proxy = http.ProxyURL(proxyURL)
-		}
+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return proxyFunc(req.URL)
+		},
 	}
 	return &HTTPClient{
 		client: &http.Client{

pkg/utils/http_client_proxy_test.go

@@ -0,0 +1,74 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"net/http"
+	"testing"
+)
+
+// proxyForURL returns the proxy URL string that the client's transport would
+// use for rawurl ("" means a direct connection / no proxy).
+func proxyForURL(t *testing.T, rawurl string) string {
+	t.Helper()
+	c := NewHTTPClient(0, nil)
+	tr, ok := c.client.Transport.(*http.Transport)
+	if !ok || tr.Proxy == nil {
+		t.Fatalf("transport proxy func is not set")
+	}
+	req, err := http.NewRequest(http.MethodGet, rawurl, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	u, err := tr.Proxy(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u == nil {
+		return ""
+	}
+	return u.String()
+}
+
+// TestNewHTTPClientHonorsNoProxy verifies that a host listed in NO_PROXY is
+// reached directly even when HTTP(S)_PROXY is set, while other hosts use it.
+func TestNewHTTPClientHonorsNoProxy(t *testing.T) {
+	t.Setenv("TIUP_INNER_HTTP_PROXY", "")
+	t.Setenv("HTTP_PROXY", "http://proxy.example.com:3128")
+	t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128")
+	t.Setenv("NO_PROXY", "10.0.0.0/8,.internal.example.com")
+
+	if got := proxyForURL(t, "http://pd-1.internal.example.com:2379/pd/api/v1/config"); got != "" {
+		t.Errorf("host in NO_PROXY must bypass the proxy, got %q", got)
+	}
+	if got := proxyForURL(t, "https://tiup-mirrors.pingcap.com/timestamp.json"); got != "http://proxy.example.com:3128" {
+		t.Errorf("external host must use the proxy, got %q", got)
+	}
+}
+
+// TestNewHTTPClientInnerProxyRespectsNoProxy verifies the TIUP_INNER_HTTP_PROXY
+// override still applies to external hosts but also respects NO_PROXY.
+func TestNewHTTPClientInnerProxyRespectsNoProxy(t *testing.T) {
+	t.Setenv("HTTP_PROXY", "")
+	t.Setenv("HTTPS_PROXY", "")
+	t.Setenv("NO_PROXY", ".internal.example.com")
+	t.Setenv("TIUP_INNER_HTTP_PROXY", "http://inner.example.com:3128")
+
+	if got := proxyForURL(t, "https://tiup-mirrors.pingcap.com/timestamp.json"); got != "http://inner.example.com:3128" {
+		t.Errorf("external host must use the inner proxy, got %q", got)
+	}
+	if got := proxyForURL(t, "http://pd-1.internal.example.com:2379/x"); got != "" {
+		t.Errorf("host in NO_PROXY must bypass the inner proxy, got %q", got)
+	}
+}