lib/promscrape: disable support for service discovery and metrics scrape via http2

Reasons for disabling http2:

- http2 is used very rarely comparing to http for Prometheus metrics exposition and service discovery
- http2 is much harder to debug than http
- http2 has very bad security record because of its complexity - see https://portswigger.net/research/http2

VictoriaMetrics components are compiled with nethttpomithttp2 tag because of these issues.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4283
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4274

This is a follow-up for 72c3cd47eb
This commit is contained in:
Aliaksandr Valialkin 2023-07-06 15:59:56 -07:00
parent 7f3b5431a1
commit 8a07621a0c
No known key found for this signature in database
GPG key ID: A72BEC6CD3D0DED1
18 changed files with 34 additions and 61 deletions

View file

@ -127,8 +127,15 @@ type HTTPClientConfig struct {
// FollowRedirects specifies whether the client should follow HTTP 3xx redirects.
FollowRedirects *bool `yaml:"follow_redirects,omitempty"`
// EnableHTTP2 specifies whether the client should configure HTTP2.
EnableHTTP2 *bool `yaml:"enable_http2,omitempty"`
// Do not support enable_http2 option because of the following reasons:
//
// - http2 is used very rarely comparing to http for Prometheus metrics exposition and service discovery
// - http2 is much harder to debug than http
// - http2 has very bad security record because of its complexity - see https://portswigger.net/research/http2
//
// VictoriaMetrics components are compiled with nethttpomithttp2 tag because of these issues.
//
// EnableHTTP2 bool
}
// ProxyClientConfig represents proxy client config.

View file

