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

Automatically check/set DNS settings? #775

Closed
matt-allan opened this issue May 1, 2019 · 7 comments
Closed

Automatically check/set DNS settings? #775

matt-allan opened this issue May 1, 2019 · 7 comments

Comments

@matt-allan
Copy link
Contributor

There have been a few issues caused by 127.0.0.1 missing from the DNS servers:

What if we could make Valet check if the local DNS server is setup and either show a warning with instructions to fix it or fix it automatically?

It looks like this is possible with the networksetup utility, i.e.

$ networksetup -listallnetworkservices                                                                                                                                   
An asterisk (*) denotes that a network service is disabled.
Thunderbolt Ethernet
iPhone USB
Wi-Fi
Bluetooth PAN
PIA (L2TP)

$ networksetup -getdnsservers 'Wi-Fi'

1.1.1.1
1.0.0.1

$ networksetup -setdnsservers 'Wi-fi' 127.0.0.1 1.1.1.1 1.0.0.1

$ networksetup -getdnsservers 'Wi-Fi'

127.0.0.1
1.1.1.1
1.0.0.1

Any interest in doing this? If so I would be willing to put together a PR.

@drbyte
Copy link
Contributor

drbyte commented May 1, 2019

I like the possibility of automating this.

The wrinkle is that I've seen Valet run on numerous machines without ever having to set any DNS entries in the Mac ... which is cleaner. Granted, that isn't always "better". This is an opinionated tool, so being opinionated about this step may simplify usage (and support) even more.

Question: Is the change just as easily undone (removed)?

Structurally I'd put the code for it with the other Dnsmasq stuff, and then call it from there.
(I have a bunch of code I'm testing for a valet doctor command and improvements to valet uninstall, and your DNS calls could be nicely added in those.) (For now, maybe putting your check+warning+fix call to this as a last step of the install might make sense?)

@matt-allan
Copy link
Contributor Author

Hi,

Thanks for the prompt reply.

The wrinkle is that I've seen Valet run on numerous machines without ever having to set any DNS entries in the Mac ... which is cleaner.

Anecdotal, but in my case the problem was I had set custom DNS servers (Cloudflare's) and the problem was only noticeable when making curl requests from a PHP script to a valet domain. If I remove the custom DNS servers it works, and if I use file_get_contents instead of curl it works. I setup a small example repo that illustrates the problem if you want to try different configurations.

I imagine a lot of user's don't both have custom DNS servers and are making curl requests between Valet sites. I personally only noticed the issue because I was testing OAuth flows.

I think Valet would only need to add the dns server if networksetup -getdnsservers returned custom DNS servers.

Question: Is the change just as easily undone (removed)?

So you can remove the 127.0.0.1 entry. For the example above you would remove it like this:

networksetup -setdnsservers 'Wi-fi' 1.1.1.1 1.0.0.1

The problem is we would have to remember if they already had the 127.0.0.1 entry or not so we don't accidentally remove it if we weren't the ones that added it. It's also possible the user started relying on the 127.0.0.1 entry for some other reason since they started using Valet (i.e. using dnsmasq with vagrant), in which case uninstalling Valet would break other things.

I don't know very much about DNS, is there a downside to leaving the 127.0.0.1 entry? Does it make DNS resolution slower?

What if we check if 127.0.0.1 is in the entries and prompt the user to ask if they want to remove it when they valet uninstall?

Structurally I'd put the code for it with the other Dnsmasq stuff, and then call it from there.

For now, maybe putting your check+warning+fix call to this as a last step of the install might make sense?

That sounds good to me 👍

@drbyte
Copy link
Contributor

drbyte commented May 4, 2019

I don't know very much about DNS, is there a downside to leaving the 127.0.0.1 entry? Does it make DNS resolution slower?

Yes -- if it's listed as a server to check, and if nothing gives a hard fail/deny then it'll wait for a timeout before moving on to the next one in the list.
As you say, we could explore storing whether we added it as part of valet install, and optionally ask them about removing it later. Those are both a bit clunky, but may be necessary.

I wonder if there's a "simple" way to detect whether there's actually a DNS service responding on 127.0.0.1 and only add/remove if not? That might be more idempotent than logging and asking the user a question they may not know how to answer.

@matt-allan
Copy link
Contributor Author

It looks like we might be able to do this with dig:

$ dig +short +time=2 @127.0.0.1 TXT CHAOS version.bind
"dnsmasq-2.80"

When dnsmasq is not running:

$ dig +short +time=2 @127.0.0.1 TXT CHAOS version.bind
;; connection timed out; no servers could be reached

@drbyte
Copy link
Contributor

drbyte commented May 6, 2019

That's promising!

@drbyte
Copy link
Contributor

drbyte commented Dec 7, 2019

@driesvints
Copy link
Member

Hi there, we're cleaning up the Valet issue tracker and are moving all feature requests/support questions to the Discussions tab. We'll be using the issue tracker solely for bugs with Valet from now on. You're welcome to continue the discussion on in the Discussions tab. Thanks!

@laravel laravel locked and limited conversation to collaborators Dec 3, 2021
@driesvints driesvints converted this issue into a discussion Dec 3, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants