Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
74aa5d231d | ||
|
|
79da9c811c | ||
|
|
4dca8e4e56 | ||
|
|
f3231a3c5a | ||
|
|
cca2e3f926 |
+270
-75
@@ -1,6 +1,6 @@
|
||||
---
|
||||
title: Repository Policies
|
||||
last_modified: 2026-07-06
|
||||
last_modified: 2026-09-08
|
||||
---
|
||||
|
||||
This document covers repository structure, tooling, and workflow standards. Code
|
||||
@@ -60,17 +60,28 @@ style conventions are in separate documents:
|
||||
prerequisite since nvm requires bash. yarn is then pinned via
|
||||
`corepack prepare yarn@<version> --activate`. Never install "latest" or "lts";
|
||||
always exact versions. `script/cibuild` runs the CI build: it changes to the
|
||||
repo root and runs `docker build .`; the Gitea workflow calls it. Four further
|
||||
scripts are our own extensions to the standard: `script/check` runs
|
||||
`script/test`, `script/lint`, and `script/fmt-check`; `script/precommit` is
|
||||
what the git pre-commit hook runs, and it calls `script/check`;
|
||||
`script/install-precommit` installs the git pre-commit hook (the `make hooks`
|
||||
target shims to it); and `script/projectname` (literally that filename) simply
|
||||
outputs the project's name. Scripts that need the name call
|
||||
`script/projectname` — e.g. `script/docker` assembles its image tag from it —
|
||||
so those scripts stay byte-identical across all repos. Repo-type-specific
|
||||
pre-commit extras (e.g. `go mod tidy` verification in Go repos) belong in
|
||||
`script/precommit`, not in the hook itself. Model scripts are at
|
||||
repo root, runs `script/bootstrap`, runs `script/check`, and builds the image
|
||||
with the version; the Gitea workflow calls it. **`script/cibuild` runs
|
||||
`script/bootstrap` first**, because the workflow checks out the repo and runs
|
||||
nothing else, while `script/fmt-check` runs the formatter on the host: on a
|
||||
pristine checkout with nothing installed the run dies there, after the
|
||||
containerised gates have passed. **The bootstrap alone is not enough**:
|
||||
`script/bootstrap` installs node and yarn under nvm and leaves neither on the
|
||||
`PATH` of the shell that called it, so a bare `yarn` still exits 127. The host
|
||||
entrypoints that need yarn — `script/fmt` and `script/fmt-check` — therefore
|
||||
source nvm for the pinned node version before invoking it, exactly as
|
||||
`script/bootstrap`'s own install step does. A runner carrying nothing but
|
||||
docker and git then gets through `script/check`. Four further scripts are our
|
||||
own extensions to the standard: `script/check` runs `script/test`,
|
||||
`script/lint` and `script/fmt-check`; `script/precommit` is what the git
|
||||
pre-commit hook runs, and it calls `script/check`; `script/install-precommit`
|
||||
installs the git pre-commit hook (the `make hooks` target shims to it); and
|
||||
`script/projectname` (literally that filename) simply outputs the project's
|
||||
name. Scripts that need the name call `script/projectname` — e.g.
|
||||
`script/docker` assembles its image tag from it — so those scripts stay
|
||||
byte-identical across all repos. Repo-type-specific pre-commit extras (e.g.
|
||||
`go mod tidy` verification in Go repos) belong in `script/precommit`, not in
|
||||
the hook itself. Model scripts are at
|
||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/script/<name>`. The README
|
||||
must document the provided scripts in an **Entrypoints** section (see the
|
||||
README requirements below).
|
||||
@@ -89,87 +100,140 @@ style conventions are in separate documents:
|
||||
contributor should be able to understand the entire development workflow by
|
||||
reading the Makefile.
|
||||
|
||||
- Every repo should have a `Dockerfile`. All Dockerfiles must run `make check`
|
||||
as a build step so the build fails if the branch is not green. For non-server
|
||||
repos, the Dockerfile should bring up a development environment and run
|
||||
`make check`. For server repos, `make check` should run as an early build
|
||||
stage before the final image is assembled. Dockerfiles install development
|
||||
prerequisites by running `script/bootstrap` rather than duplicating installs
|
||||
inline; COPY `script/` and the dependency manifests (`package.json` +
|
||||
`yarn.lock`, `go.mod` + `go.sum`, etc.) before running it so the bootstrap
|
||||
layer stays cached until dependencies change.
|
||||
- Every repo should have a `Dockerfile`, and it carries the repo's gates: a
|
||||
`lint` phase and a `test` phase, with the final stage depending on both so the
|
||||
image cannot be built unless they pass. For non-server repos the final stage
|
||||
brings up a development environment; for server repos it is the runtime image.
|
||||
Dockerfiles install development prerequisites by running `script/bootstrap`
|
||||
rather than duplicating installs inline; COPY `script/` and the dependency
|
||||
manifests (`package.json` + `yarn.lock`, `go.mod` + `go.sum`, etc.) before
|
||||
running it.
|
||||
|
||||
- **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go
|
||||
repos use a multistage build where linting runs in an independent stage based
|
||||
on the `golangci/golangci-lint` image (pinned by hash). This stage runs
|
||||
`make fmt-check` and `make lint` before the full build begins. The build stage
|
||||
then declares an explicit dependency on the lint stage via
|
||||
`COPY --from=lint /src/go.sum /dev/null`, which forces BuildKit to complete
|
||||
linting before proceeding to compilation and tests. This ensures lint failures
|
||||
surface in seconds rather than minutes, without blocking on dependency
|
||||
download or compilation in the build stage.
|
||||
- **Linting and testing run in Docker, as phases of the `Dockerfile`.** There is
|
||||
no separate lint file. `script/lint` and `script/test` each build one phase
|
||||
and nothing else:
|
||||
|
||||
The standard pattern for a Go repo Dockerfile is:
|
||||
```sh
|
||||
docker build --no-cache --target lint -t "$(script/projectname)-lint" .
|
||||
docker build --no-cache --target test -t "$(script/projectname)-test" .
|
||||
```
|
||||
|
||||
**A stage that is not the last one in the file is built only when the final
|
||||
stage's chain depends on it, or when `--target` names it.** That is why the
|
||||
two gates are always invoked by name here, and why the final stage carries a
|
||||
`COPY --from=` of a harmless file from each of them: without that edge a
|
||||
plain `docker build .` builds the last stage alone and exits 0 having linted
|
||||
and tested nothing.
|
||||
|
||||
**Every `docker build` in `script/` is tagged**, here and in
|
||||
`script/cibuild` and `script/docker`. An untagged build leaves a dangling
|
||||
image behind on every invocation, on every developer host and every CI
|
||||
runner; a tagged one replaces the previous image.
|
||||
|
||||
Inside a phase the tool is invoked directly — `golangci-lint`, `go test`,
|
||||
`eslint`, `prettier` — never through `make lint` or `script/test`, which are
|
||||
themselves a `docker build` and would recurse into a daemon that does not
|
||||
exist in a build step. Formatting is the exception and stays on the host:
|
||||
`script/fmt` writes the working tree, and `script/fmt-check` is its
|
||||
read-only twin.
|
||||
|
||||
**No lint verdict may come from a host invocation of the linter.** On a
|
||||
shared host golangci-lint reads a result cache keyed on file content rather
|
||||
than location, so a second checkout of the same content is served the first
|
||||
one's findings, and a host-global lock in `$TMPDIR` makes concurrent runs
|
||||
exit non-zero with `parallel golangci-lint is running` — a status a caller
|
||||
cannot tell from real findings. Both have produced wrong verdicts in this
|
||||
org, in both directions. A container has its own cache, its own `TMPDIR` and
|
||||
a digest-pinned binary, so neither is reachable.
|
||||
|
||||
- **Any build that runs checks is built with `--no-cache`.** Docker invalidates
|
||||
a `COPY` layer only when the copied content changes, so on an unchanged tree
|
||||
the check `RUN` is served from cache, nothing executes, and the build still
|
||||
exits 0. Every `docker build` in `script/` therefore passes `--no-cache`:
|
||||
`script/lint`, `script/test`, `script/cibuild` and `script/docker` are the
|
||||
four, and there is no fifth — `script/check` runs the two gate phases and
|
||||
`script/fmt-check`, and builds no image of its own. A bare `docker build .` is
|
||||
not evidence that anything ran: a sub-second build reporting success is a
|
||||
cache hit, not a result. Never invalidate by pruning — `docker builder prune`
|
||||
and friends destroy a build cache shared with every other build on the host.
|
||||
|
||||
- **The gate phases are separate stages, and the build stage depends on both.**
|
||||
The lint phase is based on the `golangci/golangci-lint` image (pinned by
|
||||
hash), so lint failures surface in seconds rather than after a full compile,
|
||||
and the test phase is based on the Go image. The canonical Go repo
|
||||
`Dockerfile`:
|
||||
|
||||
```dockerfile
|
||||
# Lint stage — fast feedback on formatting and lint issues
|
||||
# Lint phase
|
||||
# golangci/golangci-lint:v2.x.x, YYYY-MM-DD
|
||||
FROM golangci/golangci-lint@sha256:... AS lint
|
||||
WORKDIR /src
|
||||
COPY go.mod go.sum ./
|
||||
RUN go mod download
|
||||
COPY . .
|
||||
RUN make fmt-check
|
||||
RUN make lint
|
||||
RUN golangci-lint run --config .golangci.yml ./...
|
||||
|
||||
# Build stage
|
||||
# Test phase
|
||||
# golang:1.x-alpine, YYYY-MM-DD
|
||||
FROM golang@sha256:... AS test
|
||||
WORKDIR /src
|
||||
COPY go.mod go.sum ./
|
||||
RUN go mod download
|
||||
COPY . .
|
||||
RUN go test -timeout 90s -race -cover ./... || \
|
||||
{ echo "--- Rerunning with -v for details ---"; \
|
||||
go test -timeout 90s -race -v ./...; exit 1; }
|
||||
|
||||
# Build stage. Nothing is wanted from either phase above; the copies
|
||||
# are what make BuildKit build them first, so this stage cannot run
|
||||
# unless lint and test passed.
|
||||
# golang:1.x-alpine, YYYY-MM-DD
|
||||
FROM golang@sha256:... AS builder
|
||||
COPY --from=lint /src/go.sum /dev/null
|
||||
COPY --from=test /src/go.sum /dev/null
|
||||
WORKDIR /src
|
||||
|
||||
# Force BuildKit to run the lint stage before proceeding
|
||||
COPY --from=lint /src/go.sum /dev/null
|
||||
|
||||
COPY go.mod go.sum ./
|
||||
RUN go mod download
|
||||
COPY . .
|
||||
RUN make test
|
||||
|
||||
ARG VERSION=dev
|
||||
RUN CGO_ENABLED=0 go build -trimpath \
|
||||
-ldflags="-s -w -X main.Version=${VERSION}" \
|
||||
-o /app ./cmd/app/
|
||||
|
||||
# Runtime stage
|
||||
# Runtime stage, and the last one
|
||||
FROM alpine@sha256:...
|
||||
COPY --from=builder /app /usr/local/bin/app
|
||||
ENTRYPOINT ["app"]
|
||||
```
|
||||
|
||||
Key points:
|
||||
- The lint stage uses the `golangci/golangci-lint` image directly (it
|
||||
includes both Go and the linter), so there is no need to install the
|
||||
linter separately.
|
||||
- `COPY --from=lint /src/go.sum /dev/null` is a no-op file copy that creates
|
||||
a stage dependency. BuildKit runs stages in parallel by default; without
|
||||
this line, the build stage would not wait for lint to finish and a lint
|
||||
failure might not fail the overall build.
|
||||
- The lint phase uses the `golangci/golangci-lint` image directly (it has
|
||||
both Go and the linter), so nothing needs installing.
|
||||
- `COPY --from=<phase> /src/go.sum /dev/null` is a no-op copy whose only
|
||||
purpose is the ordering edge. BuildKit runs stages in parallel by default,
|
||||
and a stage nothing depends on is not built at all, so without these two
|
||||
lines a red gate would not fail the build.
|
||||
- Keep the runtime stage last, and if you add a stage after it, give it the
|
||||
same two copies. A plain `docker build .` builds the last stage's chain
|
||||
and nothing else.
|
||||
- If the project uses `//go:embed` directives that reference build artifacts
|
||||
(e.g. a web frontend compiled in a separate stage), the lint stage must
|
||||
(e.g. a web frontend compiled in a separate stage), the lint phase must
|
||||
create placeholder files so the embed directives resolve. Example:
|
||||
`RUN mkdir -p web/dist && touch web/dist/index.html web/dist/style.css`.
|
||||
The lint stage should not depend on the actual build output — it exists to
|
||||
fail fast.
|
||||
- If the project requires CGO or system libraries for linting (e.g.
|
||||
`vips-dev`), install them in the lint stage with `apk add`.
|
||||
- The build stage runs `make test` after compilation setup. Tests run in the
|
||||
build stage, not the lint stage, because they may require compiled
|
||||
artifacts or heavier dependencies.
|
||||
`vips-dev`), install them in the lint phase with `apk add`.
|
||||
- `ARG VERSION=dev` is declared in the stage that compiles and supplied by
|
||||
`script/docker` and `script/cibuild`; no stage may call `git describe`.
|
||||
|
||||
- Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that
|
||||
runs `script/cibuild` (which runs `docker build .`) on push. Since the
|
||||
Dockerfile already runs `make check`, a successful build implies all checks
|
||||
pass.
|
||||
runs `script/cibuild` on push, and checks out the repo as its only other step.
|
||||
That script bootstraps, runs the gate phases, and then builds the image, so a
|
||||
successful run means every check passed; a bare `docker build .` does not
|
||||
carry the same guarantee, because its gate phases may come from the cache. The
|
||||
image build is uncached and so runs the gate phases a second time. That is the
|
||||
price of the rule above, and it is worth paying: the image that ships is built
|
||||
from a run of its own gates rather than from a cache entry.
|
||||
|
||||
- Use platform-standard formatters: `black` for Python, `prettier` for
|
||||
JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with
|
||||
@@ -189,14 +253,21 @@ style conventions are in separate documents:
|
||||
module under test to verify it compiles/parses. There is no excuse for
|
||||
`make test` to be a no-op.
|
||||
|
||||
- `make test` must complete in under 20 seconds. Add a 30-second timeout in the
|
||||
Makefile.
|
||||
- `make test` must complete in under 60 seconds. That is the hard cap, and a
|
||||
suite that exceeds it fails. Under 20 seconds is the target. A suite between
|
||||
20 and 60 seconds is still green, but the overage must be filed as an
|
||||
improvement bug against that repo. Add a 90-second timeout to the test
|
||||
invocation (`go test -timeout 90s`). The backstop deliberately sits above the
|
||||
hard cap so that it catches a genuinely hung test rather than a merely slow
|
||||
one.
|
||||
|
||||
- **`make test` should use the conditional verbose rerun pattern.** Run tests
|
||||
without `-v` (verbose) first. If tests fail, automatically rerun with `-v` to
|
||||
show full output. This keeps CI logs and `docker build` output clean on
|
||||
success (just package/suite summaries) while providing full diagnostic detail
|
||||
on failure (every test case, every assertion). The general shell pattern:
|
||||
- **The test command should use the conditional verbose rerun pattern.** Run
|
||||
tests without `-v` (verbose) first. If tests fail, automatically rerun with
|
||||
`-v` to show full output. This keeps CI logs and `docker build` output clean
|
||||
on success (just package/suite summaries) while providing full diagnostic
|
||||
detail on failure (every test case, every assertion). The command lives in the
|
||||
`test` phase of the `Dockerfile`, since `script/test` builds that phase; the
|
||||
Makefile form below is the same pattern for any repo-local invocation:
|
||||
|
||||
```makefile
|
||||
test:
|
||||
@@ -209,11 +280,24 @@ style conventions are in separate documents:
|
||||
|
||||
```makefile
|
||||
test:
|
||||
@go test -timeout 30s -race -cover ./... || \
|
||||
@go test -count=1 -timeout 90s -race -cover ./... || \
|
||||
{ echo "--- Rerunning with -v for details ---"; \
|
||||
go test -timeout 30s -race -v ./...; exit 1; }
|
||||
go test -count=1 -timeout 90s -race -v ./...; exit 1; }
|
||||
```
|
||||
|
||||
`-count=1` is required on both invocations: it defeats Go's test _result_
|
||||
cache, so the target cannot report a pass it did not earn, and the rerun
|
||||
reproduces a failure instead of replaying it. It leaves the build cache
|
||||
alone, so it costs the runtime of the suite and no recompilation.
|
||||
|
||||
Note that this is a second, independent cache, stacked below the Docker
|
||||
layer cache that [issue #26](https://git.eeqj.de/sneak/prompts/issues/26)
|
||||
addresses. `CHECK_EPOCH` guarantees the `RUN make test` _step_ re-executes;
|
||||
it does not guarantee `go test` inside that step does any work, because the
|
||||
`GOCACHE` baked into earlier image layers survives into the re-executed
|
||||
step. They are two separate defects requiring two separate fixes, and a fix
|
||||
for one must not be recorded as covering the other.
|
||||
|
||||
Python example:
|
||||
|
||||
```makefile
|
||||
@@ -239,10 +323,83 @@ style conventions are in separate documents:
|
||||
must be in `.gitignore`. No exceptions.
|
||||
|
||||
- `.gitignore` should be comprehensive from the start: OS files (`.DS_Store`),
|
||||
editor files (`.swp`, `*~`), language build artifacts, and `node_modules/`.
|
||||
Fetch the standard `.gitignore` from
|
||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when setting up
|
||||
a new repo.
|
||||
editor files (`.swp`, `*~`), in-repo agent scratch directories (`.claude/`),
|
||||
language build artifacts, and `node_modules/`. Fetch the standard `.gitignore`
|
||||
from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when
|
||||
setting up a new repo. These patterns are written to `.gitignore`'s own
|
||||
semantics, in which an unanchored pattern already matches at every depth; they
|
||||
are not a `.dockerignore` and must not be transplanted into one unmodified.
|
||||
|
||||
- **`.dockerignore` does not use `.gitignore` semantics, and copying patterns
|
||||
across unmodified leaves secrets in the build context.** Docker matches with
|
||||
`moby/patternmatcher`: `filepath.Match` semantics plus a `**` extension, so
|
||||
`*` does not cross `/` and a pattern without a leading `**/` is anchored at
|
||||
the build-context root. A `.dockerignore` listing `.env`, `*.pem` and `*.key`
|
||||
therefore excludes only the copies at the repository root, while `config/.env`
|
||||
and `certs/server.key` still reach the context and can land in an image layer
|
||||
— which is more dangerous than a short file with no secret patterns at all,
|
||||
because it reads as solved and stops anyone looking. Give every
|
||||
depth-independent pattern the `**/` prefix and leave only genuinely
|
||||
root-anchored entries unprefixed: `.git`, and the repo's own host-built
|
||||
binary, written `/myapp` and never `**/myapp`, which would also match
|
||||
`cmd/myapp/` and delete the package directory from the context. Matching is
|
||||
case-sensitive, and an ALL-CAPS twin per pattern still misses `Server.Key`, so
|
||||
secret names use character ranges — `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`,
|
||||
and likewise for `.envrc` and the extensionless SSH keys. Where such a pattern
|
||||
also catches something the build needs, re-include it with a negation
|
||||
(`!docs/example.env`); deleting the pattern reopens the exposure for every
|
||||
other file it covers. Fetch the standard `.dockerignore` from
|
||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` and extend
|
||||
it with the repo's own artifacts.
|
||||
|
||||
- **In-repo agent scratch belongs in both files, written to each file's own
|
||||
semantics.** `.claude/` holds one worktree per in-flight agent — an entire
|
||||
additional checkout of the repo — so under `COPY . .` the build context
|
||||
inflates by a multiple of the repo and another session's unreviewed work can
|
||||
be copied into an image layer. In `.gitignore` the entry is `.claude/`,
|
||||
unanchored. In `.dockerignore` it is `.claude`, anchored and with **no** `**/`
|
||||
prefix, because the prefixed form would also delete any nested directory of
|
||||
that name from the build. Anchoring carries a known gap that the canonical
|
||||
`.dockerignore` states in its own comment, since consuming repos receive the
|
||||
file and not the tracker: the directory is created in the agent's working
|
||||
directory, so a repo running agents in subdirectories still ships
|
||||
`services/api/.claude/` and must add its own anchored entry there.
|
||||
|
||||
- **Excluding `.git` means `git describe` cannot run inside any build stage, and
|
||||
it fails quietly there.** In a build stage there is no repository, so
|
||||
`git describe` writes nothing to stdout, `-X main.Version=` comes out empty,
|
||||
the binary reports no version at all, and the build still exits 0. Compute the
|
||||
version on the host and thread it in as a build arg. `script/docker` and
|
||||
`script/cibuild` do this, byte-identically across repos:
|
||||
|
||||
```sh
|
||||
# Own line: a failing command substitution inside an argument does not
|
||||
# trip `set -e`, so the inline form degrades to an empty constant.
|
||||
version="$(git describe --tags --always --dirty 2>/dev/null || true)"
|
||||
[ -n "$version" ] || version="unknown"
|
||||
docker build --no-cache \
|
||||
--build-arg VERSION="$version" \
|
||||
-t "$(script/projectname)" .
|
||||
```
|
||||
|
||||
`--always` makes an untagged repo yield an abbreviated commit hash rather
|
||||
than failing, and the `[ -n "$version" ]` line is the single place the
|
||||
fallback is applied — a live check that fires on a build from an export with
|
||||
no `.git` and on a repository with no commits yet. Do not fold it into the
|
||||
substitution as `|| echo unknown`, which makes the guard unreachable. The
|
||||
Dockerfile's side is `ARG VERSION=dev` in the stage that compiles, declared
|
||||
there because `ARG` is stage-scoped; passing `VERSION` to a repo whose
|
||||
Dockerfile declares no such `ARG` is ignored and costs nothing, which is why
|
||||
the scripts stay byte-identical. One consequence for CI: the standard
|
||||
checkout action clones shallow and fetches no tags, so a repo that embeds a
|
||||
tag-derived version must set `fetch-depth: 0` on its checkout step.
|
||||
|
||||
- **Verify `.dockerignore` by enumerating the image, not by reading the
|
||||
patterns.** Plant files at the root _and_ at least two directories deep, build
|
||||
a probe image that does `COPY . .`, and list what actually landed
|
||||
(`docker run --rm --entrypoint find IMAGE /app`). The `transferring context`
|
||||
size is not a substitute: a nested secret is a few bytes, and BuildKit
|
||||
transfers only the delta from the previous build.
|
||||
|
||||
- **No build artifacts in version control.** Code-derived data (compiled
|
||||
bundles, minified output, generated assets) must never be committed to the
|
||||
@@ -258,9 +415,45 @@ style conventions are in separate documents:
|
||||
- Make all changes on a feature branch. You can do whatever you want on a
|
||||
feature branch.
|
||||
|
||||
- `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only
|
||||
manually by the user. Fetch from
|
||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`.
|
||||
- `.golangci.yml` is standardized. The vendored copy in a consuming repo must
|
||||
_NEVER_ be modified by an agent: fetch it from
|
||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml` and keep it
|
||||
byte-identical, so that no repo can quietly loosen its own linting. Linter
|
||||
configuration changes are made to the canonical copy in the `prompts` repo and
|
||||
reach consuming repos by re-vendoring; an agent may open a PR against
|
||||
canonical, which only the user merges. One list is exempt from byte-identity,
|
||||
because it cannot be written once for every repo: the `deny` list of the
|
||||
`test-support` depguard rule, where a repo names its own test-support packages
|
||||
by full import path. A repo adds entries there and changes nothing else, and a
|
||||
re-vendor carries its entries forward. The canonical golangci-lint version is
|
||||
v2.12.2 (released 2026-05-06), pinned as the digest of the lint phase's base
|
||||
image
|
||||
(`golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`,
|
||||
which reports `2.12.2 built with go1.26.2 from c0d3ddc9`). That digest is the
|
||||
only pin, since no repo installs golangci-lint on the host: bumping the
|
||||
version means changing it and nothing else.
|
||||
|
||||
- **`script/bootstrap` installs a pinned tool by comparing versions, never by
|
||||
testing presence.** An `if ! command -v <tool>; then install; fi` guard tests
|
||||
`PATH` only, so on an already-provisioned machine the pin is inert and a
|
||||
version bump is a silent no-op — while the Dockerfile, installing into a clean
|
||||
image, gets the pinned version, so a local `make check` and `make docker` can
|
||||
disagree about what the tool even is. The canonical form:
|
||||
- compares the installed version against the pin over the **whole** version
|
||||
token; a parser that stops at the first `-` reports `2.12.2` for a host
|
||||
running `2.12.2-rc1` and skips the install;
|
||||
- treats absent, non-zero, empty or unrecognised `--version` output as a
|
||||
mismatch, so the failure direction is a redundant install and never a
|
||||
skipped one;
|
||||
- after installing, re-resolves the binary the way callers do — `hash -r`,
|
||||
then through `PATH`, not through the directory the installer wrote to —
|
||||
and fails naming the resolved path, since an install that a shadowing
|
||||
binary hides succeeds while changing nothing any caller sees;
|
||||
- is actually called, and prints the version on both success paths: a
|
||||
function defined and never invoked has the same exit status and the same
|
||||
empty output as one that worked.
|
||||
|
||||
Keep it POSIX sh: no arrays, no `[[`, no `grep -P`.
|
||||
|
||||
- When pinning images or packages by hash, add a comment above the reference
|
||||
with the version and date (YYYY-MM-DD).
|
||||
@@ -379,7 +572,9 @@ style conventions are in separate documents:
|
||||
language-specific config). Everything else goes in a subdirectory. Canonical
|
||||
subdirectory names:
|
||||
- `bin/` — executable scripts and tools
|
||||
- `cmd/` — Go command entrypoints
|
||||
- `cmd/` — Go command entrypoints; thin only: one `main.go` per binary whose
|
||||
body is a single call into `internal/` or `pkg/`, no project logic in
|
||||
`cmd/`
|
||||
- `configs/` — configuration templates and examples
|
||||
- `deploy/` — deployment manifests (k8s, compose, terraform)
|
||||
- `docs/` — documentation and markdown (README.md stays in root)
|
||||
|
||||
@@ -46,6 +46,36 @@ P2: security: per-IP rate limiting on the image routes
|
||||
not pixad's limit); `miss` 70 r/s, p50 52 ms, p95 91 ms, p99 122 ms, peak 100
|
||||
MiB, one fetch per request; `herd` 74 r/s, p50 52 ms, p95 69 ms, p99 111 ms,
|
||||
peak 60 MiB, 188 fetches for 749 requests.
|
||||
- 2026-10-04 `REPO_POLICIES.md` matches the canonical copy again (closes #196):
|
||||
it is replaced, unchanged, by `prompts/REPO_POLICIES.md` from `sneak/prompts`
|
||||
`main`. The rules it adds that pixa's tree breaks are filed:
|
||||
https://git.eeqj.de/sneak/pixa/issues/202 (lint and tests as `Dockerfile`
|
||||
phases built with `--no-cache`), https://git.eeqj.de/sneak/pixa/issues/203
|
||||
(the workflow's `script/docker-smoke` step),
|
||||
https://git.eeqj.de/sneak/pixa/issues/204 (`.claude/` in `.gitignore`),
|
||||
https://git.eeqj.de/sneak/pixa/issues/205 (`.dockerignore` patterns at every
|
||||
depth), https://git.eeqj.de/sneak/pixa/issues/206 (a thin
|
||||
`cmd/pixad/main.go`) and https://git.eeqj.de/sneak/pixa/issues/208
|
||||
(`fetch-depth: 0` on the CI checkout, so the build sees the tags). Its rule
|
||||
that no build stage runs `git describe` is not followed: pixa takes the
|
||||
version from the `.git` in the build context, per
|
||||
https://git.eeqj.de/sneak/pixa/issues/166, as the copy on `sneak/prompts`
|
||||
`next` already says.
|
||||
- 2026-10-04 an integration test of the image proxy flow (closes #80):
|
||||
`TestImageProxyFlow` in `internal/server` starts the database, handlers and
|
||||
middleware from the constructors `pixad` uses, with a fresh state directory,
|
||||
and replaces only the upstream origin with a local test server. For a resize
|
||||
with a change to JPEG and for `orig`, the first request goes through the
|
||||
router, the real fetcher, libvips, the disk cache and SQLite and answers 200
|
||||
with the right content type and size and `X-Pixa-Cache: MISS`; the second
|
||||
answers `HIT` with the same image and the upstream has had one request; the
|
||||
source and the converted image are then in `cache/sources` and
|
||||
`cache/variants`, with their rows in `source_content`, `source_metadata` and
|
||||
`variant_content`. Two optional fields make this possible, which `pixad` does
|
||||
not set and the config file and environment cannot:
|
||||
`httpfetcher.Config.DialContext` connects in place of the dialer that refuses
|
||||
internal addresses, the URL and redirect checks still running, and
|
||||
`handlers.Params.Fetcher` replaces the fetcher the handlers build.
|
||||
- 2026-10-04 a URL made on the generator page with a `ttl` is tested to
|
||||
expire (closes #199): a new test in `internal/handlers` makes a URL on the
|
||||
generator page with a `ttl` of one second, checks that `/v1/e/` serves it at
|
||||
@@ -606,6 +636,5 @@ P2: security: per-IP rate limiting on the image routes
|
||||
- optional Sentry error reporting
|
||||
- comprehensive request logging
|
||||
- Prometheus performance metrics
|
||||
- integration tests for the image proxy flow
|
||||
- measure the 1k to 5k req/s target with `script/loadtest` on a machine not
|
||||
shared with other work
|
||||
|
||||
@@ -1,84 +1,9 @@
|
||||
// Command loadtest-origin is the upstream host script/loadtest points pixad
|
||||
// at. It answers every request, whatever its path, with the same generated
|
||||
// JPEG, so each new path is a new source image for pixad to fetch, and it logs
|
||||
// one line per request, so its log counts pixad's fetches.
|
||||
// at; internal/loadtestorigin says what it does.
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"image"
|
||||
"image/color"
|
||||
"image/jpeg"
|
||||
"log/slog"
|
||||
"math"
|
||||
"net/http"
|
||||
"os"
|
||||
"time"
|
||||
)
|
||||
|
||||
const (
|
||||
listenAddress = ":80"
|
||||
readHeaderTimeout = 10 * time.Second
|
||||
imageWidth = 1600
|
||||
imageHeight = 1200
|
||||
jpegQuality = 85
|
||||
)
|
||||
import "sneak.berlin/go/pixa/internal/loadtestorigin"
|
||||
|
||||
func main() {
|
||||
photo, err := makeJPEG()
|
||||
if err != nil {
|
||||
slog.Error("cannot make the image", "error", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
server := &http.Server{
|
||||
Addr: listenAddress,
|
||||
Handler: newHandler(photo),
|
||||
ReadHeaderTimeout: readHeaderTimeout,
|
||||
}
|
||||
|
||||
err = server.ListenAndServe()
|
||||
slog.Error("server stopped", "error", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
// newHandler answers every request with photo and logs the request's path.
|
||||
func newHandler(photo []byte) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
slog.Info("request", "path", r.URL.Path)
|
||||
w.Header().Set("Content-Type", "image/jpeg")
|
||||
_, _ = w.Write(photo)
|
||||
})
|
||||
}
|
||||
|
||||
// makeJPEG draws colour gradients crossed with a fine pattern, so the image
|
||||
// has detail to decode and does not compress to almost nothing.
|
||||
func makeJPEG() ([]byte, error) {
|
||||
img := image.NewRGBA(image.Rect(0, 0, imageWidth, imageHeight))
|
||||
|
||||
// red and green count up from 0 to 255 and wrap around, along each row
|
||||
// and down the image.
|
||||
var green uint8
|
||||
|
||||
for y := range imageHeight {
|
||||
var red uint8
|
||||
|
||||
for x := range imageWidth {
|
||||
img.SetRGBA(x, y, color.RGBA{
|
||||
R: red, G: green, B: red ^ green, A: math.MaxUint8,
|
||||
})
|
||||
red++
|
||||
}
|
||||
|
||||
green++
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: jpegQuality})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return buf.Bytes(), nil
|
||||
loadtestorigin.Run()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"go.uber.org/fx"
|
||||
"go.uber.org/fx/fxtest"
|
||||
|
||||
"sneak.berlin/go/pixa/internal/config"
|
||||
"sneak.berlin/go/pixa/internal/database"
|
||||
"sneak.berlin/go/pixa/internal/globals"
|
||||
"sneak.berlin/go/pixa/internal/healthcheck"
|
||||
"sneak.berlin/go/pixa/internal/logger"
|
||||
)
|
||||
|
||||
// TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided builds the handlers as
|
||||
// pixad does, in an fx app that provides no fetcher, and requests an image
|
||||
// from 192.0.2.10, which is on the allowlist and in blocked_networks. The URL
|
||||
// check accepts that address; only the dialer that refuses internal
|
||||
// addresses checks blocked_networks, so the answer is 403 only if the
|
||||
// fetcher the handlers build from the config connects with that dialer. Any
|
||||
// other dialer would try to connect until the upstream fetch timeout, which
|
||||
// is short so that the test then fails quickly.
|
||||
func TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const host = "192.0.2.10"
|
||||
|
||||
stateDir := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
SigningKey: testSigningKey,
|
||||
StateDir: stateDir,
|
||||
DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"),
|
||||
AllowlistHosts: []string{host},
|
||||
BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")},
|
||||
UpstreamFetchTimeout: 2 * time.Second,
|
||||
// With no connection slots, the fetch would fail before dialing.
|
||||
UpstreamConnections: config.DefaultUpstreamConnections,
|
||||
}
|
||||
|
||||
var h *Handlers
|
||||
|
||||
app := fxtest.New(t,
|
||||
fx.Supply(cfg),
|
||||
fx.Provide(globals.New, logger.New, database.New, healthcheck.New, New),
|
||||
fx.Populate(&h),
|
||||
)
|
||||
app.RequireStart()
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
r := chi.NewRouter()
|
||||
r.Get("/v1/image/*", h.HandleImage())
|
||||
|
||||
rec := sendGet(t, r, photoURL(host))
|
||||
checkErrorBody(t, rec, http.StatusForbidden, "forbidden")
|
||||
}
|
||||
@@ -28,6 +28,11 @@ type Params struct {
|
||||
Healthcheck *healthcheck.Healthcheck
|
||||
Database *database.Database
|
||||
Config *config.Config
|
||||
|
||||
// Fetcher, when provided, fetches upstream images in place of the
|
||||
// fetcher the handlers build from the config. Only tests provide one;
|
||||
// pixad does not.
|
||||
Fetcher httpfetcher.Fetcher `optional:"true"`
|
||||
}
|
||||
|
||||
// Handlers provides HTTP request handlers.
|
||||
@@ -36,6 +41,7 @@ type Handlers struct {
|
||||
hc *healthcheck.Healthcheck
|
||||
db *database.Database
|
||||
config *config.Config
|
||||
fetcher httpfetcher.Fetcher
|
||||
imgSvc *imgcache.Service
|
||||
imgCache *imgcache.Cache
|
||||
sessMgr *session.Manager
|
||||
@@ -59,6 +65,7 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
|
||||
hc: params.Healthcheck,
|
||||
db: params.Database,
|
||||
config: params.Config,
|
||||
fetcher: params.Fetcher,
|
||||
csrfProtect: csrfProtect,
|
||||
refererBlocklist: allowlist.New(params.Config.RefererBlocklist),
|
||||
}
|
||||
@@ -128,10 +135,12 @@ func (s *Handlers) initImageService() error {
|
||||
fetcherCfg.MaxConnections = s.config.UpstreamConnections
|
||||
fetcherCfg.BlockedNetworks = s.config.BlockedNetworks
|
||||
|
||||
// Create the service
|
||||
// Create the service. With no fetcher provided, it builds its own from
|
||||
// fetcherCfg.
|
||||
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
|
||||
Cache: cache,
|
||||
FetcherConfig: fetcherCfg,
|
||||
Fetcher: s.fetcher,
|
||||
SigningKey: s.config.SigningKey,
|
||||
Allowlist: s.config.AllowlistHosts,
|
||||
MaxConcurrentProcessing: s.config.MaxConcurrentProcessing,
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
package httpfetcher
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestNewUsesCheckedDialerWithoutDialContext checks that a fetcher built
|
||||
// without DialContext, as pixa builds it, refuses to connect to a local
|
||||
// server.
|
||||
func TestNewUsesCheckedDialerWithoutDialContext(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := startUpstream(t)
|
||||
transport := transportOf(t, New(DefaultConfig()))
|
||||
|
||||
addr := srv.Listener.Addr().String()
|
||||
|
||||
_, err := transport.DialContext(testContext(t), "tcp", addr)
|
||||
if !errors.Is(err, ErrSSRFBlocked) {
|
||||
t.Fatalf("DialContext(%s) error = %v, want ErrSSRFBlocked", addr, err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDialContextReplacesOnlyTheDialer checks that a fetcher built with
|
||||
// DialContext connects through it, while the URL check still refuses a
|
||||
// loopback URL and the redirect check a redirect to a link-local address.
|
||||
func TestDialContextReplacesOnlyTheDialer(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := startUpstream(t)
|
||||
dialer := &recordingDialer{target: srv.Listener.Addr().String()}
|
||||
|
||||
cfg := DefaultConfig()
|
||||
cfg.AllowHTTP = true
|
||||
cfg.DialContext = dialer.dialContext
|
||||
f := New(cfg)
|
||||
|
||||
if body := fetchBody(t, f, "/image"); body != imagePayload {
|
||||
t.Errorf("body = %q, want %q", body, imagePayload)
|
||||
}
|
||||
|
||||
_, err := f.Fetch(testContext(t), "http://127.0.0.1/image")
|
||||
if !errors.Is(err, ErrSSRFBlocked) {
|
||||
t.Errorf("Fetch(loopback URL) error = %v, want ErrSSRFBlocked", err)
|
||||
}
|
||||
|
||||
_, err = f.Fetch(testContext(t), upstreamURL("/redirect/private"))
|
||||
if !errors.Is(err, ErrSSRFBlocked) {
|
||||
t.Errorf("Fetch(/redirect/private) error = %v, want ErrSSRFBlocked", err)
|
||||
}
|
||||
|
||||
// The upstream server is reached through DialContext, and nothing else
|
||||
// is asked of it.
|
||||
dialed := dialer.dialedAddrs()
|
||||
if len(dialed) == 0 {
|
||||
t.Error("DialContext was never called")
|
||||
}
|
||||
|
||||
for _, addr := range dialed {
|
||||
if addr != net.JoinHostPort(testPublicHost, "80") {
|
||||
t.Errorf("DialContext was asked to connect to %s", addr)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -137,6 +137,11 @@ type Config struct {
|
||||
// BlockedNetworks are operator-supplied CIDR ranges refused by the
|
||||
// dialer, in addition to the always-enforced built-in ranges.
|
||||
BlockedNetworks []netip.Prefix
|
||||
// DialContext, when set, makes the fetcher's connections in place of
|
||||
// the dialer that refuses internal addresses; the URL and redirect
|
||||
// checks still run. Only tests set it, to reach a local server; the
|
||||
// config file and the environment cannot.
|
||||
DialContext func(ctx context.Context, network, addr string) (net.Conn, error)
|
||||
}
|
||||
|
||||
// DefaultConfig returns a Config with sensible defaults.
|
||||
@@ -190,13 +195,19 @@ func New(config *Config) *HTTPFetcher {
|
||||
config = DefaultConfig()
|
||||
}
|
||||
|
||||
// Create transport with SSRF-safe dialer. The dialer re-resolves and
|
||||
// re-checks at connect time (closing the DNS-rebinding window) against
|
||||
// both the built-in ranges and the operator-supplied blocklist.
|
||||
transport := &http.Transport{
|
||||
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
// Unless config.DialContext replaces it, the transport connects with
|
||||
// the SSRF-safe dialer, which re-resolves and re-checks at connect time
|
||||
// (closing the DNS-rebinding window) against both the built-in ranges
|
||||
// and the operator-supplied blocklist.
|
||||
dialContext := config.DialContext
|
||||
if dialContext == nil {
|
||||
dialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
return dialSSRFSafe(ctx, network, addr, config.BlockedNetworks)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
transport := &http.Transport{
|
||||
DialContext: dialContext,
|
||||
TLSHandshakeTimeout: DefaultTLSTimeout,
|
||||
MaxIdleConns: DefaultMaxIdleConns,
|
||||
IdleConnTimeout: DefaultIdleConnTimeout,
|
||||
|
||||
@@ -42,7 +42,8 @@ type Service struct {
|
||||
type ServiceConfig struct {
|
||||
// Cache is the cache instance
|
||||
Cache *Cache
|
||||
// FetcherConfig configures the upstream fetcher (ignored if Fetcher is set)
|
||||
// FetcherConfig configures the upstream fetcher built when Fetcher is
|
||||
// not set. Its AllowHTTP and MaxResponseSize are used either way.
|
||||
FetcherConfig *httpfetcher.Config
|
||||
// Fetcher is an optional custom fetcher (for testing)
|
||||
Fetcher httpfetcher.Fetcher
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
// Package loadtestorigin is the upstream host script/loadtest points pixad
|
||||
// at, run by cmd/loadtest-origin. It answers every request, whatever its path,
|
||||
// with the same generated JPEG, so each new path is a new source image for
|
||||
// pixad to fetch, and it logs one line per request, so its log counts pixad's
|
||||
// fetches.
|
||||
package loadtestorigin
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"image"
|
||||
"image/color"
|
||||
"image/jpeg"
|
||||
"log/slog"
|
||||
"math"
|
||||
"net/http"
|
||||
"os"
|
||||
"time"
|
||||
)
|
||||
|
||||
const (
|
||||
listenAddress = ":80"
|
||||
readHeaderTimeout = 10 * time.Second
|
||||
imageWidth = 1600
|
||||
imageHeight = 1200
|
||||
jpegQuality = 85
|
||||
)
|
||||
|
||||
// Run makes the image and serves it on port 80 until the server fails, then
|
||||
// exits the process with status 1.
|
||||
func Run() {
|
||||
photo, err := makeJPEG()
|
||||
if err != nil {
|
||||
slog.Error("cannot make the image", "error", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
server := &http.Server{
|
||||
Addr: listenAddress,
|
||||
Handler: newHandler(photo),
|
||||
ReadHeaderTimeout: readHeaderTimeout,
|
||||
}
|
||||
|
||||
err = server.ListenAndServe()
|
||||
slog.Error("server stopped", "error", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
// newHandler answers every request with photo and logs the request's path.
|
||||
func newHandler(photo []byte) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
slog.Info("request", "path", r.URL.Path)
|
||||
w.Header().Set("Content-Type", "image/jpeg")
|
||||
_, _ = w.Write(photo)
|
||||
})
|
||||
}
|
||||
|
||||
// makeJPEG draws colour gradients crossed with a fine pattern, so the image
|
||||
// has detail to decode and does not compress to almost nothing.
|
||||
func makeJPEG() ([]byte, error) {
|
||||
img := image.NewRGBA(image.Rect(0, 0, imageWidth, imageHeight))
|
||||
|
||||
// red and green count up from 0 to 255 and wrap around, along each row
|
||||
// and down the image.
|
||||
var green uint8
|
||||
|
||||
for y := range imageHeight {
|
||||
var red uint8
|
||||
|
||||
for x := range imageWidth {
|
||||
img.SetRGBA(x, y, color.RGBA{
|
||||
R: red, G: green, B: red ^ green, A: math.MaxUint8,
|
||||
})
|
||||
red++
|
||||
}
|
||||
|
||||
green++
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: jpegQuality})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return buf.Bytes(), nil
|
||||
}
|
||||
+1
-1
@@ -1,4 +1,4 @@
|
||||
package main
|
||||
package loadtestorigin
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
@@ -0,0 +1,302 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"database/sql"
|
||||
"encoding/hex"
|
||||
"image"
|
||||
"image/color"
|
||||
"image/jpeg"
|
||||
"image/png"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"go.uber.org/fx"
|
||||
"go.uber.org/fx/fxtest"
|
||||
|
||||
"sneak.berlin/go/pixa/internal/config"
|
||||
"sneak.berlin/go/pixa/internal/database"
|
||||
"sneak.berlin/go/pixa/internal/globals"
|
||||
"sneak.berlin/go/pixa/internal/handlers"
|
||||
"sneak.berlin/go/pixa/internal/healthcheck"
|
||||
"sneak.berlin/go/pixa/internal/httpfetcher"
|
||||
"sneak.berlin/go/pixa/internal/logger"
|
||||
"sneak.berlin/go/pixa/internal/middleware"
|
||||
)
|
||||
|
||||
// upstreamHost is the upstream host of the image URLs below. It is a
|
||||
// documentation address (RFC 5737), which the fetcher's URL check accepts as
|
||||
// public; the fetcher's dial function connects it to the test upstream server.
|
||||
const upstreamHost = "192.0.2.10"
|
||||
|
||||
// TestImageProxyFlow requests images through pixa's router, handlers,
|
||||
// upstream fetcher, image processor, disk cache and database, with only the
|
||||
// upstream origin replaced by a local test server. The first request for a URL
|
||||
// is fetched and converted; the second is served from the cache without
|
||||
// another upstream request. The source and the converted image are then on
|
||||
// disk, with their rows in the database.
|
||||
func TestImageProxyFlow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
source := encodeTestPNG(t, 64, 48)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
sizeFormat string // the <size>.<format> part of the image URL
|
||||
contentType string
|
||||
decodeConfig func(io.Reader) (image.Config, error)
|
||||
width, height int
|
||||
}{
|
||||
{"resize and convert to JPEG", "32x24.jpeg", "image/jpeg",
|
||||
jpeg.DecodeConfig, 32, 24},
|
||||
{"orig", "orig.orig", "image/png", png.DecodeConfig, 64, 48},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var upstreamRequests atomic.Int32
|
||||
|
||||
upstream := httptest.NewServer(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
upstreamRequests.Add(1)
|
||||
w.Header().Set("Content-Type", "image/png")
|
||||
_, _ = w.Write(source)
|
||||
}))
|
||||
t.Cleanup(upstream.Close)
|
||||
|
||||
s, db, stateDir := startImageProxy(t, upstream)
|
||||
target := "/v1/image/" + upstreamHost + "/photo.png/" + tt.sizeFormat
|
||||
|
||||
first := getImage(t, s, target)
|
||||
|
||||
if got := first.Header().Get("X-Pixa-Cache"); got != "MISS" {
|
||||
t.Errorf("first X-Pixa-Cache = %q, want MISS", got)
|
||||
}
|
||||
|
||||
if got := first.Header().Get("Content-Type"); got != tt.contentType {
|
||||
t.Errorf("Content-Type = %q, want %q", got, tt.contentType)
|
||||
}
|
||||
|
||||
decoded, err := tt.decodeConfig(bytes.NewReader(first.Body.Bytes()))
|
||||
if err != nil {
|
||||
t.Fatalf("decoding the image: %v", err)
|
||||
}
|
||||
|
||||
if decoded.Width != tt.width || decoded.Height != tt.height {
|
||||
t.Errorf("image is %dx%d, want %dx%d",
|
||||
decoded.Width, decoded.Height, tt.width, tt.height)
|
||||
}
|
||||
|
||||
second := getImage(t, s, target)
|
||||
|
||||
if got := second.Header().Get("X-Pixa-Cache"); got != "HIT" {
|
||||
t.Errorf("second X-Pixa-Cache = %q, want HIT", got)
|
||||
}
|
||||
|
||||
if !bytes.Equal(second.Body.Bytes(), first.Body.Bytes()) {
|
||||
t.Error("the second response is not the image the first served")
|
||||
}
|
||||
|
||||
if got := upstreamRequests.Load(); got != 1 {
|
||||
t.Errorf("upstream received %d requests, want 1", got)
|
||||
}
|
||||
|
||||
checkSourceCached(t, db, stateDir, source)
|
||||
checkVariantCached(t, db, stateDir, first.Body.Bytes(), tt.contentType)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// startImageProxy starts the components pixad's fx app builds, from a config
|
||||
// with a fresh state directory and upstreamHost on the allowlist, and with an
|
||||
// upstream fetcher that connects every upstream address to upstream. It
|
||||
// returns the server with its routes, the database and the state directory.
|
||||
func startImageProxy(
|
||||
t *testing.T, upstream *httptest.Server,
|
||||
) (*Server, *sql.DB, string) {
|
||||
t.Helper()
|
||||
|
||||
stateDir := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
SigningKey: testSigningKey,
|
||||
StateDir: stateDir,
|
||||
DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"),
|
||||
AllowlistHosts: []string{upstreamHost},
|
||||
// The test upstream server has no TLS.
|
||||
AllowHTTP: true,
|
||||
// A limit of its own, so the cache does not size itself from the
|
||||
// host's free disk space.
|
||||
CacheMaxBytes: 64 << 20,
|
||||
CacheMaxBytesExplicit: true,
|
||||
UpstreamMaxResponseSize: config.DefaultUpstreamMaxResponseSize,
|
||||
DownstreamTimeout: config.DefaultDownstreamTimeout,
|
||||
}
|
||||
|
||||
fetcherCfg := httpfetcher.DefaultConfig()
|
||||
fetcherCfg.AllowHTTP = true
|
||||
fetcherCfg.DialContext = func(
|
||||
ctx context.Context, network, _ string,
|
||||
) (net.Conn, error) {
|
||||
var dialer net.Dialer
|
||||
|
||||
return dialer.DialContext(ctx, network, upstream.Listener.Addr().String())
|
||||
}
|
||||
fetcher := httpfetcher.New(fetcherCfg)
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
mw *middleware.Middleware
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := fxtest.New(t,
|
||||
fx.Supply(cfg),
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
database.New,
|
||||
healthcheck.New,
|
||||
handlers.New,
|
||||
middleware.New,
|
||||
func() httpfetcher.Fetcher { return fetcher },
|
||||
),
|
||||
fx.Populate(&h, &mw, &db),
|
||||
)
|
||||
app.RequireStart()
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
// Requests go straight to the router, as in newTestServer; the server's
|
||||
// own start hook, which listens on a port, is left out.
|
||||
s := &Server{config: cfg, mw: mw, h: h}
|
||||
s.SetupRoutes()
|
||||
|
||||
return s, db.DB(), stateDir
|
||||
}
|
||||
|
||||
// getImage sends a GET for target to s and fails unless it answers 200.
|
||||
func getImage(t *testing.T, s *Server, target string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
s.ServeHTTP(rec, httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, target, nil))
|
||||
t.Logf("GET %s: %d, X-Pixa-Cache %s",
|
||||
target, rec.Code, rec.Header().Get("X-Pixa-Cache"))
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("GET %s status = %d, want %d; body %s",
|
||||
target, rec.Code, http.StatusOK, rec.Body.String())
|
||||
}
|
||||
|
||||
return rec
|
||||
}
|
||||
|
||||
// checkSourceCached checks that source is stored under its SHA-256 in
|
||||
// cache/sources, recorded in source_content, and that the source URL's row in
|
||||
// source_metadata points at it.
|
||||
func checkSourceCached(t *testing.T, db *sql.DB, stateDir string, source []byte) {
|
||||
t.Helper()
|
||||
|
||||
sum := sha256.Sum256(source)
|
||||
hash := hex.EncodeToString(sum[:])
|
||||
|
||||
checkFile(t, filepath.Join(stateDir, "cache", "sources", hash[0:2], hash[2:4], hash),
|
||||
source)
|
||||
|
||||
var rows int
|
||||
|
||||
err := db.QueryRowContext(t.Context(),
|
||||
"SELECT COUNT(*) FROM source_content WHERE content_hash = ?", hash,
|
||||
).Scan(&rows)
|
||||
if err != nil || rows != 1 {
|
||||
t.Errorf("source_content rows for the source = %d (error %v), want 1",
|
||||
rows, err)
|
||||
}
|
||||
|
||||
var metadataHash string
|
||||
|
||||
err = db.QueryRowContext(t.Context(),
|
||||
`SELECT content_hash FROM source_metadata
|
||||
WHERE source_host = ? AND source_path = ?`,
|
||||
upstreamHost, "/photo.png",
|
||||
).Scan(&metadataHash)
|
||||
if err != nil || metadataHash != hash {
|
||||
t.Errorf("source_metadata content_hash = %q (error %v), want %q",
|
||||
metadataHash, err, hash)
|
||||
}
|
||||
}
|
||||
|
||||
// checkVariantCached checks that the converted image served is recorded in
|
||||
// variant_content with contentType, and stored under its cache key in
|
||||
// cache/variants.
|
||||
func checkVariantCached(
|
||||
t *testing.T, db *sql.DB, stateDir string, served []byte, contentType string,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
var cacheKey, storedType string
|
||||
|
||||
err := db.QueryRowContext(t.Context(),
|
||||
"SELECT cache_key, content_type FROM variant_content",
|
||||
).Scan(&cacheKey, &storedType)
|
||||
if err != nil {
|
||||
t.Fatalf("variant_content row: %v", err)
|
||||
}
|
||||
|
||||
if storedType != contentType {
|
||||
t.Errorf("variant_content content_type = %q, want %q",
|
||||
storedType, contentType)
|
||||
}
|
||||
|
||||
checkFile(t, filepath.Join(stateDir, "cache", "variants",
|
||||
cacheKey[0:2], cacheKey[2:4], cacheKey), served)
|
||||
}
|
||||
|
||||
// checkFile checks that the file at path holds want.
|
||||
func checkFile(t *testing.T, path string, want []byte) {
|
||||
t.Helper()
|
||||
|
||||
//nolint:gosec // G304: a path under the test's state directory
|
||||
got, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Errorf("reading %s: %v", path, err)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if !bytes.Equal(got, want) {
|
||||
t.Errorf("%s holds %d bytes that are not the %d expected",
|
||||
path, len(got), len(want))
|
||||
}
|
||||
}
|
||||
|
||||
// encodeTestPNG returns an opaque width x height PNG of one color.
|
||||
func encodeTestPNG(t *testing.T, width, height int) []byte {
|
||||
t.Helper()
|
||||
|
||||
img := image.NewRGBA(image.Rect(0, 0, width, height))
|
||||
for y := range height {
|
||||
for x := range width {
|
||||
img.Set(x, y, color.RGBA{R: 200, G: 40, B: 40, A: 255})
|
||||
}
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err := png.Encode(&buf, img)
|
||||
if err != nil {
|
||||
t.Fatalf("encoding the test PNG: %v", err)
|
||||
}
|
||||
|
||||
return buf.Bytes()
|
||||
}
|
||||
Reference in New Issue
Block a user