From c3566febd35fcf3a187d0a30492b119ac4f9d7df Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 4 Nov 2026 07:26:08 +0000 Subject: [PATCH] Report every add-on update in one check and say why an old package will not start Checking several add-ons of one library in a row reached the registry only for the first, because the library interval also throttled the per-add-on tag resolution. The others answered unknown at once, so a check-all reported one update and the next pass the next one. The library interval now spares only the library fetch; each add-on is throttled by its own interval. Turning on an installed package whose manifest this host refuses now fails with the typed installed_incompatible cause and a message that says to upgrade, instead of the generic owner-unavailable 503. The upgrade of such a package stays allowed. Co-Authored-By: Claude Sonnet 5.5 --- core/http/endpoints/addons/gateway.go | 2 + core/services/addons/gateway/carrier.go | 4 +- core/services/addons/manager.go | 11 ++++ core/services/addons/updates/checker.go | 18 ++++-- core/services/addons/updates/checker_test.go | 37 +++++++++++ core/services/addons/updates_test.go | 65 ++++++++++++++++++++ docs/content/features/addons.md | 3 + scripts/addons/suites.json | 3 + 8 files changed, 135 insertions(+), 8 deletions(-) diff --git a/core/http/endpoints/addons/gateway.go b/core/http/endpoints/addons/gateway.go index fa0d4413f3..10052384cc 100644 --- a/core/http/endpoints/addons/gateway.go +++ b/core/http/endpoints/addons/gateway.go @@ -213,6 +213,8 @@ func operationCauseMessage(code string) string { return "The uploaded file is gone: it expired, or another user uploaded it. Upload it again. Nothing was installed" case managerpkg.CauseVersionNotNewer: return "This version of the add-on is installed already. Nothing was installed" + case managerpkg.CauseInstalledIncompatible: + return "This version of the add-on is not compatible with this server, so it cannot be turned on. Upgrade it to a newer version, then turn it on. Nothing was changed" case managerpkg.CauseDowngradeRefused: return "The file holds an older version than the installed one. Confirm the downgrade to install it. Nothing was installed" } diff --git a/core/services/addons/gateway/carrier.go b/core/services/addons/gateway/carrier.go index e937523a5c..4da40b29e6 100644 --- a/core/services/addons/gateway/carrier.go +++ b/core/services/addons/gateway/carrier.go @@ -642,7 +642,7 @@ func operationCause(e error) string { func IsOperationCause(code string) bool { switch code { case addons.CauseLibraryUnavailable, addons.CauseLibraryConflict, addons.CauseReleaseNotFound, addons.CausePackageFetch, addons.CauseInstallStorage, - addons.CauseFileMissing, addons.CauseVersionNotNewer, addons.CauseDowngradeRefused: + addons.CauseFileMissing, addons.CauseVersionNotNewer, addons.CauseDowngradeRefused, addons.CauseInstalledIncompatible: return true } return false @@ -825,7 +825,7 @@ func (c *Carrier) send(ctx context.Context, a contract.Actor, wire wireRequest) case wirePackage: return nil, fmt.Errorf("%w: %w", addons.ErrOperationFailed, packages.ErrPackage) case addons.CauseLibraryUnavailable, addons.CauseLibraryConflict, addons.CauseReleaseNotFound, addons.CausePackageFetch, addons.CauseInstallStorage, - addons.CauseFileMissing, addons.CauseVersionNotNewer, addons.CauseDowngradeRefused: + addons.CauseFileMissing, addons.CauseVersionNotNewer, addons.CauseDowngradeRefused, addons.CauseInstalledIncompatible: return nil, fmt.Errorf("%w: %w", addons.ErrOperationFailed, &addons.FailureCause{Code: b.Type}) case wireNotPermitted: return nil, ErrNotPermitted diff --git a/core/services/addons/manager.go b/core/services/addons/manager.go index 26cb1bb82c..10be4ade1d 100644 --- a/core/services/addons/manager.go +++ b/core/services/addons/manager.go @@ -85,6 +85,10 @@ const ( // CauseDowngradeRefused: the file holds an older version and the request // did not confirm the downgrade. CauseDowngradeRefused = "downgrade_refused" + // CauseInstalledIncompatible: the installed package was built for an API, + // protocol or data format this host no longer runs. It cannot be turned + // on; an upgrade to a compatible release can. + CauseInstalledIncompatible = "installed_incompatible" ) // FailureCause is the typed cause of a failed operation. Its text is fixed and @@ -776,6 +780,13 @@ func (m *Manager) submit(ctx context.Context, actor contract.Actor, cmd Lifecycl if m.dataBlocked(cmd.AddonID) { return fail() } + // A package the host cannot run would only fail to start and leave + // the add-on off with no reason. Say so, and keep the upgrade open. + if r, ok := currentReceipt(m.journal.Addons[cmd.AddonID]); ok && m.installer != nil { + if contract.ValidateManifest(r.Manifest, m.installer.Host) != nil { + return fail(&FailureCause{Code: CauseInstalledIncompatible}) + } + } next.DesiredEnabled = true case "disable": next.DesiredEnabled = false diff --git a/core/services/addons/updates/checker.go b/core/services/addons/updates/checker.go index 5475e9a9c8..4375ac8639 100644 --- a/core/services/addons/updates/checker.go +++ b/core/services/addons/updates/checker.go @@ -308,23 +308,29 @@ func (c *Checker) evaluateAndStore(ctx context.Context, in Installation, refresh c.mu.Lock() recentAddon := now.Sub(c.lastAddr[in.AddonID]) < c.cfg.MinRefresh && !c.lastAddr[in.AddonID].IsZero() recentLib := now.Sub(c.lastLib[src]) < c.cfg.MinRefresh && !c.lastLib[src].IsZero() - if recentAddon || recentLib { - throttled = true - } else { + // The interval of the library only spares its fetch. Add-ons that + // share a library are checked in a row, and each of them still has + // its own check to run (the tag of a development build is resolved + // per add-on): only the add-on's own interval throttles its answer. + throttled = recentAddon + if !recentAddon { c.lastAddr[in.AddonID] = now - c.lastLib[src] = now c.prune(c.lastAddr) + } + fetch := !recentAddon && !recentLib + if fetch { + c.lastLib[src] = now c.prune(c.lastLib) } c.mu.Unlock() - if !throttled && src != "" { + if fetch && src != "" { // A failed fetch is not fatal: the evaluation below reads the // catalog's own state and reports the failure as a status. _ = c.cat.Refresh(ctx, src) } } // The tag of a development build is resolved by an explicit check that - // reached the library; a throttled one reuses the last resolution. + // reached the library or was the add-on's own turn; a throttled one reuses the last resolution. res := c.evaluate(ctx, in, refresh && !throttled) st := res.Status st.Throttled = throttled diff --git a/core/services/addons/updates/checker_test.go b/core/services/addons/updates/checker_test.go index fea0e41b69..df0105b085 100644 --- a/core/services/addons/updates/checker_test.go +++ b/core/services/addons/updates/checker_test.go @@ -508,6 +508,43 @@ var _ = Describe("Update detection", func() { Expect(r.lib.refreshes.Load()).To(Equal(int32(2))) Expect(st.Throttled).To(BeFalse()) }) + It("reports every add-on of one library that has an update when they are checked in a row", func() { + // Several development add-ons share one library. The first check + // fetches the library; the others must still resolve their own + // tag, or only the first one would ever report an update. + ids := []string{"alpha", "beta", "gamma"} + dev := func(id string) catalog.Release { + r := rel("0.0.0-development", "development", "") + r.ID, r.Name, r.Mutable = id, id, true + r.Artifact = "oci://registry.example/addons:" + id + "-development" + return r + } + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + lib := &library{now: func() time.Time { return now }, state: catalog.RefreshState{CheckedAt: now, LastSuccess: now}, hasState: true} + inst := &installs{in: map[string]updates.Installation{}} + for _, id := range ids { + lib.cached = append(lib.cached, dev(id)) + in := installed("0.0.0-dev.2cadf319a0e1", "0.0.0-development", "development", digestA) + in.AddonID = id + inst.in[id] = in + } + lib.remote = lib.cached + checker := updates.New(updates.Config{Host: host, TTL: time.Hour, MaxEntries: 8, MinRefresh: time.Minute, Now: func() time.Time { return now }, + Lookup: func(context.Context, string, string) (string, error) { return digestB, nil }}, lib, inst) + for _, id := range ids { + st, e := checker.Check(context.Background(), id, true) + Expect(e).NotTo(HaveOccurred()) + Expect(st.State).To(Equal(updates.UpdateAvailable), id) + Expect(st.CandidateDigest).To(Equal(digestB), id) + } + Expect(lib.refreshes.Load()).To(Equal(int32(1)), "the library is still fetched once") + // A status read afterwards answers the same for all of them. + for _, id := range ids { + st, e := checker.Check(context.Background(), id, false) + Expect(e).NotTo(HaveOccurred()) + Expect(st.State).To(Equal(updates.UpdateAvailable), id) + } + }) It("shares one check among concurrent callers", func() { r := newRig(installed("1.0.0", "development", "development", digestA), rel("1.0.0", "development", digestA)) r.lib.delay = make(chan struct{}) diff --git a/core/services/addons/updates_test.go b/core/services/addons/updates_test.go index 7cee601a63..36a9278a76 100644 --- a/core/services/addons/updates_test.go +++ b/core/services/addons/updates_test.go @@ -7,6 +7,7 @@ import ( "context" "crypto/sha256" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -531,6 +532,70 @@ var _ = Describe("explicit upgrade from an authoritative update check", func() { Expect(installed[0].Update.State).To(Equal("check-failed")) }) + It("reports the pending update of an installed add-on that is turned off, and keeps the desired state through an upgrade", func() { + r := newUpdateRig(true) + installed := r.installChannel().Generations[0].Receipt + // Never enabled: the owner's desired state is off. + next := r.lib.publish("1.2.3-dev", "development", "build-2", catalog.Requirements{}) + st, e := r.m.CheckUpdates(ctx, actor, "sample") + Expect(e).NotTo(HaveOccurred()) + Expect(st.State).To(Equal(updates.UpdateAvailable), "a disabled add-on is still checked") + list, e := r.m.Apps(ctx, actor) + Expect(e).NotTo(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Enabled).To(BeFalse()) + Expect(list[0].Update).To(Equal("update-pending")) + inst, e := r.m.Installed(ctx, actor) + Expect(e).NotTo(HaveOccurred()) + Expect(inst[0].Update).NotTo(BeNil()) + Expect(inst[0].Update.Available).To(BeTrue()) + // Upgrading it does not turn it on. + _, e = r.m.Submit(ctx, actor, upgrade("up-off", installed.Digest, next.Digest)) + Expect(e).NotTo(HaveOccurred()) + after, _ := r.m.Status(ctx, "sample") + Expect(after.DesiredEnabled).To(BeFalse()) + // Turning it on is durable, and a later upgrade keeps it on. + _, e = r.m.Submit(ctx, actor, LifecycleCommand{OperationID: "enable-1", AddonID: "sample", Action: "enable", Generation: after.Generation}) + Expect(e).NotTo(HaveOccurred()) + again := r.lib.publish("1.2.3-dev", "development", "build-3", catalog.Requirements{}) + r.tick(time.Minute) + st, _ = r.m.CheckUpdates(ctx, actor, "sample") + Expect(st.State).To(Equal(updates.UpdateAvailable)) + _, e = r.m.Submit(ctx, actor, upgrade("up-on", next.Digest, again.Digest)) + Expect(e).NotTo(HaveOccurred()) + final, _ := r.m.Status(ctx, "sample") + Expect(final.DesiredEnabled).To(BeTrue(), "an upgrade must not flip the desired state") + }) + + It("refuses to turn on an installed package the host cannot run with a typed cause, and an upgrade makes it startable", func() { + r := newUpdateRig(true) + installed := r.installChannel().Generations[0].Receipt + // The installed package was built for an API range this host refuses. + r.m.mu.Lock() + st := r.m.journal.Addons["sample"] + for i := range st.Generations { + st.Generations[i].Receipt.Manifest.API = contract.VersionRange{Min: 1, Max: 1} + } + r.m.journal.Addons["sample"] = st + r.m.mu.Unlock() + _, e := r.m.Submit(ctx, actor, LifecycleCommand{OperationID: "enable-old", AddonID: "sample", Action: "enable", Generation: 1}) + Expect(e).To(HaveOccurred()) + var cause *FailureCause + Expect(errors.As(e, &cause)).To(BeTrue(), "the refusal is typed, not a generic outage") + Expect(cause.Code).To(Equal(CauseInstalledIncompatible)) + after, _ := r.m.Status(ctx, "sample") + Expect(after.DesiredEnabled).To(BeFalse()) + // The check still runs for it and offers the upgrade. + next := r.lib.publish("1.2.3-dev", "development", "build-2", catalog.Requirements{}) + check, e := r.m.CheckUpdates(ctx, actor, "sample") + Expect(e).NotTo(HaveOccurred()) + Expect(check.State).To(Equal(updates.UpdateAvailable)) + _, e = r.m.Submit(ctx, actor, upgrade("up-old", installed.Digest, next.Digest)) + Expect(e).NotTo(HaveOccurred(), "an incompatible installed package can be upgraded") + _, e = r.m.Submit(ctx, actor, LifecycleCommand{OperationID: "enable-new", AddonID: "sample", Action: "enable", Generation: 2}) + Expect(e).NotTo(HaveOccurred()) + }) + It("does not accept pins without an update checker or on another action", func() { r := newUpdateRig(false) s := r.installChannel() diff --git a/docs/content/features/addons.md b/docs/content/features/addons.md index 1fc9f7cd0d..5ad01e9d0d 100644 --- a/docs/content/features/addons.md +++ b/docs/content/features/addons.md @@ -4731,6 +4731,9 @@ operations. **Check for updates** in the bar of the list reads the library of every installed add-on again; it applies nothing, and an add-on that cannot be checked does not stop the others. +Add-ons that share a library are all checked, whether they are turned on or off, and every +one with a newer build is listed together. An installed version that this server can no longer +run cannot be turned on: the server says so and you upgrade it first. An upgrade started from the list, the details, **Find add-ons** or **Upgrade all** keeps running when you leave the page. It appears in the Activity page under the **Add-ons** diff --git a/scripts/addons/suites.json b/scripts/addons/suites.json index bdce7750cd..80d62a001a 100644 --- a/scripts/addons/suites.json +++ b/scripts/addons/suites.json @@ -4623,6 +4623,7 @@ "go: marks a cached answer older than the time to live as stale", "go: refreshes the library only on an explicit refresh and applies nothing", "go: limits explicit refreshes to one per interval and says so", + "go: reports every add-on of one library that has an update when they are checked in a row", "go: shares one check among concurrent callers", "go: never lets an explicit refresh join a plain evaluation that is already in flight", "go: keeps the work of a canceled caller from spoiling the answer of the others", @@ -4642,6 +4643,8 @@ "go: keeps the last good generation when the candidate cannot be staged", "go: never switches the library or channel, and a failed check is never up to date", "go: lets an administrator refresh and everyone with the feature read a redacted status", + "go: reports the pending update of an installed add-on that is turned off, and keeps the desired state through an upgrade", + "go: refuses to turn on an installed package the host cannot run with a typed cause, and an upgrade makes it startable", "go: shows the pending update in the apps list from the cache only", "go: does not accept pins without an update checker or on another action", "go: keeps the unpinned typed upgrade of an earlier release working",