Draft: fix(datastore): read container virtual upstreams by repositories.id

What changed and why

ContainerVirtualResolutionStore.UpstreamsByVirtualRepository filtered on container_virtual_repository_upstreams.container_virtual_repository_id. That column holds a container_virtual_repositories.id. The insert mints that id with newID(), so it never equals the virtual repository's repositories.id. The request path holds only the repositories.id (namespace.Resolution.RepositoryID), and no code maps one id to the other. The virtual read handlers would therefore get an empty upstream list for every virtual repository. No production code calls the read yet, so nothing on main misbehaves today.

The read now takes the virtual repository's repositories.id. It joins container_virtual_repositories on namespace_id and id, and filters on its repository_id, in the same statement. This matches the plan's Naming Conventions entry: "the list read joins from repositories through container_virtual_repositories in one statement, so no separate finder exists".

  • The list is still one statement, so a resolution still issues 2 + K statements (spec Acceptance Criteria, "Statement count follows upstream kinds").
  • Every partitioned table in the statement binds the namespace literal. The EXPLAIN suite now expects four tables, and each prunes to one partition.
  • VirtualAssemblyRequest.ContainerVirtualRepositoryID and VirtualReadRequest.ContainerVirtualRepositoryID keep their names, because !2940 (merged)'s tests set that field. Their doc comments now say they carry the repositories.id.
  • The assembly's read-failure log now reports the id as virtual_repository_id, the field the serve failure log already uses. docs/dev/observability.md says so.

Tests

  • A new case seeds a virtual repository whose two ids differ. The repositories.id resolves the list, and the container_virtual_repositories.id resolves nothing.
  • The datastore and OCI integration fixtures now key the read on the repositories.id. Their association inserts still reference the container_virtual_repositories.id.

Before merge

!2940 (merged) adds wire_oci_virtual_integration_test.go, whose exclusion-audit test resolves by the container_virtual_repositories.id. That file is not on main yet, so this branch cannot carry the change now. After !2940 (merged) merges, this branch rebases onto main and adds the change below unchanged. The seed then returns the repositories.id, and the test resolves by it.

diff --git a/cmd/artifact-registry/wire_oci_virtual_integration_test.go b/cmd/artifact-registry/wire_oci_virtual_integration_test.go
index 3a6d68670..2ccebb77c 100644
--- a/cmd/artifact-registry/wire_oci_virtual_integration_test.go
+++ b/cmd/artifact-registry/wire_oci_virtual_integration_test.go
@@ -73,12 +73,14 @@ func startOCIVirtualBootClient(t *testing.T, label string) *postgres.Client {
 	return client
 }
 
-// seedOCIVirtualContainerRepository returns the container_virtual_repositories.id
-// of a kind=1 repository named repo.
-func seedOCIVirtualContainerRepository(t *testing.T, client *postgres.Client, nsID uuid.UUID) uuid.UUID {
+// seedOCIVirtualContainerRepository seeds a kind=1 repository named repo and returns its repositories.id,
+// which a read addresses, and its container_virtual_repositories.id, which an association references.
+func seedOCIVirtualContainerRepository(
+	t *testing.T, client *postgres.Client, nsID uuid.UUID,
+) (repositoryID, virtualID uuid.UUID) {
 	t.Helper()
 
-	repositoryID := uuid.Must(uuid.NewV7())
+	repositoryID = uuid.Must(uuid.NewV7())
 	_, err := client.DB().ExecContext(t.Context(),
 		`INSERT INTO repositories (id, namespace_id, name, format, kind, visibility)
 		VALUES ($1, $2, 'repo', $3, $4, $5)`,
@@ -91,13 +93,13 @@ func seedOCIVirtualContainerRepository(t *testing.T, client *postgres.Client, ns
 		uuid.Must(uuid.NewV7()), nsID, repositoryID)
 	require.NoError(t, err, "seeding container_repositories")
 
-	virtualID := uuid.Must(uuid.NewV7())
+	virtualID = uuid.Must(uuid.NewV7())
 	_, err = client.DB().ExecContext(t.Context(),
 		`INSERT INTO container_virtual_repositories (namespace_id, id, repository_id) VALUES ($1, $2, $3)`,
 		nsID, virtualID, repositoryID)
 	require.NoError(t, err, "seeding container_virtual_repositories")
 
-	return virtualID
+	return repositoryID, virtualID
 }
 
 // seedOCIVirtualDeniedHostedUpstream lists one hosted upstream at position 1
@@ -456,7 +458,7 @@ func TestBuildOCIVirtualComposition_ResolverRecordsExclusionsThroughTheRootsAudi
 
 	client := startOCIVirtualBootClient(t, "oci-wire-virtual-audit")
 	nsID := testutil.SeedNamespace(t, client.DB(), "virtual-audit")
-	virtualID := seedOCIVirtualContainerRepository(t, client, nsID)
+	repositoryID, virtualID := seedOCIVirtualContainerRepository(t, client, nsID)
 	seedOCIVirtualDeniedHostedUpstream(t, client, nsID, virtualID, imageName)
 
 	var buf bytes.Buffer
@@ -467,7 +469,7 @@ func TestBuildOCIVirtualComposition_ResolverRecordsExclusionsThroughTheRootsAudi
 
 	_, err = composition.Resolution.Resolve(t.Context(), oci.VirtualReadRequest{
 		NamespaceID:                  nsID,
-		ContainerVirtualRepositoryID: virtualID,
+		ContainerVirtualRepositoryID: repositoryID,
 		Reference: oci.VirtualHostedReference{
 			Kind: oci.VirtualHostedReferenceTag, ImageName: imageName, TagName: "latest",
 		},

E2E catalog and user docs

  • No e2e scenario changes. docs/testing/e2e/oci.md and docker.md keep every client journey against a virtual repository out of scope, and the virtual read routes still answer their interim 501.
  • No docs/user/ change. No customer can reach this read yet.

Related to #291 (closed)

🤖 Generated with Claude Code

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading