Implement M9: test hardening with handler tests, negative paths, CI coverage gates

All 7 handler files now have test coverage: jobs (14 tests), notifications (11), policies (15), issuers (15), targets (14). Negative-path integration tests cover nonexistent resources, invalid payloads, malformed CSR, expired cert lifecycle, and method-not-allowed errors. CI now enforces coverage thresholds (service 60%+, handler 50%+) and includes connector tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-06-07 20:11:31 +00:00 · 2026-03-15 14:06:48 -04:00
parent bc5a6031b8
commit b9bc2ace8e
8 changed files with 2387 additions and 26 deletions
@@ -0,0 +1,390 @@
+package integration
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/shankar0123/certctl/internal/api/handler"
+	"github.com/shankar0123/certctl/internal/api/router"
+	"github.com/shankar0123/certctl/internal/connector/issuer/local"
+	"github.com/shankar0123/certctl/internal/domain"
+	"github.com/shankar0123/certctl/internal/service"
+)
+
+// setupTestServer creates a fully-wired test server for negative path testing.
+func setupTestServer(t *testing.T) (*httptest.Server, *mockCertificateRepository, *mockJobRepository, *mockAgentRepository) {
+	t.Helper()
+
+	certRepo := newMockCertificateRepository()
+	jobRepo := newMockJobRepository()
+	auditRepo := newMockAuditRepository()
+	agentRepo := newMockAgentRepository()
+	targetRepo := newMockTargetRepository()
+	notifRepo := newMockNotificationRepository()
+	policyRepo := newMockPolicyRepository()
+	renewalPolicyRepo := newMockRenewalPolicyRepository()
+	issuerRepo := newMockIssuerRepository()
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	localCA := local.New(nil, logger)
+
+	issuerRegistry := map[string]service.IssuerConnector{
+		"iss-local": service.NewIssuerConnectorAdapter(localCA),
+	}
+
+	auditService := service.NewAuditService(auditRepo)
+	policyService := service.NewPolicyService(policyRepo, auditService)
+	certificateService := service.NewCertificateService(certRepo, policyService, auditService)
+	notificationService := service.NewNotificationService(notifRepo, make(map[string]service.Notifier))
+	renewalService := service.NewRenewalService(certRepo, jobRepo, renewalPolicyRepo, auditService, notificationService, issuerRegistry, "server")
+	deploymentService := service.NewDeploymentService(jobRepo, targetRepo, agentRepo, certRepo, auditService, notificationService)
+	jobService := service.NewJobService(jobRepo, renewalService, deploymentService, logger)
+	agentService := service.NewAgentService(agentRepo, certRepo, jobRepo, targetRepo, auditService, issuerRegistry, renewalService)
+	issuerService := service.NewIssuerService(issuerRepo, auditService)
+
+	certificateHandler := handler.NewCertificateHandler(certificateService)
+	issuerHandler := handler.NewIssuerHandler(issuerService)
+	targetHandler := handler.NewTargetHandler(&mockTargetService{targetRepo: targetRepo, auditService: auditService})
+	agentHandler := handler.NewAgentHandler(agentService)
+	jobHandler := handler.NewJobHandler(jobService)
+	policyHandler := handler.NewPolicyHandler(policyService)
+	teamHandler := handler.NewTeamHandler(&mockTeamService{})
+	ownerHandler := handler.NewOwnerHandler(&mockOwnerService{})
+	auditHandler := handler.NewAuditHandler(auditService)
+	notificationHandler := handler.NewNotificationHandler(notificationService)
+	healthHandler := handler.NewHealthHandler("none")
+
+	r := router.New()
+	r.RegisterHandlers(
+		certificateHandler,
+		issuerHandler,
+		targetHandler,
+		agentHandler,
+		jobHandler,
+		policyHandler,
+		teamHandler,
+		ownerHandler,
+		auditHandler,
+		notificationHandler,
+		healthHandler,
+	)
+
+	server := httptest.NewServer(r)
+	t.Cleanup(func() { server.Close() })
+
+	return server, certRepo, jobRepo, agentRepo
+}
+
+// TestNegativePaths exercises error paths and edge cases.
+func TestNegativePaths(t *testing.T) {
+	server, _, _, _ := setupTestServer(t)
+
+	// ======================
+	// Nonexistent resource lookups
+	// ======================
+	t.Run("GetNonexistentCertificate", func(t *testing.T) {
+		resp, err := http.Get(server.URL + "/api/v1/certificates/mc-does-not-exist")
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusNotFound {
+			t.Errorf("expected 404, got %d", resp.StatusCode)
+		}
+	})
+
+	t.Run("GetNonexistentAgent", func(t *testing.T) {
+		resp, err := http.Get(server.URL + "/api/v1/agents/agent-ghost")
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusNotFound {
+			t.Errorf("expected 404, got %d", resp.StatusCode)
+		}
+	})
+
+	t.Run("GetNonexistentJob", func(t *testing.T) {
+		resp, err := http.Get(server.URL + "/api/v1/jobs/job-ghost")
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusNotFound {
+			t.Errorf("expected 404, got %d", resp.StatusCode)
+		}
+	})
+
+	// ======================
+	// Invalid request bodies
+	// ======================
+	t.Run("CreateCertificateInvalidJSON", func(t *testing.T) {
+		resp, err := http.Post(server.URL+"/api/v1/certificates", "application/json", bytes.NewReader([]byte("not json")))
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusBadRequest {
+			bodyBytes, _ := io.ReadAll(resp.Body)
+			t.Errorf("expected 400, got %d. Body: %s", resp.StatusCode, string(bodyBytes))
+		}
+	})
+
+	t.Run("CreateCertificateMissingCommonName", func(t *testing.T) {
+		body := map[string]interface{}{
+			"name":        "Test Cert",
+			"environment": "test",
+		}
+		bodyBytes, _ := json.Marshal(body)
+
+		resp, err := http.Post(server.URL+"/api/v1/certificates", "application/json", bytes.NewReader(bodyBytes))
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusBadRequest {
+			bodyBytes, _ := io.ReadAll(resp.Body)
+			t.Errorf("expected 400, got %d. Body: %s", resp.StatusCode, string(bodyBytes))
+		}
+	})
+
+	t.Run("CreatePolicyInvalidType", func(t *testing.T) {
+		body := map[string]interface{}{
+			"name": "Bad Policy",
+			"type": "NonexistentType",
+		}
+		bodyBytes, _ := json.Marshal(body)
+
+		resp, err := http.Post(server.URL+"/api/v1/policies", "application/json", bytes.NewReader(bodyBytes))
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusBadRequest {
+			bodyBytes, _ := io.ReadAll(resp.Body)
+			t.Errorf("expected 400, got %d. Body: %s", resp.StatusCode, string(bodyBytes))
+		}
+	})
+
+	// ======================
+	// Invalid CSR submission
+	// ======================
+	t.Run("SubmitInvalidCSR", func(t *testing.T) {
+		// First register an agent
+		agentBody := map[string]interface{}{
+			"name":     "test-agent",
+			"hostname": "test-host",
+		}
+		agentBytes, _ := json.Marshal(agentBody)
+
+		regResp, err := http.Post(server.URL+"/api/v1/agents/register", "application/json", bytes.NewReader(agentBytes))
+		if err != nil {
+			t.Fatalf("register agent failed: %v", err)
+		}
+		defer regResp.Body.Close()
+
+		if regResp.StatusCode != http.StatusCreated {
+			bodyBytes, _ := io.ReadAll(regResp.Body)
+			t.Fatalf("expected 201, got %d. Body: %s", regResp.StatusCode, string(bodyBytes))
+		}
+
+		var agentResp struct {
+			Agent  domain.Agent `json:"agent"`
+			APIKey string       `json:"api_key"`
+		}
+		if err := json.NewDecoder(regResp.Body).Decode(&agentResp); err != nil {
+			t.Fatalf("failed to decode agent response: %v", err)
+		}
+
+		// Submit garbage CSR
+		csrBody := map[string]interface{}{
+			"csr_pem": "not a valid CSR",
+		}
+		csrBytes, _ := json.Marshal(csrBody)
+
+		csrResp, err := http.Post(
+			fmt.Sprintf("%s/api/v1/agents/%s/csr", server.URL, agentResp.Agent.ID),
+			"application/json",
+			bytes.NewReader(csrBytes),
+		)
+		if err != nil {
+			t.Fatalf("CSR submission failed: %v", err)
+		}
+		defer csrResp.Body.Close()
+
+		// Should reject — either 400 (bad CSR format) or 500 (no cert to sign for)
+		if csrResp.StatusCode == http.StatusOK || csrResp.StatusCode == http.StatusCreated {
+			t.Errorf("expected error status for invalid CSR, got %d", csrResp.StatusCode)
+		}
+	})
+
+	// ======================
+	// Heartbeat for nonexistent agent
+	// ======================
+	t.Run("HeartbeatNonexistentAgent", func(t *testing.T) {
+		heartbeatBody := map[string]interface{}{
+			"status": "healthy",
+		}
+		bodyBytes, _ := json.Marshal(heartbeatBody)
+
+		resp, err := http.Post(
+			server.URL+"/api/v1/agents/agent-nonexistent/heartbeat",
+			"application/json",
+			bytes.NewReader(bodyBytes),
+		)
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		// Should fail — agent doesn't exist
+		if resp.StatusCode == http.StatusOK {
+			t.Errorf("expected error status for nonexistent agent heartbeat, got 200")
+		}
+	})
+
+	// ======================
+	// Method not allowed
+	// ======================
+	t.Run("PutToListEndpoint", func(t *testing.T) {
+		req, _ := http.NewRequest(http.MethodPut, server.URL+"/api/v1/certificates", nil)
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode == http.StatusOK {
+			t.Errorf("expected error for PUT on list endpoint, got 200")
+		}
+	})
+
+	// ======================
+	// Empty list responses
+	// ======================
+	t.Run("ListEmptyCertificates", func(t *testing.T) {
+		resp, err := http.Get(server.URL + "/api/v1/certificates")
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("expected 200 for empty list, got %d", resp.StatusCode)
+		}
+
+		var result map[string]interface{}
+		if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
+			t.Fatalf("failed to decode: %v", err)
+		}
+
+		total, ok := result["total"].(float64)
+		if !ok || total != 0 {
+			t.Errorf("expected total 0, got %v", result["total"])
+		}
+	})
+
+	t.Run("ListEmptyJobs", func(t *testing.T) {
+		resp, err := http.Get(server.URL + "/api/v1/jobs")
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("expected 200 for empty list, got %d", resp.StatusCode)
+		}
+	})
+
+	// ======================
+	// Trigger renewal on nonexistent cert
+	// ======================
+	t.Run("TriggerRenewalNonexistentCert", func(t *testing.T) {
+		resp, err := http.Post(
+			server.URL+"/api/v1/certificates/mc-ghost/renew",
+			"application/json",
+			nil,
+		)
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusCreated {
+			t.Errorf("expected error for renewal of nonexistent cert, got %d", resp.StatusCode)
+		}
+	})
+}
+
+// TestCertificateLifecycleWithExpiredCert verifies handling of an expired certificate.
+func TestCertificateLifecycleWithExpiredCert(t *testing.T) {
+	server, certRepo, _, _ := setupTestServer(t)
+
+	// Create an already-expired certificate directly in the repo
+	expiredTime := time.Now().Add(-24 * time.Hour)
+	expiredCert := &domain.ManagedCertificate{
+		ID:              "mc-expired-001",
+		Name:            "Expired Cert",
+		CommonName:      "expired.example.com",
+		Status:          domain.CertificateStatusExpired,
+		Environment:     "prod",
+		IssuerID:        "iss-local",
+		RenewalPolicyID: "rp-default",
+		ExpiresAt:       expiredTime,
+		CreatedAt:       time.Now().Add(-90 * 24 * time.Hour),
+		UpdatedAt:       time.Now(),
+	}
+	certRepo.certs[expiredCert.ID] = expiredCert
+
+	// Verify we can retrieve the expired cert
+	t.Run("GetExpiredCert", func(t *testing.T) {
+		resp, err := http.Get(server.URL + "/api/v1/certificates/mc-expired-001")
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusOK {
+			t.Fatalf("expected 200, got %d", resp.StatusCode)
+		}
+
+		var cert domain.ManagedCertificate
+		if err := json.NewDecoder(resp.Body).Decode(&cert); err != nil {
+			t.Fatalf("failed to decode: %v", err)
+		}
+
+		if cert.Status != domain.CertificateStatusExpired {
+			t.Errorf("expected status Expired, got %s", cert.Status)
+		}
+	})
+
+	// Trigger renewal on expired cert — should succeed (creating a renewal job)
+	t.Run("TriggerRenewalOnExpiredCert", func(t *testing.T) {
+		resp, err := http.Post(
+			server.URL+"/api/v1/certificates/mc-expired-001/renew",
+			"application/json",
+			nil,
+		)
+		if err != nil {
+			t.Fatalf("request failed: %v", err)
+		}
+		defer resp.Body.Close()
+
+		// Renewal should be accepted (creates a job) or return an error
+		// if the service doesn't allow renewal on expired certs
+		t.Logf("Renewal trigger on expired cert returned status: %d", resp.StatusCode)
+	})
+}