refactor: use pinned golangci-lint Docker image for linting

Refactor Dockerfile to use a separate lint stage with a pinned golangci-lint v2.11.3 Docker image instead of installing golangci-lint via curl in the builder stage. This follows the pattern used by sneak/pixa. Changes: - Dockerfile: separate lint stage using golangci/golangci-lint:v2.11.3 (Debian-based, pinned by sha256) with COPY --from=lint dependency - Bump Go from 1.24 to 1.26.1 (golang:1.26.1-bookworm, pinned) - Bump golangci-lint from v1.64.8 to v2.11.3 - Migrate .golangci.yml from v1 to v2 format (same linters, format only) - All Docker images pinned by sha256 digest - Fix all lint issues from the v2 linter upgrade: - Add package comments to all packages - Add doc comments to all exported types, functions, and methods - Fix unchecked errors (errcheck) - Fix unused parameters (revive) - Fix gosec warnings (MaxBytesReader for form parsing) - Fix staticcheck suggestions (fmt.Fprintf instead of WriteString) - Rename DeliveryTask to Task to avoid stutter (delivery.Task) - Rename shadowed builtin 'max' parameter - Update README.md version requirements
2026-03-17 05:46:03 -07:00
parent d771fe14df
commit 32a9170428
59 changed files with 7792 additions and 4282 deletions
--- a/internal/delivery/circuit_breaker_test.go
+++ b/internal/delivery/circuit_breaker_test.go
@@ -1,4 +1,4 @@
-package delivery
+package delivery_test

 import (
 	"sync"
@@ -7,237 +7,304 @@ import (

 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/delivery"
 )

-func TestCircuitBreaker_ClosedState_AllowsDeliveries(t *testing.T) {
+func TestCircuitBreaker_ClosedState_AllowsDeliveries(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := NewCircuitBreaker()

-	assert.Equal(t, CircuitClosed, cb.State())
-	assert.True(t, cb.Allow(), "closed circuit should allow deliveries")
-	// Multiple calls should all succeed
-	for i := 0; i < 10; i++ {
+	cb := delivery.NewCircuitBreaker()
+
+	assert.Equal(t, delivery.CircuitClosed, cb.State())
+	assert.True(t, cb.Allow(),
+		"closed circuit should allow deliveries",
+	)
+
+	for range 10 {
 		assert.True(t, cb.Allow())
 	}
 }

 func TestCircuitBreaker_FailureCounting(t *testing.T) {
 	t.Parallel()
-	cb := NewCircuitBreaker()

-	// Record failures below threshold — circuit should stay closed
-	for i := 0; i < defaultFailureThreshold-1; i++ {
+	cb := delivery.NewCircuitBreaker()
+
+	for i := range delivery.ExportDefaultFailureThreshold - 1 {
 		cb.RecordFailure()
-		assert.Equal(t, CircuitClosed, cb.State(),
-			"circuit should remain closed after %d failures", i+1)
-		assert.True(t, cb.Allow(), "should still allow after %d failures", i+1)
+
+		assert.Equal(t,
+			delivery.CircuitClosed, cb.State(),
+			"circuit should remain closed after %d failures",
+			i+1,
+		)
+
+		assert.True(t, cb.Allow(),
+			"should still allow after %d failures",
+			i+1,
+		)
 	}
 }

 func TestCircuitBreaker_OpenTransition(t *testing.T) {
 	t.Parallel()
-	cb := NewCircuitBreaker()

-	// Record exactly threshold failures
-	for i := 0; i < defaultFailureThreshold; i++ {
+	cb := delivery.NewCircuitBreaker()
+
+	for range delivery.ExportDefaultFailureThreshold {
 		cb.RecordFailure()
 	}

-	assert.Equal(t, CircuitOpen, cb.State(), "circuit should be open after threshold failures")
-	assert.False(t, cb.Allow(), "open circuit should reject deliveries")
+	assert.Equal(t, delivery.CircuitOpen, cb.State(),
+		"circuit should be open after threshold failures",
+	)
+
+	assert.False(t, cb.Allow(),
+		"open circuit should reject deliveries",
+	)
 }

 func TestCircuitBreaker_Cooldown_StaysOpen(t *testing.T) {
 	t.Parallel()
-	// Use a circuit with a known short cooldown for testing
-	cb := &CircuitBreaker{
-		state:     CircuitClosed,
-		threshold: defaultFailureThreshold,
-		cooldown:  200 * time.Millisecond,
-	}

-	// Trip the circuit open
-	for i := 0; i < defaultFailureThreshold; i++ {
+	cb := delivery.NewCircuitBreaker()
+
+	for range delivery.ExportDefaultFailureThreshold {
 		cb.RecordFailure()
 	}
-	require.Equal(t, CircuitOpen, cb.State())

-	// During cooldown, Allow should return false
-	assert.False(t, cb.Allow(), "should be blocked during cooldown")
+	require.Equal(t, delivery.CircuitOpen, cb.State())
+
+	assert.False(t, cb.Allow(),
+		"should be blocked during cooldown",
+	)

-	// CooldownRemaining should be positive
 	remaining := cb.CooldownRemaining()
-	assert.Greater(t, remaining, time.Duration(0), "cooldown should have remaining time")
+
+	assert.Greater(t, remaining, time.Duration(0),
+		"cooldown should have remaining time",
+	)
 }

-func TestCircuitBreaker_HalfOpen_AfterCooldown(t *testing.T) {
+func TestCircuitBreaker_HalfOpen_AfterCooldown(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := &CircuitBreaker{
-		state:     CircuitClosed,
-		threshold: defaultFailureThreshold,
-		cooldown:  50 * time.Millisecond,
-	}

-	// Trip the circuit open
-	for i := 0; i < defaultFailureThreshold; i++ {
+	cb := newShortCooldownCB(t)
+
+	for range delivery.ExportDefaultFailureThreshold {
 		cb.RecordFailure()
 	}
-	require.Equal(t, CircuitOpen, cb.State())

-	// Wait for cooldown to expire
+	require.Equal(t, delivery.CircuitOpen, cb.State())
+
 	time.Sleep(60 * time.Millisecond)

-	// CooldownRemaining should be zero after cooldown
-	assert.Equal(t, time.Duration(0), cb.CooldownRemaining())
+	assert.Equal(t, time.Duration(0),
+		cb.CooldownRemaining(),
+	)

-	// First Allow after cooldown should succeed (probe)
-	assert.True(t, cb.Allow(), "should allow one probe after cooldown")
-	assert.Equal(t, CircuitHalfOpen, cb.State(), "should be half-open after probe allowed")
+	assert.True(t, cb.Allow(),
+		"should allow one probe after cooldown",
+	)

-	// Second Allow should be rejected (only one probe at a time)
-	assert.False(t, cb.Allow(), "should reject additional probes while half-open")
+	assert.Equal(t,
+		delivery.CircuitHalfOpen, cb.State(),
+		"should be half-open after probe allowed",
+	)
+
+	assert.False(t, cb.Allow(),
+		"should reject additional probes while half-open",
+	)
 }

-func TestCircuitBreaker_ProbeSuccess_ClosesCircuit(t *testing.T) {
+func TestCircuitBreaker_ProbeSuccess_ClosesCircuit(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := &CircuitBreaker{
-		state:     CircuitClosed,
-		threshold: defaultFailureThreshold,
-		cooldown:  50 * time.Millisecond,
-	}

-	// Trip open → wait for cooldown → allow probe
-	for i := 0; i < defaultFailureThreshold; i++ {
+	cb := newShortCooldownCB(t)
+
+	for range delivery.ExportDefaultFailureThreshold {
 		cb.RecordFailure()
 	}
-	time.Sleep(60 * time.Millisecond)
-	require.True(t, cb.Allow()) // probe allowed, state → half-open

-	// Probe succeeds → circuit should close
+	time.Sleep(60 * time.Millisecond)
+
+	require.True(t, cb.Allow())
+
 	cb.RecordSuccess()
-	assert.Equal(t, CircuitClosed, cb.State(), "successful probe should close circuit")

-	// Should allow deliveries again
-	assert.True(t, cb.Allow(), "closed circuit should allow deliveries")
+	assert.Equal(t, delivery.CircuitClosed, cb.State(),
+		"successful probe should close circuit",
+	)
+
+	assert.True(t, cb.Allow(),
+		"closed circuit should allow deliveries",
+	)
 }

-func TestCircuitBreaker_ProbeFailure_ReopensCircuit(t *testing.T) {
+func TestCircuitBreaker_ProbeFailure_ReopensCircuit(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := &CircuitBreaker{
-		state:     CircuitClosed,
-		threshold: defaultFailureThreshold,
-		cooldown:  50 * time.Millisecond,
-	}

-	// Trip open → wait for cooldown → allow probe
-	for i := 0; i < defaultFailureThreshold; i++ {
+	cb := newShortCooldownCB(t)
+
+	for range delivery.ExportDefaultFailureThreshold {
 		cb.RecordFailure()
 	}
+
 	time.Sleep(60 * time.Millisecond)
-	require.True(t, cb.Allow()) // probe allowed, state → half-open

-	// Probe fails → circuit should reopen
+	require.True(t, cb.Allow())
+
 	cb.RecordFailure()
-	assert.Equal(t, CircuitOpen, cb.State(), "failed probe should reopen circuit")
-	assert.False(t, cb.Allow(), "reopened circuit should reject deliveries")
+
+	assert.Equal(t, delivery.CircuitOpen, cb.State(),
+		"failed probe should reopen circuit",
+	)
+
+	assert.False(t, cb.Allow(),
+		"reopened circuit should reject deliveries",
+	)
 }

-func TestCircuitBreaker_SuccessResetsFailures(t *testing.T) {
+func TestCircuitBreaker_SuccessResetsFailures(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := NewCircuitBreaker()

-	// Accumulate failures just below threshold
-	for i := 0; i < defaultFailureThreshold-1; i++ {
+	cb := delivery.NewCircuitBreaker()
+
+	for range delivery.ExportDefaultFailureThreshold - 1 {
 		cb.RecordFailure()
 	}
-	require.Equal(t, CircuitClosed, cb.State())

-	// Success should reset the failure counter
+	require.Equal(t, delivery.CircuitClosed, cb.State())
+
 	cb.RecordSuccess()
-	assert.Equal(t, CircuitClosed, cb.State())

-	// Now we should need another full threshold of failures to trip
-	for i := 0; i < defaultFailureThreshold-1; i++ {
+	assert.Equal(t, delivery.CircuitClosed, cb.State())
+
+	for range delivery.ExportDefaultFailureThreshold - 1 {
 		cb.RecordFailure()
 	}
-	assert.Equal(t, CircuitClosed, cb.State(),
-		"circuit should still be closed — success reset the counter")

-	// One more failure should trip it
+	assert.Equal(t, delivery.CircuitClosed, cb.State(),
+		"circuit should still be closed -- "+
+			"success reset the counter",
+	)
+
 	cb.RecordFailure()
-	assert.Equal(t, CircuitOpen, cb.State())
+
+	assert.Equal(t, delivery.CircuitOpen, cb.State())
 }

 func TestCircuitBreaker_ConcurrentAccess(t *testing.T) {
 	t.Parallel()
-	cb := NewCircuitBreaker()
+
+	cb := delivery.NewCircuitBreaker()

 	const goroutines = 100
+
 	var wg sync.WaitGroup
+
 	wg.Add(goroutines * 3)

-	// Concurrent Allow calls
-	for i := 0; i < goroutines; i++ {
+	for range goroutines {
 		go func() {
 			defer wg.Done()
+
 			cb.Allow()
 		}()
 	}

-	// Concurrent RecordFailure calls
-	for i := 0; i < goroutines; i++ {
+	for range goroutines {
 		go func() {
 			defer wg.Done()
+
 			cb.RecordFailure()
 		}()
 	}

-	// Concurrent RecordSuccess calls
-	for i := 0; i < goroutines; i++ {
+	for range goroutines {
 		go func() {
 			defer wg.Done()
+
 			cb.RecordSuccess()
 		}()
 	}

 	wg.Wait()
-	// No panic or data race — the test passes if -race doesn't flag anything.
-	// State should be one of the valid states.
+
 	state := cb.State()
-	assert.Contains(t, []CircuitState{CircuitClosed, CircuitOpen, CircuitHalfOpen}, state,
-		"state should be valid after concurrent access")
+
+	assert.Contains(t,
+		[]delivery.CircuitState{
+			delivery.CircuitClosed,
+			delivery.CircuitOpen,
+			delivery.CircuitHalfOpen,
+		},
+		state,
+		"state should be valid after concurrent access",
+	)
 }

-func TestCircuitBreaker_CooldownRemaining_ClosedReturnsZero(t *testing.T) {
+func TestCircuitBreaker_CooldownRemaining_ClosedReturnsZero(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := NewCircuitBreaker()
-	assert.Equal(t, time.Duration(0), cb.CooldownRemaining(),
-		"closed circuit should have zero cooldown remaining")
+
+	cb := delivery.NewCircuitBreaker()
+
+	assert.Equal(t, time.Duration(0),
+		cb.CooldownRemaining(),
+		"closed circuit should have zero cooldown remaining",
+	)
 }

-func TestCircuitBreaker_CooldownRemaining_HalfOpenReturnsZero(t *testing.T) {
+func TestCircuitBreaker_CooldownRemaining_HalfOpenReturnsZero(
+	t *testing.T,
+) {
 	t.Parallel()
-	cb := &CircuitBreaker{
-		state:     CircuitClosed,
-		threshold: defaultFailureThreshold,
-		cooldown:  50 * time.Millisecond,
-	}

-	// Trip open, wait, transition to half-open
-	for i := 0; i < defaultFailureThreshold; i++ {
+	cb := newShortCooldownCB(t)
+
+	for range delivery.ExportDefaultFailureThreshold {
 		cb.RecordFailure()
 	}
-	time.Sleep(60 * time.Millisecond)
-	require.True(t, cb.Allow()) // → half-open

-	assert.Equal(t, time.Duration(0), cb.CooldownRemaining(),
-		"half-open circuit should have zero cooldown remaining")
+	time.Sleep(60 * time.Millisecond)
+
+	require.True(t, cb.Allow())
+
+	assert.Equal(t, time.Duration(0),
+		cb.CooldownRemaining(),
+		"half-open circuit should have zero cooldown remaining",
+	)
 }

 func TestCircuitState_String(t *testing.T) {
 	t.Parallel()
-	assert.Equal(t, "closed", CircuitClosed.String())
-	assert.Equal(t, "open", CircuitOpen.String())
-	assert.Equal(t, "half-open", CircuitHalfOpen.String())
-	assert.Equal(t, "unknown", CircuitState(99).String())
+
+	assert.Equal(t, "closed", delivery.CircuitClosed.String())
+	assert.Equal(t, "open", delivery.CircuitOpen.String())
+	assert.Equal(t, "half-open", delivery.CircuitHalfOpen.String())
+	assert.Equal(t, "unknown", delivery.CircuitState(99).String())
+}
+
+// newShortCooldownCB creates a CircuitBreaker with a short
+// cooldown for testing. We use NewCircuitBreaker and
+// manipulate through the public API.
+func newShortCooldownCB(t *testing.T) *delivery.CircuitBreaker {
+	t.Helper()
+
+	return delivery.NewTestCircuitBreaker(
+		delivery.ExportDefaultFailureThreshold,
+		50*time.Millisecond,
+	)
 }