sshguard + service: init at 2.0.0 #23697
Conversation
|
@sargon, thanks for your PR! By analyzing the history of the files in this pull request, we identified @zohl, @edolstra and @offlinehacker to be potential reviewers. |
| @@ -0,0 +1,41 @@ | |||
| { stdenv, fetchgit, autoreconfHook, yacc, flex, pkgs}: | |||
There was a problem hiding this comment.
You don't need to pass the full pkgs to your derivation
| homepage = https://sshguard.net; | ||
| license = stdenv.lib.licenses.bsd3; | ||
| maintainers = [ maintainers.sargon ]; | ||
| platforms = stdenv.lib.platforms.all; |
There was a problem hiding this comment.
You don't need to rewrite stdenv.lib for licence and platforms.
Also on platforms : you are explicitly building for iptables backend so it won't work on Darwin.
| url = "https://bitbucket.org/sshguard/sshguard.git"; | ||
| rev = "8600fd5b70cd80fad87c414fd7c410082991a209"; | ||
| sha256 = "1i2ic8nca4f61pl2ggjp4x4an37hhvjgk2wv4smjsj6lvz3bw06a"; | ||
| }; |
There was a problem hiding this comment.
Why using fetchgit when you can get the stable release source from their SourceForge repo using fetchurl ?
https://sourceforge.net/projects/sshguard/files/sshguard/
|
Ah nice! I've made some comments on your |
|
Thanks for the review, I fixed most of your suggestions, but struggling with the multi platform support. |
sargon
force-pushed
the
master
branch
2 times, most recently
from
2f05b7b
to
05fd89c
Compare
|
Okay. I didn't see to get it right. So I reduced it to linux support and fix it all up into one patch. So someone with more experience in nix packages can pick it up. Or I will do it later when I got my mind around the builtins and stuff.... |
|
It's the safe course to target the package at Linux platforms only. It could still be enhanced for other platforms later on. Now on your Either you stick to v1.7.1 and your fetching should be something like src = fetchurl {
url = "mirror://sourceforge/sshguard/sshguard-${version}.tar.gz";
sha256 = "...";
}... or you try to build the v2.0.0 [release note]
|
|
Okay. You are right. I picked the 2.0.0 in advance to your urlfetch suggestion, but to my shame I didn't study the release notes that intensive. I will further adopt your proposals. Should we, for the sake of the PR title, close this one and start a new one stating the version number? |
|
We can change the title, no problem. Just force-push your changes. The code looks also good to me apart from @c0bw3b's comments. If you're ready I can give this a spin and merge. |
sargon
force-pushed
the
master
branch
2 times, most recently
from
771813d
to
f5529f8
Compare
sargon
changed the title
|
Okay, this should do the job. |
|
Tested here with |
|
@fpletz: I think you can give it a spin and merge, if you are satisfied with the changes. |
|
Another fixup from my side. Leaving creation of the ipsets to sshguards fw scripts seems no good idea. Sometimes the post startup actions gets started so fast, on some of my systems, that the first run of the sshguard firewall scripts hasn't happened. Hence the startup failed and systemd is immediately restarting the service. So we run into a classic timing problem here. So now the problematic system is starting the service correctly during boot. @fpletz merge? |
|
@sargon You have pushed the wrong commit. This seems to be a module for nullmailer. :) |
|
Narf. Must have happened this morning, just wanted to have that online. Fixed the branch, next time I should really not take the master branch for pull requests. |
|
@fpletz may you have another look? |
Motivation for this change
sshguard has a smaller footprint than e.g. fail2ban and support blacklisting repeating attackers.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)