feat(crowdsec): LAPI endpoint and key for the edge firewall bouncers #751

Merged

k.renner merged 5 commits from crowdsec-edge-lapi into master 2026-09-02 11:15:57 +00:00

Conversation 8 Commits 5 Files Changed 12

+411 -3

claude-bot commented 2026-09-02 09:50:36 +00:00

Member

Copy Link

Copy Source

Enforces the decisions the in-cluster CrowdSec already detects, at the point where public traffic actually enters. Covers both halves — cluster and hosts — and is inert on merge: the bouncer ships in dry-run and creates no nftables rule until crowdsec_bouncer_mode is flipped per host.

Why the LAPI is exposed publicly rather than reached over the mesh

The proxies cannot reach the LAPI over NetBird. The netbird-proxy container runs non-root — that is exactly what the ip_unprivileged_port_start task in the role exists for — so it has no NET_ADMIN and no /dev/net/tun, and its embedded NetBird client terminates the tunnel in userspace. The tunnel belongs to that process, not to the host netns, so a second process on the same host cannot route through it. The proxies appear as mesh peers, but that peer is the container.

The alternative was a host-level NetBird client plus the LAPI on a LoadBalancer VIP: two more peers, an ACL policy in NetBird management, and a CNP rule. This reuses the path every other service already takes.

Access control: the bouncer key, and deliberately no IP allowlist

An IPAllowList was in the first revision and came out in bc896e55, because it would have rejected the bouncer.

A request originating on a proxy host does not arrive carrying that host's public address. The client connects to the local proxy on 127.0.0.1, the reverse-proxy writes what it saw into X-Forwarded-For, and Traefik resolves that into ClientHost. Measured against netbird-proxy-watchdog, which makes exactly this kind of request every round:

172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-immich-...
172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-home-...

Two probes one second apart, one per watchdog target. 172.18.0.3 is a Docker bridge address on the proxy side. Allowlisting 172.18.0.0/16 instead would not be a control: a private range on a host whose addressing we do not own, admitting anything else that originates there.

So the API key is the control, which is what it always was. The endpoint serves read-only decision streams to a valid key; the admin surface is cscli, in-cluster. Not behind the Authentik gate either — the client is a daemon, not a browser that can complete an OIDC flow.

No CNP change is needed. crowdsec-ns.yaml already admits the traefik pods with no port restriction, and ingress-default-deny carries no egress: section.

Why BOUNCER_KEY_edge is separate, and how it rotates

BOUNCER_KEY_ingress rotates quarterly and both its consumers are in-cluster, so ESO propagates within 10m. The edge hosts are Ansible-managed and read OpenBao on no schedule — a rotation would leave them 401ing until someone ran the playbook. This key is deliberately not in the rotator; it rotates by hand, OpenBao first, then plays/install-netbird-proxy.yml.

Both keys are listed in the LAPI's Reloader annotation. They are env vars from a secretKeyRef and env is fixed at pod start, so without a restart the LAPI would keep trusting the old value while the bouncer already presents the new one. (Missed in the first revision, caught by the review bot, fixed in a88bebc7.)

Verified that env-var registration survives the LAPI's wiped DB: the chart's entrypoint runs register_bouncer for every BOUNCER_KEY_* on each start and skips names that already exist. That is also why BOUNCER_KEY_ingress is still registered today despite lapi.persistentVolume.data being off.

The whitelist

capi_whitelists.yaml is the only declarative allowlist this chart offers. cscli allowlists objects live in the LAPI database, which runs without persistence here deliberately, so anything created that way is gone at the next restart. This file is a ConfigMap mount and survives by construction.

It matters because the first bouncer to arm enforces 23,893 CAPI decisions on day one, on the only external entry path.

It lists the proxy addresses and the NetBird management host, and not the home WAN address, which turns out not to be needed: opentofu/unifi_nat_hairpin.tf DNATs the public proxy addresses to the Traefik VIP on the UDM, so LAN clients reach the cluster directly with a LAN source and never traverse the proxies.

The host side

ansible/roles/crowdsec-edge-bouncer, applied by install-netbird-proxy.yml, with the trigger path added to deploy-netbird-proxy.yml.

Static release tarball, not a package. crowdsec-firewall-bouncer-nftables in AUR depends on crowdsec and would pull the whole Security Engine onto hosts that need only the bouncer; the group is mixed (proxy-1 Arch, VPS-1 Debian); and AUR would be a pattern this repo does not otherwise have. Same artefact the AUR package downloads — v0.0.36, pinned by sha256, and only the binary is unpacked, because the install/upgrade/uninstall scripts beside it manage systemd and config themselves.

retry_initial_connect: true is not cosmetic. Without it the process exits when the LAPI is unreachable at startup, and since a clean shutdown deletes the nftables tables, the systemd restart loop would leave the host with no blocklist at all until the first successful connect. Present in the code, absent from the shipped example.

nftables_hooks: [input, forward]. input is the traffic this exists for — the reverse-proxy is host-networked and owns :443. forward covers the bridge-networked http-redirect sidecar, whose packets never traverse INPUT. That is the same Docker bridge that showed up as 172.18.0.3 above.

Prometheus enabled but bound to loopback. These hosts are public and have no metrics collection path today; opening :60601 to the internet to gain a scrape target is the wrong trade.

Rollback is systemctl stop — a clean stop deletes both tables by itself. Only after a kill -9 do they need removing by hand.

Arming it

1. Merge. Nothing changes: dry-run creates no rules.
2. Watch journalctl -u crowdsec-firewall-bouncer -f on proxy-2 (dual-stack; proxy-1 is the v6-only member and the harder one) for the false-positive window.
3. Flip crowdsec_bouncer_mode: nftables for that host, re-run the play, verify with nft list table ip crowdsec.
4. Only then proxy-1.

Not included, deliberately

Monitoring — the hosts have no metric export at all, so this needs its own change — and the netbird-proxy-watchdog adjustment: it restarts the proxy container on probe failure, which does nothing against an nftables rule and would spin uselessly if the bouncer ever blocked its own probe path.

Enforces the decisions the in-cluster CrowdSec already detects, at the point where public traffic actually enters. Covers both halves — cluster and hosts — and is **inert on merge**: the bouncer ships in `dry-run` and creates no nftables rule until `crowdsec_bouncer_mode` is flipped per host. ## Why the LAPI is exposed publicly rather than reached over the mesh The proxies cannot reach the LAPI over NetBird. The `netbird-proxy` container runs **non-root** — that is exactly what the `ip_unprivileged_port_start` task in the role exists for — so it has no `NET_ADMIN` and no `/dev/net/tun`, and its embedded NetBird client terminates the tunnel in userspace. The tunnel belongs to that process, not to the host netns, so a second process on the same host cannot route through it. The proxies appear as mesh peers, but that peer is the container. The alternative was a host-level NetBird client plus the LAPI on a LoadBalancer VIP: two more peers, an ACL policy in NetBird management, and a CNP rule. This reuses the path every other service already takes. ## Access control: the bouncer key, and deliberately no IP allowlist An IPAllowList was in the first revision and came out in `bc896e55`, because it would have rejected the bouncer. A request originating on a proxy host does not arrive carrying that host's public address. The client connects to the local proxy on `127.0.0.1`, the reverse-proxy writes what it saw into `X-Forwarded-For`, and Traefik resolves that into `ClientHost`. Measured against `netbird-proxy-watchdog`, which makes exactly this kind of request every round: 172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-immich-... 172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-home-... Two probes one second apart, one per watchdog target. `172.18.0.3` is a Docker bridge address on the proxy side. Allowlisting `172.18.0.0/16` instead would not be a control: a private range on a host whose addressing we do not own, admitting anything else that originates there. So the API key is the control, which is what it always was. The endpoint serves read-only decision streams to a valid key; the admin surface is `cscli`, in-cluster. Not behind the Authentik gate either — the client is a daemon, not a browser that can complete an OIDC flow. **No CNP change is needed.** `crowdsec-ns.yaml` already admits the traefik pods with no port restriction, and `ingress-default-deny` carries no `egress:` section. ## Why `BOUNCER_KEY_edge` is separate, and how it rotates `BOUNCER_KEY_ingress` rotates quarterly and both its consumers are in-cluster, so ESO propagates within 10m. The edge hosts are Ansible-managed and read OpenBao on no schedule — a rotation would leave them 401ing until someone ran the playbook. This key is deliberately **not** in the rotator; it rotates by hand, OpenBao first, then `plays/install-netbird-proxy.yml`. Both keys are listed in the LAPI's Reloader annotation. They are env vars from a `secretKeyRef` and env is fixed at pod start, so without a restart the LAPI would keep trusting the old value while the bouncer already presents the new one. (Missed in the first revision, caught by the review bot, fixed in `a88bebc7`.) Verified that env-var registration survives the LAPI's wiped DB: the chart's entrypoint runs `register_bouncer` for every `BOUNCER_KEY_*` on each start and skips names that already exist. That is also why `BOUNCER_KEY_ingress` is still registered today despite `lapi.persistentVolume.data` being off. ## The whitelist `capi_whitelists.yaml` is the **only** declarative allowlist this chart offers. `cscli allowlists` objects live in the LAPI database, which runs without persistence here deliberately, so anything created that way is gone at the next restart. This file is a ConfigMap mount and survives by construction. It matters because the first bouncer to arm enforces **23,893 CAPI decisions** on day one, on the only external entry path. It lists the proxy addresses and the NetBird management host, and **not** the home WAN address, which turns out not to be needed: `opentofu/unifi_nat_hairpin.tf` DNATs the public proxy addresses to the Traefik VIP on the UDM, so LAN clients reach the cluster directly with a LAN source and never traverse the proxies. ## The host side `ansible/roles/crowdsec-edge-bouncer`, applied by `install-netbird-proxy.yml`, with the trigger path added to `deploy-netbird-proxy.yml`. **Static release tarball, not a package.** `crowdsec-firewall-bouncer-nftables` in AUR depends on `crowdsec` and would pull the whole Security Engine onto hosts that need only the bouncer; the group is mixed (proxy-1 Arch, VPS-1 Debian); and AUR would be a pattern this repo does not otherwise have. Same artefact the AUR package downloads — v0.0.36, pinned by sha256, and only the binary is unpacked, because the install/upgrade/uninstall scripts beside it manage systemd and config themselves. **`retry_initial_connect: true`** is not cosmetic. Without it the process exits when the LAPI is unreachable at startup, and since a clean shutdown deletes the nftables tables, the systemd restart loop would leave the host with no blocklist at all until the first successful connect. Present in the code, absent from the shipped example. **`nftables_hooks: [input, forward]`.** `input` is the traffic this exists for — the reverse-proxy is host-networked and owns :443. `forward` covers the bridge-networked `http-redirect` sidecar, whose packets never traverse INPUT. That is the same Docker bridge that showed up as `172.18.0.3` above. **Prometheus enabled but bound to loopback.** These hosts are public and have no metrics collection path today; opening :60601 to the internet to gain a scrape target is the wrong trade. **Rollback** is `systemctl stop` — a clean stop deletes both tables by itself. Only after a `kill -9` do they need removing by hand. ## Arming it 1. Merge. Nothing changes: `dry-run` creates no rules. 2. Watch `journalctl -u crowdsec-firewall-bouncer -f` on proxy-2 (dual-stack; proxy-1 is the v6-only member and the harder one) for the false-positive window. 3. Flip `crowdsec_bouncer_mode: nftables` for that host, re-run the play, verify with `nft list table ip crowdsec`. 4. Only then proxy-1. ## Not included, deliberately Monitoring — the hosts have no metric export at all, so this needs its own change — and the `netbird-proxy-watchdog` adjustment: it restarts the proxy container on probe failure, which does nothing against an nftables rule and would spin uselessly if the bouncer ever blocked its own probe path.

