Author SHA1 Message Date
sneak b41d29cc96 Rewrite README.md sentence by sentence
check / check (push) Waiting to run
Rewritten from scratch against the code on next. The old text was
the list of what a reader needs covered.

Model: fable-5-1
2026-10-25 05:11:36 +00:00
sneak 54b643044f Merge branch 'main' into next
check / check (push) Waiting to run
2026-10-25 02:51:56 +01:00
clawbot 55ccb28270 Run the tests with cgo off and without the race detector (closes #538)
check / check (push) Canceled after 0s
sneak ruled the race detector out
(#538 (comment)), and on
Linux it was the only reason the tests needed cgo. The test phase and the
browser test binary now build with CGO_ENABLED=0 and without -race, so no
test binary compiles mattn's C SQLite. The test phase keeps the builder
stage's Debian Go image. Its comments, the README and the code comments that
said the suite runs under -race now say what is true, and the README notes
the exception to the vendored REPO_POLICIES.md.

Model: opus-5-5
2026-10-21 17:28:50 +02:00
clawbot 06be075b70 Store events and create deliveries in the delivery engine (closes #544)
check / check (push) Canceled after 0s
The receiver, resubmit and replay handlers now read and check the
request and make one call. Engine.StoreEvent stores an event with a
pending delivery for each active target and the totals in one
transaction, then queues them; Engine.CreateReplay does the same for
one replay delivery. The handlers take *delivery.Engine, so the
Notifier interface and its bridge are gone, and the engine's notify is
no longer exported. Archives and CircuitBreakers stay because the
handler tests fake them. The tests that read a queued task moved to
the engine's tests; the handler tests use an engine that is never
started.

Model: opus-5-5
2026-10-16 00:51:26 +02:00
clawbot 8406c77949 Store a target's settings as columns (closes #543)
check / check (push) Canceled after 0s
A target's settings move from the per-type JSON string in
targets.config to columns: url, headers, timeout_seconds,
forward_query, archive_expiry and archive_rotation. A column a type
does not use stays empty. headers holds the header map as JSON text
through GORM's JSON serializer, and archive_expiry holds a
time.Duration, 0 meaning never. The forms check each setting once,
with the same refusals and messages, and the edit form is filled from
the columns, so the per-type config structs, their parsers and the
"could not be read" branches are gone. A delivery task carries the
six settings in place of the JSON string; tests check that each copy
into a task, and back out of one, keeps all six.

Model: opus-5-5
2026-10-15 23:02:39 +02:00
sneak 766f33a956 Milestone: next into main (#380)
check / check (push) Canceled after 0s
Reviewed-on: #380
2026-10-15 11:36:37 +02:00
84 changed files with 3904 additions and 6154 deletions
+13 -8
View File
@@ -111,11 +111,16 @@ RUN --network=none golangci-lint run --config .golangci.yml --build-tags browser
# Nothing is wanted from js-lint; the copy is what makes this phase run it.
COPY --from=js-lint /src/yarn.lock /dev/null
# Test phase. -race needs cgo and so a C compiler, which the Debian Go image
# ships and the alpine one does not. `make test` (script/test) builds this
# stage alone; the build stage below depends on it.
# Test phase. The tests run with cgo off and so without Go's race detector,
# which needs cgo on Linux; sneak ruled the race detector out in
# https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273.
# `make test` (script/test) builds this stage alone; the build stage below
# depends on it.
#
# golang:1.26.1-bookworm (Debian-based), 2026-03-17
# golang:1.26.1-bookworm (Debian-based), 2026-03-17: the builder stage's
# image, which that stage needs for its git and make. Its test step runs
# under the image's bash, and the tests in internal/versionscript use its
# git, make and sh, and skip when they are missing.
FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS test
WORKDIR /src
@@ -131,8 +136,8 @@ RUN script/assets
# -timeout applies to each package on its own, so 90s has only to clear the
# slowest one. -p 4 -parallel 8 keep the run under 2 GB of memory: at most
# four test binaries build or run at once, each with at most eight parallel
# tests. Under -race every test binary and every link costs a few hundred MB,
# so the defaults (one per core) add up to several GB on a many-core host.
# tests. The defaults are one per core, so without these the memory the run
# needs grows with the host's core count.
#
# The first run has no -v: go test then prints one result line per package,
# with its coverage, and for a package that fails, everything its tests
@@ -154,12 +159,12 @@ RUN script/assets
SHELL ["/bin/bash", "-o", "pipefail", "-c"]
RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \
export TMPDIR=/tmp/tests GOTMPDIR=/tmp; \
go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
CGO_ENABLED=0 go test -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \
packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \
if [ -n "$tests" ]; then \
echo "--- Rerunning the failed tests with -v for details ---"; \
go test -race -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \
CGO_ENABLED=0 go test -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \
fi; \
exit 1
+3 -3
View File
@@ -15,9 +15,9 @@ RUN go mod download
COPY . .
# The test binary embeds the templates and static files, so the browser
# stage needs nothing else. -p 4 keeps the compile's memory down, as in
# the test phase of Dockerfile.
RUN make assets && go test -c -p 4 -tags browser -o /browser.test ./internal/server
# stage needs nothing else. It is built with cgo off and -p 4 keeps the
# compile's memory down, both as in the test phase of Dockerfile.
RUN make assets && CGO_ENABLED=0 go test -c -p 4 -tags browser -o /browser.test ./internal/server
# chromedp/headless-shell:151.0.7922.109 (Debian trixie), 2026-08-11. The
# browser is on PATH as headless-shell, where the test's browser library
+1494 -3367
View File
File diff suppressed because it is too large Load Diff
-3
View File
@@ -211,9 +211,6 @@ func newApp() *fx.App {
delivery.New,
archive.NewWriters,
archive.NewSweeper,
// Wire *delivery.Engine as delivery.Notifier so the
// webhook handler can notify the engine of new deliveries.
func(e *delivery.Engine) delivery.Notifier { return e },
// Wire *archive.Writers as archive.Archives so deleting
// or renaming a webhook or target reaches its archive
// files.
+7 -15
View File
@@ -143,26 +143,15 @@ func archiveNamePart(name string) string {
}
// Write writes the full event of a delivery to a database target as
// a row into the target's archive database, honouring the optional
// per-target expiry and rotation parsed from the target config JSON.
// With rotation, the event goes to the file for the period of its
// receive time.
// a row into the target's archive database, honouring the target's
// ArchiveExpiry and ArchiveRotation. With rotation, the event goes to
// the file for the period of its receive time.
func (ws *Writers) Write(d *database.Delivery) error {
webhookID := d.Event.WebhookID
if webhookID == "" {
return errArchiveMissingWebhookID
}
expiry, err := ParseExpiry(d.Target.Config)
if err != nil {
return err
}
rotation, err := ParseRotation(d.Target.Config)
if err != nil {
return err
}
// An event whose stored row was gone before its delivery ran has
// no receive time (see delivery's Engine.hydrateEvent), and goes
// to the file for now.
@@ -187,7 +176,10 @@ func (ws *Writers) Write(d *database.Delivery) error {
ContentType: d.Event.ContentType,
}
return w.write(row, expiry, archivePeriod(rotation, receivedAt))
return w.write(
row, d.Target.ArchiveExpiry,
archivePeriod(d.Target.ArchiveRotation, receivedAt),
)
}
// Rename implements Archives. It renames every one of a database
+20 -36
View File
@@ -123,9 +123,10 @@ func databaseTargetDelivery(
TargetID: tgt.ID,
Event: event,
Target: database.Target{
Name: tgt.Name,
Type: database.TargetTypeDatabase,
Config: tgt.Config,
Name: tgt.Name,
Type: database.TargetTypeDatabase,
ArchiveExpiry: tgt.ArchiveExpiry,
ArchiveRotation: tgt.ArchiveRotation,
},
}
}
@@ -355,13 +356,16 @@ func TestParseArchiveExpiry(t *testing.T) {
want time.Duration
wantErr bool
}{
{"empty config", "", 0, false},
{"explicit never", `{"expiry":"never"}`, 0, false},
{"empty expiry", `{"expiry":""}`, 0, false},
{"duration", `{"expiry":"1h"}`, time.Hour, false},
{"unparseable", `{"expiry":"nonsense"}`, 0, true},
{"zero duration", `{"expiry":"0s"}`, 0, true},
{"negative duration", `{"expiry":"-5h"}`, 0, true},
{"empty", "", 0, false},
{"explicit never", "never", 0, false},
{"hour", "1h", time.Hour, false},
{"thirty days", "720h", 720 * time.Hour, false},
{"minutes", "30m", 30 * time.Minute, false},
{"unparseable", "nonsense", 0, true},
{"days are not a Go unit", "7d", 0, true},
{"zero duration", "0s", 0, true},
{"bare zero", "0", 0, true},
{"negative duration", "-5h", 0, true},
}
for _, tc := range cases {
@@ -381,26 +385,6 @@ func TestParseArchiveExpiry(t *testing.T) {
}
}
func TestValidateArchiveExpiry(t *testing.T) {
t.Parallel()
valid := []string{"", "never", "1h", "720h", "30m"}
for _, in := range valid {
require.NoError(t,
archive.ValidateExpiry(in),
"expiry %q should be accepted", in,
)
}
invalid := []string{"nonsense", "7d", "-5h", "0s", "0"}
for _, in := range invalid {
require.Error(t,
archive.ValidateExpiry(in),
"expiry %q should be rejected", in,
)
}
}
// TestArchiveFileName pins the archive file name and the rules
// that make a webhook or target name safe to put in it.
func TestArchiveFileName(t *testing.T) {
@@ -473,8 +457,8 @@ func TestWrite_EachTargetHasItsOwnArchive(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
first := env.seedDatabaseTarget(t, "")
second := env.addDatabaseTarget(t, first.WebhookID, "")
first := env.seedDatabaseTarget(t, 0, "")
second := env.addDatabaseTarget(t, first.WebhookID, 0, "")
event := newEvent(`{"n":1}`)
@@ -503,7 +487,7 @@ func TestRename_MovesTheFile(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
oldPath := env.archivePath(tgt)
first := newEvent(`{"n":1}`)
@@ -545,7 +529,7 @@ func TestRename_NeverReplacesAFile(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
oldPath := env.archivePath(tgt)
first := newEvent(`{"n":1}`)
@@ -593,7 +577,7 @@ func TestRename_BeforeTheNameIsSaved(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
require.NoError(
t, env.writers.Rename(tgt.ID, "Orders", "Archive"),
@@ -721,7 +705,7 @@ func TestRenameLock_HeldByOneEditAtATime(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
event := newEvent(`{"n":1}`)
env.writers.RenameLock().Lock()
+9 -9
View File
@@ -37,9 +37,9 @@ func TestEvictWebhook_ClosesAndRemovesWriter(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
first := env.seedDatabaseTarget(t, "")
second := env.addDatabaseTarget(t, first.WebhookID, "")
other := env.seedDatabaseTarget(t, "")
first := env.seedDatabaseTarget(t, 0, "")
second := env.addDatabaseTarget(t, first.WebhookID, 0, "")
other := env.seedDatabaseTarget(t, 0, "")
for _, tgt := range []*database.Target{first, second, other} {
deliverTo(t, env, tgt)
@@ -80,8 +80,8 @@ func TestEvictTarget_LeavesOtherTargets(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
doomed := env.seedDatabaseTarget(t, "")
kept := env.addDatabaseTarget(t, doomed.WebhookID, "")
doomed := env.seedDatabaseTarget(t, 0, "")
kept := env.addDatabaseTarget(t, doomed.WebhookID, 0, "")
deliverTo(t, env, doomed)
deliverTo(t, env, kept)
@@ -303,14 +303,14 @@ func (r *racingWrites) wait() (bool, error) {
// contending for the writer's mutex when the eviction takes it.
// The write that wins the mutex after the eviction must abandon
// its work rather than reopen the archive, leaving the writer
// permanently handle-free. Run under -race.
// permanently handle-free.
func TestEvictWebhook_RacingWriteDoesNotReopenHandle(
t *testing.T,
) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
// Prime the registry so the test can hold the very writer the
// eviction is about to detach.
@@ -355,7 +355,7 @@ func TestEvictWebhook_LaterDeliveryRecreatesWriter(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
deliverTo(t, env, tgt)
require.True(t, env.writers.ExportHasArchiveWriter(tgt.ID))
@@ -380,7 +380,7 @@ func TestEvictAll_WriteAfterIsRefused(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, "")
deliverTo(t, env, tgt)
+5 -36
View File
@@ -1,7 +1,6 @@
package archive
import (
"encoding/json"
"errors"
"fmt"
"os"
@@ -54,35 +53,10 @@ func ValidateRotation(rotation string) error {
}
}
// ParseRotation reads the rotation from a database target's
// config JSON. An empty config or an empty rotation is none.
func ParseRotation(configJSON string) (string, error) {
if configJSON == "" {
return archiveRotationNone, nil
}
var cfg DatabaseTargetConfig
err := json.Unmarshal([]byte(configJSON), &cfg)
if err != nil {
return "", fmt.Errorf("parsing database target config: %w", err)
}
err = ValidateRotation(cfg.Rotation)
if err != nil {
return "", err
}
if cfg.Rotation == "" {
return archiveRotationNone, nil
}
return cfg.Rotation, nil
}
// archivePeriod returns the period, in UTC, that a rotation puts an
// event received at receivedAt in: "2026-10" for monthly,
// "2026-10-01" for daily, "2026-10-01-19" for hourly, and "" for none.
// "2026-10-01" for daily, "2026-10-01-19" for hourly, and "" for none
// or an empty rotation.
func archivePeriod(rotation string, receivedAt time.Time) string {
switch rotation {
case archiveRotationMonthly:
@@ -115,16 +89,11 @@ func PathAt(
webhook *database.Webhook,
target *database.Target,
receivedAt time.Time,
) (string, error) {
rotation, err := ParseRotation(target.Config)
if err != nil {
return "", err
}
) string {
return archivePeriodPath(
Path(dbMgr, webhook, target),
archivePeriod(rotation, receivedAt),
), nil
archivePeriod(target.ArchiveRotation, receivedAt),
)
}
// archiveFiles lists the database target's archive files that exist,
+20 -33
View File
@@ -14,16 +14,12 @@ import (
"sneak.berlin/go/webhooker/internal/database/databasetest"
)
// The archive rotations, and the configs of a daily target and of one
// whose rotation is not one of the four.
// The archive rotations.
const (
rotationNone = "none"
rotationMonthly = "monthly"
rotationDaily = "daily"
rotationHourly = "hourly"
dailyConfig = `{"rotation":"daily"}`
weeklyConfig = `{"rotation":"weekly"}`
)
// The periods the tests archive into most: two days, and an hour of
@@ -108,7 +104,7 @@ func TestWrite_RotatesAtEachPeriodBoundary(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"rotation":"`+tc.rotation+`"}`)
tgt := env.seedDatabaseTarget(t, 0, tc.rotation)
path := env.archivePath(tgt)
first := env.deliverReceivedAt(t, tgt, tc.before)
@@ -127,7 +123,7 @@ func TestWrite_RotatesAtEachPeriodBoundary(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"rotation":"`+rotationNone+`"}`)
tgt := env.seedDatabaseTarget(t, 0, rotationNone)
first := env.deliverReceivedAt(t, tgt, cases[0].before)
second := env.deliverReceivedAt(t, tgt, cases[0].after)
@@ -144,7 +140,7 @@ func TestWrite_EventWithoutReceiveTime(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"rotation":"`+rotationMonthly+`"}`)
tgt := env.seedDatabaseTarget(t, 0, rotationMonthly)
path := env.archivePath(tgt)
before := time.Now().UTC().Format("2006-01")
@@ -169,13 +165,13 @@ func TestWrite_RotationChangeKeepsOldFiles(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, rotationNone)
path := env.archivePath(tgt)
at := time.Date(2026, 3, 4, 12, 0, 0, 0, time.UTC)
first := env.deliverReceivedAt(t, tgt, at)
tgt.Config = dailyConfig
tgt.ArchiveRotation = rotationDaily
second := env.deliverReceivedAt(t, tgt, at)
assert.Equal(t, []string{first}, archivedEventIDs(t, path))
@@ -193,9 +189,7 @@ func TestArchiveSweep_PrunesEveryFile(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(
t, `{"expiry":"1h","rotation":"`+rotationDaily+`"}`,
)
tgt := env.seedDatabaseTarget(t, time.Hour, rotationDaily)
path := env.archivePath(tgt)
emptied := periodPath(path, dayPeriod)
kept := periodPath(path, nextDayPeriod)
@@ -225,13 +219,13 @@ func TestRename_MovesEveryFile(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t, 0, rotationNone)
oldPath := env.archivePath(tgt)
day := time.Date(2026, 3, 4, 12, 0, 0, 0, time.UTC)
unrotated := env.deliverReceivedAt(t, tgt, day)
tgt.Config = dailyConfig
tgt.ArchiveRotation = rotationDaily
first := env.deliverReceivedAt(t, tgt, day)
second := env.deliverReceivedAt(t, tgt, day.Add(24*time.Hour))
@@ -267,7 +261,7 @@ func TestRename_NeverReplacesARotatedFile(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, dailyConfig)
tgt := env.seedDatabaseTarget(t, 0, rotationDaily)
oldPath := env.archivePath(tgt)
day := time.Date(2026, 3, 4, 12, 0, 0, 0, time.UTC)
@@ -341,34 +335,27 @@ func TestArchivePathAt(t *testing.T) {
at := time.Date(2026, 10, 1, 19, 30, 0, 0, time.UTC)
cases := map[string]string{
"": "",
`{"rotation":"` + rotationNone + `"}`: "",
`{"rotation":"` + rotationMonthly + `"}`: "-2026-10",
dailyConfig: "-2026-10-01",
`{"rotation":"` + rotationHourly + `"}`: "-2026-10-01-19",
"": "",
rotationNone: "",
rotationMonthly: "-2026-10",
rotationDaily: "-2026-10-01",
rotationHourly: "-2026-10-01-19",
}
for config, period := range cases {
for rotation, period := range cases {
target := &database.Target{
BaseModel: database.BaseModel{ID: "tgt-id"},
Name: "Archive",
Config: config,
BaseModel: database.BaseModel{ID: "tgt-id"},
Name: "Archive",
ArchiveRotation: rotation,
}
got, err := archive.PathAt(dbMgr, webhook, target, at)
require.NoError(t, err, config)
assert.Equal(t,
filepath.Join(
dataDir, "archive-orders-archive-tgt-id"+period+".db",
),
got, config,
archive.PathAt(dbMgr, webhook, target, at), rotation,
)
}
_, err := archive.PathAt(dbMgr, webhook, &database.Target{
Config: weeklyConfig,
}, at)
require.Error(t, err)
}
// TestValidateArchiveRotation accepts the four rotations, and empty,
+5 -16
View File
@@ -199,22 +199,11 @@ func (s *Sweeper) sweep(ctx context.Context) {
}
}
// sweepTarget prunes the archive of a single database target.
// A missing, empty, or "never" expiry parses as a zero duration
// and is skipped entirely, so those archives are never pruned.
// sweepTarget prunes the archive of a single database target. A
// target with an ArchiveExpiry of 0 keeps its archive forever and is
// skipped entirely, so that archive is never pruned.
func (s *Sweeper) sweepTarget(target *database.Target) {
expiry, err := ParseExpiry(target.Config)
if err != nil {
s.log.Error(
"archive sweep: invalid database target config",
"webhook_id", target.WebhookID,
"target_id", target.ID,
"error", err,
)
return
}
expiry := target.ArchiveExpiry
if expiry <= 0 {
return
}
@@ -223,7 +212,7 @@ func (s *Sweeper) sweepTarget(target *database.Target) {
return
}
err = s.writers.sweepArchive(target.ID, expiry)
err := s.writers.sweepArchive(target.ID, expiry)
if err == nil {
return
}
+56 -62
View File
@@ -106,10 +106,10 @@ func (env *archiveEnv) archivePath(tgt *database.Target) string {
)
}
// seedDatabaseTarget creates a webhook with one database target
// carrying the given target config JSON, and returns the target.
// seedDatabaseTarget creates a webhook with one database target with
// the given archive expiry and rotation, and returns the target.
func (env *archiveEnv) seedDatabaseTarget(
t *testing.T, configJSON string,
t *testing.T, expiry time.Duration, rotation string,
) *database.Target {
t.Helper()
@@ -124,23 +124,27 @@ func (env *archiveEnv) seedDatabaseTarget(
Create(wh).Error,
)
return env.addDatabaseTarget(t, wh.ID, configJSON)
return env.addDatabaseTarget(t, wh.ID, expiry, rotation)
}
// addDatabaseTarget creates one more database target on an
// existing webhook and returns it.
func (env *archiveEnv) addDatabaseTarget(
t *testing.T, webhookID, configJSON string,
t *testing.T,
webhookID string,
expiry time.Duration,
rotation string,
) *database.Target {
t.Helper()
tgt := &database.Target{
WebhookID: webhookID,
Name: "Archive",
Type: database.TargetTypeDatabase,
Active: true,
Config: configJSON,
MaxAttempts: 1,
WebhookID: webhookID,
Name: "Archive",
Type: database.TargetTypeDatabase,
Active: true,
ArchiveExpiry: expiry,
ArchiveRotation: rotation,
MaxAttempts: 1,
}
require.NoError(
t,
@@ -323,7 +327,7 @@ func TestArchiveSweeper_LoopOutlivesStartHookContext(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
now := time.Now()
path := env.seedArchiveRows(
@@ -377,7 +381,7 @@ func TestArchiveSweep_DoesNotResurrectEvictedWriter(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
@@ -411,7 +415,7 @@ func TestArchiveSweep_LeavesNoRegistryEntry(t *testing.T) {
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
path := env.seedArchiveRows(
t, tgt,
time.Now().Add(-48*time.Hour),
@@ -444,7 +448,7 @@ func TestArchiveSweep_KeepsWriterAdoptedByDelivery(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
@@ -498,7 +502,7 @@ func TestArchiveSweep_KeepsWriterAdoptedDuringSweep(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
@@ -551,8 +555,8 @@ func TestArchiveSweep_KeepsWriterAdoptedDuringSweep(
// TestArchiveSweep_ContinuesAfterPerTargetFailure proves a
// failure for one target does not abort the sweep for the
// others: an unparseable expiry and an unreadable archive both
// have to be logged and stepped over.
// others: an unreadable archive has to be logged and stepped
// over.
func TestArchiveSweep_ContinuesAfterPerTargetFailure(
t *testing.T,
) {
@@ -560,21 +564,16 @@ func TestArchiveSweep_ContinuesAfterPerTargetFailure(
env := setupArchiveTest(t)
// Seeded first so the sweep reaches them before the healthy
// Seeded first so the sweep reaches it before the healthy
// target: targets come back in insertion order.
badConfig := env.seedDatabaseTarget(t, `{"expiry":"!!!"}`)
env.seedArchiveRows(
t, badConfig, time.Now().Add(-48*time.Hour),
)
corrupt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
corrupt := env.seedDatabaseTarget(t, time.Hour, "")
require.NoError(t, os.WriteFile(
env.archivePath(corrupt),
[]byte("this is not a sqlite database"),
0o600,
))
healthy := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
healthy := env.seedDatabaseTarget(t, time.Hour, "")
healthyPath := env.seedArchiveRows(
t, healthy,
time.Now().Add(-48*time.Hour),
@@ -631,7 +630,7 @@ func TestArchiveSweep_PrunesIdleArchive(t *testing.T) {
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
now := time.Now()
path := env.seedArchiveRows(
@@ -670,7 +669,7 @@ func TestArchiveSweep_LeavesArchiveClosed(t *testing.T) {
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
path := env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
@@ -710,7 +709,7 @@ func TestArchiveSweep_ClosesHandleOfRegisteredWriter(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
@@ -795,36 +794,29 @@ func TestArchiveSweep_CancelledSweepLogsNoError(t *testing.T) {
}
// TestArchiveSweep_NeverExpiryUntouched proves the sweep is a
// no-op for the default retention policy, so archives with no
// expiry (or the literal "never") behave exactly as before.
// no-op for the default retention policy, so an archive with an
// expiry of 0, which keeps it forever, is left alone.
func TestArchiveSweep_NeverExpiryUntouched(t *testing.T) {
t.Parallel()
for _, configJSON := range []string{
`{"expiry":"never"}`,
`{"expiry":""}`,
"",
} {
env := setupArchiveTest(t)
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, configJSON)
path := env.seedArchiveRows(
t, tgt,
time.Now().Add(-10000*time.Hour),
)
tgt := env.seedDatabaseTarget(t, 0, "")
path := env.seedArchiveRows(
t, tgt,
time.Now().Add(-10000*time.Hour),
)
env.sweeper.ExportSweep(context.Background())
env.sweeper.ExportSweep(context.Background())
assert.Equal(
t, []string{sweepRowOld}, archivedEventIDs(t, path),
"config %q must keep rows forever", configJSON,
)
assert.False(
t, env.writers.ExportHasArchiveWriter(tgt.ID),
"config %q must leave no registry entry behind",
configJSON,
)
}
assert.Equal(
t, []string{sweepRowOld}, archivedEventIDs(t, path),
"an expiry of 0 must keep rows forever",
)
assert.False(
t, env.writers.ExportHasArchiveWriter(tgt.ID),
"an expiry of 0 must leave no registry entry behind",
)
}
// TestArchiveSweep_NeverExpirySkipsBeforeOpening pins the
@@ -845,7 +837,7 @@ func TestArchiveSweep_NeverExpirySkipsBeforeOpening(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"never"}`)
tgt := env.seedDatabaseTarget(t, 0, "")
path := env.archivePath(tgt)
seedUnmigratedArchive(t, path)
@@ -897,7 +889,7 @@ func TestArchiveSweep_DoesNotCreateArchiveFile(t *testing.T) {
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
path := env.archivePath(tgt)
require.NoFileExists(t, path)
@@ -928,7 +920,7 @@ func TestArchiveSweep_DoesNotCreateAfterWriterExists(
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
path, err := env.writers.ExportEnsureArchiveWriter(tgt.ID)
require.NoError(t, err)
@@ -947,7 +939,7 @@ func TestArchiveSweep_SkipsDeletedWebhookTargets(t *testing.T) {
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
path := env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
@@ -967,15 +959,17 @@ func TestArchiveSweep_SkipsDeletedWebhookTargets(t *testing.T) {
)
}
// TestArchiveSweep_ConcurrentWrites proves the sweep serialises
// against writes through the target's writer mutex. Run under
// -race, an unsynchronised sweep would be caught here.
// TestArchiveSweep_ConcurrentWrites runs the sweep while writes to
// the same target are under way. The sweep serialises against them
// through the target's writer mutex; the suite runs without Go's race
// detector, so a sweep that did not would fail this test only by
// panicking, hanging or leaving no archive file.
func TestArchiveSweep_ConcurrentWrites(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
deliveries := make(
[]*database.Delivery, 0, sweepConcurrentWrites,
@@ -1020,7 +1014,7 @@ func TestArchiveSweeper_StopsCleanly(t *testing.T) {
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"1h"}`)
tgt := env.seedDatabaseTarget(t, time.Hour, "")
env.seedArchiveRows(
t, tgt, time.Now().Add(-48*time.Hour),
)
+14 -70
View File
@@ -1,7 +1,6 @@
package archive
import (
"encoding/json"
"errors"
"fmt"
"io/fs"
@@ -18,8 +17,9 @@ import (
"sneak.berlin/go/webhooker/internal/gormlog"
)
// ExpiryNever is the expiry sentinel (and default) that
// disables pruning so archived rows are kept forever.
// ExpiryNever is the archive expiry, as the target forms take it,
// that disables pruning so archived rows are kept forever. It is
// stored as an ArchiveExpiry of 0.
const ExpiryNever = "never"
// archiveReopenDebounce bounds how often an archive file is
@@ -79,19 +79,6 @@ var (
)
)
// DatabaseTargetConfig is the optional per-target JSON config
// for a database (archive) target.
type DatabaseTargetConfig struct {
// Expiry is a Go duration (e.g. "720h") after which
// archived rows are pruned, or "never" (the default) to
// keep them forever.
Expiry string `json:"expiry"`
// Rotation is none (the default), monthly, daily or hourly: see
// archivePeriod.
Rotation string `json:"rotation"`
}
// archivedEvent is one fully captured webhook event stored in a
// database target's archive for long-term retention. It is a
// self-contained copy — independent of the per-webhook event
@@ -112,63 +99,20 @@ type archivedEvent struct {
ArchivedAt time.Time `gorm:"index"`
}
// ParseExpiry reads the optional expiry from a database
// target's config JSON. An empty config, an empty expiry, or
// the literal "never" all mean keep forever, returned as a zero
// duration. Any other value must parse as a positive Go
// duration; a set-but-invalid value (unparseable, zero, or
// negative) is an error rather than a silent default, matching
// ValidateExpiry at target creation.
func ParseExpiry(
configJSON string,
) (time.Duration, error) {
if configJSON == "" {
return 0, nil
}
var cfg DatabaseTargetConfig
err := json.Unmarshal([]byte(configJSON), &cfg)
if err != nil {
return 0, fmt.Errorf(
"parsing database target config: %w", err,
)
}
if cfg.Expiry == "" || cfg.Expiry == ExpiryNever {
return 0, nil
}
dur, err := time.ParseDuration(cfg.Expiry)
if err != nil {
return 0, fmt.Errorf(
"parsing archive expiry %q: %w", cfg.Expiry, err,
)
}
if dur <= 0 {
return 0, fmt.Errorf(
"%w: %q", errArchiveExpiryNotPositive, cfg.Expiry,
)
}
return dur, nil
}
// ValidateExpiry checks a user-supplied archive expiry
// for a database target at configuration time. Valid values are
// empty, "never" (both meaning keep forever), or a positive Go
// duration such as "720h". Anything else is an error, so a bad
// expiry is rejected when the target is created rather than
// failing every subsequent delivery.
func ValidateExpiry(expiry string) error {
// ParseExpiry reads a user-supplied archive expiry for a database
// target, as the target forms submit it, into the ArchiveExpiry the
// target stores. Empty and "never" both mean keep forever, which is
// 0; anything else must be a positive Go duration such as "720h". Any
// other value is an error, so a bad expiry is refused at the form
// rather than stored.
func ParseExpiry(expiry string) (time.Duration, error) {
if expiry == "" || expiry == ExpiryNever {
return nil
return 0, nil
}
dur, err := time.ParseDuration(expiry)
if err != nil {
return fmt.Errorf(
return 0, fmt.Errorf(
"expiry must be %q or a Go duration "+
"such as \"720h\": %w",
ExpiryNever, err,
@@ -176,12 +120,12 @@ func ValidateExpiry(expiry string) error {
}
if dur <= 0 {
return fmt.Errorf(
return 0, fmt.Errorf(
"%w: %q", errArchiveExpiryNotPositive, expiry,
)
}
return nil
return dur, nil
}
// archiveWriter owns one database target's archive SQLite files.
+1 -1
View File
@@ -28,7 +28,7 @@ func TestEventDBCreateOmitsAssociations(t *testing.T) {
WebhookID: webhookID,
Name: "leaky-target",
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"https://hooks.example/secret"}`,
URL: "https://hooks.example/secret",
MaxAttempts: 1,
}
target.ID = uuid.New().String()
+18 -7
View File
@@ -29,8 +29,10 @@ func marshalModel(t *testing.T, v any) string {
// these tags the first handler that marshals a model serialises the
// secret with it. Each field below is a live credential:
//
// - Target.Config holds an incoming-webhook URL whose path segments
// are the bearer token.
// - Target.URL is an incoming-webhook URL whose path segments are
// the bearer token.
// - Target.Headers holds header values, routinely authorization
// tokens.
// - APIKey.Key is a bearer token outright.
// - Setting.Value holds the session encryption key.
// - User.Password holds the Argon2 hash, and was already tagged.
@@ -44,11 +46,20 @@ func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
model any
}{
{
name: "target config",
name: "target url",
model: database.Target{
Name: keptField,
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
URL: "https://h/s/" + marker,
MaxAttempts: 1,
},
},
{
name: "target headers",
model: database.Target{
Name: keptField,
Type: database.TargetTypeHTTP,
Headers: map[string]string{"Authorization": marker},
MaxAttempts: 1,
},
},
@@ -87,10 +98,10 @@ func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
}
}
// TestWebhookMarshalsNoTargetConfig covers the nested case: a webhook
// TestWebhookMarshalsNoTargetURL covers the nested case: a webhook
// marshalled with its targets preloaded must not carry the credential
// through the association either.
func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
func TestWebhookMarshalsNoTargetURL(t *testing.T) {
t.Parallel()
const marker = "QQNESTEDMARKERQQ"
@@ -99,7 +110,7 @@ func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
Name: keptField,
Targets: []database.Target{{
Name: "slack",
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
URL: "https://h/s/" + marker,
MaxAttempts: 1,
}},
})
+33 -8
View File
@@ -1,5 +1,7 @@
package database
import "time"
// TargetType represents the type of delivery target
type TargetType string
@@ -11,7 +13,9 @@ const (
TargetTypeSlack TargetType = "slack"
)
// Target represents a delivery target for a webhook
// Target represents a delivery target for a webhook. Its settings
// are the fields from URL to ArchiveRotation; each type uses some of
// them, and the ones a type does not use stay empty.
type Target struct {
BaseModel
@@ -20,14 +24,35 @@ type Target struct {
Type TargetType `gorm:"not null" json:"type"`
Active bool `gorm:"default:true" json:"active"`
// Configuration fields (JSON stored based on type).
// URL is where an http or slack target sends each event.
//
// json:"-" because the blob holds the target's credential — a
// Slack incoming-webhook URL, or an http destination whose path
// segments are the secret. delivery.TargetView is the masking
// barrier for the HTML path; this tag is the barrier for any
// handler that marshals the model itself.
Config string `gorm:"type:text" json:"-"` // JSON configuration
// json:"-" because the URL is the target's credential: a Slack
// incoming-webhook URL, or an http destination whose path segments
// are the secret. delivery.TargetView is the masking barrier for
// the HTML path; this tag is the barrier for any handler that
// marshals the model itself.
URL string `json:"-"`
// Headers is an http target's extra request headers, name to
// value, stored as JSON text. json:"-" for the same reason as URL:
// a header value is routinely an authorization token.
Headers map[string]string `gorm:"serializer:json" json:"-"`
// TimeoutSeconds is an http target's per-request timeout; 0 uses
// the delivery engine's shared client timeout.
TimeoutSeconds int `json:"timeoutSeconds,omitempty"`
// ForwardQuery passes each event's query string on to an http
// target, appended to URL. Off, URL is sent exactly as stored.
ForwardQuery bool `json:"forwardQuery,omitempty"`
// ArchiveExpiry is how long a database target keeps each archived
// event; 0 keeps them forever.
ArchiveExpiry time.Duration `json:"archiveExpiry,omitempty"`
// ArchiveRotation is how often a database target starts a new
// archive file: none, monthly, daily or hourly.
ArchiveRotation string `json:"archiveRotation,omitempty"`
// MaxAttempts is how many times an HTTP or Slack target attempts
// each delivery in all, at least 1. 1 is a single attempt with no
+20 -1
View File
@@ -16,7 +16,7 @@ func newHTTPTarget(maxAttempts int) *database.Target {
WebhookID: uuid.New().String(),
Name: "target",
Type: database.TargetTypeHTTP,
Config: `{"url":"https://example.com/"}`,
URL: "https://example.com/",
MaxAttempts: maxAttempts,
}
}
@@ -42,3 +42,22 @@ func TestTargetCreate_AcceptsOneAttempt(t *testing.T) {
require.NoError(t, db.Create(newHTTPTarget(1)).Error)
}
// TestTargetHeaders_StoredAsJSON reads the headers column back as
// text, to show it holds the header map as JSON.
func TestTargetHeaders_StoredAsJSON(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
target := newHTTPTarget(1)
target.Headers = map[string]string{"Authorization": "Bearer x"}
require.NoError(t, db.Create(target).Error)
var headers string
require.NoError(t, db.Raw(
"SELECT headers FROM targets WHERE id = ?", target.ID,
).Scan(&headers).Error)
assert.JSONEq(t, `{"Authorization":"Bearer x"}`, headers)
}
+4 -3
View File
@@ -67,9 +67,10 @@ func DefaultPasswordConfig() *PasswordConfig {
// testArgon2Memory is the Argon2id memory cost, in KiB, that a test
// binary hashes with: 1 MB instead of the shipped 64 MB. Every test
// that starts a database hashes the bootstrap admin password, dozens
// of them run in parallel, and under the race detector each 64 MB hash
// holds about 150 MB. VerifyPassword reads the cost from the hash it
// checks, so verification follows.
// of them run in parallel, and each 64 MB hash holds at least 64 MB
// while it runs (about 150 MB when the suite ran under the race
// detector). VerifyPassword reads the cost from the hash it checks, so
// verification follows.
const testArgon2Memory = 1024
// hashAtShippedCostInTest makes a test binary hash at the shipped
+12 -7
View File
@@ -87,13 +87,18 @@ func TestMainSchemaHoldsEveryModel(t *testing.T) {
Active: true,
})
assertRoundTrip(t, db, &database.Target{
BaseModel: newBaseModel(),
WebhookID: uuid.New().String(),
Name: "target",
Type: database.TargetTypeHTTP,
Active: true,
Config: `{"url":"https://example.com/"}`,
MaxAttempts: 3,
BaseModel: newBaseModel(),
WebhookID: uuid.New().String(),
Name: "target",
Type: database.TargetTypeHTTP,
Active: true,
URL: "https://example.com/",
Headers: map[string]string{"X-Api-Key": "key"},
TimeoutSeconds: 30,
ForwardQuery: true,
ArchiveExpiry: 720 * time.Hour,
ArchiveRotation: "daily",
MaxAttempts: 3,
})
}
+6 -1
View File
@@ -79,7 +79,12 @@ CREATE TABLE targets (
name text NOT NULL,
type text NOT NULL,
active numeric DEFAULT true,
config text,
url text,
headers text,
timeout_seconds integer,
forward_query numeric,
archive_expiry integer,
archive_rotation text,
max_attempts integer NOT NULL,
PRIMARY KEY (id),
CONSTRAINT fk_webhooks_targets
+3 -19
View File
@@ -46,12 +46,7 @@ func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
t.Run(target, func(t *testing.T) {
t.Parallel()
cfg := &delivery.HTTPTargetConfig{
URL: target,
Timeout: 5,
}
client := engine.ExportClientForRequest(cfg, nil)
client := engine.ExportClientForRequest(5, nil)
require.NotSame(t, engine.ExportClient(), client,
"a per-target timeout must yield a "+
@@ -101,11 +96,7 @@ func TestClientForRequest_NoTimeoutUnchanged(t *testing.T) {
engine := newSSRFTestEngine()
cfg := &delivery.HTTPTargetConfig{
URL: "https://example.com/hook",
}
client := engine.ExportClientForRequest(cfg, nil)
client := engine.ExportClientForRequest(0, nil)
assert.Same(t, engine.ExportClient(), client,
"without a per-target timeout the shared client "+
@@ -123,15 +114,8 @@ func TestClientForRequest_HeadersKeepSSRFGuard(t *testing.T) {
engine := newSSRFTestEngine()
cfg := &delivery.HTTPTargetConfig{
URL: "https://example.com/with-headers",
Headers: map[string]string{
"X-Api-Key": "configured",
},
}
client := engine.ExportClientForRequest(
cfg, []string{"X-Api-Key"},
0, []string{probeHeaderName},
)
require.NotNil(t, client.CheckRedirect,
+101 -84
View File
@@ -1,5 +1,5 @@
// Package delivery manages asynchronous event delivery
// to configured targets.
// Package delivery stores each event with its deliveries and delivers
// them, asynchronously, to configured targets.
package delivery
import (
@@ -23,10 +23,11 @@ import (
const (
// deliveryChannelSize is the buffer size for the delivery
// channel. New Tasks from the webhook handler are sent
// here. Workers drain this channel. Sized large enough
// that the webhook handler should never block under
// normal load.
// channel. The Tasks of newly stored deliveries, and of the
// pending deliveries the recovery routine hands back, are
// sent here. Workers drain this channel. Sized large enough
// that no task should be dropped under normal load: one that
// does not fit is left for the sweep.
deliveryChannelSize = 10000
// retryChannelSize is the buffer size for the retry
@@ -105,12 +106,19 @@ type Task struct {
WebhookID string
EntrypointID string
TargetID string
TargetName string
TargetType database.TargetType
TargetConfig string
MaxAttempts int
// The target's fields, as they were when the task was made.
TargetID string
TargetName string
TargetType database.TargetType
TargetURL string
TargetHeaders map[string]string
TimeoutSeconds int
ForwardQuery bool
ArchiveExpiry time.Duration
ArchiveRotation string
MaxAttempts int
// The event's fields.
Method string
RawQuery string
Headers string
@@ -120,17 +128,11 @@ type Task struct {
AttemptNum int
}
// Notifier is the interface for notifying the delivery
// engine about new deliveries.
type Notifier interface {
Notify(tasks []Task)
}
// CircuitBreakers is how the handlers read a target's circuit
// breaker, so the webhook page and the event log can say that
// deliveries to the target are paused and until when. It keeps the
// handlers free of the engine's internals and is trivially faked in
// tests.
// deliveries to the target are paused and until when. The handlers
// hold the engine itself as well; they read breakers through this
// interface so that a test can make one read as open.
type CircuitBreakers interface {
StateAndCooldown(targetID string) (CircuitState, time.Duration)
}
@@ -150,12 +152,13 @@ type EngineParams struct {
}
// Engine processes queued deliveries in the background
// using a bounded worker pool architecture. It owns only
// the cross-target machinery: the worker pool, the queue
// and retry channels, the recovery routine, and the persistence
// helpers and Scheduler that individual targets rely on.
// Each target type owns its own delivery, including retries,
// backoff, and circuit breaking.
// using a bounded worker pool architecture. It also stores each
// event with its deliveries and creates replay deliveries (see
// store.go). Beyond that it owns only the cross-target machinery:
// the worker pool, the delivery and retry channels, the recovery
// routine, and the persistence helpers and Scheduler that
// individual targets rely on. Each target type owns its own
// delivery, including retries, backoff, and circuit breaking.
type Engine struct {
database *database.Database
dbManager *database.WebhookDBManager
@@ -225,38 +228,6 @@ func New(
return e
}
// Notify signals the delivery engine that new deliveries
// are ready.
func (e *Engine) Notify(tasks []Task) {
for i := range tasks {
// Owned before it is queued, and until the worker that runs
// it returns. A task can sit in a 10000-deep channel for a
// long time on a healthy system, and nothing may re-send it
// while it waits. See inflight.go.
if !e.inflight.retainIdle(tasks[i].DeliveryID) {
e.log.Warn(
"delivery already in flight, not queued again",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
continue
}
select {
case e.deliveryCh <- tasks[i]:
default:
e.inflight.release(tasks[i].DeliveryID)
e.log.Warn(
"delivery channel full, "+
"task will be recovered by the sweep",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
}
}
}
// StateAndCooldown implements CircuitBreakers. It returns the state of
// the target's circuit breaker and, while the breaker is open, what is
// left of its cooldown; the cooldown is zero once that has passed and
@@ -317,6 +288,38 @@ func (e *Engine) ScheduleRetry(
})
}
// notify queues the tasks of deliveries already stored. StoreEvent and
// CreateReplay call it once their transaction has committed.
func (e *Engine) notify(tasks []Task) {
for i := range tasks {
// Owned before it is queued, and until the worker that runs
// it returns. A task can sit in a 10000-deep channel for a
// long time on a healthy system, and nothing may re-send it
// while it waits. See inflight.go.
if !e.inflight.retainIdle(tasks[i].DeliveryID) {
e.log.Warn(
"delivery already in flight, not queued again",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
continue
}
select {
case e.deliveryCh <- tasks[i]:
default:
e.inflight.release(tasks[i].DeliveryID)
e.log.Warn(
"delivery channel full, "+
"task will be recovered by the sweep",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
}
}
}
// userAgent is the User-Agent header of every http and slack
// delivery request: the program name and the running build's
// version.
@@ -465,9 +468,9 @@ func (e *Engine) processNewTask(
}
// The recovery routine's run at start can send and release this
// delivery before the receiver's Notify queues it. Ownership cannot
// refuse a delivery nobody holds, so the row decides whether it
// still needs sending.
// delivery before StoreEvent or CreateReplay queues it. Ownership
// cannot refuse a delivery nobody holds, so the row decides whether
// it still needs sending.
row, err := e.loadDelivery(webhookDB, task.DeliveryID)
if err != nil {
e.log.Error(
@@ -892,7 +895,7 @@ func (e *Engine) recoverSingleRetry(
return
}
e.ScheduleRetry(buildRecoveryTask(
e.ScheduleRetry(buildTask(
d, webhookID, &event, &target, attemptNum+1,
), remaining)
@@ -914,7 +917,7 @@ func (e *Engine) recoverSingleRetry(
// it stopped working on before then: for example one whose bookkeeping
// write failed, whose attempt a stop cut off, that was still queued
// when the engine last stopped, whose task was dropped because the
// delivery channel was full (by Notify or by queueRecovered), or whose
// delivery channel was full (by notify or by queueRecovered), or whose
// worker could not open the webhook's database.
//
// At start every pending delivery is taken, however recent, so one a
@@ -1543,10 +1546,15 @@ func buildEventFromTask(task *Task) database.Event {
func buildTargetFromTask(task *Task) database.Target {
target := database.Target{
Name: task.TargetName,
Type: task.TargetType,
Config: task.TargetConfig,
MaxAttempts: task.MaxAttempts,
Name: task.TargetName,
Type: task.TargetType,
URL: task.TargetURL,
Headers: task.TargetHeaders,
TimeoutSeconds: task.TimeoutSeconds,
ForwardQuery: task.ForwardQuery,
ArchiveExpiry: task.ArchiveExpiry,
ArchiveRotation: task.ArchiveRotation,
MaxAttempts: task.MaxAttempts,
}
target.ID = task.TargetID
@@ -1810,7 +1818,11 @@ func (e *Engine) loadTarget(
return target, nil
}
func buildRecoveryTask(
// buildTask builds the task that carries delivery d of event to target
// through the engine's channels, as attempt attemptNum. The body is
// carried only when it is under MaxInlineBodySize; a larger one is read
// from the event row when the attempt runs (see hydrateEvent).
func buildTask(
d *database.Delivery,
webhookID string,
event *database.Event,
@@ -1825,21 +1837,26 @@ func buildRecoveryTask(
}
return Task{
DeliveryID: d.ID,
EventID: d.EventID,
WebhookID: webhookID,
EntrypointID: event.EntrypointID,
TargetID: target.ID,
TargetName: target.Name,
TargetType: target.Type,
TargetConfig: target.Config,
MaxAttempts: target.MaxAttempts,
Method: event.Method,
RawQuery: event.RawQuery,
Headers: event.Headers,
ContentType: event.ContentType,
Body: bodyPtr,
AttemptNum: attemptNum,
DeliveryID: d.ID,
EventID: d.EventID,
WebhookID: webhookID,
EntrypointID: event.EntrypointID,
TargetID: target.ID,
TargetName: target.Name,
TargetType: target.Type,
TargetURL: target.URL,
TargetHeaders: target.Headers,
TimeoutSeconds: target.TimeoutSeconds,
ForwardQuery: target.ForwardQuery,
ArchiveExpiry: target.ArchiveExpiry,
ArchiveRotation: target.ArchiveRotation,
MaxAttempts: target.MaxAttempts,
Method: event.Method,
RawQuery: event.RawQuery,
Headers: event.Headers,
ContentType: event.ContentType,
Body: bodyPtr,
AttemptNum: attemptNum,
}
}
@@ -1981,7 +1998,7 @@ func (e *Engine) sendRecoveredDeliveries(
continue
}
task := buildRecoveryTask(
task := buildTask(
&deliveries[i], webhookID, &event, &target,
attempts[deliveries[i].ID]+1,
)
+88 -140
View File
@@ -3,7 +3,6 @@ package delivery_test
import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"log/slog"
@@ -97,17 +96,6 @@ func iSeedWebhookDB(
return db
}
func iHTTPConfig(url string) string {
cfg := delivery.HTTPTargetConfig{URL: url}
data, err := json.Marshal(cfg)
if err != nil {
panic("failed to marshal HTTPTargetConfig")
}
return string(data)
}
func iEngine(
t *testing.T, workers int,
) *delivery.Engine {
@@ -169,24 +157,24 @@ func iSeedDelivery(
func iTask(
d database.Delivery,
event database.Event,
webhookID, targetID, name, config string,
webhookID, targetID, name, targetURL string,
maxAttempts, attemptNum int,
body *string,
) delivery.Task {
return delivery.Task{
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: webhookID,
TargetID: targetID,
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetConfig: config,
MaxAttempts: maxAttempts,
Method: event.Method,
Headers: event.Headers,
ContentType: event.ContentType,
Body: body,
AttemptNum: attemptNum,
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: webhookID,
TargetID: targetID,
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetURL: targetURL,
MaxAttempts: maxAttempts,
Method: event.Method,
Headers: event.Headers,
ContentType: event.ContentType,
Body: body,
AttemptNum: attemptNum,
}
}
@@ -239,10 +227,9 @@ func TestProcessNewTask_InlineBody(t *testing.T) {
)
bodyStr := event.Body
cfg := iHTTPConfig(ts.URL)
task := iTask(
d, event, s.WebhookID, targetID,
"test-target", cfg, 1, 1, &bodyStr,
"test-target", ts.URL, 1, 1, &bodyStr,
)
s.Engine.ExportProcessNewTask(
@@ -290,10 +277,9 @@ func TestProcessNewTask_LargeBody_FetchFromDB(
database.DeliveryStatusPending,
)
cfg := iHTTPConfig(ts.URL)
task := iTask(
d, event, s.WebhookID, targetID,
"test-large", cfg, 1, 1, nil,
"test-large", ts.URL, 1, 1, nil,
)
s.Engine.ExportProcessNewTask(
@@ -313,16 +299,16 @@ func TestProcessNewTask_InvalidWebhookID(t *testing.T) {
s := newISetup(t)
task := delivery.Task{
DeliveryID: uuid.New().String(),
EventID: uuid.New().String(),
WebhookID: uuid.New().String(),
TargetID: uuid.New().String(),
TargetName: "test",
TargetType: database.TargetTypeHTTP,
TargetConfig: iHTTPConfig("http://localhost:9999"),
MaxAttempts: 1,
Body: nil,
AttemptNum: 1,
DeliveryID: uuid.New().String(),
EventID: uuid.New().String(),
WebhookID: uuid.New().String(),
TargetID: uuid.New().String(),
TargetName: "test",
TargetType: database.TargetTypeHTTP,
TargetURL: "http://localhost:9999",
MaxAttempts: 1,
Body: nil,
AttemptNum: 1,
}
s.Engine.ExportProcessNewTask(
@@ -363,7 +349,6 @@ func TestProcessRetryTask_SuccessfulRetry(t *testing.T) {
)
bodyStr := event.Body
cfg := iHTTPConfig(ts.URL)
// The target row exists because the engine confirms a scheduled
// retry's target has not been deleted before it runs it. A retry
@@ -372,12 +357,12 @@ func TestProcessRetryTask_SuccessfulRetry(t *testing.T) {
// task. See https://git.eeqj.de/sneak/webhooker/issues/107.
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "retry-target",
database.TargetTypeHTTP, cfg, 5,
database.TargetTypeHTTP, ts.URL, 5,
)
task := iTask(
d, event, s.WebhookID, targetID,
"retry-target", cfg, 5, 2, &bodyStr,
"retry-target", ts.URL, 5, 2, &bodyStr,
)
s.Engine.ExportProcessRetryTask(
@@ -410,10 +395,9 @@ func TestProcessRetryTask_SkipsNonRetryingDelivery(
)
bodyStr := event.Body
cfg := iHTTPConfig("http://localhost:1")
task := iTask(
d, event, s.WebhookID, targetID,
"skip-target", cfg, 5, 2, &bodyStr,
"skip-target", "http://localhost:1", 5, 2, &bodyStr,
)
s.Engine.ExportProcessRetryTask(
@@ -460,16 +444,14 @@ func TestProcessRetryTask_LargeBody_FetchFromDB(
database.DeliveryStatusRetrying,
)
cfg := iHTTPConfig(ts.URL)
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "retry-large",
database.TargetTypeHTTP, cfg, 5,
database.TargetTypeHTTP, ts.URL, 5,
)
task := iTask(
d, event, s.WebhookID, targetID,
"retry-large", cfg, 5, 2, nil,
"retry-large", ts.URL, 5, 2, nil,
)
s.Engine.ExportProcessRetryTask(
@@ -510,7 +492,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
)
task.TargetType = database.TargetTypeLog
s.Engine.Notify([]delivery.Task{task})
s.Engine.ExportNotify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, d.ID)
@@ -570,16 +552,15 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
s.Engine.ExportStart()
bodyStr := event.Body
cfg := iHTTPConfig(ts.URL)
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "retry-chan-test",
database.TargetTypeHTTP, cfg, 5,
database.TargetTypeHTTP, ts.URL, 5,
)
task := iTask(
d, event, s.WebhookID, targetID,
"retry-chan-test", cfg, 5, 2, &bodyStr,
"retry-chan-test", ts.URL, 5, 2, &bodyStr,
)
s.Engine.ExportRetryCh() <- task
@@ -685,7 +666,7 @@ func iCreateTarget(
mainDB *gorm.DB,
targetID, webhookID, name string,
targetType database.TargetType,
config string,
targetURL string,
maxAttempts int,
) {
t.Helper()
@@ -695,7 +676,7 @@ func iCreateTarget(
Name: name,
Type: targetType,
Active: true,
Config: config,
URL: targetURL,
MaxAttempts: maxAttempts,
}
target.ID = targetID
@@ -736,7 +717,7 @@ func TestRecoverWebhookDeliveries_RetryingDeliveries(
iCreateTarget(t, s.MainDB, targetID,
s.WebhookID, "retry-recovery",
database.TargetTypeHTTP,
iHTTPConfig("http://example.com/hook"), 5,
"http://example.com/hook", 5,
)
event := iSeedEvent(
@@ -796,7 +777,7 @@ func iSeedRetryingWithType(
iCreateTarget(t, s.MainDB, targetID,
s.WebhookID, "mutated-target", targetType,
iHTTPConfig("http://example.com/hook"), 5,
"http://example.com/hook", 5,
)
event := iSeedEvent(
@@ -934,14 +915,14 @@ func TestFailUnretryableRetry_WritesNoTargetRow(
targetID := uuid.New().String()
// A Slack incoming-webhook URL: the target config IS the
// A Slack incoming-webhook URL: the target's URL IS the
// credential, which is what makes a leaked target row a
// disclosure rather than a curiosity.
hookURL := "https://hooks.slack.com/services/T00/B00/x"
iCreateTarget(t, s.MainDB, targetID,
s.WebhookID, "credential-bearing",
database.TargetTypeLog, iHTTPConfig(hookURL), 5,
database.TargetTypeLog, hookURL, 5,
)
event := iSeedEvent(
@@ -1186,7 +1167,7 @@ func TestRecoverDeliveries_SkipsAWebhookDeletedAfterTheListIsRead(
assert.False(t, dbMgr.DBExists(webhookID))
}
// --- HTTP Config with custom headers ---
// --- HTTP target with custom headers ---
func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
t.Parallel()
@@ -1208,16 +1189,6 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
)
defer ts.Close()
cfg := delivery.HTTPTargetConfig{
URL: ts.URL,
Headers: map[string]string{
"Authorization": "Bearer secret-token",
},
}
cfgJSON, err := json.Marshal(cfg)
require.NoError(t, err)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID,
`{"auth":"test"}`,
@@ -1232,9 +1203,12 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
bodyStr := event.Body
task := iTask(
d, event, s.WebhookID, targetID,
"auth-target", string(cfgJSON),
"auth-target", ts.URL,
1, 1, &bodyStr,
)
task.TargetHeaders = map[string]string{
headerAuthorization: "Bearer secret-token",
}
s.Engine.ExportProcessNewTask(
context.TODO(), &task,
@@ -1247,11 +1221,14 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
// --- HTTP delivery with custom timeout ---
// TestDeliverHTTP_TargetTimeout proves a task's TimeoutSeconds reaches
// the delivery through the engine. The destination answers after two
// seconds, inside the engine's own five-second client timeout, so only
// the task's one second can fail the delivery.
func TestDeliverHTTP_TargetTimeout(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
e := iEngine(t, 1)
s := newISetup(t)
ts := httptest.NewServer(
http.HandlerFunc(
@@ -1263,31 +1240,30 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) {
)
defer ts.Close()
cfg := delivery.HTTPTargetConfig{
URL: ts.URL,
Timeout: 1,
}
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"timeout":"test"}`,
)
targetID := uuid.New().String()
cfgJSON, err := json.Marshal(cfg)
require.NoError(t, err)
event, del := iSeedEventAndDelivery(
t, db, `{"timeout":"test"}`,
string(cfgJSON),
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
task, d := iHTTPTaskAndDelivery(
event, del, "timeout-target",
string(cfgJSON), 1, 1,
bodyStr := event.Body
task := iTask(
d, event, s.WebhookID, targetID,
"timeout-target", ts.URL, 1, 1, &bodyStr,
)
task.TimeoutSeconds = 1
e.ExportDeliverHTTP(context.TODO(), db, d, task)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
iAssertStatus(t, db, del.ID,
iAssertStatus(t, s.WebhookDB, d.ID,
database.DeliveryStatusFailed,
)
iAssertResultFailed(t, db, del.ID)
iAssertResultFailed(t, s.WebhookDB, d.ID)
}
// TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog pins the size
@@ -1325,8 +1301,6 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
))
defer ts.Close()
cfgJSON := iHTTPConfig(ts.URL)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"cut":"test"}`,
)
@@ -1340,7 +1314,7 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
bodyStr := event.Body
task := iTask(
del, event, s.WebhookID, targetID,
"cut-target", cfgJSON, 1, 1, &bodyStr,
"cut-target", ts.URL, 1, 1, &bodyStr,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -1372,7 +1346,7 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
func iSeedEventAndDelivery(
t *testing.T,
db *gorm.DB,
body, _ string,
body string,
) (database.Event, database.Delivery) {
t.Helper()
@@ -1403,19 +1377,19 @@ func iSeedEventAndDelivery(
func iHTTPTaskAndDelivery(
event database.Event,
del database.Delivery,
name, config string,
name, targetURL string,
maxAttempts, attemptNum int,
) (*delivery.Task, *database.Delivery) {
task := &delivery.Task{
DeliveryID: del.ID,
EventID: event.ID,
WebhookID: event.WebhookID,
TargetID: del.TargetID,
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetConfig: config,
MaxAttempts: maxAttempts,
AttemptNum: attemptNum,
DeliveryID: del.ID,
EventID: event.ID,
WebhookID: event.WebhookID,
TargetID: del.TargetID,
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetURL: targetURL,
MaxAttempts: maxAttempts,
AttemptNum: attemptNum,
}
d := &database.Delivery{
@@ -1426,7 +1400,7 @@ func iHTTPTaskAndDelivery(
Target: database.Target{
Name: name,
Type: database.TargetTypeHTTP,
Config: config,
URL: targetURL,
MaxAttempts: maxAttempts,
},
}
@@ -1455,56 +1429,30 @@ func iAssertResultFailed(
assert.NotEmpty(t, result.Error)
}
// --- HTTP request with invalid config ---
// --- HTTP request with no URL ---
func TestDeliverHTTP_InvalidConfig(t *testing.T) {
// TestDeliverHTTP_NoURL proves an http target without a URL, which
// the forms never store, fails its delivery and records why.
func TestDeliverHTTP_NoURL(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
e := iEngine(t, 1)
event, del := iSeedEventAndDelivery(
t, db, `{"config":"invalid"}`, "",
)
event, del := iSeedEventAndDelivery(t, db, `{"url":"none"}`)
task, d := iHTTPTaskAndDelivery(
event, del, "bad-config", `not-json`, 1, 1,
)
task, d := iHTTPTaskAndDelivery(event, del, "no-url", "", 1, 1)
e.ExportDeliverHTTP(context.TODO(), db, d, task)
iAssertStatus(t, db, del.ID,
database.DeliveryStatusFailed,
)
iAssertResultFailed(t, db, del.ID)
}
// TestDeliverHTTP_InvalidConfigUnrecordedStaysPending: a delivery is
// failed for an invalid config only once the reason is recorded.
// Unrecorded, it stays pending, where the sweep finds it again.
func TestDeliverHTTP_InvalidConfigUnrecordedStaysPending(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
e := testEngine(t, 1)
event, del := iSeedEventAndDelivery(
t, db, `{"config":"invalid"}`, "",
)
task, d := iHTTPTaskAndDelivery(
event, del, "bad-config", `not-json`, 1, 1,
)
require.NoError(t, db.Exec("drop table delivery_results").Error)
e.ExportDeliverHTTP(context.TODO(), db, d, task)
iAssertStatus(t, db, del.ID,
database.DeliveryStatusPending,
)
}
// --- Notify batching ---
// --- notify batching ---
func TestNotify_MultipleTasks(t *testing.T) {
t.Parallel()
@@ -1519,7 +1467,7 @@ func TestNotify_MultipleTasks(t *testing.T) {
}
}
e.Notify(tasks)
e.ExportNotify(tasks)
for i := range 5 {
select {
+8 -9
View File
@@ -29,7 +29,7 @@ const (
// OnStart before the caller may enqueue work. A worker pool
// wrongly rooted in the already-done hook context has nothing
// but ctx.Done() ready in its select, so it is deterministically
// gone by the end of this window. Without the wait, Notify would
// gone by the end of this window. Without the wait, notify would
// race the pool's very first select, in which a ready ctx.Done()
// and a ready deliveryCh are chosen between at random and a
// doomed pool still delivers.
@@ -182,7 +182,7 @@ func TestEngine_WorkersOutliveStartHookContext(t *testing.T) {
// routine's run at start cannot enqueue it.
deliveryID, task := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{task})
s.Engine.ExportNotify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, deliveryID)
}
@@ -202,7 +202,7 @@ func TestEngine_StopHookStopsWorkers(t *testing.T) {
// Let the pool prove it is running before stopping it, so a
// fast OnStop cannot pass by stopping something already dead.
firstID, firstTask := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{firstTask})
s.Engine.ExportNotify([]delivery.Task{firstTask})
iWaitForDelivered(t, s.WebhookDB, firstID)
var stopErr error
@@ -232,7 +232,7 @@ func TestEngine_StopHookStopsWorkers(t *testing.T) {
// With every worker gone, a freshly notified task must sit
// untouched in the queue rather than being delivered.
secondID, secondTask := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{secondTask})
s.Engine.ExportNotify([]delivery.Task{secondTask})
time.Sleep(200 * time.Millisecond)
@@ -309,13 +309,12 @@ func seedHeldDelivery(
))
t.Cleanup(ts.Close)
cfg := iHTTPConfig(ts.URL)
targetID := uuid.New().String()
iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "held",
database.TargetTypeHTTP, cfg, maxAttempts,
database.TargetTypeHTTP, ts.URL, maxAttempts,
)
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"held":true}`)
@@ -324,7 +323,7 @@ func seedHeldDelivery(
bodyStr := event.Body
return iTask(
d, event, s.WebhookID, targetID, "held", cfg,
d, event, s.WebhookID, targetID, "held", ts.URL,
maxAttempts, attemptNum, &bodyStr,
), arrived
}
@@ -384,7 +383,7 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
t, s, 1, database.DeliveryStatusPending, 1,
)
s.Engine.Notify([]delivery.Task{task})
s.Engine.ExportNotify([]delivery.Task{task})
stopDuringAttempt(t, lc, arrived)
iAssertStatus(
@@ -449,7 +448,7 @@ func deliverToArchive(t *testing.T, s iSetup) string {
task.TargetID = tgt.ID
task.TargetType = database.TargetTypeDatabase
s.Engine.Notify([]delivery.Task{task})
s.Engine.ExportNotify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, deliveryID)
+59 -211
View File
@@ -65,20 +65,6 @@ func testEngine(
)
}
func newHTTPTargetConfig(url string) string {
cfg := delivery.HTTPTargetConfig{URL: url}
data, err := json.Marshal(cfg)
if err != nil {
panic(
"failed to marshal HTTPTargetConfig: " +
err.Error(),
)
}
return string(data)
}
func seedEvent(
t *testing.T, db *gorm.DB, body string,
) database.Event {
@@ -141,19 +127,19 @@ type httpDeliveryFixture struct {
func buildHTTPFixture(
dlv database.Delivery,
event database.Event,
targetID, name, config string,
targetID, name, targetURL string,
maxAttempts, attemptNum int,
) httpDeliveryFixture {
task := &delivery.Task{
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: event.WebhookID,
TargetID: targetID,
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetConfig: config,
MaxAttempts: maxAttempts,
AttemptNum: attemptNum,
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: event.WebhookID,
TargetID: targetID,
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetURL: targetURL,
MaxAttempts: maxAttempts,
AttemptNum: attemptNum,
}
d := &database.Delivery{
@@ -164,7 +150,7 @@ func buildHTTPFixture(
Target: database.Target{
Name: name,
Type: database.TargetTypeHTTP,
Config: config,
URL: targetURL,
MaxAttempts: maxAttempts,
},
}
@@ -232,7 +218,7 @@ func TestNotify_NonBlocking(t *testing.T) {
done := make(chan struct{})
go func() {
e.Notify([]delivery.Task{
e.ExportNotify([]delivery.Task{
{DeliveryID: "overflow-1"},
{DeliveryID: "overflow-2"},
})
@@ -243,7 +229,7 @@ func TestNotify_NonBlocking(t *testing.T) {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal(
"Notify blocked when delivery channel was full",
"notify blocked when delivery channel was full",
)
}
}
@@ -275,9 +261,8 @@ func TestDeliverHTTP_Success(t *testing.T) {
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID, "test-http", cfg, 1, 1,
dlv, event, targetID, "test-http", ts.URL, 1, 1,
)
e.ExportDeliverHTTP(
@@ -321,10 +306,9 @@ func TestDeliverHTTP_Failure(t *testing.T) {
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-fail", cfg, 1, 1,
"test-http-fail", ts.URL, 1, 1,
)
e.ExportDeliverHTTP(
@@ -351,7 +335,7 @@ func TestDeliverDatabase_ImmediateSuccess(
// The database target archives for real, so the engine needs
// the target in the main database and a data directory.
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t)
event := seedEvent(t, db, `{"db":"target"}`)
d := seedDatabaseTargetDelivery(t, db, event, tgt)
@@ -452,10 +436,9 @@ func TestDeliverHTTP_WithRetries_Success(t *testing.T) {
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-retry", cfg, 5, 1,
"test-http-retry", ts.URL, 5, 1,
)
e.ExportDeliverHTTP(
@@ -510,13 +493,12 @@ func TestDeliverHTTP_MaxAttemptsExhausted(t *testing.T) {
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
sched := &recordingScheduler{}
for attempt := 1; attempt <= maxAttempts; attempt++ {
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-exhaust", cfg, maxAttempts, attempt,
"test-http-exhaust", ts.URL, maxAttempts, attempt,
)
e.ExportDeliverHTTPWithScheduler(
@@ -574,10 +556,9 @@ func TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker(t *testing.T) {
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-single", cfg, 1, 1,
"test-http-single", ts.URL, 1, 1,
)
sched := &recordingScheduler{}
@@ -624,10 +605,9 @@ func TestDeliverHTTP_SchedulesRetryOnFailure(
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-schedule", cfg, 5, 1,
"test-http-schedule", ts.URL, 5, 1,
)
e.ExportDeliverHTTP(
@@ -789,7 +769,7 @@ func seedConcurrencyTasks(
t *testing.T,
db *gorm.DB,
n int,
targetCfg string,
targetURL string,
) ([]database.Delivery, []delivery.Task) {
t.Helper()
@@ -815,21 +795,21 @@ func seedConcurrencyTasks(
Target: database.Target{
Name: fmt.Sprintf("task-%d", i),
Type: database.TargetTypeHTTP,
Config: targetCfg,
URL: targetURL,
MaxAttempts: 1,
},
}
tasks[i].ID = dlv.ID
dtasks[i] = delivery.Task{
DeliveryID: tasks[i].ID,
EventID: tasks[i].EventID,
TargetID: tasks[i].TargetID,
TargetName: tasks[i].Target.Name,
TargetType: tasks[i].Target.Type,
TargetConfig: tasks[i].Target.Config,
MaxAttempts: 1,
AttemptNum: 1,
DeliveryID: tasks[i].ID,
EventID: tasks[i].EventID,
TargetID: tasks[i].TargetID,
TargetName: tasks[i].Target.Name,
TargetType: tasks[i].Target.Type,
TargetURL: tasks[i].Target.URL,
MaxAttempts: 1,
AttemptNum: 1,
}
}
@@ -865,9 +845,8 @@ func TestWorkerPool_BoundedConcurrency(t *testing.T) {
defer ts.Close()
e := testEngine(t, numWorkers)
cfg := newHTTPTargetConfig(ts.URL)
tasks, dtasks := seedConcurrencyTasks(
t, db, numTasks, cfg,
t, db, numTasks, ts.URL,
)
runConcurrencyWorkers(
@@ -948,12 +927,9 @@ func TestDeliverHTTP_CircuitBreakerBlocks(t *testing.T) {
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(
"http://will-not-be-called.invalid",
)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-cb-block", cfg, 5, 1,
"test-cb-block", "http://will-not-be-called.invalid", 5, 1,
)
e.ExportDeliverHTTP(
@@ -1016,9 +992,8 @@ func TestDeliverHTTP_HalfOpenBreakerDelaysQueuedTasks(t *testing.T) {
require.True(t, cb.Allow(), "the probe delivery should go through")
require.Equal(t, delivery.CircuitHalfOpen, cb.State())
cfg := newHTTPTargetConfig(
"http://will-not-be-called.invalid",
)
const targetURL = "http://will-not-be-called.invalid"
sched := &recordingScheduler{}
const queued, passes = 3, 4
@@ -1040,7 +1015,7 @@ func TestDeliverHTTP_HalfOpenBreakerDelaysQueuedTasks(t *testing.T) {
fix := buildHTTPFixture(
row, event, targetID,
"test-cb-half-open", cfg, 5, 1,
"test-cb-half-open", targetURL, 5, 1,
)
e.ExportDeliverHTTPWithScheduler(
@@ -1155,62 +1130,6 @@ func TestStateAndCooldown_ReadsHTTPAndSlackBreakers(t *testing.T) {
assert.Zero(t, cooldown, "half-open")
}
func TestParseHTTPConfig_Valid(t *testing.T) {
t.Parallel()
e := testEngine(t, 1)
cfg, err := e.ExportParseHTTPConfig(
`{"url":"https://example.com/hook",` +
`"headers":{"X-Token":"secret"}}`,
)
require.NoError(t, err)
assert.Equal(t, "https://example.com/hook", cfg.URL)
assert.Equal(t, "secret", cfg.Headers["X-Token"])
}
func TestParseHTTPConfig_Empty(t *testing.T) {
t.Parallel()
e := testEngine(t, 1)
_, err := e.ExportParseHTTPConfig("")
assert.Error(t, err,
"empty config should return error",
)
}
func TestParseHTTPConfig_MissingURL(t *testing.T) {
t.Parallel()
e := testEngine(t, 1)
_, err := e.ExportParseHTTPConfig(
`{"headers":{"X-Token":"secret"}}`,
)
assert.Error(t, err,
"config without URL should return error",
)
}
func TestParseHTTPConfig_Undecodable(t *testing.T) {
t.Parallel()
e := testEngine(t, 1)
_, err := e.ExportParseHTTPConfig(
`{"url":"https://example.com/hook","timeout":"soon"}`,
)
assert.Error(t, err,
"config that does not decode should return error, "+
"even when the part that did names a URL",
)
}
func TestScheduleRetry_SendsToRetryChannel(
t *testing.T,
) {
@@ -1358,7 +1277,7 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
e := testEngine(t, 1)
cfg := &delivery.HTTPTargetConfig{
target := &database.Target{
URL: ts.URL,
Headers: map[string]string{
"X-Target-Auth": "bearer xyz",
@@ -1373,7 +1292,7 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
}
statusCode, _, _, err := e.ExportDoHTTPRequest(
context.TODO(), cfg, event,
context.TODO(), target, event,
)
require.NoError(t, err)
@@ -1415,7 +1334,7 @@ func TestDoHTTPRequest_CutShortResponseIsAnError(t *testing.T) {
_, body, _, err := e.ExportDoHTTPRequest(
context.TODO(),
&delivery.HTTPTargetConfig{URL: ts.URL},
&database.Target{URL: ts.URL},
&database.Event{},
)
@@ -1467,9 +1386,9 @@ func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
})
require.NoError(t, err)
cfg := &delivery.HTTPTargetConfig{}
var configured map[string]string
if tc.configured != "" {
cfg.Headers = map[string]string{
configured = map[string]string{
headerContentType: tc.configured,
}
}
@@ -1488,7 +1407,7 @@ func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
Headers: string(inbound),
ContentType: tc.event,
},
cfg,
configured,
"webhooker/dev",
)
@@ -1519,7 +1438,7 @@ func TestApplyRequestHeaders_UndecodableInboundForwardsNothing(
&database.Event{
Headers: `{"X-Custom":["value1"],"X-Broken":"not a list"}`,
},
&delivery.HTTPTargetConfig{},
nil,
"webhooker/dev",
)
@@ -1537,7 +1456,7 @@ func TestProcessDelivery_RoutesToCorrectHandler(
// The database target archives for real, so the engine needs
// the target in the main database and a data directory.
env := setupArchiveTest(t)
archive := env.seedDatabaseTarget(t, "")
archive := env.seedDatabaseTarget(t)
tests := []struct {
name string
@@ -1622,51 +1541,6 @@ func TestMaxInlineBodySize_Constant(t *testing.T) {
)
}
func TestParseSlackConfig_Valid(t *testing.T) {
t.Parallel()
e := testEngine(t, 1)
cfg, err := e.ExportParseSlackConfig(
`{"webhookUrl":"https://hooks.slack.com/services/T00/B00/xxx"}`,
)
require.NoError(t, err)
assert.Equal(t,
"https://hooks.slack.com/services/T00/B00/xxx",
cfg.WebhookURL,
)
}
func TestParseSlackConfig_Empty(t *testing.T) {
t.Parallel()
e := testEngine(t, 1)
_, err := e.ExportParseSlackConfig("")
assert.Error(t, err,
"empty config should return error",
)
}
func TestParseSlackConfig_MissingWebhookURL(
t *testing.T,
) {
t.Parallel()
e := testEngine(t, 1)
_, err := e.ExportParseSlackConfig(
`{"other":"field"}`,
)
assert.Error(t, err,
"config without webhook_url should return error",
)
}
func TestFormatSlackMessage_JSONBody(t *testing.T) {
t.Parallel()
@@ -1763,11 +1637,11 @@ func TestFormatSlackMessage_LargeJSONTruncated(
}
// buildSlackDelivery creates a database.Delivery
// with a Slack target config for testing.
// to a Slack target with the given webhook URL for testing.
func buildSlackDelivery(
dlv database.Delivery,
event database.Event,
targetID, name, slackCfg string,
targetID, name, webhookURL string,
) *database.Delivery {
d := &database.Delivery{
EventID: event.ID,
@@ -1777,7 +1651,7 @@ func buildSlackDelivery(
Target: database.Target{
Name: name,
Type: database.TargetTypeSlack,
Config: slackCfg,
URL: webhookURL,
MaxAttempts: 1,
},
}
@@ -1815,11 +1689,10 @@ func TestDeliverSlack_Success(t *testing.T) {
)
defer ts.Close()
e, targetID, slackCfg, event, dlv :=
setupSlackTest(t, db, ts.URL)
e, targetID, event, dlv := setupSlackTest(t, db)
d := buildSlackDelivery(
dlv, event, targetID, "test-slack", slackCfg,
dlv, event, targetID, "test-slack", ts.URL,
)
e.ExportDeliverSlack(context.TODO(), db, d)
@@ -1838,9 +1711,8 @@ func TestDeliverSlack_Success(t *testing.T) {
func setupSlackTest(
t *testing.T,
db *gorm.DB,
serverURL string,
) (
*delivery.Engine, string, string,
*delivery.Engine, string,
database.Event, database.Delivery,
) {
t.Helper()
@@ -1848,13 +1720,6 @@ func setupSlackTest(
e := testEngine(t, 1)
targetID := uuid.New().String()
cfgBytes, err := json.Marshal(
delivery.SlackTargetConfig{
WebhookURL: serverURL,
},
)
require.NoError(t, err)
event := seedEvent(
t, db, `{"action":"test","data":"value"}`,
)
@@ -1864,7 +1729,7 @@ func setupSlackTest(
database.DeliveryStatusPending,
)
return e, targetID, string(cfgBytes), event, dlv
return e, targetID, event, dlv
}
func assertSlackPayload(
@@ -1913,13 +1778,6 @@ func TestDeliverSlack_Failure(t *testing.T) {
e := testEngine(t, 1)
targetID := uuid.New().String()
slackCfg, err := json.Marshal(
delivery.SlackTargetConfig{
WebhookURL: ts.URL,
},
)
require.NoError(t, err)
event := seedEvent(t, db, `{"test":true}`)
dlv := seedDelivery(
@@ -1929,7 +1787,7 @@ func TestDeliverSlack_Failure(t *testing.T) {
d := buildSlackDelivery(
dlv, event, targetID,
"test-slack-fail", string(slackCfg),
"test-slack-fail", ts.URL,
)
e.ExportDeliverSlack(context.TODO(), db, d)
@@ -1943,7 +1801,10 @@ func TestDeliverSlack_Failure(t *testing.T) {
)
}
func TestDeliverSlack_InvalidConfig(t *testing.T) {
// TestDeliverSlack_NoURL proves a Slack target without a URL, which
// the forms never store, fails its delivery rather than reporting it
// delivered.
func TestDeliverSlack_NoURL(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
@@ -1964,7 +1825,6 @@ func TestDeliverSlack_InvalidConfig(t *testing.T) {
Target: database.Target{
Name: "test-slack-bad",
Type: database.TargetTypeSlack,
Config: `{"not_webhook_url":"missing"}`,
MaxAttempts: 1,
},
}
@@ -2002,13 +1862,6 @@ func TestProcessDelivery_RoutesToSlack(t *testing.T) {
e := testEngine(t, 1)
slackCfg, err := json.Marshal(
delivery.SlackTargetConfig{
WebhookURL: ts.URL,
},
)
require.NoError(t, err)
event := seedEvent(t, db, `{"route":"slack"}`)
dlv := seedDelivery(
@@ -2018,7 +1871,7 @@ func TestProcessDelivery_RoutesToSlack(t *testing.T) {
d := buildSlackDelivery(
dlv, event, dlv.TargetID,
"test-slack-route", string(slackCfg),
"test-slack-route", ts.URL,
)
task := &delivery.Task{
@@ -2133,7 +1986,7 @@ func TestDeliverLog_LogsFullContent(t *testing.T) {
func buildSlackRetryDelivery(
dlv database.Delivery,
event database.Event,
targetID, cfg string,
targetID, webhookURL string,
) *database.Delivery {
d := &database.Delivery{
EventID: event.ID,
@@ -2143,7 +1996,7 @@ func buildSlackRetryDelivery(
Target: database.Target{
Name: "test-slack-retry",
Type: database.TargetTypeSlack,
Config: cfg,
URL: webhookURL,
MaxAttempts: 5,
},
}
@@ -2162,11 +2015,6 @@ func TestDeliverSlack_WithRetries_SchedulesRetry(
e := testEngine(t, 1)
targetID := uuid.New().String()
slackCfg, err := json.Marshal(
delivery.SlackTargetConfig{WebhookURL: ts.URL},
)
require.NoError(t, err)
event := seedEvent(t, db, `{"slack":"retry"}`)
dlv := seedDelivery(
@@ -2175,7 +2023,7 @@ func TestDeliverSlack_WithRetries_SchedulesRetry(
)
d := buildSlackRetryDelivery(
dlv, event, targetID, string(slackCfg),
dlv, event, targetID, ts.URL,
)
task := &delivery.Task{
+4 -6
View File
@@ -69,7 +69,6 @@ func TestEventDBHoldsNoTargetRows(t *testing.T) {
))
defer ts.Close()
cfg := iHTTPConfig(ts.URL)
targetID := uuid.New().String()
dbPath := s.DBMgr.DBPath(s.WebhookID)
@@ -85,7 +84,7 @@ func TestEventDBHoldsNoTargetRows(t *testing.T) {
)
task := iTask(
d, event, s.WebhookID, targetID,
"leaky-target", cfg, 5, 1, &body,
"leaky-target", ts.URL, 5, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -102,7 +101,7 @@ func TestEventDBHoldsNoTargetRows(t *testing.T) {
// https://git.eeqj.de/sneak/webhooker/issues/107.
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "leaky-target",
database.TargetTypeHTTP, cfg, 5,
database.TargetTypeHTTP, ts.URL, 5,
)
rd := iSeedDelivery(
@@ -111,7 +110,7 @@ func TestEventDBHoldsNoTargetRows(t *testing.T) {
)
rTask := iTask(
rd, event, s.WebhookID, targetID,
"leaky-target", cfg, 5, 2, &body,
"leaky-target", ts.URL, 5, 2, &body,
)
s.Engine.ExportProcessRetryTask(context.TODO(), &rTask)
@@ -138,7 +137,6 @@ func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) {
))
defer ts.Close()
cfg := iHTTPConfig(ts.URL)
targetID := uuid.New().String()
event := iSeedEvent(
@@ -152,7 +150,7 @@ func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) {
)
task := iTask(
d, event, s.WebhookID, targetID,
"leaky-target", cfg, 1, 1, &body,
"leaky-target", ts.URL, 1, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
+6 -20
View File
@@ -93,17 +93,6 @@ func (s *tsSink) text(t *testing.T) string {
}
}
func tsSlackConfig(t *testing.T, url string) string {
t.Helper()
data, err := json.Marshal(
delivery.SlackTargetConfig{WebhookURL: url},
)
require.NoError(t, err)
return string(data)
}
// tsSeedEvent writes an event whose CreatedAt is tsEventCreatedAt
// rather than the write time, so an assertion on the rendered
// timestamp cannot pass by accident against "roughly now".
@@ -142,7 +131,7 @@ func tsSeedEvent(
// tsSeedTarget writes the slack target row into the main database.
// The retry path confirms the target still exists before sending.
func tsSeedTarget(
t *testing.T, mainDB *gorm.DB, webhookID, config string,
t *testing.T, mainDB *gorm.DB, webhookID, webhookURL string,
) database.Target {
t.Helper()
@@ -150,7 +139,7 @@ func tsSeedTarget(
WebhookID: webhookID,
Name: "slack-sink",
Type: database.TargetTypeSlack,
Config: config,
URL: webhookURL,
Active: true,
MaxAttempts: 1,
}
@@ -176,7 +165,7 @@ func tsTask(
TargetID: target.ID,
TargetName: target.Name,
TargetType: database.TargetTypeSlack,
TargetConfig: target.Config,
TargetURL: target.URL,
MaxAttempts: 1,
Method: event.Method,
Headers: event.Headers,
@@ -226,8 +215,7 @@ func (c tsCase) run(t *testing.T) (iSetup, database.Delivery, string) {
s := newISetup(t)
sink := newTSSink(t)
cfg := tsSlackConfig(t, sink.URL)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, sink.URL)
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
d := iSeedDelivery(
@@ -338,8 +326,7 @@ func TestFormatSlackMessageOverTaskReconstructedEvent(
s := newISetup(t)
cfg := tsSlackConfig(t, tsUndeliverableHook)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, tsUndeliverableHook)
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
d := iSeedDelivery(
@@ -410,8 +397,7 @@ func TestEventReconstructionSurvivesAReapedRow(t *testing.T) {
s := newISetup(t)
cfg := tsSlackConfig(t, tsUndeliverableHook)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, tsUndeliverableHook)
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
d := iSeedDelivery(
+11 -20
View File
@@ -77,10 +77,10 @@ func ExportIsForwardableHeader(name string) bool {
func ExportApplyRequestHeaders(
req *http.Request,
event *database.Event,
cfg *HTTPTargetConfig,
configured map[string]string,
userAgent string,
) []string {
return applyRequestHeaders(req, event, cfg, userAgent)
return applyRequestHeaders(req, event, configured, userAgent)
}
// ExportTruncate exposes truncate for testing.
@@ -158,6 +158,11 @@ func (e *Engine) ExportUpdateDeliveryStatus(
return e.updateDeliveryStatus(webhookDB, d, "", status)
}
// ExportNotify exposes notify.
func (e *Engine) ExportNotify(tasks []Task) {
e.notify(tasks)
}
// ExportProcessNewTask exposes processNewTask.
func (e *Engine) ExportProcessNewTask(
ctx context.Context, task *Task,
@@ -216,37 +221,23 @@ func (e *Engine) ExportSetSlackCircuitBreaker(
e.slackTarget.circuitBreakers.Store(targetID, cb)
}
// ExportParseHTTPConfig exposes parseHTTPConfig.
func (e *Engine) ExportParseHTTPConfig(
configJSON string,
) (*HTTPTargetConfig, error) {
return parseHTTPConfig(configJSON)
}
// ExportParseSlackConfig exposes parseSlackConfig.
func (e *Engine) ExportParseSlackConfig(
configJSON string,
) (*SlackTargetConfig, error) {
return parseSlackConfig(configJSON)
}
// ExportDoHTTPRequest exposes the http target's
// doHTTPRequest.
func (e *Engine) ExportDoHTTPRequest(
ctx context.Context,
cfg *HTTPTargetConfig,
target *database.Target,
event *database.Event,
) (int, string, int64, error) {
return e.httpTarget.doHTTPRequest(ctx, cfg, event)
return e.httpTarget.doHTTPRequest(ctx, target, event)
}
// ExportClientForRequest exposes the http target's
// clientForRequest.
func (e *Engine) ExportClientForRequest(
cfg *HTTPTargetConfig,
timeoutSeconds int,
originScoped []string,
) *http.Client {
return e.httpTarget.clientForRequest(cfg, originScoped)
return e.httpTarget.clientForRequest(timeoutSeconds, originScoped)
}
// ErrExportTooManyRedirects exposes the sentinel the redirect
+1 -1
View File
@@ -61,7 +61,7 @@ func (s *inflightSet) retain(deliveryID string) {
// retainIdle takes the first reference on a delivery, and reports
// whether it got it. It fails when the engine already owns the
// delivery, which is what makes two claimants — Notify and the
// delivery, which is what makes two claimants — notify and the
// recovery routine, say — mutually exclusive rather than merely
// atomic.
func (s *inflightSet) retainIdle(deliveryID string) bool {
+9 -9
View File
@@ -84,9 +84,9 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
)
rAgePending(t, s.WebhookDB, d.ID)
// Queued exactly as the receiver queues it, and never dequeued:
// Queued exactly as StoreEvent queues it, and never dequeued:
// no workers are running in this engine.
s.Engine.Notify([]delivery.Task{{
s.Engine.ExportNotify([]delivery.Task{{
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: s.WebhookID,
@@ -109,7 +109,7 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
// TestRecoveryAndSweepDoNotDoubleDispatch drives the recovery routine's
// run at start and a tick of the sweep at once against one aged pending
// row. The engine runs them one after the other, but Notify can race
// row. The engine runs them one after the other, but notify can race
// either, and ownership is what keeps any two claimants from both
// sending. Before ownership the two runs both dispatched the row.
func TestRecoveryAndSweepDoNotDoubleDispatch(t *testing.T) {
@@ -224,7 +224,7 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
body := `{"released":true}`
s.Engine.Notify([]delivery.Task{{
s.Engine.ExportNotify([]delivery.Task{{
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: s.WebhookID,
@@ -248,10 +248,10 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
}
// TestNotifyAfterRecoveryDoesNotSendAgain is the startup race of
// https://git.eeqj.de/sneak/webhooker/issues/299. The receiver has
// https://git.eeqj.de/sneak/webhooker/issues/299. StoreEvent has
// written a delivery, the recovery routine's run at start finds it
// pending, sends it and releases it, and only then does the receiver's
// Notify for it arrive. Nothing owns the delivery by then, so Notify
// pending, sends it and releases it, and only then does StoreEvent's
// notify for it arrive. Nothing owns the delivery by then, so notify
// takes it.
func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
t.Parallel()
@@ -289,7 +289,7 @@ func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
body := event.Body
s.Engine.Notify([]delivery.Task{{
s.Engine.ExportNotify([]delivery.Task{{
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: s.WebhookID,
@@ -300,7 +300,7 @@ func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
EntrypointID: event.EntrypointID,
}})
// Notify took the delivery, and a worker releases it once it has
// notify took the delivery, and a worker releases it once it has
// run the task.
require.Eventually(
t,
+12 -15
View File
@@ -192,7 +192,7 @@ func mDeliverOK(t *testing.T, s iSetup) {
body := event.Body
task := iTask(
d, event, s.WebhookID, targetID,
"metrics-ok", iHTTPConfig(ts.URL), 3, 1, &body,
"metrics-ok", ts.URL, 3, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -226,18 +226,17 @@ func mExhaustRetries(t *testing.T, s iSetup) {
)
body := event.Body
cfg := iHTTPConfig(ts.URL)
// The retry below is only run if its target still exists; see
// https://git.eeqj.de/sneak/webhooker/issues/107.
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "metrics-fail",
database.TargetTypeHTTP, cfg, 2,
database.TargetTypeHTTP, ts.URL, 2,
)
first := iTask(
d, event, s.WebhookID, targetID,
"metrics-fail", cfg, 2, 1, &body,
"metrics-fail", ts.URL, 2, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &first)
@@ -251,7 +250,7 @@ func mExhaustRetries(t *testing.T, s iSetup) {
// deterministic and off the wall clock.
second := iTask(
d, event, s.WebhookID, targetID,
"metrics-fail", cfg, 2, 2, &body,
"metrics-fail", ts.URL, 2, 2, &body,
)
s.Engine.ExportProcessRetryTask(
@@ -289,7 +288,6 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
)
body := event.Body
cfg := iHTTPConfig(ts.URL)
// An attempt budget above the failure threshold, so the breaker
// rather than the budget is what stops the delivery.
@@ -299,12 +297,12 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
// see https://git.eeqj.de/sneak/webhooker/issues/107.
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "metrics-trip",
database.TargetTypeHTTP, cfg, maxAttempts,
database.TargetTypeHTTP, ts.URL, maxAttempts,
)
first := iTask(
d, event, s.WebhookID, targetID,
"metrics-trip", cfg, maxAttempts, 1, &body,
"metrics-trip", ts.URL, maxAttempts, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &first)
@@ -316,7 +314,7 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
ExportDefaultFailureThreshold; attempt++ {
task := iTask(
d, event, s.WebhookID, targetID,
"metrics-trip", cfg, maxAttempts, attempt, &body,
"metrics-trip", ts.URL, maxAttempts, attempt, &body,
)
s.Engine.ExportProcessRetryTask(
@@ -363,17 +361,16 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
)
body := event.Body
cfg := iHTTPConfig(ts.URL)
maxAttempts := delivery.ExportDefaultFailureThreshold + 5
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "metrics-blocked",
database.TargetTypeHTTP, cfg, maxAttempts,
database.TargetTypeHTTP, ts.URL, maxAttempts,
)
first := iTask(
d, event, s.WebhookID, targetID,
"metrics-blocked", cfg, maxAttempts, 1, &body,
"metrics-blocked", ts.URL, maxAttempts, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &first)
@@ -382,7 +379,7 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
ExportDefaultFailureThreshold; attempt++ {
task := iTask(
d, event, s.WebhookID, targetID,
"metrics-blocked", cfg, maxAttempts, attempt, &body,
"metrics-blocked", ts.URL, maxAttempts, attempt, &body,
)
s.Engine.ExportProcessRetryTask(context.TODO(), &task)
@@ -405,7 +402,7 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
blocked := iTask(
d, event, s.WebhookID, targetID,
"metrics-blocked", cfg, maxAttempts,
"metrics-blocked", ts.URL, maxAttempts,
delivery.ExportDefaultFailureThreshold+1, &body,
)
@@ -474,7 +471,7 @@ func TestDeliveryMetrics_QueueDepthGauges(t *testing.T) {
iCreateTarget(t, s.MainDB, targetID, s.WebhookID,
"queue-depth-target", database.TargetTypeHTTP,
iHTTPConfig("https://example.com/hook"), 3,
"https://example.com/hook", 3,
)
event := iSeedEvent(
@@ -102,7 +102,7 @@ func TestQueueDepthSample_LogsNoBoundValue(t *testing.T) {
iCreateTarget(t, mainDB.DB(), targetID, webhookID,
"queue-depth-gormlog-target", database.TargetTypeHTTP,
iHTTPConfig("https://example.com/hook"), 3,
"https://example.com/hook", 3,
)
event := iSeedEvent(
@@ -357,7 +357,7 @@ func TestFailedResultWriteLeavesDeliveryRecoverable(
Target: database.Target{
Name: "unwritable",
Type: database.TargetTypeHTTP,
Config: iHTTPConfig(ts.URL),
URL: ts.URL,
MaxAttempts: 1,
},
}
@@ -448,7 +448,7 @@ func TestFailedResultWriteWithRetriesLeavesDeliveryRecoverable(
Target: database.Target{
Name: "unwritable",
Type: database.TargetTypeHTTP,
Config: iHTTPConfig(ts.URL),
URL: ts.URL,
MaxAttempts: 3,
},
}
+50 -1
View File
@@ -56,7 +56,7 @@ func recoverySetup(t *testing.T) (iSetup, string) {
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "recovery",
database.TargetTypeHTTP,
iHTTPConfig("http://example.com/hook"), 20,
"http://example.com/hook", 20,
)
return s, targetID
@@ -205,6 +205,55 @@ func TestRecovery_PendingOlderThanTheAgeBound(t *testing.T) {
}
}
// TestRecovery_TaskCarriesEveryTargetSetting: the task the recovery
// routine queues carries every one of the target's settings, all six
// set here.
// They are copied into the task one field at a time, and a field left
// out is sent empty without any error.
func TestRecovery_TaskCarriesEveryTargetSetting(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s := newISetup(t)
iCreateWebhook(t, s.MainDB, s.WebhookID, "recovery")
target := database.Target{
WebhookID: s.WebhookID,
Name: "recovery",
Type: database.TargetTypeHTTP,
Active: true,
URL: "http://example.com/hook",
Headers: map[string]string{headerAuthorization: "Bearer t"},
TimeoutSeconds: 7,
ForwardQuery: true,
ArchiveExpiry: time.Hour,
ArchiveRotation: "daily",
MaxAttempts: 1,
}
require.NoError(t, s.MainDB.Create(&target).Error)
id := seedRecoveryDelivery(
t, s, target.ID, database.DeliveryStatusPending, 0, false,
)
rAgePending(t, s.WebhookDB, id)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
task := requireQueued(t, s.Engine.ExportDeliveryCh())
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.URL, task.TargetURL)
assert.Equal(t, target.Headers, task.TargetHeaders)
assert.Equal(t, target.TimeoutSeconds, task.TimeoutSeconds)
assert.Equal(t, target.ForwardQuery, task.ForwardQuery)
assert.Equal(t, target.ArchiveExpiry, task.ArchiveExpiry)
assert.Equal(t, target.ArchiveRotation, task.ArchiveRotation)
})
}
}
// TestRecovery_PendingYoungerThanTheAgeBound: a pending delivery touched
// a moment ago, as one a stop cut off is, is sent at start, which is how
// it reaches its target on the next start
+9 -17
View File
@@ -85,19 +85,14 @@ func deliverWithProbeHeaders(
database.DeliveryStatusPending,
)
cfg, err := json.Marshal(delivery.HTTPTargetConfig{
URL: targetURL,
Headers: map[string]string{
probeHeaderName: probeHeaderValue,
},
})
require.NoError(t, err)
body := event.Body
task := iTask(
d, event, s.WebhookID, targetID,
"redirect-target", string(cfg), 1, 1, &body,
"redirect-target", targetURL, 1, 1, &body,
)
task.TargetHeaders = map[string]string{
probeHeaderName: probeHeaderValue,
}
s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -309,8 +304,7 @@ func TestRedirectPolicy_StopsAtHopCap(t *testing.T) {
)
client := engine.ExportClientForRequest(
&delivery.HTTPTargetConfig{URL: srv.URL},
[]string{probeHeaderName},
0, []string{probeHeaderName},
)
require.NotNil(t, client.CheckRedirect)
@@ -350,7 +344,7 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
inbound, err := json.Marshal(map[string][]string{
inboundHeaderName: {inboundHeaderValue},
"Content-Type": {testContentType},
"User-Agent": {"curl/8.7.1"},
headerUserAgent: {"curl/8.7.1"},
"Host": {"inbound.example.com"},
})
require.NoError(t, err)
@@ -369,11 +363,9 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
Headers: string(inbound),
ContentType: testContentType,
},
&delivery.HTTPTargetConfig{
Headers: map[string]string{
probeHeaderName: probeHeaderValue,
"Content-Type": testContentType,
},
map[string]string{
probeHeaderName: probeHeaderValue,
"Content-Type": testContentType,
},
"webhooker/dev",
)
+164
View File
@@ -0,0 +1,164 @@
package delivery
import (
"fmt"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
)
// StoreEvent stores event with one pending delivery for each of its
// webhook's active targets, and adds them to the totals, in one
// transaction, then queues the deliveries. It returns how many
// deliveries it created.
//
// The receiver stores every event it accepts through here, and a
// resubmit stores its copy of a stored event, so every event is
// stored, retried, SSRF-guarded and circuit-broken the same way however
// it entered the system. CreateReplay is the only other place a
// delivery is created.
//
// The targets are the webhook's active ones as they stand now: a
// target created after a resubmitted event first arrived is included,
// and an inactive one is skipped. An event with no active target is
// stored with no deliveries.
func (e *Engine) StoreEvent(event *database.Event) (int, error) {
var targets []database.Target
err := e.database.DB().Where(
"webhook_id = ? AND active = ?", event.WebhookID, true,
).Find(&targets).Error
if err != nil {
return 0, fmt.Errorf("loading active targets: %w", err)
}
webhookDB, err := e.dbManager.GetDB(event.WebhookID)
if err != nil {
return 0, fmt.Errorf("getting webhook database: %w", err)
}
event.BodyBytes = int64(len(event.Body))
tasks := make([]Task, 0, len(targets))
err = webhookDB.Transaction(func(tx *gorm.DB) error {
err := tx.Create(event).Error
if err != nil {
return fmt.Errorf("creating event: %w", err)
}
for i := range targets {
dlv, err := createDelivery(tx, event.ID, targets[i].ID, false)
if err != nil {
return err
}
tasks = append(tasks, buildTask(
dlv, event.WebhookID, event, &targets[i], 1,
))
}
err = database.AddEventTotals(tx, database.EventTotals{
Events: 1, LastEventAt: &event.CreatedAt,
})
if err != nil {
return err
}
// A resubmitted copy did not arrive on its entrypoint's URL, so
// it leaves the entrypoint's last event as it is.
if event.ResubmittedFromID != nil {
return nil
}
return database.AddEntrypointTotals(tx, database.EntrypointTotals{
EntrypointID: event.EntrypointID, LastEventAt: event.CreatedAt,
})
})
if err != nil {
return 0, err
}
// Counted here, after the commit: an event exists once it is
// durably stored, which is what the delivery counters are compared
// against on a dashboard. A resubmitted event counts too, because
// it produces deliveries that the delivery side counts; the
// resubmit counter is what separates the two.
e.mtr.EventReceived()
e.notify(tasks)
return len(tasks), nil
}
// CreateReplay creates one more pending delivery of the stored event
// eventID to target, marked as a replay, adds it to the target's totals
// in the same transaction, and queues it. It returns the new
// delivery's id.
//
// The caller passes the target as it stands now and has already
// decided the replay may go ahead. The delivery being replayed is not
// touched.
func (e *Engine) CreateReplay(
webhookID, eventID string, target *database.Target,
) (string, error) {
webhookDB, err := e.dbManager.GetDB(webhookID)
if err != nil {
return "", fmt.Errorf("getting webhook database: %w", err)
}
event, err := e.loadEvent(webhookDB, eventID)
if err != nil {
return "", err
}
var dlv *database.Delivery
err = webhookDB.Transaction(func(tx *gorm.DB) error {
var err error
dlv, err = createDelivery(tx, event.ID, target.ID, true)
return err
})
if err != nil {
return "", err
}
e.notify([]Task{buildTask(dlv, webhookID, &event, target, 1)})
return dlv.ID, nil
}
// createDelivery writes a pending delivery of the event to the target
// on the caller's transaction, and adds it to the target's totals.
//
// Neither Event nor Target is set on the row, and the per-webhook
// database omits associations on every write in any case (see
// internal/database/event_db_isolation.go), so the target's settings
// are never copied into the event database.
func createDelivery(
tx *gorm.DB, eventID, targetID string, replay bool,
) (*database.Delivery, error) {
dlv := &database.Delivery{
EventID: eventID,
TargetID: targetID,
Status: database.DeliveryStatusPending,
Replay: replay,
}
err := tx.Create(dlv).Error
if err != nil {
return nil, fmt.Errorf(
"creating delivery for target %s: %w", targetID, err,
)
}
err = database.AddTargetTotals(tx, database.TargetTotals{
TargetID: targetID, Deliveries: 1,
})
if err != nil {
return nil, err
}
return dlv, nil
}
+386
View File
@@ -0,0 +1,386 @@
package delivery_test
import (
"net/http"
"strings"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// storeTestTarget creates an active http target of the setup's webhook
// with every one of a target's settings set, so a task that drops one
// of them differs from the target.
func storeTestTarget(
t *testing.T, s iSetup, name string,
) database.Target {
t.Helper()
target := database.Target{
WebhookID: s.WebhookID,
Name: name,
Type: database.TargetTypeHTTP,
Active: true,
URL: "http://93.184.216.34/" + name,
Headers: map[string]string{"X-Api-Key": "k"},
TimeoutSeconds: 7,
ForwardQuery: true,
ArchiveExpiry: time.Hour,
ArchiveRotation: rotationDaily,
MaxAttempts: 3,
}
require.NoError(t, s.MainDB.Create(&target).Error)
return target
}
// newStoreTestEvent returns an event of the setup's webhook as the
// receiver builds one, not yet stored.
func newStoreTestEvent(s iSetup, body string) *database.Event {
return &database.Event{
WebhookID: s.WebhookID,
EntrypointID: uuid.New().String(),
Method: http.MethodPost,
RawQuery: "a=1&b=2",
Headers: `{"X-Test":["yes"]}`,
ContentType: testContentType,
Body: body,
}
}
// queuedTasks takes every task the engine has queued so far. The
// engine in these tests is never started, so nothing else takes them.
func queuedTasks(e *delivery.Engine) []delivery.Task {
var tasks []delivery.Task
for {
select {
case task := <-e.ExportDeliveryCh():
tasks = append(tasks, task)
default:
return tasks
}
}
}
// assertTaskFor proves task carries delivery deliveryID of event to
// target: every one of the target's settings and every event field.
// They are copied into the task one field at a time, and a field left
// out is sent empty without any error.
func assertTaskFor(
t *testing.T,
task delivery.Task,
deliveryID string,
event *database.Event,
target *database.Target,
) {
t.Helper()
assert.Equal(t, deliveryID, task.DeliveryID)
assert.Equal(t, event.ID, task.EventID)
assert.Equal(t, event.WebhookID, task.WebhookID)
assert.Equal(t, event.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Name, task.TargetName)
assert.Equal(t, target.Type, task.TargetType)
assert.Equal(t, target.URL, task.TargetURL)
assert.Equal(t, target.Headers, task.TargetHeaders)
assert.Equal(t, target.TimeoutSeconds, task.TimeoutSeconds)
assert.Equal(t, target.ForwardQuery, task.ForwardQuery)
assert.Equal(t, target.ArchiveExpiry, task.ArchiveExpiry)
assert.Equal(t, target.ArchiveRotation, task.ArchiveRotation)
assert.Equal(t, target.MaxAttempts, task.MaxAttempts)
assert.Equal(t, event.Method, task.Method)
assert.Equal(t, event.RawQuery, task.RawQuery)
assert.Equal(t, event.Headers, task.Headers)
assert.Equal(t, event.ContentType, task.ContentType)
require.NotNil(t, task.Body)
assert.Equal(t, event.Body, *task.Body)
assert.Equal(t, 1, task.AttemptNum)
}
// storedDeliveries reads every delivery of an event.
func storedDeliveries(
t *testing.T, db *gorm.DB, eventID string,
) []database.Delivery {
t.Helper()
var deliveries []database.Delivery
require.NoError(t, db.Where("event_id = ?", eventID).
Find(&deliveries).Error)
return deliveries
}
// storedEventTotals reads the webhook's event totals.
func storedEventTotals(t *testing.T, db *gorm.DB) database.EventTotals {
t.Helper()
var totals database.EventTotals
require.NoError(t, db.First(&totals).Error)
return totals
}
// countRows counts the rows of model in db.
func countRows(t *testing.T, db *gorm.DB, model any) int64 {
t.Helper()
var n int64
require.NoError(t, db.Model(model).Count(&n).Error)
return n
}
// TestStoreEvent_StoresDeliveriesToActiveTargets proves StoreEvent
// stores the event with one pending delivery to each active target and
// none to an inactive one, adds them to the totals, and queues one task
// per delivery.
func TestStoreEvent_StoresDeliveriesToActiveTargets(t *testing.T) {
t.Parallel()
s := newISetup(t)
targets := []database.Target{
storeTestTarget(t, s, "first"),
storeTestTarget(t, s, "second"),
}
off := storeTestTarget(t, s, "off")
require.NoError(t, s.MainDB.Model(&database.Target{}).
Where("id = ?", off.ID).Update("active", false).Error)
event := newStoreTestEvent(s, `{"hello":"world"}`)
created, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
assert.Equal(t, 2, created)
var stored database.Event
require.NoError(t, s.WebhookDB.First(&stored, "id = ?", event.ID).Error)
assert.Equal(t, event.Body, stored.Body)
assert.Equal(t, int64(len(event.Body)), stored.BodyBytes)
rows := storedDeliveries(t, s.WebhookDB, event.ID)
queued := queuedTasks(s.Engine)
require.Len(t, rows, 2)
require.Len(t, queued, 2)
deliveries := map[string]database.Delivery{}
for _, dlv := range rows {
deliveries[dlv.TargetID] = dlv
}
tasks := map[string]delivery.Task{}
for _, task := range queued {
tasks[task.TargetID] = task
}
for i := range targets {
dlv, ok := deliveries[targets[i].ID]
require.True(t, ok, "no delivery to %s", targets[i].Name)
assert.Equal(t, database.DeliveryStatusPending, dlv.Status)
assert.False(t, dlv.Replay)
assert.Equal(t, int64(1),
targetTotals(t, s.WebhookDB, targets[i].ID).Deliveries)
assertTaskFor(t, tasks[targets[i].ID], dlv.ID, event, &targets[i])
}
totals := storedEventTotals(t, s.WebhookDB)
assert.Equal(t, int64(1), totals.Events)
require.NotNil(t, totals.LastEventAt)
assert.True(t, totals.LastEventAt.Equal(stored.CreatedAt))
var entrypoint database.EntrypointTotals
require.NoError(t, s.WebhookDB.First(
&entrypoint, "entrypoint_id = ?", event.EntrypointID,
).Error)
assert.True(t, entrypoint.LastEventAt.Equal(stored.CreatedAt))
}
// TestStoreEvent_ResubmittedCopyLeavesTheEntrypoint proves a
// resubmitted copy is counted among the webhook's events but leaves its
// entrypoint's last event as it is: it did not arrive on that URL.
func TestStoreEvent_ResubmittedCopyLeavesTheEntrypoint(t *testing.T) {
t.Parallel()
s := newISetup(t)
storeTestTarget(t, s, "only")
sourceID := uuid.New().String()
event := newStoreTestEvent(s, `{"copy":true}`)
event.ResubmittedFromID = &sourceID
created, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
assert.Equal(t, 1, created)
assert.Len(t, queuedTasks(s.Engine), 1)
assert.Equal(t, int64(1), storedEventTotals(t, s.WebhookDB).Events)
assert.Zero(t, countRows(t, s.WebhookDB, &database.EntrypointTotals{}))
}
// TestStoreEvent_NoActiveTarget proves an event of a webhook with no
// active target is stored with no delivery and nothing queued.
func TestStoreEvent_NoActiveTarget(t *testing.T) {
t.Parallel()
s := newISetup(t)
event := newStoreTestEvent(s, `{"to":"nobody"}`)
created, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
assert.Zero(t, created)
assert.Equal(t, int64(1), countRows(t, s.WebhookDB, &database.Event{}))
assert.Empty(t, storedDeliveries(t, s.WebhookDB, event.ID))
assert.Empty(t, queuedTasks(s.Engine))
}
// TestStoreEvent_LargeBodyIsNotCarried proves a body at the inline
// limit is stored whole but left off the task, for the attempt to read
// from the event row.
func TestStoreEvent_LargeBodyIsNotCarried(t *testing.T) {
t.Parallel()
s := newISetup(t)
storeTestTarget(t, s, "only")
event := newStoreTestEvent(
s, strings.Repeat("x", delivery.MaxInlineBodySize),
)
_, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
tasks := queuedTasks(s.Engine)
require.Len(t, tasks, 1)
assert.Nil(t, tasks[0].Body)
var stored database.Event
require.NoError(t, s.WebhookDB.First(&stored, "id = ?", event.ID).Error)
assert.Equal(t, event.Body, stored.Body)
}
// TestStoreEvent_RollsBackWhenAWriteFails proves the event, its
// deliveries and its totals are one transaction: when the entrypoint
// totals write fails, the last one, after the event, its delivery, the
// target totals and the event totals were written, none of them is left
// and nothing is queued.
func TestStoreEvent_RollsBackWhenAWriteFails(t *testing.T) {
t.Parallel()
s := newISetup(t)
storeTestTarget(t, s, "only")
require.NoError(t,
s.WebhookDB.Migrator().DropTable(&database.EntrypointTotals{}))
_, err := s.Engine.StoreEvent(newStoreTestEvent(s, `{"lost":true}`))
require.Error(t, err)
assert.Zero(t, countRows(t, s.WebhookDB, &database.Event{}))
assert.Zero(t, countRows(t, s.WebhookDB, &database.Delivery{}))
assert.Zero(t, countRows(t, s.WebhookDB, &database.TargetTotals{}))
totals := storedEventTotals(t, s.WebhookDB)
assert.Zero(t, totals.Events)
assert.Nil(t, totals.LastEventAt)
assert.Empty(t, queuedTasks(s.Engine))
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// TestCreateReplay_CreatesAndQueuesAReplay proves CreateReplay adds a
// new pending delivery marked as a replay, counts it in the target's
// totals, queues its task built from the target it was given and the
// stored event, and leaves the delivery it repeats as it was. The
// stored event has every event field set, so a task that drops one of
// them differs from the event.
func TestCreateReplay_CreatesAndQueuesAReplay(t *testing.T) {
t.Parallel()
s := newISetup(t)
target := storeTestTarget(t, s, "replayed")
event := newStoreTestEvent(s, `{"replay":"me"}`)
require.NoError(t, s.WebhookDB.Create(event).Error)
original := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusFailed,
)
deliveryID, err := s.Engine.CreateReplay(s.WebhookID, event.ID, &target)
require.NoError(t, err)
replay := storedDelivery(t, s.WebhookDB, deliveryID)
assert.Equal(t, event.ID, replay.EventID)
assert.Equal(t, target.ID, replay.TargetID)
assert.Equal(t, database.DeliveryStatusPending, replay.Status)
assert.True(t, replay.Replay)
assert.Equal(t, database.DeliveryStatusFailed,
storedDelivery(t, s.WebhookDB, original.ID).Status)
assert.Equal(t, int64(1),
targetTotals(t, s.WebhookDB, target.ID).Deliveries)
tasks := queuedTasks(s.Engine)
require.Len(t, tasks, 1)
assertTaskFor(t, tasks[0], deliveryID, event, &target)
}
// TestCreateReplay_RollsBackWhenAWriteFails proves the replay delivery
// and its target totals are one transaction: when the totals write
// fails, after the delivery was written, no new delivery is left and
// nothing is queued.
func TestCreateReplay_RollsBackWhenAWriteFails(t *testing.T) {
t.Parallel()
s := newISetup(t)
target := storeTestTarget(t, s, "replayed")
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"replay":"me"}`)
iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusFailed,
)
require.NoError(t,
s.WebhookDB.Migrator().DropTable(&database.TargetTotals{}))
_, err := s.Engine.CreateReplay(s.WebhookID, event.ID, &target)
require.Error(t, err)
assert.Len(t, storedDeliveries(t, s.WebhookDB, event.ID), 1)
assert.Empty(t, queuedTasks(s.Engine))
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// storedDelivery reads one delivery back.
func storedDelivery(
t *testing.T, db *gorm.DB, deliveryID string,
) database.Delivery {
t.Helper()
var dlv database.Delivery
require.NoError(t, db.First(&dlv, "id = ?", deliveryID).Error)
return dlv
}
-137
View File
@@ -1,137 +0,0 @@
package delivery
import (
"encoding/json"
"errors"
"fmt"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
)
// errUnknownTargetTypeForEdit is returned when a stored target has a
// type the edit form has no field set for.
var errUnknownTargetTypeForEdit = errors.New(
"unknown target type",
)
// TargetConfigForm is the UNMASKED projection of a target's stored
// configuration, for pre-filling the target edit form.
//
// It is the deliberate exception to the rule TargetView enforces
// everywhere else: TargetView exists so that no template can render
// a target's stored blob, because a destination URL's path segments
// and a header value are both routinely the credential. An operator
// cannot correct a value they cannot see, so the edit form — and
// only the edit form — is shown the full value.
//
// Everything that keeps that exception narrow lives at the call
// site: the route is behind RequireAuth and the webhook's ownership
// check, and its group sets NoCache so the rendered secret is not
// written to a shared cache. Do not reach for this type from any
// other page.
type TargetConfigForm struct {
// URL is the destination for an HTTP target and the webhook
// URL for a Slack target.
URL string
// Headers is the HTTP target's configured headers in the
// textarea representation, one "Name: value" per line.
Headers string
// Timeout is the HTTP target's per-request timeout in seconds,
// empty when unset.
Timeout string
// ForwardQuery is the HTTP target's setting that passes each
// event's query string on to it.
ForwardQuery bool
// Expiry is the database (archive) target's row expiry.
Expiry string
// Rotation is the database (archive) target's rotation.
Rotation string
}
// NewTargetConfigForm parses a target's stored configuration into
// the edit form's fields.
//
// A configuration that does not parse is an error rather than a
// zero-valued form that silently looks like a target with no
// settings. The caller shows the operator that the stored value
// could not be read, so that saving the form is understood as
// replacing it rather than preserving it.
func NewTargetConfigForm(
t *database.Target,
) (TargetConfigForm, error) {
switch t.Type {
case database.TargetTypeHTTP:
cfg, err := parseHTTPConfig(t.Config)
if err != nil {
return TargetConfigForm{}, err
}
return TargetConfigForm{
URL: cfg.URL,
Headers: FormatTargetHeaders(cfg.Headers),
Timeout: FormatTargetTimeout(cfg.Timeout),
ForwardQuery: cfg.ForwardQuery,
}, nil
case database.TargetTypeSlack:
cfg, err := parseSlackConfig(t.Config)
if err != nil {
return TargetConfigForm{}, err
}
return TargetConfigForm{URL: cfg.WebhookURL}, nil
case database.TargetTypeDatabase:
return databaseConfigForm(t.Config)
case database.TargetTypeLog:
// The log target takes no configuration.
return TargetConfigForm{}, nil
default:
return TargetConfigForm{}, fmt.Errorf(
"%w: %q", errUnknownTargetTypeForEdit, t.Type,
)
}
}
// databaseConfigForm parses an archive target's optional expiry and
// rotation. An absent, empty or never expiry yields an empty expiry,
// on which the edit form starts at never; saving it unchanged stores
// never, which means the same as an empty expiry. An absent rotation
// is empty too, and the form starts at none. An expiry that is set
// but not a valid duration, or a rotation that is not one of the
// four, is an error, not a blank field.
func databaseConfigForm(
configJSON string,
) (TargetConfigForm, error) {
if configJSON == "" {
return TargetConfigForm{}, nil
}
var cfg archive.DatabaseTargetConfig
err := json.Unmarshal([]byte(configJSON), &cfg)
if err != nil {
return TargetConfigForm{}, fmt.Errorf(
"parsing config JSON: %w", err,
)
}
err = archive.ValidateRotation(cfg.Rotation)
if err != nil {
return TargetConfigForm{}, err
}
form := TargetConfigForm{Rotation: cfg.Rotation}
if cfg.Expiry == "" || cfg.Expiry == archive.ExpiryNever {
return form, nil
}
err = archive.ValidateExpiry(cfg.Expiry)
if err != nil {
return TargetConfigForm{}, err
}
form.Expiry = cfg.Expiry
return form, nil
}
+33 -74
View File
@@ -9,16 +9,17 @@ import (
"sneak.berlin/go/webhooker/internal/database"
)
// configUnavailable is what a target's configuration renders
// as when it is absent, of an unknown type, or does not
// parse. The stored blob is never shown as a fallback: it can
// hold a credential (a Slack incoming webhook URL is a bearer
// token) and a UI that prints it leaks that credential into
// browser history, screenshots and screen shares.
// configUnavailable is what a target's settings render as when
// the target is of an unknown type, and what its URL renders as
// when it does not parse into a scheme and host. The stored URL is
// never shown as a fallback: it can be a credential (a Slack
// incoming webhook URL is a bearer token) and a UI that prints it
// leaks that credential into browser history, screenshots and
// screen shares.
const configUnavailable = "(unavailable)"
// ConfigField is one labelled, display-safe value derived
// from a target's stored configuration.
// from a target's settings.
type ConfigField struct {
Label string
Value string
@@ -32,9 +33,9 @@ type ConfigField struct {
const deletedNameSuffix = " (deleted)"
// TargetView is the display-safe projection of a target for
// the UI. It deliberately has no raw configuration field, so
// no template — present or future — can render the stored
// blob.
// the UI. It deliberately has no URL or header field, so no
// template — present or future — can render the stored
// credential.
type TargetView struct {
ID string
Name string
@@ -61,8 +62,7 @@ func (v TargetView) DisplayName() string {
}
// NewTargetViews projects targets for rendering, replacing
// each stored configuration blob with named, display-safe
// fields.
// each target's settings with named, display-safe fields.
//
// A soft-deleted row projects exactly as a live one does, minus
// the deleted marker on its name: masking is a property of the
@@ -91,8 +91,8 @@ func NewTargetViews(
}
// targetConfigFields returns the display-safe fields for a
// target's configuration. Anything it cannot parse becomes
// the neutral placeholder.
// target's settings. A target of an unknown type gets the
// neutral placeholder.
func targetConfigFields(
t *database.Target,
) []ConfigField {
@@ -102,36 +102,25 @@ func targetConfigFields(
case database.TargetTypeHTTP:
return httpConfigFields(t)
case database.TargetTypeDatabase:
return databaseConfigFields(t.Config)
return databaseConfigFields(t)
case database.TargetTypeLog:
// The log target takes no configuration.
// The log target takes no settings.
return nil
default:
return unavailableConfigFields()
return []ConfigField{{
Label: "Configuration",
Value: configUnavailable,
}}
}
}
// unavailableConfigFields is the neutral placeholder shown
// for a configuration that could not be presented.
func unavailableConfigFields() []ConfigField {
return []ConfigField{{
Label: "Configuration",
Value: configUnavailable,
}}
}
// slackConfigFields describes a Slack target: its masked
// webhook URL and its number of delivery attempts. Only the
// masked URL is shown; the full URL is the credential.
func slackConfigFields(t *database.Target) []ConfigField {
cfg, err := parseSlackConfig(t.Config)
if err != nil {
return unavailableConfigFields()
}
return []ConfigField{{
Label: "Webhook URL",
Value: cfg.MaskedWebhookURL(),
Value: MaskURL(t.URL),
}, maxAttemptsField(t)}
}
@@ -146,33 +135,28 @@ func slackConfigFields(t *database.Target) []ConfigField {
// whose path segments are the credential, and the field takes
// an arbitrary URL, so no segment can be assumed non-secret.
func httpConfigFields(t *database.Target) []ConfigField {
cfg, err := parseHTTPConfig(t.Config)
if err != nil {
return unavailableConfigFields()
}
fields := []ConfigField{{
Label: "Destination URL",
Value: MaskURL(cfg.URL),
Value: MaskURL(t.URL),
}}
if cfg.Timeout > 0 {
if t.TimeoutSeconds > 0 {
fields = append(fields, ConfigField{
Label: "Timeout",
Value: strconv.Itoa(cfg.Timeout) + "s",
Value: strconv.Itoa(t.TimeoutSeconds) + "s",
})
}
if len(cfg.Headers) > 0 {
if len(t.Headers) > 0 {
fields = append(fields, ConfigField{
Label: "Headers",
Value: fmt.Sprintf(
"%d configured", len(cfg.Headers),
"%d configured", len(t.Headers),
),
})
}
if cfg.ForwardQuery {
if t.ForwardQuery {
fields = append(fields, ConfigField{
Label: "Query string",
Value: "passed on to this target",
@@ -185,8 +169,7 @@ func httpConfigFields(t *database.Target) []ConfigField {
}
// maxAttemptsField describes a target's number of delivery
// attempts, which lives on the target row rather than in its
// configuration blob.
// attempts.
func maxAttemptsField(t *database.Target) ConfigField {
return ConfigField{
Label: "Delivery attempts",
@@ -196,24 +179,11 @@ func maxAttemptsField(t *database.Target) ConfigField {
// databaseConfigFields describes an archive target by its
// expiry in plain units, such as "30 days", or "never" when
// the archive is kept forever, and by its rotation. An expiry
// that is set but not a valid duration, or a rotation that is
// not one of the four, is reported as unavailable rather than
// echoed back.
func databaseConfigFields(configJSON string) []ConfigField {
expiry, err := archive.ParseExpiry(configJSON)
if err != nil {
return unavailableConfigFields()
}
rotation, err := archive.ParseRotation(configJSON)
if err != nil {
return unavailableConfigFields()
}
// the archive is kept forever, and by its rotation.
func databaseConfigFields(t *database.Target) []ConfigField {
value := archive.ExpiryNever
if expiry > 0 {
value = plainDuration(expiry)
if t.ArchiveExpiry > 0 {
value = plainDuration(t.ArchiveExpiry)
}
return []ConfigField{{
@@ -221,7 +191,7 @@ func databaseConfigFields(configJSON string) []ConfigField {
Value: value,
}, {
Label: "Archive rotation",
Value: rotation,
Value: t.ArchiveRotation,
}}
}
@@ -257,14 +227,3 @@ func plainDuration(d time.Duration) string {
return d.String()
}
// MaskedWebhookURL returns the Slack webhook URL reduced to
// its scheme and host, with the path, query and any userinfo
// elided. The path segments are the credential, so none of
// them is shown: the field accepts an arbitrary URL, so no
// segment can be assumed non-secret. A URL that does not
// parse into a scheme and host yields the neutral
// placeholder, never the raw string.
func (c *SlackTargetConfig) MaskedWebhookURL() string {
return MaskURL(c.WebhookURL)
}
+58 -122
View File
@@ -35,18 +35,17 @@ const (
viewMaxAttempts = "Delivery attempts"
)
// The archive rotations a database target's settings can name, and
// the config of one whose rotation is not one of the four.
// The archive rotations a database target's settings can name.
const (
rotationNone = "none"
rotationMonthly = "monthly"
rotationDaily = "daily"
rotationHourly = "hourly"
weeklyConfig = `{"rotation":"weekly"}`
)
func TestMaskedWebhookURL(t *testing.T) {
// TestNewTargetViews_SlackMasksWebhookURL shows what a Slack target's
// URL renders as on the target list.
func TestNewTargetViews_SlackMasksWebhookURL(t *testing.T) {
t.Parallel()
tests := map[string]struct {
@@ -90,28 +89,24 @@ func TestMaskedWebhookURL(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()
cfg := &delivery.SlackTargetConfig{
WebhookURL: tc.url,
}
target := slackTarget()
target.URL = tc.url
assert.Equal(
t, tc.want, cfg.MaskedWebhookURL(),
t, tc.want,
fieldMap(viewFor(t, target).Config)["Webhook URL"],
)
})
}
}
// TestMaskedWebhookURL_NeverLeaksPath is the direct
// TestNewTargetViews_SlackNeverLeaksPath is the direct
// expression of the rule: whatever the input, the masked
// value never contains a path segment of it.
func TestMaskedWebhookURL_NeverLeaksPath(t *testing.T) {
func TestNewTargetViews_SlackNeverLeaksPath(t *testing.T) {
t.Parallel()
cfg := &delivery.SlackTargetConfig{
WebhookURL: slackWebhookURL,
}
masked := cfg.MaskedWebhookURL()
masked := fieldMap(viewFor(t, slackTarget()).Config)["Webhook URL"]
assert.NotContains(t, masked, "T00000000")
assert.NotContains(t, masked, "B00000000")
@@ -177,11 +172,10 @@ func TestNewTargetViews_DeletedTarget(t *testing.T) {
// share.
func slackTarget() database.Target {
return database.Target{
Name: slackTargetName,
Type: database.TargetTypeSlack,
Active: true,
Config: `{"webhookUrl":"` +
slackWebhookURL + `"}`,
Name: slackTargetName,
Type: database.TargetTypeSlack,
Active: true,
URL: slackWebhookURL,
MaxAttempts: 1,
}
}
@@ -233,12 +227,12 @@ func TestNewTargetViews_HTTP(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + viewExampleHook + `",` +
`"timeout":30,` +
`"headers":{"Authorization":"Bearer sekrit"},` +
`"forwardQuery":true}`,
MaxAttempts: 5,
Type: database.TargetTypeHTTP,
URL: viewExampleHook,
TimeoutSeconds: 30,
Headers: map[string]string{headerAuthorization: "Bearer sekrit"},
ForwardQuery: true,
MaxAttempts: 5,
})
fields := fieldMap(view.Config)
@@ -268,7 +262,7 @@ func TestNewTargetViews_HTTPSingleAttempt(t *testing.T) {
view := viewFor(t, database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + viewExampleHook + `"}`,
URL: viewExampleHook,
MaxAttempts: 1,
})
@@ -291,7 +285,7 @@ func TestNewTargetViews_HTTPMasksDestinationURL(t *testing.T) {
view := viewFor(t, database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + slackWebhookURL + `"}`,
URL: slackWebhookURL,
MaxAttempts: 1,
})
@@ -315,25 +309,20 @@ func TestNewTargetViews_Database(t *testing.T) {
t.Parallel()
tests := map[string]struct {
config string
expiry time.Duration
want string
}{
"empty config": {config: "", want: viewExpiryNever},
"empty expiry": {config: `{}`, want: viewExpiryNever},
"never literal": {
config: `{"expiry":"` + viewExpiryNever + `"}`,
want: viewExpiryNever,
},
"1h": {config: `{"expiry":"1h"}`, want: "1 hour"},
"12h": {config: `{"expiry":"12h"}`, want: "12 hours"},
"24h": {config: `{"expiry":"24h"}`, want: "1 day"},
"720h": {config: `{"expiry":"720h"}`, want: "30 days"},
"2160h": {config: `{"expiry":"2160h"}`, want: "90 days"},
"8760h": {config: `{"expiry":"8760h"}`, want: "365 days"},
"36h": {config: `{"expiry":"36h"}`, want: "36 hours"},
"1h30m": {config: `{"expiry":"1h30m"}`, want: "90 minutes"},
"45s": {config: `{"expiry":"45s"}`, want: "45 seconds"},
"1.5s": {config: `{"expiry":"1.5s"}`, want: "1.5s"},
"never": {expiry: 0, want: viewExpiryNever},
"1h": {expiry: time.Hour, want: "1 hour"},
"12h": {expiry: 12 * time.Hour, want: "12 hours"},
"24h": {expiry: 24 * time.Hour, want: "1 day"},
"720h": {expiry: 720 * time.Hour, want: "30 days"},
"2160h": {expiry: 2160 * time.Hour, want: "90 days"},
"8760h": {expiry: 8760 * time.Hour, want: "365 days"},
"36h": {expiry: 36 * time.Hour, want: "36 hours"},
"1h30m": {expiry: 90 * time.Minute, want: "90 minutes"},
"45s": {expiry: 45 * time.Second, want: "45 seconds"},
"1.5s": {expiry: 1500 * time.Millisecond, want: "1.5s"},
}
for name, tc := range tests {
@@ -341,8 +330,9 @@ func TestNewTargetViews_Database(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeDatabase,
Config: tc.config,
Type: database.TargetTypeDatabase,
ArchiveExpiry: tc.expiry,
ArchiveRotation: rotationNone,
})
assert.Equal(
@@ -358,33 +348,23 @@ func TestNewTargetViews_Database(t *testing.T) {
}
// TestNewTargetViews_DatabaseRotation proves the target list shows a
// database target's rotation, none when it has none stored.
// database target's rotation.
func TestNewTargetViews_DatabaseRotation(t *testing.T) {
t.Parallel()
// Each stored config, and the rotation the list shows for it.
tests := map[string]string{
"": rotationNone,
`{"rotation":""}`: rotationNone,
}
for _, rotation := range []string{
rotationNone, rotationMonthly, rotationDaily, rotationHourly,
} {
tests[`{"rotation":"`+rotation+`"}`] = rotation
}
for config, want := range tests {
t.Run(config, func(t *testing.T) {
t.Run(rotation, func(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeDatabase,
Config: config,
Type: database.TargetTypeDatabase,
ArchiveRotation: rotation,
})
assert.Equal(
t, want, fieldMap(view.Config)["Archive rotation"],
t, rotation, fieldMap(view.Config)["Archive rotation"],
)
})
}
@@ -394,72 +374,28 @@ func TestNewTargetViews_Log(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeLog,
Config: "",
Type: database.TargetTypeLog,
})
assert.Empty(t, view.Config)
}
// TestNewTargetViews_Unpresentable proves that no config the
// view cannot present falls back to the stored blob.
func TestNewTargetViews_Unpresentable(t *testing.T) {
// TestNewTargetViews_UnknownType proves that a target of a type the
// view does not know shows the neutral placeholder, and never its
// URL.
func TestNewTargetViews_UnknownType(t *testing.T) {
t.Parallel()
const blob = `{"webhookUrl":"https://hooks.slack.com` +
slackSecretPath + `"`
view := viewFor(t, database.Target{
Type: database.TargetType("carrier-pigeon"),
URL: slackWebhookURL,
})
tests := map[string]database.Target{
"unknown target type": {
Type: database.TargetType("carrier-pigeon"),
Config: blob,
assert.Equal(
t,
map[string]string{
"Configuration": viewUnavailable,
},
"unparseable json": {
Type: database.TargetTypeSlack,
Config: blob,
MaxAttempts: 1,
},
"empty slack config": {
Type: database.TargetTypeSlack,
MaxAttempts: 1,
},
"slack config without url": {
Type: database.TargetTypeSlack,
Config: `{}`,
MaxAttempts: 1,
},
"unparseable http json": {
Type: database.TargetTypeHTTP,
Config: `{"url":`,
MaxAttempts: 1,
},
"unparseable archive json": {
Type: database.TargetTypeDatabase,
Config: `{"expiry":`,
},
"invalid archive expiry": {
Type: database.TargetTypeDatabase,
Config: `{"expiry":"a fortnight"}`,
},
"invalid archive rotation": {
Type: database.TargetTypeDatabase,
Config: weeklyConfig,
},
}
for name, target := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()
view := viewFor(t, target)
assert.Equal(
t,
map[string]string{
"Configuration": viewUnavailable,
},
fieldMap(view.Config),
)
})
}
fieldMap(view.Config),
)
}
+108 -19
View File
@@ -1,6 +1,7 @@
package delivery_test
import (
"context"
"database/sql"
"fmt"
"log/slog"
@@ -20,6 +21,7 @@ import (
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/database/databasetest"
"sneak.berlin/go/webhooker/internal/db/migrations"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/gormlog"
)
@@ -78,10 +80,11 @@ func (env *archiveEnv) archivePath(tgt *database.Target) string {
)
}
// seedDatabaseTarget creates a webhook with one database target
// carrying the given target config JSON, and returns the target.
// seedDatabaseTarget creates a webhook with one database target,
// which keeps its archive forever in one file, and returns the
// target.
func (env *archiveEnv) seedDatabaseTarget(
t *testing.T, configJSON string,
t *testing.T,
) *database.Target {
t.Helper()
@@ -97,12 +100,12 @@ func (env *archiveEnv) seedDatabaseTarget(
)
tgt := &database.Target{
WebhookID: wh.ID,
Name: "Archive",
Type: database.TargetTypeDatabase,
Active: true,
Config: configJSON,
MaxAttempts: 1,
WebhookID: wh.ID,
Name: "Archive",
Type: database.TargetTypeDatabase,
Active: true,
ArchiveRotation: "none",
MaxAttempts: 1,
}
require.NoError(
t,
@@ -136,9 +139,10 @@ func seedDatabaseTargetDelivery(
Status: database.DeliveryStatusPending,
Event: event,
Target: database.Target{
Name: tgt.Name,
Type: database.TargetTypeDatabase,
Config: tgt.Config,
Name: tgt.Name,
Type: database.TargetTypeDatabase,
ArchiveExpiry: tgt.ArchiveExpiry,
ArchiveRotation: tgt.ArchiveRotation,
},
}
d.ID = dlv.ID
@@ -218,7 +222,7 @@ func TestDeliverDatabase_ArchivesEvent(t *testing.T) {
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, "")
tgt := env.seedDatabaseTarget(t)
webhookDB := testWebhookDB(t)
event := seedEvent(t, webhookDB, `{"archived":true}`)
@@ -256,9 +260,9 @@ func TestDeliverDatabase_ArchivesEvent(t *testing.T) {
}
// TestDeliverDatabase_ArchiveFailureFailsDelivery verifies that
// an archive error (here: an unparseable expiry in the target
// config) fails the delivery loudly: the attempt is recorded as
// failed with the error and the delivery is marked failed, not
// an archive error (here: an event with no webhook id to record)
// fails the delivery loudly: the attempt is recorded as failed
// with the error and the delivery is marked failed, not
// delivered.
func TestDeliverDatabase_ArchiveFailureFailsDelivery(
t *testing.T,
@@ -266,11 +270,12 @@ func TestDeliverDatabase_ArchiveFailureFailsDelivery(
t.Parallel()
env := setupArchiveTest(t)
tgt := env.seedDatabaseTarget(t, `{"expiry":"nonsense"}`)
tgt := env.seedDatabaseTarget(t)
webhookDB := testWebhookDB(t)
event := seedEvent(t, webhookDB, `{"archived":false}`)
d := seedDatabaseTargetDelivery(t, webhookDB, event, tgt)
d.Event.WebhookID = ""
env.eng.ExportDeliverDatabase(webhookDB, d)
@@ -295,12 +300,96 @@ func TestDeliverDatabase_ArchiveFailureFailsDelivery(
"the attempt must be recorded as failed",
)
assert.Contains(t,
results[0].Error, "nonsense",
results[0].Error, "without a webhook id",
"the archive error must be recorded on the attempt",
)
assert.NoFileExists(t,
env.archivePath(tgt),
"no archive file should exist for a failed config",
"no archive file should exist for a failed write",
)
}
// seedArchivedRow creates the archive file at path holding one row, for
// eventID, archived at archivedAt.
func seedArchivedRow(
t *testing.T, path, eventID string, archivedAt time.Time,
) {
t.Helper()
sqlDB, err := sql.Open(
"sqlite", fmt.Sprintf("file:%s?mode=rwc", path),
)
require.NoError(t, err)
defer func() { _ = sqlDB.Close() }()
gdb, err := gorm.Open(
sqlite.Dialector{Conn: sqlDB},
&gorm.Config{Logger: gormlog.New(slog.New(slog.DiscardHandler))},
)
require.NoError(t, err)
require.NoError(t, migrations.Apply(gdb, migrations.Archive))
require.NoError(t, gdb.Exec(
"INSERT INTO archived_events (event_id, archived_at) VALUES (?, ?)",
eventID, archivedAt,
).Error)
}
// TestDeliverDatabase_TaskArchiveSettings proves a task's ArchiveExpiry
// and ArchiveRotation reach the archive through the engine. The
// target's row has neither, so only the task can bring them: with daily
// rotation the event goes to the file for its day, and opening that
// file with a one-hour expiry prunes the row it already held, archived
// two hours ago.
func TestDeliverDatabase_TaskArchiveSettings(t *testing.T) {
t.Parallel()
s := newISetup(t)
webhook := database.Webhook{UserID: uuid.NewString(), Name: "settings"}
webhook.ID = s.WebhookID
require.NoError(t, s.MainDB.Create(&webhook).Error)
target := database.Target{
WebhookID: s.WebhookID,
Name: "archive",
Type: database.TargetTypeDatabase,
Active: true,
MaxAttempts: 1,
}
require.NoError(t, s.MainDB.Create(&target).Error)
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"archived":true}`)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusPending,
)
task := iTask(
d, event, s.WebhookID, target.ID, target.Name, "", 1, 1,
&event.Body,
)
task.TargetType = database.TargetTypeDatabase
task.ArchiveExpiry = time.Hour
task.ArchiveRotation = "daily"
daily := target
daily.ArchiveRotation = task.ArchiveRotation
dailyFile := archive.PathAt(s.DBMgr, &webhook, &daily, event.CreatedAt)
seedArchivedRow(t, dailyFile, "expired", time.Now().Add(-2*time.Hour))
s.Engine.ExportProcessNewTask(context.TODO(), &task)
iAssertStatus(t, s.WebhookDB, d.ID, database.DeliveryStatusDelivered)
var rows []archivedRow
require.NoError(t, openArchiveDBForRead(t, dailyFile).
Table("archived_events").Find(&rows).Error)
require.Len(t, rows, 1)
assert.Equal(t, event.ID, rows[0].EventID,
"the event goes to its day's file and the expired row is pruned",
)
}
+7 -9
View File
@@ -17,7 +17,7 @@ import (
const MaxTargetTimeoutSeconds = 300
// Errors returned when a target's header or timeout form input
// cannot be turned into a configuration.
// cannot be turned into the target's setting.
//
// None of these ever quotes a header VALUE. A target header value
// is routinely an authorization token, and these messages are shown
@@ -48,8 +48,8 @@ var (
)
// isReservedTargetHeader reports whether name (canonicalised) is a
// header a target configuration may not set, because the delivery
// path or net/http itself writes it regardless.
// header a target may not set, because the delivery path or
// net/http itself writes it regardless.
//
// These are rejected rather than accepted-and-ignored. Storing a
// header that provably never reaches the wire tells the operator
@@ -77,13 +77,11 @@ func isReservedTargetHeader(name string) bool {
// ParseTargetHeaders turns the target form's headers field — one
// "Name: value" pair per line, blank lines ignored — into the map
// stored in HTTPTargetConfig.Headers. Names are canonicalised, so a
// stored in database.Target.Headers. Names are canonicalised, so a
// name repeated in a different case is still a duplicate rather than
// one pair silently overwriting the other.
//
// An input with no pairs yields an empty map, which omitempty drops
// from the stored config, so a target configured with no headers
// stores no headers key at all.
// An input with no pairs yields an empty map.
func ParseTargetHeaders(raw string) (map[string]string, error) {
headers := make(map[string]string)
@@ -223,8 +221,8 @@ func FormatTargetHeaders(headers map[string]string) string {
// ParseTargetTimeout interprets the target form's timeout field as
// a whole number of seconds. An empty field means "unset" and yields
// 0, which omitempty drops from the stored config and which the
// delivery path reads as "use the shared client's timeout".
// 0, which the delivery path reads as "use the shared client's
// timeout".
//
// Anything else that is not a whole number in range is an error, not
// a silently substituted default: a target whose timeout was typed
+4 -99
View File
@@ -1,23 +1,21 @@
package delivery_test
import (
"encoding/json"
"strconv"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// Literals these tests repeat, named so that the header names and the
// keep-forever archive config each have one definition.
// Literals these tests repeat, named so that each header name has one
// definition.
const (
headerAuthorization = "Authorization"
headerContentType = "Content-Type"
headerUserAgent = "User-Agent"
bearerValue = "Bearer abc"
archiveConfigNever = "{\"expiry\":\"never\"}"
)
func TestParseTargetHeaders_AcceptsPairs(t *testing.T) {
@@ -41,23 +39,13 @@ func TestParseTargetHeaders_AcceptsPairs(t *testing.T) {
)
}
// A configuration with no headers must stay indistinguishable from
// one written before the field existed, so omitempty drops the key.
// An input of blank lines is no headers, not an error.
func TestParseTargetHeaders_EmptyInputYieldsNoHeaders(t *testing.T) {
t.Parallel()
got, err := delivery.ParseTargetHeaders("\n \n")
require.NoError(t, err)
assert.Empty(t, got)
encoded, err := json.Marshal(delivery.HTTPTargetConfig{
URL: "https://example.com/h",
Headers: got,
})
require.NoError(t, err)
assert.JSONEq(
t, `{"url":"https://example.com/h"}`, string(encoded),
)
}
func TestParseTargetHeaders_Rejects(t *testing.T) {
@@ -181,89 +169,6 @@ func TestFormatTargetTimeout(t *testing.T) {
assert.Empty(t, delivery.FormatTargetTimeout(-1))
}
func TestNewTargetConfigForm(t *testing.T) {
t.Parallel()
form, err := delivery.NewTargetConfigForm(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"https://example.com/h",` +
`"headers":{"Authorization":"Bearer abc"},` +
`"timeout":9}`,
MaxAttempts: 1,
})
require.NoError(t, err)
assert.Equal(t, "https://example.com/h", form.URL)
assert.Equal(t, "Authorization: Bearer abc\n", form.Headers)
assert.Equal(t, "9", form.Timeout)
form, err = delivery.NewTargetConfigForm(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"https://hooks.example/s"}`,
MaxAttempts: 1,
})
require.NoError(t, err)
assert.Equal(t, "https://hooks.example/s", form.URL)
form, err = delivery.NewTargetConfigForm(&database.Target{
Type: database.TargetTypeDatabase,
Config: `{"expiry":"720h","rotation":"daily"}`,
})
require.NoError(t, err)
assert.Equal(t, "720h", form.Expiry)
assert.Equal(t, rotationDaily, form.Rotation)
form, err = delivery.NewTargetConfigForm(&database.Target{
Type: database.TargetTypeLog,
})
require.NoError(t, err)
assert.Empty(t, form.URL)
}
// A keep-forever archive target yields an empty expiry, so the edit
// form starts on never; saving it unchanged stores never, which means
// the same as an empty expiry.
func TestNewTargetConfigForm_DatabaseNeverIsBlank(t *testing.T) {
t.Parallel()
for _, cfg := range []string{"", `{}`, archiveConfigNever} {
form, err := delivery.NewTargetConfigForm(
&database.Target{
Type: database.TargetTypeDatabase,
Config: cfg,
},
)
require.NoError(t, err, cfg)
assert.Empty(t, form.Expiry, cfg)
}
}
// An unreadable stored config is an error rather than a blank form
// that looks like a target with no settings, so the caller can tell
// the operator that saving replaces the stored value.
func TestNewTargetConfigForm_UnreadableConfigErrors(t *testing.T) {
t.Parallel()
cases := []*database.Target{
{Type: database.TargetTypeHTTP, Config: "not json", MaxAttempts: 1},
{Type: database.TargetTypeHTTP, Config: `{}`, MaxAttempts: 1},
{Type: database.TargetTypeSlack, Config: "", MaxAttempts: 1},
{
Type: database.TargetTypeDatabase,
Config: `{"expiry":"soon"}`,
},
{
Type: database.TargetTypeDatabase,
Config: weeklyConfig,
},
{Type: database.TargetType("nope")},
}
for _, target := range cases {
_, err := delivery.NewTargetConfigForm(target)
require.Error(t, err, target.Type)
}
}
// The ceiling exists so one misconfigured target cannot hold a
// delivery worker indefinitely, and it is inclusive.
func TestParseTargetTimeout_CeilingIsInclusive(t *testing.T) {
+23 -97
View File
@@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -17,29 +16,6 @@ import (
"sneak.berlin/go/webhooker/internal/database"
)
// Sentinel errors returned by the config parsers.
var (
errEmptyTargetConfig = errors.New(
"empty target config",
)
errMissingTargetURL = errors.New(
"target URL is required",
)
)
// HTTPTargetConfig holds configuration for http target
// types.
type HTTPTargetConfig struct {
URL string `json:"url"`
Headers map[string]string `json:"headers,omitempty"`
Timeout int `json:"timeout,omitempty"`
// ForwardQuery passes each event's query string on to the target,
// appended to URL. Off, the target URL is sent exactly as
// configured.
ForwardQuery bool `json:"forwardQuery,omitempty"`
}
// httpCore holds the retry, backoff, and circuit-breaker
// machinery shared by the HTTP and Slack targets. Each of
// those targets owns its own httpCore instance (and thus its
@@ -385,34 +361,8 @@ func (t *httpTarget) Deliver(
task *Task,
sched Scheduler,
) {
cfg, err := parseHTTPConfig(d.Target.Config)
if err != nil {
t.eng.log.Error(
"invalid HTTP target config",
"target_id", d.TargetID,
"error", err,
)
recErr := t.eng.recordResult(
webhookDB, d, task.AttemptNum,
false, 0, "", err.Error(), 0,
)
if recErr != nil {
t.eng.bookkeepingFailed(d, recErr)
return
}
t.eng.settleStatus(
webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed,
)
return
}
attempt := func() attemptResult {
return t.attempt(ctx, cfg, &d.Event)
return t.attempt(ctx, &d.Target, &d.Event)
}
t.deliver(
@@ -427,11 +377,11 @@ func (t *httpTarget) Deliver(
// string; only a transport-level error does.
func (t *httpTarget) attempt(
ctx context.Context,
cfg *HTTPTargetConfig,
target *database.Target,
event *database.Event,
) attemptResult {
statusCode, respBody, duration, reqErr :=
t.doHTTPRequest(ctx, cfg, event)
t.doHTTPRequest(ctx, target, event)
success := reqErr == nil &&
statusCode >= httpSuccessMin &&
@@ -453,7 +403,7 @@ func (t *httpTarget) attempt(
func (t *httpTarget) doHTTPRequest(
ctx context.Context,
cfg *HTTPTargetConfig,
target *database.Target,
event *database.Event,
) (int, string, int64, error) {
start := time.Now()
@@ -461,7 +411,7 @@ func (t *httpTarget) doHTTPRequest(
req, reqErr := http.NewRequestWithContext(
ctx,
http.MethodPost,
cfg.URL,
target.URL,
bytes.NewReader([]byte(event.Body)),
)
if reqErr != nil {
@@ -471,15 +421,15 @@ func (t *httpTarget) doHTTPRequest(
)
}
if cfg.ForwardQuery {
if target.ForwardQuery {
appendQuery(req.URL, event.RawQuery)
}
originScoped := applyRequestHeaders(
req, event, cfg, t.eng.userAgent(),
req, event, target.Headers, t.eng.userAgent(),
)
client := t.clientForRequest(cfg, originScoped)
client := t.clientForRequest(target.TimeoutSeconds, originScoped)
resp, doErr := executeHTTPRequest(client, req)
@@ -519,17 +469,17 @@ func appendQuery(u *url.URL, rawQuery string) {
}
// clientForRequest returns the client for one delivery attempt.
// originScoped is the header set applyRequestHeaders built for that
// attempt; a request with neither a per-target timeout nor an
// origin-scoped header gets the shared client, because there is
// then nothing for the redirect policy to strip and net/http's
// default policy already withholds Authorization and Cookie across
// hosts.
// timeoutSeconds is the target's TimeoutSeconds, and originScoped is
// the header set applyRequestHeaders built for that attempt; a
// request with neither a per-target timeout nor an origin-scoped
// header gets the shared client, because there is then nothing for
// the redirect policy to strip and net/http's default policy already
// withholds Authorization and Cookie across hosts.
func (t *httpTarget) clientForRequest(
cfg *HTTPTargetConfig,
timeoutSeconds int,
originScoped []string,
) *http.Client {
if cfg.Timeout <= 0 && len(originScoped) == 0 {
if timeoutSeconds <= 0 && len(originScoped) == 0 {
return t.client
}
@@ -542,9 +492,9 @@ func (t *httpTarget) clientForRequest(
Transport: t.client.Transport,
}
if cfg.Timeout > 0 {
if timeoutSeconds > 0 {
client.Timeout = time.Duration(
cfg.Timeout,
timeoutSeconds,
) * time.Second
}
@@ -555,31 +505,6 @@ func (t *httpTarget) clientForRequest(
return client
}
func parseHTTPConfig(
configJSON string,
) (*HTTPTargetConfig, error) {
if configJSON == "" {
return nil, errEmptyTargetConfig
}
var cfg HTTPTargetConfig
err := json.Unmarshal(
[]byte(configJSON), &cfg,
)
if err != nil {
return nil, fmt.Errorf(
"parsing config JSON: %w", err,
)
}
if cfg.URL == "" {
return nil, errMissingTargetURL
}
return &cfg, nil
}
// isForwardableHeader returns true if the header should
// be forwarded to targets.
func isForwardableHeader(name string) bool {
@@ -611,11 +536,12 @@ func isForwardableHeader(name string) bool {
// wins, otherwise the event's ContentType, otherwise none. The inbound
// Content-Type in the event's headers is never forwarded.
//
// userAgent is set last, over any configured or inbound User-Agent.
// configured is the target's Headers. userAgent is set last, over
// any configured or inbound User-Agent.
func applyRequestHeaders(
req *http.Request,
event *database.Event,
cfg *HTTPTargetConfig,
configured map[string]string,
userAgent string,
) []string {
if event.ContentType != "" {
@@ -626,7 +552,7 @@ func applyRequestHeaders(
originScoped := forwardEventHeaders(req, event)
for k, v := range cfg.Headers {
for k, v := range configured {
req.Header.Set(k, v)
originScoped[http.CanonicalHeaderKey(k)] = struct{}{}
}
@@ -691,7 +617,7 @@ func forwardEventHeaders(
}
// executeHTTPRequest sends an HTTP request using the provided
// client. URLs are validated by the config parsers and the
// client. URLs are validated by the target forms and the
// SSRF-safe transport before reaching here.
//
// Transport failures are masked here, at the single point
+18 -28
View File
@@ -2,7 +2,6 @@ package delivery_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
@@ -18,25 +17,13 @@ import (
// with.
const eventQuery = "a=1&b=2"
// httpTargetConfig is the stored configuration of an HTTP target at
// targetURL.
func httpTargetConfig(
t *testing.T, targetURL string, forwardQuery bool,
) string {
t.Helper()
cfg, err := json.Marshal(delivery.HTTPTargetConfig{
URL: targetURL, ForwardQuery: forwardQuery,
})
require.NoError(t, err)
return string(cfg)
}
// deliverWithQuery sends one event that arrived with eventQuery to an
// HTTP target configured with cfg, through the path a received event's
// delivery takes, and returns the attempt it recorded.
func deliverWithQuery(t *testing.T, cfg string) database.DeliveryResult {
// HTTP target at targetURL, with its ForwardQuery setting as given,
// through the path a received event's delivery takes, and returns the
// attempt it recorded.
func deliverWithQuery(
t *testing.T, targetURL string, forwardQuery bool,
) database.DeliveryResult {
t.Helper()
s := newISetup(t)
@@ -47,8 +34,10 @@ func deliverWithQuery(t *testing.T, cfg string) database.DeliveryResult {
database.DeliveryStatusPending,
)
task := iTask(
d, event, s.WebhookID, d.TargetID, "query", cfg, 1, 1, &event.Body,
d, event, s.WebhookID, d.TargetID, "query", targetURL, 1, 1,
&event.Body,
)
task.ForwardQuery = forwardQuery
task.RawQuery = eventQuery
s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -111,9 +100,9 @@ func TestDeliverHTTP_ForwardQuery(t *testing.T) {
))
t.Cleanup(ts.Close)
result := deliverWithQuery(t, httpTargetConfig(
result := deliverWithQuery(
t, ts.URL+tc.path, tc.forwardQuery,
))
)
assert.True(t, result.Success)
require.Len(t, received, 1)
@@ -144,12 +133,13 @@ func TestDeliverHTTP_ForwardedQueryKeepsTheTargetURLMasked(t *testing.T) {
t.Cleanup(ts.Close)
target := &database.Target{
Type: database.TargetTypeHTTP,
Config: httpTargetConfig(t, ts.URL+"/in?token="+secret, true),
MaxAttempts: 1,
Type: database.TargetTypeHTTP,
URL: ts.URL + "/in?token=" + secret,
ForwardQuery: true,
MaxAttempts: 1,
}
deliverWithQuery(t, target.Config)
deliverWithQuery(t, target.URL, target.ForwardQuery)
require.Len(t, received, 1)
sent := <-received
@@ -164,9 +154,9 @@ func TestDeliverHTTP_ForwardedQueryKeepsTheTargetURLMasked(t *testing.T) {
}
// Nothing listens on port 1.
failed := deliverWithQuery(t, httpTargetConfig(
failed := deliverWithQuery(
t, "http://127.0.0.1:1/in?token="+secret, true,
))
)
require.NotEmpty(t, failed.Error)
assert.NotContains(t, failed.Error, secret)
assert.NotContains(t, failed.Error, eventQuery)
+6 -27
View File
@@ -18,7 +18,7 @@ const RedactionMarker = "(redacted)"
// line behind the delivery path's own URL masking.
//
// It removes byte-identical echoes of strings taken from the
// target's stored configuration, and nothing else. Anything
// target's URL and headers, and nothing else. Anything
// the remote re-encodes survives: JSON "\/" escaping (what
// PHP's json_encode emits by default), percent-encoding, HTML
// entities, and an echo of only part of a path. It cannot
@@ -115,7 +115,7 @@ func (r Redactor) secretPrefixSuffix(s string) int {
}
// targetSecrets returns the credential-bearing strings a
// target's configuration carries.
// target's settings carry.
//
// The destination URL contributes. Its path, query and
// userinfo are the credential for both target types that have
@@ -129,36 +129,15 @@ func (r Redactor) secretPrefixSuffix(s string) int {
// picks out. That is the same class-based rule applied to
// URLs: an echoed Accept or User-Agent still renders, an
// echoed Authorization does not.
//
// A setting a type does not use is empty and contributes
// nothing, so a database or log target has nothing to redact.
func targetSecrets(t *database.Target) []string {
if t == nil {
return nil
}
switch t.Type {
case database.TargetTypeSlack:
cfg, err := parseSlackConfig(t.Config)
if err != nil {
return nil
}
return urlSecrets(cfg.WebhookURL)
case database.TargetTypeHTTP:
cfg, err := parseHTTPConfig(t.Config)
if err != nil {
return nil
}
return append(
urlSecrets(cfg.URL),
headerSecrets(cfg.Headers)...,
)
case database.TargetTypeDatabase, database.TargetTypeLog:
// Neither has a destination URL, so neither has
// anything to redact.
return nil
default:
return nil
}
return append(urlSecrets(t.URL), headerSecrets(t.Headers)...)
}
// urlSecrets returns the substrings of a destination URL that
+29 -29
View File
@@ -23,7 +23,7 @@ func TestRedactor_RemovesSlackWebhookURL(t *testing.T) {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
URL: redactWebhookURL,
MaxAttempts: 1,
})
@@ -54,7 +54,7 @@ func TestRedactor_RemovesSecretSeveredByACut(t *testing.T) {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
URL: redactWebhookURL,
MaxAttempts: 1,
})
@@ -86,18 +86,19 @@ func TestRedactor_RedactsCredentialShapedHeaderValues(
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"https://example.com/in",` +
`"headers":{` +
`"Authorization":"Bearer AAAAAAAAAAAA",` +
`"Cookie":"session=BBBBBBBBBBBB",` +
`"X-Api-Key":"CCCCCCCCCCCC",` +
`"X-Hub-Signature":"sha256=DDDDDDDDDDDD",` +
`"X-Sig":"EEEEEEEEEEEE",` +
`"X-Pass":"FFFFFFFFFFFF",` +
`"X-HMAC":"GGGGGGGGGGGG",` +
`"X-Credential":"HHHHHHHHHHHH",` +
`"Accept":"application/json",` +
`"User-Agent":"webhooker/1.0"}}`,
URL: "https://example.com/in",
Headers: map[string]string{
headerAuthorization: "Bearer AAAAAAAAAAAA",
"Cookie": "session=BBBBBBBBBBBB",
probeHeaderName: "CCCCCCCCCCCC",
"X-Hub-Signature": "sha256=DDDDDDDDDDDD",
"X-Sig": "EEEEEEEEEEEE",
"X-Pass": "FFFFFFFFFFFF",
"X-HMAC": "GGGGGGGGGGGG",
"X-Credential": "HHHHHHHHHHHH",
"Accept": "application/json",
headerUserAgent: "webhooker/1.0",
},
MaxAttempts: 1,
})
@@ -136,9 +137,9 @@ func TestRedactor_IgnoresVeryShortHeaderValues(t *testing.T) {
t.Parallel()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"https://example.com/in",` +
`"headers":{"X-Api-Key":"ab"}}`,
Type: database.TargetTypeHTTP,
URL: "https://example.com/in",
Headers: map[string]string{probeHeaderName: "ab"},
MaxAttempts: 1,
})
@@ -155,7 +156,7 @@ func TestRedactor_RemovesBarePath(t *testing.T) {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
URL: redactWebhookURL,
MaxAttempts: 1,
})
@@ -189,7 +190,7 @@ func TestRedactor_RemovesHTTPURLQueryAndUserinfo(t *testing.T) {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + raw + `"}`,
URL: raw,
MaxAttempts: 1,
})
@@ -239,7 +240,7 @@ func TestRedactor_RemovesEchoedQueryOfURLWithoutPath(t *testing.T) {
} {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + dest + `"}`,
URL: dest,
MaxAttempts: 1,
})
@@ -261,7 +262,7 @@ func TestRedactor_LeavesUnrelatedTextAlone(t *testing.T) {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
URL: redactWebhookURL,
MaxAttempts: 1,
})
@@ -288,7 +289,7 @@ func TestRedactor_EmptyUserinfoDoesNotShredTheBody(t *testing.T) {
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + dest + `"}`,
URL: dest,
MaxAttempts: 1,
})
@@ -306,11 +307,10 @@ func TestRedactor_EmptyUserinfoDoesNotShredTheBody(t *testing.T) {
)
}
// TestRedactor_ZeroValueAndConfiglessTargets pins that a
// caller with no target, an unparseable config, or a target
// type with no destination URL gets a redactor that changes
// nothing rather than one that panics.
func TestRedactor_ZeroValueAndConfiglessTargets(t *testing.T) {
// TestRedactor_ZeroValueAndURLlessTargets pins that a caller
// with no target, or a target with no destination URL, gets a
// redactor that changes nothing rather than one that panics.
func TestRedactor_ZeroValueAndURLlessTargets(t *testing.T) {
t.Parallel()
const text = "some response body"
@@ -323,8 +323,8 @@ func TestRedactor_ZeroValueAndConfiglessTargets(t *testing.T) {
for _, tgt := range []database.Target{
{Type: database.TargetTypeLog},
{Type: database.TargetTypeDatabase},
{Type: database.TargetTypeSlack, Config: "not json", MaxAttempts: 1},
{Type: database.TargetTypeHTTP, Config: "", MaxAttempts: 1},
{Type: database.TargetTypeSlack, MaxAttempts: 1},
{Type: database.TargetTypeHTTP, MaxAttempts: 1},
} {
assert.Equal(
t, text,
+19 -81
View File
@@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -15,18 +14,6 @@ import (
"sneak.berlin/go/webhooker/internal/database"
)
// errMissingWebhookURL is returned when a Slack target config
// omits its webhook URL.
var errMissingWebhookURL = errors.New(
"webhook_url is required",
)
// SlackTargetConfig holds configuration for slack target
// types.
type SlackTargetConfig struct {
WebhookURL string `json:"webhookUrl"`
}
// slackTarget delivers events to Slack incoming webhooks. It
// formats the event into a Slack message and posts it as
// JSON. It shares the retry core with the HTTP target: a
@@ -46,19 +33,6 @@ func (t *slackTarget) Deliver(
task *Task,
sched Scheduler,
) {
cfg, err := parseSlackConfig(d.Target.Config)
if err != nil {
t.eng.log.Error(
"invalid Slack target config",
"target_id", d.TargetID,
"error", err,
)
t.failConfig(webhookDB, d, err)
return
}
msg := FormatSlackMessage(&d.Event)
payload, err := json.Marshal(
@@ -71,13 +45,26 @@ func (t *slackTarget) Deliver(
"error", err,
)
t.failConfig(webhookDB, d, err)
recErr := t.eng.recordResult(
webhookDB, d, 1,
false, 0, "", err.Error(), 0,
)
if recErr != nil {
t.eng.bookkeepingFailed(d, recErr)
return
}
t.eng.settleStatus(
webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed,
)
return
}
attempt := func() attemptResult {
return t.attempt(ctx, cfg, payload)
return t.attempt(ctx, d.Target.URL, payload)
}
t.deliver(
@@ -86,37 +73,13 @@ func (t *slackTarget) Deliver(
)
}
// failConfig records a first-attempt failure for a delivery
// that could not be prepared (bad config or unmarshalable
// payload) and marks it failed.
func (t *slackTarget) failConfig(
webhookDB *gorm.DB,
d *database.Delivery,
err error,
) {
recErr := t.eng.recordResult(
webhookDB, d, 1,
false, 0, "", err.Error(), 0,
)
if recErr != nil {
t.eng.bookkeepingFailed(d, recErr)
return
}
t.eng.settleStatus(
webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed,
)
}
// attempt performs a single Slack POST and derives its
// outcome, preserving the engine's original semantics: a
// attempt performs a single Slack POST to targetURL and derives
// its outcome, preserving the engine's original semantics: a
// non-2xx response records an "HTTP <code>" error string and
// a transport error records a "sending request" error.
func (t *slackTarget) attempt(
ctx context.Context,
cfg *SlackTargetConfig,
targetURL string,
payload []byte,
) attemptResult {
start := time.Now()
@@ -124,7 +87,7 @@ func (t *slackTarget) attempt(
req, err := http.NewRequestWithContext(
ctx,
http.MethodPost,
cfg.WebhookURL,
targetURL,
bytes.NewReader(payload),
)
if err != nil {
@@ -186,31 +149,6 @@ func (t *slackTarget) readSlackResponse(
}
}
func parseSlackConfig(
configJSON string,
) (*SlackTargetConfig, error) {
if configJSON == "" {
return nil, errEmptyTargetConfig
}
var cfg SlackTargetConfig
err := json.Unmarshal(
[]byte(configJSON), &cfg,
)
if err != nil {
return nil, fmt.Errorf(
"parsing config JSON: %w", err,
)
}
if cfg.WebhookURL == "" {
return nil, errMissingWebhookURL
}
return &cfg, nil
}
// FormatSlackMessage builds a Slack-compatible message
// string from a webhook event.
func FormatSlackMessage(
+8 -9
View File
@@ -45,7 +45,7 @@ func tSeedDeletedTarget(
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, name,
database.TargetTypeHTTP, iHTTPConfig(url), 5,
database.TargetTypeHTTP, url, 5,
)
event := iSeedEvent(
@@ -122,9 +122,9 @@ func TestProcessDelivery_UnknownTargetType_RecordsWhy(
)
target := database.Target{
Name: "mystery",
Type: tUnknownType,
Config: iHTTPConfig("http://example.com/hook"),
Name: "mystery",
Type: tUnknownType,
URL: "http://example.com/hook",
}
target.ID = targetID
@@ -140,7 +140,7 @@ func TestProcessDelivery_UnknownTargetType_RecordsWhy(
body := event.Body
task := iTask(
seeded, event, s.WebhookID, targetID, "mystery",
target.Config, 0, 1, &body,
target.URL, 0, 1, &body,
)
task.TargetType = tUnknownType
@@ -346,11 +346,10 @@ func tRetryChainSetup(
iCreateWebhook(t, s.MainDB, s.WebhookID, name)
targetID := uuid.New().String()
cfg := iHTTPConfig(ts.URL)
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, name,
database.TargetTypeHTTP, cfg, 5,
database.TargetTypeHTTP, ts.URL, 5,
)
event := iSeedEvent(
@@ -367,7 +366,7 @@ func tRetryChainSetup(
body := event.Body
return iTask(
d, event, s.WebhookID, targetID, name, cfg, 5, 2, &body,
d, event, s.WebhookID, targetID, name, ts.URL, 5, 2, &body,
), targetID
}
@@ -528,7 +527,7 @@ func TestRecoverSingleRetry_TargetUnreadable_LeavesDeliveryAlone(
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "healthy",
database.TargetTypeHTTP,
iHTTPConfig("http://example.com/hook"), 5,
"http://example.com/hook", 5,
)
event := iSeedEvent(
+4 -16
View File
@@ -2,7 +2,6 @@ package delivery_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
@@ -71,13 +70,6 @@ func deliverSlackTo(
e := testEngine(t, 1)
targetID := uuid.New().String()
slackCfg, err := json.Marshal(
delivery.SlackTargetConfig{
WebhookURL: webhookURL,
},
)
require.NoError(t, err)
event := seedEvent(t, db, `{"test":true}`)
dlv := seedDelivery(
@@ -87,7 +79,7 @@ func deliverSlackTo(
d := buildSlackDelivery(
dlv, event, targetID,
"test-slack-mask", string(slackCfg),
"test-slack-mask", webhookURL,
)
e.ExportDeliverSlack(context.TODO(), db, d)
@@ -160,13 +152,9 @@ func TestDoHTTPRequest_TransportErrorMasksURL(t *testing.T) {
e := testEngine(t, 1)
cfg, err := e.ExportParseHTTPConfig(
newHTTPTargetConfig(host + maskSecretPath),
)
require.NoError(t, err)
statusCode, _, _, reqErr := e.ExportDoHTTPRequest(
context.TODO(), cfg,
context.TODO(),
&database.Target{URL: host + maskSecretPath},
&database.Event{Body: `{"test":true}`},
)
require.Error(t, reqErr)
@@ -188,7 +176,7 @@ func TestDoHTTPRequest_UnparsableURLIsMasked(t *testing.T) {
statusCode, _, _, reqErr := e.ExportDoHTTPRequest(
context.TODO(),
&delivery.HTTPTargetConfig{
&database.Target{
URL: "https://hooks.example.com" + maskSecretPath + "\n",
},
&database.Event{},
+3 -9
View File
@@ -2,7 +2,6 @@ package delivery_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/netip"
@@ -59,9 +58,9 @@ func TestUserAgent_IsTheBuildVersion(t *testing.T) {
statusCode, _, _, err := e.ExportDoHTTPRequest(
context.Background(),
&delivery.HTTPTargetConfig{
&database.Target{
URL: ts.URL,
Headers: map[string]string{"User-Agent": "configured/1"},
Headers: map[string]string{headerUserAgent: "configured/1"},
},
&database.Event{Headers: `{"User-Agent":["curl/8"]}`},
)
@@ -73,18 +72,13 @@ func TestUserAgent_IsTheBuildVersion(t *testing.T) {
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),
dlv, event, targetID, "test-slack", ts.URL,
))
require.Len(t, userAgents, 1, "the slack target sent no request")
assert.Equal(t, want, <-userAgents, "slack target")
+23 -1
View File
@@ -1,8 +1,14 @@
package handlers
import (
"strconv"
"time"
)
const (
// archiveExpiryNever is the archive expiry that keeps archived
// events forever. A stored empty expiry means the same.
// events forever. It is stored as an ArchiveExpiry of 0, and an
// empty expiry on a form means the same.
archiveExpiryNever = "never"
// tmplKeyArchiveExpiryChoices is the template data key for the
@@ -56,3 +62,19 @@ func archiveExpiryOptions(expiry string) []archiveChoice {
return append([]archiveChoice{own}, options...)
}
// formatArchiveExpiry writes a stored ArchiveExpiry the way the
// expiry select spells its values, so the target edit form starts on
// the stored choice: never for 0, a whole number of hours as "720h",
// and anything else as Go writes a duration, which the form accepts
// back unchanged.
func formatArchiveExpiry(expiry time.Duration) string {
switch {
case expiry == 0:
return archiveExpiryNever
case expiry%time.Hour == 0:
return strconv.FormatInt(int64(expiry/time.Hour), 10) + "h"
default:
return expiry.String()
}
}
+20 -16
View File
@@ -7,6 +7,7 @@ import (
"regexp"
"strings"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -94,14 +95,18 @@ func TestArchiveExpiryChoices(t *testing.T) {
env := setupSourceTest(t)
choices := []struct{ value, shown string }{
{expiryNever, expiryNever},
{"1h", "1 hour"},
{"12h", "12 hours"},
{"24h", "1 day"},
{"720h", "30 days"},
{"2160h", "90 days"},
{"8760h", "365 days"},
choices := []struct {
value string
stored time.Duration
shown string
}{
{expiryNever, 0, expiryNever},
{"1h", time.Hour, "1 hour"},
{"12h", 12 * time.Hour, "12 hours"},
{"24h", 24 * time.Hour, "1 day"},
{"720h", 720 * time.Hour, "30 days"},
{"2160h", 2160 * time.Hour, "90 days"},
{"8760h", 8760 * time.Hour, "365 days"},
}
for _, choice := range choices {
@@ -122,9 +127,7 @@ func TestArchiveExpiryChoices(t *testing.T) {
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.JSONEq(
t, `{"expiry":"`+choice.value+`"}`, targets[0].Config,
)
assert.Equal(t, choice.stored, targets[0].ArchiveExpiry)
assert.Equal(
t, []string{choice.shown},
@@ -138,7 +141,7 @@ func TestArchiveExpiryChoices(t *testing.T) {
}
// TestArchiveExpiryEditStartsOnStoredValue checks the edit form of a
// database target whose stored expiry is empty, which selects never,
// database target whose stored expiry is 0, which selects never,
// and of one whose expiry is not one of the choices, which is listed
// first as its own selected entry and saved unchanged.
func TestArchiveExpiryEditStartsOnStoredValue(t *testing.T) {
@@ -148,7 +151,8 @@ func TestArchiveExpiryEditStartsOnStoredValue(t *testing.T) {
webhook := seedWebhookWithRetention(t, env.db, 30)
empty := seedConfiguredTarget(
t, env.db, webhook.ID, database.TargetTypeDatabase, "",
t, env.db, webhook.ID, database.TargetTypeDatabase,
database.Target{},
)
_, selected := expirySelected(t, env, webhook.ID, empty.ID)
@@ -157,7 +161,7 @@ func TestArchiveExpiryEditStartsOnStoredValue(t *testing.T) {
webhook = seedWebhookWithRetention(t, env.db, 30)
unlisted := seedConfiguredTarget(
t, env.db, webhook.ID, database.TargetTypeDatabase,
`{"expiry":"36h"}`,
database.Target{ArchiveExpiry: 36 * time.Hour},
)
assert.Equal(t, []string{"36 hours"}, expiryShown(t, env, webhook.ID))
@@ -179,7 +183,7 @@ func TestArchiveExpiryEditStartsOnStoredValue(t *testing.T) {
w := submitTargetEdit(env, webhook.ID, unlisted.ID, form)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.JSONEq(
t, `{"expiry":"36h"}`, storedTarget(t, env, unlisted.ID).Config,
assert.Equal(
t, 36*time.Hour, storedTarget(t, env, unlisted.ID).ArchiveExpiry,
)
}
+38 -24
View File
@@ -16,8 +16,12 @@ import (
"sneak.berlin/go/webhooker/internal/database"
)
// rotationNone is the archive rotation that keeps one file.
const rotationNone = "none"
// The archive rotations these tests store: none keeps one file.
const (
rotationNone = "none"
rotationDaily = "daily"
rotationHourly = "hourly"
)
// rotationShown returns the archive rotations the webhook page's
// target list shows.
@@ -41,7 +45,9 @@ func TestArchiveRotationChoices(t *testing.T) {
env := setupSourceTest(t)
for _, rotation := range []string{rotationNone, "monthly", "daily", "hourly"} {
for _, rotation := range []string{
rotationNone, "monthly", rotationDaily, rotationHourly,
} {
t.Run(rotation, func(t *testing.T) {
t.Parallel()
@@ -60,10 +66,8 @@ func TestArchiveRotationChoices(t *testing.T) {
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.JSONEq(t,
`{"expiry":"720h","rotation":"`+rotation+`"}`,
targets[0].Config,
)
assert.Equal(t, 720*time.Hour, targets[0].ArchiveExpiry)
assert.Equal(t, rotation, targets[0].ArchiveRotation)
assert.Equal(t,
[]string{rotation}, rotationShown(t, env, webhook.ID))
@@ -78,26 +82,37 @@ func TestArchiveRotationChoices(t *testing.T) {
w = submitTargetEdit(env, webhook.ID, targets[0].ID, form)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.JSONEq(t,
`{"expiry":"720h","rotation":"hourly"}`,
storedTarget(t, env, targets[0].ID).Config,
)
stored := storedTarget(t, env, targets[0].ID)
assert.Equal(t, 720*time.Hour, stored.ArchiveExpiry)
assert.Equal(t, rotationHourly, stored.ArchiveRotation)
})
}
}
// TestArchiveRotationEditStartsOnNone checks the edit form of a
// database target with no rotation stored starts on none.
// TestArchiveRotationEditStartsOnNone checks that a database target
// added with no rotation is stored with none and shown with none, and
// that its edit form starts on none.
func TestArchiveRotationEditStartsOnNone(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
webhook := seedWebhookWithRetention(t, env.db, 30)
target := seedConfiguredTarget(
t, env.db, webhook.ID, database.TargetTypeDatabase, "",
)
page := targetEditPage(t, env, webhook.ID, target.ID)
form := url.Values{}
form.Set("name", "archive")
form.Set("type", string(database.TargetTypeDatabase))
w := serveTarget(
env, http.MethodPost, "/hook/"+webhook.ID+"/targets", form,
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.Equal(t, rotationNone, targets[0].ArchiveRotation)
page := targetEditPage(t, env, webhook.ID, targets[0].ID)
assert.Equal(t, []string{rotationNone}, selectedIn(page, "rotation"))
assert.Equal(t, []string{rotationNone}, rotationShown(t, env, webhook.ID))
}
@@ -125,7 +140,7 @@ func TestArchiveRotationRefused(t *testing.T) {
target := seedConfiguredTarget(
t, env.db, webhook.ID, database.TargetTypeDatabase,
`{"rotation":"daily"}`,
database.Target{ArchiveRotation: rotationDaily},
)
form.Del("type")
@@ -133,8 +148,8 @@ func TestArchiveRotationRefused(t *testing.T) {
w = submitTargetEdit(env, webhook.ID, target.ID, form)
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(t, w.Body.String(), "Invalid archive rotation")
assert.JSONEq(t,
`{"rotation":"daily"}`, storedTarget(t, env, target.ID).Config,
assert.Equal(
t, rotationDaily, storedTarget(t, env, target.ID).ArchiveRotation,
)
}
@@ -161,9 +176,8 @@ func TestHandleSourceCreateSubmit_ArchiveRotation(t *testing.T) {
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.JSONEq(t,
`{"expiry":"720h","rotation":"daily"}`, targets[0].Config,
)
assert.Equal(t, 720*time.Hour, targets[0].ArchiveExpiry)
assert.Equal(t, rotationDaily, targets[0].ArchiveRotation)
}
// TestArchiveFileView_Rotated describes a daily target's archive files
@@ -178,7 +192,7 @@ func TestArchiveFileView_Rotated(t *testing.T) {
webhook := seedWebhookWithRetention(t, env.db, 30)
target := seedConfiguredTarget(
t, env.db, webhook.ID, database.TargetTypeDatabase,
`{"rotation":"daily"}`,
database.Target{ArchiveRotation: rotationDaily},
)
path := archive.Path(env.dbMgr, &webhook, target)
+12 -90
View File
@@ -5,9 +5,7 @@ import (
"github.com/go-chi/chi"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// The outcomes of a replay POST, as the notice codes its redirect
@@ -46,12 +44,12 @@ const (
// HandleDeliveryReplay re-sends a finished delivery's event to its
// target.
//
// A replay never touches the delivery it repeats. It creates a NEW
// pending delivery row for the same event and target and hands it to
// the delivery engine through the same Notifier the receiver uses, so
// the original's status, attempts and timestamps stand as the record
// of what actually happened, and the replay is retried, SSRF-guarded
// and circuit-broken exactly as a first attempt is.
// A replay never touches the delivery it repeats. The delivery engine
// creates a NEW pending delivery row for the same event and target and
// queues it as it queues the receiver's, so the original's status,
// attempts and timestamps stand as the record of what actually
// happened, and the replay is retried, SSRF-guarded and circuit-broken
// exactly as a first attempt is.
//
// What is re-sent is the stored EVENT body, never the response the
// original delivery received.
@@ -145,7 +143,8 @@ func (h *Handlers) loadReplaySource(
return &original, true
}
// queueReplay writes the new delivery and hands it to the engine.
// queueReplay has the delivery engine create and queue the new
// delivery, unless an earlier replay is still in flight.
func (h *Handlers) queueReplay(
w http.ResponseWriter,
r *http.Request,
@@ -171,18 +170,8 @@ func (h *Handlers) queueReplay(
return
}
var event database.Event
err = webhookDB.
First(&event, "id = ?", original.EventID).Error
if err != nil {
h.serverError(w, r, "failed to load event for replay", err)
return
}
task, err := createReplayDelivery(
webhookDB, webhook.ID, &event, target,
deliveryID, err := h.engine.CreateReplay(
webhook.ID, original.EventID, target,
)
if err != nil {
h.serverError(
@@ -193,15 +182,14 @@ func (h *Handlers) queueReplay(
}
h.mtr.DeliveryReplayed(target.Type)
h.notifier.Notify([]delivery.Task{task})
h.log.Info(
"delivery replay queued",
"webhook_id", webhook.ID,
"event_id", event.ID,
"event_id", original.EventID,
"target_id", target.ID,
"replayed_delivery_id", original.ID,
"delivery_id", task.DeliveryID,
"delivery_id", deliveryID,
)
redirectToEventLog(w, r, webhook, replayQueued)
@@ -263,72 +251,6 @@ func countInFlightDeliveries(
return count, err
}
// createReplayDelivery writes the new pending delivery row, adds it to
// its target's totals in the same transaction, and returns the task
// that carries it to the delivery engine.
//
// The row is written with associations omitted, and neither Event nor
// Target is populated on it: GORM's SaveBeforeAssociations would
// otherwise upsert the whole target row — plaintext config, which for a
// Slack target is the credential — into the per-webhook event database.
// See https://git.eeqj.de/sneak/webhooker/issues/206.
func createReplayDelivery(
webhookDB *gorm.DB,
webhookID string,
event *database.Event,
target *database.Target,
) (delivery.Task, error) {
dlv := &database.Delivery{
EventID: event.ID,
TargetID: target.ID,
Status: database.DeliveryStatusPending,
Replay: true,
}
err := webhookDB.Transaction(func(tx *gorm.DB) error {
err := tx.Omit(clause.Associations).Create(dlv).Error
if err != nil {
return err
}
return database.AddTargetTotals(tx, database.TargetTotals{
TargetID: dlv.TargetID, Deliveries: 1,
})
})
if err != nil {
return delivery.Task{}, err
}
return delivery.Task{
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: webhookID,
EntrypointID: event.EntrypointID,
TargetID: target.ID,
TargetName: target.Name,
TargetType: target.Type,
TargetConfig: target.Config,
MaxAttempts: target.MaxAttempts,
Method: event.Method,
RawQuery: event.RawQuery,
Headers: event.Headers,
ContentType: event.ContentType,
Body: replayBody(event.Body),
AttemptNum: 1,
}, nil
}
// replayBody returns the stored event body for a replay task to carry
// inline, or nil when it is large enough that the engine should fetch
// it from the per-webhook database instead.
func replayBody(body string) *string {
if len(body) >= delivery.MaxInlineBodySize {
return nil
}
return &body
}
// redirectToEventLog redirects a replay or resubmit back to the event
// log it was triggered from, carrying the outcome as its notice. A
// Replay form carries the list it was pressed in as show, so a replay
+54 -85
View File
@@ -12,7 +12,6 @@ import (
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
@@ -23,7 +22,7 @@ const paramDeliveryID = "deliveryID"
// replayTargetURL is a public destination, so a target configured with
// it is one the SSRF guard would accept. Nothing in these tests
// dispatches to it: the notifier is recorded, not run.
// dispatches to it: the delivery engine is never started.
const replayTargetURL = "http://93.184.216.34/hook"
// replayEventQuery is the query string a seeded event arrived with.
@@ -163,14 +162,8 @@ func postReplay(
// TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal is the
// core requirement: replaying a failed delivery succeeds, appends a
// new delivery, and leaves the original row and its recorded attempt
// exactly as they were.
//
// It also pins the two things a replay would be wrong to get from the
// original: the task carries the target's CURRENT configuration, which
// this test changes between the failure and the replay, and it carries
// the stored EVENT body rather than anything the failed attempt
// received back.
// new delivery marked as a replay, and leaves the original row and its
// recorded attempt exactly as they were.
func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
t *testing.T,
) {
@@ -181,10 +174,9 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -192,7 +184,7 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
event, original := seedFailedDelivery(
@@ -204,15 +196,6 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
before := loadDelivery(t, webhookDB, original.ID)
// The operator fixes the destination, which is the whole reason
// to replay. The replay must use this, not the config the
// original delivery ran against.
const fixedConfig = `{"url":"http://93.184.216.34/fixed"}`
require.NoError(t, db.DB().Model(&database.Target{}).
Where("id = ?", tgt.ID).
Update("config", fixedConfig).Error)
w := postReplay(t, h, sess, wh.ID, original.ID)
require.Equal(t, http.StatusSeeOther, w.Code)
@@ -234,14 +217,9 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
assert.Equal(
t, database.DeliveryStatusPending, replayed.Status,
)
assert.True(t, replayed.Replay)
assertDeliveryUntouched(t, webhookDB, before)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assertReplayTask(
t, tasks[0], wh.ID, event, tgt, replayed.ID, fixedConfig,
)
assertNoLeakedTarget(t, webhookDB)
}
@@ -275,45 +253,6 @@ func assertDeliveryUntouched(
)
}
// assertReplayTask proves the task handed to the delivery engine is
// the one the receiver would build for this event and this target, and
// that it carries wantConfig — the target's configuration as it stands
// now rather than as the original delivery ran against it.
func assertReplayTask(
t *testing.T,
task delivery.Task,
webhookID string,
event *database.Event,
target *database.Target,
wantDeliveryID, wantConfig string,
) {
t.Helper()
assert.Equal(t, wantDeliveryID, task.DeliveryID)
assert.Equal(t, event.ID, task.EventID)
assert.Equal(t, webhookID, task.WebhookID)
assert.Equal(t, event.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Type, task.TargetType)
assert.JSONEq(
t, wantConfig, task.TargetConfig,
"replay must use the target's current configuration",
)
assert.Equal(t, event.Method, task.Method)
assert.Equal(
t, replayEventQuery, task.RawQuery,
"replay re-sends the stored query string",
)
assert.Equal(t, event.Headers, task.Headers)
assert.Equal(t, event.ContentType, task.ContentType)
assert.Equal(t, 1, task.AttemptNum)
require.NotNil(t, task.Body)
assert.Equal(
t, event.Body, *task.Body,
"replay re-sends the stored event body",
)
}
// assertNoLeakedTarget proves the per-webhook database holds no
// target: an association write would have upserted the whole target,
// plaintext config and all, into the event database. See
@@ -327,9 +266,49 @@ func assertNoLeakedTarget(t *testing.T, webhookDB *gorm.DB) {
)
}
// TestHandleDeliveryReplay_FailedStoreIsAnError proves a replay the
// engine cannot store answers with the 500 error page and adds no
// delivery.
func TestHandleDeliveryReplay_FailedStoreIsAnError(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
database.Target{URL: replayTargetURL},
)
event, original := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID)
// Without its target totals table the webhook's database refuses
// the totals write, which comes after the delivery row.
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
require.NoError(t,
webhookDB.Migrator().DropTable(&database.TargetTotals{}))
w := postReplay(t, h, sess, wh.ID, original.ID)
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Contains(t, w.Body.String(), "500 Internal Server Error")
assert.Len(t, listDeliveries(t, webhookDB, event.ID), 1)
}
// TestHandleDeliveryReplay_RefusesDeletedTarget proves the required
// refusal: a target deleted since the delivery ran is reported as
// deleted rather than erroring, and nothing is created or queued.
// deleted rather than erroring, and nothing is created.
func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
t.Parallel()
@@ -338,10 +317,9 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -349,7 +327,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
event, original := seedFailedDelivery(
@@ -377,10 +355,6 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
t, listDeliveries(t, webhookDB, event.ID), 1,
"a refused replay must create no delivery",
)
assert.Empty(
t, notif.Tasks(),
"a refused replay must queue nothing",
)
// The refusal is specific, which is why the target is looked up
// including soft-deleted rows: an id that never named a target
@@ -413,10 +387,9 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -424,7 +397,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
event, original := seedFailedDelivery(
@@ -454,10 +427,6 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
t, listDeliveries(t, webhookDB, event.ID), 2,
"the refused second replay must add nothing",
)
assert.Len(
t, notif.Tasks(), 1,
"only the first replay reaches the delivery engine",
)
// A delivery the engine has not finished is not replayable
// either, which is the same rule seen from the other side.
@@ -498,7 +467,7 @@ func TestHandleDeliveryReplay_ReturnsToTheListItWasPressedIn(
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
_, original := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID)
@@ -556,7 +525,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
_, original := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID)
@@ -615,7 +584,7 @@ func TestHandleDeliveryReplay_LabelsTheReplay(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
event, original := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID)
+12 -12
View File
@@ -86,12 +86,12 @@ func seedFailedDeliveryWithResponse(
}
// seedFailureAndRender seeds a failed delivery against a
// target of the given type and config, and returns the
// target of the given type and URL, and returns the
// rendered event log page.
func seedFailureAndRender(
t *testing.T,
targetType database.TargetType,
config, responseBody string,
targetURL, responseBody string,
) string {
t.Helper()
@@ -109,7 +109,7 @@ func seedFailureAndRender(
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, targetType, config,
t, db, wh.ID, targetType, database.Target{URL: targetURL},
)
seedFailedDeliveryWithResponse(
@@ -129,7 +129,7 @@ func TestHandleSourceLogs_RendersFailedAttempt(t *testing.T) {
body := seedFailureAndRender(
t,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook/abc"}`,
"https://example.com/hook/abc",
"upstream exploded",
)
@@ -167,7 +167,7 @@ func TestHandleSourceLogs_EscapesResponseBody(t *testing.T) {
body := seedFailureAndRender(
t,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook/abc"}`,
"https://example.com/hook/abc",
payload,
)
@@ -189,7 +189,7 @@ func TestHandleSourceLogs_RedactsCredentialEchoedInResponse(
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
slackWebhookURL,
"no_service: "+slackWebhookURL,
)
@@ -228,7 +228,7 @@ func TestHandleSourceLogs_RedactsCredentialEchoedInError(
tgt := seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
database.Target{URL: slackWebhookURL},
)
dlv := seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, "")
@@ -300,7 +300,7 @@ func TestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCut(
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
slackWebhookURL,
stored,
)
@@ -335,7 +335,7 @@ func TestHandleSourceLogs_RedactsCredentialSeveredBySQLCut(
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
slackWebhookURL,
stored,
)
@@ -373,7 +373,7 @@ func TestHandleSourceLogs_RedactsForSoftDeletedTarget(
tgt := seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
database.Target{URL: slackWebhookURL},
)
seedFailedDeliveryWithResponse(
@@ -413,7 +413,7 @@ func TestHandleSourceLogs_BoundsRenderedAttempts(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeLog, "",
t, db, wh.ID, database.TargetTypeLog, database.Target{},
)
dlv := seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, "")
@@ -480,7 +480,7 @@ func TestHandleSourceLogs_BoundsOversizeResponse(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeLog, "",
t, db, wh.ID, database.TargetTypeLog, database.Target{},
)
stored := strings.Repeat("A", responseCap*4) + tail
+3 -3
View File
@@ -13,8 +13,8 @@ import (
// DeliveryView is the display-safe projection of a delivery
// for the event log page. Its target is a TargetView, so the
// stored configuration blob — which holds the target's
// credential — has no path to the template.
// target's URL and headers — which hold the target's
// credential — have no path to the template.
type DeliveryView struct {
ID string
Status database.DeliveryStatus
@@ -126,7 +126,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
// loadTargetMap loads targets into a map of display-safe
// views keyed by target ID, each paired with its redactor.
// The projection happens here so that no caller can hand a
// raw target, configuration blob and all, to a template: the
// raw target, URL and headers and all, to a template: the
// raw rows do not leave this function.
//
// The load is Unscoped because deleting a target only soft
+17 -25
View File
@@ -171,37 +171,29 @@ func loadResubmitSource(
return src, true, nil
}
// queueResubmit stores the copy and fans it out to the webhook's
// active targets.
// queueResubmit has the delivery engine store the copy, with a delivery
// for each of the webhook's active targets as they stand now, through
// the same StoreEvent the receiver uses. The one difference is that the
// copy leaves its entrypoint's last event as it is, since it did not
// arrive on that URL.
func (h *Handlers) queueResubmit(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
src resubmitSource,
) {
// The receiver's own query, run now: an active target created
// after the original event arrived is included, and an
// inactive one is skipped rather than refused.
targets, err := h.loadActiveTargets(webhook.ID)
if err != nil {
h.serverError(w, r, "failed to query targets", err)
return
event := &database.Event{
WebhookID: webhook.ID,
EntrypointID: src.EntrypointID,
Method: src.Method,
RawQuery: src.RawQuery,
Headers: src.Headers,
ContentType: src.ContentType,
Body: string(src.Body),
ResubmittedFromID: &src.ID,
}
event, tasks, err := h.createAndFanOut(
eventSource{
WebhookID: webhook.ID,
EntrypointID: src.EntrypointID,
Method: src.Method,
RawQuery: src.RawQuery,
HeadersJSON: src.Headers,
ContentType: src.ContentType,
Body: src.Body,
ResubmittedFromID: &src.ID,
},
targets,
)
deliveries, err := h.engine.StoreEvent(event)
if err != nil {
h.serverError(w, r, "failed to store resubmitted event", err)
@@ -215,11 +207,11 @@ func (h *Handlers) queueResubmit(
"webhook_id", webhook.ID,
"event_id", event.ID,
"resubmitted_from_id", src.ID,
"target_count", len(tasks),
"target_count", deliveries,
)
code := resubmitQueued
if len(tasks) == 0 {
if deliveries == 0 {
code = resubmitNoTargets
}
+81 -102
View File
@@ -19,7 +19,7 @@ import (
// resubmitTargetURL is a public destination, so a target configured
// with it is one the SSRF guard would accept. Nothing in these tests
// dispatches to it: the notifier is recorded, not run.
// dispatches to it: the delivery engine is never started.
const resubmitTargetURL = "http://93.184.216.34/hook"
// resubmitEventHeaders and resubmitEventQuery are the stored header
@@ -134,10 +134,9 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -151,7 +150,7 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
// captured. It has no prior delivery of anything.
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`"}`,
database.Target{URL: resubmitTargetURL},
)
w := postResubmit(t, h, sess, wh.ID, original.ID)
@@ -189,11 +188,6 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
t, database.DeliveryStatusPending, deliveries[0].Status,
)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assert.Equal(t, deliveries[0].ID, tasks[0].DeliveryID)
assertResubmitTask(t, tasks[0], wh.ID, &fresh, tgt)
assertNoLeakedTarget(t, webhookDB)
}
@@ -225,32 +219,6 @@ func assertEventCopy(
)
}
// assertResubmitTask proves the task handed to the delivery engine is
// the one the receiver would build for the NEW event and this target.
func assertResubmitTask(
t *testing.T,
task delivery.Task,
webhookID string,
fresh *database.Event,
target *database.Target,
) {
t.Helper()
assert.Equal(t, fresh.ID, task.EventID)
assert.Equal(t, webhookID, task.WebhookID)
assert.Equal(t, fresh.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Type, task.TargetType)
assert.Equal(t, fresh.Method, task.Method)
assert.Equal(t, fresh.RawQuery, task.RawQuery)
assert.Equal(t, fresh.Headers, task.Headers)
assert.Equal(t, fresh.ContentType, task.ContentType)
assert.Equal(t, 1, task.AttemptNum)
require.NotNil(t, task.Body)
assert.Equal(t, fresh.Body, *task.Body)
}
// TestHandleEventResubmit_IsRepeatable proves the requirement replay
// deliberately does not meet: firing the same captured event at a
// backend over and over must work, with no in-flight refusal, and each
@@ -263,10 +231,9 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -274,7 +241,7 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`"}`,
database.Target{URL: resubmitTargetURL},
)
original := seedStoredEvent(t, dbMgr, wh.ID, `{"fire":"again"}`)
@@ -300,30 +267,18 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
require.NoError(t, err)
events := listEvents(t, webhookDB)
require.Len(t, events, presses+1)
require.Len(
t, events, presses+1, "each resubmit creates its own event",
)
tasks := notif.Tasks()
require.Len(t, tasks, presses)
assert.Empty(
t, listDeliveries(t, webhookDB, original.ID),
"each resubmit delivers its own new event",
)
seen := make(map[string]struct{}, presses)
for _, task := range tasks {
assert.Equal(t, tgt.ID, task.TargetID)
assert.NotEqual(
t, original.ID, task.EventID,
"each resubmit delivers its own new event",
)
_, dup := seen[task.EventID]
assert.False(t, dup, "each resubmit creates its own event")
seen[task.EventID] = struct{}{}
require.Len(t, listDeliveries(t, webhookDB, task.EventID), 1)
}
// Every copy names the same source event, so twenty presses stay
// traceable to the one captured request.
// Every copy has its own delivery, and names the same source
// event, so twenty presses stay traceable to the one captured
// request.
for _, e := range events {
if e.ID == original.ID {
continue
@@ -331,6 +286,10 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
require.NotNil(t, e.ResubmittedFromID)
assert.Equal(t, original.ID, *e.ResubmittedFromID)
deliveries := listDeliveries(t, webhookDB, e.ID)
require.Len(t, deliveries, 1)
assert.Equal(t, tgt.ID, deliveries[0].TargetID)
}
}
@@ -346,10 +305,9 @@ func TestHandleEventResubmit_OversizeBodySurvivesIntact(t *testing.T) {
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -368,7 +326,7 @@ func TestHandleEventResubmit_OversizeBodySurvivesIntact(t *testing.T) {
wh := seedWebhook(t, db)
seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`"}`,
database.Target{URL: resubmitTargetURL},
)
original := seedStoredEvent(t, dbMgr, wh.ID, stored)
@@ -385,22 +343,7 @@ func TestHandleEventResubmit_OversizeBodySurvivesIntact(t *testing.T) {
t, stored, fresh.Body,
"the stored body must be copied byte for byte",
)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assert.Nil(
t, tasks[0].Body,
"a body over the inline limit is fetched from the new "+
"event row rather than carried on the task",
)
// The engine's own read of the body, against the new event id:
// what it would send is what was stored.
var loaded database.Event
require.NoError(t, webhookDB.Select("body").
First(&loaded, "id = ?", tasks[0].EventID).Error)
assert.Equal(t, stored, loaded.Body)
assert.Len(t, listDeliveries(t, webhookDB, fresh.ID), 1)
}
// TestHandleEventResubmit_SkipsInactiveTarget proves a deactivated
@@ -414,10 +357,9 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -425,11 +367,11 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
wh := seedWebhook(t, db)
active := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`"}`,
database.Target{URL: resubmitTargetURL},
)
off := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`/off"}`,
database.Target{URL: resubmitTargetURL + "/off"},
)
require.NoError(t, db.DB().Model(&database.Target{}).
@@ -448,21 +390,19 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
"an inactive target is skipped, not an error",
)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assert.Equal(t, active.ID, tasks[0].TargetID)
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
fresh := theOtherEvent(t, listEvents(t, webhookDB), original.ID)
require.Len(t, listDeliveries(t, webhookDB, fresh.ID), 1)
deliveries := listDeliveries(t, webhookDB, fresh.ID)
require.Len(t, deliveries, 1)
assert.Equal(t, active.ID, deliveries[0].TargetID)
}
// TestHandleEventResubmit_NoActiveTargetsStillStoresEvent proves a
// source with nothing to deliver to behaves as the receiver does: the
// event is stored, nothing is queued, and the operator is told so
// rather than being shown an error.
// event is stored with no delivery, and the operator is told so rather
// than being shown an error.
func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
t *testing.T,
) {
@@ -473,10 +413,9 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -493,12 +432,14 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
w.Header().Get("Location"),
)
assert.Empty(t, notif.Tasks())
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
assert.Len(t, listEvents(t, webhookDB), 2)
events := listEvents(t, webhookDB)
assert.Len(t, events, 2)
fresh := theOtherEvent(t, events, original.ID)
assert.Empty(t, listDeliveries(t, webhookDB, fresh.ID))
}
// TestHandleEventResubmit_RefusesEventOfAnotherWebhook proves the
@@ -514,10 +455,9 @@ func TestHandleEventResubmit_RefusesEventOfAnotherWebhook(
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -528,7 +468,7 @@ func TestHandleEventResubmit_RefusesEventOfAnotherWebhook(
mine := seedWebhook(t, db)
seedConfiguredTarget(
t, db, mine.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`"}`,
database.Target{URL: resubmitTargetURL},
)
seedStoredEvent(t, dbMgr, mine.ID, `{"mine":true}`)
@@ -548,10 +488,49 @@ func TestHandleEventResubmit_RefusesEventOfAnotherWebhook(
w = postResubmit(t, h, sess, mine.ID, "not-a-uuid")
assert.Equal(t, http.StatusNotFound, w.Code)
assert.Empty(
t, notif.Tasks(),
"a refused resubmit must queue nothing",
for _, webhookID := range []string{theirs.ID, mine.ID} {
webhookDB, err := dbMgr.GetDB(webhookID)
require.NoError(t, err)
assert.Len(
t, listEvents(t, webhookDB), 1,
"a refused resubmit must store nothing",
)
}
}
// TestHandleEventResubmit_FailedStoreIsAnError proves a copy the engine
// cannot store answers with the 500 error page and leaves no new event.
func TestHandleEventResubmit_FailedStoreIsAnError(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
original := seedStoredEvent(t, dbMgr, wh.ID, `{"not":"stored"}`)
// Without its event totals table the webhook's database refuses
// the totals write, which comes after the event row.
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
require.NoError(t,
webhookDB.Migrator().DropTable(&database.EventTotals{}))
w := postResubmit(t, h, sess, wh.ID, original.ID)
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Contains(t, w.Body.String(), "500 Internal Server Error")
assert.Len(t, listEvents(t, webhookDB), 1)
}
// TestHandleSourceLogs_ShowsResubmitProvenance proves the event log
@@ -575,7 +554,7 @@ func TestHandleSourceLogs_ShowsResubmitProvenance(t *testing.T) {
wh := seedWebhook(t, db)
seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+resubmitTargetURL+`"}`,
database.Target{URL: resubmitTargetURL},
)
original := seedStoredEvent(t, dbMgr, wh.ID, `{"trace":"me"}`)
+17 -25
View File
@@ -133,36 +133,28 @@ func (s *Handlers) RenderTemplateForTest(
s.renderTemplate(w, r, pageTemplate, data)
}
// BuildSlackTargetConfigForTest exposes
// buildSlackTargetConfig for use in the handlers_test package.
func (s *Handlers) BuildSlackTargetConfigForTest(
// SetSlackTargetSettingsForTest exposes setSlackTargetSettings for use
// in the handlers_test package.
func (s *Handlers) SetSlackTargetSettingsForTest(
ctx context.Context,
target *database.Target,
targetURL string,
) (string, string, error) {
return s.buildSlackTargetConfig(ctx, targetURL)
) string {
return s.setSlackTargetSettings(ctx, target, targetURL)
}
// BuildHTTPTargetConfigForTest exposes buildHTTPTargetConfig
// for use in the handlers_test package, taking the form fields
// an HTTP target's configuration is built from.
func (s *Handlers) BuildHTTPTargetConfigForTest(
ctx context.Context,
targetURL, headers, timeout string,
) (string, string, error) {
return s.buildHTTPTargetConfig(ctx, targetFormInput{
URL: targetURL,
Headers: headers,
Timeout: timeout,
})
}
// BuildDatabaseTargetConfigForTest exposes
// buildDatabaseTargetConfig for use in the handlers_test
// package.
func BuildDatabaseTargetConfigForTest(
// SetDatabaseTargetSettingsForTest exposes setDatabaseTargetSettings
// for use in the handlers_test package.
func SetDatabaseTargetSettingsForTest(
target *database.Target,
expiry, rotation string,
) (string, string, error) {
return buildDatabaseTargetConfig(expiry, rotation)
) string {
return setDatabaseTargetSettings(target, expiry, rotation)
}
// FormatArchiveExpiryForTest exposes formatArchiveExpiry.
func FormatArchiveExpiryForTest(expiry time.Duration) string {
return formatArchiveExpiry(expiry)
}
// ArchiveFileViewForTest exposes archiveFileView, which describes a
+3 -3
View File
@@ -64,7 +64,7 @@ type HandlersParams struct {
Healthcheck *healthcheck.Healthcheck
Session *session.Session
Middleware *middleware.Middleware
Notifier delivery.Notifier
Engine *delivery.Engine
Archives archive.Archives
CircuitBreakers delivery.CircuitBreakers
SSRFGuard *delivery.Guard
@@ -82,7 +82,7 @@ type Handlers struct {
dbMgr *database.WebhookDBManager
session *session.Session
mw *middleware.Middleware
notifier delivery.Notifier
engine *delivery.Engine
archives archive.Archives
breakers delivery.CircuitBreakers
mtr *metrics.Set
@@ -138,7 +138,7 @@ func New(
s.dbMgr = params.WebhookDBMgr
s.session = params.Session
s.mw = params.Middleware
s.notifier = params.Notifier
s.engine = params.Engine
s.archives = params.Archives
s.breakers = params.CircuitBreakers
s.mtr = params.Metrics
+92 -87
View File
@@ -8,6 +8,7 @@ import (
"log/slog"
"net/http"
"net/http/httptest"
"strconv"
"sync"
"testing"
"time"
@@ -29,33 +30,6 @@ import (
"sneak.berlin/go/webhooker/internal/session"
)
// recordingNotifier is a delivery.Notifier that records the tasks it
// was handed, so a test can prove a handler queued the delivery it
// claims to have queued — and, on the refusal paths, that it queued
// nothing.
type recordingNotifier struct {
mu sync.Mutex
tasks []delivery.Task
}
func (n *recordingNotifier) Notify(tasks []delivery.Task) {
n.mu.Lock()
defer n.mu.Unlock()
n.tasks = append(n.tasks, tasks...)
}
// Tasks returns a copy of the recorded tasks.
func (n *recordingNotifier) Tasks() []delivery.Task {
n.mu.Lock()
defer n.mu.Unlock()
out := make([]delivery.Task, len(n.tasks))
copy(out, n.tasks)
return out
}
// recordingArchives is an archive.Archives that records what it
// was asked to do, so a test can prove that a deletion or rename
// path reached the archive writers. After FailRenames, every
@@ -263,11 +237,12 @@ func newTestAppWithConfig(
database.NewWebhookDBManager,
healthcheck.New,
session.New,
func() *recordingNotifier {
return &recordingNotifier{}
},
func(n *recordingNotifier) delivery.Notifier {
return n
archive.NewWriters,
// The engine is built on a lifecycle that is never started,
// so no worker runs: every delivery the engine creates for a
// handler stays pending, where the test can read it.
func(p delivery.EngineParams) *delivery.Engine {
return delivery.New(fxtest.NewLifecycle(t), p)
},
func() *recordingArchives {
return &recordingArchives{}
@@ -354,7 +329,7 @@ func TestHandleIndex_Authenticated(t *testing.T) {
)
}
func TestBuildSlackTargetConfig_AcceptsPublicURL(t *testing.T) {
func TestSetSlackTargetSettings_AcceptsPublicURL(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
@@ -364,16 +339,17 @@ func TestBuildSlackTargetConfig_AcceptsPublicURL(t *testing.T) {
t.Cleanup(app.RequireStop)
cfg, errMsg, err := h.BuildSlackTargetConfigForTest(
t.Context(), "http://93.184.216.34/services/T00/B00/xxx",
)
const hookURL = "http://93.184.216.34/services/T00/B00/xxx"
var target database.Target
errMsg := h.SetSlackTargetSettingsForTest(t.Context(), &target, hookURL)
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.Contains(t, cfg, "webhookUrl")
assert.Equal(t, hookURL, target.URL)
}
func TestBuildSlackTargetConfig_RejectsReservedURL(t *testing.T) {
func TestSetSlackTargetSettings_RejectsReservedURL(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
@@ -383,13 +359,14 @@ func TestBuildSlackTargetConfig_RejectsReservedURL(t *testing.T) {
t.Cleanup(app.RequireStop)
cfg, errMsg, err := h.BuildSlackTargetConfigForTest(
t.Context(), "http://169.254.169.254/latest/meta-data/",
var target database.Target
errMsg := h.SetSlackTargetSettingsForTest(
t.Context(), &target, "http://169.254.169.254/latest/meta-data/",
)
require.NoError(t, err)
assert.Contains(t, errMsg, "Invalid target URL")
assert.Empty(t, cfg)
assert.Empty(t, target.URL)
}
func TestRenderTemplate(t *testing.T) {
@@ -483,72 +460,100 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
)
}
func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
func TestSetDatabaseTargetSettings_Valid(t *testing.T) {
t.Parallel()
// Empty expiry and rotation: the keep-forever, one-file default,
// empty config.
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest("", "")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.Empty(t, cfg)
cases := []struct {
expiry, rotation string
wantExpiry time.Duration
wantRotation string
}{
// Empty expiry and rotation: the keep-forever, one-file
// default.
{"", "", 0, rotationNone},
{expiryNever, "", 0, rotationNone},
{" 36h ", "", 36 * time.Hour, rotationNone},
{"", rotationDaily, 0, rotationDaily},
{"90m", rotationHourly, 90 * time.Minute, rotationHourly},
}
// Explicit never is stored as config.
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("never", "")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.JSONEq(t, `{"expiry":"never"}`, cfg)
for _, tc := range cases {
var target database.Target
// A positive duration is stored as config.
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("720h", "")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.JSONEq(t, `{"expiry":"720h"}`, cfg)
errMsg := handlers.SetDatabaseTargetSettingsForTest(
&target, tc.expiry, tc.rotation,
)
// A rotation is stored as config, with or without an expiry.
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("", "daily")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.JSONEq(t, `{"rotation":"daily"}`, cfg)
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest(
"720h", "hourly",
)
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.JSONEq(t, `{"expiry":"720h","rotation":"hourly"}`, cfg)
assert.Empty(t, errMsg, tc)
assert.Equal(t, tc.wantExpiry, target.ArchiveExpiry, tc)
assert.Equal(t, tc.wantRotation, target.ArchiveRotation, tc)
}
}
func TestBuildDatabaseTargetConfig_RejectsBadExpiry(
// TestSetDatabaseTargetSettings_RejectsBadExpiry pins each refusal's
// whole message, as the form shows it.
func TestSetDatabaseTargetSettings_RejectsBadExpiry(
t *testing.T,
) {
t.Parallel()
for _, bad := range []string{"nonsense", "7d", "-5h"} {
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest(bad, "")
cases := map[string]string{
"nonsense": `Invalid archive expiry: expiry must be "never" or ` +
`a Go duration such as "720h": time: invalid duration ` +
`"nonsense"`,
"7d": `Invalid archive expiry: expiry must be "never" or a Go ` +
`duration such as "720h": time: unknown unit "d" in ` +
`duration "7d"`,
"-5h": `Invalid archive expiry: expiry must be a positive ` +
`duration or "never": "-5h"`,
}
require.NoError(t, err)
assert.Contains(
t, errMsg, "Invalid archive expiry",
"expiry %q should be refused", bad,
for bad, want := range cases {
var target database.Target
errMsg := handlers.SetDatabaseTargetSettingsForTest(
&target, bad, "",
)
assert.Empty(t, cfg)
assert.Equal(t, want, errMsg, "expiry %q", bad)
}
}
func TestBuildDatabaseTargetConfig_RejectsBadRotation(
func TestSetDatabaseTargetSettings_RejectsBadRotation(
t *testing.T,
) {
t.Parallel()
for _, bad := range []string{"weekly", "Daily", " none"} {
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest("", bad)
var target database.Target
require.NoError(t, err)
assert.Contains(
t, errMsg, "Invalid archive rotation",
"rotation %q should be refused", bad,
errMsg := handlers.SetDatabaseTargetSettingsForTest(
&target, "", bad,
)
assert.Equal(
t,
"Invalid archive rotation: rotation must be none, "+
"monthly, daily or hourly: "+strconv.Quote(bad),
errMsg,
)
assert.Empty(t, cfg)
}
}
// TestFormatArchiveExpiry covers the expiries the expiry choices do
// not: TestArchiveExpiryChoices shows the edit form starting on each
// choice. An expiry in whole hours is written in hours, and any other
// as Go writes it, which the form accepts back unchanged.
func TestFormatArchiveExpiry(t *testing.T) {
t.Parallel()
cases := map[time.Duration]string{
0: expiryNever,
36 * time.Hour: "36h",
90 * time.Minute: "1h30m0s",
}
for expiry, want := range cases {
assert.Equal(t, want, handlers.FormatArchiveExpiryForTest(expiry))
}
}
+23 -12
View File
@@ -7,6 +7,7 @@ import (
"net/http/httptest"
"net/url"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -54,15 +55,26 @@ func TestHandleSourceCreateSubmit_CreatesRequestedTargets(t *testing.T) {
env := setupSourceTest(t)
// Each value the archive pruning choice submits, after an empty
// one that stands for the archive checkbox left off.
expiries := []string{
"", "never", "1h", "12h", "24h", "720h", "2160h", "8760h",
// Each value the archive pruning choice submits and the expiry it
// stores, after an empty one that stands for the archive checkbox
// left off.
expiries := []struct {
value string
stored time.Duration
}{
{"", 0},
{expiryNever, 0},
{"1h", time.Hour},
{"12h", 12 * time.Hour},
{"24h", 24 * time.Hour},
{"720h", 720 * time.Hour},
{"2160h", 2160 * time.Hour},
{"8760h", 8760 * time.Hour},
}
for _, httpURL := range []string{"", editOriginalURL} {
for _, expiry := range expiries {
name := "url=" + httpURL + " archive=" + expiry
name := "url=" + httpURL + " archive=" + expiry.value
t.Run(name, func(t *testing.T) {
t.Parallel()
@@ -72,9 +84,9 @@ func TestHandleSourceCreateSubmit_CreatesRequestedTargets(t *testing.T) {
form.Set("http_url", httpURL)
form.Set("archive_expiry", "720h")
if expiry != "" {
if expiry.value != "" {
form.Set("archive", "on")
form.Set("archive_expiry", expiry)
form.Set("archive_expiry", expiry.value)
}
w := submitCreateForm(env, form)
@@ -96,17 +108,16 @@ func TestHandleSourceCreateSubmit_CreatesRequestedTargets(t *testing.T) {
wantCount++
assert.Equal(t, "HTTP", byType[database.TargetTypeHTTP].Name)
assert.JSONEq(t, `{"url":"`+httpURL+`"}`,
byType[database.TargetTypeHTTP].Config)
assert.Equal(t, httpURL, byType[database.TargetTypeHTTP].URL)
}
if expiry != "" {
if expiry.value != "" {
wantCount++
assert.Equal(t, "Archive",
byType[database.TargetTypeDatabase].Name)
assert.JSONEq(t, `{"expiry":"`+expiry+`"}`,
byType[database.TargetTypeDatabase].Config)
assert.Equal(t, expiry.stored,
byType[database.TargetTypeDatabase].ArchiveExpiry)
}
assert.Len(t, byType, wantCount)
+62 -86
View File
@@ -5,6 +5,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/go-chi/chi"
"github.com/stretchr/testify/assert"
@@ -25,25 +26,24 @@ const (
slackSecretPath
)
// seedConfiguredTarget inserts a target with a stored config
// blob and returns it.
// seedConfiguredTarget inserts an active target of targetType with
// the settings (URL to ArchiveRotation) that settings carries, and
// returns it.
func seedConfiguredTarget(
t *testing.T,
db *database.Database,
webhookID string,
targetType database.TargetType,
config string,
settings database.Target,
) *database.Target {
t.Helper()
tgt := &database.Target{
WebhookID: webhookID,
Name: "t-" + string(targetType),
Type: targetType,
Active: true,
Config: config,
MaxAttempts: 1,
}
tgt := &settings
tgt.WebhookID = webhookID
tgt.Name = "t-" + string(targetType)
tgt.Type = targetType
tgt.Active = true
tgt.MaxAttempts = 1
require.NoError(
t,
@@ -108,89 +108,59 @@ func serveSourceDetailPage(
return w
}
// TestHandleSourceDetail_MasksSlackWebhookURL is the
// load-bearing regression test for the credential leak: the
// rendered page must show the Slack target without any of the
// secret path segments of its webhook URL.
func TestHandleSourceDetail_MasksSlackWebhookURL(t *testing.T) {
// TestHandleSourceDetail_MasksTargetURL is the load-bearing
// regression test for the credential leak: the rendered page must
// show a Slack target, and an http target, without any of the secret
// path segments of its URL. An http target's destination is routinely
// an incoming-webhook endpoint too, so the same Slack URL serves for
// both.
func TestHandleSourceDetail_MasksTargetURL(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
labels := map[database.TargetType]string{
database.TargetTypeSlack: "Webhook URL",
database.TargetTypeHTTP: "Destination URL",
}
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
for targetType, label := range labels {
t.Run(string(targetType), func(t *testing.T) {
t.Parallel()
t.Cleanup(app.RequireStop)
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
wh := seedWebhook(t, db)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
body := renderSourceDetailPage(t, h, sess, wh.ID)
t.Cleanup(app.RequireStop)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.NotContains(
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
assert.NotContains(t, body, "webhookUrl")
wh := seedWebhook(t, db)
seedConfiguredTarget(
t, db, wh.ID, targetType,
database.Target{URL: slackWebhookURL},
)
assert.Contains(t, body, "Webhook URL")
assert.Contains(t, body, "https://hooks.slack.com/...")
}
body := renderSourceDetailPage(t, h, sess, wh.ID)
// TestHandleSourceDetail_MasksHTTPDestinationURL is the
// regression test for the same leak reached through the http
// target: its destination is routinely an incoming-webhook
// endpoint whose path segments are the credential, so the
// rendered page must not contain them.
func TestHandleSourceDetail_MasksHTTPDestinationURL(
t *testing.T,
) {
t.Parallel()
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.NotContains(
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
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)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeHTTP,
`{"url":"`+slackWebhookURL+`"}`,
)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.NotContains(
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
assert.Contains(t, body, "Destination URL")
assert.Contains(t, body, "https://hooks.slack.com/...")
assert.Contains(t, body, label)
assert.Contains(t, body, "https://hooks.slack.com/...")
})
}
}
// TestHandleSourceDetail_RendersNamedTargetFields proves the
// other target types render labelled fields rather than the
// stored blob.
// stored values.
func TestHandleSourceDetail_RendersNamedTargetFields(
t *testing.T,
) {
@@ -212,18 +182,24 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook","timeout":30,`+
`"headers":{"Authorization":"Bearer sekrit"}}`,
database.Target{
URL: "https://example.com/hook",
TimeoutSeconds: 30,
Headers: map[string]string{"X-Api-Key": "sekrit"},
},
)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeDatabase,
`{"expiry":"720h"}`,
database.Target{
ArchiveExpiry: 720 * time.Hour,
ArchiveRotation: rotationNone,
},
)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetType("carrier-pigeon"),
`{"beak":"sharp"}`,
database.Target{URL: "https://pigeon.example/beak"},
)
body := renderSourceDetailPage(t, h, sess, wh.ID)
@@ -242,8 +218,8 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
assert.Contains(t, list, "Archive rotation: none")
assert.Contains(t, body, `<option value="database">Archive</option>`)
// An unknown type gets the neutral placeholder, never the
// stored blob.
// An unknown type gets the neutral placeholder, never its
// stored URL.
assert.Contains(t, body, "(unavailable)")
assert.NotContains(t, body, "beak")
}
@@ -161,7 +161,7 @@ func TestHandleSourceLogs_MasksDeletedTargetConfig(t *testing.T) {
tgt := seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
database.Target{URL: slackWebhookURL},
)
seedDeliveredEvent(t, dbMgr, wh.ID, tgt.ID)
@@ -175,7 +175,6 @@ func TestHandleSourceLogs_MasksDeletedTargetConfig(t *testing.T) {
assert.NotContains(
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
assert.NotContains(t, body, "webhookUrl")
// The name is there; only the credential is not.
assert.Contains(t, body, tgt.Name+deletedMarker)
+1 -2
View File
@@ -138,7 +138,7 @@ func TestHandleSourceLogs_MasksSlackWebhookURL(t *testing.T) {
tgt := seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
database.Target{URL: slackWebhookURL},
)
seedDeliveredEvent(t, dbMgr, wh.ID, tgt.ID)
@@ -151,7 +151,6 @@ func TestHandleSourceLogs_MasksSlackWebhookURL(t *testing.T) {
assert.NotContains(
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
assert.NotContains(t, body, "webhookUrl")
// The page still identifies the delivery's target.
assert.Contains(t, body, tgt.Name)
+72 -107
View File
@@ -2,7 +2,6 @@ package handlers
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
@@ -65,20 +64,14 @@ func (h *Handlers) processTargetCreate(
) {
in := targetFormInputFrom(r)
target, errMsg, err := h.newTarget(r.Context(), webhook.ID, in)
if err != nil {
h.serverError(w, r, "failed to encode target config", err)
return
}
target, errMsg := h.newTarget(r.Context(), webhook.ID, in)
if errMsg != "" {
h.renderSourceDetail(w, r, webhook, in, errMsg)
return
}
err = h.db.DB().Create(target).Error
err := h.db.DB().Create(target).Error
if err != nil {
h.serverError(w, r, "failed to create target", err)
@@ -93,15 +86,13 @@ func (h *Handlers) processTargetCreate(
// newTarget validates a new target for a webhook and returns the row
// to create, or, when it refuses the target, the message the form
// shows. An error is the server's fault, not a refusal: the accepted
// configuration could not be encoded. Every form that creates a
// target goes through here, so they all accept and refuse the same
// things.
// shows. Every form that creates a target goes through here, so they
// all accept and refuse the same things.
func (h *Handlers) newTarget(
ctx context.Context,
webhookID string,
in targetFormInput,
) (*database.Target, string, error) {
) (*database.Target, string) {
target := &database.Target{
WebhookID: webhookID,
Type: in.Type,
@@ -109,34 +100,33 @@ func (h *Handlers) newTarget(
MaxAttempts: 1,
}
errMsg, err := h.setTargetFromForm(ctx, target, in)
if err != nil || errMsg != "" {
return nil, errMsg, err
errMsg := h.setTargetFromForm(ctx, target, in)
if errMsg != "" {
return nil, errMsg
}
return target, "", nil
return target, ""
}
// setTargetFromForm validates a target form against the target's type
// and, when it accepts it, sets the target's name, configuration and
// number of delivery attempts from it. It returns the message the form
// shows for anything it refuses, an unknown type among them, and then
// leaves the target unchanged; an error is the server's fault, as for
// newTarget.
// and sets the target's name, settings and number of delivery attempts
// from it. It returns the message the form shows for anything it
// refuses, an unknown type among them; the caller then discards the
// target, which may be partly set.
// The add target form and the target edit form both go through here,
// so the two cannot come to disagree about what a target may be.
func (h *Handlers) setTargetFromForm(
ctx context.Context,
target *database.Target,
in targetFormInput,
) (string, error) {
) string {
if in.Name == "" {
return "Name is required", nil
return "Name is required"
}
configJSON, errMsg, err := h.buildTargetConfig(ctx, target.Type, in)
if err != nil || errMsg != "" {
return errMsg, err
errMsg := h.setTargetSettings(ctx, target, in)
if errMsg != "" {
return errMsg
}
// An empty max_attempts keeps the target's count: the default of
@@ -147,14 +137,13 @@ func (h *Handlers) setTargetFromForm(
// target is delivering with.
maxAttempts, err := parseMaxAttempts(in.MaxAttempts, target.MaxAttempts)
if err != nil {
return "Invalid delivery attempts: " + attemptsErrorMessage(err), nil
return "Invalid delivery attempts: " + attemptsErrorMessage(err)
}
target.Name = in.Name
target.Config = configJSON
target.MaxAttempts = maxAttempts
return "", nil
return ""
}
// targetFormInput carries the raw values of a target form. Both the
@@ -213,84 +202,81 @@ func targetFormInputFrom(r *http.Request) targetFormInput {
}
}
// buildTargetConfig builds the JSON config string for a target from
// the submitted form values, or returns the message the form shows
// for a value it refuses. An error is the server's fault, not a
// refusal: the accepted configuration could not be encoded. Which
// fields of in apply depends on the target type; a type without a URL
// ignores any URL submitted.
func (h *Handlers) buildTargetConfig(
// setTargetSettings sets a target's settings from the submitted form
// values, or returns the message the form shows for a value it
// refuses. Which fields of in apply depends on the target type; a
// type without a URL ignores any URL submitted, and the settings a
// type does not use are left empty.
func (h *Handlers) setTargetSettings(
ctx context.Context,
targetType database.TargetType,
target *database.Target,
in targetFormInput,
) (string, string, error) {
switch targetType {
) string {
switch target.Type {
case database.TargetTypeHTTP:
return h.buildHTTPTargetConfig(ctx, in)
return h.setHTTPTargetSettings(ctx, target, in)
case database.TargetTypeSlack:
return h.buildSlackTargetConfig(ctx, in.URL)
return h.setSlackTargetSettings(ctx, target, in.URL)
case database.TargetTypeDatabase:
return buildDatabaseTargetConfig(in.Expiry, in.Rotation)
return setDatabaseTargetSettings(target, in.Expiry, in.Rotation)
case database.TargetTypeLog:
return "", "", nil
return ""
default:
return "", "Invalid target type", nil
return "Invalid target type"
}
}
// buildHTTPTargetConfig builds config JSON for an HTTP target: an
// SSRF-validated destination plus the optional headers, timeout and
// query string setting the delivery path honours.
func (h *Handlers) buildHTTPTargetConfig(
// setHTTPTargetSettings sets an HTTP target's SSRF-validated
// destination plus the optional headers, timeout and query string
// setting the delivery path honours.
func (h *Handlers) setHTTPTargetSettings(
ctx context.Context,
target *database.Target,
in targetFormInput,
) (string, string, error) {
) string {
errMsg := h.validateTargetURL(
ctx, in.URL, "URL is required for HTTP targets",
)
if errMsg != "" {
return "", errMsg, nil
return errMsg
}
headers, err := delivery.ParseTargetHeaders(in.Headers)
if err != nil {
return "", fmt.Sprintf("Invalid headers: %v", err), nil
return fmt.Sprintf("Invalid headers: %v", err)
}
timeout, err := delivery.ParseTargetTimeout(in.Timeout)
if err != nil {
return "", fmt.Sprintf("Invalid timeout: %v", err), nil
return fmt.Sprintf("Invalid timeout: %v", err)
}
configJSON, err := marshalTargetConfig(delivery.HTTPTargetConfig{
URL: in.URL,
Headers: headers,
Timeout: timeout,
ForwardQuery: in.ForwardQuery,
})
target.URL = in.URL
target.Headers = headers
target.TimeoutSeconds = timeout
target.ForwardQuery = in.ForwardQuery
return configJSON, "", err
return ""
}
// buildSlackTargetConfig builds config JSON for a Slack target,
// whose whole configuration is one SSRF-validated webhook URL.
func (h *Handlers) buildSlackTargetConfig(
// setSlackTargetSettings sets a Slack target's one setting, its
// SSRF-validated webhook URL.
func (h *Handlers) setSlackTargetSettings(
ctx context.Context,
target *database.Target,
targetURL string,
) (string, string, error) {
) string {
errMsg := h.validateTargetURL(
ctx, targetURL,
"Webhook URL is required for Slack targets",
)
if errMsg != "" {
return "", errMsg, nil
return errMsg
}
configJSON, err := marshalTargetConfig(delivery.SlackTargetConfig{
WebhookURL: targetURL,
})
target.URL = targetURL
return configJSON, "", err
return ""
}
// validateTargetURL refuses an empty or SSRF-blocked destination,
@@ -341,52 +327,31 @@ func (h *Handlers) validateTargetURL(
return ""
}
// marshalTargetConfig serialises a target configuration for storage.
func marshalTargetConfig(cfg any) (string, error) {
configBytes, err := json.Marshal(cfg)
if err != nil {
return "", err
}
return string(configBytes), nil
}
// buildDatabaseTargetConfig builds config JSON for a database
// (archive) target. The optional expiry and rotation are validated
// here, at creation time, so a bad value is refused instead of
// failing every subsequent delivery. Each is stored only when set,
// and with neither the config is empty (the keep-forever, one-file
// default).
func buildDatabaseTargetConfig(
// setDatabaseTargetSettings sets a database (archive) target's
// expiry and rotation. Both are optional: an empty or never expiry
// keeps the archive forever, and an empty rotation is stored as none,
// one file. A bad value is refused here, at the form, instead of
// being stored.
func setDatabaseTargetSettings(
target *database.Target,
expiry, rotation string,
) (string, string, error) {
expiry = strings.TrimSpace(expiry)
err := archive.ValidateExpiry(expiry)
) string {
archiveExpiry, err := archive.ParseExpiry(strings.TrimSpace(expiry))
if err != nil {
return "", fmt.Sprintf("Invalid archive expiry: %v", err), nil
return fmt.Sprintf("Invalid archive expiry: %v", err)
}
err = archive.ValidateRotation(rotation)
if err != nil {
return "", fmt.Sprintf("Invalid archive rotation: %v", err), nil
return fmt.Sprintf("Invalid archive rotation: %v", err)
}
cfg := map[string]any{}
if expiry != "" {
cfg["expiry"] = expiry
if rotation == "" {
rotation = archiveRotationNone
}
if rotation != "" {
cfg["rotation"] = rotation
}
target.ArchiveExpiry = archiveExpiry
target.ArchiveRotation = rotation
if len(cfg) == 0 {
return "", "", nil
}
configJSON, err := marshalTargetConfig(cfg)
return configJSON, "", err
return ""
}
@@ -176,7 +176,7 @@ func TestHandleTargetCreate_BodyURLStillCreatesTheTarget(t *testing.T) {
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.Contains(t, targets[0].Config, targetSecretSegments)
assert.Equal(t, targetSecretURL, targets[0].URL)
// The body carried the credential, so the access log must still
// not have it: the log records the request line only.
+74 -33
View File
@@ -6,73 +6,114 @@ import (
"net/url"
"strings"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
)
// TestHandleTargetCreate_EveryType adds a target of each type. Each
// submission carries a url: only the http and slack types store one.
// TestHandleTargetCreate_EveryType adds a target of each type and
// checks the settings it stores. Each submission carries a url and an
// expiry: only the http and slack types store the url, only the
// database type the expiry, and every setting a type does not use
// stays empty.
func TestHandleTargetCreate_EveryType(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
// fields is the rest of each submission, as a query string.
// fields is the rest of each submission, as a query string, and
// want holds the settings stored.
cases := []struct {
targetType database.TargetType
fields string
wantConfig string
want database.Target
wantAttempts int
}{
{
database.TargetTypeHTTP, "timeout=12&max_attempts=3",
`{"url":"` + editOriginalURL + `","timeout":12}`, 3,
database.TargetTypeHTTP,
"timeout=12&max_attempts=3&forward_query=on",
database.Target{
URL: editOriginalURL,
Headers: map[string]string{},
TimeoutSeconds: 12,
ForwardQuery: true,
}, 3,
},
{
database.TargetTypeSlack, "max_attempts=4",
`{"webhookUrl":"` + editOriginalURL + `"}`, 4,
database.Target{URL: editOriginalURL}, 4,
},
{
database.TargetTypeDatabase, "expiry=720h",
`{"expiry":"720h"}`, 1,
database.TargetTypeDatabase, "rotation=daily",
database.Target{
ArchiveExpiry: 720 * time.Hour,
ArchiveRotation: rotationDaily,
}, 1,
},
{database.TargetTypeLog, "", "", 1},
{database.TargetTypeLog, "", database.Target{}, 1},
}
for _, tc := range cases {
t.Run(string(tc.targetType), func(t *testing.T) {
t.Parallel()
webhook := seedWebhookWithRetention(t, env.db, 30)
form, err := url.ParseQuery(tc.fields)
require.NoError(t, err)
form.Set("name", "every-type")
form.Set("type", string(tc.targetType))
form.Set("url", editOriginalURL)
w := serveTarget(
env, http.MethodPost,
"/hook/"+webhook.ID+"/targets", form,
_, got := addTargetThroughForm(
t, env, tc.targetType,
"url="+editOriginalURL+"&expiry=720h&"+tc.fields,
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.Equal(t, tc.targetType, targets[0].Type)
assert.Equal(t, tc.wantAttempts, targets[0].MaxAttempts)
if tc.wantConfig == "" {
assert.Empty(t, targets[0].Config)
} else {
assert.JSONEq(t, tc.wantConfig, targets[0].Config)
}
assert.Equal(t, tc.targetType, got.Type)
assert.Equal(t, tc.wantAttempts, got.MaxAttempts)
assert.Equal(t, tc.want, targetSettingsOf(got))
})
}
}
// addTargetThroughForm adds a target of targetType named every-type to
// a new webhook through the add target form, with the rest of the form
// given as a query string, and returns the webhook and the target the
// form stored.
func addTargetThroughForm(
t *testing.T,
env *sourceTestEnv,
targetType database.TargetType,
fields string,
) (database.Webhook, database.Target) {
t.Helper()
webhook := seedWebhookWithRetention(t, env.db, 30)
form, err := url.ParseQuery(fields)
require.NoError(t, err)
form.Set("name", "every-type")
form.Set("type", string(targetType))
w := serveTarget(
env, http.MethodPost, "/hook/"+webhook.ID+"/targets", form,
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
return webhook, targets[0]
}
// targetSettingsOf returns a target's settings, URL to
// ArchiveRotation, and nothing else, for comparing with the settings a
// test expects.
func targetSettingsOf(target database.Target) database.Target {
return database.Target{
URL: target.URL,
Headers: target.Headers,
TimeoutSeconds: target.TimeoutSeconds,
ForwardQuery: target.ForwardQuery,
ArchiveExpiry: target.ArchiveExpiry,
ArchiveRotation: target.ArchiveRotation,
}
}
// TestHandleTargetCreate_RefusedFormComesBack refuses a target of each
// type and checks that the webhook page comes back with the add target
// form open on that type, the values entered, and the reason.
+27 -52
View File
@@ -25,22 +25,15 @@ const (
tmplKeyMaxTimeout = "MaxTimeout"
)
// configUnreadableMessage is shown when a target's stored
// configuration does not parse. It says plainly that saving replaces
// the stored value rather than preserving it, because the form
// cannot pre-fill what it could not read.
const configUnreadableMessage = "The stored configuration for this " +
"target could not be read. Enter the values below; saving " +
"replaces the stored configuration."
// targetEditView is the display model for the target edit page: the
// target's row fields as stored. The values the form shows, the
// UNMASKED configuration among them, come separately, as a
// UNMASKED URL and headers among them, come separately, as a
// targetFormInput.
//
// It deliberately omits database.Target's raw Config blob: the form
// renders named fields, and giving the template the blob as well
// would put an unreviewed second path to the credential on the page.
// It deliberately omits database.Target's settings: the form renders
// them from the targetFormInput, and giving the template the target's
// URL and headers as well would put an unreviewed second path to the
// credential on the page.
type targetEditView struct {
ID string
Name string
@@ -48,13 +41,16 @@ type targetEditView struct {
Active bool
}
// HandleTargetEdit shows the form to edit a target.
// HandleTargetEdit shows the form to edit a target, filled with the
// target's settings as stored.
//
// This page is the one place the full destination URL and header
// values are shown. It is reachable only through the
// /hook/{sourceID} route group, which supplies RequireAuth and
// NoCache, and only for a target of a webhook the session's user
// owns; masking (delivery.TargetView) is unchanged everywhere else.
// values are shown: an operator cannot correct a value they cannot
// see. It is reachable only through the /hook/{sourceID} route
// group, which supplies RequireAuth and NoCache, and only for a
// target of a webhook the session's user owns; masking
// (delivery.TargetView) is unchanged everywhere else. Do not fill a
// form like this one on any other page.
func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
webhook, target, ok := h.ownedTarget(w, r)
@@ -62,34 +58,19 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
return
}
cfg, err := delivery.NewTargetConfigForm(target)
msg := ""
if err != nil {
// The error carries the parse failure, never the
// blob, so it is safe to log against the target id.
h.log.Warn(
"stored target config could not be read for editing",
"target_id", target.ID,
"error", err,
)
msg = configUnreadableMessage
}
form := targetFormInput{
Name: target.Name,
URL: cfg.URL,
Headers: cfg.Headers,
Timeout: cfg.Timeout,
ForwardQuery: cfg.ForwardQuery,
URL: target.URL,
Headers: delivery.FormatTargetHeaders(target.Headers),
Timeout: delivery.FormatTargetTimeout(target.TimeoutSeconds),
ForwardQuery: target.ForwardQuery,
MaxAttempts: strconv.Itoa(target.MaxAttempts),
Expiry: cfg.Expiry,
Rotation: cfg.Rotation,
Expiry: formatArchiveExpiry(target.ArchiveExpiry),
Rotation: target.ArchiveRotation,
}
h.renderTargetEdit(
w, r, webhook, target, form, msg, http.StatusOK,
w, r, webhook, target, form, "", http.StatusOK,
)
}
}
@@ -125,11 +106,11 @@ func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
// does, so an edited destination is SSRF-validated exactly as a new
// one is.
//
// The target's type is not editable. Each type stores a different
// configuration shape and its delivery history is recorded against
// the target row, so changing the type of an existing target is
// really the creation of a different one. The stored type decides
// which fields the form offers and which builder runs.
// The target's type is not editable. Each type uses different
// settings and its delivery history is recorded against the target
// row, so changing the type of an existing target is really the
// creation of a different one. The stored type decides which fields
// the form offers and which settings are set.
func (h *Handlers) applyTargetEdit(
w http.ResponseWriter,
r *http.Request,
@@ -142,13 +123,7 @@ func (h *Handlers) applyTargetEdit(
// as stored, for the page shown again when the save is refused.
edited := *target
errMsg, err := h.setTargetFromForm(r.Context(), &edited, in)
if err != nil {
h.serverError(w, r, "failed to encode target config", err)
return
}
errMsg := h.setTargetFromForm(r.Context(), &edited, in)
if errMsg != "" {
h.renderTargetEdit(
w, r, webhook, target, in, errMsg, http.StatusBadRequest,
@@ -160,7 +135,7 @@ func (h *Handlers) applyTargetEdit(
// A new name renames the archive file before it is saved (see
// archive.Writers.Rename). If either step fails, it goes back to
// the name that is still stored.
err = h.renameTargetArchive(
err := h.renameTargetArchive(
target, webhook.Name, target.Name, edited.Name,
)
if err == nil {
+116 -64
View File
@@ -2,19 +2,18 @@ package handlers_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"
"github.com/go-chi/chi"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// The destinations the target edit tests configure. Both are literal
@@ -150,27 +149,6 @@ func storedTarget(
return target
}
// storedHTTPConfig reloads a target and parses its stored HTTP
// configuration.
func storedHTTPConfig(
t *testing.T,
env *sourceTestEnv,
targetID string,
) delivery.HTTPTargetConfig {
t.Helper()
var cfg delivery.HTTPTargetConfig
require.NoError(
t,
json.Unmarshal(
[]byte(storedTarget(t, env, targetID).Config), &cfg,
),
)
return cfg
}
// editForm is the fully populated edit submission for an HTTP
// target.
func editForm(targetURL, headers, timeout string) url.Values {
@@ -197,12 +175,9 @@ func submitTargetEdit(
)
}
// TestHandleTargetCreate_Configuration covers the half of the gap
// that is not about editing at all: HTTPTargetConfig has carried
// Headers and Timeout, and the delivery path has honoured them, but
// the create form wrote {"url":...} and nothing else, so a
// destination needing an Authorization header could not be
// configured through the UI at all.
// TestHandleTargetCreate_Configuration proves the add target form
// stores an HTTP target's headers and timeout, and leaves them empty
// when the form leaves them empty.
func TestHandleTargetCreate_Configuration(t *testing.T) {
t.Parallel()
@@ -213,9 +188,9 @@ func TestHandleTargetCreate_Configuration(t *testing.T) {
assertCreateStoresHeadersAndTimeout(t, env)
})
t.Run("without them keeps a url-only config", func(t *testing.T) {
t.Run("without them stores only the url", func(t *testing.T) {
t.Parallel()
assertCreateKeepsURLOnlyConfig(t, env)
assertCreateStoresOnlyTheURL(t, env)
})
}
@@ -228,33 +203,31 @@ func assertCreateStoresHeadersAndTimeout(
t, env, editAuthHeader+"\nX-Tenant: acme\n", "12",
)
cfg := storedHTTPConfig(t, env, target.ID)
stored := storedTarget(t, env, target.ID)
assert.Equal(t, editOriginalURL, cfg.URL)
assert.Equal(t, 12, cfg.Timeout)
assert.Equal(t, editOriginalURL, stored.URL)
assert.Equal(t, 12, stored.TimeoutSeconds)
assert.Equal(
t,
map[string]string{
"Authorization": "Bearer " + editBearerSecret,
"X-Tenant": "acme",
},
cfg.Headers,
stored.Headers,
)
}
// Without the new fields the stored shape must be the same
// {"url":...} the create form wrote before they existed, so no
// existing target's configuration is rewritten by this change.
func assertCreateKeepsURLOnlyConfig(
func assertCreateStoresOnlyTheURL(
t *testing.T, env *sourceTestEnv,
) {
t.Helper()
_, target := seedHTTPTarget(t, env, "", "")
assert.JSONEq(
t, `{"url":"`+editOriginalURL+`"}`, target.Config,
)
assert.Equal(t, editOriginalURL, target.URL)
assert.Empty(t, target.Headers)
assert.Zero(t, target.TimeoutSeconds)
assert.False(t, target.ForwardQuery)
}
// TestHandleTargetEditSubmit_Saves is the round trip the issue asks
@@ -299,13 +272,8 @@ func assertEditChangesDestination(
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.Equal(
t,
editReplacedURL,
storedHTTPConfig(t, env, target.ID).URL,
)
reloaded := storedTarget(t, env, target.ID)
assert.Equal(t, editReplacedURL, reloaded.URL)
assert.Equal(t, "edited-name", reloaded.Name)
assert.Equal(t, 5, reloaded.MaxAttempts)
assert.Equal(
@@ -333,16 +301,16 @@ func assertEditRoundTripsHeadersAndTimeout(
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
cfg := storedHTTPConfig(t, env, target.ID)
stored := storedTarget(t, env, target.ID)
assert.Equal(t, 21, cfg.Timeout)
assert.Equal(t, 21, stored.TimeoutSeconds)
assert.Equal(
t,
map[string]string{
"Authorization": "Bearer rotated",
"X-Trace": "on",
},
cfg.Headers,
stored.Headers,
)
}
@@ -362,10 +330,10 @@ func assertEditClearingRemovesThem(
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
cfg := storedHTTPConfig(t, env, target.ID)
stored := storedTarget(t, env, target.ID)
assert.Empty(t, cfg.Headers)
assert.Zero(t, cfg.Timeout)
assert.Empty(t, stored.Headers)
assert.Zero(t, stored.TimeoutSeconds)
}
// Delivery attempts are offered only by the forms for target types
@@ -442,6 +410,90 @@ func TestHandleTargetEdit_CallsTheDatabaseTypeArchive(t *testing.T) {
assert.Contains(t, page, `class="label">Archive rotation</label>`)
}
// TestHandleTargetEdit_EveryType edits a target of each type: the edit
// form starts on the settings the add target form stored, and saving
// it stores the settings submitted.
func TestHandleTargetEdit_EveryType(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
// created and edited are the add target form's and the edit
// form's fields, as query strings; shown is what the edit form
// must show of created; want is the settings edited stores.
cases := []struct {
targetType database.TargetType
created, edited string
shown []string
want database.Target
}{
{
database.TargetTypeHTTP,
"url=" + editOriginalURL +
"&headers=X-Tenant:+acme&timeout=7&forward_query=on",
"url=" + editReplacedURL + "&timeout=9",
[]string{
`value="` + editOriginalURL + `"`,
"X-Tenant: acme",
`value="7"`,
`name="forward_query" value="on" checked`,
},
database.Target{
URL: editReplacedURL,
Headers: map[string]string{},
TimeoutSeconds: 9,
},
},
{
database.TargetTypeSlack,
"url=" + editOriginalURL,
"url=" + editReplacedURL,
[]string{`value="` + editOriginalURL + `"`},
database.Target{URL: editReplacedURL},
},
{
database.TargetTypeDatabase,
"expiry=720h&rotation=daily",
"expiry=24h&rotation=hourly",
[]string{
`<option value="720h" selected>`,
`<option value="daily" selected>`,
},
database.Target{
ArchiveExpiry: 24 * time.Hour,
ArchiveRotation: rotationHourly,
},
},
{database.TargetTypeLog, "", "", nil, database.Target{}},
}
for _, tc := range cases {
t.Run(string(tc.targetType), func(t *testing.T) {
t.Parallel()
webhook, target := addTargetThroughForm(
t, env, tc.targetType, tc.created,
)
page := targetEditPage(t, env, webhook.ID, target.ID)
for _, shown := range tc.shown {
assert.Contains(t, page, shown)
}
form, err := url.ParseQuery(tc.edited)
require.NoError(t, err)
form.Set("name", "edited")
w := submitTargetEdit(env, webhook.ID, target.ID, form)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
got := storedTarget(t, env, target.ID)
assert.Equal(t, "edited", got.Name)
assert.Equal(t, tc.want, targetSettingsOf(got))
})
}
}
// TestHandleTarget_ForwardQuery covers the HTTP target's setting that
// passes each event's query string on to it: the add target form
// stores it checked, the edit form starts with it checked and turns it
@@ -467,7 +519,7 @@ func TestHandleTarget_ForwardQuery(t *testing.T) {
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.True(t, storedHTTPConfig(t, env, targets[0].ID).ForwardQuery)
assert.True(t, storedTarget(t, env, targets[0].ID).ForwardQuery)
w = serveTarget(
env, http.MethodGet, targetsPath+"/"+targets[0].ID+"/edit", nil,
@@ -479,7 +531,7 @@ func TestHandleTarget_ForwardQuery(t *testing.T) {
w = submitTargetEdit(env, webhook.ID, targets[0].ID, edit)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.False(t, storedHTTPConfig(t, env, targets[0].ID).ForwardQuery)
assert.False(t, storedTarget(t, env, targets[0].ID).ForwardQuery)
edit.Set("url", editBlockedURL)
edit.Set("forward_query", "on")
@@ -568,7 +620,7 @@ func assertEditRejectsBlockedDestination(
assert.Contains(t, w.Body.String(), "Invalid target URL")
assert.Equal(
t, editOriginalURL,
storedHTTPConfig(t, env, target.ID).URL,
storedTarget(t, env, target.ID).URL,
"a rejected edit must leave the stored config alone",
)
}
@@ -597,9 +649,9 @@ func assertEditIgnoresQueryString(
assert.Equal(t, http.StatusBadRequest, w.Code)
cfg := storedHTTPConfig(t, env, target.ID)
assert.Equal(t, editOriginalURL, cfg.URL)
assert.Empty(t, cfg.Headers)
stored := storedTarget(t, env, target.ID)
assert.Equal(t, editOriginalURL, stored.URL)
assert.Empty(t, stored.Headers)
}
func assertEditRejectsHeaders(
@@ -617,7 +669,7 @@ func assertEditRejectsHeaders(
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(t, w.Body.String(), "Invalid headers")
assert.Empty(
t, storedHTTPConfig(t, env, target.ID).Headers,
t, storedTarget(t, env, target.ID).Headers,
"a rejected header must not be stored",
)
}
@@ -637,7 +689,7 @@ func assertEditRejectsTimeout(
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(t, w.Body.String(), "Invalid timeout")
assert.Equal(
t, 9, storedHTTPConfig(t, env, target.ID).Timeout,
t, 9, storedTarget(t, env, target.ID).TimeoutSeconds,
"a rejected timeout must leave the stored one alone",
)
}
@@ -757,7 +809,7 @@ func assertTargetOfAnotherWebhook404s(
assert.Equal(
t, editOriginalURL,
storedHTTPConfig(t, env, target.ID).URL,
storedTarget(t, env, target.ID).URL,
)
}
@@ -778,7 +830,7 @@ func assertWebhookOfAnotherUser404s(
target := seedConfiguredTarget(
t, env.db, other.ID, database.TargetTypeHTTP,
`{"url":"`+editOriginalURL+`"}`,
database.Target{URL: editOriginalURL},
)
w := serveTarget(
+1 -4
View File
@@ -243,10 +243,7 @@ func readTargetDeliveries(
func (h *Handlers) archiveFileView(
webhook *database.Webhook, target *database.Target, now time.Time,
) *ArchiveFileView {
current, err := archive.PathAt(h.dbMgr, webhook, target, now)
if err != nil {
return h.archiveUnreadable(&ArchiveFileView{}, target, err)
}
current := archive.PathAt(h.dbMgr, webhook, target, now)
view := &ArchiveFileView{Name: filepath.Base(current)}
+1 -3
View File
@@ -60,7 +60,5 @@ func TestHandleTargetToggle_DoesNotUndoAnEdit(t *testing.T) {
assert.False(t, stored.Active)
assert.Equal(t, "edited-name", stored.Name)
assert.Equal(t, 5, stored.MaxAttempts)
assert.Equal(
t, editReplacedURL, storedHTTPConfig(t, env, tgt.ID).URL,
)
assert.Equal(t, editReplacedURL, stored.URL)
}
+20 -259
View File
@@ -2,14 +2,11 @@ package handlers
import (
"encoding/json"
"fmt"
"io"
"net/http"
"github.com/go-chi/chi"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/middleware"
)
@@ -70,8 +67,9 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
}
}
// processWebhookRequest reads the body, serializes headers, loads
// targets, and delivers the event.
// processWebhookRequest reads the request into an event and has the
// delivery engine store it, with a delivery for each of the webhook's
// active targets, and answers the sender.
//
// Nothing about the request itself is authenticated: the entrypoint
// UUID in the path is the credential, and reaching here means it
@@ -93,30 +91,24 @@ func (h *Handlers) processWebhookRequest(
return
}
targets, err := h.loadActiveTargets(entrypoint.WebhookID)
event := &database.Event{
WebhookID: entrypoint.WebhookID,
EntrypointID: entrypoint.ID,
Method: r.Method,
RawQuery: r.URL.RawQuery,
Headers: string(headersJSON),
ContentType: r.Header.Get("Content-Type"),
Body: string(body),
}
deliveries, err := h.engine.StoreEvent(event)
if err != nil {
h.receiverError(w, "failed to query targets", err)
h.receiverError(w, "failed to store webhook event", err)
return
}
h.createAndDeliverEvent(
w, r, entrypoint, body, headersJSON, targets,
)
}
// loadActiveTargets returns all active targets for a webhook.
func (h *Handlers) loadActiveTargets(
webhookID string,
) ([]database.Target, error) {
var targets []database.Target
err := h.db.DB().Where(
"webhook_id = ? AND active = ?",
webhookID, true,
).Find(&targets).Error
return targets, err
h.finishWebhookResponse(w, event, entrypoint, deliveries)
}
// lookupEntrypoint finds an entrypoint by UUID path.
@@ -177,28 +169,6 @@ func (h *Handlers) readWebhookBody(
return body, true
}
// createAndDeliverEvent stores the received event, fans it out to the
// webhook's targets, and answers the sender.
func (h *Handlers) createAndDeliverEvent(
w http.ResponseWriter,
r *http.Request,
entrypoint database.Entrypoint,
body, headersJSON []byte,
targets []database.Target,
) {
event, tasks, err := h.createAndFanOut(
requestEventSource(r, entrypoint, headersJSON, body),
targets,
)
if err != nil {
h.receiverError(w, "failed to store webhook event", err)
return
}
h.finishWebhookResponse(w, event, entrypoint, tasks)
}
// receiverError logs an error and answers the sender with a plain-text
// 500. The receiver's answers are for programs, so it never sends the
// error page the web UI uses.
@@ -212,172 +182,19 @@ func (h *Handlers) receiverError(
)
}
// eventSource carries the fields a new event is built from. The
// receiver fills it from the live request; the resubmit handler fills
// it from a stored event. Both then go through createAndFanOut, so an
// event is constructed and fanned out in one place however it entered
// the system.
type eventSource struct {
WebhookID string
EntrypointID string
Method string
RawQuery string
HeadersJSON string
ContentType string
Body []byte
// ResubmittedFromID names the event this one copies. Only the
// resubmit path sets it.
ResubmittedFromID *string
}
// event builds the row this source stores.
func (s eventSource) event() *database.Event {
return &database.Event{
WebhookID: s.WebhookID,
EntrypointID: s.EntrypointID,
Method: s.Method,
RawQuery: s.RawQuery,
Headers: s.HeadersJSON,
Body: string(s.Body),
BodyBytes: int64(len(s.Body)),
ContentType: s.ContentType,
ResubmittedFromID: s.ResubmittedFromID,
}
}
// requestEventSource describes the event a live receiver request
// stores.
func requestEventSource(
r *http.Request,
entrypoint database.Entrypoint,
headersJSON, body []byte,
) eventSource {
return eventSource{
WebhookID: entrypoint.WebhookID,
EntrypointID: entrypoint.ID,
Method: r.Method,
RawQuery: r.URL.RawQuery,
HeadersJSON: string(headersJSON),
ContentType: r.Header.Get("Content-Type"),
Body: body,
}
}
// createAndFanOut writes the event and one pending delivery per target,
// and adds them to the webhook's running totals, in a single
// transaction, then hands the tasks to the delivery engine. Every
// event is created here, received or resubmitted, so a resubmitted
// event is retried, SSRF-guarded and circuit-broken exactly as a
// received one is. Per-delivery replay is the one other path that
// creates a delivery: it adds one to an existing event without
// coming through here.
//
// The tasks are returned as well as queued, so a caller can report how
// many targets the event went to.
func (h *Handlers) createAndFanOut(
src eventSource,
targets []database.Target,
) (*database.Event, []delivery.Task, error) {
webhookDB, err := h.dbMgr.GetDB(src.WebhookID)
if err != nil {
return nil, nil, fmt.Errorf(
"getting webhook database: %w", err,
)
}
tx := webhookDB.Begin()
if tx.Error != nil {
return nil, nil, fmt.Errorf(
"beginning transaction: %w", tx.Error,
)
}
event := src.event()
err = tx.Create(event).Error
if err != nil {
tx.Rollback()
return nil, nil, fmt.Errorf("creating event: %w", err)
}
tasks, err := buildDeliveryTasks(
tx, event, targets, inlineBody(src.Body),
)
if err != nil {
tx.Rollback()
return nil, nil, err
}
err = database.AddEventTotals(tx, database.EventTotals{
Events: 1, LastEventAt: &event.CreatedAt,
})
if err != nil {
tx.Rollback()
return nil, nil, err
}
// A resubmitted copy did not arrive on its entrypoint's URL, so it
// leaves the entrypoint's last event as it is.
if src.ResubmittedFromID == nil {
err = database.AddEntrypointTotals(tx, database.EntrypointTotals{
EntrypointID: event.EntrypointID, LastEventAt: event.CreatedAt,
})
if err != nil {
tx.Rollback()
return nil, nil, err
}
}
err = tx.Commit().Error
if err != nil {
return nil, nil, fmt.Errorf(
"committing transaction: %w", err,
)
}
// Counted here, after the commit: an event exists once it is
// durably stored, which is what the delivery counters are
// compared against on a dashboard. A resubmitted event counts
// too, because it produces deliveries that the delivery side
// counts; the resubmit counter is what separates the two.
h.mtr.EventReceived()
if len(tasks) > 0 {
h.notifier.Notify(tasks)
}
return event, tasks, nil
}
// inlineBody returns a pointer to body as a string if it fits
// within the inline size limit, or nil otherwise.
func inlineBody(body []byte) *string {
if len(body) < delivery.MaxInlineBodySize {
s := string(body)
return &s
}
return nil
}
// finishWebhookResponse logs the event and writes the HTTP response.
// finishWebhookResponse logs the stored event, with how many
// deliveries it has, and writes the HTTP response.
func (h *Handlers) finishWebhookResponse(
w http.ResponseWriter,
event *database.Event,
entrypoint database.Entrypoint,
tasks []delivery.Task,
deliveries int,
) {
h.log.Info("webhook event created",
"event_id", event.ID,
"webhook_id", entrypoint.WebhookID,
"entrypoint_id", entrypoint.ID,
"target_count", len(tasks),
"target_count", deliveries,
)
w.WriteHeader(http.StatusOK)
@@ -389,59 +206,3 @@ func (h *Handlers) finishWebhookResponse(
)
}
}
// buildDeliveryTasks creates one pending delivery per target in the
// transaction, adds each to its target's totals, and returns the tasks
// for the delivery engine. The caller owns the transaction and rolls
// it back on error.
func buildDeliveryTasks(
tx *gorm.DB,
event *database.Event,
targets []database.Target,
bodyPtr *string,
) ([]delivery.Task, error) {
tasks := make([]delivery.Task, 0, len(targets))
for i := range targets {
dlv := &database.Delivery{
EventID: event.ID,
TargetID: targets[i].ID,
Status: database.DeliveryStatusPending,
}
err := tx.Create(dlv).Error
if err != nil {
return nil, fmt.Errorf(
"creating delivery for target %s: %w",
targets[i].ID, err,
)
}
err = database.AddTargetTotals(tx, database.TargetTotals{
TargetID: targets[i].ID, Deliveries: 1,
})
if err != nil {
return nil, err
}
tasks = append(tasks, delivery.Task{
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: event.WebhookID,
EntrypointID: event.EntrypointID,
TargetID: targets[i].ID,
TargetName: targets[i].Name,
TargetType: targets[i].Type,
TargetConfig: targets[i].Config,
MaxAttempts: targets[i].MaxAttempts,
Method: event.Method,
RawQuery: event.RawQuery,
Headers: event.Headers,
ContentType: event.ContentType,
Body: bodyPtr,
AttemptNum: 1,
})
}
return tasks, nil
}
+6 -12
View File
@@ -115,13 +115,7 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
return
}
targets, errMsg, err := h.newWebhookTargets(r.Context(), in)
if err != nil {
h.serverError(w, r, "failed to encode target config", err)
return
}
targets, errMsg := h.newWebhookTargets(r.Context(), in)
if errMsg != "" {
refuse(errMsg)
@@ -147,7 +141,7 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
func (h *Handlers) newWebhookTargets(
ctx context.Context,
in sourceFormInput,
) ([]*database.Target, string, error) {
) ([]*database.Target, string) {
var requested []targetFormInput
if in.HTTPURL != "" {
@@ -170,15 +164,15 @@ func (h *Handlers) newWebhookTargets(
targets := make([]*database.Target, 0, len(requested))
for _, form := range requested {
target, errMsg, err := h.newTarget(ctx, "", form)
if err != nil || errMsg != "" {
return nil, errMsg, err
target, errMsg := h.newTarget(ctx, "", form)
if errMsg != "" {
return nil, errMsg
}
targets = append(targets, target)
}
return targets, "", nil
return targets, ""
}
// createWebhookWithEntrypoint creates a webhook, its default
+1 -1
View File
@@ -109,7 +109,7 @@ func (h *Handlers) renderSourceDetail(
data := map[string]any{
tmplKeyWebhook: &webhook,
// Targets are projected to a display-safe view: a
// target's stored config blob holds a credential, and it
// target's URL and headers hold a credential, and they
// must never reach a template.
"Entrypoints": entrypointViews,
"Targets": h.targetRows(&webhook, targets),
+1 -1
View File
@@ -153,7 +153,7 @@ func seedStatsHistory(
first := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
database.Target{URL: replayTargetURL},
)
second := seedTarget(t, db, wh.ID, database.TargetTypeLog)
+84
View File
@@ -206,3 +206,87 @@ func receivedLine(t *testing.T, buf *bytes.Buffer) map[string]any {
return found[0]
}
// TestHandleWebhook_StoresTheEvent proves an accepted request is
// answered with the receiver's 200 and body, and that the event is
// stored with a pending delivery to the webhook's active target.
func TestHandleWebhook_StoresTheEvent(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
webhook := seedWebhook(t, db)
entrypoint := seedEntrypoint(t, db, webhook.ID)
target := seedConfiguredTarget(
t, db, webhook.ID, database.TargetTypeHTTP,
database.Target{URL: replayTargetURL},
)
w := httptest.NewRecorder()
h.HandleWebhook().ServeHTTP(w, postRequest(
"/h/"+entrypoint.Path, nil,
map[string]string{"uuid": entrypoint.Path},
))
require.Equal(t, http.StatusOK, w.Code)
assert.JSONEq(t, `{"status":"ok"}`, w.Body.String())
webhookDB, err := dbMgr.GetDB(webhook.ID)
require.NoError(t, err)
events := listEvents(t, webhookDB)
require.Len(t, events, 1)
assert.Equal(t, entrypoint.ID, events[0].EntrypointID)
deliveries := listDeliveries(t, webhookDB, events[0].ID)
require.Len(t, deliveries, 1)
assert.Equal(t, target.ID, deliveries[0].TargetID)
assert.Equal(t, database.DeliveryStatusPending, deliveries[0].Status)
}
// TestHandleWebhook_FailedStoreAnswersPlainError proves that an event
// the engine cannot store is answered with the receiver's plain-text
// 500, and that nothing of it is left stored.
func TestHandleWebhook_FailedStoreAnswersPlainError(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
webhook := seedWebhook(t, db)
entrypoint := seedEntrypoint(t, db, webhook.ID)
// Without its event totals table the webhook's database refuses
// the totals write, which comes after the event row.
webhookDB, err := dbMgr.GetDB(webhook.ID)
require.NoError(t, err)
require.NoError(t,
webhookDB.Migrator().DropTable(&database.EventTotals{}))
w := httptest.NewRecorder()
h.HandleWebhook().ServeHTTP(w, postRequest(
"/h/"+entrypoint.Path, nil,
map[string]string{"uuid": entrypoint.Path},
))
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Equal(t, "Internal server error\n", w.Body.String())
assert.Empty(t, listEvents(t, webhookDB))
}
+7 -6
View File
@@ -21,8 +21,9 @@ const budget = 64
// batchRunes is how many consecutive code points the charge test logs
// in one value from U+1000 up. Logging each of those on its own line
// is too slow for the suite under the race detector; 4,096 at a time
// is 271 batches, each logged on two lines, so 542 lines per handler.
// was too slow for the suite when it ran under the race detector;
// 4,096 at a time is 271 batches, each logged on two lines, so 542
// lines per handler.
const batchRunes = 4096
// newHandlers are the two handlers internal/logger can install. Time
@@ -152,10 +153,10 @@ func TestEncodedBytes_ChargesAtLeastWhatTheHandlersEmit(t *testing.T) {
batches = append(batches, batch.String())
}
// What EncodedBytes charges for each batch. Under -race -cover this
// takes longer than logging the batches, so it is worked out once,
// by whichever handler finishes logging first, while the other is
// still logging.
// What EncodedBytes charges for each batch. When the suite ran
// under -race -cover this took longer than logging the batches, so
// it is worked out once, by whichever handler finishes logging
// first, while the other is still logging.
charged := sync.OnceValue(func() []int {
costs := make([]int, len(batches))
+6 -5
View File
@@ -131,10 +131,6 @@ func run(
return code, stdout.String(), stderr.String()
}
type noopNotifier struct{}
func (n *noopNotifier) Notify([]delivery.Task) {}
type noopArchives struct {
renameMu sync.Mutex
}
@@ -190,7 +186,12 @@ func newServerApp(
database.NewWebhookDBManager,
healthcheck.New,
session.New,
func() delivery.Notifier { return &noopNotifier{} },
archive.NewWriters,
// The engine is built on a lifecycle that is never started,
// so no worker runs.
func(p delivery.EngineParams) *delivery.Engine {
return delivery.New(fxtest.NewLifecycle(t), p)
},
func() archive.Archives { return &noopArchives{} },
func() delivery.CircuitBreakers {
return &noopCircuitBreakers{}
+39 -19
View File
@@ -1183,16 +1183,26 @@ func checkNewWebhookTargets(
) {
t.Helper()
// Each value the pruning choice submits, after an empty one that
// stands for archive left off.
expiries := []string{
"", "never", "1h", "12h", "24h", "720h", "2160h", "8760h",
// Each value the pruning choice submits and the expiry it stores,
// after an empty one that stands for archive left off.
expiries := []struct {
value string
stored time.Duration
}{
{"", 0},
{"never", 0},
{"1h", time.Hour},
{"12h", 12 * time.Hour},
{"24h", 24 * time.Hour},
{"720h", 720 * time.Hour},
{"2160h", 2160 * time.Hour},
{"8760h", 8760 * time.Hour},
}
for _, httpURL := range []string{"", publicTargetURL} {
for _, expiry := range expiries {
name := "url=" + httpURL + " archive=" + expiry
want := map[database.TargetType]string{}
name := "url=" + httpURL + " archive=" + expiry.value
want := map[database.TargetType]database.Target{}
require.NoError(t, chromedp.Run(
ctx,
@@ -1205,10 +1215,12 @@ func checkNewWebhookTargets(
"#http_url", httpURL, chromedp.ByQuery,
)))
want[database.TargetTypeHTTP] = `{"url":"` + httpURL + `"}`
want[database.TargetTypeHTTP] = database.Target{
URL: httpURL, Headers: map[string]string{},
}
}
if expiry != "" {
if expiry.value != "" {
// The choice showing moves Create down, so it is
// waited for before Create is clicked.
click(ctx, t, archiveBox)
@@ -1216,29 +1228,30 @@ func checkNewWebhookTargets(
"%s: checking archive does not show the pruning choice",
name)
require.NoError(t, chromedp.Run(ctx, chromedp.SetValue(
pruningChoice, expiry, chromedp.BySearch,
pruningChoice, expiry.value, chromedp.BySearch,
)))
// The rotation choice is left on none.
want[database.TargetTypeDatabase] = `{"expiry":"` + expiry +
`","rotation":"none"}`
want[database.TargetTypeDatabase] = database.Target{
ArchiveExpiry: expiry.stored, ArchiveRotation: "none",
}
}
clickAndLoad(ctx, t, createButton)
require.Truef(t, shown(ctx, `//h1[text()="`+name+`"]`),
"%s: the new webhook's page does not open", name)
assert.Equalf(t, want, targetConfigs(t, env, name),
assert.Equalf(t, want, targetSettings(t, env, name),
"%s: the webhook does not have the targets asked for", name)
}
}
}
// targetConfigs reads the targets of the webhook named name, and
// returns each one's stored configuration by its type.
func targetConfigs(
// targetSettings reads the targets of the webhook named name, and
// returns each one's stored settings by its type.
func targetSettings(
t *testing.T, env *testEnv, name string,
) map[database.TargetType]string {
) map[database.TargetType]database.Target {
t.Helper()
var (
@@ -1251,12 +1264,19 @@ func targetConfigs(
require.NoError(t, env.db.DB().
Where("webhook_id = ?", webhook.ID).Find(&targets).Error)
configs := map[database.TargetType]string{}
settings := map[database.TargetType]database.Target{}
for _, target := range targets {
configs[target.Type] = target.Config
settings[target.Type] = database.Target{
URL: target.URL,
Headers: target.Headers,
TimeoutSeconds: target.TimeoutSeconds,
ForwardQuery: target.ForwardQuery,
ArchiveExpiry: target.ArchiveExpiry,
ArchiveRotation: target.ArchiveRotation,
}
}
return configs
return settings
}
// checkRefusedNewWebhook submits the new webhook page with archive
+7 -8
View File
@@ -10,14 +10,12 @@ import (
"sneak.berlin/go/webhooker/internal/server"
)
// earlyStopIterations is how many start/stop cycles the race test
// earlyStopIterations is how many start/stop cycles the test below
// runs. The window it aims at is the gap between the OnStart hook
// returning and the serving goroutine reaching its first field
// access, which is microseconds wide. The race detector reports an
// unsynchronised pair whenever it observes one, but it has to observe
// one, so a single cycle can miss purely on scheduling. Repetition
// makes the observation reliable; the collaborators are built once,
// so the cycles themselves are cheap.
// access, which is microseconds wide, so a single cycle can miss it
// purely on scheduling. Repetition makes hitting it reliable; the
// collaborators are built once, so the cycles themselves are cheap.
const earlyStopIterations = 25
// TestEarlyShutdown_NoPanicAndNoRace stops the application
@@ -38,8 +36,9 @@ const earlyStopIterations = 25
//
// httpServer is now built in New, on the constructing goroutine, so
// it is written before any hook exists and can never be nil.
// sentryEnabled is atomic. This test is what catches either one
// coming back — under -race, which is how the suite runs.
// sentryEnabled is atomic. This test catches the nil dereference
// coming back, as a panic. The suite runs without Go's race
// detector, so the data race coming back would go unreported here.
func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) {
t.Parallel()
+1 -1
View File
@@ -311,7 +311,7 @@ func (s *Server) setupSourceRoutes() {
r.Post("/targets", s.h.HandleTargetCreate())
// The edit form is the one page that renders a target's
// destination URL and header values in full; see
// delivery.TargetConfigForm. It belongs to this group for
// Handlers.HandleTargetEdit. It belongs to this group for
// its RequireAuth and NoCache, which are what keep that
// exception from reaching an unauthenticated request or a
// shared cache.
+7 -6
View File
@@ -48,10 +48,6 @@ const (
metricsAuthValue = "s3cret"
)
type noopNotifier struct{}
func (n *noopNotifier) Notify([]delivery.Task) {}
// noopArchives satisfies handlers.New's archive.Archives
// dependency. No test here checks what gets evicted or renamed, so
// it records nothing. Its rename lock is a plain mutex.
@@ -145,7 +141,12 @@ func newTestEnvWithConfig(
database.NewWebhookDBManager,
healthcheck.New,
session.New,
func() delivery.Notifier { return &noopNotifier{} },
archive.NewWriters,
// The engine is built on a lifecycle that is never started,
// so no worker runs and nothing is sent anywhere.
func(p delivery.EngineParams) *delivery.Engine {
return delivery.New(fxtest.NewLifecycle(t), p)
},
func() archive.Archives { return &noopArchives{} },
func() delivery.CircuitBreakers {
return &noopCircuitBreakers{}
@@ -414,7 +415,7 @@ func (e *testEnv) seedTarget(
Name: "routed-target",
Type: database.TargetTypeHTTP,
Active: true,
Config: `{"url":"http://93.184.216.34/hook"}`,
URL: "http://93.184.216.34/hook",
MaxAttempts: 1,
}