Alerting: Apply negative matchers for route matching (#77292)

public/app/features/alerting/unified/routeGroupsMatcher.ts

@@ -32,17 +32,13 @@ export const routeGroupsMatcher = {
 
     instancesToMatch.forEach((instance) => {
       const matchingRoutes = findMatchingRoutes(normalizedRootRoute, Object.entries(instance));
-      matchingRoutes.forEach(({ route, details, labelsMatch }) => {
+      matchingRoutes.forEach(({ route, labelsMatch }) => {
-        // Only to convert Label[] to Labels[] - needs better approach
-        const matchDetails = new Map(
-          Array.from(details.entries()).map(([matcher, labels]) => [matcher, Object.fromEntries(labels)])
-        );
-
         const currentRoute = result.get(route.id);
+
         if (currentRoute) {
-          currentRoute.push({ instance, matchDetails, labelsMatch });
+          currentRoute.push({ instance, labelsMatch });
         } else {
-          result.set(route.id, [{ instance, matchDetails, labelsMatch }]);
+          result.set(route.id, [{ instance, labelsMatch }]);
         }
       });
     });

public/app/features/alerting/unified/utils/__snapshots__/notification-policies.test.ts.snap

@@ -0,0 +1,56 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`matchLabels should match with non-equal matchers 1`] = `
+Map {
+  [
+    "team",
+    "operations",
+  ] => {
+    "match": true,
+    "matcher": [
+      "team",
+      "=",
+      "operations",
+    ],
+  },
+}
+`;
+
+exports[`matchLabels should match with non-matching matchers 1`] = `
+Map {
+  [
+    "team",
+    "operations",
+  ] => {
+    "match": true,
+    "matcher": [
+      "team",
+      "=",
+      "operations",
+    ],
+  },
+}
+`;
+
+exports[`matchLabels should not match with a set of matchers 1`] = `
+Map {
+  [
+    "team",
+    "operations",
+  ] => {
+    "match": true,
+    "matcher": [
+      "team",
+      "=",
+      "operations",
+    ],
+  },
+  [
+    "foo",
+    "bar",
+  ] => {
+    "match": false,
+    "matcher": null,
+  },
+}
+`;

public/app/features/alerting/unified/utils/matchers.ts

@@ -109,34 +109,3 @@ export const normalizeMatchers = (route: Route): ObjectMatcher[] => {
 };
 
 export type Label = [string, string];
-type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean;
-const OperatorFunctions: Record<MatcherOperator, OperatorPredicate> = {
-  [MatcherOperator.equal]: (lv, mv) => lv === mv,
-  [MatcherOperator.notEqual]: (lv, mv) => lv !== mv,
-  [MatcherOperator.regex]: (lv, mv) => new RegExp(mv).test(lv),
-  [MatcherOperator.notRegex]: (lv, mv) => !new RegExp(mv).test(lv),
-};
-
-function isLabelMatch(matcher: ObjectMatcher, label: Label) {
-  const [labelKey, labelValue] = label;
-  const [matcherKey, operator, matcherValue] = matcher;
-
-  // not interested, keys don't match
-  if (labelKey !== matcherKey) {
-    return false;
-  }
-
-  const matchFunction = OperatorFunctions[operator];
-  if (!matchFunction) {
-    throw new Error(`no such operator: ${operator}`);
-  }
-
-  return matchFunction(labelValue, matcherValue);
-}
-
-// check if every matcher returns "true" for the set of labels
-export function labelsMatchObjectMatchers(matchers: ObjectMatcher[], labels: Label[]) {
-  return matchers.every((matcher) => {
-    return labels.some((label) => isLabelMatch(matcher, label));
-  });
-}

public/app/features/alerting/unified/utils/notification-policies.test.ts

@@ -5,6 +5,7 @@ import {
   normalizeRoute,
   getInheritedProperties,
   computeInheritedTree,
+  matchLabels,
 } from './notification-policies';
 
 import 'core-js/stable/structured-clone';
@@ -22,6 +23,7 @@ describe('findMatchingRoutes', () => {
     routes: [
       {
         receiver: 'A',
+        object_matchers: [['team', MatcherOperator.equal, 'operations']],
         routes: [
           {
             receiver: 'B1',
@@ -29,10 +31,9 @@ describe('findMatchingRoutes', () => {
           },
           {
             receiver: 'B2',
-            object_matchers: [['region', MatcherOperator.notEqual, 'europe']],
+            object_matchers: [['region', MatcherOperator.equal, 'nasa']],
           },
         ],
-        object_matchers: [['team', MatcherOperator.equal, 'operations']],
       },
       {
         receiver: 'C',
@@ -55,6 +56,19 @@ describe('findMatchingRoutes', () => {
     expect(matches[0].route).toHaveProperty('receiver', 'A');
   });
 
+  it('should match route with negative matchers', () => {
+    const policiesWithNegative = {
+      ...policies,
+      routes: policies.routes?.concat({
+        receiver: 'D',
+        object_matchers: [['name', MatcherOperator.notEqual, 'gilles']],
+      }),
+    };
+    const matches = findMatchingRoutes(policiesWithNegative, [['name', 'konrad']]);
+    expect(matches).toHaveLength(1);
+    expect(matches[0].route).toHaveProperty('receiver', 'D');
+  });
+
   it('should match child route of matching parent', () => {
     const matches = findMatchingRoutes(policies, [
       ['team', 'operations'],
@@ -378,3 +392,47 @@ describe('normalizeRoute', () => {
     expect(normalized).not.toHaveProperty('match_re');
   });
 });
+
+describe('matchLabels', () => {
+  it('should match with non-matching matchers', () => {
+    const result = matchLabels(
+      [
+        ['foo', MatcherOperator.equal, ''],
+        ['team', MatcherOperator.equal, 'operations'],
+      ],
+      [['team', 'operations']]
+    );
+
+    expect(result).toHaveProperty('matches', true);
+    expect(result.labelsMatch).toMatchSnapshot();
+  });
+
+  it('should match with non-equal matchers', () => {
+    const result = matchLabels(
+      [
+        ['foo', MatcherOperator.notEqual, 'bar'],
+        ['team', MatcherOperator.equal, 'operations'],
+      ],
+      [['team', 'operations']]
+    );
+
+    expect(result).toHaveProperty('matches', true);
+    expect(result.labelsMatch).toMatchSnapshot();
+  });
+
+  it('should not match with a set of matchers', () => {
+    const result = matchLabels(
+      [
+        ['foo', MatcherOperator.notEqual, 'bar'],
+        ['team', MatcherOperator.equal, 'operations'],
+      ],
+      [
+        ['team', 'operations'],
+        ['foo', 'bar'],
+      ]
+    );
+
+    expect(result).toHaveProperty('matches', false);
+    expect(result.labelsMatch).toMatchSnapshot();
+  });
+});

public/app/features/alerting/unified/utils/notification-policies.ts

@@ -11,111 +11,84 @@ import { Labels } from 'app/types/unified-alerting-dto';
 
 import { Label, normalizeMatchers } from './matchers';
 
-type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean;
+// 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
-const OperatorFunctions: Record<MatcherOperator, OperatorPredicate> = {
-  [MatcherOperator.equal]: (lv, mv) => lv === mv,
-  [MatcherOperator.notEqual]: (lv, mv) => lv !== mv,
-  [MatcherOperator.regex]: (lv, mv) => Boolean(lv.match(new RegExp(mv))),
-  [MatcherOperator.notRegex]: (lv, mv) => !Boolean(lv.match(new RegExp(mv))),
-};
-
-function isLabelMatch(matcher: ObjectMatcher, label: Label) {
-  const [labelKey, labelValue] = label;
-  const [matcherKey, operator, matcherValue] = matcher;
-
-  // not interested, keys don't match
-  if (labelKey !== matcherKey) {
-    return false;
-  }
-
-  const matchFunction = OperatorFunctions[operator];
-  if (!matchFunction) {
-    throw new Error(`no such operator: ${operator}`);
-  }
-
-  return matchFunction(labelValue, matcherValue);
-}
-
 interface LabelMatchResult {
   match: boolean;
-  matchers: ObjectMatcher[];
+  matcher: ObjectMatcher | null;
 }
 
+type LabelsMatch = Map<Label, LabelMatchResult>;
+
 interface MatchingResult {
   matches: boolean;
-  details: Map<ObjectMatcher, Label[]>;
+  labelsMatch: LabelsMatch;
-  labelsMatch: Map<Label, LabelMatchResult>;
 }
 
-// check if every matcher returns "true" for the set of labels
+// returns a match results for given set of matchers (from a policy for instance) and a set of labels
-function matchLabels(matchers: ObjectMatcher[], labels: Label[]): MatchingResult {
+export function matchLabels(matchers: ObjectMatcher[], labels: Label[]): MatchingResult {
-  const details = new Map<ObjectMatcher, Label[]>();
+  const matches = matchLabelsSet(matchers, labels);
 
-  // If a policy has no matchers it still can be a match, hence matchers can be empty and match can be true
+  // create initial map of label => match result
-  // So we cannot use empty array of matchers as an indicator of no match
+  const labelsMatch: LabelsMatch = new Map(labels.map((label) => [label, { match: false, matcher: null }]));
-  const labelsMatch = new Map<Label, { match: boolean; matchers: ObjectMatcher[] }>(
-    labels.map((label) => [label, { match: false, matchers: [] }])
-  );
 
-  const matches = matchers.every((matcher) => {
+  // for each matcher, check which label it matched for
-    const matchingLabels = labels.filter((label) => isLabelMatch(matcher, label));
+  matchers.forEach((matcher) => {
+    const matchingLabel = labels.find((label) => isLabelMatch(matcher, label));
 
-    matchingLabels.forEach((label) => {
+    // record that matcher for the label
-      const labelMatch = labelsMatch.get(label);
+    if (matchingLabel) {
-      // The condition is just to satisfy TS. The map should have all the labels due to the previous map initialization
+      labelsMatch.set(matchingLabel, {
-      if (labelMatch) {
+        match: true,
-        labelMatch.match = true;
+        matcher,
-        labelMatch.matchers.push(matcher);
+      });
     }
   });
 
-    if (matchingLabels.length === 0) {
+  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;
     }
-
+  }
-    details.set(matcher, matchingLabels);
+  return true;
-    return matchingLabels.length > 0;
-  });
-
-  return { matches, details, labelsMatch };
 }
 
 export interface AlertInstanceMatch {
   instance: Labels;
-  matchDetails: Map<ObjectMatcher, Labels>;
+  labelsMatch: LabelsMatch;
-  labelsMatch: Map<Label, LabelMatchResult>;
 }
 
 export interface RouteMatchResult<T extends Route> {
   route: T;
-  details: Map<ObjectMatcher, Label[]>;
+  labelsMatch: LabelsMatch;
-  labelsMatch: Map<Label, LabelMatchResult>;
 }
 
 // Match does a depth-first left-to-right search through the route tree
 // and returns the matching routing nodes.
 
 // If the current node is not a match, return nothing
-// const normalizedMatchers = normalizeMatchers(root);
 // Normalization should have happened earlier in the code
-function findMatchingRoutes<T extends Route>(root: T, labels: Label[]): Array<RouteMatchResult<T>> {
+function findMatchingRoutes<T extends Route>(route: T, labels: Label[]): Array<RouteMatchResult<T>> {
-  let matches: Array<RouteMatchResult<T>> = [];
+  let childMatches: Array<RouteMatchResult<T>> = [];
 
   // If the current node is not a match, return nothing
-  const matchResult = matchLabels(root.object_matchers ?? [], labels);
+  const matchResult = matchLabels(route.object_matchers ?? [], labels);
   if (!matchResult.matches) {
     return [];
   }
 
   // If the current node matches, recurse through child nodes
-  if (root.routes) {
+  if (route.routes) {
-    for (let index = 0; index < root.routes.length; index++) {
+    for (const child of route.routes) {
-      let child = root.routes[index];
       let matchingChildren = findMatchingRoutes(child, labels);
       // TODO how do I solve this typescript thingy? It looks correct to me /shrug
       // @ts-ignore
-      matches = matches.concat(matchingChildren);
+      childMatches = childMatches.concat(matchingChildren);
       // we have matching children and we don't want to continue, so break here
       if (matchingChildren.length && !child.continue) {
         break;
@@ -124,11 +97,11 @@ function findMatchingRoutes<T extends Route>(root: T, labels: Label[]): Array<Ro
   }
 
   // If no child nodes were matches, the current node itself is a match.
-  if (matches.length === 0) {
+  if (childMatches.length === 0) {
-    matches.push({ route: root, details: matchResult.details, labelsMatch: matchResult.labelsMatch });
+    childMatches.push({ route, labelsMatch: matchResult.labelsMatch });
   }
 
-  return matches;
+  return childMatches;
 }
 
 // This is a performance improvement to normalize matchers only once and use the normalized version later on
@@ -249,4 +222,48 @@ export function computeInheritedTree<T extends Route>(parent: T): T {
   };
 }
 
-export { findMatchingAlertGroups, findMatchingRoutes, getInheritedProperties };
+type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean;
+const OperatorFunctions: Record<MatcherOperator, OperatorPredicate> = {
+  [MatcherOperator.equal]: (lv, mv) => lv === mv,
+  [MatcherOperator.notEqual]: (lv, mv) => lv !== mv,
+  [MatcherOperator.regex]: (lv, mv) => new RegExp(mv).test(lv),
+  [MatcherOperator.notRegex]: (lv, mv) => !new RegExp(mv).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 {
+  let [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 { findMatchingAlertGroups, findMatchingRoutes, getInheritedProperties, isLabelMatchInSet };