Implements 3 deferred security tickets (TICKET-003, TICKET-007, TICKET-010) and performs comprehensive documentation audit to eliminate drift between code and docs. Code changes: - TICKET-003: Repository integration tests with testcontainers-go (50+ subtests) - TICKET-007: CertificateService decomposition into RevocationSvc + CAOperationsSvc - TICKET-010: Request body size limits via http.MaxBytesReader middleware - Fix missing slog import in certificate.go after service decomposition Documentation updates: - README: Fix endpoint count (97→93), expand env var reference (15→39 vars) - CLAUDE.md: Fix OpenAPI operation count (85→93), update file locations - architecture.md: Add body size limits section, middleware chain ordering - CONTRIBUTING.md: New contributor guide with architecture conventions, test patterns, middleware ordering, CI thresholds - SECURITY_REMEDIATION.md: Removed from repo (moved to cowork, gitignored) - Test files: Add doc comments to all new test files Documentation that should exist but doesn't yet: - Architecture diagrams (C4 model or similar) - Threat model document - Testing philosophy guide - Disaster recovery runbook - Upgrade guide (migration between versions) - API versioning strategy document Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7.5 KiB
Contributing to certctl
Architecture Conventions
certctl follows a strict Handler -> Service -> Repository layering.
Handlers define their own service interfaces (dependency inversion). A handler never imports a concrete service type. This means adding a method to a service requires updating the corresponding handler interface and mock.
Services contain business logic. Each service should have at most 5-6 direct dependencies. If a service exceeds ~500 lines or ~6 dependencies, decompose it using the facade/delegation pattern (see CertificateService -> RevocationSvc + CAOperationsSvc for the reference implementation).
Repositories are PostgreSQL implementations behind interfaces defined in internal/repository/interfaces.go. All SQL is hand-written (no ORM). Use IF NOT EXISTS for schema, ON CONFLICT for idempotent upserts.
Connectors implement pluggable interfaces for issuers (issuer.Connector), targets (target.Connector), and notifiers (Notifier). The IssuerConnectorAdapter bridges the connector-layer interface with the service-layer interface to maintain dependency inversion.
When to Split vs. Extend
Split a component when it exceeds ~500 lines, mixes distinct responsibilities (e.g., CRUD + revocation + CRL generation), or has more than 6 dependencies. Use the facade pattern to avoid breaking handler interfaces.
Extend an existing component when the new functionality is tightly coupled to existing state and adding a new file would create unnecessary indirection.
Middleware Stack Ordering
The HTTP middleware chain is order-sensitive. The current ordering in cmd/server/main.go:
RequestID- assigns a unique request IDNewLogging- structured slog middleware with request ID propagationRecovery- panic recovery (must be early to catch panics in later middleware)NewBodyLimit- request body size limits viahttp.MaxBytesReader(before auth to reject oversized payloads early)NewCORS- CORS preflight handling (deny-by-default)NewAuth- API key / JWT authenticationNewAuditLog- records every API call to the audit trail (after auth so actor is available)
When rate limiting is enabled, NewRateLimiter is inserted between NewBodyLimit and NewCORS.
Contributors adding new middleware must respect this ordering. Body-level middleware goes before auth. Auth-dependent middleware goes after auth.
Test Patterns and Conventions
Test File Organization
Every package with production code should have corresponding _test.go files in the same package (not a _test package). Test helpers belong in testutil_test.go within the package.
Mock Naming Convention
Mock types in test files must be unexported (lowercase). The convention:
// Good - unexported, test-only
type mockCertificateService struct { ... }
func newMockCertificateService() *mockCertificateService { ... }
// Bad - exported, leaks into package API
type MockCertificateService struct { ... }
Known exception: Handler test files currently use exported Mock types (e.g., MockCertificateService). This is a known deviation being tracked for cleanup.
Service Layer Tests
Service tests use mock repositories defined in internal/service/testutil_test.go. The pattern:
func TestMyService_Method(t *testing.T) {
repo := newMockCertificateRepository()
auditRepo := newMockAuditRepository()
auditService := NewAuditService(auditRepo)
svc := NewMyService(repo, auditService)
// Set up test data
repo.AddCert(&domain.ManagedCertificate{...})
// Exercise
err := svc.DoSomething(context.Background(), "cert-1")
// Verify
if err != nil {
t.Fatalf("expected no error, got: %v", err)
}
}
Handler Layer Tests
Handler tests use httptest.NewRequest and httptest.NewRecorder. Each handler test file defines its own mock service type implementing the handler's service interface:
type mockFooService struct {
err error
// fields for capturing calls and returning data
}
func TestFooHandler_List(t *testing.T) {
mock := &mockFooService{}
handler := NewFooHandler(mock)
// ...
}
Repository Integration Tests
Repository tests in internal/repository/postgres/ use testcontainers-go to spin up a real PostgreSQL 16 container. Key patterns:
setupTestDB(t)creates a shared container for the test runfreshSchema(t, db)creates an isolated PostgreSQL schema per test (CREATE SCHEMA test_xxx; SET search_path TO test_xxx)- All migrations are run in each schema so tests start with a clean database
- Tests are skipped in CI short mode (
testing.Short()) since they require Docker - Run locally with:
go test ./internal/repository/postgres/... -v
Fuzz Tests
Fuzz tests use Go's native testing/fuzz framework. Located in *_fuzz_test.go files. Seed corpora include known adversarial inputs (SQL injection, shell metacharacters, etc.). Run with: go test -fuzz=FuzzValidateShellCommand ./internal/validation/...
CI Coverage Thresholds
The CI pipeline enforces per-layer coverage floors:
| Layer | Threshold | Package Pattern |
|---|---|---|
| Service | 60% | internal/service |
| Handler | 60% | internal/api/handler |
| Domain | 40% | internal/domain |
| Middleware | 50% | internal/api/middleware |
Adding a new package with tests? Ensure it's included in the go test command in .github/workflows/ci.yml.
Race Detection
All tests run with -race in CI. Never use shared mutable state without synchronization. The scheduler uses sync/atomic.Bool guards; follow the same pattern for any concurrent code.
Adding New Features
- Domain model in
internal/domain/- types, constants, validation helpers - Migration in
migrations/-000N_feature.up.sqland.down.sql, idempotent - Repository interface in
internal/repository/interfaces.go, implementation ininternal/repository/postgres/ - Service in
internal/service/with tests - Handler in
internal/api/handler/defining its own service interface, with tests - Route registration via
HandlerRegistrystruct ininternal/api/router/router.go - Wire in
cmd/server/main.go - OpenAPI spec update in
api/openapi.yaml - GUI page in
web/src/pages/with route inweb/src/main.tsx - Seed data in
migrations/seed_demo.sqlfor demo mode
Every backend feature ships with its corresponding GUI surface.
Environment
- Go 1.25+, PostgreSQL 16+, Node.js 22+ (frontend)
- No ORM - raw
database/sql+lib/pq - No web framework -
net/httpstdlib routing - Minimal dependencies: 5 direct Go dependencies (see
go.mod) - Frontend: Vite + React 18 + TypeScript + TanStack Query + Recharts + Tailwind CSS
Documentation That Should Exist But Doesn't Yet
The following are recommended future additions:
- Architecture diagrams (Mermaid in
docs/architecture.mdcovers some, but data flow diagrams for key workflows like renewal and revocation would help) - Threat model (formal STRIDE analysis for the control plane, agent communication, and key management boundaries)
- Testing philosophy guide (rationale for mock-vs-real testing decisions, when to use testcontainers vs mocks)
- Disaster recovery runbook (PostgreSQL backup/restore, agent re-registration, CA key rotation procedures)
- Upgrade guide (migration steps between major versions, breaking change policy)
- API versioning strategy (how breaking changes will be handled when /api/v2 is needed)