Add webhook context during condition evaluation

[mwelcker]

NetBox version

v3.4.6

Feature type

Change to existing functionality

Proposed functionality

When using conditions in webhooks, you are limited to the data attribute. To improve the flexibility with webhook conditions, it would be better to pass the whole webhook context to the evaluation. I suggest an extension of the process_webhook() function.

I currently see two options, one that would impact existing implementations and one that would not:

Here is an example of the current conditions implementation.

{
    "attr": "status.value",
    "value": "active"
}

Implementation 1 - with side effects to existing implementation

# New Implementation
    # Prepare context data for headers & body templates
    context = {
        'event': dict(ObjectChangeActionChoices)[event].lower(),
        'timestamp': timestamp,
        'model': model_name,
        'username': username,
        'request_id': request_id,
        'data': data,
        'snapshots': snapshots,
    }

    # Evaluate webhook conditions (if any)
    if not eval_conditions(webhook, context):
        return

Instead of passing only data, we could pass the whole context to the eval_conditions() function. We now need to change our conditions to:

{
    "attr": "data.status.value",
    "value": "active"
}

Implementation 2 - side-effect free

Instead of passing the context, we could add additional keys to the data object to stay compatible with existing implementations.

# New Implementation
    # Prepare context data for headers & body templates
    context = {
        'event': dict(ObjectChangeActionChoices)[event].lower(),
        'timestamp': timestamp,
        'model': model_name,
        'username': username,
        'request_id': request_id,
        'data': data,
        'snapshots': snapshots,
    }

    # Evaluate webhook conditions (if any)
    if not eval_conditions(webhook, data.update(context) ): # <- We add the entries from the context to the data dict
        return

I can create the PR, but I would like to know, what you think of my idea. Are there any other side effects?

Use case

Conditions could easily use the whole webhook context for evaluation, not just the plain data object. This is helpful, If you want to trigger webhooks based on snapshots.prechange or snapshots.postchange.

Database changes

No changes are required.

External dependencies

none

[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.

[jeremystretch]

The first proposed implementation above is not ideal because it effects a breaking change for existing webhooks. The second is untenable because it risks overwriting object data with context data (name collisions).

Instead, we could designate a specific _context key to reference the context data. This ensures that we won't accidentally conflict with any actual object attributes. Then you could do something like:

{
  "attr": "_context.username",
  "value": "johnny"
}

I think this still needs some discussion about what specific context to include. Snapshots are referenced in the cited use case; what about the user attributes, request ID, and so on?

[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.

[Jypear]

I just thought I would chime in on this as it matches a particular piece of functionality we were looking at that currently isn't possible.

The main use case we were after was the ability to reference the snapshot data so we can trigger a webhook only when specific fields changes.
For the most part this is already discussed, but look at the code/docs a bit more I've realised there would be no way to compare prechange and postchange in a single expression because the value field must be a valid string or integer type etc.

Let me know if this is beyond the scope of discussion here and needs a new issue raising, but would it be also possible to add an additional feature to this feature? Could we add a function to the condition parsing which will make the conditional parsing for a value key be treated the same as the attr key?

An example would be:

In netbox/extras/conditions.py the Condition class init method has an additional default field something like "model_value" (or something more relevant):

    def __init__(self, attr, value, op=EQ, negate=False, model_value=False):
        if op not in self.OPERATORS:
            raise ValueError(f"Unknown operator: {op}. Must be one of: {', '.join(self.OPERATORS)}")
        if type(value) not in self.TYPES:
            raise ValueError(f"Unsupported value type: {type(value)}")
        if op not in self.TYPES[type(value)]:
            raise ValueError(f"Invalid type for {op} operation: {type(value)}")

        self.attr = attr
        self.value = value
        self.eval_func = getattr(self, f'eval_{op}')
        self.negate = negate
        self.model_value = model_value

# Added model_value into init function

Then in the eval method if this model_value is set to true, then treat the value field the same as the attr field, using the nested _get function that the to get the value from a field.

    def eval(self, data):
        """
        Evaluate the provided data to determine whether it matches the condition.
        """
        def _get(obj, key):
            if isinstance(obj, list):
                return [dict.get(i, key) for i in obj]

            return dict.get(obj, key)

        # insert small code to overwrite the original value with the value from the model
        if self.model_value:
        # if the boolean is true use the same logic as the attr
            try:
                self.value = functools.reduce(_get, self.value.split('.'), data)
            except TypeError:
                self.value = "" # Empty string because None is not supported

        try:
            value = functools.reduce(_get, self.attr.split('.'), data)
        except TypeError:
            # Invalid key path
            value = None
        result = self.eval_func(value)

        if self.negate:
            return not result
        return result

This was only from roughly looking, there may be a better way to do it. The result would be that we can now compare values between different fields in the conditional logic.

This would come in handy when trying to compare a prechange and postchange in the snapshot but it may have other uses if a user wanted to compare something between two fields in a model and trigger a webhook based on that difference.
Example of condition:

{
  "attr": "_context.snapshot.prechange.status",
  "value": "_context.snapshot.postchange.status"
  "model_value": true
  "negate": true
}

So if the prechange does not equal the post change then send the webhook.

Happy to raise a different issue discussing this if needs be.

To bring the discussion back to the original request however, for myself personally, the only other useful context data I can see is the username (which is originally pointed by Jeremy) like triggering a webhook for a certain API account would be useful.

Potentially the timestamp could be useful, I noticed the conditional logic supports regex, and regex could be used to parse the timestamp to only send a webhook in a particular time of day or something along those lines but that's clutching at straws.

Overall having an ability to run conditions against snapshots, usernames and timestamps would be useful for us and not all context data may need to be included if it causes issues in the long run.

I would be more than happy to create a PR for this if the ideas above are still open and maintainers are open to the change?

[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.

[github-actions[bot]]

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.