From 97a4d2b51dd78fb62380aeb15698358c5c6dc094 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 12:09:46 +0200 Subject: [PATCH 01/11] docs: add local+CI unification design (2026-04-23) Captures the approved brainstorm: full-pipeline rework where builds and tests run identically local and in CI via docker-wrapped matrices, a single phpup binary with subcommands, and a pluggable --registry that defaults to GHCR but accepts an oci-layout filesystem path. Six PR rollout, each independently shippable. --- .../2026-04-23-local-ci-unification-design.md | 361 ++++++++++++++++++ 1 file changed, 361 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-23-local-ci-unification-design.md diff --git a/docs/superpowers/specs/2026-04-23-local-ci-unification-design.md b/docs/superpowers/specs/2026-04-23-local-ci-unification-design.md new file mode 100644 index 0000000..e9c55fb --- /dev/null +++ b/docs/superpowers/specs/2026-04-23-local-ci-unification-design.md @@ -0,0 +1,361 @@ +# Local + CI Unification Design + +**Status:** Draft — 2026-04-23 +**Scope:** Full pipeline (builds + tests) rework, phased over 6 PRs. + +## Context + +Today's CI/test topology has accumulated pain the authors explicitly want to eliminate: + +- **Too many jobs per PR/push.** Dynamic matrices expand to 10–200+ jobs; the catalog-driven extension matrix is the main offender. +- **CI-only failure modes.** Jobs fail on issues that cannot be reproduced locally (runner-image preinstalls, GHCR timing, cross-job artifact handoff). +- **Chicken-and-egg artifacts.** `build-extension.yml` depends on `build-php-core.yml` having pushed to GHCR first; lockfile auto-commits back into the PR head ref mid-run. Feature branches are not self-contained. +- **Runner-image coupling.** Validation jobs run directly on `ubuntu-24.04` GitHub-hosted runners; builder scripts are careful, but workflow orchestration still assumes preinstalled Go/jq/yq/etc. +- **Local ≠ CI.** `make local-ci` exists, but it pulls already-published bundles from GHCR by digest — so it cannot validate the *current* branch's artifacts before GHCR has them. Local and CI diverge in orchestration (shell vs Actions YAML) and scope (2 fixtures vs 62). + +**Goal.** Every build and test runs locally in docker with no external service dependencies, and CI runs the *same stack and mechanics* — only the `--registry` target differs. Matrix is flat: hard environment axes only (OS × ARCH × PHP version). All entry points are `make` targets used identically by developers and CI. + +**Non-goal.** Unit tests (`go test ./...`) stay as they are. They run on a plain GitHub-hosted runner. + +## Architecture + +**One binary** (`phpup`) with subcommands. **One artifact format** (OCI layout on disk, same schema as GHCR blobs). **One test harness** (docker-wrapped matrix). **One CI workflow** (`ci.yml`, thin matrix). + +``` +┌────────────────────────────┐ ┌────────────────────────────┐ +│ developer laptop │ │ github actions runner │ +│ make ci │ │ ci.yml matrix │ +│ phpup test ──────┐ │ │ phpup test ──────┐ │ +└─────────────────────┼──────┘ └─────────────────────┼──────┘ + │ │ + ▼ ▼ + ┌─────────────────────────────┐ ┌─────────────────────────────┐ + │ docker run ubuntu:22|24.04 │ │ docker run ubuntu:22|24.04 │ + │ (bare, hermetic, no apt) │ │ (bare, hermetic, no apt) │ + └────────────┬────────────────┘ └──────────────┬──────────────┘ + │ │ + ▼ ▼ + ┌──────────────────────────────────────────────────────────────┐ + │ registry (pluggable via --registry) │ + │ oci-layout:./out/oci-layout ← local, hermetic (default) │ + │ ghcr.io/buildrush ← prod publish (CI main-only)│ + └──────────────────────────────────────────────────────────────┘ + +phpup build writes → +phpup test reads ← (builds first on cache miss within the same cell) +phpup push promotes oci-layout → ghcr.io (CI-only, on main, after pipeline passes) +``` + +**Key properties** + +- The GitHub-hosted runner is just a docker host. No `apt-get install` in workflows, no reliance on runner preinstalls. +- Local and CI share the identical code path (`phpup test`); only `--registry` differs. +- **No cross-job GHCR roundtrip.** A pipeline cell builds PHP-core → builds extensions → runs fixtures against the just-built artifacts in *its own* OCI layout. GHCR is touched only by the `publish` job after the full matrix passes on `main`. +- Chicken-egg eliminated: nothing waits for a prior job's GHCR push. + +## CLI surface + +Single binary `cmd/phpup/` with subcommands. Maintainer-only subcommands live under `phpup internal <…>`. + +| Subcommand | Replaces | Who runs it | +|---|---|---| +| `phpup install` (default) | today's `phpup` | end users (GitHub Action) | +| `phpup build php` | `builders/linux/build-php.sh` caller | maintainers + CI | +| `phpup build ext` | `builders/linux/build-ext.sh` caller | maintainers + CI | +| `phpup test` | `test/smoke/local-ci.sh` + `compat-harness.yml` orchestration | maintainers + CI | +| `phpup plan` | `cmd/planner` | CI | +| `phpup lockfile update` | `cmd/lockfile-update` | CI | +| `phpup push` | *new* — promote oci-layout → registry | CI (on main) | +| `phpup internal gc` | `cmd/gc-bundles` | cron | +| `phpup internal hermetic-audit` | `cmd/hermetic-audit` | CI cell | +| `phpup internal compat-diff` | `cmd/compat-diff` | maintainers | +| `phpup internal test-cell` | *new* — inner-container fixture runner spawned by `phpup test` | test-cell container only | + +**Global flag:** `--registry ` (env `INPUT_REGISTRY` / `PHPUP_REGISTRY`), accepted by every subcommand. +- `ghcr.io/buildrush` (default for `phpup push` and for `phpup install` when invoked as a published GitHub Action). +- `oci-layout:` (default for `phpup build`, `phpup test`, and `make ci-cell`). + +## Registry abstraction + +New package `internal/registry/`: + +```go +package registry + +type Ref struct { + Scheme string // "ghcr" | "oci-layout" + Host string // "ghcr.io/buildrush" or absolute filesystem path + Name string // "php-core", "php-ext-redis", ... + Digest string // "sha256:…" +} + +type Meta struct { /* mirrors existing meta.json */ } + +type Store interface { + Fetch(ctx context.Context, r Ref) (io.ReadCloser, *Meta, error) + Push(ctx context.Context, r Ref, bundle io.Reader, meta *Meta) error + Has(ctx context.Context, r Ref) (bool, error) + Resolve(ctx context.Context, key string) (Ref, error) // lockfile lookup +} + +func Open(uri string) (Store, error) // dispatch by scheme +``` + +Two backends: +- `internal/registry/remote` — wraps `go-containerregistry/pkg/v1/remote` (absorbs current `internal/oci/client.go`). +- `internal/registry/layout` — wraps `go-containerregistry/pkg/v1/layout` (OCI layout directory: `index.json` + `blobs/sha256/…`). No daemon, no network. + +Every call site that today references `"ghcr.io/buildrush"` string-literally goes through `registry.Open(flag.Value)`. The existing `internal/oci` package becomes a thin adapter — or is deleted once call sites migrate. + +Reused: `internal/lockfile` (keys → digests, no registry hostname stored), `internal/planner.spec_hash` (spec_hash is already digest-deriving input), `internal/catalog` (unchanged). + +## `phpup build php|ext` + +Wraps `builders/linux/*.sh` **unchanged**. The subcommand adds docker orchestration, spec-hash cache probe, OCI-layout write. + +``` +phpup build php --php 8.4 --os jammy --arch amd64 --ts nts \ + --registry oci-layout:./out/oci-layout \ + --cache ./.cache/phpup-build + +Flow: + 1. spec_hash ← sha256( + builders/linux/build-php.sh, + builders/common/**, + catalog/php-versions.yaml[php:8.4], + os, arch, ts) + 2. registry.Has(Ref{Name:"php-core", Digest:}) → if true, print + "cache hit" and exit 0. + 3. docker run --rm --platform linux/$arch \ + -v $PWD/builders:/builders:ro \ + -v $CACHE:/cache \ + -v $OUT:/out \ + ubuntu:$os \ + bash /builders/linux/build-php.sh --php 8.4 --ts nts + (cross-arch via qemu-user-static when host ≠ target) + 4. Pack /out/bundle.tar.zst + /out/meta.json. + 5. registry.Push(...) → write into OCI layout (or GHCR). +``` + +`phpup build ext` uses the same shape, additionally fetching the prerequisite `php-core` from the same registry before invoking `build-ext.sh`. Because both builds write to the *same* local OCI layout within one process, there is no cross-job handoff. + +## `phpup test` + +Single matrix runner that replaces `test/smoke/local-ci.sh` *and* the orchestration inside `compat-harness.yml`. Fixtures stay in `test/compat/fixtures.yaml`; goldens in `test/compat/testdata/`. + +``` +phpup test \ + --registry oci-layout:./out/oci-layout \ + --os jammy,noble \ + --arch amd64,arm64 \ + --php 8.1,8.2,8.3,8.4,8.5 \ + --fixtures test/compat/fixtures.yaml \ + --cache ./.cache/phpup-test \ + [--parallel N] + +For each (os, arch, php) cell: + 1. docker run --rm --platform linux/$arch \ + --network none \ + -v $PWD/out/oci-layout:/registry:ro \ + -v $PWD/phpup:/usr/local/bin/phpup:ro \ + ubuntu:$os \ + /usr/local/bin/phpup internal test-cell \ + --registry oci-layout:/registry \ + --fixtures-filter "os=$os,arch=$arch,php=$php" + 2. Inside the container: `phpup internal test-cell` iterates matching + fixtures, runs `phpup install` per fixture, invokes + test/compat/probe.sh, diffs against goldens. + 3. Report per-fixture pass/fail with structured output (JSON summary + uploaded as artifact). +``` + +Notes: +- Bare `ubuntu:22.04` / `ubuntu:24.04` images — no preinstalled tools except what `phpup install` and the bundles themselves provide. +- `--network none` by default. A fixture that needs network (e.g., fetching a remote PHAR) opts in explicitly in `fixtures.yaml`. +- Cross-arch via `qemu-user-static` when host arch ≠ target; native runners preferred in CI (`ubuntu-24.04-arm` for arm64). + +## CI workflow (`ci.yml`) + +One workflow, three jobs plus publish. Runs on every PR and every push to `main`. + +```yaml +name: ci +on: + pull_request: + push: + branches: [main] + +jobs: + lint: + runs-on: ubuntu-24.04 + steps: [checkout, setup-go, make lint] + # gofmt, vet, golangci-lint, mod tidy, eslint, prettier + + unit: + runs-on: ubuntu-24.04 + steps: [checkout, setup-go, make test] + # go test -race -cover ./... (non-goal: unchanged) + + pipeline: + needs: [lint, unit] + strategy: + fail-fast: false + matrix: + os: [jammy, noble] + arch: [amd64, arm64] + php: ["8.1", "8.2", "8.3", "8.4", "8.5"] + runs-on: >- + ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-24.04' }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + - name: Restore cache + uses: actions/cache@v4 + with: + path: | + out/oci-layout + .cache/phpup-build + key: phpup-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.php }}-${{ hashFiles('builders/**', 'catalog/**') }} + restore-keys: | + phpup-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.php }}- + - run: make ci-cell OS=${{matrix.os}} ARCH=${{matrix.arch}} PHP=${{matrix.php}} + - uses: actions/upload-artifact@v4 + if: github.ref == 'refs/heads/main' + with: + name: oci-layout-${{matrix.os}}-${{matrix.arch}}-${{matrix.php}} + path: out/oci-layout + + publish: + needs: pipeline + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 + with: + pattern: oci-layout-* + path: out/merged-layout + merge-multiple: true + - run: phpup push --from oci-layout:./out/merged-layout --to ghcr.io/buildrush + - run: phpup lockfile update + - name: Commit lockfile + run: | + git config user.name "buildrush-bot" + git add bundles.lock + git diff --cached --quiet || git commit -m "chore: update bundles.lock" + git push +``` + +**Job count per PR:** 1 (lint) + 1 (unit) + 20 (2 OS × 2 arch × 5 PHP) = **22**. +**Job count per `main` push:** 22 + 1 (publish) = **23**. + +(Down from 10–200+ dynamic jobs today. The full validation matrix — including arm64 and cross-OS — runs on every PR; no deferral of defect detection to `main`. Only the GHCR `publish` job is `main`-only, because it's promotion, not validation.) + +**Local equivalence — `make ci-cell` does exactly what CI does:** + +```make +ci-cell: + phpup build php --os $(OS) --arch $(ARCH) --php $(PHP) \ + --registry oci-layout:./out/oci-layout --cache ./.cache/phpup-build + phpup build ext --os $(OS) --arch $(ARCH) --php $(PHP) --all \ + --registry oci-layout:./out/oci-layout --cache ./.cache/phpup-build + # --all expands to every catalog extension whose manifest matches the + # (os, arch, php) tuple. Individual --ext redis can be passed instead. + phpup test --os $(OS) --arch $(ARCH) --php $(PHP) \ + --registry oci-layout:./out/oci-layout + +ci: + for os in jammy noble; do for arch in amd64 arm64; do for php in 8.1 8.2 8.3 8.4 8.5; do \ + $(MAKE) ci-cell OS=$$os ARCH=$$arch PHP=$$php ; \ + done; done; done +``` + +## Caching + +Two layers, both already aligned with how the codebase thinks about content-addressing. + +**Layer 1 — OCI layout is the cache.** Content-addressed by design. Each bundle lives at `blobs/sha256/`. Before `phpup build` compiles anything, it computes `spec_hash` for the (builder scripts, common envs, catalog entry, os, arch, php, ts) tuple and probes `registry.Has(ref)`. Hit ⇒ skip compilation. This is the same mechanism today used to avoid redundant GHCR pushes; it's relocated behind the `registry.Store` interface. + +**Layer 2 — GitHub Actions cache.** `actions/cache@v4` persists `out/oci-layout/` + `.cache/phpup-build/` across CI runs. Cache key: `phpup-${os}-${arch}-${php}-${hashFiles('builders/**','catalog/**')}` with loose prefix restore-keys. Hit ⇒ the layout already has the digest ⇒ Layer 1 short-circuits ⇒ cell spends its time only on fixture tests. + +**Cache invalidation:** +- `builders/` touched → spec_hash changes for affected bundles. +- Catalog touched → spec_hash changes for affected entries only (the `hashFiles` key is coarse, busting Layer 2 for every cell, but Layer 1 inside each cell still dedupes every unaffected digest — so the cost is bounded to re-running the short-circuited builds, not rebuilding). +- Unchanged branches → full cache hit → cells run in a few seconds each (just the fixture probe). + +## Critical files + +**New:** +- `internal/registry/registry.go` — `Store` interface + `Open(uri)`. +- `internal/registry/remote/remote.go` — GHCR backend. +- `internal/registry/layout/layout.go` — OCI-layout backend. +- `cmd/phpup/build.go` — `phpup build php|ext` subcommand. +- `cmd/phpup/test.go` — `phpup test` subcommand + `phpup test-cell` inner loop. +- `cmd/phpup/push.go` — `phpup push` (oci-layout → ghcr.io promoter). +- `.github/workflows/ci.yml` — unified workflow. + +**Modified:** +- `cmd/phpup/main.go` — subcommand dispatch, `--registry` flag, consolidation entry. +- `internal/oci/client.go` — in PR 1 refactored to delegate to `internal/registry`; deleted once all call sites migrate (by end of PR 3). +- `Makefile` — `ci`, `ci-cell`, update `bundle-php` / `bundle-ext` to call `phpup build`. + +**Deleted (PR 5):** +- `.github/workflows/build-php-core.yml` +- `.github/workflows/build-extension.yml` +- `.github/workflows/plan.yml`, `plan-and-build.yml`, `on-push.yml` +- `.github/workflows/integration-test.yml`, `compat-harness.yml` +- `.github/workflows/nightly.yml`, `manual.yml`, `gc-bundles.yml` +- `test/smoke/local-ci.sh` (folded into `phpup test`) + +**Retained:** +- `.github/workflows/ci-lint.yml` → folded into `ci.yml::lint`. +- `.github/workflows/watch-*.yml`, `security-rebuild.yml` (orthogonal; re-trigger `ci.yml`). +- `.github/workflows/release-please.yml`, `check-release-pr.yml` (release engineering). + +**Deleted (PR 6):** +- `cmd/planner/`, `cmd/lockfile-update/`, `cmd/gc-bundles/`, `cmd/hermetic-audit/`, `cmd/compat-diff/` — all moved to `phpup` subcommands. + +## Rollout — six PRs + +Each PR is independently shippable and ends with CI green. No long-lived feature branch. + +1. **Registry abstraction + `--registry` on `phpup install`.** Adds `internal/registry/` with both backends. `phpup install` accepts `--registry` (env fallback, default `ghcr.io/buildrush`). `internal/oci/client.go` delegates. No workflow changes. +2. **`phpup build php|ext` subcommand.** Docker-wraps existing shell builders. `make bundle-php` / `make bundle-ext` call `phpup build`. Existing `build-php-core.yml` / `build-extension.yml` internally switch to `phpup build` (same job shape, same GHCR push). +3. **`phpup test` subcommand + `make ci-cell`.** Orchestrates per-cell fixture run in bare-ubuntu containers. `test/smoke/local-ci.sh` becomes a thin wrapper (deleted in PR 5). +4. **New `ci.yml` in parallel with old workflows.** Both systems run on every PR/push for one week. `publish` job gated behind an env flag during the grace period. Both sides must pass to merge. +5. **Cut over.** Delete the old workflows and `test/smoke/local-ci.sh`. Flip `publish` to sole GHCR writer. +6. **Consolidate remaining CLI binaries.** Move `planner`, `lockfile-update`, `gc-bundles`, `hermetic-audit`, `compat-diff` under `phpup` subcommands. Delete the old `cmd/*` entrypoints. + +**Rollback posture.** After PR 4 the old pipeline still works and can be re-enabled by reverting PR 5 alone. PRs 1–3 are additive; PR 6 is cosmetic. + +## Verification + +**Per-PR acceptance:** + +- PR 1: unit tests for `internal/registry/{remote,layout}` round-trip Push/Fetch/Has against a `t.TempDir()` layout; against a local `distribution/distribution:3` spun up by the test. `phpup install --registry oci-layout:` succeeds end-to-end against a layout populated with a real bundle. +- PR 2: `make bundle-php` and `make bundle-ext` produce identical tarball contents (byte-for-byte or digest-equal) to the pre-change shell-only path. Cache probe short-circuits a second invocation. +- PR 3: `make ci-cell OS=jammy ARCH=amd64 PHP=8.4` runs to completion locally without network (after initial docker image pull) and produces green fixture output. Cross-arch `ARCH=arm64` runs under qemu. +- PR 4: new `ci.yml` passes alongside old workflows for at least one week (~7 days of merged PRs) with zero divergence in artifact digests. +- PR 5: `main`-branch push still publishes to GHCR; released bundles have identical digests to the pre-cutover run. +- PR 6: `phpup plan`, `phpup lockfile update`, etc., produce byte-identical output to the old binaries for the same inputs. + +**End-to-end local verification (before merging PR 5):** + +```bash +# Hermetic local reproduction of CI +make clean +make ci # 20 cells, each a full build + test loop inside bare-ubuntu +# Exit 0 with no network access beyond initial docker image pull. +``` + +**End-to-end CI verification:** + +- Open a draft PR on a feature branch; observe 22 jobs complete green. +- Merge to `main`; observe `publish` job run, GHCR artifacts updated, lockfile commit pushed. +- Release workflow (`release-please.yml`) unaffected by upstream changes. + +## Open questions / future work + +- **Matrix breadth expansion.** If PHP 8.6 lands, the matrix grows to 24 cells; still well inside the "small matrix" envelope. If additional OS variants are added (Debian, Alpine) the axes multiply — worth revisiting cell-sharding at that point. +- **Layer 2 cache granularity.** `hashFiles('builders/**','catalog/**')` is coarse. Layer 1 absorbs the waste for now; a finer key (per-entry spec_hash) can replace it if cache turnaround becomes a bottleneck. +- **Persistent local registry option.** If developers ask for it later, `Open("http://localhost:5000/buildrush")` already works (the `remote` backend handles any go-containerregistry ref); no design change needed. From 624f72f5c93b05e09d7b21f1f9521f12d4d00e94 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 12:28:06 +0200 Subject: [PATCH 02/11] feat(registry): add Store interface + URI dispatch New internal/registry package introduces a backend-agnostic Store interface (Fetch, Push, Has, ResolveDigest) and a URI-dispatching Open() function. Concrete layout and remote backends arrive in the next two tasks; this commit lands the public surface and scheme parsing only, gated behind clear "implemented in Task N" stubs. --- internal/registry/layout.go | 41 +++++++++++ internal/registry/registry.go | 110 +++++++++++++++++++++++++++++ internal/registry/registry_test.go | 59 ++++++++++++++++ internal/registry/remote.go | 39 ++++++++++ 4 files changed, 249 insertions(+) create mode 100644 internal/registry/layout.go create mode 100644 internal/registry/registry.go create mode 100644 internal/registry/registry_test.go create mode 100644 internal/registry/remote.go diff --git a/internal/registry/layout.go b/internal/registry/layout.go new file mode 100644 index 0000000..5a6e80d --- /dev/null +++ b/internal/registry/layout.go @@ -0,0 +1,41 @@ +package registry + +import ( + "context" + "errors" + "io" +) + +// layoutStore is the filesystem OCI-layout backed Store. +// +// The real implementation (readdir, blob lookup, manifest walk) lands in +// Task 2; this file only provides the constructor and Kind() so that Open +// can dispatch and callers can begin wiring to the interface. +type layoutStore struct { + root string +} + +func openLayout(path string) (*layoutStore, error) { + if path == "" { + return nil, errors.New("registry: oci-layout URI requires a path") + } + return &layoutStore{root: path}, nil +} + +func (s *layoutStore) Kind() string { return "layout" } + +func (s *layoutStore) Fetch(_ context.Context, _ Ref) (io.ReadCloser, *Meta, error) { + return nil, nil, errors.New("layout.Fetch: implemented in Task 2") +} + +func (s *layoutStore) Push(_ context.Context, _ Ref, _ io.Reader, _ *Meta) error { + return errors.New("layout.Push: implemented in Task 2") +} + +func (s *layoutStore) Has(_ context.Context, _ Ref) (bool, error) { + return false, errors.New("layout.Has: implemented in Task 2") +} + +func (s *layoutStore) ResolveDigest(_ context.Context, _ string) (string, error) { + return "", errors.New("layout.ResolveDigest: implemented in Task 2") +} diff --git a/internal/registry/registry.go b/internal/registry/registry.go new file mode 100644 index 0000000..10a4467 --- /dev/null +++ b/internal/registry/registry.go @@ -0,0 +1,110 @@ +// Package registry provides a backend-agnostic abstraction for fetching and +// pushing OCI-style bundles used by setup-php. +// +// Two concrete backends are supported, selected by URI scheme via Open: +// +// - Remote HTTPS registries (e.g. "ghcr.io/buildrush"): implemented on top +// of go-containerregistry and used for published bundles. +// - Local filesystem OCI layouts (e.g. "oci-layout:/path/to/layout"): used +// for the local-CI smoke pipeline and tests. +// +// The public surface lives in this file (Ref, Meta, Store, Open). Concrete +// store implementations live in layout.go and remote.go. +package registry + +import ( + "context" + "errors" + "fmt" + "io" + "strings" +) + +// ErrUnsupported is returned by Store implementations when the caller invokes +// an operation the backend cannot serve (for example, pushing to a read-only +// remote). +var ErrUnsupported = errors.New("registry: operation not supported by this backend") + +// Ref identifies a bundle within a Store by its logical Name and, optionally, +// its content-addressed Digest (in the usual "sha256:..." form). +type Ref struct { + Name string + Digest string +} + +// String renders the Ref as "name@digest" when a Digest is present, or just +// "name" otherwise. It is suitable for logs and error messages; it is not a +// canonical OCI reference. +func (r Ref) String() string { + if r.Digest == "" { + return r.Name + } + return r.Name + "@" + r.Digest +} + +// Meta describes bundle metadata persisted alongside the payload. Fields are +// kept minimal on purpose; callers that need richer structure layer it on top. +type Meta struct { + SchemaVersion int `json:"schema_version"` + Kind string `json:"kind"` +} + +// Store is the backend-agnostic interface for fetching and pushing bundles. +// +// Kind returns a short identifier for the backend ("remote", "layout") and is +// intended for logs and test assertions only. +type Store interface { + Kind() string + Fetch(ctx context.Context, ref Ref) (io.ReadCloser, *Meta, error) + Push(ctx context.Context, ref Ref, body io.Reader, meta *Meta) error + Has(ctx context.Context, ref Ref) (bool, error) + ResolveDigest(ctx context.Context, name string) (string, error) +} + +// Open dispatches on the given URI and returns a Store. +// +// The accepted forms are: +// +// - "oci-layout:" — a local filesystem OCI image layout. +// - a bare host[/path] that "looks like" a remote (the head segment before +// the first "/" must contain a dot and only [a-zA-Z0-9.-] characters), +// for example "ghcr.io/buildrush". +// +// Any other input — including the empty string — is rejected with an error +// that mentions "scheme". +func Open(uri string) (Store, error) { + if uri == "" { + return nil, errors.New("registry: empty URI") + } + if strings.HasPrefix(uri, "oci-layout:") { + return openLayout(strings.TrimPrefix(uri, "oci-layout:")) + } + if looksLikeRemote(uri) { + return openRemote(uri) + } + return nil, fmt.Errorf("registry: unrecognised scheme in %q", uri) +} + +// looksLikeRemote returns true when the URI's head segment (up to the first +// "/") is a plausible registry host: it must contain at least one "." and be +// composed exclusively of ASCII letters, digits, dots, and hyphens. +func looksLikeRemote(uri string) bool { + head := uri + if i := strings.IndexByte(uri, '/'); i >= 0 { + head = uri[:i] + } + if head == "" || !strings.ContainsRune(head, '.') { + return false + } + for _, r := range head { + switch { + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r >= '0' && r <= '9': + case r == '.' || r == '-': + default: + return false + } + } + return true +} diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go new file mode 100644 index 0000000..ab0694e --- /dev/null +++ b/internal/registry/registry_test.go @@ -0,0 +1,59 @@ +package registry + +import ( + "strings" + "testing" +) + +func TestOpen_RemoteSchemeReturnsRemoteStore(t *testing.T) { + s, err := Open("ghcr.io/buildrush") + if err != nil { + t.Fatalf("Open(ghcr.io/buildrush) err = %v, want nil", err) + } + if s == nil { + t.Fatal("Open returned nil store") + } + if got := s.Kind(); got != "remote" { + t.Errorf("Kind() = %q, want %q", got, "remote") + } +} + +func TestOpen_LayoutSchemeReturnsLayoutStore(t *testing.T) { + s, err := Open("oci-layout:/tmp/nonexistent") + if err != nil { + t.Fatalf("Open(oci-layout:...) err = %v, want nil", err) + } + if got := s.Kind(); got != "layout" { + t.Errorf("Kind() = %q, want %q", got, "layout") + } +} + +func TestOpen_EmptyURIIsError(t *testing.T) { + if _, err := Open(""); err == nil { + t.Fatal("Open(\"\") want error, got nil") + } +} + +func TestOpen_UnknownSchemeIsError(t *testing.T) { + _, err := Open("fluffy-clouds:whatever") + if err == nil { + t.Fatal("want error for unknown scheme, got nil") + } + if !strings.Contains(err.Error(), "scheme") { + t.Errorf("error = %q, want it to mention \"scheme\"", err) + } +} + +func TestRefString_Remote(t *testing.T) { + r := Ref{Name: "php-core", Digest: "sha256:abc"} + if got := r.String(); got != "php-core@sha256:abc" { + t.Errorf("Ref.String() = %q, want %q", got, "php-core@sha256:abc") + } +} + +func TestRefString_EmptyDigest(t *testing.T) { + r := Ref{Name: "php-core"} + if got := r.String(); got != "php-core" { + t.Errorf("Ref.String() = %q, want %q", got, "php-core") + } +} diff --git a/internal/registry/remote.go b/internal/registry/remote.go new file mode 100644 index 0000000..7c5fd75 --- /dev/null +++ b/internal/registry/remote.go @@ -0,0 +1,39 @@ +package registry + +import ( + "context" + "errors" + "io" +) + +// remoteStore is the HTTPS-registry backed Store. +// +// The real implementation (go-containerregistry remote.Image, auth, retries) +// lands in Task 3; this file provides the constructor and Kind() so Open can +// dispatch, plus Push gated on ErrUnsupported since published remotes are +// read-only from the action's perspective. +type remoteStore struct { + root string +} + +func openRemote(uri string) (*remoteStore, error) { + return &remoteStore{root: uri}, nil +} + +func (s *remoteStore) Kind() string { return "remote" } + +func (s *remoteStore) Fetch(_ context.Context, _ Ref) (io.ReadCloser, *Meta, error) { + return nil, nil, errors.New("remote.Fetch: implemented in Task 3") +} + +func (s *remoteStore) Push(_ context.Context, _ Ref, _ io.Reader, _ *Meta) error { + return ErrUnsupported +} + +func (s *remoteStore) Has(_ context.Context, _ Ref) (bool, error) { + return false, errors.New("remote.Has: implemented in Task 3") +} + +func (s *remoteStore) ResolveDigest(_ context.Context, _ string) (string, error) { + return "", errors.New("remote.ResolveDigest: implemented in Task 3") +} From 891d8932408a391e997747228f6b4b8c97ad5dca Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 12:34:33 +0200 Subject: [PATCH 03/11] fix(registry): address Task 1 review findings - Correct Open's godoc to match distinct "empty URI" vs "scheme" error branches. - Add table-driven Open dispatch test covering looksLikeRemote edge cases (dotless host, uppercase host, multi-segment path, leading slash, host-only). - Rename remoteStore.root to remoteStore.base so the field doesn't falsely mirror layoutStore.root (which is a filesystem path). - Document *Meta nil semantics on Store.Fetch / Store.Push. - Rename TestRefString_Remote -> TestRefString_WithDigest for symmetry. No behaviour changes; all existing call sites unaffected. --- internal/registry/registry.go | 9 +++- internal/registry/registry_test.go | 66 +++++++++++++++++++++++++++++- internal/registry/remote.go | 4 +- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 10a4467..3cab626 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -53,6 +53,10 @@ type Meta struct { // // Kind returns a short identifier for the backend ("remote", "layout") and is // intended for logs and test assertions only. +// +// Fetch may return a nil *Meta when the backend has no sidecar metadata for +// the ref; callers must tolerate that. Push accepts a nil meta to write a +// bundle without a meta sidecar. type Store interface { Kind() string Fetch(ctx context.Context, ref Ref) (io.ReadCloser, *Meta, error) @@ -70,8 +74,9 @@ type Store interface { // the first "/" must contain a dot and only [a-zA-Z0-9.-] characters), // for example "ghcr.io/buildrush". // -// Any other input — including the empty string — is rejected with an error -// that mentions "scheme". +// Any unrecognised URI form is rejected with an error. The empty string yields +// "registry: empty URI"; other unrecognised forms yield an error whose message +// contains "scheme". func Open(uri string) (Store, error) { if uri == "" { return nil, errors.New("registry: empty URI") diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index ab0694e..f165a3b 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -44,13 +44,77 @@ func TestOpen_UnknownSchemeIsError(t *testing.T) { } } -func TestRefString_Remote(t *testing.T) { +func TestRefString_WithDigest(t *testing.T) { r := Ref{Name: "php-core", Digest: "sha256:abc"} if got := r.String(); got != "php-core@sha256:abc" { t.Errorf("Ref.String() = %q, want %q", got, "php-core@sha256:abc") } } +func TestOpen_URIFormDispatch(t *testing.T) { + cases := []struct { + name string + uri string + wantKind string // empty when an error is expected + wantErrSubstring string // empty when success is expected + }{ + { + name: "dotless host is not remote", + uri: "localhost/foo", + wantErrSubstring: "scheme", + }, + { + name: "host only succeeds as remote", + uri: "ghcr.io", + wantKind: "remote", + }, + { + name: "uppercase host succeeds as remote", + uri: "GHCR.IO/foo", + wantKind: "remote", + }, + { + name: "leading slash has empty head", + uri: "/leading-slash", + wantErrSubstring: "scheme", + }, + { + name: "multi-segment path succeeds as remote", + uri: "ghcr.io/buildrush/extra/path/segments", + wantKind: "remote", + }, + { + name: "underscore in path does not affect head", + uri: "ghcr.io/weird_path", + wantKind: "remote", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s, err := Open(tc.uri) + if tc.wantErrSubstring != "" { + if err == nil { + t.Fatalf("Open(%q) err = nil, want error containing %q", tc.uri, tc.wantErrSubstring) + } + if !strings.Contains(err.Error(), tc.wantErrSubstring) { + t.Errorf("Open(%q) err = %q, want substring %q", tc.uri, err, tc.wantErrSubstring) + } + return + } + if err != nil { + t.Fatalf("Open(%q) err = %v, want nil", tc.uri, err) + } + if s == nil { + t.Fatalf("Open(%q) returned nil store", tc.uri) + } + if got := s.Kind(); got != tc.wantKind { + t.Errorf("Open(%q).Kind() = %q, want %q", tc.uri, got, tc.wantKind) + } + }) + } +} + func TestRefString_EmptyDigest(t *testing.T) { r := Ref{Name: "php-core"} if got := r.String(); got != "php-core" { diff --git a/internal/registry/remote.go b/internal/registry/remote.go index 7c5fd75..60b6a7f 100644 --- a/internal/registry/remote.go +++ b/internal/registry/remote.go @@ -13,11 +13,11 @@ import ( // dispatch, plus Push gated on ErrUnsupported since published remotes are // read-only from the action's perspective. type remoteStore struct { - root string + base string } func openRemote(uri string) (*remoteStore, error) { - return &remoteStore{root: uri}, nil + return &remoteStore{base: uri}, nil } func (s *remoteStore) Kind() string { return "remote" } From 837b88df8204922c8af8a61898199434c8982241 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 12:41:02 +0200 Subject: [PATCH 04/11] feat(registry): add oci-layout backend layoutStore reads/writes bundles as two-layer OCI images (layer 0 = bundle bytes, layer 1 = meta.json) under an on-disk layout directory. Push/Has/Fetch round-trip verified; legacy bundles without a meta sidecar return Meta{SchemaVersion:1} to match runtime expectations. Has/Fetch also accept a digest-only match so layouts populated by oras copy (which doesn't set our annotation) are interoperable. --- internal/registry/layout.go | 253 +++++++++++++++++++++++++++++-- internal/registry/layout_test.go | 113 ++++++++++++++ 2 files changed, 353 insertions(+), 13 deletions(-) create mode 100644 internal/registry/layout_test.go diff --git a/internal/registry/layout.go b/internal/registry/layout.go index 5a6e80d..0be6659 100644 --- a/internal/registry/layout.go +++ b/internal/registry/layout.go @@ -2,15 +2,31 @@ package registry import ( "context" + "encoding/json" "errors" + "fmt" "io" + "io/fs" + + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/layout" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" ) -// layoutStore is the filesystem OCI-layout backed Store. -// -// The real implementation (readdir, blob lookup, manifest walk) lands in -// Task 2; this file only provides the constructor and Kind() so that Open -// can dispatch and callers can begin wiring to the interface. +// annotationBundleName is the OCI manifest annotation key used to tag each +// pushed bundle with its logical Ref.Name, so Has/Fetch can walk the index +// and find the right manifest without relying on external state. +const annotationBundleName = "io.buildrush.bundle.name" + +// layoutStore is a filesystem-backed Store implemented over an on-disk OCI +// image layout (pkg/v1/layout). Bundles are stored as two-layer OCI images: +// layer 0 carries the bundle bytes, layer 1 (when present) carries the +// serialised Meta sidecar. Legacy bundles without a meta layer Fetch back +// with a default Meta{SchemaVersion:1}, matching the runtime's tolerance in +// internal/oci/client.go. type layoutStore struct { root string } @@ -24,18 +40,229 @@ func openLayout(path string) (*layoutStore, error) { func (s *layoutStore) Kind() string { return "layout" } -func (s *layoutStore) Fetch(_ context.Context, _ Ref) (io.ReadCloser, *Meta, error) { - return nil, nil, errors.New("layout.Fetch: implemented in Task 2") +// openOrInit returns a writable layout.Path, creating an empty layout at +// s.root on first use. +func (s *layoutStore) openOrInit() (layout.Path, error) { + if p, err := layout.FromPath(s.root); err == nil { + return p, nil + } + p, err := layout.Write(s.root, empty.Index) + if err != nil { + return layout.Path(""), fmt.Errorf("layout: init %q: %w", s.root, err) + } + return p, nil +} + +// open returns a read-only handle to the layout at s.root. A missing layout +// surfaces as the underlying error from layout.FromPath. +func (s *layoutStore) open() (layout.Path, error) { + return layout.FromPath(s.root) +} + +func (s *layoutStore) Push(_ context.Context, ref Ref, body io.Reader, meta *Meta) error { + if ref.Name == "" { + return errors.New("layout.Push: ref.Name required") + } + bundleBytes, err := io.ReadAll(body) + if err != nil { + return fmt.Errorf("layout.Push: read bundle: %w", err) + } + + layers := []v1.Layer{static.NewLayer(bundleBytes, types.OCILayer)} + if meta != nil { + metaBytes, err := json.Marshal(meta) + if err != nil { + return fmt.Errorf("layout.Push: marshal meta: %w", err) + } + layers = append(layers, static.NewLayer(metaBytes, types.OCILayer)) + } + + img, err := mutate.AppendLayers(empty.Image, layers...) + if err != nil { + return fmt.Errorf("layout.Push: append layers: %w", err) + } + // Annotate both the image manifest and the index descriptor. The index + // descriptor annotation is what Has/Fetch walk over (go-containerregistry's + // partial.Descriptor does not propagate manifest-level annotations into the + // index), while the manifest-level annotation keeps the round-trip + // self-describing for tools that inspect the OCI image directly. + annotations := map[string]string{annotationBundleName: ref.Name} + annotated, ok := mutate.Annotations(img, annotations).(v1.Image) + if !ok { + return errors.New("layout.Push: mutate.Annotations did not return v1.Image") + } + + p, err := s.openOrInit() + if err != nil { + return err + } + if err := p.AppendImage(annotated, layout.WithAnnotations(annotations)); err != nil { + return fmt.Errorf("layout.Push: append image: %w", err) + } + return nil +} + +func (s *layoutStore) Has(_ context.Context, ref Ref) (bool, error) { + p, err := layout.FromPath(s.root) + if err != nil { + // Absent layout is not an error — the ref simply isn't present yet. + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } + return false, fmt.Errorf("layout.Has: open %q: %w", s.root, err) + } + if ref.Digest == "" { + // Without a digest we can't match a specific manifest. Treat as "not + // present" rather than erroring so callers can probe empty stores. + return false, nil + } + idx, err := p.ImageIndex() + if err != nil { + return false, fmt.Errorf("layout.Has: load index: %w", err) + } + manifest, err := idx.IndexManifest() + if err != nil { + return false, fmt.Errorf("layout.Has: parse index: %w", err) + } + digestOnlyMatch := false + for i := range manifest.Manifests { + m := &manifest.Manifests[i] + if m.Digest.String() != ref.Digest { + continue + } + if m.Annotations[annotationBundleName] == ref.Name { + return true, nil + } + digestOnlyMatch = true + } + // Fallback for layouts populated by tools (e.g. `oras copy`) that don't + // set our bundle-name annotation. + return digestOnlyMatch, nil } -func (s *layoutStore) Push(_ context.Context, _ Ref, _ io.Reader, _ *Meta) error { - return errors.New("layout.Push: implemented in Task 2") +func (s *layoutStore) Fetch(_ context.Context, ref Ref) (io.ReadCloser, *Meta, error) { + p, err := s.open() + if err != nil { + return nil, nil, fmt.Errorf("layout.Fetch: open %q: %w", s.root, err) + } + idx, err := p.ImageIndex() + if err != nil { + return nil, nil, fmt.Errorf("layout.Fetch: load index: %w", err) + } + manifest, err := idx.IndexManifest() + if err != nil { + return nil, nil, fmt.Errorf("layout.Fetch: parse index: %w", err) + } + + var ( + chosen v1.Hash + found bool + digestOnly v1.Hash + digestOnlyHit bool + ) + for i := range manifest.Manifests { + m := &manifest.Manifests[i] + if ref.Digest != "" && m.Digest.String() != ref.Digest { + continue + } + if m.Annotations[annotationBundleName] == ref.Name { + chosen = m.Digest + found = true + break + } + if ref.Digest != "" && m.Digest.String() == ref.Digest { + digestOnly = m.Digest + digestOnlyHit = true + } + } + if !found { + if digestOnlyHit { + chosen = digestOnly + found = true + } + } + if !found { + return nil, nil, fmt.Errorf("layout.Fetch: %s not found in %q", ref, s.root) + } + + img, err := idx.Image(chosen) + if err != nil { + return nil, nil, fmt.Errorf("layout.Fetch: load image %s: %w", chosen, err) + } + layers, err := img.Layers() + if err != nil { + return nil, nil, fmt.Errorf("layout.Fetch: list layers: %w", err) + } + if len(layers) == 0 { + return nil, nil, fmt.Errorf("layout.Fetch: image %s has no layers", chosen) + } + + bundle, err := layers[0].Uncompressed() + if err != nil { + return nil, nil, fmt.Errorf("layout.Fetch: open bundle layer: %w", err) + } + + meta := &Meta{SchemaVersion: 1} + if len(layers) >= 2 { + mrc, err := layers[1].Uncompressed() + if err != nil { + _ = bundle.Close() + return nil, nil, fmt.Errorf("layout.Fetch: open meta layer: %w", err) + } + metaBytes, err := io.ReadAll(mrc) + _ = mrc.Close() + if err != nil { + _ = bundle.Close() + return nil, nil, fmt.Errorf("layout.Fetch: read meta layer: %w", err) + } + parsed := &Meta{} + if err := json.Unmarshal(metaBytes, parsed); err != nil { + _ = bundle.Close() + return nil, nil, fmt.Errorf("layout.Fetch: parse meta: %w", err) + } + if parsed.SchemaVersion == 0 { + parsed.SchemaVersion = 1 + } + meta = parsed + } + return bundle, meta, nil } -func (s *layoutStore) Has(_ context.Context, _ Ref) (bool, error) { - return false, errors.New("layout.Has: implemented in Task 2") +func (s *layoutStore) ResolveDigest(_ context.Context, reference string) (string, error) { + return "", fmt.Errorf("layout.ResolveDigest: not supported on layout backend (reference=%q)", reference) } -func (s *layoutStore) ResolveDigest(_ context.Context, _ string) (string, error) { - return "", errors.New("layout.ResolveDigest: implemented in Task 2") +// list walks the index and returns one Ref per manifest. Intended for tests +// only — callers in production should track their own refs or use a remote +// backend that can advertise them. +func (s *layoutStore) list(_ context.Context) ([]Ref, error) { + p, err := layout.FromPath(s.root) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + return nil, fmt.Errorf("layout.list: open %q: %w", s.root, err) + } + idx, err := p.ImageIndex() + if err != nil { + return nil, fmt.Errorf("layout.list: load index: %w", err) + } + manifest, err := idx.IndexManifest() + if err != nil { + return nil, fmt.Errorf("layout.list: parse index: %w", err) + } + if len(manifest.Manifests) == 0 { + return nil, nil + } + refs := make([]Ref, 0, len(manifest.Manifests)) + for i := range manifest.Manifests { + m := &manifest.Manifests[i] + refs = append(refs, Ref{ + Name: m.Annotations[annotationBundleName], + Digest: m.Digest.String(), + }) + } + return refs, nil } + +var _ Store = (*layoutStore)(nil) diff --git a/internal/registry/layout_test.go b/internal/registry/layout_test.go new file mode 100644 index 0000000..2d0255f --- /dev/null +++ b/internal/registry/layout_test.go @@ -0,0 +1,113 @@ +package registry + +import ( + "bytes" + "context" + "io" + "path/filepath" + "testing" +) + +func newTestLayoutStore(t *testing.T) *layoutStore { + t.Helper() + dir := filepath.Join(t.TempDir(), "layout") + s, err := openLayout(dir) + if err != nil { + t.Fatalf("openLayout: %v", err) + } + return s +} + +func TestLayoutStore_RoundTrip(t *testing.T) { + ctx := context.Background() + s := newTestLayoutStore(t) + + payload := []byte("fake bundle bytes — in reality this would be bundle.tar.zst") + meta := &Meta{SchemaVersion: 2, Kind: "php-core"} + + // Before Push, Has must return false. + ref := Ref{Name: "php-core"} // digest filled in by Push + has, err := s.Has(ctx, ref) + if err != nil { + t.Fatalf("Has before Push: %v", err) + } + if has { + t.Fatal("Has returned true on empty layout") + } + + if err := s.Push(ctx, ref, bytes.NewReader(payload), meta); err != nil { + t.Fatalf("Push: %v", err) + } + + // Walk the index to find the just-pushed digest, then assert Has and Fetch. + got, err := s.list(ctx) + if err != nil { + t.Fatalf("list after Push: %v", err) + } + if len(got) != 1 { + t.Fatalf("list after Push: got %d entries, want 1", len(got)) + } + stored := got[0] + if stored.Name != "php-core" { + t.Fatalf("stored.Name = %q, want %q", stored.Name, "php-core") + } + if stored.Digest == "" { + t.Fatal("stored.Digest empty") + } + + has, err = s.Has(ctx, stored) + if err != nil { + t.Fatalf("Has after Push: %v", err) + } + if !has { + t.Fatal("Has returned false after Push") + } + + rc, metaOut, err := s.Fetch(ctx, stored) + if err != nil { + t.Fatalf("Fetch after Push: %v", err) + } + defer rc.Close() + gotBytes, err := io.ReadAll(rc) + if err != nil { + t.Fatalf("read bundle bytes: %v", err) + } + if !bytes.Equal(gotBytes, payload) { + t.Fatalf("bundle bytes mismatch: got %q, want %q", gotBytes, payload) + } + if metaOut == nil || metaOut.SchemaVersion != 2 || metaOut.Kind != "php-core" { + t.Fatalf("meta mismatch: got %+v, want {SchemaVersion:2 Kind:php-core}", metaOut) + } +} + +func TestLayoutStore_FetchMissingRef_Errors(t *testing.T) { + ctx := context.Background() + s := newTestLayoutStore(t) + _, _, err := s.Fetch(ctx, Ref{Name: "php-core", Digest: "sha256:deadbeef"}) + if err == nil { + t.Fatal("Fetch on empty layout: want error, got nil") + } +} + +func TestLayoutStore_TolerateMissingMeta(t *testing.T) { + // A bundle pushed without a meta sidecar must Fetch back with a + // default Meta{SchemaVersion:1} — matching the legacy behaviour the + // runtime already tolerates (see internal/oci/client.go Fetch path). + ctx := context.Background() + s := newTestLayoutStore(t) + payload := []byte("legacy-bundle") + if err := s.Push(ctx, Ref{Name: "php-core"}, bytes.NewReader(payload), nil); err != nil { + t.Fatalf("Push nil meta: %v", err) + } + got, err := s.list(ctx) + if err != nil || len(got) != 1 { + t.Fatalf("list: %v / %d", err, len(got)) + } + _, meta, err := s.Fetch(ctx, got[0]) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if meta == nil || meta.SchemaVersion != 1 { + t.Fatalf("meta = %+v, want {SchemaVersion:1}", meta) + } +} From fe8076e3a7ac5322ebe1cd0da094d0b78a4f8034 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 12:51:19 +0200 Subject: [PATCH 05/11] fix(registry): tighten layout annotation matching + compression contract - Has/Fetch digest-only fallback now only accepts manifests that have no annotationBundleName set; manifests with a mismatching annotation are an affirmative negative. Prevents cross-linking a Ref{Name:"X"} probe to a manifest whose annotation says it is Y. - Switch Fetch to layers[i].Compressed() so returned bytes match the remote-backed Store path (ghcr bundles are tar+zstd and callers decompress). Document the Store.Fetch contract. - Add three digest-fallback tests covering exact-match preference, oras-style no-annotation acceptance, and wrong-annotation rejection. - Strengthen TestLayoutStore_FetchMissingRef_Errors to also exercise the populated-layout "not-found" branch with a valid-form digest. - Drop a dead-branch predicate in Fetch's match loop. --- internal/registry/layout.go | 46 +++++---- internal/registry/layout_test.go | 166 ++++++++++++++++++++++++++++++- internal/registry/registry.go | 5 + 3 files changed, 196 insertions(+), 21 deletions(-) diff --git a/internal/registry/layout.go b/internal/registry/layout.go index 0be6659..6a88db4 100644 --- a/internal/registry/layout.go +++ b/internal/registry/layout.go @@ -124,20 +124,26 @@ func (s *layoutStore) Has(_ context.Context, ref Ref) (bool, error) { if err != nil { return false, fmt.Errorf("layout.Has: parse index: %w", err) } - digestOnlyMatch := false + fallback := false for i := range manifest.Manifests { m := &manifest.Manifests[i] if m.Digest.String() != ref.Digest { continue } - if m.Annotations[annotationBundleName] == ref.Name { + ann, hasAnn := m.Annotations[annotationBundleName] + if hasAnn && ann == ref.Name { return true, nil } - digestOnlyMatch = true + // Only treat a digest-only match as a fallback candidate when the + // manifest has NO bundle-name annotation at all (e.g. it was + // populated by `oras copy` without our annotation). A manifest + // that carries the annotation but names a *different* bundle is + // an affirmative negative — we must not cross-link it. + if !hasAnn { + fallback = true + } } - // Fallback for layouts populated by tools (e.g. `oras copy`) that don't - // set our bundle-name annotation. - return digestOnlyMatch, nil + return fallback, nil } func (s *layoutStore) Fetch(_ context.Context, ref Ref) (io.ReadCloser, *Meta, error) { @@ -157,29 +163,31 @@ func (s *layoutStore) Fetch(_ context.Context, ref Ref) (io.ReadCloser, *Meta, e var ( chosen v1.Hash found bool - digestOnly v1.Hash - digestOnlyHit bool + fallback v1.Hash + fallbackFound bool ) for i := range manifest.Manifests { m := &manifest.Manifests[i] if ref.Digest != "" && m.Digest.String() != ref.Digest { continue } - if m.Annotations[annotationBundleName] == ref.Name { + ann, hasAnn := m.Annotations[annotationBundleName] + if hasAnn && ann == ref.Name { chosen = m.Digest found = true break } - if ref.Digest != "" && m.Digest.String() == ref.Digest { - digestOnly = m.Digest - digestOnlyHit = true + // See Has: we only accept a digest-only fallback when the manifest + // carries no bundle-name annotation. An annotation that names a + // different bundle is an affirmative negative. + if ref.Digest != "" && !hasAnn { + fallback = m.Digest + fallbackFound = true } } - if !found { - if digestOnlyHit { - chosen = digestOnly - found = true - } + if !found && fallbackFound { + chosen = fallback + found = true } if !found { return nil, nil, fmt.Errorf("layout.Fetch: %s not found in %q", ref, s.root) @@ -197,14 +205,14 @@ func (s *layoutStore) Fetch(_ context.Context, ref Ref) (io.ReadCloser, *Meta, e return nil, nil, fmt.Errorf("layout.Fetch: image %s has no layers", chosen) } - bundle, err := layers[0].Uncompressed() + bundle, err := layers[0].Compressed() if err != nil { return nil, nil, fmt.Errorf("layout.Fetch: open bundle layer: %w", err) } meta := &Meta{SchemaVersion: 1} if len(layers) >= 2 { - mrc, err := layers[1].Uncompressed() + mrc, err := layers[1].Compressed() if err != nil { _ = bundle.Close() return nil, nil, fmt.Errorf("layout.Fetch: open meta layer: %w", err) diff --git a/internal/registry/layout_test.go b/internal/registry/layout_test.go index 2d0255f..10218ae 100644 --- a/internal/registry/layout_test.go +++ b/internal/registry/layout_test.go @@ -5,7 +5,15 @@ import ( "context" "io" "path/filepath" + "strings" "testing" + + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/layout" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" ) func newTestLayoutStore(t *testing.T) *layoutStore { @@ -83,10 +91,164 @@ func TestLayoutStore_RoundTrip(t *testing.T) { func TestLayoutStore_FetchMissingRef_Errors(t *testing.T) { ctx := context.Background() s := newTestLayoutStore(t) - _, _, err := s.Fetch(ctx, Ref{Name: "php-core", Digest: "sha256:deadbeef"}) - if err == nil { + // Use a valid-form digest so future refactors that start parsing digests + // via v1.NewHash don't regress this test silently. + emptyLayoutRef := Ref{Name: "php-core", Digest: "sha256:" + strings.Repeat("0", 64)} + if _, _, err := s.Fetch(ctx, emptyLayoutRef); err == nil { t.Fatal("Fetch on empty layout: want error, got nil") } + + // Populated layout, wrong digest — must still error (exercises the + // "not found in index" branch, not just the open() failure). + if err := s.Push(ctx, Ref{Name: "php-core"}, bytes.NewReader([]byte("x")), nil); err != nil { + t.Fatalf("Push: %v", err) + } + missing := Ref{Name: "php-core", Digest: "sha256:" + strings.Repeat("0", 64)} + if _, _, err := s.Fetch(ctx, missing); err == nil { + t.Fatal("Fetch with wrong digest in populated layout: want error, got nil") + } +} + +// pushImageWithoutAnnotation simulates an `oras copy`-style append: it writes +// a single-layer OCI image into the layout WITHOUT the io.buildrush.bundle.name +// annotation, so Has/Fetch must rely on the digest-only fallback path. +func pushImageWithoutAnnotation(t *testing.T, s *layoutStore, payload []byte) v1.Hash { + t.Helper() + img, err := mutate.AppendLayers(empty.Image, static.NewLayer(payload, types.OCILayer)) + if err != nil { + t.Fatalf("append layer: %v", err) + } + p, err := s.openOrInit() + if err != nil { + t.Fatalf("openOrInit: %v", err) + } + if err := p.AppendImage(img); err != nil { + t.Fatalf("AppendImage: %v", err) + } + d, err := img.Digest() + if err != nil { + t.Fatalf("Digest: %v", err) + } + return d +} + +// indexEntryForName returns the digest of the manifest that Push tagged with +// the given Ref.Name annotation. Fails the test if no such entry exists. +func indexEntryForName(t *testing.T, s *layoutStore, name string) string { + t.Helper() + p, err := layout.FromPath(s.root) + if err != nil { + t.Fatalf("FromPath: %v", err) + } + idx, err := p.ImageIndex() + if err != nil { + t.Fatalf("ImageIndex: %v", err) + } + m, err := idx.IndexManifest() + if err != nil { + t.Fatalf("IndexManifest: %v", err) + } + for i := range m.Manifests { + if m.Manifests[i].Annotations[annotationBundleName] == name { + return m.Manifests[i].Digest.String() + } + } + t.Fatalf("no manifest tagged %q in index", name) + return "" +} + +func TestLayoutStore_DigestOnlyFallback_PrefersExactAnnotationMatch(t *testing.T) { + ctx := context.Background() + s := newTestLayoutStore(t) + + // Two pushes with distinct payloads → distinct digests. + if err := s.Push(ctx, Ref{Name: "php-core"}, bytes.NewReader([]byte("core-bytes")), nil); err != nil { + t.Fatalf("Push core: %v", err) + } + if err := s.Push(ctx, Ref{Name: "php-ext-redis"}, bytes.NewReader([]byte("redis-bytes")), nil); err != nil { + t.Fatalf("Push redis: %v", err) + } + + coreDigest := indexEntryForName(t, s, "php-core") + redisDigest := indexEntryForName(t, s, "php-ext-redis") + if coreDigest == redisDigest { + t.Fatalf("expected distinct digests, got %q", coreDigest) + } + + // Correct Name+Digest → must fetch the right bundle. + rc, _, err := s.Fetch(ctx, Ref{Name: "php-core", Digest: coreDigest}) + if err != nil { + t.Fatalf("Fetch(core,coreDigest): %v", err) + } + got, _ := io.ReadAll(rc) + _ = rc.Close() + if !bytes.Equal(got, []byte("core-bytes")) { + t.Fatalf("bundle bytes = %q, want %q", got, "core-bytes") + } + + // Wrong Name + correct-Digest-of-another-manifest → affirmative negative. + has, err := s.Has(ctx, Ref{Name: "php-core", Digest: redisDigest}) + if err != nil { + t.Fatalf("Has(core,redisDigest): %v", err) + } + if has { + t.Fatal("Has(core,redisDigest) = true; want false (annotation mismatch)") + } + if _, _, err := s.Fetch(ctx, Ref{Name: "php-core", Digest: redisDigest}); err == nil { + t.Fatal("Fetch(core,redisDigest): want error, got nil") + } +} + +func TestLayoutStore_DigestOnlyFallback_AcceptsManifestWithoutAnnotation(t *testing.T) { + ctx := context.Background() + s := newTestLayoutStore(t) + + payload := []byte("oras-copied-bytes") + d := pushImageWithoutAnnotation(t, s, payload) + + // Probe with any Name — should succeed because there is no affirmative + // "wrong name" signal on this manifest (annotation absent). + ref := Ref{Name: "whatever", Digest: d.String()} + has, err := s.Has(ctx, ref) + if err != nil { + t.Fatalf("Has: %v", err) + } + if !has { + t.Fatal("Has on un-annotated manifest = false; want true (digest-only fallback)") + } + rc, _, err := s.Fetch(ctx, ref) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + got, _ := io.ReadAll(rc) + _ = rc.Close() + if !bytes.Equal(got, payload) { + t.Fatalf("bundle bytes = %q, want %q", got, payload) + } +} + +func TestLayoutStore_DigestOnlyFallback_RejectsManifestWithWrongAnnotation(t *testing.T) { + ctx := context.Background() + s := newTestLayoutStore(t) + + if err := s.Push(ctx, Ref{Name: "php-ext-redis"}, bytes.NewReader([]byte("redis-bytes")), nil); err != nil { + t.Fatalf("Push: %v", err) + } + redisDigest := indexEntryForName(t, s, "php-ext-redis") + + // Probe with a different Name but the redis manifest's digest. The + // manifest's annotation affirmatively says it's redis, so we must refuse. + probe := Ref{Name: "php-core", Digest: redisDigest} + has, err := s.Has(ctx, probe) + if err != nil { + t.Fatalf("Has: %v", err) + } + if has { + t.Fatal("Has on wrong-annotation manifest = true; want false") + } + if _, _, err := s.Fetch(ctx, probe); err == nil { + t.Fatal("Fetch on wrong-annotation manifest: want error, got nil") + } } func TestLayoutStore_TolerateMissingMeta(t *testing.T) { diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 3cab626..2733c94 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -57,6 +57,11 @@ type Meta struct { // Fetch may return a nil *Meta when the backend has no sidecar metadata for // the ref; callers must tolerate that. Push accepts a nil meta to write a // bundle without a meta sidecar. +// +// Fetch returns the raw layer blob (compressed on the wire; callers +// decompress as needed). This matches the behavior of internal/oci/client.go +// so that a layout-backed store returns bytes byte-identical to what a +// remote-backed store would return for the same manifest. type Store interface { Kind() string Fetch(ctx context.Context, ref Ref) (io.ReadCloser, *Meta, error) From b420521599b3b9d17c7384797df2f8642b3b92bf Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 12:55:40 +0200 Subject: [PATCH 06/11] feat(registry): add remote (HTTPS OCI) backend remoteStore wraps go-containerregistry/pkg/v1/remote for Fetch / Has / ResolveDigest; Push returns ErrUnsupported (remote push lands with the phpup build subcommand in PR 2). Fetch uses layers[i].Compressed() to match the layout backend's raw-blob contract. Auth resolves from INPUT_GITHUB-TOKEN / GITHUB_TOKEN or falls through to anonymous. Tests use the in-process pkg/registry server via httptest, so no network or docker is needed. looksLikeRemote now accepts ':' in the host segment so host:port forms (used by the test registry and by private self-hosted registries on explicit ports) dispatch correctly. --- go.sum | 4 + internal/registry/registry.go | 6 +- internal/registry/remote.go | 133 +++++++++++++++++++++--- internal/registry/remote_test.go | 168 +++++++++++++++++++++++++++++++ 4 files changed, 295 insertions(+), 16 deletions(-) create mode 100644 internal/registry/remote_test.go diff --git a/go.sum b/go.sum index 64c0e94..41293c6 100644 --- a/go.sum +++ b/go.sum @@ -26,10 +26,14 @@ github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOf github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/vbatts/tar-split v0.12.2 h1:w/Y6tjxpeiFMR47yzZPlPj/FcPLpXbTUi/9H7d3CPa4= github.com/vbatts/tar-split v0.12.2/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= +golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= +golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= +golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 2733c94..24fd3b5 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -97,7 +97,9 @@ func Open(uri string) (Store, error) { // looksLikeRemote returns true when the URI's head segment (up to the first // "/") is a plausible registry host: it must contain at least one "." and be -// composed exclusively of ASCII letters, digits, dots, and hyphens. +// composed exclusively of ASCII letters, digits, dots, hyphens, and colons +// (colons appear in host[:port] forms like "127.0.0.1:5000" used by the +// in-process test registry). func looksLikeRemote(uri string) bool { head := uri if i := strings.IndexByte(uri, '/'); i >= 0 { @@ -111,7 +113,7 @@ func looksLikeRemote(uri string) bool { case r >= 'a' && r <= 'z': case r >= 'A' && r <= 'Z': case r >= '0' && r <= '9': - case r == '.' || r == '-': + case r == '.' || r == '-' || r == ':': default: return false } diff --git a/internal/registry/remote.go b/internal/registry/remote.go index 60b6a7f..685bf5c 100644 --- a/internal/registry/remote.go +++ b/internal/registry/remote.go @@ -2,38 +2,143 @@ package registry import ( "context" + "encoding/json" "errors" + "fmt" "io" + "os" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) -// remoteStore is the HTTPS-registry backed Store. -// -// The real implementation (go-containerregistry remote.Image, auth, retries) -// lands in Task 3; this file provides the constructor and Kind() so Open can -// dispatch, plus Push gated on ErrUnsupported since published remotes are -// read-only from the action's perspective. +// remoteStore is the HTTPS-registry backed Store. It wraps +// go-containerregistry/pkg/v1/remote for Fetch / Has / ResolveDigest; Push +// returns ErrUnsupported because remote pushes land with the `phpup build` +// subcommand in a later PR. type remoteStore struct { base string + auth authn.Authenticator } +// openRemote resolves auth from the environment (INPUT_GITHUB-TOKEN first, +// then GITHUB_TOKEN; anonymous if both are empty) and returns the store. func openRemote(uri string) (*remoteStore, error) { - return &remoteStore{base: uri}, nil + token := os.Getenv("INPUT_GITHUB-TOKEN") + if token == "" { + token = os.Getenv("GITHUB_TOKEN") + } + var auth authn.Authenticator + if token != "" { + auth = &authn.Basic{Username: "token", Password: token} + } else { + auth = authn.Anonymous + } + return &remoteStore{base: uri, auth: auth}, nil } func (s *remoteStore) Kind() string { return "remote" } -func (s *remoteStore) Fetch(_ context.Context, _ Ref) (io.ReadCloser, *Meta, error) { - return nil, nil, errors.New("remote.Fetch: implemented in Task 3") +// refFor constructs a name.Reference of the form "/@". +// Both the ref name and digest are required; a missing value is an error +// because the remote backend is strictly digest-addressed. +func (s *remoteStore) refFor(r Ref) (name.Reference, error) { + if r.Name == "" { + return nil, errors.New("remote: ref.Name required") + } + if r.Digest == "" { + return nil, errors.New("remote: ref.Digest required") + } + return name.ParseReference(fmt.Sprintf("%s/%s@%s", s.base, r.Name, r.Digest)) +} + +func (s *remoteStore) Has(ctx context.Context, ref Ref) (bool, error) { + r, err := s.refFor(ref) + if err != nil { + return false, fmt.Errorf("remote.Has %s: %w", ref, err) + } + if _, err := remote.Head(r, remote.WithAuth(s.auth), remote.WithContext(ctx)); err != nil { + var terr *transport.Error + if errors.As(err, &terr) && terr.StatusCode == 404 { + return false, nil + } + return false, fmt.Errorf("remote.Has %s: %w", ref, err) + } + return true, nil +} + +func (s *remoteStore) Fetch(ctx context.Context, ref Ref) (io.ReadCloser, *Meta, error) { + r, err := s.refFor(ref) + if err != nil { + return nil, nil, fmt.Errorf("remote.Fetch %s: %w", ref, err) + } + desc, err := remote.Get(r, remote.WithAuth(s.auth), remote.WithContext(ctx)) + if err != nil { + return nil, nil, fmt.Errorf("remote.Fetch %s: %w", ref, err) + } + img, err := desc.Image() + if err != nil { + return nil, nil, fmt.Errorf("remote.Fetch %s: load image: %w", ref, err) + } + layers, err := img.Layers() + if err != nil { + return nil, nil, fmt.Errorf("remote.Fetch %s: list layers: %w", ref, err) + } + if len(layers) == 0 { + return nil, nil, fmt.Errorf("remote.Fetch %s: image has no layers", ref) + } + + // Use Compressed() so the returned bytes match what the layout backend + // returns for the same manifest — both surfaces are "raw layer blob". + bundle, err := layers[0].Compressed() + if err != nil { + return nil, nil, fmt.Errorf("remote.Fetch %s: open bundle layer: %w", ref, err) + } + + meta := &Meta{SchemaVersion: 1} + if len(layers) >= 2 { + mrc, err := layers[1].Compressed() + if err != nil { + _ = bundle.Close() + return nil, nil, fmt.Errorf("remote.Fetch %s: open meta layer: %w", ref, err) + } + metaBytes, err := io.ReadAll(mrc) + _ = mrc.Close() + if err != nil { + _ = bundle.Close() + return nil, nil, fmt.Errorf("remote.Fetch %s: read meta layer: %w", ref, err) + } + parsed := &Meta{} + if err := json.Unmarshal(metaBytes, parsed); err != nil { + _ = bundle.Close() + return nil, nil, fmt.Errorf("remote.Fetch %s: parse meta: %w", ref, err) + } + if parsed.SchemaVersion == 0 { + parsed.SchemaVersion = 1 + } + meta = parsed + } + return bundle, meta, nil } +// Push is deliberately unsupported in PR 1; remote publication lands with the +// `phpup build` subcommand in a follow-up PR. func (s *remoteStore) Push(_ context.Context, _ Ref, _ io.Reader, _ *Meta) error { return ErrUnsupported } -func (s *remoteStore) Has(_ context.Context, _ Ref) (bool, error) { - return false, errors.New("remote.Has: implemented in Task 3") +func (s *remoteStore) ResolveDigest(ctx context.Context, reference string) (string, error) { + ref, err := name.ParseReference(reference) + if err != nil { + return "", fmt.Errorf("remote.ResolveDigest %q: %w", reference, err) + } + desc, err := remote.Head(ref, remote.WithAuth(s.auth), remote.WithContext(ctx)) + if err != nil { + return "", fmt.Errorf("remote.ResolveDigest %q: %w", reference, err) + } + return desc.Digest.String(), nil } -func (s *remoteStore) ResolveDigest(_ context.Context, _ string) (string, error) { - return "", errors.New("remote.ResolveDigest: implemented in Task 3") -} +var _ Store = (*remoteStore)(nil) diff --git a/internal/registry/remote_test.go b/internal/registry/remote_test.go new file mode 100644 index 0000000..098e44c --- /dev/null +++ b/internal/registry/remote_test.go @@ -0,0 +1,168 @@ +package registry + +import ( + "bytes" + "context" + "io" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" +) + +// startTestRegistry spins up an in-process OCI registry and returns its +// host:port. It's torn down by t.Cleanup. +func startTestRegistry(t *testing.T) string { + t.Helper() + srv := httptest.NewServer(registry.New()) + t.Cleanup(srv.Close) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parse server URL: %v", err) + } + return u.Host +} + +// seedTestRegistry pushes a two-layer image (bundle + meta.json) to the +// registry at path "/buildrush/php-core:seed". Returns the manifest digest. +func seedTestRegistry(t *testing.T, host string, bundle, metaJSON []byte) string { + t.Helper() + ref, err := name.ParseReference(host + "/buildrush/php-core:seed") + if err != nil { + t.Fatalf("parse ref: %v", err) + } + bundleLayer := static.NewLayer(bundle, types.OCILayer) + img := empty.Image + img, err = mutate.AppendLayers(img, bundleLayer) + if err != nil { + t.Fatalf("append bundle: %v", err) + } + if metaJSON != nil { + metaLayer := static.NewLayer(metaJSON, types.OCILayer) + img, err = mutate.AppendLayers(img, metaLayer) + if err != nil { + t.Fatalf("append meta: %v", err) + } + } + if err := remote.Write(ref, img); err != nil { + t.Fatalf("seed registry: %v", err) + } + d, err := img.Digest() + if err != nil { + t.Fatalf("digest: %v", err) + } + return d.String() +} + +func TestRemoteStore_FetchSeededImage(t *testing.T) { + ctx := context.Background() + host := startTestRegistry(t) + payload := []byte("remote bundle payload") + metaJSON := []byte(`{"schema_version":2,"kind":"php-core"}`) + digest := seedTestRegistry(t, host, payload, metaJSON) + + s, err := Open(host + "/buildrush") + if err != nil { + t.Fatalf("Open: %v", err) + } + if s.Kind() != "remote" { + t.Fatalf("Kind = %q, want remote", s.Kind()) + } + + rc, meta, err := s.Fetch(ctx, Ref{Name: "php-core", Digest: digest}) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + defer rc.Close() + got, err := io.ReadAll(rc) + if err != nil { + t.Fatalf("read: %v", err) + } + if !bytes.Equal(got, payload) { + t.Fatalf("payload = %q, want %q", got, payload) + } + if meta == nil || meta.SchemaVersion != 2 || meta.Kind != "php-core" { + t.Fatalf("meta = %+v", meta) + } +} + +func TestRemoteStore_FetchWithoutMetaLayer(t *testing.T) { + ctx := context.Background() + host := startTestRegistry(t) + payload := []byte("legacy-bundle-no-meta") + digest := seedTestRegistry(t, host, payload, nil) // no meta layer + + s, _ := Open(host + "/buildrush") + rc, meta, err := s.Fetch(ctx, Ref{Name: "php-core", Digest: digest}) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + defer rc.Close() + got, _ := io.ReadAll(rc) + if !bytes.Equal(got, payload) { + t.Fatalf("payload mismatch") + } + if meta == nil || meta.SchemaVersion != 1 { + t.Fatalf("meta = %+v, want SchemaVersion:1 (legacy default)", meta) + } +} + +func TestRemoteStore_HasSeededImage(t *testing.T) { + ctx := context.Background() + host := startTestRegistry(t) + digest := seedTestRegistry(t, host, []byte("x"), nil) + s, _ := Open(host + "/buildrush") + has, err := s.Has(ctx, Ref{Name: "php-core", Digest: digest}) + if err != nil { + t.Fatalf("Has: %v", err) + } + if !has { + t.Fatal("Has returned false for seeded ref") + } +} + +func TestRemoteStore_HasMissingRef_FalseNoError(t *testing.T) { + ctx := context.Background() + host := startTestRegistry(t) + s, _ := Open(host + "/buildrush") + missing := Ref{Name: "php-core", Digest: "sha256:" + strings.Repeat("0", 64)} + has, err := s.Has(ctx, missing) + if err != nil { + t.Fatalf("Has on missing ref: err = %v, want nil (404 should be \"not present\", not an error)", err) + } + if has { + t.Fatal("Has returned true on empty registry") + } +} + +func TestRemoteStore_PushReturnsUnsupported(t *testing.T) { + ctx := context.Background() + host := startTestRegistry(t) + s, _ := Open(host + "/buildrush") + err := s.Push(ctx, Ref{Name: "php-core"}, bytes.NewReader([]byte("x")), nil) + if err != ErrUnsupported { + t.Fatalf("Push err = %v, want ErrUnsupported", err) + } +} + +func TestRemoteStore_ResolveDigestReturnsSeededDigest(t *testing.T) { + ctx := context.Background() + host := startTestRegistry(t) + digest := seedTestRegistry(t, host, []byte("x"), nil) + s, _ := Open(host + "/buildrush") + got, err := s.ResolveDigest(ctx, host+"/buildrush/php-core:seed") + if err != nil { + t.Fatalf("ResolveDigest: %v", err) + } + if got != digest { + t.Fatalf("ResolveDigest = %q, want %q", got, digest) + } +} From 06013444fd32f6512437aab4c413959af81834d8 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 13:03:13 +0200 Subject: [PATCH 07/11] fix(registry): symmetrize remote.Has with layout + lock host:port dispatch - remoteStore.Has now returns (false, nil) for empty-digest probes so both backends behave identically when asked "do you have this yet?" The Fetch path still rejects empty digest as an error. - Add TestRemoteStore_HasEmptyDigest_FalseNoError to lock the contract. - Add host:port row to TestOpen_URIFormDispatch so the looksLikeRemote scope expansion is covered at the dispatch layer, not implicitly. - Use errors.Is for the ErrUnsupported assertion in the Push test. - Comment the 401-as-not-present gotcha for when private-repo probes become relevant in Task 6/7. --- internal/registry/registry_test.go | 5 +++++ internal/registry/remote.go | 11 +++++++++++ internal/registry/remote_test.go | 18 +++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index f165a3b..27bbed3 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -88,6 +88,11 @@ func TestOpen_URIFormDispatch(t *testing.T) { uri: "ghcr.io/weird_path", wantKind: "remote", }, + { + name: "host with port succeeds as remote", + uri: "127.0.0.1:5000/foo", + wantKind: "remote", + }, } for _, tc := range cases { diff --git a/internal/registry/remote.go b/internal/registry/remote.go index 685bf5c..c13c003 100644 --- a/internal/registry/remote.go +++ b/internal/registry/remote.go @@ -55,11 +55,22 @@ func (s *remoteStore) refFor(r Ref) (name.Reference, error) { } func (s *remoteStore) Has(ctx context.Context, ref Ref) (bool, error) { + // Symmetry with layoutStore.Has: an empty-digest probe is a legal + // "not present" query. Fetch still rejects empty Digest as an error + // because you can't fetch "nothing". + if ref.Digest == "" { + return false, nil + } r, err := s.refFor(ref) if err != nil { return false, fmt.Errorf("remote.Has %s: %w", ref, err) } if _, err := remote.Head(r, remote.WithAuth(s.auth), remote.WithContext(ctx)); err != nil { + // NOTE: Some registries (notably GHCR) return 401 Unauthorized for + // blobs in private repos when the caller is anonymous, to avoid leaking + // existence of private content. That will propagate as an error here. + // If/when private-repo probes become a real use case (Task 6/7), consider + // treating (terr.StatusCode == 401 && auth == anonymous) as "not present". var terr *transport.Error if errors.As(err, &terr) && terr.StatusCode == 404 { return false, nil diff --git a/internal/registry/remote_test.go b/internal/registry/remote_test.go index 098e44c..b742c1e 100644 --- a/internal/registry/remote_test.go +++ b/internal/registry/remote_test.go @@ -3,6 +3,7 @@ package registry import ( "bytes" "context" + "errors" "io" "net/http/httptest" "net/url" @@ -148,11 +149,26 @@ func TestRemoteStore_PushReturnsUnsupported(t *testing.T) { host := startTestRegistry(t) s, _ := Open(host + "/buildrush") err := s.Push(ctx, Ref{Name: "php-core"}, bytes.NewReader([]byte("x")), nil) - if err != ErrUnsupported { + if !errors.Is(err, ErrUnsupported) { t.Fatalf("Push err = %v, want ErrUnsupported", err) } } +func TestRemoteStore_HasEmptyDigest_FalseNoError(t *testing.T) { + // Symmetry with layoutStore.Has: a probe with empty digest is a legal + // "not present" query across both backends (see Store.Has docstring). + ctx := context.Background() + host := startTestRegistry(t) + s, _ := Open(host + "/buildrush") + has, err := s.Has(ctx, Ref{Name: "php-core"}) // Digest intentionally empty + if err != nil { + t.Fatalf("Has empty-digest: err = %v, want nil (symmetry with layout backend)", err) + } + if has { + t.Fatal("Has empty-digest returned true on empty registry") + } +} + func TestRemoteStore_ResolveDigestReturnsSeededDigest(t *testing.T) { ctx := context.Background() host := startTestRegistry(t) From fb66cd190d66fa6cabc2567343cf5467cc6fc844 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 13:07:06 +0200 Subject: [PATCH 08/11] refactor(oci): delegate to internal/registry internal/oci.Client now holds a registry.Store; its public surface (NewClient, FetchAll, Fetch, Exists, ResolveDigest, ResolvedBundle, FetchResult, Metadata) is unchanged so existing call sites in cmd/phpup, cmd/planner and cmd/lockfile-update compile without modification. NewClient now accepts "oci-layout:" URIs in addition to remote hosts, enabling hermetic local fetches without touching GHCR. The facade itself is slated for deletion by end of PR 3 of the local+CI unification rollout. --- internal/oci/client.go | 222 ++++++++++++++++-------------------- internal/oci/client_test.go | 23 +++- internal/oci/meta.go | 25 ++++ 3 files changed, 144 insertions(+), 126 deletions(-) create mode 100644 internal/oci/meta.go diff --git a/internal/oci/client.go b/internal/oci/client.go index 6714033..92ba627 100644 --- a/internal/oci/client.go +++ b/internal/oci/client.go @@ -2,22 +2,26 @@ package oci import ( "context" - "encoding/json" "fmt" "io" + "os" "sync" - "github.com/google/go-containerregistry/pkg/authn" - "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/buildrush/setup-php/internal/registry" ) +// Client is a thin facade over an internal/registry.Store. Its public surface +// is preserved for backwards compatibility with the pre-refactor call sites +// in cmd/phpup, cmd/planner, and cmd/lockfile-update. The facade itself is +// slated for deletion by end of PR 3 of the local+CI unification rollout. type Client struct { - registry string - token string - auth authn.Authenticator + registryURI string + token string + store registry.Store } +// FetchResult is returned by Fetch/FetchAll. Data is the raw (compressed) +// bundle layer bytes; callers decompress as needed. type FetchResult struct { Key string Digest string @@ -25,25 +29,16 @@ type FetchResult struct { Metadata Metadata } -// Metadata is the parsed OCI sidecar meta.json shipped alongside every -// bundle. Missing schema_version is treated as 1 so pre-slice bundles -// referenced by released lockfiles remain loadable. +// Metadata mirrors registry.Meta; kept here for backwards compatibility with +// callers that import oci.Metadata directly. Will be removed when the facade +// is deleted (scheduled for end of PR 3 of the local+CI unification rollout). type Metadata struct { SchemaVersion int `json:"schema_version"` Kind string `json:"kind"` } -func parseMetaJSON(data []byte) (Metadata, error) { - var m Metadata - if err := json.Unmarshal(data, &m); err != nil { - return Metadata{}, fmt.Errorf("parse meta.json: %w", err) - } - if m.SchemaVersion == 0 { - m.SchemaVersion = 1 - } - return m, nil -} - +// ResolvedBundle identifies a bundle to fetch. Kind selects the OCI name +// prefix ("php-core", "php-ext-", "php-tool-"). type ResolvedBundle struct { Key string Digest string @@ -52,39 +47,57 @@ type ResolvedBundle struct { Kind string } -func NewClient(registry, token string) (*Client, error) { - var auth authn.Authenticator +// NewClient preserves the legacy constructor shape. The registryURI argument +// may be any URI accepted by registry.Open — including the new +// "oci-layout:" form added in PR 1 — so callers that switch to an +// oci-layout source do not need a different entry point. +// +// Token is honoured by the remote backend via env lookup +// (INPUT_GITHUB-TOKEN / GITHUB_TOKEN). If the caller passes an explicit +// token and the env is empty, we thread it through by setting +// INPUT_GITHUB-TOKEN for this process. This matches the behaviour of the +// pre-refactor code, which built an authn.Basic directly from the token +// arg. No-op for the layout backend. +func NewClient(registryURI, token string) (*Client, error) { if token != "" { - auth = &authn.Basic{Username: "token", Password: token} - } else { - auth = authn.Anonymous + setIfUnset("INPUT_GITHUB-TOKEN", token) + } + s, err := registry.Open(registryURI) + if err != nil { + return nil, fmt.Errorf("oci.NewClient: %w", err) } - return &Client{registry: registry, token: token, auth: auth}, nil + return &Client{registryURI: registryURI, token: token, store: s}, nil } +func setIfUnset(key, value string) { + if os.Getenv(key) == "" { + _ = os.Setenv(key, value) + } +} + +// FetchAll concurrently fetches every bundle in the slice. On the first error +// from any goroutine it returns a wrapped error tagged with the failing +// bundle's Key; successful sibling fetches are discarded. func (c *Client) FetchAll(ctx context.Context, bundles []ResolvedBundle) ([]FetchResult, error) { if len(bundles) == 0 { return nil, nil } - results := make([]FetchResult, len(bundles)) errs := make([]error, len(bundles)) var wg sync.WaitGroup - for i := range bundles { wg.Add(1) - go func(idx int, bundle *ResolvedBundle) { + go func(idx int, b *ResolvedBundle) { defer wg.Done() - result, err := c.Fetch(ctx, bundle) + r, err := c.Fetch(ctx, b) if err != nil { errs[idx] = err return } - results[idx] = *result + results[idx] = *r }(i, &bundles[i]) } wg.Wait() - for i, err := range errs { if err != nil { return nil, fmt.Errorf("fetch %s: %w", bundles[i].Key, err) @@ -93,116 +106,77 @@ func (c *Client) FetchAll(ctx context.Context, bundles []ResolvedBundle) ([]Fetc return results, nil } -func (c *Client) Fetch(ctx context.Context, bundle *ResolvedBundle) (*FetchResult, error) { - ref, err := c.bundleRef(bundle) +// Fetch delegates to the underlying Store, mapping the ResolvedBundle to a +// registry.Ref by kind-prefix, and normalises the returned Meta into a +// package-local Metadata (defaulting SchemaVersion to 1 when absent, matching +// the pre-refactor permissive behaviour for legacy bundles). +func (c *Client) Fetch(ctx context.Context, b *ResolvedBundle) (*FetchResult, error) { + ref := registry.Ref{Name: ociName(b), Digest: b.Digest} + rc, meta, err := c.store.Fetch(ctx, ref) if err != nil { - return nil, err + return nil, fmt.Errorf("fetch %s: %w", b.Key, err) } - - desc, err := remote.Get(ref, remote.WithAuth(c.auth), remote.WithContext(ctx)) - if err != nil { - return nil, fmt.Errorf("fetch %s: %w", bundle.Key, err) - } - - layer, err := desc.Image() + defer func() { _ = rc.Close() }() + data, err := io.ReadAll(rc) if err != nil { - return nil, fmt.Errorf("get image %s: %w", bundle.Key, err) + return nil, fmt.Errorf("read %s: %w", b.Key, err) } - - layers, err := layer.Layers() - if err != nil || len(layers) == 0 { - return nil, fmt.Errorf("get layers %s: no layers found", bundle.Key) + out := &FetchResult{ + Key: b.Key, + Digest: b.Digest, + Data: data, } - - rc, err := layers[0].Compressed() - if err != nil { - return nil, fmt.Errorf("read layer %s: %w", bundle.Key, err) - } - defer func() { _ = rc.Close() }() - - var data []byte - buf := make([]byte, 32*1024) - for { - n, err := rc.Read(buf) - if n > 0 { - data = append(data, buf[:n]...) - } - if err != nil { - break - } + if meta != nil { + out.Metadata = Metadata{SchemaVersion: meta.SchemaVersion, Kind: meta.Kind} } - - // The manifest digest in bundle.Digest was already verified by the - // OCI library when fetching via content-addressed reference. The - // layer integrity is covered by the manifest's layer descriptors, - // which the library also validates. No extra hashing needed here. - - // Second layer, if present, is the meta.json sidecar. Absence is - // tolerated for forward-compat with legacy bundles. - var meta Metadata - if len(layers) >= 2 { - mrc, err := layers[1].Compressed() - if err != nil { - return nil, fmt.Errorf("read meta layer %s: %w", bundle.Key, err) - } - mbuf, readErr := io.ReadAll(mrc) - _ = mrc.Close() - if readErr != nil { - return nil, fmt.Errorf("read meta bytes %s: %w", bundle.Key, readErr) - } - meta, err = parseMetaJSON(mbuf) - if err != nil { - return nil, fmt.Errorf("parse meta %s: %w", bundle.Key, err) - } - } else { - meta.SchemaVersion = 1 // legacy bundle; permissive default + if out.Metadata.SchemaVersion == 0 { + out.Metadata.SchemaVersion = 1 } - - return &FetchResult{ - Key: bundle.Key, - Digest: bundle.Digest, - Data: data, - Metadata: meta, - }, nil + return out, nil } +// Exists reports whether the given ref (optionally fully-qualified with the +// registry host prefix) and digest are present in the backing Store. func (c *Client) Exists(ctx context.Context, ref, digest string) (bool, error) { - r, err := name.ParseReference(ref) - if err != nil { - return false, err - } - _, err = remote.Head(r, remote.WithAuth(c.auth), remote.WithContext(ctx)) - if err != nil { - return false, fmt.Errorf("check existence %s: %w", ref, err) - } - return true, nil + return c.store.Has(ctx, registry.Ref{Name: stripHost(ref, c.registryURI), Digest: digest}) } // ResolveDigest looks up the OCI manifest digest for the given tagged or -// digest-form reference. Returns "sha256:..." on success. +// digest-form reference. Returns "sha256:..." on success. Layout-backed +// clients surface an ErrUnsupported-style error from the store. func (c *Client) ResolveDigest(ctx context.Context, ref string) (string, error) { - r, err := name.ParseReference(ref) - if err != nil { - return "", fmt.Errorf("parse ref %s: %w", ref, err) - } - desc, err := remote.Head(r, remote.WithAuth(c.auth), remote.WithContext(ctx)) - if err != nil { - return "", fmt.Errorf("head %s: %w", ref, err) - } - return desc.Digest.String(), nil + return c.store.ResolveDigest(ctx, ref) } -func (c *Client) bundleRef(bundle *ResolvedBundle) (name.Reference, error) { - var refStr string - switch bundle.Kind { +// ociName maps a ResolvedBundle to the OCI name within the Store. +// Mirrors the prior bundleRef() behaviour: +// +// - "php" → "php-core" +// - "ext" → "php-ext-" +// - "tool" → "php-tool-" +// - other → b.Name (defensive passthrough) +func ociName(b *ResolvedBundle) string { + switch b.Kind { case "php": - refStr = fmt.Sprintf("%s/php-core@%s", c.registry, bundle.Digest) + return "php-core" case "ext": - refStr = fmt.Sprintf("%s/php-ext-%s@%s", c.registry, bundle.Name, bundle.Digest) + return "php-ext-" + b.Name case "tool": - refStr = fmt.Sprintf("%s/php-tool-%s@%s", c.registry, bundle.Name, bundle.Digest) + return "php-tool-" + b.Name default: - return nil, fmt.Errorf("unknown bundle kind %q", bundle.Kind) + return b.Name + } +} + +// stripHost removes a leading "/" from ref so Exists() can +// accept either a fully-qualified reference or a bare name. Mirrors the +// prior client's tolerance for both forms. +func stripHost(ref, root string) string { + if root == "" { + return ref + } + if len(ref) > len(root)+1 && ref[:len(root)] == root && ref[len(root)] == '/' { + return ref[len(root)+1:] } - return name.ParseReference(refStr) + return ref } diff --git a/internal/oci/client_test.go b/internal/oci/client_test.go index bd67e61..a2af26b 100644 --- a/internal/oci/client_test.go +++ b/internal/oci/client_test.go @@ -10,8 +10,8 @@ func TestNewClient(t *testing.T) { if err != nil { t.Fatalf("NewClient() error = %v", err) } - if c.registry != "ghcr.io/buildrush" { - t.Errorf("registry = %q, want ghcr.io/buildrush", c.registry) + if c.registryURI != "ghcr.io/buildrush" { + t.Errorf("registryURI = %q, want ghcr.io/buildrush", c.registryURI) } } @@ -58,3 +58,22 @@ func TestParseMetaJSON_MalformedJSON_ReturnsError(t *testing.T) { t.Fatal("expected error on malformed JSON, got nil") } } + +func TestNewClient_AcceptsOciLayoutURI(t *testing.T) { + c, err := NewClient("oci-layout:/tmp/nonexistent-oci-layout", "") + if err != nil { + t.Fatalf("NewClient(oci-layout:) err = %v, want nil", err) + } + if c == nil { + t.Fatal("NewClient returned nil") + } + // FetchAll(nil) should return (nil, nil) on any backend — verifies the + // delegation path is wired up without needing a real bundle on disk. + results, err := c.FetchAll(context.Background(), nil) + if err != nil { + t.Fatalf("FetchAll(nil) on layout-backed client: %v", err) + } + if len(results) != 0 { + t.Errorf("FetchAll(nil) returned %d results, want 0", len(results)) + } +} diff --git a/internal/oci/meta.go b/internal/oci/meta.go new file mode 100644 index 0000000..f4b0b65 --- /dev/null +++ b/internal/oci/meta.go @@ -0,0 +1,25 @@ +package oci + +import ( + "encoding/json" + "fmt" +) + +// parseMetaJSON decodes an OCI sidecar meta.json payload into Metadata. +// Missing schema_version defaults to 1 so pre-slice bundles referenced by +// released lockfiles remain loadable. +// +// The registry package owns equivalent decoding inside Store.Fetch; this +// helper survives here to keep the oci facade's legacy test surface intact +// until the facade itself is deleted (scheduled for end of PR 3 of the +// local+CI unification rollout). +func parseMetaJSON(data []byte) (Metadata, error) { + var m Metadata + if err := json.Unmarshal(data, &m); err != nil { + return Metadata{}, fmt.Errorf("parse meta.json: %w", err) + } + if m.SchemaVersion == 0 { + m.SchemaVersion = 1 + } + return m, nil +} From cedd490e01953822e993ed679a6b17a91d0a366d Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 13:15:09 +0200 Subject: [PATCH 09/11] fix(oci): validate bundle Kind + make token env write concurrency-safe - Fetch now errors with "unknown bundle kind %q" before calling ociName, restoring the pre-refactor diagnostic that returned a clear error instead of silently constructing a wrong remote reference. - The token env-var write in NewClient is now gated by sync.Once, making the "first non-empty token wins" semantics explicit and race-free. Godoc spells out the contract. - Add regression tests for both paths. --- internal/oci/client.go | 41 +++++++++++++++++++++++++++---------- internal/oci/client_test.go | 31 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/internal/oci/client.go b/internal/oci/client.go index 92ba627..1868803 100644 --- a/internal/oci/client.go +++ b/internal/oci/client.go @@ -53,15 +53,17 @@ type ResolvedBundle struct { // oci-layout source do not need a different entry point. // // Token is honoured by the remote backend via env lookup -// (INPUT_GITHUB-TOKEN / GITHUB_TOKEN). If the caller passes an explicit -// token and the env is empty, we thread it through by setting -// INPUT_GITHUB-TOKEN for this process. This matches the behaviour of the -// pre-refactor code, which built an authn.Basic directly from the token -// arg. No-op for the layout backend. +// (INPUT_GITHUB-TOKEN / GITHUB_TOKEN). When token is non-empty and +// INPUT_GITHUB-TOKEN is unset, NewClient writes the token into +// INPUT_GITHUB-TOKEN for the remote backend's env-based auth. This write +// happens at most once per process (via sync.Once) — a subsequent +// NewClient call with a different token is a no-op. For production use +// this is fine (phpup runs one binary with one token); if you need +// multiple registries with different credentials in the same process, +// plumb auth via registry.Open directly once that's supported (slated +// for PR 2). No-op for the layout backend. func NewClient(registryURI, token string) (*Client, error) { - if token != "" { - setIfUnset("INPUT_GITHUB-TOKEN", token) - } + ensureTokenEnv(token) s, err := registry.Open(registryURI) if err != nil { return nil, fmt.Errorf("oci.NewClient: %w", err) @@ -69,10 +71,21 @@ func NewClient(registryURI, token string) (*Client, error) { return &Client{registryURI: registryURI, token: token, store: s}, nil } -func setIfUnset(key, value string) { - if os.Getenv(key) == "" { - _ = os.Setenv(key, value) +var tokenEnvOnce sync.Once + +// ensureTokenEnv writes INPUT_GITHUB-TOKEN=token exactly once per process, +// and only if the env var is not already set. Subsequent calls (even with +// a different token) are no-ops — this matches the pre-refactor behaviour +// where the first NewClient fixed the auth for the process lifetime. +func ensureTokenEnv(token string) { + if token == "" { + return } + tokenEnvOnce.Do(func() { + if os.Getenv("INPUT_GITHUB-TOKEN") == "" { + _ = os.Setenv("INPUT_GITHUB-TOKEN", token) + } + }) } // FetchAll concurrently fetches every bundle in the slice. On the first error @@ -111,6 +124,12 @@ func (c *Client) FetchAll(ctx context.Context, bundles []ResolvedBundle) ([]Fetc // package-local Metadata (defaulting SchemaVersion to 1 when absent, matching // the pre-refactor permissive behaviour for legacy bundles). func (c *Client) Fetch(ctx context.Context, b *ResolvedBundle) (*FetchResult, error) { + switch b.Kind { + case "php", "ext", "tool": + // ok + default: + return nil, fmt.Errorf("fetch %s: unknown bundle kind %q", b.Key, b.Kind) + } ref := registry.Ref{Name: ociName(b), Digest: b.Digest} rc, meta, err := c.store.Fetch(ctx, ref) if err != nil { diff --git a/internal/oci/client_test.go b/internal/oci/client_test.go index a2af26b..7c70e30 100644 --- a/internal/oci/client_test.go +++ b/internal/oci/client_test.go @@ -2,6 +2,8 @@ package oci import ( "context" + "os" + "strings" "testing" ) @@ -77,3 +79,32 @@ func TestNewClient_AcceptsOciLayoutURI(t *testing.T) { t.Errorf("FetchAll(nil) returned %d results, want 0", len(results)) } } + +func TestFetch_UnknownKind_Errors(t *testing.T) { + c, err := NewClient("oci-layout:/tmp/nonexistent-for-fetch-kind-test", "") + if err != nil { + t.Fatalf("NewClient: %v", err) + } + b := &ResolvedBundle{Key: "weird:thing", Kind: "unknown"} + _, err = c.Fetch(context.Background(), b) + if err == nil { + t.Fatal("Fetch with unknown kind: want error, got nil") + } + if !strings.Contains(err.Error(), "unknown bundle kind") { + t.Errorf("error = %q, want substring \"unknown bundle kind\"", err) + } +} + +func TestEnsureTokenEnv_IsIdempotent(t *testing.T) { + // Cannot reliably assert what the env becomes on the first call (sync.Once + // may have been tripped by a prior test in this process), but we CAN assert + // that a second call with a different token does not change whatever is + // already there. + ensureTokenEnv("token-A") + before := os.Getenv("INPUT_GITHUB-TOKEN") + ensureTokenEnv("token-B") + after := os.Getenv("INPUT_GITHUB-TOKEN") + if before != after { + t.Fatalf("ensureTokenEnv mutated env on second call: before=%q after=%q", before, after) + } +} From d74a691b95319d39fec52b06ad3297a431f4408a Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 13:18:50 +0200 Subject: [PATCH 10/11] feat(phpup): add --registry flag (env: INPUT_REGISTRY / PHPUP_REGISTRY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit phpup install now accepts an explicit --registry URI and falls back through INPUT_REGISTRY (GitHub Actions input convention) and PHPUP_REGISTRY (local/CLI convention) before the hardcoded ghcr.io/buildrush default. Combined with internal/registry's oci-layout backend, end users can run phpup against a local filesystem layout — e.g. PHPUP_REGISTRY=oci-layout:./out/oci-layout — for hermetic dev loops. Action input "registry" surfaces the same knob to Action consumers; GitHub Actions forwards it as INPUT_REGISTRY and src/index.js already passes the full env to phpup at exec time, so no shim changes are needed. --- action.yml | 7 ++++++ cmd/phpup/main.go | 25 ++++++++++++++++++++- cmd/phpup/registry_flag_test.go | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 cmd/phpup/registry_flag_test.go diff --git a/action.yml b/action.yml index 0887b31..b819351 100644 --- a/action.yml +++ b/action.yml @@ -45,6 +45,13 @@ inputs: description: 'GitHub token for authenticated registry access' required: false default: ${{ github.token }} + registry: + description: > + OCI artifact store used to fetch PHP and extension bundles. + Defaults to ghcr.io/buildrush. Set to oci-layout: to + fetch from a local OCI-layout directory (used by local CI). + required: false + default: "" outputs: php-version: description: 'The installed PHP version' diff --git a/cmd/phpup/main.go b/cmd/phpup/main.go index 6b95895..0799a6e 100644 --- a/cmd/phpup/main.go +++ b/cmd/phpup/main.go @@ -3,6 +3,7 @@ package main import ( "context" _ "embed" + "flag" "fmt" "log" "os" @@ -29,12 +30,34 @@ import ( //go:embed bundles.lock var embeddedLockfile []byte +// resolveRegistry picks the registry URI with precedence: +// 1. --registry flag (cliFlag, passed in) +// 2. INPUT_REGISTRY env var (GitHub Actions input convention) +// 3. PHPUP_REGISTRY env var (local/CLI convention) +// 4. default "ghcr.io/buildrush" +func resolveRegistry(cliFlag string) string { + if cliFlag != "" { + return cliFlag + } + if v := os.Getenv("INPUT_REGISTRY"); v != "" { + return v + } + if v := os.Getenv("PHPUP_REGISTRY"); v != "" { + return v + } + return "ghcr.io/buildrush" +} + func main() { if len(os.Args) > 1 && os.Args[1] == "--version" { fmt.Printf("phpup %s (%s) built %s\n", version.Version, version.Commit, version.BuildDate) return } + registryFlag := flag.String("registry", "", + "OCI artifact store (e.g., ghcr.io/buildrush or oci-layout:./out/oci-layout). Overrides INPUT_REGISTRY / PHPUP_REGISTRY; defaults to ghcr.io/buildrush.") + flag.Parse() + ctx := context.Background() // 1. Parse inputs @@ -136,7 +159,7 @@ func main() { if token == "" { token = os.Getenv("GITHUB_TOKEN") } - registry := "ghcr.io/buildrush" + registry := resolveRegistry(*registryFlag) client, err := oci.NewClient(registry, token) if err != nil { log.Fatalf("create OCI client: %v", err) diff --git a/cmd/phpup/registry_flag_test.go b/cmd/phpup/registry_flag_test.go new file mode 100644 index 0000000..c5edc49 --- /dev/null +++ b/cmd/phpup/registry_flag_test.go @@ -0,0 +1,39 @@ +package main + +import "testing" + +func TestResolveRegistry_FlagBeatsEnv(t *testing.T) { + t.Setenv("INPUT_REGISTRY", "env-input") + t.Setenv("PHPUP_REGISTRY", "env-phpup") + got := resolveRegistry("flag-value") + if got != "flag-value" { + t.Errorf("resolveRegistry flag = %q, want flag-value", got) + } +} + +func TestResolveRegistry_InputEnvBeatsPhpupEnv(t *testing.T) { + t.Setenv("INPUT_REGISTRY", "env-input") + t.Setenv("PHPUP_REGISTRY", "env-phpup") + got := resolveRegistry("") + if got != "env-input" { + t.Errorf("resolveRegistry = %q, want env-input", got) + } +} + +func TestResolveRegistry_PhpupEnvUsedWhenInputEmpty(t *testing.T) { + t.Setenv("INPUT_REGISTRY", "") + t.Setenv("PHPUP_REGISTRY", "env-phpup") + got := resolveRegistry("") + if got != "env-phpup" { + t.Errorf("resolveRegistry = %q, want env-phpup", got) + } +} + +func TestResolveRegistry_DefaultsToGHCR(t *testing.T) { + t.Setenv("INPUT_REGISTRY", "") + t.Setenv("PHPUP_REGISTRY", "") + got := resolveRegistry("") + if got != "ghcr.io/buildrush" { + t.Errorf("resolveRegistry = %q, want ghcr.io/buildrush", got) + } +} From 124fd354fc6836c433c4454a55dd22e56e26fedd Mon Sep 17 00:00:00 2001 From: Maximilian Date: Thu, 23 Apr 2026 13:31:42 +0200 Subject: [PATCH 11/11] test(registry): add oci-layout e2e verification harness test/registry/e2e_layout.sh seeds a local OCI layout from GHCR via oras, then runs phpup with PHPUP_REGISTRY pointing at the layout and asserts the resolved php binary runs. Provides a manual gate for PR 1 of the local+CI unification rollout and a fixture that PR 4's CI rework will reuse. The script exercises the Task 2 digest-only fallback in the layout backend: oras cp does not emit the io.buildrush.bundle.name annotation setup-php's pusher writes, so the layout's Has/Fetch must match by digest alone when no annotation is present. Pre-reqs documented in the script header (oras, jq, go); GHCR access via GITHUB_TOKEN/GHCR_TOKEN when private or rate-limited. Fails fast on non-linux hosts since the published php-core bundles are linux-only (run on CI or wrap in a linux container). Verified end-to-end on linux/arm64 by running the script inside an ubuntu:22.04 container: seeds the layout, phpup install completes without GHCR access, and the composed PHP 8.4.20 binary reports "PHP 8.4.20 (cli) (NTS)". --- test/registry/e2e_layout.sh | 170 ++++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100755 test/registry/e2e_layout.sh diff --git a/test/registry/e2e_layout.sh b/test/registry/e2e_layout.sh new file mode 100755 index 0000000..29f0633 --- /dev/null +++ b/test/registry/e2e_layout.sh @@ -0,0 +1,170 @@ +#!/usr/bin/env bash +# End-to-end verification that `phpup install` can source bundles from a +# local OCI layout via the --registry flag (or PHPUP_REGISTRY env var). +# +# Flow: +# 1. Build phpup from the working tree. +# 2. Resolve a real php-core digest from bundles.lock. +# 3. Seed a fresh on-disk OCI layout from GHCR with `oras cp` (ONE-TIME +# network access). +# 4. Run phpup with PHPUP_REGISTRY=oci-layout: — phpup must source +# the bundle from the layout, never contacting GHCR again. +# 5. Assert the resolved PHP binary exists and executes. +# +# Intended as the manual gate for PR 1 of the local+CI unification rollout +# and a fixture that PR 4's CI rework will reuse. +# +# Requires (hard): +# - oras (https://oras.land) — copies the manifest into an OCI layout. +# - jq — reads bundles.lock. +# - go — builds phpup. +# Requires (soft): +# - GHCR read access. Set GITHUB_TOKEN or GHCR_TOKEN if the image is +# private or you are rate-limited by anonymous pulls. Otherwise +# anonymous access to public buildrush images is sufficient. +# +# Platform: the published php-core bundles contain linux binaries, so the +# `php --version` smoke step only succeeds on a linux host. The script +# fails fast on non-linux with a clear message — wrap it in a linux +# container or run on CI. +# +# Usage: +# ./test/registry/e2e_layout.sh +# +# Environment overrides: +# WORK= — reuse a scratch dir instead of a fresh mktemp -d +# (the dir is still wiped by the EXIT trap). +# PHP_VERSION=8.4 — PHP major.minor to exercise (must have an entry in +# cmd/phpup/bundles.lock). +# KEEP_WORK=1 — skip cleanup so you can inspect the layout on +# failure. + +set -euo pipefail + +PHP_VERSION="${PHP_VERSION:-8.4}" + +require() { + local cmd="$1" + if ! command -v "$cmd" >/dev/null 2>&1; then + echo "FAIL: missing required command: $cmd" >&2 + echo " install it and re-run. See the header of this script for details." >&2 + exit 1 + fi +} + +require oras +require jq +require go + +# Resolve the phpup/bundles.lock arch key from the host. +HOST_UNAME="$(uname -m)" +case "$HOST_UNAME" in + x86_64|amd64) LOCK_ARCH="x86_64"; RUNNER_ARCH="X64" ;; + aarch64|arm64) LOCK_ARCH="aarch64"; RUNNER_ARCH="ARM64" ;; + *) echo "FAIL: unsupported host arch: $HOST_UNAME" >&2; exit 1 ;; +esac + +# The published core bundle is linux-only. On darwin we can seed the +# layout fine, but the final `php --version` step would try to exec a +# linux ELF and fail. Bail early with a clear message. +HOST_OS="$(uname -s)" +if [[ "$HOST_OS" != "Linux" ]]; then + echo "FAIL: this harness exercises linux php bundles and must run on a" >&2 + echo " linux host. Detected: $HOST_OS. Wrap in a linux container or" >&2 + echo " run on CI. (Docker-based packaging is Task 3 of PR 3.)" >&2 + exit 1 +fi + +REPO_ROOT="$(git -C "$(dirname "$0")" rev-parse --show-toplevel)" +cd "$REPO_ROOT" + +WORK="${WORK:-$(mktemp -d)}" +LAYOUT="$WORK/oci-layout" +BIN="$WORK/phpup" +INSTALL_DIR="$WORK/buildrush" + +cleanup() { + if [[ "${KEEP_WORK:-0}" == "1" ]]; then + echo "KEEP_WORK=1: leaving $WORK in place for inspection" >&2 + return + fi + rm -rf "$WORK" +} +trap cleanup EXIT + +echo "==> Work dir: $WORK" + +echo "==> Regenerate embedded lockfile and build phpup" +make cmd/phpup/bundles.lock >/dev/null +go build -o "$BIN" ./cmd/phpup + +echo "==> Resolve php-core digest for php:$PHP_VERSION:linux:$LOCK_ARCH:nts" +PHP_KEY="php:$PHP_VERSION:linux:$LOCK_ARCH:nts" +PHP_CORE_DIGEST="$(jq -r --arg k "$PHP_KEY" '.bundles[$k].digest // ""' cmd/phpup/bundles.lock)" +if [[ -z "$PHP_CORE_DIGEST" || "$PHP_CORE_DIGEST" == "null" ]]; then + echo "FAIL: could not resolve $PHP_KEY digest from cmd/phpup/bundles.lock" >&2 + exit 1 +fi +echo " digest: $PHP_CORE_DIGEST" + +echo "==> Seed local OCI layout from GHCR (one-time network access)" +mkdir -p "$LAYOUT" + +# If a token is available, log oras in against ghcr.io so private/rate- +# limited pulls work. Anonymous pulls succeed for public images, so this +# step is best-effort. +ORAS_TOKEN="${GITHUB_TOKEN:-${GHCR_TOKEN:-}}" +if [[ -n "$ORAS_TOKEN" ]]; then + echo " authenticating to ghcr.io (token provided)" + echo "$ORAS_TOKEN" | oras login ghcr.io --username "${GHCR_USERNAME:-oauth2}" --password-stdin >/dev/null +fi + +# `oras cp` with `--to-oci-layout` pulls the manifest (by digest) into an +# OCI layout directory. The `:seed` tag is a hint that lands in the +# layout's index.json — the layout backend in internal/registry matches +# refs by digest, so the tag value doesn't have to match anything phpup +# expects. +oras cp --to-oci-layout \ + "ghcr.io/buildrush/php-core@$PHP_CORE_DIGEST" \ + "$LAYOUT:seed" + +# Sanity-check the layout structure before handing off to phpup. +if [[ ! -s "$LAYOUT/index.json" ]]; then + echo "FAIL: oras cp did not produce an index.json under $LAYOUT" >&2 + exit 1 +fi + +echo "==> Run phpup install against the local layout (offline after seed)" +export PHPUP_REGISTRY="oci-layout:$LAYOUT" +export BUILDRUSH_DIR="$INSTALL_DIR" +export INPUT_VERBOSE="true" +export RUNNER_OS="Linux" +export RUNNER_ARCH="$RUNNER_ARCH" + +# Redirect env export to a file under $WORK so the script doesn't pollute +# the caller's shell (phpup appends PATH/PHPRC lines to $GITHUB_ENV when +# set, and falls back to a stdout echo otherwise; either is fine here). +export GITHUB_ENV="$WORK/github_env" +export GITHUB_PATH="$WORK/github_path" +export GITHUB_OUTPUT="$WORK/github_output" +: >"$GITHUB_ENV" +: >"$GITHUB_PATH" +: >"$GITHUB_OUTPUT" + +# INPUT_PHP-VERSION uses a literal dash (GitHub Actions convention). +# Plain `export` can't declare names with dashes, so hand it to phpup via +# env's KEY=VALUE form. +env "INPUT_PHP-VERSION=$PHP_VERSION" "$BIN" + +echo "==> Assert the resolved php binary exists and runs" +# See cmd/phpup/main.go:detectLayout — BinDir = /core/usr/local/bin. +PHP_BIN="$INSTALL_DIR/core/usr/local/bin/php" +if [[ ! -x "$PHP_BIN" ]]; then + echo "FAIL: expected PHP binary missing at $PHP_BIN" >&2 + echo "---- $INSTALL_DIR layout ----" >&2 + find "$INSTALL_DIR" -maxdepth 5 >&2 || true + exit 1 +fi +"$PHP_BIN" --version + +echo "==> PASS"