[mashkovd]

Why

The ERPact copy (tenant erpact) runs images that exist only as prebuilt
Docker Hub images, built by hand outside any repository. This adds a
reproducible build from the Forgejo sources into the private registry
git.mctl.ai/erpact/*. It stays out of ghcr.io/mctlhq because these images
are a customer's code: the registry's pull credential must reach the erpact
org only.

The build recipe itself (Containerfile, apps.lock, constraints) lives in
Forgejo erpact/mctl-apps under images/, in a separate Forgejo PR. No
customer code is in this repository.

What

• .github/workflows/erpact-images.yaml: workflow_dispatch with image
  (frappe-customer / frappe-control / site-deployer), version (semver)
  and source_ref.
  • Runs on main only. It logs into Vault through GitHub OIDC as the new
    role erpact-images, which is bound to this file @refs/heads/main.
    There are no repo secrets and no fallback.
  • Two tokens of one bot account: read:repository is the only credential
    the build sees (a BuildKit secret); write:package is used by the push
    step only.
  • It builds to an OCI archive and pushes with regctl in 20 MB chunks.
    Cloudflare (Free) rejects bodies over 100 MB with 413, and Traefik ends a
    request whose body takes over 60 s. This was measured on 2026-10-04 with
    a throwaway 300 MB layer, which was then deleted.
  • Versions are immutable. Only a 404 counts as "tag absent"; any other
    answer fails the run.
• Forgejo reconcile (reconcile.yaml): a scoped team whose units are exactly
  ["repo.packages"] may list no repositories.
  • Forgejo grants org packages by the highest team permission the account
    holds in the org (services/packages/perm.go), whatever the units. So a
    team like this is how an account gets the registry and no repository.
  • Every other team without repos still fails the plan.
  • Existing team descriptions are unchanged.
• tests/test_forgejo_reconcile_teams.py and a new forgejo-reconcile-teams
  CI job. The test copies the repos / scoped_teams locals and the
  precondition out of main.tf verbatim and evaluates them with OpenTofu
  across 12 cases.
  • Mutation check: restoring the old precondition fails the two
    packages-only cases.
  • A too-loose contains(units, "repo.packages") fails the two mixed-unit
    cases.
• The Vault policy github-actions-erpact-images and the role recipe in
  vault-config/README.md. These are a manual apply, not done.
• Space: [packages] LIMIT_TOTAL_OWNER_SIZE = 4G per owner in the Forgejo
  chart config. Changing it restarts the Forgejo pod once; the strategy is
  Recreate, so expect about a minute of downtime on sync.
  • The workflow keeps the newest 10 semver tags per image and deletes older
    ones after a successful push.
  • The prune reads every page before deleting anything, and refuses if the
    version just pushed is not in the listing.
  • Forgejo's own cleanup rules are web-UI only, with no API, so they can't be
    declared.
  • Builds have provenance and SBOM turned off, so each tag is one plain
    manifest.
• A "Container registry" section in docs/runbooks/forgejo.md.

Known behaviour (accepted)

Forgejo grants org packages by the highest team permission. Org members with
write therefore can push and delete erpact images too, not only erpact-ci;
that includes Developers and the erpact-deployer bot. This is documented in
the runbook and accepted as is.

Not in this PR / order of operations

1. Merge this PR, so the reconcile ConfigMap accepts packages-only teams.
2. With the owner's OK, add the bot erpact-ci to the reconcile manifest in
   Vault, with teams ImageSources (read, repo.code, 6 source repos) and
   Packagers (write, repo.packages). The manifest is written with CAS on
   the version read. Writing the manifest before step 1 would fail the
   reconcile CronJob.
3. Mint the tokens and write secret/platform/forgejo/erpact-images.
4. Apply the Vault policy and role.

The pull secret in ns erpact and the imagePullSecrets wiring are a
separate follow-up PR. Switching the running images is another one after a
successful build.

No overlap with #1648: it reserves ghcr.io/mctlhq names, and this workflow
neither pushes there nor touches build-image.yaml or image-names.txt.

Tests

• python3 tests/test_forgejo_reconcile_teams.py: 12 cases, green, and red
  on both mutations.
• python3 tests/test_forgejo_reconcile_pii.py: green.
• actionlint is clean on erpact-images.yaml.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @mashkovd's task in 4m 29s —— View job

---

Review complete ✅

• Gather context (diff, changed files)
• Review .github/workflows/erpact-images.yaml
• Review platform-gitops/infra-components/data/forgejo/reconcile.yaml
• Review tests/test_forgejo_reconcile_teams.py
• Review Vault policy + README + runbook docs
• Post inline findings (none needed — no P1/P2/P3)
• Submit verdict: APPROVE

Verdict: APPROVE — 0 P1, 0 P2, 0 P3 findings.

Reviewed against git diff 6fe2f06c...5c4ffb80 (pinned to the commit SHA, not the mutable PR head):

• erpact-images.yaml: credential flow is tight — OIDC → Vault JWT login → immediate revoke-self after reading the secret, tokens masked and written to $RUNNER_TEMP under umask 077, cleaned up in an if: always() step. source-token (read-only) is the only credential the BuildKit build sees, via --secret, never as a build-arg. registry-token (write-only) is used solely by the push step.
• Input validation: image/version/source_ref are checked single-line; version is strict MAJOR.MINOR.PATCH; source_ref must start with an alnum and reject .., which blocks leading-dash flag injection into git fetch origin "$SOURCE_REF". All three validated before any of them reach a shell command.
• Immutability check: only an HTTP 404 is treated as "tag absent"; any other status (including network failure, mapped to 000) fails the run. Concurrency is scoped per-image, closing the race between two runs passing the check and racing to push the same tag.
• Forgejo reconcile precondition (reconcile.yaml): the new exception — try(each.value.units, null) == ["repo.packages"] — correctly requires an exact units match, so a team mixing repo.packages with any other unit still needs explicit repos (confirmed both orderings are covered by the test). The existing cross-org repo check (contains(keys(local.repos), "${org}/${r}")) is preserved unchanged for teams that do list repos.
• tests/test_forgejo_reconcile_teams.py: extracts the real repos/scoped_teams locals and the live precondition expression out of the ConfigMap's embedded main.tf (rather than reimplementing the rule) and evaluates them with tofu console against 12 manifests. The regex boundaries used to extract the Terraform blocks line up correctly with how the YAML block scalar's indentation is stripped. Logic, as traced by hand, matches all 12 expected verdicts.
• Vault policy/README: scoped to read-only on exactly secret/data/platform/forgejo/erpact-images, role bound to job_workflow_ref on refs/heads/main only — consistent with the existing github-actions role pattern in this file.

Note: tofu, python3 execution, and actionlint were not available/runnable in this review sandbox, so the test suite and lint were verified by static reading rather than execution — the PR description's own run results (12/12 green, mutation-tested) are taken at face value.

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-medium · commit 5c4ffb8.

No significant issues found.

[claude]

Claude finished @mashkovd's task in 3m 20s —— View job

---

Review complete ✅

• Gather context (diff, changed files)
• Review .github/workflows/erpact-images.yaml
• Review platform-gitops/infra-components/data/forgejo/reconcile.yaml
• Review tests/test_forgejo_reconcile_teams.py
• Review Vault policy + README + runbook docs
• Post inline findings (if any)
• Submit verdict

Verdict: APPROVE (commit 0474e635)

Reviewed the full diff (git diff 6fe2f06c...0474e635) across all 8 changed files. This is a careful, well-documented PR:

• erpact-images.yaml: input validation, Vault OIDC login with self-revocation, immutable-tag check (only 404 = absent, any other code fails closed), chunked push sized for Cloudflare/Traefik limits, scoped prune of old semver tags, and credential cleanup in an always() step. Concurrency group correctly serializes per-image runs so the tag-absence check can't race a push.
• reconcile.yaml precondition change: verified the new (repos non-empty || units == ["repo.packages"]) logic matches all 12 cases in the new test, including the "packages-only but foreign repo" rejection case.
• tests/test_forgejo_reconcile_teams.py: extracts the real precondition/locals from main.tf via regex rather than hand-duplicating it, which avoids silent drift between test and implementation — good design.
• Vault policy/README and docs/runbooks/forgejo.md additions are accurate and consistent with the workflow's actual behavior.

1 P3 nit (non-blocking), posted inline: the top-of-file comment in erpact-images.yaml claims registry-token is "used only by the push step, after the build has finished," but it's also read earlier (tag-existence check) and later (prune step). The underlying security property ("the build never sees it") still holds — just the comment's framing is slightly inaccurate.

No P1/P2 findings.

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-medium · commit 0474e63.

No significant issues found.

[claude]

No P1/P2 findings (0 P3). Good to merge. erpact-images.yaml, the reconcile.yaml precondition change, and the new OpenTofu-backed test were reviewed in detail: credential handling (OIDC->Vault, scoped tokens, revoke-self, umask 077 temp files, masked secrets, cleanup on always()), input validation (semver, single-line, ref-injection guard before the fetch), the packages-only team precondition (exact-match on units, cross-org repo check preserved), and the 12-case test all check out.

[claude]

P3: This comment says registry-token is "used only by the push step, after the build has finished." It's actually also read by Refuse an existing tag (before the build even starts) and by Prune old versions (after push). The security property that matters — "the build never sees it" — is still true, but the "only the push step" framing is inaccurate and could mislead someone auditing credential scope later.

[claude]

No P1/P2 findings (1 P3). Good to merge.