From e9bbf33193826d83a4c9090f31930d56abb82d2d Mon Sep 17 00:00:00 2001 From: shankar Date: Mon, 20 Apr 2026 18:53:01 +0000 Subject: [PATCH] G-1: renewal-policies API + frontend FK-drift fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three frontend call sites (OnboardingWizard.tsx:603, CertificatesPage.tsx:52, CertificateDetailPage.tsx:169) populated the renewal_policy_id dropdown from getPolicies() — the compliance-rule endpoint returning pol-* IDs — which violated the FK managed_certificates.renewal_policy_id REFERENCES renewal_policies(id) ON DELETE RESTRICT. Create would fail pg 23503 at insert. Backend (new): - RenewalPolicyRepository CRUD + ListAll/ExistsByID (pg 23503 → ErrRenewalPolicyInUse → HTTP 409; pg 23505 → ErrRenewalPolicyDuplicateName → HTTP 409) - RenewalPolicyService with repo-only constructor. Service sentinels var-alias the repo sentinels so errors.Is walks across layers. - RenewalPolicyHandler with validation bounds: name 1–255; renewal_window_days [1,365] default 30; max_retries [0,10] not defaulted; retry_interval_seconds [60,86400] default 3600; alert_thresholds_days [0,365] default [30,14,7,0]. Auto-generated IDs rp-. - Router registers 5 routes under /api/v1/renewal-policies[/{id}]. Frontend: - CertificatesPage/CertificateDetailPage/OnboardingWizard now call getRenewalPolicies() and render rp-* IDs. - client.ts adds getRenewalPolicies/createRenewalPolicy/updateRenewalPolicy/ deleteRenewalPolicy. types.ts adds the RenewalPolicy shape. OpenAPI: RenewalPolicies tag + 5 operations + 3 schemas (RenewalPolicy, RenewalPolicyCreateRequest, RenewalPolicyUpdateRequest). 409 responses on create/update duplicate-name and delete FK-in-use. No migration — renewal_policies table already exists from the initial schema (000001). Tests: - internal/service/renewal_policy_test.go: CRUD + validation + sentinel error wrapping. - internal/api/handler/renewal_policy_handler_test.go: handler endpoint contracts including 400/404/409. - web/src/api/client.test.ts: 4 subtests covering the 4 new API functions. Phase 3 gates all green: go vet, build, short tests, race tests (service/ handler/router/scheduler), staticcheck (G-1 packages), govulncheck (0 reachable), coverage (service 69.7%, handler 79.0%, domain 86.9%, middleware 80.6% — all above thresholds), tsc, vitest (256 passed), vite build, OpenAPI structural validation. --- api/openapi.yaml | 259 +++++++++++ cmd/server/main.go | 10 + internal/api/handler/renewal_policy.go | 243 ++++++++++ .../handler/renewal_policy_handler_test.go | 434 ++++++++++++++++++ internal/api/router/router.go | 17 +- internal/integration/lifecycle_test.go | 19 + internal/repository/interfaces.go | 42 +- .../repository/postgres/renewal_policy.go | 277 +++++++++-- internal/service/renewal_policy.go | 211 +++++++++ internal/service/renewal_policy_test.go | 341 ++++++++++++++ internal/service/testutil_test.go | 58 ++- web/src/api/client.test.ts | 58 +++ web/src/api/client.ts | 25 +- web/src/api/types.ts | 25 + web/src/pages/CertificateDetailPage.tsx | 16 +- web/src/pages/CertificatesPage.tsx | 10 +- web/src/pages/OnboardingWizard.test.tsx | 9 +- web/src/pages/OnboardingWizard.tsx | 8 +- 18 files changed, 2004 insertions(+), 58 deletions(-) create mode 100644 internal/api/handler/renewal_policy.go create mode 100644 internal/api/handler/renewal_policy_handler_test.go create mode 100644 internal/service/renewal_policy.go create mode 100644 internal/service/renewal_policy_test.go diff --git a/api/openapi.yaml b/api/openapi.yaml index 36f3170..0b6c5c9 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -42,6 +42,8 @@ tags: description: Job queue — issuance, renewal, deployment, validation - name: Policies description: Policy rules and violation tracking + - name: RenewalPolicies + description: Lifecycle renewal policies (distinct from compliance policy rules above) - name: Profiles description: Certificate enrollment profiles with crypto constraints - name: Teams @@ -1528,6 +1530,137 @@ paths: "500": $ref: "#/components/responses/InternalError" + # ─── Renewal Policies ──────────────────────────────────────────────── + # G-1: lifecycle policies (rp-* ids, table renewal_policies). DISTINCT from + # /api/v1/policies above, which returns compliance rules (pol-* ids, table + # policy_rules). `managed_certificates.renewal_policy_id` FK points at + # renewal_policies(id) — populating that dropdown from /api/v1/policies + # caused 23503 FK violations; hence this endpoint. + /api/v1/renewal-policies: + get: + tags: [RenewalPolicies] + summary: List renewal policies + operationId: listRenewalPolicies + parameters: + - $ref: "#/components/parameters/page" + - $ref: "#/components/parameters/per_page" + responses: + "200": + description: Paginated list of renewal policies + content: + application/json: + schema: + allOf: + - $ref: "#/components/schemas/PaginationEnvelope" + - type: object + properties: + data: + type: array + items: + $ref: "#/components/schemas/RenewalPolicy" + "500": + $ref: "#/components/responses/InternalError" + post: + tags: [RenewalPolicies] + summary: Create renewal policy + operationId: createRenewalPolicy + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/RenewalPolicyCreateRequest" + responses: + "201": + description: Renewal policy created + content: + application/json: + schema: + $ref: "#/components/schemas/RenewalPolicy" + "400": + $ref: "#/components/responses/BadRequest" + "409": + description: Duplicate policy name + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "500": + $ref: "#/components/responses/InternalError" + + /api/v1/renewal-policies/{id}: + get: + tags: [RenewalPolicies] + summary: Get renewal policy + operationId: getRenewalPolicy + parameters: + - $ref: "#/components/parameters/resourceId" + responses: + "200": + description: Renewal policy details + content: + application/json: + schema: + $ref: "#/components/schemas/RenewalPolicy" + "400": + $ref: "#/components/responses/BadRequest" + "404": + $ref: "#/components/responses/NotFound" + "500": + $ref: "#/components/responses/InternalError" + put: + tags: [RenewalPolicies] + summary: Update renewal policy + operationId: updateRenewalPolicy + parameters: + - $ref: "#/components/parameters/resourceId" + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/RenewalPolicyUpdateRequest" + responses: + "200": + description: Renewal policy updated + content: + application/json: + schema: + $ref: "#/components/schemas/RenewalPolicy" + "400": + $ref: "#/components/responses/BadRequest" + "404": + $ref: "#/components/responses/NotFound" + "409": + description: Duplicate policy name + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "500": + $ref: "#/components/responses/InternalError" + delete: + tags: [RenewalPolicies] + summary: Delete renewal policy + operationId: deleteRenewalPolicy + parameters: + - $ref: "#/components/parameters/resourceId" + responses: + "204": + description: Renewal policy deleted + "400": + $ref: "#/components/responses/BadRequest" + "404": + $ref: "#/components/responses/NotFound" + "409": + description: Policy in use by one or more certificates (FK restrict) + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "500": + $ref: "#/components/responses/InternalError" + # ─── Profiles ──────────────────────────────────────────────────────── /api/v1/profiles: get: @@ -3765,6 +3898,132 @@ components: type: string format: date-time + # ─── Renewal Policies ───────────────────────────────────────────── + # G-1: renewal_policies table — lifecycle policies, referenced by + # managed_certificates.renewal_policy_id ON DELETE RESTRICT. Distinct + # from PolicyRule above (compliance rules, table policy_rules). + RenewalPolicy: + type: object + required: + - id + - name + - renewal_window_days + - auto_renew + - max_retries + - retry_interval_seconds + - alert_thresholds_days + - created_at + - updated_at + properties: + id: + type: string + description: Human-readable ID, prefixed `rp-` (e.g., `rp-default`). + name: + type: string + description: Unique display name (UNIQUE in DB). + renewal_window_days: + type: integer + minimum: 1 + maximum: 365 + description: Days before expiry to trigger renewal. + auto_renew: + type: boolean + description: Whether renewal is triggered automatically by the scheduler. + max_retries: + type: integer + minimum: 0 + maximum: 10 + description: Maximum renewal retry attempts on failure. + retry_interval_seconds: + type: integer + minimum: 60 + maximum: 86400 + description: Seconds to wait between retry attempts. + alert_thresholds_days: + type: array + items: + type: integer + minimum: 0 + maximum: 365 + description: Days-before-expiry thresholds at which to emit alerts. + certificate_profile_id: + type: string + nullable: true + description: Optional certificate profile binding. Read-only at this endpoint; UI does not currently edit this field. + created_at: + type: string + format: date-time + updated_at: + type: string + format: date-time + + RenewalPolicyCreateRequest: + type: object + required: + - name + properties: + id: + type: string + description: Optional human-readable ID. Auto-generated from name when omitted. + name: + type: string + minLength: 1 + maxLength: 255 + renewal_window_days: + type: integer + minimum: 1 + maximum: 365 + default: 30 + auto_renew: + type: boolean + default: true + max_retries: + type: integer + minimum: 0 + maximum: 10 + description: Required. Not defaulted — 0 is a valid operator choice. + retry_interval_seconds: + type: integer + minimum: 60 + maximum: 86400 + default: 3600 + alert_thresholds_days: + type: array + items: + type: integer + minimum: 0 + maximum: 365 + default: [30, 14, 7, 0] + + RenewalPolicyUpdateRequest: + type: object + description: Partial update. Omitted fields are left unchanged. + properties: + name: + type: string + minLength: 1 + maxLength: 255 + renewal_window_days: + type: integer + minimum: 1 + maximum: 365 + auto_renew: + type: boolean + max_retries: + type: integer + minimum: 0 + maximum: 10 + retry_interval_seconds: + type: integer + minimum: 60 + maximum: 86400 + alert_thresholds_days: + type: array + items: + type: integer + minimum: 0 + maximum: 365 + # ─── Profiles ──────────────────────────────────────────────────── CertificateProfile: type: object diff --git a/cmd/server/main.go b/cmd/server/main.go index dd11311..84d872c 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -147,6 +147,11 @@ func main() { auditService := service.NewAuditService(auditRepo) policyService := service.NewPolicyService(policyRepo, auditService) policyService.SetCertRepo(certificateRepo) // D-008: CertificateLifetime arm needs CertificateVersion.NotBefore/NotAfter + // G-1: RenewalPolicyService — distinct from PolicyService (compliance rules). + // Drives /api/v1/renewal-policies CRUD; the service layer owns slugify + validation, + // the repo layer owns sentinel translation for 23505 (name UNIQUE) and 23503 + // (FK-RESTRICT against managed_certificates.renewal_policy_id). + renewalPolicyService := service.NewRenewalPolicyService(renewalPolicyRepo) certificateService := service.NewCertificateService(certificateRepo, policyService, auditService) notifierRegistry := make(map[string]service.Notifier) @@ -368,6 +373,10 @@ func main() { agentHandler := handler.NewAgentHandler(agentService) jobHandler := handler.NewJobHandler(jobService) policyHandler := handler.NewPolicyHandler(policyService) + // G-1: RenewalPolicyHandler — /api/v1/renewal-policies CRUD. Value-returning + // constructor matches the house pattern (PolicyHandler, IssuerHandler etc.); + // the registry stores it by value in HandlerRegistry.RenewalPolicies. + renewalPolicyHandler := handler.NewRenewalPolicyHandler(renewalPolicyService) profileHandler := handler.NewProfileHandler(profileService) teamHandler := handler.NewTeamHandler(teamService) ownerHandler := handler.NewOwnerHandler(ownerService) @@ -508,6 +517,7 @@ func main() { Agents: agentHandler, Jobs: jobHandler, Policies: policyHandler, + RenewalPolicies: renewalPolicyHandler, Profiles: profileHandler, Teams: teamHandler, Owners: ownerHandler, diff --git a/internal/api/handler/renewal_policy.go b/internal/api/handler/renewal_policy.go new file mode 100644 index 0000000..41b2424 --- /dev/null +++ b/internal/api/handler/renewal_policy.go @@ -0,0 +1,243 @@ +package handler + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "strconv" + "strings" + + "github.com/shankar0123/certctl/internal/api/middleware" + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/service" +) + +// RenewalPolicyService defines the service interface for renewal policy +// operations. G-1: all methods take ctx so the handler can propagate +// request-scoped cancellation/deadlines through the full stack. +type RenewalPolicyService interface { + ListRenewalPolicies(ctx context.Context, page, perPage int) ([]domain.RenewalPolicy, int64, error) + GetRenewalPolicy(ctx context.Context, id string) (*domain.RenewalPolicy, error) + CreateRenewalPolicy(ctx context.Context, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) + UpdateRenewalPolicy(ctx context.Context, id string, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) + DeleteRenewalPolicy(ctx context.Context, id string) error +} + +// RenewalPolicyHandler serves /api/v1/renewal-policies CRUD endpoints. +// +// G-1 design note: the service-level `ErrRenewalPolicyDuplicateName` / +// `ErrRenewalPolicyInUse` sentinels alias the repository sentinels (same var +// identity), so `errors.Is` walks transparently across layers. Delete/Update +// not-found detection intentionally uses a `strings.Contains(err.Error(), +// "not found")` substring check — the repo wraps `sql.ErrNoRows` as +// `fmt.Errorf("renewal policy not found: %s", id)` which strips the sentinel, +// and the handler red-tests' `ErrMockNotFound = errors.New("mock not found +// error")` follows the same substring convention. +type RenewalPolicyHandler struct { + svc RenewalPolicyService +} + +// NewRenewalPolicyHandler constructs the handler with its service dependency. +// Returned by value to match the house pattern (PolicyHandler, IssuerHandler +// etc.) — the registry stores handlers by value in router.HandlerRegistry. +func NewRenewalPolicyHandler(svc RenewalPolicyService) RenewalPolicyHandler { + return RenewalPolicyHandler{svc: svc} +} + +// ListRenewalPolicies lists all renewal policies (paginated). +// GET /api/v1/renewal-policies?page=1&per_page=50 +func (h RenewalPolicyHandler) ListRenewalPolicies(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + + requestID := middleware.GetRequestID(r.Context()) + + page := 1 + perPage := 50 + query := r.URL.Query() + if p := query.Get("page"); p != "" { + if parsed, err := strconv.Atoi(p); err == nil && parsed > 0 { + page = parsed + } + } + if pp := query.Get("per_page"); pp != "" { + if parsed, err := strconv.Atoi(pp); err == nil && parsed > 0 && parsed <= 500 { + perPage = parsed + } + } + + policies, total, err := h.svc.ListRenewalPolicies(r.Context(), page, perPage) + if err != nil { + ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to list renewal policies", requestID) + return + } + + response := PagedResponse{ + Data: policies, + Total: total, + Page: page, + PerPage: perPage, + } + + JSON(w, http.StatusOK, response) +} + +// GetRenewalPolicy retrieves a single renewal policy by ID. +// GET /api/v1/renewal-policies/{id} +func (h RenewalPolicyHandler) GetRenewalPolicy(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + + requestID := middleware.GetRequestID(r.Context()) + + id := strings.TrimPrefix(r.URL.Path, "/api/v1/renewal-policies/") + parts := strings.Split(id, "/") + if len(parts) == 0 || parts[0] == "" { + ErrorWithRequestID(w, http.StatusBadRequest, "Renewal policy ID is required", requestID) + return + } + id = parts[0] + + policy, err := h.svc.GetRenewalPolicy(r.Context(), id) + if err != nil { + // Matches the PolicyHandler.GetPolicy convention: any error from the + // service surfaces as 404. The repo wraps sql.ErrNoRows as + // "renewal policy not found: %s" and there's no other expected failure + // mode on Get — the caller gets a clean 404. + ErrorWithRequestID(w, http.StatusNotFound, "Renewal policy not found", requestID) + return + } + + JSON(w, http.StatusOK, policy) +} + +// CreateRenewalPolicy inserts a new renewal policy. +// POST /api/v1/renewal-policies +// +// Error mapping: +// - invalid JSON / missing name → 400 +// - ErrRenewalPolicyDuplicateName (pg 23505 on name UNIQUE) → 409 +// - anything else → 500 +func (h RenewalPolicyHandler) CreateRenewalPolicy(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + + requestID := middleware.GetRequestID(r.Context()) + + var rp domain.RenewalPolicy + if err := json.NewDecoder(r.Body).Decode(&rp); err != nil { + ErrorWithRequestID(w, http.StatusBadRequest, "Invalid request body", requestID) + return + } + + if err := ValidateRequired("name", rp.Name); err != nil { + ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID) + return + } + + created, err := h.svc.CreateRenewalPolicy(r.Context(), rp) + if err != nil { + if errors.Is(err, service.ErrRenewalPolicyDuplicateName) { + ErrorWithRequestID(w, http.StatusConflict, "A renewal policy with that name already exists", requestID) + return + } + ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to create renewal policy", requestID) + return + } + + JSON(w, http.StatusCreated, created) +} + +// UpdateRenewalPolicy replaces the fields of an existing renewal policy. +// PUT /api/v1/renewal-policies/{id} +// +// Error mapping: +// - invalid JSON / empty ID → 400 +// - ErrRenewalPolicyDuplicateName → 409 +// - error text contains "not found" → 404 (see struct doc comment re: substring check) +// - anything else → 500 +func (h RenewalPolicyHandler) UpdateRenewalPolicy(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPut { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + + requestID := middleware.GetRequestID(r.Context()) + + id := strings.TrimPrefix(r.URL.Path, "/api/v1/renewal-policies/") + parts := strings.Split(id, "/") + if len(parts) == 0 || parts[0] == "" { + ErrorWithRequestID(w, http.StatusBadRequest, "Renewal policy ID is required", requestID) + return + } + id = parts[0] + + var rp domain.RenewalPolicy + if err := json.NewDecoder(r.Body).Decode(&rp); err != nil { + ErrorWithRequestID(w, http.StatusBadRequest, "Invalid request body", requestID) + return + } + + updated, err := h.svc.UpdateRenewalPolicy(r.Context(), id, rp) + if err != nil { + if errors.Is(err, service.ErrRenewalPolicyDuplicateName) { + ErrorWithRequestID(w, http.StatusConflict, "A renewal policy with that name already exists", requestID) + return + } + if strings.Contains(err.Error(), "not found") { + ErrorWithRequestID(w, http.StatusNotFound, "Renewal policy not found", requestID) + return + } + ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to update renewal policy", requestID) + return + } + + JSON(w, http.StatusOK, updated) +} + +// DeleteRenewalPolicy removes a renewal policy. +// DELETE /api/v1/renewal-policies/{id} +// +// Error mapping: +// - empty ID (trailing slash) → 400 +// - ErrRenewalPolicyInUse (pg 23503 FK-RESTRICT against managed_certificates.renewal_policy_id) → 409 +// - error text contains "not found" → 404 +// - anything else → 500 +func (h RenewalPolicyHandler) DeleteRenewalPolicy(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + + requestID := middleware.GetRequestID(r.Context()) + + id := strings.TrimPrefix(r.URL.Path, "/api/v1/renewal-policies/") + parts := strings.Split(id, "/") + if len(parts) == 0 || parts[0] == "" { + ErrorWithRequestID(w, http.StatusBadRequest, "Renewal policy ID is required", requestID) + return + } + id = parts[0] + + if err := h.svc.DeleteRenewalPolicy(r.Context(), id); err != nil { + if errors.Is(err, service.ErrRenewalPolicyInUse) { + ErrorWithRequestID(w, http.StatusConflict, "Renewal policy is still referenced by managed certificates", requestID) + return + } + if strings.Contains(err.Error(), "not found") { + ErrorWithRequestID(w, http.StatusNotFound, "Renewal policy not found", requestID) + return + } + ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to delete renewal policy", requestID) + return + } + + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/api/handler/renewal_policy_handler_test.go b/internal/api/handler/renewal_policy_handler_test.go new file mode 100644 index 0000000..1aee979 --- /dev/null +++ b/internal/api/handler/renewal_policy_handler_test.go @@ -0,0 +1,434 @@ +package handler + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/service" +) + +// G-1 red tests: lock in the HTTP surface of /api/v1/renewal-policies before +// the production code exists. Every subtest here references a symbol that +// Phase 2b must introduce: +// +// - NewRenewalPolicyHandler(svc) (constructor) +// - RenewalPolicyService (service-layer interface, in this package) +// - handler.ListRenewalPolicies / GetRenewalPolicy / CreateRenewalPolicy / +// UpdateRenewalPolicy / DeleteRenewalPolicy +// - service.ErrRenewalPolicyDuplicateName (pg 23505 → 409 mapping) +// - service.ErrRenewalPolicyInUse (pg 23503 → 409 mapping) + +// MockRenewalPolicyService is a mock implementation of RenewalPolicyService. +type MockRenewalPolicyService struct { + ListRenewalPoliciesFn func(page, perPage int) ([]domain.RenewalPolicy, int64, error) + GetRenewalPolicyFn func(id string) (*domain.RenewalPolicy, error) + CreateRenewalPolicyFn func(rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) + UpdateRenewalPolicyFn func(id string, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) + DeleteRenewalPolicyFn func(id string) error +} + +func (m *MockRenewalPolicyService) ListRenewalPolicies(_ context.Context, page, perPage int) ([]domain.RenewalPolicy, int64, error) { + if m.ListRenewalPoliciesFn != nil { + return m.ListRenewalPoliciesFn(page, perPage) + } + return nil, 0, nil +} + +func (m *MockRenewalPolicyService) GetRenewalPolicy(_ context.Context, id string) (*domain.RenewalPolicy, error) { + if m.GetRenewalPolicyFn != nil { + return m.GetRenewalPolicyFn(id) + } + return nil, nil +} + +func (m *MockRenewalPolicyService) CreateRenewalPolicy(_ context.Context, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + if m.CreateRenewalPolicyFn != nil { + return m.CreateRenewalPolicyFn(rp) + } + return nil, nil +} + +func (m *MockRenewalPolicyService) UpdateRenewalPolicy(_ context.Context, id string, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + if m.UpdateRenewalPolicyFn != nil { + return m.UpdateRenewalPolicyFn(id, rp) + } + return nil, nil +} + +func (m *MockRenewalPolicyService) DeleteRenewalPolicy(_ context.Context, id string) error { + if m.DeleteRenewalPolicyFn != nil { + return m.DeleteRenewalPolicyFn(id) + } + return nil +} + +// ----- List ----- + +func TestListRenewalPolicies_Success(t *testing.T) { + now := time.Now() + rp1 := domain.RenewalPolicy{ + ID: "rp-default", Name: "Default", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + rp2 := domain.RenewalPolicy{ + ID: "rp-urgent", Name: "Urgent", RenewalWindowDays: 7, + MaxRetries: 5, RetryInterval: 600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + + mock := &MockRenewalPolicyService{ + ListRenewalPoliciesFn: func(page, perPage int) ([]domain.RenewalPolicy, int64, error) { + return []domain.RenewalPolicy{rp1, rp2}, 2, nil + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodGet, "/api/v1/renewal-policies", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.ListRenewalPolicies(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", w.Code) + } + + var resp PagedResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if resp.Total != 2 { + t.Errorf("expected total 2, got %d", resp.Total) + } +} + +func TestListRenewalPolicies_ServiceError(t *testing.T) { + mock := &MockRenewalPolicyService{ + ListRenewalPoliciesFn: func(page, perPage int) ([]domain.RenewalPolicy, int64, error) { + return nil, 0, ErrMockServiceFailed + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodGet, "/api/v1/renewal-policies", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.ListRenewalPolicies(w, req) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d", w.Code) + } +} + +func TestListRenewalPolicies_MethodNotAllowed(t *testing.T) { + handler := NewRenewalPolicyHandler(&MockRenewalPolicyService{}) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/renewal-policies", nil) + w := httptest.NewRecorder() + + handler.ListRenewalPolicies(w, req) + + if w.Code != http.StatusMethodNotAllowed { + t.Fatalf("expected status 405, got %d", w.Code) + } +} + +// ----- Get ----- + +func TestGetRenewalPolicy_Success(t *testing.T) { + now := time.Now() + mock := &MockRenewalPolicyService{ + GetRenewalPolicyFn: func(id string) (*domain.RenewalPolicy, error) { + return &domain.RenewalPolicy{ + ID: id, Name: "Default", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + }, nil + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodGet, "/api/v1/renewal-policies/rp-default", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.GetRenewalPolicy(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", w.Code) + } +} + +func TestGetRenewalPolicy_NotFound(t *testing.T) { + mock := &MockRenewalPolicyService{ + GetRenewalPolicyFn: func(id string) (*domain.RenewalPolicy, error) { + return nil, ErrMockNotFound + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodGet, "/api/v1/renewal-policies/nonexistent", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.GetRenewalPolicy(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", w.Code) + } +} + +// ----- Create ----- + +func TestCreateRenewalPolicy_Success(t *testing.T) { + now := time.Now() + mock := &MockRenewalPolicyService{ + CreateRenewalPolicyFn: func(rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + rp.ID = "rp-new" + rp.CreatedAt = now + rp.UpdatedAt = now + return &rp, nil + }, + } + + body := map[string]interface{}{ + "name": "New Policy", + "renewal_window_days": 30, + "max_retries": 3, + "retry_interval_seconds": 3600, + "auto_renew": true, + } + bodyBytes, _ := json.Marshal(body) + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodPost, "/api/v1/renewal-policies", bytes.NewReader(bodyBytes)) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.CreateRenewalPolicy(w, req) + + if w.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d", w.Code) + } +} + +func TestCreateRenewalPolicy_MissingName(t *testing.T) { + body := map[string]interface{}{ + "renewal_window_days": 30, + "max_retries": 3, + "retry_interval_seconds": 3600, + } + bodyBytes, _ := json.Marshal(body) + + handler := NewRenewalPolicyHandler(&MockRenewalPolicyService{}) + req := httptest.NewRequest(http.MethodPost, "/api/v1/renewal-policies", bytes.NewReader(bodyBytes)) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.CreateRenewalPolicy(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", w.Code) + } +} + +func TestCreateRenewalPolicy_InvalidJSON(t *testing.T) { + handler := NewRenewalPolicyHandler(&MockRenewalPolicyService{}) + req := httptest.NewRequest(http.MethodPost, "/api/v1/renewal-policies", bytes.NewReader([]byte("not json"))) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.CreateRenewalPolicy(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", w.Code) + } +} + +func TestCreateRenewalPolicy_DuplicateName(t *testing.T) { + // Service bubbles up ErrRenewalPolicyDuplicateName (pg 23505) → handler maps to 409. + mock := &MockRenewalPolicyService{ + CreateRenewalPolicyFn: func(rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + return nil, service.ErrRenewalPolicyDuplicateName + }, + } + + body := map[string]interface{}{ + "name": "Duplicate", + "renewal_window_days": 30, + "max_retries": 3, + "retry_interval_seconds": 3600, + } + bodyBytes, _ := json.Marshal(body) + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodPost, "/api/v1/renewal-policies", bytes.NewReader(bodyBytes)) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.CreateRenewalPolicy(w, req) + + if w.Code != http.StatusConflict { + t.Fatalf("expected status 409 on duplicate name, got %d", w.Code) + } +} + +func TestCreateRenewalPolicy_MethodNotAllowed(t *testing.T) { + handler := NewRenewalPolicyHandler(&MockRenewalPolicyService{}) + req := httptest.NewRequest(http.MethodGet, "/api/v1/renewal-policies", nil) + w := httptest.NewRecorder() + + handler.CreateRenewalPolicy(w, req) + + if w.Code != http.StatusMethodNotAllowed { + t.Fatalf("expected status 405, got %d", w.Code) + } +} + +// ----- Update ----- + +func TestUpdateRenewalPolicy_Success(t *testing.T) { + now := time.Now() + mock := &MockRenewalPolicyService{ + UpdateRenewalPolicyFn: func(id string, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + return &domain.RenewalPolicy{ + ID: id, Name: rp.Name, RenewalWindowDays: rp.RenewalWindowDays, + MaxRetries: rp.MaxRetries, RetryInterval: rp.RetryInterval, + AutoRenew: rp.AutoRenew, + CreatedAt: now, UpdatedAt: now, + }, nil + }, + } + + body := map[string]interface{}{ + "name": "Updated Policy", + "renewal_window_days": 45, + "max_retries": 5, + "retry_interval_seconds": 1800, + "auto_renew": true, + } + bodyBytes, _ := json.Marshal(body) + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodPut, "/api/v1/renewal-policies/rp-default", bytes.NewReader(bodyBytes)) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.UpdateRenewalPolicy(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", w.Code) + } +} + +func TestUpdateRenewalPolicy_NotFound(t *testing.T) { + mock := &MockRenewalPolicyService{ + UpdateRenewalPolicyFn: func(id string, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + return nil, ErrMockNotFound + }, + } + + body := map[string]interface{}{ + "name": "Updated", + "renewal_window_days": 30, + "max_retries": 3, + "retry_interval_seconds": 3600, + } + bodyBytes, _ := json.Marshal(body) + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodPut, "/api/v1/renewal-policies/rp-missing", bytes.NewReader(bodyBytes)) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.UpdateRenewalPolicy(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", w.Code) + } +} + +// ----- Delete ----- + +func TestDeleteRenewalPolicy_Success(t *testing.T) { + var deletedID string + mock := &MockRenewalPolicyService{ + DeleteRenewalPolicyFn: func(id string) error { + deletedID = id + return nil + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/renewal-policies/rp-default", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.DeleteRenewalPolicy(w, req) + + if w.Code != http.StatusNoContent { + t.Fatalf("expected status 204, got %d", w.Code) + } + if deletedID != "rp-default" { + t.Errorf("expected deleted ID 'rp-default', got '%s'", deletedID) + } +} + +func TestDeleteRenewalPolicy_NotFound(t *testing.T) { + mock := &MockRenewalPolicyService{ + DeleteRenewalPolicyFn: func(id string) error { + return ErrMockNotFound + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/renewal-policies/rp-missing", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.DeleteRenewalPolicy(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", w.Code) + } +} + +func TestDeleteRenewalPolicy_InUseConflict(t *testing.T) { + // Service bubbles up ErrRenewalPolicyInUse (pg 23503 FK-RESTRICT) → handler maps to 409. + mock := &MockRenewalPolicyService{ + DeleteRenewalPolicyFn: func(id string) error { + return service.ErrRenewalPolicyInUse + }, + } + + handler := NewRenewalPolicyHandler(mock) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/renewal-policies/rp-active", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.DeleteRenewalPolicy(w, req) + + if w.Code != http.StatusConflict { + t.Fatalf("expected status 409 on in-use conflict, got %d", w.Code) + } +} + +func TestDeleteRenewalPolicy_EmptyID(t *testing.T) { + handler := NewRenewalPolicyHandler(&MockRenewalPolicyService{}) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/renewal-policies/", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + + handler.DeleteRenewalPolicy(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", w.Code) + } +} diff --git a/internal/api/router/router.go b/internal/api/router/router.go index d52e0b5..6bb08e4 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -65,8 +65,9 @@ type HandlerRegistry struct { Verification handler.VerificationHandler Export handler.ExportHandler Digest handler.DigestHandler - HealthChecks *handler.HealthCheckHandler - BulkRevocation handler.BulkRevocationHandler + HealthChecks *handler.HealthCheckHandler + BulkRevocation handler.BulkRevocationHandler + RenewalPolicies handler.RenewalPolicyHandler } // RegisterHandlers sets up all API routes with their handlers. @@ -167,6 +168,18 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { r.Register("DELETE /api/v1/policies/{id}", http.HandlerFunc(reg.Policies.DeletePolicy)) r.Register("GET /api/v1/policies/{id}/violations", http.HandlerFunc(reg.Policies.ListViolations)) + // Renewal Policies routes: /api/v1/renewal-policies + // G-1: fixes frontend FK drift — OnboardingWizard + CertificatesPage dropdowns + // were previously populating renewal_policy_id from /api/v1/policies (compliance + // rules, pol-* IDs), violating FK managed_certificates.renewal_policy_id → + // renewal_policies(id) ON DELETE RESTRICT. This block is the backend half; the + // frontend half swaps getPolicies → getRenewalPolicies at 3 call sites. + r.Register("GET /api/v1/renewal-policies", http.HandlerFunc(reg.RenewalPolicies.ListRenewalPolicies)) + r.Register("POST /api/v1/renewal-policies", http.HandlerFunc(reg.RenewalPolicies.CreateRenewalPolicy)) + r.Register("GET /api/v1/renewal-policies/{id}", http.HandlerFunc(reg.RenewalPolicies.GetRenewalPolicy)) + r.Register("PUT /api/v1/renewal-policies/{id}", http.HandlerFunc(reg.RenewalPolicies.UpdateRenewalPolicy)) + r.Register("DELETE /api/v1/renewal-policies/{id}", http.HandlerFunc(reg.RenewalPolicies.DeleteRenewalPolicy)) + // Profiles routes: /api/v1/profiles r.Register("GET /api/v1/profiles", http.HandlerFunc(reg.Profiles.ListProfiles)) r.Register("POST /api/v1/profiles", http.HandlerFunc(reg.Profiles.CreateProfile)) diff --git a/internal/integration/lifecycle_test.go b/internal/integration/lifecycle_test.go index eed3b12..d0b22a7 100644 --- a/internal/integration/lifecycle_test.go +++ b/internal/integration/lifecycle_test.go @@ -1151,6 +1151,25 @@ func (m *mockRenewalPolicyRepository) List(ctx context.Context) ([]*domain.Renew return policies, nil } +// Create/Update/Delete satisfy the G-1 interface extension. The integration +// harness never drives the CRUD endpoints directly — these methods exist +// purely for interface compliance so the binary still builds. +func (m *mockRenewalPolicyRepository) Create(ctx context.Context, policy *domain.RenewalPolicy) error { + m.policies[policy.ID] = policy + return nil +} + +func (m *mockRenewalPolicyRepository) Update(ctx context.Context, id string, policy *domain.RenewalPolicy) error { + policy.ID = id + m.policies[id] = policy + return nil +} + +func (m *mockRenewalPolicyRepository) Delete(ctx context.Context, id string) error { + delete(m.policies, id) + return nil +} + type mockIssuerRepository struct { issuers map[string]*domain.Issuer } diff --git a/internal/repository/interfaces.go b/internal/repository/interfaces.go index 247d5d8..c1af13d 100644 --- a/internal/repository/interfaces.go +++ b/internal/repository/interfaces.go @@ -2,11 +2,27 @@ package repository import ( "context" + "errors" "time" "github.com/shankar0123/certctl/internal/domain" ) +// Repository-level sentinel errors. Repositories (primarily the postgres +// implementation) translate RDBMS-specific errors into these typed envelopes +// so the service/handler layers can branch with errors.Is without importing +// lib/pq or care about SQLSTATE codes. +// +// G-1: renewal-policy sentinels — DuplicateName → HTTP 409 (pg 23505 on +// renewal_policies.name UNIQUE), InUse → HTTP 409 (pg 23503 on the FK from +// managed_certificates.renewal_policy_id to renewal_policies.id with ON +// DELETE RESTRICT). Both map onto the same 409 status but with distinct +// messages so operators can tell them apart. +var ( + ErrRenewalPolicyDuplicateName = errors.New("renewal policy name already exists") + ErrRenewalPolicyInUse = errors.New("renewal policy is still referenced by managed certificates") +) + // CertificateRepository defines operations for managing certificates. type CertificateRepository interface { // List returns a paginated list of certificates matching the filter criteria. @@ -258,11 +274,35 @@ type JobRepository interface { } // RenewalPolicyRepository defines operations for managing renewal policies. +// +// G-1: extended with Create/Update/Delete so the new /api/v1/renewal-policies +// CRUD surface has a repo contract to lean on. Delete must map the PostgreSQL +// 23503 (foreign_key_violation on managed_certificates.renewal_policy_id +// REFERENCES renewal_policies(id) ON DELETE RESTRICT) onto the typed +// ErrRenewalPolicyInUse sentinel so the handler can emit a 409 Conflict +// instead of an opaque 500. Create/Update map PostgreSQL 23505 +// (unique_violation on renewal_policies.name) onto ErrRenewalPolicyDuplicateName +// for the same 409 Conflict reason. +// +// List stays single-shot (no pagination params) because the production row +// count is in the single digits — the service layer paginates/sorts in Go. +// Changing the signature would churn every mock without functional benefit. type RenewalPolicyRepository interface { // Get retrieves a renewal policy by ID. Get(ctx context.Context, id string) (*domain.RenewalPolicy, error) - // List returns all renewal policies. + // List returns all renewal policies, ordered by name. List(ctx context.Context) ([]*domain.RenewalPolicy, error) + // Create inserts a new renewal policy. The caller is responsible for + // populating Name; Create auto-generates ID (as rp-) if empty. + // Returns ErrRenewalPolicyDuplicateName on pg 23505. + Create(ctx context.Context, policy *domain.RenewalPolicy) error + // Update modifies an existing renewal policy in-place. Returns + // sql.ErrNoRows-wrapped error when id is unknown, or + // ErrRenewalPolicyDuplicateName on pg 23505 (name collision with another row). + Update(ctx context.Context, id string, policy *domain.RenewalPolicy) error + // Delete removes a renewal policy. Returns ErrRenewalPolicyInUse when the + // policy is still referenced by rows in managed_certificates (pg 23503). + Delete(ctx context.Context, id string) error } // PolicyRepository defines operations for managing compliance policies and violations. diff --git a/internal/repository/postgres/renewal_policy.go b/internal/repository/postgres/renewal_policy.go index d58a0d6..8b6f1bf 100644 --- a/internal/repository/postgres/renewal_policy.go +++ b/internal/repository/postgres/renewal_policy.go @@ -4,46 +4,61 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" + "regexp" + "strings" + + "github.com/lib/pq" "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/repository" ) -// RenewalPolicyRepository implements repository.RenewalPolicyRepository +// RenewalPolicyRepository implements repository.RenewalPolicyRepository. type RenewalPolicyRepository struct { db *sql.DB } -// NewRenewalPolicyRepository creates a new RenewalPolicyRepository +// NewRenewalPolicyRepository creates a new RenewalPolicyRepository. func NewRenewalPolicyRepository(db *sql.DB) *RenewalPolicyRepository { return &RenewalPolicyRepository{db: db} } -// Get retrieves a renewal policy by ID -func (r *RenewalPolicyRepository) Get(ctx context.Context, id string) (*domain.RenewalPolicy, error) { +// SELECT column order is the shared contract between scanRenewalPolicy and +// every SELECT/RETURNING in this file. Keep them in lockstep; if you add a +// new column, add it to all SELECTs, all scan calls, and scanRenewalPolicy. +// +// Note: certificate_profile_id and agent_group_id live on renewal_policies +// (migrations 000003 and 000004) but are deliberately NOT read here — that +// pre-existing drift is out of G-1's minimum-viable-delta and is tracked in +// the design doc §8. Introducing them would change struct shapes / JSON tags +// and require domain-layer churn we're not taking on in this change. +const renewalPolicyColumns = ` + id, name, renewal_window_days, auto_renew, max_retries, + retry_interval_minutes, alert_thresholds_days, created_at, updated_at +` + +// scanRenewalPolicy decodes one renewal_policies row from a Row or Rows +// scanner, unmarshaling alert_thresholds_days JSONB into the domain slice. +// Malformed JSONB silently falls back to DefaultAlertThresholds() — same +// behavior as the pre-G-1 code so we don't change observable semantics. +func scanRenewalPolicy(scanner interface { + Scan(dest ...any) error +}) (*domain.RenewalPolicy, error) { var policy domain.RenewalPolicy var thresholdsJSON []byte - err := r.db.QueryRowContext(ctx, ` - SELECT id, name, renewal_window_days, auto_renew, max_retries, - retry_interval_minutes, alert_thresholds_days, created_at, updated_at - FROM renewal_policies - WHERE id = $1 - `, id).Scan(&policy.ID, &policy.Name, &policy.RenewalWindowDays, &policy.AutoRenew, + if err := scanner.Scan( + &policy.ID, &policy.Name, &policy.RenewalWindowDays, &policy.AutoRenew, &policy.MaxRetries, &policy.RetryInterval, &thresholdsJSON, - &policy.CreatedAt, &policy.UpdatedAt) - - if err != nil { - if err == sql.ErrNoRows { - return nil, fmt.Errorf("renewal policy not found: %s", id) - } - return nil, fmt.Errorf("failed to query renewal policy: %w", err) + &policy.CreatedAt, &policy.UpdatedAt, + ); err != nil { + return nil, err } - // Parse alert thresholds from JSONB if len(thresholdsJSON) > 0 { if err := json.Unmarshal(thresholdsJSON, &policy.AlertThresholdsDays); err != nil { - // Fall back to defaults if JSON is malformed policy.AlertThresholdsDays = domain.DefaultAlertThresholds() } } @@ -51,14 +66,23 @@ func (r *RenewalPolicyRepository) Get(ctx context.Context, id string) (*domain.R return &policy, nil } -// List returns all renewal policies +// Get retrieves a renewal policy by ID. +func (r *RenewalPolicyRepository) Get(ctx context.Context, id string) (*domain.RenewalPolicy, error) { + row := r.db.QueryRowContext(ctx, `SELECT `+renewalPolicyColumns+` FROM renewal_policies WHERE id = $1`, id) + policy, err := scanRenewalPolicy(row) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("renewal policy not found: %s", id) + } + return nil, fmt.Errorf("failed to query renewal policy: %w", err) + } + return policy, nil +} + +// List returns all renewal policies, ordered by name (matches the index on +// renewal_policies.name from migration 000001 so ORDER BY is index-served). func (r *RenewalPolicyRepository) List(ctx context.Context) ([]*domain.RenewalPolicy, error) { - rows, err := r.db.QueryContext(ctx, ` - SELECT id, name, renewal_window_days, auto_renew, max_retries, - retry_interval_minutes, alert_thresholds_days, created_at, updated_at - FROM renewal_policies - ORDER BY name - `) + rows, err := r.db.QueryContext(ctx, `SELECT `+renewalPolicyColumns+` FROM renewal_policies ORDER BY name`) if err != nil { return nil, fmt.Errorf("failed to query renewal policies: %w", err) } @@ -66,22 +90,11 @@ func (r *RenewalPolicyRepository) List(ctx context.Context) ([]*domain.RenewalPo var policies []*domain.RenewalPolicy for rows.Next() { - var policy domain.RenewalPolicy - var thresholdsJSON []byte - - if err := rows.Scan(&policy.ID, &policy.Name, &policy.RenewalWindowDays, &policy.AutoRenew, - &policy.MaxRetries, &policy.RetryInterval, &thresholdsJSON, - &policy.CreatedAt, &policy.UpdatedAt); err != nil { + policy, err := scanRenewalPolicy(rows) + if err != nil { return nil, fmt.Errorf("failed to scan renewal policy: %w", err) } - - if len(thresholdsJSON) > 0 { - if err := json.Unmarshal(thresholdsJSON, &policy.AlertThresholdsDays); err != nil { - policy.AlertThresholdsDays = domain.DefaultAlertThresholds() - } - } - - policies = append(policies, &policy) + policies = append(policies, policy) } if err := rows.Err(); err != nil { @@ -90,3 +103,187 @@ func (r *RenewalPolicyRepository) List(ctx context.Context) ([]*domain.RenewalPo return policies, nil } + +// slugRegex matches non-alphanumeric characters that slugifyPolicyName strips. +var slugRegex = regexp.MustCompile(`[^a-z0-9-]+`) + +// slugifyPolicyName produces `rp-` for an auto-generated policy ID. +// Slug: lowercase, spaces→hyphens, non-alphanumeric stripped, trimmed to 64 +// chars. Matches the existing seed convention (rp-default, rp-standard, +// rp-urgent). Collision resolution is handled by Create's retry loop. +func slugifyPolicyName(name string) string { + slug := strings.ToLower(strings.TrimSpace(name)) + slug = strings.ReplaceAll(slug, " ", "-") + slug = slugRegex.ReplaceAllString(slug, "") + slug = strings.Trim(slug, "-") + if slug == "" { + slug = "policy" + } + if len(slug) > 64 { + slug = slug[:64] + } + return "rp-" + slug +} + +// isUniqueViolation reports whether err is a PostgreSQL 23505 unique_violation. +// Used by Create/Update to translate name-collision errors onto the typed +// ErrRenewalPolicyDuplicateName sentinel. +func isUniqueViolation(err error) bool { + var pqErr *pq.Error + return errors.As(err, &pqErr) && pqErr.Code == "23505" +} + +// isForeignKeyViolation reports whether err is a PostgreSQL 23503 +// foreign_key_violation. Used by Delete to translate ON DELETE RESTRICT +// failures onto the typed ErrRenewalPolicyInUse sentinel. +func isForeignKeyViolation(err error) bool { + var pqErr *pq.Error + return errors.As(err, &pqErr) && pqErr.Code == "23503" +} + +// Create inserts a new renewal policy. If policy.ID is empty, auto-generates +// `rp-` with -2/-3/... suffixes on collision (up to 10 attempts). +// Returns ErrRenewalPolicyDuplicateName on pg 23505 (name collision). +// +// alert_thresholds_days is marshaled to JSONB here rather than relying on the +// DB default because the service layer already applies DefaultAlertThresholds +// for empty input — the DB default is a safety net, not the primary path. +func (r *RenewalPolicyRepository) Create(ctx context.Context, policy *domain.RenewalPolicy) error { + if policy == nil { + return errors.New("renewal policy is nil") + } + + thresholdsJSON, err := json.Marshal(policy.AlertThresholdsDays) + if err != nil { + return fmt.Errorf("failed to marshal alert thresholds: %w", err) + } + + // ID auto-generation with collision retry. We attempt up to 10 suffix + // variants (rp-foo, rp-foo-2, ..., rp-foo-10) before giving up — the + // 23505 error the caller gets back past that point is on Name (their + // job to fix) rather than on a slug-collision we swallowed. + baseID := policy.ID + if baseID == "" { + baseID = slugifyPolicyName(policy.Name) + } + + insertSQL := ` + INSERT INTO renewal_policies ( + id, name, renewal_window_days, auto_renew, max_retries, + retry_interval_minutes, alert_thresholds_days, created_at, updated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, NOW(), NOW()) + RETURNING ` + renewalPolicyColumns + + maxAttempts := 10 + if policy.ID != "" { + // Caller supplied a specific ID — no collision-retry, just one shot. + maxAttempts = 1 + } + + for attempt := 1; attempt <= maxAttempts; attempt++ { + candidateID := baseID + if attempt > 1 { + candidateID = fmt.Sprintf("%s-%d", baseID, attempt) + } + + row := r.db.QueryRowContext(ctx, insertSQL, + candidateID, policy.Name, policy.RenewalWindowDays, policy.AutoRenew, + policy.MaxRetries, policy.RetryInterval, thresholdsJSON, + ) + + inserted, scanErr := scanRenewalPolicy(row) + if scanErr == nil { + *policy = *inserted + return nil + } + + if isUniqueViolation(scanErr) { + // Determine which unique constraint — if it's the name UNIQUE + // we can't recover (caller has to pick a new name); if it's the + // primary-key slug collision we loop to the next suffix. + var pqErr *pq.Error + errors.As(scanErr, &pqErr) + // Postgres reports the constraint name in pqErr.Constraint; + // renewal_policies_name_key is the name UNIQUE, renewal_policies_pkey + // is the PK. Name collision is terminal, PK collision is retryable. + if pqErr.Constraint != "" && !strings.Contains(pqErr.Constraint, "pkey") { + return repository.ErrRenewalPolicyDuplicateName + } + // PK collision — try next suffix. + continue + } + + return fmt.Errorf("failed to insert renewal policy: %w", scanErr) + } + + // Exhausted retry budget on PK collisions — surface as duplicate so the + // caller at least gets a 409 rather than a mysterious 500. + return repository.ErrRenewalPolicyDuplicateName +} + +// Update modifies an existing renewal policy by ID. Returns an error wrapping +// sql.ErrNoRows when id is unknown (detected by RETURNING returning zero rows), +// or ErrRenewalPolicyDuplicateName on pg 23505 (name collision with another row). +func (r *RenewalPolicyRepository) Update(ctx context.Context, id string, policy *domain.RenewalPolicy) error { + if policy == nil { + return errors.New("renewal policy is nil") + } + + thresholdsJSON, err := json.Marshal(policy.AlertThresholdsDays) + if err != nil { + return fmt.Errorf("failed to marshal alert thresholds: %w", err) + } + + row := r.db.QueryRowContext(ctx, ` + UPDATE renewal_policies SET + name = $2, + renewal_window_days = $3, + auto_renew = $4, + max_retries = $5, + retry_interval_minutes = $6, + alert_thresholds_days = $7, + updated_at = NOW() + WHERE id = $1 + RETURNING `+renewalPolicyColumns, + id, policy.Name, policy.RenewalWindowDays, policy.AutoRenew, + policy.MaxRetries, policy.RetryInterval, thresholdsJSON, + ) + + updated, err := scanRenewalPolicy(row) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("renewal policy not found: %s", id) + } + if isUniqueViolation(err) { + return repository.ErrRenewalPolicyDuplicateName + } + return fmt.Errorf("failed to update renewal policy: %w", err) + } + + *policy = *updated + return nil +} + +// Delete removes a renewal policy by ID. Returns ErrRenewalPolicyInUse when +// the policy is still referenced by rows in managed_certificates (pg 23503 +// foreign_key_violation against the ON DELETE RESTRICT FK from +// managed_certificates.renewal_policy_id). Returns an error wrapping +// sql.ErrNoRows when id is unknown. +func (r *RenewalPolicyRepository) Delete(ctx context.Context, id string) error { + result, err := r.db.ExecContext(ctx, `DELETE FROM renewal_policies WHERE id = $1`, id) + if err != nil { + if isForeignKeyViolation(err) { + return repository.ErrRenewalPolicyInUse + } + return fmt.Errorf("failed to delete renewal policy: %w", err) + } + + rows, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("failed to read RowsAffected for delete: %w", err) + } + if rows == 0 { + return fmt.Errorf("renewal policy not found: %s", id) + } + return nil +} diff --git a/internal/service/renewal_policy.go b/internal/service/renewal_policy.go new file mode 100644 index 0000000..3165f27 --- /dev/null +++ b/internal/service/renewal_policy.go @@ -0,0 +1,211 @@ +package service + +import ( + "context" + "fmt" + "regexp" + "strings" + "time" + + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/repository" +) + +// G-1: service-level sentinels alias the repository sentinels so errors.Is +// walks transparently across layers. Do NOT errors.New a fresh copy — the +// handler's `errors.Is(err, repository.ErrRenewalPolicyInUse)` branch and +// the service-layer tests' `errors.Is(err, service.ErrRenewalPolicyInUse)` +// branch need to match the same sentinel var identity. +var ( + ErrRenewalPolicyDuplicateName = repository.ErrRenewalPolicyDuplicateName + ErrRenewalPolicyInUse = repository.ErrRenewalPolicyInUse +) + +// RenewalPolicyService implements the /api/v1/renewal-policies CRUD surface. +// +// G-1 scope note: the red-test contract pins NewRenewalPolicyService to a +// repo-only signature (no auditService). Renewal-policy CRUD does not emit +// audit events in this change — if audit coverage is needed later, add a +// SetAuditService setter rather than churning the constructor signature. +type RenewalPolicyService struct { + repo repository.RenewalPolicyRepository +} + +// NewRenewalPolicyService constructs the service bound to its repository. +func NewRenewalPolicyService(repo repository.RenewalPolicyRepository) *RenewalPolicyService { + return &RenewalPolicyService{repo: repo} +} + +// rpSlugRegex matches non-alphanumeric characters that slugifyRenewalPolicyName strips. +// Mirrors the identical regex in internal/repository/postgres/renewal_policy.go — +// the service owns the rp- convention so the repo's retry loop is a +// pure PK-collision safety net, not the primary ID generator. +var rpSlugRegex = regexp.MustCompile(`[^a-z0-9-]+`) + +// slugifyRenewalPolicyName produces `rp-` for an auto-generated policy +// ID. Slug: lowercase, spaces→hyphens, non-alphanumeric stripped, trimmed to +// 64 chars. Matches the seed convention (rp-default, rp-standard, rp-urgent) +// and the repo's slugifyPolicyName byte-for-byte. +func slugifyRenewalPolicyName(name string) string { + slug := strings.ToLower(strings.TrimSpace(name)) + slug = strings.ReplaceAll(slug, " ", "-") + slug = rpSlugRegex.ReplaceAllString(slug, "") + slug = strings.Trim(slug, "-") + if slug == "" { + slug = "policy" + } + if len(slug) > 64 { + slug = slug[:64] + } + return "rp-" + slug +} + +// ListRenewalPolicies returns a single page of renewal policies sorted by +// name (the repo's ORDER BY name is index-served via idx_renewal_policies_name). +// Pagination is done in Go rather than SQL — the expected row count is in the +// single digits so LIMIT/OFFSET would be premature optimization and would +// churn the repo contract for no measurable benefit (design doc §Known +// Caller Audit). +// +// Bounds: page defaults to 1, per_page defaults to 50, caps at 500 to match +// the /api/v1/policies handler's behavior. Past-end slices return an empty +// slice with no error — callers use `total` to detect end of pagination. +func (s *RenewalPolicyService) ListRenewalPolicies(ctx context.Context, page, perPage int) ([]domain.RenewalPolicy, int64, error) { + if page < 1 { + page = 1 + } + if perPage < 1 { + perPage = 50 + } + if perPage > 500 { + perPage = 500 + } + + items, err := s.repo.List(ctx) + if err != nil { + return nil, 0, fmt.Errorf("failed to list renewal policies: %w", err) + } + + total := int64(len(items)) + start := (page - 1) * perPage + if start >= int(total) { + return nil, total, nil + } + end := start + perPage + if end > int(total) { + end = int(total) + } + + out := make([]domain.RenewalPolicy, 0, end-start) + for _, p := range items[start:end] { + if p != nil { + out = append(out, *p) + } + } + return out, total, nil +} + +// GetRenewalPolicy retrieves one renewal policy by ID. Not-found errors +// surface from the repo verbatim; the handler translates them to 404. +func (s *RenewalPolicyService) GetRenewalPolicy(ctx context.Context, id string) (*domain.RenewalPolicy, error) { + return s.repo.Get(ctx, id) +} + +// validateBounds enforces the design doc §Validation Bounds invariants: +// - name required, ≤ 255 chars +// - renewal_window_days in [1, 365] +// - max_retries in [0, 10] +// - retry_interval_seconds in [60, 86400] +// - alert_thresholds_days each in [0, 365] +// +// Called after applyCreateDefaults so zero-value fields that the caller +// expects to be defaulted don't trip the range checks. +func (s *RenewalPolicyService) validateBounds(rp *domain.RenewalPolicy) error { + if strings.TrimSpace(rp.Name) == "" { + return fmt.Errorf("name is required") + } + if len(rp.Name) > 255 { + return fmt.Errorf("name must be 255 characters or fewer, got %d", len(rp.Name)) + } + if rp.RenewalWindowDays < 1 || rp.RenewalWindowDays > 365 { + return fmt.Errorf("renewal_window_days must be between 1 and 365, got %d", rp.RenewalWindowDays) + } + if rp.MaxRetries < 0 || rp.MaxRetries > 10 { + return fmt.Errorf("max_retries must be between 0 and 10, got %d", rp.MaxRetries) + } + if rp.RetryInterval < 60 || rp.RetryInterval > 86400 { + return fmt.Errorf("retry_interval_seconds must be between 60 and 86400, got %d", rp.RetryInterval) + } + for i, t := range rp.AlertThresholdsDays { + if t < 0 || t > 365 { + return fmt.Errorf("alert_thresholds_days[%d]=%d must be between 0 and 365", i, t) + } + } + return nil +} + +// applyCreateDefaults fills in zero-valued optional fields with the design +// doc defaults. Name is never defaulted — missing name fails validation. +// MaxRetries=0 is a legal explicit value (no retries), so it is NOT +// defaulted; the DB default column handles that path if needed. +func (s *RenewalPolicyService) applyCreateDefaults(rp *domain.RenewalPolicy) { + if rp.RenewalWindowDays == 0 { + rp.RenewalWindowDays = 30 + } + if rp.RetryInterval == 0 { + rp.RetryInterval = 3600 + } + if len(rp.AlertThresholdsDays) == 0 { + rp.AlertThresholdsDays = domain.DefaultAlertThresholds() + } +} + +// CreateRenewalPolicy inserts a new renewal policy. Auto-generates +// `rp-` for ID if empty. Defaults are applied before bounds +// validation so a caller can omit RenewalWindowDays / RetryInterval and +// still pass bounds. Returns ErrRenewalPolicyDuplicateName unwrapped from +// the repo when a name collision occurs (pg 23505 on the UNIQUE constraint); +// the handler surfaces that as 409 Conflict. +func (s *RenewalPolicyService) CreateRenewalPolicy(ctx context.Context, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + s.applyCreateDefaults(&rp) + if err := s.validateBounds(&rp); err != nil { + return nil, err + } + if rp.ID == "" { + rp.ID = slugifyRenewalPolicyName(rp.Name) + } + if rp.CreatedAt.IsZero() { + rp.CreatedAt = time.Now() + } + if err := s.repo.Create(ctx, &rp); err != nil { + // Propagate repository sentinels verbatim — service-level sentinels + // alias repo sentinels (same var identity), so errors.Is walks + // through without any translation. + return nil, err + } + return &rp, nil +} + +// UpdateRenewalPolicy replaces the fields of an existing renewal policy. +// Applies the same defaults+bounds as Create so partial updates do not slip +// an invalid row past validation via zero-value fields. id in the path wins +// over any id the caller supplied in the body. +func (s *RenewalPolicyService) UpdateRenewalPolicy(ctx context.Context, id string, rp domain.RenewalPolicy) (*domain.RenewalPolicy, error) { + s.applyCreateDefaults(&rp) + if err := s.validateBounds(&rp); err != nil { + return nil, err + } + rp.ID = id + if err := s.repo.Update(ctx, id, &rp); err != nil { + return nil, err + } + return &rp, nil +} + +// DeleteRenewalPolicy removes a renewal policy. Returns ErrRenewalPolicyInUse +// when the policy is still referenced by rows in managed_certificates (the +// repo translates pg 23503 FK_RESTRICT violations onto that sentinel). The +// handler surfaces that as 409 Conflict. +func (s *RenewalPolicyService) DeleteRenewalPolicy(ctx context.Context, id string) error { + return s.repo.Delete(ctx, id) +} diff --git a/internal/service/renewal_policy_test.go b/internal/service/renewal_policy_test.go new file mode 100644 index 0000000..b9ef3d5 --- /dev/null +++ b/internal/service/renewal_policy_test.go @@ -0,0 +1,341 @@ +package service + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/domain" +) + +// G-1 red tests: lock in the behavior of RenewalPolicyService before +// the production code exists. Every subtest here references a type or +// method that Phase 2b must introduce: +// +// - NewRenewalPolicyService(repo) (constructor) +// - svc.ListRenewalPolicies(ctx, page, pp) ([]RenewalPolicy, int64, error) +// - svc.GetRenewalPolicy(ctx, id) (*RenewalPolicy, error) +// - svc.CreateRenewalPolicy(ctx, rp) (*RenewalPolicy, error) +// - svc.UpdateRenewalPolicy(ctx, id, rp) (*RenewalPolicy, error) +// - svc.DeleteRenewalPolicy(ctx, id) error +// - ErrRenewalPolicyDuplicateName sentinel (pg 23505 → 409) +// - ErrRenewalPolicyInUse sentinel (pg 23503 → 409) +// +// Once Phase 2b lands, these should all turn green without modification. + +func TestRenewalPolicyService_List_Success(t *testing.T) { + ctx := context.Background() + now := time.Now() + repo := &mockRenewalPolicyRepo{ + Policies: map[string]*domain.RenewalPolicy{ + "rp-default": { + ID: "rp-default", Name: "Default", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + }, + "rp-urgent": { + ID: "rp-urgent", Name: "Urgent", RenewalWindowDays: 7, + MaxRetries: 5, RetryInterval: 600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + }, + }, + } + svc := NewRenewalPolicyService(repo) + + items, total, err := svc.ListRenewalPolicies(ctx, 1, 50) + if err != nil { + t.Fatalf("ListRenewalPolicies failed: %v", err) + } + if total != 2 { + t.Errorf("expected total 2, got %d", total) + } + if len(items) != 2 { + t.Errorf("expected 2 items, got %d", len(items)) + } +} + +func TestRenewalPolicyService_List_Empty(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + items, total, err := svc.ListRenewalPolicies(ctx, 1, 50) + if err != nil { + t.Fatalf("ListRenewalPolicies failed: %v", err) + } + if total != 0 { + t.Errorf("expected total 0, got %d", total) + } + if len(items) != 0 { + t.Errorf("expected 0 items, got %d", len(items)) + } +} + +func TestRenewalPolicyService_List_Pagination(t *testing.T) { + ctx := context.Background() + now := time.Now() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + // Seed 5 policies, names A..E so the mock's sort.Slice yields a deterministic + // ordering that pagination boundaries can assert against. + for _, name := range []string{"A", "B", "C", "D", "E"} { + p := &domain.RenewalPolicy{ + ID: "rp-" + strings.ToLower(name), Name: name, + RenewalWindowDays: 30, MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + repo.Policies[p.ID] = p + } + svc := NewRenewalPolicyService(repo) + + // Page 1, size 2 → [A, B] + page1, total, err := svc.ListRenewalPolicies(ctx, 1, 2) + if err != nil { + t.Fatalf("page 1 failed: %v", err) + } + if total != 5 { + t.Errorf("expected total 5, got %d", total) + } + if len(page1) != 2 || page1[0].Name != "A" || page1[1].Name != "B" { + t.Errorf("unexpected page 1 slice: %+v", page1) + } + + // Page 3, size 2 → [E] (single-item last page) + page3, _, err := svc.ListRenewalPolicies(ctx, 3, 2) + if err != nil { + t.Fatalf("page 3 failed: %v", err) + } + if len(page3) != 1 || page3[0].Name != "E" { + t.Errorf("unexpected page 3 slice: %+v", page3) + } + + // Page 4, size 2 → [] (past the end, no error) + page4, _, err := svc.ListRenewalPolicies(ctx, 4, 2) + if err != nil { + t.Fatalf("page 4 failed: %v", err) + } + if len(page4) != 0 { + t.Errorf("expected empty past-end slice, got %+v", page4) + } +} + +func TestRenewalPolicyService_List_RepoError(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{ + Policies: map[string]*domain.RenewalPolicy{}, + ListErr: errors.New("boom"), + } + svc := NewRenewalPolicyService(repo) + + _, _, err := svc.ListRenewalPolicies(ctx, 1, 50) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestRenewalPolicyService_Get_Success(t *testing.T) { + ctx := context.Background() + now := time.Now() + rp := &domain.RenewalPolicy{ + ID: "rp-default", Name: "Default", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{"rp-default": rp}} + svc := NewRenewalPolicyService(repo) + + got, err := svc.GetRenewalPolicy(ctx, "rp-default") + if err != nil { + t.Fatalf("GetRenewalPolicy failed: %v", err) + } + if got.Name != "Default" { + t.Errorf("expected name Default, got %s", got.Name) + } +} + +func TestRenewalPolicyService_Get_NotFound(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + _, err := svc.GetRenewalPolicy(ctx, "rp-missing") + if err == nil { + t.Fatal("expected error for missing policy, got nil") + } +} + +func TestRenewalPolicyService_Create_Success(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + rp := domain.RenewalPolicy{ + Name: "Weekly Renewal", + RenewalWindowDays: 7, + MaxRetries: 3, + RetryInterval: 3600, + AutoRenew: true, + } + created, err := svc.CreateRenewalPolicy(ctx, rp) + if err != nil { + t.Fatalf("CreateRenewalPolicy failed: %v", err) + } + if created.ID == "" { + t.Fatal("expected auto-generated ID, got empty") + } + // ID convention: rp- matches seed rows rp-default/rp-standard/rp-urgent. + if !strings.HasPrefix(created.ID, "rp-") { + t.Errorf("expected ID prefix rp-, got %s", created.ID) + } + if created.CreatedAt.IsZero() { + t.Error("expected CreatedAt to be populated") + } +} + +func TestRenewalPolicyService_Create_MissingName(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + _, err := svc.CreateRenewalPolicy(ctx, domain.RenewalPolicy{ + RenewalWindowDays: 30, MaxRetries: 3, RetryInterval: 3600, + }) + if err == nil { + t.Fatal("expected validation error for missing name, got nil") + } +} + +func TestRenewalPolicyService_Create_BoundsViolation(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + // RenewalWindowDays out of range [1, 365] + _, err := svc.CreateRenewalPolicy(ctx, domain.RenewalPolicy{ + Name: "Bad Window", + RenewalWindowDays: 999, + MaxRetries: 3, + RetryInterval: 3600, + }) + if err == nil { + t.Fatal("expected bounds violation on RenewalWindowDays, got nil") + } +} + +func TestRenewalPolicyService_Create_DuplicateName(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{ + Policies: map[string]*domain.RenewalPolicy{}, + CreateErr: ErrRenewalPolicyDuplicateName, + } + svc := NewRenewalPolicyService(repo) + + _, err := svc.CreateRenewalPolicy(ctx, domain.RenewalPolicy{ + Name: "Duplicate", + RenewalWindowDays: 30, + MaxRetries: 3, + RetryInterval: 3600, + }) + if err == nil { + t.Fatal("expected duplicate-name error, got nil") + } + if !errors.Is(err, ErrRenewalPolicyDuplicateName) { + t.Errorf("expected ErrRenewalPolicyDuplicateName, got %v", err) + } +} + +func TestRenewalPolicyService_Update_Success(t *testing.T) { + ctx := context.Background() + now := time.Now() + rp := &domain.RenewalPolicy{ + ID: "rp-default", Name: "Default", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{"rp-default": rp}} + svc := NewRenewalPolicyService(repo) + + updated, err := svc.UpdateRenewalPolicy(ctx, "rp-default", domain.RenewalPolicy{ + Name: "Default Renamed", + RenewalWindowDays: 45, + MaxRetries: 5, + RetryInterval: 1800, + AutoRenew: true, + }) + if err != nil { + t.Fatalf("UpdateRenewalPolicy failed: %v", err) + } + if updated.Name != "Default Renamed" { + t.Errorf("expected updated name, got %s", updated.Name) + } + if updated.RenewalWindowDays != 45 { + t.Errorf("expected window 45, got %d", updated.RenewalWindowDays) + } +} + +func TestRenewalPolicyService_Update_NotFound(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + _, err := svc.UpdateRenewalPolicy(ctx, "rp-missing", domain.RenewalPolicy{ + Name: "X", RenewalWindowDays: 30, MaxRetries: 3, RetryInterval: 3600, + }) + if err == nil { + t.Fatal("expected error for missing policy, got nil") + } +} + +func TestRenewalPolicyService_Delete_Success(t *testing.T) { + ctx := context.Background() + now := time.Now() + rp := &domain.RenewalPolicy{ + ID: "rp-default", Name: "Default", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{"rp-default": rp}} + svc := NewRenewalPolicyService(repo) + + if err := svc.DeleteRenewalPolicy(ctx, "rp-default"); err != nil { + t.Fatalf("DeleteRenewalPolicy failed: %v", err) + } + if _, exists := repo.Policies["rp-default"]; exists { + t.Error("expected policy to be removed from repo") + } +} + +func TestRenewalPolicyService_Delete_NotFound(t *testing.T) { + ctx := context.Background() + repo := &mockRenewalPolicyRepo{Policies: map[string]*domain.RenewalPolicy{}} + svc := NewRenewalPolicyService(repo) + + err := svc.DeleteRenewalPolicy(ctx, "rp-missing") + if err == nil { + t.Fatal("expected error for missing policy, got nil") + } +} + +func TestRenewalPolicyService_Delete_InUseConflict(t *testing.T) { + ctx := context.Background() + now := time.Now() + rp := &domain.RenewalPolicy{ + ID: "rp-active", Name: "Active", RenewalWindowDays: 30, + MaxRetries: 3, RetryInterval: 3600, AutoRenew: true, + CreatedAt: now, UpdatedAt: now, + } + repo := &mockRenewalPolicyRepo{ + Policies: map[string]*domain.RenewalPolicy{"rp-active": rp}, + DeleteErr: ErrRenewalPolicyInUse, + } + svc := NewRenewalPolicyService(repo) + + err := svc.DeleteRenewalPolicy(ctx, "rp-active") + if err == nil { + t.Fatal("expected in-use conflict error, got nil") + } + if !errors.Is(err, ErrRenewalPolicyInUse) { + t.Errorf("expected ErrRenewalPolicyInUse, got %v", err) + } +} diff --git a/internal/service/testutil_test.go b/internal/service/testutil_test.go index a05e176..6f71999 100644 --- a/internal/service/testutil_test.go +++ b/internal/service/testutil_test.go @@ -749,11 +749,21 @@ func (m *mockPolicyRepo) AddRule(rule *domain.PolicyRule) { m.Rules[rule.ID] = rule } -// mockRenewalPolicyRepo is a test implementation of RenewalPolicyRepository +// mockRenewalPolicyRepo is a test implementation of RenewalPolicyRepository. +// +// G-1: repo contract extended with Create/Update/Delete to support the +// /api/v1/renewal-policies CRUD endpoints. Per-method *Err fields let tests +// force specific repo failures (duplicate name → 23505, FK RESTRICT on Delete +// → 23503) without standing up a real Postgres connection. The sentinel +// errors `ErrRenewalPolicyDuplicateName` and `ErrRenewalPolicyInUse` are the +// typed envelopes the service / handler layers translate into 409 Conflict. type mockRenewalPolicyRepo struct { - Policies map[string]*domain.RenewalPolicy - GetErr error - ListErr error + Policies map[string]*domain.RenewalPolicy + GetErr error + ListErr error + CreateErr error + UpdateErr error + DeleteErr error } func (m *mockRenewalPolicyRepo) Get(ctx context.Context, id string) (*domain.RenewalPolicy, error) { @@ -775,9 +785,49 @@ func (m *mockRenewalPolicyRepo) List(ctx context.Context) ([]*domain.RenewalPoli for _, p := range m.Policies { policies = append(policies, p) } + // Deterministic ordering mirrors the production repo's ORDER BY name, + // so pagination-boundary assertions don't become flaky under map + // iteration randomness. + sort.Slice(policies, func(i, j int) bool { + return policies[i].Name < policies[j].Name + }) return policies, nil } +func (m *mockRenewalPolicyRepo) Create(ctx context.Context, policy *domain.RenewalPolicy) error { + if m.CreateErr != nil { + return m.CreateErr + } + if _, exists := m.Policies[policy.ID]; exists { + return m.CreateErr + } + m.Policies[policy.ID] = policy + return nil +} + +func (m *mockRenewalPolicyRepo) Update(ctx context.Context, id string, policy *domain.RenewalPolicy) error { + if m.UpdateErr != nil { + return m.UpdateErr + } + if _, exists := m.Policies[id]; !exists { + return errNotFound + } + policy.ID = id + m.Policies[id] = policy + return nil +} + +func (m *mockRenewalPolicyRepo) Delete(ctx context.Context, id string) error { + if m.DeleteErr != nil { + return m.DeleteErr + } + if _, exists := m.Policies[id]; !exists { + return errNotFound + } + delete(m.Policies, id) + return nil +} + func (m *mockRenewalPolicyRepo) AddPolicy(policy *domain.RenewalPolicy) { m.Policies[policy.ID] = policy } diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index d776c6c..8ece8b0 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -33,6 +33,10 @@ import { updatePolicy, deletePolicy, getPolicyViolations, + getRenewalPolicies, + createRenewalPolicy, + updateRenewalPolicy, + deleteRenewalPolicy, getIssuers, createIssuer, testIssuerConnection, @@ -575,6 +579,60 @@ describe('API Client', () => { }); }); + // ─── Renewal Policies (G-1) ───────────────────────── + // Distinct from compliance Policies above. Populates the + // `renewal_policy_id` dropdown on OnboardingWizard + CertificatesPage + + // CertificateDetailPage.InlinePolicyEditor. Hits `/api/v1/renewal-policies`. + + describe('RenewalPolicies', () => { + it('getRenewalPolicies sends GET', async () => { + mockFetch.mockReturnValueOnce(mockJsonResponse({ data: [], total: 0, page: 1, per_page: 50 })); + await getRenewalPolicies(); + expect(mockFetch.mock.calls[0][0]).toContain('/api/v1/renewal-policies'); + }); + + it('createRenewalPolicy sends POST with body', async () => { + mockFetch.mockReturnValueOnce( + mockJsonResponse({ + id: 'rp-new', + name: 'New Policy', + renewal_window_days: 30, + max_retries: 3, + retry_interval_seconds: 3600, + auto_renew: true, + }), + ); + await createRenewalPolicy({ + name: 'New Policy', + renewal_window_days: 30, + max_retries: 3, + retry_interval_seconds: 3600, + auto_renew: true, + }); + const [url, init] = mockFetch.mock.calls[0]; + expect(url).toBe('/api/v1/renewal-policies'); + expect(init.method).toBe('POST'); + expect(JSON.parse(init.body).name).toBe('New Policy'); + }); + + it('updateRenewalPolicy sends PUT with partial data', async () => { + mockFetch.mockReturnValueOnce(mockJsonResponse({ id: 'rp-default', name: 'Renamed' })); + await updateRenewalPolicy('rp-default', { name: 'Renamed' }); + const [url, init] = mockFetch.mock.calls[0]; + expect(url).toBe('/api/v1/renewal-policies/rp-default'); + expect(init.method).toBe('PUT'); + expect(JSON.parse(init.body)).toEqual({ name: 'Renamed' }); + }); + + it('deleteRenewalPolicy sends DELETE', async () => { + mockFetch.mockReturnValueOnce(mockJsonResponse({ message: 'deleted' })); + await deleteRenewalPolicy('rp-default'); + const [url, init] = mockFetch.mock.calls[0]; + expect(url).toBe('/api/v1/renewal-policies/rp-default'); + expect(init.method).toBe('DELETE'); + }); + }); + // ─── Issuers ──────────────────────────────────────── describe('Issuers', () => { diff --git a/web/src/api/client.ts b/web/src/api/client.ts index e7b9c2e..8de0291 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1,4 +1,4 @@ -import type { Certificate, CertificateVersion, Agent, Job, Notification, AuditEvent, PolicyRule, PolicyViolation, Issuer, Target, CertificateProfile, Owner, Team, AgentGroup, PaginatedResponse, DashboardSummary, CertificateStatusCount, ExpirationBucket, JobTrendDataPoint, IssuanceRateDataPoint, MetricsResponse, DiscoveredCertificate, DiscoveryScan, DiscoverySummary, NetworkScanTarget, EndpointHealthCheck, HealthHistoryEntry, HealthCheckSummary, AgentDependencyCounts, RetireAgentResponse, BlockedByDependenciesResponse } from './types'; +import type { Certificate, CertificateVersion, Agent, Job, Notification, AuditEvent, PolicyRule, PolicyViolation, RenewalPolicy, Issuer, Target, CertificateProfile, Owner, Team, AgentGroup, PaginatedResponse, DashboardSummary, CertificateStatusCount, ExpirationBucket, JobTrendDataPoint, IssuanceRateDataPoint, MetricsResponse, DiscoveredCertificate, DiscoveryScan, DiscoverySummary, NetworkScanTarget, EndpointHealthCheck, HealthHistoryEntry, HealthCheckSummary, AgentDependencyCounts, RetireAgentResponse, BlockedByDependenciesResponse } from './types'; const BASE = '/api/v1'; @@ -344,6 +344,29 @@ export const deletePolicy = (id: string) => export const getPolicyViolations = (id: string) => fetchJSON>(`${BASE}/policies/${id}/violations`); +// G-1: Renewal Policies (/api/v1/renewal-policies) — lifecycle policies with +// rp-* IDs in the renewal_policies table. Distinct from getPolicies() above +// which hits /api/v1/policies and returns PolicyRule (compliance, pol-* IDs). +// OnboardingWizard, CertificatesPage, and CertificateDetailPage populate the +// `renewal_policy_id` dropdown from this endpoint; populating it from +// getPolicies() produced FK violations on certificate insert/update. +export const getRenewalPolicies = (page = 1, perPage = 50) => { + const qs = new URLSearchParams({ page: String(page), per_page: String(perPage) }).toString(); + return fetchJSON>(`${BASE}/renewal-policies?${qs}`); +}; + +export const getRenewalPolicy = (id: string) => + fetchJSON(`${BASE}/renewal-policies/${id}`); + +export const createRenewalPolicy = (data: Partial) => + fetchJSON(`${BASE}/renewal-policies`, { method: 'POST', body: JSON.stringify(data) }); + +export const updateRenewalPolicy = (id: string, data: Partial) => + fetchJSON(`${BASE}/renewal-policies/${id}`, { method: 'PUT', body: JSON.stringify(data) }); + +export const deleteRenewalPolicy = (id: string) => + fetchJSON(`${BASE}/renewal-policies/${id}`, { method: 'DELETE' }); + // Issuers export const getIssuers = (params: Record = {}) => { const qs = new URLSearchParams({ page: '1', per_page: '50', ...params }).toString(); diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 9beb046..a8d5ab4 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -228,6 +228,31 @@ export interface PolicyViolation { created_at: string; } +/** + * G-1: RenewalPolicy is the lifecycle policy attached to managed certificates + * via `managed_certificates.renewal_policy_id` (FK ON DELETE RESTRICT → `rp-*` + * IDs in the `renewal_policies` table). Distinct from `PolicyRule` above, which + * models compliance rules in the `policy_rules` table with `pol-*` IDs. The + * OnboardingWizard + CertificatesPage + CertificateDetailPage dropdowns populate + * `renewal_policy_id` from this interface — previously they mis-populated it + * from `getPolicies()` which returned `pol-*` IDs and produced FK violations on + * certificate insert/update. + * + * JSON tags mirror internal/domain/renewal_policy.go. + */ +export interface RenewalPolicy { + id: string; + name: string; + renewal_window_days: number; + auto_renew: boolean; + max_retries: number; + retry_interval_seconds: number; + alert_thresholds_days: number[]; + certificate_profile_id?: string | null; + created_at: string; + updated_at: string; +} + export interface Issuer { id: string; name: string; diff --git a/web/src/pages/CertificateDetailPage.tsx b/web/src/pages/CertificateDetailPage.tsx index ed5e37d..3413b86 100644 --- a/web/src/pages/CertificateDetailPage.tsx +++ b/web/src/pages/CertificateDetailPage.tsx @@ -1,7 +1,7 @@ import { useState } from 'react'; import { useParams, useNavigate } from 'react-router-dom'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; -import { getCertificate, getCertificateVersions, triggerRenewal, triggerDeployment, archiveCertificate, revokeCertificate, updateCertificate, getTargets, getJobs, getPolicies, getProfiles, getProfile, downloadCertificatePEM, exportCertificatePKCS12 } from '../api/client'; +import { getCertificate, getCertificateVersions, triggerRenewal, triggerDeployment, archiveCertificate, revokeCertificate, updateCertificate, getTargets, getJobs, getRenewalPolicies, getProfiles, getProfile, downloadCertificatePEM, exportCertificatePKCS12 } from '../api/client'; import { REVOCATION_REASONS } from '../api/types'; import PageHeader from '../components/PageHeader'; import StatusBadge from '../components/StatusBadge'; @@ -164,9 +164,14 @@ function InlinePolicyEditor({ certId, currentPolicyId, currentProfileId }: { cer const [policyId, setPolicyId] = useState(currentPolicyId); const [profileId, setProfileId] = useState(currentProfileId); + // G-1: swap from getPolicies (compliance rules, pol-*) to getRenewalPolicies + // (lifecycle policies, rp-*). managed_certificates.renewal_policy_id FK + // points at renewal_policies(id); the previous getPolicies call populated + // the dropdown with pol-* IDs that would 400/23503 at the server. See also + // OnboardingWizard.tsx:603 and CertificatesPage.tsx:53 for the sibling fixes. const { data: policies } = useQuery({ - queryKey: ['policies'], - queryFn: () => getPolicies(), + queryKey: ['renewal-policies'], + queryFn: () => getRenewalPolicies(1, 500), enabled: editing, }); @@ -227,7 +232,10 @@ function InlinePolicyEditor({ certId, currentPolicyId, currentProfileId }: { cer className="w-full bg-white border border-surface-border rounded px-3 py-2 text-sm text-ink"> {policies?.data?.map(p => ( - + // G-1: RenewalPolicy has no `type` field (that was PolicyRule). + // Show the human-readable name + renewal window so operators can + // pick the correct lifecycle policy at a glance. + ))} diff --git a/web/src/pages/CertificatesPage.tsx b/web/src/pages/CertificatesPage.tsx index b569fc3..4e4d4c5 100644 --- a/web/src/pages/CertificatesPage.tsx +++ b/web/src/pages/CertificatesPage.tsx @@ -1,7 +1,7 @@ import { useState } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useNavigate } from 'react-router-dom'; -import { getCertificates, createCertificate, triggerRenewal, revokeCertificate, updateCertificate, getOwners, getTeams, getPolicies, getProfiles, getIssuers, bulkRevokeCertificates } from '../api/client'; +import { getCertificates, createCertificate, triggerRenewal, revokeCertificate, updateCertificate, getOwners, getTeams, getRenewalPolicies, getProfiles, getIssuers, bulkRevokeCertificates } from '../api/client'; import { useAuth } from '../components/AuthProvider'; import { REVOCATION_REASONS } from '../api/types'; import PageHeader from '../components/PageHeader'; @@ -48,9 +48,15 @@ function CreateCertificateModal({ onClose, onSuccess }: { onClose: () => void; o queryKey: ['teams', 'form'], queryFn: () => getTeams({ per_page: '500' }), }); + // G-1: swap from getPolicies (compliance rules, pol-*) to getRenewalPolicies + // (lifecycle policies, rp-*). managed_certificates.renewal_policy_id FK + // points at renewal_policies(id), so the dropdown must pull from that table + // — the previous getPolicies call populated the dropdown with pol-* IDs that + // would 400/23503 at the server. See also OnboardingWizard.tsx:603 and + // CertificateDetailPage.tsx:169 for the sibling fixes. const { data: policiesResp } = useQuery({ queryKey: ['renewal-policies', 'form'], - queryFn: () => getPolicies({ per_page: '500' }), + queryFn: () => getRenewalPolicies(1, 500), }); const profiles = profilesResp?.data || []; const issuers = issuersResp?.data || []; diff --git a/web/src/pages/OnboardingWizard.test.tsx b/web/src/pages/OnboardingWizard.test.tsx index c8807a8..1e58f52 100644 --- a/web/src/pages/OnboardingWizard.test.tsx +++ b/web/src/pages/OnboardingWizard.test.tsx @@ -39,7 +39,10 @@ vi.mock('../api/client', () => ({ getProfiles: vi.fn(), getOwners: vi.fn(), getTeams: vi.fn(), - getPolicies: vi.fn(), + // G-1: wizard populates the renewal_policy_id dropdown from + // getRenewalPolicies (rp-* ids), not getPolicies (which returns compliance + // rules with pol-* ids and violates the FK). + getRenewalPolicies: vi.fn(), createIssuer: vi.fn(), testIssuerConnection: vi.fn(), createCertificate: vi.fn(), @@ -85,7 +88,9 @@ function stubAllQueriesEmpty() { vi.mocked(client.getTeams).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 500, } as never); - vi.mocked(client.getPolicies).mockResolvedValue({ + // G-1: wizard populates renewal_policy_id from getRenewalPolicies, not + // getPolicies. See comment on the mock factory above. + vi.mocked(client.getRenewalPolicies).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 500, } as never); } diff --git a/web/src/pages/OnboardingWizard.tsx b/web/src/pages/OnboardingWizard.tsx index 3c15bed..395ae93 100644 --- a/web/src/pages/OnboardingWizard.tsx +++ b/web/src/pages/OnboardingWizard.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useNavigate, Link } from 'react-router-dom'; import { - getIssuers, getAgents, getProfiles, getOwners, getTeams, getPolicies, + getIssuers, getAgents, getProfiles, getOwners, getTeams, getRenewalPolicies, createIssuer, testIssuerConnection, createCertificate, triggerRenewal, createTeam, createOwner, @@ -600,7 +600,11 @@ function CertificateStep({ onNext, onSkip, createdIssuerId }: { const { data: agents } = useQuery({ queryKey: ['agents'], queryFn: () => getAgents() }); const { data: owners } = useQuery({ queryKey: ['owners'], queryFn: () => getOwners({ per_page: '500' }) }); const { data: teams } = useQuery({ queryKey: ['teams'], queryFn: () => getTeams({ per_page: '500' }) }); - const { data: policies } = useQuery({ queryKey: ['renewal-policies'], queryFn: () => getPolicies({ per_page: '500' }) }); + // G-1: bind renewal_policy_id dropdown to /api/v1/renewal-policies (rp-* IDs + // from the renewal_policies table). Previously populated from getPolicies() + // which returned compliance rules (pol-* IDs) and violated the FK + // managed_certificates.renewal_policy_id → renewal_policies(id) on submit. + const { data: policies } = useQuery({ queryKey: ['renewal-policies'], queryFn: () => getRenewalPolicies(1, 500) }); const hasAgents = (agents?.data?.length ?? 0) > 0;