fix pushing images when the historical hold does not match the default hold in the account

This commit is contained in:
Evan Jarrett
2025-12-09 11:38:26 -06:00
parent c4a9e4bf00
commit 5dff759064
3 changed files with 93 additions and 16 deletions
+1 -1
View File
@@ -11,7 +11,7 @@ architecture: amd64
environment:
IMAGE_REGISTRY: atcr.io
IMAGE_USER: evan.jarrett.net
IMAGE_USER: atcr.io
steps:
- name: Login to registry
+12 -4
View File
@@ -64,18 +64,26 @@ func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore {
return blobStore
}
// For pull operations, check database for hold DID from the most recent manifest
// This ensures blobs are fetched from the hold recorded in the manifest, not re-discovered
// Determine if this is a pull (GET) or push (PUT/POST/HEAD/etc) operation
// Pull operations use the historical hold DID from the database (blobs are where they were pushed)
// Push operations use the discovery-based hold DID from user's profile/default
// This allows users to change their default hold and have new pushes go there
isPull := false
if method, ok := ctx.Value("http.request.method").(string); ok {
isPull = method == "GET"
}
holdDID := r.Ctx.HoldDID // Default to discovery-based DID
holdSource := "discovery"
if r.Ctx.Database != nil {
// Only query database for pull operations
if isPull && r.Ctx.Database != nil {
// Query database for the latest manifest's hold DID
if dbHoldDID, err := r.Ctx.Database.GetLatestHoldDIDForRepo(r.Ctx.DID, r.Ctx.Repository); err == nil && dbHoldDID != "" {
// Use hold DID from database (pull case - use historical reference)
holdDID = dbHoldDID
holdSource = "database"
slog.Debug("Using hold from database manifest", "component", "storage/blobs", "did", r.Ctx.DID, "repo", r.Ctx.Repository, "hold", dbHoldDID)
slog.Debug("Using hold from database manifest (pull)", "component", "storage/blobs", "did", r.Ctx.DID, "repo", r.Ctx.Repository, "hold", dbHoldDID)
} else if err != nil {
// Log error but don't fail - fall back to discovery-based DID
slog.Warn("Failed to query database for hold DID", "component", "storage/blobs", "error", err)
+80 -11
View File
@@ -109,24 +109,90 @@ func TestRoutingRepository_ManifestStoreCaching(t *testing.T) {
assert.NotNil(t, repo.manifestStore)
}
// TestRoutingRepository_Blobs_WithDatabase tests blob store with database hold DID
func TestRoutingRepository_Blobs_WithDatabase(t *testing.T) {
// TestRoutingRepository_Blobs_PullUsesDatabase tests that GET (pull) uses database hold DID
func TestRoutingRepository_Blobs_PullUsesDatabase(t *testing.T) {
dbHoldDID := "did:web:database.hold.io"
discoveryHoldDID := "did:web:discovery.hold.io"
ctx := &RegistryContext{
DID: "did:plc:test123",
Repository: "myapp",
HoldDID: "did:web:default.hold.io", // Discovery-based hold (should be overridden)
HoldDID: discoveryHoldDID, // Discovery-based hold (should be overridden for pull)
ATProtoClient: atproto.NewClient("https://pds.example.com", "did:plc:test123", ""),
Database: &mockDatabase{holdDID: dbHoldDID},
}
repo := NewRoutingRepository(nil, ctx)
// Create context with GET method (pull operation)
pullCtx := context.WithValue(context.Background(), "http.request.method", "GET")
blobStore := repo.Blobs(pullCtx)
assert.NotNil(t, blobStore)
// Verify the hold DID was updated to use the database value for pull
assert.Equal(t, dbHoldDID, repo.Ctx.HoldDID, "pull (GET) should use database hold DID")
}
// TestRoutingRepository_Blobs_PushUsesDiscovery tests that push operations use discovery hold DID
func TestRoutingRepository_Blobs_PushUsesDiscovery(t *testing.T) {
dbHoldDID := "did:web:database.hold.io"
discoveryHoldDID := "did:web:discovery.hold.io"
testCases := []struct {
name string
method string
}{
{"PUT", "PUT"},
{"POST", "POST"},
{"HEAD", "HEAD"},
{"PATCH", "PATCH"},
{"DELETE", "DELETE"},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ctx := &RegistryContext{
DID: "did:plc:test123",
Repository: "myapp-" + tc.method, // Unique repo to avoid caching
HoldDID: discoveryHoldDID,
ATProtoClient: atproto.NewClient("https://pds.example.com", "did:plc:test123", ""),
Database: &mockDatabase{holdDID: dbHoldDID},
}
repo := NewRoutingRepository(nil, ctx)
// Create context with push method
pushCtx := context.WithValue(context.Background(), "http.request.method", tc.method)
blobStore := repo.Blobs(pushCtx)
assert.NotNil(t, blobStore)
// Verify the hold DID remains the discovery-based one for push operations
assert.Equal(t, discoveryHoldDID, repo.Ctx.HoldDID, "%s should use discovery hold DID, not database", tc.method)
})
}
}
// TestRoutingRepository_Blobs_NoMethodUsesDiscovery tests that missing method defaults to discovery
func TestRoutingRepository_Blobs_NoMethodUsesDiscovery(t *testing.T) {
dbHoldDID := "did:web:database.hold.io"
discoveryHoldDID := "did:web:discovery.hold.io"
ctx := &RegistryContext{
DID: "did:plc:test123",
Repository: "myapp-nomethod",
HoldDID: discoveryHoldDID,
ATProtoClient: atproto.NewClient("https://pds.example.com", "did:plc:test123", ""),
Database: &mockDatabase{holdDID: dbHoldDID},
}
repo := NewRoutingRepository(nil, ctx)
// Context without HTTP method (shouldn't happen in practice, but test defensive behavior)
blobStore := repo.Blobs(context.Background())
assert.NotNil(t, blobStore)
// Verify the hold DID was updated to use the database value
assert.Equal(t, dbHoldDID, repo.Ctx.HoldDID, "should use database hold DID")
// Without method, should default to discovery (safer for push scenarios)
assert.Equal(t, discoveryHoldDID, repo.Ctx.HoldDID, "missing method should use discovery hold DID")
}
// TestRoutingRepository_Blobs_WithoutDatabase tests blob store with discovery-based hold
@@ -292,23 +358,26 @@ func TestRoutingRepository_ConcurrentAccess(t *testing.T) {
assert.NotNil(t, cachedBlobStore)
}
// TestRoutingRepository_Blobs_Priority tests that database hold DID takes priority over discovery
func TestRoutingRepository_Blobs_Priority(t *testing.T) {
// TestRoutingRepository_Blobs_PullPriority tests that database hold DID takes priority for pull (GET)
func TestRoutingRepository_Blobs_PullPriority(t *testing.T) {
dbHoldDID := "did:web:database.hold.io"
discoveryHoldDID := "did:web:discovery.hold.io"
ctx := &RegistryContext{
DID: "did:plc:test123",
Repository: "myapp",
Repository: "myapp-priority",
HoldDID: discoveryHoldDID, // Discovery-based hold
ATProtoClient: atproto.NewClient("https://pds.example.com", "did:plc:test123", ""),
Database: &mockDatabase{holdDID: dbHoldDID}, // Database has a different hold DID
}
repo := NewRoutingRepository(nil, ctx)
blobStore := repo.Blobs(context.Background())
// For pull (GET), database should take priority
pullCtx := context.WithValue(context.Background(), "http.request.method", "GET")
blobStore := repo.Blobs(pullCtx)
assert.NotNil(t, blobStore)
// Database hold DID should take priority over discovery
assert.Equal(t, dbHoldDID, repo.Ctx.HoldDID, "database hold DID should take priority over discovery")
// Database hold DID should take priority over discovery for pull operations
assert.Equal(t, dbHoldDID, repo.Ctx.HoldDID, "database hold DID should take priority over discovery for pull (GET)")
}