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

Allow configuring multiple listen addresses for each port #372

Merged
merged 3 commits into from
Dec 20, 2021

Conversation

ThinkChaos
Copy link
Collaborator

I use this to bind to localhost and my LAN interface, but not the WAN one.

@codecov
Copy link

codecov bot commented Dec 17, 2021

Codecov Report

Merging #372 (09bd426) into development (cc968ce) will decrease coverage by 0.34%.
The diff coverage is 77.46%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #372      +/-   ##
===============================================
- Coverage        96.17%   95.82%   -0.35%     
===============================================
  Files               30       30              
  Lines             2251     2278      +27     
===============================================
+ Hits              2165     2183      +18     
- Misses              56       63       +7     
- Partials            30       32       +2     
Impacted Files Coverage Δ
resolver/client_names_resolver.go 100.00% <ø> (ø)
cmd/root.go 58.53% <28.57%> (-3.01%) ⬇️
config/config.go 88.51% <61.53%> (-2.98%) ⬇️
server/server.go 89.73% <88.23%> (-0.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc968ce...09bd426. Read the comment docs.

Comment on lines -18 to -19
| httpPort | int (1 - 65535) | no | | HTTP listener port and optional bind ip address . If > 0, will be used for prometheus metrics, pprof, REST API, DoH ...If you wish to specify a specific IP, you can do so such as 192.168.0.1:4000. Example: 4000, :4000, 127.0.0.1:4000 |
| httpsPort | int (1 - 65535) | no | | HTTPS listener port and optional bind ip address . If > 0, will be used for prometheus metrics, pprof, REST API, DoH... If you wish to specify a specific IP, you can do so such as 192.168.0.1:443 |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the part about > 0 because as far as I can tell, 0 isn't treated specially. Maybe that's a bug, in which case I could add the docs back and fix the code.


if len(httpListeners) != 0 || len(httpsListeners) != 0 {
metrics.Start(router, cfg.Prometheus)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I haven't tested metrics as I don't have the setup for it yet, so please be mindful of this part.

Copy link
Owner

Choose a reason for hiding this comment

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

I tested it -> works


lastIdx := len(split) - 1

apiHost = strings.Join(split[:lastIdx], ":")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't completely related, but I added this behavior as I noticed the discrepancy while fixing the code to work with a list.
I can remove commit 79dcf57 if you prefer.

@0xERR0R 0xERR0R self-assigned this Dec 20, 2021
@0xERR0R 0xERR0R added the 🔨 enhancement New feature or request label Dec 20, 2021
@0xERR0R 0xERR0R added this to the 0.18 milestone Dec 20, 2021
@@ -15,7 +15,7 @@ import (
"github.com/sirupsen/logrus"
)

// ClientNamesResolver tries to determine client name by asking responsible DNS server vie rDNS (reverse lookup)
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@0xERR0R
Copy link
Owner

0xERR0R commented Dec 20, 2021

Thanks for you work! It looks good for me, works as expected! 👍

@0xERR0R 0xERR0R merged commit 69dc383 into 0xERR0R:development Dec 20, 2021
@ThinkChaos ThinkChaos deleted the feat/multi-ports branch January 24, 2023 00:35
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 this pull request may close these issues.

2 participants