[processor/remotetapprocessor] don't require Origin header #34925
Conversation
|
Could the user not use the server CORS settings to make sure the server is configured for the proper use case? Can you please add a test? |
|
no, cors in confighttp returns a header that allows the client to check in a preflight request if it is allowed to make a proper request. This is a server side check that the client includes an Origin header, which cli tools don't set (nor do they make preflight cors checks because cross origin isn't a concept that applies to a cli not run from a web origin). I'm not sure how to add a test, the go websocket client in use requires an origin to be provided to work (which may not be true of other websocket libraries). |
|
Confirmed this fixes the 403 forbidden issue I was having with the |
|
Please add changelog and let's get it in. |
|
added a changelog entry |
mwear
added
the
ready to merge
| component: remotetapprocessor | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Origin header is no longer required for websocket connections |
There was a problem hiding this comment.
Is there any tests that help confirm this behaviour or a regression isn't added in?
There was a problem hiding this comment.
like I mentioned earlier in the pr thread, there's no easy way to test it unless we want to pull in another websocket library?
…metry#34925) **Description:** Currently remotetap runs a server using [`websocket.Handler`](https://pkg.go.dev/golang.org/x/net/websocket#Handler.ServeHTTP) This internally creates a [`websocket.Server`](https://cs.opensource.google/go/x/net/+/refs/tags/v0.28.0:websocket/server.go;l=111) that [requires a `Origin` header](https://cs.opensource.google/go/x/net/+/refs/tags/v0.28.0:websocket/server.go;l=101-107) in the incoming request. While this makes sense for browser usage, for the collector we may want to use other tools e.g. CLI tools which do not set an `Origin` header. This changes the server to use `websocket.Server` directly, without setting `Handshake`, [bypassing the origin check](https://cs.opensource.google/go/x/net/+/refs/tags/v0.28.0:websocket/server.go;l=32-41) **Link to tracking Issue:** N/A issue reported in slack **Testing:** manual https://cloud-native.slack.com/archives/C01N6P7KR6W/p1724957510485869?thread_ts=1724863668.431979&cid=C01N6P7KR6W **Documentation:** none / bug fix
Description:
Currently remotetap runs a server using
websocket.HandlerThis internally creates a
websocket.Serverthat requires a
Originheader in the incoming request.While this makes sense for browser usage,
for the collector we may want to use other tools e.g. CLI tools which do not set an
Originheader.This changes the server to use
websocket.Serverdirectly, without settingHandshake,bypassing the origin check
Link to tracking Issue: N/A issue reported in slack
Testing: manual https://cloud-native.slack.com/archives/C01N6P7KR6W/p1724957510485869?thread_ts=1724863668.431979&cid=C01N6P7KR6W
Documentation: none / bug fix