From 3c1a5ab4393379421125376f69309aaab7baa271 Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:40:55 +0100 Subject: [PATCH] Alerting: Handle regex matchers with flags (#95883) * handle regex matchers with flags * add test in alertmanager.test.ts * refactor to use the same matching functions * lint * lint again * move matchers functions to matchers.ts --------- Co-authored-by: Gilles De Mey --- .../NotificationPreview.test.tsx | 25 +++++- .../unified/utils/alertmanager.test.ts | 11 ++- .../alerting/unified/utils/alertmanager.ts | 25 ++---- .../features/alerting/unified/utils/labels.ts | 2 +- .../alerting/unified/utils/matchers.ts | 76 +++++++++++++++++ .../utils/notification-policies.test.ts | 5 ++ .../unified/utils/notification-policies.ts | 81 +------------------ 7 files changed, 124 insertions(+), 101 deletions(-) diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx index ea6dbbaeab4..780f8833441 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor, within, userEvent } from 'test/test-utils'; +import { render, screen, userEvent, waitFor, within } from 'test/test-utils'; import { byRole, byTestId, byText } from 'testing-library-selector'; import { AccessControlAction } from 'app/types/accessControl'; @@ -463,4 +463,27 @@ describe('NotificationPreviewByAlertmanager', () => { expect(screen.queryByText(/regexfield/i)).not.toBeInTheDocument(); }); }); + it('matches regex with flags', async () => { + const potentialInstances: Labels[] = [{ regexfield: 'baaaaaaah' }]; + + mockApi(server).getAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, (amConfigBuilder) => + amConfigBuilder + .addReceivers((b) => b.withName('email')) + .withRoute((routeBuilder) => + routeBuilder + .withReceiver('email') + .addRoute((rb) => rb.withReceiver('email').addMatcher('regexfield', MatcherOperator.regex, '(?i)BA.*h')) + ) + ); + + render( + + ); + + expect(await screen.findByText(/regexfield/i)).toBeInTheDocument(); + }); }); diff --git a/public/app/features/alerting/unified/utils/alertmanager.test.ts b/public/app/features/alerting/unified/utils/alertmanager.test.ts index 1607a72ac6b..cc2b0b2543c 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.test.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.test.ts @@ -1,7 +1,7 @@ import { Matcher, MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; import { Labels } from 'app/types/unified-alerting-dto'; -import { labelsMatchMatchers, removeTimeIntervalFromRoute, matchersToString } from './alertmanager'; +import { labelsMatchMatchers, matchersToString, removeTimeIntervalFromRoute } from './alertmanager'; import { parseMatcher, parsePromQLStyleMatcherLooseSafe } from './matchers'; describe('Alertmanager utils', () => { @@ -92,6 +92,15 @@ describe('Alertmanager utils', () => { const matchers = parsePromQLStyleMatcherLooseSafe('foo!=bazz,bar=~ba.+'); expect(labelsMatchMatchers(labels, matchers)).toBe(true); }); + it('should match when using flags and different operators', () => { + const labels: Labels = { + foo: 'bar', + bar: 'bazz', + email: 'admin@grafana.com', + }; + const matchers = parsePromQLStyleMatcherLooseSafe('foo!=bazz,bar=~(?i)Ba.+'); + expect(labelsMatchMatchers(labels, matchers)).toBe(true); + }); }); describe('removeMuteTimingFromRoute', () => { diff --git a/public/app/features/alerting/unified/utils/alertmanager.ts b/public/app/features/alerting/unified/utils/alertmanager.ts index c7e05d82dd0..7b2e4f163b7 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.ts @@ -16,7 +16,8 @@ import { MatcherFieldValue } from '../types/silence-form'; import { getAllDataSources } from './config'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './datasource'; -import { MatcherFormatter, parsePromQLStyleMatcherLooseSafe, unquoteWithUnescape } from './matchers'; +import { objectLabelsToArray } from './labels'; +import { MatcherFormatter, matchLabelsSet, parsePromQLStyleMatcherLooseSafe, unquoteWithUnescape } from './matchers'; export function addDefaultsToAlertmanagerConfig(config: AlertManagerCortexConfig): AlertManagerCortexConfig { // add default receiver if it does not exist @@ -125,26 +126,10 @@ export function matcherToObjectMatcher(matcher: Matcher): ObjectMatcher { } export function labelsMatchMatchers(labels: Labels, matchers: Matcher[]): boolean { - return matchers.every(({ name, value, isRegex, isEqual }) => { - return Object.entries(labels).some(([labelKey, labelValue]) => { - const nameMatches = name === labelKey; - let valueMatches; - if (isEqual && !isRegex) { - valueMatches = value === labelValue; - } - if (!isEqual && !isRegex) { - valueMatches = value !== labelValue; - } - if (isEqual && isRegex) { - valueMatches = new RegExp(value).test(labelValue); - } - if (!isEqual && isRegex) { - valueMatches = !new RegExp(value).test(labelValue); - } + const labelsArray = objectLabelsToArray(labels); + const objectMatchers = matchers.map(matcherToObjectMatcher); - return nameMatches && valueMatches; - }); - }); + return matchLabelsSet(objectMatchers, labelsArray); } export function combineMatcherStrings(...matcherStrings: string[]): string { diff --git a/public/app/features/alerting/unified/utils/labels.ts b/public/app/features/alerting/unified/utils/labels.ts index 1d8fab6ec89..e76d600c400 100644 --- a/public/app/features/alerting/unified/utils/labels.ts +++ b/public/app/features/alerting/unified/utils/labels.ts @@ -8,7 +8,7 @@ export function labelsToTags(labels: Labels) { } export function objectLabelsToArray(labels: Labels): Label[] { - return Object.entries(labels).map(([label, value]) => [label, value]); + return Object.entries(labels); } export function arrayLabelsToObject(labels: Label[]): Labels { diff --git a/public/app/features/alerting/unified/utils/matchers.ts b/public/app/features/alerting/unified/utils/matchers.ts index 917e055e3bf..081f9c1f8e1 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -7,6 +7,7 @@ import { compact, uniqBy } from 'lodash'; +import { parseFlags } from '@grafana/data'; import { Matcher, MatcherOperator, ObjectMatcher, Route } from 'app/plugins/datasource/alertmanager/types'; import { Labels } from '../../../../types/unified-alerting-dto'; @@ -240,6 +241,81 @@ function matcherToOperator(matcher: Matcher): MatcherOperator { } } +// Compare set of matchers to set of label +export function matchLabelsSet(matchers: ObjectMatcher[], labels: Label[]): boolean { + for (const matcher of matchers) { + if (!isLabelMatchInSet(matcher, labels)) { + return false; + } + } + return true; +} + +type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean; +const OperatorFunctions: Record = { + [MatcherOperator.equal]: (lv, mv) => lv === mv, + [MatcherOperator.notEqual]: (lv, mv) => lv !== mv, + // At the time of writing, Alertmanager compiles to another (anchored) Regular Expression, + // so we should also anchor our UI matches for consistency with this behaviour + // https://github.com/prometheus/alertmanager/blob/fd37ce9c95898ca68be1ab4d4529517174b73c33/pkg/labels/matcher.go#L69 + [MatcherOperator.regex]: (lv, mv) => { + const valueWithFlagsParsed = parseFlags(`^(?:${mv})$`); + const re = new RegExp(valueWithFlagsParsed.cleaned, valueWithFlagsParsed.flags); + return re.test(lv); + }, + [MatcherOperator.notRegex]: (lv, mv) => { + const valueWithFlagsParsed = parseFlags(`^(?:${mv})$`); + const re = new RegExp(valueWithFlagsParsed.cleaned, valueWithFlagsParsed.flags); + return !re.test(lv); + }, +}; + +function isLabelMatchInSet(matcher: ObjectMatcher, labels: Label[]): boolean { + const [matcherKey, operator, matcherValue] = matcher; + + let labelValue = ''; // matchers that have no labels are treated as empty string label values + const labelForMatcher = Object.fromEntries(labels)[matcherKey]; + if (labelForMatcher) { + labelValue = labelForMatcher; + } + + const matchFunction = OperatorFunctions[operator]; + if (!matchFunction) { + throw new Error(`no such operator: ${operator}`); + } + + try { + // This can throw because the regex operators use the JavaScript regex engine + // and "new RegExp()" throws on invalid regular expressions. + // + // This is usually a user-error (because matcher values are taken from user input) + // but we're still logging this as a warning because it _might_ be a programmer error. + return matchFunction(labelValue, matcherValue); + } catch (err) { + console.warn(err); + return false; + } +} + +// ⚠️ DO NOT USE THIS FUNCTION FOR ROUTE SELECTION ALGORITHM +// for route selection algorithm, always compare a single matcher to the entire label set +// see "matchLabelsSet" +export function isLabelMatch(matcher: ObjectMatcher, label: Label): boolean { + const [labelKey, labelValue] = label; + const [matcherKey, operator, matcherValue] = matcher; + + if (labelKey !== matcherKey) { + return false; + } + + const matchFunction = OperatorFunctions[operator]; + if (!matchFunction) { + throw new Error(`no such operator: ${operator}`); + } + + return matchFunction(labelValue, matcherValue); +} + export type MatcherFormatter = keyof typeof matcherFormatter; export type Label = [string, string]; diff --git a/public/app/features/alerting/unified/utils/notification-policies.test.ts b/public/app/features/alerting/unified/utils/notification-policies.test.ts index 52dd314b591..d6b83406edb 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.test.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.test.ts @@ -485,6 +485,11 @@ describe('matchLabels', () => { const result = matchLabels([['foo', MatcherOperator.regex, '.*bar.*']], [['foo', 'barbarbar']]); expect(result.matches).toEqual(true); }); + + it('does match regular expressions with flags', () => { + const result = matchLabels([['foo', MatcherOperator.regex, '(?i).*BAr.*']], [['foo', 'barbarbar']]); + expect(result.matches).toEqual(true); + }); }); describe('unquoteRouteMatchers', () => { diff --git a/public/app/features/alerting/unified/utils/notification-policies.ts b/public/app/features/alerting/unified/utils/notification-policies.ts index 6f2d6969215..bb8d80ab904 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.ts @@ -1,15 +1,9 @@ import { isArray, pick, reduce } from 'lodash'; -import { - AlertmanagerGroup, - MatcherOperator, - ObjectMatcher, - Route, - RouteWithID, -} from 'app/plugins/datasource/alertmanager/types'; +import { AlertmanagerGroup, ObjectMatcher, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; import { Labels } from 'app/types/unified-alerting-dto'; -import { Label, normalizeMatchers, unquoteWithUnescape } from './matchers'; +import { isLabelMatch, Label, matchLabelsSet, normalizeMatchers, unquoteWithUnescape } from './matchers'; // If a policy has no matchers it still can be a match, hence matchers can be empty and match can be true // So we cannot use null as an indicator of no match @@ -52,16 +46,6 @@ export function matchLabels(matchers: ObjectMatcher[], labels: Label[]): Matchin return { matches, labelsMatch }; } -// Compare set of matchers to set of label -export function matchLabelsSet(matchers: ObjectMatcher[], labels: Label[]): boolean { - for (const matcher of matchers) { - if (!isLabelMatchInSet(matcher, labels)) { - return false; - } - } - return true; -} - export interface AlertInstanceMatch { instance: Labels; labelsMatch: LabelsMatch; @@ -228,59 +212,6 @@ export function computeInheritedTree(parent: T): T { }; } -type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean; -const OperatorFunctions: Record = { - [MatcherOperator.equal]: (lv, mv) => lv === mv, - [MatcherOperator.notEqual]: (lv, mv) => lv !== mv, - // At the time of writing, Alertmanager compiles to another (anchored) Regular Expression, - // so we should also anchor our UI matches for consistency with this behaviour - // https://github.com/prometheus/alertmanager/blob/fd37ce9c95898ca68be1ab4d4529517174b73c33/pkg/labels/matcher.go#L69 - [MatcherOperator.regex]: (lv, mv) => { - const re = new RegExp(`^(?:${mv})$`); - return re.test(lv); - }, - [MatcherOperator.notRegex]: (lv, mv) => { - const re = new RegExp(`^(?:${mv})$`); - return !re.test(lv); - }, -}; - -function isLabelMatchInSet(matcher: ObjectMatcher, labels: Label[]): boolean { - const [matcherKey, operator, matcherValue] = matcher; - - let labelValue = ''; // matchers that have no labels are treated as empty string label values - const labelForMatcher = Object.fromEntries(labels)[matcherKey]; - if (labelForMatcher) { - labelValue = labelForMatcher; - } - - const matchFunction = OperatorFunctions[operator]; - if (!matchFunction) { - throw new Error(`no such operator: ${operator}`); - } - - return matchFunction(labelValue, matcherValue); -} - -// ⚠️ DO NOT USE THIS FUNCTION FOR ROUTE SELECTION ALGORITHM -// for route selection algorithm, always compare a single matcher to the entire label set -// see "matchLabelsSet" -function isLabelMatch(matcher: ObjectMatcher, label: Label): boolean { - const [labelKey, labelValue] = label; - const [matcherKey, operator, matcherValue] = matcher; - - if (labelKey !== matcherKey) { - return false; - } - - const matchFunction = OperatorFunctions[operator]; - if (!matchFunction) { - throw new Error(`no such operator: ${operator}`); - } - - return matchFunction(labelValue, matcherValue); -} - // recursive function to rename receivers in all routes (notification policies) function renameReceiverInRoute(route: Route, oldName: string, newName: string) { const updated: Route = { @@ -298,10 +229,4 @@ function renameReceiverInRoute(route: Route, oldName: string, newName: string) { return updated; } -export { - findMatchingAlertGroups, - findMatchingRoutes, - getInheritedProperties, - isLabelMatchInSet, - renameReceiverInRoute, -}; +export { findMatchingAlertGroups, findMatchingRoutes, getInheritedProperties, renameReceiverInRoute };