fix(deps): drop cilium/ebpf fork, pin real upstream v0.22.0 - #909
Conversation
The pinned matthyx/ebpf fork (an intermediate, unmerged snapshot of cilium/ebpf#1988) carries a global weak-pointer kernel BTF cache: a race between one gadget's Cache.Kernel() call and a concurrently loading gadget's Cache.Module() call causes rebaseDecoder to reject a rebase purely on stale pointer identity. This made trace_network fail to start intermittently with "apply CO-RE relocations: load BTF for kmod <mod>: rebase split spec: raw BTF differs", silently leaving ContainerProfile.spec.egress/.ingress permanently null. Real upstream cilium/ebpf v0.22.0 already contains #1988's complete, structurally-correct fix (Cache is fully self-contained per-instance, no global cache, no weak.Pointer, no identity check to race against), so this drops the fork replace entirely and pins straight to v0.22.0. v0.22.0 is several minors ahead and broke two call sites inside the matthyx/inspektor-gadget fork (btf.NewBuilder's signature, and VariableSpec's field shape) - fixed there already, so this also bumps that fork's replace pin to the commit with those compat fixes. Root-caused and fixed upstream in armosec/private-node-agent#520. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PdtsXi1wvbfDu9biC21yRu
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Go module manifest updates the eBPF and TOML dependency versions. It also changes the Inspektor Gadget replacement and removes the separate eBPF replacement. ChangesGo dependency alignment
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The PR replaces the forked eBPF dependency with upstream v0.22.0 and updates the compatible Inspektor Gadget pin; reported validation checks pass, and no actionable merge-blocking risk remains beyond normal review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Reviewed the go.mod/go.sum diff — the dependency swap itself is clean:
Blocker: the DCO check is failing — the single commit has no Component tests were still finishing at review time, but everything completed so far is passing, and CodeQL/CodeRabbit/GitGuardian are green. Once DCO is fixed and tests land green, this is good to merge. |
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
Overview
trace_network(and potentially other gadgets) intermittently fails to start with:This leaves
ContainerProfile.spec.egress/.ingresspermanentlynullwhile other tracers (execs, opens) work fine. Caught via kubescape/helm-charts#904 failing armosec/system-tests'network_policy_known_servers/network_policy_multiple_replicasE2E tests.Root cause: the pinned
github.com/matthyx/ebpffork (an intermediate, unmerged snapshot ofcilium/ebpf#1988) carries a global weak-pointer kernel BTF cache. A race between one gadget'sCache.Kernel()call and a concurrently-loading gadget'sCache.Module()call causesrebaseDecoderto reject a rebase purely on stale pointer identity — unrelated to which specific kernel module or gadget hits it.Already root-caused and fixed in the sibling private repo
armosec/private-node-agent(PR #520, merged). This PR ports that fix here.The fix
replace github.com/cilium/ebpf => github.com/matthyx/ebpf ...directive entirely; bumps the directgithub.com/cilium/ebpfrequirement to real upstreamv0.22.0(no replace needed — the fork'smaincarries zero unique commits beyond real upstream v0.22.0, confirmed via the fork's own PR that produced this fix).replace github.com/inspektor-gadget/inspektor-gadget => github.com/matthyx/inspektor-gadget ...pin to the commit built against realcilium/ebpfv0.22.0 (fixes two internal API breaks inside that fork:btf.NewBuilder's new signature, andebpf.VariableSpec's field-based redesign). Confirmed this is still the fork's latest commit — no newer one exists.v0.22.0is several minors ahead of the previousv0.20.0pin; this repo's own code doesn't callbtf.NewBuilder,CollectionSpec.RewriteMaps, orebpf.VariableSpecdirectly (those breaks only affect code inside the vendoredinspektor-gadgetfork itself, already fixed there), so no node-agent source changes were needed.How to Test
go build ./...,go build -tags stacktrace ./..., andgo build -race ./...all pass clean.go vet ./...passes clean.go mod tidyis stable (no further changes) andgo mod verifyreports all modules verified.cilium/ebpf's BTF/Cache APIs directly (its only directcilium/ebpfimport isrlimit, unrelated) — all BTF/CO-RE/cache logic is reached transitively through the vendoredinspektor-gadgetfork. The regression test for the race lives upstream in that fork (seearmosec/private-node-agent#520's description for the test shape).Related issues/PRs
armosec/private-node-agent#520(private repo)🤖 Generated with Claude Code
AI-skills: none | cmds: /oh-my-claudecode:autopilot
Summary by CodeRabbit