fix(managementapi): record the acting user on repository writes

Sylvia Shenrequested to merge
1261/repository-attribution into main

What this changes

Repository create and update never wrote gitlab_created_by_user_id or gitlab_last_updated_by_user_id, so the management API answered created_by: null and updated_by: null on every repository and the monolith rendered a date with no avatar or name. The monolith side is complete; this is the service half.

  • Create. insertRepository binds the caller's GitLab user id into gitlab_created_by_user_id. The create handler reads it with Identity.GitLabUserID(), the same accessor the Maven and npm version writes use.
  • Update. Every edit of the repository records the caller in gitlab_last_updated_by_user_id, whichever table holds the field it changed. The id lives only on the repositories row. repositoryPatchAssignments adds it to the SET list beside a column the client edited (description, visibility), or alone when the same transaction changed a per-kind row:
    • PATCH with upstream_repository_ids: UpdateVirtual records it when the reconcile changed the list.
    • POST, PATCH, and DELETE on /{format}/upstream_repositories[/{association_id}]: each virtual upstream store records it when the mutation changed the list. A move to the position the entry already holds records nothing.
    • PATCH with remote settings: the three settings updaters record it when the store reports the remote row changed, and the response returns it. They lock the repositories row FOR NO KEY UPDATE before the remote row, the order deleteRepositoryTx takes them in, so a concurrent delete cannot deadlock against a settings write. That lock is the one new statement, repositories_select_lock_for_remote_settings_update.
  • No principal, no write. A request with no GitLab user (the bootstrap-token stub, a federated principal) stores NULL on create and leaves the stored updated_by as it is on update. A body that edits no column still writes nothing: RepositoryPatch.IsZero ignores the user id. A settings write that changes nothing (a respelled url) leaves the stored updater too.

Merge order

Merge !3091 (merged) (!3091 (merged)) first. It rewrites the S17 and monolith/S05 sentences that said these fields stay null until a full auth integration, and updates the OpenAPI field descriptions and the management e2e catalog. This MR implements what it describes.

