Manager: refuse to delete job when foreign keys are disabled

Just as a safety measure, before deleting a job, check that foreign key
constraints are enabled. These are optional in SQLite, and the deletion
function assumes that they are on.
This commit is contained in:
Sybren A. Stüvel 2024-01-11 17:15:43 +01:00
parent 3e46322d14
commit 6777e89589
3 changed files with 30 additions and 0 deletions

View File

@ -13,6 +13,8 @@ var (
ErrTaskNotFound = PersistenceError{Message: "task not found", Err: gorm.ErrRecordNotFound} ErrTaskNotFound = PersistenceError{Message: "task not found", Err: gorm.ErrRecordNotFound}
ErrWorkerNotFound = PersistenceError{Message: "worker not found", Err: gorm.ErrRecordNotFound} ErrWorkerNotFound = PersistenceError{Message: "worker not found", Err: gorm.ErrRecordNotFound}
ErrWorkerTagNotFound = PersistenceError{Message: "worker tag not found", Err: gorm.ErrRecordNotFound} ErrWorkerTagNotFound = PersistenceError{Message: "worker tag not found", Err: gorm.ErrRecordNotFound}
ErrDeletingWithoutFK = errors.New("refusing to delete a job when foreign keys are not enabled on the database")
) )
type PersistenceError struct { type PersistenceError struct {

View File

@ -8,6 +8,7 @@ import (
"database/sql/driver" "database/sql/driver"
"encoding/json" "encoding/json"
"errors" "errors"
"fmt"
"math" "math"
"time" "time"
@ -269,6 +270,15 @@ func (db *DB) FetchJob(ctx context.Context, jobUUID string) (*Job, error) {
// DeleteJob deletes a job from the database. // DeleteJob deletes a job from the database.
// The deletion cascades to its tasks and other job-related tables. // The deletion cascades to its tasks and other job-related tables.
func (db *DB) DeleteJob(ctx context.Context, jobUUID string) error { func (db *DB) DeleteJob(ctx context.Context, jobUUID string) error {
// As a safety measure, refuse to delete jobs unless foreign key constraints are active.
fkEnabled, err := db.areForeignKeysEnabled()
if err != nil {
return fmt.Errorf("checking whether foreign keys are enabled: %w", err)
}
if !fkEnabled {
return ErrDeletingWithoutFK
}
tx := db.gormDB.WithContext(ctx). tx := db.gormDB.WithContext(ctx).
Where("uuid = ?", jobUUID). Where("uuid = ?", jobUUID).
Delete(&Job{}) Delete(&Job{})

View File

@ -155,6 +155,24 @@ func TestDeleteJob(t *testing.T) {
"all remaining tasks should belong to the other job") "all remaining tasks should belong to the other job")
} }
func TestDeleteJobWithoutFK(t *testing.T) {
ctx, cancel, db := persistenceTestFixtures(t, 1*time.Second)
defer cancel()
authJob := createTestAuthoredJobWithTasks()
authJob.Name = "Job to delete"
persistAuthoredJob(t, ctx, db, authJob)
require.NoError(t, db.pragmaForeignKeys(false))
err := db.DeleteJob(ctx, authJob.JobID)
require.ErrorIs(t, err, ErrDeletingWithoutFK)
// Test the deletion did not happen.
_, err = db.FetchJob(ctx, authJob.JobID)
assert.NoError(t, err, "job should not have been deleted")
}
func TestRequestJobDeletion(t *testing.T) { func TestRequestJobDeletion(t *testing.T) {
ctx, close, db, job1, authoredJob1 := jobTasksTestFixtures(t) ctx, close, db, job1, authoredJob1 := jobTasksTestFixtures(t)
defer close() defer close()