From e3631d2cc2c985e1ef03b73c49b670951c889d7e Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 6 Oct 2023 17:27:59 +0200 Subject: [PATCH] Fix and improve triggering updates from UI --- netenv/online-status.go | 2 +- updates/api.go | 28 +++++++++++++++++++++++----- updates/config.go | 5 +++-- updates/main.go | 25 ++++++++++++++++--------- updates/notify.go | 36 +++++++++++++++++++++++------------- 5 files changed, 66 insertions(+), 30 deletions(-) diff --git a/netenv/online-status.go b/netenv/online-status.go index 377b67164..8be03a592 100644 --- a/netenv/online-status.go +++ b/netenv/online-status.go @@ -219,7 +219,7 @@ func updateOnlineStatus(status OnlineStatus, portalURL *url.URL, comment string) // Trigger update check when coming (semi) online. if Online() { - _ = updates.TriggerUpdate(false) + _ = updates.TriggerUpdate(false, false) } } } diff --git a/updates/api.go b/updates/api.go index 7c2cd8c0b..2a25b8bca 100644 --- a/updates/api.go +++ b/updates/api.go @@ -1,6 +1,8 @@ package updates import ( + "net/http" + "github.com/safing/portbase/api" ) @@ -10,16 +12,32 @@ const ( func registerAPIEndpoints() error { return api.RegisterEndpoint(api.Endpoint{ + Name: "Check for Updates", + Description: "Checks if new versions are available. If automatic updates are enabled, they are also downloaded and applied.", + Parameters: []api.Parameter{{ + Method: http.MethodPost, + Field: "download", + Value: "", + Description: "Force downloading and applying of all updates, regardless of auto-update settings.", + }}, Path: apiPathCheckForUpdates, Write: api.PermitUser, BelongsTo: module, - ActionFunc: func(_ *api.Request) (msg string, err error) { - if err := TriggerUpdate(false); err != nil { + ActionFunc: func(r *api.Request) (msg string, err error) { + // Check if we should also download regardless of settings. + downloadAll := r.URL.Query().Has("download") + + // Trigger update task. + err = TriggerUpdate(true, downloadAll) + if err != nil { return "", err } - return "triggered update check", nil + + // Report how we triggered. + if downloadAll { + return "downloading all updates...", nil + } + return "checking for updates...", nil }, - Name: "Check for Updates", - Description: "Checks if new versions are available and downloads and applies them, if automatic updates are enabled.", }) } diff --git a/updates/config.go b/updates/config.go index 18d41dedb..a8fff098f 100644 --- a/updates/config.go +++ b/updates/config.go @@ -24,7 +24,8 @@ var ( softwareUpdatesCurrentlyEnabled bool intelUpdatesCurrentlyEnabled bool previousDevMode bool - forceUpdate = abool.New() + forceCheck = abool.New() + forceDownload = abool.New() ) func registerConfig() error { @@ -165,7 +166,7 @@ func updateRegistryConfig(_ context.Context, _ interface{}) error { if softwareUpdatesCurrentlyEnabled || intelUpdatesCurrentlyEnabled { module.Resolve("") - if err := TriggerUpdate(false); err != nil { + if err := TriggerUpdate(true, false); err != nil { log.Warningf("updates: failed to trigger update: %s", err) } log.Infof("updates: automatic updates are now enabled") diff --git a/updates/main.go b/updates/main.go index 911062569..4f3daa62f 100644 --- a/updates/main.go +++ b/updates/main.go @@ -220,17 +220,20 @@ func start() error { } // TriggerUpdate queues the update task to execute ASAP. -func TriggerUpdate(force bool) error { +func TriggerUpdate(forceIndexCheck, downloadAll bool) error { switch { case !module.Online(): updateASAP = true - case !force && !enableSoftwareUpdates() && !enableIntelUpdates(): + case !forceIndexCheck && !enableSoftwareUpdates() && !enableIntelUpdates(): return fmt.Errorf("automatic updating is disabled") default: - if force { - forceUpdate.Set() + if forceIndexCheck { + forceCheck.Set() + } + if downloadAll { + forceDownload.Set() } updateTask.StartASAP() } @@ -263,8 +266,12 @@ func checkForUpdates(ctx context.Context) (err error) { } }() - forcedUpdate := forceUpdate.SetToIf(true, false) - if !forcedUpdate && !enableSoftwareUpdates() && !enableIntelUpdates() { + // Get flags. + forceIndexCheck := forceCheck.SetToIf(true, false) + downloadAll := forceDownload.SetToIf(true, false) + + // Check again if downloading updates is enabled, or forced. + if !forceIndexCheck && !enableSoftwareUpdates() && !enableIntelUpdates() { log.Warningf("updates: automatic updates are disabled") return nil } @@ -273,13 +280,13 @@ func checkForUpdates(ctx context.Context) (err error) { // Resolve any error and send success notification. if err == nil { log.Infof("updates: successfully checked for updates") - notifyUpdateSuccess(forcedUpdate) + notifyUpdateSuccess(forceIndexCheck) return } // Log and notify error. log.Errorf("updates: check failed: %s", err) - notifyUpdateCheckFailed(forcedUpdate, err) + notifyUpdateCheckFailed(forceIndexCheck, err) }() if err = registry.UpdateIndexes(ctx); err != nil { @@ -287,7 +294,7 @@ func checkForUpdates(ctx context.Context) (err error) { return //nolint:nakedret // TODO: Would "return err" work with the defer? } - err = registry.DownloadUpdates(ctx, forcedUpdate) + err = registry.DownloadUpdates(ctx, downloadAll) if err != nil { err = fmt.Errorf("failed to download updates: %w", err) return //nolint:nakedret // TODO: Would "return err" work with the defer? diff --git a/updates/notify.go b/updates/notify.go index fbd425ef8..d1e1622f2 100644 --- a/updates/notify.go +++ b/updates/notify.go @@ -21,7 +21,7 @@ const ( var updateFailedCnt = new(atomic.Int32) -func notifyUpdateSuccess(forced bool) { +func notifyUpdateSuccess(force bool) { updateFailedCnt.Store(0) module.Resolve(updateFailed) updateState := registry.GetState().Updates @@ -35,7 +35,7 @@ func notifyUpdateSuccess(forced bool) { time.Since(*updateState.LastDownloadAt) < 5*time.Second: // Show notification if we downloaded something within the last minute. flavor = updateSuccessDownloaded - case forced: + case force: // Always show notification if update was manually triggered. default: // Otherwise, the update was uneventful. Do not show notification. @@ -48,7 +48,7 @@ func notifyUpdateSuccess(forced bool) { EventID: updateSuccess, Type: notifications.Info, Title: "Portmaster Is Up-To-Date", - Message: "Portmaster successfully checked for updates. Everything is up to date. Most updates are applied automatically. You will be notified of important updates that need restarting.", + Message: "Portmaster successfully checked for updates. Everything is up to date.\n\n" + getUpdatingInfoMsg(), Expires: time.Now().Add(1 * time.Minute).Unix(), AvailableActions: []*notifications.Action{ { @@ -64,7 +64,7 @@ func notifyUpdateSuccess(forced bool) { - %s -Press "Download Now" or check for updates later to download and automatically apply all pending updates. You will be notified of important updates that need restarting.`, +Press "Download Now" to download and automatically apply all pending updates. You will be notified of important updates that need restarting.`, len(updateState.PendingDownload), strings.Join(updateState.PendingDownload, "\n- "), ) @@ -84,7 +84,7 @@ Press "Download Now" or check for updates later to download and automatically ap Text: "Download Now", Type: notifications.ActionTypeWebhook, Payload: ¬ifications.ActionTypeWebhookPayload{ - URL: apiPathCheckForUpdates, + URL: apiPathCheckForUpdates + "?download", ResultAction: "display", }, }, @@ -95,15 +95,14 @@ Press "Download Now" or check for updates later to download and automatically ap msg := fmt.Sprintf( `%d updates were downloaded and applied: -- %s`, +- %s + +%s +`, len(updateState.LastDownload), strings.Join(updateState.LastDownload, "\n- "), + getUpdatingInfoMsg(), ) - if enableSoftwareUpdates() { - msg += "\n\nYou will be notified of important updates that need restarting." - } else { - msg += "\n\nAutomatic software updates are disabled, and you will be notified when a new software update is ready to be downloaded and applied." - } notifications.Notify(¬ifications.Notification{ EventID: updateSuccess, @@ -122,12 +121,23 @@ Press "Download Now" or check for updates later to download and automatically ap } } -func notifyUpdateCheckFailed(forced bool, err error) { +func getUpdatingInfoMsg() string { + switch { + case enableSoftwareUpdates() && enableIntelUpdates(): + return "You will be notified of important updates that need restarting." + case enableIntelUpdates(): + return "Automatic software updates are disabled, but you will be notified when a new software update is ready to be downloaded and applied." + default: + return "Automatic software updates are disabled. Please check for updates regularly yourself." + } +} + +func notifyUpdateCheckFailed(force bool, err error) { failedCnt := updateFailedCnt.Add(1) lastSuccess := registry.GetState().Updates.LastSuccessAt switch { - case forced: + case force: // Always show notification if update was manually triggered. case failedCnt < failedUpdateNotifyCountThreshold: // Not failed often enough for notification.