claude-bot added 1 commit 2026-09-02 09:50:37 +00:00

feat(crowdsec): LAPI endpoint and key for the edge firewall bouncers ... ea6e8e5083

Groundwork for enforcing CrowdSec decisions on proxy-1/proxy-2. Inert on its
own: an Ingress nobody calls yet, a bouncer key nobody uses yet, and a
whitelist that only ever narrows what may be banned.

The proxies cannot reach the LAPI over the mesh. The netbird-proxy container
runs non-root — that is what the ip_unprivileged_port_start task in the role
is for — so it has no NET_ADMIN and no /dev/net/tun, and its embedded NetBird
client terminates the tunnel in userspace. The tunnel belongs to that process,
not to the host netns, so a second process on the same host cannot route
through it. The alternative was a host-level NetBird client plus the LAPI on a
LoadBalancer VIP: two more peers, an ACL policy and a CNP rule. This reuses
the path every other service already takes.

Exposure is narrowed twice. The IPAllowList carries the three proxy addresses,
resolving them past the netbird-ingress pod IP through X-Forwarded-For the
same way the rate-limit middleware does — without that it would compare
against the shared forwarder IP, which is the gap behind the 2026-06-27 mass
ban. Behind it, the bouncer key. The LAPI serves read-only decision streams to
a valid key; the admin surface is cscli, in-cluster.

No CNP change is needed: crowdsec-ns.yaml already admits the traefik pods, and
ingress-default-deny carries no egress section.

BOUNCER_KEY_edge is separate from BOUNCER_KEY_ingress on purpose. The latter
rotates quarterly and both its consumers are in-cluster, so ESO propagates
within 10m. The edge hosts are Ansible-managed and read OpenBao on no
schedule, so a rotation would leave them 401ing until someone runs the
playbook. This key rotates with the playbook that carries it.

capi_whitelists.yaml is the only declarative allowlist the chart offers.
`cscli allowlists` objects live in the LAPI database, which runs without
persistence here deliberately, so anything created that way is gone at the
next restart. This file is a ConfigMap mount and survives by construction.
Upstream deprecates it; it is still the only thing that works against a LAPI
with no PVC.

It lists the proxy addresses and not the home WAN address, which is not
needed: unifi_nat_hairpin.tf DNATs the public proxy addresses to the Traefik
VIP on the UDM, so LAN clients reach the cluster directly with a LAN source
and never traverse the proxies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

review-bot requested changes 2026-09-02 09:57:16 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit ea6e8e5083a82e4e234ad7274c873e8adfb0609a.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

---

Summary

This PR adds a public Ingress + Middleware + ServersTransport for the CrowdSec LAPI, a dedicated BOUNCER_KEY_edge bouncer key (via a new ExternalSecret), and a capi_whitelists.yaml allowlist for the proxy hosts, laying groundwork for firewall bouncers on proxy-1/proxy-2. The PR description is unusually thorough and every claim I could check against the checkout (CNP rules in crowdsec-ns.yaml/ingress-isolation.yaml, the pod-CIDR excludedIPs used elsewhere, the dns.renner.uno/public labeling convention, the LAPI's HTTPS-only/self-signed-CA setup) checked out. One concrete gap and one high-risk-but-unconfirmed mechanism are worth addressing before merge.

🔴 Blocking

• kubernetes/infrastructure/crowdsec/helmrelease.yaml:118-122 — Reloader annotation not updated for the new secret. The existing podAnnotations block only watches crowdsec-bouncer-key:

  secret.reloader.stakater.com/reload: crowdsec-bouncer-key
  

  but the new BOUNCER_KEY_edge env var (helmrelease.yaml:157-161) sources from crowdsec-edge-bouncer-key, a different Secret. BOUNCER_KEY_edge is injected as a plain env var, so — exactly as the comment on this same annotation explains for the sibling key — a value change in the Secret does not propagate into the running container without a pod restart. Since this key is explicitly designed to be rotated manually (Bao kv put + Ansible playbook, per the PR body), the rotation flow as built will update the edge hosts' key but leave the in-cluster LAPI still validating the old value until someone happens to bounce the LAPI pod separately — the edge bouncers get 401s against the LAPI right after a rotation that was supposed to fix stale credentials. Fix: add the new secret name to the annotation (Reloader accepts a comma-separated list), e.g. crowdsec-bouncer-key,crowdsec-edge-bouncer-key.

🟡 Suggestions

• kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:75 uses traefik.ingress.kubernetes.io/service.serverstransport on a plain networking.k8s.io/v1 Ingress to point the backend connection at the custom ServersTransport (needed because the LAPI is HTTPS-only with a self-signed CA). I could not verify this live, but this repo has first-party evidence that this exact mechanism has failed before: kubernetes/infrastructure/external-dns-unifi/helmrelease.yaml:107-109 explains that the traefik-proxy external-dns source exists specifically for "HTTPS-backend apps whose ServersTransport reference doesn't resolve through the kubernetesIngress provider's annotation path (e.g. apps/pve)" — i.e., a prior HTTPS-backend app had to move to a native IngressRoute CRD because the annotation-based ServersTransport reference didn't take effect. The app referenced (apps/pve) is no longer present in this checkout, so I can't confirm the details of that failure, but it's the same annotation, the same kubernetesIngress provider, and the same "HTTPS backend needs a custom ServersTransport" situation as this PR. If it recurs here, every request to crowdsec.renner.uno would hit the LAPI over what Traefik treats as an untrusted/default TLS config and likely fail (502/525-class error), silently defeating the whole feature despite kubectl kustomize building cleanly. Worth a live smoke test (curl from an allowlisted source, or checking the router's effective config via the Traefik API/dashboard) before relying on this, or switching to an IngressRoute CRD to match the pattern that's already proven to work in this repo.
• No rate-limiting middleware (rate-limit-middleware.yaml, used by other public ingresses) is attached to the new LAPI Ingress. The ipAllowList already restricts sources to three known addresses so the exposure is narrow, but a compromised/misbehaving one of those three (or the shared proxy path) can still hit the LAPI unthrottled.
• The "Before merging" and "Also required, out of band" sections of the PR body both note manual prerequisites (confirming egress-vs-inbound address match; bao kv put ... crowdsec-edge-bouncer-key) that must land before/around merge. Since BOUNCER_KEY_edge is a hard secretKeyRef in the HelmRelease, merging before the Bao value exists will put the LAPI Deployment into CreateContainerConfigError — the exact "wedges the whole infrastructure Kustomization" failure mode this file's own #340 comment (helmrelease.yaml:72-74) warns about for a different reason. Worth sequencing the Bao write strictly before this merges/reconciles, not just "also required."

🟢 Nits

None beyond the items above.

Tests

This is GitOps config (Kustomize/Flux/Helm values); there's no unit-test convention in this repo for it. The PR body states kubectl kustomize was run against kubernetes/infrastructure and .../crowdsec and that the ipAllowList/capi_whitelists.yaml schemas were checked against the live CRD and the chart's values.yaml — I didn't have a working kubectl/kustomize in this review environment to independently re-run that, but manual review of the YAML structure and cross-references (kustomization list, Secret/env names, CNP rules, existing annotation conventions) found no structural errors. The one thing I'd want tested before trusting this in production is the live TLS path to the LAPI (see the ServersTransport suggestion above) — that's not something a kustomize build can catch.

Recommendation

