Skip to content

Conversation

@MatthieuCoder
Copy link

@MatthieuCoder MatthieuCoder commented Sep 25, 2025

Hello !

This pr adds support for specifying the addr argument in ListenAndServe, allowing the user to make the jwt service listen on a loopback address which is useful when using a reverse proxy / load balancer.

Some changes related to the lsp-auto-fmt behavior

This generally helps when the user wants to listen to [::1] or 127.0.0.0/8

Signed-off-by: Matthieu Pignolet <[email protected]>
@MatthieuCoder MatthieuCoder requested a review from a team as a code owner September 25, 2025 20:40
@CLAassistant
Copy link

CLAassistant commented Sep 25, 2025

CLA assistant check
All committers have signed the CLA.

@ncs-pl
Copy link

ncs-pl commented Sep 26, 2025

Oh yes please, this would be really useful to me!

Copy link
Member

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

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

Seems good, but would be good to keep backwards compat.

…lly exclusive with the `LIVEKIT_JWT_BIND` environment variable)

Signed-off-by: Matthieu Pignolet <[email protected]>
@fkwp
Copy link
Contributor

fkwp commented Oct 13, 2025

Thx a lot for this contribution.

@MatthieuCoder sorry for the delay -- team is currently sprinting for MatrixConference :-)

339229b should address this issue - do you want to add a warning in the console akin to what's being done with LIVEKIT_LOCAL_HOMESERVERS ?

Yup a big fat warning sounds right. Besides that, it would be nice to also have a test around.

kind regards

@MatthieuCoder
Copy link
Author

MatthieuCoder commented Oct 17, 2025

339229b should address this issue - do you want to add a warning in the console akin to what's being done with LIVEKIT_LOCAL_HOMESERVERS ?

Yup a big fat warning sounds right. Besides that, it would be nice to also have a test around.

This should now be implemented. Although a new variable eg. LIVEKIT_JWT_BIND_NETWORK_TYPE would be nice to allow listening on other network types such as unix just like synapse allows it.

With a default value of tcp it would allow transparent backwards-compatibility.

Regarding the tests, as this kind of "configuration" isn't tested currently, I did not implement it.

I already have a kind-of-working code but I would like some guidance regarding this contribution

@fkwp
Copy link
Contributor

fkwp commented Oct 22, 2025

339229b should address this issue - do you want to add a warning in the console akin to what's being done with LIVEKIT_LOCAL_HOMESERVERS ?

Yup a big fat warning sounds right. Besides that, it would be nice to also have a test around.

This should now be implemented. Although a new variable eg. LIVEKIT_JWT_BIND_NETWORK_TYPE would be nice to allow listening on other network types such as unix just like synapse allows it.

With a default value of tcp it would allow transparent backwards-compatibility.

Regarding the tests, as this kind of "configuration" isn't tested currently, I did not implement it.

I already have a kind-of-working code but I would like some guidance regarding this contribution

On LIVEKIT_JWT_BIND_NETWORK_TYPE I suggest to make this a new follow-up PR.

On the testing you are right -- this would probably need a small refactoring to split out the config parsing. So for now, leave that with us.

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.

5 participants