Open one connection per database instead of one per query

CONTRIBUTING.md

@@ -217,7 +217,7 @@ When adding collectors:
 
 1. **Update unit tests** in `radar_test.go`
 2. **Update integration tests** if permission-dependent
-3. **Test all 4 permission scenarios** if applicable
+3. **Test all 6 integration scenarios** if applicable
 4. **Ensure silent handling** of missing tools/permissions
 
 ## Architecture
@@ -334,31 +334,41 @@ The project uses GitHub Actions for CI/CD. See [.github/workflows/ci.yml](.githu
 
 ### Integration Test Scenarios
 
-The integration test validates radar works correctly in all permission combinations:
+The integration test runs six scenarios: the four permission combinations, then certificate and GSSAPI authentication:
 
 **Scenario 1: Root + PostgreSQL superuser**
 - Full system access (all commands available to root)
 - Full PostgreSQL access (all views, config files)
-- Expected: ~66 system collectors, ~32 PostgreSQL collectors, pg_statviz data
+- Expected: ~71 system collectors, ~55 PostgreSQL collectors, pg_statviz data
 
 **Scenario 2: Root + PostgreSQL pg_monitor role**
 - Full system access (root privileges)
 - Monitoring-level PostgreSQL access (most views, limited config)
-- Expected: ~66 system collectors, ~29 PostgreSQL collectors, pg_statviz data
+- Expected: ~71 system collectors, ~51 PostgreSQL collectors, pg_statviz data
 - Note: Some collectors unavailable (e.g., pg_hba_file_rules, subscriptions)
 
 **Scenario 3: Non-root + PostgreSQL superuser**
 - Limited system access (some commands fail without root)
 - Full PostgreSQL access (all views, config files)
-- Expected: ~63 system collectors, ~32 PostgreSQL collectors, pg_statviz data
+- Expected: ~68 system collectors, ~51 PostgreSQL collectors, pg_statviz data
 - Note: Some system collectors unavailable (e.g., ifconfig, sysctl)
 
 **Scenario 4: Non-root + PostgreSQL pg_monitor role**
 - Limited system access (non-root user)
 - Monitoring-level PostgreSQL access (pg_monitor role)
-- Expected: ~63 system collectors, ~29 PostgreSQL collectors, pg_statviz data
+- Expected: ~68 system collectors, ~47 PostgreSQL collectors, pg_statviz data
 - Note: Combines limitations of both non-root and pg_monitor
 
+**Scenario 5: Certificate authentication**
+- Root, connecting as the pg_monitor role with a client certificate over TLS
+- Expected: ~71 system collectors, ~51 PostgreSQL collectors, pg_statviz data
+- Note: Exercises `-sslmode verify-full` with `-sslcert`, `-sslkey` and `-sslrootcert`
+
+**Scenario 6: GSSAPI/Kerberos authentication**
+- Root, connecting as the pg_monitor role with a Kerberos ticket from `kinit`
+- Expected: ~71 system collectors, ~51 PostgreSQL collectors, pg_statviz data
+- Note: `pg_hba.conf` requires `hostgssenc`, so the connection must be GSSAPI-encrypted
+
 All scenarios verify that radar handles permission limitations and collects maximum available data.
 
 ### Collector Availability Differences by Permission Scenario
@@ -460,7 +470,7 @@ docs: update README with new collectors
 - Code must be formatted (gofmt)
 - Linting must pass (golangci-lint)
 - Unit tests must pass
-- Integration tests must pass (all 4 scenarios)
+- Integration tests must pass (all 6 scenarios)
 - Documentation updated if applicable
 
 ## Getting Help

README.md

@@ -280,6 +280,7 @@ radar-hostname-20260115-133700.zip
 
 - Data streams directly to ZIP file without buffering in memory
 - Sequential execution with minimal memory footprint
+- One connection per database, reused across all of that database's queries
 - Complete collection typically takes seconds
 
 ## Author

docs/index.md

@@ -256,6 +256,7 @@ radar-hostname-20260115-133700.zip
 
 - Data streams directly to ZIP file without buffering in memory
 - Sequential execution with minimal memory footprint
+- One connection per database, reused across all of that database's queries
 - Complete collection typically takes seconds
 
 ## Author

postgres.go

@@ -11,6 +11,7 @@
 package main
 
 import (
+	"context"
 	"database/sql"
 	"errors"
 	"fmt"
@@ -111,24 +112,25 @@ func collectPGConfigFile(db *sql.DB, cfg *Config, filename string, w io.Writer)
 	return err
 }
 
-// generateDatabaseTasks creates per-database collection tasks
-func generateDatabaseTasks(db *sql.DB) ([]CollectionTask, error) {
+// generateDatabaseTasks creates per-database collection tasks, returning the
+// closer for the connection they share.
+func generateDatabaseTasks(db *sql.DB) ([]CollectionTask, io.Closer, error) {
 	if db == nil {
-		return nil, fmt.Errorf("PostgreSQL not initialized")
+		return nil, nil, fmt.Errorf("PostgreSQL not initialized")
 	}
 
 	// Get list of databases
 	rows, err := db.Query("SELECT datname FROM pg_database WHERE datallowconn ORDER BY datname")
 	if err != nil {
-		return nil, fmt.Errorf("querying databases: %w", err)
+		return nil, nil, fmt.Errorf("querying databases: %w", err)
 	}
 	defer closeErrCheck(rows, "database list query rows")
 
 	var databases []string
 	for rows.Next() {
 		var dbname string
 		if err := rows.Scan(&dbname); err != nil {
-			return nil, fmt.Errorf("scanning database name: %w", err)
+			return nil, nil, fmt.Errorf("scanning database name: %w", err)
 		}
 		// Always skip template0 and template1
 		if dbname == "template0" || dbname == "template1" {
@@ -138,11 +140,12 @@ func generateDatabaseTasks(db *sql.DB) ([]CollectionTask, error) {
 	}
 
 	if err := rows.Err(); err != nil {
-		return nil, fmt.Errorf("iterating databases: %w", err)
+		return nil, nil, fmt.Errorf("iterating databases: %w", err)
 	}
 
 	// Generate tasks for each database
 	var tasks []CollectionTask
+	conns := &dbConns{}
 	allDBTasks := make([]SimpleQueryTask, 0,
 		len(perDatabaseQueryTasks)+len(pgStatvizQueryTasks)+len(spockQueryTasks))
 	allDBTasks = append(allDBTasks, perDatabaseQueryTasks...)
@@ -162,22 +165,84 @@ func generateDatabaseTasks(db *sql.DB) ([]CollectionTask, error) {
 				Name:        fmt.Sprintf("%s/%s", dbName, td.Name),
 				ArchivePath: fmt.Sprintf(td.ArchivePath, dbName),
 				Collector: func(cfg *Config, w io.Writer) error {
-					return execPGQueryOnDB(dbName, cfg, td.Query, w)
+					return conns.exec(cfg, dbName, td.Query, w)
 				},
 			})
 		}
 	}
 
-	return tasks, nil
+	return tasks, conns, nil
 }
 
-// execPGQueryOnDB executes a query on a specific database
-func execPGQueryOnDB(dbname string, cfg *Config, query string, w io.Writer) error {
+// dbConns holds the connection the per-database tasks share. They are generated
+// and run grouped by database, so holding one connection and closing it when
+// collection moves on costs one connect and one authentication per database
+// rather than one per task.
+type dbConns struct {
+	db   *sql.DB
+	name string
+	err  error // why name could not be reached, if it could not
+}
+
+// conn returns the connection for dbname, establishing one on first use and
+// closing the connection to the database collection has left behind. A database
+// that cannot be reached is recorded, so its remaining tasks skip. The database
+// radar was invoked against is served by the connection opened at startup.
+func (c *dbConns) conn(cfg *Config, dbname string) (*sql.DB, error) {
+	if dbname == c.name {
+		if c.err != nil {
+			return nil, NewSkipError(c.err.Error())
+		}
+		return c.db, nil
+	}
+	// A different database, so the held connection is finished with.
+	closeErrCheck(c, "database connection")
+
+	if dbname == cfg.Database && cfg.DB != nil {
+		return cfg.DB, nil
+	}
+
 	db, err := sql.Open("pgx", cfg.ConnectionString(dbname))
 	if err != nil {
-		return fmt.Errorf("connecting to %s: %w", dbname, err)
+		return nil, fmt.Errorf("connecting to %s: %w", dbname, err)
+	}
+	// Collectors run one at a time, so one connection is all the pool needs.
+	db.SetMaxOpenConns(1)
+	c.name = dbname
+
+	// Connect here rather than leaving it to the first query. A database that
+	// allows connections but refuses this user then costs one rejected login
+	// instead of one per task, since c.err makes the rest of them skip.
+	pooled, err := db.Conn(context.Background())
+	if err != nil {
+		closeErrCheck(db, "database connection")
+		c.err = fmt.Errorf("connecting to %s: %w", dbname, err)
+		return nil, c.err
+	}
+	// Back to the pool, which the tasks then draw it from.
+	closeErrCheck(pooled, "pooled connection")
+
+	c.db = db
+	return db, nil
+}
+
+// Close closes the connection the per-database tasks were sharing.
+func (c *dbConns) Close() error {
+	c.name, c.err = "", nil
+	if c.db == nil {
+		return nil
+	}
+	db := c.db
+	c.db = nil
+	return db.Close()
+}
+
+// exec executes a query on a specific database
+func (c *dbConns) exec(cfg *Config, dbname, query string, w io.Writer) error {
+	db, err := c.conn(cfg, dbname)
+	if err != nil {
+		return err
 	}
-	defer closeErrCheck(db, "database connection")
 
 	rows, err := db.Query(query)
 	if err != nil {

postgres_test.go

@@ -13,9 +13,11 @@ package main
 import (
 	"bytes"
 	"errors"
+	"net"
 	"os"
 	"path/filepath"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -207,6 +209,126 @@ func TestPGQueryCollectorUnavailableAsSkip(t *testing.T) {
 	}
 }
 
+// TestDBConnsReusesOneConnectionPerDatabase pins the connection budget: a task
+// on the database being collected reuses the connection already established for
+// it, and the closer releases that connection at the end.
+func TestDBConnsReusesOneConnectionPerDatabase(t *testing.T) {
+	held, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("failed to create mock: %v", err)
+	}
+	mock.ExpectClose()
+
+	cfg := &Config{Host: "127.0.0.1", Port: 1, Database: "postgres",
+		Username: "radar", SSLMode: "disable"}
+	// Seeded as though collection is part way through mydb.
+	conns := &dbConns{db: held, name: "mydb"}
+
+	got, err := conns.conn(cfg, "mydb")
+	if err != nil {
+		t.Fatalf("conn(mydb): %v", err)
+	}
+	if got != held {
+		t.Error("a second task on the same database opened another connection")
+	}
+
+	if err := conns.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+	// database/sql reports "sql: database is closed" once the pool is closed.
+	if err := held.Ping(); err == nil || !strings.Contains(err.Error(), "closed") {
+		t.Errorf("the last connection was left open: Ping = %v", err)
+	}
+	// Collection closes whether or not a database was ever opened.
+	if err := conns.Close(); err != nil {
+		t.Errorf("Close with nothing held: %v", err)
+	}
+}
+
+// TestDBConnsClosesWhenCollectionReachesTheStartupDatabase covers the ordering
+// where the database radar was invoked against is collected after another one.
+// The startup connection serves it, and the connection to the database just
+// finished must not be left open while it does.
+func TestDBConnsClosesWhenCollectionReachesTheStartupDatabase(t *testing.T) {
+	initial, initialMock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("failed to create mock: %v", err)
+	}
+	initialMock.ExpectClose()
+	defer closeErrCheck(initial, "mock database")
+
+	held, heldMock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("failed to create mock: %v", err)
+	}
+	heldMock.ExpectClose()
+
+	cfg := &Config{Host: "127.0.0.1", Port: 1, Database: "postgres",
+		Username: "radar", SSLMode: "disable", DB: initial}
+	conns := &dbConns{db: held, name: "mydb"}
+
+	got, err := conns.conn(cfg, "postgres")
+	if err != nil {
+		t.Fatalf("conn(postgres): %v", err)
+	}
+	if got != initial {
+		t.Error("the startup database opened a second connection")
+	}
+	if err := held.Ping(); err == nil || !strings.Contains(err.Error(), "closed") {
+		t.Errorf("connection to the finished database left open: Ping = %v", err)
+	}
+}
+
+// TestDBConnsAttemptsAnUnreachableDatabaseOnce verifies that a database which
+// refuses this user costs one connection attempt rather than one per task: the
+// first task reports the failure, and the rest skip without connecting again.
+func TestDBConnsAttemptsAnUnreachableDatabaseOnce(t *testing.T) {
+	// Accepts each connection and closes it, so the startup handshake fails the
+	// way a rejected login does, and counts what radar opened.
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer closeErrCheck(listener, "probe listener")
+
+	var attempts atomic.Int64
+	go func() {
+		for {
+			conn, err := listener.Accept()
+			if err != nil {
+				return
+			}
+			attempts.Add(1)
+			closeErrCheck(conn, "probe connection")
+		}
+	}()
+
+	cfg := &Config{Host: "127.0.0.1", Port: listener.Addr().(*net.TCPAddr).Port,
+		Database: "postgres", Username: "radar", SSLMode: "disable"}
+	conns := &dbConns{}
+	defer closeErrCheck(conns, "database connection")
+
+	var buf bytes.Buffer
+	var skipErr SkipError
+	if err := conns.exec(cfg, "mydb", "SELECT 1", &buf); err == nil || errors.As(err, &skipErr) {
+		t.Fatalf("first task = %v, want the failure reported", err)
+	}
+	// What the first task costs is not fixed, since database/sql retries a
+	// connection it finds broken. What matters is that the second task adds
+	// nothing.
+	attempted := attempts.Load()
+	if attempted == 0 {
+		t.Fatal("the first task never connected")
+	}
+
+	if err := conns.exec(cfg, "mydb", "SELECT 1", &buf); !errors.As(err, &skipErr) {
+		t.Errorf("second task = %v, want SkipError", err)
+	}
+	if got := attempts.Load(); got != attempted {
+		t.Errorf("second task connected again: %d attempts, want %d", got, attempted)
+	}
+}
+
 // writeFixture creates a file holding contents, failing the test if it cannot.
 func writeFixture(t *testing.T, path, contents string) {
 	t.Helper()

radar.go

@@ -543,10 +543,11 @@ func collectAll(cfg *Config, zipWriter *zip.Writer) int {
 	if !cfg.SkipPostgres {
 		// Pass cfg.DB to PostgreSQL task generators
 		pgTasks = append(pgTasks, getPostgreSQLTasks(cfg.DB)...)
-		dbTasks, err := generateDatabaseTasks(cfg.DB)
+		dbTasks, conns, err := generateDatabaseTasks(cfg.DB)
 		if err != nil {
 			errorLog.Printf("Failed to generate database tasks: %v", err)
 		} else {
+			defer closeErrCheck(conns, "database connection")
 			pgTasks = append(pgTasks, dbTasks...)
 		}
 	}

radar_test.go

@@ -821,10 +821,11 @@ func TestGenerateDatabaseTasksRegistersAllRegistries(t *testing.T) {
 		WillReturnRows(sqlmock.NewRows([]string{"datname"}).
 			AddRow("mydb").AddRow("template0").AddRow("template1"))
 
-	tasks, err := generateDatabaseTasks(db)
+	tasks, conns, err := generateDatabaseTasks(db)
 	if err != nil {
 		t.Fatalf("generateDatabaseTasks: %v", err)
 	}
+	defer closeErrCheck(conns, "database connection")
 
 	paths := archivePathsByName(t, tasks)