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

Follow HTTP redirects on websocket handshake failure #250

Closed
echlebek opened this issue Feb 9, 2024 · 0 comments · Fixed by #251
Closed

Follow HTTP redirects on websocket handshake failure #250

echlebek opened this issue Feb 9, 2024 · 0 comments · Fixed by #251
Labels
enhancement New feature or request

Comments

@echlebek
Copy link
Contributor

echlebek commented Feb 9, 2024

It would be useful to check for a valid redirect on handshake failure in the opamp-go client, and follow it to attempt to establish a websocket connection to the location specified by the redirect. This would allow more flexibility for operators who are supporting a large network of opamp servers. It is also a backwards-compatible change.

The websocket standard permits websocket servers to offer redirection to websocket clients. (See https://www.rfc-editor.org/rfc/rfc6455#section-4.2.2)

In particular,

  1. The server MAY redirect the client using a 3xx status code
    [RFC2616]. Note that this step can happen together with, before,
    or after the optional authentication step described above.

The gorilla/websocket library does not implement this optional redirect following, but it does return a valid HTTP response when a handshake error occurs. (See https://github.com/gorilla/websocket/blob/main/client.go#L412)

What I am proposing specifically:

In https://github.com/open-telemetry/opamp-go, check for an HTTP response status >= 300 && <400. Parse the location header, and if valid, set c.url to be the URL from the location header. Then, call tryConnectOnce again.

I am happy to do the implementation work this would require, and have a changelog I can contribute in the near future.

@tigrannajaryan tigrannajaryan added the enhancement New feature or request label Feb 14, 2024
tigrannajaryan pushed a commit that referenced this issue Feb 23, 2024
This commit allows the opamp client to follow redirects after websocket handshake failures. Redirect following is not implemented by gorilla/websocket, but can be handled by inspecting the returned response object for 3xx status and Location header.

Closes #250
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants