diff --git a/retrieval/manager.go b/retrieval/manager.go index 724e7788a9..b6e3d780c5 100644 --- a/retrieval/manager.go +++ b/retrieval/manager.go @@ -56,8 +56,6 @@ type ScrapeManager struct { // Run starts background processing to handle target updates and reload the scraping loops. func (m *ScrapeManager) Run(tsets <-chan map[string][]*targetgroup.Group) error { - level.Info(m.logger).Log("msg", "Starting scrape manager...") - for { select { case ts := <-tsets: diff --git a/retrieval/manager_test.go b/retrieval/manager_test.go index 40199179a9..3e54cf31a7 100644 --- a/retrieval/manager_test.go +++ b/retrieval/manager_test.go @@ -17,9 +17,11 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/discovery/targetgroup" "github.com/prometheus/prometheus/pkg/labels" ) @@ -230,3 +232,44 @@ func TestPopulateLabels(t *testing.T) { } } } + +// TestScrapeManagerReloadNoChange tests that no scrape reload happens when there is no config change. +func TestManagerReloadNoChange(t *testing.T) { + tsetName := "test" + + reloadCfg := &config.Config{ + ScrapeConfigs: []*config.ScrapeConfig{ + &config.ScrapeConfig{ + ScrapeInterval: model.Duration(3 * time.Second), + ScrapeTimeout: model.Duration(2 * time.Second), + }, + }, + } + + scrapeManager := NewScrapeManager(nil, nil) + scrapeManager.scrapeConfigs[tsetName] = reloadCfg.ScrapeConfigs[0] + // As reload never happens, new loop should never be called. + newLoop := func(_ *Target, s scraper) loop { + t.Fatal("reload happened") + return nil + } + sp := &scrapePool{ + appendable: &nopAppendable{}, + targets: map[uint64]*Target{}, + loops: map[uint64]loop{ + 1: &scrapeLoop{}, + }, + newLoop: newLoop, + logger: nil, + config: reloadCfg.ScrapeConfigs[0], + } + scrapeManager.scrapePools = map[string]*scrapePool{ + tsetName: sp, + } + + targets := map[string][]*targetgroup.Group{ + tsetName: []*targetgroup.Group{}, + } + + scrapeManager.reload(targets) +} diff --git a/util/httputil/client.go b/util/httputil/client.go index f9eed8f4d7..658532ec90 100644 --- a/util/httputil/client.go +++ b/util/httputil/client.go @@ -58,17 +58,10 @@ func NewClientFromConfig(cfg config_util.HTTPClientConfig, name string) (*http.C // If a bearer token is provided, create a round tripper that will set the // Authorization header correctly on each request. - bearerToken := string(cfg.BearerToken) - if len(bearerToken) == 0 && len(cfg.BearerTokenFile) > 0 { - b, err := ioutil.ReadFile(cfg.BearerTokenFile) - if err != nil { - return nil, fmt.Errorf("unable to read bearer token file %s: %s", cfg.BearerTokenFile, err) - } - bearerToken = strings.TrimSpace(string(b)) - } - - if len(bearerToken) > 0 { - rt = NewBearerAuthRoundTripper(bearerToken, rt) + if len(cfg.BearerToken) > 0 { + rt = NewBearerAuthRoundTripper(string(cfg.BearerToken), rt) + } else if len(cfg.BearerTokenFile) > 0 { + rt = NewBearerAuthFileRoundTripper(cfg.BearerTokenFile, rt) } if cfg.BasicAuth != nil { @@ -99,6 +92,32 @@ func (rt *bearerAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response, return rt.rt.RoundTrip(req) } +type bearerAuthFileRoundTripper struct { + bearerFile string + rt http.RoundTripper +} + +// NewBearerAuthFileRoundTripper adds the bearer token read from the provided file to a request unless +// the authorization header has already been set. This file is read for every request. +func NewBearerAuthFileRoundTripper(bearerFile string, rt http.RoundTripper) http.RoundTripper { + return &bearerAuthFileRoundTripper{bearerFile, rt} +} + +func (rt *bearerAuthFileRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if len(req.Header.Get("Authorization")) == 0 { + b, err := ioutil.ReadFile(rt.bearerFile) + if err != nil { + return nil, fmt.Errorf("unable to read bearer token file %s: %s", rt.bearerFile, err) + } + bearerToken := strings.TrimSpace(string(b)) + + req = cloneRequest(req) + req.Header.Set("Authorization", "Bearer "+bearerToken) + } + + return rt.rt.RoundTrip(req) +} + type basicAuthRoundTripper struct { username string password string diff --git a/util/httputil/client_test.go b/util/httputil/client_test.go index 49e54adc98..0465561a58 100644 --- a/util/httputil/client_test.go +++ b/util/httputil/client_test.go @@ -218,17 +218,6 @@ func TestNewClientFromInvalidConfig(t *testing.T) { InsecureSkipVerify: true}, }, errorMsg: fmt.Sprintf("unable to use specified CA cert %s:", MissingCA), - }, { - clientConfig: config_util.HTTPClientConfig{ - BearerTokenFile: MissingBearerTokenFile, - TLSConfig: config_util.TLSConfig{ - CAFile: TLSCAChainPath, - CertFile: BarneyCertificatePath, - KeyFile: BarneyKeyNoPassPath, - ServerName: "", - InsecureSkipVerify: false}, - }, - errorMsg: fmt.Sprintf("unable to read bearer token file %s:", MissingBearerTokenFile), }, } @@ -246,6 +235,47 @@ func TestNewClientFromInvalidConfig(t *testing.T) { } } +func TestMissingBearerAuthFile(t *testing.T) { + cfg := config_util.HTTPClientConfig{ + BearerTokenFile: MissingBearerTokenFile, + TLSConfig: config_util.TLSConfig{ + CAFile: TLSCAChainPath, + CertFile: BarneyCertificatePath, + KeyFile: BarneyKeyNoPassPath, + ServerName: "", + InsecureSkipVerify: false}, + } + handler := func(w http.ResponseWriter, r *http.Request) { + bearer := r.Header.Get("Authorization") + if bearer != ExpectedBearer { + fmt.Fprintf(w, "The expected Bearer Authorization (%s) differs from the obtained Bearer Authorization (%s)", + ExpectedBearer, bearer) + } else { + fmt.Fprint(w, ExpectedMessage) + } + } + + testServer, err := newTestServer(handler) + if err != nil { + t.Fatal(err.Error()) + } + defer testServer.Close() + + client, err := NewClientFromConfig(cfg, "test") + if err != nil { + t.Fatal(err) + } + + _, err = client.Get(testServer.URL) + if err == nil { + t.Fatal("No error is returned here") + } + + if !strings.Contains(err.Error(), "unable to read bearer token file missing/bearer.token: open missing/bearer.token: no such file or directory") { + t.Fatal("wrong error message being returned") + } +} + func TestBearerAuthRoundTripper(t *testing.T) { const ( newBearerToken = "goodbyeandthankyouforthefish" @@ -272,6 +302,32 @@ func TestBearerAuthRoundTripper(t *testing.T) { bearerAuthRoundTripperShouldNotModifyExistingAuthorization.RoundTrip(request) } +func TestBearerAuthFileRoundTripper(t *testing.T) { + const ( + newBearerToken = "goodbyeandthankyouforthefish" + ) + + fakeRoundTripper := testutil.NewRoundTripCheckRequest(func(req *http.Request) { + bearer := req.Header.Get("Authorization") + if bearer != ExpectedBearer { + t.Errorf("The expected Bearer Authorization (%s) differs from the obtained Bearer Authorization (%s)", + ExpectedBearer, bearer) + } + }, nil, nil) + + //Normal flow + bearerAuthRoundTripper := NewBearerAuthFileRoundTripper(BearerTokenFile, fakeRoundTripper) + request, _ := http.NewRequest("GET", "/hitchhiker", nil) + request.Header.Set("User-Agent", "Douglas Adams mind") + bearerAuthRoundTripper.RoundTrip(request) + + //Should honor already Authorization header set + bearerAuthRoundTripperShouldNotModifyExistingAuthorization := NewBearerAuthFileRoundTripper(MissingBearerTokenFile, fakeRoundTripper) + request, _ = http.NewRequest("GET", "/hitchhiker", nil) + request.Header.Set("Authorization", ExpectedBearer) + bearerAuthRoundTripperShouldNotModifyExistingAuthorization.RoundTrip(request) +} + func TestBasicAuthRoundTripper(t *testing.T) { const ( newUsername = "fordprefect"