refactor(triggers): review followups — fire-now, dedupe trigger pages, hardening
Build / build (push) Failing after 34s
Build / build (push) Failing after 34s
Follow-ups on commit 39e1e36 addressing review feedback from
go-reviewer / security-reviewer / typescript-reviewer.
Backend:
- New POST /api/triggers/{id}/fire (AdminOnly, schedule-only): operator
"Fire now" button — dispatches immediately without waiting for the
next natural interval. Persists last_fired_at BEFORE dispatch, same
ordering as the scheduler. Per-trigger in-flight guard (429 if a
fire is already running) to defend against rapid double-clicks /
runaway scripts. Refuses request when AdminOnly claims are absent
rather than logging an unattributable deploy.
- SetTriggerLastFired now validates timestamp parses as RFC3339 before
writing. Rejects empty string explicitly — empty-clears semantics
were dead (no caller) and would silently re-fire on next tick if
ever accidentally written. A future reset-cadence flow must add a
dedicated ClearTriggerLastFired so the call site is grep-able and
separately auditable.
- Scheduler logs WARN on catch-up fires (now - lastFired > 2× interval)
so the "surprise burst at restart" pattern shows up in audit logs.
- BindingResult reason strings extracted to package consts
(webhook.Reason*) so the scheduler and api fire-now classifications
stay in sync without string-matching drift.
- SECURITY NOTE on FanOutForTrigger documents that the
WebhookRequireSignature gate is ingress-only by design.
Frontend:
- Refactored /triggers/new (770 LOC → 155 LOC) and /triggers/[id]
(~350 LOC dropped) to use the shared TriggerKindForm. Eliminates the
triplicated per-kind state + buildConfig + canSubmit + template that
caused the d-unit regex drift in the prior commit.
- New seedTriggerKindFormState helper on TriggerKindForm primes the
form from a server-returned trigger config with defensive type
guards; resets per-kind slots first so re-seeding across kinds
doesn't inherit stale state.
- /triggers/[id] gains a Schedule status panel with Last Fired + Fire
Now button (gated on binding_count > 0). Confirmation dialog,
result flash, timer cleanup on unmount + new-fire (no stale-closure
race). EN+RU i18n parity.
This commit is contained in:
@@ -318,6 +318,7 @@ func (s *Server) Router() chi.Router {
|
||||
r.Delete("/triggers/{id}", s.deleteTrigger)
|
||||
r.Get("/triggers/{id}/webhook", s.getTriggerWebhook)
|
||||
r.Post("/triggers/{id}/webhook/regenerate", s.regenerateTriggerWebhook)
|
||||
r.Post("/triggers/{id}/fire", s.fireTriggerNow)
|
||||
r.Post("/triggers/{id}/bindings", s.bindWorkloadToTrigger)
|
||||
r.Put("/bindings/{bid}", s.updateBinding)
|
||||
r.Delete("/bindings/{bid}", s.deleteBinding)
|
||||
|
||||
@@ -7,13 +7,27 @@ import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"github.com/alexei/tinyforge/internal/auth"
|
||||
"github.com/alexei/tinyforge/internal/store"
|
||||
"github.com/alexei/tinyforge/internal/webhook"
|
||||
"github.com/alexei/tinyforge/internal/workload/plugin"
|
||||
)
|
||||
|
||||
// fireInFlight tracks trigger IDs that have a fire-now request actively
|
||||
// running so a runaway script or rapid double-click doesn't queue
|
||||
// duplicate deploys. Keyed by trigger ID; entries are added under the
|
||||
// mutex and removed by the handler's defer. Sufficient for an admin
|
||||
// gate — a real rate limiter belongs at the middleware layer, not here.
|
||||
var (
|
||||
fireInFlightMu sync.Mutex
|
||||
fireInFlight = map[string]struct{}{}
|
||||
)
|
||||
|
||||
// triggerView is the response shape for /api/triggers. Webhook secrets
|
||||
// are never serialized — read them via the dedicated /webhook subresource
|
||||
// where the canonical URL is composed.
|
||||
@@ -251,6 +265,126 @@ func (s *Server) getTriggerWebhook(w http.ResponseWriter, r *http.Request) {
|
||||
respondJSON(w, http.StatusOK, view)
|
||||
}
|
||||
|
||||
// fireTriggerNow dispatches a trigger immediately without waiting for
|
||||
// its next natural fire window. Used by the /triggers/[id] "Fire now"
|
||||
// button so an operator can re-test a fixed broken deploy without
|
||||
// waiting one full schedule interval.
|
||||
//
|
||||
// Scope: schedule triggers only. Other kinds (registry / git / manual)
|
||||
// already have their own dispatch paths — registry/git fire on real
|
||||
// inbound events, manual fires from the workload Deploy button. Adding
|
||||
// "fire-now" for those would duplicate those flows without adding new
|
||||
// capability.
|
||||
//
|
||||
// Side effect: updates last_fired_at to "now" (same persist-before-
|
||||
// dispatch ordering the scheduler uses) so the natural next-fire
|
||||
// window shifts forward by exactly the interval. This is the
|
||||
// principle-of-least-surprise behavior — an operator who fires now
|
||||
// is intentionally resetting the cadence.
|
||||
func (s *Server) fireTriggerNow(w http.ResponseWriter, r *http.Request) {
|
||||
id := chi.URLParam(r, "id")
|
||||
|
||||
// Per-trigger in-flight guard. AdminOnly + UI throttle is the only
|
||||
// gate against rapid double-clicks; without this guard a runaway
|
||||
// script could queue parallel fans-out of the same schedule, each
|
||||
// holding up to maxTriggerFanOutConcurrency deployer slots.
|
||||
// Returning 429 lets the client distinguish "already running" from
|
||||
// a real validation error.
|
||||
fireInFlightMu.Lock()
|
||||
if _, busy := fireInFlight[id]; busy {
|
||||
fireInFlightMu.Unlock()
|
||||
respondError(w, http.StatusTooManyRequests,
|
||||
"a fire is already in progress for this trigger")
|
||||
return
|
||||
}
|
||||
fireInFlight[id] = struct{}{}
|
||||
fireInFlightMu.Unlock()
|
||||
defer func() {
|
||||
fireInFlightMu.Lock()
|
||||
delete(fireInFlight, id)
|
||||
fireInFlightMu.Unlock()
|
||||
}()
|
||||
|
||||
trg, err := s.store.GetTriggerByID(id)
|
||||
if err != nil {
|
||||
if errors.Is(err, store.ErrNotFound) {
|
||||
respondNotFound(w, "trigger")
|
||||
return
|
||||
}
|
||||
respondError(w, http.StatusInternalServerError, "failed to load trigger")
|
||||
return
|
||||
}
|
||||
if trg.Kind != "schedule" {
|
||||
respondError(w, http.StatusBadRequest,
|
||||
"fire-now is only supported for schedule triggers")
|
||||
return
|
||||
}
|
||||
|
||||
// AdminOnly middleware guarantees claims; treat their absence as a
|
||||
// boot-time wiring bug rather than fall back to an unattributable
|
||||
// "manual" string that collides with the `manual` trigger kind in
|
||||
// audit logs.
|
||||
claims, ok := auth.ClaimsFromContext(r.Context())
|
||||
if !ok || claims.Username == "" {
|
||||
slog.Error("fire-now: missing claims under AdminOnly", "trigger", trg.Name)
|
||||
respondError(w, http.StatusInternalServerError, "missing auth context")
|
||||
return
|
||||
}
|
||||
actor := claims.Username
|
||||
|
||||
now := time.Now().UTC()
|
||||
if err := s.store.SetTriggerLastFired(trg.ID, now.Format(time.RFC3339)); err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "persist last_fired_at")
|
||||
return
|
||||
}
|
||||
|
||||
evt := plugin.InboundEvent{
|
||||
Kind: "schedule",
|
||||
Schedule: &plugin.ScheduleEvent{FiredAt: now},
|
||||
}
|
||||
results, err := s.webhook.FanOutForTrigger(r.Context(), trg, evt)
|
||||
if err != nil {
|
||||
slog.Warn("fire-now: fan-out failed",
|
||||
"trigger", trg.Name, "actor", actor, "error", err)
|
||||
// Don't expose the raw error — it can carry registry-auth or
|
||||
// compose-stdout bytes (matches the manual-deploy handler).
|
||||
respondError(w, http.StatusInternalServerError, "fire failed; see server logs")
|
||||
return
|
||||
}
|
||||
|
||||
var deployed, errored int
|
||||
for _, b := range results {
|
||||
switch {
|
||||
case b.Deployed:
|
||||
deployed++
|
||||
case b.Reason == webhook.ReasonBindingDisabled, b.Reason == webhook.ReasonNoMatch:
|
||||
// silent
|
||||
default:
|
||||
errored++
|
||||
}
|
||||
}
|
||||
// Empty fan-out (no bindings) is almost certainly an operator
|
||||
// mistake — the UI button is gated on binding_count>0, but the
|
||||
// counts can change between page load and click. Warn so the
|
||||
// no-op shows up in audit logs.
|
||||
if len(results) == 0 {
|
||||
slog.Warn("fire-now: no bindings to fire",
|
||||
"trigger", trg.Name, "actor", actor)
|
||||
} else {
|
||||
slog.Info("fire-now dispatched",
|
||||
"trigger", trg.Name, "actor", actor,
|
||||
"bindings", len(results), "deployed", deployed, "errored", errored)
|
||||
}
|
||||
|
||||
respondJSON(w, http.StatusAccepted, map[string]any{
|
||||
"trigger": trg.Name,
|
||||
"fired_at": now.Format(time.RFC3339),
|
||||
"bindings": len(results),
|
||||
"deployed": deployed,
|
||||
"errored": errored,
|
||||
})
|
||||
}
|
||||
|
||||
func (s *Server) regenerateTriggerWebhook(w http.ResponseWriter, r *http.Request) {
|
||||
id := chi.URLParam(r, "id")
|
||||
secret := generateWebhookSecret()
|
||||
|
||||
Reference in New Issue
Block a user