crowdsec-firewall-bouncer: init at 0.0.33 by TornaxO7 · Pull Request #412880 · NixOS/nixpkgs

TornaxO7 · 2025-06-01T09:05:43Z

Things done

Add a 👍 reaction to pull requests you find important.

TornaxO7 · 2025-06-01T09:15:50Z

@06kellyjac may I ask for some suggestions here?

I saw here (#279874 (comment)) that you'd like to split the package into two outputs. Should I give it a try (I've never done that before)?

06kellyjac · 2025-06-01T10:23:55Z

You could look at the crane package for reference maybe.
It has 3 outputs total: out (the main/default one), crane, and gcrane.
The binaries are moved into their respective outputs and symlinks are added to out.
There are also completion files being generated but that may not be necessary.
Overall it just means by default if someone does nix-shell -p crane or installs it they'll get both, if someone only needs crane they can specify crane.crane

For other examples searching for outputs = should find a few things. :)

06kellyjac · 2025-06-01T10:24:41Z

Or alternatively we can have it all in one output for now. Just having it packaged would be good

TornaxO7 · 2025-06-01T11:22:24Z

Or alternatively we can have it all in one output for now. Just having it packaged would be good

I think that would simplify it xD

TornaxO7 · 2025-06-01T11:25:23Z

Regarding testing: I don't really know how to test it. 👀
Running

nix build .#crowdsec-firewall-bouncer
./result/bin/crowdsec-firewall-bouncer

gives me this:

FATA[0000] configuration file is required

Running

./result/bin/crowdsec-firewall-bouncer --help

prints the help page.

TornaxO7 · 2025-06-01T14:20:55Z

Ok, I could run it and it printed some valid error messages.

TornaxO7 · 2025-06-02T14:26:05Z

Or alternatively we can have it all in one output for now. Just having it packaged would be good

@06kellyjac (just in case) I'd say, it's ready for review then. Having it as a package to continue working on the service would be very nice. :) (I'd reaaaaaaaaaaaaally love to have crowdsec as a module).

acuteaangle · 2025-06-05T17:49:32Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 412880

`x86_64-linux`

❌ 1 package failed to build:

crowdsec-firewall-bouncer

error: hash mismatch in fixed-output derivation '/nix/store/xix9x1filik5k8wa7d91s9npf1npgx2y-source.drv':
         specified: sha256-59MWll8v00CF4WA53gjHZSTFc8hpYaHENg9O7LgTCrA=
            got:    sha256-4fxxAW2sXGNxjsc75fd499ciuN8tjGqlpRIaHYUXwQ0=
error: 1 dependencies of derivation '/nix/store/6mlx6g8sbk1fpax1yj51005niv5zm4gs-crowdsec-firewall-bouncer-0.0.33-go-modules.drv' failed to build
error: 2 dependencies of derivation '/nix/store/kanxa877yhg6p5ic3phyxlwibk4gz5sc-crowdsec-firewall-bouncer-0.0.33.drv' failed to build
error: 1 dependencies of derivation '/nix/store/5ks9bcf2z8vdfmvj3awyhp6pj72m64sr-review-shell.drv' failed to build

✅ 7 packages built:

eduke32
nixpkgs-manual
super-productivity
veilid
veilid.dev
veilid.lib
zizmor

acuteaangle · 2025-06-05T17:51:16Z

src.hash seems to be outdated

TornaxO7 · 2025-06-05T18:34:14Z

That's... interesting... I successfully build with nix bulid .#crowdsec-firewall-bouncer... I'll fix it

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix

acuteaangle · 2025-06-05T19:34:05Z

That's... interesting... I successfully build with nix bulid .#crowdsec-firewall-bouncer... I'll fix it

Nix will continue using an old version; if the hash exists in the store, it'll skip fetching the source and will use what it has.

We should add versionCheckHook to fix this, but currently cs-firewall-bouncer --version gives an empty version output for me:

version:
BuildDate:
GoVersion: 1.24.3
Platform: linux

acuteaangle · 2025-06-05T20:11:16Z

~~A bit later~~ I'll try to figure out why --version gives an empty version. It's really useful to have versionCheckHook working: it
prevents Nix from using stale hashes from old versions, it helps ensure nix-update-script is functioning properly, and it act as a very basic test that the binary can run at all. It does, however, need version to be a substring of the output.

EDIT:
I won’t be able to get to this tonight.

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix

acuteaangle

This gets versionCheckHook working :)
Since we're setting the version based on src.tag, it won't help against a stale hash, but I still think it's a good idea to include it.

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix

