Skip to content

nixos/networking: deprecate networking.extraHosts #413925

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JohnRTitor
Copy link
Member

@JohnRTitor JohnRTitor commented Jun 4, 2025

There is already networking.hosts that replicates its features with the added benefit of defining multiple hostnames for same IP in a list.

Something like;

  networking.extraHosts = ''
    169.254.169.254 metadata.google.internal metadata
  '';

can be changed to:

  networking.hosts = {
    "169.254.169.254" = [
      "metadata.google.internal"
      "metadata"
    ];
  };

All modules in nixos/modules/** has been migrated to networking.hosts.
Usage and occurances in tests on Nixpkgs can be migrated in seperate PRs.

After a discussion on Matrix with K900, it's best we deprecate this. This option is set to be removed in 26.05. 1 year of deprecation period allows consumers to be migrated safely.

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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.

@JohnRTitor JohnRTitor requested a review from K900 June 4, 2025 14:29
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jun 4, 2025
Copy link
Contributor

@SigmaSquadron SigmaSquadron left a comment

Choose a reason for hiding this comment

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

I'd recommend adding an evaluation warning if the option is set, unless too many modules configure it in-tree.

@JohnRTitor
Copy link
Member Author

I am seeing 54 usage in Nixpkgs, most of which come from tests.

@khaneliman
Copy link
Contributor

khaneliman commented Jun 4, 2025

I'd recommend adding an evaluation warning if the option is set, unless too many modules configure it in-tree.

This was my first reaction, as well.. I don't know how a user would migrate away from it until they receive notice. But, if the idea is to just add a description notice for in tree development while we wait for in tree migration, that is fine. However, a warning would also drive migration quicker :P

JohnRTitor added a commit to JohnRTitor/nix-conf that referenced this pull request Jun 4, 2025
@github-actions github-actions bot added the 6.topic: nixos-container Imperative and declarative systemd-nspawn containers label Jun 4, 2025
Copy link
Member Author

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

image

I have migrated all modules in nixos/modules/** to networking.hosts.

@JohnRTitor JohnRTitor force-pushed the deprecate-extraHosts branch 3 times, most recently from 8376dbb to 8544c5f Compare June 4, 2025 19:20
@JohnRTitor JohnRTitor force-pushed the deprecate-extraHosts branch from 8544c5f to e686dd5 Compare June 4, 2025 19:31
@JohnRTitor

This comment was marked as outdated.

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

I like it. @alyssais, what do you think?

@khaneliman

This comment was marked as resolved.

@PedroHLC
Copy link
Member

PedroHLC commented Jun 4, 2025

I know some anime-styled games alternative-launchers that expect to see some very specific commentaries in /etc/hosts before launching/patching. How are we going to inject those comments without this field that allows abstract values?

FIY: This was attempted before #27791

EDIT: This seems more like a problem of vocabulary, misleading to bad usage patterns. Instead, this option should be renamed to something that does not mention “hosts”, like appendAbstractly, and document it to “use it as a last resource for automation”.

@JohnRTitor

This comment was marked as resolved.

@JohnRTitor
Copy link
Member Author

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 413925

Logs: https://github.com/JohnRTitor/nixpkgs-review-gha/actions/runs/15450221622


x86_64-linux (sandbox = true)

✅ 5 tests built:
  • nixosTests.containers-hosts
  • nixosTests.kubernetes.dns-multi-node
  • nixosTests.kubernetes.dns-single-node
  • nixosTests.kubernetes.rbac-multi-node
  • nixosTests.kubernetes.rbac-single-node

Set it for removal in 26.05 so people are forced to use it.

Usage and occurances in tests on Nixpkgs can be migrated in seperate PRs.

Signed-off-by: John Titor <[email protected]>
@JohnRTitor JohnRTitor force-pushed the deprecate-extraHosts branch from e686dd5 to 8005d49 Compare June 7, 2025 15:14
@github-actions github-actions bot added the 6.topic: testing Tooling for automated testing of packages and modules label Jun 7, 2025
Copy link
Member Author

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

Note that the first commit is "general improvement". And can be merged irrespective of the decision.

@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 25, 2025
@tomfitzhenry
Copy link
Contributor

Nice work here, would be good to see this re-based and ready for review.

@JohnRTitor
Copy link
Member Author

This kind of is "ready" for review except for the merge conflict part.

Note that treewide migration to networking.hosts had to be reverted in #415085 as it was breaking tests.

You are more than welcome to redo that PR. I am not sure on how to move forward/fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: nixos-container Imperative and declarative systemd-nspawn containers 6.topic: testing Tooling for automated testing of packages and modules 8.has: changelog This PR adds or changes release notes 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 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants