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 + Kstatements (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.ContainerVirtualRepositoryIDandVirtualReadRequest.ContainerVirtualRepositoryIDkeep their names, because !2940 (merged)'s tests set that field. Their doc comments now say they carry therepositories.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.mdsays so.
Tests
- A new case seeds a virtual repository whose two ids differ. The
repositories.idresolves the list, and thecontainer_virtual_repositories.idresolves nothing. - The datastore and OCI integration fixtures now key the read on the
repositories.id. Their association inserts still reference thecontainer_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.mdanddocker.mdkeep every client journey against a virtual repository out of scope, and the virtual read routes still answer their interim501. - No
docs/user/change. No customer can reach this read yet.
Related to #291 (closed)