Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and improve triggering updates from UI #1337

Merged
merged 1 commit into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion netenv/online-status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
28 changes: 23 additions & 5 deletions updates/api.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package updates

import (
"net/http"

"github.com/safing/portbase/api"
)

Expand All @@ -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.",
})
}
5 changes: 3 additions & 2 deletions updates/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ var (
softwareUpdatesCurrentlyEnabled bool
intelUpdatesCurrentlyEnabled bool
previousDevMode bool
forceUpdate = abool.New()
forceCheck = abool.New()
forceDownload = abool.New()
)

func registerConfig() error {
Expand Down Expand Up @@ -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")
Expand Down
25 changes: 16 additions & 9 deletions updates/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
}
Expand All @@ -273,21 +280,21 @@ 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 {
err = fmt.Errorf("failed to update indexes: %w", err)
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?
Expand Down
36 changes: 23 additions & 13 deletions updates/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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{
{
Expand All @@ -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- "),
)
Expand All @@ -84,7 +84,7 @@ Press "Download Now" or check for updates later to download and automatically ap
Text: "Download Now",
Type: notifications.ActionTypeWebhook,
Payload: &notifications.ActionTypeWebhookPayload{
URL: apiPathCheckForUpdates,
URL: apiPathCheckForUpdates + "?download",
ResultAction: "display",
},
},
Expand All @@ -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(&notifications.Notification{
EventID: updateSuccess,
Expand All @@ -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.
Expand Down
Loading