Skip to content

Conversation

@dotlambda
Copy link
Contributor

@dotlambda dotlambda commented Jun 1, 2025

See #2438 (comment).

  • have read the CONTRIBUTING.md file
  • raised a GitHub issue or discussed it on the projects chat beforehand
  • added unit tests
  • added integration tests
  • updated documentation if needed
  • updated CHANGELOG.md

@ghost
Copy link

ghost commented Jun 1, 2025

Pull Request Revisions

RevisionDescription
r1
DNS configuration default changedModified override_local_dns default from false to true in config-example.yaml

✅ AI review completed for r1
Help React with emojis to give feedback on AI-generated reviews:
  • 👍 means the feedback was helpful and actionable
  • 👎 means the feedback was incorrect or unhelpful
💬 Replying to feedback with a comment helps us improve the system. Your input also contributes to shaping future interactions with the AI reviewer.

We'd love to hear from you—reach out anytime at [email protected].

Copy link
Collaborator

@nblock nblock left a comment

Choose a reason for hiding this comment

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

Thanks for bringing this up, but I think we should change the default value in case the option dns.override_local_dns is missing in the configuration instead.

@dotlambda
Copy link
Contributor Author

we should change the default value in case the option dns.override_local_dns is missing in the configuration instead.

Sure but that would be another breaking change.

@nblock
Copy link
Collaborator

nblock commented Jun 2, 2025

Sure but that would be another breaking change.

Good point, depends on who you ask. Changing the default in the config is a breaking change for users that either started using Headscale with 0.26.0 or upgraded and migrated the config.

Changing the fallback value in the code is a breaking change for users that upgraded but did not migrate the configuration while upgrading to 0.26.0.

@kradalby what is your take?

@nblock nblock enabled auto-merge (rebase) August 19, 2025 11:54
@nblock
Copy link
Collaborator

nblock commented Aug 19, 2025

Sure but that would be another breaking change.

Let's go with your suggestion. Thx for bringing it up!

@nblock
Copy link
Collaborator

nblock commented Aug 19, 2025

@dotlambda could you please rebase and force-push? Github CI seems to be broken again :(

@kradalby kradalby closed this Aug 19, 2025
auto-merge was automatically disabled August 19, 2025 12:58

Pull request was closed

@kradalby kradalby reopened this Aug 19, 2025
@nblock nblock enabled auto-merge (rebase) August 19, 2025 12:59
@nblock nblock merged commit 51c6367 into juanfont:main Aug 19, 2025
84 of 85 checks passed
@dotlambda dotlambda deleted the patch-1 branch August 19, 2025 16:15
sysedwinistrator added a commit to sysedwinistrator/nixpkgs that referenced this pull request Jan 13, 2026
Since v0.26.0 [^1], Headscale overrides clients' local DNS settings by
default and therefore requires the configuration to provide a list of
nameservers to pass to them [^2][^3].

This commit exposes the dns.override_local_dns setting using the default
from upstream and validates that the required nameservers are provided
if it's enabled.

[^1]: https://github.com/juanfont/headscale/releases/tag/v0.26.0
[^2]: juanfont/headscale#2438
[^3]: juanfont/headscale#2640
      provide clients access to the local system's network in any way.
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.

3 participants