[PR #6425] [MERGED] [client] Categorize privileged tests behind a build tag and run them in Docker #28200

Closed
opened 2026-08-05 08:05:58 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6425
Author: @pappz
Created: 6/13/2026
Status: Merged
Merged: 6/28/2026
Merged by: @pappz

Base: mainHead: feature/privileged-test-tag


📝 Commits (10+)

  • ddc4904 [client] categorize root/system-mutating tests behind a privileged build tag
  • 9d21a2c [client] add PRIV_RUN/PRIV_PKGS filters to the privileged test harness
  • 07f3c75 [client] fix unused-helper lint after the privileged test split
  • a89a5c5 [client] fix privileged test CI failures and run the harness on macOS
  • 31beb96 Update client/internal/engine_privileged_test.go
  • e926f39 Update client/internal/routemanager/systemops/systemops_linux_test.go
  • 28bd68c Update client/internal/routemanager/systemops/systemops_windows_test.go
  • 0339328 Update client/server/server_privileged_test.go
  • 386f629 Merge branch 'main' into feature/privileged-test-tag
  • 1384b79 [ci] Run privileged-tagged tests on darwin, windows and freebsd

📊 Changes

47 files changed (+3014 additions, -2495 deletions)

View changed files

📝 .github/workflows/golang-test-darwin.yml (+1 -1)
📝 .github/workflows/golang-test-freebsd.yml (+10 -10)
📝 .github/workflows/golang-test-linux.yml (+2 -2)
📝 .github/workflows/golang-test-windows.yml (+1 -1)
📝 Makefile (+13 -1)
client/cmd/service_privileged_test.go (+196 -0)
📝 client/cmd/service_test.go (+0 -184)
📝 client/firewall/iptables/manager_linux_test.go (+2 -0)
📝 client/firewall/iptables/router_linux_test.go (+1 -1)
📝 client/firewall/nftables/manager_linux_test.go (+2 -0)
📝 client/firewall/nftables/router_linux_test.go (+1 -1)
📝 client/iface/iface_test.go (+2 -0)
📝 client/iface/wgproxy/proxy_linux_test.go (+1 -1)
📝 client/iface/wgproxy/proxy_seed_test.go (+1 -1)
📝 client/iface/wgproxy/redirect_test.go (+59 -59)
client/internal/dns/server_privileged_test.go (+485 -0)
📝 client/internal/dns/server_test.go (+0 -462)
client/internal/engine_privileged_test.go (+565 -0)
📝 client/internal/engine_test.go (+0 -539)
📝 client/internal/routemanager/manager_test.go (+2 -0)

...and 27 more files

📄 Description

Describe your changes

Previously the whole suite ran under sudo so the privileged subset was implicit and the unit tests couldn't run host-safe locally, on either Linux or macOS.

Root / system-mutating client tests (nftables/iptables/DNS, TUN/WireGuard interfaces, routes, eBPF, SSH/service install) are now gated behind a //go:build privileged tag. Mixed files were split so pure-logic tests stay in the default suite.

  • go test ./client/... (and make test-unit) now runs as a non-root user with no sudo and leaves host networking untouched.
  • A self-hosting ory/dockertest/v4 harness (client/testutil/privileged) runs the privileged suite inside a --privileged --cap-add=NET_ADMIN container via make test-privileged. A DOCKER_CI=true guard prevents container recursion.
  • PRIV_RUN / PRIV_PKGS env vars narrow the container run to a single test/package.

Test plan

  • go test -tags devcert ./client/... passes as non-root, no sudo.
  • go vet -tags 'devcert privileged' ./... compiles (incl. privileged test files) on linux/darwin/bsd/windows.
  • make test-privileged runs the suite in the container.

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Summary by CodeRabbit

  • Tests
    • Split host-mutating/privileged tests behind a privileged build-tag and expanded privileged integration coverage (DNS, routing/system ops, engine, service lifecycle, server connection retries, SSH).
    • Removed lifecycle/integration-heavy coverage from non-privileged suites so it runs only in the explicitly privileged test flow.
  • Chores
    • Updated CI to run privileged-tagged suites with the right build tags and narrower package selection (Linux/macOS/FreeBSD/Windows).
    • Added new make targets: test-unit and test-privileged, plus a containerized privileged test runner.
  • Documentation
    • Added/updated documentation for how to run and author privileged tests and how the container harness behaves.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6425 **Author:** [@pappz](https://github.com/pappz) **Created:** 6/13/2026 **Status:** ✅ Merged **Merged:** 6/28/2026 **Merged by:** [@pappz](https://github.com/pappz) **Base:** `main` ← **Head:** `feature/privileged-test-tag` --- ### 📝 Commits (10+) - [`ddc4904`](https://github.com/netbirdio/netbird/commit/ddc49049126efd4078108b44ccbc00a09038e6cb) [client] categorize root/system-mutating tests behind a privileged build tag - [`9d21a2c`](https://github.com/netbirdio/netbird/commit/9d21a2c8c1c66458c80c38cea7b66e6e7a4b40e9) [client] add PRIV_RUN/PRIV_PKGS filters to the privileged test harness - [`07f3c75`](https://github.com/netbirdio/netbird/commit/07f3c75267f3bb8f6255a06470ec3c7216d8d473) [client] fix unused-helper lint after the privileged test split - [`a89a5c5`](https://github.com/netbirdio/netbird/commit/a89a5c5b9cb77e18ba6a5e14ea18d1caf80023d4) [client] fix privileged test CI failures and run the harness on macOS - [`31beb96`](https://github.com/netbirdio/netbird/commit/31beb969b9e7cc2a044eb4a1d496b2589e99e3ce) Update client/internal/engine_privileged_test.go - [`e926f39`](https://github.com/netbirdio/netbird/commit/e926f396b47319993203f374c29b63a77f71e345) Update client/internal/routemanager/systemops/systemops_linux_test.go - [`28bd68c`](https://github.com/netbirdio/netbird/commit/28bd68c14d7d132ef9f949ecdcca7775b3e12adf) Update client/internal/routemanager/systemops/systemops_windows_test.go - [`0339328`](https://github.com/netbirdio/netbird/commit/03393286da15c478245e39d58766cf5999495679) Update client/server/server_privileged_test.go - [`386f629`](https://github.com/netbirdio/netbird/commit/386f629b966d3bcfc009c1af70812697c46d7350) Merge branch 'main' into feature/privileged-test-tag - [`1384b79`](https://github.com/netbirdio/netbird/commit/1384b79abaca6465a3931d57d08f1d76ba568fc2) [ci] Run privileged-tagged tests on darwin, windows and freebsd ### 📊 Changes **47 files changed** (+3014 additions, -2495 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/golang-test-darwin.yml` (+1 -1) 📝 `.github/workflows/golang-test-freebsd.yml` (+10 -10) 📝 `.github/workflows/golang-test-linux.yml` (+2 -2) 📝 `.github/workflows/golang-test-windows.yml` (+1 -1) 📝 `Makefile` (+13 -1) ➕ `client/cmd/service_privileged_test.go` (+196 -0) 📝 `client/cmd/service_test.go` (+0 -184) 📝 `client/firewall/iptables/manager_linux_test.go` (+2 -0) 📝 `client/firewall/iptables/router_linux_test.go` (+1 -1) 📝 `client/firewall/nftables/manager_linux_test.go` (+2 -0) 📝 `client/firewall/nftables/router_linux_test.go` (+1 -1) 📝 `client/iface/iface_test.go` (+2 -0) 📝 `client/iface/wgproxy/proxy_linux_test.go` (+1 -1) 📝 `client/iface/wgproxy/proxy_seed_test.go` (+1 -1) 📝 `client/iface/wgproxy/redirect_test.go` (+59 -59) ➕ `client/internal/dns/server_privileged_test.go` (+485 -0) 📝 `client/internal/dns/server_test.go` (+0 -462) ➕ `client/internal/engine_privileged_test.go` (+565 -0) 📝 `client/internal/engine_test.go` (+0 -539) 📝 `client/internal/routemanager/manager_test.go` (+2 -0) _...and 27 more files_ </details> ### 📄 Description ## Describe your changes Previously the whole suite ran under `sudo` so the privileged subset was implicit and the unit tests couldn't run host-safe locally, on either Linux or macOS. Root / system-mutating client tests (nftables/iptables/DNS, TUN/WireGuard interfaces, routes, eBPF, SSH/service install) are now gated behind a `//go:build privileged` tag. Mixed files were split so pure-logic tests stay in the default suite. - `go test ./client/...` (and `make test-unit`) now runs as a non-root user with **no sudo** and leaves host networking untouched. - A self-hosting `ory/dockertest/v4` harness (`client/testutil/privileged`) runs the privileged suite inside a `--privileged --cap-add=NET_ADMIN` container via `make test-privileged`. A `DOCKER_CI=true` guard prevents container recursion. - `PRIV_RUN` / `PRIV_PKGS` env vars narrow the container run to a single test/package. ## Test plan - `go test -tags devcert ./client/...` passes as non-root, no sudo. - `go vet -tags 'devcert privileged' ./...` compiles (incl. privileged test files) on linux/darwin/bsd/windows. - `make test-privileged` runs the suite in the container. ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [x] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit * **Tests** * Split host-mutating/privileged tests behind a `privileged` build-tag and expanded privileged integration coverage (DNS, routing/system ops, engine, service lifecycle, server connection retries, SSH). * Removed lifecycle/integration-heavy coverage from non-privileged suites so it runs only in the explicitly privileged test flow. * **Chores** * Updated CI to run privileged-tagged suites with the right build tags and narrower package selection (Linux/macOS/FreeBSD/Windows). * Added new make targets: `test-unit` and `test-privileged`, plus a containerized privileged test runner. * **Documentation** * Added/updated documentation for how to run and author privileged tests and how the container harness behaves. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:05:58 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#28200