feat(store): add Postgres backend for the hub store

[lerms]

Summary

Enables database.driver: postgres so the hub can run as a stateless control plane against an external Postgres, configured entirely via GitOps (SCION_SERVER_DATABASE_URL) instead of a node-local SQLite file + PVC.

This is the missing half of Postgres support. The ent layer already spoke Postgres (entc.OpenPostgres + lib/pq); this adds the hand-written store's Postgres twin and wires initStore's case "postgres": to compose them via entadapter.NewCompositeStore — exactly as the sqlite branch does.

graph TB
  subgraph cmd["cmd/server_foreground.go · initStore()"]
    D{"cfg.Database.Driver"}
  end
  D -->|"sqlite"| SQ["sqlite.New → Migrate"]
  D -->|"postgres ✨NEW"| PG["postgres.New → Migrate"]
  D -->|other| ERR["unsupported driver"]

  SQ --> ES["entc.OpenSQLite + AutoMigrate"]
  PG --> EP["entc.OpenPostgres + AutoMigrate<br/>(already existed)"]

  ES --> CS["entadapter.NewCompositeStore"]
  EP --> CS
  CS --> IF[("store.Store · 206 methods<br/>23 sub-interfaces")]

  subgraph trans["Dialect translation (sqlite → postgres)"]
    T1["? → $N placeholders"]
    T2["INSERT OR IGNORE/REPLACE → ON CONFLICT"]
    T3["sqlite_master / PRAGMA → information_schema"]
    T4["randomblob() → gen_random_uuid()"]
    T5["json_each → json_array_elements_text"]
    T6["COLLATE NOCASE → LOWER() unique idx · BLOB → BYTEA"]
  end
  PG -.->|"53 migrations · 206 methods"| trans

  classDef new fill:#1f6feb,color:#fff
  class PG,EP new

Loading

What's included

• pkg/store/postgres/ — a faithful, per-entity mirror of pkg/store/sqlite/. Same file layout, same method names/signatures/comments/control flow (206 methods in each package). The only differences are dialect mechanics:
  • ? → $N numbered placeholders
  • INSERT OR IGNORE/REPLACE → ON CONFLICT DO NOTHING / DO UPDATE
  • sqlite_master / PRAGMA table_info introspection → information_schema
  • randomblob() UUID seeds → gen_random_uuid()
  • json_each() → json_array_elements_text(); json_array() → json_build_array()
  • COLLATE NOCASE → LOWER(email) functional unique index
  • BLOB → BYTEA; datetime('now') → NOW(); INSTR/SUBSTR → split_part
• cmd/server_foreground.go — adds case "postgres": to initStore. The grove→project backfill (MigrateGroveToProjectData) is a SQLite-era data repair and is intentionally skipped on the Postgres path (a fresh Postgres DB has no legacy grove rows).
• Build tags — the lib/pq driver import is gated behind a no_postgres build tag, mirroring the existing no_sqlite tag, so slim images can exclude either driver.

Testing

Env-gated integration tests (SCION_TEST_POSTGRES_URL) that skip when the variable is unset, so the CI no_sqlite path (make test-fast, no DB) stays green.

Verified locally against PostgreSQL 16:

Gate	Result
go build ./... (default)	✅
go build -tags no_sqlite ./...	✅
go build -tags no_postgres ./...	✅
go build -tags 'no_sqlite no_postgres' ./...	✅
make lint (go vet -tags no_sqlite)	✅
make test-fast (no DB)	✅ tests skip, not fail
Live Postgres: 53/53 migrations + 16 CRUD tests	✅ 31 tables created

CRUD round-trips cover users, projects, agents, secrets (incl. upsert), groups + membership, policies, invite codes and env vars.

Notes for reviewers

