pihole: init at various #224881
pihole: init at various #224881
Conversation
| TEMPERATUREUNIT = uiConfig.temperatureUnit; | ||
| WEBTHEME = uiConfig.theme; |
There was a problem hiding this comment.
These two settings come from the pihole-admin module. It's a bit dodgy that this single config file is shared between the two.
I think patching pihole-admin to use a separate config file will be difficult, since it accesses both its own and pihole-ftl's settings here.
ofborg
bot
added
11.by: package-maintainer
10.rebuild-darwin: 0
10.rebuild-linux: 1-10
labels
williamvds
force-pushed
the
add_pihole
branch
3 times, most recently
from
a7bc8c3
to
f1a9471
Compare
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/2413 |
pkgs/tools/networking/pihole/0002-Patch-and-remove-unsupported-commands.patch
Outdated
Show resolved
Hide resolved
|
Still need to look into resholve, will request another review then |
williamvds
force-pushed
the
add_pihole
branch
from
909b43e
to
437d9f4
Compare
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/2648 |
williamvds
force-pushed
the
add_pihole
branch
2 times, most recently
from
6d0adc3
to
d4aaa1a
Compare
williamvds
force-pushed
the
add_pihole
branch
from
d4aaa1a
to
3ec0d54
Compare
|
Docs should build now since the dnsmasq PR was merged. I attempted a NixOS test, but the service startup currently performs network requests regardless of configuration. Naturally those fail due to the test sandboxing. I'll look at patching that behaviour out and implementing a test after this PR. |
Include a patch which removes the termcap linking. Without this patch, pihole-ftl fails to link with the following error: > /nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: shell.c:(.text+0x2e282): undefined reference to `stifle_history' > /nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: shell.c:(.text+0x2e28a): undefined reference to `write_history' > /nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: shell.c:(.text+0x2e886): undefined reference to `read_history' > /nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: database/CMakeFiles/sqlite3.dir/shell.c.o: in function `readline_completion': > shell.c:(.text+0x7792): undefined reference to `rl_completion_matches' > /nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: warning: creating DT_TEXTREL in a PIE > collect2: error: ld returned 1 exit status Also remove the reference to `/opt/pihole/libs`, to prevent non-sandboxed paths from being accessed at build time. Co-Authored-By: JamieMagee <jamie.magee@gmail.com> Co-Authored-By: nuxeh <1516017+nuxeh@users.noreply.github.com>
Which is the core admin script for pihole, which modifies the pihole-FTL and gravity (DNS settings) database. It requires quite a lot of patching to remove most of the hard-coded absolute paths, and add the many dependencies the script has. Patches also remove numerous unnecessary/unsupported operations from the script, including reinstalling, uninstalling, and updating pihole, pihole-FTL, and pihole-AdminLTE. Some scripts also include the installation scripts for some utilities, so these utils need to be copied over to the files which are included in the Nix package.
So that the pihole module reuse it. pihole-ftl is a fork of dnsmasq and its configuration file is compatible.
Add a module for pihole-ftl, which allows declaratively defining the setupVars.conf and pihole-FTL.conf configuration files. Also provide options for adlists to use, which can be added through the pihole script (packaged as "pihole"). Other state such as clients and groups require complex database operations, which is normally performed by the pihole admin webapp (packaged as "pihole-admin"). Extend the dnsmasq module to avoid duplication, since pihole-ftl is a soft-fork of dnsmasq which maintains compatibility. Provide the pihole script in `environment.systemPackages` so pihole-ftl can be easily administrated.
Pihole's dashboard is a web app which visualises statistics from pihole-FTL
(i.e. dnsmasq), shows query logs, and allows configuration.
With this module, configuration is largely declarative and immutable, so
settings can't be changed, but they can be viewed from the webpage.
The admin page also allows regenerating the DNS ("gravity") database, which
requires write access to the pihole state directory, as well as being able to
signal pihole-FTL to reload its DNS cache. For the latter, a polkit rule is
optionally enabled.
|
Is the failure on x86_64-linux ~expected? |
Looks like the error is in a test for a dnsmasq exporter for prometheus. I think the test is running because the dnsmasq module was modified as part of the PR. I ran the test with Since it's currently failing on master it doesn't seem to be this PR's fault |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/3809 |
|
Apologies for the delay on my part. I will review the resholve parts of this. I was going to start on it this morning, but I forgot that I need to do it at home where I have access to a linux system so I can directly poke at it and look over the output. |
|
I did start looking at this, but didn't get terribly far yet. A resholve bump that had been in staging the past few weeks landed in master/unstable this weekend, and a fix I made there for better cross support broke compatibility with one I have that fixed and opened a bump PR (linked above, again through staging). I have pihole building on top of that bump already, so I should be able to get back to reviewing now. |
|
I'm afraid I'm going to close this PR for the forseeable future. I truly appreciate all the effort with the reviews, and particularly thanks for your recent attention @abathur. Unfortunately with the continuing drama within the community I'm struggling to find the motivation to continue this effort. For my own mental health, I need to keep my mind off Nix until things and my own mind can improve. |
|
@williamvds Understandable. Struggling with the same. I planned to get back to reviewing it this afternoon, but I'll hold off. Please do ping me (here or elsewhere) if/when you feel like picking it back up. For my own reference: should I also hold queries on your resholve-specific thoughts/observations? For a concrete example, you left a well-observed comment about all scripts in one solution having a shared namespace (leading to a function in one obscuring an external executable in others). One of my TODOs regardless of this PR's status is to double-check whether I documented that behavior and reflect on whether it just needs documenting or is a footgun I should rework. I like getting fresher perspectives--but it's not worth undermining your ability to get a healthy amount of separation. |
|
@abathur thanks
I'm happy to talk about resholve outside of Nix channels, but I'd rather not test things out myself in the context of Nix & nixpkgs |
Description of changes
Pihole describes itself as "A black hole for Internet advertisements".
It's a suite for running and administrating a DNS sinkhole, which filters queries and caches their results, primarily to block advertisements and other undesirable traffic originating from the LAN.
Built on dnsmasq, the DNS server (called pihole-FTL) can also be used as a DHCP server to automatically configure LAN clients to use pihole's DNS.
The pihole-ftl package is based on #108055 by @JamieMagee (itself based on the work of @nuxeh). The remaining work was done mostly from scratch.
This PR resolves the package request issue #61617.
This PR includes packages for:
The last one needs the heaviest modifications, particularly to excise the reinstallation/upgrade functionality, which is not needed on NixOS.
NixOS modules are provided for pihole-ftl and pihole-adminlte.
pihole-ftl's module is largely a wrapper around the existing dnsmasq one, since they are mostly compatible applications. Some additional options are provided for automatically setting the adlists for pihole to download and apply.
pihole-adminlte's module is rather minimal, with some cosmetic options for temperature units and the web theme.
To do:
/etc/pihole/gravity.dbis createdThings done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)