Improve support for referencing tags in webhook conditions

[beginin]

NetBox version

v3.2.7

Feature type

Change to existing functionality

Proposed functionality

Execute webhook only when we add tag to object. { "attr": "tags", "value": "exempt", "op": "contains" }

Now, it does not work because tags are list of OrderedDict. It is not the bug #9797.

Use case

Only when we add tag to object (devices, ip-addresses and etc) webhook is executed.

[jeremystretch]

This needs more detail to be actionable. What is the proposed solution?

[beginin]

The proposed solution is to execute webhook when we assign the tag. For example: When we add tag "bmc" to device Netbox gets url to Ansible Tower witch execute playbook for configure baseboard management controller on server. Another example: When we add tag "voip phone" to device Netbox gets url to Ansible Tower witch executes playbook for configure IP phone. While Ansible receives inventory from Netbox which is filtered by tag.

[jeremystretch]

The proposed solution is to execute webhook when we assign the tag.

That's the goal, but you haven't proposed a workable implementation. Per my comment here, we can't just hack away at an existing filter by making some dangerous assumptions.

[candlerb]

The code already makes an assumption that in the expression foo.bar, the value of foo is a dict (it uses dict.get to reduce it). It catches the TypeError if that assumption is untrue, and returns None instead.

I propose the following: if foo is a list, then foo.bar collects the bar members of the list (i.e. we assume that if it's a list, it's a list of dicts)

--- a/netbox/extras/conditions.py
+++ b/netbox/extras/conditions.py
@@ -64,8 +64,13 @@ class Condition:
         """
         Evaluate the provided data to determine whether it matches the condition.
         """
+        def get(obj, key):
+            if isinstance(obj, list):
+                return [dict.get(item, key) for item in obj]
+            else:
+                return dict.get(obj, key)
         try:
-            value = functools.reduce(dict.get, self.attr.split('.'), data)
+            value = functools.reduce(get, self.attr.split('.'), data)
         except TypeError:
             # Invalid key path
             value = None

I don't think that adds much of a maintainability burden, given that this doesn't touch the expression evaluator per se, just the dot expansion of attr. And it enables the functionality that you thought already works.

---

Aside: IMO the ultimate solution here is to use a complete, safe expression language like jsonnet, rather than bare JSON plus some hand-made evaluation rules.

Current (with the above patch):

{
  "and": [
    {
      "attr": "mode.value",
      "value": "access"
    }, 
    {
      "attr": "tags.slug", 
      "value": "unifi"
    }
  ]
}

With JSONNET this becomes:

// ex1.libsonnet (user-supplied code)
function(data)
  data.mode.value == "access" &&
  std.length([1 for i in data.tags if i.slug == "unifi"]) > 0

// data.json
{
    "mode": {"value": "access"},
    "tags": [
        {"slug": "foo"},
        {"slug": "unifi"}
    ]
}

$ jsonnet --tla-code-file data=data.json ex1.libsonnet
true

You could pass in the whole object with pre/post snapshots, and/or pass in the request context; it could be used for other things like permissions evaluation too.

Jsonnet is a pure functional, side-effect free language, and hence safe to embed. I blogged about it here, albeit in the context of kubernetes resources.

[clerie]

If we don't wanna make existing operators do different things based on the type they are executed on, I see thee directions we can go:

Use case specific operators

We could add a specific operator just for checking if a tag exist:

{
  "attr": "tags",
  "value": "mytag",
  "op": "tags_contain_slug"
}

This can be implemented really easy like the other operators.

Generic operator for handling lists

Because the current condition implementation allows traversing dicts only, a generic way of matching on list items would be handy. We can make use of all and any for this.

{
  "attr": "tags",
  "op": "any",
  "conditions": {
     "and": [
        {
          "attr": "slug",
          "value": "mytag"
        }
     ]
  }
}

Implementing this requires some bigger changes to how conditions work currently. Either we tweak the Condition class until it works as the example above or we find a way to let this behavior be handled by another class.

The proposed syntax above can only handle lists containing dicts. How lists with non dict typed values can be implemented needs further discussion.

Glob patterns for lists

A less verbose syntax of the above could be using some kind of glob pattern for lists.

{
  "attr": "tags.*.slug",
  "value": "mytag"
}

This seem to look like it matches to current conditions syntax the most. It required big changes in the Condition class too, but the rest can stay as is.

This syntax is not restricted to lists containing dicts and can be used to match any type.

This has the drawback that logic operators don't work for dicts after the glob. The following example will be true for both of the following inputs:

{
  "and": [
    {
      "attr": "x.*.a",
      "value": "foo"
    },
    {
      "attr": "x.*.b",
      "value": "bar"
    }
  ]
}

Input 1:

[
  {
    "a": "foo",
    "b": "bar"
  }
]

Input 2:

[
  {
    "a": "foo"
  },
  {
    "b": "bar"
  }
]

[LilTrublMakr]

Though I most likely don't have a horse in this race, here are my 2 cents:

I really like the jsonette style of things being handled. It looks cleaner in my opinion and it better visualizes what is being pulled. And I like how easy it would be to be able to look for multiple tags without doing an AND operator. I don't know how it would affect the operators, but with that aside, it would break any webhook triggers that anyone has created and they would all need to be converted. As much as that would be a cool system to have, I don't think that is a safe immediate fix to the current problem. It would be awesome for a future deployment though.

I am not an experienced coder and I don't know how Netbox is set up. I applied the test that @candlerb proposed to my instance and the tags started being recognized by the webhook and passed on to my scripts without changing the condition at all. I think it is an immediate workable solution.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

[LilTrublMakr]

Not stale. Still an issue.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

[kkthxbye-code]

I'll give this another chance as I think it's an import feature, if no one picks it up soon, I'll give it a shot myself.

[abhi1693]

I'm going to take a run at this however I'm not currently sure if I can fix this