Request changes — 1 blocking issue (the missing Reloader annotation entry for crowdsec-edge-bouncer-key).

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `ea6e8e5083a82e4e234ad7274c873e8adfb0609a`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR adds a public Ingress + Middleware + ServersTransport for the CrowdSec LAPI, a dedicated `BOUNCER_KEY_edge` bouncer key (via a new ExternalSecret), and a `capi_whitelists.yaml` allowlist for the proxy hosts, laying groundwork for firewall bouncers on proxy-1/proxy-2. The PR description is unusually thorough and every claim I could check against the checkout (CNP rules in `crowdsec-ns.yaml`/`ingress-isolation.yaml`, the pod-CIDR `excludedIPs` used elsewhere, the `dns.renner.uno/public` labeling convention, the LAPI's HTTPS-only/self-signed-CA setup) checked out. One concrete gap and one high-risk-but-unconfirmed mechanism are worth addressing before merge. ## 🔴 Blocking - **`kubernetes/infrastructure/crowdsec/helmrelease.yaml:118-122` — Reloader annotation not updated for the new secret.** The existing `podAnnotations` block only watches `crowdsec-bouncer-key`: ``` secret.reloader.stakater.com/reload: crowdsec-bouncer-key ``` but the new `BOUNCER_KEY_edge` env var (helmrelease.yaml:157-161) sources from `crowdsec-edge-bouncer-key`, a *different* Secret. `BOUNCER_KEY_edge` is injected as a plain env var, so — exactly as the comment on this same annotation explains for the sibling key — a value change in the Secret does **not** propagate into the running container without a pod restart. Since this key is explicitly designed to be rotated manually (Bao `kv put` + Ansible playbook, per the PR body), the rotation flow as built will update the edge hosts' key but leave the in-cluster LAPI still validating the *old* value until someone happens to bounce the LAPI pod separately — the edge bouncers get 401s against the LAPI right after a rotation that was supposed to fix stale credentials. Fix: add the new secret name to the annotation (Reloader accepts a comma-separated list), e.g. `crowdsec-bouncer-key,crowdsec-edge-bouncer-key`. ## 🟡 Suggestions - **`kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:75`** uses `traefik.ingress.kubernetes.io/service.serverstransport` on a plain `networking.k8s.io/v1` Ingress to point the backend connection at the custom `ServersTransport` (needed because the LAPI is HTTPS-only with a self-signed CA). I could not verify this live, but this repo has first-party evidence that this exact mechanism has failed before: `kubernetes/infrastructure/external-dns-unifi/helmrelease.yaml:107-109` explains that the `traefik-proxy` external-dns source exists specifically for "HTTPS-backend apps whose ServersTransport reference doesn't resolve through the kubernetesIngress provider's annotation path (e.g. apps/pve)" — i.e., a prior HTTPS-backend app had to move to a native `IngressRoute` CRD because the annotation-based `ServersTransport` reference didn't take effect. The app referenced (`apps/pve`) is no longer present in this checkout, so I can't confirm the details of that failure, but it's the same annotation, the same `kubernetesIngress` provider, and the same "HTTPS backend needs a custom `ServersTransport`" situation as this PR. If it recurs here, every request to `crowdsec.renner.uno` would hit the LAPI over what Traefik treats as an untrusted/default TLS config and likely fail (502/525-class error), silently defeating the whole feature despite `kubectl kustomize` building cleanly. Worth a live smoke test (curl from an allowlisted source, or checking the router's effective config via the Traefik API/dashboard) before relying on this, or switching to an `IngressRoute` CRD to match the pattern that's already proven to work in this repo. - No rate-limiting middleware (`rate-limit-middleware.yaml`, used by other public ingresses) is attached to the new LAPI Ingress. The `ipAllowList` already restricts sources to three known addresses so the exposure is narrow, but a compromised/misbehaving one of those three (or the shared proxy path) can still hit the LAPI unthrottled. - The "Before merging" and "Also required, out of band" sections of the PR body both note manual prerequisites (confirming egress-vs-inbound address match; `bao kv put ... crowdsec-edge-bouncer-key`) that must land before/around merge. Since `BOUNCER_KEY_edge` is a hard `secretKeyRef` in the HelmRelease, merging before the Bao value exists will put the LAPI Deployment into `CreateContainerConfigError` — the exact "wedges the whole `infrastructure` Kustomization" failure mode this file's own `#340` comment (helmrelease.yaml:72-74) warns about for a different reason. Worth sequencing the Bao write strictly before this merges/reconciles, not just "also required." ## 🟢 Nits None beyond the items above. ## Tests This is GitOps config (Kustomize/Flux/Helm values); there's no unit-test convention in this repo for it. The PR body states `kubectl kustomize` was run against `kubernetes/infrastructure` and `.../crowdsec` and that the `ipAllowList`/`capi_whitelists.yaml` schemas were checked against the live CRD and the chart's `values.yaml` — I didn't have a working `kubectl`/`kustomize` in this review environment to independently re-run that, but manual review of the YAML structure and cross-references (kustomization list, Secret/env names, CNP rules, existing annotation conventions) found no structural errors. The one thing I'd want tested before trusting this in production is the live TLS path to the LAPI (see the ServersTransport suggestion above) — that's not something a kustomize build can catch. ## Recommendation Request changes — 1 blocking issue (the missing Reloader annotation entry for `crowdsec-edge-bouncer-key`). <!-- pr-review:sha=ea6e8e5083a82e4e234ad7274c873e8adfb0609a -->

claude-bot added 1 commit 2026-09-02 10:13:34 +00:00

fix(crowdsec): drop the LAPI IPAllowList, it would have rejected the bouncer ... bc896e550c

The allowlist carried the published proxy addresses on the assumption that a
request from a proxy host arrives carrying that host's public address. It does
not.

The client connects to the local proxy on 127.0.0.1, the reverse-proxy writes
what it saw into X-Forwarded-For, and Traefik resolves that into ClientHost.
Measured against netbird-proxy-watchdog, which makes exactly this kind of
request every round:

  172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-immich-...
  172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-home-...

Two probes one second apart, one per watchdog target. 172.18.0.3 is a Docker
bridge address on the proxy side.

So the allowlist would have 403ed the bouncer. Allowlisting 172.18.0.0/16
instead is not a control: it is a private range on a host whose addressing we
do not own, and it would admit anything else that happens to originate there.

The bouncer key stays as the access control, which is what it always was. The
endpoint serves read-only decision streams to a valid key; the admin surface
is cscli, in-cluster. If the source address ever becomes deterministic, an
ipAllowList can go back on top.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot commented 2026-09-02 10:13:46 +00:00

Author

Member

Copy Link

Copy Source

Korrektur, gemessen statt geraten: die IPAllowList ist raus (bc896e55).

Die Annahme war, dass ein Request vom Proxy-Host mit dessen öffentlicher Adresse ankommt. Falsch. Der Client verbindet sich auf 127.0.0.1 zum lokalen Proxy, der Reverse-Proxy schreibt in X-Forwarded-For was er gesehen hat, und Traefik löst das zu ClientHost auf. Gemessen am netbird-proxy-watchdog, der genau diese Art Request pro Runde macht:

172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-immich-...
172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-home-...

Zwei Probes in derselben Sekunde, einer je Watchdog-Ziel. 172.18.0.3 ist eine Docker-Bridge-Adresse auf Proxy-Seite.

Die Allowlist hätte den Bouncer also mit 403 abgewiesen. Stattdessen 172.18.0.0/16 einzutragen wäre keine Zugriffskontrolle, sondern ein privater Bereich auf einem Host, dessen Adressvergabe uns nicht gehört — und würde alles zulassen, was dort sonst noch entsteht.

Übrig bleibt der Bouncer-Key, der die eigentliche Kontrolle immer schon war: der Endpunkt liefert read-only Decision-Streams an einen gültigen Key, die Admin-Fläche ist cscli im Cluster.

Der Punkt „Before merging — one input needed" in der Beschreibung ist damit erledigt: es gibt keine Adresse mehr zu bestätigen.

Der OpenBao-Key ist gesetzt (homelab/infrastructure/crowdsec-edge-bouncer-key, Feld key, Version 1). Offen bleibt nur vault_crowdsec_edge_bouncer_key für Ansible — derselbe Wert, wenn die Rolle kommt.

Korrektur, gemessen statt geraten: die IPAllowList ist raus (bc896e55). Die Annahme war, dass ein Request vom Proxy-Host mit dessen öffentlicher Adresse ankommt. Falsch. Der Client verbindet sich auf `127.0.0.1` zum lokalen Proxy, der Reverse-Proxy schreibt in X-Forwarded-For was er gesehen hat, und Traefik löst das zu ClientHost auf. Gemessen am `netbird-proxy-watchdog`, der genau diese Art Request pro Runde macht: 172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-immich-... 172.18.0.3 - - [02/Sep/2026:10:12:01] "GET /" 200 websecure-home-... Zwei Probes in derselben Sekunde, einer je Watchdog-Ziel. `172.18.0.3` ist eine Docker-Bridge-Adresse auf Proxy-Seite. Die Allowlist hätte den Bouncer also mit 403 abgewiesen. Stattdessen `172.18.0.0/16` einzutragen wäre keine Zugriffskontrolle, sondern ein privater Bereich auf einem Host, dessen Adressvergabe uns nicht gehört — und würde alles zulassen, was dort sonst noch entsteht. Übrig bleibt der Bouncer-Key, der die eigentliche Kontrolle immer schon war: der Endpunkt liefert read-only Decision-Streams an einen gültigen Key, die Admin-Fläche ist `cscli` im Cluster. Der Punkt „Before merging — one input needed" in der Beschreibung ist damit erledigt: es gibt keine Adresse mehr zu bestätigen. Der OpenBao-Key ist gesetzt (`homelab/infrastructure/crowdsec-edge-bouncer-key`, Feld `key`, Version 1). Offen bleibt nur `vault_crowdsec_edge_bouncer_key` für Ansible — derselbe Wert, wenn die Rolle kommt.

review-bot reviewed 2026-09-02 10:17:43 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit bc896e550c548a8b16e530e7f44b8fa0b7a10ad9.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

---

Summary

This PR adds the plumbing for external CrowdSec bouncers on proxy-1/proxy-2: a new public crowdsec-lapi Ingress + ServersTransport for the LAPI, a dedicated BOUNCER_KEY_edge ExternalSecret kept deliberately outside the automated rotator, and a capi_whitelists.yaml allowlist for the proxy addresses. The PR body describes it as inert on its own (no consumer yet, no CNP change), and that checks out against the code — the surrounding NetworkPolicies (ingress-default-deny, crowdsec-ns.yaml) are unchanged and already permissive enough for this to work without edits. The addresses in the new allowlist match unifi_nat_hairpin.tf and ingress-isolation.yaml's toCIDRSet, as claimed. No blocking issues found; one operational gap worth fixing before the key ever needs to rotate.

🟡 Suggestions

• kubernetes/infrastructure/crowdsec/helmrelease.yaml:118-122 — podAnnotations only watches crowdsec-bouncer-key via secret.reloader.stakater.com/reload. The new crowdsec-edge-bouncer-key Secret (consumed as BOUNCER_KEY_edge, an env var via secretKeyRef) isn't listed. Env vars from secretKeyRef don't update in a running container when the backing Secret changes, so once the Ansible playbook rotates the edge key and OpenBao/ESO sync a new value into this Secret, the LAPI pod will keep running with the stale key until something else happens to restart it — the bouncers would start getting 401s with no automatic recovery. This is exactly the class of restart-wiring gap the file's own comments are careful about elsewhere (see the crowdsec-bouncer-key Reloader comment right above, and the #340 history in this same file). Reloader's annotation accepts a comma-separated list, so secret.reloader.stakater.com/reload: crowdsec-bouncer-key,crowdsec-edge-bouncer-key would close the gap. Not urgent for this PR (nothing consumes the edge key yet), but worth doing before the key is ever rotated in production.

• PR description vs. code: the PR body's "How the exposure is narrowed" section still describes an IPAllowList as the second factor in front of the LAPI. The actual diff (and lapi-ingress.yaml's own header comment, "NO IP ALLOWLIST, deliberately") no longer has one — presumably from the follow-up commit that is now HEAD (bc896e5 fix(crowdsec): drop the LAPI IPAllowList, it would have rejected the bouncer). The code is internally consistent and the in-file rationale (proxy-side requests arrive from a Docker bridge address via X-Forwarded-For, not the published proxy IP) is sound and matches the pattern used by rate-limit-middleware.yaml. Worth updating the PR description before merge so it doesn't mislead future readers about what's actually deployed.

