From bd9b0d6d82856c33f31f8872c62a557b95bc3f9a Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Thu, 13 Jul 2023 14:24:25 +0200 Subject: [PATCH] Alerting: Support spaces in alert names for creating silence links (#71280) --- .../alerting/unified/api/alertmanagerApi.ts | 3 ++- .../unified/utils/alertmanager.test.ts | 2 +- .../alerting/unified/utils/matchers.ts | 17 ++++++++-------- .../alerting/unified/utils/misc.test.ts | 20 ++++++++++++++++++- .../features/alerting/unified/utils/misc.ts | 8 ++++++++ 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/public/app/features/alerting/unified/api/alertmanagerApi.ts b/public/app/features/alerting/unified/api/alertmanagerApi.ts index bd83e5071bd..793844309a3 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -10,6 +10,7 @@ import { } from '../../../../plugins/datasource/alertmanager/types'; import { matcherToOperator } from '../utils/alertmanager'; import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; +import { wrapWithQuotes } from '../utils/misc'; import { alertingApi } from './alertingApi'; @@ -39,7 +40,7 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ // TODO Add support for active, silenced, inhibited, unprocessed filters const filterMatchers = filter?.matchers ?.filter((matcher) => matcher.name && matcher.value) - .map((matcher) => `${matcher.name}${matcherToOperator(matcher)}${matcher.value}`); + .map((matcher) => `${matcher.name}${matcherToOperator(matcher)}${wrapWithQuotes(matcher.value)}`); const { silenced, inhibited, unprocessed, active } = filter || {}; diff --git a/public/app/features/alerting/unified/utils/alertmanager.test.ts b/public/app/features/alerting/unified/utils/alertmanager.test.ts index 51b09740332..d8f3cbe59c1 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.test.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.test.ts @@ -27,7 +27,7 @@ describe('Alertmanager utils', () => { }); expect(parseMatcher('foo!~ bar')).toEqual({ name: 'foo', - value: 'bar', + value: ' bar', isRegex: true, isEqual: false, }); diff --git a/public/app/features/alerting/unified/utils/matchers.ts b/public/app/features/alerting/unified/utils/matchers.ts index 76a98bc6eab..acf751a1109 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -12,23 +12,22 @@ const matcherOperators = [ ]; export function parseMatcher(matcher: string): Matcher { - const trimmed = matcher.trim(); - if (trimmed.startsWith('{') && trimmed.endsWith('}')) { - throw new Error(`PromQL matchers not supported yet, sorry! PromQL matcher found: ${trimmed}`); + if (matcher.startsWith('{') && matcher.endsWith('}')) { + throw new Error(`PromQL matchers not supported yet, sorry! PromQL matcher found: ${matcher}`); } const operatorsFound = matcherOperators - .map((op): [MatcherOperator, number] => [op, trimmed.indexOf(op)]) + .map((op): [MatcherOperator, number] => [op, matcher.indexOf(op)]) .filter(([_, idx]) => idx > -1) .sort((a, b) => a[1] - b[1]); if (!operatorsFound.length) { - throw new Error(`Invalid matcher: ${trimmed}`); + throw new Error(`Invalid matcher: ${matcher}`); } const [operator, idx] = operatorsFound[0]; - const name = trimmed.slice(0, idx).trim(); - const value = trimmed.slice(idx + operator.length).trim(); + const name = matcher.slice(0, idx).trim(); + const value = matcher.slice(idx + operator.length); if (!name) { - throw new Error(`Invalid matcher: ${trimmed}`); + throw new Error(`Invalid matcher: ${matcher}`); } return { @@ -41,7 +40,7 @@ export function parseMatcher(matcher: string): Matcher { // Parses a list of entries like like "['foo=bar', 'baz=~bad*']" into SilenceMatcher[] export function parseQueryParamMatchers(matcherPairs: string[]): Matcher[] { - const parsedMatchers = matcherPairs.filter((x) => !!x.trim()).map((x) => parseMatcher(x.trim())); + const parsedMatchers = matcherPairs.filter((x) => !!x.trim()).map((x) => parseMatcher(x)); // Due to migration, old alert rules might have a duplicated alertname label // To handle that case want to filter out duplicates and make sure there are only unique labels diff --git a/public/app/features/alerting/unified/utils/misc.test.ts b/public/app/features/alerting/unified/utils/misc.test.ts index e07d8699771..e4aaf53b28e 100644 --- a/public/app/features/alerting/unified/utils/misc.test.ts +++ b/public/app/features/alerting/unified/utils/misc.test.ts @@ -1,4 +1,4 @@ -import { sortAlerts } from 'app/features/alerting/unified/utils/misc'; +import { sortAlerts, wrapWithQuotes, escapeQuotes } from 'app/features/alerting/unified/utils/misc'; import { SortOrder } from 'app/plugins/panel/alertlist/types'; import { Alert } from 'app/types/unified-alerting'; import { GrafanaAlertState } from 'app/types/unified-alerting-dto'; @@ -33,6 +33,24 @@ function permute(inputArray: any[]): any[] { }, []); } +describe('wrapWithQuotes', () => { + it('should work as expected', () => { + expect(wrapWithQuotes('"hello, world!"')).toBe('\\"hello, world!\\"'); + expect(wrapWithQuotes('hello, world!')).toBe('"hello, world!"'); + expect(wrapWithQuotes('hello, "world"!')).toBe('"hello, \\"world\\"!"'); + expect(wrapWithQuotes('"hello""')).toBe('\\"hello\\"\\"'); + }); +}); + +describe('escapeQuotes', () => { + it('should escape all quotes', () => { + expect(escapeQuotes('"hello, world!"')).toBe('\\"hello, world!\\"'); + expect(escapeQuotes('hello, world!')).toBe('hello, world!'); + expect(escapeQuotes('hello, "world"!')).toBe('hello, \\"world\\"!'); + expect(escapeQuotes('hello"')).toBe('hello\\"'); + }); +}); + describe('Unified Altering misc', () => { describe('sortAlerts', () => { describe('when using any sortOrder with a list of alert instances', () => { diff --git a/public/app/features/alerting/unified/utils/misc.ts b/public/app/features/alerting/unified/utils/misc.ts index 04eaae437ee..5373aeb3c88 100644 --- a/public/app/features/alerting/unified/utils/misc.ts +++ b/public/app/features/alerting/unified/utils/misc.ts @@ -102,7 +102,15 @@ export function makeAMLink(path: string, alertManagerName?: string, options?: UR return `${path}?${search.toString()}`; } +export const escapeQuotes = (input: string) => input.replace(/\"/g, '\\"'); + +export function wrapWithQuotes(input: string) { + const alreadyWrapped = input.startsWith('"') && input.endsWith('"'); + return alreadyWrapped ? escapeQuotes(input) : `"${escapeQuotes(input)}"`; +} + 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 = { alertname: rule.name, ...rule.labels,