Skip to content

Commit

Permalink
fix rule: fixed issue with alertmanager URL containing +
Browse files Browse the repository at this point in the history
Signed-off-by: Martin Chodur <m.chodur@seznam.cz>
  • Loading branch information
FUSAKLA committed Sep 28, 2019
1 parent 387bbda commit a21bf87
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -27,6 +27,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel

- [#1525](https://github.com/thanos-io/thanos/pull/1525) Thanos now deletes block's file in correct order allowing to detect partial blocks without problems.
- [#1505](https://github.com/thanos-io/thanos/pull/1505) Thanos store now removes invalid local cache blocks.
- [#1564](https://github.com/thanos-io/thanos/issues/1564) Thanos rule correctly parses Alertmanager URL if there is more `+` in it.

## v0.7.0 - 2019.09.02

Expand Down
27 changes: 18 additions & 9 deletions cmd/thanos/rule.go
Expand Up @@ -20,7 +20,7 @@ import (
"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/oklog/run"
opentracing "github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
Expand Down Expand Up @@ -51,7 +51,7 @@ import (
"github.com/thanos-io/thanos/pkg/store/storepb"
"github.com/thanos-io/thanos/pkg/tracing"
"github.com/thanos-io/thanos/pkg/ui"
kingpin "gopkg.in/alecthomas/kingpin.v2"
"gopkg.in/alecthomas/kingpin.v2"
)

// registerRule registers a rule command.
Expand Down Expand Up @@ -614,22 +614,31 @@ func (s *alertmanagerSet) get() []*url.URL {

const defaultAlertmanagerPort = 9093

func parseAlertmanagerAddress(addr string) (qType dns.QType, parsedUrl *url.URL, err error) {
qType = ""
parsedUrl, err = url.Parse(addr)
if err != nil {
return qType, nil, err
}
// The Scheme might contain DNS resolver type separated by + so we split it a part
if schemeParts := strings.Split(parsedUrl.Scheme, "+"); len(schemeParts) > 1 {
parsedUrl.Scheme = schemeParts[len(schemeParts)-1]
qType = dns.QType(strings.Join(schemeParts[:len(schemeParts)-1], "+"))
}
return qType, parsedUrl, err
}

func (s *alertmanagerSet) update(ctx context.Context) error {
var result []*url.URL
for _, addr := range s.addrs {
var (
name = addr
qtype dns.QType
resolvedDomain []string
)

if nameQtype := strings.SplitN(addr, "+", 2); len(nameQtype) == 2 {
name, qtype = nameQtype[1], dns.QType(nameQtype[0])
}

u, err := url.Parse(name)
qtype, u, err := parseAlertmanagerAddress(addr)
if err != nil {
return errors.Wrapf(err, "parse URL %q", name)
return errors.Wrapf(err, "parse URL %q", addr)
}

// Get only the host and resolve it if needed.
Expand Down
35 changes: 35 additions & 0 deletions cmd/thanos/rule_test.go
Expand Up @@ -108,3 +108,38 @@ func (m mockResolver) Resolve(ctx context.Context, name string, qtype dns.QType)
}
return nil, errors.Errorf("mockResolver not found response for name: %s", name)
}

func Test_ParseAlertmanagerAddress(t *testing.T) {
var tData = []struct {
address string
expectQueryType dns.QType
expectUrl *url.URL
expectError error
}{
{
address: "http://user:pass+word@foo.bar:3289",
expectQueryType: dns.QType(""),
expectUrl: &url.URL{Host: "foo.bar:3289", Scheme: "http", User: url.UserPassword("user", "pass+word")},
expectError: nil,
},
{
address: "dnssrvnoa+http://user:pass+word@foo.bar:3289",
expectQueryType: dns.QType("dnssrvnoa"),
expectUrl: &url.URL{Host: "foo.bar:3289", Scheme: "http", User: url.UserPassword("user", "pass+word")},
expectError: nil,
},
{
address: "foo+bar+http://foo.bar:3289",
expectQueryType: dns.QType("foo+bar"),
expectUrl: &url.URL{Host: "foo.bar:3289", Scheme: "http"},
expectError: nil,
},
}

for _, d := range tData {
q, u, e := parseAlertmanagerAddress(d.address)
testutil.Equals(t, d.expectError, e)
testutil.Equals(t, d.expectUrl, u)
testutil.Equals(t, d.expectQueryType, q)
}
}

0 comments on commit a21bf87

Please sign in to comment.