From 065a59ad0344d1f5f8621ed6ad425c74e9111119 Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Fri, 7 May 2021 17:03:00 +0200 Subject: [PATCH] Use upstream parameter follow_redirects Signed-off-by: Julien Pivotto --- CHANGELOG.md | 5 +++++ CONFIGURATION.md | 2 +- config/config.go | 22 ++++++++++++++++++++-- config/config_test.go | 6 +++--- main.go | 6 +++--- prober/http.go | 2 +- prober/http_test.go | 10 +++++----- 7 files changed, 38 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0bb560..1b6679d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ This release is built with go 1.16.4, which contains a [bugfix](https://github.com/golang/go/issues/45712) that can cause an untrusted target to make Blackbox Exporter crash. +In the HTTP probe, `no_follow_redirect` has been changed to `follow_redirect`. +This release accepts both, with a precedence to the `no_follow_redirect` parameter. +In the next release, `no_follow_redirect` will be removed. + +* [CHANGE] HTTP proble: no_follow_redirect has been renamed to follow_redirect. * [FEATURE] Add support for decompression of HTTP responses. #764 * [FEATURE] Enable TLS and basic authentication. #784 * [FEATURE] HTTP probe: *experimental* OAuth2 support. diff --git a/CONFIGURATION.md b/CONFIGURATION.md index 3928bb5..2d83584 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -62,7 +62,7 @@ The other placeholders are specified separately. [ compression: | default = "" ] # Whether or not the probe will follow any redirects. - [ no_follow_redirects: | default = false ] + [ follow_redirects: | default = true ] # Probe fails if SSL is present. [ fail_if_ssl: | default = false ] diff --git a/config/config.go b/config/config.go index 178bda2..07e84c0 100644 --- a/config/config.go +++ b/config/config.go @@ -28,6 +28,8 @@ import ( yaml "gopkg.in/yaml.v3" + "github.com/go-kit/kit/log" + "github.com/go-kit/kit/log/level" "github.com/miekg/dns" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/config" @@ -57,6 +59,7 @@ var ( // DefaultHTTPProbe set default value for HTTPProbe DefaultHTTPProbe = HTTPProbe{ IPProtocolFallback: true, + HTTPClientConfig: config.DefaultHTTPClientConfig, } // DefaultTCPProbe set default value for TCPProbe @@ -89,7 +92,7 @@ type SafeConfig struct { C *Config } -func (sc *SafeConfig) ReloadConfig(confFile string) (err error) { +func (sc *SafeConfig) ReloadConfig(confFile string, logger log.Logger) (err error) { var c = &Config{} defer func() { if err != nil { @@ -112,6 +115,17 @@ func (sc *SafeConfig) ReloadConfig(confFile string) (err error) { return fmt.Errorf("error parsing config file: %s", err) } + for name, module := range c.Modules { + if module.HTTP.NoFollowRedirects != nil { + // Hide the old flag from the /config page. + module.HTTP.NoFollowRedirects = nil + c.Modules[name] = module + if logger != nil { + level.Warn(logger).Log("msg", "no_follow_redirects is deprecated and will be removed in the next release. It is replaced by follow_redirects.", "module", name) + } + } + } + sc.Lock() sc.C = c sc.Unlock() @@ -181,7 +195,7 @@ type HTTPProbe struct { ValidHTTPVersions []string `yaml:"valid_http_versions,omitempty"` IPProtocol string `yaml:"preferred_ip_protocol,omitempty"` IPProtocolFallback bool `yaml:"ip_protocol_fallback,omitempty"` - NoFollowRedirects bool `yaml:"no_follow_redirects,omitempty"` + NoFollowRedirects *bool `yaml:"no_follow_redirects,omitempty"` FailIfSSL bool `yaml:"fail_if_ssl,omitempty"` FailIfNotSSL bool `yaml:"fail_if_not_ssl,omitempty"` Method string `yaml:"method,omitempty"` @@ -277,6 +291,10 @@ func (s *HTTPProbe) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } + if s.NoFollowRedirects != nil { + s.HTTPClientConfig.FollowRedirects = !*s.NoFollowRedirects + } + for key, value := range s.Headers { switch strings.Title(key) { case "Accept-Encoding": diff --git a/config/config_test.go b/config/config_test.go index 8a07032..097acdb 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -25,7 +25,7 @@ func TestLoadConfig(t *testing.T) { C: &Config{}, } - err := sc.ReloadConfig("testdata/blackbox-good.yml") + err := sc.ReloadConfig("testdata/blackbox-good.yml", nil) if err != nil { t.Errorf("Error loading config %v: %v", "blackbox.yml", err) } @@ -90,7 +90,7 @@ func TestLoadBadConfigs(t *testing.T) { } for _, test := range tests { t.Run(test.input, func(t *testing.T) { - got := sc.ReloadConfig(test.input) + got := sc.ReloadConfig(test.input, nil) if got == nil || got.Error() != test.want { t.Fatalf("ReloadConfig(%q) = %v; want %q", test.input, got, test.want) } @@ -103,7 +103,7 @@ func TestHideConfigSecrets(t *testing.T) { C: &Config{}, } - err := sc.ReloadConfig("testdata/blackbox-good.yml") + err := sc.ReloadConfig("testdata/blackbox-good.yml", nil) if err != nil { t.Errorf("Error loading config %v: %v", "testdata/blackbox-good.yml", err) } diff --git a/main.go b/main.go index f2294d7..d0a746f 100644 --- a/main.go +++ b/main.go @@ -224,7 +224,7 @@ func run() int { level.Info(logger).Log("msg", "Starting blackbox_exporter", "version", version.Info()) level.Info(logger).Log("build_context", version.BuildContext()) - if err := sc.ReloadConfig(*configFile); err != nil { + if err := sc.ReloadConfig(*configFile, logger); err != nil { level.Error(logger).Log("msg", "Error loading config", "err", err) return 1 } @@ -265,13 +265,13 @@ func run() int { for { select { case <-hup: - if err := sc.ReloadConfig(*configFile); err != nil { + if err := sc.ReloadConfig(*configFile, logger); err != nil { level.Error(logger).Log("msg", "Error reloading config", "err", err) continue } level.Info(logger).Log("msg", "Reloaded config file") case rc := <-reloadCh: - if err := sc.ReloadConfig(*configFile); err != nil { + if err := sc.ReloadConfig(*configFile, logger); err != nil { level.Error(logger).Log("msg", "Error reloading config", "err", err) rc <- err } else { diff --git a/prober/http.go b/prober/http.go index 39bb4d5..95c5353 100644 --- a/prober/http.go +++ b/prober/http.go @@ -373,7 +373,7 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr client.CheckRedirect = func(r *http.Request, via []*http.Request) error { level.Info(logger).Log("msg", "Received redirect", "location", r.Response.Header.Get("Location")) redirects = len(via) - if redirects > 10 || httpConfig.NoFollowRedirects { + if redirects > 10 || !httpConfig.HTTPClientConfig.FollowRedirects { level.Info(logger).Log("msg", "Not following redirect") return errors.New("don't follow redirects") } diff --git a/prober/http_test.go b/prober/http_test.go index be38038..6679b3c 100644 --- a/prober/http_test.go +++ b/prober/http_test.go @@ -526,7 +526,7 @@ func TestRedirectFollowed(t *testing.T) { registry := prometheus.NewRegistry() testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - result := ProbeHTTP(testCTX, ts.URL, config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true}}, registry, log.NewNopLogger()) + result := ProbeHTTP(testCTX, ts.URL, config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: pconfig.DefaultHTTPClientConfig}}, registry, log.NewNopLogger()) body := recorder.Body.String() if !result { t.Fatalf("Redirect test failed unexpectedly, got %s", body) @@ -554,7 +554,7 @@ func TestRedirectNotFollowed(t *testing.T) { testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() result := ProbeHTTP(testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, NoFollowRedirects: true, ValidStatusCodes: []int{302}}}, registry, log.NewNopLogger()) + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: pconfig.HTTPClientConfig{FollowRedirects: false}, ValidStatusCodes: []int{302}}}, registry, log.NewNopLogger()) body := recorder.Body.String() if !result { t.Fatalf("Redirect test failed unexpectedly, got %s", body) @@ -601,7 +601,7 @@ func TestRedirectionLimit(t *testing.T) { result := ProbeHTTP( testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true}}, + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: pconfig.DefaultHTTPClientConfig}}, registry, log.NewNopLogger()) if result { @@ -1136,7 +1136,7 @@ func TestRedirectToTLSHostWorks(t *testing.T) { testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() result := ProbeHTTP(testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true}}, registry, log.NewNopLogger()) + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: pconfig.DefaultHTTPClientConfig}}, registry, log.NewNopLogger()) if !result { t.Fatalf("Redirect test failed unexpectedly") } @@ -1209,7 +1209,7 @@ func TestCookieJar(t *testing.T) { registry := prometheus.NewRegistry() testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - result := ProbeHTTP(testCTX, ts.URL, config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true}}, registry, log.NewNopLogger()) + result := ProbeHTTP(testCTX, ts.URL, config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: pconfig.DefaultHTTPClientConfig}}, registry, log.NewNopLogger()) body := recorder.Body.String() if !result { t.Fatalf("Redirect test failed unexpectedly, got %s", body) -- 2.25.1