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.

Sign up for GitHub

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

Jump to bottom

nixos/doh-server: init #371075

Merged
merged 1 commit into from
Jan 7, 2025
Merged

nixos/doh-server: init #371075

merged 1 commit into from
Jan 7, 2025
+253 −0

Conversation

DictXiong
Copy link
Member

@DictXiong DictXiong commented Jan 5, 2025 •
edited
Loading

Added module services.doh-server which:

  • enables a dns-over-https server,
  • sets up a systemd service as this official example, and
  • implements major options in this official example.

Tested on my cloud server which provides reliable DoH service for me.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jan 5, 2025
@DictXiong DictXiong requested a review from GaetanLepage January 5, 2025 07:45
@github-actions github-actions bot added the 8.has: module (update) This PR changes an existing module in `nixos/` label Jan 5, 2025
@DictXiong DictXiong self-assigned this Jan 5, 2025
@DictXiong DictXiong force-pushed the master branch 2 times, most recently from b315658 to dfec488 Compare January 5, 2025 07:52
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 5, 2025
misuzu
misuzu requested changes Jan 5, 2025
View reviewed changes
Copy link
Contributor

@misuzu misuzu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Reviewed points
  • module path fits the guidelines
  • module has the RFC-42 style settings and configFile options instead of an ad-hoc solution (see the crab-hole module for inspiration)
  • unprivileged user is used to run the service
  • options have appropriate types
  • options have defaults
  • options have examples
  • options have descriptions
  • no unneeded package is added to environment.systemPackages
  • meta.maintainers is set
  • a release notes entry for a module is present
  • module documentation is declared in meta.doc
  • examples from the module documentation are working
  • module has a NixOS test (optional, but recommended)
  • module NixOS tests succeed on ARCHITECTURE (if exists)

@DictXiong DictXiong requested a review from misuzu January 5, 2025 16:11
@DictXiong DictXiong requested a review from GaetanLepage January 5, 2025 16:22
misuzu
misuzu requested changes Jan 5, 2025
View reviewed changes
nixos/modules/services/networking/doh-server.nix Show resolved Hide resolved
nixos/modules/module-list.nix Show resolved Hide resolved
nixos/modules/services/networking/doh-server.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.nix Outdated Show resolved Hide resolved
@misuzu

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Jan 7, 2025
@DictXiong DictXiong force-pushed the master branch 2 times, most recently from b153a04 to 73d58e0 Compare January 7, 2025 09:18
@DictXiong DictXiong requested a review from misuzu January 7, 2025 09:31
misuzu
misuzu reviewed Jan 7, 2025
View reviewed changes
nixos/modules/services/networking/doh-server.md Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.md Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.md Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.md Outdated Show resolved Hide resolved
nixos/modules/services/networking/doh-server.md Outdated Show resolved Hide resolved
@DictXiong DictXiong force-pushed the master branch 2 times, most recently from 78ab0ad to 2b956d5 Compare January 7, 2025 10:03
@DictXiong DictXiong force-pushed the master branch 2 times, most recently from 737b197 to 4472be3 Compare January 7, 2025 10:42
@DictXiong DictXiong requested a review from misuzu January 7, 2025 10:49
@DictXiong DictXiong requested a review from misuzu January 7, 2025 11:07
@misuzu
Copy link
Contributor

misuzu commented Jan 7, 2025

I'll test the examples in the doc later today

@misuzu

This comment was marked as resolved.

Sign in to view
@DictXiong
Copy link
Member Author

How about adding services.doh-server.useACMEHost option?

That would be great. Thank you so much for your detailed suggestions.

@DictXiong DictXiong requested a review from misuzu January 7, 2025 16:07
misuzu
misuzu approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@misuzu misuzu left a comment

Choose a reason for hiding this comment

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

Looks perfect!

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 7, 2025
@fricklerhandwerk fricklerhandwerk merged commit 165e372 into NixOS:master Jan 7, 2025
25 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@misuzu misuzu misuzu approved these changes

@GaetanLepage GaetanLepage Awaiting requested review from GaetanLepage

@hsjobeki hsjobeki Awaiting requested review from hsjobeki

@GetPsyched GetPsyched Awaiting requested review from GetPsyched

@fricklerhandwerk fricklerhandwerk Awaiting requested review from fricklerhandwerk

Assignees

@DictXiong DictXiong

Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@DictXiong @misuzu @GaetanLepage @fricklerhandwerk @wegank