From d7febe97fa8abe61a7f1a1b9ca02f96722de6907 Mon Sep 17 00:00:00 2001 From: Bartek Plotka Date: Mon, 16 Jan 2017 16:39:20 +0000 Subject: [PATCH 1/2] Fixed regression in -alertmanager.url flag. Basic auth was ignored. - Included basic auth parsing while parsing to AlertmanagerConfig - Added test case Signed-off-by: Bartek Plotka --- cmd/prometheus/config.go | 40 ++++++++++++++++++++++++++++++++ cmd/prometheus/config_test.go | 43 +++++++++++++++++++++++++++++++++++ cmd/prometheus/main.go | 20 +--------------- 3 files changed, 84 insertions(+), 19 deletions(-) diff --git a/cmd/prometheus/config.go b/cmd/prometheus/config.go index af124cc26..568bcaa4d 100644 --- a/cmd/prometheus/config.go +++ b/cmd/prometheus/config.go @@ -34,6 +34,8 @@ import ( "github.com/prometheus/prometheus/storage/local/index" "github.com/prometheus/prometheus/storage/remote" "github.com/prometheus/prometheus/web" + "github.com/prometheus/prometheus/config" + "github.com/prometheus/common/model" ) // cfg contains immutable configuration parameters for a running Prometheus @@ -365,6 +367,44 @@ func validateAlertmanagerURL(u string) error { return nil } +func parseAlertmanagerURLToConfig(us string) (*config.AlertmanagerConfig, error) { + u, err := url.Parse(us) + if err != nil { + return nil, err + } + acfg := &config.AlertmanagerConfig{ + Scheme: u.Scheme, + PathPrefix: u.Path, + Timeout: cfg.notifierTimeout, + ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{ + StaticConfigs: []*config.TargetGroup{ + { + Targets: []model.LabelSet{ + { + model.AddressLabel: model.LabelValue(u.Host), + }, + }, + }, + }, + }, + } + + if u.User != nil { + acfg.HTTPClientConfig = config.HTTPClientConfig{ + BasicAuth: &config.BasicAuth{ + Username: u.User.Username(), + }, + } + + if password, isSet := u.User.Password(); isSet { + acfg.HTTPClientConfig.BasicAuth.Password = password + } + } + + return acfg, nil +} + + var helpTmpl = ` usage: prometheus [] {{ range $cat, $flags := . }}{{ if ne $cat "." }} == {{ $cat | upper }} =={{ end }} diff --git a/cmd/prometheus/config_test.go b/cmd/prometheus/config_test.go index 1b7964721..406243456 100644 --- a/cmd/prometheus/config_test.go +++ b/cmd/prometheus/config_test.go @@ -80,3 +80,46 @@ func TestParse(t *testing.T) { } } } + +func TestParseAlertmanagerURLToConfig(t *testing.T) { + tests := []struct { + url string + username string + password string + }{ + { + url: "http://alertmanager.company.com", + username: "", + password: "", + }, + { + url: "https://user:password@alertmanager.company.com", + username: "user", + password: "password", + }, + } + + for i, test := range tests { + acfg, err := parseAlertmanagerURLToConfig(test.url) + if err != nil { + t.Errorf("%d. expected alertmanager URL to be valid, got %s", i, err) + } + + if acfg.HTTPClientConfig.BasicAuth != nil { + if test.username != acfg.HTTPClientConfig.BasicAuth.Username { + t.Errorf("%d. expected alertmanagerConfig username to be %q, got %q", + i, test.username, acfg.HTTPClientConfig.BasicAuth.Username) + } + + if test.password != acfg.HTTPClientConfig.BasicAuth.Password { + t.Errorf("%d. expected alertmanagerConfig password to be %q, got %q", i, + test.password, acfg.HTTPClientConfig.BasicAuth.Username) + } + continue + } + + if test.username != "" || test.password != "" { + t.Errorf("%d. expected alertmanagerConfig to have basicAuth filled, but was not", i) + } + } +} \ No newline at end of file diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index c6ce558db..de1c348a0 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -18,7 +18,6 @@ import ( "flag" "fmt" _ "net/http/pprof" // Comment this line to disable pprof endpoint. - "net/url" "os" "os/signal" "syscall" @@ -26,7 +25,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/log" - "github.com/prometheus/common/model" "github.com/prometheus/common/version" "golang.org/x/net/context" @@ -264,26 +262,10 @@ func reloadConfig(filename string, rls ...Reloadable) (err error) { // Add AlertmanagerConfigs for legacy Alertmanager URL flags. for us := range cfg.alertmanagerURLs { - u, err := url.Parse(us) + acfg, err := parseAlertmanagerURLToConfig(us) if err != nil { return err } - acfg := &config.AlertmanagerConfig{ - Scheme: u.Scheme, - PathPrefix: u.Path, - Timeout: cfg.notifierTimeout, - ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{ - StaticConfigs: []*config.TargetGroup{ - { - Targets: []model.LabelSet{ - { - model.AddressLabel: model.LabelValue(u.Host), - }, - }, - }, - }, - }, - } conf.AlertingConfig.AlertmanagerConfigs = append(conf.AlertingConfig.AlertmanagerConfigs, acfg) } From 579e33f19a704c28e19e1add8b6da7cae51ef21f Mon Sep 17 00:00:00 2001 From: Bartek Plotka Date: Mon, 16 Jan 2017 16:45:58 +0000 Subject: [PATCH 2/2] Fixed style issues. --- cmd/prometheus/config.go | 5 ++--- cmd/prometheus/config_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cmd/prometheus/config.go b/cmd/prometheus/config.go index 568bcaa4d..60b303adf 100644 --- a/cmd/prometheus/config.go +++ b/cmd/prometheus/config.go @@ -27,6 +27,8 @@ import ( "github.com/asaskevich/govalidator" "github.com/prometheus/common/log" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/notifier" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/storage/local" @@ -34,8 +36,6 @@ import ( "github.com/prometheus/prometheus/storage/local/index" "github.com/prometheus/prometheus/storage/remote" "github.com/prometheus/prometheus/web" - "github.com/prometheus/prometheus/config" - "github.com/prometheus/common/model" ) // cfg contains immutable configuration parameters for a running Prometheus @@ -404,7 +404,6 @@ func parseAlertmanagerURLToConfig(us string) (*config.AlertmanagerConfig, error) return acfg, nil } - var helpTmpl = ` usage: prometheus [] {{ range $cat, $flags := . }}{{ if ne $cat "." }} == {{ $cat | upper }} =={{ end }} diff --git a/cmd/prometheus/config_test.go b/cmd/prometheus/config_test.go index 406243456..a4e396407 100644 --- a/cmd/prometheus/config_test.go +++ b/cmd/prometheus/config_test.go @@ -83,17 +83,17 @@ func TestParse(t *testing.T) { func TestParseAlertmanagerURLToConfig(t *testing.T) { tests := []struct { - url string + url string username string password string }{ { - url: "http://alertmanager.company.com", + url: "http://alertmanager.company.com", username: "", password: "", }, { - url: "https://user:password@alertmanager.company.com", + url: "https://user:password@alertmanager.company.com", username: "user", password: "password", }, @@ -122,4 +122,4 @@ func TestParseAlertmanagerURLToConfig(t *testing.T) { t.Errorf("%d. expected alertmanagerConfig to have basicAuth filled, but was not", i) } } -} \ No newline at end of file +}