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

nixos/zapret: init #347805

Merged
merged 2 commits into from
Oct 15, 2024
Merged

nixos/zapret: init #347805

merged 2 commits into from
Oct 15, 2024

Conversation

voronind-com
Copy link
Contributor

@voronind-com voronind-com commented Oct 10, 2024

Things done

Implement a systemd service for NixOS that enables Zapret package system-wide.
I've been using it for a while now.

Compared to #327903 this module has whitelist/blacklist support as well as some other configurations. Also after running this for a while, I solved some pain-points like repetitive network slowdowns. And no "dirty" parts.

Also this is my first contribution. I'd really appreciate assistance with making things right this first time. Please tell me all your thoughts. Thanks!

Example configuration:

services.zapret = {
  enable = true;
  package = pkgs.zapret;
  params = [
    "--dpi-desync=fake,disorder2"
    "--dpi-desync-ttl=1"
    "--dpi-desync-autottl=2"
  ];
  whitelist = [
    "youtube.com"
    "googlevideo.com"
    "ytimg.com"
    "youtu.be"
  ];
  # blacklist = [ ];
  qnum = 200;
  configureFirewall = true;
  httpSupport = true;
};
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@voronind-com
Copy link
Contributor Author

Looks like I actually need to create a release notes entry. On it now.

@voronind-com
Copy link
Contributor Author

voronind-com commented Oct 10, 2024

I totally missed an already open PR here: #327903

Closing this.

@voronind-com
Copy link
Contributor Author

voronind-com commented Oct 10, 2024

Actually, now that I think about it, I'll keep this PR open as an alternative.

@voronind-com voronind-com reopened this Oct 10, 2024
@voronind-com voronind-com changed the title NixOS: Implement Zapret service. nixos/zapret: init alt. Oct 10, 2024
@ardishko
Copy link
Contributor

Why not put this in a seperate repo under a flake? The maintainers said that the other PR was done dirtily and it will probably hang as an open PR for a while due to such.

@voronind-com voronind-com mentioned this pull request Oct 10, 2024
13 tasks
@voronind-com
Copy link
Contributor Author

Why not put this in a seperate repo under a flake? The maintainers said that the other PR was done dirtily and it will probably hang as an open PR for a while due to such.

My implementation doesn't have the dirty part.

Copy link
Contributor

@ardishko ardishko left a comment

Choose a reason for hiding this comment

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

LGTM. I hope this is merged into master soon. Russian and Turkish NixOS users definitely need this right now regarding all the bans and slowdowns recently.

Copy link
Contributor

@Nishimara Nishimara left a comment

Choose a reason for hiding this comment

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

this looks cuter than mine PR :D

Copy link
Contributor

@s0me1newithhand7s s0me1newithhand7s left a comment

Choose a reason for hiding this comment

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

tysm 🙏🏻for this, and welcome to the nixos, friend. looks good to me, tho, i'll add labels in case there missing some.

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.

This is excellent! Aside from the comments below, the commits should be squashed according to the Commit Conventions.

Something like this should work:

  1. maintainers: add voronind
  2. nixos/zapret: init

nixos/modules/services/networking/zapret.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/zapret.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/zapret.nix Show resolved Hide resolved
nixos/modules/services/networking/zapret.nix Show resolved Hide resolved
nixos/modules/services/networking/zapret.nix Outdated Show resolved Hide resolved
@s0me1newithhand7s
Copy link
Contributor

This is excellent! Aside from the comments below, the commits should be squashed according to the Commit Conventions.

Something like this should work:

  1. maintainers: add voronind
  2. nixos/zapret: init

fully agree with it.

@SigmaSquadron
Copy link
Contributor

@ofborg test simple

@ardishko
Copy link
Contributor

This is excellent! Aside from the comments below, the commits should be squashed according to the Commit Conventions.
Something like this should work:

  1. maintainers: add voronind
  2. nixos/zapret: init

fully agree with it.

Me too.

@purrpurrn
Copy link
Contributor

