From d2c6ab5834f0e1208bc743397181d5955f73428b Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 18 Jul 2024 10:58:56 +0200 Subject: [PATCH] Add syncing states with notifications for new module system --- base/notifications/module-mirror.go | 164 +++++++++------------------- base/notifications/notification.go | 12 +- service/compat/notify.go | 2 +- service/mgr/states.go | 50 +++++++-- service/nameserver/module.go | 13 +-- service/netenv/main.go | 2 - service/resolver/main.go | 3 +- service/status/notifications.go | 63 +++++++---- service/updates/notify.go | 2 +- spn/captain/client.go | 36 ++---- 10 files changed, 160 insertions(+), 187 deletions(-) diff --git a/base/notifications/module-mirror.go b/base/notifications/module-mirror.go index e1614fa0d..41ef350f8 100644 --- a/base/notifications/module-mirror.go +++ b/base/notifications/module-mirror.go @@ -1,116 +1,58 @@ package notifications import ( -// "github.com/safing/portbase/modules" -// "github.com/safing/portmaster/base/log" -// "github.com/safing/portmaster/service/mgr" + "github.com/safing/portmaster/base/log" + "github.com/safing/portmaster/service/mgr" ) -// AttachToModule attaches the notification to a module and changes to the -// notification will be reflected on the module failure status. -// func (n *Notification) AttachToState(state *mgr.StateMgr) { -// if state == nil { -// log.Warningf("notifications: invalid usage: cannot attach %s to nil module", n.EventID) -// return -// } - -// n.lock.Lock() -// defer n.lock.Unlock() - -// if n.State != Active { -// log.Warningf("notifications: cannot attach module to inactive notification %s", n.EventID) -// return -// } -// if n.belongsTo != nil { -// log.Warningf("notifications: cannot override attached module for notification %s", n.EventID) -// return -// } - -// // Attach module. -// n.belongsTo = state - -// // Set module failure status. -// switch n.Type { //nolint:exhaustive -// case Info: -// m.Hint(n.EventID, n.Title, n.Message) -// case Warning: -// m.Warning(n.EventID, n.Title, n.Message) -// case Error: -// m.Error(n.EventID, n.Title, n.Message) -// default: -// log.Warningf("notifications: incompatible type for attaching to module in notification %s", n.EventID) -// m.Error(n.EventID, n.Title, n.Message+" [incompatible notification type]") -// } -// } - -// // resolveModuleFailure removes the notification from the module failure status. -// func (n *Notification) resolveModuleFailure() { -// if n.belongsTo != nil { -// // Resolve failure in attached module. -// n.belongsTo.Resolve(n.EventID) - -// // Reset attachment in order to mitigate duplicate failure resolving. -// // Re-attachment is prevented by the state check when attaching. -// n.belongsTo = nil -// } -// } - -// func init() { -// modules.SetFailureUpdateNotifyFunc(mirrorModuleStatus) -// } - -// func mirrorModuleStatus(moduleFailure uint8, id, title, msg string) { -// // Ignore "resolve all" requests. -// if id == "" { -// return -// } - -// // Get notification from storage. -// n, ok := getNotification(id) -// if ok { -// // The notification already exists. - -// // Check if we should delete it. -// if moduleFailure == modules.FailureNone && !n.Meta().IsDeleted() { - -// // Remove belongsTo, as the deletion was already triggered by the module itself. -// n.Lock() -// n.belongsTo = nil -// n.Unlock() - -// n.Delete() -// } - -// return -// } - -// // A notification for the given ID does not yet exists, create it. -// n = &Notification{ -// EventID: id, -// Title: title, -// Message: msg, -// AvailableActions: []*Action{ -// { -// Text: "Get Help", -// Type: ActionTypeOpenURL, -// Payload: "https://safing.io/support/", -// }, -// }, -// } - -// switch moduleFailure { -// case modules.FailureNone: -// return -// case modules.FailureHint: -// n.Type = Info -// n.AvailableActions = nil -// case modules.FailureWarning: -// n.Type = Warning -// n.ShowOnSystem = true -// case modules.FailureError: -// n.Type = Error -// n.ShowOnSystem = true -// } - -// Notify(n) -// } +// SyncWithState syncs the notification to a state in the given state mgr. +// The state will be removed when the notification is removed. +func (n *Notification) SyncWithState(state *mgr.StateMgr) { + if state == nil { + log.Warningf("notifications: invalid usage: cannot attach %s to nil module", n.EventID) + return + } + + n.lock.Lock() + defer n.lock.Unlock() + + if n.Meta().IsDeleted() { + log.Warningf("notifications: cannot attach module to deleted notification %s", n.EventID) + return + } + if n.State != Active { + log.Warningf("notifications: cannot attach module to inactive notification %s", n.EventID) + return + } + if n.belongsTo != nil { + log.Warningf("notifications: cannot override attached module for notification %s", n.EventID) + return + } + + // Attach module. + n.belongsTo = state + + // Create state with same ID. + state.Add(mgr.State{ + ID: n.EventID, + Name: n.Title, + Message: n.Message, + Type: notifTypeToStateType(n.Type), + Data: n.EventData, + }) +} + +func notifTypeToStateType(notifType Type) mgr.StateType { + switch notifType { + case Info: + return mgr.StateTypeHint + case Warning: + return mgr.StateTypeWarning + case Prompt: + return mgr.StateTypeUndefined + case Error: + return mgr.StateTypeError + default: + return mgr.StateTypeUndefined + } +} diff --git a/base/notifications/notification.go b/base/notifications/notification.go index 68526ab17..6db6cef47 100644 --- a/base/notifications/notification.go +++ b/base/notifications/notification.go @@ -101,7 +101,7 @@ type Notification struct { //nolint:maligned // belongsTo holds the state this notification belongs to. The notification // lifecycle will be mirrored to the specified failure status. - // belongsTo *mgr.StateMgr + belongsTo *mgr.StateMgr lock sync.Mutex actionFunction NotificationActionFn // call function to process action @@ -425,6 +425,11 @@ func (n *Notification) delete(pushUpdate bool) { n.lock.Lock() defer n.lock.Unlock() + // Check if notification is already deleted. + if n.Meta().IsDeleted() { + return + } + // Save ID for deletion id = n.EventID @@ -442,7 +447,10 @@ func (n *Notification) delete(pushUpdate bool) { dbController.PushUpdate(n) } - // n.resolveModuleFailure() + // Remove the connected state. + if n.belongsTo != nil { + n.belongsTo.Remove(n.EventID) + } } // Expired notifies the caller when the notification has expired. diff --git a/service/compat/notify.go b/service/compat/notify.go index ce2a949c7..72932aeb3 100644 --- a/service/compat/notify.go +++ b/service/compat/notify.go @@ -137,7 +137,7 @@ func (issue *systemIssue) notify(err error) { notifications.Notify(n) systemIssueNotification = n - // n.AttachToModule(module) + n.SyncWithState(module.states) // Report the raw error as module error. // FIXME(vladimir): Is there a need for this kind of error reporting? diff --git a/service/mgr/states.go b/service/mgr/states.go index 5ecb456a5..a4228522d 100644 --- a/service/mgr/states.go +++ b/service/mgr/states.go @@ -18,12 +18,32 @@ type StateMgr struct { // State describes the state of a manager or module. type State struct { - ID string // Required. - Name string // Required. - Message string // Optional. - Type StateType // Optional. - Time time.Time // Optional, will be set to current time if not set. - Data any // Optional. + // ID is a program-unique ID. + // It must not only be unique within the StateMgr, but for the whole program, + // as it may be re-used with related systems. + // Required. + ID string + + // Name is the name of the state. + // This may also serve as a notification title. + // Required. + Name string + + // Message is a more detailed message about the state. + // Optional. + Message string + + // Type defines the type of the state. + // Optional. + Type StateType + + // Time is the time when the state was created or the originating incident occured. + // Optional, will be set to current time if not set. + Time time.Time + + // Data can hold any additional data necessary for further processing of connected systems. + // Optional. + Data any } // StateType defines commonly used states. @@ -59,6 +79,7 @@ type StateUpdate struct { States []State } +// StatefulModule is used for interface checks on modules. type StatefulModule interface { States() *StateMgr } @@ -87,10 +108,10 @@ func (m *StateMgr) Add(s State) { } // Update or add state. - index := slices.IndexFunc[[]State, State](m.states, func(es State) bool { + index := slices.IndexFunc(m.states, func(es State) bool { return es.ID == s.ID }) - if index > 0 { + if index >= 0 { m.states[index] = s } else { m.states = append(m.states, s) @@ -104,11 +125,18 @@ func (m *StateMgr) Remove(id string) { m.statesLock.Lock() defer m.statesLock.Unlock() - slices.DeleteFunc[[]State, State](m.states, func(s State) bool { - return s.ID == id + var entryRemoved bool + m.states = slices.DeleteFunc(m.states, func(s State) bool { + if s.ID == id { + entryRemoved = true + return true + } + return false }) - m.statesEventMgr.Submit(m.export()) + if entryRemoved { + m.statesEventMgr.Submit(m.export()) + } } // Clear removes all states. diff --git a/service/nameserver/module.go b/service/nameserver/module.go index f2f3e4305..69fad7f28 100644 --- a/service/nameserver/module.go +++ b/service/nameserver/module.go @@ -179,7 +179,7 @@ func startListener(ip net.IP, port uint16, first bool) { } func handleListenError(err error, ip net.IP, port uint16, primaryListener bool) { - // var n *notifications.Notification + var n *notifications.Notification // Create suffix for secondary listener var secondaryEventIDSuffix string @@ -208,7 +208,7 @@ func handleListenError(err error, ip net.IP, port uint16, primaryListener bool) } // Notify user about conflicting service. - _ = notifications.Notify(¬ifications.Notification{ + n = notifications.Notify(¬ifications.Notification{ EventID: eventIDConflictingService + secondaryEventIDSuffix, Type: notifications.Error, Title: "Conflicting DNS Software", @@ -225,7 +225,7 @@ func handleListenError(err error, ip net.IP, port uint16, primaryListener bool) }) } else { // If no conflict is found, report the error directly. - _ = notifications.Notify(¬ifications.Notification{ + n = notifications.Notify(¬ifications.Notification{ EventID: eventIDListenerFailed + secondaryEventIDSuffix, Type: notifications.Error, Title: "Secure DNS Error", @@ -238,10 +238,9 @@ func handleListenError(err error, ip net.IP, port uint16, primaryListener bool) } // Attach error to module, if primary listener. - // TODO(vladimir): is this needed? - // if primaryListener { - // n.AttachToModule(module) - // } + if primaryListener { + n.SyncWithState(module.states) + } } func stop() error { diff --git a/service/netenv/main.go b/service/netenv/main.go index 608b1a066..e1a681500 100644 --- a/service/netenv/main.go +++ b/service/netenv/main.go @@ -48,7 +48,6 @@ func (ne *NetEnv) Stop() error { } func prep() error { - // FIXME(vladimir): Does this need to be in the prep function. checkForIPv6Stack() if err := registerAPIEndpoints(); err != nil { @@ -59,7 +58,6 @@ func prep() error { return err } - // FIXME(vladimir): Does this need to be in the prep function. return prepLocation() } diff --git a/service/resolver/main.go b/service/resolver/main.go index 46c26ab02..69b7c51db 100644 --- a/service/resolver/main.go +++ b/service/resolver/main.go @@ -188,8 +188,7 @@ This notification will go away when Portmaster detects a working configured DNS notifications.Notify(n) failingResolverNotification = n - // TODO(vladimir): is this needed? - // n.AttachToModule(module) + n.SyncWithState(module.states) } func resetFailingResolversNotification() { diff --git a/service/status/notifications.go b/service/status/notifications.go index c5208241a..0e7deb896 100644 --- a/service/status/notifications.go +++ b/service/status/notifications.go @@ -20,16 +20,48 @@ func (s *Status) deriveNotificationsFromStateUpdate(update mgr.StateUpdate) { for _, state := range update.States { seenStateIDs[state.ID] = struct{}{} - _, ok := notifs[state.ID] - if !ok { - n := notifications.Notify(¬ifications.Notification{ - EventID: update.Module + ":" + state.ID, - Type: stateTypeToNotifType(state.Type), - Title: state.Name, - Message: state.Message, - }) + // Check if we already have a notification registered. + if _, ok := notifs[state.ID]; ok { + continue + } + + // Check if the notification was pre-created. + // If a matching notification is found, assign it. + n := notifications.Get(state.ID) + if n != nil { notifs[state.ID] = n + continue + } + + // Create a new notification. + n = ¬ifications.Notification{ + EventID: state.ID, + Title: state.Name, + Message: state.Message, + AvailableActions: []*notifications.Action{ + { + Text: "Get Help", + Type: notifications.ActionTypeOpenURL, + Payload: "https://safing.io/support/", + }, + }, + } + switch state.Type { + case mgr.StateTypeWarning: + n.Type = notifications.Warning + n.ShowOnSystem = true + case mgr.StateTypeError: + n.Type = notifications.Error + n.ShowOnSystem = true + case mgr.StateTypeHint, mgr.StateTypeUndefined: + fallthrough + default: + n.Type = notifications.Info + n.AvailableActions = nil } + + notifs[state.ID] = n + notifications.Notify(n) } // Remove notifications. @@ -40,18 +72,3 @@ func (s *Status) deriveNotificationsFromStateUpdate(update mgr.StateUpdate) { } } } - -func stateTypeToNotifType(stateType mgr.StateType) notifications.Type { - switch stateType { - case mgr.StateTypeUndefined: - return notifications.Info - case mgr.StateTypeHint: - return notifications.Info - case mgr.StateTypeWarning: - return notifications.Warning - case mgr.StateTypeError: - return notifications.Error - default: - return notifications.Info - } -} diff --git a/service/updates/notify.go b/service/updates/notify.go index 0cd97bfde..30b2bd32b 100644 --- a/service/updates/notify.go +++ b/service/updates/notify.go @@ -164,5 +164,5 @@ func notifyUpdateCheckFailed(force bool, err error) { ResultAction: "display", }, }, - ) // FIXME: add replacement for this .AttachToModule(module) + ).SyncWithState(module.states) } diff --git a/spn/captain/client.go b/spn/captain/client.go index 49bcf7828..4a2546004 100644 --- a/spn/captain/client.go +++ b/spn/captain/client.go @@ -228,9 +228,7 @@ func clientCheckAccountAndTokens(ctx context.Context) clientComponentResult { `Please restart Portmaster.`, // TODO: Add restart button. // TODO: Use special UI restart action in order to reload UI on restart. - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) resetSPNStatus(StatusFailed, true) log.Errorf("spn/captain: client internal error: %s", err) return clientResultReconnect @@ -243,9 +241,7 @@ func clientCheckAccountAndTokens(ctx context.Context) clientComponentResult { "SPN Login Required", `Please log in to access the SPN.`, spnLoginButton, - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) resetSPNStatus(StatusFailed, true) log.Warningf("spn/captain: enabled but not logged in") return clientResultReconnect @@ -263,9 +259,7 @@ func clientCheckAccountAndTokens(ctx context.Context) clientComponentResult { "spn:failed-to-update-user", "SPN Account Server Error", fmt.Sprintf(`The status of your SPN account could not be updated: %s`, err), - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) resetSPNStatus(StatusFailed, true) log.Errorf("spn/captain: failed to update ineligible account: %s", err) return clientResultReconnect @@ -282,9 +276,7 @@ func clientCheckAccountAndTokens(ctx context.Context) clientComponentResult { "SPN Not Included In Package", "Your current Portmaster Package does not include access to the SPN. Please upgrade your package on the Account Page.", spnOpenAccountPage, - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) resetSPNStatus(StatusFailed, true) return clientResultReconnect } @@ -299,9 +291,7 @@ func clientCheckAccountAndTokens(ctx context.Context) clientComponentResult { "Portmaster Package Issue", "Cannot enable SPN: "+message, spnOpenAccountPage, - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) resetSPNStatus(StatusFailed, true) return clientResultReconnect } @@ -321,9 +311,7 @@ func clientCheckAccountAndTokens(ctx context.Context) clientComponentResult { "spn:tokens-exhausted", "SPN Access Tokens Exhausted", `The Portmaster failed to get new access tokens to access the SPN. The Portmaster will automatically retry to get new access tokens.`, - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) resetSPNStatus(StatusFailed, false) } return clientResultRetry @@ -371,9 +359,7 @@ func clientConnectToHomeHub(ctx context.Context) clientComponentResult { Key: CfgOptionHomeHubPolicyKey, }, }, - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) case errors.Is(err, ErrReInitSPNSuggested): notifications.NotifyError( @@ -389,18 +375,14 @@ func clientConnectToHomeHub(ctx context.Context) clientComponentResult { ResultAction: "display", }, }, - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) default: notifications.NotifyWarn( "spn:home-hub-failure", "SPN Failed to Connect", fmt.Sprintf("Failed to connect to a home hub: %s. The Portmaster will retry to connect automatically.", err), - ) - // TODO(vladimir): this is not needed right - // .AttachToModule(module) + ).SyncWithState(module.states) } return clientResultReconnect