-
-
Notifications
You must be signed in to change notification settings - Fork 601
Add warning to the multiple DNS advertisement #3571
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
Conversation
Quick workaround for pi-hole/pi-hole#6360 for details. Signed-off-by: wsw70 <[email protected]>
@@ -116,7 +116,7 @@ mg.include('scripts/lua/settings_header.lp','r') | |||
<div class="col-sm-12"> | |||
<div> | |||
<input type="checkbox" id="dhcp.multiDNS" data-key="dhcp.multiDNS" class="DHCPgroup"> <label for="dhcp.multiDNS"><strong>Advertise DNS server multiple times</strong></label> | |||
<p class="help-block">Advertise DNS server multiple times to clients. Some devices will add their own proprietary DNS servers to the list of DNS servers, which can cause issues with Pi-hole. This option will advertise the Pi-hole DNS server multiple times to clients, which should prevent this from happening.</p> | |||
<p class="help-block">Advertise DNS server multiple times to clients. Some devices will add their own proprietary DNS servers to the list of DNS servers, which can cause issues with Pi-hole. This option will advertise the Pi-hole DNS server multiple times to clients, which should prevent this from happening. This is not compatible with adding dhcp-option=6,... in dnsmasq_lines in /etc/pihole/config.toml or /etc/dnsmasq.d/</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few format suggestions:
<p class="help-block">Advertise DNS server multiple times to clients. Some devices will add their own proprietary DNS servers to the list of DNS servers, which can cause issues with Pi-hole. This option will advertise the Pi-hole DNS server multiple times to clients, which should prevent this from happening. This is not compatible with adding dhcp-option=6,... in dnsmasq_lines in /etc/pihole/config.toml or /etc/dnsmasq.d/</p> | |
<p class="help-block">Advertise DNS server multiple times to clients. Some devices will add their own proprietary DNS servers to the list of DNS servers, which can cause issues with Pi-hole. This option will advertise the Pi-hole DNS server multiple times to clients, which should prevent this from happening.<br>This is not compatible with adding <code>dhcp-option=6,...</code> in <code>dnsmasq_lines</code> in <em>/etc/pihole/config.toml</em> or <em>/etc/dnsmasq.d/</em></p> |
Isn't this really kind of covered by the present warning ?
There are other options that can be changed in the UI (or directly editing pihole.toml) that are incompatible with adding the same option again in A quick look at pihole.toml suggests port, cache-size, use-stale-cache, local-service, log-facility. And considering other options that are not enabled by default, probably most of the options under [dhcp]. If they all had additional warnings listed at the level of their option, the UI would get awfully cluttered. (As would all of the additional text in |
I agree with @rrobgill. We can't warn about each possible incompatible option with whatever users might set in |
I understand that everything cannot be warned against. In that case however the warning in the ligs points to a problem nowhere to be find in the config file - one needs to check Adding extra DNS servers is also something not that exotic (though less likely with Pi-hole). I am fine with closing the PR since there are now 2:0 comments stating this is not a good idea. |
Quick workaround for pi-hole/pi-hole#6360 for details.
Signed-off-by: wsw70 [email protected]
What does this PR aim to accomplish?:
Update the UI
Advertise DNS server multiple times
description to account for pi-hole/pi-hole#6360How does this PR accomplish the above?:
By adding to the description
By submitting this pull request, I confirm the following:
git rebase
)