update infrastructure for 3.0#323
Merged
turegjorup merged 78 commits intoMay 4, 2026
Merged
Conversation
…ev images to os2display
turegjorup
changed the title
…te-infrastructure-for-mono-repo # Conflicts: # CHANGELOG.md # composer.lock # infrastructure/itkdev/display-api-service/etc/confd/templates/env.local.tmpl # infrastructure/os2display/display-api-service/etc/confd/templates/env.local.tmpl # package-lock.json
Replace shivammathur/setup-php with docker compose runs against the phpfpm service so all PHP CI runs match the local docker setup (itkdev/php8.4-fpm). phpunit and doctrine now reuse compose's mariadb instead of GitHub services. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply prettier formatting to github_build_release.yml and pentatrion_vite.yaml; fix list/heading spacing, trailing whitespace and bare URL in infrastructure/Readme.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Revert phpfpm image to itkdev/php8.3-fpm and pin vimeo/psalm back to ^5.12.0. Restore composer.lock and symfony.lock to the release/3.0.0 state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The regex location for static files (jpg, png, etc.) had no try_files directive, so first-time requests to /media/cache/resolve/... 404'd without ever reaching Symfony. Adding the same fallback as location / lets LiipImagineBundle generate missing thumbnails on demand. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Local dev had its own minimal default.conf.template under .docker/, which lacked the production-only directives (regex static-files location, rate limiting, /health, security headers). That gap is exactly what hid the LiipImagineBundle thumbnail 404 from local testing. Move all dev compose files to mount the same templates/nginx.conf shipped in the production nginx image, parameterize the web root via NGINX_WEB_ROOT (prod default set in the Dockerfile), and delete the redundant .docker/ copies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Consolidate the develop and tag DockerHub workflows into a single multi-arch build that pushes to ghcr.io/os2display/*, authenticated via GITHUB_TOKEN. Provide repository-root as a named build context so the Dockerfiles' COPY --from=repository-root steps resolve. Add per-image GHA cache (mode=max) so multi-arch builds reuse builder stages across runs instead of re-running npm ci + composer install per platform. Align infrastructure/build-n-push.sh with the workflow: drop the turegjorup namespace, single-pass docker buildx --push for multi-arch manifests, and add a BUILD_LOAD=1 flag for local builds that load into the host docker daemon instead of pushing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The nginx Dockerfile had:
COPY --from=api_app_builder ... ./public
RUN chmod -R g+w /var/www/html
The RUN re-wrote every file's permissions, producing a second 19 MB
layer with byte-identical content. Replacing with --chmod on the COPY
itself drops the duplicate layer; image goes from 135 MB to 108 MB,
dive efficiency from 78.7% to 99.5%.
Apply the same pattern to docker-entrypoint.sh in the API Dockerfile
for consistency.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The API Dockerfile does a wholesale 'COPY --from=repository-root ./' that brings in everything not excluded. Without these additions, editing a docker-compose.yml, Taskfile.yml, or any quality-tool config (phpstan, psalm, rector, playwright) invalidated the cache from that COPY onward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The image carries three distinct config surfaces:
- .env — Symfony app config (consumed via %env(...)% in config/)
- nginx env — runtime template substitution into nginx.conf
- php-fpm env — runtime tuning consumed by the itkdev/php8.4-fpm base
Annotate .env with per-variable descriptions so it is the canonical,
self-documenting Symfony example, and add dedicated production-example
files for the nginx and php-fpm runtime surfaces. Variable names mirror
what the image and templates actually consume — no APP_* prefix
translation.
Validated by the env-coverage check from the consolidation plan: every
%env(...)% reference in config/ is declared in .env and every
${VAR} in the nginx template is declared in the nginx example.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
370: Fix thumbnail 404 regression in nginx static-file location
scripts/check-env-coverage.sh validates the documentation surface
introduced in the previous commit:
1. Every %env(...)% read in config/ is declared in .env.
2. Every variable in .env, the nginx example, and the PHP example
has a preceding description.
3. Every ${VAR} in the nginx template is declared in the nginx
env.production.example.
4. Every PHP_* env set in the API Dockerfile's production stage is
declared in the PHP env.production.example.
Wired into the existing 'coding-standards:check' task and a dedicated
GitHub Actions workflow that runs on push to develop and on pull
requests, using the official go-task/setup-task action.
Also fills in five PHP_OPCACHE_* and PHP_PM_*_SPARE_SERVERS variable
descriptions that check (2) caught as missing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r-mono-repo' into feature/5402-image-self-documenting # Conflicts: # CHANGELOG.md
Each PHP workflow currently spends ~30s on a cold composer install for the same composer.lock — across nine workflows that's roughly four minutes of duplicated work per push. actions/cache keyed on composer.lock makes the install a near no-op when the lockfile is unchanged. Dev installs share one key (vendor-php8.4-…); the doctrine workflow's --no-dev install gets its own (vendor-prod-php8.4-…) so prod and dev caches don't clobber each other. composer_install.yaml intentionally stays uncached: its purpose is validating that a fresh prod-mode install works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
phpfpm declares depends_on: mariadb (service_healthy), so every docker compose run on it pulls and boots mariadb plus waits for the healthcheck — wasted work for code-analysis, rector, twig-cs, php-cs-fixer, composer normalize/validate, and the prod-mode composer-install validator. --no-deps starts only the requested service. The compose file is unchanged; doctrine, phpunit, and playwright still pull mariadb because they actually need it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Vitest spends ~31s of its ~34s wall time on a cold npm install; Playwright pays the same toll. actions/cache keyed on package-lock.json turns the install into a near no-op when the lockfile is unchanged. node_modules lives at the repo root and is bind-mounted into the node service via .:/app, so the docker compose setup is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fail-fast only applies to matrix-strategy jobs; declaring it on a single-job workflow is dead config that adds reading overhead and implies a matrix where there is none. Stripping it from the seven workflows that carry it without any matrix:. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eight workflows carried an itk-dev/devops_itkdev-docker "do not edit" header that no longer applies in this repo. Stripping the three-line header preserves the existing ### ### Title / Description / Assumptions docs that follow it. The remaining eleven workflows had no header documentation; they get the same shape (title, one-paragraph purpose, Assumptions list) so a reader can land on any workflow file and learn what it does and what it relies on without leaving the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adapts itk-dev/devops_itksites's two-job api-spec workflow to this repo's v2 spec layout, with one behaviour change: stay quiet when the spec is unchanged. itksites' "no changes detected" comment lands on every PR that touches src/, which is most of them; suppressing that branch removes the noise while keeping the signal — the workflow now only posts on (a) committed spec out of date with the source, (b) non-breaking spec changes (changelog), or (c) breaking spec changes (changelog + failure). --edit-last keeps the PR to one in-place comment across pushes. Pinned oasdiff-action to v0.0.44 instead of @main; path filters restrict the run to PRs that could plausibly affect the spec; the breaking-changes job depends on the export job so a drifting spec isn't compared against the base. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tuj
requested changes
May 1, 2026
The mkb.aarhus.dk address is no longer the right inbox; ba.aarhus.dk is the current ITK Dev team mailbox. Updates the four LABEL maintainer lines in the API + Nginx Dockerfiles and the API Platform OpenAPI contact configuration so all surfaces point at the same place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Used only at build time (WORKDIR + COPY source) and not consumed by the nginx runtime template. Inlining /var/www/html in both places removes the indirection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every variable is referenced from default.conf.template; the comment makes that contract self-evident at the declaration site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Default behaviour is now host-platform build + --load into the local daemon. Multi-arch + --push to the registry only fires when PUSH=1 is set explicitly, so a bare invocation can no longer publish to GHCR by accident. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
images:build wraps `sh infrastructure/build.sh` (load locally). images:push runs the same script with PUSH=1 and prompts for confirmation first, since publishing to GHCR is non-reversible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cleanup pass on the API image based on the canonical reference Dockerfile (itk-dev/os2display-api-service@0b2cc18) raised in PR review: - Renamed `client_app_builder` → `assets_builder` to match canonical. - Adopted canonical's section-divider comment block. - Dropped APP_CLIENT_PATH and APP_API_PATH env indirections; literal paths read better and match the FPM convention (/var/www/html). - Dropped the dead APP_RELEASE_TIMESTAMP arg in the assets stage. - Expanded inline comments to document motivation (two-pass composer install, vite manifest ordering, release.json purpose, runtime composer for entrypoint dump-env, Prometheus exporter, ENTRYPOINT vs CMD) — short lines, one fact each. Behaviour unchanged. Smoke build via `sh infrastructure/build.sh` loads working images; resulting API image still ships the PHP_OPCACHE_*/PHP_PM_* tuning and the public/build vite manifest + public/release.json that downstream consumers expect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI cleanup: cache vendor + node_modules, --no-deps, doc each workflow, rework apispec
tuj
requested changes
May 1, 2026
|
|
||
| ## Dump dotenv files into PHP for better performance. | ||
| ## @see https://symfony.com/doc/6.4/configuration.html#configuring-environment-variables-in-production | ||
| composer dump-env prod |
Contributor
There was a problem hiding this comment.
Maybe we need an entry in README.md about the images, that links to infrastructure/Readme.md and explains what should be done if you want to change env variables with the images?
- Top-level README gains a "Container images" section that names the
two published images, links to infrastructure/Readme.md for the
build pipeline, and explains the env-var precedence operators
should rely on (compose env_file/environment > .env.local.php).
- infrastructure/Readme.md replaces the bare build script reference
with the new task entries (images:build / images:push) and expands
the "Build process" section into a numbered walk-through with the
motivation for each step.
- assets_builder no longer copies public/client/. Vite's entrypoints
live under assets/; public/client/online-check/ is a static path
the screen client polls at runtime, and reaches the final image
via the api_app_builder's repo-root copy. The same is true for the
public/media/thumbnail_*.png fallbacks that .dockerignore preserves.
Comment moved to api_app_builder where the path actually matters.
Smoke build with `sh infrastructure/build.sh` passes; final image
still carries /var/www/html/public/client/online-check/{index.html,
1pixel.png} and the two thumbnail PNGs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…te-infrastructure-for-mono-repo # Conflicts: # CHANGELOG.md
API Platform 3.4 emits empty header maps as YAML sequences
(`headers: []`), which oasdiff (strict OpenAPI 3.x parser) refuses to
load — every breaking-changes job since the workflow rework has been
falsely tripping the "breaking changes detected" branch with no
diff to show, because oasdiff aborts before comparing anything.
Workaround: materialise both base and revision specs into
.api-spec-tmp/ and rewrite `headers: []` to the spec-compliant
`headers: {}` before invoking oasdiff. Drop on upgrade to API
Platform 4.x — its exporter serialises empty maps correctly.
TODO marker in the workflow points there.
Verified: 31 malformed entries on the current spec, 0 after sed,
resulting YAML still parses.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prettier flagged the unquoted `headers: []` inside the step name as a nested mapping. Quoting the whole name turns it back into a plain scalar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Track the workflow's prior comment by an invisible HTML marker (<!-- api-spec-workflow -->) so subsequent runs update the same comment in place rather than leaving stale ones — and so we can distinguish this workflow's comment from any other bot comments on the PR. Three branches now write the marker plus content: - non-breaking changes (changelog populated, oasdiff success) - breaking changes (oasdiff failure) - resolved (no diff this run, but a prior comment exists) — only fires when there's something to clear, so a clean PR with no prior comment still produces no noise. Each branch updates the prior comment by id when found, posts a fresh comment otherwise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`hashFiles('changelog.md') != ''` was matching the file even when
oasdiff's `changelog` action wrote "# API Changelog 1 vs. 1\nNo
changes detected" to it (when the diff was empty), so the workflow
posted a "Non-breaking changes" comment whose body explicitly said
nothing changed.
Add a `Detect actual diff in changelog` step that greps for the
"No changes detected" placeholder and exposes a boolean output;
gate the non-breaking branch on actual changes, and the resolved
branch on no changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tuj
approved these changes
May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to ticket
#249
Description
Consolidates the four parallel image builds (
itkdev/*×os2display/*×display-api-service+nginx) into two GHCR-published images and rewires CI accordingly.Image build pipeline
ghcr.io/os2display/display-api-serviceandghcr.io/os2display/display-api-service-nginx(parallelitkdev/*andos2display/*trees deleted)..github/workflows/build-images.ymltriggers ondeveloppush and tag push; usesdocker/metadata-actionwithlatest=autoso3.0.0updates:latestand3.0.0-RC1doesn't.linux/amd64,linux/arm64) withprovenance: mode=maxandsbom: true.FROMstage rather than rebuilding the npm/composer stages —npm ci+composer installrun once per CI run instead of twice. Local nginx build resolved in ~1s after API was loaded; nginx image size 108 MB (vs. 170 MB on the previous design).infrastructure/build-n-push.sh(private namespace, single-platform--push) replaced with multi-archbuildx;BUILD_LOAD=1builds host-platform-only and loads to local daemon for smoke tests.Image internals
confd-templated configs into the upstreamnginxinc/nginx-unprivilegedtemplate mechanism (infrastructure/nginx/etc/templates/default.conf.template) with explicitNGINX_FPM_SERVICE/NGINX_FPM_PORT/NGINX_MAX_BODY_SIZE/NGINX_SET_REAL_IP_FROM/NGINX_PORTenv contract.ENTRYPOINT ["docker-entrypoint.sh"] / CMD ["php-fpm"]so thecomposer dump-env prod+cache:warmuppreamble runs for any command, not just the default.set -eu(no-x); composer-1 conditional removed (image only ships composer 2).ENVblock in the API Dockerfile collapsed to a single declaration (blank lines between\continuations are not portable).APP_RELEASE_VERSIONbuild arg dropped (declared but never referenced)./healthlocation is exact-match (location =) instead of prefix.dotenv / Symfony config surface
.envextended to be the documented source of truth for every variable read via%env(...)%inconfig/. Each variable carries a description comment.scripts/check-env-coverage.shenforces both directions in CI via.github/workflows/env_coverage.yaml: every%env(VAR)%consumed inconfig/must be declared in.env, and every variable in.envmust have a preceding description comment.config/packages/pentatrion_vite.yamladded.Compose / dev workflow
docker-compose.yml,docker-compose.server.yml,docker-compose.override.ymlupdated for the newphpfpm+nginxtopology and for theos2display↔displayproject rename./health) so Playwright gates on it.Taskfile.ymlgainscoding-standards:env:check.CI workflow cleanup
apispec.yaml,phpunit.yaml,code-analysis.yaml,composer_install.yaml,doctrine.yaml,rector.yamlsimplified to the samedocker compose run --rm phpfpm …shape.actions/checkout@v6everywhere;markdown.yaml,styles.yaml,twig.yaml,vitest.yaml,yaml.yaml,composer.yaml,javascript.yaml,php.yamlaligned.playwright.yamlgates on the nginx healthcheck.itkdev_docker_build_*.ymlandos2display_docker_build_*.ymlworkflows deleted.github_build_release.ymlwritesrelease.jsoninto the tarball and emits pre-releases for tags containing-(mirrorslatest=auto)..git-blame-ignore-revsadded so the bulk style commit doesn't pollutegit blame..dockerignoreSmoke test
BUILD_LOAD=1 sh infrastructure/build-n-push.shbuilds both images locally end-to-end. Verified the nginx image carries/var/www/html/public/release.jsonandpublic/build/.vite/, the API container exposes the fullPHP_OPCACHE_*/PHP_PM_*env surface, and the API entrypoint runsset -eu.Screenshot of the result
N/A
Checklist
Additional comments or questions
Two follow-ups intentionally deferred to subsequent PRs (per the consolidation plan):
releaseTimeand one-placerelease.jsonwrite..env.production.examplein the API image so operators can bootstrap.env.localviadocker run --rm <image> cat ….