Idempotent issue with meraki_webhook

[joshuajcoronado]

hi @kbreit

I think i've tracked down the bug that's blocking the meraki webhook integration tests that we first discovered when trying to merge #384.

I don't believe it's a change in the API by cisco, but rather an issue the with function generate_diff. In particular,

            diff = recursive_diff(before, after)
            self.result['diff'] = {'before': diff[0],
                                   'after': diff[1]}

which fails with the error The error was: TypeError: NoneType object is not subscriptable.

So i dug into recursive_diff, to figure out why it's returning the None object. Turns out, it returns None if there are NO differences between the two dictionaries passed.

Since, we check to see if an update is required at all before getting to the generate_diff function, we should never get None returned. It turns out, there is a difference tho, and it has to do with the parameter sharedSecret.

In a GET request at /networks/{networkId}/webhooks/httpServers/{httpServerId}, they return all information about the webhook EXCEPT the sharedSecret. I assume this is done for #security reasons, and is easy enough to route around, but that leaves us with a dilemma. It makes determining whether the task updated anything, as the API only returns one type of 200 code. This is because whenever a webhook ALREADY exists, we can never determine if its sharedSecret matches what cisco has. And when we hit the PUT endpoint, it only returns 201. Let me know how you want me to proceed, as i'm not too familiar if this problem has cropped up in other modules.

[kbreit]

That's indeed what's causing it. Worse is that I think it's also happening for url which is why I'm asking Meraki if it's a bug. It would make sense you can update the URL but my experimentation is showing otherwise. I had some changes to fix this which I need to apply. We basically need to check if the update is required before generating the diff. But the URL is still something that needs to be resolved, perhaps separately.

[joshuajcoronado]

oh wow, didn't realize you couldn't update the URL, that's unfortunate that doesn't work either, yah, let me know what they say

[kbreit]

If you can, please try to update the URL on your side and let me know if you're seeing different results.

[joshuajcoronado]

👍

[joshuajcoronado]

@kbreit seeing the same thing, it throwing an error since the returned object contains the OLD url, and the diff is returning None. I'm about to go on vacation until the first week of december, so, i'll be quiet on this end for awhile.

[kbreit]

Have a good thanksgiving. I'll do some work on this and the PR and get something merged. The fix for the diff should cover both scenarios.

[joshuajcoronado]

@kbreit i'm back from vacation, any changes on this?

[kbreit]

I read it wrong and it indeed isn't supposed to support URL changes. If you need a URL change you need to create a new one.

I haven't been able to do a patch for this one but should be able to take care of it soon. It will change my test and functionality of the code itself.

[kbreit]

@joshuajcoronado Sorry about the delay. I have a PR; would you please test?

[kbreit]

I was going to add a template to a webhook as the update operation. Unfortunately, I noticed I don't support applying a template to a webhook server. I need to add that function first.