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

Default DNS Upstream #1608

Merged
merged 3 commits into from
Jul 2, 2024
Merged

Default DNS Upstream #1608

merged 3 commits into from
Jul 2, 2024

Conversation

PromoFaux
Copy link
Member

What does this PR aim to accomplish?:

See https://discourse.pi-hole.net/t/how-to-modify-upstream-server-only-via-gui-on-v6/70943/13?u=promofaux

Lower the barrier to entry/reduce confusion by setting DNS upstreams to 8.8.8.8;8.8.4.4 if we detect that it has not already been set by either the environment variable or config file. Falling back to a default rather than hard-exiting is probably more user friendly, on reflection.


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

  • I have read the above and my PR is ready for review. Check this box to confirm

@PromoFaux PromoFaux requested a review from a team June 30, 2024 20:16
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/how-to-modify-upstream-server-only-via-gui-on-v6/70943/20

…ent variable from the quickstart compose file.

Signed-off-by: Adam Warner <[email protected]>
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Why not only remove the exit 1 line and keep the upstream unset + print the warning?

@PromoFaux
Copy link
Member Author

Because ultimately nobody will read the warning.

I originally put the exit 1 to make it extremely obvious that something needed configuring, but if by defaulting it does not need to be configured immediately.

If we don't default and also don't exit, the startup checks wont always work (e.g in the case of the container having itself as a resolver - either via compose/run or via the host's settings)

@Gontier-Julien
Copy link

Might i suggest on using maybe Cloudflare Dns (1.1.1.1 & 1.0.0.1) or Quad9 Dns (9.9.9.9 & 149.112.112.112)?

My reasoning behind this is that for Cloudflare is the fastest dns out here, while Quad9 offer better privacy but slower.
Both have a privacy policies meanwhile Google Dns as none as far as i'm aware.

Providing a faster and/or more private dns upstream would be better i think for some default.

Also Cloudflare dns i widely know so i don't think many people would mind + having as faster (even slightly) dns is always a better user experience.

@PromoFaux
Copy link
Member Author

Google DNS just happens to be the one that's at the top of the list, so someone setting up a bare metal installation would arrive in exactly the same place if they just mashed enter throughout the installer.

Even in setting a default, this is by no means an opinionated default - it's just what happens to be the current default (and personally the one I fall back to out of habit). Users are still free to change to whichever one they want to use. The same could be said of the Steven Black blocklist that's included by default. We're not forcing it on anyone, it just prevents Pi-hole from not working out of the box - which can put newer/unfamiliar users off.

src/bash_functions.sh Outdated Show resolved Hide resolved
Co-authored-by: yubiuser <[email protected]>
Signed-off-by: Adam Warner <[email protected]>
@PromoFaux PromoFaux merged commit e26f366 into development-v6 Jul 2, 2024
7 checks passed
@PromoFaux PromoFaux deleted the v6/default-upstream branch July 2, 2024 19:14
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.

None yet

4 participants