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

Added support for Websockets #107

Conversation

jacobm-splunk
Copy link
Contributor

This MR aims to add support for proxying websockets to wiretap.

It skips validation on this endpoint as OpenAPI does not really support websocket specification.

I am not the most familiar with gorilla/websocket so I may have missed something.

@daveshanley
Copy link
Member

Amazing work, will review ASAP!

Copy link
Member

@daveshanley daveshanley left a comment

Choose a reason for hiding this comment

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

A couple of nits, other than that - it looks great.

Didn't we discuss the need to add in the endpoints in which to forward, or are you simple just doing a straight passthrough for any endpoint?

daemon/handle_request.go Outdated Show resolved Hide resolved
daemon/handle_request.go Show resolved Hide resolved
@jacobm-splunk
Copy link
Contributor Author

A couple of nits, other than that - it looks great.

Didn't we discuss the need to add in the endpoints in which to forward, or are you simple just doing a straight passthrough for any endpoint?

I have a user add the configuration for their endpoints and then register a handler on each one of those:

// Handle Websockets
for websocket := range wiretapConfig.WebsocketConfigs {
	mux.HandleFunc(websocket, handleWebsocket)
}

If that's not doing what I expect it to do, then I can totally change it, but since the websocket is opened through a http request to a specific endpoint, then I think this should work?

@daveshanley daveshanley merged commit 3c27d94 into pb33f:main Apr 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants