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/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/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..915b62f8c 100644 --- a/internal/managementrouter/router.go +++ b/internal/managementrouter/router.go @@ -43,9 +43,12 @@ 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, 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/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_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/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/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") +} 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 +}