mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 18:21:32 +00:00
Merge branch 'fix/l1-master-bulk-action-endpoints' (L-1 master, 2 audit findings)
This commit is contained in:
@@ -344,6 +344,50 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Forbidden client-side bulk-action loop regression guard (L-1)
|
||||
# L-1 master closed cat-l-fa0c1ac07ab5 (bulk-renew loop) and
|
||||
# cat-l-8a1fb258a38a (bulk-reassign loop) by adding server-side
|
||||
# bulk endpoints (POST /api/v1/certificates/bulk-renew and
|
||||
# POST /api/v1/certificates/bulk-reassign) that the GUI calls
|
||||
# in a single round-trip. Pre-L-1 the GUI looped per-cert
|
||||
# HTTP calls — 100 selected certs = 100 round-trips × ~50–200ms
|
||||
# each = a 5–20-second wedge during which the operator stares
|
||||
# at a progress bar.
|
||||
#
|
||||
# This step grep-fails the build if either loop shape reappears
|
||||
# in CertificatesPage.tsx. Patterns catch the actual pre-L-1
|
||||
# shapes:
|
||||
# - `for (const id of ids) { await triggerRenewal(id) }`
|
||||
# - `for (const id of ids) { await updateCertificate(id, { owner_id }) }`
|
||||
# - `for (let i = 0; i < ids.length; i++) { await triggerRenewal(ids[i]) }`
|
||||
#
|
||||
# Allowed: comment lines explaining the pre-L-1 pattern in the
|
||||
# docblock above each handler. Test files (_test.tsx) exempt
|
||||
# so negative-pattern tests can keep working.
|
||||
#
|
||||
# See coverage-gap-audit-2026-04-24-v5/unified-audit.md
|
||||
# cat-l-fa0c1ac07ab5 and cat-l-8a1fb258a38a for closure
|
||||
# rationale, or web/src/api/client.ts::bulkRenewCertificates
|
||||
# / bulkReassignCertificates for the canonical call path.
|
||||
run: |
|
||||
set -e
|
||||
|
||||
BAD_LOOP=$(grep -nE 'for[[:space:]]*\(' web/src/pages/CertificatesPage.tsx 2>/dev/null \
|
||||
| grep -E 'await[[:space:]]+(triggerRenewal|updateCertificate)\(' \
|
||||
| grep -v '\.test\.' \
|
||||
| grep -vE '^\s*[^:]+:[0-9]+:\s*//' \
|
||||
|| true)
|
||||
if [ -n "$BAD_LOOP" ]; then
|
||||
echo "L-1 regression: client-side bulk-action loop reappeared in CertificatesPage.tsx:"
|
||||
echo "$BAD_LOOP"
|
||||
echo ""
|
||||
echo "Use bulkRenewCertificates({ certificate_ids: [...] }) or"
|
||||
echo "bulkReassignCertificates({ certificate_ids: [...], owner_id, team_id? })"
|
||||
echo "instead of looping per-item HTTP calls. See"
|
||||
echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md cat-l-* for rationale."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Race Detection
|
||||
run: go test -race ./internal/service/... ./internal/api/handler/... ./internal/api/middleware/... ./internal/scheduler/... ./internal/connector/... ./internal/crypto/... ./internal/domain/... ./internal/validation/... ./internal/tlsprobe/... -count=1 -timeout 300s
|
||||
|
||||
|
||||
@@ -4,6 +4,48 @@ All notable changes to certctl are documented in this file. Dates use ISO 8601.
|
||||
|
||||
## [unreleased] — 2026-04-25
|
||||
|
||||
### L-1: Client-side bulk-action loops — closed end-to-end
|
||||
|
||||
> The certctl dashboard's busiest screen (`CertificatesPage.tsx`) had two bulk-action workflows that looped per-cert HTTP calls. Selecting 100 certs and clicking "Renew" issued 100 sequential `POST /api/v1/certificates/{id}/renew` requests; "Reassign owner" issued 100 sequential `PUT /api/v1/certificates/{id}` requests. Each round-trip carried ~50–200 ms of Auth → audit-log → handler → service → repo → DB → audit-write → response, so a 100-cert bulk action was a 5–20-second wedge during which the operator stared at a progress bar. The bulk-revoke endpoint (`POST /api/v1/certificates/bulk-revoke`) already shipped in v2.0.x as the canonical pattern for this; L-1 ports that exact shape to bulk-renew (P1) and bulk-reassign (P2). One backend round-trip; one audit event for the entire operation; per-cert success/skip/error counts in a single response envelope. Bundled with two new MCP tools and an OpenAPI spec update so non-GUI callers (CLI / MCP / blackbox probes) can use the same endpoints.
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
None. Both endpoints are additive; the per-cert `POST /certificates/{id}/renew` and `PUT /certificates/{id}` paths remain available and unchanged. The frontend implementation switches from looping to single-call, but operators with custom GUIs hitting the per-cert endpoints continue to work.
|
||||
|
||||
### Added
|
||||
|
||||
- **`POST /api/v1/certificates/bulk-renew`** — enqueues a renewal job for every matching managed certificate. Supports criteria-mode (`{profile_id, owner_id, agent_id, issuer_id, team_id}`) and explicit-IDs mode (`{certificate_ids}`). Mirrors `BulkRevokeCriteria` field-for-field (sans the RFC-5280 reason code). Returns `{total_matched, total_enqueued, total_skipped, total_failed, enqueued_jobs[], errors[]}`. NOT admin-gated — bulk renewal is non-destructive (worst case it kicks off some redundant ACME orders). Status filter: certs in `Archived/Revoked/Expired/RenewalInProgress` are silent-skipped (TotalSkipped++) rather than returned as errors. Implementation: `internal/domain/bulk_renewal.go`, `internal/service/bulk_renewal.go`, `internal/api/handler/bulk_renewal.go`.
|
||||
- **`POST /api/v1/certificates/bulk-reassign`** — updates `owner_id` (required) and `team_id` (optional) on every cert in `certificate_ids`. Skips certs already owned by the target (silent no-op surfaced as `total_skipped`). Validates the target `owner_id` upfront — a non-existent owner returns 400 (via the typed `service.ErrBulkReassignOwnerNotFound` sentinel) before any cert is touched. NOT admin-gated. Implementation: `internal/domain/bulk_reassignment.go`, `internal/service/bulk_reassignment.go`, `internal/api/handler/bulk_reassignment.go`.
|
||||
- **MCP tools `certctl_bulk_renew_certificates` and `certctl_bulk_reassign_certificates`** in `internal/mcp/tools.go` + `internal/mcp/types.go`. Mirror the existing `certctl_bulk_revoke_certificates` shape so MCP consumers have a uniform bulk-action surface.
|
||||
- **OpenAPI schemas** `BulkRenewRequest`, `BulkRenewResult`, `BulkEnqueuedJob`, `BulkReassignRequest`, `BulkReassignResult` plus the two new operations with shared envelope semantics.
|
||||
- **Frontend client functions** `bulkRenewCertificates(criteria)` and `bulkReassignCertificates(request)` in `web/src/api/client.ts` with full TS types for both request and response envelopes.
|
||||
- **Service-layer regression tests** for both new services (`internal/service/bulk_renewal_test.go` + `internal/service/bulk_reassignment_test.go`): happy path, criteria-mode, status-skip semantics (RenewalInProgress / Revoked / Archived for renew; already-owned for reassign), empty-criteria rejection, partial-failure tolerance, single-bulk-audit-event contract.
|
||||
- **Handler-layer regression tests** (`internal/api/handler/bulk_renewal_handler_test.go` + `internal/api/handler/bulk_reassignment_handler_test.go`): happy path, empty-body 400, wrong-method 405, actor attribution from `middleware.GetUser`, owner-not-found-sentinel-→-400 mapping for reassign, generic-service-error-→-500.
|
||||
- **Domain-layer JSON-shape tests** pinning the wire contract for `BulkRenewalResult` / `BulkReassignmentResult` / `BulkOperationError`.
|
||||
- **CI regression guardrail** in `.github/workflows/ci.yml` (`Forbidden client-side bulk-action loop regression guard (L-1)`) — grep-fails the build if `for(...) await triggerRenewal(...)` or `for(...) await updateCertificate(...)` reappears in `web/src/pages/CertificatesPage.tsx`. Verified: passes against the post-fix tree, fires against synthetic regressions.
|
||||
|
||||
### Changed
|
||||
|
||||
- **`web/src/pages/CertificatesPage.tsx::handleBulkRenewal`** — rewritten from N-call loop to a single `bulkRenewCertificates({ certificate_ids })` call. Result envelope drives the progress UI (matched / enqueued / skipped / failed counts).
|
||||
- **`web/src/pages/CertificatesPage.tsx::handleReassign`** (in the reassign modal) — same shape: single `bulkReassignCertificates({ certificate_ids, owner_id })` call. First-error message surfaced when `total_failed > 0`.
|
||||
- **`internal/api/router/router.go`** — three bulk-* routes (revoke / renew / reassign) registered together as a block before the per-cert `{id}` routes; `HandlerRegistry` gains `BulkRenewal` and `BulkReassignment` fields.
|
||||
- **`cmd/server/main.go`** — constructs `BulkRenewalService` (threads `cfg.Keygen.Mode` so bulk-renew jobs land in the same initial status as single-cert `TriggerRenewal`) and `BulkReassignmentService` alongside the existing `BulkRevocationService`.
|
||||
|
||||
### Performance impact
|
||||
|
||||
100-cert bulk-renew workflow goes from ~10 s of sequential per-cert HTTP (worst case) to a single ~100 ms call — roughly 99% latency reduction on the canonical operator workflow. Server-side resource use also drops: one Auth pass, one audit event, one criteria-resolution query, instead of N of each.
|
||||
|
||||
### Closed audit findings
|
||||
|
||||
- `cat-l-fa0c1ac07ab5` (P1, primary) — bulk renew client-side sequential loop
|
||||
- `cat-l-8a1fb258a38a` (P2) — bulk owner-reassign client-side sequential loop
|
||||
|
||||
### Known follow-ups (deferred from L-1 scope)
|
||||
|
||||
- `cat-b-31ceb6aaa9f1` (P1, `updateOwner`/`updateTeam`/`updateAgentGroup` orphan) — different shape; the fix is "wire up the existing PUT endpoints to the GUI", not "add a bulk endpoint".
|
||||
- `cat-k-e85d1099b2d7` (P2, CertificatesPage no pagination UI) — same page; criteria-mode bulk-renew (`{owner_id: 'o-alice'}`) means an operator can already "renew all of Alice's certs" without paginating, but pagination is still wanted for the table view.
|
||||
- `cat-i-b0924b6675f8` (P1, MCP missing `claim`/`dismiss`/`acknowledge`) — L-1 added two new MCP tools but does NOT close that finding.
|
||||
|
||||
### D-1: StatusBadge enum drift + Certificate phantom fields — closed end-to-end
|
||||
|
||||
> The dashboard silently lied in five places. Agents in the `Degraded` state (the only Go-side AgentStatus that means "needs operator attention") rendered as default neutral grey because StatusBadge mapped `Stale` (a key Go has never emitted) to yellow and let the real `Degraded` value fall through to the dictionary default. Dead-letter notifications (`status: 'dead'`, retries exhausted) rendered as default neutral, visually equated with `read` (operator-acknowledged). The Certificate badge map carried a `PendingIssuance` key that no Go enum value ever emits — dead key, latent confusion vector. CertificateDetailPage's Key Algorithm and Key Size rows always rendered `—` even when the data was a single fetch away, because the lookup went through `cert.key_algorithm` directly — and the underlying `Certificate` TypeScript interface declared five optional fields (`serial_number`, `fingerprint_sha256`, `key_algorithm`, `key_size`, `issued_at`) that Go's `ManagedCertificate` has never carried (those values live on `CertificateVersion`). Five findings, two files, one frontend rebuild. Pre-D-1 the only reason this didn't trip a regression suite was that the regression suite never asserted "every Go-emitted enum value gets a non-default StatusBadge class" — D-1 fixes the visual lies and adds a 38-case Vitest property test that walks every Go enum and pins the contract.
|
||||
|
||||
@@ -470,6 +470,69 @@ paths:
|
||||
"500":
|
||||
$ref: "#/components/responses/InternalError"
|
||||
|
||||
/api/v1/certificates/bulk-renew:
|
||||
post:
|
||||
tags: [Certificates]
|
||||
summary: Bulk renew certificates by criteria or explicit IDs
|
||||
description: |
|
||||
Enqueues a renewal job for every matching managed certificate. Mirrors POST
|
||||
/api/v1/certificates/bulk-revoke shape exactly so operators who already know
|
||||
that contract have zero new surface to learn. L-1 closure
|
||||
(cat-l-fa0c1ac07ab5): pre-L-1 the GUI looped per-cert HTTP calls;
|
||||
post-L-1 it's a single POST. Status filter: certs in
|
||||
Archived/Revoked/Expired/RenewalInProgress are silent-skipped (TotalSkipped++)
|
||||
rather than returned as errors. Asynchronous: the action ENQUEUES jobs the
|
||||
scheduler picks up; per-cert {certificate_id, job_id} pairs are returned in
|
||||
enqueued_jobs. NOT admin-gated — bulk renewal is non-destructive.
|
||||
operationId: bulkRenewCertificates
|
||||
requestBody:
|
||||
required: true
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/BulkRenewRequest"
|
||||
responses:
|
||||
"200":
|
||||
description: Bulk renewal result
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/BulkRenewResult"
|
||||
"400":
|
||||
$ref: "#/components/responses/BadRequest"
|
||||
"500":
|
||||
$ref: "#/components/responses/InternalError"
|
||||
|
||||
/api/v1/certificates/bulk-reassign:
|
||||
post:
|
||||
tags: [Certificates]
|
||||
summary: Bulk reassign owner (and optionally team) for a set of certificates
|
||||
description: |
|
||||
Updates owner_id (required) and team_id (optional) on every certificate in
|
||||
certificate_ids. Skips certs already owned by the target (silent no-op,
|
||||
TotalSkipped++). L-2 closure (cat-l-8a1fb258a38a). Narrower than bulk-renew:
|
||||
explicit IDs only, no criteria-mode. The OwnerID is validated upfront — a
|
||||
non-existent owner returns 400 before any cert is touched. Verb chosen as
|
||||
POST (not PATCH) for codebase consistency with bulk-revoke and bulk-renew.
|
||||
operationId: bulkReassignCertificates
|
||||
requestBody:
|
||||
required: true
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/BulkReassignRequest"
|
||||
responses:
|
||||
"200":
|
||||
description: Bulk reassignment result
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/BulkReassignResult"
|
||||
"400":
|
||||
$ref: "#/components/responses/BadRequest"
|
||||
"500":
|
||||
$ref: "#/components/responses/InternalError"
|
||||
|
||||
# ─── Certificate Export ──────────────────────────────────────────────
|
||||
/api/v1/certificates/{id}/export/pem:
|
||||
get:
|
||||
@@ -3657,6 +3720,116 @@ components:
|
||||
type: string
|
||||
description: Per-certificate error details for failed revocations
|
||||
|
||||
# L-1 master closure (cat-l-fa0c1ac07ab5 + cat-l-8a1fb258a38a):
|
||||
# bulk-renew + bulk-reassign request/result schemas. Mirror
|
||||
# BulkRevokeRequest/Result envelope shape so frontend bulk-result
|
||||
# rendering is one helper. See internal/domain/bulk_renewal.go +
|
||||
# internal/domain/bulk_reassignment.go for the Go-side source of
|
||||
# truth.
|
||||
BulkRenewRequest:
|
||||
type: object
|
||||
description: Criteria for bulk renewal. At least one selector required.
|
||||
properties:
|
||||
profile_id:
|
||||
type: string
|
||||
description: Renew all certificates matching this profile
|
||||
owner_id:
|
||||
type: string
|
||||
description: Renew all certificates owned by this owner
|
||||
agent_id:
|
||||
type: string
|
||||
description: Renew all certificates deployed via this agent
|
||||
issuer_id:
|
||||
type: string
|
||||
description: Renew all certificates issued by this issuer
|
||||
team_id:
|
||||
type: string
|
||||
description: Renew all certificates owned by members of this team
|
||||
certificate_ids:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
description: Explicit list of certificate IDs to renew
|
||||
|
||||
BulkEnqueuedJob:
|
||||
type: object
|
||||
properties:
|
||||
certificate_id:
|
||||
type: string
|
||||
job_id:
|
||||
type: string
|
||||
description: ID of the renewal job created for this certificate
|
||||
|
||||
BulkRenewResult:
|
||||
type: object
|
||||
properties:
|
||||
total_matched:
|
||||
type: integer
|
||||
description: Number of certificates matching the criteria
|
||||
total_enqueued:
|
||||
type: integer
|
||||
description: Number of renewal jobs successfully created
|
||||
total_skipped:
|
||||
type: integer
|
||||
description: Certs already RenewalInProgress / Revoked / Archived / Expired (silent no-op)
|
||||
total_failed:
|
||||
type: integer
|
||||
description: Number of certificates whose enqueue path returned an error
|
||||
enqueued_jobs:
|
||||
type: array
|
||||
items:
|
||||
$ref: "#/components/schemas/BulkEnqueuedJob"
|
||||
description: Per-certificate {certificate_id, job_id} pairs for the successful enqueue path
|
||||
errors:
|
||||
type: array
|
||||
items:
|
||||
type: object
|
||||
properties:
|
||||
certificate_id:
|
||||
type: string
|
||||
error:
|
||||
type: string
|
||||
description: Per-certificate error details for the failure path
|
||||
|
||||
BulkReassignRequest:
|
||||
type: object
|
||||
required: [certificate_ids, owner_id]
|
||||
properties:
|
||||
certificate_ids:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
description: Explicit list of certificate IDs to reassign
|
||||
owner_id:
|
||||
type: string
|
||||
description: Required. New owner_id for every cert in certificate_ids.
|
||||
team_id:
|
||||
type: string
|
||||
description: Optional. When non-empty, also updates team_id on every cert.
|
||||
|
||||
BulkReassignResult:
|
||||
type: object
|
||||
properties:
|
||||
total_matched:
|
||||
type: integer
|
||||
total_reassigned:
|
||||
type: integer
|
||||
description: Number of certs whose owner_id (and optionally team_id) was actually mutated
|
||||
total_skipped:
|
||||
type: integer
|
||||
description: Certs already owned by the target (silent no-op)
|
||||
total_failed:
|
||||
type: integer
|
||||
errors:
|
||||
type: array
|
||||
items:
|
||||
type: object
|
||||
properties:
|
||||
certificate_id:
|
||||
type: string
|
||||
error:
|
||||
type: string
|
||||
|
||||
# ─── Issuers ─────────────────────────────────────────────────────
|
||||
IssuerType:
|
||||
type: string
|
||||
|
||||
+17
-2
@@ -411,6 +411,14 @@ func main() {
|
||||
// Initialize bulk revocation service
|
||||
bulkRevocationService := service.NewBulkRevocationService(revocationSvc, certificateRepo, auditService, logger)
|
||||
|
||||
// L-1 master (cat-l-fa0c1ac07ab5 + cat-l-8a1fb258a38a): bulk-renew
|
||||
// and bulk-reassign services. Mirror BulkRevocationService wiring so
|
||||
// the construction site is co-located with the existing bulk endpoint.
|
||||
// keygenMode is threaded so bulk-renew jobs land in the same initial
|
||||
// status (AwaitingCSR vs Pending) as single-cert TriggerRenewal.
|
||||
bulkRenewalService := service.NewBulkRenewalService(certificateRepo, jobRepo, auditService, logger, cfg.Keygen.Mode)
|
||||
bulkReassignmentService := service.NewBulkReassignmentService(certificateRepo, ownerRepo, auditService, logger)
|
||||
|
||||
// Initialize stats and metrics services
|
||||
statsService := service.NewStatsService(certificateRepo, jobRepo, agentRepo)
|
||||
// I-005: wire the notification repository so DashboardSummary.NotificationsDead
|
||||
@@ -456,6 +464,11 @@ func main() {
|
||||
exportHandler := handler.NewExportHandler(exportService)
|
||||
|
||||
bulkRevocationHandler := handler.NewBulkRevocationHandler(bulkRevocationService)
|
||||
// L-1 master closure: handlers for the new bulk-renew + bulk-reassign
|
||||
// endpoints. Both registered via HandlerRegistry below; dispatched
|
||||
// through the standard authed middleware chain (no admin gate).
|
||||
bulkRenewalHandler := handler.NewBulkRenewalHandler(bulkRenewalService)
|
||||
bulkReassignmentHandler := handler.NewBulkReassignmentHandler(bulkReassignmentService)
|
||||
|
||||
// Initialize digest service (requires email notifier)
|
||||
var digestService *service.DigestService
|
||||
@@ -595,8 +608,10 @@ func main() {
|
||||
Export: exportHandler,
|
||||
Digest: *digestHandler,
|
||||
HealthChecks: healthCheckHandler,
|
||||
BulkRevocation: bulkRevocationHandler,
|
||||
Version: versionHandler,
|
||||
BulkRevocation: bulkRevocationHandler,
|
||||
BulkRenewal: bulkRenewalHandler,
|
||||
BulkReassignment: bulkReassignmentHandler,
|
||||
Version: versionHandler,
|
||||
})
|
||||
// Register EST (RFC 7030) handlers if enabled
|
||||
if cfg.EST.Enabled {
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/api/middleware"
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
"github.com/shankar0123/certctl/internal/service"
|
||||
)
|
||||
|
||||
// BulkReassignmentService defines the service interface for bulk
|
||||
// owner-reassignment operations.
|
||||
type BulkReassignmentService interface {
|
||||
BulkReassign(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error)
|
||||
}
|
||||
|
||||
// BulkReassignmentHandler handles HTTP requests for bulk reassignment
|
||||
// operations.
|
||||
type BulkReassignmentHandler struct {
|
||||
svc BulkReassignmentService
|
||||
}
|
||||
|
||||
// NewBulkReassignmentHandler creates a new BulkReassignmentHandler.
|
||||
func NewBulkReassignmentHandler(svc BulkReassignmentService) BulkReassignmentHandler {
|
||||
return BulkReassignmentHandler{svc: svc}
|
||||
}
|
||||
|
||||
// bulkReassignRequest is the JSON shape decoded from the request body.
|
||||
type bulkReassignRequest struct {
|
||||
CertificateIDs []string `json:"certificate_ids"`
|
||||
OwnerID string `json:"owner_id"`
|
||||
TeamID string `json:"team_id,omitempty"`
|
||||
}
|
||||
|
||||
// BulkReassign handles POST /api/v1/certificates/bulk-reassign
|
||||
//
|
||||
// L-2 closure (cat-l-8a1fb258a38a): pre-L-2 the GUI looped
|
||||
// `await updateCertificate(id, { owner_id })`. Post-L-2 the GUI POSTs
|
||||
// once and the server mutates owner_id (and optionally team_id) on N
|
||||
// certs, returning per-cert success/skip/error counts.
|
||||
//
|
||||
// Narrower contract than bulk-renew: explicit IDs only, no criteria-mode.
|
||||
// OwnerID is required; TeamID is optional and updates the team only when
|
||||
// non-empty (matches the existing per-cert PUT contract).
|
||||
//
|
||||
// Auth: any authenticated caller can reassign certs they own/have
|
||||
// access to. NOT admin-gated — operators reassign ownership during
|
||||
// team transitions all the time and gating that on admin would block
|
||||
// the common-case workflow.
|
||||
//
|
||||
// Validation order: empty body → 400; empty IDs → 400; missing
|
||||
// owner_id → 400; non-existent owner_id → 400 via the
|
||||
// ErrBulkReassignOwnerNotFound sentinel mapped here.
|
||||
func (h BulkReassignmentHandler) BulkReassign(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 req bulkReassignRequest
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
ErrorWithRequestID(w, http.StatusBadRequest, "Invalid request body", requestID)
|
||||
return
|
||||
}
|
||||
|
||||
request := domain.BulkReassignmentRequest{
|
||||
CertificateIDs: req.CertificateIDs,
|
||||
OwnerID: req.OwnerID,
|
||||
TeamID: req.TeamID,
|
||||
}
|
||||
if request.IsEmpty() {
|
||||
ErrorWithRequestID(w, http.StatusBadRequest,
|
||||
"At least one certificate_id is required",
|
||||
requestID)
|
||||
return
|
||||
}
|
||||
if request.OwnerID == "" {
|
||||
ErrorWithRequestID(w, http.StatusBadRequest, "owner_id is required", requestID)
|
||||
return
|
||||
}
|
||||
|
||||
actor := resolveActor(r.Context())
|
||||
|
||||
result, err := h.svc.BulkReassign(r.Context(), request, actor)
|
||||
if err != nil {
|
||||
// Sentinel-error → 400 mapping. ErrBulkReassignOwnerNotFound
|
||||
// means the operator picked an owner that doesn't exist; this
|
||||
// is bad input (400), not a server error (500). Mirrors the
|
||||
// post-M-1 errToStatus convention rather than substring-matching
|
||||
// err.Error().
|
||||
if errors.Is(err, service.ErrBulkReassignOwnerNotFound) {
|
||||
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
|
||||
return
|
||||
}
|
||||
ErrorWithRequestID(w, http.StatusInternalServerError, "Bulk reassignment failed: "+err.Error(), requestID)
|
||||
return
|
||||
}
|
||||
|
||||
JSON(w, http.StatusOK, result)
|
||||
}
|
||||
@@ -0,0 +1,149 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
"github.com/shankar0123/certctl/internal/service"
|
||||
)
|
||||
|
||||
type mockBulkReassignmentService struct {
|
||||
BulkReassignFn func(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error)
|
||||
}
|
||||
|
||||
func (m *mockBulkReassignmentService) BulkReassign(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error) {
|
||||
if m.BulkReassignFn != nil {
|
||||
return m.BulkReassignFn(ctx, request, actor)
|
||||
}
|
||||
return &domain.BulkReassignmentResult{}, nil
|
||||
}
|
||||
|
||||
func TestBulkReassign_Handler_HappyPath(t *testing.T) {
|
||||
svc := &mockBulkReassignmentService{
|
||||
BulkReassignFn: func(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error) {
|
||||
if request.OwnerID != "o-bob" {
|
||||
t.Errorf("owner_id = %q, want 'o-bob'", request.OwnerID)
|
||||
}
|
||||
return &domain.BulkReassignmentResult{
|
||||
TotalMatched: 2, TotalReassigned: 2,
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
h := NewBulkReassignmentHandler(svc)
|
||||
|
||||
body := `{"certificate_ids":["mc-1","mc-2"],"owner_id":"o-bob"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-reassign", bytes.NewBufferString(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkReassign(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
var result domain.BulkReassignmentResult
|
||||
if err := json.NewDecoder(w.Body).Decode(&result); err != nil {
|
||||
t.Fatalf("decode failed: %v", err)
|
||||
}
|
||||
if result.TotalReassigned != 2 {
|
||||
t.Errorf("envelope drift: TotalReassigned=%d, want 2", result.TotalReassigned)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkReassign_Handler_EmptyIDs_400(t *testing.T) {
|
||||
svc := &mockBulkReassignmentService{}
|
||||
h := NewBulkReassignmentHandler(svc)
|
||||
|
||||
body := `{"certificate_ids":[],"owner_id":"o-bob"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-reassign", bytes.NewBufferString(body))
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkReassign(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d, want 400", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkReassign_Handler_MissingOwnerID_400(t *testing.T) {
|
||||
svc := &mockBulkReassignmentService{}
|
||||
h := NewBulkReassignmentHandler(svc)
|
||||
|
||||
body := `{"certificate_ids":["mc-1"]}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-reassign", bytes.NewBufferString(body))
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkReassign(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d, want 400", w.Code)
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "owner_id") {
|
||||
t.Errorf("body should name owner_id; got: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_Handler_OwnerNotFound_400 — sentinel-error → 400
|
||||
// mapping. Operator picked an owner that doesn't exist; that's bad
|
||||
// input, not a server error.
|
||||
func TestBulkReassign_Handler_OwnerNotFound_400(t *testing.T) {
|
||||
svc := &mockBulkReassignmentService{
|
||||
BulkReassignFn: func(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error) {
|
||||
return nil, fmt.Errorf("%w: %s", service.ErrBulkReassignOwnerNotFound, request.OwnerID)
|
||||
},
|
||||
}
|
||||
h := NewBulkReassignmentHandler(svc)
|
||||
|
||||
body := `{"certificate_ids":["mc-1"],"owner_id":"o-ghost"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-reassign", bytes.NewBufferString(body))
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkReassign(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d, want 400 (ErrBulkReassignOwnerNotFound → 400)", w.Code)
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "owner not found") {
|
||||
t.Errorf("body should mention 'owner not found'; got: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkReassign_Handler_WrongMethod_405(t *testing.T) {
|
||||
svc := &mockBulkReassignmentService{}
|
||||
h := NewBulkReassignmentHandler(svc)
|
||||
|
||||
for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodDelete, http.MethodPatch} {
|
||||
req := httptest.NewRequest(method, "/api/v1/certificates/bulk-reassign", nil)
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkReassign(w, req)
|
||||
if w.Code != http.StatusMethodNotAllowed {
|
||||
t.Errorf("%s → %d, want 405", method, w.Code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkReassign_Handler_GenericError_500(t *testing.T) {
|
||||
svc := &mockBulkReassignmentService{
|
||||
BulkReassignFn: func(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error) {
|
||||
return nil, errors.New("simulated outage")
|
||||
},
|
||||
}
|
||||
h := NewBulkReassignmentHandler(svc)
|
||||
body := `{"certificate_ids":["mc-1"],"owner_id":"o-bob"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-reassign", bytes.NewBufferString(body))
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkReassign(w, req)
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("status = %d, want 500", w.Code)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,96 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/api/middleware"
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
)
|
||||
|
||||
// BulkRenewalService defines the service interface for bulk certificate
|
||||
// renewal. Mirrors BulkRevocationService — handler doesn't import the
|
||||
// concrete service struct so tests can inject a mock without pulling in
|
||||
// the full service-layer dependency graph.
|
||||
type BulkRenewalService interface {
|
||||
BulkRenew(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error)
|
||||
}
|
||||
|
||||
// BulkRenewalHandler handles HTTP requests for bulk renewal operations.
|
||||
type BulkRenewalHandler struct {
|
||||
svc BulkRenewalService
|
||||
}
|
||||
|
||||
// NewBulkRenewalHandler creates a new BulkRenewalHandler.
|
||||
func NewBulkRenewalHandler(svc BulkRenewalService) BulkRenewalHandler {
|
||||
return BulkRenewalHandler{svc: svc}
|
||||
}
|
||||
|
||||
// bulkRenewRequest mirrors the BulkRenewalCriteria JSON shape (the
|
||||
// handler decodes into this struct then hands a domain.BulkRenewalCriteria
|
||||
// to the service — same indirection as bulkRevokeRequest in
|
||||
// bulk_revocation.go).
|
||||
type bulkRenewRequest struct {
|
||||
ProfileID string `json:"profile_id,omitempty"`
|
||||
OwnerID string `json:"owner_id,omitempty"`
|
||||
AgentID string `json:"agent_id,omitempty"`
|
||||
IssuerID string `json:"issuer_id,omitempty"`
|
||||
TeamID string `json:"team_id,omitempty"`
|
||||
CertificateIDs []string `json:"certificate_ids,omitempty"`
|
||||
}
|
||||
|
||||
// BulkRenew handles POST /api/v1/certificates/bulk-renew
|
||||
//
|
||||
// L-1 closure (cat-l-fa0c1ac07ab5): pre-L-1 the GUI looped
|
||||
// `await triggerRenewal(id)` over the selection. Post-L-1 it POSTs once
|
||||
// and the server enqueues N renewal jobs server-side, returning a
|
||||
// per-cert {certificate_id, job_id} envelope.
|
||||
//
|
||||
// Request shape mirrors BulkRevokeRequest (criteria-mode + IDs-mode);
|
||||
// the "renew all certs of profile X before its CA changes" use case is
|
||||
// why criteria-mode is supported in addition to explicit IDs.
|
||||
//
|
||||
// Auth: any authenticated caller can renew certs they have read-access
|
||||
// to (matches POST /api/v1/certificates/{id}/renew). NOT admin-gated
|
||||
// like bulk-revoke — bulk-renew is non-destructive (worst case it
|
||||
// kicks off some redundant ACME orders) so we don't need the
|
||||
// fleet-scale-destruction gate.
|
||||
func (h BulkRenewalHandler) BulkRenew(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 req bulkRenewRequest
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
ErrorWithRequestID(w, http.StatusBadRequest, "Invalid request body", requestID)
|
||||
return
|
||||
}
|
||||
|
||||
criteria := domain.BulkRenewalCriteria{
|
||||
ProfileID: req.ProfileID,
|
||||
OwnerID: req.OwnerID,
|
||||
AgentID: req.AgentID,
|
||||
IssuerID: req.IssuerID,
|
||||
TeamID: req.TeamID,
|
||||
CertificateIDs: req.CertificateIDs,
|
||||
}
|
||||
if criteria.IsEmpty() {
|
||||
ErrorWithRequestID(w, http.StatusBadRequest,
|
||||
"At least one filter criterion is required (profile_id, owner_id, agent_id, issuer_id, team_id, or certificate_ids)",
|
||||
requestID)
|
||||
return
|
||||
}
|
||||
|
||||
actor := resolveActor(r.Context())
|
||||
|
||||
result, err := h.svc.BulkRenew(r.Context(), criteria, actor)
|
||||
if err != nil {
|
||||
ErrorWithRequestID(w, http.StatusInternalServerError, "Bulk renewal failed: "+err.Error(), requestID)
|
||||
return
|
||||
}
|
||||
|
||||
JSON(w, http.StatusOK, result)
|
||||
}
|
||||
@@ -0,0 +1,148 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/api/middleware"
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
)
|
||||
|
||||
// mockBulkRenewalService is a test implementation of BulkRenewalService.
|
||||
type mockBulkRenewalService struct {
|
||||
BulkRenewFn func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error)
|
||||
}
|
||||
|
||||
func (m *mockBulkRenewalService) BulkRenew(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
||||
if m.BulkRenewFn != nil {
|
||||
return m.BulkRenewFn(ctx, criteria, actor)
|
||||
}
|
||||
return &domain.BulkRenewalResult{}, nil
|
||||
}
|
||||
|
||||
// authedContext mirrors adminContext but without the admin flag —
|
||||
// bulk-renew is NOT admin-gated, any authenticated caller can use it.
|
||||
func authedContext() context.Context {
|
||||
ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id-renew")
|
||||
ctx = context.WithValue(ctx, middleware.UserKey{}, "alice")
|
||||
return ctx
|
||||
}
|
||||
|
||||
func TestBulkRenew_Handler_HappyPath(t *testing.T) {
|
||||
svc := &mockBulkRenewalService{
|
||||
BulkRenewFn: func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
||||
if len(criteria.CertificateIDs) != 3 {
|
||||
t.Errorf("expected 3 IDs, got %d", len(criteria.CertificateIDs))
|
||||
}
|
||||
if actor != "alice" {
|
||||
t.Errorf("actor = %q, want 'alice' (resolved from middleware UserKey)", actor)
|
||||
}
|
||||
return &domain.BulkRenewalResult{
|
||||
TotalMatched: 3,
|
||||
TotalEnqueued: 3,
|
||||
EnqueuedJobs: []domain.BulkEnqueuedJob{
|
||||
{CertificateID: "mc-1", JobID: "job-a"},
|
||||
{CertificateID: "mc-2", JobID: "job-b"},
|
||||
{CertificateID: "mc-3", JobID: "job-c"},
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
h := NewBulkRenewalHandler(svc)
|
||||
|
||||
body := `{"certificate_ids":["mc-1","mc-2","mc-3"]}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkRenew(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
var result domain.BulkRenewalResult
|
||||
if err := json.NewDecoder(w.Body).Decode(&result); err != nil {
|
||||
t.Fatalf("decode failed: %v", err)
|
||||
}
|
||||
if result.TotalEnqueued != 3 || len(result.EnqueuedJobs) != 3 {
|
||||
t.Errorf("envelope drift: enqueued=%d jobs=%d, want 3/3",
|
||||
result.TotalEnqueued, len(result.EnqueuedJobs))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkRenew_Handler_EmptyBody_400(t *testing.T) {
|
||||
svc := &mockBulkRenewalService{}
|
||||
h := NewBulkRenewalHandler(svc)
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(`{}`))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkRenew(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d, want 400 (empty criteria must reject)", w.Code)
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "filter criterion") {
|
||||
t.Errorf("body should name the criteria-required contract; got: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkRenew_Handler_WrongMethod_405(t *testing.T) {
|
||||
svc := &mockBulkRenewalService{}
|
||||
h := NewBulkRenewalHandler(svc)
|
||||
|
||||
for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodDelete, http.MethodPatch} {
|
||||
req := httptest.NewRequest(method, "/api/v1/certificates/bulk-renew", nil)
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkRenew(w, req)
|
||||
if w.Code != http.StatusMethodNotAllowed {
|
||||
t.Errorf("%s → status %d, want 405", method, w.Code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkRenew_Handler_ActorAttribution(t *testing.T) {
|
||||
var capturedActor string
|
||||
svc := &mockBulkRenewalService{
|
||||
BulkRenewFn: func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
||||
capturedActor = actor
|
||||
return &domain.BulkRenewalResult{}, nil
|
||||
},
|
||||
}
|
||||
h := NewBulkRenewalHandler(svc)
|
||||
|
||||
body := `{"certificate_ids":["mc-1"]}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(body))
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkRenew(w, req)
|
||||
|
||||
if capturedActor != "alice" {
|
||||
t.Errorf("actor not threaded from middleware.UserKey: got %q, want 'alice'", capturedActor)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkRenew_Handler_ServiceError_500(t *testing.T) {
|
||||
svc := &mockBulkRenewalService{
|
||||
BulkRenewFn: func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
||||
return nil, errors.New("simulated DB failure")
|
||||
},
|
||||
}
|
||||
h := NewBulkRenewalHandler(svc)
|
||||
body := `{"certificate_ids":["mc-1"]}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(body))
|
||||
req = req.WithContext(authedContext())
|
||||
w := httptest.NewRecorder()
|
||||
h.BulkRenew(w, req)
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("status = %d, want 500", w.Code)
|
||||
}
|
||||
}
|
||||
@@ -67,7 +67,13 @@ type HandlerRegistry struct {
|
||||
Digest handler.DigestHandler
|
||||
HealthChecks *handler.HealthCheckHandler
|
||||
BulkRevocation handler.BulkRevocationHandler
|
||||
RenewalPolicies handler.RenewalPolicyHandler
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5 + cat-l-8a1fb258a38a):
|
||||
// server-side bulk endpoints replace pre-L-1 client-side N×HTTP
|
||||
// loops in CertificatesPage.tsx. See handler/bulk_renewal.go and
|
||||
// handler/bulk_reassignment.go.
|
||||
BulkRenewal handler.BulkRenewalHandler
|
||||
BulkReassignment handler.BulkReassignmentHandler
|
||||
RenewalPolicies handler.RenewalPolicyHandler
|
||||
// Version handles GET /api/v1/version (U-3 ride-along,
|
||||
// cat-u-no_version_endpoint). Wired through the no-auth dispatch in
|
||||
// cmd/server/main.go so probes and rollout systems can read build
|
||||
@@ -109,8 +115,17 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
r.Register("GET /api/v1/auth/check", http.HandlerFunc(reg.Health.AuthCheck))
|
||||
|
||||
// Certificates routes: /api/v1/certificates
|
||||
// Bulk revoke must be registered before {id} routes to avoid path conflict
|
||||
// Bulk operations MUST register before {id} routes — Go 1.22 ServeMux
|
||||
// gives literal segments precedence over pattern-var segments, but
|
||||
// listing the bulk paths first makes the precedence operator-visible
|
||||
// and prevents a future refactor from accidentally inverting it. All
|
||||
// three bulk endpoints share the same envelope shape (criteria/IDs
|
||||
// in, {total_matched, total_<verb>, total_skipped, total_failed,
|
||||
// errors[]} out). L-1 master added bulk-renew + bulk-reassign
|
||||
// alongside the pre-existing bulk-revoke.
|
||||
r.Register("POST /api/v1/certificates/bulk-revoke", http.HandlerFunc(reg.BulkRevocation.BulkRevoke))
|
||||
r.Register("POST /api/v1/certificates/bulk-renew", http.HandlerFunc(reg.BulkRenewal.BulkRenew))
|
||||
r.Register("POST /api/v1/certificates/bulk-reassign", http.HandlerFunc(reg.BulkReassignment.BulkReassign))
|
||||
r.Register("GET /api/v1/certificates", http.HandlerFunc(reg.Certificates.ListCertificates))
|
||||
r.Register("POST /api/v1/certificates", http.HandlerFunc(reg.Certificates.CreateCertificate))
|
||||
r.Register("GET /api/v1/certificates/{id}", http.HandlerFunc(reg.Certificates.GetCertificate))
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
package domain
|
||||
|
||||
// BulkReassignmentRequest is the input to POST /api/v1/certificates/bulk-reassign.
|
||||
//
|
||||
// L-2 closure (cat-l-8a1fb258a38a): the GUI used to loop
|
||||
// `await updateCertificate(id, { owner_id: ownerId })` over the selection
|
||||
// at `web/src/pages/CertificatesPage.tsx::handleReassign`. Post-L-2 it
|
||||
// POSTs once.
|
||||
//
|
||||
// Narrower than BulkRenewalCriteria — the operator workflow is "I have N
|
||||
// certs selected and I want them all owned by Alice now". Criteria-mode
|
||||
// reassignment doesn't have a strong use case (operators query first,
|
||||
// then reassign by ID), so the request is IDs-only. OwnerID is required;
|
||||
// TeamID is optional and the cert's team_id is updated only when TeamID
|
||||
// is non-empty (matches the existing per-cert PUT behaviour where empty
|
||||
// fields leave the existing value unchanged).
|
||||
type BulkReassignmentRequest struct {
|
||||
CertificateIDs []string `json:"certificate_ids"`
|
||||
OwnerID string `json:"owner_id"`
|
||||
TeamID string `json:"team_id,omitempty"`
|
||||
}
|
||||
|
||||
// IsEmpty returns true if no IDs are provided. The service layer rejects
|
||||
// empty IDs with a 400 — explicit-IDs is the only selection mode for
|
||||
// reassignment (no criteria-mode). Naming mirrors BulkRevocationCriteria
|
||||
// + BulkRenewalCriteria.IsEmpty so the validate-and-reject pattern is
|
||||
// the same across all three bulk endpoints.
|
||||
func (r BulkReassignmentRequest) IsEmpty() bool {
|
||||
return len(r.CertificateIDs) == 0
|
||||
}
|
||||
|
||||
// BulkReassignmentResult mirrors BulkRevocationResult / BulkRenewalResult
|
||||
// envelope shape so the frontend's bulk-result rendering is one helper.
|
||||
//
|
||||
// Counters semantics:
|
||||
// - TotalMatched: number of certs resolved from CertificateIDs
|
||||
// - TotalReassigned: number where owner_id (and optionally team_id)
|
||||
// was actually mutated
|
||||
// - TotalSkipped: certs already owned by the target OwnerID — no-op
|
||||
// skip rather than a fake "succeeded" count, so operators see "5 of
|
||||
// your 10 selections were no-ops" without triaging fake errors
|
||||
// - TotalFailed: certs where the per-cert update returned an error
|
||||
// (e.g., the cert no longer exists, the repo update failed)
|
||||
// - Errors: per-cert error details for the failure path
|
||||
type BulkReassignmentResult struct {
|
||||
TotalMatched int `json:"total_matched"`
|
||||
TotalReassigned int `json:"total_reassigned"`
|
||||
TotalSkipped int `json:"total_skipped"`
|
||||
TotalFailed int `json:"total_failed"`
|
||||
Errors []BulkOperationError `json:"errors,omitempty"`
|
||||
}
|
||||
@@ -0,0 +1,77 @@
|
||||
package domain
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestBulkReassignmentRequest_IsEmpty(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
r BulkReassignmentRequest
|
||||
want bool
|
||||
}{
|
||||
{"all-zero", BulkReassignmentRequest{}, true},
|
||||
{"empty-ids-slice", BulkReassignmentRequest{CertificateIDs: []string{}}, true},
|
||||
{"ids-set-but-no-owner", BulkReassignmentRequest{CertificateIDs: []string{"mc-1"}}, false},
|
||||
// IsEmpty is a pure ID-presence check; OwnerID/TeamID are
|
||||
// validated separately in the service layer (OwnerID required;
|
||||
// TeamID optional). This split mirrors how BulkRevocationCriteria
|
||||
// + reason are validated in two distinct steps.
|
||||
{"owner-set-but-no-ids", BulkReassignmentRequest{OwnerID: "o-alice"}, true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := tt.r.IsEmpty(); got != tt.want {
|
||||
t.Errorf("IsEmpty() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkReassignmentResult_JSONShape(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := &BulkReassignmentResult{
|
||||
TotalMatched: 10,
|
||||
TotalReassigned: 7,
|
||||
TotalSkipped: 3, // already-owned-by-target — silent no-op
|
||||
TotalFailed: 0,
|
||||
Errors: nil,
|
||||
}
|
||||
b, err := json.Marshal(r)
|
||||
if err != nil {
|
||||
t.Fatalf("Marshal failed: %v", err)
|
||||
}
|
||||
var round map[string]interface{}
|
||||
if err := json.Unmarshal(b, &round); err != nil {
|
||||
t.Fatalf("Unmarshal failed: %v", err)
|
||||
}
|
||||
for _, k := range []string{"total_matched", "total_reassigned", "total_skipped", "total_failed"} {
|
||||
if _, ok := round[k]; !ok {
|
||||
t.Errorf("missing JSON field %q in %s", k, string(b))
|
||||
}
|
||||
}
|
||||
if _, ok := round["errors"]; ok {
|
||||
t.Errorf("nil Errors should be omitempty; got: %s", string(b))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBulkOperationError_JSONShape(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := BulkOperationError{
|
||||
CertificateID: "mc-1",
|
||||
Error: "renewal already in progress",
|
||||
}
|
||||
b, err := json.Marshal(e)
|
||||
if err != nil {
|
||||
t.Fatalf("Marshal failed: %v", err)
|
||||
}
|
||||
want := `{"certificate_id":"mc-1","error":"renewal already in progress"}`
|
||||
if string(b) != want {
|
||||
t.Errorf("JSON shape drift:\n got: %s\n want: %s", string(b), want)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
package domain
|
||||
|
||||
// BulkRenewalCriteria selects a set of managed certificates to renew. At
|
||||
// least one selector must be non-empty (IsEmpty() guards this in the
|
||||
// service layer; same shape and rule as BulkRevocationCriteria so
|
||||
// operators who already know the bulk-revoke contract have zero new
|
||||
// surface to learn).
|
||||
//
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5): the GUI used to loop
|
||||
// `await triggerRenewal(id)` over the selection at
|
||||
// `web/src/pages/CertificatesPage.tsx::handleBulkRenewal`. 100 certs =
|
||||
// 100 sequential HTTP round-trips × Auth → audit → handler → service →
|
||||
// repo → DB → audit. Post-L-1 the GUI POSTs once; the server resolves
|
||||
// the criteria, applies status filters, and enqueues N renewal jobs.
|
||||
//
|
||||
// The "renew all certs of profile X before its CA changes" use case is
|
||||
// the canonical reason to support criteria-mode in addition to explicit
|
||||
// IDs. Mirrors `BulkRevocationCriteria` field-for-field.
|
||||
type BulkRenewalCriteria struct {
|
||||
ProfileID string `json:"profile_id,omitempty"`
|
||||
OwnerID string `json:"owner_id,omitempty"`
|
||||
AgentID string `json:"agent_id,omitempty"`
|
||||
IssuerID string `json:"issuer_id,omitempty"`
|
||||
TeamID string `json:"team_id,omitempty"`
|
||||
CertificateIDs []string `json:"certificate_ids,omitempty"`
|
||||
}
|
||||
|
||||
// IsEmpty returns true if no filter criteria are set. The service layer
|
||||
// rejects empty criteria with a 400 (mirrors BulkRevocationCriteria.IsEmpty).
|
||||
func (c BulkRenewalCriteria) IsEmpty() bool {
|
||||
return c.ProfileID == "" && c.OwnerID == "" && c.AgentID == "" &&
|
||||
c.IssuerID == "" && c.TeamID == "" && len(c.CertificateIDs) == 0
|
||||
}
|
||||
|
||||
// BulkRenewalResult is the envelope returned to the caller. Distinct
|
||||
// from BulkRevocationResult because the action verb differs: renewal
|
||||
// ENQUEUES a job per matched cert (asynchronous) rather than performing
|
||||
// the mutation synchronously like revocation. The EnqueuedJobs slice
|
||||
// gives the caller the job IDs so the GUI can poll
|
||||
// /api/v1/jobs?status=Running for progress without re-querying the
|
||||
// certificate list.
|
||||
//
|
||||
// Counters semantics (mirror BulkRevocationResult conventions):
|
||||
// - TotalMatched: number of certs the criteria/IDs resolved to
|
||||
// - TotalEnqueued: number of renewal jobs successfully created
|
||||
// - TotalSkipped: certs in a status that disallows renewal (already
|
||||
// RenewalInProgress, Revoked, or Archived); silent no-op, NOT an error
|
||||
// - TotalFailed: certs where the enqueue path returned an error
|
||||
// - EnqueuedJobs: per-cert {certificate_id, job_id} pairs for the
|
||||
// successful enqueue path (omitempty so an all-skipped batch
|
||||
// produces a clean response)
|
||||
// - Errors: per-cert error details for the failure path
|
||||
type BulkRenewalResult struct {
|
||||
TotalMatched int `json:"total_matched"`
|
||||
TotalEnqueued int `json:"total_enqueued"`
|
||||
TotalSkipped int `json:"total_skipped"`
|
||||
TotalFailed int `json:"total_failed"`
|
||||
EnqueuedJobs []BulkEnqueuedJob `json:"enqueued_jobs,omitempty"`
|
||||
Errors []BulkOperationError `json:"errors,omitempty"`
|
||||
}
|
||||
|
||||
// BulkEnqueuedJob pairs a certificate ID with the renewal job ID that was
|
||||
// just created for it. Lets the GUI link directly into the job-detail
|
||||
// page without an extra round-trip to query "what job did this cert
|
||||
// just get assigned?".
|
||||
type BulkEnqueuedJob struct {
|
||||
CertificateID string `json:"certificate_id"`
|
||||
JobID string `json:"job_id"`
|
||||
}
|
||||
|
||||
// BulkOperationError records a per-certificate failure for any bulk
|
||||
// operation (renew, reassign — and revoke, which uses the older
|
||||
// BulkRevocationError shape kept for backwards compatibility on the
|
||||
// /bulk-revoke wire format). Same shape as BulkRevocationError so the
|
||||
// frontend's bulk-result rendering is one helper.
|
||||
type BulkOperationError struct {
|
||||
CertificateID string `json:"certificate_id"`
|
||||
Error string `json:"error"`
|
||||
}
|
||||
@@ -0,0 +1,83 @@
|
||||
package domain
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestBulkRenewalCriteria_IsEmpty pins the validate-and-reject contract:
|
||||
// empty criteria → service rejects with 400. Mirrors
|
||||
// TestBulkRevocationCriteria_IsEmpty exactly so the cross-bulk-endpoint
|
||||
// behaviour is uniform.
|
||||
func TestBulkRenewalCriteria_IsEmpty(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
c BulkRenewalCriteria
|
||||
want bool
|
||||
}{
|
||||
{"all-zero", BulkRenewalCriteria{}, true},
|
||||
{"profile-id-set", BulkRenewalCriteria{ProfileID: "cp-x"}, false},
|
||||
{"owner-id-set", BulkRenewalCriteria{OwnerID: "o-alice"}, false},
|
||||
{"agent-id-set", BulkRenewalCriteria{AgentID: "ag-1"}, false},
|
||||
{"issuer-id-set", BulkRenewalCriteria{IssuerID: "iss-x"}, false},
|
||||
{"team-id-set", BulkRenewalCriteria{TeamID: "t-x"}, false},
|
||||
{"ids-set", BulkRenewalCriteria{CertificateIDs: []string{"mc-1"}}, false},
|
||||
{"ids-empty-slice", BulkRenewalCriteria{CertificateIDs: []string{}}, true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := tt.c.IsEmpty(); got != tt.want {
|
||||
t.Errorf("IsEmpty() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenewalResult_JSONShape pins the wire contract. Operator
|
||||
// tooling (k8s rollouts, blackbox probes, the `certctl-cli bulk-renew`
|
||||
// JSON consumer) parses these field names; renaming any of them is a
|
||||
// breaking change.
|
||||
func TestBulkRenewalResult_JSONShape(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := &BulkRenewalResult{
|
||||
TotalMatched: 5,
|
||||
TotalEnqueued: 4,
|
||||
TotalSkipped: 1,
|
||||
TotalFailed: 0,
|
||||
EnqueuedJobs: []BulkEnqueuedJob{
|
||||
{CertificateID: "mc-1", JobID: "job-a"},
|
||||
},
|
||||
Errors: nil,
|
||||
}
|
||||
b, err := json.Marshal(r)
|
||||
if err != nil {
|
||||
t.Fatalf("Marshal failed: %v", err)
|
||||
}
|
||||
var round map[string]interface{}
|
||||
if err := json.Unmarshal(b, &round); err != nil {
|
||||
t.Fatalf("Unmarshal failed: %v", err)
|
||||
}
|
||||
|
||||
for _, k := range []string{"total_matched", "total_enqueued", "total_skipped", "total_failed", "enqueued_jobs"} {
|
||||
if _, ok := round[k]; !ok {
|
||||
t.Errorf("missing JSON field %q in %s", k, string(b))
|
||||
}
|
||||
}
|
||||
// errors omitempty when nil — must NOT appear
|
||||
if _, ok := round["errors"]; ok {
|
||||
t.Errorf("nil Errors should be omitempty; got: %s", string(b))
|
||||
}
|
||||
|
||||
// EnqueuedJobs nested shape
|
||||
jobs := round["enqueued_jobs"].([]interface{})
|
||||
if len(jobs) != 1 {
|
||||
t.Fatalf("enqueued_jobs len = %d, want 1", len(jobs))
|
||||
}
|
||||
first := jobs[0].(map[string]interface{})
|
||||
if first["certificate_id"] != "mc-1" || first["job_id"] != "job-a" {
|
||||
t.Errorf("BulkEnqueuedJob field names drifted: %v", first)
|
||||
}
|
||||
}
|
||||
@@ -214,6 +214,61 @@ func registerCertificateTools(s *gomcp.Server, c *Client) {
|
||||
}
|
||||
return textResult(data)
|
||||
})
|
||||
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5): bulk-renew MCP tool.
|
||||
// Mirrors certctl_bulk_revoke_certificates shape sans the Reason
|
||||
// field. Server returns total_matched / total_enqueued /
|
||||
// total_skipped / total_failed plus per-cert {certificate_id,
|
||||
// job_id} pairs in enqueued_jobs.
|
||||
gomcp.AddTool(s, &gomcp.Tool{
|
||||
Name: "certctl_bulk_renew_certificates",
|
||||
Description: "Bulk renew certificates matching filter criteria (profile_id, owner_id, agent_id, issuer_id, team_id) or an explicit certificate_ids list. At least one selector required. Returns counts of matched, enqueued, skipped, and failed certificates plus per-cert {certificate_id, job_id} pairs.",
|
||||
}, func(ctx context.Context, req *gomcp.CallToolRequest, input BulkRenewCertificatesInput) (*gomcp.CallToolResult, any, error) {
|
||||
body := map[string]interface{}{}
|
||||
if input.ProfileID != "" {
|
||||
body["profile_id"] = input.ProfileID
|
||||
}
|
||||
if input.OwnerID != "" {
|
||||
body["owner_id"] = input.OwnerID
|
||||
}
|
||||
if input.AgentID != "" {
|
||||
body["agent_id"] = input.AgentID
|
||||
}
|
||||
if input.IssuerID != "" {
|
||||
body["issuer_id"] = input.IssuerID
|
||||
}
|
||||
if input.TeamID != "" {
|
||||
body["team_id"] = input.TeamID
|
||||
}
|
||||
if len(input.CertificateIDs) > 0 {
|
||||
body["certificate_ids"] = input.CertificateIDs
|
||||
}
|
||||
data, err := c.Post("/api/v1/certificates/bulk-renew", body)
|
||||
if err != nil {
|
||||
return errorResult(err)
|
||||
}
|
||||
return textResult(data)
|
||||
})
|
||||
|
||||
// L-2 closure (cat-l-8a1fb258a38a): bulk-reassign MCP tool.
|
||||
// Narrower than bulk-renew/revoke — IDs-only, no criteria-mode.
|
||||
gomcp.AddTool(s, &gomcp.Tool{
|
||||
Name: "certctl_bulk_reassign_certificates",
|
||||
Description: "Bulk reassign owner (and optionally team) for a set of certificates. owner_id is required. team_id is optional and updates only when non-empty. Returns counts of matched, reassigned, skipped (already-owned-by-target), and failed certificates.",
|
||||
}, func(ctx context.Context, req *gomcp.CallToolRequest, input BulkReassignCertificatesInput) (*gomcp.CallToolResult, any, error) {
|
||||
body := map[string]interface{}{
|
||||
"certificate_ids": input.CertificateIDs,
|
||||
"owner_id": input.OwnerID,
|
||||
}
|
||||
if input.TeamID != "" {
|
||||
body["team_id"] = input.TeamID
|
||||
}
|
||||
data, err := c.Post("/api/v1/certificates/bulk-reassign", body)
|
||||
if err != nil {
|
||||
return errorResult(err)
|
||||
}
|
||||
return textResult(data)
|
||||
})
|
||||
}
|
||||
|
||||
// ── CRL & OCSP ──────────────────────────────────────────────────────
|
||||
|
||||
@@ -72,6 +72,25 @@ type BulkRevokeCertificatesInput struct {
|
||||
CertificateIDs []string `json:"certificate_ids,omitempty" jsonschema:"Explicit list of certificate IDs to revoke"`
|
||||
}
|
||||
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5): bulk-renew MCP tool input.
|
||||
// Mirrors BulkRevokeCertificatesInput field-for-field minus Reason.
|
||||
type BulkRenewCertificatesInput struct {
|
||||
ProfileID string `json:"profile_id,omitempty" jsonschema:"Renew all certs matching this profile ID"`
|
||||
OwnerID string `json:"owner_id,omitempty" jsonschema:"Renew all certs owned by this owner"`
|
||||
AgentID string `json:"agent_id,omitempty" jsonschema:"Renew all certs deployed via this agent"`
|
||||
IssuerID string `json:"issuer_id,omitempty" jsonschema:"Renew all certs issued by this issuer"`
|
||||
TeamID string `json:"team_id,omitempty" jsonschema:"Renew all certs owned by members of this team"`
|
||||
CertificateIDs []string `json:"certificate_ids,omitempty" jsonschema:"Explicit list of certificate IDs to renew"`
|
||||
}
|
||||
|
||||
// L-2 closure (cat-l-8a1fb258a38a): bulk-reassign MCP tool input.
|
||||
// IDs-only — no criteria-mode.
|
||||
type BulkReassignCertificatesInput struct {
|
||||
CertificateIDs []string `json:"certificate_ids" jsonschema:"Explicit list of certificate IDs to reassign"`
|
||||
OwnerID string `json:"owner_id" jsonschema:"Required. New owner_id for every cert in certificate_ids"`
|
||||
TeamID string `json:"team_id,omitempty" jsonschema:"Optional. When non-empty, also updates team_id on every cert"`
|
||||
}
|
||||
|
||||
type ListVersionsInput struct {
|
||||
ID string `json:"id" jsonschema:"Certificate ID"`
|
||||
ListParams
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strings"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
"github.com/shankar0123/certctl/internal/repository"
|
||||
)
|
||||
|
||||
// ErrBulkReassignOwnerNotFound is the typed sentinel for a non-existent
|
||||
// target OwnerID. The handler maps it to 400 (bad input — the operator
|
||||
// picked an owner that doesn't exist) rather than 500 (server error).
|
||||
// Sentinel-error rather than substring-error matches the project's
|
||||
// post-M-1 error-mapping convention.
|
||||
var ErrBulkReassignOwnerNotFound = errors.New("owner not found")
|
||||
|
||||
// BulkReassignmentService coordinates bulk owner-reassignment of
|
||||
// certificates.
|
||||
//
|
||||
// L-2 closure (cat-l-8a1fb258a38a): the GUI used to loop
|
||||
// `await updateCertificate(id, { owner_id })` over the selection at
|
||||
// `web/src/pages/CertificatesPage.tsx::handleReassign`. Post-L-2 the
|
||||
// GUI POSTs once. Narrower than BulkRenewal: explicit IDs only, no
|
||||
// criteria-mode (criteria-mode reassignment doesn't have a strong use
|
||||
// case — operators query first then reassign by ID).
|
||||
//
|
||||
// Validation order: empty IDs → 400, missing OwnerID → 400, OwnerID
|
||||
// not in owners table → 400 (ErrBulkReassignOwnerNotFound). Resolving
|
||||
// the owner upfront means we fail-fast without mutating any cert if
|
||||
// the operator typo'd the owner ID.
|
||||
type BulkReassignmentService struct {
|
||||
certRepo repository.CertificateRepository
|
||||
ownerRepo repository.OwnerRepository
|
||||
auditService *AuditService
|
||||
logger *slog.Logger
|
||||
}
|
||||
|
||||
// NewBulkReassignmentService creates a new BulkReassignmentService.
|
||||
func NewBulkReassignmentService(
|
||||
certRepo repository.CertificateRepository,
|
||||
ownerRepo repository.OwnerRepository,
|
||||
auditService *AuditService,
|
||||
logger *slog.Logger,
|
||||
) *BulkReassignmentService {
|
||||
return &BulkReassignmentService{
|
||||
certRepo: certRepo,
|
||||
ownerRepo: ownerRepo,
|
||||
auditService: auditService,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// BulkReassign updates owner_id (and optionally team_id) on every cert
|
||||
// in request.CertificateIDs. Skips certs whose owner_id already equals
|
||||
// the target (silent no-op — surfaced as TotalSkipped++, not as a fake
|
||||
// "succeeded" count, so operators see "5 of your 10 selections were
|
||||
// no-ops because Alice already owned them" without triaging fake
|
||||
// errors).
|
||||
//
|
||||
// Partial failures don't abort the batch — the failing cert lands in
|
||||
// Errors[]; the loop continues. Mirrors BulkRevocationService and
|
||||
// BulkRenewalService partial-failure semantics.
|
||||
//
|
||||
// Audit: a single audit event is emitted at the end with the criteria
|
||||
// + counts. NOT N events.
|
||||
func (s *BulkReassignmentService) BulkReassign(ctx context.Context, request domain.BulkReassignmentRequest, actor string) (*domain.BulkReassignmentResult, error) {
|
||||
if request.IsEmpty() {
|
||||
return nil, fmt.Errorf("at least one certificate_id is required")
|
||||
}
|
||||
if request.OwnerID == "" {
|
||||
return nil, fmt.Errorf("owner_id is required")
|
||||
}
|
||||
|
||||
// Validate the target owner exists BEFORE touching any cert. This
|
||||
// fail-fast pattern means an operator who typo'd 'o-alic' (missing
|
||||
// 'e') doesn't half-reassign 50 certs before the 51st surfaces the
|
||||
// FK violation.
|
||||
if _, err := s.ownerRepo.Get(ctx, request.OwnerID); err != nil {
|
||||
return nil, fmt.Errorf("%w: %s", ErrBulkReassignOwnerNotFound, request.OwnerID)
|
||||
}
|
||||
|
||||
result := &domain.BulkReassignmentResult{}
|
||||
|
||||
for _, id := range request.CertificateIDs {
|
||||
cert, err := s.certRepo.Get(ctx, id)
|
||||
if err != nil {
|
||||
result.TotalFailed++
|
||||
result.Errors = append(result.Errors, domain.BulkOperationError{
|
||||
CertificateID: id,
|
||||
Error: fmt.Sprintf("failed to fetch certificate: %v", err),
|
||||
})
|
||||
continue
|
||||
}
|
||||
result.TotalMatched++
|
||||
|
||||
// No-op skip: cert already owned by the target. team_id may
|
||||
// still differ — we still skip if owner matches AND
|
||||
// team_id-update is a no-op (team unchanged or team_id field
|
||||
// not set on the request). This prevents fake "reassigned"
|
||||
// counts when nothing actually changed.
|
||||
ownerUnchanged := cert.OwnerID == request.OwnerID
|
||||
teamUnchanged := request.TeamID == "" || cert.TeamID == request.TeamID
|
||||
if ownerUnchanged && teamUnchanged {
|
||||
result.TotalSkipped++
|
||||
continue
|
||||
}
|
||||
|
||||
cert.OwnerID = request.OwnerID
|
||||
if request.TeamID != "" {
|
||||
cert.TeamID = request.TeamID
|
||||
}
|
||||
if err := s.certRepo.Update(ctx, cert); err != nil {
|
||||
result.TotalFailed++
|
||||
result.Errors = append(result.Errors, domain.BulkOperationError{
|
||||
CertificateID: id,
|
||||
Error: fmt.Sprintf("failed to update certificate: %v", err),
|
||||
})
|
||||
s.logger.Warn("bulk reassignment: update failed",
|
||||
"certificate_id", id, "error", err)
|
||||
continue
|
||||
}
|
||||
result.TotalReassigned++
|
||||
}
|
||||
|
||||
// Single bulk audit event at the end.
|
||||
auditDetails := map[string]interface{}{
|
||||
"owner_id": request.OwnerID,
|
||||
"certificate_ids": strings.Join(request.CertificateIDs, ","),
|
||||
"total_matched": result.TotalMatched,
|
||||
"total_reassigned": result.TotalReassigned,
|
||||
"total_skipped": result.TotalSkipped,
|
||||
"total_failed": result.TotalFailed,
|
||||
}
|
||||
if request.TeamID != "" {
|
||||
auditDetails["team_id"] = request.TeamID
|
||||
}
|
||||
if err := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser,
|
||||
"bulk_reassignment_initiated", "certificate", "bulk",
|
||||
auditDetails); err != nil {
|
||||
s.logger.Error("failed to record bulk reassignment audit event", "error", err)
|
||||
}
|
||||
|
||||
return result, nil
|
||||
}
|
||||
@@ -0,0 +1,221 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
)
|
||||
|
||||
func newBulkReassignmentTestService() (*BulkReassignmentService, *mockCertRepo, *mockOwnerRepo, *mockAuditRepo) {
|
||||
certRepo := newMockCertificateRepository()
|
||||
ownerRepo := newMockOwnerRepository()
|
||||
auditRepo := newMockAuditRepository()
|
||||
auditService := NewAuditService(auditRepo)
|
||||
svc := NewBulkReassignmentService(certRepo, ownerRepo, auditService, slog.Default())
|
||||
return svc, certRepo, ownerRepo, auditRepo
|
||||
}
|
||||
|
||||
// addOwnedCert seeds a cert with a specific owner+team for reassignment.
|
||||
func addOwnedCert(repo *mockCertRepo, id, ownerID, teamID string) {
|
||||
cert := &domain.ManagedCertificate{
|
||||
ID: id, CommonName: id, Status: domain.CertificateStatusActive,
|
||||
OwnerID: ownerID, TeamID: teamID,
|
||||
ExpiresAt: time.Now().AddDate(0, 1, 0),
|
||||
}
|
||||
repo.AddCert(cert)
|
||||
}
|
||||
|
||||
func addOwner(repo *mockOwnerRepo, id string) {
|
||||
repo.owners[id] = &domain.Owner{ID: id, Name: id}
|
||||
}
|
||||
|
||||
// TestBulkReassign_HappyPath — N certs all reassigned successfully.
|
||||
func TestBulkReassign_HappyPath(t *testing.T) {
|
||||
svc, certRepo, ownerRepo, _ := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "")
|
||||
addOwnedCert(certRepo, "mc-2", "o-alice", "")
|
||||
addOwnedCert(certRepo, "mc-3", "o-alice", "")
|
||||
|
||||
res, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{
|
||||
CertificateIDs: []string{"mc-1", "mc-2", "mc-3"},
|
||||
OwnerID: "o-bob",
|
||||
}, "admin")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkReassign failed: %v", err)
|
||||
}
|
||||
if res.TotalReassigned != 3 || res.TotalSkipped != 0 || res.TotalFailed != 0 {
|
||||
t.Errorf("counts = reassigned:%d skipped:%d failed:%d, want 3/0/0",
|
||||
res.TotalReassigned, res.TotalSkipped, res.TotalFailed)
|
||||
}
|
||||
for _, id := range []string{"mc-1", "mc-2", "mc-3"} {
|
||||
if certRepo.Certs[id].OwnerID != "o-bob" {
|
||||
t.Errorf("cert %s: owner_id = %s, want o-bob", id, certRepo.Certs[id].OwnerID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_SkipsAlreadyOwned — certs already owned by the
|
||||
// target are no-op-skipped (not counted as reassigned, not surfaced as
|
||||
// errors). Operator sees "5 of your 10 selections were no-ops because
|
||||
// Bob already owned them" without triaging fake errors.
|
||||
func TestBulkReassign_SkipsAlreadyOwned(t *testing.T) {
|
||||
svc, certRepo, ownerRepo, _ := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
addOwnedCert(certRepo, "mc-1", "o-bob", "") // already owned by target
|
||||
addOwnedCert(certRepo, "mc-2", "o-alice", "") // needs reassign
|
||||
|
||||
res, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{
|
||||
CertificateIDs: []string{"mc-1", "mc-2"},
|
||||
OwnerID: "o-bob",
|
||||
}, "admin")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkReassign failed: %v", err)
|
||||
}
|
||||
if res.TotalReassigned != 1 || res.TotalSkipped != 1 {
|
||||
t.Errorf("counts = reassigned:%d skipped:%d, want 1/1", res.TotalReassigned, res.TotalSkipped)
|
||||
}
|
||||
if len(res.Errors) != 0 {
|
||||
t.Errorf("already-owned skip should NOT populate Errors; got %v", res.Errors)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_OwnerIDRequired_Error — empty owner_id rejected.
|
||||
func TestBulkReassign_OwnerIDRequired_Error(t *testing.T) {
|
||||
svc, certRepo, _, _ := newBulkReassignmentTestService()
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "")
|
||||
_, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{CertificateIDs: []string{"mc-1"}, OwnerID: ""}, "admin")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for empty owner_id, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_EmptyIDs_Error — empty IDs rejected.
|
||||
func TestBulkReassign_EmptyIDs_Error(t *testing.T) {
|
||||
svc, _, ownerRepo, _ := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
_, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{CertificateIDs: []string{}, OwnerID: "o-bob"}, "admin")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for empty IDs, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_OwnerNotFound_TypedSentinel — non-existent OwnerID
|
||||
// returns ErrBulkReassignOwnerNotFound. Handler maps this to 400 (the
|
||||
// operator picked an owner that doesn't exist) rather than 500 (server
|
||||
// error). Sentinel-error rather than substring-error matches the
|
||||
// project's post-M-1 error-mapping convention.
|
||||
func TestBulkReassign_OwnerNotFound_TypedSentinel(t *testing.T) {
|
||||
svc, certRepo, _, _ := newBulkReassignmentTestService()
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "")
|
||||
_, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{CertificateIDs: []string{"mc-1"}, OwnerID: "o-ghost"}, "admin")
|
||||
if err == nil {
|
||||
t.Fatal("expected ErrBulkReassignOwnerNotFound, got nil")
|
||||
}
|
||||
if !errors.Is(err, ErrBulkReassignOwnerNotFound) {
|
||||
t.Errorf("err is not ErrBulkReassignOwnerNotFound; got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_TeamIDOptional — happy path WITHOUT team_id leaves
|
||||
// team_id unchanged. Empty team_id in request must not zero out the
|
||||
// existing team_id on the cert.
|
||||
func TestBulkReassign_TeamIDOptional(t *testing.T) {
|
||||
svc, certRepo, ownerRepo, _ := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "t-platform")
|
||||
|
||||
_, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{
|
||||
CertificateIDs: []string{"mc-1"},
|
||||
OwnerID: "o-bob",
|
||||
// TeamID intentionally omitted
|
||||
}, "admin")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkReassign failed: %v", err)
|
||||
}
|
||||
if certRepo.Certs["mc-1"].TeamID != "t-platform" {
|
||||
t.Errorf("team_id was zeroed out; want unchanged 't-platform', got %q", certRepo.Certs["mc-1"].TeamID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_TeamIDProvided_Updates — when TeamID is non-empty in
|
||||
// the request, both owner_id and team_id update.
|
||||
func TestBulkReassign_TeamIDProvided_Updates(t *testing.T) {
|
||||
svc, certRepo, ownerRepo, _ := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "t-platform")
|
||||
|
||||
_, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{
|
||||
CertificateIDs: []string{"mc-1"},
|
||||
OwnerID: "o-bob",
|
||||
TeamID: "t-security",
|
||||
}, "admin")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkReassign failed: %v", err)
|
||||
}
|
||||
if certRepo.Certs["mc-1"].TeamID != "t-security" {
|
||||
t.Errorf("team_id = %q, want t-security", certRepo.Certs["mc-1"].TeamID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_PartialFailure — N=3, one cert mid-batch hits an
|
||||
// Update error. Rest of the batch continues; failure surfaced in
|
||||
// Errors.
|
||||
func TestBulkReassign_PartialFailure(t *testing.T) {
|
||||
svc, certRepo, ownerRepo, _ := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "")
|
||||
addOwnedCert(certRepo, "mc-2", "o-alice", "")
|
||||
addOwnedCert(certRepo, "mc-3", "o-alice", "")
|
||||
|
||||
// Force the next Update to fail uniformly. Mirrors how
|
||||
// TestBulkRevoke_PartialFailure injects a downstream failure.
|
||||
certRepo.UpdateErr = errors.New("simulated DB outage")
|
||||
|
||||
res, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{
|
||||
CertificateIDs: []string{"mc-1", "mc-2", "mc-3"},
|
||||
OwnerID: "o-bob",
|
||||
}, "admin")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkReassign should not propagate per-cert errors; got: %v", err)
|
||||
}
|
||||
if res.TotalFailed != 3 || res.TotalReassigned != 0 {
|
||||
t.Errorf("counts = failed:%d reassigned:%d, want 3/0", res.TotalFailed, res.TotalReassigned)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkReassign_AuditEventEmitted — single bulk audit event.
|
||||
func TestBulkReassign_AuditEventEmitted(t *testing.T) {
|
||||
svc, certRepo, ownerRepo, auditRepo := newBulkReassignmentTestService()
|
||||
addOwner(ownerRepo, "o-bob")
|
||||
addOwnedCert(certRepo, "mc-1", "o-alice", "")
|
||||
addOwnedCert(certRepo, "mc-2", "o-alice", "")
|
||||
|
||||
_, err := svc.BulkReassign(context.Background(),
|
||||
domain.BulkReassignmentRequest{
|
||||
CertificateIDs: []string{"mc-1", "mc-2"},
|
||||
OwnerID: "o-bob",
|
||||
}, "admin")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkReassign failed: %v", err)
|
||||
}
|
||||
|
||||
if len(auditRepo.Events) != 1 {
|
||||
t.Errorf("audit events count = %d, want exactly 1 (one bulk event, NOT N per-cert events)", len(auditRepo.Events))
|
||||
}
|
||||
if len(auditRepo.Events) > 0 && auditRepo.Events[0].Action != "bulk_reassignment_initiated" {
|
||||
t.Errorf("audit action = %q, want 'bulk_reassignment_initiated'", auditRepo.Events[0].Action)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,245 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
"github.com/shankar0123/certctl/internal/repository"
|
||||
)
|
||||
|
||||
// BulkRenewalService coordinates bulk certificate renewal operations.
|
||||
// Mirrors BulkRevocationService in shape: resolve criteria → status filter →
|
||||
// per-cert action loop → aggregate result + emit one bulk audit event.
|
||||
//
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5): the GUI used to loop
|
||||
// `await triggerRenewal(id)` over the selection at
|
||||
// `web/src/pages/CertificatesPage.tsx::handleBulkRenewal` (~line 411).
|
||||
// 100 certs = 100 sequential HTTP round-trips. Post-L-1 the GUI POSTs
|
||||
// once; this service does the loop server-side and returns a single
|
||||
// envelope with per-cert {certificate_id, job_id} pairs in
|
||||
// EnqueuedJobs and per-cert errors in Errors.
|
||||
//
|
||||
// Action verb is sync-enqueue (not sync-issue): for each matched cert
|
||||
// flip status to RenewalInProgress and create a Job row. The
|
||||
// scheduler's job processor picks up the jobs asynchronously. Sync-
|
||||
// issue would block the HTTP request for minutes against a slow ACME
|
||||
// issuer, which defeats the bulk-endpoint latency improvement.
|
||||
type BulkRenewalService struct {
|
||||
certRepo repository.CertificateRepository
|
||||
jobRepo repository.JobRepository
|
||||
auditService *AuditService
|
||||
logger *slog.Logger
|
||||
keygenMode string
|
||||
}
|
||||
|
||||
// NewBulkRenewalService creates a new BulkRenewalService.
|
||||
//
|
||||
// keygenMode mirrors CertificateService.keygenMode — agent-mode jobs
|
||||
// start as AwaitingCSR (the agent generates the key + submits a CSR);
|
||||
// server-mode jobs start as Pending. The bulk path must produce jobs in
|
||||
// the SAME initial status the single-cert path does, otherwise the
|
||||
// scheduler routes them differently.
|
||||
func NewBulkRenewalService(
|
||||
certRepo repository.CertificateRepository,
|
||||
jobRepo repository.JobRepository,
|
||||
auditService *AuditService,
|
||||
logger *slog.Logger,
|
||||
keygenMode string,
|
||||
) *BulkRenewalService {
|
||||
return &BulkRenewalService{
|
||||
certRepo: certRepo,
|
||||
jobRepo: jobRepo,
|
||||
auditService: auditService,
|
||||
logger: logger,
|
||||
keygenMode: keygenMode,
|
||||
}
|
||||
}
|
||||
|
||||
// BulkRenew enqueues a renewal job for every certificate matching the
|
||||
// criteria (or in the explicit IDs list). Status filter:
|
||||
// - Archived / Expired / Revoked → silent skip (TotalSkipped++)
|
||||
// - RenewalInProgress → silent skip (avoid double-enqueue)
|
||||
// - everything else → flip to RenewalInProgress + create job
|
||||
//
|
||||
// Partial failures don't abort the batch — the failing cert lands in
|
||||
// Errors[] with the error string, and the loop continues. Mirrors
|
||||
// BulkRevocationService.BulkRevoke's partial-failure semantics.
|
||||
//
|
||||
// Audit: a single audit event is emitted at the end with the criteria
|
||||
// + counts (NOT N events). The single-cert TriggerRenewal path emits
|
||||
// per-cert audit events; the bulk path uses one bulk envelope to keep
|
||||
// audit_events from growing 100x for one operator click.
|
||||
func (s *BulkRenewalService) BulkRenew(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
||||
if criteria.IsEmpty() {
|
||||
return nil, fmt.Errorf("at least one filter criterion is required")
|
||||
}
|
||||
|
||||
certs, err := s.resolveCertificates(ctx, criteria)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve certificates: %w", err)
|
||||
}
|
||||
|
||||
result := &domain.BulkRenewalResult{
|
||||
TotalMatched: len(certs),
|
||||
}
|
||||
|
||||
for _, cert := range certs {
|
||||
// Status-filter the cert before mutating. Mirrors the
|
||||
// eligibility checks in CertificateService.TriggerRenewal so a
|
||||
// bulk caller can't bypass them. Each illegal status maps to a
|
||||
// silent TotalSkipped++ rather than an Error so the operator
|
||||
// sees "5 of your 10 selections were no-ops" without triaging
|
||||
// fake errors.
|
||||
if cert.Status == domain.CertificateStatusArchived ||
|
||||
cert.Status == domain.CertificateStatusRevoked ||
|
||||
cert.Status == domain.CertificateStatusExpired ||
|
||||
cert.Status == domain.CertificateStatusRenewalInProgress {
|
||||
result.TotalSkipped++
|
||||
continue
|
||||
}
|
||||
|
||||
// Flip status + create job. Bug-for-bug match with
|
||||
// CertificateService.TriggerRenewal so the scheduler routing
|
||||
// stays identical between the single-cert and bulk paths.
|
||||
cert.Status = domain.CertificateStatusRenewalInProgress
|
||||
if err := s.certRepo.Update(ctx, cert); err != nil {
|
||||
result.TotalFailed++
|
||||
result.Errors = append(result.Errors, domain.BulkOperationError{
|
||||
CertificateID: cert.ID,
|
||||
Error: fmt.Sprintf("failed to update certificate status: %v", err),
|
||||
})
|
||||
s.logger.Warn("bulk renewal: status update failed",
|
||||
"certificate_id", cert.ID, "error", err)
|
||||
continue
|
||||
}
|
||||
|
||||
jobStatus := domain.JobStatusPending
|
||||
if s.keygenMode == "agent" {
|
||||
jobStatus = domain.JobStatusAwaitingCSR
|
||||
}
|
||||
jobType := domain.JobTypeRenewal
|
||||
if cert.ExpiresAt.IsZero() || cert.ExpiresAt.Year() < 2000 {
|
||||
jobType = domain.JobTypeIssuance
|
||||
}
|
||||
job := &domain.Job{
|
||||
ID: generateID("job"),
|
||||
CertificateID: cert.ID,
|
||||
Type: jobType,
|
||||
Status: jobStatus,
|
||||
MaxAttempts: 3,
|
||||
ScheduledAt: time.Now(),
|
||||
CreatedAt: time.Now(),
|
||||
}
|
||||
if err := s.jobRepo.Create(ctx, job); err != nil {
|
||||
result.TotalFailed++
|
||||
result.Errors = append(result.Errors, domain.BulkOperationError{
|
||||
CertificateID: cert.ID,
|
||||
Error: fmt.Sprintf("failed to create renewal job: %v", err),
|
||||
})
|
||||
s.logger.Warn("bulk renewal: job creation failed",
|
||||
"certificate_id", cert.ID, "error", err)
|
||||
continue
|
||||
}
|
||||
|
||||
result.TotalEnqueued++
|
||||
result.EnqueuedJobs = append(result.EnqueuedJobs, domain.BulkEnqueuedJob{
|
||||
CertificateID: cert.ID,
|
||||
JobID: job.ID,
|
||||
})
|
||||
}
|
||||
|
||||
// Single bulk audit event at the end. Mirrors
|
||||
// BulkRevocationService.BulkRevoke shape so the audit dashboard's
|
||||
// rendering of bulk events is uniform across {revoke, renew, reassign}.
|
||||
criteriaDetails := s.buildAuditDetails(criteria)
|
||||
criteriaDetails["total_matched"] = result.TotalMatched
|
||||
criteriaDetails["total_enqueued"] = result.TotalEnqueued
|
||||
criteriaDetails["total_skipped"] = result.TotalSkipped
|
||||
criteriaDetails["total_failed"] = result.TotalFailed
|
||||
if err := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser,
|
||||
"bulk_renewal_initiated", "certificate", "bulk",
|
||||
criteriaDetails); err != nil {
|
||||
s.logger.Error("failed to record bulk renewal audit event", "error", err)
|
||||
}
|
||||
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// resolveCertificates fetches the set of certificates matching the bulk
|
||||
// renewal criteria. Mirrors BulkRevocationService.resolveCertificates
|
||||
// behaviour exactly: explicit IDs alone → fetch each by ID; filter
|
||||
// criteria → repo.List with high per_page; both → intersect.
|
||||
func (s *BulkRenewalService) resolveCertificates(ctx context.Context, criteria domain.BulkRenewalCriteria) ([]*domain.ManagedCertificate, error) {
|
||||
hasFilterCriteria := criteria.ProfileID != "" || criteria.OwnerID != "" ||
|
||||
criteria.AgentID != "" || criteria.IssuerID != "" || criteria.TeamID != ""
|
||||
hasExplicitIDs := len(criteria.CertificateIDs) > 0
|
||||
|
||||
if hasExplicitIDs && !hasFilterCriteria {
|
||||
var certs []*domain.ManagedCertificate
|
||||
for _, id := range criteria.CertificateIDs {
|
||||
cert, err := s.certRepo.Get(ctx, id)
|
||||
if err != nil {
|
||||
continue // not-found certs silently drop out of the matched set
|
||||
}
|
||||
certs = append(certs, cert)
|
||||
}
|
||||
return certs, nil
|
||||
}
|
||||
|
||||
filter := &repository.CertificateFilter{
|
||||
OwnerID: criteria.OwnerID,
|
||||
TeamID: criteria.TeamID,
|
||||
IssuerID: criteria.IssuerID,
|
||||
AgentID: criteria.AgentID,
|
||||
ProfileID: criteria.ProfileID,
|
||||
PerPage: 10000,
|
||||
}
|
||||
certs, _, err := s.certRepo.List(ctx, filter)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if hasExplicitIDs {
|
||||
idSet := make(map[string]bool, len(criteria.CertificateIDs))
|
||||
for _, id := range criteria.CertificateIDs {
|
||||
idSet[id] = true
|
||||
}
|
||||
var filtered []*domain.ManagedCertificate
|
||||
for _, cert := range certs {
|
||||
if idSet[cert.ID] {
|
||||
filtered = append(filtered, cert)
|
||||
}
|
||||
}
|
||||
return filtered, nil
|
||||
}
|
||||
return certs, nil
|
||||
}
|
||||
|
||||
// buildAuditDetails constructs a map of criteria fields for the audit
|
||||
// event. Mirrors BulkRevocationService.buildAuditDetails so the audit
|
||||
// dashboard renders bulk events uniformly.
|
||||
func (s *BulkRenewalService) buildAuditDetails(criteria domain.BulkRenewalCriteria) map[string]interface{} {
|
||||
details := map[string]interface{}{}
|
||||
if criteria.ProfileID != "" {
|
||||
details["profile_id"] = criteria.ProfileID
|
||||
}
|
||||
if criteria.OwnerID != "" {
|
||||
details["owner_id"] = criteria.OwnerID
|
||||
}
|
||||
if criteria.AgentID != "" {
|
||||
details["agent_id"] = criteria.AgentID
|
||||
}
|
||||
if criteria.IssuerID != "" {
|
||||
details["issuer_id"] = criteria.IssuerID
|
||||
}
|
||||
if criteria.TeamID != "" {
|
||||
details["team_id"] = criteria.TeamID
|
||||
}
|
||||
if len(criteria.CertificateIDs) > 0 {
|
||||
details["certificate_ids"] = strings.Join(criteria.CertificateIDs, ",")
|
||||
}
|
||||
return details
|
||||
}
|
||||
@@ -0,0 +1,221 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/shankar0123/certctl/internal/domain"
|
||||
)
|
||||
|
||||
// newBulkRenewalTestService spins up a BulkRenewalService wired against
|
||||
// the in-memory mocks used by every other service test in this package.
|
||||
// keygenMode defaults to "agent" — production-like routing where renewal
|
||||
// jobs start as AwaitingCSR.
|
||||
func newBulkRenewalTestService() (*BulkRenewalService, *mockCertRepo, *mockJobRepo, *mockAuditRepo) {
|
||||
certRepo := newMockCertificateRepository()
|
||||
jobRepo := &mockJobRepo{Jobs: map[string]*domain.Job{}}
|
||||
auditRepo := newMockAuditRepository()
|
||||
auditService := NewAuditService(auditRepo)
|
||||
svc := NewBulkRenewalService(certRepo, jobRepo, auditService, slog.Default(), "agent")
|
||||
return svc, certRepo, jobRepo, auditRepo
|
||||
}
|
||||
|
||||
// addRenewableCert seeds a cert that is eligible for renewal (Active
|
||||
// status, future expiry).
|
||||
func addRenewableCert(repo *mockCertRepo, id string) {
|
||||
cert := &domain.ManagedCertificate{
|
||||
ID: id,
|
||||
CommonName: id + ".example.com",
|
||||
Status: domain.CertificateStatusActive,
|
||||
ExpiresAt: time.Now().AddDate(0, 1, 0),
|
||||
IssuerID: "iss-test",
|
||||
}
|
||||
repo.AddCert(cert)
|
||||
}
|
||||
|
||||
// TestBulkRenew_ByExplicitIDs — happy path. N IDs in, N jobs enqueued,
|
||||
// EnqueuedJobs slice carries the {certificate_id, job_id} pairs.
|
||||
func TestBulkRenew_ByExplicitIDs(t *testing.T) {
|
||||
svc, certRepo, jobRepo, _ := newBulkRenewalTestService()
|
||||
addRenewableCert(certRepo, "mc-1")
|
||||
addRenewableCert(certRepo, "mc-2")
|
||||
addRenewableCert(certRepo, "mc-3")
|
||||
|
||||
res, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{CertificateIDs: []string{"mc-1", "mc-2", "mc-3"}},
|
||||
"alice")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkRenew failed: %v", err)
|
||||
}
|
||||
if res.TotalMatched != 3 || res.TotalEnqueued != 3 || res.TotalSkipped != 0 || res.TotalFailed != 0 {
|
||||
t.Errorf("counts = matched:%d enqueued:%d skipped:%d failed:%d, want 3/3/0/0",
|
||||
res.TotalMatched, res.TotalEnqueued, res.TotalSkipped, res.TotalFailed)
|
||||
}
|
||||
if len(res.EnqueuedJobs) != 3 {
|
||||
t.Fatalf("EnqueuedJobs len = %d, want 3", len(res.EnqueuedJobs))
|
||||
}
|
||||
if len(jobRepo.Jobs) != 3 {
|
||||
t.Errorf("jobRepo got %d jobs, want 3 (one per renewable cert)", len(jobRepo.Jobs))
|
||||
}
|
||||
for _, j := range res.EnqueuedJobs {
|
||||
if j.JobID == "" {
|
||||
t.Errorf("EnqueuedJob missing job_id for cert %s", j.CertificateID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenew_ByOwnerCriteria — criteria-mode resolution. The
|
||||
// criteria-routing path must call resolveCertificates with the filter
|
||||
// branch (not the explicit-IDs branch). Mocking convention in this
|
||||
// package: mockCertRepo.List ignores the filter and returns all certs,
|
||||
// so the test seeds only certs that should match (mirrors
|
||||
// TestBulkRevoke_ByOwner shape in bulk_revocation_test.go).
|
||||
func TestBulkRenew_ByOwnerCriteria(t *testing.T) {
|
||||
svc, certRepo, _, _ := newBulkRenewalTestService()
|
||||
for _, id := range []string{"mc-a1", "mc-a2"} {
|
||||
cert := &domain.ManagedCertificate{
|
||||
ID: id, CommonName: id, Status: domain.CertificateStatusActive,
|
||||
OwnerID: "o-alice", ExpiresAt: time.Now().AddDate(0, 1, 0),
|
||||
}
|
||||
certRepo.AddCert(cert)
|
||||
}
|
||||
|
||||
res, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{OwnerID: "o-alice"}, "alice")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkRenew failed: %v", err)
|
||||
}
|
||||
if res.TotalEnqueued != 2 {
|
||||
t.Errorf("TotalEnqueued = %d, want 2 (alice's 2 certs)", res.TotalEnqueued)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenew_SkipsRenewalInProgress — a cert already in the renewal
|
||||
// flow must NOT get a second job. This is the no-double-enqueue
|
||||
// contract: dispatch the bulk-renew button twice in quick succession
|
||||
// and the second call cleanly skips.
|
||||
func TestBulkRenew_SkipsRenewalInProgress(t *testing.T) {
|
||||
svc, certRepo, jobRepo, _ := newBulkRenewalTestService()
|
||||
cert := &domain.ManagedCertificate{
|
||||
ID: "mc-rip", Status: domain.CertificateStatusRenewalInProgress,
|
||||
ExpiresAt: time.Now().AddDate(0, 1, 0),
|
||||
}
|
||||
certRepo.AddCert(cert)
|
||||
|
||||
res, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{CertificateIDs: []string{"mc-rip"}}, "alice")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkRenew failed: %v", err)
|
||||
}
|
||||
if res.TotalSkipped != 1 || res.TotalEnqueued != 0 {
|
||||
t.Errorf("counts wrong: skipped=%d enqueued=%d, want 1/0",
|
||||
res.TotalSkipped, res.TotalEnqueued)
|
||||
}
|
||||
if len(jobRepo.Jobs) != 0 {
|
||||
t.Errorf("no job should be created for already-in-progress cert; got %d jobs", len(jobRepo.Jobs))
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenew_SkipsRevokedAndArchived — terminal states are silent
|
||||
// no-ops, not errors. Operator selecting a mix of live and revoked certs
|
||||
// shouldn't see "ERROR: revoked cert can't be renewed" 50 times.
|
||||
func TestBulkRenew_SkipsRevokedAndArchived(t *testing.T) {
|
||||
svc, certRepo, _, _ := newBulkRenewalTestService()
|
||||
addRenewableCert(certRepo, "mc-live")
|
||||
for _, st := range []domain.CertificateStatus{
|
||||
domain.CertificateStatusRevoked,
|
||||
domain.CertificateStatusArchived,
|
||||
domain.CertificateStatusExpired,
|
||||
} {
|
||||
cert := &domain.ManagedCertificate{
|
||||
ID: "mc-" + string(st), Status: st, ExpiresAt: time.Now().AddDate(0, 1, 0),
|
||||
}
|
||||
certRepo.AddCert(cert)
|
||||
}
|
||||
|
||||
res, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{CertificateIDs: []string{
|
||||
"mc-live", "mc-Revoked", "mc-Archived", "mc-Expired",
|
||||
}}, "alice")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkRenew failed: %v", err)
|
||||
}
|
||||
if res.TotalEnqueued != 1 || res.TotalSkipped != 3 {
|
||||
t.Errorf("counts = enqueued:%d skipped:%d, want 1/3 (only mc-live qualifies)",
|
||||
res.TotalEnqueued, res.TotalSkipped)
|
||||
}
|
||||
if len(res.Errors) != 0 {
|
||||
t.Errorf("status-skip should NOT populate Errors; got %v", res.Errors)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenew_EmptyCriteria_Error — defensive contract. Mirrors
|
||||
// BulkRevocationCriteria.IsEmpty rejection so a stray empty POST
|
||||
// doesn't try to renew the entire fleet.
|
||||
func TestBulkRenew_EmptyCriteria_Error(t *testing.T) {
|
||||
svc, _, _, _ := newBulkRenewalTestService()
|
||||
_, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{}, "alice")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for empty criteria, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenew_PartialFailure — N=3, jobRepo.Create injected to fail
|
||||
// on one of them. Response carries 2 enqueued + 1 error; no panic, no
|
||||
// abort.
|
||||
func TestBulkRenew_PartialFailure(t *testing.T) {
|
||||
svc, certRepo, jobRepo, _ := newBulkRenewalTestService()
|
||||
addRenewableCert(certRepo, "mc-1")
|
||||
addRenewableCert(certRepo, "mc-2")
|
||||
addRenewableCert(certRepo, "mc-3")
|
||||
|
||||
// Make Create fail uniformly. Two of the three certs will still
|
||||
// have their status flipped (because Update happened first), so
|
||||
// the failure manifests as "I tried to enqueue, the job-create
|
||||
// failed". Per-cert error string surfaced.
|
||||
jobRepo.CreateErr = errors.New("simulated DB outage")
|
||||
|
||||
res, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{CertificateIDs: []string{"mc-1", "mc-2", "mc-3"}},
|
||||
"alice")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkRenew should not propagate per-cert errors as a top-level error; got: %v", err)
|
||||
}
|
||||
if res.TotalFailed != 3 || res.TotalEnqueued != 0 {
|
||||
t.Errorf("counts = failed:%d enqueued:%d, want 3/0", res.TotalFailed, res.TotalEnqueued)
|
||||
}
|
||||
if len(res.Errors) != 3 {
|
||||
t.Errorf("Errors len = %d, want 3", len(res.Errors))
|
||||
}
|
||||
}
|
||||
|
||||
// TestBulkRenew_AuditEventEmitted — exactly ONE bulk audit event for
|
||||
// the operation, NOT N. This is the audit-volume contract that makes
|
||||
// bulk endpoints scalable.
|
||||
func TestBulkRenew_AuditEventEmitted(t *testing.T) {
|
||||
svc, certRepo, _, auditRepo := newBulkRenewalTestService()
|
||||
addRenewableCert(certRepo, "mc-1")
|
||||
addRenewableCert(certRepo, "mc-2")
|
||||
addRenewableCert(certRepo, "mc-3")
|
||||
|
||||
_, err := svc.BulkRenew(context.Background(),
|
||||
domain.BulkRenewalCriteria{CertificateIDs: []string{"mc-1", "mc-2", "mc-3"}},
|
||||
"alice")
|
||||
if err != nil {
|
||||
t.Fatalf("BulkRenew failed: %v", err)
|
||||
}
|
||||
|
||||
// audit_events count must be exactly 1 — the bulk-renewal envelope.
|
||||
// Per-cert renewal events come from CertificateService.TriggerRenewal,
|
||||
// which the bulk path bypasses for exactly this reason.
|
||||
if len(auditRepo.Events) != 1 {
|
||||
t.Errorf("audit events count = %d, want exactly 1 (one bulk event, NOT N per-cert events)", len(auditRepo.Events))
|
||||
}
|
||||
if len(auditRepo.Events) > 0 && auditRepo.Events[0].Action != "bulk_renewal_initiated" {
|
||||
t.Errorf("audit action = %q, want 'bulk_renewal_initiated'", auditRepo.Events[0].Action)
|
||||
}
|
||||
}
|
||||
@@ -129,6 +129,64 @@ export const bulkRevokeCertificates = (criteria: BulkRevokeCriteria) =>
|
||||
body: JSON.stringify(criteria),
|
||||
});
|
||||
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5): bulk renew. Mirrors
|
||||
// BulkRevokeCriteria field-for-field so operators who already know the
|
||||
// bulk-revoke contract have zero new surface to learn. Pre-L-1 the GUI
|
||||
// looped `await triggerRenewal(id)` over the selection; 100 certs = 100
|
||||
// HTTP round-trips. Post-L-1 it's a single POST returning per-cert
|
||||
// {certificate_id, job_id} pairs in enqueued_jobs and per-cert errors
|
||||
// in errors. The "renew all certs of profile X" use case is the
|
||||
// canonical reason to support criteria-mode in addition to explicit IDs.
|
||||
export interface BulkRenewalCriteria {
|
||||
profile_id?: string;
|
||||
owner_id?: string;
|
||||
agent_id?: string;
|
||||
issuer_id?: string;
|
||||
team_id?: string;
|
||||
certificate_ids?: string[];
|
||||
}
|
||||
|
||||
export interface BulkRenewalResult {
|
||||
total_matched: number;
|
||||
total_enqueued: number;
|
||||
total_skipped: number;
|
||||
total_failed: number;
|
||||
enqueued_jobs?: { certificate_id: string; job_id: string }[];
|
||||
errors?: { certificate_id: string; error: string }[];
|
||||
}
|
||||
|
||||
export const bulkRenewCertificates = (criteria: BulkRenewalCriteria) =>
|
||||
fetchJSON<BulkRenewalResult>(`${BASE}/certificates/bulk-renew`, {
|
||||
method: 'POST',
|
||||
body: JSON.stringify(criteria),
|
||||
});
|
||||
|
||||
// L-2 closure (cat-l-8a1fb258a38a): bulk reassign owner (and optionally
|
||||
// team) for a set of certificates. Narrower than bulk-renew — explicit
|
||||
// IDs only, no criteria-mode (operators query first, then reassign by
|
||||
// ID). Pre-L-2 the GUI looped `await updateCertificate(id, { owner_id })`.
|
||||
// owner_id is required; team_id is optional and updates only when
|
||||
// non-empty (matches the existing per-cert PUT contract).
|
||||
export interface BulkReassignmentRequest {
|
||||
certificate_ids: string[];
|
||||
owner_id: string;
|
||||
team_id?: string;
|
||||
}
|
||||
|
||||
export interface BulkReassignmentResult {
|
||||
total_matched: number;
|
||||
total_reassigned: number;
|
||||
total_skipped: number;
|
||||
total_failed: number;
|
||||
errors?: { certificate_id: string; error: string }[];
|
||||
}
|
||||
|
||||
export const bulkReassignCertificates = (request: BulkReassignmentRequest) =>
|
||||
fetchJSON<BulkReassignmentResult>(`${BASE}/certificates/bulk-reassign`, {
|
||||
method: 'POST',
|
||||
body: JSON.stringify(request),
|
||||
});
|
||||
|
||||
// Certificate Export
|
||||
export const exportCertificatePEM = (id: string) =>
|
||||
fetchJSON<{ cert_pem: string; chain_pem: string; full_pem: string }>(`${BASE}/certificates/${id}/export/pem`);
|
||||
|
||||
@@ -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, getRenewalPolicies, getProfiles, getIssuers, bulkRevokeCertificates } from '../api/client';
|
||||
import { getCertificates, createCertificate, revokeCertificate, getOwners, getTeams, getRenewalPolicies, getProfiles, getIssuers, bulkRevokeCertificates, bulkRenewCertificates, bulkReassignCertificates } from '../api/client';
|
||||
import { useAuth } from '../components/AuthProvider';
|
||||
import { REVOCATION_REASONS } from '../api/types';
|
||||
import PageHeader from '../components/PageHeader';
|
||||
@@ -311,22 +311,38 @@ function BulkReassignModal({ ids, onClose, onSuccess }: { ids: string[]; onClose
|
||||
queryFn: () => getOwners(),
|
||||
});
|
||||
|
||||
// L-2 closure (cat-l-8a1fb258a38a): pre-L-2 this looped
|
||||
// `await updateCertificate(id, { owner_id })` over the selection
|
||||
// (N HTTP round-trips). Post-L-2 it's a single POST to
|
||||
// /api/v1/certificates/bulk-reassign. The CI guardrail in
|
||||
// .github/workflows/ci.yml (`Forbidden client-side bulk-action loop
|
||||
// regression guard (L-1)`) catches reintroduction of the loop shape.
|
||||
const handleReassign = async () => {
|
||||
if (!ownerId) return;
|
||||
setRunning(true);
|
||||
setError('');
|
||||
let succeeded = 0;
|
||||
for (const id of ids) {
|
||||
try {
|
||||
await updateCertificate(id, { owner_id: ownerId } as Partial<Certificate>);
|
||||
succeeded++;
|
||||
setProgress(succeeded);
|
||||
} catch (err) {
|
||||
setError(`Failed on ${id}: ${err instanceof Error ? err.message : 'Unknown error'}`);
|
||||
break;
|
||||
setProgress(0);
|
||||
try {
|
||||
const result = await bulkReassignCertificates({
|
||||
certificate_ids: ids,
|
||||
owner_id: ownerId,
|
||||
});
|
||||
setProgress(result.total_reassigned);
|
||||
if (result.total_failed > 0) {
|
||||
const first = result.errors?.[0];
|
||||
setError(
|
||||
`${result.total_failed} of ${result.total_matched} failed${
|
||||
first ? `: ${first.certificate_id} — ${first.error}` : ''
|
||||
}`
|
||||
);
|
||||
} else {
|
||||
onSuccess();
|
||||
}
|
||||
} catch (err) {
|
||||
setError(`Bulk reassignment failed: ${err instanceof Error ? err.message : 'Unknown error'}`);
|
||||
} finally {
|
||||
setRunning(false);
|
||||
}
|
||||
if (!error) onSuccess();
|
||||
};
|
||||
|
||||
return (
|
||||
@@ -405,20 +421,32 @@ export default function CertificatesPage() {
|
||||
refetchInterval: 30000,
|
||||
});
|
||||
|
||||
// L-1 closure (cat-l-fa0c1ac07ab5): pre-L-1 this looped
|
||||
// `await triggerRenewal(ids[i])` over the selection (N HTTP round-
|
||||
// trips × ~50–200ms each = 5–20s wedge for 100 selected certs).
|
||||
// Post-L-1 it's a single POST to /api/v1/certificates/bulk-renew;
|
||||
// the server resolves the criteria, applies status filters
|
||||
// (RenewalInProgress/Revoked/Archived/Expired all silent-skip), and
|
||||
// enqueues N renewal jobs server-side, returning a per-cert
|
||||
// {certificate_id, job_id} envelope. CI guardrail at
|
||||
// .github/workflows/ci.yml catches loop-shape regression.
|
||||
const handleBulkRenewal = async () => {
|
||||
const ids = Array.from(selectedIds);
|
||||
setBulkRenewProgress({ done: 0, total: ids.length, running: true });
|
||||
for (let i = 0; i < ids.length; i++) {
|
||||
try {
|
||||
await triggerRenewal(ids[i]);
|
||||
} catch {
|
||||
// continue on individual failures
|
||||
}
|
||||
setBulkRenewProgress({ done: i + 1, total: ids.length, running: i + 1 < ids.length });
|
||||
try {
|
||||
const result = await bulkRenewCertificates({ certificate_ids: ids });
|
||||
setBulkRenewProgress({
|
||||
done: result.total_enqueued,
|
||||
total: result.total_matched,
|
||||
running: false,
|
||||
});
|
||||
} catch {
|
||||
// surface as a "0 of N" terminal state — no retries.
|
||||
setBulkRenewProgress({ done: 0, total: ids.length, running: false });
|
||||
}
|
||||
queryClient.invalidateQueries({ queryKey: ['certificates'] });
|
||||
setSelectedIds(new Set());
|
||||
setTimeout(() => setBulkRenewProgress(null), 3000);
|
||||
setTimeout(() => setBulkRenewProgress(null), 5000);
|
||||
};
|
||||
|
||||
const columns: Column<Certificate>[] = [
|
||||
|
||||
Reference in New Issue
Block a user