read bearer token on every request , + some http and scrape tests

read bearer token on every request
removed unuseful  scrape manager startup log
new tests -TestScrapeManagerReloadNoChange( scrape pool is not reloaded
when the config hasn't changed), TestMissingBearerAuthFile ,
TestBearerAuthFileRoundTripper

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:

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)
+}

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

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"