5 Commits
Author SHA1 Message Date
clawbot 929bb35bb0 Say what each action did in a one-line notice (closes #383)
check / check (push) Successful in 3m15s
Every action on the webhook pages, and signing out, redirects with a
fixed notice code in the URL, and the page it lands on shows one line
saying what was done. noticeFor maps codes to fixed text and an
unknown code shows nothing, so nothing from the URL is echoed. One
partial, templates/notice.html, shows the line under the navbar on
every page; the error page shows none.

Replay and resubmit use the same codes and partial in place of their
own query parameters and event log banners.

Model: opus-5-5
2026-10-02 08:16:21 +00:00
clawbot b78abdc9da Drop TODO.md's stale docs-only cache caveat (closes #421)
check / check (push) Successful in 3m11s
Since the build context carries .git and the CI fingerprint is the hash of the commit being checked, every commit's check runs the build, docs-only commits included. TODO.md's caveat that a docs-only commit replays from the layer cache was no longer true, and the README's "CI gate honesty" section already says how a check runs, so the paragraph is removed.

Model: opus-5-5
2026-10-02 10:14:40 +02:00
clawbot c23ffbac65 Widen the webhook page by half so an entrypoint URL fits on one line (closes #350)
check / check (push) Successful in 3m16s
The webhook page's maximum width goes from 72rem (1152 px) to 108rem (1728 px), half again as wide, so an entrypoint URL stays on one line in 1920- and 1440-pixel windows; the statistics pane and both columns widen with it. The title row now wraps, so a phone-width window no longer scrolls sideways.

The width is an inline style: static/css/style.css is linked by no page, and the committed Tailwind stylesheet has no class that wide. The webhook list, the event log, the navbar and the footer stay at 72rem.

Model: opus-5-5
2026-10-02 09:49:33 +02:00
clawbot 2ac4d4d793 Send the build version in the outbound User-Agent (closes #313)
check / check (push) Successful in 3m25s
The http and slack targets sent the constant User-Agent webhooker/1.0. They now send webhooker/ followed by the version the build stamped, the same value the footer shows, built in one place on the delivery engine. User-Agent stays a reserved header and is still set after the target's configured headers, so a configured one cannot override it. Tests check the header each target sends against a known version, and that a configured User-Agent is replaced.

Model: opus-5-5
2026-10-02 09:48:34 +02:00
clawbot eb4c4cc849 Serve /metrics from a registry of its own (closes #227)
check / check (push) Successful in 3m25s
A second metrics-enabled router in one process panicked on a duplicate collector registration, because every collector registered on Prometheus's global default registry. metrics.NewRegistry now builds one registry with the Go runtime and process collectors; fx provides it and the delivery metric set built on it. The middleware builds its HTTP recorder once on that registry (NewForTest on a fresh one), the engine and handlers take the metric set from fx, and nothing registers on the global default any more.

/metrics is served from the new registry with the same series names, labels and auth. A test builds two metrics-enabled routers in one process.

Model: opus-5-5
2026-10-02 09:06:20 +02:00
38 changed files with 678 additions and 297 deletions
+8 -6
View File
@@ -2951,13 +2951,15 @@ Components are wired via Uber fx in this order:
7. `healthcheck.New` — Health check service 7. `healthcheck.New` — Health check service
8. `session.New` — Cookie-based session manager (key from database) 8. `session.New` — Cookie-based session manager (key from database)
9. `handlers.New` — HTTP handlers 9. `handlers.New` — HTTP handlers
10. `middleware.New` — HTTP middleware 10. `metrics.NewRegistry` — The registry `/metrics` serves
11. `delivery.New` — Event-driven delivery engine 11. `metrics.New` — The delivery collectors, registered on that registry
12. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives 12. `middleware.New` — HTTP middleware
13. `delivery.Engine` → `delivery.Notifier` — interface bridge 13. `delivery.New` — Event-driven delivery engine
14. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so 14. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
15. `delivery.Engine` → `delivery.Notifier` — interface bridge
16. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so
deleting a webhook releases its archive writer deleting a webhook releases its archive writer
15. `server.New` — HTTP server and router 17. `server.New` — HTTP server and router
The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine, The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine,
*database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which *database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which
-6
View File
@@ -40,12 +40,6 @@ duplicate. That is deliberate — the alternative is a silent lost
delivery — and the README says so under Rationale. It is not a defect delivery — and the README says so under Rationale. It is not a defect
to re-file. to re-file.
One caveat on reading a green check: a docs-only commit deliberately
replays from the layer cache
(https://git.eeqj.de/sneak/webhooker/issues/119), so a green status on
such a commit evidences a replay rather than an executed run. A code
commit invalidates the `COPY` layer and genuinely executes.
# Next Step # Next Step
Clear the rest of the open 1.0.0 milestone Clear the rest of the open 1.0.0 milestone
+5
View File
@@ -16,6 +16,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/resetpw" "sneak.berlin/go/webhooker/internal/resetpw"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
@@ -177,6 +178,10 @@ func newApp() *fx.App {
healthcheck.New, healthcheck.New,
session.New, session.New,
handlers.New, handlers.New,
// The registry /metrics serves, and the delivery
// collectors registered on it.
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
// The one SSRF guard both target-creation validation // The one SSRF guard both target-creation validation
// and the delivery dialer consult, so they cannot // and the delivery dialer consult, so they cannot
+20 -5
View File
@@ -14,6 +14,7 @@ import (
"go.uber.org/fx" "go.uber.org/fx"
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/lifecycle" "sneak.berlin/go/webhooker/internal/lifecycle"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics" "sneak.berlin/go/webhooker/internal/metrics"
@@ -146,8 +147,10 @@ type EngineParams struct {
DB *database.Database DB *database.Database
DBManager *database.WebhookDBManager DBManager *database.WebhookDBManager
Globals *globals.Globals
Logger *logger.Logger Logger *logger.Logger
SSRFGuard *Guard SSRFGuard *Guard
Metrics *metrics.Set
} }
// Engine processes queued deliveries in the background // Engine processes queued deliveries in the background
@@ -167,10 +170,14 @@ type Engine struct {
retryCh chan Task retryCh chan Task
workers int workers int
// mtr is the delivery metric set. Production wires the // version is the running build's version, the one the web UI
// process-wide one; a test can substitute a set registered on // footer shows. userAgent puts it on every outbound request.
// a private registry so its assertions are not disturbed by version string
// deliveries other tests are making at the same time.
// mtr is the delivery metric set. Production wires the one
// registered on the registry /metrics serves; a test can
// substitute a set registered on a registry it holds, so it can
// gather what its own deliveries recorded.
mtr *metrics.Set mtr *metrics.Set
// targets maps each target type to its implementation. // targets maps each target type to its implementation.
@@ -204,7 +211,8 @@ func New(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: defaultWorkers, workers: defaultWorkers,
mtr: metrics.Default(), version: params.Globals.Version,
mtr: params.Metrics,
} }
e.initTargets(&http.Client{ e.initTargets(&http.Client{
@@ -300,6 +308,13 @@ func (e *Engine) ScheduleRetry(
}) })
} }
// userAgent is the User-Agent header of every http and slack
// delivery request: the program name and the running build's
// version.
func (e *Engine) userAgent() string {
return "webhooker/" + e.version
}
// registerHooks wires the engine's start and stop into the fx // registerHooks wires the engine's start and stop into the fx
// lifecycle. The start hook's context is deliberately ignored // lifecycle. The start hook's context is deliberately ignored
// (see start for why the worker pool must not inherit it); the // (see start for why the worker pool must not inherit it); the
+1 -5
View File
@@ -1247,11 +1247,6 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
testContentType, testContentType,
receivedHeaders.Get("Content-Type"), receivedHeaders.Get("Content-Type"),
) )
assert.Equal(t,
"webhooker/1.0",
receivedHeaders.Get("User-Agent"),
)
} }
// The event's stored inbound headers carry the same Content-Type the // The event's stored inbound headers carry the same Content-Type the
@@ -1320,6 +1315,7 @@ func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
ContentType: tc.event, ContentType: tc.event,
}, },
cfg, cfg,
"webhooker/dev",
) )
assert.Equal(t, assert.Equal(t,
+7 -6
View File
@@ -9,6 +9,7 @@ import (
"net/url" "net/url"
"time" "time"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx" "go.uber.org/fx"
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
@@ -82,8 +83,9 @@ func ExportApplyRequestHeaders(
req *http.Request, req *http.Request,
event *database.Event, event *database.Event,
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
userAgent string,
) []string { ) []string {
return applyRequestHeaders(req, event, cfg) return applyRequestHeaders(req, event, cfg, userAgent)
} }
// ExportTruncate exposes truncate for testing. // ExportTruncate exposes truncate for testing.
@@ -399,7 +401,7 @@ func NewTestEngine(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: workers, workers: workers,
mtr: metrics.Default(), mtr: metrics.New(prometheus.NewRegistry()),
} }
e.initTargets(client) e.initTargets(client)
@@ -414,7 +416,7 @@ func NewTestEngineSmallRetry(
e := &Engine{ e := &Engine{
log: log, log: log,
retryCh: make(chan Task, 1), retryCh: make(chan Task, 1),
mtr: metrics.Default(), mtr: metrics.New(prometheus.NewRegistry()),
} }
e.initTargets(nil) e.initTargets(nil)
@@ -437,7 +439,7 @@ func NewTestEngineWithDB(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: workers, workers: workers,
mtr: metrics.Default(), mtr: metrics.New(prometheus.NewRegistry()),
} }
e.initTargets(client) e.initTargets(client)
@@ -445,8 +447,7 @@ func NewTestEngineWithDB(
} }
// ExportSetMetrics substitutes the engine's metric set, so a test can // ExportSetMetrics substitutes the engine's metric set, so a test can
// assert on collectors registered on a private registry instead of // assert on collectors registered on a registry it holds.
// the process-wide ones every other test is also moving.
func (e *Engine) ExportSetMetrics(mtr *metrics.Set) { func (e *Engine) ExportSetMetrics(mtr *metrics.Set) {
e.mtr = mtr e.mtr = mtr
} }
+2 -3
View File
@@ -35,9 +35,8 @@ const (
) )
// mIsolate gives the setup's engine a metric set registered on a // mIsolate gives the setup's engine a metric set registered on a
// private registry. The process-wide collectors are moved by every // registry this test holds, so its exact assertions can gather from
// other delivery test running in parallel, so exact assertions are // it.
// only possible against a registry this test owns.
func mIsolate( func mIsolate(
t *testing.T, s iSetup, t *testing.T, s iSetup,
) *prometheus.Registry { ) *prometheus.Registry {
+1
View File
@@ -375,6 +375,7 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
"Content-Type": testContentType, "Content-Type": testContentType,
}, },
}, },
"webhooker/dev",
) )
assert.Equal(t, assert.Equal(t,
+7 -2
View File
@@ -442,7 +442,9 @@ func (t *httpTarget) doHTTPRequest(
) )
} }
originScoped := applyRequestHeaders(req, event, cfg) originScoped := applyRequestHeaders(
req, event, cfg, t.eng.userAgent(),
)
client := t.clientForRequest(cfg, originScoped) client := t.clientForRequest(cfg, originScoped)
@@ -562,10 +564,13 @@ func isForwardableHeader(name string) bool {
// Content-Type goes out once: a Content-Type configured on the target // Content-Type goes out once: a Content-Type configured on the target
// wins, otherwise the event's ContentType, otherwise none. The inbound // wins, otherwise the event's ContentType, otherwise none. The inbound
// Content-Type in the event's headers is never forwarded. // Content-Type in the event's headers is never forwarded.
//
// userAgent is set last, over any configured or inbound User-Agent.
func applyRequestHeaders( func applyRequestHeaders(
req *http.Request, req *http.Request,
event *database.Event, event *database.Event,
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
userAgent string,
) []string { ) []string {
if event.ContentType != "" { if event.ContentType != "" {
req.Header.Set( req.Header.Set(
@@ -580,7 +585,7 @@ func applyRequestHeaders(
originScoped[http.CanonicalHeaderKey(k)] = struct{}{} originScoped[http.CanonicalHeaderKey(k)] = struct{}{}
} }
req.Header.Set("User-Agent", "webhooker/1.0") req.Header.Set("User-Agent", userAgent)
// A Content-Type configured on the target describes the body // A Content-Type configured on the target describes the body
// being sent rather than the sender. A 307/308 preserves the // being sent rather than the sender. A 307/308 preserves the
+1 -1
View File
@@ -136,7 +136,7 @@ func (t *slackTarget) attempt(
} }
req.Header.Set("Content-Type", "application/json") req.Header.Set("Content-Type", "application/json")
req.Header.Set("User-Agent", "webhooker/1.0") req.Header.Set("User-Agent", t.eng.userAgent())
resp, doErr := executeHTTPRequest(t.client, req) resp, doErr := executeHTTPRequest(t.client, req)
durationMs := time.Since(start).Milliseconds() durationMs := time.Since(start).Milliseconds()
+91
View File
@@ -0,0 +1,91 @@
package delivery_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/netip"
"testing"
"github.com/google/uuid"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
)
// Both the http and the slack target send webhooker/ and the version
// in Globals, the value the web UI footer shows. A User-Agent
// configured on the target or carried in by the sender does not
// replace it.
func TestUserAgent_IsTheBuildVersion(t *testing.T) {
t.Parallel()
const want = "webhooker/1.2.3-test"
userAgents := make(chan string, 1)
ts := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
userAgents <- r.Header.Get("User-Agent")
w.WriteHeader(http.StatusOK)
},
))
defer ts.Close()
g := &globals.Globals{Version: "1.2.3-test"}
lc := fxtest.NewLifecycle(t)
log, err := logger.New(lc, logger.LoggerParams{Globals: g})
require.NoError(t, err)
e := delivery.New(lc, delivery.EngineParams{
Globals: g,
Logger: log,
// httptest listens on loopback, which the default guard
// refuses.
SSRFGuard: delivery.NewTestGuard(
netip.MustParsePrefix("127.0.0.0/8"),
),
Metrics: metrics.New(prometheus.NewRegistry()),
})
statusCode, _, _, err := e.ExportDoHTTPRequest(
context.Background(),
&delivery.HTTPTargetConfig{
URL: ts.URL,
Headers: map[string]string{"User-Agent": "configured/1"},
},
&database.Event{Headers: `{"User-Agent":["curl/8"]}`},
)
require.NoError(t, err)
require.Equal(t, http.StatusOK, statusCode)
require.Len(t, userAgents, 1, "the http target sent no request")
assert.Equal(t, want, <-userAgents, "http target")
db := testWebhookDB(t)
targetID := uuid.New().String()
slackCfg, err := json.Marshal(
delivery.SlackTargetConfig{WebhookURL: ts.URL},
)
require.NoError(t, err)
event := seedEvent(t, db, `{"action":"test"}`)
dlv := seedDelivery(
t, db, event.ID, targetID, database.DeliveryStatusPending,
)
e.ExportDeliverSlack(context.Background(), db, buildSlackDelivery(
dlv, event, targetID, "test-slack", string(slackCfg),
))
require.Len(t, userAgents, 1, "the slack target sent no request")
assert.Equal(t, want, <-userAgents, "slack target")
}
+4 -2
View File
@@ -333,7 +333,9 @@ func (h *Handlers) HandleLogout() http.HandlerFunc {
) )
} }
// Redirect to login page http.Redirect(
http.Redirect(w, r, "/pages/login", http.StatusSeeOther) w, r, withNotice("/pages/login", signedOut),
http.StatusSeeOther,
)
} }
} }
+19 -55
View File
@@ -11,72 +11,37 @@ import (
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
) )
// replayOutcomeParam is the query parameter the replay POST redirects // The outcomes of a replay POST, as the notice codes its redirect
// with and the event log page reads its banner from. // carries. noticeFor holds the line each one shows.
const replayOutcomeParam = "replay"
// replayOutcomeCode is the outcome of a replay POST. The redirect
// carries one of these fixed codes rather than a message, so nothing a
// client submits can reach the rendered page through it.
type replayOutcomeCode string
const ( const (
// replayQueued reports that a new delivery was created and handed // replayQueued reports that a new delivery was created and handed
// to the delivery engine. // to the delivery engine.
replayQueued replayOutcomeCode = "queued" replayQueued noticeCode = "replay-queued"
// replayTargetDeleted reports a target that once existed and has // replayTargetDeleted reports a target that once existed and has
// since been deleted. Deletes are soft and deliveries carry no // since been deleted. Deletes are soft and deliveries carry no
// foreign key to the target row, so the history survives its // foreign key to the target row, so the history survives its
// target and this is the ordinary case for an old event. // target and this is the ordinary case for an old event.
replayTargetDeleted replayOutcomeCode = "target-deleted" replayTargetDeleted noticeCode = "replay-target-deleted"
// replayTargetMissing reports a target id that names no row at // replayTargetMissing reports a target id that names no row at
// all, deleted or otherwise. // all, deleted or otherwise.
replayTargetMissing replayOutcomeCode = "target-missing" replayTargetMissing noticeCode = "replay-target-missing"
// replayTargetInactive reports a target the operator has // replayTargetInactive reports a target the operator has
// deactivated. A deactivated target receives no new deliveries, so // deactivated. A deactivated target receives no new deliveries, so
// a replay to it would be a delivery they switched off. // a replay to it would be a delivery they switched off.
replayTargetInactive replayOutcomeCode = "target-inactive" replayTargetInactive noticeCode = "replay-target-inactive"
// replayNotTerminal reports a delivery the engine has not finished // replayNotTerminal reports a delivery the engine has not finished
// with. // with.
replayNotTerminal replayOutcomeCode = "not-terminal" replayNotTerminal noticeCode = "replay-not-terminal"
// replayInFlight reports that an earlier replay of this event to // replayInFlight reports that an earlier replay of this event to
// this target is still running. // this target is still running.
replayInFlight replayOutcomeCode = "in-flight" replayInFlight noticeCode = "replay-in-flight"
) )
// replayOutcome returns the banner the event log page shows for an
// outcome code, and whether the replay was queued. An unrecognised
// code yields no banner.
func replayOutcome(code string) (string, bool) {
switch replayOutcomeCode(code) {
case replayQueued:
return "Replay queued: a new delivery was created against " +
"the target's current configuration.", true
case replayTargetDeleted:
return "Not replayed: the target this delivery was for has " +
"been deleted. Recreate the target, then replay.", false
case replayTargetMissing:
return "Not replayed: the target this delivery was for no " +
"longer exists.", false
case replayTargetInactive:
return "Not replayed: the target this delivery was for is " +
"deactivated. Activate it, then replay.", false
case replayNotTerminal:
return "Not replayed: this delivery has not finished yet.",
false
case replayInFlight:
return "Not replayed: a delivery of this event to this " +
"target is already in flight.", false
default:
return "", false
}
}
// HandleDeliveryReplay re-sends a finished delivery's event to its // HandleDeliveryReplay re-sends a finished delivery's event to its
// target. // target.
// //
@@ -140,14 +105,14 @@ func (h *Handlers) replayDelivery(
} }
if !original.Status.Terminal() { if !original.Status.Terminal() {
h.finishReplay(w, r, webhook, replayNotTerminal) redirectToEventLog(w, r, webhook, replayNotTerminal)
return return
} }
target, code := h.replayTarget(webhook.ID, original.TargetID) target, code := h.replayTarget(webhook.ID, original.TargetID)
if target == nil { if target == nil {
h.finishReplay(w, r, webhook, code) redirectToEventLog(w, r, webhook, code)
return return
} }
@@ -200,7 +165,7 @@ func (h *Handlers) queueReplay(
} }
if inFlight > 0 { if inFlight > 0 {
h.finishReplay(w, r, webhook, replayInFlight) redirectToEventLog(w, r, webhook, replayInFlight)
return return
} }
@@ -238,7 +203,7 @@ func (h *Handlers) queueReplay(
"delivery_id", task.DeliveryID, "delivery_id", task.DeliveryID,
) )
h.finishReplay(w, r, webhook, replayQueued) redirectToEventLog(w, r, webhook, replayQueued)
} }
// replayTarget loads the delivery's target as it stands now. // replayTarget loads the delivery's target as it stands now.
@@ -251,7 +216,7 @@ func (h *Handlers) queueReplay(
// with the returned code saying why. // with the returned code saying why.
func (h *Handlers) replayTarget( func (h *Handlers) replayTarget(
webhookID, targetID string, webhookID, targetID string,
) (*database.Target, replayOutcomeCode) { ) (*database.Target, noticeCode) {
var target database.Target var target database.Target
err := h.db.DB().Unscoped().Where( err := h.db.DB().Unscoped().Where(
@@ -361,17 +326,16 @@ func replayBody(body string) *string {
return &body return &body
} }
// finishReplay redirects back to the event log the replay was // redirectToEventLog redirects a replay or resubmit back to the event
// triggered from, carrying the outcome code the page turns into a // log it was triggered from, carrying the outcome as its notice and
// banner and the page number the form submitted. // the page number the form submitted.
func (h *Handlers) finishReplay( func redirectToEventLog(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
webhook database.Webhook, webhook database.Webhook,
code replayOutcomeCode, code noticeCode,
) { ) {
dest := "/hook/" + webhook.ID + "/events?" + dest := withNotice("/hook/"+webhook.ID+"/events", code)
replayOutcomeParam + "=" + string(code)
// The page is read from the form rather than the query string: // The page is read from the form rather than the query string:
// this is a POST, and its query string is what logs and Referer // this is a POST, and its query string is what logs and Referer
+8 -8
View File
@@ -212,7 +212,7 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?replay=queued", "/hook/"+wh.ID+"/events?notice=replay-queued",
w.Header().Get("Location"), w.Header().Get("Location"),
) )
@@ -362,7 +362,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?replay=target-deleted", "/hook/"+wh.ID+"/events?notice=replay-target-deleted",
w.Header().Get("Location"), w.Header().Get("Location"),
) )
@@ -390,7 +390,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
require.Equal(t, http.StatusSeeOther, missing.Code) require.Equal(t, http.StatusSeeOther, missing.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?replay=target-missing", "/hook/"+wh.ID+"/events?notice=replay-target-missing",
missing.Header().Get("Location"), missing.Header().Get("Location"),
) )
} }
@@ -431,7 +431,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
require.Equal(t, http.StatusSeeOther, first.Code) require.Equal(t, http.StatusSeeOther, first.Code)
require.Equal( require.Equal(
t, t,
"/hook/"+wh.ID+"/events?replay=queued", "/hook/"+wh.ID+"/events?notice=replay-queued",
first.Header().Get("Location"), first.Header().Get("Location"),
) )
@@ -439,7 +439,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
require.Equal(t, http.StatusSeeOther, second.Code) require.Equal(t, http.StatusSeeOther, second.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?replay=in-flight", "/hook/"+wh.ID+"/events?notice=replay-in-flight",
second.Header().Get("Location"), second.Header().Get("Location"),
) )
@@ -465,7 +465,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
require.Equal(t, http.StatusSeeOther, pending.Code) require.Equal(t, http.StatusSeeOther, pending.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?replay=not-terminal", "/hook/"+wh.ID+"/events?notice=replay-not-terminal",
pending.Header().Get("Location"), pending.Header().Get("Location"),
) )
} }
@@ -509,7 +509,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
assert.Contains(t, body, ">Replay<") assert.Contains(t, body, ">Replay<")
refused := renderSourceLogsPageWithQuery( refused := renderSourceLogsPageWithQuery(
t, h, sess, wh.ID, "?replay=target-deleted", t, h, sess, wh.ID, "?notice=replay-target-deleted",
) )
assert.Contains(t, refused, "alert-error") assert.Contains(t, refused, "alert-error")
@@ -517,7 +517,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
// An outcome code nobody issued renders no banner at all. // An outcome code nobody issued renders no banner at all.
unknown := renderSourceLogsPageWithQuery( unknown := renderSourceLogsPageWithQuery(
t, h, sess, wh.ID, "?replay=made-up", t, h, sess, wh.ID, "?notice=made-up",
) )
assert.NotContains(t, unknown, "alert-error") assert.NotContains(t, unknown, "alert-error")
+5 -54
View File
@@ -3,7 +3,6 @@ package handlers
import ( import (
"errors" "errors"
"net/http" "net/http"
"strconv"
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/google/uuid" "github.com/google/uuid"
@@ -11,43 +10,19 @@ import (
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
) )
// resubmitOutcomeParam is the query parameter the resubmit POST // The outcomes of a resubmit POST, as the notice codes its redirect
// redirects with and the event log page reads its banner from. // carries. noticeFor holds the line each one shows.
const resubmitOutcomeParam = "resubmit"
// resubmitOutcomeCode is the outcome of a resubmit POST. The redirect
// carries one of these fixed codes rather than a message, so nothing a
// client submits can reach the rendered page through it.
type resubmitOutcomeCode string
const ( const (
// resubmitQueued reports that a new event was stored and its // resubmitQueued reports that a new event was stored and its
// deliveries handed to the delivery engine. // deliveries handed to the delivery engine.
resubmitQueued resubmitOutcomeCode = "queued" resubmitQueued noticeCode = "resubmit-queued"
// resubmitNoTargets reports a source with no active targets. The // resubmitNoTargets reports a source with no active targets. The
// new event is stored either way, exactly as a received event // new event is stored either way, exactly as a received event
// with no targets is. // with no targets is.
resubmitNoTargets resubmitOutcomeCode = "no-targets" resubmitNoTargets noticeCode = "resubmit-no-targets"
) )
// resubmitOutcome returns the banner the event log page shows for an
// outcome code, and whether the resubmit was queued. An unrecognised
// code yields no banner.
func resubmitOutcome(code string) (string, bool) {
switch resubmitOutcomeCode(code) {
case resubmitQueued:
return "Resubmitted: a new event was created from the stored " +
"one and queued to every active target.", true
case resubmitNoTargets:
return "Resubmitted: a new event was created, but this " +
"source has no active targets, so nothing was queued.",
true
default:
return "", false
}
}
// resubmitSource is the stored event a resubmit copies. Its body is // resubmitSource is the stored event a resubmit copies. Its body is
// read as bytes rather than as a string so the copy is byte-identical // read as bytes rather than as a string so the copy is byte-identical
// to what was received, whatever the payload's encoding. // to what was received, whatever the payload's encoding.
@@ -245,29 +220,5 @@ func (h *Handlers) queueResubmit(
code = resubmitNoTargets code = resubmitNoTargets
} }
h.finishResubmit(w, r, webhook, code) redirectToEventLog(w, r, webhook, code)
}
// finishResubmit redirects back to the event log the resubmit was
// triggered from, carrying the outcome code the page turns into a
// banner and the page number the form submitted.
func (h *Handlers) finishResubmit(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
code resubmitOutcomeCode,
) {
dest := "/hook/" + webhook.ID + "/events?" +
resubmitOutcomeParam + "=" + string(code)
// The page is read from the form rather than the query string:
// this is a POST, and its query string is what logs and Referer
// headers record.
if page := pageOrFirst(
r.PostFormValue("page"),
); page > 1 {
dest += "&page=" + strconv.Itoa(page)
}
http.Redirect(w, r, dest, http.StatusSeeOther)
} }
+4 -4
View File
@@ -154,7 +154,7 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?resubmit=queued", "/hook/"+wh.ID+"/events?notice=resubmit-queued",
w.Header().Get("Location"), w.Header().Get("Location"),
) )
@@ -282,7 +282,7 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?resubmit=queued", "/hook/"+wh.ID+"/events?notice=resubmit-queued",
w.Header().Get("Location"), w.Header().Get("Location"),
"a resubmit must not be refused while an earlier "+ "a resubmit must not be refused while an earlier "+
"one is in flight", "one is in flight",
@@ -436,7 +436,7 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?resubmit=queued", "/hook/"+wh.ID+"/events?notice=resubmit-queued",
w.Header().Get("Location"), w.Header().Get("Location"),
"an inactive target is skipped, not an error", "an inactive target is skipped, not an error",
) )
@@ -482,7 +482,7 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal( assert.Equal(
t, t,
"/hook/"+wh.ID+"/events?resubmit=no-targets", "/hook/"+wh.ID+"/events?notice=resubmit-no-targets",
w.Header().Get("Location"), w.Header().Get("Location"),
) )
+21 -9
View File
@@ -12,6 +12,7 @@ import (
"net/http" "net/http"
"sync/atomic" "sync/atomic"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
@@ -64,6 +65,8 @@ type HandlersParams struct {
Notifier delivery.Notifier Notifier delivery.Notifier
Evictor delivery.WebhookEvictor Evictor delivery.WebhookEvictor
SSRFGuard *delivery.Guard SSRFGuard *delivery.Guard
Metrics *metrics.Set
Registry *prometheus.Registry
} }
// Handlers provides HTTP handler methods for all application // Handlers provides HTTP handler methods for all application
@@ -94,10 +97,10 @@ type Handlers struct {
// parsePageTemplate parses a page-specific template set from the // parsePageTemplate parses a page-specific template set from the
// embedded FS. Each page template is combined with the shared // embedded FS. Each page template is combined with the shared
// base, htmlheader, and navbar templates, and with any further files // base, htmlheader, navbar and notice templates, and with any further
// the page includes. The page file must be listed first so that its // files the page includes. The page file must be listed first so that
// root action ({{template "base" .}}) becomes the template set's entry // its root action ({{template "base" .}}) becomes the template set's
// point. // entry point.
func parsePageTemplate( func parsePageTemplate(
pageFile string, included ...string, pageFile string, included ...string,
) *template.Template { ) *template.Template {
@@ -106,6 +109,7 @@ func parsePageTemplate(
"base.html", "base.html",
"htmlheader.html", "htmlheader.html",
"navbar.html", "navbar.html",
"notice.html",
}, included...) }, included...)
return template.Must( return template.Must(
@@ -129,7 +133,7 @@ func New(
s.mw = params.Middleware s.mw = params.Middleware
s.notifier = params.Notifier s.notifier = params.Notifier
s.evictor = params.Evictor s.evictor = params.Evictor
s.mtr = metrics.Default() s.mtr = params.Metrics
s.ssrf = params.SSRFGuard s.ssrf = params.SSRFGuard
// Parse all page templates once at startup // Parse all page templates once at startup
@@ -206,11 +210,13 @@ func (s *Handlers) renderError(
// served outside the routes where NoCache runs. // served outside the routes where NoCache runs.
w.Header().Set("Cache-Control", "no-store") w.Header().Set("Cache-Control", "no-store")
// No notice: one would say an action worked above a page saying
// the request failed.
data := s.pageData(r, map[string]any{ data := s.pageData(r, map[string]any{
"Status": status, "Status": status,
"StatusText": http.StatusText(status), "StatusText": http.StatusText(status),
"Message": errorPageText(status), "Message": errorPageText(status),
}) }, nil)
var buf bytes.Buffer var buf bytes.Buffer
@@ -264,6 +270,7 @@ type templateDataWrapper struct {
User *UserInfo User *UserInfo
CSRFToken string CSRFToken string
Version string Version string
Notice *notice
Data any Data any
} }
@@ -308,12 +315,15 @@ func (s *Handlers) renderTemplate(
return return
} }
s.executeTemplate(w, r, tmpl, s.pageData(r, data)) s.executeTemplate(w, r, tmpl, s.pageData(r, data, noticeFor(r)))
} }
// pageData adds the fields the shared layout renders to a page's own // pageData adds the fields the shared layout renders to a page's own
// data. // data. The layout shows the notice, when there is one, above the
func (s *Handlers) pageData(r *http.Request, data any) any { // page.
func (s *Handlers) pageData(
r *http.Request, data any, pageNotice *notice,
) any {
userInfo := s.getUserInfo(r) userInfo := s.getUserInfo(r)
csrfToken := middleware.CSRFToken(r) csrfToken := middleware.CSRFToken(r)
@@ -327,6 +337,7 @@ func (s *Handlers) pageData(r *http.Request, data any) any {
m["User"] = userInfo m["User"] = userInfo
m["CSRFToken"] = csrfToken m["CSRFToken"] = csrfToken
m["Version"] = version m["Version"] = version
m["Notice"] = pageNotice
return m return m
} }
@@ -335,6 +346,7 @@ func (s *Handlers) pageData(r *http.Request, data any) any {
User: userInfo, User: userInfo,
CSRFToken: csrfToken, CSRFToken: csrfToken,
Version: version, Version: version,
Notice: pageNotice,
Data: data, Data: data,
} }
} }
+3
View File
@@ -20,6 +20,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
@@ -109,6 +110,8 @@ func newTestApp(
func(r *recordingEvictor) delivery.WebhookEvictor { func(r *recordingEvictor) delivery.WebhookEvictor {
return r return r
}, },
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
delivery.NewGuard, delivery.NewGuard,
handlers.New, handlers.New,
+21
View File
@@ -0,0 +1,21 @@
package handlers
import (
"net/http"
"github.com/prometheus/client_golang/prometheus/promhttp"
)
// HandleMetrics returns the Prometheus scrape handler for the
// registry built by metrics.NewRegistry, which the HTTP, delivery, Go
// runtime and process collectors register on. It is what
// promhttp.Handler builds for the global default registry, including
// the promhttp_metric_handler_* series that count scrapes, pointed at
// that registry instead.
func (s *Handlers) HandleMetrics() http.HandlerFunc {
reg := s.params.Registry
return promhttp.InstrumentMetricHandler(
reg, promhttp.HandlerFor(reg, promhttp.HandlerOpts{}),
).ServeHTTP
}
+109
View File
@@ -0,0 +1,109 @@
package handlers
import "net/http"
// noticeParam is the query parameter an action's redirect carries its
// notice code in.
const noticeParam = "notice"
// noticeCode names one of the fixed lines noticeFor knows. An action
// redirects with the code rather than the line, so nothing a client
// puts in the URL reaches the page: a code noticeFor does not know
// shows nothing.
type noticeCode string
// The codes of the actions on the webhook pages and of signing out.
// Replay's codes, with the reasons a replay can be refused, and
// resubmit's codes are defined beside those actions.
const (
webhookCreated noticeCode = "webhook-created"
webhookSaved noticeCode = "webhook-saved"
webhookDeleted noticeCode = "webhook-deleted"
entrypointAdded noticeCode = "entrypoint-added"
entrypointDeleted noticeCode = "entrypoint-deleted"
entrypointActivated noticeCode = "entrypoint-activated"
entrypointDeactivated noticeCode = "entrypoint-deactivated"
targetAdded noticeCode = "target-added"
targetSaved noticeCode = "target-saved"
targetDeleted noticeCode = "target-deleted"
targetActivated noticeCode = "target-activated"
targetDeactivated noticeCode = "target-deactivated"
signedOut noticeCode = "signed-out"
)
// notice is the line templates/notice.html shows above a page to say
// what an action did.
type notice struct {
Text string
// Failed shows the line as an error: the action was refused.
Failed bool
}
// noticeFor returns the notice the request's URL names, or nil when it
// names none or an unknown code.
func noticeFor(r *http.Request) *notice {
n, ok := map[noticeCode]notice{
webhookCreated: {Text: "Webhook created."},
webhookSaved: {Text: "Webhook saved."},
webhookDeleted: {Text: "Webhook deleted."},
entrypointAdded: {Text: "Entrypoint added."},
entrypointDeleted: {Text: "Entrypoint deleted."},
entrypointActivated: {Text: "Entrypoint activated."},
entrypointDeactivated: {Text: "Entrypoint deactivated."},
targetAdded: {Text: "Target added."},
targetSaved: {Text: "Target saved."},
targetDeleted: {Text: "Target deleted."},
targetActivated: {Text: "Target activated."},
targetDeactivated: {Text: "Target deactivated."},
signedOut: {Text: "Signed out."},
replayQueued: {
Text: "Replay queued: a new delivery was created " +
"against the target's current configuration.",
},
replayTargetDeleted: {
Text: "Not replayed: the target this delivery was for " +
"has been deleted. Recreate the target, then replay.",
Failed: true,
},
replayTargetMissing: {
Text: "Not replayed: the target this delivery was for " +
"no longer exists.",
Failed: true,
},
replayTargetInactive: {
Text: "Not replayed: the target this delivery was for " +
"is deactivated. Activate it, then replay.",
Failed: true,
},
replayNotTerminal: {
Text: "Not replayed: this delivery has not finished yet.",
Failed: true,
},
replayInFlight: {
Text: "Not replayed: a delivery of this event to this " +
"target is already in flight.",
Failed: true,
},
resubmitQueued: {
Text: "Resubmitted: a new event was created from the " +
"stored one and queued to every active target.",
},
resubmitNoTargets: {
Text: "Resubmitted: a new event was created, but this " +
"source has no active targets, so nothing was queued.",
},
}[noticeCode(r.URL.Query().Get(noticeParam))]
if !ok {
return nil
}
return &n
}
// withNotice returns path with code added as its notice.
func withNotice(path string, code noticeCode) string {
return path + "?" + noticeParam + "=" + string(code)
}
+3 -1
View File
@@ -411,7 +411,9 @@ func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
h.HandleSourceDelete().ServeHTTP(w, req) h.HandleSourceDelete().ServeHTTP(w, req)
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(t, "/hooks", w.Header().Get("Location")) assert.Equal(
t, "/hooks?notice=webhook-deleted", w.Header().Get("Location"),
)
assert.Equal( assert.Equal(
t, int64(0), t, int64(0),
+34
View File
@@ -241,3 +241,37 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
assert.Contains(t, body, "(unavailable)") assert.Contains(t, body, "(unavailable)")
assert.NotContains(t, body, "beak") assert.NotContains(t, body, "beak")
} }
// TestHandleSourceDetail_FitsWideAndNarrowWindows pins the webhook
// page's maximum width at 108rem (1728 px), half again the 72rem of
// max-w-6xl that the webhook list and the event log use, so an
// entrypoint URL fits on one line in a 1920-pixel window; and the
// wrapping of its title row, so the buttons beside the title do not
// push a phone-width window into scrolling sideways.
func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.Contains(
t, body,
`<div class="mx-auto px-6 py-8" style="max-width: 108rem"`,
)
assert.Contains(
t, body,
`<div class="flex flex-wrap justify-between items-center gap-2 mt-2">`,
)
}
+46 -41
View File
@@ -322,7 +322,8 @@ func (h *Handlers) createWebhookWithEntrypoint(
) )
http.Redirect( http.Redirect(
w, r, "/hook/"+webhook.ID, http.StatusSeeOther, w, r, withNotice("/hook/"+webhook.ID, webhookCreated),
http.StatusSeeOther,
) )
} }
@@ -579,7 +580,8 @@ func (h *Handlers) applyWebhookEdit(
} }
http.Redirect( http.Redirect(
w, r, "/hook/"+webhook.ID, http.StatusSeeOther, w, r, withNotice("/hook/"+webhook.ID, webhookSaved),
http.StatusSeeOther,
) )
} }
@@ -662,7 +664,9 @@ func (h *Handlers) deleteWebhookResources(
return return
} }
http.Redirect(w, r, "/hooks", http.StatusSeeOther) http.Redirect(
w, r, withNotice("/hooks", webhookDeleted), http.StatusSeeOther,
)
} }
// commitWebhookDeletion soft-deletes a webhook's entrypoints, // commitWebhookDeletion soft-deletes a webhook's entrypoints,
@@ -841,31 +845,16 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
totalPages++ totalPages++
} }
// The banner a replay or resubmit POST redirected back
// with. The message comes from a fixed set keyed by the
// outcome code, never from the query string itself.
replayMsg, replayOK := replayOutcome(
r.URL.Query().Get(replayOutcomeParam),
)
resubmitMsg, resubmitOK := resubmitOutcome(
r.URL.Query().Get(resubmitOutcomeParam),
)
data := map[string]any{ data := map[string]any{
tmplKeyWebhook: &webhook, tmplKeyWebhook: &webhook,
"Events": evts, "Events": evts,
"ReplayMessage": replayMsg, "Page": page,
"ReplayQueued": replayOK, "TotalPages": totalPages,
"ResubmitMessage": resubmitMsg, "TotalEvents": total,
"ResubmitQueued": resubmitOK, "HasPrev": page > 1,
"Page": page, "HasNext": page < totalPages,
"TotalPages": totalPages, "PrevPage": page - 1,
"TotalEvents": total, "NextPage": page + 1,
"HasPrev": page > 1,
"HasNext": page < totalPages,
"PrevPage": page - 1,
"NextPage": page + 1,
} }
h.renderTemplate(w, r, "source_logs.html", data) h.renderTemplate(w, r, "source_logs.html", data)
@@ -1254,7 +1243,8 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
} }
http.Redirect( http.Redirect(
w, r, "/hook/"+webhook.ID, http.StatusSeeOther, w, r, withNotice("/hook/"+webhook.ID, entrypointAdded),
http.StatusSeeOther,
) )
} }
} }
@@ -1365,7 +1355,8 @@ func (h *Handlers) processTargetCreate(
} }
http.Redirect( http.Redirect(
w, r, "/hook/"+webhook.ID, http.StatusSeeOther, w, r, withNotice("/hook/"+webhook.ID, targetAdded),
http.StatusSeeOther,
) )
} }
@@ -1643,6 +1634,7 @@ func (h *Handlers) HandleEntrypointDelete() http.HandlerFunc {
"entrypointID", &database.Entrypoint{}, "entrypointID", &database.Entrypoint{},
"failed to delete entrypoint", "failed to delete entrypoint",
nil, nil,
entrypointDeleted,
) )
} }
@@ -1655,18 +1647,21 @@ func (h *Handlers) HandleTargetDelete() http.HandlerFunc {
"targetID", &database.Target{}, "targetID", &database.Target{},
"failed to delete target", "failed to delete target",
h.evictArchiveWriterIfUnused, h.evictArchiveWriterIfUnused,
targetDeleted,
) )
} }
// deleteChildResource returns a handler that deletes a child // deleteChildResource returns a handler that deletes a child
// resource (entrypoint or target) belonging to a webhook. The // resource (entrypoint or target) belonging to a webhook. The
// optional afterDelete hook runs with the webhook's id once the // optional afterDelete hook runs with the webhook's id once the
// delete has succeeded, before the redirect. // delete has succeeded, before the redirect, which carries done as
// its notice.
func (h *Handlers) deleteChildResource( func (h *Handlers) deleteChildResource(
idParam string, idParam string,
model any, model any,
errMsg string, errMsg string,
afterDelete func(webhookID string), afterDelete func(webhookID string),
done noticeCode,
) http.HandlerFunc { ) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
userID, ok := h.getUserID(r) userID, ok := h.getUserID(r)
@@ -1708,7 +1703,7 @@ func (h *Handlers) deleteChildResource(
http.Redirect( http.Redirect(
w, r, w, r,
"/hook/"+webhook.ID, withNotice("/hook/"+webhook.ID, done),
http.StatusSeeOther, http.StatusSeeOther,
) )
} }
@@ -1719,7 +1714,7 @@ func (h *Handlers) deleteChildResource(
func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc { func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
return h.toggleChildResource( return h.toggleChildResource(
"entrypointID", "entrypointID",
func(webhookID, childID string) error { func(webhookID, childID string) (bool, error) {
var ep database.Entrypoint var ep database.Entrypoint
err := h.db.DB().Where( err := h.db.DB().Where(
@@ -1727,14 +1722,15 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
childID, webhookID, childID, webhookID,
).First(&ep).Error ).First(&ep).Error
if err != nil { if err != nil {
return err return false, err
} }
ep.Active = !ep.Active ep.Active = !ep.Active
return h.db.DB().Save(&ep).Error return ep.Active, h.db.DB().Save(&ep).Error
}, },
"failed to toggle entrypoint", "failed to toggle entrypoint",
entrypointActivated, entrypointDeactivated,
) )
} }
@@ -1742,7 +1738,7 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
func (h *Handlers) HandleTargetToggle() http.HandlerFunc { func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
return h.toggleChildResource( return h.toggleChildResource(
"targetID", "targetID",
func(webhookID, childID string) error { func(webhookID, childID string) (bool, error) {
var tgt database.Target var tgt database.Target
err := h.db.DB().Where( err := h.db.DB().Where(
@@ -1750,23 +1746,27 @@ func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
childID, webhookID, childID, webhookID,
).First(&tgt).Error ).First(&tgt).Error
if err != nil { if err != nil {
return err return false, err
} }
tgt.Active = !tgt.Active tgt.Active = !tgt.Active
return h.db.DB().Save(&tgt).Error return tgt.Active, h.db.DB().Save(&tgt).Error
}, },
"failed to toggle target", "failed to toggle target",
targetActivated, targetDeactivated,
) )
} }
// toggleChildResource returns a handler that toggles the active // toggleChildResource returns a handler that toggles the active
// state of a child resource belonging to a webhook. // state of a child resource belonging to a webhook. toggleFn returns
// the new state, and the redirect carries activated or deactivated as
// its notice to match.
func (h *Handlers) toggleChildResource( func (h *Handlers) toggleChildResource(
idParam string, idParam string,
toggleFn func(webhookID, childID string) error, toggleFn func(webhookID, childID string) (bool, error),
errMsg string, errMsg string,
activated, deactivated noticeCode,
) http.HandlerFunc { ) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
userID, ok := h.getUserID(r) userID, ok := h.getUserID(r)
@@ -1792,16 +1792,21 @@ func (h *Handlers) toggleChildResource(
return return
} }
err = toggleFn(webhook.ID, childID) active, err := toggleFn(webhook.ID, childID)
if err != nil { if err != nil {
h.serverError(w, r, errMsg, err) h.serverError(w, r, errMsg, err)
return return
} }
done := deactivated
if active {
done = activated
}
http.Redirect( http.Redirect(
w, r, w, r,
"/hook/"+webhook.ID, withNotice("/hook/"+webhook.ID, done),
http.StatusSeeOther, http.StatusSeeOther,
) )
} }
+2 -1
View File
@@ -161,7 +161,8 @@ func (h *Handlers) applyTargetEdit(
} }
http.Redirect( http.Redirect(
w, r, "/hook/"+webhook.ID, http.StatusSeeOther, w, r, withNotice("/hook/"+webhook.ID, targetSaved),
http.StatusSeeOther,
) )
} }
+27 -20
View File
@@ -3,17 +3,18 @@
// deliveries are attempted, how they end, how long they take, how // deliveries are attempted, how they end, how long they take, how
// deep the queues are, and how many circuit breakers are open. // deep the queues are, and how many circuit breakers are open.
// //
// The inbound HTTP metrics come from the go-http-metrics recorder in // It also builds the registry the authenticated /metrics route
// internal/middleware and land on prometheus.DefaultRegisterer. These // serves. In production, these collectors, the inbound HTTP metrics
// collectors register there too, so both surfaces are gathered by the // recorded in internal/middleware, and the Go runtime and process
// one promhttp handler mounted on the authenticated /metrics route. // collectors all register on that one registry, never on Prometheus's
// global default.
package metrics package metrics
import ( import (
"sync"
"time" "time"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/collectors"
"github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/promauto"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
) )
@@ -57,25 +58,31 @@ var knownTargetTypes = []database.TargetType{
database.TargetTypeSlack, database.TargetTypeSlack,
} }
// defaultSet is the process-wide metric set, registered on the same // NewRegistry returns the registry /metrics serves, carrying the Go
// registry the HTTP middleware and the /metrics handler already use. // runtime and process collectors that Prometheus's global default
// It is built on first use rather than in an init so that a test // registry carries, so the go_* and process_* series stay in the
// binary that never touches metrics never registers them. // scrape.
// //
//nolint:gochecknoglobals // one process-wide registration, by design // A registry of its own, rather than the global default, is what lets
var defaultSet = sync.OnceValue(func() *Set { // two dependency graphs in one process — two tests, say — each
return New(prometheus.DefaultRegisterer) // register their collectors without the second registration
}) // panicking.
func NewRegistry() *prometheus.Registry {
reg := prometheus.NewRegistry()
reg.MustRegister(
collectors.NewGoCollector(),
collectors.NewProcessCollector(
collectors.ProcessCollectorOpts{},
),
)
// Default returns the process-wide metric set. return reg
func Default() *Set {
return defaultSet()
} }
// Set is one registered group of webhooker's delivery collectors. // Set is one registered group of webhooker's delivery collectors.
// Production uses the single Default set; tests build their own // Production builds one on the registry /metrics serves; tests build
// against a private registry so assertions are not disturbed by // one on a registry of their own so they can gather what their own
// deliveries other tests are making concurrently. // deliveries recorded.
type Set struct { type Set struct {
eventsReceived prometheus.Counter eventsReceived prometheus.Counter
deliveryAttempts *prometheus.CounterVec deliveryAttempts *prometheus.CounterVec
@@ -93,7 +100,7 @@ type Set struct {
// New registers a full set of delivery collectors on reg and returns // New registers a full set of delivery collectors on reg and returns
// it. It panics if reg already holds them, which is the intended // it. It panics if reg already holds them, which is the intended
// behaviour for a duplicate registration. // behaviour for a duplicate registration.
func New(reg prometheus.Registerer) *Set { func New(reg *prometheus.Registry) *Set {
factory := promauto.With(reg) factory := promauto.With(reg)
s := &Set{ s := &Set{
+1 -2
View File
@@ -10,8 +10,7 @@ import (
// MetricsMiddlewareForTest builds the metrics recording middleware // MetricsMiddlewareForTest builds the metrics recording middleware
// against a caller-supplied recorder, so a test can gather from its // against a caller-supplied recorder, so a test can gather from its
// own Prometheus registry rather than the process-wide default one // own Prometheus registry without building a whole Middleware.
// that Middleware.Metrics uses.
func MetricsMiddlewareForTest( func MetricsMiddlewareForTest(
rec httpmetrics.Recorder, rec httpmetrics.Recorder,
) func(http.Handler) http.Handler { ) func(http.Handler) http.Handler {
+7 -8
View File
@@ -7,7 +7,6 @@ import (
"github.com/go-chi/chi" "github.com/go-chi/chi"
httpmetrics "github.com/slok/go-http-metrics/metrics" httpmetrics "github.com/slok/go-http-metrics/metrics"
prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
ghmm "github.com/slok/go-http-metrics/middleware" ghmm "github.com/slok/go-http-metrics/middleware"
"github.com/slok/go-http-metrics/middleware/std" "github.com/slok/go-http-metrics/middleware/std"
) )
@@ -151,17 +150,17 @@ func (r boundedLabelRecorder) AddInflightRequests(
var _ httpmetrics.Recorder = boundedLabelRecorder{} var _ httpmetrics.Recorder = boundedLabelRecorder{}
// Metrics returns middleware that records Prometheus HTTP metrics on // Metrics returns middleware that records Prometheus HTTP metrics
// the default registry, which is the one the /metrics route gathers. // with the Middleware's one recorder, which New builds on the registry
// the /metrics route serves and NewForTest on a registry of its own.
// Every call reuses that recorder, so any number of routers can
// install it.
func (s *Middleware) Metrics() func(http.Handler) http.Handler { func (s *Middleware) Metrics() func(http.Handler) http.Handler {
return metricsMiddleware( return metricsMiddleware(s.metricsRecorder)
prommetrics.NewRecorder(prommetrics.Config{}),
)
} }
// metricsMiddleware builds the recording middleware against a given // metricsMiddleware builds the recording middleware against a given
// recorder, so tests can gather from a registry of their own instead // recorder, so tests can gather from a registry of their own.
// of the process-wide default.
func metricsMiddleware( func metricsMiddleware(
rec httpmetrics.Recorder, rec httpmetrics.Recorder,
) func(http.Handler) http.Handler { ) func(http.Handler) http.Handler {
+28 -3
View File
@@ -57,9 +57,8 @@ const (
// Server.setupWebhookRoutes inside it. That ordering is the whole // Server.setupWebhookRoutes inside it. That ordering is the whole
// defect, so a test that flattens it would prove nothing. // defect, so a test that flattens it would prove nothing.
// //
// The recorder writes to a registry of the test's own rather than the // The recorder writes to a registry of the test's own, so each test
// process-wide default one, so each test observes only its own // observes only its own traffic.
// traffic.
func metricsTestRouter( func metricsTestRouter(
t *testing.T, t *testing.T,
receiverLimit int, receiverLimit int,
@@ -455,3 +454,29 @@ func TestMetrics_StatusAndSizeStillRecorded(t *testing.T) {
"the interceptor must still count written bytes", "the interceptor must still count written bytes",
) )
} }
// TestMetrics_WorksOnNewForTestMiddleware pins that a Middleware built
// by NewForTest has a recorder of its own: its Metrics() serves a
// request instead of panicking, and a second one does not collide
// with the first.
func TestMetrics_WorksOnNewForTestMiddleware(t *testing.T) {
t.Parallel()
log := slog.New(slog.DiscardHandler)
cfg := &config.Config{Environment: "prod"}
ok := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(okBody))
})
for range 2 {
h := middleware.NewForTest(log, cfg, nil).Metrics()(ok)
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, okRoute, nil,
)
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code)
}
}
+19 -4
View File
@@ -14,6 +14,9 @@ import (
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/go-chi/chi/middleware" "github.com/go-chi/chi/middleware"
"github.com/go-chi/cors" "github.com/go-chi/cors"
"github.com/prometheus/client_golang/prometheus"
httpmetrics "github.com/slok/go-http-metrics/metrics"
prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/globals" "sneak.berlin/go/webhooker/internal/globals"
@@ -149,10 +152,11 @@ const (
type MiddlewareParams struct { type MiddlewareParams struct {
fx.In fx.In
Logger *logger.Logger Logger *logger.Logger
Globals *globals.Globals Globals *globals.Globals
Config *config.Config Config *config.Config
Session *session.Session Session *session.Session
Registry *prometheus.Registry
} }
// Middleware provides HTTP middleware for logging, CORS, auth, and // Middleware provides HTTP middleware for logging, CORS, auth, and
@@ -162,6 +166,14 @@ type Middleware struct {
params *MiddlewareParams params *MiddlewareParams
session *session.Session session *session.Session
// metricsRecorder records the inbound HTTP metrics. New builds
// it on the registry /metrics serves, NewForTest on a registry
// of its own. Either way it is built once per Middleware and
// Metrics reuses it, because building it registers its
// collectors, and a second registration on the same registry
// panics.
metricsRecorder httpmetrics.Recorder
// loginGuard counts failed credential verifications and bounds // loginGuard counts failed credential verifications and bounds
// concurrent password hashing. It is built on first use so that // concurrent password hashing. It is built on first use so that
// every construction path gets one; see guard(). // every construction path gets one; see guard().
@@ -180,6 +192,9 @@ func New(
s.params = &params s.params = &params
s.log = params.Logger.Get() s.log = params.Logger.Get()
s.session = params.Session s.session = params.Session
s.metricsRecorder = prommetrics.NewRecorder(
prommetrics.Config{Registry: params.Registry},
)
return s, nil return s, nil
} }
+8
View File
@@ -3,12 +3,17 @@ package middleware
import ( import (
"log/slog" "log/slog"
"github.com/prometheus/client_golang/prometheus"
prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
// NewForTest creates a Middleware with the minimum dependencies // NewForTest creates a Middleware with the minimum dependencies
// needed for testing. This bypasses the fx lifecycle. // needed for testing. This bypasses the fx lifecycle.
//
// Its metrics recorder writes to a fresh registry of its own, so
// Metrics() works on it and two of them never collide.
func NewForTest( func NewForTest(
log *slog.Logger, log *slog.Logger,
cfg *config.Config, cfg *config.Config,
@@ -20,5 +25,8 @@ func NewForTest(
Config: cfg, Config: cfg,
}, },
session: sess, session: sess,
metricsRecorder: prommetrics.NewRecorder(
prommetrics.Config{Registry: prometheus.NewRegistry()},
),
} }
} }
+3
View File
@@ -24,6 +24,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/resetpw" "sneak.berlin/go/webhooker/internal/resetpw"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
@@ -163,6 +164,8 @@ func newServerApp(
session.New, session.New,
func() delivery.Notifier { return &noopNotifier{} }, func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} }, func() delivery.WebhookEvictor { return &noopEvictor{} },
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
delivery.NewGuard, delivery.NewGuard,
handlers.New, handlers.New,
+16
View File
@@ -83,6 +83,22 @@ func TestErrorPage_DeletedTarget(t *testing.T) {
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks) assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
} }
// TestErrorPage_ShowsNoNotice pins that a notice code in the URL of a
// page that fails is not shown above the error.
func TestErrorPage_ShowsNoNotice(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
userID, _ := env.seedUser(t, "owner", "somepassword")
cookies := env.authCookies(t, userID, "owner")
w := env.get("/hook/no-such-webhook?notice=webhook-saved", cookies)
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
assert.NotContains(t, w.Body.String(), "Webhook saved.")
}
func TestErrorPage_UnknownPath(t *testing.T) { func TestErrorPage_UnknownPath(t *testing.T) {
t.Parallel() t.Parallel()
+1 -7
View File
@@ -7,7 +7,6 @@ import (
sentryhttp "github.com/getsentry/sentry-go/http" sentryhttp "github.com/getsentry/sentry-go/http"
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/go-chi/chi/middleware" "github.com/go-chi/chi/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/webhooker/static" "sneak.berlin/go/webhooker/static"
) )
@@ -149,12 +148,7 @@ func (s *Server) setupRoutes() {
if s.params.Config.MetricsAuthEnabled() { if s.params.Config.MetricsAuthEnabled() {
s.router.Group(func(r chi.Router) { s.router.Group(func(r chi.Router) {
r.Use(s.mw.MetricsAuth()) r.Use(s.mw.MetricsAuth())
r.Get( r.Get("/metrics", s.h.HandleMetrics())
"/metrics",
http.HandlerFunc(
promhttp.Handler().ServeHTTP,
),
)
}) })
} }
+132 -34
View File
@@ -24,6 +24,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
@@ -113,6 +114,8 @@ func newTestEnvWithConfig(
session.New, session.New,
func() delivery.Notifier { return &noopNotifier{} }, func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} }, func() delivery.WebhookEvictor { return &noopEvictor{} },
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
delivery.NewGuard, delivery.NewGuard,
handlers.New, handlers.New,
@@ -260,6 +263,24 @@ func (e *testEnv) urlFrom(
return html.UnescapeString(match[1]) return html.UnescapeString(match[1])
} }
// requireNotice requires w to redirect to dest carrying the notice
// code, then renders that page and requires it to show text.
func (e *testEnv) requireNotice(
t *testing.T,
w *httptest.ResponseRecorder,
dest, code, text string,
cookies []*http.Cookie,
) {
t.Helper()
require.Equal(t, http.StatusSeeOther, w.Code)
require.Equal(t, dest+"?notice="+code, w.Header().Get("Location"))
page := e.get(w.Header().Get("Location"), cookies)
require.Equal(t, http.StatusOK, page.Code)
assert.Contains(t, page.Body.String(), text)
}
// authCookies forges an authenticated session for the given user. // authCookies forges an authenticated session for the given user.
func (e *testEnv) authCookies( func (e *testEnv) authCookies(
t *testing.T, t *testing.T,
@@ -738,6 +759,31 @@ func TestPagesLogin_ReturnsToTheRequestedPage(t *testing.T) {
assert.Equal(t, asked, w.Header().Get("Location")) assert.Equal(t, asked, w.Header().Get("Location"))
} }
// TestPagesLogout_SaysSignedOut signs out with the navbar's form and
// lands on the sign-in page, which says so.
func TestPagesLogout_SaysSignedOut(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
userID, _ := env.seedUser(t, "leaver", "somepassword")
token, cookies := env.csrfFrom(
t, "/hooks", env.authCookies(t, userID, "leaver"),
)
form := url.Values{}
form.Set("csrf_token", token)
w := env.post(
env.urlFrom(t, "/hooks", `action="(/pages/logout)"`, cookies),
form, cookies,
)
// The sign-in page is requested without the session cookie, which
// the logout told the browser to delete.
env.requireNotice(t, w, "/pages/login", "signed-out", "Signed out.", nil)
}
// --- /user/{username} group --- // --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged // TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -855,9 +901,9 @@ func TestHooks_ListAndNewWebhookForm(t *testing.T) {
require.NoError(t, require.NoError(t,
env.db.DB().Where("name = ?", "created").First(&created).Error, env.db.DB().Where("name = ?", "created").First(&created).Error,
) )
assert.Equal( env.requireNotice(
t, "/hook/"+created.ID, w.Header().Get("Location"), t, w, "/hook/"+created.ID, "webhook-created", "Webhook created.",
"creating a webhook should redirect to its page", cookies,
) )
} }
@@ -888,8 +934,7 @@ func TestHook_EditFormAndDelete(t *testing.T) {
env.urlFrom(t, editPage, `action="(/hook/[^/"]+/edit)"`, cookies), env.urlFrom(t, editPage, `action="(/hook/[^/"]+/edit)"`, cookies),
form, cookies, form, cookies,
) )
require.Equal(t, http.StatusSeeOther, w.Code) env.requireNotice(t, w, page, "webhook-saved", "Webhook saved.", cookies)
assert.Equal(t, page, w.Header().Get("Location"))
var edited database.Webhook var edited database.Webhook
@@ -903,16 +948,17 @@ func TestHook_EditFormAndDelete(t *testing.T) {
env.urlFrom(t, page, `action="(/hook/[^/"]+/delete)"`, cookies), env.urlFrom(t, page, `action="(/hook/[^/"]+/delete)"`, cookies),
form, cookies, form, cookies,
) )
require.Equal(t, http.StatusSeeOther, w.Code) env.requireNotice(
assert.Equal(t, "/hooks", w.Header().Get("Location")) t, w, "/hooks", "webhook-deleted", "Webhook deleted.", cookies,
)
assert.Equal( assert.Equal(
t, http.StatusNotFound, env.get(page, cookies).Code, t, http.StatusNotFound, env.get(page, cookies).Code,
"a deleted webhook's page should be gone", "a deleted webhook's page should be gone",
) )
} }
// TestHook_EntrypointActions adds, deactivates and deletes an // TestHook_EntrypointActions adds, deactivates, activates and deletes
// entrypoint with the forms on the webhook page, each submitted to // an entrypoint with the forms on the webhook page, each submitted to
// the action and with the token the page rendered. // the action and with the token the page rendered.
func TestHook_EntrypointActions(t *testing.T) { func TestHook_EntrypointActions(t *testing.T) {
t.Parallel() t.Parallel()
@@ -930,16 +976,19 @@ func TestHook_EntrypointActions(t *testing.T) {
form.Set("csrf_token", token) form.Set("csrf_token", token)
// submit posts the webhook page's form whose action pattern // submit posts the webhook page's form whose action pattern
// captures, and requires the redirect back to that page. // captures, and requires the redirect back to that page with the
submit := func(pattern string) { // notice code, and the page to show text.
submit := func(pattern, code, text string) {
t.Helper() t.Helper()
w := env.post(env.urlFrom(t, page, pattern, cookies), form, cookies) w := env.post(env.urlFrom(t, page, pattern, cookies), form, cookies)
require.Equal(t, http.StatusSeeOther, w.Code) env.requireNotice(t, w, page, code, text, cookies)
require.Equal(t, page, w.Header().Get("Location"))
} }
submit(`action="(/hook/[^/"]+/entrypoints)"`) toggle := `action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`
submit(`action="(/hook/[^/"]+/entrypoints)"`,
"entrypoint-added", "Entrypoint added.")
var added database.Entrypoint var added database.Entrypoint
@@ -948,7 +997,7 @@ func TestHook_EntrypointActions(t *testing.T) {
) )
require.True(t, added.Active) require.True(t, added.Active)
submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`) submit(toggle, "entrypoint-deactivated", "Entrypoint deactivated.")
var toggled database.Entrypoint var toggled database.Entrypoint
@@ -957,7 +1006,10 @@ func TestHook_EntrypointActions(t *testing.T) {
) )
assert.False(t, toggled.Active, "the toggle should deactivate it") assert.False(t, toggled.Active, "the toggle should deactivate it")
submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`) submit(toggle, "entrypoint-activated", "Entrypoint activated.")
submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`,
"entrypoint-deleted", "Entrypoint deleted.")
var left int64 var left int64
@@ -968,8 +1020,8 @@ func TestHook_EntrypointActions(t *testing.T) {
// TestHook_TargetActions adds a target with the form on the webhook // TestHook_TargetActions adds a target with the form on the webhook
// page, follows its Edit link to the target edit form and submits // page, follows its Edit link to the target edit form and submits
// it, then deactivates and deletes it, every URL and token taken from // it, then deactivates, activates and deletes it, every URL and token
// the rendered pages. // taken from the rendered pages.
func TestHook_TargetActions(t *testing.T) { func TestHook_TargetActions(t *testing.T) {
t.Parallel() t.Parallel()
@@ -984,27 +1036,29 @@ func TestHook_TargetActions(t *testing.T) {
// submit posts form, with the token, to the action pattern // submit posts form, with the token, to the action pattern
// captures on the page at from, and requires the redirect back to // captures on the page at from, and requires the redirect back to
// the webhook page. // the webhook page with the notice code, and that page to show
submit := func(from, pattern string, form url.Values) { // text.
submit := func(from, pattern string, form url.Values, code, text string) {
t.Helper() t.Helper()
form.Set("csrf_token", token) form.Set("csrf_token", token)
w := env.post(env.urlFrom(t, from, pattern, cookies), form, cookies) w := env.post(env.urlFrom(t, from, pattern, cookies), form, cookies)
require.Equal(t, http.StatusSeeOther, w.Code) env.requireNotice(t, w, page, code, text, cookies)
require.Equal(t, page, w.Header().Get("Location"))
} }
toggle := `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`
submit(page, `action="(/hook/[^/"]+/targets)"`, url.Values{ submit(page, `action="(/hook/[^/"]+/targets)"`, url.Values{
"name": {"added"}, "name": {"added"},
"type": {string(database.TargetTypeLog)}, "type": {string(database.TargetTypeLog)},
}) }, "target-added", "Target added.")
editPage := env.urlFrom( editPage := env.urlFrom(
t, page, `href="(/hook/[^/"]+/targets/[^/"]+/edit)"`, cookies, t, page, `href="(/hook/[^/"]+/targets/[^/"]+/edit)"`, cookies,
) )
submit(editPage, `action="(/hook/[^/"]+/targets/[^/"]+/edit)"`, submit(editPage, `action="(/hook/[^/"]+/targets/[^/"]+/edit)"`,
url.Values{"name": {"renamed"}}) url.Values{"name": {"renamed"}}, "target-saved", "Target saved.")
var edited database.Target var edited database.Target
@@ -1014,8 +1068,8 @@ func TestHook_TargetActions(t *testing.T) {
assert.Equal(t, "renamed", edited.Name) assert.Equal(t, "renamed", edited.Name)
require.True(t, edited.Active) require.True(t, edited.Active)
submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`, submit(page, toggle, url.Values{},
url.Values{}) "target-deactivated", "Target deactivated.")
var toggled database.Target var toggled database.Target
@@ -1024,8 +1078,11 @@ func TestHook_TargetActions(t *testing.T) {
) )
assert.False(t, toggled.Active, "the toggle should deactivate it") assert.False(t, toggled.Active, "the toggle should deactivate it")
submit(page, toggle, url.Values{},
"target-activated", "Target activated.")
submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/delete)"`, submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/delete)"`,
url.Values{}) url.Values{}, "target-deleted", "Target deleted.")
var left int64 var left int64
@@ -1061,9 +1118,9 @@ func TestHook_ResubmitFromEventLog(t *testing.T) {
env.urlFrom(t, logsPath, `action="(/hook/[^"]+/resubmit)"`, cookies), env.urlFrom(t, logsPath, `action="(/hook/[^"]+/resubmit)"`, cookies),
form, cookies, form, cookies,
) )
require.Equal(t, http.StatusSeeOther, w.Code) env.requireNotice(
assert.Equal( t, w, logsPath, "resubmit-no-targets",
t, logsPath+"?resubmit=no-targets", w.Header().Get("Location"), "this source has no active targets", cookies,
) )
webhookDB, err := env.dbMgr.GetDB(wh.ID) webhookDB, err := env.dbMgr.GetDB(wh.ID)
@@ -1299,10 +1356,8 @@ func TestDeliveryReplay_PostOnlyAndCSRFProtected(t *testing.T) {
html.UnescapeString(action[1]), form, cookies, html.UnescapeString(action[1]), form, cookies,
) )
require.Equal(t, http.StatusSeeOther, w.Code) env.requireNotice(
assert.Equal( t, w, logsPath, "replay-queued", "Replay queued:", cookies,
t, logsPath+"?replay=queued",
w.Header().Get("Location"),
) )
assert.Equal( assert.Equal(
t, int64(2), env.countDeliveries(t, wh.ID), t, int64(2), env.countDeliveries(t, wh.ID),
@@ -1478,3 +1533,46 @@ func TestMetricsRouteUnmountedOnHalfSetConfig(t *testing.T) {
}) })
} }
} }
// TestTwoMetricsRoutersInOneProcess pins
// https://git.eeqj.de/sneak/webhooker/issues/227: a second
// metrics-enabled router in one process used to panic, because the
// HTTP metrics registered on Prometheus's global default registry.
// Two routers are built over separate dependency graphs and a third
// over the first graph again, and each must still serve the HTTP,
// delivery, Go runtime and process series, and the series counting
// scrapes of /metrics itself.
func TestTwoMetricsRoutersInOneProcess(t *testing.T) {
t.Parallel()
first := newTestEnvWithConfig(
t, metricsConfig(t, metricsUser, metricsAuthValue),
)
second := newTestEnvWithConfig(
t, metricsConfig(t, metricsUser, metricsAuthValue),
)
third := &testEnv{
router: server.NewRouterForTest(
first.log.Get(), first.cfg, first.mw, first.hnd,
),
}
for _, env := range []*testEnv{first, second, third} {
env.get("/", nil)
scrape := env.metricsRequest(metricsUser, metricsAuthValue)
require.Equal(t, http.StatusOK, scrape.Code)
for _, series := range []string{
"http_request_duration_seconds",
"http_response_size_bytes",
"http_requests_inflight",
"webhooker_events_received_total",
"go_goroutines",
"process_start_time_seconds",
"promhttp_metric_handler_requests_total",
} {
assert.Contains(t, scrape.Body.String(), series)
}
}
}
+1
View File
@@ -7,6 +7,7 @@
<body class="bg-gray-50 min-h-screen flex flex-col"> <body class="bg-gray-50 min-h-screen flex flex-col">
<div class="flex-grow"> <div class="flex-grow">
{{template "navbar" .}} {{template "navbar" .}}
{{template "notice" .}}
{{block "content" .}}{{end}} {{block "content" .}}{{end}}
</div> </div>
{{template "footer" .}} {{template "footer" .}}
+7
View File
@@ -0,0 +1,7 @@
{{define "notice"}}
{{with .Notice}}
<div class="max-w-6xl mx-auto px-6 pt-4">
<div class="{{if .Failed}}alert-error{{else}}alert-success{{end}}">{{.Text}}</div>
</div>
{{end}}
{{end}}
+6 -2
View File
@@ -3,10 +3,14 @@
{{define "title"}}{{.Webhook.Name}} - Webhooker{{end}} {{define "title"}}{{.Webhook.Name}} - Webhooker{{end}}
{{define "content"}} {{define "content"}}
<div class="max-w-6xl mx-auto px-6 py-8" x-data="{ showAddEntrypoint: false, showAddTarget: false }"> <!-- 108rem, half again the 72rem (max-w-6xl) of the webhook list, the
event log, the navbar and the footer, so an entrypoint URL fits on
one line. An inline style, because the committed tailwind.css has
no class this wide. -->
<div class="mx-auto px-6 py-8" style="max-width: 108rem" x-data="{ showAddEntrypoint: false, showAddTarget: false }">
<div class="mb-6"> <div class="mb-6">
<a href="/hooks" class="text-sm text-primary-600 hover:text-primary-700">&larr; Back to webhooks</a> <a href="/hooks" class="text-sm text-primary-600 hover:text-primary-700">&larr; Back to webhooks</a>
<div class="flex justify-between items-center mt-2"> <div class="flex flex-wrap justify-between items-center gap-2 mt-2">
<div> <div>
<h1 class="text-2xl font-medium text-gray-900">{{.Webhook.Name}}</h1> <h1 class="text-2xl font-medium text-gray-900">{{.Webhook.Name}}</h1>
{{if .Webhook.Description}} {{if .Webhook.Description}}
-8
View File
@@ -12,14 +12,6 @@
</div> </div>
</div> </div>
{{if .ReplayMessage}}
<div class="{{if .ReplayQueued}}alert-success{{else}}alert-error{{end}}">{{.ReplayMessage}}</div>
{{end}}
{{if .ResubmitMessage}}
<div class="{{if .ResubmitQueued}}alert-success{{else}}alert-error{{end}}">{{.ResubmitMessage}}</div>
{{end}}
<div class="card"> <div class="card">
<div class="divide-y divide-gray-100"> <div class="divide-y divide-gray-100">
{{range .Events}} {{range .Events}}