fix(tests): add more test coverage

cmd/karma/views.go

@@ -38,12 +38,10 @@ func pong(c *gin.Context) {
 
 func compressResponse(data []byte) ([]byte, error) {
 	var b bytes.Buffer
-	gz, err := gzip.NewWriterLevel(&b, 3)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create new compression writer: %s", err.Error())
-	}
+	// this only fails if we pass unsupported level (3 is valid)
+	gz, _ := gzip.NewWriterLevel(&b, 3)
 
-	_, err = gz.Write(data)
+	_, err := gz.Write(data)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compress data: %s", err.Error())
 	}

cmd/karma/views_test.go

@@ -30,7 +30,7 @@ var upstreamSetup = false
 func mockConfig() {
 	log.SetLevel(log.ErrorLevel)
 	os.Setenv("ALERTMANAGER_URI", "http://localhost")
-	os.Setenv("LABELS_COLOR_UNIQUE", "alertname")
+	os.Setenv("LABELS_COLOR_UNIQUE", "alertname @receiver @alertmanager")
 
 	f := pflag.NewFlagSet(".", pflag.ExitOnError)
 	config.SetupFlags(f)
@@ -142,7 +142,7 @@ func TestAlerts(t *testing.T) {
 			if len(ur.Filters) != 3 {
 				t.Errorf("[%s] Got %d filter(s) in response, expected %d", version, len(ur.Filters), 3)
 			}
-			if len(ur.Colors) != 1 {
+			if len(ur.Colors) != 3 {
 				t.Errorf("[%s] Got %d color(s) in response, expected %d", version, len(ur.Colors), 1)
 			}
 			if len(ur.Grids[0].AlertGroups) != 1 {

internal/filters/filter.go

@@ -93,12 +93,8 @@ func NewFilter(expression string) FilterT {
 	if matched == "" && operator == "" && value == "" {
 		// no "filter=" part, just the value, use fuzzy filter
 		f := newFuzzyFilter()
-		matcher, err := newMatcher(regexpOperator)
-		if err != nil {
-			f.init("", nil, expression, false, expression)
-		} else {
-			f.init("", &matcher, expression, true, expression)
-		}
+		matcher, _ := newMatcher(regexpOperator)
+		f.init("", &matcher, expression, true, expression)
 		return f
 	}
 
@@ -107,11 +103,6 @@ func NewFilter(expression string) FilterT {
 		return &invalid
 	}
 
-	if operator == "" {
-		// no operator, no valid filter here
-		return &invalid
-	}
-
 	// we have "filter=" part, lookup filter that matches
 	for _, fc := range AllFilters {
 		f := fc.Factory()
@@ -122,16 +113,10 @@ func NewFilter(expression string) FilterT {
 		if !slices.StringInSlice(fc.SupportedOperators, operator) {
 			return &invalid
 		}
-		matcher, err := newMatcher(operator)
-		if err != nil {
-			f.init(matched, nil, expression, false, "")
-		} else {
-			if value != "" {
-				f.init(matched, &matcher, expression, true, value)
-				return f
-			}
-			f.init(matched, &matcher, expression, false, "")
-		}
+		// we validate operator above, no need to re-check
+		matcher, _ := newMatcher(operator)
+		f.init(matched, &matcher, expression, true, value)
+		return f
 	}
 
 	return &invalid

internal/filters/filter_test.go

@@ -83,6 +83,30 @@ var tests = []filterTest{
 		Alert:      models.Alert{State: "suppressed", InhibitedBy: []string{"999"}},
 		IsMatch:    false,
 	},
+	{
+		Expression: "@state==active",
+		IsValid:    false,
+	},
+	{
+		Expression: "@state!!active",
+		IsValid:    false,
+	},
+	{
+		Expression: "@state!!",
+		IsValid:    false,
+	},
+	{
+		Expression: "@state=",
+		IsValid:    false,
+	},
+	{
+		Expression: "@state==",
+		IsValid:    false,
+	},
+	{
+		Expression: "@state<=active",
+		IsValid:    false,
+	},
 
 	{
 		Expression: "@silence_id=abcdef",
@@ -152,6 +176,13 @@ var tests = []filterTest{
 		Silence:    models.Silence{ID: "1", TicketID: "1"},
 		IsMatch:    true,
 	},
+	{
+		Expression: "@silence_ticket=1",
+		IsValid:    true,
+		Alert:      models.Alert{State: "active", SilencedBy: []string{}},
+		Silence:    models.Silence{ID: "1", TicketID: "1"},
+		IsMatch:    false,
+	},
 	{
 		Expression: "@silence_ticket=2",
 		IsValid:    true,
@@ -223,6 +254,13 @@ var tests = []filterTest{
 		Silence:    models.Silence{ID: "1", CreatedBy: "john"},
 		IsMatch:    true,
 	},
+	{
+		Expression: "@silence_author=john",
+		IsValid:    true,
+		Alert:      models.Alert{State: "active", SilencedBy: []string{}},
+		Silence:    models.Silence{ID: "1", CreatedBy: "john"},
+		IsMatch:    false,
+	},
 	{
 		Expression: "@silence_author=john",
 		IsValid:    true,
@@ -708,6 +746,20 @@ func TestLimitFilter(t *testing.T) {
 			if f.GetHits() != ft.Hits {
 				t.Errorf("[%s] GetHits() returned %#v hits, expected %d", ft.Expression, f.GetHits(), ft.Hits)
 			}
+		} else {
+			func() {
+				didPanic := false
+				defer func() {
+					if r := recover(); r != nil {
+						didPanic = true
+					}
+				}()
+				alert := models.Alert{}
+				f.Match(&alert, 0)
+				if !didPanic {
+					t.Errorf("[%s] Match() on invalid filter didn't cause panic", ft.Expression)
+				}
+			}()
 		}
 	}
 }

internal/filters/invalid_match_test.go

@@ -0,0 +1,47 @@
+package filters
+
+import (
+	"testing"
+
+	"github.com/prymitive/karma/internal/models"
+)
+
+func TestMatchOnInvalidFilter(t *testing.T) {
+	for _, ft := range AllFilters {
+		f := ft.Factory()
+		m, _ := newMatcher(f.GetMatcher())
+		f.init(f.GetName(), &m, f.GetRawText(), false, f.GetValue())
+		func() {
+			didPanic := false
+			defer func() {
+				if r := recover(); r != nil {
+					didPanic = true
+				}
+			}()
+			alert := models.Alert{}
+			f.Match(&alert, 0)
+			if !didPanic {
+				t.Errorf("[%s] Match() on invalid filter didn't cause panic", ft.Label)
+			}
+		}()
+	}
+}
+
+func TestGetMatcherNil(t *testing.T) {
+	f := alertFilter{}
+	m := f.GetMatcher()
+	if m != "" {
+		t.Errorf("Got %q from empty filter GetMatcher()", m)
+	}
+}
+
+func TestGetMatcherNotNil(t *testing.T) {
+	matcher, _ := newMatcher("=")
+	f := alertFilter{
+		Matcher: matcher,
+	}
+	m := f.GetMatcher()
+	if m != "=" {
+		t.Errorf("Got %q from empty filter GetMatcher()", m)
+	}
+}

internal/filters/matcher_test.go

@@ -151,5 +151,15 @@ func TestNewMatcher(t *testing.T) {
 			t.Errorf("Got wrong matcher for %s: %s", operator, m.GetOperator())
 		}
 	}
+}
 
+func TestInvalidMatcher(t *testing.T) {
+	operator := "<>"
+	m, err := newMatcher(operator)
+	if err == nil {
+		t.Errorf("NewMatcher(%s) didn't return any error: %s", operator, m)
+	}
+	if m != nil {
+		t.Errorf("NewMatcher(%s) returned non-nil value: %s", operator, m)
+	}
 }

internal/mapper/v017/alerts.go

@@ -17,10 +17,8 @@ type AlertMapper struct {
 
 // IsSupported returns true if given version string is supported
 func (m AlertMapper) IsSupported(version string) bool {
-	versionRange, err := semver.NewConstraint(">=0.17.0")
-	if err != nil {
-		panic(err)
-	}
+	// no need to check for errors as we pass static value
+	versionRange, _ := semver.NewConstraint(">=0.17.0")
 	return versionRange.Check(semver.MustParse(version))
 }
 

internal/mapper/v017/silences.go

@@ -16,10 +16,8 @@ type SilenceMapper struct {
 
 // IsSupported returns true if given version string is supported
 func (m SilenceMapper) IsSupported(version string) bool {
-	versionRange, err := semver.NewConstraint(">=0.17.0")
-	if err != nil {
-		panic(err)
-	}
+	// no need to check for errors as we pass static value
+	versionRange, _ := semver.NewConstraint(">=0.17.0")
 	return versionRange.Check(semver.MustParse(version))
 }
 

internal/mapper/v017/status.go

@@ -16,10 +16,8 @@ type StatusMapper struct {
 
 // IsSupported returns true if given version string is supported
 func (s StatusMapper) IsSupported(version string) bool {
-	versionRange, err := semver.NewConstraint(">=0.17.0")
-	if err != nil {
-		panic(err)
-	}
+	// no need to check for errors as we pass static value
+	versionRange, _ := semver.NewConstraint(">=0.17.0")
 	return versionRange.Check(semver.MustParse(version))
 }
 

internal/uri/uri_test.go

@@ -1,9 +1,10 @@
 package uri_test
 
 import (
+	"bytes"
+	"compress/gzip"
 	"crypto/tls"
 	"crypto/x509"
-	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -16,30 +17,80 @@ import (
 )
 
 type httpTransportTest struct {
-	timeout   time.Duration
-	tlsConfig *tls.Config
-	useTLS    bool
-	failed    bool
-	headers   map[string]string
+	name                  string
+	timeout               time.Duration
+	tlsConfig             *tls.Config
+	useTLS                bool
+	failed                bool
+	headers               map[string]string
+	responseCode          int
+	responseBody          []byte
+	responseContentLength int64
+	responseHeaders       map[string]string
 }
 
 var httpTransportTests = []httpTransportTest{
 	{
-		// plain HTTP request, should work
+		name:                  "plain HTTP request, should work",
+		responseCode:          200,
+		responseBody:          []byte("1234"),
+		responseContentLength: 4,
 	},
 	{
-		// just enable TLS, will use proper RootCA certs so it should work
-		useTLS: true,
+		name:         "plain HTTP request, 404, should fail",
+		responseCode: 404,
+		failed:       true,
 	},
 	{
-		// use empty RootCA pool so we fail on verifying server certificate
-		useTLS:    true,
-		tlsConfig: &tls.Config{RootCAs: x509.NewCertPool()},
-		failed:    true,
+		name:                  "gzipped HTTP response, should work",
+		responseCode:          200,
+		responseBody:          gzipString("1234"),
+		responseContentLength: 4,
+		responseHeaders: map[string]string{
+			"Content-Encoding": "gzip",
+		},
 	},
 	{
-		headers: map[string]string{"X-Auth-Test": "tokenValue"},
+		name:                  "invalid gzipped HTTP response, should fail",
+		responseCode:          200,
+		responseBody:          []byte("1234"),
+		responseContentLength: 4,
+		responseHeaders: map[string]string{
+			"Content-Encoding": "gzip",
+		},
+		failed: true,
 	},
+	{
+		name:                  "enable TLS, will use proper RootCA certs so it should work",
+		useTLS:                true,
+		responseCode:          200,
+		responseBody:          []byte("1234"),
+		responseContentLength: 4,
+	},
+	{
+		name:                  "use empty RootCA pool so we fail on verifying server certificate",
+		useTLS:                true,
+		tlsConfig:             &tls.Config{RootCAs: x509.NewCertPool()},
+		failed:                true,
+		responseCode:          200,
+		responseBody:          []byte("1234"),
+		responseContentLength: 4,
+	},
+	{
+		name:                  "auth headers test",
+		headers:               map[string]string{"X-Auth-Test": "tokenValue"},
+		responseCode:          200,
+		responseBody:          []byte("1234"),
+		responseContentLength: 4,
+	},
+}
+
+func gzipString(s string) []byte {
+	var b bytes.Buffer
+	gz := gzip.NewWriter(&b)
+	_, _ = gz.Write([]byte(s))
+	_ = gz.Close()
+	return b.Bytes()
 }
 
 func readAll(source io.ReadCloser) (int64, error) {
@@ -59,54 +110,72 @@ func readAll(source io.ReadCloser) (int64, error) {
 
 func TestHTTPReader(t *testing.T) {
 	log.SetLevel(log.FatalLevel)
-
-	responseBody := "1234"
-	handler := func(w http.ResponseWriter, r *http.Request) {
-		fmt.Fprintln(w, responseBody)
-	}
-	plainTS := httptest.NewServer(http.HandlerFunc(handler))
-	defer plainTS.Close()
-
-	tlsTS := httptest.NewTLSServer(http.HandlerFunc(handler))
-
-	defer tlsTS.Close()
-	caPool := x509.NewCertPool()
-	caPool.AddCert(tlsTS.Certificate())
-
 	for _, testCase := range httpTransportTests {
-		var amURI string
-		if testCase.useTLS {
-			amURI = tlsTS.URL
-		} else {
-			amURI = plainTS.URL
-		}
-
-		tlsConfig := testCase.tlsConfig
-		if tlsConfig == nil {
-			tlsConfig = &tls.Config{RootCAs: caPool}
-		}
-
-		transp, err := uri.NewReader(amURI, testCase.timeout, &http.Transport{TLSClientConfig: tlsConfig}, testCase.headers)
-		if err != nil {
-			t.Errorf("[%v] failed to create new HTTP transport: %s", testCase, err)
-		}
-
-		source, err := transp.Read(amURI, testCase.headers)
-		if err != nil {
-			if !testCase.failed {
-				t.Errorf("[%v] unexpected failure while creating reader: %s", testCase, err)
+		t.Run(testCase.name, func(t *testing.T) {
+			handler := func(w http.ResponseWriter, r *http.Request) {
+				for k, v := range testCase.responseHeaders {
+					w.Header().Set(k, v)
+				}
+				w.WriteHeader(testCase.responseCode)
+				_, _ = w.Write(testCase.responseBody)
 			}
-			continue
-		}
-		got, err := readAll(source)
-		source.Close()
 
-		if err != nil {
-			t.Errorf("[%v] Read() failed: %s", testCase, err)
+			tlsConfig := testCase.tlsConfig
+
+			var server *httptest.Server
+			if testCase.useTLS {
+				server = httptest.NewTLSServer(http.HandlerFunc(handler))
+				if tlsConfig == nil {
+					caPool := x509.NewCertPool()
+					caPool.AddCert(server.Certificate())
+					tlsConfig = &tls.Config{RootCAs: caPool}
+				}
+			} else {
+				server = httptest.NewServer(http.HandlerFunc(handler))
+			}
+			defer server.Close()
+
+			transp, err := uri.NewReader(server.URL, testCase.timeout, &http.Transport{TLSClientConfig: tlsConfig}, testCase.headers)
+			if err != nil {
+				t.Errorf("[%v] failed to create new HTTP transport: %s", testCase, err)
+			}
+
+			source, err := transp.Read(server.URL, testCase.headers)
+			if err != nil {
+				if !testCase.failed {
+					t.Errorf("[%v] unexpected failure while creating reader: %s", testCase, err)
+				}
+			} else {
+				got, err := readAll(source)
+				source.Close()
+
+				if err != nil {
+					t.Errorf("[%v] Read() failed: %s", testCase, err)
+				}
+
+				if got != testCase.responseContentLength {
+					t.Errorf("[%v] Wrong response size, got %d, expected %d", testCase, got, testCase.responseContentLength)
+				}
+			}
+		})
+	}
+}
+
+func TestInvalidNewReaderURI(t *testing.T) {
+	tests := []string{
+		"%gh&%ij",
+		"httpz://",
+	}
+	for _, testCase := range tests {
+		_, err := uri.NewReader(testCase, time.Second, nil, map[string]string{})
+		if err == nil {
+			t.Errorf("uri.NewReader(%q) didn't trigger any error", testCase)
 		}
 
-		if got != int64(len(responseBody)+1) {
-			t.Errorf("[%v] Wrong response size, got %d, expected %d", testCase, got, len(responseBody))
+		r, _ := uri.NewReader("http://localhost", time.Second, nil, map[string]string{})
+		_, err = r.Read(testCase, map[string]string{})
+		if err == nil {
+			t.Errorf("Reader.Read(%q) didn't trigger any error", testCase)
 		}
 	}
 }