fix(handlers): defer router init until metadata DB resolves prefer-fallback

Hayley Swimelarrequested to merge
fix-prefer-fallback-router into master

Why

When GitLab 19.0 sets database.enabled = "prefer" on an upgraded install, the registry binary correctly falls back to legacy filesystem metadata ("database prefer mode enabled, but found filesystem metadata: falling back to legacy metadata"). But every /gitlab/v1/... route still breaks: requests return 500 (nil app.db panic), 405, or 200 from apiBase. The Rails-side container registry UI fails as a result.

The cause: initRouter ran at app.go:202, before initializeMetadataDatabase at line 412. The two IsEnabled() captures inside initRouter (at lines 1300 and 1318) saw a stale true after fallback flipped config.Database.PreferFallback = true. The dbAssertionhandler.wrap 404 shortcut at lines 1352-1356 never engaged.

The user workaround was registry['database']['enabled'] = "false" in gitlab.rb, which made IsEnabled() return false at router-build time and let the wrap return 404.

Fixes issue 600955.

What

One statement moved. app.initRouter() now runs immediately after initializeMetadataDatabase. The body of initRouter reads only app.Config at construction time and binds methods that read backing state per-request, so moving the call down is safe.

No changes to initRouter, dbAssertionhandler, recordLSNMiddleware, or initializeMetadataDatabase themselves. No nil-guards.

Test plan

New test TestNewApp_PreferFallback_V1RoutesReturn404 boots a prefer-fallback app (happy-path fixture, FS-locked, prefer mode) and walks all eleven /gitlab/v1/... routes. Each row asserts 404 and a non-empty Gitlab-Container-Registry-Version header.

Pre-fix: all eleven rows fail (a mix of 500, EOF from nil-deref panics, 405, and 200 from apiBase). Post-fix: all eleven return 404.

Existing TestInitializeMetadataDatabase_FSLockedPreferMode continues to pass.

Backport

This needs a v4.40.x patch for GitLab 19.0.1. Both commits cherry-pick cleanly onto v4.40.0-gitlab. Zero conflicting commits to app.go, app_integration_test.go, or integration_helpers_test.go between v4.40.0-gitlab and the branch base, verified 2026-05-22.

Context for LLM agents

Rationale

  • Per-request IsEnabled() evaluation inside dbAssertionhandler.wrap. Rejected because a Config read on every v1 request widens where IsEnabled() semantics can drift, and a separate nil-guard inside recordLSNMiddleware.body would still be needed.
  • Hybrid: reorder for dbAssertionhandler, nil-guard recordLSNMiddleware. Rejected because it leaves a "middleware registered but harmless" invariant future readers have to verify against a runtime branch instead of the structure of NewApp.
  • Chosen: reorder one statement. Ordering becomes a structural property of NewApp. The comment at the new call site and the regression test pin it.

Consequences: ordering remains implicit in line position, mitigated by the comment and the test.

Non-goals

  • Refactoring NewApp toward a builder pattern. One dependency tightened, not the larger init shape.
  • Per-route 404 differentiation. The wrap returns the same 404 for all eleven v1 routes, matching the working workaround behavior.
  • LSN middleware behavior under load-balancing plus fallback. Both gates read IsEnabled() at the same point, so the LSN gate short-circuits on the first conjunct once dbAssertionhandler returns 404.

Merge request reports