lib/promscrape: adds validation for proxy_url scheme (#4823)

* lib/promscrape: adds validation for proxy_url scheme
adds tests
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4811

* Update lib/proxy/proxy.go

* Update lib/proxy/proxy.go

---------

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>
This commit is contained in:
Nikolay 2023-08-12 14:03:08 +02:00 committed by GitHub
parent 072d891ed9
commit f111ddb862
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 4 deletions

View file

@ -42,6 +42,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
* FEATURE: [Official Grafana dashboards for VictoriaMetrics](https://grafana.com/orgs/victoriametrics): correctly calculate `Bytes per point` value for single-server and cluster VM dashboards. Before, the calculation mistakenly accounted for the number of entries in indexdb in denominator, which could have shown lower values than expected. * FEATURE: [Official Grafana dashboards for VictoriaMetrics](https://grafana.com/orgs/victoriametrics): correctly calculate `Bytes per point` value for single-server and cluster VM dashboards. Before, the calculation mistakenly accounted for the number of entries in indexdb in denominator, which could have shown lower values than expected.
* FEATURE: [Alerting rules for VictoriaMetrics](https://github.com/VictoriaMetrics/VictoriaMetrics/tree/master/deployment/docker#alerts): `ConcurrentFlushesHitTheLimit` alerting rule was moved from [single-server](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts.yml) and [cluster](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts-cluster.yml) alerts to the [list of "health" alerts](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts-health.yml) as it could be related to many VictoriaMetrics components. * FEATURE: [Alerting rules for VictoriaMetrics](https://github.com/VictoriaMetrics/VictoriaMetrics/tree/master/deployment/docker#alerts): `ConcurrentFlushesHitTheLimit` alerting rule was moved from [single-server](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts.yml) and [cluster](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts-cluster.yml) alerts to the [list of "health" alerts](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts-health.yml) as it could be related to many VictoriaMetrics components.
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly validate scheme for `proxy_url` field at the scrape config. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4811) for details.
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly apply `if` filters during [relabeling](https://docs.victoriametrics.com/vmagent.html#relabeling-enhancements). Previously the `if` filter could improperly work. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4806) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4816). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly apply `if` filters during [relabeling](https://docs.victoriametrics.com/vmagent.html#relabeling-enhancements). Previously the `if` filter could improperly work. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4806) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4816).
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): use local scrape timestamps for the scraped metrics unless `honor_timestamps: true` option is explicitly set at [scrape_config](https://docs.victoriametrics.com/sd_configs.html#scrape_configs). This fixes gaps for metrics collected from [cadvisor](https://github.com/google/cadvisor) or similar exporters, which export metrics with invalid timestamps. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4697) and [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4697#issuecomment-1654614799) for details. The issue has been introduced in [v1.68.0](#v1680). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): use local scrape timestamps for the scraped metrics unless `honor_timestamps: true` option is explicitly set at [scrape_config](https://docs.victoriametrics.com/sd_configs.html#scrape_configs). This fixes gaps for metrics collected from [cadvisor](https://github.com/google/cadvisor) or similar exporters, which export metrics with invalid timestamps. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4697) and [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4697#issuecomment-1654614799) for details. The issue has been introduced in [v1.68.0](#v1680).
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): keep unmatched series at [stream aggregation](https://docs.victoriametrics.com/stream-aggregation.html) when `-remoteWrite.streamAggr.dropInput` is set to `false` to match intended behaviour introduced at [v1.92.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.92.0). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4804). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): keep unmatched series at [stream aggregation](https://docs.victoriametrics.com/stream-aggregation.html) when `-remoteWrite.streamAggr.dropInput` is set to `false` to match intended behaviour introduced at [v1.92.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.92.0). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4804).

View file

@ -18,6 +18,17 @@ import (
"golang.org/x/net/proxy" "golang.org/x/net/proxy"
) )
var validURLSchemes = []string{"http", "https", "socks5", "tls+socks5"}
func isURLSchemeValid(scheme string) bool {
for _, vs := range validURLSchemes {
if scheme == vs {
return true
}
}
return false
}
// URL implements YAML.Marshaler and yaml.Unmarshaler interfaces for url.URL. // URL implements YAML.Marshaler and yaml.Unmarshaler interfaces for url.URL.
type URL struct { type URL struct {
URL *url.URL URL *url.URL
@ -114,6 +125,9 @@ func (u *URL) UnmarshalYAML(unmarshal func(interface{}) error) error {
if err != nil { if err != nil {
return fmt.Errorf("cannot parse proxy_url=%q as *url.URL: %w", s, err) return fmt.Errorf("cannot parse proxy_url=%q as *url.URL: %w", s, err)
} }
if !isURLSchemeValid(parsedURL.Scheme) {
return fmt.Errorf("cannot parse proxy_url=%q unsupported scheme format=%q, valid schemes: %s", s, parsedURL.Scheme, validURLSchemes)
}
u.URL = parsedURL u.URL = parsedURL
return nil return nil
} }
@ -124,10 +138,8 @@ func (u *URL) NewDialFunc(ac *promauth.Config) (fasthttp.DialFunc, error) {
return defaultDialFunc, nil return defaultDialFunc, nil
} }
pu := u.URL pu := u.URL
switch pu.Scheme { if !isURLSchemeValid(pu.Scheme) {
case "http", "https", "socks5", "tls+socks5": return nil, fmt.Errorf("unknown scheme=%q for proxy_url=%q, must be in %s", pu.Scheme, pu.Redacted(), validURLSchemes)
default:
return nil, fmt.Errorf("unknown scheme=%q for proxy_url=%q, must be http, https, socks5 or tls+socks5", pu.Scheme, pu.Redacted())
} }
isTLS := (pu.Scheme == "https" || pu.Scheme == "tls+socks5") isTLS := (pu.Scheme == "https" || pu.Scheme == "tls+socks5")
proxyAddr := addMissingPort(pu.Host, isTLS) proxyAddr := addMissingPort(pu.Host, isTLS)

34
lib/proxy/proxy_test.go Normal file
View file

@ -0,0 +1,34 @@
package proxy
import (
"testing"
"gopkg.in/yaml.v3"
)
func TestURLParseSuccess(t *testing.T) {
f := func(src string) {
t.Helper()
var u URL
if err := yaml.Unmarshal([]byte(src), &u); err != nil {
t.Fatalf("unexpected error for url: %s: %s", src, err)
}
}
f("http://some-url/path")
f("https://some-url/path")
f("socks5://some-url/path")
f("tls+socks5://some-sock-path")
}
func TestParseFail(t *testing.T) {
f := func(src string) {
t.Helper()
var u URL
if err := yaml.Unmarshal([]byte(src), &u); err == nil {
t.Fatalf("want error for url: %s", src)
}
}
f("bad-scheme://my-url")
f("unix://my-socket.sock")
f("http://some-url:bad-port")
}