Amend ADR-007 container virtual table definitions

Why is this change being made?

ADR-007's three container_virtual_* table definitions do not match what the S32 container-virtual spec requires under its ## Data Model section, and what the S32 container-virtual plan will build in Steps 1 to 3. The spec's ## Follow-ups tracks this amendment under artifact-registry#264.

The migration author is about to write these tables, so a schema statement that cannot be built, or a referential action nobody wrote down, becomes a decision made in a migration file instead of in the ADR.

1. Reverse index added (addition)

container_virtual_repository_upstreams now carries an index on (namespace_id, upstream_repository_id). Neither existing unique entry has that as a usable prefix, and PostgreSQL does not index foreign-key columns automatically, so the delete guard and the association surface — both of which ask "which virtual repositories list this repository as an upstream?" — had no index to use.

Maven gains the same index, for the reason under Maven below.

2. DEFERRABLE INITIALLY DEFERRED is a constraint clause, not an index clause (correction)

The ordered-upstream entry was written as a unique index with DEFERRABLE INITIALLY DEFERRED appended. That is not constructible: CREATE UNIQUE INDEX ... DEFERRABLE is a syntax error in PostgreSQL, and deferral exists only on a constraint added with ALTER TABLE ... ADD CONSTRAINT ... UNIQUE (...) DEFERRABLE INITIALLY DEFERRED.

The entry is rewritten as a unique constraint, same column list, same "deferrable to allow reordering within a transaction" rationale.

The npm virtual table already carries such a constraint — unique_nvru_ns_id_repository_id_position, added by 20260828102654_create_npm_virtual_repository_upstreams.sql — so the container migration copies a sibling rather than establishing the pattern. The ADR now names it, along with the one thing not to copy: npm's position floor is 0 and the container table's is 1. The added prose also records the consequence a migration author would otherwise hit at runtime, that a deferrable unique constraint cannot arbitrate ON CONFLICT.

The other unique entry on that table, and the ones on the other two container virtual tables, are genuinely indexes and are unchanged.

3. The ON DELETE actions are stated (addition)

The ERD gave NOT NULL and the composite FK targets and no referential action. All seven foreign keys across the three container virtual tables are now enumerated individually, because a reader cannot tell an unstated action from a defaulted one.

Table Foreign key Action
container_virtual_repositories (repository_id, namespace_id)repositories CASCADE
container_virtual_repositories namespace_idnamespaces NO ACTION
container_virtual_repository_upstreams namespace_idnamespaces NO ACTION
container_virtual_repository_upstreams (container_virtual_repository_id, namespace_id)container_virtual_repositories CASCADE
container_virtual_repository_upstreams (upstream_repository_id, namespace_id)repositories NO ACTION
container_virtual_upstream_rules namespace_idnamespaces NO ACTION
container_virtual_upstream_rules (container_virtual_repository_upstream_id, namespace_id)container_virtual_repository_upstreams CASCADE

Only the first was already covered: the general rule gives every format child table ON DELETE CASCADE on that edge, and the S32 spec agrees. It is listed for completeness and its action is unchanged.

The two foreign keys on container_virtual_repository_upstreams take opposite actions, which is the pair that earns a sentence of rationale in the diff: deleting a virtual repository takes its associations with it, while deleting a repository still listed as an upstream is refused, so the list cannot silently lose a position.

4. CHECK constraints stated (addition)

The value legends were prose annotations — rule_type "0=allow, 1=deny", target_field "0=image, 1=tag", pattern "limit 255", position "NOT NULL" — with nothing saying the database enforces them. They are now stated as constraints: CHECK rule_type IN (0, 1), CHECK target_field IN (0, 1), CHECK char_length(pattern) <= 255, and CHECK position >= 1.

This is not cosmetic. The S32 plan declines to filter a corrupt target_field in its slice because the CHECK is the boundary rejection, and its rule adapter carries a corrupt value through to the matcher rather than dropping it. That argument rests on the constraint existing, so the ADR should say it does.

### Repositories: the delete path

