Skip to content

Closes #20003: Introduce mechanism to register callbacks for webhook context #20025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: feature
Choose a base branch
from

Conversation

jeremystretch
Copy link
Member

Closes: #20003

  • Simplify the signature of enqueue_event() to pass the request object directly (instead of the user & request ID)
  • Attach the request object to the event when it is enqueued for processing
  • Alter the signature of process_event_rules() to take a request object rather than a request ID
  • Alter the parameters recorded for send_webhook tasks
    • Replace model_name with object_type
    • Add request if present
  • Introduce the process_event_rules() function for use by plugins
    • Add webhook_callbacks to the NetBox registry
  • Extend send_webhook() to gather additional context data from registered webhook callbacks
  • Update event tests

@jeremystretch jeremystretch requested review from a team and arthanson and removed request for a team August 5, 2025 18:48
@jeremystretch jeremystretch marked this pull request as ready for review August 5, 2025 18:49
@jeremystretch
Copy link
Member Author

One bit I'm still not sure about: Should we namespace the context data to avoid collisions among callbacks from different plugins?

@mrmrcoleman
Copy link

Capturing some points here (I would like to be corrected on anything that I have misunderstood)

  • In the namespacing scenario one would reference variables like payload.context.pluginname.key
  • In the non-namespacing scenario one would reference variables like payload.context.key

The risk is that two plugins define the same key. My instinct tells me to namespace it because even if the risk of collisions is small the user impact is, presulably, large e.g. I get very difficult to solve issues that require me to introspect how NetBox works to figure out

@jeremystretch
Copy link
Member Author

I have two counterpoints to share:

  1. Plugins can always voluntarily namespace keys by attaching an arbitrary prefix (e.g. my_plugin_foo).
  2. There may be cases where namespacing is undesirable. For example, support a webhook receiver wants to check for an error key set by any plugin. It is much simpler to check for context.error than to iterate through all (or some) dictionaries under context to determine if any of them contain error.

That said, I have no strong opinion either way.

@arthanson
Copy link
Collaborator

Should this be documented, maybe in the webhooks docs: https://netboxlabs.com/docs/netbox/integrations/webhooks/

Copy link
Collaborator

@arthanson arthanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works, just wondering about documenting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants