From c90a5063ef23729ad45f9f5cc9ef89a1a7646cea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sun, 31 Oct 2021 22:29:45 +0000 Subject: [PATCH] fix(api): move static & valueOnly label information to a map --- .../084_invalid_valueonly_regex.txt | 14 ++ cmd/karma/views.go | 47 +++++- cmd/karma/views_test.go | 146 +++++++++++++++++- internal/alertmanager/dedup.go | 12 +- internal/config/config.go | 7 +- internal/config/config_test.go | 2 +- internal/config/models.go | 2 +- internal/models/api.go | 11 +- internal/models/api_test.go | 36 +++-- internal/slices/slices.go | 10 ++ internal/transform/strip.go | 24 ++- 11 files changed, 267 insertions(+), 44 deletions(-) create mode 100644 cmd/karma/tests/testscript/084_invalid_valueonly_regex.txt diff --git a/cmd/karma/tests/testscript/084_invalid_valueonly_regex.txt b/cmd/karma/tests/testscript/084_invalid_valueonly_regex.txt new file mode 100644 index 000000000..a886c220a --- /dev/null +++ b/cmd/karma/tests/testscript/084_invalid_valueonly_regex.txt @@ -0,0 +1,14 @@ +karma.bin-should-fail --config.file=karma.yaml +! stdout . +cmp stderr stderr.txt + +-- stderr.txt -- +level=error msg="Execution failed" error="valueOnly regex rule '.++++' is invalid: error parsing regexp: invalid nested repetition operator: `++`" +-- karma.yaml -- +alertmanager: + servers: + - name: default + uri: https://127.0.0.1:9093 +labels: + valueOnly_re: + - .++++ diff --git a/cmd/karma/views.go b/cmd/karma/views.go index 3795b2284..874892e01 100644 --- a/cmd/karma/views.go +++ b/cmd/karma/views.go @@ -186,9 +186,6 @@ func alerts(w http.ResponseWriter, r *http.Request) { }, ValueMapping: map[string]map[string]string{}, }, - StaticColorLabels: config.Config.Labels.Color.Static, - ValueOnlyLabels: config.Config.Labels.ValueOnly, - ValueOnlyRegexLabels: config.Config.Labels.AnchoredValueOnlyRegex, AnnotationsDefaultHidden: config.Config.Annotations.Default.Hidden, AnnotationsHidden: config.Config.Annotations.Hidden, AnnotationsVisible: config.Config.Annotations.Visible, @@ -206,6 +203,7 @@ func alerts(w http.ResponseWriter, r *http.Request) { }, HistoryEnabled: config.Config.History.Enabled, GridGroupLimit: config.Config.Grid.GroupLimit, + Labels: models.LabelsSettings{}, } resp.Authentication = models.AuthenticationInfo{ Enabled: config.Config.Authentication.Enabled, @@ -233,7 +231,9 @@ func alerts(w http.ResponseWriter, r *http.Request) { // need to overwrite settings as they can have user specific data newResp := models.AlertsResponse{} _ = json.Unmarshal(rawData, &newResp) + labels := newResp.Settings.Labels newResp.Settings = resp.Settings + newResp.Settings.Labels = labels newResp.Timestamp = string(ts) newResp.Authentication = resp.Authentication newData, _ := json.Marshal(&newResp) @@ -480,6 +480,7 @@ func alerts(w http.ResponseWriter, r *http.Request) { for _, filter := range matchFilters { if filter.GetValue() != "" && filter.GetMatcher() == "=" { transform.ColorLabel(colors, filter.GetName(), filter.GetValue()) + labelSettings(filter.GetName(), resp.Settings.Labels) } } @@ -496,6 +497,8 @@ func alerts(w http.ResponseWriter, r *http.Request) { } sort.Strings(resp.LabelNames) + labelsSettings(sortedGrids, resp.Settings.Labels) + resp.Grids = sortedGrids resp.Silences = silences resp.Colors = colors @@ -511,6 +514,44 @@ func alerts(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(data.([]byte)) } +func labelsSettings(grids []models.APIGrid, store models.LabelsSettings) { + labelSettings("@alertmanager", store) + labelSettings("@cluster", store) + labelSettings("@receiver", store) + labelSettings("@state", store) + for _, grid := range grids { + labelSettings(grid.LabelName, store) + for _, ag := range grid.AlertGroups { + for _, label := range ag.Labels { + labelSettings(label.Name, store) + } + for _, alert := range ag.Alerts { + for _, label := range alert.Labels { + labelSettings(label.Name, store) + } + } + } + } +} + +func labelSettings(name string, store models.LabelsSettings) { + var isStatic, isValueOnly bool + if slices.StringInSlice(config.Config.Labels.Color.Static, name) { + isStatic = true + } + if slices.StringInSlice(config.Config.Labels.ValueOnly, name) || slices.MatchesAnyRegex(name, config.Config.Labels.CompiledValueOnlyRegex) { + isValueOnly = true + } + if isStatic || isValueOnly { + if _, ok := store[name]; !ok { + store[name] = models.LabelSettings{ + IsStatic: isStatic, + IsValueOnly: isValueOnly, + } + } + } +} + // autocomplete endpoint, json, used for filter autocomplete hints func autocomplete(w http.ResponseWriter, r *http.Request) { noCache(w) diff --git a/cmd/karma/views_test.go b/cmd/karma/views_test.go index 728d9971c..df1ad10ce 100644 --- a/cmd/karma/views_test.go +++ b/cmd/karma/views_test.go @@ -22,6 +22,7 @@ import ( "github.com/prymitive/karma/internal/config" "github.com/prymitive/karma/internal/mock" "github.com/prymitive/karma/internal/models" + "github.com/prymitive/karma/internal/regex" "github.com/prymitive/karma/internal/slices" "github.com/go-chi/chi/v5" @@ -1103,9 +1104,6 @@ func TestEmptySettings(t *testing.T) { } expectedSettings := models.Settings{ - StaticColorLabels: []string{}, - ValueOnlyLabels: []string{}, - ValueOnlyRegexLabels: []string{}, AnnotationsDefaultHidden: false, AnnotationsHidden: []string{}, AnnotationsVisible: []string{}, @@ -1130,6 +1128,7 @@ func TestEmptySettings(t *testing.T) { }, HistoryEnabled: true, GridGroupLimit: 40, + Labels: models.LabelsSettings{}, } if diff := cmp.Diff(expectedSettings, ur.Settings); diff != "" { @@ -3865,3 +3864,144 @@ func TestCounters(t *testing.T) { }) } } + +func TestLabelSettings(t *testing.T) { + type testCaseT struct { + static []string + valueOnly []string + valueOnlyRe []*regexp.Regexp + labels models.LabelsSettings + } + + testCases := []testCaseT{ + { + static: []string{}, + valueOnly: []string{}, + valueOnlyRe: []*regexp.Regexp{}, + labels: models.LabelsSettings{}, + }, + { + static: []string{"job"}, + valueOnly: []string{}, + valueOnlyRe: []*regexp.Regexp{}, + labels: models.LabelsSettings{ + "job": models.LabelSettings{ + IsStatic: true, + IsValueOnly: false, + }, + }, + }, + { + static: []string{"job"}, + valueOnly: []string{"job"}, + valueOnlyRe: []*regexp.Regexp{}, + labels: models.LabelsSettings{ + "job": models.LabelSettings{ + IsStatic: true, + IsValueOnly: true, + }, + }, + }, + { + static: []string{}, + valueOnly: []string{}, + valueOnlyRe: []*regexp.Regexp{regex.MustCompileAnchored("al.*e")}, + labels: models.LabelsSettings{ + "alertname": models.LabelSettings{ + IsStatic: false, + IsValueOnly: true, + }, + }, + }, + { + static: []string{"@alertmanager", "@cluster", "@receiver", "@state"}, + valueOnly: []string{}, + valueOnlyRe: []*regexp.Regexp{regex.MustCompileAnchored("@.+")}, + labels: models.LabelsSettings{ + "@alertmanager": models.LabelSettings{ + IsStatic: true, + IsValueOnly: true, + }, + "@cluster": models.LabelSettings{ + IsStatic: true, + IsValueOnly: true, + }, + "@receiver": models.LabelSettings{ + IsStatic: true, + IsValueOnly: true, + }, + "@state": models.LabelSettings{ + IsStatic: true, + IsValueOnly: true, + }, + }, + }, + { + static: []string{}, + valueOnly: []string{"alertname"}, + valueOnlyRe: []*regexp.Regexp{}, + labels: models.LabelsSettings{ + "alertname": models.LabelSettings{ + IsStatic: false, + IsValueOnly: true, + }, + }, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + zerolog.SetGlobalLevel(zerolog.ErrorLevel) + mockCache() + version := mock.ListAllMocks()[0] + + payload, err := json.Marshal(models.AlertsRequest{ + Filters: []string{}, + GridLimits: map[string]int{}, + DefaultGroupLimit: 5, + }) + if err != nil { + t.Error(err) + t.FailNow() + } + + defer func() { + config.Config.Labels.Color.Static = []string{} + config.Config.Labels.ValueOnly = []string{} + config.Config.Labels.CompiledValueOnlyRegex = []*regexp.Regexp{} + }() + + for i, tc := range testCases { + mockConfig() + t.Logf("Testing alerts using mock files from Alertmanager %s", version) + mockAlerts(version) + config.Config.Labels.Color.Static = tc.static + config.Config.Labels.ValueOnly = tc.valueOnly + config.Config.Labels.CompiledValueOnlyRegex = tc.valueOnlyRe + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + r := testRouter() + setupRouter(r, nil) + for i := 1; i <= 3; i++ { + req := httptest.NewRequest("POST", "/alerts.json", bytes.NewReader(payload)) + resp := httptest.NewRecorder() + r.ServeHTTP(resp, req) + if resp.Code != http.StatusOK { + t.Errorf("POST /alerts.json returned status %d", resp.Code) + } + ur := models.AlertsResponse{} + err := json.Unmarshal(resp.Body.Bytes(), &ur) + if err != nil { + t.Errorf("Failed to unmarshal response: %s", err) + } + if ur.TotalAlerts == 0 { + t.Error("TotalAlerts=0") + t.FailNow() + } + if diff := cmp.Diff(tc.labels, ur.Settings.Labels); diff != "" { + t.Errorf("Wrong labels returned (-want +got):\n%s", diff) + } + } + }) + } +} diff --git a/internal/alertmanager/dedup.go b/internal/alertmanager/dedup.go index 998f91167..0739d20ae 100644 --- a/internal/alertmanager/dedup.go +++ b/internal/alertmanager/dedup.go @@ -83,14 +83,18 @@ func DedupAlerts() []models.AlertGroup { continue } ag := models.AlertGroup(agList[0]) - ag.Labels = transform.StripLables(config.Config.Labels.Keep, config.Config.Labels.Strip, - config.Config.Labels.CompiledKeepRegex, config.Config.Labels.CompiledStripRegex, ag.Labels) + ag.Labels = transform.StripLables( + config.Config.Labels.Keep, config.Config.Labels.Strip, + config.Config.Labels.CompiledKeepRegex, config.Config.Labels.CompiledStripRegex, + ag.Labels) ag.Alerts = make(models.AlertList, 0, len(alerts)) for _, alert := range alerts { alert := alert // scopelint pin // strip labels and annotations user doesn't want to see in the UI - alert.Labels = transform.StripLables(config.Config.Labels.Keep, config.Config.Labels.Strip, - config.Config.Labels.CompiledKeepRegex, config.Config.Labels.CompiledStripRegex, alert.Labels) + alert.Labels = transform.StripLables( + config.Config.Labels.Keep, config.Config.Labels.Strip, + config.Config.Labels.CompiledKeepRegex, config.Config.Labels.CompiledStripRegex, + alert.Labels) alert.Annotations = transform.StripAnnotations(config.Config.Annotations.Keep, config.Config.Annotations.Strip, alert.Annotations) // calculate final alert state based on the most important value found // in the list of states from all instances diff --git a/internal/config/config.go b/internal/config/config.go index 0bbe01c0f..bdf6099af 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -382,9 +382,12 @@ func (config *configSchema) Read(flags *pflag.FlagSet) (string, error) { } } - config.Labels.AnchoredValueOnlyRegex = make([]string, len(config.Labels.ValueOnlyRegex)) + config.Labels.CompiledValueOnlyRegex = make([]*regexp.Regexp, len(config.Labels.ValueOnlyRegex)) for i, valueOnlyRegex := range config.Labels.ValueOnlyRegex { - config.Labels.AnchoredValueOnlyRegex[i] = regex.WrapRegexWithAnchors(valueOnlyRegex) + config.Labels.CompiledValueOnlyRegex[i], err = regex.CompileAnchored(valueOnlyRegex) + if err != nil { + return "", fmt.Errorf("valueOnly regex rule '%s' is invalid: %s", valueOnlyRegex, err) + } } for labelName, customColors := range config.Labels.Color.Custom { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1554ca036..c59a77ff1 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -386,7 +386,7 @@ func TestDefaultConfig(t *testing.T) { expectedConfig.Labels.Color.Unique = []string{} expectedConfig.Labels.ValueOnly = []string{} expectedConfig.Labels.ValueOnlyRegex = []string{} - expectedConfig.Labels.AnchoredValueOnlyRegex = []string{} + expectedConfig.Labels.CompiledValueOnlyRegex = []*regexp.Regexp{} expectedConfig.Grid.Auto.Ignore = []string{} expectedConfig.Grid.Auto.Order = []string{} expectedConfig.Receivers.Keep = []string{} diff --git a/internal/config/models.go b/internal/config/models.go index 5cfcaef52..91b02eea5 100644 --- a/internal/config/models.go +++ b/internal/config/models.go @@ -156,7 +156,7 @@ type configSchema struct { CompiledStripRegex []*regexp.Regexp `yaml:"-"` ValueOnly []string `yaml:"valueOnly" koanf:"valueOnly"` ValueOnlyRegex []string `yaml:"valueOnly_re" koanf:"valueOnly_re"` - AnchoredValueOnlyRegex []string `yaml:"-"` + CompiledValueOnlyRegex []*regexp.Regexp `yaml:"-"` Color struct { Custom CustomLabelColors Static []string diff --git a/internal/models/api.go b/internal/models/api.go index cd13e9989..746eaa629 100644 --- a/internal/models/api.go +++ b/internal/models/api.go @@ -409,11 +409,15 @@ type AlertAcknowledgementSettings struct { Comment string `json:"comment"` } +type LabelSettings struct { + IsStatic bool `json:"isStatic"` + IsValueOnly bool `json:"isValueOnly"` +} + +type LabelsSettings map[string]LabelSettings + // Settings is used to export karma configuration that is used by UI type Settings struct { - StaticColorLabels []string `json:"staticColorLabels"` - ValueOnlyLabels []string `json:"valueOnlyLabels"` - ValueOnlyRegexLabels []string `json:"valueOnlyRegexLabels"` AnnotationsDefaultHidden bool `json:"annotationsDefaultHidden"` AnnotationsHidden []string `json:"annotationsHidden"` AnnotationsVisible []string `json:"annotationsVisible"` @@ -423,6 +427,7 @@ type Settings struct { AlertAcknowledgement AlertAcknowledgementSettings `json:"alertAcknowledgement"` HistoryEnabled bool `json:"historyEnabled"` GridGroupLimit int `json:"gridGroupLimit"` + Labels LabelsSettings `json:"labels"` } type AuthenticationInfo struct { diff --git a/internal/models/api_test.go b/internal/models/api_test.go index 79831c958..f6d41dc17 100644 --- a/internal/models/api_test.go +++ b/internal/models/api_test.go @@ -15,7 +15,7 @@ func TestDedupSharedMaps(t *testing.T) { ag := models.APIAlertGroup{ AlertGroup: models.AlertGroup{ Labels: models.Labels{ - {"alertname", "FakeAlert"}, + {Name: "alertname", Value: "FakeAlert"}, }, Alerts: models.AlertList{ models.Alert{ @@ -31,9 +31,9 @@ func TestDedupSharedMaps(t *testing.T) { }, }, Labels: models.Labels{ - {"alertname", "FakeAlert"}, - {"job", "node_exporter"}, - {"instance", "1"}, + {Name: "alertname", Value: "FakeAlert"}, + {Name: "job", Value: "node_exporter"}, + {Name: "instance", Value: "1"}, }, Alertmanager: []models.AlertmanagerInstance{ { @@ -61,9 +61,9 @@ func TestDedupSharedMaps(t *testing.T) { }, }, Labels: models.Labels{ - {"alertname", "FakeAlert"}, - {"job", "node_exporter"}, - {"instance", "2"}, + {Name: "alertname", Value: "FakeAlert"}, + {Name: "job", Value: "node_exporter"}, + {Name: "instance", Value: "2"}, }, Alertmanager: []models.AlertmanagerInstance{ { @@ -91,10 +91,10 @@ func TestDedupSharedMaps(t *testing.T) { }, }, Labels: models.Labels{ - {"alertname", "FakeAlert"}, - {"job", "blackbox"}, - {"instance", "3"}, - {"extra", "ignore"}, + {Name: "alertname", Value: "FakeAlert"}, + {Name: "job", Value: "blackbox"}, + {Name: "instance", Value: "3"}, + {Name: "extra", Value: "ignore"}, }, Alertmanager: []models.AlertmanagerInstance{ { @@ -126,8 +126,18 @@ func TestDedupSharedMapsSingleGroup(t *testing.T) { ag := models.APIAlertGroup{ AlertGroup: models.AlertGroup{ Alerts: models.AlertList{ - models.Alert{State: models.AlertStateActive, Labels: models.Labels{{"foo", "bar"}}}, - models.Alert{State: models.AlertStateUnprocessed, Labels: models.Labels{{"foo", "bar"}}}, + models.Alert{ + State: models.AlertStateActive, + Labels: models.Labels{ + {Name: "foo", Value: "bar"}, + }, + }, + models.Alert{ + State: models.AlertStateUnprocessed, + Labels: models.Labels{ + {Name: "foo", Value: "bar"}, + }, + }, }, }, } diff --git a/internal/slices/slices.go b/internal/slices/slices.go index da293869f..fbbecd7cc 100644 --- a/internal/slices/slices.go +++ b/internal/slices/slices.go @@ -3,6 +3,7 @@ package slices import ( "crypto/sha1" "fmt" + "regexp" ) // BoolInSlice returns true if given bool is found in a slice of bools @@ -69,3 +70,12 @@ func StringSliceDiff(slice1 []string, slice2 []string) ([]string, []string) { return missing, extra } + +func MatchesAnyRegex(value string, regexes []*regexp.Regexp) bool { + for _, regex := range regexes { + if regex.MatchString(value) { + return true + } + } + return false +} diff --git a/internal/transform/strip.go b/internal/transform/strip.go index cde0f8767..c29d361bb 100644 --- a/internal/transform/strip.go +++ b/internal/transform/strip.go @@ -17,15 +17,20 @@ func StripLables(keptLabels, ignoredLabels []string, keptLabelsRegex, ignoredLab // empty keep lists means keep everything by default keepAll := len(keptLabels) == 0 && len(keptLabelsRegex) == 0 labels := models.Labels{} + var inKeep, inStrip bool for _, label := range sourceLabels { // is explicitly marked to be kept - inKeep := slices.StringInSlice(keptLabels, label.Name) || matchesAnyRegex(label.Name, keptLabelsRegex) + inKeep = slices.StringInSlice(keptLabels, label.Name) || slices.MatchesAnyRegex(label.Name, keptLabelsRegex) // is explicitly marked to be stripped - inStrip := slices.StringInSlice(ignoredLabels, label.Name) || matchesAnyRegex(label.Name, ignoredLabelsRegex) + inStrip = slices.StringInSlice(ignoredLabels, label.Name) || slices.MatchesAnyRegex(label.Name, ignoredLabelsRegex) if (keepAll || inKeep) && !inStrip { - // strip leading and trailing space in label value - // this is to normalize values in case space is added by Alertmanager rules - labels = labels.Set(label.Name, strings.TrimSpace(label.Value)) + l := models.Label{ + Name: label.Name, + // strip leading and trailing space in label value + // this is to normalize values in case space is added by Alertmanager rules + Value: strings.TrimSpace(label.Value), + } + labels = labels.Add(l) } } sort.Sort(labels) @@ -65,12 +70,3 @@ func StripAnnotations(keptAnnotations, ignoredAnnotations []string, sourceAnnota } return annotations } - -func matchesAnyRegex(value string, regexes []*regexp.Regexp) bool { - for _, regex := range regexes { - if regex.MatchString(value) { - return true - } - } - return false -}