influxdata / influxdata/influxdb

NotificationRules - GET with tag filter returns also rules that have no defined tags

Open
#23,782 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
31.7k
Forks
3.7k
Avg merge
13h 37m
Merged PRs (30d)
8

Description

Steps to reproduce:
List the minimal actions needed to reproduce the behavior.

  1. Create a set of NotificationRules in which some rules have defined tagRules e.g. tagRules: [{key: 'bird', value: 'jay', operator: 'equal'}] and in which some rules have tagRules: undefined.
  2. Send a GET request to /notificationRules with the query param ?tag=bird:jay

Expected behavior:
Expected the result set in the response to contain only those rules that have the tagRule [{key: 'bird', value: 'jay', operator: 'equal'}]

Actual behavior:
The result set contains rules with the tagRule [{key: 'bird', value: 'jay'}] but also rules that have NO DEFINED TAGRULES property.

partial result set from console log - note first record has no tagRules property.

resp.body {
   "notificationRules": [
...
      {
         "id": "0a17441fa27a8000",
         "name": "Hfbb68d5",
         "description": "ifcq3f6zv9o1v17k",
         "endpointID": "0a174419c0295000",
         "orgID": "34f7c8beea197115",
         "ownerID": "0a17441857695000",
         "taskID": "0a17441fa4bb6000",
         "sleepUntil": "2022-10-06T11:34:13.618Z",
         "every": "10m",
         "offset": "0s",
         "runbookLink": "/api/v2private/notebooks?orgID=9c5955fc99a60b8f",
         "statusRules": [
            {
               "currentLevel": "CRIT",
               "previousLevel": "ANY"
            }
         ],
         "limit": 3,
         "limitEvery": 3600,
         "createdAt": "2022-10-06T11:34:13.897611229Z",
         "updatedAt": "2022-10-06T11:34:13.897611229Z",
         "type": "http",
         "labels": [],
         "links": {
            "self": "/api/v2/notificationRules/0a17441fa27a8000",
            "labels": "/api/v2/notificationRules/0a17441fa27a8000/labels",
            "members": "/api/v2/notificationRules/0a17441fa27a8000/members",
            "owners": "/api/v2/notificationRules/0a17441fa27a8000/owners",
            "query": "/api/v2/notificationRules/0a17441fa27a8000/query"
         },
         "status": "active",
         "latestCompleted": "2022-10-06T11:34:13Z",
         "latestScheduled": "2022-10-06T11:34:13Z"
      },
      {
         "id": "0a17441ffbba8000",
         "name": "H83c66e4",
         "description": "y4s81uwfwch479e0",
         "endpointID": "0a174419c0295000",
         "orgID": "34f7c8beea197115",
         "ownerID": "0a17441857695000",
         "taskID": "0a17441ffe7b6000",
         "sleepUntil": "2022-10-06T11:34:13.974Z",
         "every": "10m",
         "offset": "0s",
         "runbookLink": "/api/v2private/notebooks?orgID=9c5955fc99a60b8f",
         "tagRules": [
            {
               "key": "bird",
               "value": "jay",
               "operator": "equal"
            }
         ],
         "statusRules": [
            {
               "currentLevel": "CRIT",
               "previousLevel": "ANY"
            }
         ],
         "limit": 3,
         "limitEvery": 3600,
         "createdAt": "2022-10-06T11:34:14.254648434Z",
         "updatedAt": "2022-10-06T11:34:14.254648434Z",
         "type": "http",
         "labels": [],
         "links": {
            "self": "/api/v2/notificationRules/0a17441ffbba8000",
            "labels": "/api/v2/notificationRules/0a17441ffbba8000/labels",
            "members": "/api/v2/notificationRules/0a17441ffbba8000/members",
            "owners": "/api/v2/notificationRules/0a17441ffbba8000/owners",
            "query": "/api/v2/notificationRules/0a17441ffbba8000/query"
         },
         "status": "active",
         "latestCompleted": "2022-10-06T11:34:14Z",
         "latestScheduled": "2022-10-06T11:34:14Z"
      },
      {
         "id": "0a17442055fa8000",
         "name": "P2e4598e",
         "description": "a55odxcu5l907msv",
         "endpointID": "0a174419c0295000",
         "orgID": "34f7c8beea197115",
         "ownerID": "0a17441857695000",
         "taskID": "0a17442058fb6000",
         "sleepUntil": "2022-10-06T11:34:14.336Z",
         "every": "10m",
         "offset": "0s",
         "runbookLink": "/api/v2private/notebooks?orgID=9c5955fc99a60b8f",
         "tagRules": [
            {
               "key": "bird",
               "value": "jay",
               "operator": "equal"
            }
...

Digging in Code

I'm a newbie to GOLANG so this may not be correct (relying on other C lang family experience), but looking into notification/rule/rule.go I see the following function.

// MatchesTags returns true if the Rule matches all of the tags
func (b *Base) MatchesTags(tags []influxdb.Tag) bool {
        if len(tags) == 0 {
                return true
        }
        // for each tag in NR
        // if there exists
        // a key value match with operator == equal in tags
        // or
        // a key match with a value mismatch with operator == notequal in tags
        // then true

        for _, NRtag := range b.TagRules {
                isNRTagInFilterTags := false

                for _, filterTag := range tags {
                        if NRtag.Key == filterTag.Key {
                                if NRtag.Operator == influxdb.Equal && NRtag.Value == filterTag.Value {
                                        isNRTagInFilterTags = true
                                }
                                if NRtag.Operator == influxdb.NotEqual && NRtag.Value != filterTag.Value {
                                        isNRTagInFilterTags = true
                                }
                        }
                }
                if !isNRTagInFilterTags {
                        return false
                }
        }
        return true
}

What happens to the for _, NRtag := range b.TagRules block when b has no TagRules? Isn't the block skipped and the function returns true?

Environment info:

Testing in K8SIDPE remocal.

latest commit

commit a980408747a2fe2493b06c28957cf9de94256a83 (HEAD -> master, origin/master, origin/HEAD)
Author: opeyemi <solomonope@users.noreply.github.com>
Date:   Thu Oct 6 09:50:03 2022 +0200

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in notification/rule/rule.go and inspect Base.MatchesTags alongside the GET /notificationRules tag-filter path. Reproduce the case with rules that have matching tagRules and rules with no tagRules, then add or update coverage so the response contains only rules matching the requested tag. Done means the filter excludes rules without defined matching tags.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.