diff --git a/go.mod b/go.mod index 0a9dbe32c1..8d37f04f60 100644 --- a/go.mod +++ b/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 diff --git a/pkg/cluster/spec/tikv.go b/pkg/cluster/spec/tikv.go index a52d77ed13..42fee5e2fc 100644 --- a/pkg/cluster/spec/tikv.go +++ b/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 } diff --git a/pkg/cluster/spec/tikv_proxy_test.go b/pkg/cluster/spec/tikv_proxy_test.go new file mode 100644 index 0000000000..0ed63f0a0a --- /dev/null +++ b/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) + } +} diff --git a/pkg/utils/http_client.go b/pkg/utils/http_client.go index 8e9b14b61d..441edfba5d 100644 --- a/pkg/utils/http_client.go +++ b/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{ diff --git a/pkg/utils/http_client_proxy_test.go b/pkg/utils/http_client_proxy_test.go new file mode 100644 index 0000000000..5b40c49fdf --- /dev/null +++ b/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) + } +}