netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Try NetBox Cloud free: https://netboxlabs.com/free-netbox-cloud/
http://netboxlabs.com/oss/netbox/
Apache License 2.0
16.15k stars 2.58k forks source link

webhook on device deletion gets triggered in both update and delete #6957

Closed ifoughal closed 2 years ago

ifoughal commented 3 years ago

NetBox version

v2.11.11

Python version

3.8.10

Steps to Reproduce

  1. create a webhook on the admin pannel:

    • both webhooks are using post method on target jenkins server.
    • update device webhook has update method on netbox side checked.
    • deleted device webhook has update method on netbox side checked. -webhook url:
    • update device additional headers:
      • jobQuietPeriod: 0
      • token: updates_nodes_token
    • delete device additional headers:
      • jobQuietPeriod: 0
      • token: delete_nodes_token
    • with ssl_verification disabled for both.
  2. after adding afforementionned webhooks and enabling them. I proceeded with my tests, using update / delete methods to trigger jenkins builds.

    • the update device webhook works as intended, so it triggers the build and everything works as intended.

    • however when the deletion method is used on a device, the delete webhook would trigger both webhooks, thus triggering both my jenkins pipelines, even though the pipelines are using completely different tokens, thus leading me to believe that the delete method on Netbox triggers both delete and update webhooks if they are configered on the same object.

    • The bug actually is visual I believe, because when the url for the webhook is exactly the same, I get the following message while trying to add the second webhook:

      • "Webhook with this URL, Type create, Type update and Type delete already exists."
    • it seems to be a front end, of default value for the front end check boxes related issue, as when you try to create a webhook with the same url + token without sperate headers, even though the first is on delete and the second is on update, netbox doesn't allow that. but if you select create with any of the other options, then it works.

Expected Behavior

Upon deleting a device, I expected to trigger the deletion webhook only.

Observed Behavior

Impossible to create a delete and an update webhooks with the same url + headers, but possible with create + delete or create + update webhooks.

jeremystretch commented 3 years ago

Please provide complete steps to reproduce the reported behavior, beginning with the creation of the webhook.

ifoughal commented 3 years ago

Please provide complete steps to reproduce the reported behavior, beginning with the creation of the webhook.

I have provided more information by updating the issue.

TLDR;

The issue is related to how netbox interprets the default values for the update, create, delete checkboxes.

From my inderstanding, it's impossible to create an update and a delete webhook seperately, because netbox thinks they are duplicates as one or the other is always enabled, thus allowing create with any of the two options, but in reality one or maybe both of them are always enabled by default.

jeremystretch commented 3 years ago

I'm sorry but you're trying to pack too much into a single bug report. Is the issue that you believe an update webhook is firing erroneously, or is that you're unable to create multiple webhooks in NetBox? Let's pick one thing and focus on that.

ifoughal commented 3 years ago

I'm sorry but you're trying to pack too much into a single bug report. Is the issue that you believe an update webhook is firing erroneously, or is that you're unable to create multiple webhooks in NetBox? Let's pick one thing and focus on that.

The update webhook is indeed triggered erroneously when a delete action is on a device object.

I believe that the issue lays somewhere on the admin pannel front end, since it seems that netbox thinks that both webhooks (delete & update) already exist when I try to add a new webhook for the delete with the exact same url.

ifoughal commented 3 years ago

Is the issue that you believe an update webhook is firing erroneously, or is that you're unable to create multiple webhooks in NetBox? Let's pick one thing and focus on that.

After some thourough testing with Jenkins, I have finally figured it out.

During the deletion procedure of a device, exactly 2 update methods get applied on the device itself.

My guess is, some backend scripts trigger these updates during probably the deletion of the primary addresses, and other cascading deletions similar to this.

The solution here would be to work on the deletion method, so that it doesn't trigger any other webhooks, as this breaks automation processes relying on webhooks (eg: deleting a node from netbox triggers a deletion on the monitoring service, but would also trigger an update pipeline at the same time, due to how the deletion process works currently.)

ifoughal commented 2 years ago

I'm closing this ticket, as it seems that it has been partially solved on the beta version with issue #6238

kevinsdirectory commented 2 years ago

@idrisFoughali I have the same problem, how did you prevent from triggering webhook update when deleting a device.

ifoughal commented 2 years ago

@idrisFoughali I have the same problem, how did you prevent from triggering webhook update when deleting a device.

Sadly I never did, so far I have not managed to implement a deletion webhook for objects that have cascading deletions with updates in between (such as management interface and its ip addresses getting deleted during a device deletion process, which then updates the devices while deleting it, thus triggering a false positive)

With the new webhook implementation, this might maybe get resolved, I guess we'll have to wait and see, but it seems that the netbox devs are not that interested in fixing this.

Edit: Reopened the issue.

jeremystretch commented 2 years ago

but it seems that the netbox devs are not that interested in fixing this.

That or we've been busy triaging ~150 issues per month from the thousands of other people who use NetBox and this one bug report, which hasn't had a single other user comment until 17 hours ago, hasn't been a priority. If you'd like to own this and propose a solution I'll gladly assign it to you.

ifoughal commented 2 years ago

That or we've been busy triaging ~150 issues per month from the thousands of other people who use NetBox and this one bug report, which hasn't had a single other user comment until 17 hours ago, hasn't been a priority. If you'd like to own this and propose a solution I'll gladly assign it to you.

Apologies, it really was not in my intentions to downplay what you guys have been doing in any way, what I mean't was basically that you have other priorities, and that you don't want to prioritise this issue. That's all.

jeremystretch commented 2 years ago

I'm still not sure what this issue is supposed to address. If you're seeing webhooks fire for both the update and delete actions when deleting a device, that is to be expected in many cases. For example, if a device has a primary IP address assigned to it, and that IP is deleted immediately prior to the device being deleted (because its assigned interface on the device has been deleted), the device will be updated to nullify its primary IP, and then it will be deleted. If this is the case, there's no action be taken because there's nothing to fix.

If your issue is not what I described above, please elaborate on exactly what the problem is.

ifoughal commented 2 years ago

I'm still not sure what this issue is supposed to address. If you're seeing webhooks fire for both the update and delete actions when deleting a device, that is to be expected in many cases. For example, if a device has a primary IP address assigned to it, and that IP is deleted immediately prior to the device being deleted (because its assigned interface on the device has been deleted), the device will be updated to nullify its primary IP, and then it will be deleted. If this is the case, there's no action be taken because there's nothing to fix.

That is the exact scenario, as you have described.

If there's no action to be taken from your side, then you may go ahead with the closure if this issue.

For future users who might be facing the same issue as us, our use case was automating devices addition and removal from the monitoring services automatically, by triggering the webhook. This has affected the automation process on our side, therefore Netbox as a dynamic inventory can't be fully exploited in this scenario.

jeremystretch commented 2 years ago

I don't understand why that would be the case: The webhook clearly indicates what type of action is being taken (update vs. deletion) in each request. You might also look into defining a set of conditions for finer control over when webhooks fire.

At any rate, there doesn't seem to be any action to take here so I'll close this out.