Turkish & Russian users can already use this PR/MR, but the quality of the module may vary. It should be good enough for now, as it is an emergency.

Guide:
To use Zapret:

  1. In your flake.nix, define:

    inputs.zapret.url = "github:NixOS/nixpkgs/3b34229a4940f2457902ed884af16955044eee7b";
  2. Add the following snippet to your Nix(OS) configuration:

    imports = [
      "${inputs.zapret}/nixos/modules/services/networking/zapret.nix"
    ];

@voronind-com
Copy link
Contributor Author

voronind-com commented Oct 11, 2024

At least rename services.zapret, please. That would help avoid name conflict with aca/zapret-flake.nix

Sorry, I need other maintainers guidance on this. I don't see a reason why nixpkgs should follow flakes from the wild, but I might be wrong. For now it follows consistent logic of all other NixOS package-specific services. Anyway why would anyone use both at the same time?

@SigmaSquadron
Copy link
Contributor

Sorry, I need other maintainers guidance on this. I don't see a reason why nixpkgs should follow flakes from the wild, but I might be wrong. For now it follows consistent logic of all other NixOS package-specific services. Anyway why would anyone use both at the same time?

There's no need to care about what other repos do. services.zapret is appropriate.

@Nishimara
Copy link
Contributor

Nishimara commented Oct 11, 2024

In that case, I don't see much advantage of your solution over https://github.com/aca/zapret-flake.nix or nishimara's solution.

If you decide to keep this flake in inputs after merge, you need to disable zapret in nixpkgs

disabledModules = [
  "services/networking/zapret.nix"
];

@ardishko
Copy link
Contributor

Just dropping in to ask about OfBorg's checks since I'm not familiar. It's been about 12 hours, why havent the darwin checks been completed yet?

@SigmaSquadron
Copy link
Contributor

Just dropping in to ask about OfBorg's checks since I'm not familiar. It's been about 12 hours, why havent the darwin checks been completed yet?

Don't worry about it. Darwin checks are optional for Linux/NixOS changes and they usually take days to complete, especially near the final release staging cycles.

Emily has been working on improving the Darwin OfBorg CI situation; hopefully their completion times will approach Linux's soon.

@ardishko
Copy link
Contributor

Can we skip the check if it's optional? I remember someone did that with a command once.

@SigmaSquadron
Copy link
Contributor

No. I suggest ignoring it.

@voronind-com
Copy link
Contributor Author

No. I suggest ignoring it.

Then we still need someone who could merge this. Or is it automatic?

@SigmaSquadron
Copy link
Contributor

SigmaSquadron commented Oct 11, 2024 via email

@s0me1newithhand7s
Copy link
Contributor

No. I suggest ignoring it.

Then we still need someone who could merge this. Or is it automatic?

that's funny how you got actually a lot approvals from members, but not from committers. 👀

@voronind-com
Copy link
Contributor Author

Checks just passed. Darwin took like 3 days wow.

@s0me1newithhand7s
Copy link
Contributor

Checks just passed.

hooray!

@ardishko
Copy link
Contributor

Hopefully a commiter sees this now.

@voronind-com
Copy link
Contributor Author

voronind-com commented Oct 15, 2024

I'm in chat with a commiter. We added the optionalString and now waiting for the ofborg eval before merging. I also re-tested locally.

@s0me1newithhand7s
Copy link
Contributor

I'm in chat with a commiter. We added the optionalString and now waiting for the ofborg eval before merging. I also re-tested locally.

good luck, pal 👍🏻

@azahi azahi changed the title nixos/zapret: init alt. nixos/zapret: init Oct 15, 2024
@azahi azahi merged commit e233795 into NixOS:master Oct 15, 2024
31 of 32 checks passed
@s0me1newithhand7s
Copy link
Contributor

congrats @voronind-com! and welcome to the nixos/nixpkgs. 🎉

@voronind-com voronind-com deleted the zapret-service branch October 15, 2024 21:49
@ardishko
Copy link
Contributor

YOOOOOOOOOOOOOOOOOOOOOOOOOOO!!!!!! FINALLY!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.