test: trivial ddev-xhgui change for #8609 phase 2 manual test 3 - #31
Open
rfay wants to merge 29 commits into
Open
test: trivial ddev-xhgui change for #8609 phase 2 manual test 3#31rfay wants to merge 29 commits into
rfay wants to merge 29 commits into
Conversation
…ware CI/Buildkite, for ddev#8609 Phase 2 of ddev#8609: CI now detects a changed container image, waits for a maintainer's approval, and builds/pushes it automatically for both maintainer and fork PRs, without ever exposing the DockerHub push token to a job that runs fork-supplied build content. Buildkite no longer races the push. - Phase 2 of ddev#8609 Phase 1 (ddev#8612) gave every image a content-addressed tag and made `make` auto-build changed images locally. The CI/registry side was untouched: a maintainer still had to notice a container changed and manually run `push-tagged-image.yml`/`push-tagged-dbimage.yml`, and fork contributors couldn't get an image change pushed without a maintainer doing it by hand. Two new workflows split build from push so a push secret never shares a job with untrusted (fork) build content: - `image-build-push.yml` (untrusted side, no secrets anywhere): a `detect` job recomputes each image's real content hash and checks the registry directly (never trusting the tag string committed in `versionconstants.go`, so a fork can't forge it); an `approval` job gates on the new `image-push` GitHub Environment before any expensive/untrusted build work runs; a `build` job builds per-arch using the same local-build Makefile targets Phase 1's `autotag-images` already uses, then uploads a `docker save` tarball plus tag metadata as an artifact. - `image-push.yml` (trusted side): triggered by `workflow_run` once the build workflow completes, so it always runs the default-branch version of itself and never checks out or executes the PR's code. It downloads the artifact, re-validates the tag against a strict format + reserved-tag blocklist (`containers/validate-image-tag.sh`), loads and pushes each per-arch image, assembles the multi-arch manifest, and comments on the PR. New scripts: `containers/registry-tag-exists.sh` (registry existence check, the same `docker buildx imagetools inspect` idiom already used in the post-push wait-loop) and `containers/validate-image-tag.sh`, both with bash test harnesses matching `containers/autotag_test.sh`'s style. Buildkite gets a `.buildkite/wait-for-images.sh` self-guarding step (wired into `test.sh`/`perf.sh`) that polls the registry for the tags a checkout actually needs before pulling anything, instead of racing the push - implementing the "lighter" option from the issue's open Buildkite decision. The existing `push-tagged-image.yml`/`push-tagged-dbimage.yml` are untouched and remain for manual re-pushes and the 18 `ddev-dbserver` variants Phase 1 doesn't auto-build. Requires one-time setup on the test repo (see `release-management.md`'s new "One-time setup" section): create a GitHub Environment named `image-push` with required reviewers, and add `PUSH_SERVICE_ACCOUNT_TOKEN` as a secret scoped to that environment. 1. Run the new unit tests directly: `containers/registry_tag_exists_test.sh` and `containers/validate_image_tag_test.sh` (no Docker daemon or network needed). 2. On a test repo (`image-push.yml` must be on its default branch - `workflow_run` triggers only fire for the default-branch copy of the listening workflow), push a trivial change to `containers/ddev-xhgui/` on a branch and open a PR. Confirm: `detect` flags it, `approval` blocks `build` until a reviewer approves, `build` produces artifacts with no secrets referenced in that job, `image-push` fires on completion (its own approval), loads and pushes the image, and comments on the PR. 3. Negative test: hand-edit `versionconstants.go` on that branch to a bogus `latest-<fakehash>`-shaped tag and confirm `detect` ignores it (recomputes the real hash from content) and that `containers/validate-image-tag.sh` rejects a manufactured `latest`/`vX.Y.Z` tag directly. Two new bash test harnesses (`containers/registry_tag_exists_test.sh`, `containers/validate_image_tag_test.sh`) stub `docker`/use pure string checks, no daemon or network required, wired into `container-tests.yml`'s existing unit-test job alongside `autotag_test.sh`. No Go code changed. No behavior change for anyone not touching `containers/`. Requires one-time manual GitHub Environment setup (documented) before the automatic push path is live on a given repo; until then `detect`/`approval`/`build` still run harmlessly (approval job would just wait indefinitely with no reviewers configured). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…ix, for ddev#8609 The automatic build/push flow (and Phase 1's autotag-images before it) only auto-builds the default ddev-dbserver variant (mariadb_11.8). Tests exercising other db types/versions (TestDdevAllDatabases and similar) still need a manual push. Confirmed via a live test run on ddev-test/ddev that this is working as designed, not a bug - recording it as a follow-up to revisit, likely by moving full-matrix builds later in the flow rather than the pre-approval detect/build stage, since building all ~19 variants on every PR would be expensive. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…recation warning Caught via a live test run on ddev-test/ddev: image-push.yml's use of actions/download-artifact@v4 still targets Node 20 internally, which GitHub now flags as deprecated and force-runs on Node 24 anyway. v8 uses Node 24 natively with the same github-token/run-id/pattern/path inputs, so this is a straight version bump, not a behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…t, for ddev#8609 Caught via a live test run on ddev-test/ddev: unlike push-tagged-image.yml and push-tagged-dbimage.yml, image-push.yml wasn't cleaning up the intermediary <tag>-amd64/<tag>-arm64 tags after assembling the multi-arch manifest, leaving them on DockerHub permanently. Adds the same JWT-token DELETE cleanup those workflows already do, right after each docker buildx imagetools create call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…sh race Buildkite has, for ddev#8609 ## Short Summary (TL;DR) test-reusable.yml and test-wsl2-reusable.yml pull pinned images the same way Buildkite does - a fresh runner never builds a changed image itself, so they can race image-push.yml's approval/build/push exactly like Buildkite could. Adds the same wait-for-images.sh guard there too, and moves the script out of .buildkite/ since it's no longer Buildkite-specific. ## The Issue Related to ddev#8609 (phase 2). Confirmed live on ddev-test/ddev PR #30: a `containers/ddev-xhgui` change passed "Test Nginx-FPM" regardless of push status only because that test never pulls ddev-xhgui (it's gated behind XHProf mode). If the changed image had been ddev-webserver (used by nearly every test), or a test that does exercise xhgui had run, it would have raced the push and failed on the pull - `autotag.sh`'s no-op fast path trusts the committed tag without verifying the image exists anywhere, and a brand-new GitHub-hosted runner has no local build to fall back on. ## How This PR Solves The Issue - Moves `wait-for-images.sh` from `.buildkite/` to `containers/`, since both Buildkite and the GitHub-hosted test workflows need it now. - Adds a "Wait for pushed images" step to `test-reusable.yml`, right after the public-variables fetch and before anything Docker-dependent. - Adds the equivalent call to `.github/workflows/wsl2-test.sh` (the script `test-wsl2-reusable.yml` runs inside the WSL2 guest), right after Docker is confirmed ready and before `make` builds the binary. - Fixes a latent bug the new test caught: `tag="$(tag_for "$tag_var")"` silently killed the whole script under `set -e -o pipefail` when a tag var was missing from `versionconstants.go`, before ever reaching the intended "could not find..." error message. Same pattern exists in Phase 1's `autotag.sh` (`EXISTING_TAG="$(grep ... | sed ...)"`) - confirmed it has the identical silent-death bug, but left it alone since it's already-shipped code and this PR's scope is the wait-for-images guard; flagging separately. - Adds `containers/wait_for_images_test.sh` (same bash-harness style as `autotag_test.sh`), covering the fast path, eventual recovery within budget, giving up after exhausting the budget, and the missing-tag-var error path. Wired into `container-tests.yml`. ## Manual Testing Instructions Run `containers/wait_for_images_test.sh` directly (stubs `docker`/`sleep`, no daemon or network). For the real path: open a PR that changes an image test-reusable.yml/test-wsl2-reusable.yml actually depend on (e.g. `containers/ddev-webserver`) before approving the corresponding `image-push` run, and confirm the "Wait for pushed images" step polls rather than failing outright, then succeeds once the push lands. ## Automated Testing Overview New `containers/wait_for_images_test.sh`, run directly and via `container-tests.yml`'s unit-test job alongside the other containers/ bash tests. ## Release/Deployment Notes No behavior change when nothing changed under `containers/` (fast path, single registry check per image, no wait). Only affects PRs where an auto-managed image's tag doesn't yet exist in the registry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…est workflows The previous commit only staged the .buildkite -> containers rename (a mis-staged `git add` silently dropped everything else). This commit carries the actual content: the new "Wait for pushed images" steps in test-reusable.yml and wsl2-test.sh, the wait-for-images.sh silent-exit fix, the new containers/wait_for_images_test.sh, and its wiring into container-tests.yml. See the previous commit's message for the full rationale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
… for ddev#8609 ## Short Summary (TL;DR) A push to `main` or a same-repo PR (no fork involved) no longer needs any manual approval to build/push a changed container image - only actual fork PRs still go through the two-approval fork-safe flow. Reported as awkward friction on #30, where a maintainer-pushed, non-fork PR still required two approval clicks. ## The Issue Related to ddev#8609 (phase 2). The `approval` gate (before `build`) and `image-push.yml`'s own approval exist purely because of the fork threat model: GitHub won't hand secrets to a job running a fork's Dockerfile, and even if it would, you don't want to. Neither concern applies to a `push` event (only write-access collaborators can push branches to the base repo) or a same-repo PR (head and base are the same repo, exactly as trusted as `main-build.yml`, which already uses this same secret unguarded). Gating those cases behind an approval was unjustified friction, not a security requirement. ## How This PR Solves The Issue `detect` now also computes `is_fork` using the same fork-check idiom already used in `push-tagged-image.yml`/`push-tagged-dbimage.yml`/`test-pull-push-providers.yml` (`github.event.pull_request.head.repo.owner.login != github.repository_owner`, false for any non-`pull_request` event). Jobs branch on it: - `is_fork == 'true'`: unchanged `approval` -> `build` (no secrets, artifact hand-off) -> `image-push.yml` (its own approval) flow. - `is_fork == 'false'`: new `build-and-push` job builds and pushes directly per arch, then `create-manifests` assembles the multi-arch manifest, cleans up the intermediary per-arch tags, and comments on the PR if there is one. Neither job declares `environment: image-push`, so they read `PUSH_SERVICE_ACCOUNT_TOKEN` as a plain repository secret with no approval gate - the same access level `main-build.yml` already has. Updated `building-contributing.md`/`release-management.md` to describe the fork-vs-non-fork split instead of a blanket "one approval click." ## Manual Testing Instructions Replayed the `is_fork` bash logic directly against `pull_request`-from-fork, `pull_request`-same-repo, and `push` event shapes - resolves to `true`/`false`/`false` respectively. Replayed `create-manifests`' push/imagetools-create/cleanup logic against a stubbed `docker`/`curl` - confirms both `ddev-webserver` and `ddev-webserver-prod` get manifests created and per-arch tags cleaned up. On `ddev-test/ddev`: push directly to a branch (non-fork) with a container change and confirm no approval prompt appears at all before the image is pushed. ## Automated Testing Overview No new bash test files (this is workflow-YAML branching); verified via direct bash replay of the fork-detection and manifest-creation logic (see above), plus `make staticrequired`. ## Release/Deployment Notes Reduces friction for maintainer/same-repo workflows; fork PRs are unaffected and keep the full two-approval flow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…ovals, for ddev#8609 ## Short Summary (TL;DR) The pre-build and pre-push approvals both showed up as "image-push" in GitHub's approval prompt, since that's the environment name it surfaces, not the job name - making it hard to tell which one a reviewer was being asked about. The pre-build gate now uses its own `image-build-approval` environment. ## The Issue Related to ddev#8609 (phase 2). Reported after testing on ddev-test/ddev: "Review pending deployments" only distinguishes by environment name, and both approval points shared `environment: image-push`, so approvers had to dig into which workflow/job they were actually looking at. ## How This PR Solves The Issue The `approval` job in image-build-push.yml (fork-PR pre-build gate) now uses `environment: image-build-approval` instead of `image-push`. It needs no secret - only `image-push.yml`'s `push` job still needs the DockerHub token, so that's the only one that keeps `environment: image-push`. Sharpened both jobs' `name:` fields ("Approve: build this fork PR's Dockerfile(s)" / "Approve: push the built image(s) to DockerHub") for the same reason. Updated release-management.md's setup checklist to create both environments, and noted that referencing an environment that doesn't exist yet auto-creates it with no protection rules - so it's important to verify each one actually has a `required_reviewers` rule (e.g. via `gh api .../environments/<name>`) before relying on it. ## Manual Testing Instructions Open a fork PR that changes a container image and confirm the two approval prompts now name different environments (`image-build-approval` then `image-push`). Requires creating the new `image-build-approval` GitHub Environment (with required reviewers, no secret) on the test/target repo first - until then this gate would silently not gate at all. ## Automated Testing Overview No behavioral logic changed (only environment names/job names), verified via `yaml.safe_load` that both jobs' `environment`/`name` fields resolve as intended, plus the existing containers/*_test.sh suite and `make staticrequired`. ## Release/Deployment Notes Requires the one-time creation of the `image-build-approval` GitHub Environment before this lands, on both ddev-test/ddev and ddev/ddev, mirroring image-push's required reviewers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…pproval, for ddev#8609 [skip ci] Fork PRs now only need one approval (before push), not two. The `build` job never holds registry credentials, so gating it before it runs wasn't protecting a secret - it was only a compute/abuse control, and that's judged not worth the extra friction here. Related to ddev#8609 (phase 2). Following on from the previous commit (separate `image-build-approval`/`image-push` environments so the two prompts were distinguishable): on reflection, the pre-build `approval` job never had anything to protect - `permissions: contents: read`, no secrets referenced at all. Its only justification was resource/abuse control on untrusted compute (mirroring GitHub's first-time-contributor hold) - a real but lower-severity concern than the actual registry-mutation approval, and one this project is choosing to accept in exchange for one fewer manual click. Removes the `approval` job and its `image-build-approval` environment entirely. `build` now depends only on `detect` and runs immediately for fork PRs (still with zero secrets). The push-side approval on `image-push`'s environment, in the separate trusted `image-push.yml` workflow, is unchanged - it's still the only real gate. Updated `building-contributing.md`/`release-management.md` to describe the single-approval flow and drop the now-unneeded `image-build-approval` environment setup step (it was never actually created on either repo, so nothing to clean up there). Open a fork PR that changes a container image and confirm `build` starts immediately (no waiting job before it), and the only approval prompt appears before `image-push.yml`'s push step. No behavioral logic changed beyond removing a job; verified via `yaml.safe_load` that `build`'s `needs`/`environment` fields are as intended, plus the existing containers/*_test.sh suite and `make staticrequired`. Simplifies the fork-PR flow to one approval. The `image-push` environment/secret setup from previous commits is unchanged and still required. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…letions, for ddev#8609 [skip ci] ## Short Summary (TL;DR) A maintainer-pushed (non-fork) PR still triggered the image-push approval prompt, even though that path already builds and pushes directly with no gate. image-push.yml's `push` job had no fork check of its own, so it asked for approval on every completion of "Image build", fork or not. ## The Issue Observed live on ddev-test/ddev: a PR pushed by a maintainer (not a fork) still required approval before "Image push" could run, contradicting the intended design (docs already say non-fork changes need zero approval clicks). ## How This PR Solves The Issue image-push.yml is triggered by `workflow_run` on completion of "Image build" - which runs for every PR/push, not just fork ones. Only the fork path (`build` job) uploads artifacts for it to find; the non-fork path (`build-and-push`/`create-manifests`) already pushed directly in the other workflow. But the `push` job's `environment: image-push` gate had no condition tied to fork status, so GitHub created an approval request before the job's steps (including the "no artifacts" fallback) ever ran. Added a job-level `if:` comparing `workflow_run.head_repository.full_name` to `workflow_run.repository.full_name` - the standard fork signal for `workflow_run` events - so the job (and its environment gate) is skipped entirely for non-fork completions. ## Manual Testing Instructions Push a container change directly on ddev-test/ddev (no fork) and confirm no approval prompt appears anywhere in the run. Open a fork PR with a container change and confirm the push-side approval still appears as before. ## Automated Testing Overview YAML-only change; verified with `python3 -c "import yaml; yaml.safe_load(...)"` and `make staticrequired`. ## Release/Deployment Notes No behavior change for forks. Removes an unnecessary/unintended approval prompt for maintainer-pushed changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…ers, for ddev#8609 [skip ci] ## Short Summary (TL;DR) wait-for-images.sh checked ddev/<image> instead of the repo's actual DOCKER_ORG, so on ddev-test/ddev it polled a registry the image was never pushed to and timed out even after the real push succeeded. ## The Issue Observed live on ddev-test/ddev (PR #30): "Wait for pushed images" polled `ddev/ddev-webserver:<tag>` for 20 attempts and gave up, even though the image had already landed at `ddevhq/ddev-webserver:<tag>`. test-reusable.yml and test-wsl2-reusable.yml never exported DOCKER_ORG, so the script's `DOCKER_ORG="${DOCKER_ORG:-ddev}"` fallback silently used the wrong org on any repo where vars.DOCKER_ORG isn't "ddev". ## How This PR Solves The Issue Added `DOCKER_ORG: ${{ vars.DOCKER_ORG }}` to test-reusable.yml's job-level env (same pattern main-build.yml already uses), and to test-wsl2-reusable.yml's job-level env plus its `wsl -u testuser` export list, since that job crosses into a WSL2 shell that doesn't inherit GitHub Actions env directly. Buildkite's test.sh/perf.sh are unchanged - they only ever target the real ddev/ddev registry and have no vars.DOCKER_ORG equivalent to read. ## Manual Testing Instructions Re-run a GitHub-hosted test job on a ddev-test/ddev PR with a pushed container change and confirm "Wait for pushed images" checks ddevhq/... and passes on the first attempt once the image is up. ## Automated Testing Overview YAML-only change; verified with `python3 -c "import yaml; yaml.safe_load(...)"` on both files. ## Release/Deployment Notes No effect on ddev/ddev, where vars.DOCKER_ORG is already "ddev". Fixes the check for any repo (like ddev-test/ddev) using a different org. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
… [skip ci] ## Short Summary (TL;DR) The old default (20 attempts x 15s = 5 minutes) was already shorter than ddev-webserver's ~6-8 minute build time alone, before counting a fork PR's approval-click delay or the push/manifest steps afterward. Bumped to 40 x 30s (~20 minutes). ## The Issue Racing test jobs (test-reusable.yml, test-wsl2-reusable.yml, Buildkite test.sh/perf.sh) call wait-for-images.sh before pulling any DDEV image. Its old default budget was too short to reliably outlast a real ddev-webserver build, so it would likely give up before the image ever became available - not just in the slow "waiting on maintainer approval" case, but even in the ordinary same-repo build-and-push path with no human delay at all. ## How This PR Solves The Issue Raised WAIT_FOR_IMAGES_ATTEMPTS/WAIT_FOR_IMAGES_SLEEP defaults from 20/15 to 40/30 in containers/wait-for-images.sh, giving ~20 minutes of headroom. Callers can still override both via env if needed; none of the existing callers (test-reusable.yml, test-wsl2-reusable.yml, .buildkite/test.sh, .buildkite/perf.sh) set these explicitly, so they all pick up the new defaults. ## Manual Testing Instructions None beyond the existing unit test - this only changes two default numbers. ## Automated Testing Overview containers/wait_for_images_test.sh already overrides both env vars in every scenario it exercises, so it's unaffected; ran it directly to confirm. ## Release/Deployment Notes Racing GitHub-hosted/Buildkite jobs will wait longer (up to ~20 min instead of ~5) before giving up on a missing image, trading a bit of idle runner time for far fewer spurious "gave up waiting" failures on the common slow-build path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…ionconstants.go, for ddev#8609 [skip ci] ## Short Summary (TL;DR) wait-for-images.sh read its expected tag straight from versionconstants.go, but that file's committed tag only has its hash kept current locally - the branch prefix can be stale from whenever that image last actually changed. It now recomputes the tag from real content the same way image-build-push.yml's detect job does, so it always checks the tag CI actually built. ## The Issue Observed live on ddev-test/ddev (PR #30): a GitHub-hosted test job waited the full retry budget and gave up on `ddevhq/ddev-traefik-router:main-c96123b524`, even though the image had been pushed - just under `ddevhq/ddev-traefik-router:<current-branch>-c96123b524`. The hash matched; only the branch prefix was wrong. autotag.sh only rewrites versionconstants.go's committed tag string when the *hash* changes (containers/autotag.sh:73-79), so an image untouched since some earlier branch keeps that branch's name in its committed tag indefinitely - a stale value the rest of this design already explicitly refuses to trust (detect never reads it either). ## How This PR Solves The Issue wait-for-images.sh now takes a required WAIT_FOR_IMAGES_BRANCH and recomputes each image's tag via hash-paths.sh, using the same repo_suffix/hash-paths list and `<branch>-<hash>` formula as image-build-push.yml's detect job - never reading versionconstants.go at all. Threaded WAIT_FOR_IMAGES_BRANCH through every caller: test-reusable.yml and test-wsl2-reusable.yml set it to `github.head_ref || github.ref_name` (matching detect exactly), and .buildkite/test.sh/perf.sh set it to `$BUILDKITE_BRANCH`. Rewrote wait_for_images_test.sh to compute expected tags via the real hash-paths.sh against this checkout's actual content instead of a fabricated versionconstants.go fixture. ## Manual Testing Instructions Re-run a GitHub-hosted or Buildkite test job on a ddev-test/ddev PR with a pushed container change and confirm "Wait for pushed images" checks `<branch>-<hash>` and passes without timing out. ## Automated Testing Overview containers/wait_for_images_test.sh rewritten and passing (fast path, delayed-recovery, give-up, and a new required-WAIT_FOR_IMAGES_BRANCH check). Ran `make staticrequired` clean. ## Release/Deployment Notes No effect on the actual build/push decision (detect's logic is unchanged) - only fixes what the downstream wait check looks for. Every caller of wait-for-images.sh must now set WAIT_FOR_IMAGES_BRANCH; all current callers do. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
…h tag, for ddev#8609 [skip ci] ## Short Summary (TL;DR) wait-for-images.sh previously only logged while waiting or giving up; a run where every image was already available printed nothing at all, making it hard to confirm at a glance which tags it actually checked. ## The Issue Useful while diagnosing the tag-mismatch bug fixed in the previous commit: there was no positive confirmation of which image:tag the script found, only silence on success or a wait/give-up message on failure. ## How This PR Solves The Issue Added `echo "wait-for-images.sh: found ${image_repo}:${tag}"` right after the existence check succeeds, for every image, not just ones that had to wait. ## Manual Testing Instructions Run containers/wait-for-images.sh in a repo where all tags already exist and confirm one "found ..." line prints per image. ## Automated Testing Overview Extended wait_for_images_test.sh's fast-path case to assert the confirmation line appears; ran the suite. ## Release/Deployment Notes Log-output only; no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqmDPsdWxQjwGu3LFNzGLg
wait-for-images.sh computed <current-branch>-<hash>, but the tag ddev pulls is the one committed in versionconstants.go, and autotag.sh rewrites that line (branch prefix included) only when the hash changes. The two agree only on a branch that changed the image, so any pull request not touching containers/ would poll for 20 minutes and fail every Buildkite and GitHub test job. detect had the mirror-image bug: it re-pushed all five images under a fresh branch prefix on any containers/ change, no-op or not. containers/required-image-tag.sh now resolves the tag once, for both callers, and says which case applies: "committed" (hash still matches, wait for that exact tag) or "recomputed" (content changed, make builds it locally, nothing to wait for). wait-for-images.sh no longer needs a branch name at all, which removed the WAIT_FOR_IMAGES_BRANCH plumbing from its four callers. Also from review: - github.head_ref reached a run: block spliced into the script. Git ref names permit quotes and backticks, and detect emits is_fork, so injected code could set is_fork=false and route fork content into build-and-push, the job that loads PUSH_SERVICE_ACCOUNT_TOKEN. It now arrives via env:, and is_fork moved to its own step. - image-push.yml validated the tag but pushed to whatever repository names the fork-produced artifact listed. validate-image-repo.sh constrains them to $DOCKER_ORG plus a known suffix. - image-push.yml lacked actions: read for a cross-run artifact download, and continue-on-error turned that into a "nothing needed pushing" comment. A new ungated check-artifacts job gates the environment job instead, so a fork build with nothing to push no longer asks for an approval, and a failed download is now fatal. - Artifact retention 1 -> 7 days; the gate is a human approval. - DDEV_IMAGE_TAG was not passed, so com.ddev.image-tag recorded <tag>-<arch> rather than the tag people pull, which imageVersionMismatch() compares against. - DOCKER_ORG falls back to ddev instead of producing "/ddev-webserver". - validate-image-tag.sh's reserved-literal and vX.Y.Z checks were unreachable behind the format check; they now test the part before the hash, rejecting latest-0123456789 and v1.2.3-0123456789. - containers/image-configs.sh replaces the image list that was duplicated across wait-for-images.sh, image-build-push.yml, and a test. - BSD wc -l padding failed 5 checks on macOS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…8609 GetDBImage() builds every variant's reference from one shared BaseDBTag, so a change under containers/ddev-dbserver moves the tag for all 20 at once. But `make` built only mariadb_11.8 and the automatic flow pushed only ddev-dbserver-mariadb-11.8, leaving the other 19 referenced at a tag that existed nowhere. TestDdevAllDatabases and every test pinning a non-default database (db_test.go, snapshot_test.go, config_test.go, debug-migrate-database_test.go) would fail after any dbserver change. Introduced by phase 1 (ddev#8612): before that, BaseDBTag was hand-bumped after someone ran push-tagged-dbimage.yml for all 20, so the tag never moved ahead of the images. detect's matrix now covers every variant. It emits one entry per (image, arch) rather than a cross product, because the four oldest variants are amd64-only, and create-manifests takes its arch list from detect instead of assuming both. Artifact names key on repo_suffix rather than make_dir, which all 20 db variants share and would have collided on. wait-for-images.sh now fails fast, naming the command to run, when a non-locally-built image is out of date. Those 19 variants have no local fallback, and the tag `make` would invent depends on the runner's branch name (detached HEAD on a PR checkout), so it need not match what CI pushed. The variant list was duplicated four ways - this Makefile's three target lists, push-tagged-dbimage.yml's matrix and MULTI_ARCH_IMAGES, its multi-arch case statement, and image-configs.sh. It now lives in containers/ddev-dbserver/variants.txt, rendered per consumer by variants.sh. The generated lists are byte-identical to the ones they replace, verified for both host arches. validate-image-repo.sh drops its name pattern for the exact list, so a correctly-shaped repository that isn't published can't slip past. variants.txt sits inside the hashed dbserver directory deliberately: adding a database version has to change the content hash, or detect would find the tag already present and never build the new variant. That also means this commit moves BaseDBTag, so CI has to push all 20 variants before the non-default database tests can pass - the first live exercise of the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…[skip ci] Adds the two things that have never run in CI and no harness covers: that a ddev-dbserver change builds and pushes all 20 variants (this PR is itself the first live test, since adding variants.txt moved BaseDBTag), and that push-tagged-dbimage.yml still works after its matrix was moved to variants.sh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…it, for ddev#8609 push-tagged-image.yml and push-tagged-dbimage.yml required a tag input whose description only mentioned release versions (v1.25.0). Since phase 1 they have a second use - publishing the content-addressed tag a branch needs - and that tag is derivable, so typing it by hand is both unnecessary and the way it ends up not matching versionconstants.go. That is exactly what happened pushing the db images for this branch: main-1dc90407ef was entered where the checkout needed 20260814_rfay_docker_update_phase_2-1dc90407ef, and the two don't interoperate. The input is now optional. Left empty, a resolve step derives the tag through the new containers/image-tag-for.sh, a lookup over image-configs.sh, so it matches versionconstants.go by construction. Supplied, it is used as-is, which is still what a release push wants. Either way the result goes through validate-image-tag.sh (or the release-tag shape) before anything is pushed. An image the automatic flow doesn't cover - test-ssh-server - has no hash to derive, so the resolver fails with that explanation rather than inventing one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tag was <branch>-<hash>, but only the hash carries meaning: two tags with the same hash suffix are the same content by construction. Storing the whole string in versionconstants.go and matching it exactly turned a decorative prefix into a coordination requirement spanning the branch that built the image, the organization it was pushed to, and every consumer. That is not hypothetical. Pushing this branch's db images to the test org produced main-1dc90407ef while the checkout needed 20260814_rfay_docker_update_phase_2-1dc90407ef - identical content, two strings, no interoperability, and the same shape as workarounds hit before. Tags are now the bare hash. The same content resolves to the same tag on any branch, in any fork, in any organization, so autotag.sh, detect, and every test runner agree with no coordination at all. Readability is preserved three ways rather than in the tag: - autotag.sh writes the <branch>-<hash> alias as a trailing comment on the versionconstants.go line, so the file still says where the image came from. - A companion <Name>TagBranch variable carries the same thing as data, and `ddev version` gains an image-tag-branches row - collapsed to one branch name when every image came from the same one, expanded when they didn't. - Both push paths publish the <branch>-<hash> alias next to the hash tag off the same manifest, for anyone browsing the registry. The alias branch comes from a trusted context, is sanitized, and goes through validate-image-tag.sh, which now accepts both forms - so a fork branch named v1.25.0 cannot publish something that reads as a release. Comparison against versionconstants.go is now exact rather than a trailing hash match, so a line still in the old form is stale and `make` migrates it. wait-for-images.sh no longer has to fail when a non-locally-built image is out of date: the tag it would need is branch-independent, so it is the same string image-build-push.yml pushes, and waiting for it is well defined. Note for deployment: this invalidates every published content-hash tag, so the first run has to build and push all 24 images (44 jobs). Until it does, integration tests on this branch cannot pull. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the republish-everything step as must-test item 0: the scheme change invalidates every existing content-hash tag, so the first CI run has to push all 24 images before integration tests can pull. Notes that the manual db push now has two paths to test (derived tag and explicit release tag). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… for ddev#8609 image-build-push.yml and image-push.yml publish a <branch>-<hash> alias next to the bare hash tag, so the registry tag list still says which branch produced an image. push-tagged-image.yml and push-tagged-dbimage.yml did not, so a manual push - the path used to seed the tags for this branch - left a bare hash with nothing readable beside it. Both now derive the alias in the same step that resolves the tag, sanitize the branch, and run it through validate-image-tag.sh before use. Only a content hash gets one; a vX.Y.Z release tag is already readable. The four amd64-only db variants are pushed straight to the final tag and never reach the manifest loop, so they get the alias in a separate pass over the variant list rather than being silently skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ns, for ddev#8609 With tags reduced to bare content hashes, nothing on an image said where it came from. containers/image-metadata.sh is now the single definition of that metadata - source, url, documentation, vendor, licenses, revision, version, created, title - rendered in whichever form a caller needs. Labels and annotations both, because they are not interchangeable: - Labels live in the image config, travel with a docker pull, and show up in docker inspect offline. That is what a support report can rely on, and it is where the commit now appears. - Annotations live on the manifest index, which is what a tag points at, so they describe the tag as a whole rather than one platform and a registry can show them without pulling. A tag has no comment field; this is the closest standard equivalent. Labels are wired into containers_shared.mk (covering traefik, ssh-agent and xhgui), ddev-webserver's own build rule, and ddev-dbserver/build_image.sh, which deliberately does not include the shared makefile. Annotations are added at all four imagetools create sites. One caveat worth knowing: imagetools inherits the source media type, and a Docker manifest list has nowhere to store annotations. Images whose per-arch builds came from buildx are OCI indexes and keep them; images built with the classic builder are not and silently would not. Both automatic paths now check the result and log when annotations were dropped instead of leaving it to be discovered later. Existing published images keep whatever they were pushed with - image-metadata.sh is not in any image's hash paths, so this does not force a rebuild. New pushes pick the metadata up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding the label wiring touched containers_shared.mk, ddev-webserver/Makefile and ddev-dbserver/build_image.sh, all of which are inside their images' hash paths, so every tag moved. `make` rebuilt the changed images locally and rewrote versionconstants.go; without this the committed tags would name images CI is no longer building. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ddev#8609 Adding the OCI labels changed containers_shared.mk and two other files inside image hash paths, which moved every tag - and nothing caught that the commit went out without running `make`, leaving versionconstants.go naming images CI no longer builds. The pre-commit and pre-push hooks both run `make staticrequired`, so that is the place to notice. containers/check-image-tags.sh compares each tag against the current content hash and fails naming the stale variables and the fix. It is a check rather than a fix on purpose: correcting it means building the changed image, which is minutes of Docker work that has no business running inside a commit hook. No Docker and no network here - it costs about a second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same-repo PR test on ddev-test/ddev: confirm only xhgui builds via build-and-push with no approval gate, per HANDOFF.md manual test 3.
|
Pushed updated container image(s) for this PR:
|
|
Download the artifacts for this pull request:
See Testing a PR.
|
rfay
force-pushed
the
main
branch
2 times, most recently
from
August 21, 2026 12:02
ec1c2db to
7dbe495
Compare
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
1 participant
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.
Short Summary (TL;DR)
Manual test 3 from HANDOFF.md for PR ddev#8707 (ddev/ddev): a trivial same-repo
ddev-xhguiDockerfile change to confirmdetectbuilds only xhgui andbuild-and-pushruns with no approval gate.The Issue
How This PR Solves The Issue
Adds a comment line to
containers/ddev-xhgui/Dockerfileand regeneratespkg/versionconstants/versionconstants.goviamake, which bumpsXhguiTag/XhguiTagBranchfor this branch. This is the minimal change needed to exercisedetect→build-and-push→create-manifestsfor a single image on a same-repo (non-fork) branch.Manual Testing Instructions
Watch this PR's checks on
ddev-test/ddev:detectshould list only xhgui as changed.build-and-pushshould run both arches with no approval gate.create-manifestsshould comment on this PR.docker buildx imagetools inspect ddev/ddev-xhgui:<tag>shows both platforms andcom.ddev.image-tagis the final tag, not<tag>-amd64.Automated Testing Overview
None; this is a manual CI exercise, not a code change requiring new tests.
Release/Deployment Notes
None; this branch is for testing on
ddev-test/ddevonly and will not be merged.