acuteaangle · 2025-06-10T09:02:39Z

I managed to get extracting version information from git working, but it feels like a decent amount of complexity just to get the full benefit of versionCheckHook. I’ll throw the patch here anyways, in case future reviewers have any thoughts about it.

crowdsec-firewall-bouncer-extract-metadata-from-git.patch

From: Summer Tea <acuteaangle@disroot.org>
Date: Tue, 10 Jun 2025 03:36:14 -0400
Subject: [RFC PATCH] crowdsec-firewall-bouncer: extract metadata from git
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Extract versioning information from `git` to allow `versionCheckHook`
to validate that {version} and {src.hash} are maintained in sync.

`.git` is deleted in postFetch after the desired information is written
out, to avoid reproducibility issues described in [#8567].

Additionally, `-version` gives unsurprising output—in contrast to fields
previously being blank or unique to Nix.

The upstream Makefile sets the version metadata to the following values:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
version.Version=$(git describe --tags)
version.BuildDate=$(date +%F"_"%T)
version.Tag=$(git rev-parse HEAD)'
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
While these are questionably named and perhaps do not produce 'optimal'
output, for now replicate what the upstream Makefile does, with the
exception of substituting the commit date for the build date.

`git describe --tags` seems to produce different output in fetchgit,
despite operating on the same commit.
tag `v0.0.33` (cb8b3e3) gives `v0.0.33` when run interactively against
the upstream repo, but within fetchgit it instead returns
`v0.0.33-rc1-1-gcb8b3e3`, which is technically correct, but different.

This whole version system seems fairly janky. `-version` prints
`v0.0.33-rc1-1-gcb8b3e3-cb8b3e3c654499f745ff487eb1c327d7234a533f`,
catenating the output of `git describe --tags` (which may already
include the commit hash) with the commit hash.

Either `git log --tags --long --abbrev=40` or `git log --tags --abbrev=0`
seems like it would make more sense here, but out of scope
for Nixpkgs.

[#8567]: https://github.com/NixOS/nixpkgs/issues/8567
---

I managed to get extracting version information from `git` working,
but it feels like a decent amount of complexity just to get the full
benefit of `versionCheckHook`. I’ll throw the patch here anyways,
in case future reviewers have any thoughts about it.

 .../cr/crowdsec-firewall-bouncer/package.nix  | 40 +++++++++++++++++--
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix b/pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix
index f7b3116e41fd..2b640d3f7c2c 100644
--- a/pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix
+++ b/pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix
@@ -13,14 +13,46 @@ buildGoModule rec {
     owner = "crowdsecurity";
     repo = "cs-firewall-bouncer";
     tag = "v${version}";
-    hash = "sha256-4fxxAW2sXGNxjsc75fd499ciuN8tjGqlpRIaHYUXwQ0=";
+    hash = "sha256-lfIRKFGVB++sAVDnGujh0VwCyZmfdxXtl3rK8V7xVr0=";
+    leaveDotGit = true;
+    deepClone = true; # needed for `git describe`
+    # By extracting metadata from the git repository in postFetch, we can
+    # delete the rest of `.git` afterwards and avoid the reproducibility
+    # issues described in <https://github.com/NixOS/nixpkgs/issues/8567>.
+    postFetch = ''
+      pushd "$out"
+      # Store the current commit hash and the output of `git describe`
+      # to give to the program during the build.
+      #
+      # Do this rather than directly supplying `{version}` or `{src.tag}`,
+      # as it allows `versionCheckHook` to function as a better sanity check.
+      git describe --tags > COMMIT_DESCRIBE
+      git rev-parse HEAD > COMMIT
+      # Format using `date` rather than git’s builtin `--date` option, as
+      # it’s an easy way to be _certain_ times will be in UTC (without relying
+      # TZ=UTC0), and we can directly use the expected format string from the
+      # Makefile, since it would call `date`.
+      #
+      # While it would be possible to leave the date unset (the program
+      # would print an empty string in its place) or set it to a fixed epoch,
+      # providing the commit date lets it give a useful and unsurprising value.
+      date -u -d "@$(git log -1 --pretty=%ct)" '+%F_%T' > SOURCE_DATE
+      # We want to remove *every* `.git` directory, in case of submodules.
+      find -name .git -print0 | xargs -0 rm -rf
+      popd
+    '';
   };

   vendorHash = "sha256-Bhp6Z2UlCJ32vdc3uINCGleZFP2WeUn/XK+Q29szUzQ=";

-  ldflags = [
-    "-X github.com/crowdsecurity/go-cs-lib/version.Version=${src.tag}"
-  ];
+  preBuild = ''
+    # Poorly named, but what the programs expects
+    pushd "$src"
+    ldflags+=" -X github.com/crowdsecurity/go-cs-lib/version.Version=$(< COMMIT_DESCRIBE)"
+    ldflags+=" -X github.com/crowdsecurity/go-cs-lib/version.BuildDate=$(< SOURCE_DATE)"
+    ldflags+=" -X github.com/crowdsecurity/go-cs-lib/version.Tag=$(< COMMIT)"
+    popd
+  '';

   nativeInstallCheckInputs = [ versionCheckHook ];

acuteaangle · 2025-06-10T09:07:29Z

Next chance I get some more free time, I’m hoping to try out this package with the WIP module at #387625.
It’d be nice to know if this actually functions :)

Defelo

also please don't forget to squash your commits 😉

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix

TornaxO7 · 2025-06-13T09:05:04Z

also please don't forget to squash your commits 😉

Aye! (And: Long time no see :D)

06kellyjac · 2025-06-13T12:20:08Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 412880

`x86_64-linux`

✅ 1 package built:

crowdsec-firewall-bouncer

06kellyjac

Happy to maintain with you but otherwise LGTM

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix

Defelo · 2025-06-13T14:03:27Z

`nixpkgs-review` result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 412880

Logs: https://github.com/Defelo/nixpkgs-review-gha/actions/runs/15636318062

Download packages from cache:

x86_64-linux

nix-store -r --add-root nixpkgs-pr-412880-x86_64-linux \
  --option binary-caches 'https://cache.nixos.org/ https://attic.defelo.de/nixpkgs' \
  --option trusted-public-keys '
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  nixpkgs:xeaAWa3crK09hMmFiygBeRmLq3hUjUShgaAwYVUEtw0=
  ' \
  /nix/store/c80hgminwhdpnv1530h7ksndfgvpj5xs-crowdsec-firewall-bouncer-0.0.33

aarch64-linux

nix-store -r --add-root nixpkgs-pr-412880-aarch64-linux \
  --option binary-caches 'https://cache.nixos.org/ https://attic.defelo.de/nixpkgs' \
  --option trusted-public-keys '
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  nixpkgs:xeaAWa3crK09hMmFiygBeRmLq3hUjUShgaAwYVUEtw0=
  ' \
  /nix/store/9fj8dgci75z25g3jcm6yk5mgfvfh3yxj-crowdsec-firewall-bouncer-0.0.33

aarch64-darwin

nix-store -r --add-root nixpkgs-pr-412880-aarch64-darwin \
  --option binary-caches 'https://cache.nixos.org/ https://attic.defelo.de/nixpkgs' \
  --option trusted-public-keys '
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  nixpkgs:xeaAWa3crK09hMmFiygBeRmLq3hUjUShgaAwYVUEtw0=
  ' \
  /nix/store/cc7bs5qyvcymicr2s1mq9dqijgmkxkzi-crowdsec-firewall-bouncer-0.0.33

`x86_64-linux` (sandbox = true)

✅ 1 package built:

crowdsec-firewall-bouncer

`aarch64-linux` (sandbox = true)

✅ 1 package built:

crowdsec-firewall-bouncer

`x86_64-darwin` (sandbox = true)

✅ 1 package built:

crowdsec-firewall-bouncer

`aarch64-darwin` (sandbox = true)

✅ 1 package built:

crowdsec-firewall-bouncer

Defelo

Diff LGTM, however can't test this myself.

Maybe consider rewriting your commit message to mention each co-author only once and remove the old messages ("Update ...", "format ...")

TornaxO7 · 2025-06-13T16:57:20Z

Diff LGTM, however can't test this myself.

Maybe consider rewriting your commit message to mention each co-author only once and remove the old messages ("Update ...", "format ...")

ok

Co-authored-by: Summer Tea <79724236+acuteaangle@users.noreply.github.com> Co-authored-by: Felix Bargfeldt <41747605+Defelo@users.noreply.github.com> Co-authored-by: j-k <dev@j-k.io>

TornaxO7 force-pushed the crowdsec-firewall-bouncer branch from 44cae38 to 7629126 Compare June 1, 2025 09:11

TornaxO7 marked this pull request as draft June 1, 2025 09:15

TornaxO7 marked this pull request as ready for review June 1, 2025 11:22

acuteaangle reviewed Jun 5, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

TornaxO7 force-pushed the crowdsec-firewall-bouncer branch from 906289c to f0ca41b Compare June 6, 2025 03:52

acuteaangle suggested changes Jun 6, 2025

View reviewed changes

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

TornaxO7 force-pushed the crowdsec-firewall-bouncer branch 2 times, most recently from c5944de to f9a35db Compare June 6, 2025 17:25

acuteaangle reviewed Jun 6, 2025

View reviewed changes

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

acuteaangle reviewed Jun 6, 2025

View reviewed changes

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

Defelo requested changes Jun 10, 2025

View reviewed changes

TornaxO7 force-pushed the crowdsec-firewall-bouncer branch from 5706932 to 8927354 Compare June 13, 2025 09:10

06kellyjac approved these changes Jun 13, 2025

View reviewed changes

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/cr/crowdsec-firewall-bouncer/package.nix Outdated Show resolved Hide resolved

Defelo approved these changes Jun 13, 2025

View reviewed changes

wegank added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Jun 13, 2025

crowdsec-firewall-bouncer: init at 0.0.33

7895713

Co-authored-by: Summer Tea <79724236+acuteaangle@users.noreply.github.com> Co-authored-by: Felix Bargfeldt <41747605+Defelo@users.noreply.github.com> Co-authored-by: j-k <dev@j-k.io>

TornaxO7 force-pushed the crowdsec-firewall-bouncer branch from c561bfb to 7895713 Compare June 13, 2025 17:00

Defelo merged commit e2d93b5 into NixOS:master Jun 14, 2025
15 of 16 checks passed

TornaxO7 deleted the crowdsec-firewall-bouncer branch June 14, 2025 14:39

Uh oh!

Conversation

TornaxO7 commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

TornaxO7 commented Jun 1, 2025

Uh oh!

06kellyjac commented Jun 1, 2025

Uh oh!

06kellyjac commented Jun 1, 2025

Uh oh!

TornaxO7 commented Jun 1, 2025

Uh oh!

TornaxO7 commented Jun 1, 2025

Uh oh!

TornaxO7 commented Jun 1, 2025

Uh oh!

TornaxO7 commented Jun 2, 2025

Uh oh!

acuteaangle commented Jun 5, 2025

nixpkgs-review result

x86_64-linux

Uh oh!

acuteaangle commented Jun 5, 2025

Uh oh!

TornaxO7 commented Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acuteaangle commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

acuteaangle commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acuteaangle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

acuteaangle commented Jun 10, 2025

Uh oh!

acuteaangle commented Jun 10, 2025

Uh oh!

Defelo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TornaxO7 commented Jun 13, 2025

Uh oh!

06kellyjac commented Jun 13, 2025

nixpkgs-review result

x86_64-linux

Uh oh!

06kellyjac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Defelo commented Jun 13, 2025

nixpkgs-review result

x86_64-linux (sandbox = true)

aarch64-linux (sandbox = true)

x86_64-darwin (sandbox = true)

TornaxO7 commented Jun 1, 2025 •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

acuteaangle commented Jun 5, 2025 •

edited

Loading

acuteaangle commented Jun 5, 2025 •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

`nixpkgs-review` result

`x86_64-linux` (sandbox = true)

`aarch64-linux` (sandbox = true)

`x86_64-darwin` (sandbox = true)

`aarch64-darwin` (sandbox = true)