From f3cc1f7700aa7e841754b0f552d9dece7c6bc02f Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 7 Jan 2025 16:59:58 +0100 Subject: [PATCH] Alerting: Allow notification policy filters to match quoted matchers (#98525) --- .../notification-policies/Filters.test.ts | 47 +++++++++++++++++++ .../notification-policies/Filters.tsx | 16 +++++-- .../alerting/unified/utils/matchers.test.ts | 11 +++++ .../alerting/unified/utils/matchers.ts | 4 ++ 4 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 public/app/features/alerting/unified/components/notification-policies/Filters.test.ts diff --git a/public/app/features/alerting/unified/components/notification-policies/Filters.test.ts b/public/app/features/alerting/unified/components/notification-policies/Filters.test.ts new file mode 100644 index 00000000000..74b51b772bc --- /dev/null +++ b/public/app/features/alerting/unified/components/notification-policies/Filters.test.ts @@ -0,0 +1,47 @@ +import { MatcherOperator, ObjectMatcher } from 'app/plugins/datasource/alertmanager/types'; + +import { findRoutesByMatchers } from './Filters'; + +describe('findRoutesByMatchers', () => { + it('should match even if keys or values are quoted', () => { + const routes = [ + { id: '0', matchers: ['foo=bar'] }, + { id: '0', matchers: ['foo="bar"'] }, + { id: '0', matchers: ['"foo"=bar'] }, + { id: '0', matchers: ['"foo"="bar"'] }, + ]; + + const matchers: ObjectMatcher[] = [ + ['foo', MatcherOperator.equal, 'bar'], + ['foo', MatcherOperator.equal, '"bar"'], + ['"foo"', MatcherOperator.equal, 'bar'], + ['"foo"', MatcherOperator.equal, '"bar"'], + ]; + + routes.forEach((route) => { + matchers.forEach((matcher) => { + expect(findRoutesByMatchers(route, [matcher])).toBe(true); + }); + }); + }); + + it('should match even if keys or values are quoted with special characters', () => { + const routes = [ + { id: '0', matchers: ['foo="bar baz"'] }, + { id: '0', matchers: ['"foo"="bar baz"'] }, + ]; + + const matchers: ObjectMatcher[] = [ + ['foo', MatcherOperator.equal, 'bar baz'], + ['foo', MatcherOperator.equal, '"bar baz"'], + ['"foo"', MatcherOperator.equal, 'bar baz'], + ['"foo"', MatcherOperator.equal, '"bar baz"'], + ]; + + matchers.forEach((matcher) => { + routes.forEach((route) => { + expect(findRoutesByMatchers(route, [matcher])).toBe(true); + }); + }); + }); +}); diff --git a/public/app/features/alerting/unified/components/notification-policies/Filters.tsx b/public/app/features/alerting/unified/components/notification-policies/Filters.tsx index dfb6b3be323..845afd2b2a0 100644 --- a/public/app/features/alerting/unified/components/notification-policies/Filters.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/Filters.tsx @@ -14,6 +14,7 @@ import { normalizeMatchers, parsePromQLStyleMatcherLoose, parsePromQLStyleMatcherLooseSafe, + unquoteIfRequired, } from '../../utils/matchers'; interface NotificationPoliciesFilterProps { @@ -174,11 +175,20 @@ export function findRoutesMatchingPredicate( } export function findRoutesByMatchers(route: RouteWithID, labelMatchersFilter: ObjectMatcher[]): boolean { - const routeMatchers = normalizeMatchers(route); - - return labelMatchersFilter.every((filter) => routeMatchers.some((matcher) => isEqual(filter, matcher))); + const filters = labelMatchersFilter.map(unquoteMatchersIfRequired); + const routeMatchers = normalizeMatchers(route).map(unquoteMatchersIfRequired); + return filters.every((filter) => routeMatchers.some((matcher) => isEqual(filter, matcher))); } +/** + * This function is mostly used for decoding matchers like "test"="test" into test=test to remove quotes when they're not needed. + * This mimicks the behaviour in Alertmanager where it decodes the label matchers in the same way and makes searching for policies + * easier in case the label keys or values are quoted when they shouldn't really be. + */ +const unquoteMatchersIfRequired = ([key, operator, value]: ObjectMatcher): ObjectMatcher => { + return [unquoteIfRequired(key), operator, unquoteIfRequired(value)]; +}; + const getNotificationPoliciesFilters = (searchParams: URLSearchParams) => ({ queryString: searchParams.get('queryString') ?? undefined, contactPoint: searchParams.get('contactPoint') ?? undefined, diff --git a/public/app/features/alerting/unified/utils/matchers.test.ts b/public/app/features/alerting/unified/utils/matchers.test.ts index 69487604a8c..f89e3fa8230 100644 --- a/public/app/features/alerting/unified/utils/matchers.test.ts +++ b/public/app/features/alerting/unified/utils/matchers.test.ts @@ -13,6 +13,7 @@ import { parseQueryParamMatchers, quoteWithEscape, quoteWithEscapeIfRequired, + unquoteIfRequired, unquoteWithUnescape, } from './matchers'; @@ -129,6 +130,16 @@ describe('unquoteWithUnescape', () => { }); }); +describe('unquoteIfRequired', () => { + it('should unquote strings with no special character', () => { + expect(unquoteIfRequired('"test"')).toBe('test'); + }); + + it('should not unquote strings with special character', () => { + expect(unquoteIfRequired('"test this"')).toBe('"test this"'); + }); +}); + describe('isPromQLStyleMatcher', () => { it('should detect promQL style matcher', () => { expect(isPromQLStyleMatcher('{ foo=bar }')).toBe(true); diff --git a/public/app/features/alerting/unified/utils/matchers.ts b/public/app/features/alerting/unified/utils/matchers.ts index 081f9c1f8e1..17e60b2915a 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -183,6 +183,10 @@ export function quoteWithEscapeIfRequired(input: string) { return shouldQuote ? quoteWithEscape(input) : input; } +export function unquoteIfRequired(input: string) { + return quoteWithEscapeIfRequired(unquoteWithUnescape(input)); +} + export const encodeMatcher = ({ name, operator, value }: MatcherFieldValue) => { const encodedLabelName = quoteWithEscapeIfRequired(name); const encodedLabelValue = quoteWithEscape(value);