Author SHA1 Message Date
clawbot 0cef03d3a5 Run the checks on every script/cibuild and script/docker build (closes #101)
check / check (push) Successful in 7m45s
Both scripts pass a new CHECK_EPOCH, which the Dockerfile's make
fmt-check, make lint and make test steps name in their commands. On an
unchanged tree Docker used to serve those steps from its build cache, so
a run could pass without checking anything. The script/bootstrap steps
stay cached. A plain docker build . still works, as upaas builds the
image that way: it leaves CHECK_EPOCH empty and reuses the check steps
only for an identical build context.

Model: opus-5-5
2026-10-03 15:53:10 +00:00
13 changed files with 107 additions and 289 deletions
+11 -5
View File
@@ -18,9 +18,14 @@ COPY . .
# Tells script/lint it is inside a container, so it runs the linter. # Tells script/lint it is inside a container, so it runs the linter.
ENV container=docker ENV container=docker
# Run formatting check and linter # Run formatting check and linter. script/cibuild and script/docker pass
RUN make fmt-check # a new CHECK_EPOCH on every run, and each check step names it in its
RUN make lint # command, so a new value reruns the step instead of reusing a cached
# success that checked nothing. A plain `docker build .` leaves it empty
# and reuses the check steps only for an identical build context.
ARG CHECK_EPOCH
RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check
RUN echo "check epoch: ${CHECK_EPOCH}" && make lint
# Build stage # Build stage
# golang:1.25.4-alpine, 2026-02-25 # golang:1.25.4-alpine, 2026-02-25
@@ -39,8 +44,9 @@ RUN script/bootstrap
# Copy source code # Copy source code
COPY . . COPY . .
# Run tests # Run tests; a new CHECK_EPOCH reruns them, as in the lint stage.
RUN make test ARG CHECK_EPOCH
RUN echo "check epoch: ${CHECK_EPOCH}" && make test
# VERSION is declared here, not earlier: a new value reruns only the # VERSION is declared here, not earlier: a new value reruns only the
# build, not script/bootstrap or the tests. Given none, the version is # build, not script/bootstrap or the tests. Given none, the version is
+3 -2
View File
@@ -345,8 +345,9 @@ them. We provide:
- `script/check` — run test, lint, and fmt-check - `script/check` — run test, lint, and fmt-check
- `script/docker` — build the Docker image tagged via `script/projectname` - `script/docker` — build the Docker image tagged via `script/projectname`
- `script/docker-smoke` — build the image, start it, wait for it to be healthy - `script/docker-smoke` — build the image, start it, wait for it to be healthy
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile - `script/cibuild` — CI entrypoint: `docker build .` with a new
runs the checks, so a green build implies a green repo) `CHECK_EPOCH` on every run, so the Dockerfile's checks run instead of
coming from the build cache, and a green run implies a green repo
- `script/precommit` — pre-commit checks (`go mod tidy` guard, then - `script/precommit` — pre-commit checks (`go mod tidy` guard, then
`script/check`) `script/check`)
- `script/install-precommit` — install the git pre-commit hook that - `script/install-precommit` — install the git pre-commit hook that
+9 -11
View File
@@ -29,17 +29,15 @@ P2: security: referer blacklist
# Completed Steps # Completed Steps
- 2026-10-03 shutdown sets the exit code and waits for image processing - 2026-10-03 every `script/cibuild` and `script/docker` run executes the checks
(closes #86): fx alone handles SIGINT and SIGTERM, and the server's own (closes #101): the `Dockerfile` declares `CHECK_EPOCH` above `make fmt-check`
signal handler is gone; fx's `Run` in `cmd/pixad` exits with the shutdown's and `make lint` in the lint stage and above `make test` in the build stage,
code: 0 for a signal, 1 when the HTTP server cannot listen or the app fails and each of those steps names it in its command; both scripts pass a new value
to start or to stop; the server's stop hook, which fx waits for, stops the on every run, so Docker runs the checks instead of reusing cached results,
HTTP server, waits for the images still being processed, both within 5 while the `script/bootstrap` steps stay cached; a plain `docker build .` still
seconds, then flushes Sentry; images still being processed after that are works, leaves it empty, and reuses the check steps only for an identical build
logged with their count and make the exit code 1; a Sentry DSN that cannot be context; the `script/cibuild` comment and `README.md` no longer say that any
used fails startup, so the stop hooks of what had already started run, successful build implies a green repo.
instead of exiting the process from a goroutine; the eviction loop is left to
#102.
- 2026-09-29 only the image routes send CORS headers (closes #98): the CORS - 2026-09-29 only the image routes send CORS headers (closes #98): the CORS
middleware, with the `access_control_allow_origin` origin, moved from the middleware, with the `access_control_allow_origin` origin, moved from the
router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it
-5
View File
@@ -4,8 +4,6 @@ package main
import ( import (
"fmt" "fmt"
"os" "os"
"os/signal"
"syscall"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx" "go.uber.org/fx"
@@ -47,9 +45,6 @@ func run(_ *cobra.Command, _ []string) {
_ = os.Setenv("PIXA_CONFIG_PATH", configPath) _ = os.Setenv("PIXA_CONFIG_PATH", configPath)
} }
// A write to a closed stdout or stderr must not end the process.
signal.Ignore(syscall.SIGPIPE)
fx.New( fx.New(
fx.Provide( fx.Provide(
config.New, config.New,
-6
View File
@@ -81,12 +81,6 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
return s, nil return s, nil
} }
// WaitForProcessing waits until no image is being processed, or until ctx
// ends, and returns how many images were still being processed then.
func (s *Handlers) WaitForProcessing(ctx context.Context) int {
return s.imgSvc.WaitForProcessing(ctx)
}
// initImageService initializes the image cache and service. // initImageService initializes the image cache and service.
func (s *Handlers) initImageService() error { func (s *Handlers) initImageService() error {
// Create the cache. cache_max_bytes: 0 disables the disk cache // Create the cache. cache_max_bytes: 0 disables the disk cache
-25
View File
@@ -331,31 +331,6 @@ func FormatToMIME(format Format) string {
} }
} }
// WaitForProcessing waits until no image is being processed, or until ctx
// ends, and returns how many images were still being processed then. It
// waits by taking every slot in processingSemaphore as it frees up, so no
// new image starts meanwhile, and gives them all back before it returns.
func (p *ImageProcessor) WaitForProcessing(ctx context.Context) int {
taken := 0
defer func() {
for range taken {
<-p.processingSemaphore
}
}()
for taken < cap(p.processingSemaphore) {
select {
case p.processingSemaphore <- struct{}{}:
taken++
case <-ctx.Done():
return len(p.processingSemaphore) - taken
}
}
return 0
}
// acquireSlot takes a slot in processingSemaphore, waiting at most // acquireSlot takes a slot in processingSemaphore, waiting at most
// processingWaitTimeout for one to free up, and returns the func that gives // processingWaitTimeout for one to free up, and returns the func that gives
// it back. A free slot is taken even when ctx has ended; only the wait for // it back. A free slot is taken even when ctx has ended; only the wait for
@@ -300,69 +300,3 @@ func TestProcessReleasesSlotOnError(t *testing.T) {
}) })
} }
} }
// TestWaitForProcessing holds a processing slot with a Process call that
// cannot finish reading its input. WaitForProcessing must report that image
// when its context ends first, wait for it otherwise, return 0 once it has
// finished, and give back the slots it took while waiting.
func TestWaitForProcessing(t *testing.T) {
t.Parallel()
proc := New(Params{MaxConcurrentProcessing: 2})
gate := make(chan struct{})
entered := make(chan struct{}, 1)
results := make(chan error, 1)
openGate := sync.OnceFunc(func() { close(gate) })
t.Cleanup(openGate)
processInBackground(proc, &gatedReader{
data: bytes.NewReader(createTestJPEG(t, 10, 10)), gate: gate,
entered: entered, counter: &readingCounter{},
}, results)
waitForEntries(t, entered, 1)
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()
stillProcessing := proc.WaitForProcessing(ctx)
t.Logf("WaitForProcessing() after its context ended: %d", stillProcessing)
if stillProcessing != 1 {
t.Errorf("WaitForProcessing() after its context ended = %d, want 1",
stillProcessing)
}
waited := make(chan int, 1)
go func() { waited <- proc.WaitForProcessing(t.Context()) }()
select {
case got := <-waited:
t.Fatalf("WaitForProcessing() = %d while an image was being processed",
got)
case <-time.After(100 * time.Millisecond):
}
openGate()
err := <-results
if err != nil {
t.Errorf("Process() error = %v, want nil", err)
}
select {
case got := <-waited:
if got != 0 {
t.Errorf("WaitForProcessing() once processing finished = %d, want 0",
got)
}
case <-time.After(5 * time.Second):
t.Fatal("WaitForProcessing() did not return once processing finished")
}
if held := len(proc.processingSemaphore); held != 0 {
t.Errorf("%d slots still held after WaitForProcessing() returned", held)
}
}
-6
View File
@@ -195,12 +195,6 @@ func (s *Service) Stats(ctx context.Context) (*CacheStats, error) {
return s.cache.Stats(ctx) return s.cache.Stats(ctx)
} }
// WaitForProcessing waits until no image is being processed, or until ctx
// ends, and returns how many images were still being processed then.
func (s *Service) WaitForProcessing(ctx context.Context) int {
return s.processor.WaitForProcessing(ctx)
}
// ValidateRequest validates the request signature if required. // ValidateRequest validates the request signature if required.
func (s *Service) ValidateRequest(req *ImageRequest) error { func (s *Service) ValidateRequest(req *ImageRequest) error {
// Check if host is allowed (no signature required) // Check if host is allowed (no signature required)
+6 -8
View File
@@ -5,8 +5,6 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"time" "time"
"go.uber.org/fx"
) )
// HTTP server configuration constants. // HTTP server configuration constants.
@@ -38,19 +36,19 @@ func (s *Server) newHTTPServer() *http.Server {
} }
} }
// serveUntilShutdown serves on s.httpServer until it is shut down. When it
// stops for any other reason, such as its port being in use, it asks fx to
// shut down with exit code 1.
func (s *Server) serveUntilShutdown() { func (s *Server) serveUntilShutdown() {
s.httpServer = s.newHTTPServer()
s.SetupRoutes()
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr) s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
err := s.httpServer.ListenAndServe() err := s.httpServer.ListenAndServe()
if err != nil && !errors.Is(err, http.ErrServerClosed) { if err != nil && !errors.Is(err, http.ErrServerClosed) {
s.log.Error("listen error", "error", err) s.log.Error("listen error", "error", err)
err = s.shutdowner.Shutdown(fx.ExitCode(1)) if s.cancelFunc != nil {
if err != nil { s.cancelFunc()
s.log.Error("shutdown request failed", "error", err)
} }
} }
} }
+63 -52
View File
@@ -3,10 +3,12 @@ package server
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"net/http" "net/http"
"os"
"os/signal"
"syscall"
"time" "time"
"github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go"
@@ -25,10 +27,6 @@ const (
SentryFlushTimeout = 2 * time.Second SentryFlushTimeout = 2 * time.Second
) )
// errStillProcessing is returned by the server's stop hook when images are
// still being processed once ShutdownTimeout has passed.
var errStillProcessing = errors.New("images still being processed at shutdown")
// Params defines dependencies for Server. // Params defines dependencies for Server.
type Params struct { type Params struct {
fx.In fx.In
@@ -38,7 +36,6 @@ type Params struct {
Config *config.Config Config *config.Config
Middleware *middleware.Middleware Middleware *middleware.Middleware
Handlers *handlers.Handlers Handlers *handlers.Handlers
Shutdowner fx.Shutdowner
} }
// Server is the main HTTP server. // Server is the main HTTP server.
@@ -48,58 +45,59 @@ type Server struct {
globals *globals.Globals globals *globals.Globals
mw *middleware.Middleware mw *middleware.Middleware
h *handlers.Handlers h *handlers.Handlers
shutdowner fx.Shutdowner
startupTime time.Time startupTime time.Time
exitCode int
sentryEnabled bool sentryEnabled bool
cancelFunc context.CancelFunc
httpServer *http.Server httpServer *http.Server
router *chi.Mux router *chi.Mux
} }
// New creates a new Server instance. Its start hook starts Sentry and the // New creates a new Server instance.
// HTTP server; its stop hook, which fx runs on SIGINT, SIGTERM or a
// shutdown request, shuts them down.
func New(lc fx.Lifecycle, params Params) (*Server, error) { func New(lc fx.Lifecycle, params Params) (*Server, error) {
s := &Server{ s := &Server{
log: params.Logger.Get(), log: params.Logger.Get(),
config: params.Config, config: params.Config,
globals: params.Globals, globals: params.Globals,
mw: params.Middleware, mw: params.Middleware,
h: params.Handlers, h: params.Handlers,
shutdowner: params.Shutdowner,
} }
lc.Append(fx.Hook{ lc.Append(fx.Hook{
OnStart: func(_ context.Context) error { OnStart: func(ctx context.Context) error {
s.startupTime = time.Now() s.startupTime = time.Now()
go s.Run(context.WithoutCancel(ctx))
err := s.enableSentry() return nil
if err != nil { },
return err OnStop: func(_ context.Context) error {
if s.cancelFunc != nil {
s.cancelFunc()
} }
s.SetupRoutes()
s.httpServer = s.newHTTPServer()
go s.serveUntilShutdown()
return nil return nil
}, },
OnStop: s.cleanShutdown,
}) })
return s, nil return s, nil
} }
// Run starts the server.
func (s *Server) Run(ctx context.Context) {
s.enableSentry()
s.serve(ctx)
}
// MaintenanceMode returns whether maintenance mode is enabled. // MaintenanceMode returns whether maintenance mode is enabled.
func (s *Server) MaintenanceMode() bool { func (s *Server) MaintenanceMode() bool {
return s.config.MaintenanceMode return s.config.MaintenanceMode
} }
func (s *Server) enableSentry() error { func (s *Server) enableSentry() {
s.sentryEnabled = false s.sentryEnabled = false
if s.config.SentryDSN == "" { if s.config.SentryDSN == "" {
return nil return
} }
err := sentry.Init(sentry.ClientOptions{ err := sentry.Init(sentry.ClientOptions{
@@ -107,42 +105,55 @@ func (s *Server) enableSentry() error {
Release: fmt.Sprintf("%s-%s", s.globals.Appname, s.globals.Version), Release: fmt.Sprintf("%s-%s", s.globals.Appname, s.globals.Version),
}) })
if err != nil { if err != nil {
return fmt.Errorf("sentry init failure: %w", err) s.log.Error("sentry init failure", "error", err)
os.Exit(1)
} }
s.log.Info("sentry error reporting activated") s.log.Info("sentry error reporting activated")
s.sentryEnabled = true s.sentryEnabled = true
return nil
} }
// cleanShutdown stops the HTTP server, waits for the images still being func (s *Server) serve(ctx context.Context) int {
// processed, then flushes Sentry. The first two share ShutdownTimeout. It ctx, cancelFunc := context.WithCancel(ctx)
// returns errStillProcessing when images are still being processed after s.cancelFunc = cancelFunc
// that, as their work is abandoned.
func (s *Server) cleanShutdown(ctx context.Context) error {
s.log.Info("shutting down")
ctxShutdown, shutdownCancel := context.WithTimeout(ctx, ShutdownTimeout) go func() {
c := make(chan os.Signal, 1)
signal.Ignore(syscall.SIGPIPE)
signal.Notify(c, os.Interrupt, syscall.SIGTERM)
sig := <-c
s.log.Info("signal received", "signal", sig)
if s.cancelFunc != nil {
s.cancelFunc()
}
}()
go s.serveUntilShutdown()
<-ctx.Done()
s.cleanShutdown(ctx)
return s.exitCode
}
func (s *Server) cleanShutdown(ctx context.Context) {
s.exitCode = 0
ctxShutdown, shutdownCancel := context.WithTimeout(
context.WithoutCancel(ctx), ShutdownTimeout)
defer shutdownCancel() defer shutdownCancel()
err := s.httpServer.Shutdown(ctxShutdown) if s.httpServer != nil {
if err != nil { err := s.httpServer.Shutdown(ctxShutdown)
s.log.Error("server clean shutdown failed", "error", err) if err != nil {
s.log.Error("server clean shutdown failed", "error", err)
}
} }
stillProcessing := s.h.WaitForProcessing(ctxShutdown)
if s.sentryEnabled { if s.sentryEnabled {
sentry.Flush(SentryFlushTimeout) sentry.Flush(SentryFlushTimeout)
} }
if stillProcessing > 0 {
s.log.Error("images still being processed at shutdown",
"count", stillProcessing)
return errStillProcessing
}
return nil
} }
-96
View File
@@ -1,96 +0,0 @@
package server
import (
"log/slog"
"net"
"testing"
"time"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/logger"
)
// shutdownRecorder is an fx.Shutdowner that sends the options of each
// shutdown request on requests.
type shutdownRecorder struct {
requests chan []fx.ShutdownOption
}
func (r shutdownRecorder) Shutdown(opts ...fx.ShutdownOption) error {
r.requests <- opts
return nil
}
// TestSentryInitFailureFailsStartup checks that a Sentry DSN that cannot be
// used makes the server's start hook fail, so fx stops what has already
// started, instead of the process exiting from a goroutine.
func TestSentryInitFailureFailsStartup(t *testing.T) {
t.Parallel()
lc := fxtest.NewLifecycle(t)
log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
if err != nil {
t.Fatalf("logger.New() error = %v", err)
}
_, err = New(lc, Params{
Logger: log,
Globals: &globals.Globals{Appname: "pixad"},
Config: &config.Config{SentryDSN: "not-a-dsn"},
})
if err != nil {
t.Fatalf("New() error = %v", err)
}
err = lc.Start(t.Context())
t.Logf("Start() error = %v", err)
if err == nil {
t.Fatal("Start() error = nil, want the Sentry initialization error")
}
}
// TestListenErrorRequestsShutdownWithExitCode1 occupies the server's port
// and checks that the listen error asks fx to shut down with exit code 1.
func TestListenErrorRequestsShutdownWithExitCode1(t *testing.T) {
t.Parallel()
busy, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", ":0")
if err != nil {
t.Fatalf("Listen() error = %v", err)
}
t.Cleanup(func() { _ = busy.Close() })
addr, ok := busy.Addr().(*net.TCPAddr)
if !ok {
t.Fatalf("listener address %v is not a TCP address", busy.Addr())
}
requests := make(chan []fx.ShutdownOption, 1)
s := &Server{
log: slog.New(slog.DiscardHandler),
config: &config.Config{Port: addr.Port},
shutdowner: shutdownRecorder{requests: requests},
}
s.httpServer = s.newHTTPServer()
go s.serveUntilShutdown()
select {
case opts := <-requests:
t.Logf("shutdown options = %v", opts)
if len(opts) != 1 || opts[0] != fx.ExitCode(1) {
t.Errorf("shutdown options = %v, want [fx.ExitCode(1)]", opts)
}
case <-time.After(5 * time.Second):
t.Fatal("no shutdown was requested after the listen error")
}
}
+8 -4
View File
@@ -1,15 +1,19 @@
#!/bin/sh #!/bin/sh
# script/cibuild: run the CI build. The Dockerfile runs the checks # script/cibuild: run the CI build. The Dockerfile runs the checks
# (make fmt-check, lint, test), so a successful build implies a green # (make fmt-check, lint, test) as build steps. This script passes a new
# repo. Generic: needs no adaptation. The Gitea workflow runs this on # CHECK_EPOCH on every run, so Docker runs those steps instead of
# push. # reusing cached results: a successful run means the checks passed on
# this tree. A plain `docker build .` can reuse them and proves nothing
# by itself. Generic: needs no adaptation. The Gitea workflow runs this
# on push.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build . epoch="$(date +%s)$$"
docker build --build-arg CHECK_EPOCH="$epoch" .
} }
main "$@" main "$@"
+7 -3
View File
@@ -1,7 +1,9 @@
#!/bin/sh #!/bin/sh
# script/docker: build the Docker image tagged with the project name. # script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname. # Identical in all repos; the tag comes from script/projectname. Like
# Generic: needs no adaptation. # script/cibuild, it passes a new CHECK_EPOCH, so the build runs the
# checks instead of reusing cached results. Generic: needs no
# adaptation.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -9,7 +11,9 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build -t "$("$SCRIPT_DIR/projectname")" . epoch="$(date +%s)$$"
docker build --build-arg CHECK_EPOCH="$epoch" \
-t "$("$SCRIPT_DIR/projectname")" .
} }
main "$@" main "$@"