diff --git a/cmd/karma/main.go b/cmd/karma/main.go index d7e23c980..a1b3305d8 100644 --- a/cmd/karma/main.go +++ b/cmd/karma/main.go @@ -56,8 +56,13 @@ var ( ) func getViewURL(sub string) string { - u := path.Join(config.Config.Listen.Prefix, sub) - if strings.HasSuffix(sub, "/") && !strings.HasSuffix(u, "/") { + var fixedSub string + fixedSub = sub + if !strings.HasPrefix(sub, "/") { + fixedSub = "/" + sub + } + u := path.Join(config.Config.Listen.Prefix, fixedSub) + if strings.HasSuffix(fixedSub, "/") && !strings.HasSuffix(u, "/") { // if sub path had trailing slash then add it here, since path.Join will // skip it return u + "/" diff --git a/cmd/karma/tests/testscript/invalid_listen_prefix.txt b/cmd/karma/tests/testscript/invalid_listen_prefix.txt new file mode 100644 index 000000000..d1b1a46a3 --- /dev/null +++ b/cmd/karma/tests/testscript/invalid_listen_prefix.txt @@ -0,0 +1,4 @@ +# Raises an error if listen.prefix is invalid +karma.bin-should-fail --log.format=text --log.config=false --log.level=error --alertmanager.uri http://localhost --listen.prefix karma +! stdout . +stderr 'msg="listen.prefix must start with ''\/'', got \\"karma\\"' diff --git a/internal/alertmanager/model_test.go b/internal/alertmanager/model_test.go index 5ccff879f..13b68528a 100644 --- a/internal/alertmanager/model_test.go +++ b/internal/alertmanager/model_test.go @@ -1,7 +1,11 @@ package alertmanager import ( + "fmt" "testing" + + "github.com/prymitive/karma/internal/config" + log "github.com/sirupsen/logrus" ) type uriTest struct { @@ -101,3 +105,91 @@ func TestAlertmanagerURI(t *testing.T) { } } } + +func TestAlertmanagerSilenceByID(t *testing.T) { + am, err := NewAlertmanager("cluster", "test", "http://localhost") + if err != nil { + t.Error(err) + } + + _, err = am.SilenceByID("foo") + if err == nil { + t.Error("am.SilenceByID(foo) didn't return any error") + } +} + +func TestAlertmanagerInternalURI(t *testing.T) { + type testCaseT struct { + prefix string + proxy bool + uri string + } + tests := []testCaseT{ + { + prefix: "/", + proxy: false, + uri: "http://localhost", + }, + { + prefix: "/", + proxy: true, + uri: "/proxy/alertmanager/default", + }, + { + prefix: "/root", + proxy: true, + uri: "/root/proxy/alertmanager/default", + }, + { + prefix: "/root/", + proxy: true, + uri: "/root/proxy/alertmanager/default", + }, + { + prefix: "root", + proxy: true, + uri: "/root/proxy/alertmanager/default", + }, + { + prefix: "root/", + proxy: true, + uri: "/root/proxy/alertmanager/default", + }, + } + + for _, testCase := range tests { + t.Run(fmt.Sprintf("prefix=%q proxy=%v uri=%q", testCase.prefix, testCase.proxy, testCase.uri), func(t *testing.T) { + config.Config.Listen.Prefix = testCase.prefix + am, err := NewAlertmanager("cluster", "default", "http://localhost", WithProxy(testCase.proxy)) + if err != nil { + t.Error(err) + } + + uri := am.InternalURI() + if uri != testCase.uri { + t.Errorf("am.InternalURI() returned %q, expected %q", uri, testCase.uri) + } + }) + } +} + +func TestAlertmanagerSanitizedURI(t *testing.T) { + am, err := NewAlertmanager("cluster", "test", "http://user:pass@localhost") + if err != nil { + t.Error(err) + } + + uri := am.SanitizedURI() + if uri != "http://user:xxx@localhost" { + t.Errorf("am.SanitizedURI(http://user:pass@localhost) returned %q", uri) + } +} + +func TestAlertmanagerPullWithInvalidURI(t *testing.T) { + log.SetLevel(log.PanicLevel) + am, _ := NewAlertmanager("cluster", "test", "%gh&%ij") + err := am.Pull() + if err == nil { + t.Error("am.Pull(invalid uri) didn't return any error") + } +} diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index 508c7b62d..816dc6668 100644 --- a/internal/alertmanager/models.go +++ b/internal/alertmanager/models.go @@ -3,6 +3,7 @@ package alertmanager import ( "fmt" "net/http" + "net/url" "path" "sort" "strings" @@ -159,14 +160,10 @@ func (am *Alertmanager) pullSilences(version string) error { func (am *Alertmanager) InternalURI() string { if am.ProxyRequests { sub := fmt.Sprintf("/proxy/alertmanager/%s", am.Name) - uri := path.Join(config.Config.Listen.Prefix, sub) - if strings.HasSuffix(sub, "/") { - // if sub path had trailing slash then add it here, since path.Join will - // skip it - return uri + "/" + if strings.HasPrefix(config.Config.Listen.Prefix, "/") { + return path.Join(config.Config.Listen.Prefix, sub) } - - return uri + return path.Join("/"+config.Config.Listen.Prefix, sub) } // strip all user/pass information, fetch() doesn't support it anyway @@ -311,6 +308,12 @@ func (am *Alertmanager) Pull() error { version := am.probeVersion() + // verify that URI is correct + _, err := url.Parse(am.URI) + if err != nil { + return err + } + status, err := am.fetchStatus(version) if err != nil { am.clearData()