From 2aba238f3145ec80348fa4288af607f728b08dd1 Mon Sep 17 00:00:00 2001 From: Philippe Laflamme Date: Thu, 5 Apr 2018 04:08:18 -0400 Subject: [PATCH] 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 | 5 + config/config.go | 9 +- config/config_test.go | 23 +++- config/testdata/conf.good.yml | 1 + .../marathon_authtoken_authtokenfile.bad.yml | 9 ++ .../marathon_authtoken_basicauth.bad.yml | 11 ++ .../marathon_authtoken_bearertoken.bad.yml | 9 ++ discovery/marathon/marathon.go | 118 +++++++++++------- discovery/marathon/marathon_test.go | 22 ++-- docs/configuration/configuration.md | 42 +++++-- util/httputil/client.go | 14 ++- 11 files changed, 190 insertions(+), 73 deletions(-) create mode 100644 config/testdata/marathon_authtoken_authtokenfile.bad.yml create mode 100644 config/testdata/marathon_authtoken_basicauth.bad.yml create mode 100644 config/testdata/marathon_authtoken_bearertoken.bad.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bd9ac485..f43ef00f6 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/config/config.go b/config/config.go index 797dfe4a0..746aae3bf 100644 --- a/config/config.go +++ b/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.TLSConfig.CAFile = join(mcfg.TLSConfig.CAFile) - mcfg.TLSConfig.CertFile = join(mcfg.TLSConfig.CertFile) - mcfg.TLSConfig.KeyFile = join(mcfg.TLSConfig.KeyFile) + mcfg.AuthTokenFile = join(mcfg.AuthTokenFile) + mcfg.HTTPClientConfig.BearerTokenFile = join(mcfg.HTTPClientConfig.BearerTokenFile) + mcfg.HTTPClientConfig.TLSConfig.CAFile = join(mcfg.HTTPClientConfig.TLSConfig.CAFile) + 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) diff --git a/config/config_test.go b/config/config_test.go index 9826bbb00..8caaebd55 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -384,11 +384,13 @@ var expectedConf = &Config{ Servers: []string{ "https://marathon.example.com:443", }, - Timeout: model.Duration(30 * time.Second), RefreshInterval: model.Duration(30 * time.Second), - TLSConfig: config_util.TLSConfig{ - CertFile: filepath.FromSlash("testdata/valid_cert_file"), - KeyFile: filepath.FromSlash("testdata/valid_key_file"), + 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"), + }, }, }, }, @@ -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", diff --git a/config/testdata/conf.good.yml b/config/testdata/conf.good.yml index fa4938a62..df578952f 100644 --- a/config/testdata/conf.good.yml +++ b/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 diff --git a/config/testdata/marathon_authtoken_authtokenfile.bad.yml b/config/testdata/marathon_authtoken_authtokenfile.bad.yml new file mode 100644 index 000000000..b31c6f154 --- /dev/null +++ b/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 diff --git a/config/testdata/marathon_authtoken_basicauth.bad.yml b/config/testdata/marathon_authtoken_basicauth.bad.yml new file mode 100644 index 000000000..64300f407 --- /dev/null +++ b/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 diff --git a/config/testdata/marathon_authtoken_bearertoken.bad.yml b/config/testdata/marathon_authtoken_bearertoken.bad.yml new file mode 100644 index 000000000..36eeb801e --- /dev/null +++ b/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 diff --git a/discovery/marathon/marathon.go b/discovery/marathon/marathon.go index 934e124b3..452ae9389 100644 --- a/discovery/marathon/marathon.go +++ b/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,19 +75,17 @@ var ( }) // DefaultSDConfig is the default Marathon SD configuration. DefaultSDConfig = SDConfig{ - Timeout: model.Duration(30 * time.Second), RefreshInterval: model.Duration(30 * time.Second), } ) // 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"` - BearerToken config_util.Secret `yaml:"bearer_token,omitempty"` - BearerTokenFile string `yaml:"bearer_token_file,omitempty"` + Servers []string `yaml:"servers,omitempty"` + RefreshInterval model.Duration `yaml:"refresh_interval,omitempty"` + AuthToken config_util.Secret `yaml:"auth_token,omitempty"` + AuthTokenFile string `yaml:"auth_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 { - return fmt.Errorf("at most one of bearer_token & bearer_token_file must be configured") + if len(c.AuthToken) > 0 && len(c.AuthTokenFile) > 0 { + 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 conf.BearerTokenFile != "" { - bf, err := ioutil.ReadFile(conf.BearerTokenFile) - if err != nil { - return nil, err - } - token = strings.TrimSpace(string(bf)) + if len(conf.AuthToken) > 0 { + rt, err = newAuthTokenRoundTripper(conf.AuthToken, rt) + } else if len(conf.AuthTokenFile) > 0 { + rt, err = newAuthTokenFileRoundTripper(conf.AuthTokenFile, rt) } - - client := &http.Client{ - Timeout: time.Duration(conf.Timeout), - Transport: &http.Transport{ - TLSClientConfig: tls, - DialContext: conntrack.NewDialContextFunc( - conntrack.DialWithTracing(), - conntrack.DialWithName("marathon_sd"), - ), - }, + if err != nil { + return nil, err } 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 diff --git a/discovery/marathon/marathon_test.go b/discovery/marathon/marathon_test.go index cabd222d6..6b7c6f315 100644 --- a/discovery/marathon/marathon_test.go +++ b/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 } ) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 7378efa4a..c63ff020e 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -800,16 +800,42 @@ See below for the configuration options for Marathon discovery: servers: - -# Optional bearer token authentication information. -# It is mutually exclusive with `bearer_token_file`. -[ bearer_token: ] - -# Optional bearer token file authentication information. -# It is mutually exclusive with `bearer_token`. -[ bearer_token_file: ] - # Polling interval [ refresh_interval: | 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: ] + +# 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: ] + +# Sets the `Authorization` header on every request with the +# configured username and password. +# This is mutually exclusive with other authentication mechanisms. +basic_auth: + [ username: ] + [ password: ] + +# 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: ] + +# 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: + [ ] + +# Optional proxy URL. +[ proxy_url: ] ``` By default every app listed in Marathon will be scraped by Prometheus. If not all diff --git a/util/httputil/client.go b/util/httputil/client.go index 658532ec9..0caa2a802 100644 --- a/util/httputil/client.go +++ b/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 newClient(rt), nil + // Return a new configured RoundTripper. + return rt, nil } type bearerAuthRoundTripper struct {