Repository navigation
hosting-image-mirror: copy the pinned platform images from ACR into the fleet registry when it lacks them (Plugins#1722) - #4110
Conversation
…he fleet registry when it lacks them (MeshWeaver.Plugins#1722) cr.meshweaver.cloud serves only what was pushed to it; CD publishes to ACR. A record pinning a tag the fleet registry lacked sent a human to `crane copy` as the registry's publisher (runbook step 4) — or, before #1546, into ImagePullBackOff at step 11. This command is that copy, for the portal image and (--also) the migration image, which must ride the same tag (Memex#141): present → KEPT, never re-pushed; digest compared with ACR's, a difference is the fact image_drift (a re-tagged upstream build is the operator's call). absent → the source is asked (absent there too → refused naming the sealed set to pin), crane copy runs, BOTH digests are read back and must match. Credentials: the publisher password from the vault and the ACR token from `az acr login --expose-token` reach crane on stdin (--password-stdin) into a per-process DOCKER_CONFIG — never argv, never printed, never a fact. The ACR short name derives from the source host; a non-ACR source is refused (the fleet's upstream is ACR). Rights: AcrPull on the source registry for the operator identity, not granted today — without it the step is RED naming the grant and the crane commands. Dockerfile: crane (go-containerregistry v0.20.3, per TARGETARCH) — nothing else in the image can do a manifest+blob copy. Tests: 35 cases against crane/az stubs that record logins by host+user only. Workflow: plan list, floor 24, stubs, tool check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8914176 to
cac1703
Compare
|
Corrected the tracked-file floor in 🤖 Generated with Claude Code |
|
Why The push credential is deliberately the registry's 🤖 Generated with Claude Code |
There was a problem hiding this comment.
🔵 Needs a closer look
Critical and moderate findings remain in argument handling, registry error classification, drift detection, validation, and workflow completeness.
Pull request overview
Adds hosting-image-mirror to copy pinned platform images from ACR into the fleet registry with credential isolation, digest verification, tests, and deployment integration.
Changes:
- Adds multi-image mirroring, validation, drift detection, and secure authentication.
- Adds Crane tooling, Azure/Crane stubs, and behavior tests.
- Updates lifecycle documentation and workflow image checks.
File summaries
| File | Review summary |
|---|---|
deploy/aks/operator/test/stubs/image-mirror/crane |
Adds Crane test stub; no final comments. |
deploy/aks/operator/test/stubs/image-mirror/az |
Adds Azure CLI test stub; no final comments. |
deploy/aks/operator/test/run-tests.sh |
Nit (2 votes): read-back digest mismatch is not tested. Nit (1 vote): copy-failure coverage does not exercise the mismatch guard; add successful-copy/wrong-digest coverage. |
deploy/aks/operator/Dockerfile |
Adds architecture-specific Crane tooling; no final comments. |
deploy/aks/operator/bin/hosting-image-mirror |
Critical (1 vote): missing argument handling can loop indefinitely. Critical (1 vote): registry errors are treated as absent images. Moderate (3 votes): all-present images skip source drift comparison. Moderate (2 votes): case-sensitive self-mirror validation can be bypassed. Moderate (1 vote): source validation permits multi-label ACR hosts. Nit (2 votes): read-back digest mismatch lacks test coverage. |
deploy/aks/INSTANCE-LIFECYCLE.md |
Nit (1 vote): documents AcrPull but omits the required ACR Reader grant. |
.github/workflows/hosting-operator.yml |
Moderate (1 vote): tracked-file floor is too low for the 22-command plan list and supporting files. |
Review details
Suppressed comments (3)
.github/workflows/hosting-operator.yml:69
- The plan list below contains 22
hosting-*commands, not 21; withrun.sh,_common.sh, and_audit.jq, the tracked-file floor should be 25. At 24, losing one of the 25 files still passes this guard, so a clean checkout can ship an incomplete operator image without CI noticing.
if [ "$tracked" -lt 25 ]; then
echo "::error::only ${tracked} file(s) tracked under deploy/aks/operator/bin/ — expected 25 (22 commands + run.sh + _common.sh + _audit.jq). The floor is the REAL count — raise it with every command added, or a lost file passes."
deploy/aks/INSTANCE-LIFECYCLE.md:127
- This lifecycle summary says the operator only needs
AcrPull, but the command's own refusal and the provisioning requirements also requireReaderon the ACR resource soaz acr login --name ... --expose-tokencan resolve the registry. Omitting that grant sends operators to provision an incomplete permission set and leaves the step red.
argv, never printed. Needs `AcrPull` on the source registry for the operator identity.
deploy/aks/operator/test/run-tests.sh:403
HOSTING_IM_COPY_FAIL=1makescrane copyexit before a destination is written, so this only covers the command-failure branch. It never exercises the read-back mismatch guard at lines 141–143; a regression that accepts a different destination digest would still pass. Add a stub mode that writes a different destination digest and assert that the command refuses without reporting it as mirrored.
# A copy that does not land as the same digest is a failure, not a pass.
im "$IM_SRC" HOSTING_IM_COPY_FAIL=1 -- "${IM_ARGS[@]}"
[ "$_im_rc" -ne 0 ] && ok "a failed copy fails the step" || bad "copy fail" "exited 0"
rm -rf "$_im_state"
- Files reviewed: 6/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
#4100 (hosting-kv-ensure --db-connection) landed first. Every conflict was "both sides added": the INSTANCE-LIFECYCLE guarantee list keeps main's extended kv-ensure bullet AND the image-mirror one, the behaviour suite keeps both blocks (353 passed, 0 failed = 326 + main's 27), and the workflow's tracked-stub list is the union (image-mirror's crane and az stubs included). The tracked-file floor is CORRECTED rather than carried: main declared 23 while already tracking 24 (its comment said "20 commands" — stale, so one lost script would have passed). With hosting-image-mirror the real count is 25, and the floor and its comment now say 25. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merge resolved by content; floor settled at 25 — measured, not assumed.
The floor, and how it was counted. Guards run on the pushed head: The follow-up is still needed and is now precisely scoped. Each of #4103/#4105/#4109/#4110 sets the floor true for its own merge, so as the siblings land the real count rises 25 → 26 → 27 → 28 while the floor stays 25 — up to 3 files of slack. After all four have merged, a one-line PR sets 🤖 Generated with Claude Code |
…(workflow floor 27 = 24 commands, the stub list, run-tests.sh sections, INSTANCE-LIFECYCLE bullets); operator suite 415/415 on the merged tree Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqTDZe4Skkn81GuGkwMrXS
…t union; the bin floor is the COUNT of the merged tree: 28 (25 commands + 3); operator suite 453/453 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqTDZe4Skkn81GuGkwMrXS
|
⛔ Merge-queue steward: left dequeued — the group build failed on an assertion the flake catalogue does not know.
Not a catalogued flake. Fix the failure, or — with run URLs, an issue and an assertion-message pattern — add it to |
|
ℹ️ Merge-queue steward: no action — removed from the queue with reason |
|
The merge-queue red on this PR's group build ( |
Operator half of MeshWeaver.Plugins#1722 (the plan half —
Mirror platform imagesas the first Provision/Roll step — is the paired Plugins PR;Pairs-with: none — this PR removes no public surface).The hand step it replaces
Memex
docs/new-deployment.mdstep 4:What changes
New command
hosting-image-mirror --registry <fleet host> --repository <path> --tag <tag> [--also <path>]… --vault … --publisher-secret … [--source <acr host>] [--publisher <user>]:image_drift(a re-tagged upstream build is the operator's call);crane copyruns, and both digests are read back and must match before it reports;--also;az acr login --expose-token) reach crane on stdin (--password-stdin) into a per-processDOCKER_CONFIG— never argv, never printed, never a fact;--sourceis refused (the fleet's upstream is ACR and the token comes fromaz);--registryequal to--sourceis refused.Dockerfile: adds
crane(go-containerregistry v0.20.3, selected perTARGETARCH) — nothing else in the image can do a manifest+blob copy, and the image's own tool check now asserts it.Rights the operator identity still lacks
AcrPullonmeshweaver(plus Reader on the registry resource soaz acr login --name meshweaver --expose-tokenresolves it) —hosting-operatordoes not hold it today. Until it is granted the step is RED naming the grant and the exactcrane copyfallback; nothing is copied and nothing else is touched. Key Vault get onmemexcloud-Registry-PublisherPasswordis under the existing Key Vault Secrets Officer grant.Gates run
The crane/az stubs record logins by host and user only — never a credential — and the suite asserts no sentinel appears in any output or argv.
hosting-operator.yml: plan-command list, tracked-file floor 23 → 24, stub list, andcraneadded to the in-image tool assertion. (The floor line conflicts textually with the sibling PRs #4100/#4103/#4105/#4109, which bump the same number; whichever lands last mergesmainand re-counts.)Ships how
Hop (C):
hosting-operator:<sha>on merge tomain; reaches a Provision whenoperator.imageonDeployments/memexmoves to it. Merging is not shipping.🤖 Generated with Claude Code