Review found that the NO ACTION key item 3 adds gives DELETE FROM repositories a second refusal path, and that ### Repositories named artifacts as the only blocker while linking the reader straight at the new key. That section now:

  • Lists the junction key, format-generically across all three virtual slices.

  • Names both refusal paths, artifacts and in-use-as-upstream.

  • States two tests and how they compose, rather than picking one axis. Direction gates first, so only a parent-to-child edge cascades. What the child holds decides among those edges. upstream_repository_id fails the first test, so no reading of what a junction row holds makes it cascade.

  • Records what an operator actually sees. A referential action refuses the delete without reporting it: a synchronous delete fails as a statement the API layer has to translate, and an asynchronous one stalls a purge no caller sees. The management API is the refusal a caller can rely on, and the paragraph points at ADR-009's Repository Deletion for it.

    An earlier revision of this branch also asserted that ADR-009 states the artifacts case and not the upstream-reference case. That sentence is gone. It held only until !20941 (merged) merges — the MR adding the missing case — so this ADR would have landed a claim about a sibling document that its sibling had already falsified. ADR-007 has no reason to track what ADR-009 omits, and !20941 (merged) puts the either destructive value rule in the contract that owns it.

There is deliberately no count of refusal paths in that prose. An earlier revision said "two", and artifact_type_repository_lifecycle_policy_settings and upload_sessions each draw a repositories foreign key with no action stated, so the number was wrong. A count over the keys this document draws is a measurement that drifts as tables are added and nothing checks. The list is the enumeration; the prose does not restate its size. Both entries are named for what they are — declared in an ERD, absent from the shipped schema, and NO ACTION by default if ever added as drawn.

Maven

Item 2's correction and item 1's index both reach the maven section, because the general junction-key bullet claims NO ACTION for all three formats and maven was the only section still silent on it. The document would otherwise set the action in one place and say nothing in the other, which is the defect this MR exists to fix. Maven's ordered-upstream entry becomes a unique constraint, its junction key states ON DELETE NO ACTION, and it gains the reverse-lookup index that serves the guard.

npm needed nothing: !20938 (merged) landed the same three changes there, and this branch is rebased on main so it carries that text rather than reverting it.

The position decision, and a divergence on record

position is where the two virtual slices disagree and the ADR was silent (it gave only int position "NOT NULL"):

  • S32 fixes CHECK (position >= 1) and treats the column as 1-based and contiguous.
  • The npm virtual spec fixes CHECK (position >= 0) and states that gaps resolve identically, so [0, 2, 5] behaves as [0, 1, 2] would.

This MR records S32's constraint only. It states CHECK position >= 1 and the 1-based contiguous rule on the container table and does not touch the npm table's floor. Arbitrating for every virtual upstream table needs the npm DRI.

The divergence stays open for the S17 or S13 DRI, and it is recorded here so it is not lost. What makes leaving it open safe rather than just cheaper: the two constraints are not symmetric. S17 already specifies association position as 1-based and contiguous, and rejects a position below 1 with a 422, so the shared association surface writes values that satisfy both tables. A helper written to S17's rule works against the npm table; only a 0-based helper written against the npm table alone would fail against the container one.

Out of scope

Maven's other ERD entries still state no referential action, and container's and npm's now all do. This MR states maven's junction key alone, because that is the key the new general bullet claims an action for. The rest of maven's actions belong with the maven slice's own amendment.

Validation

  • markdownlint-cli2 and vale --minAlertLevel error on the changed file: clean. vale reports 0 errors and the same warning set as main.
  • Rendered locally with the pinned hugo-extended 0.151.0 and checked in a browser: the ERD parses, the new column annotations render, and the #repositories and 009_api_design.md#repository-deletion links resolve.
  • One file changed. No file overlap with !20929 (merged) or !20941 (merged), which touch ADR-009.

Author and Reviewer Checklist

Please verify the check list and ensure to tick them off before the MR is merged.

  • Provided a concise title for this Merge Request (MR)
  • Added a description to this MR explaining the reasons for the proposed change, per say why, not just what
  • Assign reviewers for this MR to the correct
  • For transparency, share this MR with the audience that will be impacted.

Code owners request @gitlab-org/architecture/coaches for this path. The ADR-007 DRI should approve.


Edited by Radamanthus Batnag

Merge request reports

Loading
Loading