mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-19 08:44:14 +00:00
appview: drop the testmode fall-back-to-default-hold probe
The registry middleware used to GET the user's hold's /.well-known/did.json on every registry request in testmode builds, and fall back to the appview default hold if it did not answer. The fallback only ever changed anything when the user's chosen hold differed from the default AND was down; in local development and in the integration harness the two are the same hold, so the probe's answer was discarded every time. In the production-shaped benchmark it was 94 of 150 hold requests per p90 push and 23 of 90 per pull, hiding the real hold traffic behind a testmode artifact. Remove isHoldReachable, the fallbackUnreachable field, and the probe branch. An empty choice still means the default hold. Testmode and production now take the same path here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WTdBxLFU5TpwmqVdVsN1wq
This commit is contained in:
Evan Jarrett
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
dcee8f6a62
commit
65db7945b2
@@ -96,9 +96,9 @@ client plain HTTP clients so it can reach a PDS on loopback.
|
||||
compiles them out and stays green while `make test` runs everything. The
|
||||
integration harness carries the same constraint.
|
||||
- The remaining local-dev behaviors (hold issuer-mismatch tolerance, skipped
|
||||
relay crawl requests, backfill warning suppression, registry
|
||||
fall-back-to-default-hold) read `atproto.TestModeBuild` too. There is no
|
||||
runtime test-mode config key any more.
|
||||
relay crawl requests, backfill warning suppression) read
|
||||
`atproto.TestModeBuild` too. There is no runtime test-mode config key any
|
||||
more.
|
||||
|
||||
## Architecture Overview
|
||||
|
||||
|
||||
@@ -257,17 +257,16 @@ func init() {
|
||||
// NamespaceResolver wraps a namespace and resolves names
|
||||
type NamespaceResolver struct {
|
||||
distribution.Namespace
|
||||
defaultHoldDID string // Default hold DID (e.g., "did:web:hold01.atcr.io")
|
||||
baseURL string // Base URL for error messages (e.g., "https://atcr.io")
|
||||
fallbackUnreachable bool // Fall back to the default hold when the user's hold is unreachable (testmode builds)
|
||||
refresher *oauth.Refresher // OAuth session manager (copied from global on init)
|
||||
database storage.HoldDIDLookup // Database for hold DID lookups (copied from global on init)
|
||||
authorizer auth.HoldAuthorizer // Hold authorization (copied from global on init)
|
||||
webhookDispatcher storage.PushWebhookDispatcher // Push webhook dispatcher (copied from global on init)
|
||||
manifestRefChecker storage.ManifestReferenceChecker // Manifest reference checker (copied from global on init)
|
||||
validationCache *validationCache // Request-level service token cache
|
||||
readmeFetcher *readme.Fetcher // README fetcher for repo pages
|
||||
userPrefs UserPrefsCache // Cached sailor profile preferences (copied from global on init)
|
||||
defaultHoldDID string // Default hold DID (e.g., "did:web:hold01.atcr.io")
|
||||
baseURL string // Base URL for error messages (e.g., "https://atcr.io")
|
||||
refresher *oauth.Refresher // OAuth session manager (copied from global on init)
|
||||
database storage.HoldDIDLookup // Database for hold DID lookups (copied from global on init)
|
||||
authorizer auth.HoldAuthorizer // Hold authorization (copied from global on init)
|
||||
webhookDispatcher storage.PushWebhookDispatcher // Push webhook dispatcher (copied from global on init)
|
||||
manifestRefChecker storage.ManifestReferenceChecker // Manifest reference checker (copied from global on init)
|
||||
validationCache *validationCache // Request-level service token cache
|
||||
readmeFetcher *readme.Fetcher // README fetcher for repo pages
|
||||
userPrefs UserPrefsCache // Cached sailor profile preferences (copied from global on init)
|
||||
}
|
||||
|
||||
// initATProtoResolver initializes the name resolution middleware
|
||||
@@ -288,18 +287,17 @@ func initATProtoResolver(ctx context.Context, ns distribution.Namespace, _ drive
|
||||
// Copy shared services from globals into the instance
|
||||
// This avoids accessing globals during request handling
|
||||
return &NamespaceResolver{
|
||||
Namespace: ns,
|
||||
defaultHoldDID: defaultHoldDID,
|
||||
baseURL: baseURL,
|
||||
fallbackUnreachable: atproto.TestModeBuild,
|
||||
refresher: globalRefresher,
|
||||
database: globalDatabase,
|
||||
authorizer: globalAuthorizer,
|
||||
webhookDispatcher: globalWebhookDispatcher,
|
||||
manifestRefChecker: globalManifestRefChecker,
|
||||
validationCache: newValidationCache(),
|
||||
readmeFetcher: readme.NewFetcher(),
|
||||
userPrefs: globalUserPrefs,
|
||||
Namespace: ns,
|
||||
defaultHoldDID: defaultHoldDID,
|
||||
baseURL: baseURL,
|
||||
refresher: globalRefresher,
|
||||
database: globalDatabase,
|
||||
authorizer: globalAuthorizer,
|
||||
webhookDispatcher: globalWebhookDispatcher,
|
||||
manifestRefChecker: globalManifestRefChecker,
|
||||
validationCache: newValidationCache(),
|
||||
readmeFetcher: readme.NewFetcher(),
|
||||
userPrefs: globalUserPrefs,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -708,7 +706,7 @@ func (nr *NamespaceResolver) findHoldDIDAndPrefs(ctx context.Context, did, handl
|
||||
// Both fields are known locally. The cached defaultHold was already
|
||||
// normalized to a DID by whoever wrote it, so no URL-to-DID
|
||||
// migration is needed on this path.
|
||||
return nr.applyTestModeFallback(ctx, prefs.DefaultHoldDID),
|
||||
return nr.holdOrDefault(prefs.DefaultHoldDID),
|
||||
holdPrefs{AutoRemoveUntagged: prefs.AutoRemoveUntagged.Bool}
|
||||
}
|
||||
}
|
||||
@@ -759,22 +757,15 @@ func (nr *NamespaceResolver) learnHoldPrefs(ctx context.Context, did, handle, pd
|
||||
}
|
||||
}
|
||||
|
||||
return nr.applyTestModeFallback(ctx, holdDID), holdPrefs{AutoRemoveUntagged: autoRemove}
|
||||
return nr.holdOrDefault(holdDID), holdPrefs{AutoRemoveUntagged: autoRemove}
|
||||
}
|
||||
|
||||
// applyTestModeFallback turns a user's chosen hold into the hold to actually
|
||||
// use. An empty choice means the appview default. With fallbackUnreachable set
|
||||
// (testmode builds) a chosen hold that is not answering also falls back, so a
|
||||
// developer whose local hold is down can still push.
|
||||
func (nr *NamespaceResolver) applyTestModeFallback(ctx context.Context, userHoldDID string) string {
|
||||
// holdOrDefault turns a user's chosen hold into the hold to actually use. An
|
||||
// empty choice means the appview default.
|
||||
func (nr *NamespaceResolver) holdOrDefault(userHoldDID string) string {
|
||||
if userHoldDID == "" {
|
||||
return nr.defaultHoldDID
|
||||
}
|
||||
if nr.fallbackUnreachable && !nr.isHoldReachable(ctx, userHoldDID) {
|
||||
slog.Debug("User's defaultHold unreachable, falling back to default",
|
||||
"component", "registry/middleware/testmode", "default_hold", userHoldDID)
|
||||
return nr.defaultHoldDID
|
||||
}
|
||||
return userHoldDID
|
||||
}
|
||||
|
||||
@@ -799,21 +790,6 @@ func (nr *NamespaceResolver) resolveSuccessor(ctx context.Context, holdDID strin
|
||||
return holdDID
|
||||
}
|
||||
|
||||
// isHoldReachable checks if a hold service is reachable
|
||||
// Used in test mode to fallback to default hold when user's hold is unavailable
|
||||
func (nr *NamespaceResolver) isHoldReachable(ctx context.Context, holdDID string) bool {
|
||||
holdURL, err := atproto.ResolveHoldURL(ctx, holdDID)
|
||||
if err != nil {
|
||||
slog.Debug("Cannot resolve hold URL for reachability check", "component", "registry/middleware", "holdDID", holdDID, "error", err)
|
||||
return false
|
||||
}
|
||||
|
||||
testURL := holdURL + "/.well-known/did.json"
|
||||
client := atproto.NewClient("", "", "")
|
||||
_, err = client.FetchDIDDocument(ctx, testURL)
|
||||
return err == nil
|
||||
}
|
||||
|
||||
// ExtractAuthMethod is an HTTP middleware that extracts the auth method and puller DID from the JWT Authorization header
|
||||
// and stores them in the request context for later use by the registry middleware.
|
||||
// Also stores the HTTP method for routing decisions (GET/HEAD = pull, PUT/POST = push).
|
||||
|
||||
@@ -109,7 +109,6 @@ func TestInitATProtoResolver(t *testing.T) {
|
||||
if baseURL, ok := tt.options["base_url"].(string); ok {
|
||||
assert.Equal(t, baseURL, resolver.baseURL)
|
||||
}
|
||||
assert.Equal(t, atproto.TestModeBuild, resolver.fallbackUnreachable)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -211,66 +210,6 @@ func TestFindHoldDID_Priority(t *testing.T) {
|
||||
assert.Equal(t, "did:web:profile.hold.io", holdDID, "should prioritize sailor profile over hold records")
|
||||
}
|
||||
|
||||
// TestFindHoldDID_TestModeFallback tests the testmode-build fallback when the hold is unreachable
|
||||
func TestFindHoldDID_TestModeFallback(t *testing.T) {
|
||||
// Start a mock PDS server that returns a profile with unreachable hold
|
||||
mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/xrpc/com.atproto.repo.getRecord" {
|
||||
// Return sailor profile with an unreachable hold
|
||||
profile := atproto.NewSailorProfileRecord("did:web:unreachable.hold.io")
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"value": profile,
|
||||
})
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer mockPDS.Close()
|
||||
|
||||
resolver := &NamespaceResolver{
|
||||
defaultHoldDID: "did:web:default.atcr.io",
|
||||
fallbackUnreachable: true, // what a testmode build sets
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
holdDID, _ := resolver.findHoldDIDAndPrefs(ctx, "did:plc:test123", "test.example.com", mockPDS.URL)
|
||||
|
||||
// In a testmode build with an unreachable hold, should fall back to default
|
||||
assert.Equal(t, "did:web:default.atcr.io", holdDID, "should fall back to default in a testmode build when hold unreachable")
|
||||
}
|
||||
|
||||
// TestIsHoldReachable tests the hold reachability check
|
||||
func TestIsHoldReachable(t *testing.T) {
|
||||
// Mock hold server with DID document
|
||||
mockHold := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/.well-known/did.json" {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"id": "did:web:reachable.hold.io",
|
||||
})
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer mockHold.Close()
|
||||
|
||||
resolver := &NamespaceResolver{}
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
t.Run("reachable hold", func(t *testing.T) {
|
||||
// Use URL format directly — DID resolution requires real identity directory
|
||||
reachable := resolver.isHoldReachable(ctx, mockHold.URL)
|
||||
assert.True(t, reachable, "should detect reachable hold")
|
||||
})
|
||||
|
||||
t.Run("unreachable hold", func(t *testing.T) {
|
||||
reachable := resolver.isHoldReachable(ctx, "did:web:nonexistent.example.com")
|
||||
assert.False(t, reachable, "should detect unreachable hold")
|
||||
})
|
||||
}
|
||||
|
||||
// TestRepositoryCaching tests that repositories are cached by DID+name
|
||||
func TestRepositoryCaching(t *testing.T) {
|
||||
// This test requires integration with actual repository resolution
|
||||
|
||||
Reference in New Issue
Block a user