mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-19 16:02:14 +00:00
hugocasa
ee3d82f01f
* fix(native-triggers): serialize Google channel renewal across replicas `sync_all_triggers` runs every 5 minutes on every windmill-app replica with no leader election. Multiple replicas were each rotating the webhook token, creating a new Google watch channel, and racing the trigger UPDATE — leaving the loser's new token (in `token`) and channel (in Google) orphaned. Cloud was accumulating ~5 leaked tokens/week without the silent best-effort `delete_token_by_hash` ever logging a warning. Wrap each per-trigger renewal in a transaction and acquire the row with `SELECT … FOR UPDATE SKIP LOCKED`. Contending replicas skip the row instead of duplicating the work. The lock spans `rotate_webhook_token` → Google API call → `update_native_trigger_service_config` and is only released on commit. Re-checks `should_renew_channel` after acquiring the lock so a replica that committed seconds earlier doesn't trigger a duplicate renewal. The pattern matches existing batch-cleanup paths in `monitor.rs` (job-retention sweep) and other `FOR UPDATE SKIP LOCKED` call sites. Also logs at `debug!` when `delete_token_by_hash` finds no matching row, so future investigations can distinguish "deleted" from "not found" without changing the `Ok(false)` contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup! fix(native-triggers): serialize Google channel renewal across replicas * fixup! fix(native-triggers): serialize Google channel renewal across replicas fixup! fix(native-triggers): serialize Google channel renewal across replicas Address claude review: - #5: per-skip log info -> debug (expected outcome under SKIP LOCKED) - #2: warn moved out of delete_token_by_hash to the call site that knows the expected state (try_renew_channel_locked); other callers are race-prone and shouldn't warn - #3: NULL service_config now warns (anomalous case) - #4: post-Google-API DB-update + commit failures log distinctly so the channel-orphan case is grep-able Plus: add 14d expiry to Google webhook tokens via ServiceName::webhook_token_expiration, mint fresh ephemeral-webhook-{service}-{rd5} labels at create + rotate so the existing 'ephemeral-' filter excludes them from user-token email/critical-alert paths (no filter changes in 3 places). Orphans now self-clean via the existing expiry sweep in monitor.rs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup! fix(native-triggers): serialize Google channel renewal across replicas fixup! fix(native-triggers): serialize Google channel renewal across replicas Address second-round review: - Claude #1 (P2): username_override_from_label now strips the 'ephemeral-' prefix for ephemeral-webhook-* labels, so created_by stays webhook-{service}-{rd5} instead of changing to label-ephemeral-webhook-... (preserves audit/job-list filter compatibility) - Codex (P2): updated renew_channel doc — labels are no longer copied; rotate mints fresh ephemeral-webhook-google-{rd5} with 14d expiration - Claude #3 (optional): test_rotate_webhook_token now asserts the rotated Google token has an ephemeral-webhook-google-* label and a populated expiration Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup! fix(native-triggers): serialize Google channel renewal across replicas fixup! fix(native-triggers): serialize Google channel renewal across replicas Reconsider the previous fixup: stripping the 'ephemeral-' prefix made created_by no longer match token.label exactly, defeating the linking purpose. Just allowlist 'ephemeral-webhook-' alongside the other recognized webhook/email/ws prefixes — created_by becomes ephemeral-webhook-google-XXXXX, matching token.label exactly. The 'ephemeral-' substring also informs operators that this is a system-managed auto-expiring token vs a user-managed webhook trigger. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>