5 Commits
Author SHA1 Message Date
sneak 1647b43aa6 deploy (#365)
check / check (push) Canceled after 0s
Reviewed-on: #365
2026-09-29 13:06:27 +02:00
sneakandclawbot f7151f0168 deployment to prod (#357)
check / check (push) Successful in 10s
Co-authored-by: clawbot <35+clawbot@noreply.example.org>
Reviewed-on: #357
2026-09-29 12:01:56 +02:00
sneak 3cc05a36eb deployment (#355)
check / check (push) Successful in 4m11s
Reviewed-on: #355
2026-09-29 11:54:06 +02:00
sneak 7316f0a7e2 Merge branch 'main' into prod
check / check (push) Successful in 7s
2026-09-29 11:49:18 +02:00
clawbotandsneak 5f84d891cf Deploy: main into prod (#343)
check / check (push) Successful in 3m32s
Brings `prod`, which upaas deploys, up to `main` at `9cf9cdd`, the merge of #321. `prod` was cut from `main` at `251cb3d` (1.0.0b1).

What it deploys is everything listed in #321. For running it:

- With `WEBHOOKER_ENVIRONMENT` unset, the instance runs as `prod` and sends no `Access-Control-Allow-Origin: *`.
- Each event database gains its new indexes the first time it is opened after the upgrade.
- `webhooker_delivery_retries_total` no longer counts a circuit breaker holding back a delivery that is already `retrying`.

Not in this PR yet: #340, in which the container sets its own data directory owner and mode before start. It is in progress on `next`. Once it reaches `main`, this PR carries it, because the PR follows `main`.

Model: opus-5-5
Co-authored-by: Jeffrey Paul <1+sneak@noreply.example.org>
Reviewed-on: #343
2026-09-29 02:48:03 -07:00
21 changed files with 73 additions and 333 deletions
+9 -9
View File
@@ -1439,7 +1439,7 @@ A registered user of the webhooker service.
| Field | Type | Description | | Field | Type | Description |
| ---------- | -------- | ----------- | | ---------- | -------- | ----------- |
| `id` | UUID | Primary key | | `id` | UUID | Primary key |
| `username` | string | Unique login name, at most 1024 bytes so that it fits in the session cookie | | `username` | string | Unique login name |
| `password` | string | Argon2id hash (never exposed via API) | | `password` | string | Argon2id hash (never exposed via API) |
**Relations:** Has many Webhooks. Has many APIKeys. **Relations:** Has many Webhooks. Has many APIKeys.
@@ -2379,14 +2379,14 @@ Removing either cap fails 14 subtests.
`internal/middleware/logbound_test.go` and `internal/middleware/logbound_test.go` and
`internal/handlers/logbound_test.go` drive 8 KB of client-chosen text `internal/handlers/logbound_test.go` drive 8 KB of client-chosen text
at each of these — just under 1 KB at `invalid password`, whose at each of these — 1 KB at `invalid password`, whose accounts are
accounts are shared with the successful-login line and so must stay shared with the successful-login line, where a username past 4 KB
within the 1024-byte username limit — through both handlers, and overflows the session cookie and answers 500 before that line is
through seven fills: plain text as the baseline, and then the written — through both handlers, and through seven fills: plain text
quotation mark, backslash, tab, newline, C0 control and astral as the baseline, and then the quotation mark, backslash, tab, newline,
non-printable, six characters the wider of the two handlers spends C0 control and astral non-printable, six characters the wider of the
more on than the client spent sending them. Every case holds each two handlers spends more on than the client spent sending them. Every
line to the 2,560-byte ceiling. That per-line ceiling case holds each line to the 2,560-byte ceiling. That per-line ceiling
is what the figure above states, and every row establishes it. is what the figure above states, and every row establishes it.
Three of the sites go further and bound the whole flood's output — the Three of the sites go further and bound the whole flood's output — the
-5
View File
@@ -16,7 +16,6 @@ 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"
@@ -178,10 +177,6 @@ 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
+2 -47
View File
@@ -1,58 +1,13 @@
package database package database
import (
"errors"
"fmt"
"gorm.io/gorm"
)
// MaxUsernameBytes is the longest username, in bytes, that a user may
// have. The same number appears in the check constraint on
// User.Username, because a struct tag cannot reference a constant.
//
// A login stores the username in the session cookie, and both
// securecookie and browsers refuse a cookie value past about 4096
// bytes. That value is the session base64-encoded twice, so it holds
// 4096 × 3/4 × 3/4 = 2304 bytes of session, and the signature,
// timestamp and the session's other values take about 270 of those: a
// username longer than about 2030 bytes can never log in. The limit is
// about half that, so the session can carry more values later without
// locking out an account whose username is already at the limit.
const MaxUsernameBytes = 1024
// ErrUsernameTooLong is returned when a user is saved with a username
// longer than MaxUsernameBytes.
var ErrUsernameTooLong = errors.New("username is too long")
// User represents a user of the webhooker service // User represents a user of the webhooker service
//
//nolint:lll // a struct tag cannot wrap
type User struct { type User struct {
BaseModel BaseModel
Username string `gorm:"uniqueIndex;not null;check:length(CAST(username AS BLOB)) <= 1024" json:"username"` Username string `gorm:"uniqueIndex;not null" json:"username"`
Password string `gorm:"not null" json:"-"` // Argon2 hashed Password string `gorm:"not null" json:"-"` // Argon2 hashed
// Relations // Relations
Webhooks []Webhook `json:"webhooks,omitempty"` Webhooks []Webhook `json:"webhooks,omitempty"`
APIKeys []APIKey `json:"apiKeys,omitempty"` APIKeys []APIKey `json:"apiKeys,omitempty"`
} }
// BeforeSave rejects a username longer than MaxUsernameBytes when a whole
// User is created or saved, so those calls get ErrUsernameTooLong rather
// than the database's constraint error. A column update such as
// Update("username", ...) is caught only by the check constraint, as is
// any path that writes the table without this model.
func (u *User) BeforeSave(_ *gorm.DB) error {
if len(u.Username) > MaxUsernameBytes {
return fmt.Errorf(
"%w: %d bytes, limit is %d",
ErrUsernameTooLong,
len(u.Username),
MaxUsernameBytes,
)
}
return nil
}
-65
View File
@@ -1,65 +0,0 @@
package database_test
import (
"strings"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
)
// usernameAtLimit is exactly MaxUsernameBytes long, built from a
// two-byte character. A check that counted characters rather than bytes
// would see half the length and let the one-byte-longer name through.
func usernameAtLimit() string {
return strings.Repeat("é", database.MaxUsernameBytes/2)
}
func TestUserCreate_RejectsOverlongUsername(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
err := db.Create(&database.User{
Username: usernameAtLimit() + "x",
Password: "hash",
}).Error
require.ErrorIs(t, err, database.ErrUsernameTooLong)
}
func TestUserCreate_AcceptsUsernameAtLimit(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
require.NoError(t, db.Create(&database.User{
Username: usernameAtLimit(),
Password: "hash",
}).Error)
}
// TestUsersTable_EnforcesUsernameLimitWithoutTheModel inserts with raw
// SQL, as a path that bypassed User.BeforeSave would, so only the
// table's check constraint stands between it and an over-long
// username. Accepting the name at the limit and refusing the next byte
// also pins the constraint's number to MaxUsernameBytes.
func TestUsersTable_EnforcesUsernameLimitWithoutTheModel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
insert := "INSERT INTO users (id, username, password) VALUES (?, ?, ?)"
require.NoError(t, db.Exec(
insert, uuid.New().String(), usernameAtLimit(), "hash",
).Error)
err := db.Exec(
insert, uuid.New().String(), usernameAtLimit()+"x", "hash",
).Error
require.Error(t, err)
assert.Contains(t, err.Error(), "CHECK constraint failed")
}
+5 -6
View File
@@ -148,7 +148,6 @@ type EngineParams struct {
DBManager *database.WebhookDBManager DBManager *database.WebhookDBManager
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
@@ -168,10 +167,10 @@ type Engine struct {
retryCh chan Task retryCh chan Task
workers int workers int
// mtr is the delivery metric set. Production wires the one // mtr is the delivery metric set. Production wires the
// registered on the registry /metrics serves; a test can // process-wide one; a test can substitute a set registered on
// substitute a set registered on a registry it holds, so it can // a private registry so its assertions are not disturbed by
// gather what its own deliveries recorded. // deliveries other tests are making at the same time.
mtr *metrics.Set mtr *metrics.Set
// targets maps each target type to its implementation. // targets maps each target type to its implementation.
@@ -205,7 +204,7 @@ 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: params.Metrics, mtr: metrics.Default(),
} }
e.initTargets(&http.Client{ e.initTargets(&http.Client{
+5 -5
View File
@@ -9,7 +9,6 @@ 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"
@@ -390,7 +389,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.New(prometheus.NewRegistry()), mtr: metrics.Default(),
} }
e.initTargets(client) e.initTargets(client)
@@ -405,7 +404,7 @@ func NewTestEngineSmallRetry(
e := &Engine{ e := &Engine{
log: log, log: log,
retryCh: make(chan Task, 1), retryCh: make(chan Task, 1),
mtr: metrics.New(prometheus.NewRegistry()), mtr: metrics.Default(),
} }
e.initTargets(nil) e.initTargets(nil)
@@ -428,7 +427,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.New(prometheus.NewRegistry()), mtr: metrics.Default(),
} }
e.initTargets(client) e.initTargets(client)
@@ -436,7 +435,8 @@ 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 registry it holds. // assert on collectors registered on a private registry instead of
// 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
} }
+3 -2
View File
@@ -35,8 +35,9 @@ 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
// registry this test holds, so its exact assertions can gather from // private registry. The process-wide collectors are moved by every
// it. // other delivery test running in parallel, so exact assertions are
// 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 {
-30
View File
@@ -453,33 +453,3 @@ func TestLogin_SuccessCreatesSession(t *testing.T) {
"the issued cookie must carry an authenticated session", "the issued cookie must carry an authenticated session",
) )
} }
// TestLogin_UsernameAtLimitCanLogIn shows that a username of exactly
// database.MaxUsernameBytes still fits in the session cookie. Past
// what the cookie can carry, a correct login answers 500.
func TestLogin_UsernameAtLimitCanLogIn(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
)
app := newTestApp(t, &h, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
username := strings.Repeat("a", database.MaxUsernameBytes)
hash, err := database.HashPassword(operatorPassword)
require.NoError(t, err)
require.NoError(t, db.DB().Create(&database.User{
Username: username,
Password: hash,
}).Error)
w := submitLogin(h, sharedProxyPeer, username, operatorPassword)
assert.Equal(t, http.StatusSeeOther, w.Code)
}
+1 -4
View File
@@ -12,7 +12,6 @@ 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"
@@ -62,8 +61,6 @@ 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
@@ -125,7 +122,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 = params.Metrics s.mtr = metrics.Default()
s.ssrf = params.SSRFGuard s.ssrf = params.SSRFGuard
// Parse all page templates once at startup // Parse all page templates once at startup
-3
View File
@@ -20,7 +20,6 @@ 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"
) )
@@ -110,8 +109,6 @@ 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,
+5 -3
View File
@@ -339,9 +339,11 @@ const storedUserPassword = "correct-horse-battery-staple"
// storedFillBytes is the raw length of the client-chosen value in // storedFillBytes is the raw length of the client-chosen value in
// those accounts' usernames. It is well past the 512-byte field // those accounts' usernames. It is well past the 512-byte field
// budget, so the line is still truncated, but short enough that the // budget, so the line is still truncated, but short enough that the
// whole username, markers and fill name included, stays within // session cookie a successful login writes stays inside
// database.MaxUsernameBytes. // securecookie's 4 KB limit: the cookie is written BEFORE the
const storedFillBytes = 960 // "user logged in" line, so an 8 KB username answers 500 and never
// reaches it.
const storedFillBytes = 1024
// storedFill builds a username fill of storedFillBytes raw bytes out // storedFill builds a username fill of storedFillBytes raw bytes out
// of repetitions of ch, with both markers at its far end. // of repetitions of ch, with both markers at its far end.
-20
View File
@@ -1,20 +0,0 @@
package handlers
import (
"net/http"
"github.com/prometheus/client_golang/prometheus/promhttp"
)
// HandleMetrics returns the Prometheus scrape handler for the
// registry every collector in this process registers 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
}
+20 -26
View File
@@ -3,17 +3,17 @@
// 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.
// //
// It also builds the registry the authenticated /metrics route // The inbound HTTP metrics come from the go-http-metrics recorder in
// serves. These collectors, the inbound HTTP metrics recorded in // internal/middleware and land on prometheus.DefaultRegisterer. These
// internal/middleware, and the Go runtime and process collectors all // collectors register there too, so both surfaces are gathered by the
// register on that one registry, never on Prometheus's global default. // one promhttp handler mounted on the authenticated /metrics route.
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,31 +57,25 @@ var knownTargetTypes = []database.TargetType{
database.TargetTypeSlack, database.TargetTypeSlack,
} }
// NewRegistry returns the registry /metrics serves, carrying the Go // defaultSet is the process-wide metric set, registered on the same
// runtime and process collectors that Prometheus's global default // registry the HTTP middleware and the /metrics handler already use.
// registry carries, so the go_* and process_* series stay in the // It is built on first use rather than in an init so that a test
// scrape. // binary that never touches metrics never registers them.
// //
// A registry of its own, rather than the global default, is what lets //nolint:gochecknoglobals // one process-wide registration, by design
// two dependency graphs in one process — two tests, say — each var defaultSet = sync.OnceValue(func() *Set {
// register their collectors without the second registration return New(prometheus.DefaultRegisterer)
// panicking. })
func NewRegistry() *prometheus.Registry {
reg := prometheus.NewRegistry()
reg.MustRegister(
collectors.NewGoCollector(),
collectors.NewProcessCollector(
collectors.ProcessCollectorOpts{},
),
)
return reg // Default returns the process-wide metric set.
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 builds one on the registry /metrics serves; tests build // Production uses the single Default set; tests build their own
// their own against a private registry so assertions are not // against a private registry so assertions are not disturbed by
// disturbed by deliveries other tests are making concurrently. // deliveries other tests are making concurrently.
type Set struct { type Set struct {
eventsReceived prometheus.Counter eventsReceived prometheus.Counter
deliveryAttempts *prometheus.CounterVec deliveryAttempts *prometheus.CounterVec
@@ -99,7 +93,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.Registry) *Set { func New(reg prometheus.Registerer) *Set {
factory := promauto.With(reg) factory := promauto.With(reg)
s := &Set{ s := &Set{
+2 -1
View File
@@ -10,7 +10,8 @@ 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 without building a whole Middleware. // own Prometheus registry rather than the process-wide default one
// 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 -4
View File
@@ -7,6 +7,7 @@ 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,14 +152,16 @@ 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 on
// the registry the /metrics route serves. Every call shares the one // the default registry, which is the one the /metrics route gathers.
// recorder New built, 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(s.metricsRecorder) return metricsMiddleware(
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. // recorder, so tests can gather from a registry of their own instead
// 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 {
+3 -28
View File
@@ -57,8 +57,9 @@ 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, so each test // The recorder writes to a registry of the test's own rather than the
// observes only its own traffic. // process-wide default one, so each test observes only its own
// traffic.
func metricsTestRouter( func metricsTestRouter(
t *testing.T, t *testing.T,
receiverLimit int, receiverLimit int,
@@ -454,29 +455,3 @@ 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)
}
}
+4 -17
View File
@@ -13,9 +13,6 @@ 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"
@@ -151,11 +148,10 @@ 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
@@ -165,12 +161,6 @@ type Middleware struct {
params *MiddlewareParams params *MiddlewareParams
session *session.Session session *session.Session
// metricsRecorder records the inbound HTTP metrics on the
// registry /metrics serves. It is built once, in New, because
// building it registers its collectors, and a second
// registration on the same registry panics; see Metrics.
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().
@@ -189,9 +179,6 @@ 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,17 +3,12 @@ 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,
@@ -25,8 +20,5 @@ func NewForTest(
Config: cfg, Config: cfg,
}, },
session: sess, session: sess,
metricsRecorder: prommetrics.NewRecorder(
prommetrics.Config{Registry: prometheus.NewRegistry()},
),
} }
} }
-3
View File
@@ -24,7 +24,6 @@ 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"
@@ -164,8 +163,6 @@ 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,
+7 -1
View File
@@ -7,6 +7,7 @@ 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"
) )
@@ -129,7 +130,12 @@ 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("/metrics", s.h.HandleMetrics()) r.Get(
"/metrics",
http.HandlerFunc(
promhttp.Handler().ServeHTTP,
),
)
}) })
} }
-46
View File
@@ -24,7 +24,6 @@ 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"
@@ -114,8 +113,6 @@ 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,
@@ -1030,46 +1027,3 @@ 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)
}
}
}