@ -11,8 +11,6 @@ import (
"strings"
"time"
"golang.org/x/net/http2"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/flagutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
@ -162,12 +160,6 @@ func newClient(ctx context.Context, sw *ScrapeWork) *client {
// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1017#issuecomment-767235047
Timeout: 30 * sw.ScrapeTimeout,
}
if sw.EnableHTTP2 {
_, err := http2.ConfigureTransports(sc.Transport.(*http.Transport))
if err != nil {
logger.Errorf("failed to configure HTTP/2 transport: %s", err)
}
}
if sw.DenyRedirects {
sc.CheckRedirect = func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse

View file

@ -992,10 +992,6 @@ func getScrapeWorkConfig(sc *ScrapeConfig, baseDir string, globalCfg *GlobalConf
if sc.HTTPClientConfig.FollowRedirects != nil {
denyRedirects = !*sc.HTTPClientConfig.FollowRedirects
}
enableHTTP2 := false
if sc.HTTPClientConfig.EnableHTTP2 != nil {
enableHTTP2 = !*sc.HTTPClientConfig.EnableHTTP2
}
metricsPath := sc.MetricsPath
if metricsPath == "" {
metricsPath = "/metrics"
@ -1048,7 +1044,6 @@ func getScrapeWorkConfig(sc *ScrapeConfig, baseDir string, globalCfg *GlobalConf
honorLabels: honorLabels,
honorTimestamps: honorTimestamps,
denyRedirects: denyRedirects,
enableHTTP2: enableHTTP2,
externalLabels: externalLabels,
relabelConfigs: relabelConfigs,
metricRelabelConfigs: metricRelabelConfigs,
@ -1079,7 +1074,6 @@ type scrapeWorkConfig struct {
honorLabels bool
honorTimestamps bool
denyRedirects bool
enableHTTP2 bool
externalLabels *promutils.Labels
relabelConfigs *promrelabel.ParsedConfigs
metricRelabelConfigs *promrelabel.ParsedConfigs
@ -1357,7 +1351,6 @@ func (swc *scrapeWorkConfig) getScrapeWork(target string, extraLabels, metaLabel
HonorLabels: swc.honorLabels,
HonorTimestamps: swc.honorTimestamps,
DenyRedirects: swc.denyRedirects,
EnableHTTP2: swc.enableHTTP2,
OriginalLabels: originalLabels,
Labels: labelsCopy,
ExternalLabels: swc.externalLabels,

View file

@ -110,7 +110,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, err
}
c, err := discoveryutils.NewClient(env.ResourceManagerEndpoint, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
c, err := discoveryutils.NewClient(env.ResourceManagerEndpoint, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create client for %q: %w", env.ResourceManagerEndpoint, err)
}
@ -230,7 +230,7 @@ func getRefreshTokenFunc(sdc *SDConfig, ac, proxyAC *promauth.Config, env *cloud
return nil, fmt.Errorf("unsupported `authentication_method: %q` only `OAuth` and `ManagedIdentity` are supported", authenticationMethod)
}
authClient, err := discoveryutils.NewClient(tokenEndpoint, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
authClient, err := discoveryutils.NewClient(tokenEndpoint, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot build auth client: %w", err)
}

View file

@ -67,7 +67,7 @@ func TestGetVirtualMachinesSuccess(t *testing.T) {
}
}))
defer testServer.Close()
c, err := discoveryutils.NewClient(testServer.URL, nil, nil, nil, promauth.HTTPClientConfig{})
c, err := discoveryutils.NewClient(testServer.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
if err != nil {
t.Fatalf("unexpected error at client create: %s", err)
}

View file

@ -80,7 +80,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -74,7 +74,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -36,7 +36,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -50,7 +50,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(sdc.Host, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(sdc.Host, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", sdc.Host, err)
}

View file

@ -49,7 +49,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(sdc.Host, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(sdc.Host, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", sdc.Host, err)
}

View file

@ -34,7 +34,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -44,7 +44,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -60,7 +60,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -68,7 +68,7 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
if err != nil {
return nil, fmt.Errorf("cannot parse proxy auth config: %w", err)
}
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, sdc.HTTPClientConfig)
client, err := discoveryutils.NewClient(apiServer, ac, sdc.ProxyURL, proxyAC, &sdc.HTTPClientConfig)
if err != nil {
return nil, fmt.Errorf("cannot create HTTP client for %q: %w", apiServer, err)
}

View file

@ -14,8 +14,6 @@ import (
"sync"
"time"
"golang.org/x/net/http2"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/promauth"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/proxy"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/timerpool"
@ -86,7 +84,7 @@ type HTTPClient struct {
var defaultDialer = &net.Dialer{}
// NewClient returns new Client for the given args.
func NewClient(apiServer string, ac *promauth.Config, proxyURL *proxy.URL, proxyAC *promauth.Config, httpCfg promauth.HTTPClientConfig) (*Client, error) {
func NewClient(apiServer string, ac *promauth.Config, proxyURL *proxy.URL, proxyAC *promauth.Config, httpCfg *promauth.HTTPClientConfig) (*Client, error) {
u, err := url.Parse(apiServer)
if err != nil {
return nil, fmt.Errorf("cannot parse apiServer=%q: %w", apiServer, err)
@ -155,12 +153,6 @@ func NewClient(apiServer string, ac *promauth.Config, proxyURL *proxy.URL, proxy
proxyURL.SetHeaders(proxyAC, req)
}
}
if httpCfg.EnableHTTP2 != nil && *httpCfg.EnableHTTP2 {
_, err := http2.ConfigureTransports(client.Transport.(*http.Transport))
if err != nil {
return nil, fmt.Errorf("failed to configure HTTP/2 transport: %s", err)
}
}
ctx, cancel := context.WithCancel(context.Background())
c := &Client{

View file

@ -58,22 +58,6 @@ func TestNewClientFromConfig(t *testing.T) {
},
expectedMessage: "I'm before redirect",
},
{
httpCfg: promauth.HTTPClientConfig{
EnableHTTP2: &allowed,
},
handler: func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/redirected":
fmt.Fprint(w, "I'm here to serve you!!!")
default:
w.Header().Set("Location", "/redirected")
w.WriteHeader(http.StatusFound)
fmt.Fprint(w, "I'm before redirect")
}
},
expectedMessage: "I'm here to serve you!!!",
},
}
for _, validConfig := range newClientValidConfig {
@ -83,7 +67,7 @@ func TestNewClientFromConfig(t *testing.T) {
}
defer testServer.Close()
client, err := NewClient("http://0.0.0.0:1234", nil, &proxy.URL{}, nil, validConfig.httpCfg)
client, err := NewClient("http://0.0.0.0:1234", nil, &proxy.URL{}, nil, &validConfig.httpCfg)
if err != nil {
t.Errorf("Can't create a client from this config: %+v", validConfig.httpCfg)
continue

View file

@ -66,8 +66,15 @@ type ScrapeWork struct {
// Whether to deny redirects during requests to scrape config.
DenyRedirects bool
// Whether to configure HTTP2.
EnableHTTP2 bool
// Do not support enable_http2 option because of the following reasons:
//
// - http2 is used very rarely comparing to http for Prometheus metrics exposition and service discovery
// - http2 is much harder to debug than http
// - http2 has very bad security record because of its complexity - see https://portswigger.net/research/http2
//
// VictoriaMetrics components are compiled with nethttpomithttp2 tag because of these issues.
//
// EnableHTTP2 bool
// OriginalLabels contains original labels before relabeling.
//
@ -402,8 +409,8 @@ func (sw *scrapeWork) mustSwitchToStreamParseMode(responseSize int) bool {
// getTargetResponse() fetches response from sw target in the same way as when scraping the target.
func (sw *scrapeWork) getTargetResponse() ([]byte, error) {
// use stream reader when stream mode enabled or http2 enabled
if *streamParse || sw.Config.StreamParse || sw.mustSwitchToStreamParseMode(sw.prevBodyLen) || sw.Config.EnableHTTP2 {
// use stream reader when stream mode enabled
if *streamParse || sw.Config.StreamParse || sw.mustSwitchToStreamParseMode(sw.prevBodyLen) {
// Read the response in stream mode.
sr, err := sw.GetStreamReader()
if err != nil {

View file

@ -7,7 +7,6 @@ scrape_configs:
honor_labels: true
honor_timestamps: false
follow_redirects: false
enable_http2: true
static_configs:
- targets: ["foo.bar", "aaa"]
labels:
@ -22,7 +21,6 @@ scrape_configs:
- role: endpoints
api_server: "https://localhost:1234"
follow_redirects: true
enable_http2: true
tls_config:
cert_file: valid_cert_file
key_file: valid_key_file