• The mirror is deliberately not refactored into a shared dialect abstraction — it stays line-for-line comparable with pkg/store/sqlite/ so the diff between the two is purely the dialect changes listed above, which keeps future schema/method changes easy to apply to both.
• applyMigrationWithFKOff is preserved (with the foreignKeysOffMigrations map) so the migration-runner shape matches SQLite, but in Postgres it runs a plain transaction — the one table-recreate migration (V40) is made FK-safe via DROP TABLE ... CASCADE rather than toggling PRAGMA foreign_keys.

🤖 Generated with Claude Code

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive PostgreSQL store implementation parallel to the existing SQLite backend, enabling stateless control-plane deployments. While the implementation is thorough, several critical issues and optimization opportunities were identified. Specifically, migrations V31 and V40 pose schema corruption risks by using CASCADE drops that silently destroy foreign keys; these should be replaced with direct ALTER TABLE statements. Additionally, concurrent migrations in multi-replica environments could trigger race conditions, which can be mitigated using Postgres advisory locks. Keyset pagination is currently broken or ignored in several listing methods (ListAgents, ListMessages, and ListScheduledEvents), and potential query crashes exist when parsing empty or null ancestry fields. Finally, performance can be significantly improved by replacing recursive Go-based database queries with recursive CTEs for group operations, utilizing JSONB containment operators for repository lookups, and increasing the low default connection pool limit.

[gemini-code-assist]

Recreating the groves table and dropping the old one with CASCADE is extremely dangerous in Postgres. The CASCADE option silently drops all foreign key constraints in other tables (such as agents, templates, schedules, etc.) that reference groves(id). These foreign keys are never recreated, leading to database schema corruption. Since Postgres natively supports dropping and adding constraints, we should use ALTER TABLE directly instead of recreating the table.

const migrationV40 = "ALTER TABLE groves DROP CONSTRAINT IF EXISTS groves_git_remote_key;\nALTER TABLE groves ADD CONSTRAINT groves_slug_key UNIQUE (slug);"

[lerms]

✅ Fixed in 3158517. You're right — this is genuine schema corruption in Postgres. Replaced the DROP TABLE ... CASCADE + recreate with in-place ALTER TABLE ... DROP/ADD CONSTRAINT (the SQLite source only recreates because SQLite cannot alter constraints; Postgres can).

