Use common HTTPClientConfig for marathon_sd configuration (#4009)

This adds support for basic authentication which closes #3090

The support for specifying the client timeout was removed as discussed in https://github.com/prometheus/common/pull/123. Marathon was the only sd mechanism doing this and configuring the timeout is done through `Context`.

DC/OS uses a custom `Authorization` header for authenticating. This adds 2 new configuration properties to reflect this.

Existing configuration files that use the bearer token will no longer work. More work is required to make this backwards compatible.

CHANGELOG.md

@@ -1,3 +1,8 @@
+## next release
+
+* [CHANGE] `marathon_sd`: use `auth_token` and `auth_token_file` for token-based authentication instead of `bearer_token` and `bearer_token_file` respectively.
+* [ENHANCEMENT] `marathon_sd`: adds support for basic and bearer authentication, plus all other common HTTP client options (TLS config, proxy URL, etc.)
+
 ## 2.2.0 / 2018-03-08
 
 * [CHANGE] Rename file SD mtime metric.

config/config.go

@@ -172,10 +172,11 @@ func resolveFilepaths(baseDir string, cfg *Config) {
 			kcfg.TLSConfig.KeyFile = join(kcfg.TLSConfig.KeyFile)
 		}
 		for _, mcfg := range cfg.MarathonSDConfigs {
-			mcfg.BearerTokenFile = join(mcfg.BearerTokenFile)
+			mcfg.AuthTokenFile = join(mcfg.AuthTokenFile)
-			mcfg.TLSConfig.CAFile = join(mcfg.TLSConfig.CAFile)
+			mcfg.HTTPClientConfig.BearerTokenFile = join(mcfg.HTTPClientConfig.BearerTokenFile)
-			mcfg.TLSConfig.CertFile = join(mcfg.TLSConfig.CertFile)
+			mcfg.HTTPClientConfig.TLSConfig.CAFile = join(mcfg.HTTPClientConfig.TLSConfig.CAFile)
-			mcfg.TLSConfig.KeyFile = join(mcfg.TLSConfig.KeyFile)
+			mcfg.HTTPClientConfig.TLSConfig.CertFile = join(mcfg.HTTPClientConfig.TLSConfig.CertFile)
+			mcfg.HTTPClientConfig.TLSConfig.KeyFile = join(mcfg.HTTPClientConfig.TLSConfig.KeyFile)
 		}
 		for _, consulcfg := range cfg.ConsulSDConfigs {
 			consulcfg.TLSConfig.CAFile = join(consulcfg.TLSConfig.CAFile)

config/config_test.go

@@ -384,8 +384,9 @@ var expectedConf = &Config{
 						Servers: []string{
 							"https://marathon.example.com:443",
 						},
-						Timeout:         model.Duration(30 * time.Second),
 						RefreshInterval: model.Duration(30 * time.Second),
+						AuthToken:       config_util.Secret("mysecret"),
+						HTTPClientConfig: config_util.HTTPClientConfig{
 							TLSConfig: config_util.TLSConfig{
 								CertFile: filepath.FromSlash("testdata/valid_cert_file"),
 								KeyFile:  filepath.FromSlash("testdata/valid_key_file"),
@@ -394,6 +395,7 @@ var expectedConf = &Config{
 					},
 				},
 			},
+		},
 		{
 			JobName: "service-ec2",
 
@@ -580,7 +582,7 @@ func TestElideSecrets(t *testing.T) {
 	yamlConfig := string(config)
 
 	matches := secretRe.FindAllStringIndex(yamlConfig, -1)
-	testutil.Assert(t, len(matches) == 6, "wrong number of secret matches found")
+	testutil.Assert(t, len(matches) == 7, "wrong number of secret matches found")
 	testutil.Assert(t, !strings.Contains(yamlConfig, "mysecret"),
 		"yaml marshal reveals authentication credentials.")
 }
@@ -678,7 +680,16 @@ var expectedErrors = []struct {
 		errMsg:   "at most one of basic_auth, bearer_token & bearer_token_file must be configured",
 	}, {
 		filename: "marathon_no_servers.bad.yml",
-		errMsg:   "Marathon SD config must contain at least one Marathon server",
+		errMsg:   "marathon_sd: must contain at least one Marathon server",
+	}, {
+		filename: "marathon_authtoken_authtokenfile.bad.yml",
+		errMsg:   "marathon_sd: at most one of auth_token & auth_token_file must be configured",
+	}, {
+		filename: "marathon_authtoken_basicauth.bad.yml",
+		errMsg:   "marathon_sd: at most one of basic_auth, auth_token & auth_token_file must be configured",
+	}, {
+		filename: "marathon_authtoken_bearertoken.bad.yml",
+		errMsg:   "marathon_sd: at most one of bearer_token, bearer_token_file, auth_token & auth_token_file must be configured",
 	}, {
 		filename: "url_in_targetgroup.bad.yml",
 		errMsg:   "\"http://bad\" is not a valid hostname",

config/testdata/conf.good.yml

@@ -172,6 +172,7 @@ scrape_configs:
   - servers:
     - 'https://marathon.example.com:443'
 
+    auth_token: "mysecret"
     tls_config:
       cert_file: valid_cert_file
       key_file: valid_key_file

config/testdata/marathon_authtoken_authtokenfile.bad.yml

@@ -0,0 +1,9 @@
+scrape_configs:
+  - job_name: prometheus
+
+    marathon_sd_configs:
+    - servers:
+      - 'https://localhost:1234'
+
+      auth_token: 1234
+      auth_token_file: somefile

config/testdata/marathon_authtoken_basicauth.bad.yml

@@ -0,0 +1,11 @@
+scrape_configs:
+  - job_name: prometheus
+
+    marathon_sd_configs:
+    - servers:
+      - 'https://localhost:1234'
+
+      auth_token: 1234
+      basic_auth:
+        username: user
+        password: password

config/testdata/marathon_authtoken_bearertoken.bad.yml

@@ -0,0 +1,9 @@
+scrape_configs:
+  - job_name: prometheus
+
+    marathon_sd_configs:
+    - servers:
+      - 'https://localhost:1234'
+
+      auth_token: 1234
+      bearer_token: 4567

discovery/marathon/marathon.go

@@ -27,7 +27,6 @@ import (
 
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
-	conntrack "github.com/mwitkow/go-conntrack"
 	"github.com/prometheus/client_golang/prometheus"
 	config_util "github.com/prometheus/common/config"
 	"github.com/prometheus/common/model"
@@ -76,7 +75,6 @@ var (
 		})
 	// DefaultSDConfig is the default Marathon SD configuration.
 	DefaultSDConfig = SDConfig{
-		Timeout:         model.Duration(30 * time.Second),
 		RefreshInterval: model.Duration(30 * time.Second),
 	}
 )
@@ -84,11 +82,10 @@ var (
 // SDConfig is the configuration for services running on Marathon.
 type SDConfig struct {
 	Servers          []string                     `yaml:"servers,omitempty"`
-	Timeout         model.Duration        `yaml:"timeout,omitempty"`
 	RefreshInterval  model.Duration               `yaml:"refresh_interval,omitempty"`
-	TLSConfig       config_util.TLSConfig `yaml:"tls_config,omitempty"`
+	AuthToken        config_util.Secret           `yaml:"auth_token,omitempty"`
-	BearerToken     config_util.Secret    `yaml:"bearer_token,omitempty"`
+	AuthTokenFile    string                       `yaml:"auth_token_file,omitempty"`
-	BearerTokenFile string                `yaml:"bearer_token_file,omitempty"`
+	HTTPClientConfig config_util.HTTPClientConfig `yaml:",inline"`
 }
 
 // UnmarshalYAML implements the yaml.Unmarshaler interface.
@@ -100,10 +97,19 @@ func (c *SDConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		return err
 	}
 	if len(c.Servers) == 0 {
-		return fmt.Errorf("Marathon SD config must contain at least one Marathon server")
+		return fmt.Errorf("marathon_sd: must contain at least one Marathon server")
 	}
-	if len(c.BearerToken) > 0 && len(c.BearerTokenFile) > 0 {
+	if len(c.AuthToken) > 0 && len(c.AuthTokenFile) > 0 {
-		return fmt.Errorf("at most one of bearer_token & bearer_token_file must be configured")
+		return fmt.Errorf("marathon_sd: at most one of auth_token & auth_token_file must be configured")
+	}
+	if c.HTTPClientConfig.BasicAuth != nil && (len(c.AuthToken) > 0 || len(c.AuthTokenFile) > 0) {
+		return fmt.Errorf("marathon_sd: at most one of basic_auth, auth_token & auth_token_file must be configured")
+	}
+	if (len(c.HTTPClientConfig.BearerToken) > 0 || len(c.HTTPClientConfig.BearerTokenFile) > 0) && (len(c.AuthToken) > 0 || len(c.AuthTokenFile) > 0) {
+		return fmt.Errorf("marathon_sd: at most one of bearer_token, bearer_token_file, auth_token & auth_token_file must be configured")
+	}
+	if err := c.HTTPClientConfig.Validate(); err != nil {
+		return err
 	}
 
 	return nil
@@ -123,7 +129,6 @@ type Discovery struct {
 	refreshInterval time.Duration
 	lastRefresh     map[string]*targetgroup.Group
 	appsClient      AppListClient
-	token           string
 	logger          log.Logger
 }
 
@@ -133,41 +138,77 @@ func NewDiscovery(conf SDConfig, logger log.Logger) (*Discovery, error) {
 		logger = log.NewNopLogger()
 	}
 
-	tls, err := httputil.NewTLSConfig(conf.TLSConfig)
+	rt, err := httputil.NewRoundTripperFromConfig(conf.HTTPClientConfig, "marathon_sd")
 	if err != nil {
 		return nil, err
 	}
 
-	token := string(conf.BearerToken)
+	if len(conf.AuthToken) > 0 {
-	if conf.BearerTokenFile != "" {
+		rt, err = newAuthTokenRoundTripper(conf.AuthToken, rt)
-		bf, err := ioutil.ReadFile(conf.BearerTokenFile)
+	} else if len(conf.AuthTokenFile) > 0 {
+		rt, err = newAuthTokenFileRoundTripper(conf.AuthTokenFile, rt)
+	}
 	if err != nil {
 		return nil, err
 	}
-		token = strings.TrimSpace(string(bf))
-	}
-
-	client := &http.Client{
-		Timeout: time.Duration(conf.Timeout),
-		Transport: &http.Transport{
-			TLSClientConfig: tls,
-			DialContext: conntrack.NewDialContextFunc(
-				conntrack.DialWithTracing(),
-				conntrack.DialWithName("marathon_sd"),
-			),
-		},
-	}
 
 	return &Discovery{
-		client:          client,
+		client:          &http.Client{Transport: rt},
 		servers:         conf.Servers,
 		refreshInterval: time.Duration(conf.RefreshInterval),
 		appsClient:      fetchApps,
-		token:           token,
 		logger:          logger,
 	}, nil
 }
 
+type authTokenRoundTripper struct {
+	authToken config_util.Secret
+	rt        http.RoundTripper
+}
+
+// newAuthTokenRoundTripper adds the provided auth token to a request.
+func newAuthTokenRoundTripper(token config_util.Secret, rt http.RoundTripper) (http.RoundTripper, error) {
+	return &authTokenRoundTripper{token, rt}, nil
+}
+
+func (rt *authTokenRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
+	// According to https://docs.mesosphere.com/1.11/security/oss/managing-authentication/
+	// DC/OS wants with "token=" a different Authorization header than implemented in httputil/client.go
+	// so we set this explicitly here.
+	request.Header.Set("Authorization", "token="+string(rt.authToken))
+
+	return rt.rt.RoundTrip(request)
+}
+
+type authTokenFileRoundTripper struct {
+	authTokenFile string
+	rt            http.RoundTripper
+}
+
+// newAuthTokenFileRoundTripper adds the auth token read from the file to a request.
+func newAuthTokenFileRoundTripper(tokenFile string, rt http.RoundTripper) (http.RoundTripper, error) {
+	// fail-fast if we can't read the file.
+	_, err := ioutil.ReadFile(tokenFile)
+	if err != nil {
+		return nil, fmt.Errorf("unable to read auth token file %s: %s", tokenFile, err)
+	}
+	return &authTokenFileRoundTripper{tokenFile, rt}, nil
+}
+
+func (rt *authTokenFileRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
+	b, err := ioutil.ReadFile(rt.authTokenFile)
+	if err != nil {
+		return nil, fmt.Errorf("unable to read auth token file %s: %s", rt.authTokenFile, err)
+	}
+	authToken := strings.TrimSpace(string(b))
+
+	// According to https://docs.mesosphere.com/1.11/security/oss/managing-authentication/
+	// DC/OS wants with "token=" a different Authorization header than implemented in httputil/client.go
+	// so we set this explicitly here.
+	request.Header.Set("Authorization", "token="+authToken)
+	return rt.rt.RoundTrip(request)
+}
+
 // Run implements the Discoverer interface.
 func (d *Discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
 	for {
@@ -227,7 +268,7 @@ func (d *Discovery) updateServices(ctx context.Context, ch chan<- []*targetgroup
 
 func (d *Discovery) fetchTargetGroups() (map[string]*targetgroup.Group, error) {
 	url := RandomAppsURL(d.servers)
-	apps, err := d.appsClient(d.client, url, d.token)
+	apps, err := d.appsClient(d.client, url)
 	if err != nil {
 		return nil, err
 	}
@@ -281,22 +322,15 @@ type AppList struct {
 }
 
 // AppListClient defines a function that can be used to get an application list from marathon.
-type AppListClient func(client *http.Client, url, token string) (*AppList, error)
+type AppListClient func(client *http.Client, url string) (*AppList, error)
 
 // fetchApps requests a list of applications from a marathon server.
-func fetchApps(client *http.Client, url, token string) (*AppList, error) {
+func fetchApps(client *http.Client, url string) (*AppList, error) {
 	request, err := http.NewRequest("GET", url, nil)
 	if err != nil {
 		return nil, err
 	}
 
-	// According to  https://dcos.io/docs/1.8/administration/id-and-access-mgt/managing-authentication
-	// DC/OS wants with "token=" a different Authorization header than implemented in httputil/client.go
-	// so we set this implicitly here
-	if token != "" {
-		request.Header.Set("Authorization", "token="+token)
-	}
-
 	resp, err := client.Do(request)
 	if err != nil {
 		return nil, err

discovery/marathon/marathon_test.go

@@ -43,7 +43,7 @@ func TestMarathonSDHandleError(t *testing.T) {
 	var (
 		errTesting = errors.New("testing failure")
 		ch         = make(chan []*targetgroup.Group, 1)
-		client     = func(client *http.Client, url, token string) (*AppList, error) { return nil, errTesting }
+		client     = func(client *http.Client, url string) (*AppList, error) { return nil, errTesting }
 	)
 	if err := testUpdateServices(client, ch); err != errTesting {
 		t.Fatalf("Expected error: %s", err)
@@ -58,7 +58,7 @@ func TestMarathonSDHandleError(t *testing.T) {
 func TestMarathonSDEmptyList(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) { return &AppList{}, nil }
+		client = func(client *http.Client, url string) (*AppList, error) { return &AppList{}, nil }
 	)
 	if err := testUpdateServices(client, ch); err != nil {
 		t.Fatalf("Got error: %s", err)
@@ -105,7 +105,7 @@ func marathonTestAppList(labels map[string]string, runningTasks int) *AppList {
 func TestMarathonSDSendGroup(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) {
+		client = func(client *http.Client, url string) (*AppList, error) {
 			return marathonTestAppList(marathonValidLabel, 1), nil
 		}
 	)
@@ -144,7 +144,7 @@ func TestMarathonSDRemoveApp(t *testing.T) {
 		t.Fatalf("%s", err)
 	}
 
-	md.appsClient = func(client *http.Client, url, token string) (*AppList, error) {
+	md.appsClient = func(client *http.Client, url string) (*AppList, error) {
 		return marathonTestAppList(marathonValidLabel, 1), nil
 	}
 	if err := md.updateServices(context.Background(), ch); err != nil {
@@ -152,7 +152,7 @@ func TestMarathonSDRemoveApp(t *testing.T) {
 	}
 	up1 := (<-ch)[0]
 
-	md.appsClient = func(client *http.Client, url, token string) (*AppList, error) {
+	md.appsClient = func(client *http.Client, url string) (*AppList, error) {
 		return marathonTestAppList(marathonValidLabel, 0), nil
 	}
 	if err := md.updateServices(context.Background(), ch); err != nil {
@@ -179,7 +179,7 @@ func TestMarathonSDRunAndStop(t *testing.T) {
 	if err != nil {
 		t.Fatalf("%s", err)
 	}
-	md.appsClient = func(client *http.Client, url, token string) (*AppList, error) {
+	md.appsClient = func(client *http.Client, url string) (*AppList, error) {
 		return marathonTestAppList(marathonValidLabel, 1), nil
 	}
 	ctx, cancel := context.WithCancel(context.Background())
@@ -237,7 +237,7 @@ func marathonTestAppListWithMutiplePorts(labels map[string]string, runningTasks
 func TestMarathonSDSendGroupWithMutiplePort(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) {
+		client = func(client *http.Client, url string) (*AppList, error) {
 			return marathonTestAppListWithMutiplePorts(marathonValidLabel, 1), nil
 		}
 	)
@@ -304,7 +304,7 @@ func marathonTestZeroTaskPortAppList(labels map[string]string, runningTasks int)
 func TestMarathonZeroTaskPorts(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) {
+		client = func(client *http.Client, url string) (*AppList, error) {
 			return marathonTestZeroTaskPortAppList(marathonValidLabel, 1), nil
 		}
 	)
@@ -357,7 +357,7 @@ func marathonTestAppListWithoutPortMappings(labels map[string]string, runningTas
 func TestMarathonSDSendGroupWithoutPortMappings(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) {
+		client = func(client *http.Client, url string) (*AppList, error) {
 			return marathonTestAppListWithoutPortMappings(marathonValidLabel, 1), nil
 		}
 	)
@@ -430,7 +430,7 @@ func marathonTestAppListWithoutPortDefinitions(labels map[string]string, running
 func TestMarathonSDSendGroupWithoutPortDefinitions(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) {
+		client = func(client *http.Client, url string) (*AppList, error) {
 			return marathonTestAppListWithoutPortDefinitions(marathonValidLabel, 1), nil
 		}
 	)
@@ -505,7 +505,7 @@ func marathonTestAppListWithContainerPortMappings(labels map[string]string, runn
 func TestMarathonSDSendGroupWithContainerPortMappings(t *testing.T) {
 	var (
 		ch     = make(chan []*targetgroup.Group, 1)
-		client = func(client *http.Client, url, token string) (*AppList, error) {
+		client = func(client *http.Client, url string) (*AppList, error) {
 			return marathonTestAppListWithContainerPortMappings(marathonValidLabel, 1), nil
 		}
 	)

docs/configuration/configuration.md

@@ -800,16 +800,42 @@ See below for the configuration options for Marathon discovery:
 servers:
   - <string>
 
-# Optional bearer token authentication information.
-# It is mutually exclusive with `bearer_token_file`.
-[ bearer_token: <secret> ]
-
-# Optional bearer token file authentication information.
-# It is mutually exclusive with `bearer_token`.
-[ bearer_token_file: <filename> ]
-
 # Polling interval
 [ refresh_interval: <duration> | default = 30s ]
+
+# Optional authentication information for token-based authentication
+# https://docs.mesosphere.com/1.11/security/ent/iam-api/#passing-an-authentication-token
+# It is mutually exclusive with `auth_token_file` and other authentication mechanisms.
+[ auth_token: <secret> ]
+
+# Optional authentication information for token-based authentication
+# https://docs.mesosphere.com/1.11/security/ent/iam-api/#passing-an-authentication-token
+# It is mutually exclusive with `auth_token` and other authentication mechanisms.
+[ auth_token_file: <filename> ]
+
+# Sets the `Authorization` header on every request with the
+# configured username and password.
+# This is mutually exclusive with other authentication mechanisms.
+basic_auth:
+  [ username: <string> ]
+  [ password: <string> ]
+
+# Sets the `Authorization` header on every request with
+# the configured bearer token. It is mutually exclusive with `bearer_token_file` and other authentication mechanisms.
+# NOTE: The current version of DC/OS marathon (v1.11.0) does not support standard Bearer token authentication. Use `auth_token` instead.
+[ bearer_token: <string> ]
+
+# Sets the `Authorization` header on every request with the bearer token
+# read from the configured file. It is mutually exclusive with `bearer_token` and other authentication mechanisms.
+# NOTE: The current version of DC/OS marathon (v1.11.0) does not support standard Bearer token authentication. Use `auth_token_file` instead.
+[ bearer_token_file: /path/to/bearer/token/file ]
+
+# TLS configuration for connecting to marathon servers
+tls_config:
+  [ <tls_config> ]
+
+# Optional proxy URL.
+[ proxy_url: <string> ]
 ```
 
 By default every app listed in Marathon will be scraped by Prometheus. If not all

util/httputil/client.go

@@ -34,6 +34,16 @@ func newClient(rt http.RoundTripper) *http.Client {
 // NewClientFromConfig returns a new HTTP client configured for the
 // given config.HTTPClientConfig. The name is used as go-conntrack metric label.
 func NewClientFromConfig(cfg config_util.HTTPClientConfig, name string) (*http.Client, error) {
+	rt, err := NewRoundTripperFromConfig(cfg, name)
+	if err != nil {
+		return nil, err
+	}
+	return newClient(rt), nil
+}
+
+// NewRoundTripperFromConfig returns a new HTTP RoundTripper configured for the
+// given config.HTTPClientConfig. The name is used as go-conntrack metric label.
+func NewRoundTripperFromConfig(cfg config_util.HTTPClientConfig, name string) (http.RoundTripper, error) {
 	tlsConfig, err := NewTLSConfig(cfg.TLSConfig)
 	if err != nil {
 		return nil, err
@@ -68,8 +78,8 @@ func NewClientFromConfig(cfg config_util.HTTPClientConfig, name string) (*http.C
 		rt = NewBasicAuthRoundTripper(cfg.BasicAuth.Username, string(cfg.BasicAuth.Password), rt)
 	}
 
-	// Return a new client with the configured round tripper.
+	// Return a new configured RoundTripper.
-	return newClient(rt), nil
+	return rt, nil
 }
 
 type bearerAuthRoundTripper struct {