Behavior to know

  • Repositories created before this lands keep created_by: null. The acting user is only knowable at write time, so there is nothing to backfill.
  • A settings-only or list-only PATCH, and every association route call that changes the list, now also writes the repositories row, in the same transaction as the per-kind write. internal/datastore/repositories.md (## Every edit of a repository records its updater on the repositories row) documents the rule and the lock order.

Verification

  • go test ./internal/datastore/ ./internal/managementapi/ ./cmd/artifact-registry/ passes.
  • go test -tags=integration ./internal/managementapi/ passes.
  • New integration tests in internal/datastore pass against Postgres. They cover create and update attribution, the list-only virtual update (changed list, unchanged list, no user), all three association mutations on npm, Maven, and docker (including a move to the same position and a refused associate), the settings updaters for all three families (including a respelled url, no user, and a tombstoned parent), and the settings updater's lock order against a concurrent delete (TestNpmRemoteRepositoryUpdater_TakesTheDeletesLockOrder).
  • The full internal/datastore integration suite, run against a Postgres container with CI's max_locks_per_transaction=1024, passes except TestListNpmVersionIDsForDeleteAll_HostedKeepsTheAcceptanceBound. That test seeds with raw SQL and the npm version store and compares created_at against the host clock, and it reaches none of the code changed here. Against a stock-configured container, the namespace cleanups instead fail with ERROR: out of shared memory (SQLSTATE 53200).
  • driver.sh smoke passed 94 of 94 on the earlier head (d570d0c0). Development mode establishes no principal, so it shows created_by and updated_by staying null. A non-null id is covered by the handler and integration tests, not by an end-to-end run.

Guardrail checklist

  • Tests: included; the existing update_upstream_list_test.go cases now expect the caller's id on the patch they record, and the existing association store tests pass a nil updater.
  • docs/testing/ and the OpenAPI descriptions: in !3091 (merged).
  • docs/user/**: no page describes repository attribution, so none is affected.
  • Conformance tests: no Maven, npm, or OCI protocol behavior changes.
  • Diff size: about 1,750 lines, over the 500 line ceiling: 400 production, 1,287 tests, 59 docs. About 160 of the test lines are existing association store tests gaining a nil updater argument. Splitting would not help review: the gate in repositoryPatchAssignments and the paths that set it only make sense together, and a split would leave updated_by wrong on whichever paths landed second.

This does not close #1261: the issue's last acceptance item, the monolith rendering avatar and name, still needs a check on staging after both MRs deploy.

Related to #1261

🤖 Generated with Claude Code

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand each row's details for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.insertRepository Insert n/a 1 / 1 0.01 0.392ms 153 / 0 1/64
datastore.lockRepositoryForRemoteSettingsUpdate LockRows repositories_p<N>_pkey 1 / 1 8.31 0.023ms 4 / 0 1/64
datastore.updateRepositoryTx.AttributionOnly Update repositories_p<N>_pkey 1 / 1 8.30 0.480ms 118 / 0 1/64
datastore.updateRepositoryTx.ClientColumnsAndUpdater Update repositories_p<N>_pkey 1 / 1 8.30 0.248ms 76 / 0 1/64

The UPDATE branches were also run with the recipe's 50-row write seed (1 target, 49 siblings). There the planner chose a Seq Scan over the one-page partition, the expected choice at that size. The plans below use a 5,000-row seed so they show the index path production takes.

datastore.insertRepository

Summary: Plan matches intent: a single-row Insert over a Result node, routed to one partition by namespace_id. The new gitlab_created_by_user_id bind adds one text value and no index maintenance (no index covers the column). No anomalies.

Seed shape: namespaces=1, repositories=49

Rendered SQL:

INSERT INTO public.repositories (id, namespace_id, name, format, kind, visibility, description, gitlab_created_by_user_id)
VALUES ($1::uuid, $2::uuid, $3::text, $4, $5, $6, NULL::text, $7::text)
RETURNING repositories.namespace_id AS "repositories.namespace_id",
          repositories.id AS "repositories.id",
          repositories.artifacts_count AS "repositories.artifacts_count",
          repositories.downloads_count AS "repositories.downloads_count",
          repositories.size_bytes AS "repositories.size_bytes",
          repositories.created_at AS "repositories.created_at",
          repositories.last_updated_at AS "repositories.last_updated_at",
          repositories.soft_deleted_at AS "repositories.soft_deleted_at",
          repositories.format AS "repositories.format",
          repositories.kind AS "repositories.kind",
          repositories.visibility AS "repositories.visibility",
          repositories.name AS "repositories.name",
          repositories.description AS "repositories.description",
          repositories.gitlab_created_by_user_id AS "repositories.gitlab_created_by_user_id",
          repositories.gitlab_last_updated_by_user_id AS "repositories.gitlab_last_updated_by_user_id",
          repositories.last_reconciled_at AS "repositories.last_reconciled_at";

Bound args: [f53aebe6-b750-7636-ad58-1f1cc2020b92, 7650d950-b44b-7c4f-9cc7-ef05595f454b, review-prep-new, 1, 2, 0, 4242]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                               QUERY PLAN                                               
--------------------------------------------------------------------------------------------------------
 Insert on repositories  (cost=0.00..0.01 rows=1 width=222) (actual time=0.392..0.392 rows=1 loops=1)
   Buffers: shared hit=153
   ->  Result  (cost=0.00..0.01 rows=1 width=222) (actual time=0.002..0.002 rows=1 loops=1)
 Planning Time: 0.043 ms
 Trigger for constraint fk_repositories_namespace_id_namespaces on repositories_p03: time=0.016 calls=1
 Execution Time: 0.508 ms
(6 rows)

Timings: planning 0.043ms, execution 0.508ms, total 0.551ms.

datastore.lockRepositoryForRemoteSettingsUpdate

Summary: Plan matches intent: a primary-key Index Scan on (id, namespace_id) under LockRows, pruned to 1 of 64 partitions, with soft_deleted_at IS NULL as a filter on the one matched row. A key lookup, so no LIMIT is needed. No anomalies.

Seed shape: namespaces=1, repositories=5000 (50 soft-deleted)

Rendered SQL:

SELECT repositories.id AS "repositories.id"
FROM public.repositories
WHERE ((repositories.namespace_id = $1::uuid) AND (repositories.id = $2::uuid)) AND (repositories.soft_deleted_at IS NULL)
FOR NO KEY UPDATE;

Bound args: [e851bc3b-3e9e-7e51-98ee-d707ef922755, b6d37f07-6ddd-726a-a719-c0e9fb5bfd33]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                                                         QUERY PLAN                                                                         
------------------------------------------------------------------------------------------------------------------------------------------------------------
 LockRows  (cost=0.28..8.31 rows=1 width=26) (actual time=0.022..0.023 rows=1 loops=1)
   Buffers: shared hit=4
   ->  Index Scan using repositories_p10_pkey on repositories_p10 repositories  (cost=0.28..8.30 rows=1 width=26) (actual time=0.016..0.017 rows=1 loops=1)
         Index Cond: ((id = 'b6d37f07-6ddd-726a-a719-c0e9fb5bfd33'::uuid) AND (namespace_id = 'e851bc3b-3e9e-7e51-98ee-d707ef922755'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=394
 Planning Time: 1.574 ms
 Execution Time: 0.041 ms
(10 rows)

Timings: planning 1.574ms, execution 0.041ms, total 1.615ms.

datastore.updateRepositoryTx.AttributionOnly

Summary: Plan matches intent: the attribution-only write a per-kind edit now issues sets one column through the same primary-key Index Scan as the client-column update, pruned to 1 of 64 partitions. No anomalies.

Seed shape: namespaces=1, repositories=5000 (50 soft-deleted)

Rendered SQL:

UPDATE public.repositories
SET gitlab_last_updated_by_user_id = $1::text
WHERE ((repositories.namespace_id = $2::uuid) AND (repositories.id = $3::uuid)) AND (repositories.soft_deleted_at IS NULL)
RETURNING repositories.namespace_id AS "repositories.namespace_id",
          repositories.id AS "repositories.id",
          repositories.artifacts_count AS "repositories.artifacts_count",
          repositories.downloads_count AS "repositories.downloads_count",
          repositories.size_bytes AS "repositories.size_bytes",
          repositories.created_at AS "repositories.created_at",
          repositories.last_updated_at AS "repositories.last_updated_at",
          repositories.soft_deleted_at AS "repositories.soft_deleted_at",
          repositories.format AS "repositories.format",
          repositories.kind AS "repositories.kind",
          repositories.visibility AS "repositories.visibility",
          repositories.name AS "repositories.name",
          repositories.description AS "repositories.description",
          repositories.gitlab_created_by_user_id AS "repositories.gitlab_created_by_user_id",
          repositories.gitlab_last_updated_by_user_id AS "repositories.gitlab_last_updated_by_user_id",
          repositories.last_reconciled_at AS "repositories.last_reconciled_at";

Bound args: [4242, b334aa92-2b1a-70b8-b088-3bb0a81d861a, 8c34b082-4e7b-7116-b737-396ddd588d6b]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                                                          QUERY PLAN                                                                          
--------------------------------------------------------------------------------------------------------------------------------------------------------------
 Update on repositories  (cost=0.28..8.30 rows=1 width=42) (actual time=0.479..0.480 rows=1 loops=1)
   Update on repositories_p58 repositories_1
   Buffers: shared hit=118
   ->  Index Scan using repositories_p58_pkey on repositories_p58 repositories_1  (cost=0.28..8.30 rows=1 width=42) (actual time=0.008..0.008 rows=1 loops=1)
         Index Cond: ((id = '8c34b082-4e7b-7116-b737-396ddd588d6b'::uuid) AND (namespace_id = 'b334aa92-2b1a-70b8-b088-3bb0a81d861a'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=158
 Planning Time: 0.771 ms
 Trigger for constraint fk_repositories_namespace_id_namespaces on repositories_p58: time=0.015 calls=1
 Execution Time: 0.668 ms
(12 rows)

Timings: planning 0.771ms, execution 0.668ms, total 1.439ms.

datastore.updateRepositoryTx.ClientColumnsAndUpdater

Summary: Plan matches intent: the existing metadata update with gitlab_last_updated_by_user_id added to the SET list. Primary-key Index Scan, pruned to 1 of 64 partitions; the extra column changes nothing in the plan. No anomalies.

Seed shape: namespaces=1, repositories=5000 (50 soft-deleted)

Rendered SQL:

UPDATE public.repositories
SET visibility = $1,
    description = $2::text,
    gitlab_last_updated_by_user_id = $3::text
WHERE ((repositories.namespace_id = $4::uuid) AND (repositories.id = $5::uuid)) AND (repositories.soft_deleted_at IS NULL)
RETURNING repositories.namespace_id AS "repositories.namespace_id",
          repositories.id AS "repositories.id",
          repositories.artifacts_count AS "repositories.artifacts_count",
          repositories.downloads_count AS "repositories.downloads_count",
          repositories.size_bytes AS "repositories.size_bytes",
          repositories.created_at AS "repositories.created_at",
          repositories.last_updated_at AS "repositories.last_updated_at",
          repositories.soft_deleted_at AS "repositories.soft_deleted_at",
          repositories.format AS "repositories.format",
          repositories.kind AS "repositories.kind",
          repositories.visibility AS "repositories.visibility",
          repositories.name AS "repositories.name",
          repositories.description AS "repositories.description",
          repositories.gitlab_created_by_user_id AS "repositories.gitlab_created_by_user_id",
          repositories.gitlab_last_updated_by_user_id AS "repositories.gitlab_last_updated_by_user_id",
          repositories.last_reconciled_at AS "repositories.last_reconciled_at";

Bound args: [1, review-prep-description, 4242, 71fe10e8-9f5b-74a8-9a62-dff3dd8ae448, 3dd33b92-7569-765f-a262-1aa04dbe7e78]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                                                          QUERY PLAN                                                                          
--------------------------------------------------------------------------------------------------------------------------------------------------------------
 Update on repositories  (cost=0.28..8.30 rows=1 width=76) (actual time=0.247..0.248 rows=1 loops=1)
   Update on repositories_p05 repositories_1
   Buffers: shared hit=76
   ->  Index Scan using repositories_p05_pkey on repositories_p05 repositories_1  (cost=0.28..8.30 rows=1 width=76) (actual time=0.007..0.008 rows=1 loops=1)
         Index Cond: ((id = '3dd33b92-7569-765f-a262-1aa04dbe7e78'::uuid) AND (namespace_id = '71fe10e8-9f5b-74a8-9a62-dff3dd8ae448'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=156
 Planning Time: 0.679 ms
 Trigger for constraint fk_repositories_namespace_id_namespaces on repositories_p05: time=0.012 calls=1
 Execution Time: 0.341 ms
(12 rows)

Timings: planning 0.679ms, execution 0.341ms, total 1.020ms.

Edited by Sylvia Shen

Merge request reports