## Summary Follow-up to https://gitlab.com/gitlab-org/container-registry/-/merge_requests/2927 (fixes https://gitlab.com/gitlab-org/container-registry/-/issues/2375). !2927 makes the registry accept a manifest whose `subject` is not present, as the OCI distribution spec requires (https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests). It does so behind `REGISTRY_FF_TOLERATE_MISSING_SUBJECT` by storing such a manifest with a **null `subject_id`**. That unblocks pushes but leaves the subject relationship unrecorded until (if ever) it is repaired. This issue tracks making that relationship correct. ## Background: what `subject_id` is for `manifests.subject_id` is a nullable self-referential FK set on the **referrer** manifest (e.g. an SBOM attestation or signature), pointing at its subject manifest. It has two consumers: 1. **Referrer discovery** -- `appendTagsDetailReferrers` (`registry/datastore/repository.go:982`+), used by the GitLab tag-detail API with `referrers=true`. It lists referrers by joining `m.subject_id = <subject>.id`. (There is no OCI `/v2/<name>/referrers/<digest>` endpoint today; referrers are surfaced only here.) 2. **Online GC liveness** -- `IsDangling` (`registry/datastore/gcmanifesttask.go:365`-`394`). A manifest is considered still-referenced (not deletable) if it is tagged, is a manifest-list child, **or some manifest's `subject_id` points at it**. So `subject_id` is what protects a subject from GC while a referrer exists. Both break when `subject_id` is null: a referrer pushed before its subject is invisible to discovery, and it does not protect its subject from GC. ## Problem left by !2927 When a referrer is pushed before its subject, `subject_id` is null and is **never revisited** -- even after the subject is later pushed. Concretely, for `buildx --attest type=sbom --push` the SBOM attestation and the image manifest are pushed concurrently; if the attestation lands first, its `subject_id` stays null. Effects until repaired: - The SBOM/signature does not appear in the referrers listing for the image. - GC does not treat the image as protected by that referrer (in practice the image is usually also tagged or an index child, so it survives; but an untagged subject with only not-yet-linked referrers could be GC'd). ## Diagram The whole thing turns on one nullable column, `manifests.subject_id`. **1. The relationship, normal vs degenerate** ``` NORMAL (subject exists when the referrer is pushed) attestation (referrer) image manifest (subject) +-----------------------+ +-----------------------+ | digest f8ca... | | digest 3445... | | payload.subject 3445...| | id 42 | | subject_id 42 --------+--------> | (exists) | +-----------------------+ resolved +-----------------------+ subject_id is SET -> the DB knows f8ca refers to 3445. DEGENERATE (subject absent when the referrer is pushed; what !2927 allows) attestation (referrer) image manifest (subject) +-----------------------+ +-----------------------+ | digest f8ca... | | 3445... not in DB | | payload.subject 3445...| | (yet, or forever) | | subject_id NULL x----+--------> +-----------------------+ +-----------------------+ broken payload still says "subject = 3445", but the COLUMN both features read (subject_id) is NULL. The link is not recorded. ``` **2. Why it happens: the push race (buildkit `--attest`)** ``` The index has two children, pushed concurrently: PUT /manifests/3445 (image manifest = the subject) PUT /manifests/f8ca (attestation, payload.subject = 3445) order A: image commits first 3445 stored then f8ca validated -> subject 3445 found -> subject_id = 42 => 201, healthy order B: attestation validated first f8ca validated -> subject 3445 NOT present yet before !2927: 400 MANIFEST_BLOB_UNKNOWN, push aborts <- original bug with !2927 on: 201, stored with subject_id = NULL <- degenerate row ``` **3. What breaks while `subject_id` is NULL (the two consumers)** ``` referrers listing <--- SELECT ... JOIN ... m.subject_id = <subject>.id (GitLab tag API) -> NULL never matches -> SBOM not listed online GC IsDangling <-- a subject is "still referenced" if some row has subject_id = <subject>.id -> NULL does not protect 3445 from GC -> ON DELETE CASCADE link also absent ``` The permanent degenerate state = a referrer whose subject never arrives, so `subject_id` stays NULL forever. That row must still behave like a normal manifest (GET/tag/delete/GC), and GC must collect it if untagged and unreferenced. It is not corrupt (NULL is a valid, FK-free value); it is just unlinked. **4. How this issue repairs it** ``` !2927 (now): store referrer, subject_id = NULL (never repaired) this issue: add subject_digest, ALWAYS set from payload.subject +-----------------------+ | subject_id NULL | | subject_digest 3445... | recorded even though 3445 absent +-----------------------+ when 3445 is finally pushed: UPDATE manifests SET subject_id = <3445.id> WHERE subject_digest = 3445... AND subject_id IS NULL -> link repaired -> referrers listing + GC protection correct again ``` `subject_id` is the resolved pointer both features rely on; `subject_digest` is the durable "declared intent" that lets us resolve it later, regardless of push order. ## If this is never implemented (deferral trade-off) Leaving `subject_id` NULL is not a leak and does not corrupt anything. Specifically: - **The orphan referrer is still garbage-collected normally.** Every manifest gets a review task at push time via the `gc_track_manifest_uploads` trigger (`AFTER INSERT ON manifests`, event `manifest_upload`, `review_after = now + ~1 day`). `IsDangling` (`registry/datastore/gcmanifesttask.go:365`-`394`) does not consider the manifest's *own* `subject_id`; it deletes the manifest unless it is tagged, a manifest-list child, or itself the subject of another manifest. So an untagged, otherwise-unreferenced referrer whose subject never arrives is collected ~1 day after upload, and its config/layer blobs follow via the delete triggers. Nothing accumulates. - **No dangling foreign keys.** `subject_id` is nullable and FK-free when NULL, so `fk_manifests_subject_id_manifests` (`ON DELETE CASCADE`) is simply not engaged. What you permanently lose by not doing this work: 1. **Referrer discovery is incomplete.** A referrer pushed before its subject never appears in the `referrers=true` listing (the `subject_id` join never matches). 2. **GC can over-delete a subject.** Because the NULL-`subject_id` referrer does not protect its subject, `IsDangling` may collect a subject that still has referrers, if that subject is otherwise unreferenced (untagged, not an index child). This breaks the "keep a subject alive while referrers exist" guarantee and can leave a referrer pointing at a now-absent subject. In the buildkit `--attest` flow this is masked because the subject image manifest is also the index child and tag target, so it stays protected; standalone referrer pushes (e.g. cosign-style, by digest) are the exposed case. Both effects are dormant while `REGISTRY_FF_TOLERATE_MISSING_SUBJECT` is off. Enabling the flag without this work accepts (1) and (2) as known gaps. ## Proposed work 1. **Add `manifests.subject_digest bytea` (nullable).** Populate it from the payload's `subject.digest` on **every** push that has a subject, regardless of whether the subject exists. This records the declared relationship independent of resolution. - Migration must account for `manifests` being partitioned; add an index supporting lookups by `(top_level_namespace_id, repository_id, subject_digest)`. - Plumb through `models.Manifest`, `registry/datastore/manifest.go` (Create + row scans), and the write paths (`dbPutManifestV2`, and the buildkit-index path if relevant). 2. **Backfill `subject_id` when a subject arrives.** When a manifest `M` is created, in the same transaction, run: `UPDATE manifests SET subject_id = M.id WHERE top_level_namespace_id = ... AND repository_id = ... AND subject_digest = M.digest AND subject_id IS NULL`. This links previously-orphaned referrers. - Coordinate with online GC the way manifest-list creation already does (`FindAndLockNBefore`), since setting `subject_id` changes `IsDangling` for `M`. Avoid a race where `M` is GC-reviewed as dangling between its creation and the link. 3. **Keep the read paths unchanged.** With backfill keeping `subject_id` canonical, both consumers above work as-is. (Alternative considered: make referrer discovery and `IsDangling` resolve via `subject_digest` directly and drop `subject_id`; rejected as a larger blast radius across GC and the referrers query.) 4. **Feature-flag lifecycle.** Accepting an absent subject is a spec MUST, so plan to flip `REGISTRY_FF_TOLERATE_MISSING_SUBJECT` to default-on once 1-3 land, then remove the flag. 5. **Filesystem manifest handler.** !2927 left the legacy non-DB path (`registry/storage/ocimanifesthandler.go`) strict (`tolerateMissingSubject=false`). Decide whether to make it spec-compliant or document it as legacy. 6. **Orphan handling (document, maybe out of scope).** A referrer whose subject is never pushed keeps `subject_id` null indefinitely. Decide whether GC should reap such orphans. ## Testing - Referrers listing includes a referrer pushed **before** its subject, once the subject is pushed (end-to-end). - Backfill sets `subject_id` for all matching orphans on subject creation. - GC does not delete a subject that has a backfilled referrer; and does not mis-delete during the pre-backfill window. - Extend the existing coverage: `TestVerifyManifest_OCI_MissingSubject` (validator) and `TestGitlabAPI_ManifestPut_MissingSubjectTolerated` (handler) from !2927. ## Observability The warning logged in !2927 when storing a manifest with an absent subject (`registry/handlers/manifests.go`) can be used to measure how often this happens and to detect referrers left unlinked (i.e. subjects that never arrived). ## Update: implemented as two triggers (both push orderings) The backfill in "Proposed work" #2 covered only one direction (repair when the *subject* arrives after the referrer). Review of the migrations (!2930) surfaced a second, deterministic case that a single `AFTER INSERT` trigger misses, so the implementation now uses **two** triggers. MRs: !2930 (triggers + partial index), !2931 (write path recording `subject_digest` + datastore plumbing + tests). These were split out of the original combined !2929, stacked on the `subject_digest` column (!2928, merged). 1. **`AFTER INSERT` `backfill_manifest_subject_id`** (referrer pushed before its subject). When a manifest is created, it sets `subject_id = NEW.id` for referrers in the same repository whose `subject_digest = NEW.digest` and `subject_id IS NULL`. This is the repair originally proposed. 2. **`BEFORE INSERT` `resolve_manifest_subject_id`** (subject pushed before the referrer). The `AFTER INSERT` trigger alone leaves a permanent gap here, and it is deterministic, not just a rare race. The push path is autocommit, so this ordering fails every time: the referrer handler looks up the subject (not found), the subject is then inserted (its `AFTER INSERT` fires but the referrer row does not exist yet, 0 rows updated), then the referrer is inserted with `subject_id` NULL. Re-pushing the subject does not repair it either, because that hits `ON CONFLICT DO NOTHING` and never fires `AFTER INSERT`. The `BEFORE INSERT` trigger closes this by resolving `NEW.subject_id` from `NEW.subject_digest` inside the insert statement, so it sees the committed subject even when the handler's earlier lookup missed it. ### GC coordination Both triggers run inside the insert, so linking is atomic with manifest creation. There is no window where a just-created subject looks dangling before its referrers are linked, which removes the need for the `FindAndLockNBefore` coordination floated in "Proposed work" #2. ### Residual window (gated behind the flag) One case remains uncovered: a truly simultaneous push where both transactions are still uncommitted when each runs its trigger, so neither sees the other's row. This needs read-time resolution (resolve via `subject_digest` at query time) or a reconciliation pass. It is acceptable while `REGISTRY_FF_TOLERATE_MISSING_SUBJECT` stays off and should be settled before the flag is enabled. ### Tests added (!2931) - `TestManifestStore_SubjectIDBackfilledOnSubjectCreate`: referrer before subject, linked by `AFTER INSERT`; plus GC assertions (subject protected, orphan referrer collectable). - `TestManifestStore_SubjectIDResolvedWhenSubjectExists`: subject before referrer, linked by `BEFORE INSERT` at insert time. ## Known gaps not covered by referrer/subject linking Two creation paths do not record `subject_digest`, so the triggers cannot link them: OCI image index (manifest list) referrers, and the import path. Both are pre-existing and were left out of !2931. Split into a dedicated follow-up: #2390.