Store a target's settings as columns (closes #543)

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, so nothing that reads a target parses
it. 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.

Model: opus-5-5

internal/delivery/target_http.go

@@ -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
@@ -407,34 +383,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(
@@ -449,11 +399,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 &&
@@ -475,7 +425,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()
@@ -483,7 +433,7 @@ func (t *httpTarget) doHTTPRequest(
 	req, reqErr := http.NewRequestWithContext(
 		ctx,
 		http.MethodPost,
-		cfg.URL,
+		target.URL,
 		bytes.NewReader([]byte(event.Body)),
 	)
 	if reqErr != nil {
@@ -493,15 +443,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)
 
@@ -541,17 +491,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
 	}
 
@@ -564,9 +514,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
 	}
 
@@ -577,31 +527,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 {
@@ -633,11 +558,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 != "" {
@@ -648,7 +574,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{}{}
 	}
@@ -713,7 +639,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