Feature/ecs alerts#699
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughBumps gadget VERSION to v0.48.0, switches gadget image references from quay.io to ghcr.io, refactors tracer event data to use source.DeepCopy(...) instead of pooled items, removes internal data pooling, adds ECS accessor methods on DatasourceEvent/EnrichEvent, and applies broad dependency updates in go.mod. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 61-65: Update the k8s.io/kubectl dependency to v0.35.0 to match
the other Kubernetes packages; locate the go.mod entry for the module
"k8s.io/kubectl" and change its version from v0.34.1 to v0.35.0, then run the
module tooling (e.g., go get k8s.io/kubectl@v0.35.0 and go mod tidy) to ensure
all transitive dependencies and go.sum are updated accordingly.
In `@pkg/containerwatcher/v2/tracers/kmod.go`:
- Around line 109-111: The closure passed to datasource.Subscribe captures the
loop variable d but should use the callback's source parameter instead; update
the event construction in the Subscribe callback (the function literal passed to
Subscribe in kmod.go) to set Datasource to source (and call
source.DeepCopy(data)) rather than d, so kt.callback receives the actual
triggering datasource in the utils.DatasourceEvent.
In `@pkg/containerwatcher/v2/tracers/symlink.go`:
- Around line 108-111: In the Subscribe callback inside the loop over
gadgetCtx.GetDataSources() the closure uses the loop variable d, which can be
captured incorrectly; change the callback to use the provided source parameter
when constructing the DatasourceEvent (e.g., call
st.callback(&utils.DatasourceEvent{Datasource: source, Data:
source.DeepCopy(data), EventType: utils.SymlinkEventType})) and similarly update
any other tracer callbacks with identical patterns so Release()/field access
target the correct datasource instead of the loop variable d.
🧹 Nitpick comments (3)
pkg/containerwatcher/v2/tracers/ssh.go (1)
113-115: Use callback parametersourceinstead of loop variabledfor semantic clarity.The callback receives
sourceas the data source that triggered the subscription; using it instead of the loop variabledis semantically more correct and clearer in intent.♻️ Proposed fix
- st.callback(&utils.DatasourceEvent{Datasource: d, Data: source.DeepCopy(data), EventType: utils.SSHEventType}) + st.callback(&utils.DatasourceEvent{Datasource: source, Data: source.DeepCopy(data), EventType: utils.SSHEventType})pkg/utils/datasource_event.go (1)
881-883: Add a nil guard inRelease()to avoid panics on synthetic events.Defensive checks protect callers that build
DatasourceEventmanually (tests/mocks) without a datasource.🛡️ Proposed fix
func (e *DatasourceEvent) Release() { - e.Datasource.Release(e.Data) + if e == nil || e.Datasource == nil || e.Data == nil { + return + } + e.Datasource.Release(e.Data) }pkg/containerwatcher/v2/tracers/network.go (1)
140-148: Consider nil check for event parameter.The
callbackmethod accessesevent.GetContainerID()andevent.GetPID()without checking ifeventis nil. While the current caller always passes a non-nil event, a defensive check would improve robustness.♻️ Optional defensive nil check
// callback handles events from the tracer func (nt *NetworkTracer) callback(event utils.NetworkEvent) { - if nt.eventCallback != nil { + if nt.eventCallback != nil && event != nil { // Extract container ID and process ID from the network event containerID := event.GetContainerID() processID := event.GetPID() nt.eventCallback(event, containerID, processID) } }
… to EnrichEvent interface and related types
Overview
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.