From 4d963c43b589526b9932d547b5873f223974fad3 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Fri, 2 Feb 2024 14:46:40 +0100 Subject: [PATCH] Apply matchers encoding and decoding on the RTKQ layer --- .../alerting/unified/api/alertmanagerApi.ts | 23 ++++++---- .../components/silences/SilencesTable.tsx | 9 ++-- .../alerting/unified/state/actions.ts | 12 +++++- .../alerting/unified/utils/alertmanager.ts | 36 ++++++++++++++++ .../alerting/unified/utils/amroutes.test.ts | 43 ------------------- .../alerting/unified/utils/amroutes.ts | 7 +-- .../alerting/unified/utils/matchers.ts | 42 ++++++++++-------- .../features/alerting/unified/utils/misc.ts | 4 ++ 8 files changed, 98 insertions(+), 78 deletions(-) diff --git a/public/app/features/alerting/unified/api/alertmanagerApi.ts b/public/app/features/alerting/unified/api/alertmanagerApi.ts index e97588813de..9ee4618af19 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -1,3 +1,4 @@ +import { produce } from 'immer'; import { isEmpty } from 'lodash'; import { dispatch } from 'app/store/store'; @@ -15,7 +16,7 @@ import { } from '../../../../plugins/datasource/alertmanager/types'; import { NotifierDTO } from '../../../../types'; import { withPerformanceLogging } from '../Analytics'; -import { matcherToOperator } from '../utils/alertmanager'; +import { matcherToOperator, quoteAmConfigMatchers, unquoteRouteMatchers } from '../utils/alertmanager'; import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME, @@ -196,7 +197,11 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ })); } - return result; + return produce(result, (draft) => { + if (draft.alertmanager_config.route) { + unquoteRouteMatchers(draft.alertmanager_config.route); + } + }); }) .then((result) => result ?? defaultConfig) .then((result) => ({ data: result })) @@ -224,12 +229,14 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ void, { selectedAlertmanager: string; config: AlertManagerCortexConfig } >({ - query: ({ selectedAlertmanager, config, ...rest }) => ({ - url: `/api/alertmanager/${getDatasourceAPIUid(selectedAlertmanager)}/config/api/v1/alerts`, - method: 'POST', - data: config, - ...rest, - }), + query: ({ selectedAlertmanager, config, ...rest }) => { + return { + url: `/api/alertmanager/${getDatasourceAPIUid(selectedAlertmanager)}/config/api/v1/alerts`, + method: 'POST', + data: quoteAmConfigMatchers(config), + ...rest, + }; + }, invalidatesTags: ['AlertmanagerConfiguration'], }), diff --git a/public/app/features/alerting/unified/components/silences/SilencesTable.tsx b/public/app/features/alerting/unified/components/silences/SilencesTable.tsx index 3d8c21c4b83..f6df6c0fae2 100644 --- a/public/app/features/alerting/unified/components/silences/SilencesTable.tsx +++ b/public/app/features/alerting/unified/components/silences/SilencesTable.tsx @@ -10,13 +10,15 @@ import { useDispatch } from 'app/types'; import { AlertmanagerAction, useAlertmanagerAbility } from '../../hooks/useAbilities'; import { expireSilenceAction } from '../../state/actions'; import { parseMatchers } from '../../utils/alertmanager'; +import { matcherToObjectMatcher } from '../../utils/matchers'; import { getSilenceFiltersFromUrlParams, makeAMLink } from '../../utils/misc'; import { Authorize } from '../Authorize'; import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable'; +import { Matchers } from '../notification-policies/Matchers'; import { ActionButton } from '../rules/ActionButton'; import { ActionIcon } from '../rules/ActionIcon'; -import { Matchers } from './Matchers'; +// import { Matchers } from './Matchers'; import { NoSilencesSplash } from './NoSilencesCTA'; import { SilenceDetails } from './SilenceDetails'; import { SilenceStateTag } from './SilenceStateTag'; @@ -219,8 +221,9 @@ function useColumns(alertManagerSourceName: string) { { id: 'matchers', label: 'Matching labels', - renderCell: function renderMatchers({ data: { matchers } }) { - return ; + renderCell: function renderMatchers({ data: { matchers = [] } }) { + const objectMatchers = matchers.map(matcherToObjectMatcher); + return ; }, size: 10, }, diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index a609c09e06e..8167f96936b 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -66,7 +66,11 @@ import { setRulerRuleGroup, } from '../api/ruler'; import { RuleFormType, RuleFormValues } from '../types/rule-form'; -import { addDefaultsToAlertmanagerConfig, removeMuteTimingFromRoute } from '../utils/alertmanager'; +import { + addDefaultsToAlertmanagerConfig, + quoteAmConfigMatchers, + removeMuteTimingFromRoute, +} from '../utils/alertmanager'; import { getAllRulesSourceNames, getRulesDataSource, @@ -530,7 +534,11 @@ export const updateAlertManagerConfigAction = createAsyncThunk { + const [name, operator, value] = matcherToObjectMatcher(parseMatcher(stringMatcher)); + if (isQuoted(value)) { + return `${name}${operator}${unquoteWithUnescape(value)}`; + } + return stringMatcher; + }); + } + route.routes?.forEach(unquoteRouteMatchers); +} + +export function quoteRouteMatchers(route: Route): void { + if (route.matchers) { + route.matchers = route.matchers.map((stringMatcher) => { + const matcher = parseMatcher(stringMatcher); + const { name, value, operator } = matcherToMatcherField(matcher); + return `${name}${operator}${quoteWithEscape(value)}`; + }); + } + route.routes?.forEach(quoteRouteMatchers); +} + +export function quoteAmConfigMatchers(config: AlertManagerCortexConfig): AlertManagerCortexConfig { + return produce(config, (draft) => { + if (draft.alertmanager_config.route) { + // Grafana AM doesn't use the matchers field so shouldn't be affected + quoteRouteMatchers(draft.alertmanager_config.route); + } + }); +} + function getValidRegexString(regex: string): string { // Regexes provided by users might be invalid, so we need to catch the error try { diff --git a/public/app/features/alerting/unified/utils/amroutes.test.ts b/public/app/features/alerting/unified/utils/amroutes.test.ts index d127f9d437c..033ea860133 100644 --- a/public/app/features/alerting/unified/utils/amroutes.test.ts +++ b/public/app/features/alerting/unified/utils/amroutes.test.ts @@ -53,30 +53,6 @@ describe('formAmRouteToAmRoute', () => { expect(amRoute.group_by).toStrictEqual(['SHOULD BE SET']); }); }); - - it('should quote and escape matcher values', () => { - // Arrange - const route: FormAmRoute = buildFormAmRoute({ - id: '1', - object_matchers: [ - { name: 'foo', operator: MatcherOperator.equal, value: 'bar' }, - { name: 'foo', operator: MatcherOperator.equal, value: 'bar"baz' }, - { name: 'foo', operator: MatcherOperator.equal, value: 'bar\\baz' }, - { name: 'foo', operator: MatcherOperator.equal, value: '\\bar\\baz"\\' }, - ], - }); - - // Act - const amRoute = formAmRouteToAmRoute('mimir-am', route, { id: 'root' }); - - // Assert - expect(amRoute.matchers).toStrictEqual([ - 'foo="bar"', - 'foo="bar\\"baz"', - 'foo="bar\\\\baz"', - 'foo="\\\\bar\\\\baz\\"\\\\"', - ]); - }); }); describe('amRouteToFormAmRoute', () => { @@ -125,23 +101,4 @@ describe('amRouteToFormAmRoute', () => { expect(formRoute.overrideGrouping).toBe(true); }); }); - - it('should unquote and unescape matchers values', () => { - // Arrange - const amRoute = buildAmRoute({ - matchers: ['foo=bar', 'foo="bar"', 'foo="bar"baz"', 'foo="bar\\\\baz"', 'foo="\\\\bar\\\\baz"\\\\"'], - }); - - // Act - const formRoute = amRouteToFormAmRoute(amRoute); - - // Assert - expect(formRoute.object_matchers).toStrictEqual([ - { name: 'foo', operator: MatcherOperator.equal, value: 'bar' }, - { name: 'foo', operator: MatcherOperator.equal, value: 'bar' }, - { name: 'foo', operator: MatcherOperator.equal, value: 'bar"baz' }, - { name: 'foo', operator: MatcherOperator.equal, value: 'bar\\baz' }, - { name: 'foo', operator: MatcherOperator.equal, value: '\\bar\\baz"\\' }, - ]); - }); }); diff --git a/public/app/features/alerting/unified/utils/amroutes.ts b/public/app/features/alerting/unified/utils/amroutes.ts index b34f09fe424..133dadf8977 100644 --- a/public/app/features/alerting/unified/utils/amroutes.ts +++ b/public/app/features/alerting/unified/utils/amroutes.ts @@ -9,7 +9,6 @@ import { MatcherFieldValue } from '../types/silence-form'; import { matcherToMatcherField } from './alertmanager'; import { GRAFANA_RULES_SOURCE_NAME } from './datasource'; import { normalizeMatchers, parseMatcher } from './matchers'; -import { quoteWithEscape, unquoteWithUnescape } from './misc'; import { findExistingRoute } from './routeTree'; import { isValidPrometheusDuration, safeParseDurationstr } from './time'; @@ -101,7 +100,7 @@ export const amRouteToFormAmRoute = (route: RouteWithID | Route | undefined): Fo .map(({ name, operator, value }) => ({ name, operator, - value: unquoteWithUnescape(value), + value, })) ?? []; return { @@ -185,9 +184,7 @@ export const formAmRouteToAmRoute = ( // Grafana maintains a fork of AM to support all utf-8 characters in the "object_matchers" property values but this // does not exist in upstream AlertManager if (alertManagerSourceName !== GRAFANA_RULES_SOURCE_NAME) { - amRoute.matchers = formAmRoute.object_matchers?.map( - ({ name, operator, value }) => `${name}${operator}${quoteWithEscape(value)}` - ); + amRoute.matchers = formAmRoute.object_matchers?.map(({ name, operator, value }) => `${name}${operator}${value}`); amRoute.object_matchers = undefined; } else { amRoute.object_matchers = normalizeMatchers(amRoute); diff --git a/public/app/features/alerting/unified/utils/matchers.ts b/public/app/features/alerting/unified/utils/matchers.ts index aa1b7187f85..959cce17bcc 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -60,6 +60,27 @@ export const getMatcherQueryParams = (labels: Labels) => { return matcherUrlParams; }; +export const matcherToObjectMatcher = (matcher: Matcher): ObjectMatcher => { + const { name, value, isRegex, isEqual } = matcher; + + let operator = MatcherOperator.equal; + + if (isEqual && isRegex) { + operator = MatcherOperator.regex; + } + if (!isEqual && isRegex) { + operator = MatcherOperator.notRegex; + } + if (isEqual && !isRegex) { + operator = MatcherOperator.equal; + } + if (!isEqual && !isRegex) { + operator = MatcherOperator.notEqual; + } + + return [name, operator, value]; +}; + /** * We need to deal with multiple (deprecated) properties such as "match" and "match_re" * this function will normalize all of the different ways to define matchers in to a single one. @@ -68,24 +89,11 @@ export const normalizeMatchers = (route: Route): ObjectMatcher[] => { const matchers: ObjectMatcher[] = []; if (route.matchers) { - route.matchers.forEach((matcher) => { - const { name, value, isEqual, isRegex } = parseMatcher(matcher); - let operator = MatcherOperator.equal; + route.matchers.forEach((stringMatcher) => { + const matcher = parseMatcher(stringMatcher); + const objectMatcher = matcherToObjectMatcher(matcher); - if (isEqual && isRegex) { - operator = MatcherOperator.regex; - } - if (!isEqual && isRegex) { - operator = MatcherOperator.notRegex; - } - if (isEqual && !isRegex) { - operator = MatcherOperator.equal; - } - if (!isEqual && !isRegex) { - operator = MatcherOperator.notEqual; - } - - matchers.push([name, operator, value]); + matchers.push(objectMatcher); }); } diff --git a/public/app/features/alerting/unified/utils/misc.ts b/public/app/features/alerting/unified/utils/misc.ts index 90eaadf3e2c..9669ebd337a 100644 --- a/public/app/features/alerting/unified/utils/misc.ts +++ b/public/app/features/alerting/unified/utils/misc.ts @@ -129,6 +129,10 @@ export function unquoteWithUnescape(input: string) { .replace(/\\"/g, '"'); } +export function isQuoted(input: string) { + return input.startsWith('"') && input.endsWith('"'); +} + export function makeRuleBasedSilenceLink(alertManagerSourceName: string, rule: CombinedRule) { // we wrap the name of the alert with quotes since it might contain starting and trailing spaces const labels: Labels = {