Build the image on Debian trixie for libvips 8.16 - #69
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 095397d06a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # Local tooling and editor state. | ||
| .agents/ | ||
| .cargo/ |
There was a problem hiding this comment.
Ignoring .cargo/ drops the checked-in config.toml from every Docker build, even though that file sets the compile-time AXUM_HTTP_* names consumed by axum-prometheus. Consequently the published binary exports the crate's default metric names rather than the http_requests_duration_seconds and related names used by the repository's dashboards and monitoring documentation, silently breaking those queries; retain this config in the build context or set the same variables in the builder.
Useful? React with 👍 / 👎.
| # Matches IMGFORGE_HEALTH_CHECK_PATH's default. A deployment that renames it | ||
| # should override this too. | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ | ||
| CMD curl --fail --silent http://127.0.0.1:3000/health || exit 1 |
There was a problem hiding this comment.
When a container sets IMGFORGE_BIND to another port, IMGFORGE_PATH_PREFIX, or IMGFORGE_HEALTH_CHECK_PATH, server.rs moves the health endpoint but this probe continues requesting the fixed 127.0.0.1:3000/health. curl --help all defines --fail as failing on HTTP errors, so the resulting connection error or 404 marks a healthy customized container unhealthy; construct the probe from the runtime settings or use an invariant endpoint.
Useful? React with 👍 / 👎.
| | `RUST_LOG` | Equivalent to `IMGFORGE_LOG_LEVEL`. Pick one and use it consistently. | | ||
| | `HTTP_PROXY` / `HTTPS_PROXY` | Honoured by `reqwest` for outbound source fetches. | | ||
|
|
||
| Boolean variables accept `1`, `t`, `true`, `yes`, and `on`, case-insensitively; anything else is false. |
There was a problem hiding this comment.
This blanket statement is false for IMGFORGE_ALLOW_UNSIGNED, IMGFORGE_ALLOW_SECURITY_OPTIONS, and IMGFORGE_ONLY_PRESETS: Config::from_env still enables each only when the lowercased value equals "true". In particular, an operator following this documentation and setting IMGFORGE_ONLY_PRESETS=1 believes arbitrary transformations are blocked while the restriction is actually disabled, so either route these settings through bool_var or document their narrower syntax.
AGENTS.md reference: AGENTS.md:L27-L27
Useful? React with 👍 / 👎.
095397d to
0bec0b1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bec0b1e8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| COPY --from=builder /usr/src/imgforge/target/release/imgforge . | ||
| COPY --from=builder /usr/src/imgforge/target/release/imgforge /usr/local/bin/imgforge | ||
|
|
||
| USER imgforge |
There was a problem hiding this comment.
When the documented disk or hybrid cache configuration is used, switching to USER imgforge leaves the cache mount unwritable: a fresh named volume at /var/cache/imgforge is root-owned, while deployment/deploy.sh chowns its bind-mounted directory to the host user rather than the container user's UID. FsDeviceBuilder must create its storage files there, so these supported container configurations fail during cache initialization and the server never starts. Create and chown the intended mountpoint to a stable container UID and update the deployment permissions before dropping privileges.
Useful? React with 👍 / 👎.
| # server no longer answers on and reported unhealthy while working perfectly. | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ | ||
| CMD curl --fail --silent \ | ||
| "http://127.0.0.1:${IMGFORGE_BIND##*:}${IMGFORGE_PATH_PREFIX}${IMGFORGE_HEALTH_CHECK_PATH}" || exit 1 |
There was a problem hiding this comment.
When operators use the documented normalized forms, such as IMGFORGE_PATH_PREFIX=imgforge/ or IMGFORGE_HEALTH_CHECK_PATH=health, the application adds the leading slash and removes trailing slashes, but this probe concatenates the raw environment strings. The first example therefore produces http://127.0.0.1:3000imgforge//health while the live route is /imgforge/health, marking the container unhealthy. Fresh evidence after the earlier hard-coded-probe fix is that the replacement still bypasses normalize_path_prefix and normalize_health_check_path; mirror that normalization in the shell command or probe an invariant route.
Useful? React with 👍 / 👎.
0bec0b1 to
912ed54
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 912ed54e3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| health="${health#/}"; health="${health%/}"; \ | ||
| health="${health:-health}"; \ | ||
| curl --fail --silent \ | ||
| "http://127.0.0.1:${IMGFORGE_BIND##*:}${prefix:+/$prefix}/${health}" || exit 1 |
There was a problem hiding this comment.
When IMGFORGE_BIND selects a valid non-default interface, such as 127.0.0.2:3000 or a container-interface address, the server listens only there while this probe still connects to 127.0.0.1, so Docker reports the working container as unhealthy. Fresh evidence after the earlier configurable-probe fix is that the replacement extracts only ${IMGFORGE_BIND##*:} and continues to hard-code the host; derive a probe address from the configured bind value, translating wildcard addresses to an appropriate loopback address.
Useful? React with 👍 / 👎.
912ed54 to
e7f2b4b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7f2b4b551
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| prefix="${prefix#/}"; prefix="${prefix%/}"; \ | ||
| health="${IMGFORGE_HEALTH_CHECK_PATH:-health}"; \ | ||
| health="${health#/}"; health="${health%/}"; \ |
There was a problem hiding this comment.
When either path variable contains whitespace or repeated surrounding slashes, such as IMGFORGE_PATH_PREFIX=' //imgforge// ', the server accepts it as /imgforge because normalize_path_prefix and normalize_health_check_path use trim().trim_matches('/'), but this probe removes only one slash from each end and never trims whitespace. It therefore requests a malformed or different route and marks the healthy container unhealthy. Fresh evidence after the earlier normalization fix is that the replacement still implements weaker normalization than the configuration helpers it claims to mirror.
Useful? React with 👍 / 👎.
e7f2b4b to
a28a3d6
Compare
a28a3d6 to
2b28a99
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b28a99865
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CMD bind="${IMGFORGE_BIND:-3000}"; \ | ||
| port="${bind##*:}"; \ |
There was a problem hiding this comment.
When IMGFORGE_BIND contains surrounding whitespace, the server accepts it because normalize_bind_address trims the value, but the health check retains that whitespace. For example, IMGFORGE_BIND=' 3000 ' binds the server to 0.0.0.0:3000 while producing a curl URL whose port contains spaces, so Docker marks the healthy container unhealthy; trim bind with the same semantics before splitting it.
Useful? React with 👍 / 👎.
| curl --fail --silent \ | ||
| "http://${host}:${port}${prefix:+/$prefix}/${health}" || exit 1 |
There was a problem hiding this comment.
When the container has lowercase http_proxy or ALL_PROXY configured for source fetching and the health-check target is absent from NO_PROXY, curl sends this local probe through the outbound proxy, which commonly cannot reach the container listener and therefore reports it unhealthy. curl --manual documents that proxy environment variables are equivalent to --proxy and that NO_PROXY controls bypasses; force this probe to remain local with --noproxy '*'.
Useful? React with 👍 / 👎.
2b28a99 to
4e4dedd
Compare
a1e0209 to
f7f2be9
Compare
f7f2be9 to
2c30b01
Compare
99105a4 to
67bda1b
Compare
Ubuntu 24.04 ships libvips 8.15.1, which is why HEIF output failed at encode time with "Unsupported compression" and why preserve_hdr had nothing to attach its gain map to. Trixie ships 8.16.1. Getting AVIF and HEIF actually working took one more step: Debian packages libheif's codecs as plugins that libvips only Recommends, so a --no-install-recommends image registers savers for both formats and then cannot encode with either. Verified by encoding all seven output formats through a running container, along with Accept negotiation, the caching headers, and an animated GIF surviving a resize with its four frames. Also here because the build surfaced them: a .dockerignore, without which COPY . . shipped the local target directory into the build context and ran the builder out of disk; a dependency-only build layer; a non-root user; and a HEALTHCHECK pointing at the new /health endpoint.
67bda1b to
2d8047c
Compare
1 participant
Stacked on #68.
Ubuntu 24.04 ships libvips 8.15.1, which is why HEIF output failed at encode time with
Unsupported compressionand whypreserve_hdrhad nothing to attach its gain map to. Trixie ships 8.16.1.What the build surfaced
Getting AVIF and HEIF actually working took one more step. Debian packages libheif's codecs as plugins that libvips only
Recommends, so a--no-install-recommendsimage registers savers for both formats and then cannot encode with either — exactly the failure the startup probe now reports as an unsupported format rather than a 500.libheif-plugin-aomencandlibheif-plugin-x265are named explicitly, withdav1dandlibde265for decoding AVIF and HEIF sources.A
.dockerignorewas missing.COPY . .was shipping the localtarget/directory into the build context; with a couple of gigabytes of debug artefacts in it, the build ran the builder out of disk. This was pre-existing, just never large enough to fail before.Also here
HEALTHCHECKpointing at the new/healthendpoint.Verified by encoding all seven output formats through a running container.
Validation
Plus a real image build, and a container serving every output format, Accept negotiation, the caching headers, and an animated GIF resize.