From c3f42d8a599625c0cb48aedcf0e34b2e9d8d847c Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Sun, 13 Sep 2026 09:01:31 -0500 Subject: [PATCH] fix(ebpf): Restore capability load tests after the NAT datapath rename The NAT capability test landed in the pre-rename nat66prog package after the NAT datapath moved to natprog, leaving a test-only package that no longer compiles and failing lint, build, and unit tests on main. It now targets the natprog package, the galactic-nat manifest, and its galactic-nat container. The uSID capability test read galactic-cni's capabilities from the manifest, which now grants PERFMON, so it stopped guarding against the variable packet pointer offsets it was written to catch. It now also loads with the manifest's capabilities minus PERFMON. Co-Authored-By: Claude Opus 5 (1M context) --- .../nat_caps_test.go} | 19 ++++---- internal/plumbing/ebpf/prog/usid_caps_test.go | 43 +++++++++++++------ 2 files changed, 40 insertions(+), 22 deletions(-) rename internal/plumbing/ebpf/{nat66prog/nat66_caps_test.go => natprog/nat_caps_test.go} (61%) diff --git a/internal/plumbing/ebpf/nat66prog/nat66_caps_test.go b/internal/plumbing/ebpf/natprog/nat_caps_test.go similarity index 61% rename from internal/plumbing/ebpf/nat66prog/nat66_caps_test.go rename to internal/plumbing/ebpf/natprog/nat_caps_test.go index 1c1a01a7..f132ab0a 100644 --- a/internal/plumbing/ebpf/nat66prog/nat66_caps_test.go +++ b/internal/plumbing/ebpf/natprog/nat_caps_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -package nat66prog +package natprog import ( "errors" @@ -14,21 +14,20 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/loadcaps" ) -// TestNat66_LoadsWithNat66ContainerCapabilities loads the datapath with only -// the capabilities its galactic-nat66 container holds. Every other test here -// loads as full root, which hides verifier rules that depend on -// capabilities. -func TestNat66_LoadsWithNat66ContainerCapabilities(t *testing.T) { +// TestNat_LoadsWithNatContainerCapabilities loads the datapath with only the +// capabilities its galactic-nat container holds. Every other test here loads +// as full root, which hides verifier rules that depend on capabilities. +func TestNat_LoadsWithNatContainerCapabilities(t *testing.T) { requireRoot(t) - manifest := filepath.Join("..", "..", "..", "..", "config", "galactic-nat66", "base", "daemonset.yaml") - caps, err := loadcaps.ContainerCapabilities(manifest, "galactic-nat66") + manifest := filepath.Join("..", "..", "..", "..", "config", "galactic-nat", "base", "daemonset.yaml") + caps, err := loadcaps.ContainerCapabilities(manifest, "galactic-nat") if err != nil { t.Fatal(err) } err = loadcaps.Run(caps, func() error { - var objs Nat66Objects - if err := LoadNat66Objects(&objs, nil); err != nil { + var objs NatObjects + if err := LoadNatObjects(&objs, nil); err != nil { return err } return objs.Close() diff --git a/internal/plumbing/ebpf/prog/usid_caps_test.go b/internal/plumbing/ebpf/prog/usid_caps_test.go index 79849845..2f9d0e3c 100644 --- a/internal/plumbing/ebpf/prog/usid_caps_test.go +++ b/internal/plumbing/ebpf/prog/usid_caps_test.go @@ -7,6 +7,7 @@ package prog import ( "errors" "path/filepath" + "slices" "testing" "github.com/cilium/ebpf" @@ -18,6 +19,11 @@ import ( // capabilities galactic-cni's loader container holds. Every other test here // loads as full root, which hides verifier rules that apply without // CAP_PERFMON. +// +// It also loads without PERFMON even when the manifest grants it. The datapath +// must not depend on PERFMON-only verifier allowances such as variable packet +// pointer offsets, so that the grant stays optional and a regression fails CI +// instead of hiding behind it. func TestUsid_LoadsWithGalacticCNICapabilities(t *testing.T) { requireRoot(t) @@ -26,18 +32,31 @@ func TestUsid_LoadsWithGalacticCNICapabilities(t *testing.T) { if err != nil { t.Fatal(err) } - err = loadcaps.Run(caps, func() error { - var objs UsidObjects - if err := LoadUsidObjects(&objs, nil); err != nil { - return err - } - return objs.Close() - }) - var ve *ebpf.VerifierError - if errors.As(err, &ve) { - t.Fatalf("verifier rejected the datapath with capabilities %v:\n%+v", caps, ve) + tests := []struct { + name string + caps []string + }{ + {"ManifestCapabilities", caps}, + {"ManifestCapabilitiesWithoutPerfmon", slices.DeleteFunc(slices.Clone(caps), func(c string) bool { + return c == "PERFMON" + })}, } - if err != nil { - t.Fatalf("load datapath with capabilities %v: %v", caps, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := loadcaps.Run(tt.caps, func() error { + var objs UsidObjects + if err := LoadUsidObjects(&objs, nil); err != nil { + return err + } + return objs.Close() + }) + var ve *ebpf.VerifierError + if errors.As(err, &ve) { + t.Fatalf("verifier rejected the datapath with capabilities %v:\n%+v", tt.caps, ve) + } + if err != nil { + t.Fatalf("load datapath with capabilities %v: %v", tt.caps, err) + } + }) } }