-
Notifications
You must be signed in to change notification settings - Fork 44
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
meraki_webhook query raises exception if no webhooks are defined #433
Comments
Thank you for submitting this issue. This part of the code on this module was actually refactored a week or two ago and I just need to get a new release out to fix it. I'll try for that tonight but lets keep this open until we verify this fixes your bug. |
Hi - sorry to say that the issue persists with 2.15.0
|
HI; Not sure if the following should be opened up as separate issues or not so for the moment, lets call these observations... 2> the sanitize_no_log_values() method assumes a single entry in data so will not deal correctly with anything other than one webhook - this is interesting since given the example below, I'm not sure the shared_secret is even in the response - so the code is adding it needlessly? if you see the output below - one has a shared_secret key, sanitized, one doesn't;
|
Hello! EDIT: I think I only resolve the issue with that empty data (array) |
@y0rune - I've just looked at the patch, and as i think you might have already noticed, my observation about the hardcoded array index is still valid.. and the fact it's setting a key that doesn't exist, but that might be a conscious decision. |
Out of curiosity I looked at why the an "update" of a webhook shows "changed" when it shouldn't necessarily. As far as i can see, you're in a rock/hard place here because of the API; shared-secret is never returned in the API get or update responses - and so you can't know that it does or doesn't need changing, so an API call will always be made to update it and thus "changed" .. just in case .. which is why i assume the tests for this are commented out in the test pack. so.. case closed on that point. Is it possible to update the documentation to call this out? |
@y0rune -
I also added this to the test file:
|
So I will close mine PR. Please create a PR with you code. ;) |
I would say each of those are different issues. "Query all" not working should be a new one outside of the no_log one. |
@y0rune challenge accepted... |
I can certainly update the documentation to mention how secrets are ignored by idempotency changes. And yes, since they're not returned by the API (nor should they be most likely) it's impossible to state whether it was changed. Do you have any open PR's for this one right now? |
#436 is open. I have a response to follow up. |
#436 is now merged - propose this issue can be closed. |
A query of a Meraki Network with no defined webhooks raises an exception;
Using release 2.14.0 of the cisco.meraki collection.
with the following in a playbook
the following is seen:
Note that using Postman to drive the Meraki API endpoint for the same scenario results in a 200 OK response with an empty [] list returned.
With a webhook defined, task works as expected.
The text was updated successfully, but these errors were encountered: