From 9de79f357fb6b926002ba087d11e8e0527c2de21 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Fri, 14 Nov 2025 16:21:15 +0100 Subject: [PATCH 1/3] add check for major relese update --- cmd/arduino-app-cli/system/system.go | 6 ++-- internal/api/api.go | 4 +-- internal/api/handlers/update.go | 9 ++--- internal/orchestrator/config/config.go | 45 +++++++++++++++---------- internal/update/apt/service.go | 3 +- internal/update/arduino/arduino.go | 46 +++++++++++++++++++++++--- internal/update/update.go | 10 +++--- 7 files changed, 87 insertions(+), 36 deletions(-) diff --git a/cmd/arduino-app-cli/system/system.go b/cmd/arduino-app-cli/system/system.go index 54c89128..c0e79781 100644 --- a/cmd/arduino-app-cli/system/system.go +++ b/cmd/arduino-app-cli/system/system.go @@ -42,7 +42,7 @@ func NewSystemCmd(cfg config.Configuration) *cobra.Command { } cmd.AddCommand(newDownloadImageCmd(cfg)) - cmd.AddCommand(newUpdateCmd()) + cmd.AddCommand(newUpdateCmd(cfg)) cmd.AddCommand(newCleanUpCmd(cfg, servicelocator.GetDockerClient())) cmd.AddCommand(newNetworkModeCmd()) cmd.AddCommand(newKeyboardSetCmd()) @@ -64,7 +64,7 @@ func newDownloadImageCmd(cfg config.Configuration) *cobra.Command { return cmd } -func newUpdateCmd() *cobra.Command { +func newUpdateCmd(cfg config.Configuration) *cobra.Command { var onlyArduino bool var forceYes bool cmd := &cobra.Command{ @@ -76,7 +76,7 @@ func newUpdateCmd() *cobra.Command { updater := getUpdater() - pkgs, err := updater.ListUpgradablePackages(cmd.Context(), filterFunc) + pkgs, err := updater.ListUpgradablePackages(cfg, cmd.Context(), filterFunc) if err != nil { return err } diff --git a/internal/api/api.go b/internal/api/api.go index 08d31d84..4aed3349 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -63,9 +63,9 @@ func NewHTTPRouter( mux.Handle("PUT /v1/properties/{key}", handlers.HandlePropertyUpsert(cfg)) mux.Handle("DELETE /v1/properties/{key}", handlers.HandlePropertyDelete(cfg)) - mux.Handle("GET /v1/system/update/check", handlers.HandleCheckUpgradable(updater)) + mux.Handle("GET /v1/system/update/check", handlers.HandleCheckUpgradable(cfg, updater)) mux.Handle("GET /v1/system/update/events", handlers.HandleUpdateEvents(updater)) - mux.Handle("PUT /v1/system/update/apply", handlers.HandleUpdateApply(updater)) + mux.Handle("PUT /v1/system/update/apply", handlers.HandleUpdateApply(cfg, updater)) mux.Handle("GET /v1/system/resources", handlers.HandleSystemResources()) mux.Handle("GET /v1/models", handlers.HandleModelsList(modelsIndex)) diff --git a/internal/api/handlers/update.go b/internal/api/handlers/update.go index 41ac992b..fd17be99 100644 --- a/internal/api/handlers/update.go +++ b/internal/api/handlers/update.go @@ -23,11 +23,12 @@ import ( "log/slog" "github.com/arduino/arduino-app-cli/internal/api/models" + "github.com/arduino/arduino-app-cli/internal/orchestrator/config" "github.com/arduino/arduino-app-cli/internal/render" "github.com/arduino/arduino-app-cli/internal/update" ) -func HandleCheckUpgradable(updater *update.Manager) http.HandlerFunc { +func HandleCheckUpgradable(cfg config.Configuration, updater *update.Manager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { queryParams := r.URL.Query() @@ -41,7 +42,7 @@ func HandleCheckUpgradable(updater *update.Manager) http.HandlerFunc { filterFunc = update.MatchArduinoPackage } - pkgs, err := updater.ListUpgradablePackages(r.Context(), filterFunc) + pkgs, err := updater.ListUpgradablePackages(cfg, r.Context(), filterFunc) if err != nil { if errors.Is(err, update.ErrOperationAlreadyInProgress) { render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()}) @@ -64,7 +65,7 @@ type UpdateCheckResult struct { Packages []update.UpgradablePackage `json:"updates"` } -func HandleUpdateApply(updater *update.Manager) http.HandlerFunc { +func HandleUpdateApply(cfg config.Configuration, updater *update.Manager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { queryParams := r.URL.Query() onlyArduinoPackages := false @@ -77,7 +78,7 @@ func HandleUpdateApply(updater *update.Manager) http.HandlerFunc { filterFunc = update.MatchArduinoPackage } - pkgs, err := updater.ListUpgradablePackages(r.Context(), filterFunc) + pkgs, err := updater.ListUpgradablePackages(cfg, r.Context(), filterFunc) if err != nil { if errors.Is(err, update.ErrOperationAlreadyInProgress) { render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()}) diff --git a/internal/orchestrator/config/config.go b/internal/orchestrator/config/config.go index 0838f29d..ff278ade 100644 --- a/internal/orchestrator/config/config.go +++ b/internal/orchestrator/config/config.go @@ -31,15 +31,16 @@ import ( var runnerVersion = "0.5.0" type Configuration struct { - appsDir *paths.Path - dataDir *paths.Path - routerSocketPath *paths.Path - customEIModelsDir *paths.Path - PythonImage string - UsedPythonImageTag string - RunnerVersion string - AllowRoot bool - LibrariesAPIURL *url.URL + appsDir *paths.Path + dataDir *paths.Path + routerSocketPath *paths.Path + customEIModelsDir *paths.Path + PythonImage string + UsedPythonImageTag string + RunnerVersion string + AllowRoot bool + LibrariesAPIURL *url.URL + MaxAllowedMajorVersion int } func NewFromEnv() (Configuration, error) { @@ -105,17 +106,25 @@ func NewFromEnv() (Configuration, error) { if err != nil { return Configuration{}, fmt.Errorf("invalid LIBRARIES_API_URL: %w", err) } + maxVersionStr := os.Getenv("ARDUINO_APP_CLI__MAX_UPDATE_MAJOR_VERSION") + + maxVersion, err := strconv.Atoi(maxVersionStr) + if err != nil || maxVersion <= 0 { + maxVersion = 1 + } + slog.Debug("Using max update major version", slog.Int("version", maxVersion)) c := Configuration{ - appsDir: appsDir, - dataDir: dataDir, - routerSocketPath: routerSocket, - customEIModelsDir: customEIModelsDir, - PythonImage: pythonImage, - UsedPythonImageTag: usedPythonImageTag, - RunnerVersion: runnerVersion, - AllowRoot: allowRoot, - LibrariesAPIURL: parsedLibrariesURL, + appsDir: appsDir, + dataDir: dataDir, + routerSocketPath: routerSocket, + customEIModelsDir: customEIModelsDir, + PythonImage: pythonImage, + UsedPythonImageTag: usedPythonImageTag, + RunnerVersion: runnerVersion, + AllowRoot: allowRoot, + LibrariesAPIURL: parsedLibrariesURL, + MaxAllowedMajorVersion: maxVersion, } if err := c.init(); err != nil { return Configuration{}, err diff --git a/internal/update/apt/service.go b/internal/update/apt/service.go index f3d3984e..e937d7b8 100644 --- a/internal/update/apt/service.go +++ b/internal/update/apt/service.go @@ -31,6 +31,7 @@ import ( "go.bug.st/f" "github.com/arduino/arduino-app-cli/internal/orchestrator" + "github.com/arduino/arduino-app-cli/internal/orchestrator/config" "github.com/arduino/arduino-app-cli/internal/update" ) @@ -48,7 +49,7 @@ func New() *Service { // It runs the `apt-get update` command before listing the packages to ensure the package list is up to date. // It filters the packages using the provided matcher function. // It returns a slice of UpgradablePackage or an error if the command fails. -func (s *Service) ListUpgradablePackages(ctx context.Context, matcher func(update.UpgradablePackage) bool) ([]update.UpgradablePackage, error) { +func (s *Service) ListUpgradablePackages(cfg config.Configuration, ctx context.Context, matcher func(update.UpgradablePackage) bool) ([]update.UpgradablePackage, error) { if !s.lock.TryLock() { return nil, update.ErrOperationAlreadyInProgress } diff --git a/internal/update/arduino/arduino.go b/internal/update/arduino/arduino.go index 01076dff..4c32bf2b 100644 --- a/internal/update/arduino/arduino.go +++ b/internal/update/arduino/arduino.go @@ -22,6 +22,7 @@ import ( "sync" "time" + "github.com/Masterminds/semver/v3" "github.com/arduino/arduino-cli/commands" "github.com/arduino/arduino-cli/commands/cmderrors" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -29,6 +30,7 @@ import ( "github.com/arduino/arduino-app-cli/internal/helpers" "github.com/arduino/arduino-app-cli/internal/orchestrator" + "github.com/arduino/arduino-app-cli/internal/orchestrator/config" "github.com/arduino/arduino-app-cli/internal/update" ) @@ -53,7 +55,7 @@ func setConfig(ctx context.Context, srv rpc.ArduinoCoreServiceServer) error { } // ListUpgradablePackages implements ServiceUpdater. -func (a *ArduinoPlatformUpdater) ListUpgradablePackages(ctx context.Context, _ func(update.UpgradablePackage) bool) ([]update.UpgradablePackage, error) { +func (a *ArduinoPlatformUpdater) ListUpgradablePackages(cfg config.Configuration, ctx context.Context, _ func(update.UpgradablePackage) bool) ([]update.UpgradablePackage, error) { if !a.lock.TryLock() { return nil, update.ErrOperationAlreadyInProgress } @@ -112,15 +114,51 @@ func (a *ArduinoPlatformUpdater) ListUpgradablePackages(ctx context.Context, _ f return nil, nil // No platform found } - if platformSummary.GetLatestVersion() == platformSummary.GetInstalledVersion() { - return nil, nil // No update available + installedVersionString := platformSummary.GetInstalledVersion() + + installedV, err := semver.NewVersion(installedVersionString) + if err != nil { + slog.Warn("Failed to parse installed version", "version", installedVersionString, "error", err) + return nil, nil + } + + var maxMajor uint64 + if cfg.MaxAllowedMajorVersion > 0 { + maxMajor = uint64(cfg.MaxAllowedMajorVersion) } + var bestUpdateV *semver.Version + + allReleases := platformSummary.GetReleases() + for versionString := range allReleases { + candidateV, err := semver.NewVersion(versionString) + if err != nil { + slog.Debug("Skipping unparsable version", "version", versionString, "error", err) + continue + } + if candidateV.Major() > maxMajor { + continue + } + + if !candidateV.GreaterThan(installedV) { + continue + } + + if bestUpdateV == nil || candidateV.GreaterThan(bestUpdateV) { + bestUpdateV = candidateV + } + } + if bestUpdateV == nil { + slog.Debug("No suitable updates found within major version constraint") + return nil, nil + } + slog.Debug(" bestUpdateV.Original()", bestUpdateV.Original(), "") + slog.Debug(" bestUpdateV.String()", bestUpdateV.String(), "") return []update.UpgradablePackage{{ Type: update.Arduino, Name: "arduino:zephyr", FromVersion: platformSummary.GetInstalledVersion(), - ToVersion: platformSummary.GetLatestVersion(), + ToVersion: bestUpdateV.Original(), }}, nil } diff --git a/internal/update/update.go b/internal/update/update.go index 7a254478..b906e68b 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -26,6 +26,8 @@ import ( "time" "golang.org/x/sync/errgroup" + + "github.com/arduino/arduino-app-cli/internal/orchestrator/config" ) var ErrOperationAlreadyInProgress = errors.New("an operation is already in progress") @@ -48,7 +50,7 @@ type UpgradablePackage struct { } type ServiceUpdater interface { - ListUpgradablePackages(ctx context.Context, matcher func(UpgradablePackage) bool) ([]UpgradablePackage, error) + ListUpgradablePackages(cfg config.Configuration, ctx context.Context, matcher func(UpgradablePackage) bool) ([]UpgradablePackage, error) UpgradePackages(ctx context.Context, names []string) (<-chan Event, error) } @@ -69,7 +71,7 @@ func NewManager(debUpdateService ServiceUpdater, arduinoPlatformUpdateService Se } } -func (m *Manager) ListUpgradablePackages(ctx context.Context, matcher func(UpgradablePackage) bool) ([]UpgradablePackage, error) { +func (m *Manager) ListUpgradablePackages(cfg config.Configuration, ctx context.Context, matcher func(UpgradablePackage) bool) ([]UpgradablePackage, error) { if !m.lock.TryLock() { return nil, ErrOperationAlreadyInProgress } @@ -89,7 +91,7 @@ func (m *Manager) ListUpgradablePackages(ctx context.Context, matcher func(Upgra ) g.Go(func() error { - pkgs, err := m.debUpdateService.ListUpgradablePackages(ctx, matcher) + pkgs, err := m.debUpdateService.ListUpgradablePackages(cfg, ctx, matcher) if err != nil { return err } @@ -98,7 +100,7 @@ func (m *Manager) ListUpgradablePackages(ctx context.Context, matcher func(Upgra }) g.Go(func() error { - pkgs, err := m.arduinoPlatformUpdateService.ListUpgradablePackages(ctx, matcher) + pkgs, err := m.arduinoPlatformUpdateService.ListUpgradablePackages(cfg, ctx, matcher) if err != nil { return err } From 25d8cfc4b9ea0e33936e870d5543ac0e823cf764 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Wed, 10 Dec 2025 16:46:35 +0100 Subject: [PATCH 2/3] unset version in case of errors --- internal/orchestrator/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/orchestrator/config/config.go b/internal/orchestrator/config/config.go index ff278ade..0e0ea8c3 100644 --- a/internal/orchestrator/config/config.go +++ b/internal/orchestrator/config/config.go @@ -110,7 +110,7 @@ func NewFromEnv() (Configuration, error) { maxVersion, err := strconv.Atoi(maxVersionStr) if err != nil || maxVersion <= 0 { - maxVersion = 1 + maxVersion = 0 } slog.Debug("Using max update major version", slog.Int("version", maxVersion)) From 24a2abe644bf40e40c81506e30381f4bc059bcce Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Wed, 10 Dec 2025 17:17:20 +0100 Subject: [PATCH 3/3] refactoring and testing --- internal/update/arduino/arduino.go | 55 +++++++------ internal/update/arduino/arduino_test.go | 101 ++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 22 deletions(-) create mode 100644 internal/update/arduino/arduino_test.go diff --git a/internal/update/arduino/arduino.go b/internal/update/arduino/arduino.go index 4c32bf2b..6034d230 100644 --- a/internal/update/arduino/arduino.go +++ b/internal/update/arduino/arduino.go @@ -113,26 +113,45 @@ func (a *ArduinoPlatformUpdater) ListUpgradablePackages(cfg config.Configuration if platformSummary == nil { return nil, nil // No platform found } + releasesMap := platformSummary.GetReleases() - installedVersionString := platformSummary.GetInstalledVersion() + releases := make([]string, 0, len(releasesMap)) - installedV, err := semver.NewVersion(installedVersionString) - if err != nil { - slog.Warn("Failed to parse installed version", "version", installedVersionString, "error", err) + for k := range releasesMap { + releases = append(releases, k) + } + bestVersion, err := findBestCandidate( + platformSummary.GetInstalledVersion(), + releases, + cfg.MaxAllowedMajorVersion, + ) + + if bestVersion == "" || err != nil { return nil, nil } + return []update.UpgradablePackage{{ + Type: update.Arduino, + Name: "arduino:zephyr", + FromVersion: platformSummary.GetInstalledVersion(), + ToVersion: bestVersion, + }}, nil +} +func findBestCandidate(installedStr string, availableVersions []string, maxMajorConfig int) (string, error) { + installedV, err := semver.NewVersion(installedStr) + if err != nil { + return "", err + } - var maxMajor uint64 - if cfg.MaxAllowedMajorVersion > 0 { - maxMajor = uint64(cfg.MaxAllowedMajorVersion) + maxMajor := uint64(maxMajorConfig) + if maxMajorConfig <= 0 { + maxMajor = installedV.Major() } + var bestUpdateV *semver.Version - allReleases := platformSummary.GetReleases() - for versionString := range allReleases { - candidateV, err := semver.NewVersion(versionString) + for _, vStr := range availableVersions { + candidateV, err := semver.NewVersion(vStr) if err != nil { - slog.Debug("Skipping unparsable version", "version", versionString, "error", err) continue } @@ -143,23 +162,15 @@ func (a *ArduinoPlatformUpdater) ListUpgradablePackages(cfg config.Configuration if !candidateV.GreaterThan(installedV) { continue } - if bestUpdateV == nil || candidateV.GreaterThan(bestUpdateV) { bestUpdateV = candidateV } } + if bestUpdateV == nil { - slog.Debug("No suitable updates found within major version constraint") - return nil, nil + return "", nil } - slog.Debug(" bestUpdateV.Original()", bestUpdateV.Original(), "") - slog.Debug(" bestUpdateV.String()", bestUpdateV.String(), "") - return []update.UpgradablePackage{{ - Type: update.Arduino, - Name: "arduino:zephyr", - FromVersion: platformSummary.GetInstalledVersion(), - ToVersion: bestUpdateV.Original(), - }}, nil + return bestUpdateV.Original(), nil } // UpgradePackages implements ServiceUpdater. diff --git a/internal/update/arduino/arduino_test.go b/internal/update/arduino/arduino_test.go new file mode 100644 index 00000000..986b7465 --- /dev/null +++ b/internal/update/arduino/arduino_test.go @@ -0,0 +1,101 @@ +package arduino + +import "testing" + +func TestFindBestCandidate(t *testing.T) { + tests := []struct { + name string + installed string + available []string + maxMajorConfig int + expectedVersion string + expectError bool + }{ + { + name: "Standard update: minor upgrade available", + installed: "1.0.0", + available: []string{"1.0.1", "1.1.0"}, + maxMajorConfig: 0, + expectedVersion: "1.1.0", + expectError: false, + }, + { + name: "Major update blocked by default (Config=0)", + installed: "1.9.9", + available: []string{"2.0.0", "1.9.10"}, + maxMajorConfig: 0, + expectedVersion: "1.9.10", + expectError: false, + }, + { + name: "Major update allowed by explicit config", + installed: "1.9.9", + available: []string{"2.0.0", "3.0.0"}, + maxMajorConfig: 2, + expectedVersion: "2.0.0", + expectError: false, + }, + { + name: "CRITICAL: Regression test for 'Zero Value' bug (Version 2+)", + installed: "2.1.0", + available: []string{"2.2.0", "3.0.0"}, + maxMajorConfig: 0, + expectedVersion: "2.2.0", + expectError: false, + }, + { + name: "No updates available (all older or same)", + installed: "1.5.0", + available: []string{"1.0.0", "1.5.0"}, + maxMajorConfig: 0, + expectedVersion: "", + expectError: false, + }, + { + name: "Handle unsorted list and pick highest valid", + installed: "1.0.0", + available: []string{"1.1.0", "1.5.0", "1.2.0"}, + maxMajorConfig: 0, + expectedVersion: "1.5.0", + expectError: false, + }, + { + name: "Skip invalid candidate strings", + installed: "1.0.0", + available: []string{"invalid-ver", "1.1.0"}, + maxMajorConfig: 0, + expectedVersion: "1.1.0", + expectError: false, + }, + { + name: "Error on invalid installed version string", + installed: "not-a-semver", + available: []string{"1.0.0"}, + maxMajorConfig: 0, + expectedVersion: "", + expectError: true, + }, + { + name: "Prerelease handling (standard logic ignores prereleases unless specifically handled)", + installed: "1.0.0", + available: []string{"1.0.1-beta"}, + maxMajorConfig: 0, + expectedVersion: "1.0.1-beta", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := findBestCandidate(tt.installed, tt.available, tt.maxMajorConfig) + + if (err != nil) != tt.expectError { + t.Errorf("findBestCandidate() error = %v, expectError %v", err, tt.expectError) + return + } + if got != tt.expectedVersion { + t.Errorf("findBestCandidate() = %v, want %v", got, tt.expectedVersion) + } + }) + } +}