gsadmin/certctl
1
0
Fork 0
mirror of https://github.com/shankar0123/certctl.git synced 2026-06-07 17:41:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: resolve NULL csr_pem scan errors and QA smoke test failures

Browse Source 
Root cause: certificate_versions.csr_pem is nullable in the schema but
Go code scanned it into a plain string. Used sql.NullString in
ListVersions and GetLatestVersion to handle NULL values correctly.

Also includes: partial update fetch-merge-update pattern to prevent FK
violations, nil directory guard in discovery service, diagnostic slog
logging in handlers, export handler 422 for unparseable PEM, OpenAPI
spec corrections, MCP tool description improvements, and test fixes.

Rewrites the Release Sign-Off section in testing-guide.md to individual
test-level granularity (320 rows) with smoke test results audited and
checked off (121 pass, 5 skip, 194 manual remaining).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
shankar0123
2026-03-30 00:51:18 -04:00
parent 20378ea7bb
commit a8fc177118
12 changed files with 683 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/handler/agents.go
+11
View File
@@ -3,6 +3,7 @@ package handler
import (
"context"
"encoding/json"
"log/slog"
"net/http"
"strconv"
"strings"
@@ -134,6 +135,11 @@ func (h AgentHandler) RegisterAgent(w http.ResponseWriter, r *http.Request) {
created, err := h.svc.RegisterAgent(r.Context(), agent)
if err != nil {
errMsg := err.Error()
if strings.Contains(errMsg, "unique") || strings.Contains(errMsg, "duplicate") || strings.Contains(errMsg, "already exists") {
ErrorWithRequestID(w, http.StatusConflict, "Agent with this name already exists", requestID)
return
}
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to register agent", requestID)
return
}
@@ -184,6 +190,11 @@ func (h AgentHandler) Heartbeat(w http.ResponseWriter, r *http.Request) {
}
if err := h.svc.Heartbeat(r.Context(), agentID, metadata); err != nil {
if strings.Contains(err.Error(), "not found") {
ErrorWithRequestID(w, http.StatusNotFound, "Agent not found", requestID)
return
}
slog.Error("Heartbeat failed", "agent_id", agentID, "error", err.Error())
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to record heartbeat", requestID)
return
}
internal/api/handler/certificate_handler_test.go
+14 -12
View File
@@ -353,11 +353,12 @@ func TestCreateCertificate_Success(t *testing.T) {
handler := NewCertificateHandler(mock)
certBody := domain.ManagedCertificate{
Name: "Production Cert",
CommonName: "example.com",
OwnerID: "o-alice",
TeamID: "t-platform",
IssuerID: "iss-local",
Name: "Production Cert",
CommonName: "example.com",
OwnerID: "o-alice",
TeamID: "t-platform",
IssuerID: "iss-local",
RenewalPolicyID: "rp-standard",
}
body, _ := json.Marshal(certBody)
@@ -410,11 +411,12 @@ func TestCreateCertificate_ServiceError(t *testing.T) {
handler := NewCertificateHandler(mock)
certBody := domain.ManagedCertificate{
Name: "Production Cert",
CommonName: "example.com",
OwnerID: "o-alice",
TeamID: "t-platform",
IssuerID: "iss-local",
Name: "Production Cert",
CommonName: "example.com",
OwnerID: "o-alice",
TeamID: "t-platform",
IssuerID: "iss-local",
RenewalPolicyID: "rp-standard",
}
body, _ := json.Marshal(certBody)
@@ -534,8 +536,8 @@ func TestArchiveCertificate_NotFound(t *testing.T) {
handler.ArchiveCertificate(w, req)
if w.Code != http.StatusInternalServerError {
t.Errorf("expected status %d, got %d", http.StatusInternalServerError, w.Code)
if w.Code != http.StatusNotFound {
t.Errorf("expected status %d, got %d", http.StatusNotFound, w.Code)
}
}
internal/api/handler/certificates.go
+38 -2
View File
@@ -2,6 +2,7 @@ package handler
import (
"encoding/json"
"log/slog"
"net/http"
"strconv"
"strings"
@@ -231,6 +232,14 @@ func (h CertificateHandler) CreateCertificate(w http.ResponseWriter, r *http.Req
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
return
}
if err := ValidateRequired("name", cert.Name); err != nil {
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
return
}
if err := ValidateRequired("renewal_policy_id", cert.RenewalPolicyID); err != nil {
ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
return
}
created, err := h.svc.CreateCertificate(cert)
if err != nil {
@@ -287,6 +296,11 @@ func (h CertificateHandler) UpdateCertificate(w http.ResponseWriter, r *http.Req
updated, err := h.svc.UpdateCertificate(id, cert)
if err != nil {
if strings.Contains(err.Error(), "not found") {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
slog.Error("UpdateCertificate failed", "cert_id", id, "error", err.Error())
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to update certificate", requestID)
return
}
@@ -311,6 +325,10 @@ func (h CertificateHandler) ArchiveCertificate(w http.ResponseWriter, r *http.Re
}
if err := h.svc.ArchiveCertificate(id); err != nil {
if strings.Contains(err.Error(), "not found") {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to archive certificate", requestID)
return
}
@@ -353,7 +371,12 @@ func (h CertificateHandler) GetCertificateVersions(w http.ResponseWriter, r *htt
versions, total, err := h.svc.GetCertificateVersions(certID, page, perPage)
if err != nil {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
if strings.Contains(err.Error(), "not found") {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
slog.Error("GetCertificateVersions failed", "cert_id", certID, "error", err.Error())
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to get certificate versions", requestID)
return
}
@@ -387,6 +410,19 @@ func (h CertificateHandler) TriggerRenewal(w http.ResponseWriter, r *http.Reques
certID := parts[0]
if err := h.svc.TriggerRenewal(certID); err != nil {
errMsg := err.Error()
if strings.Contains(errMsg, "not found") {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
if strings.Contains(errMsg, "cannot renew") {
ErrorWithRequestID(w, http.StatusBadRequest, errMsg, requestID)
return
}
if strings.Contains(errMsg, "already in progress") {
ErrorWithRequestID(w, http.StatusConflict, errMsg, requestID)
return
}
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to trigger renewal", requestID)
return
}
@@ -480,7 +516,7 @@ func (h CertificateHandler) RevokeCertificate(w http.ResponseWriter, r *http.Req
ErrorWithRequestID(w, http.StatusBadRequest, errMsg, requestID)
return
}
if strings.Contains(errMsg, "not found") || strings.Contains(errMsg, "failed to fetch") {
if strings.Contains(errMsg, "not found") || strings.Contains(errMsg, "failed to fetch") || strings.Contains(errMsg, "failed to get") {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
internal/api/handler/export.go
+7
View File
@@ -3,6 +3,7 @@ package handler
import (
"context"
"encoding/json"
"log/slog"
"net/http"
"strings"
@@ -49,6 +50,7 @@ func (h ExportHandler) ExportPEM(w http.ResponseWriter, r *http.Request) {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
slog.Error("ExportPEM failed", "cert_id", id, "error", err.Error())
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to export certificate", requestID)
return
}
@@ -96,6 +98,11 @@ func (h ExportHandler) ExportPKCS12(w http.ResponseWriter, r *http.Request) {
ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID)
return
}
if strings.Contains(err.Error(), "cannot be parsed") || strings.Contains(err.Error(), "no certificates found") {
ErrorWithRequestID(w, http.StatusUnprocessableEntity, "Certificate data cannot be parsed as X.509", requestID)
return
}
slog.Error("ExportPKCS12 failed", "cert_id", id, "error", err.Error())
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to export PKCS#12", requestID)
return
}