You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Docker label sync paths (mergeWebhookConfigs for initial parse, syncWebhookConfigs for live reload) do not currently forward all the
webhook global state the docs / allow-list would naturally suggest:
webhook-webhooks without label-defined webhooks: mergeWebhookConfigs returns early when len(parsed.Webhooks) == 0, so an operator who sets only ofelia.webhook-webhooks=slack-alerts on a service container
(referencing INI-defined webhooks) sees the value silently dropped.
The global selector merge should happen regardless of per-webhook
label count.
INI-added webhooks via live-reload: iniConfigUpdate parses
the reloaded INI but does not iterate over newly added [webhook "name"] sections to push them into c.WebhookConfigs;
only the embedded global fields benefit from the dual-store
collapse. A newly-added [webhook] block won't take effect
without a restart.
Suggested approach
Split the mergeWebhookConfigs early-return: per-webhook merge
gates on len(parsed.Webhooks) > 0, global-selector merge gates
on parsed.Global.Webhooks != "", ditto for any future
non-SSRF global.
Forward PresetCacheTTL (precedence: INI-set wins over
label-set).
Re-add webhook-preset-cache-ttl to globalLabelAllowList once 2
lands. Update docs accordingly.
In iniConfigUpdate, iterate parsed.WebhookConfigs.Webhooks
and add missing entries (INI source wins on collisions).
Add tests covering each of these paths through the real buildFromDockerContainers / iniConfigUpdate entry points
(the existing parity test exercises the helper directly,
missing the production seam).
Follow-up to PR #637 review (Copilot).
The Docker label sync paths (
mergeWebhookConfigsfor initial parse,syncWebhookConfigsfor live reload) do not currently forward all thewebhook global state the docs / allow-list would naturally suggest:
PresetCacheTTL:mergeWebhookConfigsonly copiesWebhooksand
AllowedHosts. PR refactor(config): collapse WebhookGlobalConfig dual-store + align label/INI key naming (#620) #637 originally allow-listedwebhook-preset-cache-ttlas a Docker label, but values set viathat label were silently dropped on reconcile because the merge
path never copies them back to
c.WebhookConfigs.Global. The labelwas removed from the allow-list in PR refactor(config): collapse WebhookGlobalConfig dual-store + align label/INI key naming (#620) #637 review to match reality;
re-enable it once the merge path forwards
PresetCacheTTL(andideally any other operator-tunable, non-SSRF webhook global) into
the live config.
webhook-webhookswithout label-defined webhooks:mergeWebhookConfigsreturns early whenlen(parsed.Webhooks) == 0, so an operator who sets onlyofelia.webhook-webhooks=slack-alertson a service container(referencing INI-defined webhooks) sees the value silently dropped.
The global selector merge should happen regardless of per-webhook
label count.
INI-added webhooks via live-reload:
iniConfigUpdateparsesthe reloaded INI but does not iterate over newly added
[webhook "name"]sections to push them intoc.WebhookConfigs;only the embedded global fields benefit from the dual-store
collapse. A newly-added
[webhook]block won't take effectwithout a restart.
Suggested approach
mergeWebhookConfigsearly-return: per-webhook mergegates on
len(parsed.Webhooks) > 0, global-selector merge gateson
parsed.Global.Webhooks != "", ditto for any futurenon-SSRF global.
PresetCacheTTL(precedence: INI-set wins overlabel-set).
webhook-preset-cache-ttltoglobalLabelAllowListonce 2lands. Update docs accordingly.
iniConfigUpdate, iterateparsed.WebhookConfigs.Webhooksand add missing entries (INI source wins on collisions).
buildFromDockerContainers/iniConfigUpdateentry points(the existing parity test exercises the helper directly,
missing the production seam).
Related
WebhookGlobalConfigdual-store + align label vs INI key naming #620