Verified against live Postgres 16 after the fix: all 7 FKs referencing the table survive (agents, project_contributors, project_sync_state, scheduled_events, schedules, templates, user_access_tokens), *_slug_key UNIQUE is present, and the git_remote UNIQUE is gone. (Constraint names keep the groves_ prefix because the later V50 table rename doesn't rename constraints — identical to the SQLite backend, harmless.)

[gemini-code-assist]

Dropping the notification_subscriptions table with CASCADE silently destroys the foreign key constraint in the notifications table (notifications.subscription_id references notification_subscriptions(id)). This constraint is never recreated, leading to schema corruption. Since Postgres natively supports adding columns and dropping NOT NULL constraints, we should use ALTER TABLE directly instead of recreating the table.

const migrationV31 = "ALTER TABLE notification_subscriptions ADD COLUMN scope TEXT NOT NULL DEFAULT 'agent';\nALTER TABLE notification_subscriptions ALTER COLUMN agent_id DROP NOT NULL;\nCREATE UNIQUE INDEX IF NOT EXISTS idx_notification_subs_unique ON notification_subscriptions(scope, COALESCE(agent_id, ''), subscriber_type, subscriber_id, grove_id);"

[lerms]

✅ Fixed in 3158517. Same CASCADE corruption — the recreate silently dropped notifications.subscription_id → notification_subscriptions(id). Replaced with in-place ADD COLUMN ... scope + ALTER COLUMN agent_id DROP NOT NULL, keeping the existing dedup unique index. Verified on live Postgres: scope (NOT NULL) and nullable agent_id are present and the dependent FK survives.

[gemini-code-assist]

In a stateless cloud topology where multiple replicas of the hub start up concurrently, they will all attempt to run database migrations at the same time. This can lead to race conditions, duplicate key errors, or table creation failures. Using a session-level Postgres advisory lock (pg_advisory_lock) at the start of the migration process ensures that only one replica runs the migrations at a time, while others wait safely.

func (s *PostgresStore) Migrate(ctx context.Context) error {
	// Acquire a session-level advisory lock to prevent concurrent migrations
	// when scaling horizontally. 123456 is an arbitrary lock ID.
	if _, err := s.db.ExecContext(ctx, "SELECT pg_advisory_lock(123456)"); err != nil {
		return fmt.Errorf("failed to acquire advisory lock: %w", err)
	}
	defer s.db.ExecContext(ctx, "SELECT pg_advisory_unlock(123456)")

[lerms]

✅ Fixed in 3158517. Added a database-global pg_advisory_lock around Migrate() so concurrent hub replicas serialize migrations — exactly the stateless scale-out this backend exists for.

One correctness refinement over the snippet: a session-level advisory lock's lock and unlock must run on the same session, but with a connection pool they can land on different pooled connections. So I pin the lock to a dedicated *sql.Conn (db.Conn(ctx) … defer lockConn.Close()); the migrations still run via the pool, and since the lock is database-global it blocks other replicas regardless. Verified no advisory locks leak after Migrate() returns.

[gemini-code-assist]

The keyset pagination logic here is broken. The query sorts by created_at DESC but filters by id < opts.Cursor. Since id is a UUID, comparing it has no relation to the created_at order, resulting in scrambled and incorrect pagination. We should use a proper keyset pagination condition comparing both created_at and id using subqueries. Additionally, appending this condition directly to conditions simplifies the code and avoids duplicating the entire query.

	if opts.Cursor != "" {
		conditions = append(conditions, fmt.Sprintf("(created_at < (SELECT created_at FROM scheduled_events WHERE id = $%d) OR (created_at = (SELECT created_at FROM scheduled_events WHERE id = $%d) AND id < $%d))", len(args)+1, len(args)+2, len(args)+3))
		args = append(args, opts.Cursor, opts.Cursor, opts.Cursor)
	}

	whereClause := ""
	if len(conditions) > 0 {
		whereClause = "WHERE " + strings.Join(conditions, " AND ")
	}

	// Get total count
	var totalCount int
	countQuery := fmt.Sprintf("SELECT COUNT(*) FROM scheduled_events %s", whereClause)
	if err := s.db.QueryRowContext(ctx, countQuery, args...).Scan(&totalCount); err != nil {
		return nil, err
	}

	// Apply pagination
	limit := opts.Limit
	if limit <= 0 {
		limit = 50
	}
	if limit > 200 {
		limit = 200
	}

	query := fmt.Sprintf("SELECT id, project_id, event_type, fire_at, payload, status, created_at, created_by, fired_at, error, schedule_id FROM scheduled_events %s ORDER BY created_at DESC LIMIT $%d", whereClause, len(args)+1)

	queryArgs := append(args, limit+1)

[lerms]

⏸️ Won't fix in this PR — faithful-mirror scope. This one does consume opts.Cursor, but the id < cursor + ORDER BY created_at DESC shape is copied verbatim from the SQLite source (ListScheduledEvents uses the identical AND id < ? / ORDER BY created_at DESC). The mixed sort-vs-filter-key critique is valid, but it applies equally to upstream SQLite — fixing it here only would make the two backends diverge. Better as a cross-backend follow-up.

[gemini-code-assist]

The ListAgents method sets NextCursor in the return value but completely ignores opts.Cursor in the SQL query. This breaks pagination entirely, causing the UI to reload the first page infinitely when clicking 'Next Page'. We should add the keyset pagination condition to conditions using the created_at and id fields.

Suggested change

	// Exclude soft-deleted agents unless explicitly requested
	if !filter.IncludeDeleted {
	conditions = append(conditions, "deleted_at IS NULL")
	}
	whereClause := ""
	if opts.Cursor != "" {
	conditions = append(conditions, fmt.Sprintf("(created_at < (SELECT created_at FROM agents WHERE id = $%d) OR (created_at = (SELECT created_at FROM agents WHERE id = $%d) AND id < $%d))", len(args)+1, len(args)+2, len(args)+3))
	args = append(args, opts.Cursor, opts.Cursor, opts.Cursor)
	}
	// Exclude soft-deleted agents unless explicitly requested
	if !filter.IncludeDeleted {
	conditions = append(conditions, "deleted_at IS NULL")
	}
	whereClause := ""

[lerms]

⏸️ Won't fix in this PR — faithful-mirror scope. Confirmed the SQLite source (ListAgents) has the identical behavior: it sets result.NextCursor = agents[limit-1].ID and never consumes opts.Cursor either. So this isn't a regression introduced by the Postgres backend — it's pre-existing upstream behavior that this PR faithfully mirrors (the stated mandate: 1-to-1 with pkg/store/sqlite, no behavioral drift).

Fixing keyset pagination is a legitimate bug, but it should land in a dedicated change that fixes both backends together so they don't diverge. Happy to file/track that as a follow-up.

[gemini-code-assist]

Fetching transitive group memberships recursively in Go code results in O(N) database roundtrips, which is highly inefficient and slow. We can optimize this to a single O(1) database query using a Recursive Common Table Expression (CTE), which is fully supported by Postgres.

// GetEffectiveGroups returns all groups a user belongs to, including transitive memberships.
func (s *PostgresStore) GetEffectiveGroups(ctx context.Context, userID string) ([]string, error) {
	rows, err := s.db.QueryContext(ctx, "WITH RECURSIVE user_groups AS (SELECT group_id FROM group_members WHERE member_type = 'user' AND member_id = $1 UNION SELECT gm.group_id FROM group_members gm INNER JOIN user_groups ug ON gm.member_type = 'group' AND gm.member_id = ug.group_id) SELECT group_id FROM user_groups", userID)
	if err != nil {
		return nil, err
	}
	defer rows.Close()

	var groupIDs []string
	for rows.Next() {
		var id string
		if err := rows.Scan(&id); err != nil {
			return nil, err
		}
		groupIDs = append(groupIDs, id)
	}
	if err := rows.Err(); err != nil {
		return nil, err
	}
	return groupIDs, nil
}

[lerms]

⏸️ Won't fix in this PR — faithful-mirror scope. The SQLite source resolves transitive memberships with Go-side iteration too (for _, m := range directMemberships … for groupID := range effectiveGroups), so this mirrors upstream rather than introducing the N-roundtrip pattern. A recursive CTE is a real improvement, but as a Postgres-only change it would diverge the backends. Good candidate for a cross-backend optimization follow-up.

[gemini-code-assist]

Checking for cycles recursively in Go code results in O(N) database roundtrips, which is highly inefficient. We can optimize this to a single O(1) database query using a Recursive Common Table Expression (CTE), which is fully supported by Postgres.

Suggested change

	// WouldCreateCycle checks if adding memberGroupID as a member of groupID would create a cycle.
	// A cycle exists if groupID is reachable from memberGroupID by following the containment relationship.
	// Example: if A contains B, and we try to add A as member of B, we'd have A->B->A (cycle).
	func (s *PostgresStore) WouldCreateCycle(ctx context.Context, groupID, memberGroupID string) (bool, error) {
	// If they're the same, it's a direct cycle
	if groupID == memberGroupID {
	return true, nil
	}
	// Check if groupID is reachable from memberGroupID by traversing DOWN the containment graph
	// (i.e., checking what groups memberGroupID contains, and what those contain, etc.)
	visited := make(map[string]bool)
	return s.hasPathDown(ctx, memberGroupID, groupID, visited)
	}
	// hasPathDown checks if 'target' is reachable from 'current' by following containment.
	// It looks at what groups 'current' contains as members.
	func (s *PostgresStore) hasPathDown(ctx context.Context, current, target string, visited map[string]bool) (bool, error) {
	if current == target {
	return true, nil
	}
	if visited[current] {
	return false, nil
	}
	visited[current] = true
	// Get all groups that 'current' contains (groups where current is the group_id)
	rows, err := s.db.QueryContext(ctx,
	"SELECT member_id FROM group_members WHERE member_type = 'group' AND group_id = $1", current)
	if err != nil {
	return false, err
	}
	defer rows.Close()
	for rows.Next() {
	var childGroupID string
	if err := rows.Scan(&childGroupID); err != nil {
	return false, err
	}
	found, err := s.hasPathDown(ctx, childGroupID, target, visited)
	if err != nil {
	return false, err
	}
	if found {
	return true, nil
	}
	}
	return false, nil
	}
	// WouldCreateCycle checks if adding memberGroupID as a member of groupID would create a cycle.
	// A cycle exists if groupID is reachable from memberGroupID by following the containment relationship.
	// Example: if A contains B, and we try to add A as member of B, we'd have A->B->A (cycle).
	func (s *PostgresStore) WouldCreateCycle(ctx context.Context, groupID, memberGroupID string) (bool, error) {
	if groupID == memberGroupID {
	return true, nil
	}
	var exists bool
	err := s.db.QueryRowContext(ctx, "WITH RECURSIVE containment AS (SELECT member_id FROM group_members WHERE member_type = 'group' AND group_id = $1 UNION SELECT gm.member_id FROM group_members gm INNER JOIN containment c ON gm.member_type = 'group' AND gm.group_id = c.member_id) SELECT EXISTS (SELECT 1 FROM containment WHERE member_id = $2)", memberGroupID, groupID).Scan(&exists)
	if err != nil {
	return false, err
	}
	return exists, nil
	}

[lerms]

⏸️ Won't fix in this PR — faithful-mirror scope. Same as GetEffectiveGroups: the SQLite source does the cycle check via Go recursion, and this package mirrors it 1-to-1. Recursive-CTE optimization is worth doing across both backends together in a follow-up.

[gemini-code-assist]

Using time.Now() in Go passes the application server's local time to the database, which can lead to issues due to clock skew or timezone mismatches. It is more robust and consistent to use the database's native NOW() function directly in the SQL query.

// CleanExpiredJoinTokens removes all expired join tokens.
func (s *PostgresStore) CleanExpiredJoinTokens(ctx context.Context) error {
	_, err := s.db.ExecContext(ctx, "DELETE FROM broker_join_tokens WHERE expires_at < NOW()")
	return err
}

[lerms]

⏸️ Won't fix in this PR — faithful-mirror scope. The SQLite source passes time.Now() as a bind parameter here too, so this mirrors upstream. Switching to the DB's NOW() is a fair robustness point (clock-skew), but it should change both backends together. Noting as a follow-up.

[gemini-code-assist]

Fetching all active GitHub installations into Go memory and filtering them in Go is highly inefficient and doesn't scale. We can delegate this filtering to the database using Postgres's native JSONB containment operator (@>) to find the matching installation in a single query.

Suggested change

	func (s *PostgresStore) GetInstallationForRepository(ctx context.Context, repoFullName string) (*store.GitHubInstallation, error) {
	// Search active installations whose repositories JSON array contains the repo.
	installations, err := s.ListGitHubInstallations(ctx, store.GitHubInstallationFilter{
	Status: store.GitHubInstallationStatusActive,
	})
	if err != nil {
	return nil, err
	}
	for i := range installations {
	for _, repo := range installations[i].Repositories {
	if repo == repoFullName {
	return &installations[i], nil
	}
	}
	}
	return nil, store.ErrNotFound
	}
	func (s *PostgresStore) GetInstallationForRepository(ctx context.Context, repoFullName string) (*store.GitHubInstallation, error) {
	var inst store.GitHubInstallation
	var repos string
	err := s.db.QueryRowContext(ctx, "SELECT installation_id, account_login, account_type, app_id, repositories, status, created_at, updated_at FROM github_installations WHERE status = $1 AND repositories::jsonb @> jsonb_build_array($2) LIMIT 1", store.GitHubInstallationStatusActive, repoFullName).Scan(
	&inst.InstallationID, &inst.AccountLogin, &inst.AccountType,
	&inst.AppID, &repos, &inst.Status, &inst.CreatedAt, &inst.UpdatedAt,
	)
	if err != nil {
	if errors.Is(err, sql.ErrNoRows) {
	return nil, store.ErrNotFound
	}
	return nil, err
	}
	unmarshalJSON(repos, &inst.Repositories)
	return &inst, nil
	}

[lerms]

⏸️ Won't fix in this PR — faithful-mirror scope. The SQLite source also fetches active installations and filters the repositories JSON array in Go (for i := range installations / for _, repo := range ...). Pushing the filter into a jsonb @> query is a nice optimization but only available on the Postgres side, so doing it here alone diverges the backends. Tracking as a follow-up.

[gemini-code-assist]

A connection pool limit of 4 is extremely low for a concurrent production Postgres database. Under concurrent load, this will quickly lead to connection starvation and high latency. We should increase the default to a more reasonable value (e.g., 25) or make it configurable.

Suggested change

	db.SetMaxOpenConns(4)
	db.SetMaxIdleConns(4)
	db.SetMaxOpenConns(25)
	db.SetMaxIdleConns(25)

[lerms]

✅ Fixed in 3158517 — bumped to SetMaxOpenConns(25) / SetMaxIdleConns(25). (The 4 was a literal carryover from the SQLite package, where it's a single-writer concession; it makes no sense for Postgres. Added a comment explaining the divergence.)

[lerms]

Response to Gemini review

Thanks for the thorough pass. I triaged all 12 comments against the SQLite source, because this PR's explicit mandate is a faithful 1-to-1 mirror of pkg/store/sqlite — so the test for "fix here" is whether an issue is Postgres-specific (must fix) vs. inherited verbatim from the SQLite backend (fixing Postgres-only would make the two backends diverge, which is the one thing this PR must not do).

✅ Fixed (5) — genuinely Postgres-specific

#	Issue	Fix
🔴 V40	DROP TABLE … CASCADE + recreate silently drops every FK referencing the table	in-place ALTER TABLE DROP/ADD CONSTRAINT
🔴 V31	same CASCADE corruption (notifications.subscription_id FK)	in-place ADD COLUMN + ALTER COLUMN DROP NOT NULL
🟠 ancestry	''::json crashes the query (SQLite's json_each tolerated it)	COALESCE(NULLIF(ancestry,''),'[]') guard
🟠 advisory lock	concurrent replicas race on migrations	pg_advisory_lock pinned to a dedicated conn
🟡 pool size	4 is a SQLite single-writer carryover	bumped to 25

All verified against live Postgres 16: 53/53 migrations apply, all 7 FKs to the ex-groves table + the notification_subscriptions FK survive, ancestry edge cases pass, no advisory locks leak, full CRUD suite green.

⏸️ Deferred (7) — faithful reproductions of the SQLite source

These are all valid observations, but the behavior is identical in pkg/store/sqlite (verified each), so they're pre-existing upstream characteristics this PR mirrors rather than regressions it introduces:

• ListAgents / ListMessages ignore opts.Cursor — SQLite source does too (only sets NextCursor)
• ListScheduledEvents keyset (id < cursor + created_at DESC) — copied verbatim from SQLite
• GetEffectiveGroups / WouldCreateCycle Go-recursion — SQLite uses Go recursion, not CTEs
• GetInstallationForRepository Go-filter — SQLite filters in Go
• CleanExpiredJoinTokens time.Now() — SQLite passes time.Now()

Fixing these (keyset pagination, recursive CTEs, jsonb @>, DB-side NOW()) is worthwhile, but each should land in a change that touches both backends together to keep them in lockstep. Happy to file a tracking issue for the cross-backend optimization pass if useful.

[ptone]

See the broad outline of a plan in #53 , more work on this is starting this week