Compare commits
1
Commits
next
...
44c95318da
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
44c95318da |
@@ -79,7 +79,8 @@ directory, read once at startup before anything else looks at the
|
|||||||
environment.
|
environment.
|
||||||
|
|
||||||
The file is optional and having none is the normal case for a
|
The file is optional and having none is the normal case for a
|
||||||
deployment. A file that is there but cannot be parsed aborts startup
|
deployment. An empty file is the same as none: it has nothing in it to
|
||||||
|
apply. A file that is there but cannot be parsed aborts startup
|
||||||
with a message naming it, because a single malformed line makes none
|
with a message naming it, because a single malformed line makes none
|
||||||
of the file apply: every variable in it silently reverts to its
|
of the file apply: every variable in it silently reverts to its
|
||||||
default, which is exactly the failure [Invalid values abort
|
default, which is exactly the failure [Invalid values abort
|
||||||
@@ -564,7 +565,8 @@ its Argon2id hash. There is no second account and no forgot-password
|
|||||||
flow, so the banner and the reset command below are the only two ways
|
flow, so the banner and the reset command below are the only two ways
|
||||||
in.
|
in.
|
||||||
|
|
||||||
A start that finds no `webhooker.db` in `DATA_DIR` also logs
|
A start that finds no `webhooker.db` in `DATA_DIR`, or a zero-length
|
||||||
|
one (which SQLite opens as an empty database), also logs
|
||||||
`created a new, empty database` at `WARN`, with the file's path,
|
`created a new, empty database` at `WARN`, with the file's path,
|
||||||
shortly before the banner. On a deployment that has run before, that
|
shortly before the banner. On a deployment that has run before, that
|
||||||
line means `DATA_DIR` was empty, most often because its volume is not
|
line means `DATA_DIR` was empty, most often because its volume is not
|
||||||
@@ -1939,10 +1941,17 @@ encryption key is generated and stored, and an `admin` user is created.
|
|||||||
the deliveries per target, kept through retention
|
the deliveries per target, kept through retention
|
||||||
|
|
||||||
Per-webhook databases are created automatically when a webhook is
|
Per-webhook databases are created automatically when a webhook is
|
||||||
created (and lazily on first access for webhooks that predate this
|
created. They are managed by the `WebhookDBManager` component, which
|
||||||
feature). They are managed by the `WebhookDBManager` component, which
|
|
||||||
handles connection pooling, lazy opening, migrations, and cleanup.
|
handles connection pooling, lazy opening, migrations, and cleanup.
|
||||||
|
|
||||||
|
A per-webhook database that is missing or zero-length later means its
|
||||||
|
webhook's events and pending deliveries are gone. The next time it is
|
||||||
|
opened, an empty one is created in its place, so the webhook keeps
|
||||||
|
receiving, and `created a new, empty database` is logged at `WARN` with
|
||||||
|
the file's path. A file there that SQLite cannot open fails that
|
||||||
|
webhook alone, with an `ERROR` naming the webhook on every access and a
|
||||||
|
500 to its senders, so one damaged file does not stop the others.
|
||||||
|
|
||||||
This separation provides:
|
This separation provides:
|
||||||
|
|
||||||
- **Isolation** — a high-volume webhook won't cause lock contention or
|
- **Isolation** — a high-volume webhook won't cause lock contention or
|
||||||
@@ -2007,7 +2016,9 @@ After each write the archive handle is closed
|
|||||||
and reopened, debounced to at most once per second, so an operator can
|
and reopened, debounced to at most once per second, so an operator can
|
||||||
move the archive file away for offline archiving without stopping the
|
move the archive file away for offline archiving without stopping the
|
||||||
service; a moved or removed archive file is recreated automatically on
|
service; a moved or removed archive file is recreated automatically on
|
||||||
the next write. An optional `expiry` in the target's config JSON (e.g.
|
the next write. A zero-length archive file is written to as a new
|
||||||
|
archive: SQLite opens it as an empty database, so it holds nothing to
|
||||||
|
lose. An optional `expiry` in the target's config JSON (e.g.
|
||||||
`{"expiry":"720h"}`) is validated when the target is created — the
|
`{"expiry":"720h"}`) is validated when the target is created — the
|
||||||
default (unset or the literal `never`) keeps rows forever — and rows
|
default (unset or the literal `never`) keeps rows forever — and rows
|
||||||
older than the expiry are pruned each time the archive is (re)opened. An
|
older than the expiry are pruned each time the archive is (re)opened. An
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -119,3 +120,26 @@ func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) {
|
|||||||
t, second, created, "an existing database is not new",
|
t, second, created, "an existing database is not new",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestZeroLengthDatabase_IsLoggedAsNew covers what
|
||||||
|
// https://git.eeqj.de/sneak/webhooker/issues/290 found: SQLite opens a
|
||||||
|
// zero-length file as an empty database, so a start on one is a first
|
||||||
|
// start, and it must say so exactly as a start with no file does.
|
||||||
|
func TestZeroLengthDatabase_IsLoggedAsNew(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, database.MainDBFileName)
|
||||||
|
require.NoError(t, os.WriteFile(path, nil, database.SQLiteFilePerm))
|
||||||
|
|
||||||
|
var out bytes.Buffer
|
||||||
|
|
||||||
|
db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil)))
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, db.Close())
|
||||||
|
|
||||||
|
assert.Contains(
|
||||||
|
t, out.String(),
|
||||||
|
`level=WARN msg="created a new, empty database" path=`+path,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|||||||
@@ -8,7 +8,6 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"io/fs"
|
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -203,8 +202,7 @@ func (d *Database) connectTo(dataDir string) error {
|
|||||||
// Checked before opening, which creates the file. A DATA_DIR that
|
// Checked before opening, which creates the file. A DATA_DIR that
|
||||||
// is unexpectedly empty -- its volume not mounted, say -- looks
|
// is unexpectedly empty -- its volume not mounted, say -- looks
|
||||||
// exactly like a first start, so a new database is a warning.
|
// exactly like a first start, so a new database is a warning.
|
||||||
_, statErr := os.Stat(dbPath)
|
created := missingOrEmpty(dbPath)
|
||||||
created := errors.Is(statErr, fs.ErrNotExist)
|
|
||||||
|
|
||||||
// Opened through OpenSQLite so this handle carries the same WAL
|
// Opened through OpenSQLite so this handle carries the same WAL
|
||||||
// journaling, busy timeout, immediate-transaction locking, and pool
|
// journaling, busy timeout, immediate-transaction locking, and pool
|
||||||
|
|||||||
@@ -152,6 +152,20 @@ func reserveSQLiteFile(path string, create bool) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// missingOrEmpty reports whether opening path in SQLiteModeCreate
|
||||||
|
// would start a new, empty database: the file is not there, or it is
|
||||||
|
// zero-length, which SQLite opens as an empty database. A file left at
|
||||||
|
// zero length by an interrupted first start or a truncated copy holds
|
||||||
|
// as little as a missing one, and must be reported the same way.
|
||||||
|
func missingOrEmpty(path string) bool {
|
||||||
|
info, err := os.Stat(path)
|
||||||
|
if errors.Is(err, fs.ErrNotExist) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
return err == nil && info.Size() == 0
|
||||||
|
}
|
||||||
|
|
||||||
// sqliteSidecarPaths returns the files SQLite maintains beside a
|
// sqliteSidecarPaths returns the files SQLite maintains beside a
|
||||||
// database under WAL. They carry the same rows as the database itself,
|
// database under WAL. They carry the same rows as the database itself,
|
||||||
// so a fix that tightens only the main file has fixed nothing.
|
// so a fix that tightens only the main file has fixed nothing.
|
||||||
|
|||||||
@@ -98,34 +98,18 @@ func NewWebhookDBManager(
|
|||||||
return m, nil
|
return m, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetDB returns the database connection for a webhook,
|
// GetDB returns the database connection for a webhook, opening it on
|
||||||
// creating the database file lazily if it doesn't exist.
|
// first use.
|
||||||
|
//
|
||||||
|
// The file is made by CreateDB when the webhook is created. One that is
|
||||||
|
// missing or zero-length here means the webhook's events and pending
|
||||||
|
// deliveries are gone: an empty database is created in its place so
|
||||||
|
// the webhook keeps receiving, and that is logged as a warning naming
|
||||||
|
// the file, as a new main database is.
|
||||||
func (m *WebhookDBManager) GetDB(
|
func (m *WebhookDBManager) GetDB(
|
||||||
webhookID string,
|
webhookID string,
|
||||||
) (*gorm.DB, error) {
|
) (*gorm.DB, error) {
|
||||||
// Fast path: already open
|
return m.getDB(webhookID, false)
|
||||||
if val, ok := m.dbs.Load(webhookID); ok {
|
|
||||||
return asGormDB(val, webhookID)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Slow path: open the database under the lock, looking in the
|
|
||||||
// cache again first. A caller that raced another one here then
|
|
||||||
// waits for its handle instead of opening a second one.
|
|
||||||
m.mu.Lock()
|
|
||||||
defer m.mu.Unlock()
|
|
||||||
|
|
||||||
if val, ok := m.dbs.Load(webhookID); ok {
|
|
||||||
return asGormDB(val, webhookID)
|
|
||||||
}
|
|
||||||
|
|
||||||
db, err := m.openDB(webhookID)
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
|
|
||||||
m.dbs.Store(webhookID, db)
|
|
||||||
|
|
||||||
return db, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// asGormDB returns a value read from the cache as the database
|
// asGormDB returns a value read from the cache as the database
|
||||||
@@ -143,12 +127,12 @@ func asGormDB(val any, webhookID string) (*gorm.DB, error) {
|
|||||||
return db, nil
|
return db, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// CreateDB explicitly creates a new per-webhook database file
|
// CreateDB creates a new webhook's database file and runs
|
||||||
// and runs migrations.
|
// migrations.
|
||||||
func (m *WebhookDBManager) CreateDB(
|
func (m *WebhookDBManager) CreateDB(
|
||||||
webhookID string,
|
webhookID string,
|
||||||
) error {
|
) error {
|
||||||
_, err := m.GetDB(webhookID)
|
_, err := m.getDB(webhookID, true)
|
||||||
|
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -266,6 +250,48 @@ func (m *WebhookDBManager) DBPath(
|
|||||||
return m.dbPath(webhookID)
|
return m.dbPath(webhookID)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// getDB is GetDB, and CreateDB when isNew is true: the webhook has just
|
||||||
|
// been created, so a missing file is expected rather than lost.
|
||||||
|
func (m *WebhookDBManager) getDB(
|
||||||
|
webhookID string, isNew bool,
|
||||||
|
) (*gorm.DB, error) {
|
||||||
|
// Fast path: already open
|
||||||
|
if val, ok := m.dbs.Load(webhookID); ok {
|
||||||
|
return asGormDB(val, webhookID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Slow path: open the database under the lock, looking in the
|
||||||
|
// cache again first. A caller that raced another one here then
|
||||||
|
// waits for its handle instead of opening a second one.
|
||||||
|
m.mu.Lock()
|
||||||
|
defer m.mu.Unlock()
|
||||||
|
|
||||||
|
if val, ok := m.dbs.Load(webhookID); ok {
|
||||||
|
return asGormDB(val, webhookID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Checked before opening, which creates the file. See GetDB.
|
||||||
|
path := m.dbPath(webhookID)
|
||||||
|
replaced := !isNew && missingOrEmpty(path)
|
||||||
|
|
||||||
|
db, err := m.openDB(webhookID)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
if replaced {
|
||||||
|
m.log.Warn(
|
||||||
|
"created a new, empty database",
|
||||||
|
"webhook_id", webhookID,
|
||||||
|
"path", path,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
m.dbs.Store(webhookID, db)
|
||||||
|
|
||||||
|
return db, nil
|
||||||
|
}
|
||||||
|
|
||||||
func (m *WebhookDBManager) dbPath(
|
func (m *WebhookDBManager) dbPath(
|
||||||
webhookID string,
|
webhookID string,
|
||||||
) string {
|
) string {
|
||||||
|
|||||||
@@ -289,6 +289,75 @@ func TestWebhookDBManager_LazyCreation(t *testing.T) {
|
|||||||
assert.True(t, mgr.DBExists(webhookID))
|
assert.True(t, mgr.DBExists(webhookID))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A webhook's database is made by CreateDB along with the webhook. One
|
||||||
|
// that GetDB finds missing or zero-length has lost the webhook's events
|
||||||
|
// and pending deliveries, so the empty database made in its place is
|
||||||
|
// logged as a warning naming the file
|
||||||
|
// (https://git.eeqj.de/sneak/webhooker/issues/290). CreateDB, and
|
||||||
|
// reopening a database that is there, log no such warning.
|
||||||
|
func TestWebhookDBManager_LostDatabaseIsLogged(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const created = `level=WARN msg="created a new, empty database"`
|
||||||
|
|
||||||
|
open := func(
|
||||||
|
t *testing.T, prepare func(*database.WebhookDBManager, string),
|
||||||
|
) (string, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var logs bytes.Buffer
|
||||||
|
|
||||||
|
mgr := database.NewTestWebhookDBManagerWithLogger(
|
||||||
|
t.TempDir(),
|
||||||
|
slog.New(slog.NewTextHandler(&logs, nil)),
|
||||||
|
)
|
||||||
|
|
||||||
|
webhookID := uuid.New().String()
|
||||||
|
prepare(mgr, webhookID)
|
||||||
|
|
||||||
|
_, err := mgr.GetDB(webhookID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, mgr.CloseAll())
|
||||||
|
|
||||||
|
return logs.String(),
|
||||||
|
" webhook_id=" + webhookID + " path=" + mgr.DBPath(webhookID)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Run("missing", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
logs, fields := open(
|
||||||
|
t, func(*database.WebhookDBManager, string) {},
|
||||||
|
)
|
||||||
|
assert.Contains(t, logs, created+fields)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("zero-length", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
logs, fields := open(
|
||||||
|
t, func(mgr *database.WebhookDBManager, webhookID string) {
|
||||||
|
require.NoError(t, os.WriteFile(
|
||||||
|
mgr.DBPath(webhookID), nil, database.SQLiteFilePerm,
|
||||||
|
))
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert.Contains(t, logs, created+fields)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("created with the webhook, then reopened", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
logs, _ := open(
|
||||||
|
t, func(mgr *database.WebhookDBManager, webhookID string) {
|
||||||
|
require.NoError(t, mgr.CreateDB(webhookID))
|
||||||
|
require.NoError(t, mgr.CloseAll())
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert.NotContains(t, logs, created)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func TestWebhookDBManager_DeliveryWorkflow(t *testing.T) {
|
func TestWebhookDBManager_DeliveryWorkflow(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user