Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solve #717 Validating config should try to compile regexes #729

Merged
merged 6 commits into from
Jan 6, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 57 additions & 5 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"errors"
"fmt"
"os"
"regexp"
"runtime"
"sync"
"time"
Expand Down Expand Up @@ -114,6 +115,53 @@ func (sc *SafeConfig) ReloadConfig(confFile string) (err error) {
return nil
}

// Regexp encapsulates a regexp.Regexp and makes it YAML marshalable.
type Regexp struct {
*regexp.Regexp
original string
}

// NewRegexp creates a new anchored Regexp and returns an error if the
// passed-in regular expression does not compile.
func NewRegexp(s string) (Regexp, error) {
regex, err := regexp.Compile(s)
return Regexp{
Regexp: regex,
original: s,
}, err
}

// UnmarshalYAML implements the yaml.Unmarshaler interface.
func (re *Regexp) UnmarshalYAML(unmarshal func(interface{}) error) error {
var s string
if err := unmarshal(&s); err != nil {
return err
}
r, err := NewRegexp(s)
if err != nil {
return fmt.Errorf("\"Could not compile regular expression\" regexp=\"%s\"", s)
}
*re = r
return nil
}

// MarshalYAML implements the yaml.Marshaler interface.
func (re Regexp) MarshalYAML() (interface{}, error) {
if re.original != "" {
return re.original, nil
}
return nil, nil
}

// MustNewRegexp works like NewRegexp, but panics if the regular expression does not compile.
func MustNewRegexp(s string) Regexp {
re, err := NewRegexp(s)
if err != nil {
panic(err)
}
return re
}

