feat(automation): native container auto-update (Watchtower-style) + auto-heal (#3) #19
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/portainer#19
Reference in New Issue
Block a user
Delete Branch "feat/3-auto-update"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Нативный авто-апдейт образов (Watchtower-style) + авто-хил контейнеров для Portainer CE, без сайдкаров
watchtower/autoheal. Closes #3. Бэкенд Go + фронт React, чистый CE (реюз только zlib-кода, EE-проприетарщина иisBE-гейтинг не трогаются). Один PR на весь эпик (5 майлстоунов собраны вfeat/3-auto-updateотdevelop).Новый бэкенд-пакет
api/containerautomation/(scheduler-джобы + lifecycle +Reload()на изменение настроек), глобальные настройкиSettings.ContainerAutomation, реюз существующих CE-кирпичей: движок дайджестовapi/docker/images/{digest,status,registry}.go,api/scheduler,api/stacks/deployments(deployer +RedeployWhenChanged),ContainerService.Recreate.Что вошло (по майлстоунам)
unhealthy-контейнеров, scope labeled/all, меткиio.portainer.autoheal.*(+watchtower/autoheal-алиасы), cooldown + max-retries/окно (защита от restart-storm; retry-state переживает фазуstarting), only-running, overlap-guard. Глоб. панельAutoHealPanel+ read-only строка в деталях контейнера.GET /docker/{id}/containers/{containerId}/image_statusповерхContainerImageStatus(24ч-кэш реально читается на входе, skip digest-pinned/local-only, registry-креды, SSRF). Свой бейджUpdateStatusBadge+ колонка «Update available» (не EE-компонент, без isBE).resolveContainerUpdatePath/applyContainerUpdate/groupContainersForUpdate): standalone→recreate-with-pull, stack-managed→redeploy-with-pull (контейнер НЕ отрывается от стека; неразрешённый стек→throw, не голый recreate), external compose→flag-only. Кнопка в ItemView (видна наoutdated), bulk «Update» с дедупом стеков (один redeploy на стек) и auth-гейтингом stack-redeploy подPortainerStackUpdate.start_periodи толерантностью к транзиентным inspect → при unhealthy re-tag старого образа +Recreate(pull=false); cleanup только после confirmed-healthy; digest-pinned образы гейт пропускают; shutdown не вызывает ложный rollback); per-endpoint enable (Endpoint.ContainerAutomationDisabled, zero-value участвует, без миграции); notifier-seam (лог-реализация, события updated/rollback/update-failed/heal-restarted).Инварианты (проверены ревью)
images.Outdated; на Skipped/Error/Processing/Updated — не трогаем; на провале pull/registry-auth running-контейнер цел.isBEне затронуты; EE-компонентImageStatus(на develop до мержа #1) оставлен как есть.How verified (на собранной ветке
feat/3-auto-update)go build ./api/...— чисто;go vet(containerautomation/settings/datastore/docker/endpoints/cmd) — чисто;go test -race ./api/containerautomation/...+ datastore + settings + docker/containers —ok;gofmt -l— пусто.pnpm typecheck— чисто;pnpm lint— 0 ошибок (176 пред-существующих warnings); vitest по затронутой поверхности (update/badge/queries/ItemView actions+details/AutoHeal+AutoUpdate панели) — 98 тестов в 16 файлах зелёные.Только ручная проверка (нужен живой Docker): реальные рестарт unhealthy, детект против registry, recreate/stack-redeploy «Update now» и авто-апдейт, health-gated rollback, per-endpoint disable, Swarm.
Известные ограничения (документированы в коде)
Review checklist
Add a native, CE-only auto-heal daemon that restarts Docker containers whose healthcheck reports "unhealthy", replacing the willfarrell/autoheal sidecar. Backend: - New package api/containerautomation (service lifecycle + scheduler job, per-endpoint heal pass, label/scope parsing, in-memory cooldown/retry state). - Settings.ContainerAutomation.AutoHeal {Enabled, CheckInterval, Scope} with fresh-install defaults and a 2.43.0 migration backfilling existing installs. - Settings update handler reloads/stops the job via a small Reloader interface (no import cycle); service bootstrapped from main.go after stack schedules. Frontend: - Global AutoHealPanel in SettingsView (enable / interval / scope) via useUpdateSettingsMutation, plus settings TS types. - Read-only per-container Auto-heal row in the container details view (Docker labels are immutable at runtime; opt-in is set via Create/Edit form labels). Tests: Go unit tests for label/scope resolution and the cooldown/retry decision; vitest for the panel and the per-container row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Add native CE detection of "a newer image is available" for running containers, surfaced as a read-only HTTP endpoint and a containers-list badge/column. No applying of updates (M3/M4), no auto-heal (M1). Backend: - New CE handler GET /docker/{id}/containers/{containerId}/image_status backed by the existing zlib/CE digest engine (images.NewClientWithRegistry + ContainerImageStatus). Honors nodeName, authz, and routes registry calls through the credential store / SSRF AllowList. Engine failures degrade to a 200 {Status:"error"} so the UI stays graceful. Response shape: {Status, Message?}. Frontend (CE-only, no isBE gating; the EE ImageStatus component is left untouched): - useContainerImageStatus TanStack Query hook (5min staleTime, no refetch-on-focus; backend caches 24h) calling the non-proxied endpoint. - UpdateStatusBadge component (own assets, neutral on skipped/error). - "Update available" column in the containers datatable; one cached, non-blocking query per visible row. Tests: Go response-shape unit test; vitest for the badge (all statuses) and the hook (url + nodeName query param via msw). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Add a discoverable per-container "Update now" action, shown only when the image status is `outdated`, plus a bulk "Update selected" action in the containers list. Both manual paths share ONE apply primitive (applyContainerUpdate / useUpdateContainerImage) that also backs the future M4 auto-update job: - standalone container -> recreate-with-pull (existing recreate endpoint) - stack-managed -> stack redeploy-with-pull (existing git/file stack update mutations), so the container stays in its stack and is never recreated out-of-band - externally-managed -> refused; the details button is disabled with an compose explanatory tooltip and the bulk action skips it Decision logic lives in the pure, unit-tested resolveContainerUpdatePath / groupContainersForUpdate helpers. The bulk action filters to outdated containers and redeploys each owning stack exactly once even when several of its containers are selected, reporting per-item success/failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Add an optional periodic auto-update daemon that detects outdated container images and applies updates, replacing the containrrr/watchtower sidecar. It extends M1's containerautomation service/scheduler/labels infrastructure and reuses the existing zlib image-detection engine, the standalone Recreate path and the stack deployer. Backend: - api/containerautomation/autoupdate.go: scheduler job iterating Docker (non-edge) endpoints -> in-scope running containers -> ContainerImageStatus; for Outdated: standalone -> ContainerService.Recreate(pull); stack-managed -> one stack redeploy-with-pull per stack per tick (git via RedeployWhenChanged, file via the deployer directly); external compose -> detect only. Monitor-only containers are status-checked (warms the badge cache) but never applied. Overlap guard (atomic), pull/registry-auth failure -> leave running container untouched, conservative cleanup of the dangling old image on the Cleanup flag (non-forced ImageRemove only succeeds when truly unused). - labels.go: update enable / monitor-only labels with watchtower aliases, InUpdateScope, IsMonitorOnly, and pure resolveContainerUpdateRouting / groupContainersForUpdate (Go analogue of M3's TS routing + grouping). - service.go: run both jobs, Reload restarts/stops each per settings; NewService also takes ContainerService, StackDeployer and GitService. - Settings.ContainerAutomation.AutoUpdate {Enabled, PollInterval, Scope, Cleanup} with fresh-install defaults and a 2.43.0 backfill (extends M1's migration; golden test data updated). settings handler validates + reloads. Frontend: - Global AutoUpdatePanel in SettingsView (enable / poll interval / scope / cleanup) via useUpdateSettingsMutation, plus settings TS types. - Read-only per-container Auto-update row in the container details view (Docker labels are immutable at runtime), surfacing monitor-only. Tests: Go unit tests for the update label aliases, scope, monitor-only, the routing decision and the one-redeploy-per-stack grouping; vitest for the panel and the per-container row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Ревью
70f7fe5e— раунд 1 (ПОЛНЫЕ 8 аспектов, отдельный субагент на каждый: security, stability, regressions, test-coverage, conventions, documentation, simplification, architecture).Вердикт: CHANGES. Фича крепкая — security / regressions / architecture чисто, миграция идемпотентна и выключена по умолчанию. Но есть один реальный баг стабильности (бесконечная пере-раскатка при откате), дыра в тестах ядра обновления и набор правок комментариев/конвенций. Отвечайте по id (
fixed:/wontfix:).Что сделать
F1 [stability] Бесконечный цикл update→rollback при стабильно-нездоровом новом образе —
api/containerautomation/autoupdate.go:303-326(+rollback.go:299-337)Когда rollback включён и свежий образ упорно не проходит health-check: апдейт пересоздаёт контейнер новым образом → health-gate проваливается →
rollback()ретегает старый образ обратно наoriginalRefи пересоздаёт, оставляя контейнер на старом (всё ещё устаревшем) образе. На следующем тике опроса (по умолчанию 6 ч) контейнер снова попадает в кандидаты (он по-прежнему outdated — удалённый digest впереди), снова pull сломанного образа → снова rollback. Гварда нет:s.retries/decideRestartиспользуются только в auto-heal (проверено grep'ом — на update-пути их нет). Итог: рабочий контейнер дёргается каждый интервал опроса бесконечно (pull + два пересоздания + ожидание health), пока оператор не выключит auto-update или не починят upstream-образ. Сам откат гарантирует вечный повтор сломанного апдейта.Fix: после успешного отката записывать «провалившуюся» цель (digest целевого образа, не прошедшего health, или
containerID+oldImageID) в per-service map — по аналогии с существующимs.retries/decideRestartв auto-heal — и вupdateEndpoint/updateStandaloneпропускать авто-апдейт для контейнера, чья текущая outdated-цель совпадает с недавно откатанной, пока не пройдёт cooldown или удалённый digest не изменится.F2 [test coverage]
applyContainerUpdateбез юнит-теста —app/react/docker/containers/update/applyContainerUpdate.ts:20-83Единый примитив за кнопкой «Update now» и массовым «Update selected» (и, по его же доке, за авто-апдейтом — «manual and automatic updates behave identically»). Нетривиальная логика без теста: диспатч standalone/stack/external (59-82), защитный throw при отсутствии резолвнутого стека (70-73) и особенно
redeployStackWithPullgit-vs-file (20-42), которая собирает два разных payload'а мутации. Соседние чистые хелперы (resolveContainerUpdatePath.test.ts,groupContainersForUpdate.test.ts) покрыты, а сам диспатч и маппинг payload — нет. Компонент-тесты мокают хуки мутаций и этот файл не трогают. Ошибка в имени поля или потерянныйpullImageмолча сломает все пути обновления.Fix: добавить
applyContainerUpdate.test.ts(мокнутьrecreateContainer,updateStack,updateGitStack,getStackFile) с проверкой: (1) standalone →recreateContainerс верными environmentId/id/pullImage/nodeName, возврат'standalone'; (2) git-стек →updateGitStackсRepullImageAndRedeploy=pullImage; (3) file-стек → fetch файла +updateStackсrepullImageAndRedeploy=pullImageи сохранённым содержимым; (4) external и «стек не найден» → throwEXTERNAL_STACK_UPDATE_ERROR.F3 [documentation] Комментарии в
settings_update.goназывают только auto-heal, хотя касаются и auto-update —api/http/handler/settings/settings_update.go:71и:207-208(
:207-208) сказано «auto-heal job is rescheduled», ноContainerAutomationService.Reload()→stop()+start()перезапускает иstartHeal, иstartUpdate. (:71) комментарий// Native container automation settings (auto-heal)стоит на поле, которое несёт иAutoHeal, иAutoUpdate(этот же текст уходит в swagger-описание). Этот же PR и вводит auto-update — комментарии устарели в момент написания.Fix: «…auto-heal and auto-update jobs are rescheduled (or stopped)…» и «(auto-heal / auto-update)».
F4 [documentation] Устаревшее «(future M4)» — авто-апдейт уже в этом PR —
app/react/docker/containers/update/applyContainerUpdate.ts:45-48(+types.ts:9,resolveContainerUpdatePath.ts:10)Дока утверждает, что примитив — «единый путь… и (future M4) auto-update job». Но (1) M4 уже не future — демон auto-update едет в этом же PR (
api/containerautomation/autoupdate.go); (2) это отдельная Go-реализация (resolveContainerUpdateRouting/groupContainersForUpdateвlabels.go— явный «Go-аналог» этих TS-функций), она не идёт через TS-примитив. Заявление о едином общем коде ложно.Fix: убрать «(future M4)» и утверждение, что бэкенд-демон использует этот TS-путь; описать примитивы как фронтовые («Update now» + bulk), отметив, что демон зеркалит ту же маршрутизацию на Go. То же в
types.ts:9иresolveContainerUpdatePath.ts:10.F5 [conventions] Настройки ContainerAutomation объявлены инлайновым анонимным вложенным структом вместо именованных типов —
api/portainer.go:1216-1235Каждый другой под-блок
Settings— именованный тип (Edge,GlobalDeploymentOptions,LDAPSettings, …). НовыйContainerAutomation— двухуровневый анонимный структ; из-за этого пакетcontainerautomationи хендлер лезут на три уровня вглубь (settings.ContainerAutomation.AutoUpdate.RollbackTimeout) и не могут передавать типизированный под-структ, тогда как фронт уже зеркалит это именованными типами. NB: имяAutoUpdateSettingsна:51уже занято несвязанным GitOps-апдейтом — новым типам нужны другие имена (напр.ContainerAutomationSettings/ContainerAutoHealSettings/ContainerAutoUpdateSettings).Fix: объявить именованные типы в блоке
type (...)(сохранив JSON-теги) и заменить поле наContainerAutomation ContainerAutomationSettings.F6 [simplification] Дублирующая
parseEnable— копияboolLabel—api/containerautomation/labels.go:31-50parseEnable(34-50) логически байт-в-байт совпадает сboolLabel(76-92) и имеет единственного вызывающего —InScope.Fix: удалить
parseEnable, вInScope(стр. 59) зватьenabled, present := boolLabel(labels, labelEnable, labelEnableAlias).F7 [simplification] Неиспользуемая зависимость
gitService—api/containerautomation/service.go:50,72,82,96(+api/cmd/portainer/main.go:582)gitServiceпринимаетсяNewService, кладётся в структ и документируется как «used by the auto-update job», но нигде не читается (s.gitServiceне встречается); git-stack путь вupdateStackсейчас detect-only.Fix: убрать поле, параметр, присваивание и упоминание в доке + аргумент на call-site в
main.go. Если это намеренный задел под git-stack авто-апдейт — ответьтеwontfix:с пояснением.Маркер
reviewed_headобновлён на70f7fe5e. После правок верните лейблreview/needs.F1: record rolled-back targets per service (endpointID/containerName + remote digest) and skip auto-update during a 24h cooldown unless the remote digest changes — breaks the infinite update→rollback loop on a persistently unhealthy image, without blocking a genuinely new image. F2: unit-test applyContainerUpdate dispatch/payload mapping. F3: settings_update.go comments mention auto-heal AND auto-update. F4: drop stale '(future M4)' TS docs; primitives are frontend-only. F5: replace the anonymous ContainerAutomation settings struct with named types (identical JSON tags). F6: drop parseEnable (duplicate of boolLabel). F7: remove the unused gitService dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Раунд 1 отработан, все семь пунктов.
F1: fixed — добавил защиту от бесконечного цикла update→rollback. После УСПЕШНОГО отката цель записывается в per-service map
rolledBack(ключendpointID/containerName, хранит remote-дайджест упавшего образа и время), по образцу существующегоretries/decideRestartиз auto-heal.updateStandaloneпропускает авто-апдейт, если текущая outdated-цель совпадает с недавно откатанной — пока не истёк cooldown (24h) ИЛИ не сменился удалённый дайджест. Ключ по ИМЕНИ контейнера, а не ID, т.к.Recreateприсваивает новый ID, но сохраняет имя (переименовывает старый в-old); и апдейт, и rollback идут через одинRecreate, имя стабильно.shouldSkipRolledBackкаждый тик перерезолвит remote-дайджест: новый upstream-образ (digest≠записанного) снимает skip — реально новый образ не блокируется; истёкший cooldown очищает запись.pruneRolledBackчистит мапу. ЧистаяdecideUpdateSkip+TestDecideUpdateSkipна все ветки. Откат — только standalone, поэтому guard вupdateStandaloneпокрывает петлю полностью.F2: fixed — добавил
applyContainerUpdate.test.ts(vitest): standalone (+флаг pullImage), git-стек (RepullImageAndRedeploy), file-стек (фетч файла + сохранение содержимого), external → throw, «стек не найден» → throw. 6/6 на стенде.F3: fixed — комментарии в
settings_update.goтеперь упоминают и auto-heal, и auto-update («…rescheduled (or stopped)…», «(auto-heal / auto-update)»).F4: fixed — из TS-доков (
applyContainerUpdate.ts,types.ts,resolveContainerUpdatePath.ts) убрал «(future M4)» и утверждение, что бэкенд-демон ходит через TS-примитив; описал примитивы как фронтовые, демон зеркалит маршрутизацию отдельно на Go.F5: fixed — анонимный вложенный структ ContainerAutomation в
portainer.goзаменён именованными типамиContainerAutomationSettings/ContainerAutoHealSettings/ContainerAutoUpdateSettingsс БАЙТ-ИДЕНТИЧНЫМИ JSON-тегами (проверено поле в поле; сохранённые настройки десериализуются без изменений). Все места доступа (autoupdate.go/service.go/datastore init+migrator/settings_update.go) компилируются.F6: fixed — удалил
parseEnable(байт-в-байт дубльboolLabel),InScopeзовётboolLabel(labels, labelEnable, labelEnableAlias).F7: fixed (removed) —
gitServiceнигде не читался вcontainerautomation(greps.gitServiceпуст, маркера phase-2 нет); убрал поле, параметр конструктора, присваивание, упоминание в доке и аргумент вmain.go:582.gitServiceещё используется в main.go в других местах — unused-var нет.Проверка на стенде:
go build ./api/...— ok;go vet ./api/containerautomation/...— ok;go test ./api/containerautomation/...— ok (вкл. новыйTestDecideUpdateSkip);gofmt -lчисто; vitestapplyContainerUpdate.test.ts— 6/6.Развилка для ревьюера (не баг, осознанный консервативный компромисс): если на момент отката не удалось зарезолвить remote-дайджест и записан пустой дайджест,
shouldSkipRolledBackблокирует именно этот контейнер на полный cooldown (24h) без перерезолва — т.е. в этом узком случае даже реально новый образ может подождать до 24h. Выбрано в пользу «лучше переждать, чем снова войти в петлю». Если предпочтёте перерезолвить и при пустом записанном дайджесте — скажите, поправлю.Коммит
922f506fe. Вернулreview/needs.Ревью
922f506fe— раунд 2 (ПОЛНЫЕ 8 аспектов, отдельный субагент на каждый).Вердикт: CHANGES. Все семь находок раунда 1 закрыты и проверены независимо:
rolledBack(endpointID/containerName→ remote-digest+время),updateStandaloneскипает авто-апдейт пока digest совпадает и не истёк cooldown 24h. Петля рвётся; новый upstream-образ (digest≠записанного) НЕ блокируется (skip снимается); провал отката тоже не зацикливается (сломанный образ становится running →ContainerImageStatusрепортит current → не пере選бирается). Name-keying корректен (Recreateсохраняет имя — проверено вapi/docker/container.go), коллизий между endpoint'ами нет (ключ含 endpointID), мапа под своим mutex'ом, утечки нет (pruneRolledBack+ clear). ЧистыйdecideUpdateSkip+TestDecideUpdateSkipна все ветки.applyContainerUpdateбез теста) —applyContainerUpdate.test.tsпокрывает standalone(±pullImage)/git-стек/file-стек/external-throw/стек-не-найден-throw с реальными ассертами.parseEnable), F7 (неиспользуемыйgitService) — удалены чисто.security / regressions / conventions / documentation / architecture / simplification — чисто. Остался один мелкий хвост по тестам. Отвечайте по id.
Что сделать
F8 [test coverage] Новый
pruneRolledBackбез теста (его сиблингpruneRetries— покрыт) —api/containerautomation/service.go:272-281pruneRolledBack— гард от неограниченного ростаrolledBack-мапы, зовётся каждый тик (autoupdate.go:89), со своей граничной логикой (now.Sub(rec.at) >= updateRollbackCooldown→ delete), и не покрыт. Точный сиблингpruneRetriesПОКРЫТ (TestPruneRetries, autoheal_test.go:77-99) — по собственной планке проекта этот pruner тоже надо тестить.TestDecideUpdateSkipпроверяет сравнение cooldown семантически, но не то, что функция итерирует мапу и удаляет только истёкшие, сохраняя свежие — регресс тут тихо течёт памятью или выселяет живые guard-записи.Fix: добавить
TestPruneRolledBackпо образцуTestPruneRetries: засидитьService.rolledBackсвежей записью (в cooldown), записью ровно на границе и давно-устаревшей; вызватьpruneRolledBack(now); проверить, что свежая сохранена, а граничная и устаревшая удалены.Маркер
reviewed_headобновлён на922f506fe. После правок верните лейблreview/needs.F8: fixed — добавил
TestPruneRolledBackвrollback_test.go(рядом сTestDecideUpdateSkip), по образцуTestPruneRetries. Сидитs.rolledBackтремя записями на фиксированныйnow: свежая (at = now - cooldown/2) → сохраняется; ровно на границе (at = now - updateRollbackCooldown) → удаляется (продакшн использует инклюзивное>=); давно устаревшая (at = now - 2*cooldown) → удаляется. Послеs.pruneRolledBack(now)проверяется, что осталась только свежая иlen == 1. Граничную семантику (>=) подтвердил по кодуpruneRolledBack(service.go).Проверка на стенде:
go test ./api/containerautomation/...— ok (вкл. новыйTestPruneRolledBack,TestPruneRetries,TestDecideUpdateSkip);gofmt -lчисто.Коммит
617180652. Вернулreview/needs.Ревью
617180652— раунд 3 (ПОЛНЫЕ 8 аспектов, отдельный субагент на каждый).Вердикт: PASS. F8 закрыта:
TestPruneRolledBackдобавлен (по образцуTestPruneRetries) и проверен независимо — сидитs.rolledBackтремя записями на фиксированныйnow(freshcooldown/2→ сохранена; edge ровно-updateRollbackCooldown→ удалена; stale2×→ удалена) +len==1; граница инклюзивна (now.Sub(at) >= updateRollbackCooldown) и совпадает с продакшеном. Дельта раунда — только этот тест-файл. Все 8 аспектов — LGTM. Все находки всех раундов (F1 петля update→rollback и далее) закрыты — готово к мержу.Маркер
reviewed_headобновлён на617180652.Независимое пост-ревью перед мержем (мейнтейнер) — вердикт: CHANGES
Цикл сошёлся честно, бэкенд компилируется, проходит
go vetи все тесты (containerautomation, container-хендлеры,migrator— зелёные). Но при независимой перепроверке всплыл один значимый функциональный баг (его петля сама нашла и сама же закрыла неверным обоснованием) плюс набор краевых случаев в демоне. Ниже — что нужно поправить.F1 [major] Детект обновлений устаревает до 24 часов — и демон, и бейдж
api/docker/images/status.go:38,137-147(+ запись:175, инвалидация вrecreate.go/endpoint_force_update_service.go)statusCache— кэш на 24 ч, ключ — локальныйimageID. Новое чтение делает его авторитетным на пути детекции, и демон авто-апдейта ходит через тот же путь (autoupdate.go:159 ContainerImageStatus). Когда апстрим пушит новый образ под тем же тегом, локальныйimageIDне меняется → кэш до суток отдаёт «актуально». Все пути инвалидации (EvictImageStatus) чистят поcontainerID/стеку/serviceID, но никогда поimageID. Итог: контейнер, бывший up-to-date, до 24 ч считается up-to-date, и авто-апдейт с интервалом 6 ч его не подхватит — интервал опроса < 24 ч практически бессмыслен для свежезапушенных образов.Это ровно то, что было поймано в M2 (раунд 1) и «закрыто» комментарием в коде, который фактически неверен:
«a hit returns the same value the full computation would have produced»— при новом образе полный расчёт дал быOutdated. Кэшируются такжеSkipped/Outdated(а не «только успешные»).Fix: либо привязать TTL записи по
imageIDк интервалу опроса (минуты), а не 24 ч; либо инвалидировать запись поimageIDтам, где это уместно; как минимум — исправить комментарий, чтобы он не маскировал поведение.F2 [warning] Loop-guard (update→rollback) только в памяти — не переживает рестарт
api/containerautomation/autoupdate.go(rolledBack),service.go pruneRolledBackКарта
rolledBackне персистится. После рестарта Portainer она пуста → один лишний цикл update→rollback на каждый рестарт; при флаппинге рестартов петля возобновляется. Минимум — задокументировать in-memory-природу гарда; лучше — персистить ключ+digest+время.F3 [warning] Гард ломается на безымянных/вручную переименованных контейнерах
api/containerautomation/autoupdate.go:350-393Ключ гарда —
endpointID/name. Для контейнера без имениrecordRolledBackзапись вообще не делает → standalone без имени будет апдейт↔роллбек на каждом тике без подавления. При ручном переименовании контейнера между тиками запись осиротеет, гард не сработает. Завязать на стабильный идентификатор (label/compose-id) либо при включённом rollback пропускать update для безымянного случая.F4 [warning]
ContainerInspectбез таймаутаapi/containerautomation/autoupdate.go:263Единственный движковый вызов на
s.baseCtxнапрямую (cli.ContainerInspect(s.baseCtx, c.ID)) — все остальные обёрнуты вcontext.WithTimeout. Зависший/недоступный engine заблокирует весь последовательный тик до самого shutdown. Обернуть вcontext.WithTimeout(s.baseCtx, endpointTimeout).F5 [suggestion]
Reload()не прерывает выполняющийся тикapi/containerautomation/service.go:128-136Смена интервала применяется только после конца текущего (потенциально многоминутного) тика, а старый тик доработает на старом контексте (отменяется лишь на shutdown). Данные не бьются — atomic overlap-guard (
updateRunning/running) и мьютексы на картах корректны (перепроверено), так что это поведенческая шероховатость, а не баг. Достаточно задокументировать, либо давать джобу собственный отменяемый контекст.F6 [suggestion] Нет нижней границы для auto-heal
CheckIntervalapi/http/handler/settings/settings_update.go:144-147Для auto-update floor есть (
PollInterval≥ 1m,RollbackTimeout≥ 10s), аAutoHeal.CheckIntervalвалидируется только какd <= 0. Валидное1msпройдёт и будет долбить Docker листингом/инспектом. Ввести разумный минимум (≥1s) по аналогии с auto-update.F7 [suggestion] Миграция дописана в чужой незарелизенный бакет
2.43.0— боль форкаapi/datastore/migrator/migrator.go:275-279,migrate_2_43_0.goСейчас корректно (2.43.0 ещё не выпущена, последний тег 2.42.0; бэкфилл идемпотентен; фикстура
output_24_to_latest.jsonобновлена; тесты проходят). Но для форка это гарантированные конфликты с upstream-овскимmigrate_2_43_0.goи риск рассинхронаMigratorCount→ лишние прогоны группы. Лучше выделить форк-специфичную версию/неймспейс миграций.F8 [suggestion] Вводящий в заблуждение комментарий про SSRF/AllowList
api/http/handler/docker/containers/image_status.go:62-64Комментарий заявляет, что исходящий запрос к реестру «honors the outbound SSRF/AllowList», но в
RemoteDigest→docker.GetDigestтакой проверки нет (HEAD идёт напрямую на хост из image reference). Поведение унаследовано от апстримногоContainersImageStatus— не новая дыра, но комментарий маскирует реальность. Убрать заявление либо реально пропускать через фильтр.F9 [suggestion] Фронт: рассинхрон паритета валидации и инвалидации
AutoHealPanel/validation.ts—0s/0mпроходят фронт-валидацию, но бэк отдаёт 400 (d <= 0). Добавить тест/минимум, как вAutoUpdatePanel.update/useBulkUpdateContainerImages.ts:20— дубль константыSTALE_TIME(второй источник правды). Экспортировать изuseContainerImageStatus.ts.update/useUpdateContainerImage.ts:17-32— при redeploy стека инвалидируется бейдж только репрезентативного контейнера; у соседних контейнеров стека бейдж остаётся «outdated» (спасает толькоrouter.reload()). Инвалидировать по стеку/эндпоинту или убрать вводящий в заблуждение комментарий.Процессное (для информации)
Чистые decision-функции покрыты тестами почти идеально, а вся оркестрация (
update/updateEndpoint/updateStandalone/rollback/heal/Reload) — без тестов. Все F1–F4 живут именно там. Желательно хотя бы один интеграционный тест с фейковым Docker-клиентом на каждый путь демона.Что чисто: security/RBAC эндпоинта (bouncer +
CheckEndpointAuthorization), креды реестра не утекают в ответ; миграция идемпотентна; демон защищён от битых интервалов fallback'ом; atomic overlap-guard и мьютексы на картах; git-стеки detect-only, file-стеки редеплоятся через deployer (контейнеры не осиротевают), per-stack дедуп; ECR-refresh зеркалит userless git-путь; AUTH-гейтинг stack-redeploy наPortainerStackUpdate; чистый CE безisBE.Блокеров уровня «не собирается» нет; нужно закрыть F1 (главное) и F2–F4, остальное — по возможности.
F1: cap the image-status cache TTL at 5m (was 24h) — the cache is keyed by the LOCAL imageID, which doesn't change when upstream pushes a new image under the same tag, so the 24h TTL hid new images from both the badge and the auto-update daemon; a short TTL re-resolves the remote digest within the poll window. F2: document that the update->rollback guard map is in-memory (restart implication). F3: skip auto-update for an unnamed container when rollback is on (the endpoint+name keyed guard can't record it, so it would loop) — pure skipUnnamedForRollback + test. F4: wrap the pre-update ContainerInspect in context.WithTimeout(endpointTimeout). F5: document Reload() does not interrupt an in-flight tick. F6: floor auto-heal CheckInterval at 1s (mirrors auto-update) + test. F7: wontfix — migration is currently correct; namespace rework is out of scope. F8: correct the misleading SSRF/AllowList comment (no filter is applied). F9: front auto-heal interval floor + test; dedup STALE_TIME; fix invalidation comment. Also refresh three stale '24h/long-lived cache' comments to match the 5m TTL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Спасибо за независимый пост-ревью — отработал все 9 пунктов.
F1: fixed (главный) — TTL
statusCacheснижен с 24h до короткогоstatusCacheTTL = 5m(api/docker/images/status.go). Кэш ключуется по ЛОКАЛЬНОМУimageID, который не меняется при push нового образа под тем же тегом → imageID-инвалидация структурно невозможна, лечится только временной границей. 5m < дефолтного интервала (6h) и достаточно мало, чтобы демон (идёт через тот жеContainerImageStatus) перепроверял remote digest в пределах окна опроса. На флоре poll=1m латентность детекта = max(poll, ~5m), т.е. до ~5m — осознанный bounded-компромисс, кратно лучше суток; привязывать TTL к poll нельзя дёшево (кэш в общем пакетеdocker/images, используется и бейджем UI). Вводящий в заблуждение комментарий («hit returns the same value the full computation would have produced») переписан под реальное поведение; +TestStatusCacheTTLIsShort(падает при возврате к 24h). Заодно подчистил три оставшихся комментария про «24h/long-lived cache» (settings_update.go, AutoUpdatePanel/validation.ts, updateAvailable.tsx).F2: documented — расширил докстринг
Service.rolledBack: карта in-memory, рестарт её обнуляет → максимум один лишний цикл update→rollback на рестарт; персист отложен (нужна схема в datastore, вне scope).F3: fixed — добавил чистую
skipUnnamedForRollback(rollback, name)и зову первой вupdateStandalone: при rollback ON и безымянном контейнере — пропуск авто-апдейта (гард ключуется endpoint+name, безымянный не записывается → иначе вечная петля); при rollback OFF петли нет, контейнер апдейтится. +TestSkipUnnamedForRollback(4 кейса).F4: fixed — pre-update
cli.ContainerInspectобёрнут вcontext.WithTimeout(s.baseCtx, endpointTimeout)(тот же таймаут, что у остальных engine-вызовов),cancel()сразу после.F5: documented — отметил в
Reload(), что смена интервала не прерывает уже идущий тик (доработает на pre-reload контексте, отменяется лишь на shutdown); overlap-guard и мьютексы делают это безопасным — поведенческая шероховатость, не баг.F6: fixed — ввёл
minAutoHealCheckInterval = 1s, валидацияd < 1s(зеркалит auto-update флор, стиль сообщения совпадает) + тест.F7: wontfix — миграция сейчас корректна (2.43.0 не выпущена, идемпотентна, фикстуры/тесты зелёные). Migrator ныне неймспейсит строго по версии релиза; перенос в другой бакет лишь переносит тот же риск конфликта и форсит регенерацию
output_24_to_latest.json+ counts — дороже и рискованнее. Форк-неймспейс миграций — отдельное архитектурное изменение вне scope этого PR.F8: fixed — убрал ложное «honors the outbound SSRF/AllowList» в
image_status.go: HEAD к реестру НЕ проходит SSRF/AllowList-фильтр (унаследовано от upstream), фильтр не добавлял (вне scope), комментарий теперь честный.F9: fixed — (a)
AutoHealPanel/validation.tsтеперь с флором 1s (зеркалитAutoUpdatePanel,0s/500msбольше не проходят фронт при 400 на бэке) + тест; (b)STALE_TIMEэкспортирован изuseContainerImageStatus.ts(единый источник), дубль вuseBulkUpdateContainerImages.tsзаменён импортом; (c) комментарий инвалидации вuseUpdateContainerImage.tsприведён к реальности (инвалидируется бейдж только репрезентативного контейнера, sibling'и освежаются по staleTime/focus) — инвалидация по эндпоинту регрессировала бы standalone-кейс (общий хелпер, в ключах нет stack→container маппинга).Проверка на стенде:
go build ./api/...ok,go vet ./api/containerautomation/...чисто,go test ./api/containerautomation/... ./api/docker/... ./api/http/handler/settings/...— все PASS,gofmt -lчисто; vitest по затронутым валидациям/обновлению — зелёный. Внутренний ревью подтвердил корректность F1/F3/F4/F6/F9 и non-vacuous тесты.Процессное (интеграционные тесты оркестрации с фейковым Docker-клиентом) — согласен, это следующий заход; готов сделать отдельным PR, чтобы не раздувать этот.
Коммит
be3bfd051. Вернулreview/needs.Ревью
be3bfd051— раунд после твоего независимого пост-ревью (ПОЛНЫЕ 8 аспектов, отдельный субагент на каждый).Вердикт: CHANGES (один suggestion). Все 9 твоих пунктов отработаны и проверены независимо:
statusCacheTTL24h→5m (нетNoExpirationни у одного writer'а;TestStatusCacheTTLIsShortпинит TTL ≤10m И реальную экспирацию, падает на возврате к 24h). При дефолтном poll (6h) демон всегда пересчитывает; на флоре 1m латентность детекта ≤~5m — осознанный bounded-компромисс, кратно лучше суток. NB (regressions): в basedevelopinput-side чтениеstatusCache, badge-эндпоинт, bulk-update и демон — ВСЕ вводятся этим PR (в базе уContainerImageStatus/CachedResourceImageStatusнет прод-потребителей, 24h-кэш дремал), т.е. 24h→5m — тюнинг новой фичи, не слом существующего поведения. Комментарии переписаны под реальность, 3 хвоста про «24h» подчищены.skipUnnamedForRollback(rollback, name)зовётся первым вupdateStandalone(rollback-on+безымянный → skip, иначе proceed);TestSkipUnnamedForRollbackна 4 ветки. (Вторичный sub-case ручного переименования mid-cooldown не гардится отдельно, но ограничен одним лишним циклом и self-heal'ится — ниже порога.)context.WithTimeout(s.baseCtx, endpointTimeout); грэп подтвердил — raw-baseCtx движковых вызовов в пакете не осталось.rolledBackзеркалит существующийretries, self-correcting, честно задокументирован; миграция идемпотентна/однократна).minAutoHealCheckInterval = 1s, зеркалит auto-update floor + тест боундари.STALE_TIMEэкспортирован единым источником (дубль убран),AutoHealPanel/validation.test.tsзеркалит AutoUpdatePanel (0s/500ms/1ms reject, 1s/30s/2m accept).security / stability / regressions / documentation / conventions / architecture — чисто. Один новый пункт от моего ревью. Отвечай по id.
Что сделать
F10 [simplification] Раунд закрыл дубль
STALE_TIME(F9), но открыл новый — Go-duration parser скопирован дословно в AutoHealPanel —app/react/portainer/settings/SettingsView/AutoHealPanel/validation.tsФикс F6 (добавление floor в AutoHealPanel/validation.ts) скопировал ~30 строк дословно из AutoUpdatePanel/validation.ts: идентичные
durationPattern,unitSecondsи байт-в-байтparseGoDurationSecondsтеперь живут в ОБОИХ файлах (грэп: только эти два, общего модуля нет). Это ровно та проблема single-source, что F9 указал дляSTALE_TIME— раунд её закрыл, но открыл эту. Нетривиальная парсинг-логика → две копии могут разойтись (правка юнита/регекса) и тихо рассинхронить клиентские floor'ы двух панелей. Фикс механический и самодостаточный, и симметричен тому, что ты уже сделал для STALE_TIME в этом же раунде.Fix: вынести
durationPattern,unitSeconds,parseGoDurationSecondsв общий модуль (напр.SettingsView/parseGoDuration.ts), импортировать из обоих validation.ts, удалив локальные копии.Маркер
reviewed_headобновлён наbe3bfd051. После правок верните лейблreview/needs.Отдельно (вне scope PR, завёл issue): оркестрация демона (
updateStandalone/rollback/healEndpoint/updateStack/Reload) — без тестов, а F1–F4 жили именно там; покрытие требует fake-Docker-client seam (расширяет scope за PR), ты сам отметил это non-blocking и кодер обещал follow-up. Зафиксировал как debt-issue для трекинга.F10: fixed — вынес дублированный Go-duration парсер в общий модуль
app/react/portainer/settings/SettingsView/parseGoDuration.ts(экспортируетdurationPatternиparseGoDurationSeconds,unitSecondsприватный). Обаvalidation.ts(AutoUpdatePanel и AutoHealPanel) теперь импортируют из него, локальные копии удалены — single source of truth, как сделал дляSTALE_TIMEв прошлом раунде; floor'ы двух панелей больше не могут разойтись при правке регекса/юнита. Панель-специфичные пороги (minPollIntervalSeconds/minRollbackTimeoutSeconds/minCheckIntervalSeconds) остались локальными.Проверка на стенде:
tsc --noEmit— exit 0;eslintпо 3 файлам — чисто;vitestпо обеим панелям (AutoHeal/AutoUpdate validation) — 14 passed (оба валидатора работают через общий парсер без изменений поведения).Про твою отдельную заметку (оркестрация демона без тестов, fake-Docker-client seam вне scope) — согласен, это debt отдельным заходом, как и отметил мейнтейнер; зафиксировано issue.
Коммит
f658d67cc. Вернулreview/needs.Ревью
f658d67c— раунд после пост-ревью (9 аспектов, включая COHERENCE — целостный взгляд на PR против цели).Вердикт: PASS. F10 закрыт и проверён независимо несколькими аспектами:
F10 (дубль Go-duration-парсера, открытый фиксом F6 в прошлом раунде) — вынесен в общий
app/react/portainer/settings/SettingsView/parseGoDuration.ts(экспортdurationPattern+parseGoDurationSeconds,unitSeconds/componentPatternприватны); обаAutoHealPanel/validation.tsиAutoUpdatePanel/validation.tsимпортируют из../parseGoDuration, локальные копии удалены полностью.µs-микрознак, та жеunitSeconds-карта, та же логикаparseGoDurationSeconds); единственное отличие — лидирующий комментарий, на поведение не влияет.minPollIntervalSeconds=60,minRollbackTimeoutSeconds=10,minCheckIntervalSeconds=1) — в общий модуль ничего не уехало, рассинхрона floor'ов двух панелей нет. Это ровно тот single-source-фикс, что я делал дляSTALE_TIME(F9), симметрично.durationPatternбезg-флага → ни.matches()(yup), ни.test()не мутируютlastIndex, гонок shared-state нет.Coherence (целостный взгляд): F10-извлечение не рассинхронило фронт/бэк. Проверен паритет floor'ов фронт↔бэк против
api/http/handler/settings/settings_update.go: checkInterval FE 1s ↔ BEminAutoHealCheckInterval=time.Second; pollInterval FE 60s ↔ BEminAutoUpdatePollInterval=time.Minute; rollbackTimeout FE 10s ↔ BEminAutoUpdateRollbackTimeout=10*time.Second— все три совпадают. Сквозной поток держится: детект (statusCacheTTL=5m) ↔ poll-floor 1m согласованы; все поля, что читает демон, либо валидируются (интервалы+scope, фронт+бэк), либо булевы; form-дефолты ('6h'/'120s'/'30s') удовлетворяют обоим floor'ам и равны демоновским; loop-guard (rolledBack-map + cooldown + prune + unnamed-skip) и health-gate→rollback порядок согласованы; миграция — opt-in default-off (новый nested-struct в zero-value). Цель PR достигнута.security (нет ReDoS — regex линейный, детерминированный unit-токен, заякорен с обоих концов; клиентская косметика, бэк энфорсит floor'ы независимо) / test-coverage (ни одного осиротевшего теста; общий парсер покрыт транзитивно — обе панельные сюиты гоняют все ветки на accept И reject сторонах всех трёх floor'ов; отдельный тест был бы избыточен) / conventions (размещение/именование/экспорт-стиль соответствуют, зеркалят F9-извлечение) / documentation / architecture — все LGTM.
Все находки всех раундов закрыты. Объективные проверки: запустить vitest/tsc сам не могу (нет node_modules в окружении ревью). Базис PASS: кодер отчитался о реальном прогоне (vitest по обеим панелям зелёный, tsc 0, eslint чисто), И два аспекта (regressions, test-coverage) НЕЗАВИСИМО подтвердили, что извлечение byte-identical и обе панельные сюиты гоняют все ветки через общий парсер без изменения поведения. Готово к мержу.
Маркер
reviewed_headобновлён наf658d67c.надо еще в настройках поддержать REST API для уведомдения. чтобы можно было указать http адрес который бы дергался при обновлениях и в адресе подставлялась бы строка с сообщнием
Сообщение какое-то такое для обновления:
Environment | nebula.lc
Stack [cache-demo]
Update [esphome]: 59b94983c73a → 2231ca5d676d
для autoheal сам придумай
а поставить руки отсохнут?
Add an opt-in webhook notification for container-automation events (image update, rollback, update-failed, auto-heal restart), plugging into the existing Notifier seam in notify.go. - Settings: new ContainerAutomation.Notification.WebhookURL (shared across update + heal), persisted and validated in the settings update handler (optional; http/https only; accepts the {{message}} placeholder). - webhookNotifier reads the current URL from the datastore per event (UI changes take effect without a restart). If the URL contains {{message}} it substitutes the URL-encoded message and issues a GET; otherwise it POSTs the message as the body. Delivery, the env/stack name lookups, and any panic run in a goroutine under recover() with a 10s timeout — strictly best-effort, never blocks or crashes the automation daemon. multiNotifier fans events to logNotifier + webhook and isolates a panic in any one notifier. - Message format (maintainer's spec): Environment | <env> Stack [<name>] (Container [<name>] for non-stack events) Update [<name>]: <old> -> <new> Auto-heal: 'Auto-heal: restarted unhealthy container'. - New NotificationPanel in settings to configure the URL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Обе твои просьбы из этого раунда сделаны, два коммита:
1. Интерактивный бейдж статуса образа (коммит
6aecdfbe4):useApplyContainerImageUpdate, чтобы details-кнопка и бейдж в списке жили на одной реализации.statusCache5м +remoteDigestCache5с), поэтому простой рефетч был пустышкой. Добавил на эндпоинте опциональный?force=true, который для ручной перепроверки обходит ОБА кэша (и делает свежий HEAD в реестр), но всё равно перезаписывает их; дефолтный путь (авто-бейджи + демон авто-апдейта) кэши использует как раньше.2. Webhook-нотификатор (коммит
eb35e9c47): в настройках новая панель «Notifications» — HTTP-URL, который дёргается при событиях автоматизации (update / rollback / update-failed / auto-heal). В URL подставляется URL-энкоженное сообщение на месте плейсхолдера{{message}}(GET); если плейсхолдера нет — POST телом. Формат как просил:Для autoheal:
Environment | … / Container [<имя>] / Auto-heal: restarted unhealthy container. Доставка строго best-effort — HTTP + чтения env/stack + любая паника в горутине подrecover()с таймаутом 10с, автоматизацию не блокирует и не роняет. Использован уже заложенный в PR seamNotifier(logNotifier + webhook через multiNotifier). URL опционален, валидируется (http/https).Проверки (на ветке):
go build ./...— 0;go test ./api/containerautomation/... ./api/http/handler/settings/... ./api/datastore/...— ok (incl. golden-миграция);gofmt/go vet— чисто;tsc --noEmit— 0;eslint— 0;vitest— бейдж/панель/логика зелёные.Внутренний ревью прошёл по обоим (бейдж, force-recheck, вебхук): проверены паритет апдейт-флоу, обход кэшей, SSRF/инъекции (message URL-энкодится, CRLF-инъекция через имена невозможна), best-effort/отсутствие блокировок и утечек, golden-миграция. Замечания уровня suggestion закрыл (force пробит и через
remoteDigestCache; formatMessage+доставка целиком в горутине под recover; multiNotifier изолирует панику).Стенд пересоберу следующим шагом, чтоб пощупал вживую.
Ревью
eb35e9c47— дельта из двух коммитов (интерактивный бейдж6aecdfbe4+ webhook-нотификаторeb35e9c47) поверх approved-базыf658d67c. Полный фан-аут 9 аспектов (security/stability/regressions/test-coverage/conventions/documentation/simplification/architecture/coherence), отдельный субагент на каждый.Вердикт: CHANGES. Обе твои просьбы реализованы и в целом крепко: бейдж-интеракция и webhook работают, best-effort доставка изолирована, инъекции/утечки секретов нет. Осталось закрыть пару тест-дыр и один abuse-вектор force-перепроверки; плюс один вопрос по формату сообщения для стеков — тебе на решение (ниже). Отвечай по id.
Что подтверждено чистым (проверено по коду независимо)
useApplyContainerImageUpdate→ approved-примитивapplyContainerUpdate), кнопкаdisabledна время апдейта (двойной сабмит закрыт тестом). Клик «Up to date» → реальный?force=true, обходит оба кэша (statusCache5м +remoteDigestCache5с) и делает свежий registry-HEAD; бейдж обновляется на месте (общий queryKey инвалидируется/перезаписывается).{{message}}) кодирует сообщениеurl.QueryEscape— крафт-имя контейнера/стека/образа не вырвется из query, не подменит path и не вставит параметры (пробел/&///CR/LF percent-энкодятся). POST-режим кладёт сообщение только в тело, единственный хедер — константный Content-Type → CRLF-инъекция через имена невозможна. Секреты не утекают:formatMessageчитает только имена/дайджесты/event.Err, реестровых кред нет. URL — глобальная настройка подAdminAccess(не-админ не выставит), валидируется http/https+host.recover(), ограничена 10с; вызывается вне удержания мьютексов; висящий/битый endpoint не блокирует и не роняет тик автоматизации;multiNotifierизолирует панику каждого нотификатора. Утечки горутин нет.UpdateNowButtonотрефакторен в общий хук поведение-идентично (тот же confirm/роутинг/pull/permission-гейт); Settings — чисто аддитивное поле (json:"Notification", старые теги целы, golden-миграцияoutput_24_to_latest.json— легитимный дефолт); валидация бьёт только по новому полю.Notifier+logNotifier+webhookNotifierчерезmultiNotifier) — тот шов, что заложили в эпике, наполнен ровно одним реальным консюмером. Именованный типContainerAutomationNotificationSettings(паритет с F5 прошлого раунда), панель зеркалитAutoUpdatePanel. Комментарии точны, ложного «SSRF-фильтруется» (готча прошлого эпика) больше нет.Что сделать
F1 [test coverage] Клик бейджа/
UpdateNowButton→ диспатч апдейта не проверен (только disable-гейт) —app/react/docker/containers/update/useApplyContainerImageUpdate.ts(+UpdateNowButton.test.tsx)Тесты проверяют, что во время апдейта кнопка
disabled(анти-даблклик — хорошо), но НИ ОДИН тест не гоняет самapply(): confirm-диалог → резолвpullImage→updateMutation.mutate→ роутинг standalone-recreate vs stack-redeploy.UpdateNowButton.test.tsxмокаетmockMutate, но ни разу не кликает и не ассертит, что мутация вызвана. Роутинг нетривиален и сейчас без единого ассерта, что апдейт вообще стартует.Fix: тест, где
confirmContainerRecreation/confirmStackUpdateзамоканы на resolve, кликается кнопка, и ассертитсяmockMutateвызван РОВНО один раз с вернымpullImage— по обеим веткам (standalone и stack).F2 [test coverage] force не проверен на перезапись кэшей при УСПЕШНОМ пересчёте —
api/docker/images/status.go:208,289(statusCache.Set/remoteDigestCache.Set)Оба форс-тета в
status_test.goпадают в ошибку ДО write-back, так что вторая половина контракта («force не только обходит, но и обновляет оба кэша свежим значением») не покрыта. Если бы write-back сломался — force работал бы, но следующий non-force чтение каждый раз пересчитывало.Fix: форс-тест с успешным пересчётом, ассертить, что последующий non-force возвращает свежее значение из кэша.
F3 [security]
?force=trueдоступен любому env-авторизованному пользователю без троттла → амплификация нагрузки на реестр —api/http/handler/docker/containers/image_status.go(+status.gobypass)Роут обёрнут
AuthenticatedAccess+CheckEndpointAuthorization— это ЛЮБОЙ пользователь с доступом к окружению, не только админ.forceснимает оба кэша и на каждый вызов делает исходящий registry-HEAD; троттла/дебаунса нет. Скрипт в цикле…?force=trueжжёт общий pull-rate квоты инстанса и молотит реестр (унаследованный факт: этот HEAD ещё и не проходит SSRF-фильтр). До этого коммита кэш демпфировал повторы — force его убирает.Fix: коалесить одновременные форс-перепроверки одного образа и/или короткий серверный min-интервал на форс (≈ TTL
remoteDigestCache). Если считаешь env-авторизованных достаточно доверенными — ответьwontfix:с обоснованием, приму.Для vvzvlad — решение по формату (НЕ задача кодеру)
Твой канонический пример был для СТЕКА:
Реализация даёт этот формат (с
old → newдайджестами) ТОЛЬКО для standalone-контейнеров (под строкойContainer [имя]). Для СТЕКОВ (как в твоём примере) сообщение выходит:— субъект = имя стека, без per-image дайджестов и без
esphome. Причина структурная: апдейт стека — это цельныйDeployComposeStack, per-service дайджесты сейчас не отслеживаются, поэтому одно событие на весь стек. Чтобы выдать ровно твой формат (Update [esphome]: <old> → <new>внутри стека), нужен per-service трекинг дайджестов на redeploy — заметное расширение. Реши: устраиваетUpdate [<стек>]: image updated, или заводим отдельную задачу на per-service дайджесты для стеков? (Функционально webhook работает в обоих случаях — вопрос только про детальность сообщения.)Объективные проверки (запущены в окружении ревью):
tsc --noEmit -p .(весь проект) — exit 0, 0 ошибок;eslintпо 17 изменённым TS/TSX — чисто;vitest(бейдж/панель/updateAvailable/useContainerImageStatus/NotificationPanel) — 10 passed.go build/go vet/go test ./api/containerautomation/… ./api/http/handler/settings/… ./api/datastore/…зелёные, вкл. golden-миграцию), И тесты независимо вычитаны (webhook-энкодинг/доставка и force-bypass покрыты non-vacuous — падают при регрессе).Маркер
reviewed_headобновлён наeb35e9c47. После правок верниreview/needs.Дополнение к ревью
eb35e9c47— vvzvlad принял решение по формату webhook для стеков (в моём комменте выше это был вопрос «тебе на решение»). Теперь это задача кодеру, наравне с F1–F3.F4 [coherence/feature] Webhook при обновлении контейнера ИЗ СТЕКА должен показывать стек + затронутый(е) контейнер(ы) и их образ(ы) —
api/containerautomation/labels.go:170,199-204+autoupdate.go:509-559+webhook.goformatMessageСейчас событие стека выходит
Stack [cache-demo] / Update [cache-demo]: image updated— без контейнеров и образов. Причём данные ЕСТЬ: на фазе детекта (updateEndpoint) по каждому устаревшему контейнеру собираетсяUpdateCandidate{Name, ImageID (старый), Labels}, но при схлопывании кандидатов в стек (groupContainersForUpdate, labels.go:199-204)StackUpdateсохраняет толькоStackID/IsGit, а имена/образы контейнеров отбрасываются.Fix:
StackUpdate(labels.go:170) список затронутых контейнеров (минимумName+ старыйImageID+ имя образа), заполнять его на коллапсе (labels.go:199-204 — кандидатыcтам на руках).updateStack(autoupdate.go:509) эмитит сообщение, называющее стек + эти контейнеры/образы. Несколько устаревших сервисов в стеке → несколько строкUpdate [<container>]: <image>(или одно событие с перечислением — на твоё усмотрение, лишь бы были видны контейнер и образ).<old> → <new>; если нет — покажи имя образа/старый дайджест, доставку из-за отсутствия нового дайджеста НЕ блокировать.Цель (слова vvzvlad): «просто показывай из какого стека контейнер обновился» + «показать имеджи для контейнеров из стека». Итог —
Stack [cache-demo] / Update [esphome]: …вместо голого «image updated».Оговорка (мой недочёт): в основном ревью я оценил это как «заметное расширение / нужен per-service трекинг дайджестов» — это было преувеличение. Имя контейнера + старый образ + стек уже собраны на детекте; расширять надо только per-service НОВЫЙ дайджест (и то best-effort). Остальные пункты ревью (F1–F3) в силе; отвечай по всем id,
review/needsвернёшь после F1–F4.Поправка к F4 (моя прошлая формулировка была криво — как агрегат по стеку). vvzvlad уточнил и подтвердил окончательную постановку — отменяет предыдущий вариант F4:
F4 [feature] Уведомления — ПО КОНТЕЙНЕРУ; если контейнер в стеке, показать имя стека —
api/containerautomation/autoupdate.go(updateStandalone/updateStack),webhook.goformatMessage,EventТребование (дословно от vvzvlad): «одно уведомление на каждый контейнер, который Portainer авто-обновил; если контейнер в стеке — в этом уведомлении показать имя стека. Всё». Не по сервису, не агрегатом по стеку — по контейнеру.
Текущее состояние:
updateStandaloneэмититEventUpdated{ContainerName, OldDigest, NewDigest}(autoupdate.go:354) → сообщениеContainer [name] / Update [name]: old → new. Это и есть нужная форма.updateStack), который шлёт одно голое событие сStackIDи «image updated». Именно этого не хватает.Fix:
EventUpdated(по одному на каждый реально обновлённый контейнер), а не одно агрегированное событие на стек.Eventимя стека (compose-project). Источник — меткаcom.docker.compose.projectконтейнера, она уже лежит вUpdateCandidate.Labelsна фазе детекта; тащитьStackID/Stack().Readне обязательно.formatMessageпечатает строкуStack [<name>]когда имя стека задано (в дополнение кContainer [name]+Update […]: old → new).NewDigestдля стек-члена: апдейт идёт цельным redeploy, новый image id на руках не лежит — снять его best-effort ре-inspect'ом обновлённого контейнера после redeploy. Если недоступно дёшево — показать имя образа/старый дайджест; доставку не блокировать. (Old digest/имя контейнера/имя стека — бесплатно, уже на детекте.)Итог для контейнера esphome из стека cache-demo:
F1–F3 из основного ревью в силе. Отвечай по всем id,
review/needs— после F1–F4.Все четыре закрыл (коммит
5bb678d3b).F1: fixed — тест на реальный диспатч апдейта.
UpdateNowButton.test.tsx: кликаю кнопку,confirmContainerRecreation/confirmStackUpdateзамоканы на resolve, ассертюmutateвызван РОВНО один раз с вернымpullImageпо обеим веткам (standalone{pullLatest:true}→pullImage:true; stack{repullImageAndRedeploy:false}→pullImage:false), и НЕ вызван при отмене диалога. 9/9 зелёные.F2: fixed — Go-тест на write-back при УСПЕШНОМ пересчёте.
status_test.go: forced-recompute до успеха (httptest TLS-реестр v2 + фейковый docker-движок, локальный дайджест == remote HEAD →Updated), затем NON-force чтение отдаётUpdatedиз кэша, счётчики inspect/HEAD остаются на 1 (второго похода в реестр нет).F3: fixed (не wontfix) — троттл форс-перепроверки.
golang.org/x/sync/singleflightуже был в go.mod → коалесинг одновременных?forceодного образа (N параллельных → 1 registry-HEAD) + min-interval 5с (= TTLremoteDigestCache) черезforcedResultCache, кэшируются ТОЛЬКО успехи (ошибка не глушит следующий реальный рекчек). Non-force путь вынес вcomputeImageStatusбайт-в-байт как был (демон + фоновые бейджи не изменились). ТестTestForcedRechecksCollapseToSingleRegistryHead: 16 конкурентных форс-вызовов → ровно 1 HEAD + 1 inspect.-raceчисто.F4: fixed — уведомления ПО КОНТЕЙНЕРУ + имя стека. Контейнер-член стека теперь даёт своё
EventUpdated(по одному на каждый обновлённый контейнер, не одно агрегатное на стек). ВEventдобавилStackName— из лейблаcom.docker.compose.project(безStack().Read). Новый дайджест — best-effort ре-inspect контейнера после redeploy (не доступен дёшево → пустойNewDigest, доставку не блокирую, строка деградирует к «image updated»). Git-стек остаётся detect-only.groupContainersForUpdateпереписан: редеплой стека по-прежнему один, но все члены собираются вStackUpdate.Containers. Итог ровно твой формат:Тесты: пер-контейнерные события стека (2 члена→2 события со StackName+old→new), git-detect-only, формат-сообщения для стек-члена, fallback без нового дайджеста.
Внутренний ревью прошёл (F3 concurrency/cache и F4 пер-контейнер проверены независимо, non-force путь не изменён,
-race/vet/gofmt чисто). Из его suggestions (не блокеры): ре-inspect членов стека идёт последовательно (best-effort, каждый ограничен endpointTimeout, стеки обычно мелкие — принял как есть); singleflight делит панику между ожидающими (для read-only ок). tsc/eslint/vitest — зелёные.Проверки:
go build ./...0;go test ./api/containerautomation/... ./api/docker/images/... ./api/http/handler/... ./api/datastore/...— ok;gofmt/go vetчисто;tsc0;vitest(UpdateNowButton) 9 passed.Ре-ревью
5bb678d3b— коммит с фиксами F1–F4 поверхeb35e9c47. Проверял каждыйfixed:по КОДУ (не по прозе), фан-аут на F3/F4/тесты+регрессии.Вердикт: PASS. Все четыре находки закрыты и подтверждены независимо; регрессий нет. Готово к мержу.
Проверка фиксов
UpdateNowButton.test.tsxгоняет РЕАЛЬНЫЙuseApplyContainerImageUpdate(мокнуты только confirm-диалоги/хуки-соседи и конкретная мутация, не сам хук/компонент): клик → standaloneconfirmContainerRecreation{pullLatest:true}→mutateровно один раз сpullImage:true; stackconfirmStackUpdate{repullImageAndRedeploy:false}→pullImage:false; отмена →mutateне вызван. Ветки дискриминируют разные источники pullImage — non-vacuous.vitest9 passed.TestContainerImageStatusForcedSuccessRepopulatesCache: успешный force (inspect=1/HEAD=1) →statusCacheсодержитUpdated→ последующий NON-force отдаёт из кэша со счётчиками на 1 (реестр повторно не бьётся). УберёшьstatusCache.Set— тест падёт. Non-vacuous.?force=true) — закрыт корректно. Прочиталstatus.goсам: non-force путь вынесен вcomputeImageStatus(...,false)БАЙТ-в-байт (тот жеstatusCache-ключ/TTL,checkStatus(force=false)продолжает переиспользоватьremoteDigestCache) — демон и фоновые бейджи не изменились. Force идёт черезforcedResultCache(min-interval 5с == TTLremoteDigestCache) +singleflightпоimageID;forcedResultCache.Setтолько ПОСЛЕif err != nil { return }— ошибки не кэшируются, следующий реальный рекчек проходит.TestForcedRechecksCollapseToSingleRegistryHead: 16 конкурентных → ровно 1 HEAD+1 inspect (уберёшь coalescing — 16 HEAD, тест падёт). Абьюз-вектор ограничен: 1 HEAD / 5с на образ, ключ по локальномуimageIDне обходится другими ref'ами.updateStackредеплоит стек один раз, затем цикл поst.Containersэмитит ОТДЕЛЬНОЕEventUpdatedна каждый обновлённый контейнер (не одно агрегатное), каждое сContainerName+Image+OldDigest+StackName.StackNameберётся из меткиcom.docker.compose.project(безStack().Read— вformatMessageветкаStackName!=""идёт первой и шунтируетStackID/stackName()). Новый дайджест — best-effort ре-inspect каждого контейнера ПО ЕГО ИМЕНИ (boundedendpointTimeout, пусто→деградирует к «image updated», доставка не блокируется, кросс-атрибуции нет). Git-стек detect-only, standalone-путь без изменений.groupContainersForUpdate(черезstackIndex) держит одинStackUpdateна стек (редеплой один, кандидаты не теряются/не дублируются). Итог — ровно твой формат:Тесты пер-контейнерных стек-событий / git-detect-only / формата / fallback без дайджеста — non-vacuous, пинят именно per-container (не агрегат).
Регрессии — нет
computeImageStatus-извлечение поведение-идентично (единственное отличие — значение одного лог-поля, косметика);Event.StackNameаддитивно (standalone-события его не ставят → старый рендер цел);logNotifierполе игнорит; webhook encoding/RBAC/best-effort прошлого раунда не тронуты (новая веткаStackName!=""добавлена передStackID, существующие пути не задевает).Объективные проверки (запущены в окружении ревью)
tsc --noEmit -p .— exit 0;eslint(UpdateNowButton.test) — чисто;vitest— 9 passed.go build/vet/test+-raceпо F3, зелёные) И тесты F1–F4 независимо вычитаны как non-vacuous (падают при регрессе соответствующей логики).Маркер
reviewed_headобновлён на5bb678d3b.Разделил уведомления по механизмам (запрос vvzvlad: «разные на каждый механизм — хил и апдейт»). Коммит поверх approved-базы, поэтому возвращаю #19 в review.
Было: один общий
Notification.WebhookURLна всю автоматизацию.Стало: два независимо-опциональных URL в настройках:
UpdateWebhookURL— события авто-апдейта (EventUpdated/EventRollback/EventUpdateFailed);HealWebhookURL— авто-хил (EventHealRestarted).Нотификатор роутит по типу события (
webhookURLForKind); пустой URL глушит ТОЛЬКО свой механизм. Задал только update-URL → шлётся только про апдейты, хил молчит; и наоборот; оба заданы → каждый в свой адрес. Тракт доставки не тронут (горутина/recover/таймаут,{{message}}GET-подстановка vs POST-тело, per-container/stack формат). В панели настроек — два поля (Auto-update / Auto-heal webhook URL), каждое валидируется (http/https), плейсхолдеры разные. Golden-миграция обновлена.Проверки:
go build ./...0;go test ./api/containerautomation/... ./api/http/handler/settings/... ./api/datastore/...— ok (incl. migrator golden);tsc0;eslintчисто;vitestNotificationPanel — 8 passed.Внутренний ревью прошёл (маршрутизация полная — все EventKind покрыты, независимая опциональность подтверждена непустыми тестами, тракт доставки/валидация/golden корректны, регрессий нет; APPROVE). Замечания-suggestions принял к сведению: нет data-миграции старого
WebhookURL→новых полей — но фича из неreleased эпика, инсталляций со старым полем нет. Возвращаю review/needs.Split the single container-automation webhook URL into two independently optional URLs — UpdateWebhookURL (fired on update/rollback/update-failed) and HealWebhookURL (fired on auto-heal restart). The notifier routes each event to its mechanism's URL by kind; an empty URL silences only that mechanism, so a user can enable notifications for updates without heal (or vice-versa). Settings gain both fields (each validated http/https, {{message}} allowed), the NotificationPanel exposes two labeled inputs, and the golden migration output is updated. Delivery path (goroutine/recover/timeout, {{message}} GET vs POST, per-container stack message format) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Ре-ревью
492d3d01b— новый коммит поверх approved-5bb678d3b: разделение webhook по механизмам (update vs heal, запрос vvzvlad «разные на каждый механизм»). Merge-scoping: ревьюю только эту дельту (тракт доставки прошлого раунда не тронут). Фан-аут coherence+security / regressions+migration / test-coverage; ключевое сверял по коду сам.Вердикт: PASS. Разделение сделано ровно по запросу, независимая опциональность работает, миграция-переименование безопасна, регрессий нет. Готово к мержу.
Проверка
webhookURLForKindисчерпывающе роутит все 4EventKind:EventUpdated/EventRollback/EventUpdateFailed→UpdateWebhookURL,EventHealRestarted→HealWebhookURL,default → ""(неизвестный kind молча не шлёт). Ни одно update-событие не течёт на heal-URL и наоборот.Notifyчитает URL по kind'у и короткозамыкается на пустом ДО спавна горутины — пустой URL глушит ТОЛЬКО свой механизм. Независимая опциональность (только update / только heal / оба / ни один) подтверждена рантайм-тестами (два httptest-сервера, ассертят что второй НЕ получил запрос).validateNotificationWebhookURL(http/https + непустой host), подAdminAccess; дырки «один валидируется, другой нет» нет. Тракт доставки (deliver/formatMessage:url.QueryEscapeGET / plain POST-body / bounded timeout / recover / без утечки секретов) НЕ тронут — изменилась только СЕЛЕКЦИЯ URL по kind'у. Кросс-маршрутизации сообщений между эндпоинтами нет.WebhookURL→ два поля) — безопасно, проверил сам. Ни одна выпущенная версия не персистилаNotification.WebhookURL: весь блокContainerAutomationвводится миграцией2.43.0(не выпущена, последний тег 2.42.0), аmigrateContainerAutomationSettings_2_43_0бэкфилит ТОЛЬКОAutoHeal/AutoUpdate—WebhookURLвсегда оставался Go-zero-value. Удалять нечего, потери настроенного webhook'а нет.init.go— аддитивные zero-value дефолты обоих полей (value-структ, без nil-deref); goldenoutput_24_to_latest.json— легитимное отражение нового дефолта (Heal/Updateалфавитно, MarshalIndent сортирует ключи), чужие данные не тронуты; десериализация гипотетического legacy-ключа — тихий no-op (оба поля пустые, без паники).*cmp.Or(payload.X, ¤t.X)независимо на поле. Фронт↔бэк совпадают точно:UpdateWebhookURL/HealWebhookURLвportainer.go↔types.ts↔ payload панели.Объективные проверки (запущены в окружении ревью)
tsc --noEmit -p .— exit 0;eslintпо 6 изменённым TS/TSX — чисто;vitest run NotificationPanel/— 8 passed.go build/testincl. migrator golden, зелёные) И тесты (роутинг/валидация/миграция) независимо вычитаны как non-vacuous, миграционное решение проверено по migrator/тегам.Маркер
reviewed_headобновлён на492d3d01b.