🟢 Nits

• kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:46-48,75 — the crowdsec-service.crowdsec.svc.cluster.local server name and crowdsec-root-secret CA secret name are assumptions about the upstream crowdsecurity/crowdsec chart's internal naming (not vendored in this repo, so not independently verifiable from the checkout). The PR body says these were checked against the live cluster and chart values.yaml at tag crowdsec-0.24.0; I have no way to re-verify that here, just flagging the dependency.

Tests

No automated tests exist for this kind of change in the repo (it's Kustomize/HelmRelease YAML), consistent with how the rest of kubernetes/infrastructure is reviewed — verification is via kubectl kustomize build + live cluster checks, both of which the PR body reports doing. That's proportionate for this change; nothing here needs unit-style coverage.

Recommendation

Looks reasonable — no blocking issues; comments above are optional (the Reloader annotation gap is worth picking up before the edge key's first rotation, but doesn't affect this PR's stated inert, no-consumer-yet state).

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `bc896e550c548a8b16e530e7f44b8fa0b7a10ad9`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR adds the plumbing for external CrowdSec bouncers on proxy-1/proxy-2: a new public `crowdsec-lapi` Ingress + `ServersTransport` for the LAPI, a dedicated `BOUNCER_KEY_edge` ExternalSecret kept deliberately outside the automated rotator, and a `capi_whitelists.yaml` allowlist for the proxy addresses. The PR body describes it as inert on its own (no consumer yet, no CNP change), and that checks out against the code — the surrounding NetworkPolicies (`ingress-default-deny`, `crowdsec-ns.yaml`) are unchanged and already permissive enough for this to work without edits. The addresses in the new allowlist match `unifi_nat_hairpin.tf` and `ingress-isolation.yaml`'s `toCIDRSet`, as claimed. No blocking issues found; one operational gap worth fixing before the key ever needs to rotate. ## 🟡 Suggestions - `kubernetes/infrastructure/crowdsec/helmrelease.yaml:118-122` — `podAnnotations` only watches `crowdsec-bouncer-key` via `secret.reloader.stakater.com/reload`. The new `crowdsec-edge-bouncer-key` Secret (consumed as `BOUNCER_KEY_edge`, an env var via `secretKeyRef`) isn't listed. Env vars from `secretKeyRef` don't update in a running container when the backing Secret changes, so once the Ansible playbook rotates the edge key and OpenBao/ESO sync a new value into this Secret, the LAPI pod will keep running with the stale key until something else happens to restart it — the bouncers would start getting 401s with no automatic recovery. This is exactly the class of restart-wiring gap the file's own comments are careful about elsewhere (see the `crowdsec-bouncer-key` Reloader comment right above, and the `#340` history in this same file). Reloader's annotation accepts a comma-separated list, so `secret.reloader.stakater.com/reload: crowdsec-bouncer-key,crowdsec-edge-bouncer-key` would close the gap. Not urgent for this PR (nothing consumes the edge key yet), but worth doing before the key is ever rotated in production. - PR description vs. code: the PR body's "How the exposure is narrowed" section still describes an IPAllowList as the second factor in front of the LAPI. The actual diff (and `lapi-ingress.yaml`'s own header comment, "NO IP ALLOWLIST, deliberately") no longer has one — presumably from the follow-up commit that is now HEAD (`bc896e5 fix(crowdsec): drop the LAPI IPAllowList, it would have rejected the bouncer`). The code is internally consistent and the in-file rationale (proxy-side requests arrive from a Docker bridge address via X-Forwarded-For, not the published proxy IP) is sound and matches the pattern used by `rate-limit-middleware.yaml`. Worth updating the PR description before merge so it doesn't mislead future readers about what's actually deployed. ## 🟢 Nits - `kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:46-48,75` — the `crowdsec-service.crowdsec.svc.cluster.local` server name and `crowdsec-root-secret` CA secret name are assumptions about the upstream `crowdsecurity/crowdsec` chart's internal naming (not vendored in this repo, so not independently verifiable from the checkout). The PR body says these were checked against the live cluster and chart `values.yaml` at tag `crowdsec-0.24.0`; I have no way to re-verify that here, just flagging the dependency. ## Tests No automated tests exist for this kind of change in the repo (it's Kustomize/HelmRelease YAML), consistent with how the rest of `kubernetes/infrastructure` is reviewed — verification is via `kubectl kustomize` build + live cluster checks, both of which the PR body reports doing. That's proportionate for this change; nothing here needs unit-style coverage. ## Recommendation Looks reasonable — no blocking issues; comments above are optional (the Reloader annotation gap is worth picking up before the edge key's first rotation, but doesn't affect this PR's stated inert, no-consumer-yet state). <!-- pr-review:sha=bc896e550c548a8b16e530e7f44b8fa0b7a10ad9 -->

claude-bot added 1 commit 2026-09-02 10:19:15 +00:00

feat(crowdsec): firewall bouncer on the NetBird proxy hosts ... 22143c6a7c

Enforces the decisions the in-cluster CrowdSec already detects, at the point
where public traffic actually enters. These hosts terminate TLS for every
*.renner.uno name, so a drop here never reaches the mesh, the relay or the
cluster. Detection stays where it is: the reverse-proxy produces no parseable
access log and has no collection, while Traefik has a working one, and every
scenario in use needs HTTP detail that does not exist at this layer.

Deployed in dry-run. It pulls decisions and logs what it would block without
creating a single nftables rule. The first bouncer to arm anywhere enforces
~24k CAPI decisions at once on the only external entry path, so the
false-positive window has to come before the enforcement rather than after it.
Arming is flipping crowdsec_bouncer_mode per host.

The static release tarball, not a package. The AUR package
crowdsec-firewall-bouncer-nftables depends on `crowdsec` and would pull the
whole Security Engine onto hosts that need only the bouncer; the group is
mixed (proxy-1 Arch, VPS-1 Debian); and the AUR would be a pattern this repo
does not otherwise have. The tarball is the same artefact the AUR package
downloads, pinned by version and sha256, and only the binary is unpacked —
the install/upgrade/uninstall scripts beside it manage systemd and config
themselves, which would leave two owners for the same files.

retry_initial_connect is set, and it is not cosmetic. Without it the process
exits when the LAPI is unreachable at startup, and since a clean shutdown
deletes the nftables tables, the systemd restart loop would leave the host
with no blocklist at all until the first successful connect. The flag exists
in the code but not in the shipped example config.

nftables_hooks covers input and forward. input is the traffic this exists for,
because the reverse-proxy is host-networked and owns :443. forward covers the
bridge-networked http-redirect sidecar, whose packets never traverse INPUT —
the same Docker bridge that showed up as 172.18.0.3 in the Traefik access log.

Prometheus is enabled but bound to loopback. These hosts are public and have
no metrics collection path today; opening :60601 to the internet to gain a
scrape target is the wrong trade.

No CA file: Traefik serves Let's Encrypt for crowdsec.renner.uno, and
ca_cert_path is added to the system pool rather than replacing it.

Rollback is `systemctl stop` — a clean stop deletes both tables by itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot commented 2026-09-02 10:19:34 +00:00

Author

Member

Copy Link

Copy Source

Die Ansible-Rolle ist drin (22143c6a) — ansible/roles/crowdsec-edge-bouncer, eingehängt in install-netbird-proxy.yml, plus der Trigger-Pfad in deploy-netbird-proxy.yml. Der PR deckt damit beide Hälften ab und bleibt trotzdem inert.

Warum inert: Die Rolle deployt in mode: dry-run. Der Bouncer holt Decisions und loggt, was er blocken würde, legt aber keine einzige nftables-Regel an. Der erste Bouncer, der irgendwo scharf geht, setzt ~24k CAPI-Decisions auf einen Schlag durch — auf dem einzigen externen Eintrittspfad. Das FP-Fenster muss also vor die Durchsetzung, nicht danach. Scharfschalten heißt crowdsec_bouncer_mode: nftables, pro Host.

Statisches Release-Binary statt Paket. crowdsec-firewall-bouncer-nftables aus AUR hängt von crowdsec ab und würde die komplette Security Engine auf Hosts ziehen, die nur den Bouncer brauchen. Die Gruppe ist gemischt (proxy-1 Arch, VPS-1 Debian), und AUR wäre ein Muster, das es hier sonst nicht gibt. Das Tarball ist dasselbe Artefakt, das auch das AUR-Paket zieht — v0.0.36, per sha256 gepinnt, und nur das Binary wird ausgepackt: die install/upgrade/uninstall-Skripte daneben verwalten systemd und Config selbst, was zwei Besitzer für dieselben Dateien ergäbe.

retry_initial_connect: true ist kein Detail. Ohne das beendet sich der Prozess, wenn die LAPI beim Start nicht erreichbar ist, und weil ein sauberer Shutdown die nftables-Tabellen löscht, stünde der Host im systemd-Restart-Loop komplett ohne Blockliste da. Das Flag existiert im Code, aber nicht in der mitgelieferten Beispielkonfiguration.

nftables_hooks: [input, forward]. input ist der eigentliche Zweck — der Reverse-Proxy ist host-networked und besitzt :443. forward deckt den bridge-networked http-redirect-Sidecar ab, dessen Pakete INPUT nie durchlaufen. Das ist dieselbe Docker-Bridge, die vorhin als 172.18.0.3 im Traefik-Log auftauchte.

Prometheus an, aber auf Loopback. Die Hosts sind öffentlich und haben heute überhaupt keinen Metrik-Pfad; :60601 ins Internet zu öffnen, nur um ein Scrape-Ziel zu haben, ist der falsche Tausch. Gehört zusammen mit Host-Metriken angegangen.

Rollback ist systemctl stop — ein sauberer Stop löscht beide Tabellen selbst. Nur nach einem kill -9 müssen sie von Hand weg.

Offen vor dem Merge: vault_crowdsec_edge_bouncer_key im Ansible-Vault, mit demselben Wert wie homelab/infrastructure/crowdsec-edge-bouncer-key in OpenBao. Den kann ich nicht setzen — das Vault-Passwort ist ein CI-Secret.

Die Ansible-Rolle ist drin (22143c6a) — `ansible/roles/crowdsec-edge-bouncer`, eingehängt in `install-netbird-proxy.yml`, plus der Trigger-Pfad in `deploy-netbird-proxy.yml`. Der PR deckt damit beide Hälften ab und bleibt trotzdem inert. **Warum inert:** Die Rolle deployt in `mode: dry-run`. Der Bouncer holt Decisions und loggt, was er blocken würde, legt aber keine einzige nftables-Regel an. Der erste Bouncer, der irgendwo scharf geht, setzt ~24k CAPI-Decisions auf einen Schlag durch — auf dem einzigen externen Eintrittspfad. Das FP-Fenster muss also vor die Durchsetzung, nicht danach. Scharfschalten heißt `crowdsec_bouncer_mode: nftables`, pro Host. **Statisches Release-Binary statt Paket.** `crowdsec-firewall-bouncer-nftables` aus AUR hängt von `crowdsec` ab und würde die komplette Security Engine auf Hosts ziehen, die nur den Bouncer brauchen. Die Gruppe ist gemischt (proxy-1 Arch, VPS-1 Debian), und AUR wäre ein Muster, das es hier sonst nicht gibt. Das Tarball ist dasselbe Artefakt, das auch das AUR-Paket zieht — v0.0.36, per sha256 gepinnt, und nur das Binary wird ausgepackt: die install/upgrade/uninstall-Skripte daneben verwalten systemd und Config selbst, was zwei Besitzer für dieselben Dateien ergäbe. **`retry_initial_connect: true`** ist kein Detail. Ohne das beendet sich der Prozess, wenn die LAPI beim Start nicht erreichbar ist, und weil ein sauberer Shutdown die nftables-Tabellen löscht, stünde der Host im systemd-Restart-Loop komplett ohne Blockliste da. Das Flag existiert im Code, aber nicht in der mitgelieferten Beispielkonfiguration. **`nftables_hooks: [input, forward]`.** input ist der eigentliche Zweck — der Reverse-Proxy ist host-networked und besitzt :443. forward deckt den bridge-networked `http-redirect`-Sidecar ab, dessen Pakete INPUT nie durchlaufen. Das ist dieselbe Docker-Bridge, die vorhin als `172.18.0.3` im Traefik-Log auftauchte. **Prometheus an, aber auf Loopback.** Die Hosts sind öffentlich und haben heute überhaupt keinen Metrik-Pfad; :60601 ins Internet zu öffnen, nur um ein Scrape-Ziel zu haben, ist der falsche Tausch. Gehört zusammen mit Host-Metriken angegangen. **Rollback** ist `systemctl stop` — ein sauberer Stop löscht beide Tabellen selbst. Nur nach einem `kill -9` müssen sie von Hand weg. Offen vor dem Merge: `vault_crowdsec_edge_bouncer_key` im Ansible-Vault, mit demselben Wert wie `homelab/infrastructure/crowdsec-edge-bouncer-key` in OpenBao. Den kann ich nicht setzen — das Vault-Passwort ist ein CI-Secret.

review-bot reviewed 2026-09-02 10:25:28 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit 22143c6a7c7ae1192ed788b40ad661405157bf8f.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

---

Summary

Groundwork PR: exposes the CrowdSec LAPI publicly via a new Traefik Ingress/ServersTransport (mTLS to the backend), adds a BOUNCER_KEY_edge secret/ExternalSecret, a CAPI whitelist for the proxy addresses, and a new crowdsec-edge-bouncer Ansible role deployed in dry-run mode. The design reasoning in the PR body is unusually thorough and I could verify most of its factual claims against the checkout (non-root NetBird container, BOUNCER_KEY_ingress survives LAPI restarts, crowdsec-bouncer-key rotator excludes the new key, the three proxy IPs match unifi_nat_hairpin.tf/ingress-isolation.yaml). No blocking defects found, but there's a notable mismatch between what the PR description says protects the LAPI and what's actually in the diff, plus one unverified-in-this-environment external dependency and a pattern this repo has hit trouble with before.

🟡 Suggestions

• PR description vs. code disagree on the LAPI's protection model. The PR body's "How the exposure is narrowed" section says exposure is narrowed by an IPAllowList with the three proxy addresses via ipStrategy.excludedIPs. The actual kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:29-33 explicitly does the opposite: "NO IP ALLOWLIST, deliberately", because bouncer requests arrive from a Docker bridge address, not a stable client IP, so an allowlist would either 403 the bouncer or have to permit a private range. The code's own reasoning is sound and I found no allowlist middleware referenced anywhere in the diff or the ingress annotations — the LAPI is protected by TLS + bouncer key + the entrypoint-wide rate-limit middleware (traefik/helmrelease.yaml:49, confirmed applied to all websecure routers) only. This isn't a code bug, but the PR description overstates the defense-in-depth to a reviewer relying on it — worth fixing the description (or confirming I'm missing an allowlist applied elsewhere) before merge.

• The exact traefik.ingress.kubernetes.io/service.serverstransport annotation pattern used here has reportedly failed to resolve before in this cluster. kubernetes/infrastructure/external-dns-unifi/helmrelease.yaml:107-109 carries a comment that IngressRoute CRDs are used "for HTTPS-backend apps whose ServersTransport reference doesn't resolve through the kubernetesIngress provider's annotation path (e.g. apps/pve)" — i.e., this repo previously worked around exactly this annotation mechanism failing for a plain Ingress and switched that app to a native IngressRoute. lapi-ingress.yaml:447-448 uses the same annotation-based mechanism (service.serversscheme + service.serverstransport) that reportedly didn't resolve for apps/pve. The naming (crowdsec-crowdsec-lapi@kubernetescrd) looks correct per Traefik's <namespace>-<name>@kubernetescrd convention for annotation-resolved CRD refs, but given this repo's own history with this exact pattern, I'd verify live (once the vault key exists) that Traefik actually trusts the backend cert and doesn't 495/502, rather than relying on kubectl kustomize building cleanly — a bad resolution here would silently leave the route non-functional (or, worse, fail open to plaintext if Traefik ever falls back) rather than fail the build.

• Version pin unverifiable in this environment. crowdsec_bouncer_version: v0.0.36 / crowdsec_bouncer_sha256: f86e4b72... (ansible/roles/crowdsec-edge-bouncer/defaults/main.yml:47-48) — this review environment has no outbound network access, so I could not confirm the checksum against the actual GitHub release asset. Worth a final manual check before the role is actually run against a host.

🟢 Nits

• ansible/roles/crowdsec-edge-bouncer/tasks/main.yml: the "Enable and start the bouncer" task runs before Ansible's end-of-play handler flush, so a first-time deploy starts the service once with the freshly-templated config and then immediately restarts it again via the notified handler. Harmless (service isn't up yet either way) but slightly redundant; not worth changing.

Tests

No automated tests exist for Ansible roles or Kustomize manifests elsewhere in this repo, so the absence here is consistent with convention. The PR's own "Verification" section (kubectl kustomize builds, CRD schema checked against the live cluster, chart values checked against the pinned tag) is the right kind of evidence for this repo and I was able to independently confirm several of its factual claims (non-root NetBird container, bouncer-key survival semantics, rotator exclusion, IP consistency across files). Two things remain genuinely untestable pre-merge and are already called out in the PR body: (1) the LAPI ingress can't be end-to-end verified until vault_crowdsec_edge_bouncer_key / the matching OpenBao value exist, and (2) whether the proxies' actual egress address matches the published sourceRange-equivalent addresses baked into capi_whitelists.yaml can only be confirmed from a live Traefik access log after deploy. Both are operational follow-ups, not gaps in this PR's own scope, since the bouncer role deploys in dry-run and the k8s side is additive/inert until the key exists.

Recommendation

Looks reasonable — no blocking issues; comments above (especially the PR-description/allowlist mismatch) are worth addressing before merge but aren't defects in the code itself.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `22143c6a7c7ae1192ed788b40ad661405157bf8f`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Groundwork PR: exposes the CrowdSec LAPI publicly via a new Traefik `Ingress`/`ServersTransport` (mTLS to the backend), adds a `BOUNCER_KEY_edge` secret/ExternalSecret, a CAPI whitelist for the proxy addresses, and a new `crowdsec-edge-bouncer` Ansible role deployed in `dry-run` mode. The design reasoning in the PR body is unusually thorough and I could verify most of its factual claims against the checkout (non-root NetBird container, `BOUNCER_KEY_ingress` survives LAPI restarts, `crowdsec-bouncer-key` rotator excludes the new key, the three proxy IPs match `unifi_nat_hairpin.tf`/`ingress-isolation.yaml`). No blocking defects found, but there's a notable mismatch between what the PR description says protects the LAPI and what's actually in the diff, plus one unverified-in-this-environment external dependency and a pattern this repo has hit trouble with before. ## 🟡 Suggestions - **PR description vs. code disagree on the LAPI's protection model.** The PR body's "How the exposure is narrowed" section says exposure is narrowed by an `IPAllowList` with the three proxy addresses via `ipStrategy.excludedIPs`. The actual `kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:29-33` explicitly does the opposite: *"NO IP ALLOWLIST, deliberately"*, because bouncer requests arrive from a Docker bridge address, not a stable client IP, so an allowlist would either 403 the bouncer or have to permit a private range. The code's own reasoning is sound and I found no allowlist middleware referenced anywhere in the diff or the ingress annotations — the LAPI is protected by TLS + bouncer key + the entrypoint-wide rate-limit middleware (`traefik/helmrelease.yaml:49`, confirmed applied to all `websecure` routers) only. This isn't a code bug, but the PR description overstates the defense-in-depth to a reviewer relying on it — worth fixing the description (or confirming I'm missing an allowlist applied elsewhere) before merge. - **The exact `traefik.ingress.kubernetes.io/service.serverstransport` annotation pattern used here has reportedly failed to resolve before in this cluster.** `kubernetes/infrastructure/external-dns-unifi/helmrelease.yaml:107-109` carries a comment that IngressRoute CRDs are used "for HTTPS-backend apps whose ServersTransport reference doesn't resolve through the kubernetesIngress provider's annotation path (e.g. apps/pve)" — i.e., this repo previously worked around exactly this annotation mechanism failing for a plain `Ingress` and switched that app to a native `IngressRoute`. `lapi-ingress.yaml:447-448` uses the same annotation-based mechanism (`service.serversscheme` + `service.serverstransport`) that reportedly didn't resolve for `apps/pve`. The naming (`crowdsec-crowdsec-lapi@kubernetescrd`) looks correct per Traefik's `<namespace>-<name>@kubernetescrd` convention for annotation-resolved CRD refs, but given this repo's own history with this exact pattern, I'd verify live (once the vault key exists) that Traefik actually trusts the backend cert and doesn't 495/502, rather than relying on `kubectl kustomize` building cleanly — a bad resolution here would silently leave the route non-functional (or, worse, fail open to plaintext if Traefik ever falls back) rather than fail the build. - **Version pin unverifiable in this environment.** `crowdsec_bouncer_version: v0.0.36` / `crowdsec_bouncer_sha256: f86e4b72...` (`ansible/roles/crowdsec-edge-bouncer/defaults/main.yml:47-48`) — this review environment has no outbound network access, so I could not confirm the checksum against the actual GitHub release asset. Worth a final manual check before the role is actually run against a host. ## 🟢 Nits - `ansible/roles/crowdsec-edge-bouncer/tasks/main.yml`: the "Enable and start the bouncer" task runs before Ansible's end-of-play handler flush, so a first-time deploy starts the service once with the freshly-templated config and then immediately restarts it again via the notified handler. Harmless (service isn't up yet either way) but slightly redundant; not worth changing. ## Tests No automated tests exist for Ansible roles or Kustomize manifests elsewhere in this repo, so the absence here is consistent with convention. The PR's own "Verification" section (`kubectl kustomize` builds, CRD schema checked against the live cluster, chart values checked against the pinned tag) is the right kind of evidence for this repo and I was able to independently confirm several of its factual claims (non-root NetBird container, bouncer-key survival semantics, rotator exclusion, IP consistency across files). Two things remain genuinely untestable pre-merge and are already called out in the PR body: (1) the LAPI ingress can't be end-to-end verified until `vault_crowdsec_edge_bouncer_key` / the matching OpenBao value exist, and (2) whether the proxies' actual egress address matches the published `sourceRange`-equivalent addresses baked into `capi_whitelists.yaml` can only be confirmed from a live Traefik access log after deploy. Both are operational follow-ups, not gaps in this PR's own scope, since the bouncer role deploys in `dry-run` and the k8s side is additive/inert until the key exists. ## Recommendation Looks reasonable — no blocking issues; comments above (especially the PR-description/allowlist mismatch) are worth addressing before merge but aren't defects in the code itself. <!-- pr-review:sha=22143c6a7c7ae1192ed788b40ad661405157bf8f -->

claude-bot added 1 commit 2026-09-02 10:29:21 +00:00

feat(crowdsec): vault the edge bouncer key for the proxy hosts ... 7897b50318

Completes the pair: the ExternalSecret feeds BOUNCER_KEY_edge to the LAPI, this
feeds the same value to the bouncers that authenticate against it.

A single encrypted value rather than a whole-file vault like its siblings in
group_vars/all and group_vars/edge_host. Appending to one of those means
decrypting it first, which exposes every other secret in that file to add one
line. encrypt_string keeps the blast radius at this value, and the plaintext
never reaches a file: it goes from `bao kv get -field=key` through a pipe
straight into the encryption.

Scoped to netbird_proxy rather than all, because only those hosts run a
bouncer.

Not enrolled in the quarterly rotator, unlike crowdsec-bouncer-key. Nothing on
these hosts re-reads OpenBao on a schedule, so a rotation there would leave
them 401ing against the LAPI until someone ran the playbook. Rotating this one
means updating OpenBao and re-running plays/install-netbird-proxy.yml, in that
order.

Verified with `ansible-inventory --list`: the inventory parses and the value
decrypts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

review-bot requested changes 2026-09-02 10:35:12 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit 7897b5031826e0bc58467f75b012c8d70e053386.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

---

Summary

This PR lays the groundwork for edge CrowdSec enforcement: a public Traefik Ingress/ServersTransport exposing the in-cluster LAPI to the two NetBird proxy hosts, a new BOUNCER_KEY_edge wired into the LAPI, a ConfigMap-based CAPI whitelist for the proxy addresses, and a new Ansible role that installs the crowdsec-firewall-bouncer on those hosts (starting in dry-run, deliberately inert). The change is well-reasoned and its own comments verify most of the claims made in the PR body against the actual chart/CNP/CRD state in this repo (whitelist addresses, CNP admission, crowdsec-root-secret name, the Traefik CRD reference format). One real defect: the Reloader annotation that's supposed to make key rotation work doesn't cover the new secret, which breaks the rotation story the PR itself documents. There's also a mismatch between the PR description and the shipped Ingress design worth reconciling before merge.

🔴 Blocking

• kubernetes/infrastructure/crowdsec/helmrelease.yaml:118-122 (Reloader doesn't cover the new secret) — BOUNCER_KEY_edge (added at line 157) is sourced from the new crowdsec-edge-bouncer-key Secret, but podAnnotations at line 122 only lists secret.reloader.stakater.com/reload: crowdsec-bouncer-key. Per kubernetes/infrastructure/reloader/helmrelease.yaml's own comment, Reloader "never blanket-reloads un-annotated resources," and it only watches secrets named in the annotation (repo convention for multiple secrets is comma-separated, e.g. kubernetes/apps/home-assistant/shelly-manager-deployment.yaml:15: shelly-manager-auth,shelly-manager-llat). Env vars from a secretKeyRef are fixed at pod start, not live-updated, so when crowdsec-edge-bouncer-key is rotated (the PR's own documented procedure: bao kv put ... + re-run install-netbird-proxy.yml), the LAPI pod keeps running with the old key it already registered — ESO will sync the new Secret, but nothing restarts the LAPI to pick it up or re-run register_bouncer. The edge hosts will then present the new key while the LAPI still only trusts the old one, causing 401s exactly at the moment this key is rotated — the scenario the PR is designed to handle. Fix: extend the annotation to crowdsec-bouncer-key,crowdsec-edge-bouncer-key.

🟡 Suggestions

• PR description vs. shipped design (IP allowlist) — The PR body's "How the exposure is narrowed" section states the exposure is narrowed with "IPAllowList with the three proxy addresses... via ipStrategy.excludedIPs", but kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:24-33 explicitly ships no IP allowlist, with a well-reasoned comment explaining why one doesn't work here (a bouncer-originated request to the LAPI shows a Docker-bridge address via X-Forwarded-For, not the proxy's public address). The code's reasoning is sound and the API key is a legitimate sole control, but the PR description doesn't match the code — worth updating the description so the review record (and anyone reading it later) isn't misled into thinking there's a second control layer that isn't actually there.
• ansible/roles/crowdsec-edge-bouncer/templates/crowdsec-firewall-bouncer.service.j2:14 — Type=notify requires crowdsec-firewall-bouncer to call sd_notify(READY=1). Nothing else in this repo installs this specific binary via a hand-written unit (the other hand-rolled units here — node_exporter, chrony_exporter, edge-alloy, the watchdog service — all use Type=simple), so I couldn't cross-check the convention against prior art, and I have no way to run the binary here to confirm it notifies. If it doesn't, systemctl start will block for the default TimeoutStartSec (90s) and then be killed/marked failed even though the process is otherwise healthy. Worth confirming with systemctl status crowdsec-firewall-bouncer (or checking upstream's shipped unit file) on the first real rollout.

🟢 Nits

• ansible/roles/crowdsec-edge-bouncer/tasks/main.yml: on a first install, three tasks (Install the bouncer binary, Deploy the bouncer configuration, Deploy the bouncer systemd unit) all notify the restart handler, and the subsequent Enable and start the bouncer task then also starts the (not-yet-existing) service. The handler fires once at play end and restarts the just-started service — harmless, but a redundant blip immediately after the first start.

Tests

No automated tests accompany this (expected for this repo — infra changes here are typically verified via kubectl kustomize build checks, which the PR body says were run, and CI has a ci-manifests-test.yml for manifest validation). There's no way to exercise the Ansible role or the Reloader/rotation gap above without a live rollout; the Reloader annotation gap in particular won't be caught by any kustomize/build check since it's a build-clean, runtime-only defect — worth a manual note in the runbook or an actual rotation drill before this key is ever rotated for real.

Recommendation

Request changes — 1 blocking issue.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `7897b5031826e0bc58467f75b012c8d70e053386`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR lays the groundwork for edge CrowdSec enforcement: a public Traefik `Ingress`/`ServersTransport` exposing the in-cluster LAPI to the two NetBird proxy hosts, a new `BOUNCER_KEY_edge` wired into the LAPI, a ConfigMap-based CAPI whitelist for the proxy addresses, and a new Ansible role that installs the `crowdsec-firewall-bouncer` on those hosts (starting in `dry-run`, deliberately inert). The change is well-reasoned and its own comments verify most of the claims made in the PR body against the actual chart/CNP/CRD state in this repo (whitelist addresses, CNP admission, `crowdsec-root-secret` name, the Traefik CRD reference format). One real defect: the Reloader annotation that's supposed to make key rotation work doesn't cover the new secret, which breaks the rotation story the PR itself documents. There's also a mismatch between the PR description and the shipped Ingress design worth reconciling before merge. ## 🔴 Blocking - **`kubernetes/infrastructure/crowdsec/helmrelease.yaml:118-122` (Reloader doesn't cover the new secret)** — `BOUNCER_KEY_edge` (added at line 157) is sourced from the new `crowdsec-edge-bouncer-key` Secret, but `podAnnotations` at line 122 only lists `secret.reloader.stakater.com/reload: crowdsec-bouncer-key`. Per `kubernetes/infrastructure/reloader/helmrelease.yaml`'s own comment, Reloader "never blanket-reloads un-annotated resources," and it only watches secrets named in the annotation (repo convention for multiple secrets is comma-separated, e.g. `kubernetes/apps/home-assistant/shelly-manager-deployment.yaml:15`: `shelly-manager-auth,shelly-manager-llat`). Env vars from a `secretKeyRef` are fixed at pod start, not live-updated, so when `crowdsec-edge-bouncer-key` is rotated (the PR's own documented procedure: `bao kv put ...` + re-run `install-netbird-proxy.yml`), the LAPI pod keeps running with the old key it already registered — ESO will sync the new Secret, but nothing restarts the LAPI to pick it up or re-run `register_bouncer`. The edge hosts will then present the new key while the LAPI still only trusts the old one, causing 401s exactly at the moment this key is rotated — the scenario the PR is designed to handle. Fix: extend the annotation to `crowdsec-bouncer-key,crowdsec-edge-bouncer-key`. ## 🟡 Suggestions - **PR description vs. shipped design (IP allowlist)** — The PR body's "How the exposure is narrowed" section states the exposure is narrowed with "**IPAllowList** with the three proxy addresses... via `ipStrategy.excludedIPs`", but `kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:24-33` explicitly ships **no** IP allowlist, with a well-reasoned comment explaining why one doesn't work here (a bouncer-originated request to the LAPI shows a Docker-bridge address via `X-Forwarded-For`, not the proxy's public address). The code's reasoning is sound and the API key is a legitimate sole control, but the PR description doesn't match the code — worth updating the description so the review record (and anyone reading it later) isn't misled into thinking there's a second control layer that isn't actually there. - **`ansible/roles/crowdsec-edge-bouncer/templates/crowdsec-firewall-bouncer.service.j2:14`** — `Type=notify` requires `crowdsec-firewall-bouncer` to call `sd_notify(READY=1)`. Nothing else in this repo installs this specific binary via a hand-written unit (the other hand-rolled units here — `node_exporter`, `chrony_exporter`, `edge-alloy`, the watchdog service — all use `Type=simple`), so I couldn't cross-check the convention against prior art, and I have no way to run the binary here to confirm it notifies. If it doesn't, `systemctl start` will block for the default `TimeoutStartSec` (90s) and then be killed/marked failed even though the process is otherwise healthy. Worth confirming with `systemctl status crowdsec-firewall-bouncer` (or checking upstream's shipped unit file) on the first real rollout. ## 🟢 Nits - `ansible/roles/crowdsec-edge-bouncer/tasks/main.yml`: on a first install, three tasks (`Install the bouncer binary`, `Deploy the bouncer configuration`, `Deploy the bouncer systemd unit`) all `notify` the restart handler, and the subsequent `Enable and start the bouncer` task then also starts the (not-yet-existing) service. The handler fires once at play end and restarts the just-started service — harmless, but a redundant blip immediately after the first start. ## Tests No automated tests accompany this (expected for this repo — infra changes here are typically verified via `kubectl kustomize` build checks, which the PR body says were run, and CI has a `ci-manifests-test.yml` for manifest validation). There's no way to exercise the Ansible role or the Reloader/rotation gap above without a live rollout; the Reloader annotation gap in particular won't be caught by any kustomize/build check since it's a build-clean, runtime-only defect — worth a manual note in the runbook or an actual rotation drill before this key is ever rotated for real. ## Recommendation Request changes — 1 blocking issue. <!-- pr-review:sha=7897b5031826e0bc58467f75b012c8d70e053386 -->

claude-bot referenced this issue from a commit 2026-09-02 11:08:21 +00:00

fix(crowdsec): make Reloader watch the edge bouncer key too

claude-bot added 1 commit 2026-09-02 11:08:22 +00:00

fix(crowdsec): make Reloader watch the edge bouncer key too ... a88bebc76d

Found by the pr-review bot on #751, and it is the exact failure this design
was written to avoid.

BOUNCER_KEY_edge is consumed as an env var from a secretKeyRef, and env is
fixed at pod start. The Reloader annotation listed only crowdsec-bouncer-key,
so a rotation of the edge key would update OpenBao, sync into the Secret, and
reach the hosts through the Ansible play — while the LAPI kept the old value
in its environment and went on trusting the old key. The bouncers would then
present the new one and get 401s, at exactly the moment the rotation was
supposed to be seamless.

The comment directly above that annotation already stated the rule — "it's an
env-var secret, so a restart is required to pick up a rotated key". I added a
second env-var secret and did not extend the list.

Comma-separated, matching shelly-manager-deployment.yaml.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot commented 2026-09-02 11:09:20 +00:00

Author

Member

Copy Link

Copy Source

Review-Findings adressiert.

1. Reloader-Annotation — echter Bug, behoben in a88bebc7.

Der Fund stimmt und trifft genau den Fehler, den dieses Design vermeiden sollte. BOUNCER_KEY_edge kommt als env aus einem secretKeyRef, und env steht beim Pod-Start fest. Die Annotation listete nur crowdsec-bouncer-key — eine Rotation des Edge-Keys hätte OpenBao aktualisiert, ins Secret gesynct und über die Ansible-Play die Hosts erreicht, während die LAPI den alten Wert in ihrer Umgebung behält und weiter den alten Key vertraut. Die Bouncer präsentieren dann den neuen und bekommen 401 — genau in dem Moment, in dem die Rotation nahtlos sein sollte.

Der Kommentar direkt über der Annotation formulierte die Regel bereits: „it's an env-var secret, so a restart is required to pick up a rotated key". Ich habe ein zweites env-var-Secret hinzugefügt und die Liste nicht erweitert.

Jetzt crowdsec-bouncer-key,crowdsec-edge-bouncer-key, kommagetrennt wie in shelly-manager-deployment.yaml.

2. Beschreibung vs. Manifest — Beschreibung war veraltet, korrigiert.

Auch richtig. Die IPAllowList war in der ersten Revision, kam in bc896e55 raus, und die PR-Beschreibung behauptete sie weiter. Die Beschreibung ist jetzt neu geschrieben und dokumentiert, warum es keine gibt: ein Request vom Proxy-Host kommt nicht mit dessen öffentlicher Adresse an, sondern mit einer Docker-Bridge-Adresse (172.18.0.3, gemessen am Watchdog) — die Allowlist hätte den Bouncer mit 403 abgewiesen.

3. Type=notify — nicht verifizierbar, aber belegt.

Fair markiert als unverifizierbar. Der Beleg: das Upstream-Repo liefert seine eigene Unit mit Type=notify aus (config/crowdsec-firewall-bouncer.service), und das AUR-Paket installiert exakt diese. Ich verwende dasselbe Binary aus demselben Release-Tarball, nur mit eigener Unit — die Abweichung ist bewusst und betrifft nur After=: upstream ordnet sich nach crowdsec.service, das es hier nicht gibt, weil die LAPI im Cluster läuft. nss-lookup.target steht stattdessen drin, weil api_url ein öffentlicher Name ist, der vor dem ersten Connect auflösen muss.

Falls sd_notify wider Erwarten nicht greift, schlägt das laut und sofort fehl: systemd läuft in den TimeoutStartSec und markiert den Dienst als failed, statt still zu laufen. Im dry-run-Zustand ohne Auswirkung auf Traffic.

Review-Findings adressiert. **1. Reloader-Annotation — echter Bug, behoben in a88bebc7.** Der Fund stimmt und trifft genau den Fehler, den dieses Design vermeiden sollte. `BOUNCER_KEY_edge` kommt als env aus einem `secretKeyRef`, und env steht beim Pod-Start fest. Die Annotation listete nur `crowdsec-bouncer-key` — eine Rotation des Edge-Keys hätte OpenBao aktualisiert, ins Secret gesynct und über die Ansible-Play die Hosts erreicht, während die LAPI den alten Wert in ihrer Umgebung behält und weiter den alten Key vertraut. Die Bouncer präsentieren dann den neuen und bekommen 401 — genau in dem Moment, in dem die Rotation nahtlos sein sollte. Der Kommentar direkt über der Annotation formulierte die Regel bereits: „it's an env-var secret, so a restart is required to pick up a rotated key". Ich habe ein zweites env-var-Secret hinzugefügt und die Liste nicht erweitert. Jetzt `crowdsec-bouncer-key,crowdsec-edge-bouncer-key`, kommagetrennt wie in `shelly-manager-deployment.yaml`. **2. Beschreibung vs. Manifest — Beschreibung war veraltet, korrigiert.** Auch richtig. Die IPAllowList war in der ersten Revision, kam in `bc896e55` raus, und die PR-Beschreibung behauptete sie weiter. Die Beschreibung ist jetzt neu geschrieben und dokumentiert, *warum* es keine gibt: ein Request vom Proxy-Host kommt nicht mit dessen öffentlicher Adresse an, sondern mit einer Docker-Bridge-Adresse (`172.18.0.3`, gemessen am Watchdog) — die Allowlist hätte den Bouncer mit 403 abgewiesen. **3. `Type=notify` — nicht verifizierbar, aber belegt.** Fair markiert als unverifizierbar. Der Beleg: das Upstream-Repo liefert seine eigene Unit mit `Type=notify` aus (`config/crowdsec-firewall-bouncer.service`), und das AUR-Paket installiert exakt diese. Ich verwende dasselbe Binary aus demselben Release-Tarball, nur mit eigener Unit — die Abweichung ist bewusst und betrifft nur `After=`: upstream ordnet sich nach `crowdsec.service`, das es hier nicht gibt, weil die LAPI im Cluster läuft. `nss-lookup.target` steht stattdessen drin, weil `api_url` ein öffentlicher Name ist, der vor dem ersten Connect auflösen muss. Falls `sd_notify` wider Erwarten nicht greift, schlägt das laut und sofort fehl: systemd läuft in den `TimeoutStartSec` und markiert den Dienst als failed, statt still zu laufen. Im `dry-run`-Zustand ohne Auswirkung auf Traffic.

review-bot reviewed 2026-09-02 11:12:01 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit a88bebc76df813509fc2c9b9365bdac27056d719.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

---

Summary

Adds a public LAPI Ingress for CrowdSec plus a new crowdsec-edge-bouncer Ansible role for proxy-1/proxy-2, gated behind a dedicated (non-rotated) bouncer key and a CAPI whitelist for the three proxy addresses. The role deploys in dry-run mode by default, so nothing enforces yet. I cross-checked the claims in the PR description against the actual manifests/role and the surrounding repo (network policies, existing ExternalSecret/Ingress conventions, the netbird-ingress-egress CIDR list, the hairpin Terraform, Reloader's multi-secret annotation syntax) and everything is internally consistent and matches existing patterns. No blocking issues found.

🟡 Suggestions

• PR description is stale relative to the shipped code on the IP-allowlist point. The description's "How the exposure is narrowed" section says the Ingress uses an IPAllowList with ipStrategy.excludedIPs (same mechanism as rate-limit-middleware.yaml), and "Before merging — one input needed" asks for confirmation of the sourceRange CIDRs. But kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:448-468 explicitly does not add an IP allowlist — its own comment explains this was tried and abandoned, because requests from the proxy hosts arrive via X-Forwarded-For carrying a Docker-bridge address (172.18.0.3), not the published proxy IP, so an allowlist would either 403 the real bouncer or have to admit a private range that isn't a real control. Since the final design relies on the bouncer key alone (which is sound, per the in-file comment), the "one input needed" ask and the IPAllowList description are obsolete and should be edited out of the PR body before merge so future readers of the merge commit / PR history aren't misled about the actual security model.

Tests

No automated tests exist for Ansible roles or kustomize manifests elsewhere in this repo, and this PR follows that norm. The PR body documents manual verification (kubectl kustomize builds, CRD schema checks against the live cluster, a real access-log capture proving the Docker-bridge-IP behavior that killed the allowlist idea). That's consistent with how this repo validates infra changes, and the role deploys in dry-run with an explicit runbook (journalctl / nft list / rollback commands) for validating in production before flipping to enforcing — appropriate given the blast radius (~24k CAPI decisions enforced on first arm).

Recommendation

Looks reasonable — no blocking issues; comment above is optional (recommend tidying the PR description before merge, but it doesn't block the code).

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `a88bebc76df813509fc2c9b9365bdac27056d719`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds a public LAPI Ingress for CrowdSec plus a new `crowdsec-edge-bouncer` Ansible role for proxy-1/proxy-2, gated behind a dedicated (non-rotated) bouncer key and a CAPI whitelist for the three proxy addresses. The role deploys in `dry-run` mode by default, so nothing enforces yet. I cross-checked the claims in the PR description against the actual manifests/role and the surrounding repo (network policies, existing ExternalSecret/Ingress conventions, the `netbird-ingress-egress` CIDR list, the hairpin Terraform, Reloader's multi-secret annotation syntax) and everything is internally consistent and matches existing patterns. No blocking issues found. ## 🟡 Suggestions - **PR description is stale relative to the shipped code on the IP-allowlist point.** The description's "How the exposure is narrowed" section says the Ingress uses an `IPAllowList` with `ipStrategy.excludedIPs` (same mechanism as `rate-limit-middleware.yaml`), and "Before merging — one input needed" asks for confirmation of the `sourceRange` CIDRs. But `kubernetes/infrastructure/crowdsec/lapi-ingress.yaml:448-468` explicitly does **not** add an IP allowlist — its own comment explains this was tried and abandoned, because requests from the proxy hosts arrive via `X-Forwarded-For` carrying a Docker-bridge address (`172.18.0.3`), not the published proxy IP, so an allowlist would either 403 the real bouncer or have to admit a private range that isn't a real control. Since the final design relies on the bouncer key alone (which is sound, per the in-file comment), the "one input needed" ask and the `IPAllowList` description are obsolete and should be edited out of the PR body before merge so future readers of the merge commit / PR history aren't misled about the actual security model. ## Tests No automated tests exist for Ansible roles or kustomize manifests elsewhere in this repo, and this PR follows that norm. The PR body documents manual verification (`kubectl kustomize` builds, CRD schema checks against the live cluster, a real access-log capture proving the Docker-bridge-IP behavior that killed the allowlist idea). That's consistent with how this repo validates infra changes, and the role deploys in `dry-run` with an explicit runbook (journalctl / nft list / rollback commands) for validating in production before flipping to enforcing — appropriate given the blast radius (~24k CAPI decisions enforced on first arm). ## Recommendation Looks reasonable — no blocking issues; comment above is optional (recommend tidying the PR description before merge, but it doesn't block the code). <!-- pr-review:sha=a88bebc76df813509fc2c9b9365bdac27056d719 -->

k.renner merged commit 60cfcf9e7e into master 2026-09-02 11:15:57 +00:00

k.renner referenced this issue from a commit 2026-09-02 11:15:59 +00:00

Merge pull request 'feat(crowdsec): LAPI endpoint and key for the edge firewall bouncers' (#751) from crowdsec-edge-lapi into master

claude-bot referenced this pull request 2026-09-02 11:26:05 +00:00

fix(crowdsec): fetch the bouncer on the controller, not on the host #755

claude-bot referenced this pull request 2026-09-02 11:46:42 +00:00

fix(crowdsec): unpack the bouncer on the host, not on the controller #758

Sign in to join this conversation.

Reviewers

No Reviewers

review-bot

Labels

Clear labels

apollo

Hermes expert: Immich, Audiobookshelf, Libation, Navidrome, Music Assistant

atlas

Hermes expert: Talos, Flux, network, Authentik, Matrix, monitoring

clio

Hermes expert: Paperless, wiki, ical-proxy, TRMNL, websites

daedalus

Hermes expert: Gitea, Woodpecker, Renovate, review-bot, Hermes

hestia

Hermes expert: Home Assistant, IoT, MQTT, ESPHome, voice

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

alert-monitor-bot claude-bot (Claude Bot) flux hermes-bot issue-fix-bot k.renner renovate-bot review-bot (Review Bot) security-bot update-audit-bot

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#751