Skip to content

webhooks: surface label-defined webhook global selector and PresetCacheTTL through reload merge path #640

@CybotTM

Description

@CybotTM

Follow-up to PR #637 review (Copilot).

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:

  • PresetCacheTTL: mergeWebhookConfigs only copies Webhooks
    and AllowedHosts. PR refactor(config): collapse WebhookGlobalConfig dual-store + align label/INI key naming (#620) #637 originally allow-listed
    webhook-preset-cache-ttl as a Docker label, but values set via
    that label were silently dropped on reconcile because the merge
    path never copies them back to c.WebhookConfigs.Global. The label
    was 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 (and
    ideally any other operator-tunable, non-SSRF webhook global) into
    the live config.

  • 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

  1. 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.
  2. Forward PresetCacheTTL (precedence: INI-set wins over
    label-set).
  3. Re-add webhook-preset-cache-ttl to globalLabelAllowList once 2
    lands. Update docs accordingly.
  4. In iniConfigUpdate, iterate parsed.WebhookConfigs.Webhooks
    and add missing entries (INI source wins on collisions).
  5. 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).

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions