Skip to content
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

feat: Add webhook support #1046

Closed
wants to merge 3 commits into from

Conversation

petkostas
Copy link
Contributor

Webhooks will allow Hoverfly to trigger external events and actions based on specific conditions or responses in the simulation. This can enable new functionality and automation to your testing process.
By allowing Hoverfly to simulate external systems and webhooks, we can create a more realistic testing environment that mirrors your production environment. Handling webhooks within Hoverfly helps us avoid the need for additional middleware, testing tools or infrastructure. This can make setting up and maintaining our local/testing environment easier.

@petkostas petkostas marked this pull request as draft January 26, 2023 18:07
@petkostas
Copy link
Contributor Author

petkostas commented Jan 26, 2023

  • Add POC
  • Manual test
  • Extend webhooks to support bodyFiles
  • Properly implement Headers for webhooks
  • Improve observability
  • Test coverage
  • Feedback and code improvements

This was tested with the following response payload (destination endpoints omitted)

{
    "response":
    {
        "status": 200,
        "body": "Hello there!",
        "encodedBody": false,
        "headers":
        {
            ...
        },
        "postActionHooks":
        [
            {
                "name": "test",
                "parameters":
                {
                    "body": "{\"name\":\"Hoverfly\"}",
                    "destination": "webhook.site",
                    "encodedBody": false,
                    "headers": {},
                    "method": "POST",
                    "scheme": "https"
                }
            }
        ],
        "templated": false
    }
}

Result (webhook.site)
Screenshot 2023-01-26 at 19 14 29

@petkostas petkostas force-pushed the add-webhook-support branch 7 times, most recently from caa6692 to ce50a7d Compare January 26, 2023 19:10
@kapishmalik
Copy link
Collaborator

@petkostas I just saw PR out of curiosity. I have one concern. It seems like callback will be sent first before simulation response to end user. In real scenario, we always expect accepted response followed by callback.

@petkostas
Copy link
Contributor Author

petkostas commented Jan 26, 2023

@kapishmalik , you are correct. I am checking on moving the call post-response right now. The problem is that we don't return the pair, and I am losing the information. This will require a bit more effort unless you have another idea for it.

@kapishmalik
Copy link
Collaborator

@petkostas I would recommend to add another method ProcessPostHooks in all of our different modes.go files in similar way we have Process method. This method will be called from hoverfly.go from where this Process is being called(post calling this method) in a separate go routine where we can also add some default delay in order to ensure simulation response received first followed by webhook/callback.

@kapishmalik
Copy link
Collaborator

Also this callback won't be resilient as if it fails then there is no retry mechanism to send it back again. We may also think how we can do that as well.

@kapishmalik
Copy link
Collaborator

I have another concern. Generally callbacks also has some hmac or some encoded details which is sent back so that receiver can cross verify that it is authentic callback or webhook. How we will achieve that. Do we send hardcoded details in body or header?

@petkostas
Copy link
Contributor Author

petkostas commented Jan 26, 2023

I have another concern. Generally callbacks also has some hmac or some encoded details which is sent back so that receiver can cross verify that it is authentic callback or webhook. How we will achieve that. Do we send hardcoded details in body or header?

@kapishmalik I added support for different fields including headers and queries (not implemented yet), if you want, you can contribute to the PR and add that part of the functionality, I will try to add by tomorrow the ProcessPostHooks functionality in the relevant Hoverfly modes.
I believe on an API virtualisation level, we can be less restrictive as well, I kind of follow Wiremocks approach here.
But surely, if you have any ideas to keep it simple (in terms of virtualizing an API), let me know.

@kapishmalik
Copy link
Collaborator

kapishmalik commented Jan 27, 2023

@petkostas no worries... you can go ahead with whatever seems suitable and feasible to you for now.

BTW, I am also exploring Go Plugins. We can define an interface and ask the user to implement it and plug his code into hoverfly so that such uses cases become very easy to implement in the future. Today, you have a use case of getting a callback, tomorrow someone will also have some other use case. IMO, better would be to have interface that takes the Request/Response pair as an argument and do whatever they want to do. I believe wire mock also tried to achieve this as an extension in Java. But there is one drawback that Go plugins are not supported in windows. Also, I am still yet to explore more pros/cons of this approach.

Also, there was a discussion in open issue #690 to add hooks to middleware and the option to specify when to execute it. It will be great for this use case if we can specify post-execution(POST_RESPONSE_HANDLE) and write middleware only for this. IMO, it will be better to re-use existing feature instead of building specific feature just for async requests.

@petkostas @tommysitu what are your thoughts about the same?

References:
https://pkg.go.dev/plugin#pkg-overview
https://medium.com/learning-the-go-programming-language/writing-modular-go-programs-with-plugins-ec46381ee1a9

@petkostas petkostas force-pushed the add-webhook-support branch 4 times, most recently from e10483d to 5899273 Compare January 28, 2023 18:31
@petkostas
Copy link
Contributor Author

