Improve support for referencing tags in webhook conditions #6738

Closed
opened 2025-12-29 19:44:47 +01:00 by adam · 10 comments
Owner

Originally created by @beginin on GitHub (Jul 29, 2022).

Originally assigned to: @abhi1693 on GitHub.

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.

Originally created by @beginin on GitHub (Jul 29, 2022). Originally assigned to: @abhi1693 on GitHub. ### 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.
adam added the status: acceptedtype: feature labels 2025-12-29 19:44:47 +01:00
adam closed this issue 2025-12-29 19:44:47 +01:00
Author
Owner

@jeremystretch commented on GitHub (Aug 1, 2022):

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

@jeremystretch commented on GitHub (Aug 1, 2022): This needs more detail to be actionable. What is the proposed solution?
Author
Owner

@beginin commented on GitHub (Aug 2, 2022):

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.

@beginin commented on GitHub (Aug 2, 2022): 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.
Author
Owner

@jeremystretch commented on GitHub (Aug 10, 2022):

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.

@jeremystretch commented on GitHub (Aug 10, 2022): > 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](https://github.com/netbox-community/netbox/pull/9877#issuecomment-1201160251), we can't just hack away at an existing filter by making some dangerous assumptions.
Author
Owner

@candlerb commented on GitHub (Sep 26, 2022):

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.

@candlerb commented on GitHub (Sep 26, 2022): 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](https://github.com/netbox-community/netbox/discussions/9683#discussioncomment-3107739). ------- Aside: IMO the ultimate solution here is to use a complete, safe expression language like [jsonnet](https://jsonnet.org/), 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](https://brian-candler.medium.com/streamlining-kubernetes-application-deployment-with-jsonnet-711e15e9c665), albeit in the context of kubernetes resources.
Author
Owner

@clerie commented on GitHub (Sep 26, 2022):

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"
  }
]
@clerie commented on GitHub (Sep 26, 2022): 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: ```json { "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`](https://docs.python.org/3.8/library/functions.html#all) and [`any`](https://docs.python.org/3.8/library/functions.html#any) for this. ```json { "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. ```json { "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: ```json { "and": [ { "attr": "x.*.a", "value": "foo" }, { "attr": "x.*.b", "value": "bar" } ] } ``` Input 1: ```json [ { "a": "foo", "b": "bar" } ] ``` Input 2: ```json [ { "a": "foo" }, { "b": "bar" } ] ```
Author
Owner

@LilTrublMakr commented on GitHub (Sep 27, 2022):

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.

@LilTrublMakr commented on GitHub (Sep 27, 2022): 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.
Author
Owner

@github-actions[bot] commented on GitHub (Nov 27, 2022):

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.

@github-actions[bot] commented on GitHub (Nov 27, 2022): 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](https://github.com/netbox-community/netbox/blob/develop/CONTRIBUTING.md).
Author
Owner

@github-actions[bot] commented on GitHub (Mar 23, 2023):

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.

@github-actions[bot] commented on GitHub (Mar 23, 2023): 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](https://github.com/netbox-community/netbox/blob/develop/CONTRIBUTING.md).
Author
Owner

@kkthxbye-code commented on GitHub (Mar 23, 2023):

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.

@kkthxbye-code commented on GitHub (Mar 23, 2023): 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.
Author
Owner

@abhi1693 commented on GitHub (May 5, 2023):

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

@abhi1693 commented on GitHub (May 5, 2023): I'm going to take a run at this however I'm not currently sure if I can fix this
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#6738