From f3900a4635ce0c210042efa0a74df625fd3b8004 Mon Sep 17 00:00:00 2001 From: Shirly Radco Date: Thu, 12 Mar 2026 20:34:26 +0200 Subject: [PATCH 1/4] router: add GET /rules endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add GET /api/v1/alerting/rules with Prometheus rule group retrieval, relabeling, and query filters. Fetch failures surface as warnings. Signed-off-by: Shirly Radco Signed-off-by: João Vilaça Signed-off-by: Aviv Litman Co-authored-by: AI Assistant --- internal/managementrouter/alerts_get.go | 49 -- internal/managementrouter/alerts_get_test.go | 8 + internal/managementrouter/query_filters.go | 60 +- .../managementrouter/query_filters_test.go | 186 ++++++ internal/managementrouter/router.go | 5 +- internal/managementrouter/rules_get.go | 49 ++ internal/managementrouter/rules_get_test.go | 186 ++++++ pkg/alertcomponent/matcher.go | 149 +++-- pkg/alertcomponent/matcher_test.go | 8 + pkg/k8s/prometheus_alerts.go | 70 +- pkg/k8s/prometheus_alerts_test.go | 149 +++++ pkg/k8s/prometheus_rules_types.go | 5 + pkg/k8s/rule_label_matchers.go | 34 +- pkg/k8s/rule_label_matchers_test.go | 24 + pkg/k8s/types.go | 5 +- pkg/management/get_rules.go | 395 ++++++++++++ pkg/management/get_rules_test.go | 610 ++++++++++++++++++ pkg/management/testutils/k8s_client_mock.go | 12 +- pkg/management/types.go | 4 + test/e2e/relabeled_rules_test.go | 471 ++++++++++++++ 20 files changed, 2302 insertions(+), 177 deletions(-) create mode 100644 internal/managementrouter/query_filters_test.go create mode 100644 internal/managementrouter/rules_get.go create mode 100644 internal/managementrouter/rules_get_test.go create mode 100644 pkg/management/get_rules.go create mode 100644 pkg/management/get_rules_test.go create mode 100644 test/e2e/relabeled_rules_test.go diff --git a/internal/managementrouter/alerts_get.go b/internal/managementrouter/alerts_get.go index a1f38652b..a3894be7a 100644 --- a/internal/managementrouter/alerts_get.go +++ b/internal/managementrouter/alerts_get.go @@ -1,7 +1,6 @@ package managementrouter import ( - "context" "encoding/json" "net/http" @@ -46,51 +45,3 @@ func (hr *httpRouter) GetAlerts(w http.ResponseWriter, req *http.Request) { log.WithError(err).Warn("failed to encode alerts response") } } - -//nolint:unused // used by the rules listing handler in a subsequent branch -func (hr *httpRouter) rulesWarnings(ctx context.Context) []string { - health, ok := hr.alertingHealth(ctx) - if !ok { - return nil - } - - if health.UserWorkloadEnabled && health.UserWorkload != nil { - return buildRouteWarnings(health.UserWorkload.Prometheus, k8s.UserWorkloadRouteName, "user workload Prometheus") - } - - return nil -} - -//nolint:unused // called by rulesWarnings, used in a subsequent branch -func (hr *httpRouter) alertingHealth(ctx context.Context) (k8s.AlertingHealth, bool) { - if hr.managementClient == nil { - return k8s.AlertingHealth{}, false - } - - health, err := hr.managementClient.GetAlertingHealth(ctx) - if err != nil { - log.WithError(err).Warn("alerting health unavailable") - return k8s.AlertingHealth{}, false - } - - return health, true -} - -//nolint:unused // called by rulesWarnings, used in a subsequent branch -func buildRouteWarnings(route k8s.AlertingRouteHealth, expectedName string, friendlyName string) []string { - if route.Name != "" && route.Name != expectedName { - return nil - } - if route.FallbackReachable { - return nil - } - - switch route.Status { - case k8s.RouteNotFound: - return []string{friendlyName + " route is missing"} - case k8s.RouteUnreachable: - return []string{friendlyName + " route is unreachable"} - default: - return nil - } -} diff --git a/internal/managementrouter/alerts_get_test.go b/internal/managementrouter/alerts_get_test.go index 36c6444c8..afaaef697 100644 --- a/internal/managementrouter/alerts_get_test.go +++ b/internal/managementrouter/alerts_get_test.go @@ -66,6 +66,14 @@ func decodeAlertsResp(t *testing.T, w *httptest.ResponseRecorder) managementrout return resp } +func TestGetAlerts_RepeatedStateRejected(t *testing.T) { + f := newAGFixture(t) + w := f.get(t, "/api/v1/alerting/alerts?state=&state=firing") + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body) + } +} + func TestGetAlerts_ParsesFlatQueryParams(t *testing.T) { f := newAGFixture(t) var captured k8s.GetAlertsRequest diff --git a/internal/managementrouter/query_filters.go b/internal/managementrouter/query_filters.go index f0f3d6aa4..6827cd520 100644 --- a/internal/managementrouter/query_filters.go +++ b/internal/managementrouter/query_filters.go @@ -4,6 +4,8 @@ import ( "fmt" "net/url" "strings" + + "github.com/openshift/monitoring-plugin/pkg/k8s" ) var validStates = map[string]bool{ @@ -12,27 +14,65 @@ var validStates = map[string]bool{ "silenced": true, } -// parseStateAndLabels returns the optional state filter and label matches. -// Any query param other than "state" is treated as a label match. -// Returns an error if the state value is not one of the known states. -func parseStateAndLabels(q url.Values) (string, map[string]string, error) { +// reservedQueryKeys lists query parameter names that have special meaning +// and must not be treated as label equality filters. +var reservedQueryKeys = map[string]bool{ + "state": true, + "match[]": true, +} + +// parseStateLabelsAndMatchers returns the optional state filter, label equality +// matches, and Prometheus-style label matchers from the query string. +// +// An empty state is allowed and means "all states". Repeated state values +// are rejected. Reserved keys ("state", "match[]") are handled specially. +// Every other key is treated as a label equality filter +// (e.g. ?severity=critical). Repeated values for a label key are rejected. +// +// match[] values follow upstream Prometheus API conventions and may contain +// equality, inequality, regex, or negative-regex matchers: +// +// ?match[]=severity="critical"&match[]=alertname=~"Kube.*" +func parseStateLabelsAndMatchers(q url.Values) (string, map[string]string, []string, error) { + if len(q["state"]) > 1 { + return "", nil, nil, fmt.Errorf("multiple values for state filter: only a single value is supported") + } state := strings.ToLower(strings.TrimSpace(q.Get("state"))) if state != "" && !validStates[state] { - return "", nil, fmt.Errorf("invalid state filter %q: must be one of pending, firing, silenced", state) + return "", nil, nil, fmt.Errorf("invalid state filter %q: must be one of pending, firing, silenced", state) } labels := make(map[string]string) for key, vals := range q { - if key == "state" { + if reservedQueryKeys[key] { continue } + if len(vals) > 1 { + return "", nil, nil, fmt.Errorf("multiple values for label filter %q: only a single value is supported", key) + } if len(vals) == 0 || strings.TrimSpace(vals[0]) == "" { continue } - if len(vals) > 1 { - return "", nil, fmt.Errorf("multiple values for label filter %q: only a single value is supported", key) - } labels[strings.TrimSpace(key)] = strings.TrimSpace(vals[0]) } - return state, labels, nil + + var matchers []string + for _, raw := range q["match[]"] { + v := strings.TrimSpace(raw) + if v != "" { + matchers = append(matchers, v) + } + } + if err := k8s.ParseRuleMatchers(matchers); err != nil { + return "", nil, nil, err + } + + return state, labels, matchers, nil +} + +// parseStateAndLabels returns the optional state filter and label matches. +// Any query param other than reserved keys is treated as a label match. +func parseStateAndLabels(q url.Values) (string, map[string]string, error) { + state, labels, _, err := parseStateLabelsAndMatchers(q) + return state, labels, err } diff --git a/internal/managementrouter/query_filters_test.go b/internal/managementrouter/query_filters_test.go new file mode 100644 index 000000000..1a2a611a6 --- /dev/null +++ b/internal/managementrouter/query_filters_test.go @@ -0,0 +1,186 @@ +package managementrouter + +import ( + "net/url" + "testing" +) + +func TestParseStateLabelsAndMatchers(t *testing.T) { + tests := []struct { + name string + query string + wantState string + wantLabels map[string]string + wantMatchers []string + wantMatchersLen int + wantErr bool + }{ + { + name: "empty query", + query: "", + wantState: "", + wantLabels: map[string]string{}, + wantMatchers: nil, + }, + { + name: "state only", + query: "state=firing", + wantState: "firing", + wantLabels: map[string]string{}, + }, + { + name: "flat labels only", + query: "severity=critical&namespace=openshift-monitoring", + wantState: "", + wantLabels: map[string]string{ + "severity": "critical", + "namespace": "openshift-monitoring", + }, + }, + { + name: "match[] only with equality", + query: `match[]=severity="critical"`, + wantState: "", + wantLabels: map[string]string{}, + wantMatchers: []string{ + `severity="critical"`, + }, + }, + { + name: "match[] with regex", + query: `match[]=alertname=~"Kube.*CPU.*"`, + wantState: "", + wantLabels: map[string]string{}, + wantMatchers: []string{ + `alertname=~"Kube.*CPU.*"`, + }, + }, + { + name: "multiple match[] values", + query: `match[]=severity="critical"&match[]=namespace="openshift-monitoring"`, + wantState: "", + wantLabels: map[string]string{}, + wantMatchersLen: 2, + }, + { + name: "mixed flat labels and match[]", + query: `state=firing&team=sre&match[]=severity=~"critical|warning"`, + wantState: "firing", + wantLabels: map[string]string{ + "team": "sre", + }, + wantMatchers: []string{ + `severity=~"critical|warning"`, + }, + }, + { + name: "match[] is not treated as a label", + query: `match[]=severity="critical"`, + wantState: "", + wantLabels: map[string]string{}, + }, + { + name: "invalid state", + query: "state=invalid", + wantErr: true, + }, + { + name: "repeated state values are rejected", + query: "state=&state=firing", + wantErr: true, + }, + { + name: "empty match[] values are skipped", + query: `match[]=&match[]=%20&match[]=severity="warning"`, + wantState: "", + wantLabels: map[string]string{}, + wantMatchersLen: 1, + }, + { + name: "repeated label values are rejected", + query: "severity=critical&severity=warning", + wantErr: true, + }, + { + name: "repeated label with leading empty value is rejected", + query: "namespace=&namespace=ns1", + wantErr: true, + }, + { + name: "invalid match[] is rejected", + query: `match[]=severity=`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + q, err := url.ParseQuery(tt.query) + if err != nil { + t.Fatalf("invalid test query: %v", err) + } + + state, labels, matchers, err := parseStateLabelsAndMatchers(q) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if state != tt.wantState { + t.Errorf("state = %q, want %q", state, tt.wantState) + } + + if tt.wantLabels != nil { + if len(labels) != len(tt.wantLabels) { + t.Errorf("labels length = %d, want %d", len(labels), len(tt.wantLabels)) + } + for k, v := range tt.wantLabels { + if labels[k] != v { + t.Errorf("labels[%q] = %q, want %q", k, labels[k], v) + } + } + if _, found := labels["match[]"]; found { + t.Error("match[] should not appear in labels map") + } + } + + if tt.wantMatchers != nil { + if len(matchers) != len(tt.wantMatchers) { + t.Errorf("matchers length = %d, want %d", len(matchers), len(tt.wantMatchers)) + } + for i, want := range tt.wantMatchers { + if i < len(matchers) && matchers[i] != want { + t.Errorf("matchers[%d] = %q, want %q", i, matchers[i], want) + } + } + } + + if tt.wantMatchersLen > 0 && len(matchers) != tt.wantMatchersLen { + t.Errorf("matchers length = %d, want %d", len(matchers), tt.wantMatchersLen) + } + }) + } +} + +func TestParseStateAndLabelsBackcompat(t *testing.T) { + q, _ := url.ParseQuery(`state=firing&severity=critical&match[]=alertname=~"Foo.*"`) + + state, labels, err := parseStateAndLabels(q) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if state != "firing" { + t.Errorf("state = %q, want %q", state, "firing") + } + if labels["severity"] != "critical" { + t.Errorf("severity = %q, want %q", labels["severity"], "critical") + } + if _, found := labels["match[]"]; found { + t.Error("match[] should not appear in labels map") + } +} diff --git a/internal/managementrouter/router.go b/internal/managementrouter/router.go index 8706b7b04..f0ac5cfb8 100644 --- a/internal/managementrouter/router.go +++ b/internal/managementrouter/router.go @@ -43,9 +43,10 @@ func New(managementClient management.Client) *mux.Router { BaseURL: "/api/v1/alerting", BaseRouter: r, }) - // GET /alerts is not yet in the OpenAPI spec; registered manually - // until its branch adds the spec entry and generated bindings. + // GET /alerts and GET /rules are not yet in the OpenAPI spec; registered + // manually until their respective branches add the spec entries. r.HandleFunc("/api/v1/alerting/alerts", hr.GetAlerts).Methods(http.MethodGet) + r.HandleFunc("/api/v1/alerting/rules", hr.GetRules).Methods(http.MethodGet) return r } diff --git a/internal/managementrouter/rules_get.go b/internal/managementrouter/rules_get.go new file mode 100644 index 000000000..fd4d7d05a --- /dev/null +++ b/internal/managementrouter/rules_get.go @@ -0,0 +1,49 @@ +package managementrouter + +import ( + "encoding/json" + "net/http" + + "github.com/openshift/monitoring-plugin/pkg/k8s" +) + +type GetRulesResponse struct { + Data GetRulesResponseData `json:"data"` + Warnings []string `json:"warnings,omitempty"` +} + +type GetRulesResponseData struct { + Groups []k8s.PrometheusRuleGroup `json:"groups"` +} + +// GetRules serves GET /api/v1/alerting/rules. +func (hr *httpRouter) GetRules(w http.ResponseWriter, req *http.Request) { + state, labels, matchers, err := parseStateLabelsAndMatchers(req.URL.Query()) + if err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + ctx := req.Context() + + groups, warnings, err := hr.managementClient.EnrichRules(ctx, k8s.GetRulesRequest{ + Labels: labels, + Matchers: matchers, + State: state, + }) + if err != nil { + handleError(w, err) + return + } + + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "no-store") + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(GetRulesResponse{ + Data: GetRulesResponseData{ + Groups: groups, + }, + Warnings: warnings, + }); err != nil { + log.WithError(err).Warn("failed to encode rules response") + } +} diff --git a/internal/managementrouter/rules_get_test.go b/internal/managementrouter/rules_get_test.go new file mode 100644 index 000000000..c437939ef --- /dev/null +++ b/internal/managementrouter/rules_get_test.go @@ -0,0 +1,186 @@ +package managementrouter_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/openshift/monitoring-plugin/internal/managementrouter" + "github.com/openshift/monitoring-plugin/pkg/k8s" +) + +func decodeRulesResp(t *testing.T, w *httptest.ResponseRecorder) managementrouter.GetRulesResponse { + t.Helper() + var resp managementrouter.GetRulesResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + return resp +} + +func TestGetRules_ParsesQueryParams(t *testing.T) { + f := newAGFixture(t) + var captured k8s.GetRulesRequest + f.mockPrometheusAlerts.FetchRulesFunc = func(_ context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + captured = req + return []k8s.PrometheusRuleGroup{}, nil, nil + } + + q := url.Values{} + q.Set("namespace", "ns1") + q.Set("severity", "critical") + q.Set("state", "firing") + q.Add("match[]", `alertname=~"Kube.*"`) + w := f.get(t, "/api/v1/alerting/rules?"+q.Encode()) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body) + } + if captured.State != "firing" { + t.Errorf("expected state=firing, got %q", captured.State) + } + if captured.Labels["namespace"] != "ns1" { + t.Errorf("expected namespace=ns1, got %q", captured.Labels["namespace"]) + } + if captured.Labels["severity"] != "critical" { + t.Errorf("expected severity=critical, got %q", captured.Labels["severity"]) + } + if len(captured.Matchers) != 1 || captured.Matchers[0] != `alertname=~"Kube.*"` { + t.Errorf("expected matchers [alertname=~\"Kube.*\"], got %v", captured.Matchers) + } +} + +func TestGetRules_ReturnsGroups(t *testing.T) { + f := newAGFixture(t) + f.mockPrometheusAlerts.SetRuleGroups([]k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + {Name: "HighCPUUsage", Type: k8s.RuleTypeAlerting}, + }, + }, + }) + + w := f.get(t, "/api/v1/alerting/rules") + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body) + } + if ct := w.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("expected Content-Type application/json, got %q", ct) + } + resp := decodeRulesResp(t, w) + if len(resp.Data.Groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(resp.Data.Groups)) + } + if resp.Data.Groups[0].Name != "group-a" { + t.Errorf("expected group-a, got %q", resp.Data.Groups[0].Name) + } + if len(resp.Data.Groups[0].Rules) != 1 || resp.Data.Groups[0].Rules[0].Name != "HighCPUUsage" { + t.Errorf("expected rule HighCPUUsage, got %+v", resp.Data.Groups[0].Rules) + } +} + +func TestGetRules_WarningsSurfacedFromFetchRules(t *testing.T) { + f := newAGFixture(t) + f.mockPrometheusAlerts.FetchRulesFunc = func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return []k8s.PrometheusRuleGroup{}, []string{ + "failed to get user workload rules: connection refused", + }, nil + } + f.rebuild() + + w := f.get(t, "/api/v1/alerting/rules") + resp := decodeRulesResp(t, w) + if len(resp.Warnings) != 1 { + t.Fatalf("expected 1 warning, got %d: %v", len(resp.Warnings), resp.Warnings) + } + if resp.Warnings[0] != "failed to get user workload rules: connection refused" { + t.Errorf("unexpected warning: %s", resp.Warnings[0]) + } +} + +func TestGetRules_NoWarningsWhenAllEndpointsSucceed(t *testing.T) { + f := newAGFixture(t) + f.mockPrometheusAlerts.SetRuleGroups([]k8s.PrometheusRuleGroup{}) + f.rebuild() + + w := f.get(t, "/api/v1/alerting/rules") + resp := decodeRulesResp(t, w) + if len(resp.Warnings) != 0 { + t.Errorf("expected no warnings, got: %v", resp.Warnings) + } +} + +func TestGetRules_Returns500OnError(t *testing.T) { + f := newAGFixture(t) + f.mockPrometheusAlerts.FetchRulesFunc = func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return nil, nil, fmt.Errorf("connection error") + } + + w := f.get(t, "/api/v1/alerting/rules") + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body) + } + if body := w.Body.String(); !strings.Contains(body, "An unexpected error occurred") { + t.Errorf("expected error message, got: %s", body) + } +} + +func TestGetRules_RepeatedStateRejected(t *testing.T) { + f := newAGFixture(t) + w := f.get(t, "/api/v1/alerting/rules?state=&state=firing") + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body) + } +} + +func TestGetRules_MissingAuthHeaderReturns401(t *testing.T) { + f := newAGFixture(t) + req := httptest.NewRequest(http.MethodGet, "/api/v1/alerting/rules", nil) + w := httptest.NewRecorder() + f.router.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d: %s", w.Code, w.Body) + } +} + +func TestGetRules_InvalidMatcherReturns400WithoutFetch(t *testing.T) { + f := newAGFixture(t) + called := false + f.mockPrometheusAlerts.FetchRulesFunc = func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + called = true + return nil, nil, nil + } + + q := url.Values{} + q.Add("match[]", "severity=") + w := f.get(t, "/api/v1/alerting/rules?"+q.Encode()) + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body) + } + if called { + t.Error("FetchRules should not be called for invalid match[]") + } +} + +func TestGetRules_RepeatedEmptyNamespaceReturns400WithoutFetch(t *testing.T) { + f := newAGFixture(t) + called := false + f.mockPrometheusAlerts.FetchRulesFunc = func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + called = true + return nil, nil, nil + } + + w := f.get(t, "/api/v1/alerting/rules?namespace=&namespace=ns1") + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body) + } + if called { + t.Error("FetchRules should not be called for repeated namespace") + } +} diff --git a/pkg/alertcomponent/matcher.go b/pkg/alertcomponent/matcher.go index 2c8600852..f8c6e344b 100644 --- a/pkg/alertcomponent/matcher.go +++ b/pkg/alertcomponent/matcher.go @@ -35,8 +35,8 @@ func labelValueMatcher(key string, values ...string) LabelsMatcher { return NewLabelsMatcher(key, NewStringValuesMatcher(values...)) } -func componentRule(component string, ms ...LabelsMatcher) componentMatcher { - return componentMatcher{component: component, matchers: ms} +func componentRule(component string, matchers ...LabelsMatcher) componentMatcher { + return componentMatcher{component: component, matchers: matchers} } // LabelsMatcher represents a matcher definition for a set of labels. @@ -50,8 +50,8 @@ func NewLabelsMatcher(key string, matcher ValueMatcher) LabelsMatcher { return labelMatcher{key: key, matcher: matcher} } -func NewStringValuesMatcher(keys ...string) ValueMatcher { - return stringMatcher(keys) +func NewStringValuesMatcher(values ...string) ValueMatcher { + return stringMatcher(values) } func NewRegexValuesMatcher(regexes ...*regexp.Regexp) ValueMatcher { @@ -74,11 +74,11 @@ func (l labelMatcher) Matches(labels model.LabelSet) (bool, []model.LabelName) { // Equals implements the LabelsMatcher interface. func (l labelMatcher) Equals(other LabelsMatcher) bool { - ol, ok := other.(labelMatcher) + otherLabel, ok := other.(labelMatcher) if !ok { return false } - return l.key == ol.key && l.matcher.Equals(ol.matcher) + return l.key == otherLabel.key && l.matcher.Equals(otherLabel.matcher) } // ValueMatcher represents a matcher for a specific value. @@ -100,11 +100,11 @@ func (s stringMatcher) Matches(value string) bool { // Equals implements the ValueMatcher interface. func (s stringMatcher) Equals(other ValueMatcher) bool { - o, ok := other.(stringMatcher) + otherStrings, ok := other.(stringMatcher) if !ok { return false } - return equalsNoOrder(s, o) + return equalsNoOrder(s, otherStrings) } // regexpMatcher is a matcher for a list of regular expressions. @@ -113,45 +113,44 @@ func (s stringMatcher) Equals(other ValueMatcher) bool { type regexpMatcher []*regexp.Regexp func (r regexpMatcher) Matches(value string) bool { - for _, re := range r { - if re.MatchString(value) { - return true - } - } - return false + return slices.ContainsFunc(r, func(re *regexp.Regexp) bool { + return re.MatchString(value) + }) } // Equals implements the ValueMatcher interface. func (r regexpMatcher) Equals(other ValueMatcher) bool { - o, ok := other.(regexpMatcher) + otherRegexp, ok := other.(regexpMatcher) if !ok { return false } - s1 := make([]string, 0, len(r)) - for _, re := range r { - s1 = append(s1, re.String()) - } - s2 := make([]string, 0, len(o)) - for _, re := range o { - s2 = append(s2, re.String()) + return equalsNoOrder(regexpPatterns(r), regexpPatterns(otherRegexp)) +} + +func regexpPatterns(regexps regexpMatcher) []string { + patterns := make([]string, 0, len(regexps)) + for _, re := range regexps { + patterns = append(patterns, re.String()) } - return equalsNoOrder(s1, s2) + return patterns } -func equalsNoOrder(a, b []string) bool { - if len(a) != len(b) { +func equalsNoOrder(left, right []string) bool { + if len(left) != len(right) { return false } - seen := make(map[string]int, len(a)) - for _, v := range a { - seen[v]++ + counts := make(map[string]int, len(left)) + for _, value := range left { + counts[value]++ } - for _, v := range b { - if seen[v] == 0 { + // Lengths already match, so any extra or missing value shows up as + // a zero count while walking right. + for _, value := range right { + if counts[value] == 0 { return false } - seen[v]-- + counts[value]-- } return true } @@ -168,48 +167,48 @@ type componentMatcher struct { // // It returns the component and the keys that matched. // If no match is found, it returns an empty component and nil keys. -func findComponent(compMatchers []componentMatcher, labels model.LabelSet) ( - component string, keys []model.LabelName) { - for _, compMatcher := range compMatchers { - for _, labelsMatcher := range compMatcher.matchers { - if matches, keys := labelsMatcher.Matches(labels); matches { - return compMatcher.component, keys +func findComponent(rules []componentMatcher, labels model.LabelSet) (string, []model.LabelName) { + for _, rule := range rules { + for _, labelsMatcher := range rule.matchers { + if match, matchedKeys := labelsMatcher.Matches(labels); match { + return rule.component, matchedKeys } } } return "", nil } -// componentMatcherFn is a function that tries matching provided labels to a component. -// It returns the layer, component and the keys from the labels that were used for matching. -// If no match is found, it returns an empty layer, component and nil keys. -type componentMatcherFn func(labels model.LabelSet) (layer, comp model.LabelValue, keys []model.LabelName) - -func evalMatcherFns(fns []componentMatcherFn, labels model.LabelSet) ( - layer, comp string, labelsSubset model.LabelSet) { - for _, fn := range fns { - if layer, comp, keys := fn(labels); layer != "" { - return string(layer), string(comp), getLabelsSubset(labels, keys...) +// componentMatcherFn tries to match labels to a layer and component. +// It returns the matched label keys, or empty values when there is no match. +type componentMatcherFn func(labels model.LabelSet) (layer, component model.LabelValue, keys []model.LabelName) + +func evalMatcherFns(matchers []componentMatcherFn, labels model.LabelSet) ( + layer, component string, labelsSubset model.LabelSet, +) { + for _, fn := range matchers { + matchedLayer, matchedComponent, keys := fn(labels) + if matchedLayer != "" { + return string(matchedLayer), string(matchedComponent), getLabelsSubset(labels, keys...) } } return "Others", "Others", getLabelsSubset(labels) } -// getLabelsSubset returns a subset of the labels with given keys. -func getLabelsSubset(m model.LabelSet, keys ...model.LabelName) model.LabelSet { - keys = append([]model.LabelName{ +// getLabelsSubset returns namespace, alertname, severity, and any extra keys +// that were used to classify the alert. +func getLabelsSubset(labels model.LabelSet, extraKeys ...model.LabelName) model.LabelSet { + keys := append([]model.LabelName{ model.LabelName(labelNamespace), model.LabelName(managementlabels.AlertNameLabel), model.LabelName(labelSeverity), - }, keys...) - return getMapSubset(m, keys...) + }, extraKeys...) + return getMapSubset(labels, keys...) } -// getMapSubset returns a subset of the labels with given keys. -func getMapSubset(m model.LabelSet, keys ...model.LabelName) model.LabelSet { +func getMapSubset(labels model.LabelSet, keys ...model.LabelName) model.LabelSet { subset := make(model.LabelSet, len(keys)) for _, key := range keys { - if val, ok := m[key]; ok { + if val, ok := labels[key]; ok { subset[key] = val } } @@ -308,18 +307,18 @@ var ( var cvoAlerts = []model.LabelValue{"ClusterOperatorDown", "ClusterOperatorDegraded"} -func cvoAlertsMatcher(labels model.LabelSet) (layer, comp model.LabelValue, keys []model.LabelName) { - if slices.Contains(cvoAlerts, labels[managementlabels.AlertNameLabel]) { - component := labels["name"] - if component == "" { - component = "version" - } - return "cluster", component, nil +func cvoAlertsMatcher(labels model.LabelSet) (layer, component model.LabelValue, keys []model.LabelName) { + if !slices.Contains(cvoAlerts, labels[managementlabels.AlertNameLabel]) { + return "", "", nil } - return "", "", nil + component = labels["name"] + if component == "" { + component = "version" + } + return "cluster", component, nil } -func kubevirtOperatorMatcher(labels model.LabelSet) (layer, comp model.LabelValue, keys []model.LabelName) { +func kubevirtOperatorMatcher(labels model.LabelSet) (layer, component model.LabelValue, keys []model.LabelName) { if labels["kubernetes_operator_part_of"] != "kubevirt" { return "", "", nil } @@ -340,27 +339,27 @@ func kubevirtOperatorMatcher(labels model.LabelSet) (layer, comp model.LabelValu } } -func computeMatcher(labels model.LabelSet) (layer, comp model.LabelValue, keys []model.LabelName) { +func computeMatcher(labels model.LabelSet) (layer, component model.LabelValue, keys []model.LabelName) { if slices.Contains(nodeAlerts, labels[managementlabels.AlertNameLabel]) { return "cluster", "compute", nil } return "", "", nil } -func coreMatcher(labels model.LabelSet) (layer, comp model.LabelValue, keys []model.LabelName) { - // Try matching against core components. - if component, keys := findComponent(coreMatchers, labels); component != "" { - return "cluster", model.LabelValue(component), keys +func coreMatcher(labels model.LabelSet) (layer, component model.LabelValue, keys []model.LabelName) { + matched, matchedKeys := findComponent(coreMatchers, labels) + if matched == "" { + return "", "", nil } - return "", "", nil + return "cluster", model.LabelValue(matched), matchedKeys } -func workloadMatcher(labels model.LabelSet) (layer, comp model.LabelValue, keys []model.LabelName) { - // Try matching against workload components. - if component, keys := findComponent(workloadMatchers, labels); component != "" { - return "namespace", model.LabelValue(component), keys +func workloadMatcher(labels model.LabelSet) (layer, component model.LabelValue, keys []model.LabelName) { + matched, matchedKeys := findComponent(workloadMatchers, labels) + if matched == "" { + return "", "", nil } - return "", "", nil + return "namespace", model.LabelValue(matched), matchedKeys } // DetermineComponent determines the component for a given set of labels. diff --git a/pkg/alertcomponent/matcher_test.go b/pkg/alertcomponent/matcher_test.go index 259ce44d3..239cd88b7 100644 --- a/pkg/alertcomponent/matcher_test.go +++ b/pkg/alertcomponent/matcher_test.go @@ -394,7 +394,15 @@ func TestValueMatcherEquals(t *testing.T) { } r1 := NewRegexValuesMatcher(regexp.MustCompile("^Argo")) + r2 := NewRegexValuesMatcher(regexp.MustCompile("^Argo")) + r3 := NewRegexValuesMatcher(regexp.MustCompile("^Kube")) if s1.Equals(r1) { t.Error("expected string matcher not to equal regexp matcher") } + if !r1.Equals(r2) { + t.Error("expected regexp matchers with the same patterns to be equal") + } + if r1.Equals(r3) { + t.Error("expected regexp matchers with different patterns not to be equal") + } } diff --git a/pkg/k8s/prometheus_alerts.go b/pkg/k8s/prometheus_alerts.go index 25a8d4839..4a39b1a58 100644 --- a/pkg/k8s/prometheus_alerts.go +++ b/pkg/k8s/prometheus_alerts.go @@ -171,33 +171,50 @@ func (pa *prometheusAlerts) FetchAlerts(ctx context.Context, req GetAlertsReques return out, warnings, nil } -func (pa *prometheusAlerts) GetRules(ctx context.Context, req GetRulesRequest) ([]PrometheusRuleGroup, error) { +func (pa *prometheusAlerts) FetchRules(ctx context.Context, req GetRulesRequest) ([]PrometheusRuleGroup, []string, error) { + namespaceScoped := namespaceFromLabels(req.Labels) != "" + platformRules, err := pa.getRulesViaProxy(ctx, ClusterMonitoringNamespace, PlatformRouteName, AlertSourcePlatform) if err != nil { // Namespace-scoped callers (Thanos tenancy) often lack platform // Prometheus access. Soft-fail so tenancy results are still returned. - if namespaceFromLabels(req.Labels) == "" { - return nil, err + if !namespaceScoped { + return nil, nil, err } - prometheusLog.Warnf("failed to get platform rules (continuing with namespace filter): %v", err) } - userRules, err := pa.getUserWorkloadRules(ctx, req) + userRules, userErr := pa.getUserWorkloadRules(ctx, req) + groups, warnings, err := mergeRuleFetchResults(platformRules, err, userRules, userErr, namespaceScoped) if err != nil { - prometheusLog.Warnf("failed to get user workload rules: %v", err) + return nil, nil, err } - groups := append(platformRules, userRules...) - matchers, err := compileRuleLabelMatchers(req) if err != nil { - return nil, err + return nil, nil, err } if len(matchers) == 0 { - return groups, nil + return groups, warnings, nil } - return filterRuleGroupsByLabelMatchers(groups, matchers), nil + return filterRuleGroupsByLabelMatchers(groups, matchers), warnings, nil +} + +// mergeRuleFetchResults combines platform and user-workload rule groups. +// Platform fetch errors are fatal for cluster-wide requests and warnings +// for namespace-scoped requests. User-workload errors are always warnings. +func mergeRuleFetchResults(platform []PrometheusRuleGroup, platformErr error, user []PrometheusRuleGroup, userErr error, namespaceScoped bool) ([]PrometheusRuleGroup, []string, error) { + var warnings []string + if platformErr != nil { + if !namespaceScoped { + return nil, nil, platformErr + } + warnings = append(warnings, fmt.Sprintf("failed to get platform rules: %v", platformErr)) + } + if userErr != nil { + warnings = append(warnings, fmt.Sprintf("failed to get user workload rules: %v", userErr)) + } + return append(platform, user...), warnings, nil } func (pa *prometheusAlerts) alertingHealth(ctx context.Context) AlertingHealth { @@ -652,18 +669,12 @@ func (pa *prometheusAlerts) getRulesViaProxy(ctx context.Context, namespace stri if err != nil { return nil, err } - - var rulesResp prometheusRulesResponse - if err := json.Unmarshal(raw, &rulesResp); err != nil { - return nil, fmt.Errorf("decode prometheus response: %w", err) - } - - if rulesResp.Status != "success" { - return nil, fmt.Errorf("prometheus API returned non-success status: %s", rulesResp.Status) + groups, err := parsePrometheusRulesResponse(raw, "prometheus") + if err != nil { + return nil, err } - - applyRuleSource(rulesResp.Data.Groups, source) - return rulesResp.Data.Groups, nil + applyRuleSource(groups, source) + return groups, nil } func (pa *prometheusAlerts) getRulesViaThanosTenancy(ctx context.Context, namespace string, source string) ([]PrometheusRuleGroup, error) { @@ -671,17 +682,22 @@ func (pa *prometheusAlerts) getRulesViaThanosTenancy(ctx context.Context, namesp if err != nil { return nil, err } + groups, err := parsePrometheusRulesResponse(raw, "thanos") + if err != nil { + return nil, err + } + applyRuleSource(groups, source) + return groups, nil +} +func parsePrometheusRulesResponse(raw []byte, apiName string) ([]PrometheusRuleGroup, error) { var rulesResp prometheusRulesResponse if err := json.Unmarshal(raw, &rulesResp); err != nil { - return nil, fmt.Errorf("decode thanos response: %w", err) + return nil, fmt.Errorf("decode %s response: %w", apiName, err) } - if rulesResp.Status != "success" { - return nil, fmt.Errorf("thanos API returned non-success status: %s", rulesResp.Status) + return nil, fmt.Errorf("%s API returned non-success status: %s", apiName, rulesResp.Status) } - - applyRuleSource(rulesResp.Data.Groups, source) return rulesResp.Data.Groups, nil } diff --git a/pkg/k8s/prometheus_alerts_test.go b/pkg/k8s/prometheus_alerts_test.go index c21f564ff..62daf4c5d 100644 --- a/pkg/k8s/prometheus_alerts_test.go +++ b/pkg/k8s/prometheus_alerts_test.go @@ -2,6 +2,7 @@ package k8s import ( "encoding/json" + "errors" "testing" "time" ) @@ -329,3 +330,151 @@ func TestLabelsMatch(t *testing.T) { }) } } + +// --- parsePrometheusRulesResponse --- + +func TestParsePrometheusRulesResponse_Success(t *testing.T) { + raw, err := json.Marshal(prometheusRulesResponse{ + Status: "success", + Data: prometheusRulesData{ + Groups: []PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []PrometheusRule{ + {Name: "AlertA", Type: RuleTypeAlerting, Labels: map[string]string{"severity": "critical"}}, + }, + }, + }, + }, + }) + if err != nil { + t.Fatalf("marshal fixture: %v", err) + } + + groups, err := parsePrometheusRulesResponse(raw, "prometheus") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(groups)) + } + if groups[0].Name != "group-a" { + t.Errorf("expected group name group-a, got %q", groups[0].Name) + } + if len(groups[0].Rules) != 1 || groups[0].Rules[0].Name != "AlertA" { + t.Errorf("expected rule AlertA, got %+v", groups[0].Rules) + } +} + +func TestParsePrometheusRulesResponse_InvalidJSON(t *testing.T) { + _, err := parsePrometheusRulesResponse([]byte("not json"), "prometheus") + if err == nil { + t.Fatal("expected error for invalid JSON") + } +} + +func TestParsePrometheusRulesResponse_NonSuccessStatus(t *testing.T) { + raw, err := json.Marshal(prometheusRulesResponse{Status: "error"}) + if err != nil { + t.Fatalf("marshal fixture: %v", err) + } + _, err = parsePrometheusRulesResponse(raw, "thanos") + if err == nil { + t.Fatal("expected error for non-success status") + } +} + +// --- applyRuleSource --- + +func TestApplyRuleSource(t *testing.T) { + groups := []PrometheusRuleGroup{ + { + Name: "g", + Rules: []PrometheusRule{ + { + Name: "A", + Labels: nil, + Alerts: []PrometheusRuleAlert{{Labels: nil}, {Labels: map[string]string{"alertname": "A"}}}, + }, + }, + }, + } + applyRuleSource(groups, AlertSourcePlatform) + rule := groups[0].Rules[0] + if rule.Labels[AlertSourceLabel] != AlertSourcePlatform { + t.Errorf("rule source = %q, want %q", rule.Labels[AlertSourceLabel], AlertSourcePlatform) + } + if rule.Alerts[0].Labels[AlertSourceLabel] != AlertSourcePlatform { + t.Errorf("alert[0] source = %q, want %q", rule.Alerts[0].Labels[AlertSourceLabel], AlertSourcePlatform) + } + if rule.Alerts[1].Labels[AlertSourceLabel] != AlertSourcePlatform { + t.Errorf("alert[1] source = %q, want %q", rule.Alerts[1].Labels[AlertSourceLabel], AlertSourcePlatform) + } + if rule.Alerts[1].Labels["alertname"] != "A" { + t.Errorf("alert[1] alertname = %q, want %q", rule.Alerts[1].Labels["alertname"], "A") + } +} + +// --- mergeRuleFetchResults --- + +func TestMergeRuleFetchResults(t *testing.T) { + platform := []PrometheusRuleGroup{{Name: "platform"}} + user := []PrometheusRuleGroup{{Name: "user"}} + platErr := errors.New("platform down") + userErr := errors.New("user down") + + t.Run("both succeed", func(t *testing.T) { + groups, warnings, err := mergeRuleFetchResults(platform, nil, user, nil, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 2 { + t.Fatalf("expected 2 groups, got %d", len(groups)) + } + if len(warnings) != 0 { + t.Errorf("expected no warnings, got %v", warnings) + } + }) + + t.Run("cluster-wide platform error is fatal", func(t *testing.T) { + _, _, err := mergeRuleFetchResults(nil, platErr, user, nil, false) + if err == nil { + t.Fatal("expected platform error") + } + if !errors.Is(err, platErr) { + t.Errorf("expected platform error, got %v", err) + } + }) + + t.Run("namespace-scoped platform error is a warning", func(t *testing.T) { + groups, warnings, err := mergeRuleFetchResults(nil, platErr, user, nil, true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || groups[0].Name != "user" { + t.Fatalf("expected user group, got %+v", groups) + } + if len(warnings) != 1 { + t.Fatalf("expected 1 warning, got %v", warnings) + } + if warnings[0] != "failed to get platform rules: platform down" { + t.Errorf("unexpected warning %q", warnings[0]) + } + }) + + t.Run("user error is always a warning", func(t *testing.T) { + groups, warnings, err := mergeRuleFetchResults(platform, nil, nil, userErr, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || groups[0].Name != "platform" { + t.Fatalf("expected platform group, got %+v", groups) + } + if len(warnings) != 1 { + t.Fatalf("expected 1 warning, got %v", warnings) + } + if warnings[0] != "failed to get user workload rules: user down" { + t.Errorf("unexpected warning %q", warnings[0]) + } + }) +} diff --git a/pkg/k8s/prometheus_rules_types.go b/pkg/k8s/prometheus_rules_types.go index 3f5c289fb..c41ea4e89 100644 --- a/pkg/k8s/prometheus_rules_types.go +++ b/pkg/k8s/prometheus_rules_types.go @@ -5,6 +5,11 @@ import ( "time" ) +const ( + RuleTypeAlerting = "alerting" + RuleTypeRecording = "recording" +) + // GetRulesRequest holds parameters for filtering rules alerts. type GetRulesRequest struct { // Labels filters rules by exact label equality. The special key "namespace" diff --git a/pkg/k8s/rule_label_matchers.go b/pkg/k8s/rule_label_matchers.go index d52c5e7d7..8f1aca0d9 100644 --- a/pkg/k8s/rule_label_matchers.go +++ b/pkg/k8s/rule_label_matchers.go @@ -2,6 +2,7 @@ package k8s import ( "fmt" + "maps" "strings" "github.com/prometheus/prometheus/model/labels" @@ -10,16 +11,29 @@ import ( const namespaceLabelKey = "namespace" +// LabelsWithoutNamespace returns a copy of labels without the tenancy +// "namespace" key. For rule queries that key selects the user-workload +// endpoint and is not a rule label filter. +func LabelsWithoutNamespace(labels map[string]string) map[string]string { + out := maps.Clone(labels) + delete(out, namespaceLabelKey) + return out +} + +// ParseRuleMatchers compiles Prometheus-style match[] values. Invalid syntax +// is returned as an error so callers can reject the request before fetching. +func ParseRuleMatchers(rawMatchers []string) error { + _, err := parseRuleMatcherSelectors(rawMatchers) + return err +} + func compileRuleLabelMatchers(req GetRulesRequest) ([]*labels.Matcher, error) { var out []*labels.Matcher - for k, v := range req.Labels { + for k, v := range LabelsWithoutNamespace(req.Labels) { if strings.TrimSpace(k) == "" { continue } - if k == namespaceLabelKey { - continue - } m, err := labels.NewMatcher(labels.MatchEqual, k, v) if err != nil { return nil, fmt.Errorf("invalid label matcher %q=%q: %w", k, v, err) @@ -27,7 +41,16 @@ func compileRuleLabelMatchers(req GetRulesRequest) ([]*labels.Matcher, error) { out = append(out, m) } - for _, raw := range req.Matchers { + matchers, err := parseRuleMatcherSelectors(req.Matchers) + if err != nil { + return nil, err + } + return append(out, matchers...), nil +} + +func parseRuleMatcherSelectors(rawMatchers []string) ([]*labels.Matcher, error) { + var out []*labels.Matcher + for _, raw := range rawMatchers { sel := strings.TrimSpace(raw) if sel == "" { continue @@ -41,7 +64,6 @@ func compileRuleLabelMatchers(req GetRulesRequest) ([]*labels.Matcher, error) { } out = append(out, matchers...) } - return out, nil } diff --git a/pkg/k8s/rule_label_matchers_test.go b/pkg/k8s/rule_label_matchers_test.go index 34169eaa7..1df44b698 100644 --- a/pkg/k8s/rule_label_matchers_test.go +++ b/pkg/k8s/rule_label_matchers_test.go @@ -56,3 +56,27 @@ func TestCompileRuleLabelMatchers_AcceptsSelectorBody(t *testing.T) { t.Fatalf("expected severity matcher, got %q", matchers[0].Name) } } + +func TestParseRuleMatchers_InvalidSyntax(t *testing.T) { + err := ParseRuleMatchers([]string{`severity=`}) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestLabelsWithoutNamespace(t *testing.T) { + in := map[string]string{ + "namespace": "ns1", + "severity": "critical", + } + got := LabelsWithoutNamespace(in) + if _, found := got["namespace"]; found { + t.Fatal("expected namespace key to be removed") + } + if got["severity"] != "critical" { + t.Errorf("expected severity=critical, got %q", got["severity"]) + } + if in["namespace"] != "ns1" { + t.Fatal("expected original map to keep namespace") + } +} diff --git a/pkg/k8s/types.go b/pkg/k8s/types.go index 91e2e0733..29881fcca 100644 --- a/pkg/k8s/types.go +++ b/pkg/k8s/types.go @@ -48,8 +48,9 @@ type PrometheusAlertsInterface interface { // FetchAlerts retrieves Prometheus alerts with optional state filtering. // Non-fatal endpoint failures are returned as warnings rather than errors. FetchAlerts(ctx context.Context, req GetAlertsRequest) ([]PrometheusAlert, []string, error) - // GetRules retrieves Prometheus alerting rules and active alerts - GetRules(ctx context.Context, req GetRulesRequest) ([]PrometheusRuleGroup, error) + // FetchRules retrieves Prometheus alerting rules and active alerts. + // Non-fatal endpoint failures are returned as warnings rather than errors. + FetchRules(ctx context.Context, req GetRulesRequest) ([]PrometheusRuleGroup, []string, error) } // PrometheusRuleInterface defines operations for managing PrometheusRules diff --git a/pkg/management/get_rules.go b/pkg/management/get_rules.go new file mode 100644 index 000000000..6dce848cc --- /dev/null +++ b/pkg/management/get_rules.go @@ -0,0 +1,395 @@ +package management + +import ( + "context" + "fmt" + "math" + "sort" + "strings" + "time" + "unicode" + + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/model/relabel" + "github.com/prometheus/prometheus/promql/parser" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/pkg/managementlabels" +) + +// EnrichRules retrieves Prometheus rule groups and applies relabeling. +// Non-fatal endpoint failures are returned as warnings. +func (c *client) EnrichRules(ctx context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + groups, warnings, err := c.k8sClient.PrometheusAlerts().FetchRules(ctx, req) + if err != nil { + return nil, nil, fmt.Errorf("failed to get prometheus rules: %w", err) + } + + configs := c.k8sClient.RelabeledRules().Config() + relabeledByAlert := indexRelabeledRules(c.k8sClient.RelabeledRules().List(ctx)) + labelFilters := k8s.LabelsWithoutNamespace(req.Labels) + applyFilters := req.State != "" || len(labelFilters) > 0 + + // Deduplicate rules that carry the same openshift_io_alert_rule_id across + // groups. This occurs when the same PrometheusRule group name is defined in + // multiple CRDs — Prometheus returns separate groups with identical rules + // that hash to the same ID after enrichment. + seenIDs := make(map[string]struct{}) + + filteredGroups := make([]k8s.PrometheusRuleGroup, 0, len(groups)) + for groupIdx := range groups { + group := groups[groupIdx] + filteredRules := make([]k8s.PrometheusRule, 0, len(group.Rules)) + + for ruleIdx := range group.Rules { + rule := group.Rules[ruleIdx] + if applyFilters && rule.Type != k8s.RuleTypeAlerting { + continue + } + applyRelabeledRuleLabels(&rule, relabeledByAlert) + + if ruleID := rule.Labels[k8s.AlertRuleLabelId]; ruleID != "" { + if _, seen := seenIDs[ruleID]; seen { + continue + } + seenIDs[ruleID] = struct{}{} + } + + if len(rule.Alerts) == 0 { + if applyFilters && rule.Type == k8s.RuleTypeAlerting { + continue + } + filteredRules = append(filteredRules, rule) + continue + } + + relabeledAlerts := make([]k8s.PrometheusRuleAlert, 0, len(rule.Alerts)) + for _, alert := range rule.Alerts { + if alert.State == "pending" || alert.State == "firing" { + if alert.Labels[k8s.AlertSourceLabel] != k8s.AlertSourceUser { + // Apply relabeling to the "real" alert labels only; preserve plugin meta labels. + src := alert.Labels[k8s.AlertSourceLabel] + in := make(map[string]string, len(alert.Labels)) + for k, v := range alert.Labels { + in[k] = v + } + delete(in, k8s.AlertSourceLabel) + + relabeledLabels, keep := relabel.Process(labels.FromMap(in), configs...) + if !keep { + continue + } + alert.Labels = relabeledLabels.Map() + if src != "" { + alert.Labels[k8s.AlertSourceLabel] = src + } + } + } + + if req.State != "" && alert.State != req.State { + continue + } + if !ruleAlertLabelsMatch(labelFilters, &alert) { + continue + } + relabeledAlerts = append(relabeledAlerts, alert) + } + rule.Alerts = relabeledAlerts + + if applyFilters && rule.Type == k8s.RuleTypeAlerting && len(rule.Alerts) == 0 { + continue + } + + filteredRules = append(filteredRules, rule) + } + + group.Rules = filteredRules + if applyFilters && len(group.Rules) == 0 { + continue + } + filteredGroups = append(filteredGroups, group) + } + + return filteredGroups, warnings, nil +} + +func indexRelabeledRules(rules []monitoringv1.Rule) map[string][]monitoringv1.Rule { + byAlert := make(map[string][]monitoringv1.Rule, len(rules)) + for _, rule := range rules { + alertName := rule.Alert + if alertName == "" && rule.Labels != nil { + alertName = rule.Labels[managementlabels.AlertNameLabel] + } + if alertName == "" { + continue + } + byAlert[alertName] = append(byAlert[alertName], rule) + } + return byAlert +} + +func relabeledAlertName(rule *monitoringv1.Rule) string { + if rule == nil { + return "" + } + if rule.Alert != "" { + return rule.Alert + } + if rule.Labels != nil { + return rule.Labels[managementlabels.AlertNameLabel] + } + return "" +} + +func applyRelabeledRuleLabels(rule *k8s.PrometheusRule, relabeledByAlert map[string][]monitoringv1.Rule) { + if rule == nil || rule.Name == "" || rule.Type == k8s.RuleTypeRecording { + return + } + + // Preserve plugin meta labels added during API fetch. + source := "" + if rule.Labels != nil { + source = rule.Labels[k8s.AlertSourceLabel] + } + + match := findRelabeledMatch(rule, relabeledByAlert[rule.Name]) + if match == nil || match.Labels == nil { + return + } + + // Replace rule labels with the relabeled cache version so that actions which + // remove/rename labels (e.g. LabelDrop/LabelKeep/LabelMap) are faithfully reflected. + labelsOut := make(map[string]string, len(match.Labels)+1) + for k, v := range match.Labels { + labelsOut[k] = v + } + if source != "" { + labelsOut[k8s.AlertSourceLabel] = source + } + rule.Labels = labelsOut +} + +func findRelabeledMatch(rule *k8s.PrometheusRule, candidates []monitoringv1.Rule) *monitoringv1.Rule { + // Strict match first (preserves correctness when multiple rules share alertname). + for i := range candidates { + candidate := &candidates[i] + if promRuleMatchesRelabeled(rule, candidate) { + return candidate + } + } + + // If relabeling modified rule labels (e.g. severity), strict label matching may fail. + // Retry on a best-effort basis using (alertname, expr, for) only. If this is ambiguous, + // do not guess. + var relaxed *monitoringv1.Rule + for i := range candidates { + candidate := &candidates[i] + if rule == nil || candidate == nil { + continue + } + candidateName := relabeledAlertName(candidate) + if rule.Name == "" || candidateName == "" || rule.Name != candidateName { + continue + } + if canonicalizePromQL(rule.Query) != canonicalizePromQL(candidate.Expr.String()) { + continue + } + if !durationMatches(rule.Duration, candidate.For) { + continue + } + if relaxed != nil { + // ambiguous + relaxed = nil + break + } + relaxed = candidate + } + if relaxed != nil { + return relaxed + } + + // Fallback: if alertname is globally unique, avoid brittle PromQL/metadata matching. + // This helps when Prometheus stringifies PromQL differently than PrometheusRule YAML + // (e.g. label matcher ordering). + if len(candidates) == 1 { + return &candidates[0] + } + return nil +} + +func promRuleMatchesRelabeled(rule *k8s.PrometheusRule, candidate *monitoringv1.Rule) bool { + if rule == nil || candidate == nil { + return false + } + candidateName := relabeledAlertName(candidate) + if rule.Name == "" || candidateName == "" || rule.Name != candidateName { + return false + } + if canonicalizePromQL(rule.Query) != canonicalizePromQL(candidate.Expr.String()) { + return false + } + if !durationMatches(rule.Duration, candidate.For) { + return false + } + if !stringMapEqual(filterBusinessLabels(rule.Labels), filterBusinessLabels(candidate.Labels)) { + return false + } + return true +} + +func canonicalizePromQL(in string) string { + s := strings.TrimSpace(in) + if s == "" { + return "" + } + expr, err := parser.ParseExpr(s) + if err == nil && expr != nil { + parser.Inspect(expr, func(node parser.Node, _ []parser.Node) error { + switch n := node.(type) { + case *parser.VectorSelector: + sort.Slice(n.LabelMatchers, func(i, j int) bool { + mi, mj := n.LabelMatchers[i], n.LabelMatchers[j] + if mi == nil || mj == nil { + return mi != nil + } + if mi.Name != mj.Name { + return mi.Name < mj.Name + } + if mi.Type != mj.Type { + return mi.Type < mj.Type + } + return mi.Value < mj.Value + }) + case *parser.AggregateExpr: + sort.Strings(n.Grouping) + case *parser.BinaryExpr: + if n.VectorMatching != nil { + sort.Strings(n.VectorMatching.MatchingLabels) + sort.Strings(n.VectorMatching.Include) + } + } + return nil + }) + + return expr.String() + } + return normalizeSpaceOutsideQuotes(s) +} + +func normalizeSpaceOutsideQuotes(in string) string { + if in == "" { + return "" + } + in = strings.TrimSpace(in) + + var b strings.Builder + b.Grow(len(in)) + + inQuote := false + escaped := false + pendingSpace := false + lastNoSpaceToken := false + + isNoSpaceToken := func(r rune) bool { + switch r { + case '(', ')', '{', '}', ',', '+', '-', '*', '/', '%', '^', '=', '!', '<', '>': + return true + default: + return false + } + } + + for _, r := range in { + if escaped { + if pendingSpace { + if !lastNoSpaceToken { + b.WriteByte(' ') + } + pendingSpace = false + } + b.WriteRune(r) + escaped = false + lastNoSpaceToken = false + continue + } + + if inQuote && r == '\\' { + if pendingSpace { + if !lastNoSpaceToken { + b.WriteByte(' ') + } + pendingSpace = false + } + b.WriteRune(r) + escaped = true + lastNoSpaceToken = false + continue + } + + if r == '"' { + if pendingSpace { + if !lastNoSpaceToken { + b.WriteByte(' ') + } + pendingSpace = false + } + inQuote = !inQuote + b.WriteRune(r) + lastNoSpaceToken = false + continue + } + + if !inQuote && unicode.IsSpace(r) { + pendingSpace = true + continue + } + + if pendingSpace && !lastNoSpaceToken && !isNoSpaceToken(r) { + b.WriteByte(' ') + } + pendingSpace = false + + b.WriteRune(r) + lastNoSpaceToken = !inQuote && isNoSpaceToken(r) + } + + return strings.TrimSpace(b.String()) +} + +func durationMatches(seconds float64, duration *monitoringv1.Duration) bool { + if duration == nil { + return seconds == 0 + } + parsed, err := model.ParseDuration(string(*duration)) + if err != nil { + return false + } + return math.Abs(time.Duration(parsed).Seconds()-seconds) < 0.001 +} + +func stringMapEqual(a, b map[string]string) bool { + if len(a) == 0 && len(b) == 0 { + return true + } + if len(a) != len(b) { + return false + } + for k, v := range a { + if b[k] != v { + return false + } + } + return true +} + +func ruleAlertLabelsMatch(labels map[string]string, alert *k8s.PrometheusRuleAlert) bool { + for key, value := range labels { + if alertValue, exists := alert.Labels[key]; !exists || alertValue != value { + return false + } + } + + return true +} diff --git a/pkg/management/get_rules_test.go b/pkg/management/get_rules_test.go new file mode 100644 index 000000000..7a2fd2f22 --- /dev/null +++ b/pkg/management/get_rules_test.go @@ -0,0 +1,610 @@ +package management_test + +import ( + "context" + "testing" + "time" + + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/relabel" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/pkg/management" + "github.com/openshift/monitoring-plugin/pkg/management/testutils" + "github.com/openshift/monitoring-plugin/pkg/managementlabels" +) + +// grFixture builds a management client with a PrometheusAlerts mock returning +// the given groups and a RelabeledRules mock returning the given configs/rules. +type grFixture struct { + groups []k8s.PrometheusRuleGroup + relabelRules []monitoringv1.Rule + relabelConfig []*relabel.Config +} + +func (f grFixture) client(t *testing.T) management.Client { + t.Helper() + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return f.groups, nil, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { return f.relabelRules }, + ConfigFunc: func() []*relabel.Config { return f.relabelConfig }, + } + }, + } + return management.New(context.Background(), mockK8s) +} + +// threeAlertGroup returns a rule group containing one alerting rule with +// firing Alert1, pending Alert2, and inactive Alert3. +func threeAlertGroup() []k8s.PrometheusRuleGroup { + return []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + { + Name: "rule-a", + Type: k8s.RuleTypeAlerting, + Alerts: []k8s.PrometheusRuleAlert{ + {State: "firing", Labels: map[string]string{"alertname": "Alert1", "severity": "warning"}}, + {State: "pending", Labels: map[string]string{"alertname": "Alert2", "severity": "critical"}}, + {State: "inactive", Labels: map[string]string{"alertname": "Alert3", "severity": "warning"}}, + }, + }, + }, + }, + } +} + +func dropAlert2ReplaceAlert1Severity() []*relabel.Config { + return []*relabel.Config{ + { + SourceLabels: model.LabelNames{"alertname"}, + Regex: relabel.MustNewRegexp("Alert2"), + Action: relabel.Drop, + NameValidationScheme: model.UTF8Validation, + }, + { + SourceLabels: model.LabelNames{"alertname"}, + Regex: relabel.MustNewRegexp("Alert1"), + TargetLabel: "severity", + Replacement: "critical", + Action: relabel.Replace, + NameValidationScheme: model.UTF8Validation, + }, + } +} + +func TestEnrichRules_AppliesRelabelConfigsToPendingFiringOnly(t *testing.T) { + f := grFixture{ + groups: threeAlertGroup(), + relabelRules: []monitoringv1.Rule{}, + relabelConfig: dropAlert2ReplaceAlert1Severity(), + } + client := f.client(t) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(groups)) + } + rules := groups[0].Rules + if len(rules) != 1 { + t.Fatalf("expected 1 rule, got %d", len(rules)) + } + alerts := rules[0].Alerts + if len(alerts) != 2 { + t.Fatalf("expected 2 alerts after drop, got %d", len(alerts)) + } + if alerts[0].Labels["alertname"] != "Alert1" || alerts[0].Labels["severity"] != "critical" { + t.Errorf("alert[0]: got alertname=%s severity=%s", alerts[0].Labels["alertname"], alerts[0].Labels["severity"]) + } + if alerts[1].Labels["alertname"] != "Alert3" || alerts[1].Labels["severity"] != "warning" { + t.Errorf("alert[1]: got alertname=%s severity=%s", alerts[1].Labels["alertname"], alerts[1].Labels["severity"]) + } +} + +func TestEnrichRules_FiltersByStateAndLabels(t *testing.T) { + f := grFixture{ + groups: threeAlertGroup(), + relabelRules: []monitoringv1.Rule{}, + relabelConfig: dropAlert2ReplaceAlert1Severity(), + } + client := f.client(t) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{ + State: "firing", + Labels: map[string]string{"severity": "critical"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(groups)) + } + alerts := groups[0].Rules[0].Alerts + if len(alerts) != 1 { + t.Fatalf("expected 1 alert, got %d", len(alerts)) + } + if alerts[0].Labels["alertname"] != "Alert1" || alerts[0].Labels["severity"] != "critical" { + t.Errorf("unexpected alert: %v", alerts[0].Labels) + } +} + +func TestEnrichRules_DropsNonMatchingRulesWhenFiltered(t *testing.T) { + f := grFixture{ + groups: threeAlertGroup(), + relabelRules: []monitoringv1.Rule{}, + relabelConfig: dropAlert2ReplaceAlert1Severity(), + } + client := f.client(t) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{ + State: "firing", + Labels: map[string]string{"severity": "does-not-exist"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 0 { + t.Errorf("expected 0 groups, got %d", len(groups)) + } +} + +func TestEnrichRules_AddsManagedByLabelsFromRelabeledRules(t *testing.T) { + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + { + Name: "AlertWithManagedBy", + Type: "alerting", + Query: "up == 0", + Labels: map[string]string{"severity": "critical"}, + Annotations: map[string]string{"summary": "test alert"}, + }, + }, + }, + }, nil, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { + return []monitoringv1.Rule{ + { + Alert: "AlertWithManagedBy", + Expr: intstr.FromString("up ==\n 0"), + Labels: map[string]string{ + "severity": "critical", + k8s.AlertRuleLabelId: "alert-id-1", + k8s.PrometheusRuleLabelNamespace: "openshift-monitoring", + k8s.PrometheusRuleLabelName: "platform-rule", + managementlabels.RuleManagedByLabel: "operator", + managementlabels.RelabelConfigManagedByLabel: "gitops", + }, + Annotations: map[string]string{"summary": "test alert"}, + }, + } + }, + ConfigFunc: func() []*relabel.Config { return []*relabel.Config{} }, + } + }, + } + client := management.New(context.Background(), mockK8s) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || len(groups[0].Rules) != 1 { + t.Fatalf("expected 1 group with 1 rule") + } + rule := groups[0].Rules[0] + checks := map[string]string{ + k8s.AlertRuleLabelId: "alert-id-1", + k8s.PrometheusRuleLabelNamespace: "openshift-monitoring", + k8s.PrometheusRuleLabelName: "platform-rule", + managementlabels.RuleManagedByLabel: "operator", + managementlabels.RelabelConfigManagedByLabel: "gitops", + } + for k, want := range checks { + if got := rule.Labels[k]; got != want { + t.Errorf("label[%s]: want %q, got %q", k, want, got) + } + } +} + +func TestEnrichRules_EnrichesWithAllLabelTypes(t *testing.T) { + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + { + Name: "ARCUpdatedRule", + Type: "alerting", + Query: "up == 0", + Labels: map[string]string{"severity": "warning", k8s.AlertSourceLabel: k8s.AlertSourcePlatform}, + }, + }, + }, + }, nil, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { + return []monitoringv1.Rule{ + { + Alert: "ARCUpdatedRule", + Expr: intstr.FromString("up ==\n 0"), + Labels: map[string]string{ + "severity": "critical", + "team": "sre", + k8s.AlertRuleLabelId: "rid-arc-1", + k8s.PrometheusRuleLabelNamespace: "openshift-monitoring", + k8s.PrometheusRuleLabelName: "platform-rule", + k8s.AlertRuleClassificationComponentKey: "compute", + k8s.AlertRuleClassificationLayerKey: "cluster", + managementlabels.RuleManagedByLabel: "operator", + managementlabels.RelabelConfigManagedByLabel: "gitops", + }, + }, + } + }, + ConfigFunc: func() []*relabel.Config { return []*relabel.Config{} }, + } + }, + } + client := management.New(context.Background(), mockK8s) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || len(groups[0].Rules) != 1 { + t.Fatalf("expected 1 group with 1 rule") + } + rule := groups[0].Rules[0] + checks := map[string]string{ + k8s.AlertSourceLabel: k8s.AlertSourcePlatform, + k8s.AlertRuleLabelId: "rid-arc-1", + k8s.PrometheusRuleLabelNamespace: "openshift-monitoring", + k8s.PrometheusRuleLabelName: "platform-rule", + k8s.AlertRuleClassificationComponentKey: "compute", + k8s.AlertRuleClassificationLayerKey: "cluster", + "severity": "critical", + "team": "sre", + managementlabels.RuleManagedByLabel: "operator", + managementlabels.RelabelConfigManagedByLabel: "gitops", + } + for k, want := range checks { + if got := rule.Labels[k]; got != want { + t.Errorf("label[%s]: want %q, got %q", k, want, got) + } + } +} + +func TestEnrichRules_EnrichesWhenAlertFieldEmpty(t *testing.T) { + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + { + Name: "EmptyAlertFieldRule", + Type: "alerting", + Query: "up == 0", + Labels: map[string]string{"severity": "warning", k8s.AlertSourceLabel: k8s.AlertSourcePlatform}, + }, + }, + }, + }, nil, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { + return []monitoringv1.Rule{ + { + Alert: "", + Expr: intstr.FromString("up ==\n 0"), + Labels: map[string]string{ + managementlabels.AlertNameLabel: "EmptyAlertFieldRule", + "severity": "critical", + k8s.AlertRuleLabelId: "rid-empty-alert-1", + k8s.PrometheusRuleLabelNamespace: "openshift-monitoring", + k8s.PrometheusRuleLabelName: "platform-rule", + }, + }, + } + }, + ConfigFunc: func() []*relabel.Config { return []*relabel.Config{} }, + } + }, + } + client := management.New(context.Background(), mockK8s) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || len(groups[0].Rules) != 1 { + t.Fatalf("expected 1 group with 1 rule") + } + rule := groups[0].Rules[0] + checks := map[string]string{ + k8s.AlertSourceLabel: k8s.AlertSourcePlatform, + k8s.AlertRuleLabelId: "rid-empty-alert-1", + k8s.PrometheusRuleLabelNamespace: "openshift-monitoring", + k8s.PrometheusRuleLabelName: "platform-rule", + "severity": "critical", + } + for k, want := range checks { + if got := rule.Labels[k]; got != want { + t.Errorf("label[%s]: want %q, got %q", k, want, got) + } + } +} + +func TestEnrichRules_NoEnrichmentWhenMultipleCandidatesMatch(t *testing.T) { + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + { + Name: "AmbiguousRule", + Type: "alerting", + Query: "up == 0", + Labels: map[string]string{"severity": "warning", k8s.AlertSourceLabel: k8s.AlertSourcePlatform}, + }, + }, + }, + }, nil, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { + return []monitoringv1.Rule{ + { + Alert: "", + Expr: intstr.FromString("up ==\n 0"), + Labels: map[string]string{ + managementlabels.AlertNameLabel: "AmbiguousRule", + "severity": "critical", + k8s.AlertRuleLabelId: "rid-amb-1", + }, + }, + { + Alert: "", + Expr: intstr.FromString("up==0"), + Labels: map[string]string{ + managementlabels.AlertNameLabel: "AmbiguousRule", + "severity": "critical", + k8s.AlertRuleLabelId: "rid-amb-2", + }, + }, + } + }, + ConfigFunc: func() []*relabel.Config { return []*relabel.Config{} }, + } + }, + } + client := management.New(context.Background(), mockK8s) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || len(groups[0].Rules) != 1 { + t.Fatalf("expected 1 group with 1 rule") + } + rule := groups[0].Rules[0] + if rule.Labels[k8s.AlertSourceLabel] != k8s.AlertSourcePlatform { + t.Errorf("expected source=%s, got %s", k8s.AlertSourcePlatform, rule.Labels[k8s.AlertSourceLabel]) + } + if _, hasId := rule.Labels[k8s.AlertRuleLabelId]; hasId { + t.Errorf("expected no AlertRuleLabelId on ambiguous rule, but found: %s", rule.Labels[k8s.AlertRuleLabelId]) + } + if rule.Labels["severity"] != "warning" { + t.Errorf("expected severity=warning (from original), got %s", rule.Labels["severity"]) + } +} + +func TestEnrichRules_PropagatesFetchWarnings(t *testing.T) { + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return threeAlertGroup(), []string{"failed to get user workload rules: connection refused"}, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { return nil }, + ConfigFunc: func() []*relabel.Config { return nil }, + } + }, + } + client := management.New(context.Background(), mockK8s) + + groups, warnings, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(groups)) + } + if len(warnings) != 1 { + t.Fatalf("expected 1 warning, got %d: %v", len(warnings), warnings) + } + if warnings[0] != "failed to get user workload rules: connection refused" { + t.Errorf("unexpected warning %q", warnings[0]) + } +} + +func TestEnrichRules_NamespaceOnlyKeepsInactiveRules(t *testing.T) { + f := grFixture{ + groups: []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + {Name: "InactiveRule", Type: k8s.RuleTypeAlerting}, + { + Name: "FiringWithoutNamespaceLabel", + Type: k8s.RuleTypeAlerting, + Alerts: []k8s.PrometheusRuleAlert{ + {State: "firing", Labels: map[string]string{"alertname": "FiringWithoutNamespaceLabel", "severity": "warning"}}, + }, + }, + {Name: "record:foo", Type: k8s.RuleTypeRecording, Query: "vector(1)"}, + }, + }, + }, + } + client := f.client(t) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{ + Labels: map[string]string{"namespace": "ns1"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(groups)) + } + if len(groups[0].Rules) != 3 { + t.Fatalf("expected 3 rules, got %d", len(groups[0].Rules)) + } +} + +func TestEnrichRules_NamespaceAndSeverityStillFilters(t *testing.T) { + f := grFixture{ + groups: threeAlertGroup(), + relabelConfig: dropAlert2ReplaceAlert1Severity(), + } + client := f.client(t) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{ + State: "firing", + Labels: map[string]string{ + "namespace": "ns1", + "severity": "critical", + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 { + t.Fatalf("expected 1 group, got %d", len(groups)) + } + alerts := groups[0].Rules[0].Alerts + if len(alerts) != 1 { + t.Fatalf("expected 1 alert, got %d", len(alerts)) + } + if alerts[0].Labels["alertname"] != "Alert1" { + t.Errorf("expected Alert1, got %v", alerts[0].Labels) + } +} + +func promDuration(s string) *monitoringv1.Duration { + d := monitoringv1.Duration(s) + return &d +} + +func TestEnrichRules_MatchesPrometheusDayDuration(t *testing.T) { + mockK8s := &testutils.MockClient{ + PrometheusAlertsFunc: func() k8s.PrometheusAlertsInterface { + return &testutils.MockPrometheusAlertsInterface{ + FetchRulesFunc: func(_ context.Context, _ k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + return []k8s.PrometheusRuleGroup{ + { + Name: "group-a", + Rules: []k8s.PrometheusRule{ + { + Name: "DayForRule", + Type: k8s.RuleTypeAlerting, + Query: "up == 0", + Duration: (24 * time.Hour).Seconds(), + Labels: map[string]string{"severity": "warning", k8s.AlertSourceLabel: k8s.AlertSourcePlatform}, + }, + }, + }, + }, nil, nil + }, + } + }, + RelabeledRulesFunc: func() k8s.RelabeledRulesInterface { + return &testutils.MockRelabeledRulesInterface{ + ListFunc: func(_ context.Context) []monitoringv1.Rule { + return []monitoringv1.Rule{ + { + Alert: "DayForRule", + Expr: intstr.FromString("up == 0"), + For: promDuration("1d"), + Labels: map[string]string{ + "severity": "warning", + k8s.AlertRuleLabelId: "rid-day", + }, + }, + { + Alert: "DayForRule", + Expr: intstr.FromString("up == 0"), + For: promDuration("5m"), + Labels: map[string]string{ + "severity": "warning", + k8s.AlertRuleLabelId: "rid-five-min", + }, + }, + } + }, + ConfigFunc: func() []*relabel.Config { return []*relabel.Config{} }, + } + }, + } + client := management.New(context.Background(), mockK8s) + + groups, _, err := client.EnrichRules(context.Background(), k8s.GetRulesRequest{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(groups) != 1 || len(groups[0].Rules) != 1 { + t.Fatalf("expected 1 group with 1 rule") + } + got := groups[0].Rules[0].Labels[k8s.AlertRuleLabelId] + if got != "rid-day" { + t.Errorf("expected AlertRuleLabelId=rid-day, got %q", got) + } +} diff --git a/pkg/management/testutils/k8s_client_mock.go b/pkg/management/testutils/k8s_client_mock.go index cb5514d55..c1e287d34 100644 --- a/pkg/management/testutils/k8s_client_mock.go +++ b/pkg/management/testutils/k8s_client_mock.go @@ -110,7 +110,7 @@ func (m *MockClient) Namespace() k8s.NamespaceInterface { type MockPrometheusAlertsInterface struct { FetchAlertsFunc func(ctx context.Context, req k8s.GetAlertsRequest) ([]k8s.PrometheusAlert, []string, error) - GetRulesFunc func(ctx context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, error) + FetchRulesFunc func(ctx context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) ActiveAlerts []k8s.PrometheusAlert RuleGroups []k8s.PrometheusRuleGroup @@ -134,14 +134,14 @@ func (m *MockPrometheusAlertsInterface) FetchAlerts(ctx context.Context, req k8s return []k8s.PrometheusAlert{}, nil, nil } -func (m *MockPrometheusAlertsInterface) GetRules(ctx context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, error) { - if m.GetRulesFunc != nil { - return m.GetRulesFunc(ctx, req) +func (m *MockPrometheusAlertsInterface) FetchRules(ctx context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) { + if m.FetchRulesFunc != nil { + return m.FetchRulesFunc(ctx, req) } if m.RuleGroups != nil { - return m.RuleGroups, nil + return m.RuleGroups, nil, nil } - return []k8s.PrometheusRuleGroup{}, nil + return []k8s.PrometheusRuleGroup{}, nil, nil } type MockPrometheusRuleInterface struct { diff --git a/pkg/management/types.go b/pkg/management/types.go index 7fb1bfb9c..fd158a7db 100644 --- a/pkg/management/types.go +++ b/pkg/management/types.go @@ -53,6 +53,10 @@ type Client interface { // Non-fatal endpoint failures are returned as warnings. EnrichAlerts(ctx context.Context, req k8s.GetAlertsRequest) ([]k8s.PrometheusAlert, []string, error) + // EnrichRules retrieves Prometheus rule groups and applies relabeling. + // Non-fatal endpoint failures are returned as warnings. + EnrichRules(ctx context.Context, req k8s.GetRulesRequest) ([]k8s.PrometheusRuleGroup, []string, error) + // GetAlertingHealth retrieves the alerting stack health status GetAlertingHealth(ctx context.Context) (k8s.AlertingHealth, error) } diff --git a/test/e2e/relabeled_rules_test.go b/test/e2e/relabeled_rules_test.go new file mode 100644 index 000000000..e69116e42 --- /dev/null +++ b/test/e2e/relabeled_rules_test.go @@ -0,0 +1,471 @@ +//go:build e2e + +package e2e + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "testing" + "time" + + osmv1 "github.com/openshift/api/monitoring/v1" + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/test/e2e/framework" +) + +type listRulesResponse struct { + Data struct { + Groups []k8s.PrometheusRuleGroup `json:"groups"` + } `json:"data"` +} + +func listRules(ctx context.Context, f *framework.Framework) ([]k8s.PrometheusRule, error) { + rules, status, err := listRulesWithToken(ctx, f, f.BearerToken, "") + if err != nil { + return nil, err + } + if status != http.StatusOK { + return nil, fmt.Errorf("unexpected status code: %d", status) + } + return rules, nil +} + +func TestPrometheusRuleAppearsInMemory(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + + testNamespace, cleanup, err := f.CreateUserNamespace(ctx, "test-prometheus-rule") + if err != nil { + t.Fatalf("Failed to create test namespace: %v", err) + } + defer func() { + if err := cleanup(); err != nil { + t.Logf("cleanup failed: %v", err) + } + }() + + testAlertName := "TestAlert" + forDuration := monitoringv1.Duration("5m") + testRule := monitoringv1.Rule{ + Alert: testAlertName, + Expr: intstr.FromString("up == 0"), + For: &forDuration, + Labels: map[string]string{ + "severity": "warning", + }, + Annotations: map[string]string{ + "description": "Test alert for e2e testing", + "summary": "Test alert", + }, + } + + _, err = createPrometheusRule(ctx, f, testNamespace, testRule) + if err != nil { + t.Fatalf("Failed to create PrometheusRule: %v", err) + } + + err = wait.PollUntilContextTimeout(ctx, 2*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + rules, err := listRules(ctx, f) + if err != nil { + t.Logf("Failed to list rules: %v", err) + return false, nil + } + + for _, rule := range rules { + if rule.Name == testAlertName { + expectedLabels := map[string]string{ + k8s.PrometheusRuleLabelNamespace: testNamespace, + k8s.PrometheusRuleLabelName: "test-prometheus-rule", + } + + if err := compareRuleLabels(t, testAlertName, rule.Labels, expectedLabels); err != nil { + return false, err + } + + if _, ok := rule.Labels[k8s.AlertRuleLabelId]; !ok { + t.Errorf("Alert %s missing openshift_io_alert_rule_id label", testAlertName) + return false, fmt.Errorf("alert missing openshift_io_alert_rule_id label") + } + + t.Logf("Found alert %s in memory with all expected labels", testAlertName) + return true, nil + } + } + + t.Logf("Alert %s not found in memory yet (found %d rules)", testAlertName, len(rules)) + return false, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for alert to appear in memory: %v", err) + } +} + +func TestRelabelAlert(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + + testNamespace, cleanup, err := f.CreatePlatformNamespace(ctx, "test-relabel-alert") + if err != nil { + t.Fatalf("Failed to create test namespace: %v", err) + } + defer func() { + if err := cleanup(); err != nil { + t.Logf("cleanup failed: %v", err) + } + }() + + forDuration := monitoringv1.Duration("5m") + + criticalRule := monitoringv1.Rule{ + Alert: "TestRelabelAlert", + Expr: intstr.FromString("up == 0"), + For: &forDuration, + Labels: map[string]string{ + "severity": "critical", + "team": "web", + }, + Annotations: map[string]string{ + "description": "Critical alert for relabel testing", + "summary": "Critical test alert", + }, + } + + warningRule := monitoringv1.Rule{ + Alert: "TestRelabelAlert", + Expr: intstr.FromString("up == 1"), + For: &forDuration, + Labels: map[string]string{ + "severity": "warning", + "team": "web", + }, + Annotations: map[string]string{ + "description": "Warning alert for relabel testing", + "summary": "Warning test alert", + }, + } + + _, err = createPrometheusRule(ctx, f, testNamespace, criticalRule, warningRule) + if err != nil { + t.Fatalf("Failed to create PrometheusRule: %v", err) + } + + relabelConfigName := "change-critical-team" + arc := &osmv1.AlertRelabelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: relabelConfigName, + Namespace: k8s.ClusterMonitoringNamespace, + }, + Spec: osmv1.AlertRelabelConfigSpec{ + Configs: []osmv1.RelabelConfig{ + { + SourceLabels: []osmv1.LabelName{"alertname", "severity"}, + Regex: "TestRelabelAlert;critical", + Separator: ";", + TargetLabel: "team", + Replacement: "ops", + Action: "Replace", + }, + }, + }, + } + + _, err = f.Osmv1clientset.MonitoringV1().AlertRelabelConfigs(k8s.ClusterMonitoringNamespace).Create( + ctx, arc, metav1.CreateOptions{}, + ) + if err != nil { + t.Fatalf("Failed to create AlertRelabelConfig: %v", err) + } + defer func() { + err = f.Osmv1clientset.MonitoringV1().AlertRelabelConfigs(k8s.ClusterMonitoringNamespace).Delete(ctx, relabelConfigName, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("Failed to delete AlertRelabelConfig: %v", err) + } + }() + + err = wait.PollUntilContextTimeout(ctx, 2*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + rules, err := listRules(ctx, f) + if err != nil { + t.Logf("Failed to list rules: %v", err) + return false, nil + } + + foundCriticalWithOps := false + + for _, rule := range rules { + if rule.Name == "TestRelabelAlert" { + if rule.Labels["team"] == "ops" && rule.Labels["severity"] == "critical" { + t.Logf("Found critical alert with team=ops (relabeling successful)") + foundCriticalWithOps = true + } + } + } + + if foundCriticalWithOps { + t.Logf("Relabeling verified: critical alert has team=ops") + return true, nil + } + + t.Logf("Waiting for relabeling to take effect") + return false, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for relabeling to take effect: %v", err) + } +} + +func createPrometheusRule(ctx context.Context, f *framework.Framework, namespace string, rules ...monitoringv1.Rule) (*monitoringv1.PrometheusRule, error) { + interval := monitoringv1.Duration("30s") + prometheusRule := &monitoringv1.PrometheusRule{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-prometheus-rule", + Namespace: namespace, + }, + Spec: monitoringv1.PrometheusRuleSpec{ + Groups: []monitoringv1.RuleGroup{ + { + Name: "test-group", + Interval: &interval, + Rules: rules, + }, + }, + }, + } + + return f.Monitoringv1clientset.MonitoringV1().PrometheusRules(namespace).Create( + ctx, prometheusRule, metav1.CreateOptions{}, + ) +} + +func compareRuleLabels(t *testing.T, alertName string, foundLabels map[string]string, wantedLabels map[string]string) error { + t.Helper() + if foundLabels == nil { + t.Errorf("Alert %s has no labels", alertName) + return fmt.Errorf("alert has no labels") + } + + for key, wantValue := range wantedLabels { + if gotValue, ok := foundLabels[key]; !ok { + t.Errorf("Alert %s missing %s label", alertName, key) + return fmt.Errorf("alert missing %s label", key) + } else if gotValue != wantValue { + t.Errorf("Alert %s has wrong %s label. Expected %s, got %s", + alertName, key, wantValue, gotValue) + return fmt.Errorf("alert has wrong %s label", key) + } + } + + return nil +} + +// TestRBAC_GetRules verifies Thanos-tenancy RBAC for GET /rules. +// +// With ?namespace=: User A (no perms) gets HTTP 200 without the UWM rule in +// ns Y; User B (monitoring-rules-view in Y) sees Y but not Z; cluster-admin +// sees Y. +// +// Without ?namespace=: fan-out must not leak the rule to unprivileged users +// and must still return it for namespace-scoped viewers. +func TestRBAC_GetRules(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + + nsY, cleanupY, err := f.CreateUserNamespace(ctx, "test-rbac-get-rules-y") + if err != nil { + t.Fatalf("Failed to create namespace Y: %v", err) + } + defer func() { + if err := cleanupY(); err != nil { + t.Logf("cleanup namespace Y failed: %v", err) + } + }() + + nsZ, cleanupZ, err := f.CreateUserNamespace(ctx, "test-rbac-get-rules-z") + if err != nil { + t.Fatalf("Failed to create namespace Z: %v", err) + } + defer func() { + if err := cleanupZ(); err != nil { + t.Logf("cleanup namespace Z failed: %v", err) + } + }() + + userA, err := f.CreateAnonymousUser(ctx, "e2e-rbac-rules-a", "default") + if err != nil { + t.Fatalf("Failed to create unprivileged user A: %v", err) + } + defer func() { + if err := userA.Cleanup(); err != nil { + t.Logf("cleanup user A failed: %v", err) + } + }() + + userB, err := f.CreateUserWithClusterRole(ctx, "e2e-rbac-rules-b", nsY, "monitoring-rules-view") + if err != nil { + t.Fatalf("Failed to create scoped user B: %v", err) + } + defer func() { + if err := userB.Cleanup(); err != nil { + t.Logf("cleanup user B failed: %v", err) + } + }() + + nsYName := "E2ERBACGetRulesTestY" + nsZName := "E2ERBACGetRulesTestZ" + forDuration := monitoringv1.Duration("5m") + ruleY := monitoringv1.Rule{ + Alert: nsYName, + Expr: intstr.FromString("vector(1)"), + For: &forDuration, + Labels: map[string]string{ + "severity": "none", + "e2e_test": "rbac_get_rules", + }, + } + ruleZ := monitoringv1.Rule{ + Alert: nsZName, + Expr: intstr.FromString("vector(1)"), + For: &forDuration, + Labels: map[string]string{ + "severity": "none", + "e2e_test": "rbac_get_rules", + }, + } + + if _, err = createPrometheusRule(ctx, f, nsY, ruleY); err != nil { + t.Fatalf("Failed to create PrometheusRule in nsY: %v", err) + } + if _, err = createPrometheusRule(ctx, f, nsZ, ruleZ); err != nil { + t.Fatalf("Failed to create PrometheusRule in nsZ: %v", err) + } + + err = wait.PollUntilContextTimeout(ctx, 2*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + rulesY, status, err := listRulesWithToken(ctx, f, f.BearerToken, nsY) + if err != nil { + t.Logf("Admin GET /rules nsY failed: %v", err) + return false, nil + } + if status != http.StatusOK { + t.Logf("Admin GET /rules nsY returned status %d, retrying", status) + return false, nil + } + rulesZ, status, err := listRulesWithToken(ctx, f, f.BearerToken, nsZ) + if err != nil { + t.Logf("Admin GET /rules nsZ failed: %v", err) + return false, nil + } + if status != http.StatusOK { + t.Logf("Admin GET /rules nsZ returned status %d, retrying", status) + return false, nil + } + if containsRule(rulesY, nsYName) && containsRule(rulesZ, nsZName) { + return true, nil + } + t.Logf("Waiting for rules %s and %s", nsYName, nsZName) + return false, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for admin to see rules: %v", err) + } + + cases := []struct { + name string + token string + namespace string + ruleName string + wantRule bool + }{ + {"UserA_NoPerms_NamespaceY", userA.Token, nsY, nsYName, false}, + {"UserA_NoPerms_NamespaceZ", userA.Token, nsZ, nsZName, false}, + {"UserA_NoPerms_NoNamespace_Y", userA.Token, "", nsYName, false}, + {"UserA_NoPerms_NoNamespace_Z", userA.Token, "", nsZName, false}, + {"UserB_RulesView_NamespaceY", userB.Token, nsY, nsYName, true}, + {"UserB_RulesView_NamespaceZ", userB.Token, nsZ, nsZName, false}, + {"UserB_RulesView_NoNamespace_Y", userB.Token, "", nsYName, true}, + {"UserB_RulesView_NoNamespace_Z", userB.Token, "", nsZName, false}, + {"UserC_ClusterAdmin_NamespaceY", f.BearerToken, nsY, nsYName, true}, + {"UserC_ClusterAdmin_NamespaceZ", f.BearerToken, nsZ, nsZName, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rules, status, err := listRulesWithToken(ctx, f, tc.token, tc.namespace) + if err != nil { + t.Fatalf("GET /rules request failed: %v", err) + } + if status != http.StatusOK { + t.Fatalf("Expected status %d, got %d", http.StatusOK, status) + } + got := containsRule(rules, tc.ruleName) + if got != tc.wantRule { + t.Fatalf("Rule %s visibility: want %v, got %v (%d rules returned)", tc.ruleName, tc.wantRule, got, len(rules)) + } + }) + } +} + +func containsRule(rules []k8s.PrometheusRule, alertName string) bool { + for _, r := range rules { + if r.Name == alertName { + return true + } + } + return false +} + +// listRulesWithToken calls GET /rules with an optional namespace query param. +// A non-OK status is not an error — callers must assert on status explicitly. +func listRulesWithToken(ctx context.Context, f *framework.Framework, token, namespace string) (rules []k8s.PrometheusRule, status int, err error) { + rulesURL := f.PluginURL + "/api/v1/alerting/rules" + if namespace != "" { + rulesURL += "?" + url.Values{"namespace": {namespace}}.Encode() + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rulesURL, nil) + if err != nil { + return nil, 0, err + } + req.Header.Set("Authorization", "Bearer "+token) + + resp, err := f.HTTPClient().Do(req) + if err != nil { + return nil, 0, err + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil && err == nil { + err = fmt.Errorf("closing response body: %w", closeErr) + } + }() + + if resp.StatusCode != http.StatusOK { + return nil, resp.StatusCode, nil + } + + var listResp listRulesResponse + if decodeErr := json.NewDecoder(resp.Body).Decode(&listResp); decodeErr != nil { + return nil, resp.StatusCode, decodeErr + } + + for _, group := range listResp.Data.Groups { + rules = append(rules, group.Rules...) + } + return rules, resp.StatusCode, nil +} From ded4dcce47b0515ec4519d3f217b42e7fb57c95c Mon Sep 17 00:00:00 2001 From: Shirly Radco Date: Thu, 12 Mar 2026 20:34:26 +0200 Subject: [PATCH 2/4] router: add GET /health endpoint and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add GET /api/v1/alerting/health endpoint with handler tests. Signed-off-by: Shirly Radco Signed-off-by: João Vilaça Signed-off-by: Aviv Litman Co-authored-by: AI Assistant --- internal/managementrouter/health_get.go | 33 +++++++ internal/managementrouter/health_get_test.go | 93 ++++++++++++++++++++ internal/managementrouter/router.go | 6 +- pkg/management/get_alerting_health_test.go | 53 +++++++++++ test/e2e/health_test.go | 63 +++++++++++++ 5 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 internal/managementrouter/health_get.go create mode 100644 internal/managementrouter/health_get_test.go create mode 100644 pkg/management/get_alerting_health_test.go create mode 100644 test/e2e/health_test.go diff --git a/internal/managementrouter/health_get.go b/internal/managementrouter/health_get.go new file mode 100644 index 000000000..5eb698e0b --- /dev/null +++ b/internal/managementrouter/health_get.go @@ -0,0 +1,33 @@ +package managementrouter + +import ( + "encoding/json" + "net/http" + + "github.com/openshift/monitoring-plugin/pkg/k8s" +) + +type GetHealthResponse struct { + Alerting *k8s.AlertingHealth `json:"alerting,omitempty"` +} + +// GetHealth serves GET /api/v1/alerting/health. +func (hr *httpRouter) GetHealth(w http.ResponseWriter, req *http.Request) { + resp := GetHealthResponse{} + + if hr.managementClient != nil { + health, err := hr.managementClient.GetAlertingHealth(req.Context()) + if err != nil { + handleError(w, err) + return + } + resp.Alerting = &health + } + + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "no-store") + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(resp); err != nil { + log.WithError(err).Warn("failed to encode health response") + } +} diff --git a/internal/managementrouter/health_get_test.go b/internal/managementrouter/health_get_test.go new file mode 100644 index 000000000..8a0d8d265 --- /dev/null +++ b/internal/managementrouter/health_get_test.go @@ -0,0 +1,93 @@ +package managementrouter_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/openshift/monitoring-plugin/internal/managementrouter" + "github.com/openshift/monitoring-plugin/pkg/k8s" +) + +func sampleAlertingHealth() k8s.AlertingHealth { + return k8s.AlertingHealth{ + Platform: &k8s.AlertingStackHealth{ + Prometheus: k8s.AlertingRouteHealth{Name: "prometheus-k8s", Namespace: "openshift-monitoring", Status: k8s.RouteReachable}, + Alertmanager: k8s.AlertingRouteHealth{Name: "alertmanager-main", Namespace: "openshift-monitoring", Status: k8s.RouteReachable}, + }, + UserWorkloadEnabled: true, + UserWorkload: &k8s.AlertingStackHealth{ + Prometheus: k8s.AlertingRouteHealth{Name: "prometheus-user-workload", Namespace: "openshift-user-workload-monitoring", Status: k8s.RouteReachable}, + Alertmanager: k8s.AlertingRouteHealth{Name: "alertmanager-user-workload", Namespace: "openshift-user-workload-monitoring", Status: k8s.RouteReachable}, + }, + } +} + +func TestGetHealth_Returns200(t *testing.T) { + f := newAGFixture(t) + f.mockK8s.AlertingHealthFunc = func(_ context.Context) (k8s.AlertingHealth, error) { + return sampleAlertingHealth(), nil + } + + w := f.get(t, "/api/v1/alerting/health") + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body) + } + if ct := w.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("expected Content-Type application/json, got %q", ct) + } +} + +func TestGetHealth_ReturnsAlertingStructure(t *testing.T) { + f := newAGFixture(t) + f.mockK8s.AlertingHealthFunc = func(_ context.Context) (k8s.AlertingHealth, error) { + return sampleAlertingHealth(), nil + } + + w := f.get(t, "/api/v1/alerting/health") + var response managementrouter.GetHealthResponse + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("decode error: %v", err) + } + if response.Alerting == nil { + t.Fatal("expected non-nil Alerting in response") + } + if response.Alerting.Platform == nil || response.Alerting.Platform.Prometheus.Name != "prometheus-k8s" { + t.Errorf("expected platform prometheus-k8s, got %+v", response.Alerting.Platform) + } + if !response.Alerting.UserWorkloadEnabled { + t.Error("expected UserWorkloadEnabled=true") + } + if response.Alerting.UserWorkload == nil || response.Alerting.UserWorkload.Prometheus.Name != "prometheus-user-workload" { + t.Errorf("expected user workload prometheus-user-workload, got %+v", response.Alerting.UserWorkload) + } +} + +func TestGetHealth_Returns500OnError(t *testing.T) { + f := newAGFixture(t) + f.mockK8s.AlertingHealthFunc = func(_ context.Context) (k8s.AlertingHealth, error) { + return k8s.AlertingHealth{}, fmt.Errorf("connection refused") + } + + w := f.get(t, "/api/v1/alerting/health") + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body) + } + if body := w.Body.String(); !strings.Contains(body, "An unexpected error occurred") { + t.Errorf("expected error message, got: %s", body) + } +} + +func TestGetHealth_MissingAuthHeaderReturns401(t *testing.T) { + f := newAGFixture(t) + req := httptest.NewRequest(http.MethodGet, "/api/v1/alerting/health", nil) + w := httptest.NewRecorder() + f.router.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d: %s", w.Code, w.Body) + } +} diff --git a/internal/managementrouter/router.go b/internal/managementrouter/router.go index f0ac5cfb8..915b62f8c 100644 --- a/internal/managementrouter/router.go +++ b/internal/managementrouter/router.go @@ -43,10 +43,12 @@ func New(managementClient management.Client) *mux.Router { BaseURL: "/api/v1/alerting", BaseRouter: r, }) - // GET /alerts and GET /rules are not yet in the OpenAPI spec; registered - // manually until their respective branches add the spec entries. + // GET /alerts, GET /rules, and GET /health are not yet in the OpenAPI + // spec; registered manually until their respective branches add the spec + // entries. r.HandleFunc("/api/v1/alerting/alerts", hr.GetAlerts).Methods(http.MethodGet) r.HandleFunc("/api/v1/alerting/rules", hr.GetRules).Methods(http.MethodGet) + r.HandleFunc("/api/v1/alerting/health", hr.GetHealth).Methods(http.MethodGet) return r } diff --git a/pkg/management/get_alerting_health_test.go b/pkg/management/get_alerting_health_test.go new file mode 100644 index 000000000..ec104abbc --- /dev/null +++ b/pkg/management/get_alerting_health_test.go @@ -0,0 +1,53 @@ +package management_test + +import ( + "context" + "testing" + "time" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/pkg/management" + "github.com/openshift/monitoring-plugin/pkg/management/testutils" +) + +func TestGetAlertingHealth_SetsDeadlineWhenCallerHasNone(t *testing.T) { + var hasDeadline bool + mockK8s := &testutils.MockClient{ + AlertingHealthFunc: func(ctx context.Context) (k8s.AlertingHealth, error) { + _, hasDeadline = ctx.Deadline() + return k8s.AlertingHealth{}, nil + }, + } + client := management.New(context.Background(), mockK8s) + if _, err := client.GetAlertingHealth(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !hasDeadline { + t.Fatal("expected GetAlertingHealth to set a deadline when the caller did not") + } +} + +func TestGetAlertingHealth_PreservesCallerDeadline(t *testing.T) { + callerDeadline := time.Now().Add(2 * time.Second) + ctx, cancel := context.WithDeadline(context.Background(), callerDeadline) + defer cancel() + + var gotDeadline time.Time + var sawDeadline bool + mockK8s := &testutils.MockClient{ + AlertingHealthFunc: func(ctx context.Context) (k8s.AlertingHealth, error) { + gotDeadline, sawDeadline = ctx.Deadline() + return k8s.AlertingHealth{}, nil + }, + } + client := management.New(context.Background(), mockK8s) + if _, err := client.GetAlertingHealth(ctx); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !sawDeadline { + t.Fatal("expected the caller's deadline to be forwarded") + } + if !gotDeadline.Equal(callerDeadline) { + t.Errorf("expected caller deadline %v, got %v", callerDeadline, gotDeadline) + } +} diff --git a/test/e2e/health_test.go b/test/e2e/health_test.go new file mode 100644 index 000000000..955b49ec1 --- /dev/null +++ b/test/e2e/health_test.go @@ -0,0 +1,63 @@ +//go:build e2e + +package e2e + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/test/e2e/framework" +) + +func TestGetHealth(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + + healthURL := f.PluginURL + "/api/v1/alerting/health" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, healthURL, nil) + if err != nil { + t.Fatalf("Failed to create HTTP request: %v", err) + } + if f.BearerToken != "" { + req.Header.Set("Authorization", "Bearer "+f.BearerToken) + } + + resp, err := f.HTTPClient().Do(req) + if err != nil { + t.Fatalf("Failed to make health request: %v", err) + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + t.Logf("closing response body: %v", closeErr) + } + }() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("Expected status 200, got %d", resp.StatusCode) + } + + var healthResp struct { + Alerting *k8s.AlertingHealth `json:"alerting"` + } + if err := json.NewDecoder(resp.Body).Decode(&healthResp); err != nil { + t.Fatalf("Failed to decode health response: %v", err) + } + + if healthResp.Alerting == nil { + t.Fatal("Expected 'alerting' field in health response") + } + + if healthResp.Alerting.Platform == nil { + t.Error("Expected 'platform' field in alerting health") + } + + t.Logf("Health response: userWorkloadEnabled=%v", healthResp.Alerting.UserWorkloadEnabled) + t.Log("GET /health e2e test passed successfully") +} From 63a08fb2dcc97989fe722f358c91bd5b5e6238b8 Mon Sep 17 00:00:00 2001 From: Shirly Radco Date: Thu, 12 Mar 2026 20:42:50 +0200 Subject: [PATCH 3/4] k8s: add orphan AlertRelabelConfig GC Detect and remove orphan AlertRelabelConfig resources that no longer have a matching PrometheusRule, preventing stale relabel configs from accumulating. Signed-off-by: Shirly Radco Co-authored-by: AI Assistant --- pkg/k8s/alert_relabel_config_gc.go | 52 ++++++++ pkg/k8s/alert_relabel_config_gc_test.go | 168 ++++++++++++++++++++++++ pkg/k8s/relabeled_rules.go | 19 ++- 3 files changed, 233 insertions(+), 6 deletions(-) create mode 100644 pkg/k8s/alert_relabel_config_gc.go create mode 100644 pkg/k8s/alert_relabel_config_gc_test.go diff --git a/pkg/k8s/alert_relabel_config_gc.go b/pkg/k8s/alert_relabel_config_gc.go new file mode 100644 index 000000000..7c9e92a2e --- /dev/null +++ b/pkg/k8s/alert_relabel_config_gc.go @@ -0,0 +1,52 @@ +package k8s + +import ( + "context" + + "github.com/openshift/monitoring-plugin/pkg/managementlabels" +) + +// gcOrphanedARCs deletes AlertRelabelConfigs whose associated alert rule no +// longer exists. This handles the case where an operator (or manual action) +// removes rules from a PrometheusRule or deletes the CR entirely — the ARCs +// that were created by the plugin for classification/drop/stamp become orphans. +// +// Only ARCs carrying the plugin's alertRuleId annotation are considered. +// GitOps-managed ARCs are never deleted automatically; a warning is logged +// so that operators can clean them up manually. +func (rrm *relabeledRulesManager) gcOrphanedARCs(ctx context.Context, liveRuleIDs map[string]struct{}) { + if rrm.alertRelabelConfigs == nil { + return + } + + arcs, err := rrm.alertRelabelConfigs.List(ctx, "") + if err != nil { + log.Errorf("orphan ARC GC: failed to list ARCs: %v", err) + return + } + + for i := range arcs { + arc := &arcs[i] + + ruleID, ok := arc.Annotations[managementlabels.ARCAnnotationAlertRuleIDKey] + if !ok || ruleID == "" { + continue + } + + if _, alive := liveRuleIDs[ruleID]; alive { + continue + } + + if IsManagedByGitOps(arc.Annotations, arc.Labels) { + log.Warnf("orphan ARC GC: ARC %s/%s (ruleId=%s) is orphaned but GitOps-managed — skipping deletion, manual cleanup required", arc.Namespace, arc.Name, ruleID) + continue + } + + if err := rrm.alertRelabelConfigs.Delete(ctx, arc.Namespace, arc.Name); err != nil { + log.Errorf("orphan ARC GC: failed to delete ARC %s/%s: %v", arc.Namespace, arc.Name, err) + continue + } + + log.Infof("orphan ARC GC: deleted orphaned ARC %s/%s (ruleId=%s)", arc.Namespace, arc.Name, ruleID) + } +} diff --git a/pkg/k8s/alert_relabel_config_gc_test.go b/pkg/k8s/alert_relabel_config_gc_test.go new file mode 100644 index 000000000..e139ad965 --- /dev/null +++ b/pkg/k8s/alert_relabel_config_gc_test.go @@ -0,0 +1,168 @@ +package k8s + +import ( + "context" + "testing" + + osmv1 "github.com/openshift/api/monitoring/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/monitoring-plugin/pkg/managementlabels" +) + +type mockARCInterface struct { + arcs map[string]*osmv1.AlertRelabelConfig + deleted []string +} + +func (m *mockARCInterface) List(_ context.Context, _ string) ([]osmv1.AlertRelabelConfig, error) { + var result []osmv1.AlertRelabelConfig + for _, arc := range m.arcs { + result = append(result, *arc) + } + return result, nil +} + +func (m *mockARCInterface) Get(_ context.Context, ns, name string) (*osmv1.AlertRelabelConfig, bool, error) { + if arc, ok := m.arcs[ns+"/"+name]; ok { + return arc, true, nil + } + return nil, false, nil +} + +func (m *mockARCInterface) Create(_ context.Context, arc osmv1.AlertRelabelConfig) (*osmv1.AlertRelabelConfig, error) { + return &arc, nil +} + +func (m *mockARCInterface) Update(_ context.Context, _ osmv1.AlertRelabelConfig) error { return nil } + +func (m *mockARCInterface) Delete(_ context.Context, ns, name string) error { + m.deleted = append(m.deleted, ns+"/"+name) + delete(m.arcs, ns+"/"+name) + return nil +} + +func newARC(ns, name, ruleID string, annotations, labels map[string]string) *osmv1.AlertRelabelConfig { + if annotations == nil { + annotations = map[string]string{} + } + if ruleID != "" { + annotations[managementlabels.ARCAnnotationAlertRuleIDKey] = ruleID + } + return &osmv1.AlertRelabelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + Annotations: annotations, + Labels: labels, + }, + } +} + +func TestGCOrphanedARCs_DeletesOrphan(t *testing.T) { + mock := &mockARCInterface{ + arcs: map[string]*osmv1.AlertRelabelConfig{ + "openshift-monitoring/arc-orphan": newARC("openshift-monitoring", "arc-orphan", "rule-gone", nil, nil), + }, + } + rrm := &relabeledRulesManager{alertRelabelConfigs: mock} + + rrm.gcOrphanedARCs(context.Background(), map[string]struct{}{}) + + if len(mock.deleted) != 1 || mock.deleted[0] != "openshift-monitoring/arc-orphan" { + t.Fatalf("expected orphan ARC to be deleted, got deleted=%v", mock.deleted) + } +} + +func TestGCOrphanedARCs_KeepsLiveRule(t *testing.T) { + mock := &mockARCInterface{ + arcs: map[string]*osmv1.AlertRelabelConfig{ + "openshift-monitoring/arc-live": newARC("openshift-monitoring", "arc-live", "rule-alive", nil, nil), + }, + } + rrm := &relabeledRulesManager{alertRelabelConfigs: mock} + + rrm.gcOrphanedARCs(context.Background(), map[string]struct{}{"rule-alive": {}}) + + if len(mock.deleted) != 0 { + t.Fatalf("expected no deletions, got deleted=%v", mock.deleted) + } +} + +func TestGCOrphanedARCs_SkipsGitOpsManaged(t *testing.T) { + mock := &mockARCInterface{ + arcs: map[string]*osmv1.AlertRelabelConfig{ + "openshift-monitoring/arc-gitops": newARC("openshift-monitoring", "arc-gitops", "rule-gone", + map[string]string{"argocd.argoproj.io/tracking-id": "some-id"}, nil), + }, + } + rrm := &relabeledRulesManager{alertRelabelConfigs: mock} + + rrm.gcOrphanedARCs(context.Background(), map[string]struct{}{}) + + if len(mock.deleted) != 0 { + t.Fatalf("expected GitOps-managed ARC to be preserved, got deleted=%v", mock.deleted) + } +} + +func TestGCOrphanedARCs_SkipsARCWithoutAnnotation(t *testing.T) { + mock := &mockARCInterface{ + arcs: map[string]*osmv1.AlertRelabelConfig{ + "openshift-monitoring/arc-manual": newARC("openshift-monitoring", "arc-manual", "", nil, nil), + }, + } + rrm := &relabeledRulesManager{alertRelabelConfigs: mock} + + rrm.gcOrphanedARCs(context.Background(), map[string]struct{}{}) + + if len(mock.deleted) != 0 { + t.Fatalf("expected ARC without annotation to be preserved, got deleted=%v", mock.deleted) + } +} + +func TestGCOrphanedARCs_MixedScenario(t *testing.T) { + mock := &mockARCInterface{ + arcs: map[string]*osmv1.AlertRelabelConfig{ + "openshift-monitoring/arc-live": newARC("openshift-monitoring", "arc-live", "rule-1", nil, nil), + "openshift-monitoring/arc-orphan1": newARC("openshift-monitoring", "arc-orphan1", "rule-deleted-1", nil, nil), + "openshift-monitoring/arc-orphan2": newARC("openshift-monitoring", "arc-orphan2", "rule-deleted-2", nil, nil), + "openshift-monitoring/arc-gitops": newARC("openshift-monitoring", "arc-gitops", "rule-deleted-3", + map[string]string{"argocd.argoproj.io/tracking-id": "t"}, nil), + "openshift-monitoring/arc-manual": newARC("openshift-monitoring", "arc-manual", "", nil, nil), + }, + } + rrm := &relabeledRulesManager{alertRelabelConfigs: mock} + + liveIDs := map[string]struct{}{"rule-1": {}} + rrm.gcOrphanedARCs(context.Background(), liveIDs) + + deletedSet := map[string]bool{} + for _, d := range mock.deleted { + deletedSet[d] = true + } + + if len(mock.deleted) != 2 { + t.Fatalf("expected 2 deletions, got %d: %v", len(mock.deleted), mock.deleted) + } + if !deletedSet["openshift-monitoring/arc-orphan1"] { + t.Error("expected arc-orphan1 to be deleted") + } + if !deletedSet["openshift-monitoring/arc-orphan2"] { + t.Error("expected arc-orphan2 to be deleted") + } + if deletedSet["openshift-monitoring/arc-live"] { + t.Error("arc-live should not have been deleted") + } + if deletedSet["openshift-monitoring/arc-gitops"] { + t.Error("arc-gitops should not have been deleted (GitOps-managed)") + } + if deletedSet["openshift-monitoring/arc-manual"] { + t.Error("arc-manual should not have been deleted (no annotation)") + } +} + +func TestGCOrphanedARCs_NilInterface(t *testing.T) { + rrm := &relabeledRulesManager{alertRelabelConfigs: nil} + // Should not panic + rrm.gcOrphanedARCs(context.Background(), map[string]struct{}{}) +} diff --git a/pkg/k8s/relabeled_rules.go b/pkg/k8s/relabeled_rules.go index 02452c385..19f5acbd2 100644 --- a/pkg/k8s/relabeled_rules.go +++ b/pkg/k8s/relabeled_rules.go @@ -149,7 +149,7 @@ func newRelabeledRulesManager(ctx context.Context, namespaceManager NamespaceInt return nil, fmt.Errorf("failed to sync RelabeledRulesConfig informer") } - if err := rrm.sync(ctx); err != nil { + if err := rrm.sync(ctx, "initial-sync"); err != nil { return nil, fmt.Errorf("initial relabeled rules sync failed: %w", err) } @@ -180,7 +180,7 @@ func (rrm *relabeledRulesManager) processNextWorkItem(ctx context.Context) bool defer rrm.queue.Done(key) - if err := rrm.sync(ctx); err != nil { + if err := rrm.sync(ctx, key); err != nil { log.Errorf("error syncing relabeled rules: %v", err) rrm.queue.AddRateLimited(key) return true @@ -191,7 +191,7 @@ func (rrm *relabeledRulesManager) processNextWorkItem(ctx context.Context) bool return true } -func (rrm *relabeledRulesManager) sync(ctx context.Context) error { +func (rrm *relabeledRulesManager) sync(ctx context.Context, key string) error { relabelConfigs, err := rrm.loadRelabelConfigs() if err != nil { return fmt.Errorf("failed to load relabel configs: %w", err) @@ -201,13 +201,20 @@ func (rrm *relabeledRulesManager) sync(ctx context.Context) error { rrm.relabelConfigs = relabelConfigs rrm.mu.Unlock() - alerts := rrm.collectAlerts(ctx, relabelConfigs) + alerts, allRuleIDs := rrm.collectAlerts(ctx, relabelConfigs) rrm.mu.Lock() rrm.relabeledRules = alerts rrm.mu.Unlock() log.Infof("Synced %d relabeled rules in memory", len(alerts)) + + // GC orphaned ARCs only when triggered by PrometheusRule events or + // initial sync — secret-only changes cannot create orphans. + if key == "prometheus-rule-sync" || key == "initial-sync" { + rrm.gcOrphanedARCs(ctx, allRuleIDs) + } + return nil } @@ -256,7 +263,7 @@ func (rrm *relabeledRulesManager) loadRelabelConfigs() ([]*relabel.Config, error return configs, nil } -func (rrm *relabeledRulesManager) collectAlerts(ctx context.Context, relabelConfigs []*relabel.Config) map[string]monitoringv1.Rule { +func (rrm *relabeledRulesManager) collectAlerts(ctx context.Context, relabelConfigs []*relabel.Config) (map[string]monitoringv1.Rule, map[string]struct{}) { alerts := make(map[string]monitoringv1.Rule) seenIDs := make(map[string]struct{}) @@ -336,7 +343,7 @@ func (rrm *relabeledRulesManager) collectAlerts(ctx context.Context, relabelConf } log.Debugf("Collected %d alerts", len(alerts)) - return alerts + return alerts, seenIDs } // alertingRuleOwner returns the name of the AlertingRule CR that generated From 6018556e010e9b51a891f539561c60b35a1b1109 Mon Sep 17 00:00:00 2001 From: Shirly Radco Date: Thu, 12 Mar 2026 20:34:33 +0200 Subject: [PATCH 4/4] add alerts_effective_active_at metric Expose a Prometheus gauge metric whose value is the activeAt Unix timestamp for every effective alert (firing, pending, silenced). Labels include all alerts labels after relabeling plus enrichment labels and alertstate. Annotations are excluded since they are available from the alert rule definition. Signed-off-by: Shirly Radco Co-authored-by: AI Assistant --- go.mod | 4 +- pkg/k8s/enrich_active_at_test.go | 106 ++++++ pkg/k8s/prometheus_alerts.go | 72 +++- pkg/management/management.go | 9 + pkg/management/metrics/alerts_collector.go | 223 +++++++++++ .../metrics/alerts_collector_test.go | 354 ++++++++++++++++++ pkg/management/metrics/leader_election.go | 87 +++++ pkg/management/types.go | 6 + pkg/server/server.go | 16 +- test/e2e/alerts_effective_metric_test.go | 319 ++++++++++++++++ 10 files changed, 1188 insertions(+), 8 deletions(-) create mode 100644 pkg/k8s/enrich_active_at_test.go create mode 100644 pkg/management/metrics/alerts_collector.go create mode 100644 pkg/management/metrics/alerts_collector_test.go create mode 100644 pkg/management/metrics/leader_election.go create mode 100644 test/e2e/alerts_effective_metric_test.go diff --git a/go.mod b/go.mod index bcc7afc8a..e57ab90e6 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,8 @@ require ( github.com/openshift/library-go v0.0.0-20240905123346-5bdbfe35a6f5 github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.87.0 github.com/prometheus-operator/prometheus-operator/pkg/client v0.87.0 + github.com/prometheus/client_golang v1.23.2 + github.com/prometheus/client_model v0.6.2 github.com/prometheus/common v0.67.4 github.com/prometheus/prometheus v0.308.0 github.com/sirupsen/logrus v1.9.3 @@ -57,8 +59,6 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/prometheus/client_golang v1.23.2 // indirect - github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/procfs v0.16.1 // indirect github.com/spf13/pflag v1.0.6 // indirect github.com/x448/float16 v0.8.4 // indirect diff --git a/pkg/k8s/enrich_active_at_test.go b/pkg/k8s/enrich_active_at_test.go new file mode 100644 index 000000000..95e205591 --- /dev/null +++ b/pkg/k8s/enrich_active_at_test.go @@ -0,0 +1,106 @@ +package k8s + +import ( + "testing" + "time" +) + +func TestEnrichActiveAt_ReplacesAlertmanagerTimestamp(t *testing.T) { + amTime := time.Date(2026, 3, 10, 12, 0, 0, 0, time.UTC) + promTime := time.Date(2026, 3, 9, 8, 0, 0, 0, time.UTC) + + amAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "HighCPU", "severity": "critical", AlertSourceLabel: "platform", AlertBackendLabel: "am"}, + ActiveAt: amTime, + }} + promAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "HighCPU", "severity": "critical", AlertSourceLabel: "platform", AlertBackendLabel: "prom"}, + ActiveAt: promTime, + }} + + enrichActiveAt(amAlerts, promAlerts) + + if !amAlerts[0].ActiveAt.Equal(promTime) { + t.Errorf("expected ActiveAt=%v, got %v", promTime, amAlerts[0].ActiveAt) + } +} + +func TestEnrichActiveAt_NoMatchKeepsOriginal(t *testing.T) { + amTime := time.Date(2026, 3, 10, 12, 0, 0, 0, time.UTC) + + amAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "HighCPU", "severity": "critical"}, + ActiveAt: amTime, + }} + promAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "DiskFull", "severity": "warning"}, + ActiveAt: time.Date(2026, 3, 9, 8, 0, 0, 0, time.UTC), + }} + + enrichActiveAt(amAlerts, promAlerts) + + if !amAlerts[0].ActiveAt.Equal(amTime) { + t.Errorf("expected ActiveAt to stay %v, got %v", amTime, amAlerts[0].ActiveAt) + } +} + +func TestEnrichActiveAt_EmptyPromAlerts(t *testing.T) { + amTime := time.Date(2026, 3, 10, 12, 0, 0, 0, time.UTC) + + amAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "HighCPU"}, + ActiveAt: amTime, + }} + + enrichActiveAt(amAlerts, nil) + + if !amAlerts[0].ActiveAt.Equal(amTime) { + t.Errorf("expected ActiveAt to stay %v, got %v", amTime, amAlerts[0].ActiveAt) + } +} + +func TestEnrichActiveAt_SkipsZeroPromActiveAt(t *testing.T) { + amTime := time.Date(2026, 3, 10, 12, 0, 0, 0, time.UTC) + + amAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "HighCPU"}, + ActiveAt: amTime, + }} + promAlerts := []PrometheusAlert{{ + Labels: map[string]string{"alertname": "HighCPU"}, + }} + + enrichActiveAt(amAlerts, promAlerts) + + if !amAlerts[0].ActiveAt.Equal(amTime) { + t.Errorf("expected ActiveAt to stay %v when prom has zero time, got %v", amTime, amAlerts[0].ActiveAt) + } +} + +func TestAlertFingerprint_IgnoresMetadataLabels(t *testing.T) { + fp1 := alertFingerprint(map[string]string{ + "alertname": "HighCPU", + "severity": "critical", + AlertSourceLabel: "platform", + AlertBackendLabel: "am", + }) + fp2 := alertFingerprint(map[string]string{ + "alertname": "HighCPU", + "severity": "critical", + AlertSourceLabel: "platform", + AlertBackendLabel: "prom", + }) + + if fp1 != fp2 { + t.Errorf("fingerprints should match when only metadata labels differ:\n fp1=%q\n fp2=%q", fp1, fp2) + } +} + +func TestAlertFingerprint_DifferentLabelsProduceDifferentKeys(t *testing.T) { + fp1 := alertFingerprint(map[string]string{"alertname": "HighCPU", "severity": "critical"}) + fp2 := alertFingerprint(map[string]string{"alertname": "HighCPU", "severity": "warning"}) + + if fp1 == fp2 { + t.Error("fingerprints should differ when label values differ") + } +} diff --git a/pkg/k8s/prometheus_alerts.go b/pkg/k8s/prometheus_alerts.go index 4a39b1a58..1de7a1ec5 100644 --- a/pkg/k8s/prometheus_alerts.go +++ b/pkg/k8s/prometheus_alerts.go @@ -10,6 +10,7 @@ import ( "net/http" "net/url" "os" + "sort" "strings" "sync" "time" @@ -293,11 +294,21 @@ func (pa *prometheusAlerts) routeHealth(ctx context.Context, namespace string, r return health } +// getAlertsForSource fetches alerts from both Alertmanager and Prometheus in +// parallel and merges the results. The fallback strategy is: +// - Both succeed: AM (firing+silenced) + Prom pending, with AM timestamps +// enriched from Prometheus activeAt. +// - AM only: AM alerts returned as-is (no Prom data to enrich from). +// - Prom only: all Prom alerts returned (AM was unreachable). +// - Both fail: error propagated from Prometheus. func (pa *prometheusAlerts) getAlertsForSource(ctx context.Context, namespace string, promRouteName string, amRouteName string, source string) ([]PrometheusAlert, error) { amAlerts, amErr := pa.getAlertmanagerAlerts(ctx, namespace, amRouteName, source) promAlerts, promErr := pa.getAlertsViaProxy(ctx, namespace, promRouteName, source) if amErr == nil { + if promErr == nil { + enrichActiveAt(amAlerts, promAlerts) + } pending := filterAlertsByState(promAlerts, "pending") return append(amAlerts, pending...), nil } @@ -352,15 +363,17 @@ func (pa *prometheusAlerts) getUserWorkloadAlertsViaAlertmanager(ctx context.Con } } - pending, err := pa.getAlertsViaProxy(ctx, UserWorkloadMonitoringNamespace, UserWorkloadRouteName, AlertSourceUser) + promAlerts, err := pa.getAlertsViaProxy(ctx, UserWorkloadMonitoringNamespace, UserWorkloadRouteName, AlertSourceUser) if err != nil { - pending, err = pa.getPrometheusAlertsViaService(ctx, UserWorkloadMonitoringNamespace, UserWorkloadPrometheusServiceName, UserWorkloadPrometheusPort, AlertSourceUser) + promAlerts, err = pa.getPrometheusAlertsViaService(ctx, UserWorkloadMonitoringNamespace, UserWorkloadPrometheusServiceName, UserWorkloadPrometheusPort, AlertSourceUser) if err != nil { return alerts, nil } } - return append(alerts, filterAlertsByState(pending, "pending")...), nil + // Enrich before filtering: AM alerts need activeAt from all Prom states. + enrichActiveAt(alerts, promAlerts) + return append(alerts, filterAlertsByState(promAlerts, "pending")...), nil } func (pa *prometheusAlerts) getPrometheusAlertsViaService(ctx context.Context, namespace string, serviceName string, port int32, source string) ([]PrometheusAlert, error) { @@ -790,6 +803,59 @@ func filterAlertsByState(alerts []PrometheusAlert, state string) []PrometheusAle return out } +// enrichActiveAt replaces ActiveAt in Alertmanager-sourced alerts with the +// authoritative value from Prometheus. Alertmanager only exposes startsAt +// (when it received the alert), while Prometheus tracks the true activeAt +// (when the alert condition first became true). +func enrichActiveAt(amAlerts, promAlerts []PrometheusAlert) { + if len(promAlerts) == 0 { + return + } + + lookup := make(map[string]time.Time, len(promAlerts)) + for _, alert := range promAlerts { + fp := alertFingerprint(alert.Labels) + if !alert.ActiveAt.IsZero() { + lookup[fp] = alert.ActiveAt + } + } + + for i := range amAlerts { + fp := alertFingerprint(amAlerts[i].Labels) + if activeAt, ok := lookup[fp]; ok { + amAlerts[i].ActiveAt = activeAt + } + } +} + +// alertFingerprint builds a stable identity key from an alert's labels, +// excluding metadata labels injected by this plugin (source, backend). +// This matches the same alert *instance* across Alertmanager and Prometheus +// (which may differ only in injected metadata). It is distinct from the +// alert rule ID (GetAlertingRuleId) which identifies the *rule definition* +// and is computed from the rule spec (name, expr, duration, static labels). +func alertFingerprint(labels map[string]string) string { + keys := make([]string, 0, len(labels)) + for k := range labels { + if k == AlertSourceLabel || k == AlertBackendLabel { + continue + } + keys = append(keys, k) + } + sort.Strings(keys) + + var b strings.Builder + for i, k := range keys { + if i > 0 { + b.WriteByte('\xff') + } + b.WriteString(k) + b.WriteByte('\xfe') + b.WriteString(labels[k]) + } + return b.String() +} + func mapAlertmanagerState(state string) string { if state == "active" { return "firing" diff --git a/pkg/management/management.go b/pkg/management/management.go index 652ac14de..124a98785 100644 --- a/pkg/management/management.go +++ b/pkg/management/management.go @@ -1,9 +1,14 @@ package management import ( + "context" + "net/http" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/pkg/management/metrics" ) type client struct { @@ -20,3 +25,7 @@ type client struct { func (c *client) isPlatformManagedPrometheusRule(nn types.NamespacedName) bool { return c.k8sClient.Namespace().IsClusterMonitoringNamespace(nn.Namespace) } + +func (c *client) MetricsHandler(ctx context.Context, kubeConfig *rest.Config) (http.Handler, error) { + return metrics.NewHandler(ctx, c, kubeConfig) +} diff --git a/pkg/management/metrics/alerts_collector.go b/pkg/management/metrics/alerts_collector.go new file mode 100644 index 000000000..1c056aa98 --- /dev/null +++ b/pkg/management/metrics/alerts_collector.go @@ -0,0 +1,223 @@ +package metrics + +import ( + "context" + "fmt" + "net/http" + "sort" + "sync" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/sirupsen/logrus" + "k8s.io/client-go/rest" + + "github.com/openshift/monitoring-plugin/pkg/k8s" +) + +var metricsLog = logrus.WithField("module", "metrics") + +const ( + MetricName = "alerts_effective_active_at_timestamp_seconds" + metricHelp = "The activeAt timestamp of effective (post-ARC) alerts. " + + "Value is the Unix timestamp when the alert became active." + + DefaultSyncInterval = 30 * time.Second + + labelAlertState = "alertstate" +) + +// AlertsFetcher retrieves enriched alerts for the metric. The management.Client +// satisfies this interface — it applies ARC relabeling and computes +// classification (AlertComponent / AlertLayer) on every alert. +type AlertsFetcher interface { + EnrichAlerts(ctx context.Context, req k8s.GetAlertsRequest) ([]k8s.PrometheusAlert, []string, error) +} + +// alertMetric holds a single alert's pre-built metric data. +// The prometheus.Desc is created once during sync, not on every scrape. +type alertMetric struct { + desc *prometheus.Desc + labelValues []string + activeAtSec float64 +} + +// AlertsCollector implements prometheus.Collector. It periodically fetches +// alerts via the management client's EnrichAlerts (which applies ARC relabeling +// and computes classification) and exposes them as the +// alerts_effective_active_at_timestamp_seconds gauge. +// +// Only the leader pod (determined via Lease-based leader election) runs the +// sync loop and exposes metrics. Follower pods return nothing on Collect, +// ensuring each alert appears exactly once in Prometheus. +// +// Each alert produces one time series whose value is the alert's activeAt +// Unix timestamp. Labels are the alert's enriched labels (post-ARC, source, +// backend, component, layer) plus "alertstate". Thanos-sourced alerts are +// filtered out to avoid duplicates. Annotations are excluded because they +// are available from the alert rule definition. +type AlertsCollector struct { + fetcher AlertsFetcher + syncInterval time.Duration + isLeader func() bool + + mu sync.RWMutex + metrics []alertMetric + + sentinelDesc *prometheus.Desc +} + +// NewHandler creates a metrics HTTP handler that exposes the alerts effective +// metric. It sets up Lease-based leader election internally so that only one +// replica produces metrics, then wires the collector, registry and promhttp +// handler. Callers receive a ready-to-use http.Handler. +func NewHandler(ctx context.Context, fetcher AlertsFetcher, kubeConfig *rest.Config) (http.Handler, error) { + isLeader, err := startLeaderElection(ctx, kubeConfig, k8s.ClusterMonitoringNamespace) + if err != nil { + return nil, fmt.Errorf("start metrics leader election: %w", err) + } + + collector := NewAlertsCollector(ctx, fetcher, DefaultSyncInterval, isLeader) + registry := prometheus.NewRegistry() + registry.MustRegister(collector) + return promhttp.HandlerFor(registry, promhttp.HandlerOpts{}), nil +} + +// NewAlertsCollector creates a collector that periodically syncs alerts and +// exposes them as Prometheus metrics. The isLeader callback controls whether +// this replica actively syncs and exposes metrics (follower pods return nothing). +func NewAlertsCollector(ctx context.Context, fetcher AlertsFetcher, syncInterval time.Duration, isLeader func() bool) *AlertsCollector { + c := &AlertsCollector{ + fetcher: fetcher, + syncInterval: syncInterval, + isLeader: isLeader, + sentinelDesc: prometheus.NewDesc(MetricName, metricHelp, nil, nil), + } + go c.syncLoop(ctx) + return c +} + +// Describe sends a sentinel descriptor to satisfy the Collector contract. +func (c *AlertsCollector) Describe(ch chan<- *prometheus.Desc) { + ch <- c.sentinelDesc +} + +// Collect emits the current set of alert metrics using pre-built Descs. +// Returns nothing if this replica is not the leader. +func (c *AlertsCollector) Collect(ch chan<- prometheus.Metric) { + if !c.isLeader() { + return + } + + c.mu.RLock() + defer c.mu.RUnlock() + + for i := range c.metrics { + m := &c.metrics[i] + metric, err := prometheus.NewConstMetric(m.desc, prometheus.GaugeValue, m.activeAtSec, m.labelValues...) + if err != nil { + metricsLog.WithError(err).Warn("failed to create metric") + continue + } + ch <- metric + } +} + +func (c *AlertsCollector) syncLoop(ctx context.Context) { + c.sync(ctx) + + ticker := time.NewTicker(c.syncInterval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + c.sync(ctx) + } + } +} + +func (c *AlertsCollector) sync(ctx context.Context) { + if !c.isLeader() { + return + } + + alerts, _, err := c.fetcher.EnrichAlerts(ctx, k8s.GetAlertsRequest{}) + if err != nil { + metricsLog.WithError(err).Warn("failed to fetch alerts for effective metric") + return + } + + built := make([]alertMetric, 0, len(alerts)) + for i := range alerts { + alert := &alerts[i] + + // Drop Thanos-sourced alerts: they duplicate what Alertmanager and + // Prometheus already provide and would inflate the metric cardinality. + if alert.Labels[k8s.AlertBackendLabel] == k8s.AlertBackendThanos { + continue + } + + enrichClassificationLabels(alert) + + m := buildAlertMetric(alert) + if m != nil { + built = append(built, *m) + } + } + + c.mu.Lock() + c.metrics = built + c.mu.Unlock() + + metricsLog.Debugf("synced %d alerts for effective metric", len(built)) +} + +// enrichClassificationLabels copies the management-computed AlertComponent and +// AlertLayer into the alert's Labels map so they appear on the metric. Labels +// already set (e.g. via ARC) take precedence. +func enrichClassificationLabels(alert *k8s.PrometheusAlert) { + if alert.AlertComponent != "" { + if _, exists := alert.Labels[k8s.AlertRuleClassificationComponentKey]; !exists { + alert.Labels[k8s.AlertRuleClassificationComponentKey] = alert.AlertComponent + } + } + if alert.AlertLayer != "" { + if _, exists := alert.Labels[k8s.AlertRuleClassificationLayerKey]; !exists { + alert.Labels[k8s.AlertRuleClassificationLayerKey] = alert.AlertLayer + } + } +} + +// buildAlertMetric converts a PrometheusAlert into an alertMetric with a +// pre-built prometheus.Desc. Uses the alert's labels plus the alertstate label. +func buildAlertMetric(alert *k8s.PrometheusAlert) *alertMetric { + if alert.ActiveAt.IsZero() { + return nil + } + + labelNames := make([]string, 0, len(alert.Labels)+1) + for k := range alert.Labels { + labelNames = append(labelNames, k) + } + sort.Strings(labelNames) + labelNames = append(labelNames, labelAlertState) + + labelValues := make([]string, 0, len(labelNames)) + for _, name := range labelNames { + if name == labelAlertState { + labelValues = append(labelValues, alert.State) + } else { + labelValues = append(labelValues, alert.Labels[name]) + } + } + + return &alertMetric{ + desc: prometheus.NewDesc(MetricName, metricHelp, labelNames, nil), + labelValues: labelValues, + activeAtSec: float64(alert.ActiveAt.Unix()), + } +} diff --git a/pkg/management/metrics/alerts_collector_test.go b/pkg/management/metrics/alerts_collector_test.go new file mode 100644 index 000000000..c2c18239a --- /dev/null +++ b/pkg/management/metrics/alerts_collector_test.go @@ -0,0 +1,354 @@ +package metrics_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/pkg/management/metrics" +) + +type mockAlertsFetcher struct { + alerts []k8s.PrometheusAlert + err error +} + +func (m *mockAlertsFetcher) EnrichAlerts(_ context.Context, _ k8s.GetAlertsRequest) ([]k8s.PrometheusAlert, []string, error) { + return m.alerts, nil, m.err +} + +func collectMetrics(t *testing.T, collector prometheus.Collector) []*dto.MetricFamily { + t.Helper() + reg := prometheus.NewRegistry() + reg.MustRegister(collector) + families, err := reg.Gather() + if err != nil { + t.Fatalf("gather metrics: %v", err) + } + return families +} + +func findFamily(families []*dto.MetricFamily, name string) *dto.MetricFamily { + for _, f := range families { + if f.GetName() == name { + return f + } + } + return nil +} + +func labelValue(m *dto.Metric, name string) string { + for _, lp := range m.GetLabel() { + if lp.GetName() == name { + return lp.GetValue() + } + } + return "" +} + +func newCollector(t *testing.T, mock *mockAlertsFetcher) (prometheus.Collector, context.CancelFunc) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + collector := metrics.NewAlertsCollector(ctx, mock, 1*time.Hour, func() bool { return true }) + time.Sleep(100 * time.Millisecond) + t.Cleanup(cancel) + return collector, cancel +} + +func TestAlertsCollector_FiringAndSilenced(t *testing.T) { + activeAt := time.Date(2026, 3, 5, 10, 0, 0, 0, time.UTC) + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + { + Labels: map[string]string{"alertname": "HighCPU", "severity": "critical", "namespace": "production"}, + State: "firing", + ActiveAt: activeAt, + }, + { + Labels: map[string]string{"alertname": "DiskFull", "severity": "warning", "namespace": "storage"}, + State: "silenced", + ActiveAt: activeAt.Add(-1 * time.Hour), + }, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil { + t.Fatal("expected metric family, got nil") + } + if len(family.GetMetric()) != 2 { + t.Fatalf("expected 2 metrics, got %d", len(family.GetMetric())) + } + + var firing, silenced *dto.Metric + for _, m := range family.GetMetric() { + switch labelValue(m, "alertname") { + case "HighCPU": + firing = m + case "DiskFull": + silenced = m + } + } + + if firing == nil { + t.Fatal("expected HighCPU metric") + } + if labelValue(firing, "alertstate") != "firing" { + t.Errorf("alertstate: want firing, got %q", labelValue(firing, "alertstate")) + } + if labelValue(firing, "severity") != "critical" { + t.Errorf("severity: want critical, got %q", labelValue(firing, "severity")) + } + if labelValue(firing, "namespace") != "production" { + t.Errorf("namespace: want production, got %q", labelValue(firing, "namespace")) + } + if firing.GetGauge().GetValue() != float64(activeAt.Unix()) { + t.Errorf("gauge value: want %v, got %v", float64(activeAt.Unix()), firing.GetGauge().GetValue()) + } + + if silenced == nil { + t.Fatal("expected DiskFull metric") + } + if labelValue(silenced, "alertstate") != "silenced" { + t.Errorf("alertstate: want silenced, got %q", labelValue(silenced, "alertstate")) + } + if silenced.GetGauge().GetValue() != float64(activeAt.Add(-1*time.Hour).Unix()) { + t.Errorf("silenced gauge value mismatch") + } +} + +func TestAlertsCollector_NoAnnotationLabels(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + {Labels: map[string]string{"alertname": "TestAlert"}, State: "firing", ActiveAt: time.Now()}, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil { + t.Fatal("expected metric family") + } + if len(family.GetMetric()) != 1 { + t.Fatalf("expected 1 metric, got %d", len(family.GetMetric())) + } + for _, lp := range family.GetMetric()[0].GetLabel() { + switch lp.GetName() { + case "summary", "description", "runbook_url": + t.Errorf("unexpected annotation label: %s", lp.GetName()) + } + } +} + +func TestAlertsCollector_SkipsZeroActiveAt(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + {Labels: map[string]string{"alertname": "NoActiveAt"}, State: "firing", ActiveAt: time.Time{}}, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family != nil && len(family.GetMetric()) != 0 { + t.Errorf("expected no metrics for zero ActiveAt, got %d", len(family.GetMetric())) + } +} + +func TestAlertsCollector_EmptyAlerts(t *testing.T) { + mock := &mockAlertsFetcher{alerts: []k8s.PrometheusAlert{}} + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family != nil && len(family.GetMetric()) != 0 { + t.Errorf("expected no metrics for empty alerts, got %d", len(family.GetMetric())) + } +} + +func TestAlertsCollector_FetcherErrorProducesNoMetrics(t *testing.T) { + mock := &mockAlertsFetcher{err: errors.New("connection refused")} + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family != nil && len(family.GetMetric()) != 0 { + t.Errorf("expected no metrics on initial failure, got %d", len(family.GetMetric())) + } +} + +func TestAlertsCollector_ClassificationLabels(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + { + Labels: map[string]string{ + "alertname": "KubePodCrashLooping", + "severity": "warning", + "namespace": "kube-system", + k8s.AlertRuleLabelId: "abc123", + k8s.AlertRuleClassificationComponentKey: "kube-controller-manager", + k8s.AlertRuleClassificationLayerKey: "cluster", + }, + State: "firing", + ActiveAt: time.Now(), + }, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil || len(family.GetMetric()) != 1 { + t.Fatalf("expected 1 metric, got family=%v", family) + } + m := family.GetMetric()[0] + checks := map[string]string{ + k8s.AlertRuleLabelId: "abc123", + k8s.AlertRuleClassificationComponentKey: "kube-controller-manager", + k8s.AlertRuleClassificationLayerKey: "cluster", + "alertstate": "firing", + } + for k, want := range checks { + if got := labelValue(m, k); got != want { + t.Errorf("label[%s]: want %q, got %q", k, want, got) + } + } +} + +func TestAlertsCollector_IncludesPendingAlerts(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + {Labels: map[string]string{"alertname": "Firing"}, State: "firing", ActiveAt: time.Now()}, + {Labels: map[string]string{"alertname": "Silenced"}, State: "silenced", ActiveAt: time.Now()}, + {Labels: map[string]string{"alertname": "Pending"}, State: "pending", ActiveAt: time.Now()}, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil || len(family.GetMetric()) != 3 { + t.Fatalf("expected 3 metrics, got %v", family) + } + states := map[string]bool{} + for _, m := range family.GetMetric() { + states[labelValue(m, "alertstate")] = true + } + for _, s := range []string{"firing", "silenced", "pending"} { + if !states[s] { + t.Errorf("expected state %q in metrics", s) + } + } +} + +func TestAlertsCollector_SourceAndBackendLabels(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + { + Labels: map[string]string{ + "alertname": "HighCPU", + "severity": "critical", + k8s.AlertSourceLabel: k8s.AlertSourcePlatform, + k8s.AlertBackendLabel: k8s.AlertBackendAM, + }, + State: "firing", + ActiveAt: time.Now(), + }, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil || len(family.GetMetric()) != 1 { + t.Fatalf("expected 1 metric") + } + m := family.GetMetric()[0] + if got := labelValue(m, k8s.AlertSourceLabel); got != k8s.AlertSourcePlatform { + t.Errorf("source: want %q, got %q", k8s.AlertSourcePlatform, got) + } + if got := labelValue(m, k8s.AlertBackendLabel); got != k8s.AlertBackendAM { + t.Errorf("backend: want %q, got %q", k8s.AlertBackendAM, got) + } +} + +func TestAlertsCollector_FiltersThanosBackend(t *testing.T) { + now := time.Now() + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + {Labels: map[string]string{"alertname": "HighCPU", k8s.AlertBackendLabel: k8s.AlertBackendAM, k8s.AlertSourceLabel: k8s.AlertSourcePlatform}, State: "firing", ActiveAt: now}, + {Labels: map[string]string{"alertname": "HighCPU", k8s.AlertBackendLabel: k8s.AlertBackendThanos, k8s.AlertSourceLabel: k8s.AlertSourceUser}, State: "firing", ActiveAt: now}, + {Labels: map[string]string{"alertname": "PendingAlert", k8s.AlertBackendLabel: k8s.AlertBackendProm, k8s.AlertSourceLabel: k8s.AlertSourcePlatform}, State: "pending", ActiveAt: now}, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil || len(family.GetMetric()) != 2 { + t.Fatalf("expected 2 metrics (thanos filtered), got %v", family) + } + for _, m := range family.GetMetric() { + if labelValue(m, k8s.AlertBackendLabel) == k8s.AlertBackendThanos { + t.Error("thanos duplicate should be filtered out") + } + } +} + +func TestAlertsCollector_InjectsClassificationFromFields(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + { + Labels: map[string]string{"alertname": "TestAlert", k8s.AlertBackendLabel: k8s.AlertBackendAM}, + State: "firing", + ActiveAt: time.Now(), + AlertComponent: "networking", + AlertLayer: "cluster", + }, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil || len(family.GetMetric()) != 1 { + t.Fatalf("expected 1 metric") + } + m := family.GetMetric()[0] + if got := labelValue(m, k8s.AlertRuleClassificationComponentKey); got != "networking" { + t.Errorf("component: want networking, got %q", got) + } + if got := labelValue(m, k8s.AlertRuleClassificationLayerKey); got != "cluster" { + t.Errorf("layer: want cluster, got %q", got) + } +} + +func TestAlertsCollector_DoesNotOverwriteARCLabels(t *testing.T) { + mock := &mockAlertsFetcher{ + alerts: []k8s.PrometheusAlert{ + { + Labels: map[string]string{ + "alertname": "TestAlert", + k8s.AlertBackendLabel: k8s.AlertBackendAM, + k8s.AlertRuleClassificationComponentKey: "arc-component", + k8s.AlertRuleClassificationLayerKey: "namespace", + }, + State: "firing", + ActiveAt: time.Now(), + AlertComponent: "default-component", + AlertLayer: "cluster", + }, + }, + } + collector, _ := newCollector(t, mock) + families := collectMetrics(t, collector) + family := findFamily(families, metrics.MetricName) + if family == nil || len(family.GetMetric()) != 1 { + t.Fatalf("expected 1 metric") + } + m := family.GetMetric()[0] + if got := labelValue(m, k8s.AlertRuleClassificationComponentKey); got != "arc-component" { + t.Errorf("component: want arc-component, got %q", got) + } + if got := labelValue(m, k8s.AlertRuleClassificationLayerKey); got != "namespace" { + t.Errorf("layer: want namespace, got %q", got) + } +} diff --git a/pkg/management/metrics/leader_election.go b/pkg/management/metrics/leader_election.go new file mode 100644 index 000000000..2a71f9231 --- /dev/null +++ b/pkg/management/metrics/leader_election.go @@ -0,0 +1,87 @@ +package metrics + +import ( + "context" + "fmt" + "os" + "sync" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + coordinationv1client "k8s.io/client-go/kubernetes/typed/coordination/v1" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/leaderelection" + "k8s.io/client-go/tools/leaderelection/resourcelock" +) + +const ( + leaseName = "monitoring-plugin-metrics" + leaseDuration = 15 * time.Second + leaseRenew = 10 * time.Second + leaseRetry = 2 * time.Second +) + +// startLeaderElection sets up Lease-based leader election for the alerts +// effective metric. Returns a thread-safe isLeader callback. +func startLeaderElection(ctx context.Context, kubeConfig *rest.Config, namespace string) (func() bool, error) { + coordClient, err := coordinationv1client.NewForConfig(kubeConfig) + if err != nil { + return nil, fmt.Errorf("create coordination client: %w", err) + } + + identity, err := os.Hostname() + if err != nil { + return nil, fmt.Errorf("get hostname: %w", err) + } + + lock := &resourcelock.LeaseLock{ + LeaseMeta: metav1.ObjectMeta{ + Name: leaseName, + Namespace: namespace, + }, + Client: coordClient, + LockConfig: resourcelock.ResourceLockConfig{ + Identity: identity, + }, + } + + var mu sync.Mutex + isLeading := false + + isLeader := func() bool { + mu.Lock() + defer mu.Unlock() + return isLeading + } + + le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ + Lock: lock, + LeaseDuration: leaseDuration, + RenewDeadline: leaseRenew, + RetryPeriod: leaseRetry, + ReleaseOnCancel: true, + Callbacks: leaderelection.LeaderCallbacks{ + OnStartedLeading: func(_ context.Context) { + mu.Lock() + isLeading = true + mu.Unlock() + metricsLog.Info("became leader for alert management metrics") + }, + OnStoppedLeading: func() { + mu.Lock() + isLeading = false + mu.Unlock() + metricsLog.Info("lost leadership for alert management metrics") + }, + OnNewLeader: func(identity string) { + metricsLog.Infof("new leader for alert management metrics: %s", identity) + }, + }, + }) + if err != nil { + return nil, fmt.Errorf("create leader elector: %w", err) + } + + go le.Run(ctx) + return isLeader, nil +} diff --git a/pkg/management/types.go b/pkg/management/types.go index fd158a7db..2a314f076 100644 --- a/pkg/management/types.go +++ b/pkg/management/types.go @@ -2,8 +2,10 @@ package management import ( "context" + "net/http" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + "k8s.io/client-go/rest" "github.com/openshift/monitoring-plugin/pkg/k8s" ) @@ -59,6 +61,10 @@ type Client interface { // GetAlertingHealth retrieves the alerting stack health status GetAlertingHealth(ctx context.Context) (k8s.AlertingHealth, error) + + // MetricsHandler returns an HTTP handler that exposes alert management metrics. + // It handles leader election internally using the provided kubeConfig. + MetricsHandler(ctx context.Context, kubeConfig *rest.Config) (http.Handler, error) } // PrometheusRuleOptions specifies options for selecting PrometheusRule resources and groups diff --git a/pkg/server/server.go b/pkg/server/server.go index 5c5c49800..507e85d41 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -184,7 +184,10 @@ func createHTTPServer(ctx context.Context, cfg *Config) (*http.Server, error) { log.Info("alert management API enabled") } - router, pluginConfig := setupRoutes(cfg, managementClient) + router, pluginConfig, err := setupRoutes(ctx, cfg, managementClient, k8sconfig) + if err != nil { + return nil, fmt.Errorf("failed to set up routes: %w", err) + } router.Use(corsHeaderMiddleware()) tlsConfig := &tls.Config{} @@ -275,7 +278,7 @@ func createHTTPServer(ctx context.Context, cfg *Config) (*http.Server, error) { return httpServer, nil } -func setupRoutes(cfg *Config, managementClient management.Client) (*mux.Router, *PluginConfig) { +func setupRoutes(ctx context.Context, cfg *Config, managementClient management.Client, k8sconfig *rest.Config) (*mux.Router, *PluginConfig, error) { configHandlerFunc, pluginConfig := configHandler(cfg) router := mux.NewRouter() @@ -290,11 +293,18 @@ func setupRoutes(cfg *Config, managementClient management.Client) (*mux.Router, if managementClient != nil { managementRouter := managementrouter.New(managementClient) router.PathPrefix("/api/v1/alerting").Handler(managementRouter) + + metricsHandler, err := managementClient.MetricsHandler(ctx, k8sconfig) + if err != nil { + return nil, nil, fmt.Errorf("failed to start alert management metrics: %w", err) + } + router.Path("/metrics").Handler(metricsHandler) + log.Info("alert management metrics started") } router.PathPrefix("/").Handler(filesHandler(http.Dir(cfg.StaticPath))) - return router, pluginConfig + return router, pluginConfig, nil } func setupProxyRoutes(cfg *Config, k8sclient *dynamic.DynamicClient, kind monitoring.KindType) *mux.Router { diff --git a/test/e2e/alerts_effective_metric_test.go b/test/e2e/alerts_effective_metric_test.go new file mode 100644 index 000000000..ff1858e99 --- /dev/null +++ b/test/e2e/alerts_effective_metric_test.go @@ -0,0 +1,319 @@ +//go:build e2e + +package e2e + +import ( + "context" + "fmt" + "io" + "net/http" + "strings" + "testing" + "time" + + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/openshift/monitoring-plugin/pkg/k8s" + "github.com/openshift/monitoring-plugin/pkg/management/metrics" + "github.com/openshift/monitoring-plugin/test/e2e/framework" +) + +func fetchMetrics(f *framework.Framework) (metricsBody string, err error) { + resp, err := f.HTTPClient().Get(f.PluginURL + "/metrics") + if err != nil { + return "", err + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil && err == nil { + err = fmt.Errorf("closing response body: %w", closeErr) + } + }() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("unexpected status code: %d", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", err + } + return string(body), nil +} + +func parseMetricLines(body string) []string { + var lines []string + for _, line := range strings.Split(body, "\n") { + if strings.HasPrefix(line, metrics.MetricName+"{") { + lines = append(lines, line) + } + } + return lines +} + +func extractLabel(metricLine, labelName string) string { + key := labelName + `="` + idx := strings.Index(metricLine, key) + if idx < 0 { + return "" + } + start := idx + len(key) + end := strings.Index(metricLine[start:], `"`) + if end < 0 { + return "" + } + return metricLine[start : start+end] +} + +// TestMetricEndpointExposesEffectiveMetric +// Verifies that the /metrics endpoint exposes alerts_effective_active_at_timestamp_seconds. +func TestMetricEndpointExposesEffectiveMetric(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + var metricBody string + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + body, err := fetchMetrics(f) + if err != nil { + t.Logf("Failed to fetch metrics: %v", err) + return false, nil + } + + if !strings.Contains(body, metrics.MetricName) { + t.Logf("Metric %s not found yet (leader election may be in progress)", metrics.MetricName) + return false, nil + } + + metricBody = body + return true, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for metric to appear: %v", err) + } + + if !strings.Contains(metricBody, "# HELP "+metrics.MetricName) { + t.Error("Missing HELP line for metric") + } + if !strings.Contains(metricBody, "# TYPE "+metrics.MetricName+" gauge") { + t.Error("Missing or incorrect TYPE line for metric (expected gauge)") + } + + lines := parseMetricLines(metricBody) + if len(lines) == 0 { + t.Fatal("Expected at least one metric series, got none") + } + + t.Logf("Found %d metric series for %s", len(lines), metrics.MetricName) +} + +// TestMetricSeriesHaveRequiredLabels +// Verifies every metric series has alertname, alertstate, openshift_io_alert_source, +// openshift_io_alert_backend, and a valid timestamp value. +func TestMetricSeriesHaveRequiredLabels(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + var lines []string + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + body, err := fetchMetrics(f) + if err != nil { + t.Logf("Failed to fetch metrics: %v", err) + return false, nil + } + lines = parseMetricLines(body) + return len(lines) > 0, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for metric series: %v", err) + } + + requiredLabels := []string{ + "alertname", + "alertstate", + k8s.AlertSourceLabel, + k8s.AlertBackendLabel, + } + + for i, line := range lines { + for _, label := range requiredLabels { + val := extractLabel(line, label) + if val == "" { + t.Errorf("Series %d missing required label %q: %s", i, label, line) + } + } + + state := extractLabel(line, "alertstate") + validStates := map[string]bool{"firing": true, "pending": true, "silenced": true, "suppressed": true} + if !validStates[state] { + t.Errorf("Series %d has unexpected alertstate=%q: %s", i, state, line) + } + + parts := strings.Split(line, " ") + if len(parts) < 2 { + t.Errorf("Series %d has no value: %s", i, line) + continue + } + var ts float64 + if _, err := fmt.Sscanf(parts[len(parts)-1], "%g", &ts); err != nil { + t.Errorf("Series %d has unparseable value %q: %v", i, parts[len(parts)-1], err) + continue + } + if ts < 9.46e+08 { + t.Errorf("Series %d has suspiciously low timestamp value: %g (before year 2000)", i, ts) + } + } + + t.Logf("All %d series have required labels and valid values", len(lines)) +} + +// TestMetricIncludesClassificationLabels +// Verifies that all metric series have classification labels +// (openshift_io_alert_rule_component and openshift_io_alert_rule_layer). +func TestMetricIncludesClassificationLabels(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + var lines []string + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + body, err := fetchMetrics(f) + if err != nil { + return false, nil + } + lines = parseMetricLines(body) + return len(lines) > 0, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for metric series: %v", err) + } + + for i, line := range lines { + if extractLabel(line, k8s.AlertRuleClassificationComponentKey) == "" { + t.Errorf("Series %d missing %s label: %s", i, k8s.AlertRuleClassificationComponentKey, line) + } + if extractLabel(line, k8s.AlertRuleClassificationLayerKey) == "" { + t.Errorf("Series %d missing %s label: %s", i, k8s.AlertRuleClassificationLayerKey, line) + } + } + + t.Logf("All %d series have classification labels (component + layer)", len(lines)) +} + +// TestMetricExcludesAnnotations +// Verifies that annotations (summary, description, runbook_url) are not +// included as metric labels. +func TestMetricExcludesAnnotations(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + var lines []string + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + body, err := fetchMetrics(f) + if err != nil { + return false, nil + } + lines = parseMetricLines(body) + return len(lines) > 0, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for metric series: %v", err) + } + + annotationLabels := []string{"summary", "description", "runbook_url"} + + for i, line := range lines { + for _, annLabel := range annotationLabels { + if extractLabel(line, annLabel) != "" { + t.Errorf("Series %d contains annotation label %q (annotations should be excluded): %s", + i, annLabel, line) + } + } + } + + t.Logf("Verified %d series - none contain annotation labels", len(lines)) +} + +// TestMetricActiveAtTimestampsAreReasonable +// Verifies that activeAt timestamps are not too recent. +func TestMetricActiveAtTimestampsAreReasonable(t *testing.T) { + f, err := framework.New() + if err != nil { + t.Fatalf("Failed to create framework: %v", err) + } + + ctx := context.Background() + var lines []string + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + body, err := fetchMetrics(f) + if err != nil { + return false, nil + } + lines = parseMetricLines(body) + return len(lines) > 0, nil + }) + if err != nil { + t.Fatalf("Timeout waiting for metric series: %v", err) + } + + now := float64(time.Now().Unix()) + fiveMinutesAgo := now - 300 + + recentCount := 0 + for _, line := range lines { + alertname := extractLabel(line, "alertname") + if alertname == "Watchdog" { + continue + } + + parts := strings.Split(line, " ") + if len(parts) < 2 { + continue + } + valueStr := parts[len(parts)-1] + + var ts float64 + if _, err := fmt.Sscanf(valueStr, "%e", &ts); err != nil { + if _, err := fmt.Sscanf(valueStr, "%f", &ts); err != nil { + continue + } + } + + if ts > fiveMinutesAgo { + recentCount++ + t.Logf("WARN: %s has activeAt within last 5 minutes (ts=%.0f, now=%.0f)", alertname, ts, now) + } + } + + totalNonWatchdog := 0 + for _, line := range lines { + if extractLabel(line, "alertname") != "Watchdog" { + totalNonWatchdog++ + } + } + + if totalNonWatchdog > 0 { + recentPct := float64(recentCount) / float64(totalNonWatchdog) * 100 + if recentPct > 80 { + t.Errorf("%.0f%% of alerts (%d/%d) have activeAt within last 5 minutes — "+ + "likely using Alertmanager startsAt instead of Prometheus activeAt", + recentPct, recentCount, totalNonWatchdog) + } + } + + t.Logf("Timestamp check: %d/%d non-Watchdog alerts have recent activeAt", recentCount, totalNonWatchdog) +}