From 0e29c416b19f2e2635bcbefce6889d87ef2d7a4c Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sat, 25 Apr 2026 17:54:14 +0000 Subject: [PATCH] refactor(handler,repo): replace strings.Contains error dispatch with typed sentinels (S-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes one 2026-04-24 audit finding (P2): - cat-s6-efc7f6f6bd50: 30 strings.Contains(err.Error(), ...) sites in internal/api/handler/ — brittle to repository-layer message changes, untyped against the actual failure mode. Approach (Option B from prompt design notes): - New typed sentinels in internal/repository/errors.go: ErrNotFound, ErrForeignKeyConstraint IsForeignKeyError(err) helper (the only place substring matching at the lib/pq boundary is allowed; isolates the DB-driver string knowledge to one function). - New typed sentinel in internal/domain/errors.go: ErrValidation (reserved for future per-entity validation wrappers; not yet used by all handlers). - 49 sites in internal/repository/postgres/*.go updated to wrap sql.ErrNoRows-derived errors via fmt.Errorf("...: %w", repository.ErrNotFound). - 18 not-found handler sites + 2 FK-constraint handler sites refactored to errors.Is(err, repository.ErrNotFound) / repository.IsForeignKeyError(err). - 23 inline `fmt.Errorf("X not found")` test fixtures across handler tests rewrapped to wrap repository.ErrNotFound. - test_utils.go::ErrMockNotFound rewrapped to wrap repository.ErrNotFound; renewal_policy.go closure docblock updated to reflect the new convention. - integration test mockJobRepository.Get wraps repository.ErrNotFound. CI regression guardrail: - .github/workflows/ci.yml::"Forbidden strings.Contains(err.Error()) regression guard (S-2)" greps for the three patterns ("not found", "violates foreign key", "RESTRICT") under internal/api/handler/ and fails the build on regression. Verification: - go build ./... — clean - go vet ./... — clean - go test ./... -short -count=1 — all packages pass (handler + repository + service + integration) - golangci-lint v2.11.4 run ./... — 0 issues - S-2 guardrail dry-run on post-fix tree → empty (good) - All sibling guardrails (S-1, G-3, D-1+D-2, B-1, L-1, H-1, C-1, F-1, P-1) pass Audit findings closed: - cat-s6-efc7f6f6bd50 (P2) Deferred follow-ups: - 6 domain-specific substring patterns still inline in handlers ("cannot approve", "cannot reject", "cannot be parsed", "no certificates found", "challenge password", "invalid"/ "required" validation chains in profiles + agent_groups). Each needs its own typed sentinel, scoped per service. Documented by the S-2 CI guardrail's allowlist for closure-comments only. - Per-entity not-found sentinels (Option A — ErrCertificateNotFound, ErrAgentNotFound, etc.) deferred. Generic ErrNotFound covers the current dispatch needs; per-entity precision would let handlers return entity-aware error bodies without a domain.Type field, but not blocking. --- .github/workflows/ci.yml | 36 +++++++++++ .../api/handler/adversarial_query_test.go | 2 +- .../api/handler/agent_group_handler_test.go | 2 +- internal/api/handler/agent_groups.go | 6 +- .../api/handler/agent_retire_handler_test.go | 5 +- internal/api/handler/agents.go | 5 +- .../api/handler/certificate_handler_test.go | 14 ++--- internal/api/handler/certificates.go | 7 ++- .../api/handler/discovery_handler_test.go | 8 +-- internal/api/handler/export.go | 6 +- internal/api/handler/export_handler_test.go | 4 +- internal/api/handler/issuers.go | 6 +- internal/api/handler/job_handler_test.go | 4 +- internal/api/handler/jobs.go | 5 +- .../api/handler/network_scan_handler_test.go | 8 +-- internal/api/handler/notifications.go | 4 +- internal/api/handler/owners.go | 6 +- internal/api/handler/profiles.go | 6 +- internal/api/handler/renewal_policy.go | 19 +++--- internal/api/handler/test_utils.go | 25 +++++--- internal/domain/errors.go | 19 ++++++ internal/integration/lifecycle_test.go | 5 +- internal/repository/errors.go | 62 +++++++++++++++++++ internal/repository/postgres/agent.go | 11 ++-- internal/repository/postgres/agent_group.go | 7 ++- internal/repository/postgres/certificate.go | 6 +- internal/repository/postgres/discovery.go | 6 +- internal/repository/postgres/health_check.go | 4 +- internal/repository/postgres/issuer.go | 7 ++- internal/repository/postgres/job.go | 9 +-- internal/repository/postgres/network_scan.go | 7 ++- internal/repository/postgres/notification.go | 8 +-- internal/repository/postgres/owner.go | 7 ++- internal/repository/postgres/policy.go | 6 +- internal/repository/postgres/profile.go | 7 ++- .../repository/postgres/renewal_policy.go | 6 +- internal/repository/postgres/revocation.go | 3 +- internal/repository/postgres/target.go | 7 ++- internal/repository/postgres/team.go | 7 ++- 39 files changed, 265 insertions(+), 107 deletions(-) create mode 100644 internal/domain/errors.go create mode 100644 internal/repository/errors.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a1ae18a..0c0c269 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -518,6 +518,42 @@ jobs: fi echo "B-1 orphan-CRUD client function guardrail: all 8 functions have page consumers." + - name: Forbidden strings.Contains(err.Error()) regression guard (S-2) + # S-2 closure (cat-s6-efc7f6f6bd50): replaced 30 brittle + # substring-match error-dispatch sites in internal/api/handler/ + # with errors.Is + typed sentinels (repository.ErrNotFound, + # repository.ErrForeignKeyConstraint via the + # repository.IsForeignKeyError helper). This step grep-fails + # the build if any new strings.Contains(err.Error(), "not found") + # or strings.Contains(err.Error(), "violates foreign key") + # site appears under internal/api/handler/. + # + # Allowed: closure-comments documenting the convention (e.g. + # bulk_reassignment.go's "post-M-1 errToStatus convention" + # docblock); domain-specific substring patterns that are + # legitimately one-off ("cannot approve", "cannot reject", + # "cannot be parsed", "challenge password") — flagged as + # deferred follow-ups in the S-2 commit message. + # + # See coverage-gap-audit-2026-04-24-v5/unified-audit.md + # cat-s6-efc7f6f6bd50 for closure rationale. + run: | + set -e + BAD=$(grep -rnE 'strings\.Contains\(err\.Error\(\),\s*"(not found|violates foreign key|RESTRICT)"' internal/api/handler/ 2>/dev/null \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ + || true) + if [ -n "$BAD" ]; then + echo "S-2 regression: brittle substring-match error-dispatch reappeared:" + echo "$BAD" + echo "" + echo "Use errors.Is(err, repository.ErrNotFound) for not-found dispatch," + echo "or repository.IsForeignKeyError(err) for FK violations." + echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-s6-efc7f6f6bd50 for closure rationale." + exit 1 + fi + echo "S-2 typed-sentinel error-dispatch guardrail: clean." + - 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 diff --git a/internal/api/handler/adversarial_query_test.go b/internal/api/handler/adversarial_query_test.go index 13c57ec..224121b 100644 --- a/internal/api/handler/adversarial_query_test.go +++ b/internal/api/handler/adversarial_query_test.go @@ -522,7 +522,7 @@ func TestRevokeCertificate_AlreadyRevoked(t *testing.T) { func TestRevokeCertificate_NotFound(t *testing.T) { handler, mock := newCertHandlerWithMock() mock.RevokeCertificateFn = func(_ context.Context, id string, reason string, _ string) error { - return fmt.Errorf("certificate not found") + return fmt.Errorf("certificate not found: %w", ErrMockNotFound) } req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/mc-missing/revoke", strings.NewReader(`{"reason":"keyCompromise"}`)) diff --git a/internal/api/handler/agent_group_handler_test.go b/internal/api/handler/agent_group_handler_test.go index b3dacf1..6a5a082 100644 --- a/internal/api/handler/agent_group_handler_test.go +++ b/internal/api/handler/agent_group_handler_test.go @@ -33,7 +33,7 @@ func (m *MockAgentGroupService) GetAgentGroup(_ context.Context, id string) (*do if m.GetAgentGroupFn != nil { return m.GetAgentGroupFn(id) } - return nil, fmt.Errorf("not found") + return nil, fmt.Errorf("not found: %w", ErrMockNotFound) } func (m *MockAgentGroupService) CreateAgentGroup(_ context.Context, group domain.AgentGroup) (*domain.AgentGroup, error) { diff --git a/internal/api/handler/agent_groups.go b/internal/api/handler/agent_groups.go index e97cda9..85f0cf5 100644 --- a/internal/api/handler/agent_groups.go +++ b/internal/api/handler/agent_groups.go @@ -1,6 +1,8 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" + "errors" "context" "encoding/json" "net/http" @@ -160,7 +162,7 @@ func (h AgentGroupHandler) UpdateAgentGroup(w http.ResponseWriter, r *http.Reque updated, err := h.svc.UpdateAgentGroup(r.Context(), id, group) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Agent group not found", requestID) return } @@ -188,7 +190,7 @@ func (h AgentGroupHandler) DeleteAgentGroup(w http.ResponseWriter, r *http.Reque } if err := h.svc.DeleteAgentGroup(r.Context(), id); err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Agent group not found", requestID) return } diff --git a/internal/api/handler/agent_retire_handler_test.go b/internal/api/handler/agent_retire_handler_test.go index e245b06..7634fde 100644 --- a/internal/api/handler/agent_retire_handler_test.go +++ b/internal/api/handler/agent_retire_handler_test.go @@ -3,7 +3,6 @@ package handler import ( "context" "encoding/json" - "errors" "net/http" "net/http/httptest" "testing" @@ -142,7 +141,9 @@ func TestRetireAgentHandler_Sentinel_403(t *testing.T) { func TestRetireAgentHandler_NotFound_404(t *testing.T) { mock, handler := agentRetireTestSetup() mock.RetireAgentFn = func(agentID, actor string, force bool, reason string) (*service.AgentRetirementResult, error) { - return nil, errors.New("agent not found") + // S-2 closure (cat-s6-efc7f6f6bd50): wrap repository.ErrNotFound + // so the handler's errors.Is dispatch resolves to 404. + return nil, ErrMockNotFound } req := httptest.NewRequest(http.MethodDelete, "/api/v1/agents/unknown-id", nil) diff --git a/internal/api/handler/agents.go b/internal/api/handler/agents.go index b015420..da3fd75 100644 --- a/internal/api/handler/agents.go +++ b/internal/api/handler/agents.go @@ -1,6 +1,7 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" "context" "encoding/json" "errors" @@ -211,7 +212,7 @@ func (h AgentHandler) Heartbeat(w http.ResponseWriter, r *http.Request) { ErrorWithRequestID(w, http.StatusGone, "Agent has been retired", requestID) return } - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Agent not found", requestID) return } @@ -491,7 +492,7 @@ func (h AgentHandler) RetireAgent(w http.ResponseWriter, r *http.Request) { JSON(w, http.StatusConflict, body) return } - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Agent not found", requestID) return } diff --git a/internal/api/handler/certificate_handler_test.go b/internal/api/handler/certificate_handler_test.go index dce5b99..000beab 100644 --- a/internal/api/handler/certificate_handler_test.go +++ b/internal/api/handler/certificate_handler_test.go @@ -900,7 +900,7 @@ func TestRevokeCertificate_Handler_AlreadyRevoked(t *testing.T) { func TestRevokeCertificate_Handler_NotFound(t *testing.T) { mock := &MockCertificateService{ RevokeCertificateFn: func(_ context.Context, certID string, reason string, _ string) error { - return fmt.Errorf("failed to fetch certificate: not found") + return fmt.Errorf("failed to fetch certificate: not found: %w", ErrMockNotFound) }, } @@ -1033,7 +1033,7 @@ func TestGetDERCRL_Success(t *testing.T) { if issuerID == "iss-local" { return derCRLData, nil } - return nil, fmt.Errorf("issuer not found") + return nil, fmt.Errorf("issuer not found: %w", ErrMockNotFound) }, } @@ -1061,7 +1061,7 @@ func TestGetDERCRL_Success(t *testing.T) { func TestGetDERCRL_IssuerNotFound(t *testing.T) { mock := &MockCertificateService{ GenerateDERCRLFn: func(_ context.Context, issuerID string) ([]byte, error) { - return nil, fmt.Errorf("issuer not found") + return nil, fmt.Errorf("issuer not found: %w", ErrMockNotFound) }, } @@ -1118,7 +1118,7 @@ func TestHandleOCSP_Success(t *testing.T) { if issuerID == "iss-local" && serialHex == "12345" { return ocspResponseBytes, nil } - return nil, fmt.Errorf("certificate not found") + return nil, fmt.Errorf("certificate not found: %w", ErrMockNotFound) }, } @@ -1159,7 +1159,7 @@ func TestHandleOCSP_MissingSerial(t *testing.T) { func TestHandleOCSP_IssuerNotFound(t *testing.T) { mock := &MockCertificateService{ GetOCSPResponseFn: func(_ context.Context, issuerID string, serialHex string) ([]byte, error) { - return nil, fmt.Errorf("issuer not found") + return nil, fmt.Errorf("issuer not found: %w", ErrMockNotFound) }, } @@ -1178,7 +1178,7 @@ func TestHandleOCSP_IssuerNotFound(t *testing.T) { func TestHandleOCSP_CertNotFound(t *testing.T) { mock := &MockCertificateService{ GetOCSPResponseFn: func(_ context.Context, issuerID string, serialHex string) ([]byte, error) { - return nil, fmt.Errorf("certificate not found") + return nil, fmt.Errorf("certificate not found: %w", ErrMockNotFound) }, } @@ -1529,7 +1529,7 @@ func TestGetCertificateDeployments_Success(t *testing.T) { func TestGetCertificateDeployments_NotFound(t *testing.T) { mock := &MockCertificateService{ GetCertificateDeploymentsFn: func(_ context.Context, certID string) ([]domain.DeploymentTarget, error) { - return nil, fmt.Errorf("certificate not found") + return nil, fmt.Errorf("certificate not found: %w", ErrMockNotFound) }, } diff --git a/internal/api/handler/certificates.go b/internal/api/handler/certificates.go index 36beefd..550d2a4 100644 --- a/internal/api/handler/certificates.go +++ b/internal/api/handler/certificates.go @@ -1,6 +1,7 @@ package handler import ( + "errors" "context" "encoding/json" "log/slog" @@ -298,7 +299,7 @@ func (h CertificateHandler) UpdateCertificate(w http.ResponseWriter, r *http.Req updated, err := h.svc.UpdateCertificate(r.Context(), id, cert) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID) return } @@ -327,7 +328,7 @@ func (h CertificateHandler) ArchiveCertificate(w http.ResponseWriter, r *http.Re } if err := h.svc.ArchiveCertificate(r.Context(), id); err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID) return } @@ -373,7 +374,7 @@ func (h CertificateHandler) GetCertificateVersions(w http.ResponseWriter, r *htt versions, total, err := h.svc.GetCertificateVersions(r.Context(), certID, page, perPage) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID) return } diff --git a/internal/api/handler/discovery_handler_test.go b/internal/api/handler/discovery_handler_test.go index bcb98d1..ae627d9 100644 --- a/internal/api/handler/discovery_handler_test.go +++ b/internal/api/handler/discovery_handler_test.go @@ -300,7 +300,7 @@ func TestGetDiscovered_Success(t *testing.T) { if id == "dcert-1" { return cert, nil } - return nil, fmt.Errorf("not found") + return nil, fmt.Errorf("not found: %w", ErrMockNotFound) }, } @@ -331,7 +331,7 @@ func TestGetDiscovered_Success(t *testing.T) { func TestGetDiscovered_NotFound(t *testing.T) { mock := &MockDiscoveryService{ GetDiscoveredFn: func(ctx context.Context, id string) (*domain.DiscoveredCertificate, error) { - return nil, fmt.Errorf("not found") + return nil, fmt.Errorf("not found: %w", ErrMockNotFound) }, } @@ -412,7 +412,7 @@ func TestClaimDiscovered_MissingManagedCertID(t *testing.T) { func TestClaimDiscovered_NotFound(t *testing.T) { mock := &MockDiscoveryService{ ClaimDiscoveredFn: func(ctx context.Context, id string, managedCertID string, actor string) error { - return fmt.Errorf("discovered certificate not found") + return fmt.Errorf("discovered certificate not found: %w", ErrMockNotFound) }, } @@ -442,7 +442,7 @@ func TestDismissDiscovered_Success(t *testing.T) { if id == "dcert-1" { return nil } - return fmt.Errorf("not found") + return fmt.Errorf("not found: %w", ErrMockNotFound) }, } diff --git a/internal/api/handler/export.go b/internal/api/handler/export.go index 0e7eb27..5e31f72 100644 --- a/internal/api/handler/export.go +++ b/internal/api/handler/export.go @@ -1,6 +1,8 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" + "errors" "context" "encoding/json" "log/slog" @@ -46,7 +48,7 @@ func (h ExportHandler) ExportPEM(w http.ResponseWriter, r *http.Request) { result, err := h.svc.ExportPEM(r.Context(), id) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID) return } @@ -94,7 +96,7 @@ func (h ExportHandler) ExportPKCS12(w http.ResponseWriter, r *http.Request) { pfxData, err := h.svc.ExportPKCS12(r.Context(), id, req.Password) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Certificate not found", requestID) return } diff --git a/internal/api/handler/export_handler_test.go b/internal/api/handler/export_handler_test.go index 3c568c0..66eac9a 100644 --- a/internal/api/handler/export_handler_test.go +++ b/internal/api/handler/export_handler_test.go @@ -110,7 +110,7 @@ func TestExportPEM_Download(t *testing.T) { func TestExportPEM_NotFound(t *testing.T) { mockSvc := &MockExportService{ ExportPEMFn: func(_ context.Context, _ string) (*service.ExportPEMResult, error) { - return nil, fmt.Errorf("certificate not found") + return nil, fmt.Errorf("certificate not found: %w", ErrMockNotFound) }, } h := NewExportHandler(mockSvc) @@ -216,7 +216,7 @@ func TestExportPKCS12_EmptyPassword(t *testing.T) { func TestExportPKCS12_NotFound(t *testing.T) { mockSvc := &MockExportService{ ExportPKCS12Fn: func(_ context.Context, _ string, _ string) ([]byte, error) { - return nil, fmt.Errorf("certificate not found") + return nil, fmt.Errorf("certificate not found: %w", ErrMockNotFound) }, } h := NewExportHandler(mockSvc) diff --git a/internal/api/handler/issuers.go b/internal/api/handler/issuers.go index 7ad9641..ab81204 100644 --- a/internal/api/handler/issuers.go +++ b/internal/api/handler/issuers.go @@ -1,6 +1,8 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" + "errors" "context" "encoding/json" "log/slog" @@ -210,9 +212,9 @@ func (h IssuerHandler) DeleteIssuer(w http.ResponseWriter, r *http.Request) { } if err := h.svc.DeleteIssuer(r.Context(), id); err != nil { - if strings.Contains(err.Error(), "violates foreign key") || strings.Contains(err.Error(), "RESTRICT") { + if repository.IsForeignKeyError(err) { ErrorWithRequestID(w, http.StatusConflict, "Cannot delete issuer: certificates are still using this issuer", requestID) - } else if strings.Contains(err.Error(), "not found") { + } else if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Issuer not found", requestID) } else { ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to delete issuer", requestID) diff --git a/internal/api/handler/job_handler_test.go b/internal/api/handler/job_handler_test.go index 3d7647b..efc867a 100644 --- a/internal/api/handler/job_handler_test.go +++ b/internal/api/handler/job_handler_test.go @@ -383,7 +383,7 @@ func TestApproveJob_Success(t *testing.T) { func TestApproveJob_NotFound(t *testing.T) { mock := &MockJobService{ ApproveJobFn: func(id, actor string) error { - return fmt.Errorf("job not found: no rows") + return fmt.Errorf("job not found: no rows: %w", ErrMockNotFound) }, } @@ -527,7 +527,7 @@ func TestRejectJob_NoReason(t *testing.T) { func TestRejectJob_NotFound(t *testing.T) { mock := &MockJobService{ RejectJobFn: func(id, reason, actor string) error { - return fmt.Errorf("job not found: no rows") + return fmt.Errorf("job not found: no rows: %w", ErrMockNotFound) }, } diff --git a/internal/api/handler/jobs.go b/internal/api/handler/jobs.go index 26d1514..235c1ac 100644 --- a/internal/api/handler/jobs.go +++ b/internal/api/handler/jobs.go @@ -1,6 +1,7 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" "context" "encoding/json" "errors" @@ -167,7 +168,7 @@ func (h JobHandler) ApproveJob(w http.ResponseWriter, r *http.Request) { requestID) return } - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Job not found", requestID) return } @@ -213,7 +214,7 @@ func (h JobHandler) RejectJob(w http.ResponseWriter, r *http.Request) { actor := resolveActor(r.Context()) if err := h.svc.RejectJob(r.Context(), jobID, body.Reason, actor); err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Job not found", requestID) return } diff --git a/internal/api/handler/network_scan_handler_test.go b/internal/api/handler/network_scan_handler_test.go index 4164a83..1abe882 100644 --- a/internal/api/handler/network_scan_handler_test.go +++ b/internal/api/handler/network_scan_handler_test.go @@ -27,7 +27,7 @@ func (m *mockNetworkScanService) GetTarget(ctx context.Context, id string) (*dom return t, nil } } - return nil, fmt.Errorf("not found: %s", id) + return nil, fmt.Errorf("not found: %w", ErrMockNotFound) } func (m *mockNetworkScanService) CreateTarget(ctx context.Context, target *domain.NetworkScanTarget) (*domain.NetworkScanTarget, error) { @@ -48,7 +48,7 @@ func (m *mockNetworkScanService) UpdateTarget(ctx context.Context, id string, ta return t, nil } } - return nil, fmt.Errorf("not found: %s", id) + return nil, fmt.Errorf("not found: %w", ErrMockNotFound) } func (m *mockNetworkScanService) DeleteTarget(ctx context.Context, id string) error { @@ -58,7 +58,7 @@ func (m *mockNetworkScanService) DeleteTarget(ctx context.Context, id string) er return nil } } - return fmt.Errorf("not found: %s", id) + return fmt.Errorf("not found: %w", ErrMockNotFound) } func (m *mockNetworkScanService) TriggerScan(ctx context.Context, targetID string) (*domain.DiscoveryScan, error) { @@ -71,7 +71,7 @@ func (m *mockNetworkScanService) TriggerScan(ctx context.Context, targetID strin }, nil } } - return nil, fmt.Errorf("not found: %s", targetID) + return nil, fmt.Errorf("not found: %w", ErrMockNotFound) } func TestListNetworkScanTargets(t *testing.T) { diff --git a/internal/api/handler/notifications.go b/internal/api/handler/notifications.go index 513bfb9..85c115b 100644 --- a/internal/api/handler/notifications.go +++ b/internal/api/handler/notifications.go @@ -1,6 +1,8 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" + "errors" "context" "net/http" "strconv" @@ -170,7 +172,7 @@ func (h NotificationHandler) RequeueNotification(w http.ResponseWriter, r *http. notificationID := parts[0] if err := h.svc.RequeueNotification(r.Context(), notificationID); err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Notification not found", requestID) return } diff --git a/internal/api/handler/owners.go b/internal/api/handler/owners.go index 050b4d4..7489112 100644 --- a/internal/api/handler/owners.go +++ b/internal/api/handler/owners.go @@ -1,6 +1,8 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" + "errors" "context" "encoding/json" "net/http" @@ -184,9 +186,9 @@ func (h OwnerHandler) DeleteOwner(w http.ResponseWriter, r *http.Request) { id = parts[0] if err := h.svc.DeleteOwner(r.Context(), id); err != nil { - if strings.Contains(err.Error(), "violates foreign key") || strings.Contains(err.Error(), "RESTRICT") { + if repository.IsForeignKeyError(err) { ErrorWithRequestID(w, http.StatusConflict, "Cannot delete owner: certificates are still assigned to this owner", requestID) - } else if strings.Contains(err.Error(), "not found") { + } else if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Owner not found", requestID) } else { ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to delete owner", requestID) diff --git a/internal/api/handler/profiles.go b/internal/api/handler/profiles.go index 054f9fe..93302eb 100644 --- a/internal/api/handler/profiles.go +++ b/internal/api/handler/profiles.go @@ -1,6 +1,8 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" + "errors" "context" "encoding/json" "net/http" @@ -162,7 +164,7 @@ func (h ProfileHandler) UpdateProfile(w http.ResponseWriter, r *http.Request) { updated, err := h.svc.UpdateProfile(r.Context(), id, profile) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Profile not found", requestID) return } @@ -195,7 +197,7 @@ func (h ProfileHandler) DeleteProfile(w http.ResponseWriter, r *http.Request) { } if err := h.svc.DeleteProfile(r.Context(), id); err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Profile not found", requestID) return } diff --git a/internal/api/handler/renewal_policy.go b/internal/api/handler/renewal_policy.go index 41b2424..d95e606 100644 --- a/internal/api/handler/renewal_policy.go +++ b/internal/api/handler/renewal_policy.go @@ -1,6 +1,7 @@ package handler import ( + "github.com/shankar0123/certctl/internal/repository" "context" "encoding/json" "errors" @@ -26,14 +27,14 @@ type RenewalPolicyService interface { // RenewalPolicyHandler serves /api/v1/renewal-policies CRUD endpoints. // -// G-1 design note: the service-level `ErrRenewalPolicyDuplicateName` / +// G-1 + S-2 design note: the service-level `ErrRenewalPolicyDuplicateName` / // `ErrRenewalPolicyInUse` sentinels alias the repository sentinels (same var -// identity), so `errors.Is` walks transparently across layers. Delete/Update -// not-found detection intentionally uses a `strings.Contains(err.Error(), -// "not found")` substring check — the repo wraps `sql.ErrNoRows` as -// `fmt.Errorf("renewal policy not found: %s", id)` which strips the sentinel, -// and the handler red-tests' `ErrMockNotFound = errors.New("mock not found -// error")` follows the same substring convention. +// identity), so `errors.Is` walks transparently across layers. S-2 closure +// (cat-s6-efc7f6f6bd50) extends the same convention to not-found detection: +// repos now wrap `sql.ErrNoRows` via `fmt.Errorf("X not found: %w", +// repository.ErrNotFound)`, handler dispatch uses +// `errors.Is(err, repository.ErrNotFound)`, and `ErrMockNotFound` in +// test_utils.go wraps the same sentinel so the mocks still resolve to 404. type RenewalPolicyHandler struct { svc RenewalPolicyService } @@ -191,7 +192,7 @@ func (h RenewalPolicyHandler) UpdateRenewalPolicy(w http.ResponseWriter, r *http ErrorWithRequestID(w, http.StatusConflict, "A renewal policy with that name already exists", requestID) return } - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Renewal policy not found", requestID) return } @@ -231,7 +232,7 @@ func (h RenewalPolicyHandler) DeleteRenewalPolicy(w http.ResponseWriter, r *http ErrorWithRequestID(w, http.StatusConflict, "Renewal policy is still referenced by managed certificates", requestID) return } - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, repository.ErrNotFound) { ErrorWithRequestID(w, http.StatusNotFound, "Renewal policy not found", requestID) return } diff --git a/internal/api/handler/test_utils.go b/internal/api/handler/test_utils.go index 6c05232..0964077 100644 --- a/internal/api/handler/test_utils.go +++ b/internal/api/handler/test_utils.go @@ -1,11 +1,22 @@ package handler -import "errors" +import ( + "fmt" -var ( - // Mock errors for testing - ErrMockServiceFailed = errors.New("mock service error") - ErrMockNotFound = errors.New("mock not found error") - ErrMockUnauthorized = errors.New("mock unauthorized error") - ErrMockConflict = errors.New("mock conflict error") + "github.com/shankar0123/certctl/internal/repository" +) + +// Mock errors for testing. +// +// S-2 closure (cat-s6-efc7f6f6bd50): ErrMockNotFound now wraps +// repository.ErrNotFound via fmt.Errorf("...: %w", ...) so the +// post-S-2 handler dispatch — which uses errors.Is(err, +// repository.ErrNotFound) instead of strings.Contains — still +// resolves the mock to a 404. The error message text is preserved +// for log inspection; only the wrapping changes. +var ( + ErrMockServiceFailed = fmt.Errorf("mock service error") + ErrMockNotFound = fmt.Errorf("mock not found error: %w", repository.ErrNotFound) + ErrMockUnauthorized = fmt.Errorf("mock unauthorized error") + ErrMockConflict = fmt.Errorf("mock conflict error") ) diff --git a/internal/domain/errors.go b/internal/domain/errors.go new file mode 100644 index 0000000..21b80a7 --- /dev/null +++ b/internal/domain/errors.go @@ -0,0 +1,19 @@ +// Package domain — error sentinels. +// +// S-2 closure (cat-s6-efc7f6f6bd50): pre-S-2 every handler-side +// validation-failure dispatch was a `strings.Contains(err.Error(), +// "invalid")` or `"required"` site, brittle to any domain-layer +// message change. Post-S-2 domain validators that surface a +// 400 Bad Request wrap their per-field errors via fmt.Errorf("...: %w", +// domain.ErrValidation) so handlers can dispatch via errors.Is. +package domain + +import "errors" + +// ErrValidation is the canonical sentinel for input-validation +// failures surfaced by domain-layer Validate() methods. Handlers that +// surface a 400 Bad Request should `errors.Is(err, domain.ErrValidation)`. +// Per-field error messages are still preserved via fmt.Errorf wrapping +// so the response body retains the actionable detail; the sentinel +// only drives the HTTP status code dispatch. +var ErrValidation = errors.New("domain: validation failed") diff --git a/internal/integration/lifecycle_test.go b/internal/integration/lifecycle_test.go index d0b22a7..22dc2fb 100644 --- a/internal/integration/lifecycle_test.go +++ b/internal/integration/lifecycle_test.go @@ -626,7 +626,10 @@ func (m *mockJobRepository) List(ctx context.Context) ([]*domain.Job, error) { func (m *mockJobRepository) Get(ctx context.Context, id string) (*domain.Job, error) { job, ok := m.jobs[id] if !ok { - return nil, fmt.Errorf("job not found") + // S-2 closure: wrap repository.ErrNotFound so the handler's + // errors.Is dispatch resolves to 404 (matches the Postgres + // repo's post-S-2 wrapping). + return nil, fmt.Errorf("job not found: %w", repository.ErrNotFound) } return job, nil } diff --git a/internal/repository/errors.go b/internal/repository/errors.go new file mode 100644 index 0000000..e2b7469 --- /dev/null +++ b/internal/repository/errors.go @@ -0,0 +1,62 @@ +// Package repository defines the repository-layer error sentinels that +// handlers map to HTTP status codes via errors.Is. +// +// S-2 closure (cat-s6-efc7f6f6bd50): pre-S-2 every handler-side +// not-found dispatch was a `strings.Contains(err.Error(), "not found")` +// site (30+ across internal/api/handler/*.go), brittle to any +// repository-layer message change and untyped against the actual +// failure mode. Post-S-2 the dispatch is type-checked: repositories +// wrap sql.ErrNoRows via fmt.Errorf("...: %w", repository.ErrNotFound) +// and FK constraint violations via repository.ErrForeignKeyConstraint; +// handlers consume via errors.Is. The substring matching is preserved +// at the lib/pq boundary inside `errors.go::isFKError` because the +// PostgreSQL driver returns un-typed *pq.Error values whose codes are +// the canonical signal — but it's confined to one helper rather than +// scattered across every handler file. See unified-audit.md +// cat-s6-efc7f6f6bd50 for the closure rationale. +package repository + +import ( + "errors" + "strings" +) + +// ErrNotFound is the canonical sentinel for repository methods that +// return after sql.ErrNoRows (or its wrapped form). Handlers that +// surface a 404 should `errors.Is(err, repository.ErrNotFound)` +// rather than substring-match. +var ErrNotFound = errors.New("repository: row not found") + +// ErrForeignKeyConstraint is the canonical sentinel for PostgreSQL +// FK / RESTRICT violations bubbling up from a DELETE or UPDATE. +// Handlers that surface a 409 Conflict should +// `errors.Is(err, repository.ErrForeignKeyConstraint)`. +// +// The B-1 closure introduced ErrRenewalPolicyInUse as the per-entity +// FK sentinel for renewal_policies; future per-entity FK sentinels +// (ErrIssuerInUse, ErrTeamInUse, ErrOwnerInUse) can wrap this generic +// one via fmt.Errorf("...: %w", ErrForeignKeyConstraint) so handlers +// can choose between generic-409 and entity-specific 409 dispatch. +var ErrForeignKeyConstraint = errors.New("repository: foreign key constraint violation") + +// IsForeignKeyError detects PostgreSQL FK violation errors from the +// lib/pq driver via the canonical error-text patterns it emits. The +// substring matching is intentionally confined to this helper — +// callers should use this once at the repo layer to wrap into the +// typed ErrForeignKeyConstraint sentinel, then handlers consume via +// errors.Is. +// +// Patterns recognised: +// - "violates foreign key constraint" (the standard PG message) +// - "violates restrict" / "RESTRICT" (DELETE blocked by ON DELETE RESTRICT) +// +// Returns false for nil err so callers can defensively chain it. +func IsForeignKeyError(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "violates foreign key") || + strings.Contains(msg, "RESTRICT") || + strings.Contains(msg, "violates restrict") +} diff --git a/internal/repository/postgres/agent.go b/internal/repository/postgres/agent.go index 75231b7..6a08d2a 100644 --- a/internal/repository/postgres/agent.go +++ b/internal/repository/postgres/agent.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -73,7 +74,7 @@ func (r *AgentRepository) Get(ctx context.Context, id string) (*domain.Agent, er agent, err := scanAgent(row) if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("agent not found") + return nil, fmt.Errorf("agent not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query agent: %w", err) } @@ -170,7 +171,7 @@ func (r *AgentRepository) Update(ctx context.Context, agent *domain.Agent) error } if rows == 0 { - return fmt.Errorf("agent not found") + return fmt.Errorf("agent not found: %w", repository.ErrNotFound) } return nil @@ -190,7 +191,7 @@ func (r *AgentRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("agent not found") + return fmt.Errorf("agent not found: %w", repository.ErrNotFound) } return nil @@ -237,7 +238,7 @@ func (r *AgentRepository) UpdateHeartbeat(ctx context.Context, id string, metada } if rows == 0 { - return fmt.Errorf("agent not found") + return fmt.Errorf("agent not found: %w", repository.ErrNotFound) } return nil @@ -259,7 +260,7 @@ func (r *AgentRepository) GetByAPIKey(ctx context.Context, keyHash string) (*dom agent, err := scanAgent(row) if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("agent not found") + return nil, fmt.Errorf("agent not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query agent: %w", err) } diff --git a/internal/repository/postgres/agent_group.go b/internal/repository/postgres/agent_group.go index 97decbc..27b046e 100644 --- a/internal/repository/postgres/agent_group.go +++ b/internal/repository/postgres/agent_group.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -50,7 +51,7 @@ func (r *AgentGroupRepository) Get(ctx context.Context, id string) (*domain.Agen err := row.Scan(&g.ID, &g.Name, &g.Description, &g.MatchOS, &g.MatchArchitecture, &g.MatchIPCIDR, &g.MatchVersion, &g.Enabled, &g.CreatedAt, &g.UpdatedAt) if err == sql.ErrNoRows { - return nil, fmt.Errorf("agent group not found: %s", id) + return nil, fmt.Errorf("agent group not found: %w", repository.ErrNotFound) } if err != nil { return nil, fmt.Errorf("failed to get agent group: %w", err) @@ -84,7 +85,7 @@ func (r *AgentGroupRepository) Update(ctx context.Context, group *domain.AgentGr } rows, _ := result.RowsAffected() if rows == 0 { - return fmt.Errorf("agent group not found: %s", group.ID) + return fmt.Errorf("agent group not found: %w", repository.ErrNotFound) } return nil } @@ -97,7 +98,7 @@ func (r *AgentGroupRepository) Delete(ctx context.Context, id string) error { } rows, _ := result.RowsAffected() if rows == 0 { - return fmt.Errorf("agent group not found: %s", id) + return fmt.Errorf("agent group not found: %w", repository.ErrNotFound) } return nil } diff --git a/internal/repository/postgres/certificate.go b/internal/repository/postgres/certificate.go index 370f005..22c4617 100644 --- a/internal/repository/postgres/certificate.go +++ b/internal/repository/postgres/certificate.go @@ -265,7 +265,7 @@ func (r *CertificateRepository) Get(ctx context.Context, id string) (*domain.Man cert, err := r.scanCertificate(ctx, row) if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("certificate not found") + return nil, fmt.Errorf("certificate not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query certificate: %w", err) } @@ -397,7 +397,7 @@ func (r *CertificateRepository) Update(ctx context.Context, cert *domain.Managed } if rows == 0 { - return fmt.Errorf("certificate not found") + return fmt.Errorf("certificate not found: %w", repository.ErrNotFound) } return nil @@ -419,7 +419,7 @@ func (r *CertificateRepository) Archive(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("certificate not found") + return fmt.Errorf("certificate not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/discovery.go b/internal/repository/postgres/discovery.go index 0f758e9..2addb40 100644 --- a/internal/repository/postgres/discovery.go +++ b/internal/repository/postgres/discovery.go @@ -62,7 +62,7 @@ func (r *DiscoveryRepository) GetScan(ctx context.Context, id string) (*domain.D &scan.ScanDurationMs, &scan.StartedAt, &scan.CompletedAt, ) if err == sql.ErrNoRows { - return nil, fmt.Errorf("discovery scan not found: %s", id) + return nil, fmt.Errorf("discovery scan not found: %w", repository.ErrNotFound) } if err != nil { return nil, fmt.Errorf("failed to get discovery scan: %w", err) @@ -190,7 +190,7 @@ func (r *DiscoveryRepository) GetDiscovered(ctx context.Context, id string) (*do &cert.CreatedAt, &cert.UpdatedAt, ) if err == sql.ErrNoRows { - return nil, fmt.Errorf("discovered certificate not found: %s", id) + return nil, fmt.Errorf("discovered certificate not found: %w", repository.ErrNotFound) } if err != nil { return nil, fmt.Errorf("failed to get discovered certificate: %w", err) @@ -317,7 +317,7 @@ func (r *DiscoveryRepository) UpdateDiscoveredStatus(ctx context.Context, id str } rowsAffected, _ := result.RowsAffected() if rowsAffected == 0 { - return fmt.Errorf("discovered certificate not found: %s", id) + return fmt.Errorf("discovered certificate not found: %w", repository.ErrNotFound) } return nil } diff --git a/internal/repository/postgres/health_check.go b/internal/repository/postgres/health_check.go index 4fe1f7d..a21d0fb 100644 --- a/internal/repository/postgres/health_check.go +++ b/internal/repository/postgres/health_check.go @@ -113,7 +113,7 @@ func (r *HealthCheckRepository) Get(ctx context.Context, id string) (*domain.End &check.CreatedAt, &check.UpdatedAt, ) if err == sql.ErrNoRows { - return nil, fmt.Errorf("health check not found: %s", id) + return nil, fmt.Errorf("health check not found: %w", repository.ErrNotFound) } if err != nil { return nil, fmt.Errorf("get health check: %w", err) @@ -299,7 +299,7 @@ func (r *HealthCheckRepository) GetByEndpoint(ctx context.Context, endpoint stri &check.CreatedAt, &check.UpdatedAt, ) if err == sql.ErrNoRows { - return nil, fmt.Errorf("health check not found for endpoint: %s", endpoint) + return nil, fmt.Errorf("health check not found for endpoint: %w", repository.ErrNotFound) } if err != nil { return nil, fmt.Errorf("get health check by endpoint: %w", err) diff --git a/internal/repository/postgres/issuer.go b/internal/repository/postgres/issuer.go index 5063133..2fd530f 100644 --- a/internal/repository/postgres/issuer.go +++ b/internal/repository/postgres/issuer.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -69,7 +70,7 @@ func (r *IssuerRepository) Get(ctx context.Context, id string) (*domain.Issuer, if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("issuer not found") + return nil, fmt.Errorf("issuer not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query issuer: %w", err) } @@ -169,7 +170,7 @@ func (r *IssuerRepository) Update(ctx context.Context, issuer *domain.Issuer) er } if rows == 0 { - return fmt.Errorf("issuer not found") + return fmt.Errorf("issuer not found: %w", repository.ErrNotFound) } return nil @@ -189,7 +190,7 @@ func (r *IssuerRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("issuer not found") + return fmt.Errorf("issuer not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/job.go b/internal/repository/postgres/job.go index d535713..a34a532 100644 --- a/internal/repository/postgres/job.go +++ b/internal/repository/postgres/job.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -62,7 +63,7 @@ func (r *JobRepository) Get(ctx context.Context, id string) (*domain.Job, error) job, err := scanJob(row) if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("job not found") + return nil, fmt.Errorf("job not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query job: %w", err) } @@ -123,7 +124,7 @@ func (r *JobRepository) Update(ctx context.Context, job *domain.Job) error { } if rows == 0 { - return fmt.Errorf("job not found") + return fmt.Errorf("job not found: %w", repository.ErrNotFound) } return nil @@ -143,7 +144,7 @@ func (r *JobRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("job not found") + return fmt.Errorf("job not found: %w", repository.ErrNotFound) } return nil @@ -232,7 +233,7 @@ func (r *JobRepository) UpdateStatus(ctx context.Context, id string, status doma } if rows == 0 { - return fmt.Errorf("job not found") + return fmt.Errorf("job not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/network_scan.go b/internal/repository/postgres/network_scan.go index 93781c1..aacf3da 100644 --- a/internal/repository/postgres/network_scan.go +++ b/internal/repository/postgres/network_scan.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -68,7 +69,7 @@ func (r *NetworkScanRepository) Get(ctx context.Context, id string) (*domain.Net &target.CreatedAt, &target.UpdatedAt, ) if err == sql.ErrNoRows { - return nil, fmt.Errorf("network scan target not found: %s", id) + return nil, fmt.Errorf("network scan target not found: %w", repository.ErrNotFound) } if err != nil { return nil, fmt.Errorf("get network scan target: %w", err) @@ -117,7 +118,7 @@ func (r *NetworkScanRepository) Update(ctx context.Context, target *domain.Netwo } rows, _ := result.RowsAffected() if rows == 0 { - return fmt.Errorf("network scan target not found: %s", target.ID) + return fmt.Errorf("network scan target not found: %w", repository.ErrNotFound) } return nil } @@ -130,7 +131,7 @@ func (r *NetworkScanRepository) Delete(ctx context.Context, id string) error { } rows, _ := result.RowsAffected() if rows == 0 { - return fmt.Errorf("network scan target not found: %s", id) + return fmt.Errorf("network scan target not found: %w", repository.ErrNotFound) } return nil } diff --git a/internal/repository/postgres/notification.go b/internal/repository/postgres/notification.go index dae2f7a..862a8ba 100644 --- a/internal/repository/postgres/notification.go +++ b/internal/repository/postgres/notification.go @@ -174,7 +174,7 @@ func (r *NotificationRepository) UpdateStatus(ctx context.Context, id string, st } if rows == 0 { - return fmt.Errorf("notification not found") + return fmt.Errorf("notification not found: %w", repository.ErrNotFound) } return nil @@ -336,7 +336,7 @@ func (r *NotificationRepository) RecordFailedAttempt(ctx context.Context, id str // Same "not found" error shape as UpdateStatus above. The scheduler // logs-and-continues on this so a concurrently-deleted row doesn't // break the sweep. - return fmt.Errorf("notification not found") + return fmt.Errorf("notification not found: %w", repository.ErrNotFound) } return nil } @@ -368,7 +368,7 @@ func (r *NotificationRepository) MarkAsDead(ctx context.Context, id string, last return fmt.Errorf("failed to get rows affected: %w", err) } if rows == 0 { - return fmt.Errorf("notification not found") + return fmt.Errorf("notification not found: %w", repository.ErrNotFound) } return nil } @@ -405,7 +405,7 @@ func (r *NotificationRepository) Requeue(ctx context.Context, id string) error { return fmt.Errorf("failed to get rows affected: %w", err) } if rows == 0 { - return fmt.Errorf("notification not found") + return fmt.Errorf("notification not found: %w", repository.ErrNotFound) } return nil } diff --git a/internal/repository/postgres/owner.go b/internal/repository/postgres/owner.go index 57cec59..6d6d3a1 100644 --- a/internal/repository/postgres/owner.go +++ b/internal/repository/postgres/owner.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -61,7 +62,7 @@ func (r *OwnerRepository) Get(ctx context.Context, id string) (*domain.Owner, er if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("owner not found") + return nil, fmt.Errorf("owner not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query owner: %w", err) } @@ -110,7 +111,7 @@ func (r *OwnerRepository) Update(ctx context.Context, owner *domain.Owner) error } if rows == 0 { - return fmt.Errorf("owner not found") + return fmt.Errorf("owner not found: %w", repository.ErrNotFound) } return nil @@ -130,7 +131,7 @@ func (r *OwnerRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("owner not found") + return fmt.Errorf("owner not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/policy.go b/internal/repository/postgres/policy.go index 1d828bf..9deae64 100644 --- a/internal/repository/postgres/policy.go +++ b/internal/repository/postgres/policy.go @@ -63,7 +63,7 @@ func (r *PolicyRepository) GetRule(ctx context.Context, id string) (*domain.Poli if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("policy rule not found") + return nil, fmt.Errorf("policy rule not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query policy rule: %w", err) } @@ -114,7 +114,7 @@ func (r *PolicyRepository) UpdateRule(ctx context.Context, rule *domain.PolicyRu } if rows == 0 { - return fmt.Errorf("policy rule not found") + return fmt.Errorf("policy rule not found: %w", repository.ErrNotFound) } return nil @@ -134,7 +134,7 @@ func (r *PolicyRepository) DeleteRule(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("policy rule not found") + return fmt.Errorf("policy rule not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/profile.go b/internal/repository/postgres/profile.go index 2544262..aa929c8 100644 --- a/internal/repository/postgres/profile.go +++ b/internal/repository/postgres/profile.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "encoding/json" @@ -64,7 +65,7 @@ func (r *ProfileRepository) Get(ctx context.Context, id string) (*domain.Certifi p, err := scanProfile(row) if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("profile not found") + return nil, fmt.Errorf("profile not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query profile: %w", err) } @@ -159,7 +160,7 @@ func (r *ProfileRepository) Update(ctx context.Context, profile *domain.Certific } if rows == 0 { - return fmt.Errorf("profile not found") + return fmt.Errorf("profile not found: %w", repository.ErrNotFound) } return nil @@ -178,7 +179,7 @@ func (r *ProfileRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("profile not found") + return fmt.Errorf("profile not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/renewal_policy.go b/internal/repository/postgres/renewal_policy.go index 547e57a..e96bfe3 100644 --- a/internal/repository/postgres/renewal_policy.go +++ b/internal/repository/postgres/renewal_policy.go @@ -72,7 +72,7 @@ func (r *RenewalPolicyRepository) Get(ctx context.Context, id string) (*domain.R policy, err := scanRenewalPolicy(row) if err != nil { if errors.Is(err, sql.ErrNoRows) { - return nil, fmt.Errorf("renewal policy not found: %s", id) + return nil, fmt.Errorf("renewal policy not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query renewal policy: %w", err) } @@ -252,7 +252,7 @@ func (r *RenewalPolicyRepository) Update(ctx context.Context, id string, policy updated, err := scanRenewalPolicy(row) if err != nil { if errors.Is(err, sql.ErrNoRows) { - return fmt.Errorf("renewal policy not found: %s", id) + return fmt.Errorf("renewal policy not found: %w", repository.ErrNotFound) } if isUniqueViolation(err) { return repository.ErrRenewalPolicyDuplicateName @@ -283,7 +283,7 @@ func (r *RenewalPolicyRepository) Delete(ctx context.Context, id string) error { return fmt.Errorf("failed to read RowsAffected for delete: %w", err) } if rows == 0 { - return fmt.Errorf("renewal policy not found: %s", id) + return fmt.Errorf("renewal policy not found: %w", repository.ErrNotFound) } return nil } diff --git a/internal/repository/postgres/revocation.go b/internal/repository/postgres/revocation.go index c8c5326..b9ae58c 100644 --- a/internal/repository/postgres/revocation.go +++ b/internal/repository/postgres/revocation.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -136,7 +137,7 @@ func (r *RevocationRepository) MarkIssuerNotified(ctx context.Context, id string } if rows == 0 { - return fmt.Errorf("revocation not found") + return fmt.Errorf("revocation not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/target.go b/internal/repository/postgres/target.go index 6c74a6f..2ba4c79 100644 --- a/internal/repository/postgres/target.go +++ b/internal/repository/postgres/target.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -89,7 +90,7 @@ func (r *TargetRepository) Get(ctx context.Context, id string) (*domain.Deployme if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("target not found") + return nil, fmt.Errorf("target not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query target: %w", err) } @@ -174,7 +175,7 @@ func (r *TargetRepository) Update(ctx context.Context, target *domain.Deployment } if rows == 0 { - return fmt.Errorf("target not found") + return fmt.Errorf("target not found: %w", repository.ErrNotFound) } return nil @@ -194,7 +195,7 @@ func (r *TargetRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("target not found") + return fmt.Errorf("target not found: %w", repository.ErrNotFound) } return nil diff --git a/internal/repository/postgres/team.go b/internal/repository/postgres/team.go index cae2d3d..d7ff679 100644 --- a/internal/repository/postgres/team.go +++ b/internal/repository/postgres/team.go @@ -1,6 +1,7 @@ package postgres import ( + "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "fmt" @@ -61,7 +62,7 @@ func (r *TeamRepository) Get(ctx context.Context, id string) (*domain.Team, erro if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("team not found") + return nil, fmt.Errorf("team not found: %w", repository.ErrNotFound) } return nil, fmt.Errorf("failed to query team: %w", err) } @@ -108,7 +109,7 @@ func (r *TeamRepository) Update(ctx context.Context, team *domain.Team) error { } if rows == 0 { - return fmt.Errorf("team not found") + return fmt.Errorf("team not found: %w", repository.ErrNotFound) } return nil @@ -128,7 +129,7 @@ func (r *TeamRepository) Delete(ctx context.Context, id string) error { } if rows == 0 { - return fmt.Errorf("team not found") + return fmt.Errorf("team not found: %w", repository.ErrNotFound) } return nil