type Module struct {
Prober string `yaml:"prober,omitempty"`
Timeout time.Duration `yaml:"timeout,omitempty"`
Expand All @@ -134,8 +182,8 @@ type HTTPProbe struct {
FailIfNotSSL bool `yaml:"fail_if_not_ssl,omitempty"`
Method string `yaml:"method,omitempty"`
Headers map[string]string `yaml:"headers,omitempty"`
FailIfBodyMatchesRegexp []string `yaml:"fail_if_body_matches_regexp,omitempty"`
FailIfBodyNotMatchesRegexp []string `yaml:"fail_if_body_not_matches_regexp,omitempty"`
FailIfBodyMatchesRegexp []Regexp `yaml:"fail_if_body_matches_regexp,omitempty"`
FailIfBodyNotMatchesRegexp []Regexp `yaml:"fail_if_body_not_matches_regexp,omitempty"`
FailIfHeaderMatchesRegexp []HeaderMatch `yaml:"fail_if_header_matches,omitempty"`
FailIfHeaderNotMatchesRegexp []HeaderMatch `yaml:"fail_if_header_not_matches,omitempty"`
Body string `yaml:"body,omitempty"`
Expand All @@ -144,12 +192,12 @@ type HTTPProbe struct {

type HeaderMatch struct {
Header string `yaml:"header,omitempty"`
Regexp string `yaml:"regexp,omitempty"`
Regexp Regexp `yaml:"regexp,omitempty"`
AllowMissing bool `yaml:"allow_missing,omitempty"`
}

type QueryResponse struct {
Expect string `yaml:"expect,omitempty"`
Expect Regexp `yaml:"expect,omitempty"`
Send string `yaml:"send,omitempty"`
StartTLS bool `yaml:"starttls,omitempty"`
}
Expand Down Expand Up @@ -289,6 +337,10 @@ func (s *QueryResponse) UnmarshalYAML(unmarshal func(interface{}) error) error {
if err := unmarshal((*plain)(s)); err != nil {
return err
}
if s.Expect.Regexp == nil {
s.Expect = MustNewRegexp("")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not being set and being the empty string are different, if there's handling here it should be in the probe rather than here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted this IF statement to accept nil.

}

return nil
}

Expand All @@ -303,7 +355,7 @@ func (s *HeaderMatch) UnmarshalYAML(unmarshal func(interface{}) error) error {
return errors.New("header name must be set for HTTP header matchers")
}

if s.Regexp == "" {
if s.Regexp.Regexp == nil || s.Regexp.Regexp.String() == "" {
return errors.New("regexp must be set for HTTP header matchers")
}

Expand Down
16 changes: 16 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@ func TestLoadBadConfigs(t *testing.T) {
ConfigFile: "testdata/invalid-http-header-match.yml",
ExpectedError: "error parsing config file: regexp must be set for HTTP header matchers",
},
{
ConfigFile: "testdata/invalid-http-body-match-regexp.yml",
ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"",
},
{
ConfigFile: "testdata/invalid-http-body-not-match-regexp.yml",
ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"",
},
{
ConfigFile: "testdata/invalid-http-header-match-regexp.yml",
ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"",
},
{
ConfigFile: "testdata/invalid-tcp-query-response-regexp.yml",
ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"",
},
}
for i, test := range tests {
err := sc.ReloadConfig(test.ConfigFile)
Expand Down
7 changes: 7 additions & 0 deletions config/testdata/invalid-http-body-match-regexp.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
modules:
http_headers:
prober: http
timeout: 5s
http:
fail_if_body_matches_regexp:
- ':['
7 changes: 7 additions & 0 deletions config/testdata/invalid-http-body-not-match-regexp.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
modules:
http_headers:
prober: http
timeout: 5s
http:
fail_if_body_not_matches_regexp:
- ':['
9 changes: 9 additions & 0 deletions config/testdata/invalid-http-header-match-regexp.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
modules:
http_headers:
prober: http
timeout: 5s
http:
fail_if_header_not_matches:
- header: Access-Control-Allow-Origin
allow_missing: false
regexp: ':['
8 changes: 8 additions & 0 deletions config/testdata/invalid-tcp-query-response-regexp.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
modules:
tcp_test:
prober: tcp
timeout: 5s
tcp:
query_response:
- expect: ":["
- send: ". STARTTLS"
31 changes: 4 additions & 27 deletions prober/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"net/http/httptrace"
"net/textproto"
"net/url"
"regexp"
"strconv"
"strings"
"sync"
Expand All @@ -47,23 +46,13 @@ func matchRegularExpressions(reader io.Reader, httpConfig config.HTTPProbe, logg
return false
}
for _, expression := range httpConfig.FailIfBodyMatchesRegexp {
re, err := regexp.Compile(expression)
if err != nil {
level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", expression, "err", err)
return false
}
if re.Match(body) {
if expression.Regexp.Match(body) {
level.Error(logger).Log("msg", "Body matched regular expression", "regexp", expression)
return false
}
}
for _, expression := range httpConfig.FailIfBodyNotMatchesRegexp {
re, err := regexp.Compile(expression)
if err != nil {
level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", expression, "err", err)
return false
}
if !re.Match(body) {
if !expression.Regexp.Match(body) {
level.Error(logger).Log("msg", "Body did not match regular expression", "regexp", expression)
return false
}
Expand All @@ -83,14 +72,8 @@ func matchRegularExpressionsOnHeaders(header http.Header, httpConfig config.HTTP
}
}

re, err := regexp.Compile(headerMatchSpec.Regexp)
if err != nil {
level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", headerMatchSpec.Regexp, "err", err)
return false
}

for _, val := range values {
if re.MatchString(val) {
if headerMatchSpec.Regexp.MatchString(val) {
level.Error(logger).Log("msg", "Header matched regular expression", "header", headerMatchSpec.Header,
"regexp", headerMatchSpec.Regexp, "value_count", len(values))
return false
Expand All @@ -108,16 +91,10 @@ func matchRegularExpressionsOnHeaders(header http.Header, httpConfig config.HTTP
}
}

re, err := regexp.Compile(headerMatchSpec.Regexp)
if err != nil {
level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", headerMatchSpec.Regexp, "err", err)
return false
}

anyHeaderValueMatched := false

for _, val := range values {
if re.MatchString(val) {
if headerMatchSpec.Regexp.MatchString(val) {
anyHeaderValueMatched = true
break
}
Expand Down
52 changes: 26 additions & 26 deletions prober/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,30 +334,30 @@ func TestFailIfNotSSL(t *testing.T) {
func TestFailIfBodyMatchesRegexp(t *testing.T) {
testcases := map[string]struct {
respBody string
regexps []string
regexps []config.Regexp
expectedResult bool
}{
"one regex, match": {
respBody: "Bad news: could not connect to database server",
regexps: []string{"could not connect to database"},
regexps: []config.Regexp{config.MustNewRegexp("could not connect to database")},
expectedResult: false,
},

"one regex, no match": {
respBody: "Download the latest version here",
regexps: []string{"could not connect to database"},
regexps: []config.Regexp{config.MustNewRegexp("could not connect to database")},
expectedResult: true,
},

"multiple regexes, match": {
respBody: "internal error",
regexps: []string{"could not connect to database", "internal error"},
regexps: []config.Regexp{config.MustNewRegexp("could not connect to database"), config.MustNewRegexp("internal error")},
expectedResult: false,
},

"multiple regexes, no match": {
respBody: "hello world",
regexps: []string{"could not connect to database", "internal error"},
regexps: []config.Regexp{config.MustNewRegexp("could not connect to database"), config.MustNewRegexp("internal error")},
expectedResult: true,
},
}
Expand Down Expand Up @@ -410,7 +410,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) {
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
result := ProbeHTTP(testCTX, ts.URL,
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here"}}}, registry, log.NewNopLogger())
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here")}}}, registry, log.NewNopLogger())
body := recorder.Body.String()
if result {
t.Fatalf("Regexp test succeeded unexpectedly, got %s", body)
Expand All @@ -424,7 +424,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) {
recorder = httptest.NewRecorder()
registry = prometheus.NewRegistry()
result = ProbeHTTP(testCTX, ts.URL,
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here"}}}, registry, log.NewNopLogger())
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here")}}}, registry, log.NewNopLogger())
body = recorder.Body.String()
if !result {
t.Fatalf("Regexp test failed unexpectedly, got %s", body)
Expand All @@ -440,7 +440,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) {
recorder = httptest.NewRecorder()
registry = prometheus.NewRegistry()
result = ProbeHTTP(testCTX, ts.URL,
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here", "Copyright 2015"}}}, registry, log.NewNopLogger())
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here"), config.MustNewRegexp("Copyright 2015")}}}, registry, log.NewNopLogger())
body = recorder.Body.String()
if result {
t.Fatalf("Regexp test succeeded unexpectedly, got %s", body)
Expand All @@ -454,7 +454,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) {
recorder = httptest.NewRecorder()
registry = prometheus.NewRegistry()
result = ProbeHTTP(testCTX, ts.URL,
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here", "Copyright 2015"}}}, registry, log.NewNopLogger())
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here"), config.MustNewRegexp("Copyright 2015")}}}, registry, log.NewNopLogger())
body = recorder.Body.String()
if !result {
t.Fatalf("Regexp test failed unexpectedly, got %s", body)
Expand All @@ -467,15 +467,15 @@ func TestFailIfHeaderMatchesRegexp(t *testing.T) {
Values []string
ShouldSucceed bool
}{
{config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"text/javascript"}, false},
{config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"application/octet-stream"}, true},
{config.HeaderMatch{"content-type", "text/javascript", false}, []string{"application/octet-stream"}, true},
{config.HeaderMatch{"Content-Type", ".*", false}, []string{""}, false},
{config.HeaderMatch{"Content-Type", ".*", false}, []string{}, false},
{config.HeaderMatch{"Content-Type", ".*", true}, []string{""}, false},
{config.HeaderMatch{"Content-Type", ".*", true}, []string{}, true},
{config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false},
{config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"text/javascript"}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"application/octet-stream"}, true},
{config.HeaderMatch{"content-type", config.MustNewRegexp("text/javascript"), false}, []string{"application/octet-stream"}, true},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{""}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), true}, []string{""}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), true}, []string{}, true},
{config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false},
{config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false},
}

for i, test := range tests {
Expand Down Expand Up @@ -516,14 +516,14 @@ func TestFailIfHeaderNotMatchesRegexp(t *testing.T) {
Values []string
ShouldSucceed bool
}{
{config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"text/javascript"}, true},
{config.HeaderMatch{"content-type", "text/javascript", false}, []string{"text/javascript"}, true},
{config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"application/octet-stream"}, false},
{config.HeaderMatch{"Content-Type", ".*", false}, []string{""}, true},
{config.HeaderMatch{"Content-Type", ".*", false}, []string{}, false},
{config.HeaderMatch{"Content-Type", ".*", true}, []string{}, true},
{config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/"}, false},
{config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, true},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"text/javascript"}, true},
{config.HeaderMatch{"content-type", config.MustNewRegexp("text/javascript"), false}, []string{"text/javascript"}, true},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"application/octet-stream"}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{""}, true},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{}, false},
{config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), true}, []string{}, true},
{config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/"}, false},
{config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, true},
}

for i, test := range tests {
Expand Down
16 changes: 5 additions & 11 deletions prober/tcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"crypto/tls"
"fmt"
"net"
"regexp"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
Expand Down Expand Up @@ -146,19 +145,14 @@ func ProbeTCP(ctx context.Context, target string, module config.Module, registry
for i, qr := range module.TCP.QueryResponse {
level.Info(logger).Log("msg", "Processing query response entry", "entry_number", i)
send := qr.Send
if qr.Expect != "" {
re, err := regexp.Compile(qr.Expect)
if err != nil {
level.Error(logger).Log("msg", "Could not compile into regular expression", "regexp", qr.Expect, "err", err)
return false
}
if qr.Expect.Regexp != nil && qr.Expect.Regexp.String() != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Regexp always non-nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Regexp always non-nil?

There is the case that config.Module instance is created in prober/tcp_test.go.

module := config.Module{
TCP: config.TCPProbe{
IPProtocolFallback: true,
QueryResponse: []config.QueryResponse{
{Expect: config.MustNewRegexp("^220.*ESMTP.*$")},
{Send: "EHLO tls.prober"},
{Expect: config.MustNewRegexp("^250-STARTTLS")},
{Send: "STARTTLS"},
{Expect: config.MustNewRegexp("^220")},
{StartTLS: true},
{Send: "EHLO tls.prober"},
{Expect: config.MustNewRegexp("^250-AUTH")},
{Send: "QUIT"},
},
TLSConfig: pconfig.TLSConfig{
CAFile: tmpCaFile.Name(),
InsecureSkipVerify: false,
},
},
}

And when Expect isn't explicitly specified as following line, it's possible that qr.Expect.Rege is nil.

{Send: "EHLO tls.prober"},

But that occors only in the test senario. In this case, should I modify prober/tcp_test.go as following.

   {Expect: config.MustNewRegexp(""), Send: "EHLO tls.prober"}, 

Could you give me an advice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just need the first predicate here, only check for nil. Expecting an empty response is valid after all.

Copy link
Contributor Author

@sshota0809 sshota0809 Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just need the first predicate here, only check for nil.

Thank you. I deleted the latter predicate.

var match []int
// Read lines until one of them matches the configured regexp.
for scanner.Scan() {
level.Debug(logger).Log("msg", "Read line", "line", scanner.Text())
match = re.FindSubmatchIndex(scanner.Bytes())
match = qr.Expect.Regexp.FindSubmatchIndex(scanner.Bytes())
if match != nil {
level.Info(logger).Log("msg", "Regexp matched", "regexp", re, "line", scanner.Text())
level.Info(logger).Log("msg", "Regexp matched", "regexp", qr.Expect.Regexp, "line", scanner.Text())
break
}
}
Expand All @@ -168,11 +162,11 @@ func ProbeTCP(ctx context.Context, target string, module config.Module, registry
}
if match == nil {
probeFailedDueToRegex.Set(1)
level.Error(logger).Log("msg", "Regexp did not match", "regexp", re, "line", scanner.Text())
level.Error(logger).Log("msg", "Regexp did not match", "regexp", qr.Expect.Regexp, "line", scanner.Text())
return false
}
probeFailedDueToRegex.Set(0)
send = string(re.Expand(nil, []byte(send), scanner.Bytes(), match))
send = string(qr.Expect.Regexp.Expand(nil, []byte(send), scanner.Bytes(), match))
}
if send != "" {
level.Debug(logger).Log("msg", "Sending line", "line", send)
Expand Down
Loading