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

* Solve issue: #717:Validating config includes trying to compile regexes

Signed-off-by: sshota0809 <[email protected]>

config/config.go

@@ -17,6 +17,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"regexp"
 	"runtime"
 	"sync"
 	"time"
@@ -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"`
@@ -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"`
@@ -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"`
 }
@@ -289,6 +337,7 @@ func (s *QueryResponse) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if err := unmarshal((*plain)(s)); err != nil {
 		return err
 	}
+
 	return nil
 }
 
@@ -303,7 +352,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")
 	}
 

config/config_test.go

@@ -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)

config/testdata/invalid-http-body-match-regexp.yml

@@ -0,0 +1,7 @@
+modules:
+  http_headers:
+    prober: http
+    timeout: 5s
+    http:
+      fail_if_body_matches_regexp:
+        - ':['

config/testdata/invalid-http-body-not-match-regexp.yml

@@ -0,0 +1,7 @@
+modules:
+  http_headers:
+    prober: http
+    timeout: 5s
+    http:
+      fail_if_body_not_matches_regexp:
+        - ':['

config/testdata/invalid-http-header-match-regexp.yml

@@ -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: ':['

config/testdata/invalid-tcp-query-response-regexp.yml

@@ -0,0 +1,8 @@
+modules:
+  tcp_test:
+    prober: tcp
+    timeout: 5s
+    tcp:
+      query_response:
+        - expect: ":["
+        - send: ". STARTTLS"

prober/http.go

@@ -25,7 +25,6 @@ import (
 	"net/http/httptrace"
 	"net/textproto"
 	"net/url"
-	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -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
 		}
@@ -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
@@ -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
 			}

prober/http_test.go

@@ -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,
 		},
 	}
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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 {
@@ -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 {

prober/tcp.go

@@ -19,7 +19,6 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net"
-	"regexp"
 
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
@@ -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 {
 			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
 				}
 			}
@@ -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)