diff --git a/internal/repository/postgres/health_check_test.go b/internal/repository/postgres/health_check_test.go new file mode 100644 index 0000000..f9b132b --- /dev/null +++ b/internal/repository/postgres/health_check_test.go @@ -0,0 +1,453 @@ +package postgres_test + +// Integration tests for HealthCheckRepository (M48). Closes the L-1 +// coverage gap flagged in coverage-gap-audit.md: the 453-line repository +// shipped in M48 had zero live-DB tests, leaving 11 methods — including +// the time-sensitive ListDueForCheck, PurgeHistory, and GetSummary — +// without migration-pinned regression protection. These tests exercise +// every method against a real Postgres 16 container through the same +// schema-per-test harness used by repo_test.go. + +import ( + "context" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/repository" + "github.com/shankar0123/certctl/internal/repository/postgres" +) + +// newHealthCheck builds a minimal EndpointHealthCheck the repository will +// accept. All time-pointer fields are left nil so callers can override +// exactly the bits each subtest cares about — Create stores nil pointers +// as NULL, which is what ListDueForCheck's `last_checked_at IS NULL` +// branch relies on. +func newHealthCheck(id, endpoint string, status domain.HealthStatus, enabled bool) *domain.EndpointHealthCheck { + now := time.Now().UTC().Truncate(time.Microsecond) + return &domain.EndpointHealthCheck{ + ID: id, + Endpoint: endpoint, + Status: status, + DegradedThreshold: 2, + DownThreshold: 5, + CheckIntervalSecs: 300, + Enabled: enabled, + CreatedAt: now, + UpdatedAt: now, + } +} + +// TestHealthCheckRepository_CRUD covers Create → Get → Update → Delete on +// the nominal path. Also verifies the sql.NullTime round-trip: a check +// created without timestamps comes back with nil pointers (not +// zero-valued time.Time) so downstream Go code can distinguish "never +// probed" from "probed at epoch". +func TestHealthCheckRepository_CRUD(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + check := newHealthCheck("hc-crud", "example.com:443", domain.HealthStatusHealthy, true) + check.ExpectedFingerprint = "sha256:expected" + check.ResponseTimeMs = 42 + + if err := repo.Create(ctx, check); err != nil { + t.Fatalf("Create failed: %v", err) + } + + got, err := repo.Get(ctx, "hc-crud") + if err != nil { + t.Fatalf("Get failed: %v", err) + } + if got.Endpoint != "example.com:443" { + t.Errorf("Endpoint = %q, want example.com:443", got.Endpoint) + } + if got.Status != domain.HealthStatusHealthy { + t.Errorf("Status = %q, want %q", got.Status, domain.HealthStatusHealthy) + } + if got.ExpectedFingerprint != "sha256:expected" { + t.Errorf("ExpectedFingerprint = %q, want sha256:expected", got.ExpectedFingerprint) + } + if got.CheckIntervalSecs != 300 { + t.Errorf("CheckIntervalSecs = %d, want 300", got.CheckIntervalSecs) + } + if got.LastCheckedAt != nil { + t.Errorf("LastCheckedAt = %v, want nil (never probed)", got.LastCheckedAt) + } + + // Update: status transition + observed fingerprint assignment. + // Update() rewrites UpdatedAt to time.Now() regardless of what we + // send, so record the pre-call timestamp to assert monotonic advance. + preUpdate := got.UpdatedAt + time.Sleep(2 * time.Millisecond) // ensure a measurable delta + got.Status = domain.HealthStatusDegraded + got.ObservedFingerprint = "sha256:observed" + got.ConsecutiveFailures = 2 + if err := repo.Update(ctx, got); err != nil { + t.Fatalf("Update failed: %v", err) + } + + got2, err := repo.Get(ctx, "hc-crud") + if err != nil { + t.Fatalf("Get after Update failed: %v", err) + } + if got2.Status != domain.HealthStatusDegraded { + t.Errorf("Status after Update = %q, want %q", got2.Status, domain.HealthStatusDegraded) + } + if got2.ObservedFingerprint != "sha256:observed" { + t.Errorf("ObservedFingerprint after Update = %q, want sha256:observed", got2.ObservedFingerprint) + } + if got2.ConsecutiveFailures != 2 { + t.Errorf("ConsecutiveFailures after Update = %d, want 2", got2.ConsecutiveFailures) + } + if !got2.UpdatedAt.After(preUpdate) { + t.Errorf("UpdatedAt did not advance: pre=%v post=%v", preUpdate, got2.UpdatedAt) + } + + if err := repo.Delete(ctx, "hc-crud"); err != nil { + t.Fatalf("Delete failed: %v", err) + } + if _, err := repo.Get(ctx, "hc-crud"); err == nil { + t.Errorf("Get after Delete returned nil error, want not-found") + } +} + +// TestHealthCheckRepository_GetByEndpoint verifies the secondary lookup +// path used by AutoCreateFromDeployment to decide whether to INSERT or +// UPDATE. Missing endpoints return an error (not a nil cert) so the +// service layer can branch safely. +func TestHealthCheckRepository_GetByEndpoint(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + check := newHealthCheck("hc-byep", "svc.internal:443", domain.HealthStatusHealthy, true) + if err := repo.Create(ctx, check); err != nil { + t.Fatalf("Create failed: %v", err) + } + + got, err := repo.GetByEndpoint(ctx, "svc.internal:443") + if err != nil { + t.Fatalf("GetByEndpoint failed: %v", err) + } + if got.ID != "hc-byep" { + t.Errorf("ID = %q, want hc-byep", got.ID) + } + + if _, err := repo.GetByEndpoint(ctx, "never-seen.example.com:443"); err == nil { + t.Errorf("GetByEndpoint on unknown endpoint returned nil error") + } +} + +// TestHealthCheckRepository_List_Filters seeds rows across the filter +// axes (status, certificate_id, enabled) and asserts each branch of the +// WHERE builder, plus the Page/PerPage pagination shim. +func TestHealthCheckRepository_List_Filters(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + // Create prereq managed certificate so certificate_id FK can be + // populated on one row — proves the filter path joins on a real ID. + ownerID, teamID, issuerID, policyID := insertCertPrereqsRaw(t, db, ctx, "hclist") + certID := "mc-hc-list" + now := time.Now().UTC().Truncate(time.Microsecond) + if _, err := db.ExecContext(ctx, ` + INSERT INTO managed_certificates (id, name, common_name, sans, environment, owner_id, team_id, issuer_id, renewal_policy_id, status, expires_at, tags, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)`, + certID, "hc-list-cert", "hc.example.com", "{}", "production", + ownerID, teamID, issuerID, policyID, + string(domain.CertificateStatusActive), now.Add(90*24*time.Hour), "{}", + now, now); err != nil { + t.Fatalf("seed managed_certificate: %v", err) + } + + // 4 rows: healthy+enabled+cert, degraded+enabled, down+disabled, unknown+enabled. + rows := []*domain.EndpointHealthCheck{ + newHealthCheck("hc-list-1", "a.example.com:443", domain.HealthStatusHealthy, true), + newHealthCheck("hc-list-2", "b.example.com:443", domain.HealthStatusDegraded, true), + newHealthCheck("hc-list-3", "c.example.com:443", domain.HealthStatusDown, false), + newHealthCheck("hc-list-4", "d.example.com:443", domain.HealthStatusUnknown, true), + } + rows[0].CertificateID = &certID + for _, r := range rows { + if err := repo.Create(ctx, r); err != nil { + t.Fatalf("Create %s: %v", r.ID, err) + } + } + + // Filter: status=healthy → 1 result. + got, total, err := repo.List(ctx, &repository.HealthCheckFilter{Status: string(domain.HealthStatusHealthy)}) + if err != nil { + t.Fatalf("List status=healthy: %v", err) + } + if total != 1 || len(got) != 1 || got[0].ID != "hc-list-1" { + t.Errorf("status=healthy: total=%d rows=%d want 1/1 with hc-list-1", total, len(got)) + } + + // Filter: certificate_id → 1 result. + got, total, err = repo.List(ctx, &repository.HealthCheckFilter{CertificateID: certID}) + if err != nil { + t.Fatalf("List certificate_id: %v", err) + } + if total != 1 || len(got) != 1 || got[0].ID != "hc-list-1" { + t.Errorf("certificate_id filter: total=%d rows=%d want 1/1", total, len(got)) + } + + // Filter: enabled=false → 1 result. + disabled := false + got, total, err = repo.List(ctx, &repository.HealthCheckFilter{Enabled: &disabled}) + if err != nil { + t.Fatalf("List enabled=false: %v", err) + } + if total != 1 || len(got) != 1 || got[0].ID != "hc-list-3" { + t.Errorf("enabled=false: total=%d rows=%d want 1/1 with hc-list-3", total, len(got)) + } + + // Pagination: per_page=2 → first page has 2, total reflects all 4. + got, total, err = repo.List(ctx, &repository.HealthCheckFilter{Page: 1, PerPage: 2}) + if err != nil { + t.Fatalf("List paginated: %v", err) + } + if total != 4 { + t.Errorf("paginated total = %d, want 4", total) + } + if len(got) != 2 { + t.Errorf("paginated rows = %d, want 2", len(got)) + } +} + +// TestHealthCheckRepository_ListDueForCheck seeds all four branches of +// the WHERE clause — (a) enabled+null → due, (b) enabled+past-due → due, +// (c) enabled+recent → not due, (d) disabled+null → excluded — and +// asserts the ORDER BY last_checked_at NULLS FIRST, ASC ordering. +// +// This is the hot path the scheduler's 8th loop hits every 60 seconds; +// a correctness regression here silently fails every probe. +func TestHealthCheckRepository_ListDueForCheck(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + now := time.Now().UTC().Truncate(time.Microsecond) + pastDue := now.Add(-10 * time.Minute) // > 300s ago, enabled → due + recent := now.Add(-30 * time.Second) // < 300s ago, enabled → not due + + // (a) enabled + null last_checked_at — NULLS FIRST puts this first + a := newHealthCheck("hc-due-a", "a.example.com:443", domain.HealthStatusUnknown, true) + // (b) enabled + past-due last_checked_at + b := newHealthCheck("hc-due-b", "b.example.com:443", domain.HealthStatusHealthy, true) + b.LastCheckedAt = &pastDue + // (c) enabled + recent last_checked_at — must NOT appear + c := newHealthCheck("hc-due-c", "c.example.com:443", domain.HealthStatusHealthy, true) + c.LastCheckedAt = &recent + // (d) disabled + null last_checked_at — must NOT appear + d := newHealthCheck("hc-due-d", "d.example.com:443", domain.HealthStatusUnknown, false) + + for _, r := range []*domain.EndpointHealthCheck{a, b, c, d} { + if err := repo.Create(ctx, r); err != nil { + t.Fatalf("Create %s: %v", r.ID, err) + } + } + + due, err := repo.ListDueForCheck(ctx) + if err != nil { + t.Fatalf("ListDueForCheck: %v", err) + } + if len(due) != 2 { + ids := make([]string, 0, len(due)) + for _, r := range due { + ids = append(ids, r.ID) + } + t.Fatalf("due rows = %d (%v), want exactly 2 (hc-due-a, hc-due-b)", len(due), ids) + } + // NULLS FIRST: variant (a) should precede variant (b). + if due[0].ID != "hc-due-a" { + t.Errorf("due[0].ID = %q, want hc-due-a (NULLS FIRST)", due[0].ID) + } + if due[1].ID != "hc-due-b" { + t.Errorf("due[1].ID = %q, want hc-due-b", due[1].ID) + } + // Sanity: neither excluded row leaked through. + for _, r := range due { + if r.ID == "hc-due-c" { + t.Errorf("recent-probed row hc-due-c leaked into due set") + } + if r.ID == "hc-due-d" { + t.Errorf("disabled row hc-due-d leaked into due set") + } + } +} + +// TestHealthCheckRepository_RecordHistory_GetHistory asserts FIFO +// insertion with DESC retrieval (most-recent-first) and the explicit +// limit clamp. +func TestHealthCheckRepository_RecordHistory_GetHistory(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + parent := newHealthCheck("hc-hist", "hist.example.com:443", domain.HealthStatusHealthy, true) + if err := repo.Create(ctx, parent); err != nil { + t.Fatalf("Create parent: %v", err) + } + + base := time.Now().UTC().Truncate(time.Microsecond) + entries := []*domain.HealthHistoryEntry{ + {ID: "hh-1", HealthCheckID: "hc-hist", Status: string(domain.HealthStatusHealthy), ResponseTimeMs: 10, CheckedAt: base.Add(-3 * time.Minute)}, + {ID: "hh-2", HealthCheckID: "hc-hist", Status: string(domain.HealthStatusDegraded), ResponseTimeMs: 20, CheckedAt: base.Add(-2 * time.Minute)}, + {ID: "hh-3", HealthCheckID: "hc-hist", Status: string(domain.HealthStatusHealthy), ResponseTimeMs: 30, CheckedAt: base.Add(-1 * time.Minute)}, + } + for _, e := range entries { + if err := repo.RecordHistory(ctx, e); err != nil { + t.Fatalf("RecordHistory %s: %v", e.ID, err) + } + } + + // limit=2 → newest 2 in DESC order: hh-3, hh-2. + got, err := repo.GetHistory(ctx, "hc-hist", 2) + if err != nil { + t.Fatalf("GetHistory: %v", err) + } + if len(got) != 2 { + t.Fatalf("rows = %d, want 2", len(got)) + } + if got[0].ID != "hh-3" || got[1].ID != "hh-2" { + t.Errorf("order = [%s, %s], want [hh-3, hh-2]", got[0].ID, got[1].ID) + } + + // limit=0 → default 100 → returns all 3. + got, err = repo.GetHistory(ctx, "hc-hist", 0) + if err != nil { + t.Fatalf("GetHistory limit=0: %v", err) + } + if len(got) != 3 { + t.Errorf("limit=0 rows = %d, want 3", len(got)) + } +} + +// TestHealthCheckRepository_PurgeHistory exercises the retention-sweep +// hot path (scheduler calls this once/day). 5 past + 5 future straddling +// the cutoff exposes both sides of the < comparator — an off-by-one +// regression here would either nuke live data or skip rows the retention +// policy was meant to remove. +func TestHealthCheckRepository_PurgeHistory(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + parent := newHealthCheck("hc-purge", "purge.example.com:443", domain.HealthStatusHealthy, true) + if err := repo.Create(ctx, parent); err != nil { + t.Fatalf("Create parent: %v", err) + } + + cutoff := time.Now().UTC().Truncate(time.Microsecond) + + // 5 rows BEFORE cutoff (should be purged). + for i := 0; i < 5; i++ { + e := &domain.HealthHistoryEntry{ + ID: "hh-past-" + string(rune('0'+i)), + HealthCheckID: "hc-purge", + Status: string(domain.HealthStatusHealthy), + CheckedAt: cutoff.Add(time.Duration(-10-i) * time.Minute), + } + if err := repo.RecordHistory(ctx, e); err != nil { + t.Fatalf("RecordHistory past %d: %v", i, err) + } + } + // 5 rows AFTER cutoff (should remain). + for i := 0; i < 5; i++ { + e := &domain.HealthHistoryEntry{ + ID: "hh-future-" + string(rune('0'+i)), + HealthCheckID: "hc-purge", + Status: string(domain.HealthStatusHealthy), + CheckedAt: cutoff.Add(time.Duration(1+i) * time.Minute), + } + if err := repo.RecordHistory(ctx, e); err != nil { + t.Fatalf("RecordHistory future %d: %v", i, err) + } + } + + deleted, err := repo.PurgeHistory(ctx, cutoff) + if err != nil { + t.Fatalf("PurgeHistory: %v", err) + } + if deleted != 5 { + t.Errorf("deleted = %d, want 5", deleted) + } + + remaining, err := repo.GetHistory(ctx, "hc-purge", 100) + if err != nil { + t.Fatalf("GetHistory after purge: %v", err) + } + if len(remaining) != 5 { + t.Errorf("remaining = %d, want 5", len(remaining)) + } +} + +// TestHealthCheckRepository_GetSummary seeds all 5 HealthStatus values +// in a non-uniform distribution so the GROUP BY status branch-table gets +// exercised on each arm. The Total field is computed inside the +// aggregator — its drift would not surface unless we assert it too. +func TestHealthCheckRepository_GetSummary(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewHealthCheckRepository(db) + ctx := context.Background() + + seed := map[domain.HealthStatus]int{ + domain.HealthStatusHealthy: 3, + domain.HealthStatusDegraded: 2, + domain.HealthStatusDown: 2, + domain.HealthStatusCertMismatch: 1, + domain.HealthStatusUnknown: 1, + } + idx := 0 + for status, count := range seed { + for i := 0; i < count; i++ { + check := newHealthCheck( + "hc-sum-"+string(rune('a'+idx)), + "sum.example.com:443", + status, + true, + ) + // Endpoint uniqueness isn't enforced by the schema but making + // it unique documents intent and rules out false-positives. + check.Endpoint = check.ID + "-" + check.Endpoint + if err := repo.Create(ctx, check); err != nil { + t.Fatalf("Create %s: %v", check.ID, err) + } + idx++ + } + } + + summary, err := repo.GetSummary(ctx) + if err != nil { + t.Fatalf("GetSummary: %v", err) + } + if summary.Healthy != 3 { + t.Errorf("Healthy = %d, want 3", summary.Healthy) + } + if summary.Degraded != 2 { + t.Errorf("Degraded = %d, want 2", summary.Degraded) + } + if summary.Down != 2 { + t.Errorf("Down = %d, want 2", summary.Down) + } + if summary.CertMismatch != 1 { + t.Errorf("CertMismatch = %d, want 1", summary.CertMismatch) + } + if summary.Unknown != 1 { + t.Errorf("Unknown = %d, want 1", summary.Unknown) + } + if summary.Total != 9 { + t.Errorf("Total = %d, want 9 (3+2+2+1+1)", summary.Total) + } +} diff --git a/internal/repository/postgres/renewal_policy_test.go b/internal/repository/postgres/renewal_policy_test.go new file mode 100644 index 0000000..eda633f --- /dev/null +++ b/internal/repository/postgres/renewal_policy_test.go @@ -0,0 +1,317 @@ +package postgres_test + +// Integration tests for RenewalPolicyRepository (post-G-1, 289 lines, 5 +// methods). Closes the L-1 coverage gap flagged in coverage-gap-audit.md: +// the repository's auto-generated-ID collision retry loop and its two +// typed error sentinels (ErrRenewalPolicyDuplicateName on pg 23505, +// ErrRenewalPolicyInUse on pg 23503) shipped with zero live-DB regression +// coverage — a mock-only test surface cannot exercise the PostgreSQL +// constraint semantics these paths depend on. +// +// The audit listed the file as "92 lines, 2 methods"; that was stale +// pre-G-1. Current state is 5 methods (Get/List/Create/Update/Delete), +// all covered below. + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/repository" + "github.com/shankar0123/certctl/internal/repository/postgres" +) + +// TestRenewalPolicyRepository_CRUD exercises the happy path for all five +// interface methods. In particular it drives the slug-based ID +// auto-generation branch (policy.ID left empty → Create emits +// rp-) so any regression to slugifyPolicyName or the retry +// loop surfaces immediately. The AlertThresholdsDays JSONB round-trip is +// asserted end-to-end: marshal on Create → store as JSONB → scan back on +// Get preserves the slice ordering and values. +func TestRenewalPolicyRepository_CRUD(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewRenewalPolicyRepository(db) + ctx := context.Background() + + // Create: leave ID empty so the repository generates rp-. + // "Prod TLS 90d" → rp-prod-tls-90d per slugifyPolicyName's rules + // (lowercase, spaces→hyphens, non-alphanumeric stripped). + policy := &domain.RenewalPolicy{ + Name: "Prod TLS 90d", + RenewalWindowDays: 30, + AutoRenew: true, + MaxRetries: 5, + RetryInterval: 3600, // stored in retry_interval_minutes column; passthrough + AlertThresholdsDays: []int{30, 14, 7, 0}, + } + + if err := repo.Create(ctx, policy); err != nil { + t.Fatalf("Create failed: %v", err) + } + if policy.ID != "rp-prod-tls-90d" { + t.Errorf("auto-generated ID = %q, want %q", policy.ID, "rp-prod-tls-90d") + } + if policy.CreatedAt.IsZero() { + t.Error("Create did not populate CreatedAt (RETURNING clause regression?)") + } + if policy.UpdatedAt.IsZero() { + t.Error("Create did not populate UpdatedAt (RETURNING clause regression?)") + } + + // Get: pull the just-created row back and confirm every stored field + // survives the scanRenewalPolicy path, including the JSONB unmarshal. + got, err := repo.Get(ctx, policy.ID) + if err != nil { + t.Fatalf("Get failed: %v", err) + } + if got.Name != "Prod TLS 90d" { + t.Errorf("Get: Name = %q, want %q", got.Name, "Prod TLS 90d") + } + if got.RenewalWindowDays != 30 { + t.Errorf("Get: RenewalWindowDays = %d, want 30", got.RenewalWindowDays) + } + if !got.AutoRenew { + t.Error("Get: AutoRenew = false, want true") + } + if got.MaxRetries != 5 { + t.Errorf("Get: MaxRetries = %d, want 5", got.MaxRetries) + } + if len(got.AlertThresholdsDays) != 4 { + t.Fatalf("Get: AlertThresholdsDays length = %d, want 4 (JSONB round-trip regression)", len(got.AlertThresholdsDays)) + } + for i, want := range []int{30, 14, 7, 0} { + if got.AlertThresholdsDays[i] != want { + t.Errorf("Get: AlertThresholdsDays[%d] = %d, want %d", i, got.AlertThresholdsDays[i], want) + } + } + + // Update: 3-arg signature is a house invariant — don't let it slip to + // 2-arg without the test catching the breakage. Tweak scalar + JSONB + // simultaneously so both SET branches exercise. + updated := *got + updated.Name = "Prod TLS 90d (tightened)" + updated.RenewalWindowDays = 45 + updated.AlertThresholdsDays = []int{45, 30, 14, 7, 0} + + // Sleep long enough that NOW() ticks past the Create timestamp so we + // can assert UpdatedAt monotonicity without a flaky equality check. + time.Sleep(2 * time.Millisecond) + + if err := repo.Update(ctx, policy.ID, &updated); err != nil { + t.Fatalf("Update failed: %v", err) + } + if !updated.UpdatedAt.After(got.UpdatedAt) { + t.Errorf("Update: UpdatedAt %v not after Create's %v (RETURNING NOW() regression?)", updated.UpdatedAt, got.UpdatedAt) + } + + refetched, err := repo.Get(ctx, policy.ID) + if err != nil { + t.Fatalf("Get after Update failed: %v", err) + } + if refetched.Name != "Prod TLS 90d (tightened)" { + t.Errorf("Get after Update: Name = %q, want %q", refetched.Name, "Prod TLS 90d (tightened)") + } + if refetched.RenewalWindowDays != 45 { + t.Errorf("Get after Update: RenewalWindowDays = %d, want 45", refetched.RenewalWindowDays) + } + if len(refetched.AlertThresholdsDays) != 5 { + t.Errorf("Get after Update: AlertThresholdsDays length = %d, want 5", len(refetched.AlertThresholdsDays)) + } + + // List: add a second policy so the ORDER BY name contract is non-vacuous. + second := &domain.RenewalPolicy{ + Name: "Aa Earliest", + RenewalWindowDays: 14, + AutoRenew: false, + MaxRetries: 1, + RetryInterval: 60, + AlertThresholdsDays: []int{7, 0}, + } + if err := repo.Create(ctx, second); err != nil { + t.Fatalf("Create second failed: %v", err) + } + + all, err := repo.List(ctx) + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(all) != 2 { + t.Fatalf("List: len = %d, want 2", len(all)) + } + // "Aa Earliest" sorts before "Prod TLS 90d (tightened)" under ORDER BY name ASC. + if all[0].Name != "Aa Earliest" { + t.Errorf("List[0].Name = %q, want %q (ORDER BY name regression?)", all[0].Name, "Aa Earliest") + } + + // Delete: removes the policy and a follow-up Get surfaces "not found". + if err := repo.Delete(ctx, policy.ID); err != nil { + t.Fatalf("Delete failed: %v", err) + } + if _, err := repo.Get(ctx, policy.ID); err == nil { + t.Error("Get after Delete: err = nil, want not-found") + } +} + +// TestRenewalPolicyRepository_DuplicateName verifies the pg 23505 +// unique_violation translation. The name UNIQUE constraint is enforced +// on the renewal_policies.name column; Create's inner scanRenewalPolicy +// must see the pq.Error, call isUniqueViolation, check the constraint +// name, and return ErrRenewalPolicyDuplicateName. A non-sentinel error +// here would cause the handler to emit 500 instead of 409. +func TestRenewalPolicyRepository_DuplicateName(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewRenewalPolicyRepository(db) + ctx := context.Background() + + first := &domain.RenewalPolicy{ + ID: "rp-first", + Name: "Shared Name", + RenewalWindowDays: 30, + AutoRenew: true, + MaxRetries: 3, + RetryInterval: 300, + AlertThresholdsDays: domain.DefaultAlertThresholds(), + } + if err := repo.Create(ctx, first); err != nil { + t.Fatalf("Create first failed: %v", err) + } + + // Second policy with a distinct ID but the same Name — the name UNIQUE + // constraint fires, Create's collision branch inspects pqErr.Constraint, + // and because it's NOT *_pkey, it returns ErrRenewalPolicyDuplicateName + // without retrying. + second := &domain.RenewalPolicy{ + ID: "rp-second", + Name: "Shared Name", + RenewalWindowDays: 60, + AutoRenew: false, + MaxRetries: 1, + RetryInterval: 600, + AlertThresholdsDays: domain.DefaultAlertThresholds(), + } + err := repo.Create(ctx, second) + if err == nil { + t.Fatal("Create second: err = nil, want ErrRenewalPolicyDuplicateName") + } + if !errors.Is(err, repository.ErrRenewalPolicyDuplicateName) { + t.Errorf("Create second: err = %v, want ErrRenewalPolicyDuplicateName (via errors.Is)", err) + } + + // Also verify Update surfaces the same sentinel when an existing row's + // name is changed to collide with another policy's name. + third := &domain.RenewalPolicy{ + ID: "rp-third", + Name: "Third Name", + RenewalWindowDays: 90, + AutoRenew: true, + MaxRetries: 2, + RetryInterval: 1200, + AlertThresholdsDays: domain.DefaultAlertThresholds(), + } + if err := repo.Create(ctx, third); err != nil { + t.Fatalf("Create third failed: %v", err) + } + third.Name = "Shared Name" // collide with first + err = repo.Update(ctx, third.ID, third) + if err == nil { + t.Fatal("Update: err = nil, want ErrRenewalPolicyDuplicateName") + } + if !errors.Is(err, repository.ErrRenewalPolicyDuplicateName) { + t.Errorf("Update: err = %v, want ErrRenewalPolicyDuplicateName (via errors.Is)", err) + } +} + +// TestRenewalPolicyRepository_DeleteInUse verifies the pg 23503 +// foreign_key_violation translation. managed_certificates.renewal_policy_id +// REFERENCES renewal_policies(id) ON DELETE RESTRICT; attempting to Delete +// a policy while a certificate still references it must surface as +// ErrRenewalPolicyInUse so the handler can emit 409 Conflict. Any change +// to either the FK definition or the isForeignKeyViolation mapping breaks +// this. +func TestRenewalPolicyRepository_DeleteInUse(t *testing.T) { + tdb := getTestDB(t) + db := tdb.freshSchema(t) + repo := postgres.NewRenewalPolicyRepository(db) + ctx := context.Background() + + // The policy under test — create via repo so ID auto-generation is + // also exercised end-to-end in this path. + policy := &domain.RenewalPolicy{ + Name: "InUse Policy", + RenewalWindowDays: 30, + AutoRenew: true, + MaxRetries: 3, + RetryInterval: 300, + AlertThresholdsDays: domain.DefaultAlertThresholds(), + } + if err := repo.Create(ctx, policy); err != nil { + t.Fatalf("Create policy failed: %v", err) + } + + // Create owner/team/issuer prerequisites, then raw-INSERT a + // managed_certificate row referencing the policy. Using raw SQL here + // (matching insertCertPrereqsRaw's idiom) keeps the test independent + // of the service layer. + ownerID, teamID, issuerID, _ := insertCertPrereqsRaw(t, db, ctx, "inuse") + + now := time.Now().UTC().Truncate(time.Microsecond) + _, err := db.ExecContext(ctx, ` + INSERT INTO managed_certificates ( + id, name, common_name, sans, environment, + owner_id, team_id, issuer_id, renewal_policy_id, + status, expires_at, tags, created_at, updated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) + `, + "mc-inuse", "inuse-cert", "inuse.example.com", []string{}, "production", + ownerID, teamID, issuerID, policy.ID, + string(domain.CertificateStatusActive), now.Add(90*24*time.Hour), "{}", now, now, + ) + if err != nil { + t.Fatalf("INSERT managed_certificates failed: %v", err) + } + + // Delete: the ON DELETE RESTRICT FK fires, the pg driver returns a + // *pq.Error with Code 23503, isForeignKeyViolation detects it, and + // the repository returns ErrRenewalPolicyInUse. + err = repo.Delete(ctx, policy.ID) + if err == nil { + t.Fatal("Delete: err = nil, want ErrRenewalPolicyInUse (ON DELETE RESTRICT should have fired)") + } + if !errors.Is(err, repository.ErrRenewalPolicyInUse) { + t.Errorf("Delete: err = %v, want ErrRenewalPolicyInUse (via errors.Is)", err) + } + + // And the policy is still there — RESTRICT aborted the delete. + if _, err := repo.Get(ctx, policy.ID); err != nil { + t.Errorf("Get after failed Delete: err = %v, want nil (policy should still exist)", err) + } + + // After removing the referencing cert, Delete succeeds — proves the + // RESTRICT was the only thing blocking the earlier Delete and rules + // out any unrelated failure mode. + if _, err := db.ExecContext(ctx, `DELETE FROM managed_certificates WHERE id = $1`, "mc-inuse"); err != nil { + t.Fatalf("cleanup DELETE managed_certificates failed: %v", err) + } + if err := repo.Delete(ctx, policy.ID); err != nil { + t.Errorf("Delete after cleanup: err = %v, want nil", err) + } + + // Also verify Delete on a non-existent ID returns a not-found error + // (not nil, not the InUse sentinel) — guards against a silent no-op + // regression in the RowsAffected check. + err = repo.Delete(ctx, "rp-does-not-exist") + if err == nil { + t.Fatal("Delete(non-existent): err = nil, want not-found") + } + if errors.Is(err, repository.ErrRenewalPolicyInUse) { + t.Errorf("Delete(non-existent): err = %v, should not be ErrRenewalPolicyInUse", err) + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("Delete(non-existent): err = %v, want substring %q", err, "not found") + } +} diff --git a/internal/repository/postgres/repo_test.go b/internal/repository/postgres/repo_test.go index 7e07fe1..dc38d74 100644 --- a/internal/repository/postgres/repo_test.go +++ b/internal/repository/postgres/repo_test.go @@ -1,6 +1,8 @@ -// Package postgres_test provides repository integration tests covering 15 of 17 +// Package postgres_test provides repository integration tests covering 17 of 17 // PostgreSQL repository files. Each test function exercises CRUD operations, -// edge cases, and deduplication logic against a real database. +// edge cases, and deduplication logic against a real database. HealthCheck +// and RenewalPolicy integration tests live in sibling *_test.go files in this +// package (see health_check_test.go and renewal_policy_test.go). package postgres_test import (