From e582e84f27077d1318b5f91016ba0becf89b92a6 Mon Sep 17 00:00:00 2001 From: Alon Zivony Date: Thu, 30 May 2024 23:56:45 +0300 Subject: [PATCH] Change actions to be an interface --- pkg/ebpf/tracee.go | 32 +++---- pkg/events/dependencies/actions.go | 20 +++++ pkg/events/dependencies/errors.go | 34 ++++++++ pkg/events/dependencies/manager.go | 48 +++++----- pkg/events/dependencies/manager_test.go | 111 ++++++++++++++++++++---- 5 files changed, 187 insertions(+), 58 deletions(-) create mode 100644 pkg/events/dependencies/actions.go create mode 100644 pkg/events/dependencies/errors.go diff --git a/pkg/ebpf/tracee.go b/pkg/ebpf/tracee.go index 6f3af7f1683d..fbf7582ec557 100644 --- a/pkg/ebpf/tracee.go +++ b/pkg/ebpf/tracee.go @@ -244,24 +244,25 @@ func New(cfg config.Config) (*Tracee, error) { // before choosing them. There is no reason to choose the event in the New function anyhow. t.eventsDependencies.SubscribeAdd( dependencies.EventNodeType, - func(node interface{}) dependencies.Action { + func(node interface{}) []dependencies.Action { eventNode, ok := node.(*dependencies.EventNode) if !ok { logger.Errorw("Got node from type not requested") - return dependencies.NoAction + return nil } t.addDependencyEventToState(eventNode.GetID(), eventNode.GetDependents()) - return dependencies.NoAction + return nil }) t.eventsDependencies.SubscribeRemove( dependencies.EventNodeType, - func(node interface{}) { + func(node interface{}) []dependencies.Action { eventNode, ok := node.(*dependencies.EventNode) if !ok { logger.Errorw("Got node from type not requested") - return + return nil } t.removeEventFromState(eventNode.GetID()) + return nil }) // Initialize capabilities rings soon @@ -934,16 +935,16 @@ func (t *Tracee) validateKallsymsDependencies() { t.eventsDependencies.SubscribeAdd( dependencies.EventNodeType, - func(node interface{}) dependencies.Action { + func(node interface{}) []dependencies.Action { eventNode, ok := node.(*dependencies.EventNode) if !ok { logger.Errorw("Got node from type not requested") - return dependencies.NoAction + return nil } if !validateEvent(eventNode.GetID()) { - return dependencies.CancelNodeAdd + return []dependencies.Action{dependencies.NewCancelNodeAddAction(fmt.Errorf("event is missing ksymbols"))} } - return dependencies.NoAction + return nil }) for eventId := range t.eventsState { @@ -1151,26 +1152,26 @@ func (t *Tracee) attachProbes() error { // probes upon changes t.eventsDependencies.SubscribeAdd( dependencies.ProbeNodeType, - func(node interface{}) dependencies.Action { + func(node interface{}) []dependencies.Action { probeNode, ok := node.(*dependencies.ProbeNode) if !ok { logger.Errorw("Got node from type not requested") - return dependencies.NoAction + return nil } err := t.probes.Attach(probeNode.GetHandle(), t.cgroups) if err != nil { - return dependencies.CancelNodeAdd + return []dependencies.Action{dependencies.NewCancelNodeAddAction(err)} } - return dependencies.NoAction + return nil }) t.eventsDependencies.SubscribeRemove( dependencies.ProbeNodeType, - func(node interface{}) { + func(node interface{}) []dependencies.Action { probeNode, ok := node.(*dependencies.ProbeNode) if !ok { logger.Errorw("Got node from type not requested") - return + return nil } err := t.probes.Detach(probeNode.GetHandle()) if err != nil { @@ -1178,6 +1179,7 @@ func (t *Tracee) attachProbes() error { "probe", probeNode.GetHandle(), "error", err) } + return nil }) // Attach probes to their respective eBPF programs or cancel events if a required probe is missing. diff --git a/pkg/events/dependencies/actions.go b/pkg/events/dependencies/actions.go new file mode 100644 index 000000000000..895ba5967723 --- /dev/null +++ b/pkg/events/dependencies/actions.go @@ -0,0 +1,20 @@ +package dependencies + +// Action is a struct representing an action request for the tree by a watcher function. +// All modification operations on the tree should be done using an Action and not directly +// inside a watchers scope, to avoid bugs with operations order. +type Action interface{} + +// CancelNodeAddAction cancel the node add to the manager, and cause all its dependencies to be +// removed as well if they are not needed by any other node (similar to UnselectEvent). +// This action will not prevent other watchers from being called. +// When cancelled, event remove watchers will be called to allow cleaning. +// Please use this action instead of calling UnselectEvent directly to avoid bugs with +// operations order. +type CancelNodeAddAction struct { + Reason error +} + +func NewCancelNodeAddAction(reason error) *CancelNodeAddAction { + return &CancelNodeAddAction{Reason: reason} +} diff --git a/pkg/events/dependencies/errors.go b/pkg/events/dependencies/errors.go new file mode 100644 index 000000000000..e96c1df2a41c --- /dev/null +++ b/pkg/events/dependencies/errors.go @@ -0,0 +1,34 @@ +package dependencies + +import ( + "errors" + "fmt" + "strings" +) + +// ErrNodeAddCancelled is the error produced when cancelling a node add to the manager +// using the CancelNodeAddAction Action. +type ErrNodeAddCancelled struct { + Reasons []error +} + +func NewErrNodeAddCancelled(reasons []error) *ErrNodeAddCancelled { + return &ErrNodeAddCancelled{Reasons: reasons} +} + +func (cancelErr *ErrNodeAddCancelled) Error() string { + var errorsStrings []string + for _, err := range cancelErr.Reasons { + errorsStrings = append(errorsStrings, err.Error()) + } + return fmt.Sprintf("node add was cancelled, reasons: \"%s\"", strings.Join(errorsStrings, "\", \"")) +} + +func (cancelErr *ErrNodeAddCancelled) AddReason(reason error) { + cancelErr.Reasons = append(cancelErr.Reasons, reason) +} + +var ( + ErrNodeType = errors.New("unsupported node type") + ErrNodeNotFound = errors.New("node not found") +) diff --git a/pkg/events/dependencies/manager.go b/pkg/events/dependencies/manager.go index 5499487fffa0..217743b5e609 100644 --- a/pkg/events/dependencies/manager.go +++ b/pkg/events/dependencies/manager.go @@ -1,7 +1,6 @@ package dependencies import ( - "errors" "fmt" "reflect" @@ -10,13 +9,6 @@ import ( "github.com/aquasecurity/tracee/pkg/logger" ) -type Action int - -const ( - NoAction Action = iota - CancelNodeAdd // Remove the node from the manager (will still call other watchers) -) - type NodeType string const ( @@ -26,12 +18,6 @@ const ( IllegalNodeType NodeType = "illegal" ) -var ( - ErrNodeAddCancelled = errors.New("node add was cancelled") - ErrNodeType = errors.New("unsupported node type") - ErrNodeNotFound = errors.New("node not found") -) - // Manager is a management tree for the current dependencies of events. // As events can depend on multiple things (e.g events, probes), it manages their connections in the form of a tree. // The tree supports watcher functions for adding and removing nodes. @@ -40,8 +26,8 @@ var ( type Manager struct { events map[events.ID]*EventNode probes map[probes.Handle]*ProbeNode - onAdd map[NodeType][]func(node interface{}) Action - onRemove map[NodeType][]func(node interface{}) + onAdd map[NodeType][]func(node interface{}) []Action + onRemove map[NodeType][]func(node interface{}) []Action dependenciesGetter func(events.ID) events.Dependencies } @@ -49,19 +35,19 @@ func NewDependenciesManager(dependenciesGetter func(events.ID) events.Dependenci return &Manager{ events: make(map[events.ID]*EventNode), probes: make(map[probes.Handle]*ProbeNode), - onAdd: make(map[NodeType][]func(node interface{}) Action), - onRemove: make(map[NodeType][]func(node interface{})), + onAdd: make(map[NodeType][]func(node interface{}) []Action), + onRemove: make(map[NodeType][]func(node interface{}) []Action), dependenciesGetter: dependenciesGetter, } } // SubscribeAdd adds a watcher function called upon the addition of an event to the tree. -func (m *Manager) SubscribeAdd(subscribeType NodeType, onAdd func(node interface{}) Action) { +func (m *Manager) SubscribeAdd(subscribeType NodeType, onAdd func(node interface{}) []Action) { m.onAdd[subscribeType] = append(m.onAdd[subscribeType], onAdd) } // SubscribeRemove adds a watcher function called upon the removal of an event from the tree. -func (m *Manager) SubscribeRemove(subscribeType NodeType, onRemove func(node interface{})) { +func (m *Manager) SubscribeRemove(subscribeType NodeType, onRemove func(node interface{}) []Action) { m.onRemove[subscribeType] = append(m.onRemove[subscribeType], onRemove) } @@ -257,20 +243,30 @@ func (m *Manager) triggerOnAdd(node interface{}) error { var actions []Action addWatchers := m.onAdd[nodeType] for _, onAdd := range addWatchers { - actions = append(actions, onAdd(node)) + actions = append(actions, onAdd(node)...) } addWatchers = m.onAdd[AllNodeTypes] for _, onAdd := range addWatchers { - actions = append(actions, onAdd(node)) + actions = append(actions, onAdd(node)...) } + var cancelNodeAddErr *ErrNodeAddCancelled + shouldCancel := false for _, action := range actions { - switch action { - case CancelNodeAdd: - err = ErrNodeAddCancelled + switch typedAction := action.(type) { + case *CancelNodeAddAction: + shouldCancel = true + if cancelNodeAddErr == nil { + err = NewErrNodeAddCancelled([]error{typedAction.Reason}) + } else { + cancelNodeAddErr.AddReason(typedAction.Reason) + } } } - return err + if shouldCancel { + return cancelNodeAddErr + } + return nil } // triggerOnRemove triggers all on-remove watchers diff --git a/pkg/events/dependencies/manager_test.go b/pkg/events/dependencies/manager_test.go index ab71b19bbf7e..bf6fb6584c29 100644 --- a/pkg/events/dependencies/manager_test.go +++ b/pkg/events/dependencies/manager_test.go @@ -1,6 +1,7 @@ package dependencies_test import ( + "errors" "slices" "testing" @@ -75,23 +76,25 @@ func TestManager_AddEvent(t *testing.T) { }, } - for _, testCase := range testCases { - t.Run( - testCase.name, func(t *testing.T) { + t.Run("Sanity", func(t *testing.T) { + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { // Create a new Manager instance m := dependencies.NewDependenciesManager(getTestDependenciesFunc(testCase.deps)) var eventsAdditions []events.ID m.SubscribeAdd( dependencies.EventNodeType, - func(node interface{}) dependencies.Action { + func(node interface{}) []dependencies.Action { newEventNode := node.(*dependencies.EventNode) eventsAdditions = append(eventsAdditions, newEventNode.GetID()) - return dependencies.NoAction - }) + return nil + }, + ) // Check that multiple selects are not causing any issues for i := 0; i < 3; i++ { - m.SelectEvent(testCase.eventToAdd) + _, err := m.SelectEvent(testCase.eventToAdd) + require.NoError(t, err) depProbes := make(map[probes.Handle][]events.ID) for id, expDep := range testCase.deps { @@ -101,7 +104,10 @@ func TestManager_AddEvent(t *testing.T) { assert.ElementsMatch(t, expDep.GetIDs(), dep.GetIDs()) for _, probe := range dep.GetProbes() { - depProbes[probe.GetHandle()] = append(depProbes[probe.GetHandle()], id) + depProbes[probe.GetHandle()] = append( + depProbes[probe.GetHandle()], + id, + ) } // Test dependencies building @@ -121,8 +127,68 @@ func TestManager_AddEvent(t *testing.T) { assert.ElementsMatch(t, ids, probeNode.GetDependents()) } } - }) - } + }, + ) + } + }) + t.Run("Add cancel", func(t *testing.T) { + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + // Create a new Manager instance + m := dependencies.NewDependenciesManager(getTestDependenciesFunc(testCase.deps)) + var eventsAdditions, eventsRemove []events.ID + // Count additions + m.SubscribeAdd( + dependencies.EventNodeType, + func(node interface{}) []dependencies.Action { + newEventNode := node.(*dependencies.EventNode) + eventsAdditions = append(eventsAdditions, newEventNode.GetID()) + return nil + }, + ) + + // Count removes + m.SubscribeRemove( + dependencies.EventNodeType, + func(node interface{}) []dependencies.Action { + removeEventNode := node.(*dependencies.EventNode) + eventsRemove = append(eventsRemove, removeEventNode.GetID()) + return nil + }, + ) + + // Cancel event add + m.SubscribeAdd( + dependencies.EventNodeType, + func(node interface{}) []dependencies.Action { + newEventNode := node.(*dependencies.EventNode) + if newEventNode.GetID() == testCase.eventToAdd { + return []dependencies.Action{dependencies.NewCancelNodeAddAction(errors.New("fail"))} + } + return nil + }, + ) + + _, err := m.SelectEvent(testCase.eventToAdd) + require.IsType(t, &dependencies.ErrNodeAddCancelled{}, err) + + // Check that all the dependencies were cancelled + depProbes := make(map[probes.Handle][]events.ID) + for id := range testCase.deps { + _, err := m.GetEvent(id) + assert.ErrorIs(t, err, dependencies.ErrNodeNotFound, id) + } + for handle := range depProbes { + _, err := m.GetProbe(handle) + assert.ErrorIs(t, err, dependencies.ErrNodeNotFound, handle) + } + assert.Len(t, eventsAdditions, len(testCase.deps)) + assert.Len(t, eventsRemove, len(testCase.deps)) + assert.ElementsMatch(t, eventsAdditions, eventsRemove) + }, + ) + } + }) } func TestManager_RemoveEvent(t *testing.T) { @@ -269,16 +335,19 @@ func TestManager_RemoveEvent(t *testing.T) { var eventsRemoved []events.ID m.SubscribeRemove( dependencies.EventNodeType, - func(node interface{}) { + func(node interface{}) []dependencies.Action { removedEvtNode := node.(*dependencies.EventNode) eventsRemoved = append(eventsRemoved, removedEvtNode.GetID()) + return nil }) for _, preAddedEvent := range testCase.preAddedEvents { - m.SelectEvent(preAddedEvent) + _, err := m.SelectEvent(preAddedEvent) + require.NoError(t, err) } - m.SelectEvent(testCase.eventToAdd) + _, err := m.SelectEvent(testCase.eventToAdd) + require.NoError(t, err) expectedDepProbes := make(map[probes.Handle][]events.ID) for id, expDep := range testCase.deps { @@ -292,7 +361,12 @@ func TestManager_RemoveEvent(t *testing.T) { // Check that multiple removes are not causing any issues for i := 0; i < 3; i++ { - m.RemoveEvent(testCase.eventToAdd) + err := m.RemoveEvent(testCase.eventToAdd) + if i == 0 { + require.NoError(t, err) + } else { + assert.ErrorIs(t, err, dependencies.ErrNodeNotFound, testCase.name) + } for _, id := range testCase.expectedRemovedEvents { _, err := m.GetEvent(id) @@ -435,16 +509,19 @@ func TestManager_UnselectEvent(t *testing.T) { var eventsRemoved []events.ID m.SubscribeRemove( dependencies.EventNodeType, - func(node interface{}) { + func(node interface{}) []dependencies.Action { removedEvtNode := node.(*dependencies.EventNode) eventsRemoved = append(eventsRemoved, removedEvtNode.GetID()) + return nil }) for _, preAddedEvent := range testCase.preAddedEvents { - m.SelectEvent(preAddedEvent) + _, err := m.SelectEvent(preAddedEvent) + require.NoError(t, err) } - m.SelectEvent(testCase.eventToAdd) + _, err := m.SelectEvent(testCase.eventToAdd) + require.NoError(t, err) // Check that multiple unselects are not causing any issues for i := 0; i < 3; i++ {