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_id → namespaces |
NO ACTION |
container_virtual_repository_upstreams |
namespace_id → namespaces |
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_id → namespaces |
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_idfails 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 Deletionfor 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 valuerule 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-cli2andvale --minAlertLevel erroron the changed file: clean.valereports 0 errors and the same warning set asmain.- Rendered locally with the pinned
hugo-extended 0.151.0and checked in a browser: the ERD parses, the new column annotations render, and the#repositoriesand009_api_design.md#repository-deletionlinks 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.