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

Support dynamically generating headers for each client request #297

Closed
BinaryFissionGames opened this issue Sep 4, 2024 · 6 comments · Fixed by #298
Closed

Support dynamically generating headers for each client request #297

BinaryFissionGames opened this issue Sep 4, 2024 · 6 comments · Fixed by #298

Comments

@BinaryFissionGames
Copy link
Contributor

BinaryFissionGames commented Sep 4, 2024

I would like to use the headers for token-based authentication, e.g. oAuth2. In this case, tokens may expire and need to be refreshed.

However, opamp-go currently only allows specifying a fixed set of headers in the start settings for the client.

Instead, it would be useful to provide a function that can dynamically determine headers, so that tokens could be refreshed for authentication, e.g.:

// StartSettings defines the parameters for starting the OpAMP Client.
type StartSettings struct {
  // Current settings remain unchanged ...
 
  HeaderFunc func(base http.Header) http.Header
}

Here, HeaderFunc is a function that takes in the "base" static headers, and outputs a set of new headers. This would be called on every request sent to the server, and would allow for dynamic authentication headers in case of token refresh, for instance.

@andykellr
Copy link
Contributor

This proposal looks reasonable to me.

@tigrannajaryan
Copy link
Member

I am a bit worried that we need to add more and more connection options to StartSettings.

Would it be better if client API allowed supplying http.Client in StartSettings? You could then achieve this dynamic header functionality by providing your own RoundTripper in the Client.

We had a similar discussion in this thread: #269 (comment) so it seems this may be useful in more than one use case. It would be also similar to how we allow supplying your own http.Server in the Server API.

What do you think?

cc @andykellr

@BinaryFissionGames
Copy link
Contributor Author

BinaryFissionGames commented Sep 6, 2024

I have similar concerns. Being able to specify a the whole client is something I would really like, and it would solve a few issues for us.

However, my concern with that is how we will support websockets, which is our main use-case right now (e.g. every websocket disconnect/reconnect, I'd want to potentially refresh my auth token).

The gorilla websocket library does not support specifying your own http client. There is another library that does (https://github.com/coder/websocket), but I believe we had concerns about switching libraries at this point.

@tigrannajaryan
Copy link
Member

However, my concern with that is how we will support websockets, which is our main use-case right now (e.g. every websocket disconnect/reconnect, I'd want to potentially refresh my token).

The gorilla websocket library does not support specifying your own http client. There is another library that does (https://github.com/coder/websocket), but I believe we had concerns about switching libraries at this point.

Yeah, that's pretty unfortunate :-(

We can try to offer to gorilla websocket to add ability to supply http client. Gorilla seems to be maintained again (good news!), maybe they will add it.

In the meantime I guess we will have to do our own thing to abstract away transport differences. I don't want to expose different APIs for different transports.

@tigrannajaryan
Copy link
Member

We can try to offer to gorilla websocket to add ability to supply http client. Gorilla seems to be maintained again (good news!), maybe they will add it.

@BinaryFissionGames would you be able to make a proposal to Gorilla?

@BinaryFissionGames
Copy link
Contributor Author

Yeah, I can go ahead and do that.

tigrannajaryan pushed a commit that referenced this issue Sep 12, 2024
Adds a new HeaderFunc to the StartSettings that allows for dynamically editing the headers before each HTTP request made by the OpAMP library.

Closes #297
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 a pull request may close this issue.

3 participants