diff --git a/internal/build/push.go b/internal/build/push.go index 8637388..e8ccd44 100644 --- a/internal/build/push.go +++ b/internal/build/push.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "strings" "github.com/buildrush/setup-php/internal/registry" ) @@ -25,25 +26,19 @@ func PushMain(args []string) error { return PushAll(context.Background(), *from, *to) } -// PushAll promotes every bundle in source to dest. Source MUST be an -// oci-layout (remote->remote copy isn't supported in PR 4; use oras for -// that case) — a remote source is rejected up front with an error -// mentioning the supported URI form. +// PushAll promotes every keyed bundle in source to dest. Source MUST be +// an oci-layout populated by `phpup build`, where each manifest carries +// the `io.buildrush.bundle.key` annotation — that's what lets us recover +// the canonical publication tag (e.g. "6.3.0-8.5-nts-linux-x86_64" for +// `ext:redis:6.3.0:8.5:linux:x86_64:nts`) on the destination. // -// The walk reads the source layout's index, and for each manifest -// re-pushes the layer bytes + meta to the destination via Store.Push. -// Tag derivation is currently lossy: the source manifest's original -// publication tag isn't recoverable from Fetch, so deriveTagForPromotion -// falls back to a digest-prefix tag like "sha256-abc123…". PR 6 will -// carry the original tag via a manifest annotation. +// Manifests without a bundle-key annotation are skipped silently by +// ListKeyed; the `phpup build` path always sets it. Forward push paths +// only see build-produced layouts, so this exposure is bounded. // -// The bundle-name annotation is preserved (via Annotations.BundleName) -// so the destination still resolves through LookupBySpec / Has for -// downstream consumers. The source-side spec-hash is not propagated — -// Fetch doesn't surface manifest annotations, so the promoted manifest -// ends up annotated only by what ref.Name implies. When/if LookupBySpec -// on a promoted remote becomes required, the Store interface will need -// an annotations-accessor. +// The destination manifest is annotated with BundleName/BundleKey/SpecHash +// so consumers can resolve via canonical tag (lockfile-update) AND via +// LookupBySpec (build-cache probe). func PushAll(ctx context.Context, sourceURI, destURI string) error { src, err := registry.Open(sourceURI) if err != nil { @@ -53,75 +48,77 @@ func PushAll(ctx context.Context, sourceURI, destURI string) error { if err != nil { return fmt.Errorf("phpup push: open dest %s: %w", destURI, err) } - // Source MUST expose an index walk. Only the layout backend does so - // natively in PR 4; a remote source would need a custom API to - // enumerate tagged manifests (deferred to PR 6 consolidation). + // Source MUST expose ListKeyed so we can recover the canonical tag + // from each manifest's bundle-key annotation. Only the layout backend + // implements it natively; a remote source would need a custom API to + // enumerate annotated manifests. lister, ok := src.(interface { - List(ctx context.Context) ([]registry.Ref, error) + ListKeyed(ctx context.Context) ([]registry.KeyedRef, error) }) if !ok { - return fmt.Errorf("phpup push: source %q doesn't support index walk (use oci-layout:)", sourceURI) + return fmt.Errorf("phpup push: source %q doesn't support ListKeyed (use oci-layout:)", sourceURI) } - refs, err := lister.List(ctx) + refs, err := lister.ListKeyed(ctx) if err != nil { return fmt.Errorf("phpup push: list source: %w", err) } fmt.Printf("phpup push: promoting %d manifests from %s to %s\n", len(refs), sourceURI, destURI) - for _, ref := range refs { - if err := promoteOne(ctx, src, dst, ref); err != nil { - return fmt.Errorf("phpup push: promote %s: %w", ref, err) + for _, kref := range refs { + if err := promoteOne(ctx, src, dst, kref); err != nil { + return fmt.Errorf("phpup push: promote %s: %w", kref.Key, err) } } return nil } -// promoteOne fetches a single source manifest's bundle + meta and -// pushes the payload to the destination under a derived tag. Close -// failures on the source reader are swallowed — the push has already -// succeeded at that point and the caller's next action is typically to -// print a success line and exit. -func promoteOne(ctx context.Context, src, dst registry.Store, ref registry.Ref) error { - rc, meta, err := src.Fetch(ctx, ref) +// promoteOne fetches a single source manifest's bundle + meta and pushes +// the payload to the destination under the canonical tag derived from +// the bundle key. The destination manifest carries the propagated +// BundleKey + SpecHash annotations so post-push lockfile-update and +// LookupBySpec both resolve correctly. +func promoteOne(ctx context.Context, src, dst registry.Store, kref registry.KeyedRef) error { + imageName, tag, err := canonicalRefFromBundleKey(kref.Key) + if err != nil { + return err + } + rc, meta, err := src.Fetch(ctx, registry.Ref{Name: kref.Name, Digest: kref.Digest}) if err != nil { return fmt.Errorf("fetch: %w", err) } defer func() { _ = rc.Close() }() - tag := deriveTagForPromotion(ref) - if tag == "" { - return fmt.Errorf("couldn't derive tag for %s (ref has no digest)", ref) - } - if ref.Name == "" { - return fmt.Errorf("couldn't derive name for %s (ref has no bundle-name annotation)", ref) + dstRef := registry.Ref{Name: imageName, Tag: tag} + ann := registry.Annotations{ + BundleName: imageName, + BundleKey: kref.Key, + SpecHash: kref.SpecHash, } - dstRef := registry.Ref{Name: ref.Name, Tag: tag} - ann := registry.Annotations{BundleName: ref.Name} return dst.Push(ctx, dstRef, rc, meta, ann) } -// deriveTagForPromotion produces a tag string for a manifest based on -// its digest. The canonical publication tag shape -// (e.g. "---" for php-core) isn't recoverable -// from the Fetch return — go-containerregistry's layout reader doesn't -// expose per-manifest tags on the index, and the digest alone doesn't -// encode them. For PR 4 we fall back to "sha256-" so the -// destination registry has SOMETHING to address, at the cost of losing -// human-readable tag semantics. PR 6's CLI consolidation will thread -// the original tag via the source manifest's annotation so promotions -// stay addressable under their published names. -func deriveTagForPromotion(ref registry.Ref) string { - if ref.Digest == "" { - return "" - } - // "sha256:" prefix is 7 characters; the hex payload that follows is - // 64 characters for sha256. Take 12 hex chars after the prefix — - // enough to avoid collisions in any realistic layout, short enough - // to stay within the OCI tag length limit (128 chars). - d := ref.Digest - if len(d) > 19 { - return "sha256-" + d[7:19] +// canonicalRefFromBundleKey parses a bundle key produced by +// internal/lockfile.{PHPBundleKey,ExtBundleKey} and returns the +// (image-name, tag) pair that mirrors what `phpup lockfile-update` +// queries against ghcr.io. The two paths must agree for `phpup push` → +// `lockfile-update` to round-trip. +// +// php:::: +// → ("php-core", "---") +// ext:::::: +// → ("php-ext-", "----") +// +// Tool keys aren't supported yet — `phpup build` doesn't produce them +// and `lockfile-update` doesn't query them. Returns an explicit error +// rather than guessing a layout. +func canonicalRefFromBundleKey(key string) (imageName, tag string, err error) { + parts := strings.Split(key, ":") + switch { + case len(parts) == 5 && parts[0] == "php": + ver, osName, arch, ts := parts[1], parts[2], parts[3], parts[4] + return "php-core", fmt.Sprintf("%s-%s-%s-%s", ver, osName, arch, ts), nil + case len(parts) == 7 && parts[0] == "ext": + name, ver, phpver, osName, arch, ts := parts[1], parts[2], parts[3], parts[4], parts[5], parts[6] + return "php-ext-" + name, fmt.Sprintf("%s-%s-%s-%s-%s", ver, phpver, ts, osName, arch), nil + default: + return "", "", fmt.Errorf("canonicalRefFromBundleKey: unrecognized bundle key shape %q", key) } - // Pathological fallback — ref.Digest looks like "sha256:" - // which shouldn't happen in practice but we keep the behavior - // defined rather than panicking. - return "sha256-" + d[7:] } diff --git a/internal/build/push_test.go b/internal/build/push_test.go index 33a1136..1359512 100644 --- a/internal/build/push_test.go +++ b/internal/build/push_test.go @@ -32,10 +32,11 @@ func startPushTestRegistry(t *testing.T) string { return u.Host } -// seedLayoutForPush writes a bundle into a fresh oci-layout at -// /layout, then returns the URI. Used to populate a source layout -// for PushAll round-trip tests. -func seedLayoutForPush(t *testing.T, dir, bundleName string, body []byte, meta *registry.Meta) string { +// seedKeyedLayoutForPush writes a bundle into a fresh oci-layout at +// /layout with full annotations (name + key + spec_hash) so the +// PushAll path can recover the canonical tag from the key. Mirrors what +// `phpup build php|ext` writes for produced bundles. +func seedKeyedLayoutForPush(t *testing.T, dir, bundleName, bundleKey, specHash string, body []byte, meta *registry.Meta) string { t.Helper() layoutDir := filepath.Join(dir, "layout") s, err := registry.Open("oci-layout:" + layoutDir) @@ -45,31 +46,38 @@ func seedLayoutForPush(t *testing.T, dir, bundleName string, body []byte, meta * if err := s.Push(context.Background(), registry.Ref{Name: bundleName}, bytes.NewReader(body), meta, - registry.Annotations{BundleName: bundleName}); err != nil { + registry.Annotations{BundleName: bundleName, BundleKey: bundleKey, SpecHash: specHash}); err != nil { t.Fatalf("seed layout: %v", err) } return "oci-layout:" + layoutDir } // TestPushAll_RoundTrip_LayoutToInProcessRegistry is the happy-path -// end-to-end: seed an oci-layout with two manifests (php-core + one -// ext), run PushAll against an in-process registry as destination, and -// verify both manifests are fetchable on the destination under the -// digest-derived tag. +// end-to-end: seed an oci-layout with two annotated manifests (php-core + +// one ext), run PushAll against an in-process registry as destination, +// and verify both manifests are fetchable on the destination under the +// canonical publication tag derived from the bundle key. Also verifies +// that BundleKey and SpecHash annotations are propagated. func TestPushAll_RoundTrip_LayoutToInProcessRegistry(t *testing.T) { ctx := context.Background() tmp := t.TempDir() phpCoreBytes := []byte("fake php-core bundle") phpExtBytes := []byte("fake php-ext-redis bundle") - // Seed source layout with two bundles. - layoutURI := seedLayoutForPush(t, tmp, "php-core", phpCoreBytes, ®istry.Meta{SchemaVersion: 3, Kind: "php-core"}) - // Second push lands in the same layout path. + const phpKey = "php:8.5:linux:x86_64:nts" + const phpHash = "sha256:phpspecdigesthex0000000000000000000000000000000000000000000000" + const extKey = "ext:redis:6.3.0:8.5:linux:x86_64:nts" + const extHash = "sha256:extspecdigesthex0000000000000000000000000000000000000000000000" + + // Seed source layout with two keyed bundles. + layoutURI := seedKeyedLayoutForPush(t, tmp, "php-core", phpKey, phpHash, phpCoreBytes, + ®istry.Meta{SchemaVersion: 3, Kind: "php-core"}) layoutDir := filepath.Join(tmp, "layout") s, _ := registry.Open("oci-layout:" + layoutDir) if err := s.Push(ctx, registry.Ref{Name: "php-ext-redis"}, - bytes.NewReader(phpExtBytes), ®istry.Meta{SchemaVersion: 3, Kind: "php-ext"}, - registry.Annotations{BundleName: "php-ext-redis"}); err != nil { + bytes.NewReader(phpExtBytes), + ®istry.Meta{SchemaVersion: 3, Kind: "php-ext"}, + registry.Annotations{BundleName: "php-ext-redis", BundleKey: extKey, SpecHash: extHash}); err != nil { t.Fatalf("push ext: %v", err) } @@ -80,63 +88,63 @@ func TestPushAll_RoundTrip_LayoutToInProcessRegistry(t *testing.T) { t.Fatalf("PushAll: %v", err) } - // Re-open source to enumerate digest-derived tags the push would - // have used, so we can fetch back from the destination under the - // same tag. - lister := s.(interface { - List(ctx context.Context) ([]registry.Ref, error) - }) - refs, err := lister.List(ctx) - if err != nil { - t.Fatalf("source List: %v", err) - } - if len(refs) != 2 { - t.Fatalf("source layout has %d manifests, want 2", len(refs)) - } - - // For each seeded bundle, verify the destination has a manifest at - // the derived tag with the same layer 0 bytes. - byName := map[string][]byte{ - "php-core": phpCoreBytes, - "php-ext-redis": phpExtBytes, + // Each seeded bundle should land at the canonical tag derived from + // its bundle key — the same tag shape `lockfile-update` queries. + cases := []struct { + key string + wantName string + wantTag string + wantBody []byte + }{ + {phpKey, "php-core", "8.5-linux-x86_64-nts", phpCoreBytes}, + {extKey, "php-ext-redis", "6.3.0-8.5-nts-linux-x86_64", phpExtBytes}, } - for _, r := range refs { - tag := deriveTagForPromotion(r) - if tag == "" { - t.Fatalf("empty derived tag for %s", r) - } - want, ok := byName[r.Name] - if !ok { - t.Fatalf("unexpected source ref %s", r) - } - dest, err := name.ParseReference(destURI + "/" + r.Name + ":" + tag) - if err != nil { - t.Fatalf("parse dest ref: %v", err) - } - desc, err := remote.Get(dest) - if err != nil { - t.Fatalf("remote.Get %s: %v", dest, err) - } - img, err := desc.Image() - if err != nil { - t.Fatalf("image: %v", err) - } - layers, err := img.Layers() - if err != nil { - t.Fatalf("layers: %v", err) - } - if len(layers) < 1 { - t.Fatalf("manifest has %d layers, want >=1", len(layers)) - } - rc, err := layers[0].Compressed() - if err != nil { - t.Fatalf("layer 0: %v", err) - } - got, _ := io.ReadAll(rc) - _ = rc.Close() - if !bytes.Equal(got, want) { - t.Errorf("bundle %s: got %q, want %q", r.Name, got, want) - } + for _, tc := range cases { + t.Run(tc.wantName, func(t *testing.T) { + dest, err := name.ParseReference(destURI + "/" + tc.wantName + ":" + tc.wantTag) + if err != nil { + t.Fatalf("parse dest ref: %v", err) + } + desc, err := remote.Get(dest) + if err != nil { + t.Fatalf("remote.Get %s: %v (canonical tag missing — phpup push regression)", dest, err) + } + img, err := desc.Image() + if err != nil { + t.Fatalf("image: %v", err) + } + // Verify layer 0 bytes match the seeded body. + layers, err := img.Layers() + if err != nil { + t.Fatalf("layers: %v", err) + } + if len(layers) < 1 { + t.Fatalf("manifest has %d layers, want >=1", len(layers)) + } + rc, err := layers[0].Compressed() + if err != nil { + t.Fatalf("layer 0: %v", err) + } + got, _ := io.ReadAll(rc) + _ = rc.Close() + if !bytes.Equal(got, tc.wantBody) { + t.Errorf("bundle %s: got %q, want %q", tc.wantName, got, tc.wantBody) + } + // Verify destination annotations propagated. + mf, err := img.Manifest() + if err != nil { + t.Fatalf("manifest: %v", err) + } + if got := mf.Annotations["io.buildrush.bundle.key"]; got != tc.key { + t.Errorf("dest bundle.key = %q, want %q", got, tc.key) + } + if got := mf.Annotations["io.buildrush.bundle.name"]; got != tc.wantName { + t.Errorf("dest bundle.name = %q, want %q", got, tc.wantName) + } + if got := mf.Annotations["io.buildrush.bundle.spec-hash"]; got == "" { + t.Errorf("dest bundle.spec-hash missing (want propagated from source)") + } + }) } } @@ -152,8 +160,8 @@ func TestPushAll_RemoteSourceRejected(t *testing.T) { if err == nil { t.Fatal("PushAll with remote source: want error, got nil") } - if !strings.Contains(err.Error(), "doesn't support index walk") { - t.Errorf("err = %v, want containing \"doesn't support index walk\"", err) + if !strings.Contains(err.Error(), "doesn't support ListKeyed") { + t.Errorf("err = %v, want containing \"doesn't support ListKeyed\"", err) } } @@ -179,14 +187,11 @@ func TestPushMain_MissingFlags_Errors(t *testing.T) { } // TestPushMain_EmptyLayoutSucceeds: a source layout that exists but -// holds zero manifests is a legal no-op (symmetry with -// List's ErrNotExist handling). Covers the case where PushAll is -// invoked against a fresh build directory before any bundles have been -// pushed. Must print 0-manifests banner and return nil. +// holds zero manifests is a legal no-op (symmetry with ListKeyed's +// ErrNotExist handling). Covers the case where PushAll is invoked +// against a fresh build directory before any bundles have been pushed. +// Must print 0-manifests banner and return nil. func TestPushMain_EmptyLayoutSucceeds(t *testing.T) { - // An empty directory makes layout.FromPath fail with ErrNotExist on - // the first walk — List returns (nil, nil) for that case, so - // PushAll should successfully iterate over zero refs. host := startPushTestRegistry(t) tmp := t.TempDir() err := PushMain([]string{ @@ -198,36 +203,81 @@ func TestPushMain_EmptyLayoutSucceeds(t *testing.T) { } } -// TestDeriveTagForPromotion_DigestShortPrefix pins the fallback tag -// format PushAll emits when the source manifest has no surface-level -// tag info. Shape: "sha256-" — matches the comment on -// deriveTagForPromotion. If a future PR threads the original tag via a -// manifest annotation, this test will start failing and should be -// updated to the new contract. -func TestDeriveTagForPromotion_DigestShortPrefix(t *testing.T) { +// TestCanonicalRefFromBundleKey covers every key shape the helper accepts +// plus the rejection branch. The expected tag formats here are the same +// strings `lockfile-update` queries against ghcr.io — keep this test in +// lockstep with `internal/lockfileupdate.update.go`'s tag construction. +func TestCanonicalRefFromBundleKey(t *testing.T) { cases := []struct { - name string - digest string - want string + name string + key string + wantName string + wantTag string + wantErrSub string }{ - {"standard sha256", "sha256:abcdef012345678901234567890abcdefabcdefabcdefabcdefabcdefabcdef", "sha256-abcdef012345"}, - {"short digest (pathological)", "sha256:abc", "sha256-abc"}, + { + name: "php core 8.5 linux x86_64 nts", + key: "php:8.5:linux:x86_64:nts", + wantName: "php-core", + wantTag: "8.5-linux-x86_64-nts", + }, + { + name: "php core 8.4 linux aarch64 nts", + key: "php:8.4:linux:aarch64:nts", + wantName: "php-core", + wantTag: "8.4-linux-aarch64-nts", + }, + { + name: "ext redis 6.3.0", + key: "ext:redis:6.3.0:8.5:linux:x86_64:nts", + wantName: "php-ext-redis", + wantTag: "6.3.0-8.5-nts-linux-x86_64", + }, + { + name: "ext xdebug 3.5.1", + key: "ext:xdebug:3.5.1:8.4:linux:aarch64:nts", + wantName: "php-ext-xdebug", + wantTag: "3.5.1-8.4-nts-linux-aarch64", + }, + { + name: "tool key not supported", + key: "tool:composer:2.7.0:any:any", + wantErrSub: "unrecognized bundle key shape", + }, + { + name: "wrong prefix", + key: "container:redis:7.0", + wantErrSub: "unrecognized bundle key shape", + }, + { + name: "ext with too few parts", + key: "ext:redis:6.3.0:8.5:linux", + wantErrSub: "unrecognized bundle key shape", + }, + { + name: "empty", + key: "", + wantErrSub: "unrecognized bundle key shape", + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - got := deriveTagForPromotion(registry.Ref{Digest: tc.digest}) - if got != tc.want { - t.Errorf("deriveTagForPromotion(%q) = %q, want %q", tc.digest, got, tc.want) + gotName, gotTag, err := canonicalRefFromBundleKey(tc.key) + if tc.wantErrSub != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErrSub) { + t.Errorf("err = %v, want containing %q", err, tc.wantErrSub) + } + return + } + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + if gotName != tc.wantName { + t.Errorf("name = %q, want %q", gotName, tc.wantName) + } + if gotTag != tc.wantTag { + t.Errorf("tag = %q, want %q", gotTag, tc.wantTag) } }) } } - -// TestDeriveTagForPromotion_EmptyDigest ensures the empty-digest input -// produces an empty string (caller treats that as "can't derive a tag" -// and errors at the promoteOne boundary). -func TestDeriveTagForPromotion_EmptyDigest(t *testing.T) { - if got := deriveTagForPromotion(registry.Ref{}); got != "" { - t.Errorf("deriveTagForPromotion(empty) = %q, want empty", got) - } -} diff --git a/internal/registry/annotations.go b/internal/registry/annotations.go index f5faff8..ac3b493 100644 --- a/internal/registry/annotations.go +++ b/internal/registry/annotations.go @@ -22,7 +22,7 @@ type Annotations struct { SpecHash string // BundleKey is the canonical lockfile key, e.g. - // "php:8.4:linux:x86_64:nts" or "ext:redis:6.2.0:8.4-nts:linux:x86_64". + // "php:8.4:linux:x86_64:nts" or "ext:redis:6.2.0:8.4:linux:x86_64:nts". // Set by phpup build php|ext; read by phpup test's lockfile-override // synthesis so test containers can resolve bundles against a local // layout whose digests differ from the embedded bundles.lock.