@kapishmalik I like the suggestion, and I feel it would be even easier to integrate scenarios similar to this one. Maybe something as a follow-up to this one?
I moved some of the functionality and made it run concurrently with the rest of the code. I will update the execute function to support the rest of the missing parts and include some tests.
I tested this approach, and it is working as expected. I only discovered that the latest macOS update breaks the tests regarding the TLS certificate error message.

@kapishmalik
Copy link
Collaborator

kapishmalik commented Jan 28, 2023

@petkostas I personally feel let's do this via middleware itself and add support to run middleware post response by adding hooks as described in issue #690. It will be better if you resolve open issue #690 instead of building this. If you do this in this way then you are asking to tell user to provide webhook request in json format and there will always be scope of enhancement to that. You are asking end user to describe http request in json format and as http ll change something, there will come a need to change here to accept from user in json format and then to handle that in the code as well.

Second drawback here is if you add support then you may also need to guarantee that callback will always be sent. If you do via middleware then that responsibility goes away and it is upto end user how he implements middleware.

@petkostas
Copy link
Contributor Author

@kapishmalik sounds like a good option (#690 ) will further check it.

@kapishmalik
Copy link
Collaborator

kapishmalik commented Feb 9, 2023

@petkostas are you working on adding pre/post hook support for middleware? If not, we have a usecase, I can work on the same.

@petkostas
Copy link
Contributor Author

Hello @kapishmalik i didn't make any progress since our last chat as I am a bit blocked by work.
Sure u can pick this up and I can hope in to help/test if needed.

@kapishmalik
Copy link
Collaborator

sure, @petkostas I will pick this up and come up with PR in a week's time.

@kapishmalik
Copy link
Collaborator

@petkostas I started implementing hooks approach but realized that middleware has contract which takes in request/response details and returns modified request/response details. I was thinking to write callback middleware to run after post execution but by that time, response won't be sent to the user. It will be better to go with this approach only. @tommysitu let me know if we can implement callback via middleware by another means.

@tommysitu
Copy link
Member

Sorry just starting to catch up with the conversations here. I had thought about this feature before because it's a pain to simulate webhooks, and would be nice for Hoverfly to support it.

Firstly I don't think middleware is suitable for invoking webhook, because middleware is like an HTTP filter, it's designed to allow users to implement custom request/response filters. But webhook is an async operation that is triggered after an HTTP call is ended, so webhook should be outside the context that middleware intercepts.

Defining the webhook in the same JSON schema appears easy to implement, but it lacks the flexibility which you realized, plus it will make the schema quite bloating for a complex call back.

However I don't have a definitive answer to how it should be implemented yet, but having the following features would be useful:

  • Ability to record webhooks and store them in memory or export to a file, because this will give realistic simulation of real webhooks you are trying to fake
  • Allow a response to trigger the webhook (similar to the state transition), so having a new field in response to reference the name of the webhook would be enough, should be a very minor update to the schema
  • To allow flexible webhook (not just a playback of pre-recorded one), I reckon we can take the inspiration from middleware, but create a new interface for that, so that user can submit their own implementation (using their own code to make outbound calls). All they need to do is associate each webhook with a name which can be triggered by the response.

I think the more this webhook support is decoupled from the core simulation feature the better. 'cause two are quite different species.

@kapishmalik
Copy link
Collaborator

kapishmalik commented Feb 15, 2023

@tommysitu Will it be possible to record/capture webhooks as the server may send them to their registered endpoints? If it is an internal server then we may ask to pass via hoverfly but if it is a third-party application, we may not ask so.

IMO, what we can do is we can implement post-response hooks which can be implemented post-completion of the request irrespective of the mode. This hook can be a script that we can specify and use it. In short, we can implement points 2 and 3 as you mentioned.

it can be broken down into below stories on which we can work.

  • Expose GET/POST/PUT APIs for fetching or setting flexible post-execution hooks (i.e. it'll be a script in the same way that we have for middleware).
  • Change in simulation schema to accept registered post-execution hook for particular response details.
  • Changes related to executing post-execution hook after execution is over.
  • Add hoverctl command to add post execution hook.

If this plan looks fine then probably I can start implementing it.

I believe this feature will be equivalent to post-serve actions in wire mock.

@kapishmalik
Copy link
Collaborator

@petkostas @tommysitu Just FYI... I am currently working as per the above discussion.

@petkostas
Copy link
Contributor Author

Thanks, mate! let me know if I can help with anything, this will remove some overhead with simple cases.

@kapishmalik
Copy link
Collaborator

@tommysitu @petkostas we can go ahead and delete this PR as it is no longer required.

@petkostas
Copy link
Contributor Author

@kapishmalik yes we can, I am testing your PR 😄

@petkostas petkostas closed this Feb 22, 2023
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