mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 21:31:34 +00:00
5c01c7f21f
C-001 — CreateCertificate was server-accepted with null owner_id,
team_id, renewal_policy_id because the GUI neither collected the fields
nor enforced them, even though the backend's ManagedCertificate schema
and handler contract treat them as required. Fix the contract at all
four layers:
- web/src/pages/CertificatesPage.tsx: replace owner_id/team_id free-
text inputs with <select> elements fed by getOwners/getTeams/
getPolicies queries; mark all three required; gate the Create
button on owner_id + team_id + renewal_policy_id being set.
- internal/api/handler/certificates.go: ValidateRequired for
owner_id, team_id, renewal_policy_id on CreateCertificate so the
handler returns HTTP 400 with the offending field name before the
service layer is reached.
- internal/mcp/types.go: drop ',omitempty' from
CreateCertificateInput.RenewalPolicyID so the MCP schema reflects
the required contract; Update inputs keep partial-update semantics.
- api/openapi.yaml: 'required: [name, common_name, renewal_policy_id,
issuer_id, owner_id, team_id]' was already present on the Create
schema; clarified DeploymentTarget.agent_id description to note the
FK contract.
C-002 — CreateTargetWizard accepted an empty or bogus agent_id and the
service inserted directly, producing a Postgres 23503 FK-violation that
bubbled out as a generic HTTP 500. The FK itself (migration 000001 line
104: agent_id TEXT NOT NULL REFERENCES agents(id)) is correct; we keep
the schema strict and add validation at three layers:
- internal/service/target.go: introduce
ErrAgentNotFound sentinel and pre-validate agent_id in
TargetService.CreateTarget — empty string returns
'agent_id is required'; a nonexistent id returns the full
'referenced agent does not exist: <id>' error. Both wrap
ErrAgentNotFound via fmt.Errorf %w so callers can use errors.Is.
- internal/api/handler/targets.go: ValidateRequired on agent_id; map
errors.Is(err, service.ErrAgentNotFound) to HTTP 400 instead of
letting it fall through to the generic 500 branch.
- internal/mcp/types.go: drop ',omitempty' from
CreateTargetInput.AgentID to match the required contract.
- web/src/pages/TargetsPage.tsx: replace the free-text Agent ID input
with a <select> populated from getAgents(); include agent in the
canProceedToReview gate so Next is disabled until an agent is
chosen.
Regression coverage (21 new subtests total):
- TestCreateCertificate_MissingRequiredField_Returns400 — 6 subtests,
one per required field, each proves the handler guard fires before
the mock service is called.
- TestCreateTarget_MissingAgentID_Returns400 — handler guard.
- TestCreateTarget_NonexistentAgent_Returns400 — pins the
ErrAgentNotFound -> 400 translation.
- TestTargetService_CreateTarget_MissingAgentID — errors.Is sentinel.
- TestTargetService_CreateTarget_NonexistentAgentID — errors.Is.
- The existing TestTargetService_CreateTarget_Success, along with
TestCreateTarget_{MissingName,MissingType,NameTooLong}_* handler
tests, were updated to seed a real agent or include agent_id in
the request body so the happy paths still run cleanly.
Gates (Phase 4):
- go build/vet/test/race: green
- go test -cover: internal/service 68.7% (gate 55%),
internal/api/handler 78.9% (gate 60%)
- golangci-lint on service+handler+mcp: 0 issues
- govulncheck: no reachable vulns
- tsc --noEmit: clean
- vitest: 223/223 passing
See cowork/certctl-coverage-gap-audit.md entries C-001 and C-002.
243 lines
7.3 KiB
Go
243 lines
7.3 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"net/http"
|
|
"strconv"
|
|
"strings"
|
|
|
|
"github.com/shankar0123/certctl/internal/api/middleware"
|
|
"github.com/shankar0123/certctl/internal/domain"
|
|
"github.com/shankar0123/certctl/internal/service"
|
|
)
|
|
|
|
// TargetService defines the service interface for deployment target operations.
|
|
type TargetService interface {
|
|
ListTargets(ctx context.Context, page, perPage int) ([]domain.DeploymentTarget, int64, error)
|
|
GetTarget(ctx context.Context, id string) (*domain.DeploymentTarget, error)
|
|
CreateTarget(ctx context.Context, target domain.DeploymentTarget) (*domain.DeploymentTarget, error)
|
|
UpdateTarget(ctx context.Context, id string, target domain.DeploymentTarget) (*domain.DeploymentTarget, error)
|
|
DeleteTarget(ctx context.Context, id string) error
|
|
TestConnection(ctx context.Context, id string) error
|
|
}
|
|
|
|
// TargetHandler handles HTTP requests for deployment target operations.
|
|
type TargetHandler struct {
|
|
svc TargetService
|
|
}
|
|
|
|
// NewTargetHandler creates a new TargetHandler with a service dependency.
|
|
func NewTargetHandler(svc TargetService) TargetHandler {
|
|
return TargetHandler{svc: svc}
|
|
}
|
|
|
|
// ListTargets lists all deployment targets.
|
|
// GET /api/v1/targets?page=1&per_page=50
|
|
func (h TargetHandler) ListTargets(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodGet {
|
|
Error(w, http.StatusMethodNotAllowed, "Method not allowed")
|
|
return
|
|
}
|
|
|
|
requestID := middleware.GetRequestID(r.Context())
|
|
|
|
page := 1
|
|
perPage := 50
|
|
query := r.URL.Query()
|
|
if p := query.Get("page"); p != "" {
|
|
if parsed, err := strconv.Atoi(p); err == nil && parsed > 0 {
|
|
page = parsed
|
|
}
|
|
}
|
|
if pp := query.Get("per_page"); pp != "" {
|
|
if parsed, err := strconv.Atoi(pp); err == nil && parsed > 0 && parsed <= 500 {
|
|
perPage = parsed
|
|
}
|
|
}
|
|
|
|
targets, total, err := h.svc.ListTargets(r.Context(), page, perPage)
|
|
if err != nil {
|
|
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to list targets", requestID)
|
|
return
|
|
}
|
|
|
|
response := PagedResponse{
|
|
Data: targets,
|
|
Total: total,
|
|
Page: page,
|
|
PerPage: perPage,
|
|
}
|
|
|
|
JSON(w, http.StatusOK, response)
|
|
}
|
|
|
|
// GetTarget retrieves a single deployment target by ID.
|
|
// GET /api/v1/targets/{id}
|
|
func (h TargetHandler) GetTarget(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodGet {
|
|
Error(w, http.StatusMethodNotAllowed, "Method not allowed")
|
|
return
|
|
}
|
|
|
|
requestID := middleware.GetRequestID(r.Context())
|
|
|
|
id := strings.TrimPrefix(r.URL.Path, "/api/v1/targets/")
|
|
if id == "" || strings.Contains(id, "/") {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "Target ID is required", requestID)
|
|
return
|
|
}
|
|
|
|
target, err := h.svc.GetTarget(r.Context(), id)
|
|
if err != nil {
|
|
ErrorWithRequestID(w, http.StatusNotFound, "Target not found", requestID)
|
|
return
|
|
}
|
|
|
|
JSON(w, http.StatusOK, target)
|
|
}
|
|
|
|
// CreateTarget creates a new deployment target.
|
|
// POST /api/v1/targets
|
|
func (h TargetHandler) CreateTarget(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 target domain.DeploymentTarget
|
|
if err := json.NewDecoder(r.Body).Decode(&target); err != nil {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "Invalid request body", requestID)
|
|
return
|
|
}
|
|
|
|
// Validate required fields
|
|
if err := ValidateRequired("name", target.Name); err != nil {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
|
|
return
|
|
}
|
|
if err := ValidateStringLength("name", target.Name, 255); err != nil {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
|
|
return
|
|
}
|
|
if target.Type == "" {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "type is required", requestID)
|
|
return
|
|
}
|
|
// C-002: agent_id is a NOT NULL FK in deployment_targets (migration 000001
|
|
// line 104). Reject empty values at the boundary so callers get a clean 400
|
|
// with the field name rather than a generic "Failed to create target" 500.
|
|
if err := ValidateRequired("agent_id", target.AgentID); err != nil {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
|
|
return
|
|
}
|
|
|
|
created, err := h.svc.CreateTarget(r.Context(), target)
|
|
if err != nil {
|
|
// C-002: a nonexistent agent_id is a client error, not a server error.
|
|
// The service returns ErrAgentNotFound (wrapped via fmt.Errorf %w) when
|
|
// agentRepo.Get fails; we translate that to 400 via errors.Is.
|
|
if errors.Is(err, service.ErrAgentNotFound) {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
|
|
return
|
|
}
|
|
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to create target", requestID)
|
|
return
|
|
}
|
|
|
|
JSON(w, http.StatusCreated, created)
|
|
}
|
|
|
|
// UpdateTarget updates an existing deployment target.
|
|
// PUT /api/v1/targets/{id}
|
|
func (h TargetHandler) UpdateTarget(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodPut {
|
|
Error(w, http.StatusMethodNotAllowed, "Method not allowed")
|
|
return
|
|
}
|
|
|
|
requestID := middleware.GetRequestID(r.Context())
|
|
|
|
id := strings.TrimPrefix(r.URL.Path, "/api/v1/targets/")
|
|
parts := strings.Split(id, "/")
|
|
if len(parts) == 0 || parts[0] == "" {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "Target ID is required", requestID)
|
|
return
|
|
}
|
|
id = parts[0]
|
|
|
|
var target domain.DeploymentTarget
|
|
if err := json.NewDecoder(r.Body).Decode(&target); err != nil {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "Invalid request body", requestID)
|
|
return
|
|
}
|
|
|
|
updated, err := h.svc.UpdateTarget(r.Context(), id, target)
|
|
if err != nil {
|
|
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to update target", requestID)
|
|
return
|
|
}
|
|
|
|
JSON(w, http.StatusOK, updated)
|
|
}
|
|
|
|
// DeleteTarget deletes a deployment target.
|
|
// DELETE /api/v1/targets/{id}
|
|
func (h TargetHandler) DeleteTarget(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodDelete {
|
|
Error(w, http.StatusMethodNotAllowed, "Method not allowed")
|
|
return
|
|
}
|
|
|
|
requestID := middleware.GetRequestID(r.Context())
|
|
|
|
id := strings.TrimPrefix(r.URL.Path, "/api/v1/targets/")
|
|
if id == "" || strings.Contains(id, "/") {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "Target ID is required", requestID)
|
|
return
|
|
}
|
|
|
|
if err := h.svc.DeleteTarget(r.Context(), id); err != nil {
|
|
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to delete target", requestID)
|
|
return
|
|
}
|
|
|
|
w.WriteHeader(http.StatusNoContent)
|
|
}
|
|
|
|
// TestTargetConnection tests target connectivity by checking the assigned agent's heartbeat.
|
|
// POST /api/v1/targets/{id}/test
|
|
func (h TargetHandler) TestTargetConnection(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodPost {
|
|
Error(w, http.StatusMethodNotAllowed, "Method not allowed")
|
|
return
|
|
}
|
|
|
|
requestID := middleware.GetRequestID(r.Context())
|
|
|
|
// Extract target ID from path: /api/v1/targets/{id}/test
|
|
path := strings.TrimPrefix(r.URL.Path, "/api/v1/targets/")
|
|
parts := strings.Split(path, "/")
|
|
if len(parts) < 2 || parts[0] == "" {
|
|
ErrorWithRequestID(w, http.StatusBadRequest, "Target ID is required", requestID)
|
|
return
|
|
}
|
|
id := parts[0]
|
|
|
|
if err := h.svc.TestConnection(r.Context(), id); err != nil {
|
|
JSON(w, http.StatusOK, map[string]interface{}{
|
|
"status": "failed",
|
|
"message": err.Error(),
|
|
})
|
|
return
|
|
}
|
|
|
|
JSON(w, http.StatusOK, map[string]interface{}{
|
|
"status": "success",
|
|
"message": "Agent is online and reachable",
|
|
})
|
|
}
|