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

Possibility to specify listening IP #17

Merged
merged 2 commits into from
Sep 30, 2021
Merged

Possibility to specify listening IP #17

merged 2 commits into from
Sep 30, 2021

Conversation

fiksn
Copy link
Contributor

@fiksn fiksn commented Sep 28, 2021

I'd need this to make LnMe -listen 127.0.0.1 so instance will not be directly reachable eventhough I might make some mistake with iptables - I have some reverse proxy sitting in front of it then.

@bumi
Copy link
Owner

bumi commented Sep 28, 2021

good improvement! thanks
can we combine this with the port option?
to use --listen=":3000" or --listen="localhost:3000" ?

even though we then need to slowly deprecate the port option.

lnme.go Outdated Show resolved Hide resolved
@fiksn
Copy link
Contributor Author

fiksn commented Sep 28, 2021

good improvement! thanks can we combine this with the port option? to use --listen=":3000" or --listen="localhost:3000" ?

even though we then need to slowly deprecate the port option.

I agree, if listen was used for ip & port that would be even better. Wasn't sure about the deprecation of port - in particular how to resolve conflicting port and listen, best option is probably just to error out in such a case. And if there is just port treat it as listen=:port and write some deprecation warning to stderr.

@bumi
Copy link
Owner

bumi commented Sep 28, 2021

I think that's a good idea.
If port is set and NO listen is provided we do listen=:port. otherwise we error or ignore the PORT option and add a deprecation warning here

this way it is backward compatible for current deployments and newer deployments can use the new listen option.
We just need the support for the PORT environment variable for cloud providers like heroku

@fiksn
Copy link
Contributor Author

fiksn commented Sep 29, 2021

Is my second commit ok? I can squash the stuff into one so there will be a more linear history.

@bumi
Copy link
Owner

bumi commented Sep 29, 2021

looks great! thanks a lot.
I think I can squash it here on GitHub when merging. that's good I think.

@bumi bumi merged commit 3200622 into bumi:master Sep 30, 2021
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.

2 participants