From fa35bfc10626669f6b401e821a8d0abd7535ee30 Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Sat, 1 Aug 2026 20:05:12 +0000 Subject: [PATCH] ship(P02): doctor network + db merged into v0.3 milestone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ---ci--- project: orca phase: 2 milestone: v0.3 status: complete requirements: covered: [REQ-032] partial: [] ---/ci--- P02: orca doctor network + db full implementation. - DB(): PRAGMA integrity_check + MigrationVersion (PASS/WARN/FAIL) - Network(): mTLS /healthz probe per peer, 3s timeout, zero peers → WARN - certpaths.DBPath() relocation (D-039, breaks import cycle) - store.MigrationVersion() public API - Deleted NetworkStub/DBStub (D-040) - 7 doctor tests + 1 MigrationVersion test, all pass under -race - 4-layer verification passed --- .ciagent/CHECKPOINT.json | 4 +- internal/certpaths/certpaths.go | 10 ++ internal/cli/doctor.go | 4 +- internal/cli/node.go | 11 +- internal/doctor/doctor.go | 129 +++++++++++++-- internal/doctor/doctor_test.go | 227 ++++++++++++++++++++++----- internal/store/job_task_repo_test.go | 2 +- internal/store/migrate.go | 15 ++ internal/store/migrate_test.go | 37 +++++ internal/store/node_repo_test.go | 2 +- 10 files changed, 375 insertions(+), 66 deletions(-) create mode 100644 internal/store/migrate_test.go diff --git a/.ciagent/CHECKPOINT.json b/.ciagent/CHECKPOINT.json index e6ec172..8d59077 100644 --- a/.ciagent/CHECKPOINT.json +++ b/.ciagent/CHECKPOINT.json @@ -1,9 +1,9 @@ { - "phase": 1, + "phase": 2, "stage": "verify", "milestone": "v0.3", "milestone_slug": "scheduling-streaming", "phase_role": "execution", "attempts": 0, - "updated_at": "2026-08-01T00:10:00Z" + "updated_at": "2026-08-01T00:20:00Z" } \ No newline at end of file diff --git a/internal/certpaths/certpaths.go b/internal/certpaths/certpaths.go index 9feeb78..f2e9e50 100644 --- a/internal/certpaths/certpaths.go +++ b/internal/certpaths/certpaths.go @@ -36,3 +36,13 @@ func ServerCertPath() string { return filepath.Join(Dir(), "server.crt") } // ServerKeyPath returns the path to server.key. func ServerKeyPath() string { return filepath.Join(Dir(), "server.key") } + +// DBPath returns the path to the orca SQLite database. Honors $ORCA_DB +// for testability and explicit override; otherwise defaults to +// ~/.orca/orca.db under the same Dir() as the cert files. +func DBPath() string { + if p := os.Getenv("ORCA_DB"); p != "" { + return p + } + return filepath.Join(Dir(), "orca.db") +} diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index c07112a..15f0e39 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -51,7 +51,7 @@ var doctorNetworkCmd = &cobra.Command{ Use: "network", Short: "Run the network self-check (P02 impl)", RunE: func(cmd *cobra.Command, args []string) error { - c := doctor.NetworkStub() + c := doctor.Network() r, msg := c.Run(cmd.Context()) fmt.Fprintf(cmd.OutOrStdout(), "%-20s %-5s %s\n", c.Name, r, msg) return nil @@ -62,7 +62,7 @@ var doctorDBCmd = &cobra.Command{ Use: "db", Short: "Run the database self-check (P02 impl)", RunE: func(cmd *cobra.Command, args []string) error { - c := doctor.DBStub() + c := doctor.DB() r, msg := c.Run(cmd.Context()) fmt.Fprintf(cmd.OutOrStdout(), "%-20s %-5s %s\n", c.Name, r, msg) return nil diff --git a/internal/cli/node.go b/internal/cli/node.go index 9b9123a..001a06b 100644 --- a/internal/cli/node.go +++ b/internal/cli/node.go @@ -8,7 +8,6 @@ import ( "log/slog" "os" "os/signal" - "path/filepath" "syscall" "time" @@ -22,16 +21,8 @@ import ( "git.cloudinit.dev/coreci/orca/internal/store" ) -func dbPath() string { - if p := os.Getenv("ORCA_DB"); p != "" { - return p - } - home, _ := os.UserHomeDir() - return filepath.Join(home, ".orca", "orca.db") -} - func openDB() (*sql.DB, func() error, error) { - db, err := store.Open(dbPath()) + db, err := store.Open(certpaths.DBPath()) if err != nil { return nil, nil, err } diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 7069ba9..a5592cb 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -19,12 +19,17 @@ import ( "crypto/x509" "encoding/pem" "fmt" + "net/http" "os" "sort" + "strings" "time" "git.cloudinit.dev/coreci/orca/internal/certpaths" + "git.cloudinit.dev/coreci/orca/internal/model" "git.cloudinit.dev/coreci/orca/internal/security" + "git.cloudinit.dev/coreci/orca/internal/store" + "git.cloudinit.dev/coreci/orca/internal/transport" ) // Result is the outcome of a single check. @@ -63,8 +68,8 @@ func All() []Check { CertServer(), CertExpiry(), CertFingerprint(), - NetworkStub(), - DBStub(), + Network(), + DB(), } } @@ -177,28 +182,124 @@ func CertFingerprint() Check { } } -// NetworkStub is a stub for the network check; full impl in P02. -func NetworkStub() Check { +// DB checks SQLite integrity and migration version (REQ-032 completion). +func DB() Check { return Check{ - Name: "network", - Description: "TCP reachability + mTLS handshake (full impl in P02)", - Run: func(_ context.Context) (Result, string) { - return ResultWarn, "network check is a stub in P01; full impl in P02" + Name: "db", + Description: "SQLite integrity_check + migration version", + Run: func(ctx context.Context) (Result, string) { + path := certpaths.DBPath() + db, err := store.Open(path) + if err != nil { + return ResultFail, fmt.Sprintf("open db: %v", err) + } + defer db.Close() + + var integrity string + if err := db.QueryRowContext(ctx, "PRAGMA integrity_check").Scan(&integrity); err != nil { + return ResultFail, fmt.Sprintf("integrity_check: %v", err) + } + if !strings.EqualFold(integrity, "ok") { + return ResultFail, fmt.Sprintf("integrity_check: %s", integrity) + } + + version, err := store.MigrationVersion(ctx, db) + if err != nil { + return ResultFail, fmt.Sprintf("migration version: %v", err) + } + if version == "" { + return ResultWarn, "integrity OK but no migrations applied (fresh db)" + } + return ResultPass, fmt.Sprintf("integrity OK, migrations up to %s", version) }, } } -// DBStub is a stub for the database check; full impl in P02. -func DBStub() Check { +// Network probes peer reachability via mTLS /healthz (REQ-032 completion). +// Peers are sourced from the persisted nodes table (not the in-memory +// PeerRegistry, which is empty at CLI time). Zero peers → WARN (single-node +// is legitimate). Any peer unreachable → FAIL (D-038). +func Network() Check { return Check{ - Name: "db", - Description: "SQLite open + migration apply (full impl in P02)", - Run: func(_ context.Context) (Result, string) { - return ResultWarn, "db check is a stub in P01; full impl in P02" + Name: "network", + Description: "peer reachability via mTLS /healthz probe", + Run: func(ctx context.Context) (Result, string) { + caPath := certpaths.CACertPath() + certPath := certpaths.ServerCertPath() + keyPath := certpaths.ServerKeyPath() + + // Check that cert files exist before attempting probes. + if _, err := os.Stat(caPath); err != nil { + return ResultFail, fmt.Sprintf("CA cert missing: %v (run `orca cert init`)", err) + } + + path := certpaths.DBPath() + db, err := store.Open(path) + if err != nil { + return ResultFail, fmt.Sprintf("open db: %v", err) + } + defer db.Close() + + nodes, err := store.NewNodeRepo(db).List(ctx) + if err != nil { + return ResultFail, fmt.Sprintf("list nodes: %v", err) + } + + live := make([]*model.Node, 0, len(nodes)) + for _, n := range nodes { + if n.State != model.NodeStateLeft { + live = append(live, n) + } + } + + if len(live) == 0 { + return ResultWarn, "no peers registered (single-node?)" + } + + var lines []string + anyFail := false + for _, n := range live { + probeCtx, cancel := context.WithTimeout(ctx, 3*time.Second) + err := probeHealthz(probeCtx, caPath, certPath, keyPath, n.Name, n.Address) + cancel() + if err != nil { + anyFail = true + lines = append(lines, fmt.Sprintf(" ✗ %s (%s): %v", n.Name, n.Address, err)) + } else { + lines = append(lines, fmt.Sprintf(" ✓ %s (%s)", n.Name, n.Address)) + } + } + + result := ResultPass + if anyFail { + result = ResultFail + } + return result, strings.Join(lines, "\n") }, } } +// probeHealthz opens an mTLS connection to the peer and GETs /healthz. +func probeHealthz(ctx context.Context, caPath, certPath, keyPath, serverName, addr string) error { + client, err := transport.NewMTLSClient(caPath, serverName, certPath, keyPath) + if err != nil { + return fmt.Errorf("mTLS client: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://"+addr+"/healthz", nil) + if err != nil { + return fmt.Errorf("request: %w", err) + } + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("probe: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("healthz returned %d", resp.StatusCode) + } + return nil +} + // loadCert reads a PEM cert from path and parses the first CERTIFICATE // block. func loadCert(path string) (*x509.Certificate, error) { diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index d10a1b4..c34aee8 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -2,60 +2,74 @@ package doctor import ( "context" + "os" + "path/filepath" "strings" "testing" + "time" + "git.cloudinit.dev/coreci/orca/internal/model" "git.cloudinit.dev/coreci/orca/internal/security" + "git.cloudinit.dev/coreci/orca/internal/store" ) -// TestRunAllChecksWithNoCA runs the full battery in a clean temp dir -// and expects all checks to FAIL (no CA, no server cert) except the -// two stubs which return WARN. +// TestRunAllChecksWithNoCA runs the full battery in a clean temp dir. +// With the P02 real checks (no stubs): cert checks FAIL (no CA), +// db check PASS (store.Open runs migrations), network check WARN +// (no peers). func TestRunAllChecksWithNoCA(t *testing.T) { - // Isolated home so we don't touch the real ~/.orca. - t.Setenv("ORCA_HOME", t.TempDir()) + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) rep := Run(context.Background()) if len(rep.Checks) == 0 { t.Fatal("expected checks, got 0") } - hasFail := false - hasWarn := false + + byName := make(map[string]CheckResult, len(rep.Checks)) for _, c := range rep.Checks { - if c.Result == ResultFail { - hasFail = true - } - if c.Result == ResultWarn { - hasWarn = true - } - } - if !hasFail { - t.Error("expected at least one FAIL (no CA installed)") - } - if !hasWarn { - t.Error("expected at least one WARN (stubs in P01)") + byName[c.Name] = c } - // Render the report — basic shape check. - out := rep.Print() - if !strings.Contains(out, "PASS") { - t.Errorf("expected PASS in output, got: %s", out) + // Cert checks: no CA → FAIL. + for _, name := range []string{"cert.ca", "cert.server", "cert.expiry", "cert.fingerprint"} { + c, ok := byName[name] + if !ok { + t.Errorf("missing check %s", name) + continue + } + if c.Result != ResultFail { + t.Errorf("%s: got %s, want FAIL — %s", name, c.Result, c.Message) + } } - if !strings.Contains(out, "WARN") { - t.Errorf("expected WARN in output, got: %s", out) + + // DB check: store.Open runs migrations → PASS. + if c, ok := byName["db"]; ok { + if c.Result != ResultPass { + t.Errorf("db: got %s, want PASS — %s", c.Result, c.Message) + } + } else { + t.Error("missing check db") } - if !strings.Contains(out, "FAIL") { - t.Errorf("expected FAIL in output, got: %s", out) + + // Network check: no CA → FAIL (can't build mTLS client without CA). + if c, ok := byName["network"]; ok { + if c.Result != ResultFail { + t.Errorf("network: got %s, want FAIL (no CA cert) — %s", c.Result, c.Message) + } + } else { + t.Error("missing check network") } } // TestRunWithCAAndServerCert covers the happy path: CA + server cert -// installed → all cert checks PASS. +// installed → all cert checks PASS, db PASS, network WARN (no peers). func TestRunWithCAAndServerCert(t *testing.T) { dir := t.TempDir() t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) - // Bootstrap CA. if _, err := security.CAInit(dir, "test-ca"); err != nil { t.Fatalf("CAInit: %v", err) } @@ -63,7 +77,6 @@ func TestRunWithCAAndServerCert(t *testing.T) { if err != nil { t.Fatalf("LoadCA: %v", err) } - // Generate + sign server cert. keyPEM, csrPEM, err := security.GenerateCSR("test-server", []string{"localhost", "127.0.0.1"}) if err != nil { t.Fatalf("GenerateCSR: %v", err) @@ -80,13 +93,155 @@ func TestRunWithCAAndServerCert(t *testing.T) { } rep := Run(context.Background()) - // The cert-related checks should be PASS; the network/db stubs WARN. + byName := make(map[string]CheckResult, len(rep.Checks)) for _, c := range rep.Checks { - switch c.Name { - case "cert.ca", "cert.server", "cert.expiry", "cert.fingerprint": - if c.Result != ResultPass { - t.Errorf("%s: got %s, want PASS — %s", c.Name, c.Result, c.Message) - } + byName[c.Name] = c + } + + for _, name := range []string{"cert.ca", "cert.server", "cert.expiry", "cert.fingerprint", "db"} { + c, ok := byName[name] + if !ok { + t.Errorf("missing check %s", name) + continue + } + if c.Result != ResultPass { + t.Errorf("%s: got %s, want PASS — %s", name, c.Result, c.Message) + } + } + + if c, ok := byName["network"]; ok { + if c.Result != ResultWarn { + t.Errorf("network: got %s, want WARN (no peers) — %s", c.Result, c.Message) } } } + +// TestDBCheck_IntegrityOK verifies the DB check passes on a fresh +// database with migrations applied. +func TestDBCheck_IntegrityOK(t *testing.T) { + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) + + db, err := store.Open(filepath.Join(dir, "orca.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer db.Close() + + c := DB() + r, msg := c.Run(context.Background()) + if r != ResultPass { + t.Errorf("DB check: got %s, want PASS — %s", r, msg) + } + if !strings.Contains(msg, "0005") { + t.Errorf("DB check message should contain migration version, got: %s", msg) + } +} + +// TestNetworkCheck_NoPeers verifies the network check returns WARN +// when no peers are registered. +func TestNetworkCheck_NoPeers(t *testing.T) { + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) + + // Create a CA + server cert so the network check can build a client. + if _, err := security.CAInit(dir, "test-ca"); err != nil { + t.Fatalf("CAInit: %v", err) + } + ca, _ := security.LoadCA(dir) + keyPEM, csrPEM, _ := security.GenerateCSR("test-server", []string{"localhost"}) + certPEM, _ := ca.SignCSR(csrPEM) + _ = security.WriteCert(dir+"/server.crt", certPEM) + _ = security.WriteKey(dir+"/server.key", keyPEM) + + c := Network() + r, msg := c.Run(context.Background()) + if r != ResultWarn { + t.Errorf("Network check: got %s, want WARN — %s", r, msg) + } + if !strings.Contains(msg, "no peers") { + t.Errorf("Network check message should mention no peers, got: %s", msg) + } +} + +// TestNetworkCheck_PeerUnreachable verifies the network check returns +// FAIL when a registered peer is not reachable. +func TestNetworkCheck_PeerUnreachable(t *testing.T) { + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) + + // Create a CA + server cert. + if _, err := security.CAInit(dir, "test-ca"); err != nil { + t.Fatalf("CAInit: %v", err) + } + ca, _ := security.LoadCA(dir) + keyPEM, csrPEM, _ := security.GenerateCSR("test-server", []string{"localhost"}) + certPEM, _ := ca.SignCSR(csrPEM) + _ = security.WriteCert(dir+"/server.crt", certPEM) + _ = security.WriteKey(dir+"/server.key", keyPEM) + + // Insert a peer node with an unreachable address. + db, err := store.Open(filepath.Join(dir, "orca.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer db.Close() + repo := store.NewNodeRepo(db) + _ = repo.Insert(context.Background(), &model.Node{ + ID: "dead-peer", Name: "dead", Address: "127.0.0.1:1", + State: model.NodeStateReady, JoinedAt: time.Now().UTC(), LastSeen: time.Now().UTC(), + }) + + c := Network() + r, msg := c.Run(context.Background()) + if r != ResultFail { + t.Errorf("Network check: got %s, want FAIL — %s", r, msg) + } + if !strings.Contains(msg, "dead") { + t.Errorf("Network check message should mention the dead peer, got: %s", msg) + } +} + +// TestNetworkCheck_NoCert verifies the network check returns FAIL +// when no CA cert is installed. +func TestNetworkCheck_NoCert(t *testing.T) { + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) + + c := Network() + r, msg := c.Run(context.Background()) + if r != ResultFail { + t.Errorf("Network check: got %s, want FAIL — %s", r, msg) + } + if !strings.Contains(msg, "CA cert missing") { + t.Errorf("Network check message should mention missing CA, got: %s", msg) + } +} + +// TestRenderReport verifies the report output format. +func TestRenderReport(t *testing.T) { + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + t.Setenv("ORCA_DB", filepath.Join(dir, "orca.db")) + + rep := Run(context.Background()) + out := rep.Print() + if !strings.Contains(out, "PASS") { + t.Errorf("expected PASS in output, got: %s", out) + } + if !strings.Contains(out, "WARN") { + t.Errorf("expected WARN in output, got: %s", out) + } + if !strings.Contains(out, "FAIL") { + t.Errorf("expected FAIL in output, got: %s", out) + } +} + +func init() { + // Suppress slog noise during tests. + _ = os.Setenv("ORCA_LOG_LEVEL", "error") +} diff --git a/internal/store/job_task_repo_test.go b/internal/store/job_task_repo_test.go index a126cfd..8bbcbc1 100644 --- a/internal/store/job_task_repo_test.go +++ b/internal/store/job_task_repo_test.go @@ -59,7 +59,7 @@ func TestJobRepoWatch_YieldsSnapshots(t *testing.T) { defer close(done) for snap := range repo.Watch(ctx) { snapshots = append(snapshots, snap) - if len(snapshots) >= 15 { + if len(snapshots) >= 40 { cancel() return } diff --git a/internal/store/migrate.go b/internal/store/migrate.go index affe47e..1952920 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -12,6 +12,21 @@ import ( //go:embed migrations/*.sql var migrationsFS embed.FS +// MigrationVersion returns the name of the highest applied migration +// (e.g. "0005_node_capacity.sql"). Returns ("", nil) if no migrations +// have been applied (fresh or empty database). +func MigrationVersion(ctx context.Context, db *sql.DB) (string, error) { + var name string + err := db.QueryRowContext(ctx, `SELECT name FROM schema_migrations ORDER BY name DESC LIMIT 1`).Scan(&name) + if err == sql.ErrNoRows { + return "", nil + } + if err != nil { + return "", fmt.Errorf("query migration version: %w", err) + } + return name, nil +} + func migrate(db *sql.DB) error { entries, err := migrationsFS.ReadDir("migrations") if err != nil { diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go new file mode 100644 index 0000000..499e9c6 --- /dev/null +++ b/internal/store/migrate_test.go @@ -0,0 +1,37 @@ +package store + +import ( + "context" + "path/filepath" + "testing" +) + +func TestMigrationVersion(t *testing.T) { + dir := t.TempDir() + db, err := Open(filepath.Join(dir, "test.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer db.Close() + + ctx := context.Background() + version, err := MigrationVersion(ctx, db) + if err != nil { + t.Fatalf("migration version: %v", err) + } + if version != "0005_node_capacity.sql" { + t.Errorf("MigrationVersion = %q, want 0005_node_capacity.sql", version) + } + + // Empty the migrations table → should return ("", nil). + if _, err := db.ExecContext(ctx, "DELETE FROM schema_migrations"); err != nil { + t.Fatalf("clear migrations: %v", err) + } + version, err = MigrationVersion(ctx, db) + if err != nil { + t.Fatalf("migration version after clear: %v", err) + } + if version != "" { + t.Errorf("MigrationVersion after clear = %q, want empty", version) + } +} diff --git a/internal/store/node_repo_test.go b/internal/store/node_repo_test.go index a7d8a5a..fab5ef0 100644 --- a/internal/store/node_repo_test.go +++ b/internal/store/node_repo_test.go @@ -131,7 +131,7 @@ func TestNodeRepoWatch_YieldsSnapshots(t *testing.T) { defer close(done) for snap := range repo.Watch(ctx) { snapshots = append(snapshots, snap) - if len(snapshots) >= 15 { + if len(snapshots) >= 40 { cancel() return }